Count packets dropped by iptables in IPStats

PiperOrigin-RevId: 332486383

7 files changed, 195 insertions, 89 deletions

diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go
index 59c3101b5..b14b356d6 100644
--- a/pkg/tcpip/network/ipv4/ipv4.go
+++ b/pkg/tcpip/network/ipv4/ipv4.go
@@ -236,6 +236,7 @@ func (e *endpoint) WritePacket(r *stack.Route, gso *stack.GSO, params stack.Netw
 	ipt := e.stack.IPTables()
 	if ok := ipt.Check(stack.Output, pkt, gso, r, "", nicName); !ok {
 		// iptables is telling us to drop the packet.
+		r.Stats().IP.IPTablesOutputDropped.Increment()
 		return nil
 	}
 
@@ -300,6 +301,7 @@ func (e *endpoint) WritePackets(r *stack.Route, gso *stack.GSO, pkts stack.Packe
 		r.Stats().IP.PacketsSent.IncrementBy(uint64(n))
 		return n, err
 	}
+	r.Stats().IP.IPTablesOutputDropped.IncrementBy(uint64(len(dropped)))
 
 	// Slow path as we are dropping some packets in the batch degrade to
 	// emitting one packet at a time.
@@ -321,12 +323,15 @@ func (e *endpoint) WritePackets(r *stack.Route, gso *stack.GSO, pkts stack.Packe
 		}
 		if err := e.linkEP.WritePacket(r, gso, ProtocolNumber, pkt); err != nil {
 			r.Stats().IP.PacketsSent.IncrementBy(uint64(n))
-			return n, err
+			// Dropped packets aren't errors, so include them in
+			// the return value.
+			return n + len(dropped), err
 		}
 		n++
 	}
 	r.Stats().IP.PacketsSent.IncrementBy(uint64(n))
-	return n, nil
+	// Dropped packets aren't errors, so include them in the return value.
+	return n + len(dropped), nil
 }
 
 // WriteHeaderIncludedPacket writes a packet already containing a network
@@ -395,6 +400,7 @@ func (e *endpoint) HandlePacket(r *stack.Route, pkt *stack.PacketBuffer) {
 	ipt := e.stack.IPTables()
 	if ok := ipt.Check(stack.Input, pkt, nil, nil, "", ""); !ok {
 		// iptables is telling us to drop the packet.
+		r.Stats().IP.IPTablesInputDropped.Increment()
 		return
 	}
 
diff --git a/pkg/tcpip/network/ipv4/ipv4_test.go b/pkg/tcpip/network/ipv4/ipv4_test.go
index c1a560914..b14bc98e8 100644
--- a/pkg/tcpip/network/ipv4/ipv4_test.go
+++ b/pkg/tcpip/network/ipv4/ipv4_test.go
@@ -1047,26 +1047,32 @@ func TestReceiveFragments(t *testing.T) {
 	}
 }
 
-func TestWritePacketsStats(t *testing.T) {
+func TestWriteStats(t *testing.T) {
 	const nPackets = 3
 	tests := []struct {
-		name       string
-		setup      func(*testing.T, *stack.Stack)
-		linkEP     stack.LinkEndpoint
-		expectSent int
+		name          string
+		setup         func(*testing.T, *stack.Stack)
+		linkEP        func() stack.LinkEndpoint
+		expectSent    int
+		expectDropped int
+		expectWritten int
 	}{
 		{
 			name: "Accept all",
 			// No setup needed, tables accept everything by default.
-			setup:      func(*testing.T, *stack.Stack) {},
-			linkEP:     &limitedEP{nPackets},
-			expectSent: nPackets,
+			setup:         func(*testing.T, *stack.Stack) {},
+			linkEP:        func() stack.LinkEndpoint { return &limitedEP{nPackets} },
+			expectSent:    nPackets,
+			expectDropped: 0,
+			expectWritten: nPackets,
 		}, {
 			name: "Accept all with error",
 			// No setup needed, tables accept everything by default.
-			setup:      func(*testing.T, *stack.Stack) {},
-			linkEP:     &limitedEP{nPackets - 1},
-			expectSent: nPackets - 1,
+			setup:         func(*testing.T, *stack.Stack) {},
+			linkEP:        func() stack.LinkEndpoint { return &limitedEP{nPackets - 1} },
+			expectSent:    nPackets - 1,
+			expectDropped: 0,
+			expectWritten: nPackets - 1,
 		}, {
 			name: "Drop all",
 			setup: func(t *testing.T, stk *stack.Stack) {
@@ -1083,8 +1089,10 @@ func TestWritePacketsStats(t *testing.T) {
 					t.Fatalf("failed to replace table: %v", err)
 				}
 			},
-			linkEP:     &limitedEP{nPackets},
-			expectSent: 0,
+			linkEP:        func() stack.LinkEndpoint { return &limitedEP{nPackets} },
+			expectSent:    0,
+			expectDropped: nPackets,
+			expectWritten: nPackets,
 		}, {
 			name: "Drop some",
 			setup: func(t *testing.T, stk *stack.Stack) {
@@ -1106,38 +1114,68 @@ func TestWritePacketsStats(t *testing.T) {
 					t.Fatalf("failed to replace table: %v", err)
 				}
 			},
-			linkEP:     &limitedEP{nPackets},
-			expectSent: nPackets - 1,
+			linkEP:        func() stack.LinkEndpoint { return &limitedEP{nPackets} },
+			expectSent:    nPackets - 1,
+			expectDropped: 1,
+			expectWritten: nPackets,
 		},
 	}
 
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			rt := buildRoute(t, nil, test.linkEP)
+	// Parameterize the tests to run with both WritePacket and WritePackets.
+	writers := []struct {
+		name         string
+		writePackets func(*stack.Route, stack.PacketBufferList) (int, *tcpip.Error)
+	}{
+		{
+			name: "WritePacket",
+			writePackets: func(rt *stack.Route, pkts stack.PacketBufferList) (int, *tcpip.Error) {
+				nWritten := 0
+				for pkt := pkts.Front(); pkt != nil; pkt = pkt.Next() {
+					if err := rt.WritePacket(nil, stack.NetworkHeaderParams{}, pkt); err != nil {
+						return nWritten, err
+					}
+					nWritten++
+				}
+				return nWritten, nil
+			},
+		}, {
+			name: "WritePackets",
+			writePackets: func(rt *stack.Route, pkts stack.PacketBufferList) (int, *tcpip.Error) {
+				return rt.WritePackets(nil, pkts, stack.NetworkHeaderParams{})
+			},
+		},
+	}
 
-			var pbl stack.PacketBufferList
-			for i := 0; i < nPackets; i++ {
-				pkt := stack.NewPacketBuffer(stack.PacketBufferOptions{
-					ReserveHeaderBytes: header.UDPMinimumSize + int(rt.MaxHeaderLength()),
-					Data:               buffer.NewView(1).ToVectorisedView(),
+	for _, writer := range writers {
+		t.Run(writer.name, func(t *testing.T) {
+			for _, test := range tests {
+				t.Run(test.name, func(t *testing.T) {
+					rt := buildRoute(t, nil, test.linkEP())
+
+					var pkts stack.PacketBufferList
+					for i := 0; i < nPackets; i++ {
+						pkt := stack.NewPacketBuffer(stack.PacketBufferOptions{
+							ReserveHeaderBytes: header.UDPMinimumSize + int(rt.MaxHeaderLength()),
+							Data:               buffer.NewView(0).ToVectorisedView(),
+						})
+						pkt.TransportHeader().Push(header.UDPMinimumSize)
+						pkts.PushBack(pkt)
+					}
+
+					test.setup(t, rt.Stack())
+
+					nWritten, _ := writer.writePackets(&rt, pkts)
+
+					if got := int(rt.Stats().IP.PacketsSent.Value()); got != test.expectSent {
+						t.Errorf("sent %d packets, but expected to send %d", got, test.expectSent)
+					}
+					if got := int(rt.Stats().IP.IPTablesOutputDropped.Value()); got != test.expectDropped {
+						t.Errorf("dropped %d packets, but expected to drop %d", got, test.expectDropped)
+					}
+					if nWritten != test.expectWritten {
+						t.Errorf("wrote %d packets, but expected WritePackets to return %d", nWritten, test.expectWritten)
+					}
 				})
-				pkt.TransportHeader().Push(header.UDPMinimumSize)
-				pbl.PushBack(pkt)
-			}
-
-			test.setup(t, rt.Stack())
-
-			nWritten, err := rt.WritePackets(nil, pbl, stack.NetworkHeaderParams{})
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			got := int(rt.Stats().IP.PacketsSent.Value())
-			if got != test.expectSent {
-				t.Errorf("sent %d packets, but expected to send %d", got, test.expectSent)
-			}
-			if got != nWritten {
-				t.Errorf("sent %d packets, WritePackets returned %d", got, nWritten)
 			}
 		})
 	}
@@ -1177,7 +1215,10 @@ type limitedEP struct {
 }
 
 // MTU implements LinkEndpoint.MTU.
-func (*limitedEP) MTU() uint32 { return 0 }
+func (*limitedEP) MTU() uint32 {
+	// Give an MTU that won't cause fragmentation for IPv4+UDP.
+	return header.IPv4MinimumSize + header.UDPMinimumSize
+}
 
 // Capabilities implements LinkEndpoint.Capabilities.
 func (*limitedEP) Capabilities() stack.LinkEndpointCapabilities { return 0 }
diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go
index a4a4d6a21..ee64d92d8 100644
--- a/pkg/tcpip/network/ipv6/ipv6.go
+++ b/pkg/tcpip/network/ipv6/ipv6.go
@@ -114,6 +114,7 @@ func (e *endpoint) WritePacket(r *stack.Route, gso *stack.GSO, params stack.Netw
 	ipt := e.stack.IPTables()
 	if ok := ipt.Check(stack.Output, pkt, gso, r, "", nicName); !ok {
 		// iptables is telling us to drop the packet.
+		r.Stats().IP.IPTablesOutputDropped.Increment()
 		return nil
 	}
 
@@ -147,8 +148,11 @@ func (e *endpoint) WritePacket(r *stack.Route, gso *stack.GSO, params stack.Netw
 		return nil
 	}
 
+	if err := e.linkEP.WritePacket(r, gso, ProtocolNumber, pkt); err != nil {
+		return err
+	}
 	r.Stats().IP.PacketsSent.Increment()
-	return e.linkEP.WritePacket(r, gso, ProtocolNumber, pkt)
+	return nil
 }
 
 // WritePackets implements stack.LinkEndpoint.WritePackets.
@@ -176,6 +180,7 @@ func (e *endpoint) WritePackets(r *stack.Route, gso *stack.GSO, pkts stack.Packe
 		r.Stats().IP.PacketsSent.IncrementBy(uint64(n))
 		return n, err
 	}
+	r.Stats().IP.IPTablesOutputDropped.IncrementBy(uint64(len(dropped)))
 
 	// Slow path as we are dropping some packets in the batch degrade to
 	// emitting one packet at a time.
@@ -197,13 +202,16 @@ func (e *endpoint) WritePackets(r *stack.Route, gso *stack.GSO, pkts stack.Packe
 		}
 		if err := e.linkEP.WritePacket(r, gso, ProtocolNumber, pkt); err != nil {
 			r.Stats().IP.PacketsSent.IncrementBy(uint64(n))
-			return n, err
+			// Dropped packets aren't errors, so include them in
+			// the return value.
+			return n + len(dropped), err
 		}
 		n++
 	}
 
 	r.Stats().IP.PacketsSent.IncrementBy(uint64(n))
-	return n, nil
+	// Dropped packets aren't errors, so include them in the return value.
+	return n + len(dropped), nil
 }
 
 // WriteHeaderIncludedPacker implements stack.NetworkEndpoint. It is not yet
@@ -237,6 +245,7 @@ func (e *endpoint) HandlePacket(r *stack.Route, pkt *stack.PacketBuffer) {
 	ipt := e.stack.IPTables()
 	if ok := ipt.Check(stack.Input, pkt, nil, nil, "", ""); !ok {
 		// iptables is telling us to drop the packet.
+		r.Stats().IP.IPTablesInputDropped.Increment()
 		return
 	}
 
diff --git a/pkg/tcpip/network/ipv6/ipv6_test.go b/pkg/tcpip/network/ipv6/ipv6_test.go
index 354d3b60d..9eea1de8d 100644
--- a/pkg/tcpip/network/ipv6/ipv6_test.go
+++ b/pkg/tcpip/network/ipv6/ipv6_test.go
@@ -1710,26 +1710,32 @@ func TestInvalidIPv6Fragments(t *testing.T) {
 	}
 }
 
-func TestWritePacketsStats(t *testing.T) {
+func TestWriteStats(t *testing.T) {
 	const nPackets = 3
 	tests := []struct {
-		name       string
-		setup      func(*testing.T, *stack.Stack)
-		linkEP     stack.LinkEndpoint
-		expectSent int
+		name          string
+		setup         func(*testing.T, *stack.Stack)
+		linkEP        func() stack.LinkEndpoint
+		expectSent    int
+		expectDropped int
+		expectWritten int
 	}{
 		{
 			name: "Accept all",
 			// No setup needed, tables accept everything by default.
-			setup:      func(*testing.T, *stack.Stack) {},
-			linkEP:     &limitedEP{nPackets},
-			expectSent: nPackets,
+			setup:         func(*testing.T, *stack.Stack) {},
+			linkEP:        func() stack.LinkEndpoint { return &limitedEP{nPackets} },
+			expectSent:    nPackets,
+			expectDropped: 0,
+			expectWritten: nPackets,
 		}, {
 			name: "Accept all with error",
 			// No setup needed, tables accept everything by default.
-			setup:      func(*testing.T, *stack.Stack) {},
-			linkEP:     &limitedEP{nPackets - 1},
-			expectSent: nPackets - 1,
+			setup:         func(*testing.T, *stack.Stack) {},
+			linkEP:        func() stack.LinkEndpoint { return &limitedEP{nPackets - 1} },
+			expectSent:    nPackets - 1,
+			expectDropped: 0,
+			expectWritten: nPackets - 1,
 		}, {
 			name: "Drop all",
 			setup: func(t *testing.T, stk *stack.Stack) {
@@ -1746,8 +1752,10 @@ func TestWritePacketsStats(t *testing.T) {
 					t.Fatalf("failed to replace table: %v", err)
 				}
 			},
-			linkEP:     &limitedEP{nPackets},
-			expectSent: 0,
+			linkEP:        func() stack.LinkEndpoint { return &limitedEP{nPackets} },
+			expectSent:    0,
+			expectDropped: nPackets,
+			expectWritten: nPackets,
 		}, {
 			name: "Drop some",
 			setup: func(t *testing.T, stk *stack.Stack) {
@@ -1769,38 +1777,67 @@ func TestWritePacketsStats(t *testing.T) {
 					t.Fatalf("failed to replace table: %v", err)
 				}
 			},
-			linkEP:     &limitedEP{nPackets},
-			expectSent: nPackets - 1,
+			linkEP:        func() stack.LinkEndpoint { return &limitedEP{nPackets} },
+			expectSent:    nPackets - 1,
+			expectDropped: 1,
+			expectWritten: nPackets,
 		},
 	}
 
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			rt := buildRoute(t, nil, test.linkEP)
+	writers := []struct {
+		name         string
+		writePackets func(*stack.Route, stack.PacketBufferList) (int, *tcpip.Error)
+	}{
+		{
+			name: "WritePacket",
+			writePackets: func(rt *stack.Route, pkts stack.PacketBufferList) (int, *tcpip.Error) {
+				nWritten := 0
+				for pkt := pkts.Front(); pkt != nil; pkt = pkt.Next() {
+					if err := rt.WritePacket(nil, stack.NetworkHeaderParams{}, pkt); err != nil {
+						return nWritten, err
+					}
+					nWritten++
+				}
+				return nWritten, nil
+			},
+		}, {
+			name: "WritePackets",
+			writePackets: func(rt *stack.Route, pkts stack.PacketBufferList) (int, *tcpip.Error) {
+				return rt.WritePackets(nil, pkts, stack.NetworkHeaderParams{})
+			},
+		},
+	}
 
-			var pbl stack.PacketBufferList
-			for i := 0; i < nPackets; i++ {
-				pkt := stack.NewPacketBuffer(stack.PacketBufferOptions{
-					ReserveHeaderBytes: header.UDPMinimumSize + int(rt.MaxHeaderLength()),
-					Data:               buffer.NewView(1).ToVectorisedView(),
+	for _, writer := range writers {
+		t.Run(writer.name, func(t *testing.T) {
+			for _, test := range tests {
+				t.Run(test.name, func(t *testing.T) {
+					rt := buildRoute(t, nil, test.linkEP())
+
+					var pkts stack.PacketBufferList
+					for i := 0; i < nPackets; i++ {
+						pkt := stack.NewPacketBuffer(stack.PacketBufferOptions{
+							ReserveHeaderBytes: header.UDPMinimumSize + int(rt.MaxHeaderLength()),
+							Data:               buffer.NewView(0).ToVectorisedView(),
+						})
+						pkt.TransportHeader().Push(header.UDPMinimumSize)
+						pkts.PushBack(pkt)
+					}
+
+					test.setup(t, rt.Stack())
+
+					nWritten, _ := writer.writePackets(&rt, pkts)
+
+					if got := int(rt.Stats().IP.PacketsSent.Value()); got != test.expectSent {
+						t.Errorf("sent %d packets, but expected to send %d", got, test.expectSent)
+					}
+					if got := int(rt.Stats().IP.IPTablesOutputDropped.Value()); got != test.expectDropped {
+						t.Errorf("dropped %d packets, but expected to drop %d", got, test.expectDropped)
+					}
+					if nWritten != test.expectWritten {
+						t.Errorf("wrote %d packets, but expected WritePackets to return %d", nWritten, test.expectWritten)
+					}
 				})
-				pkt.TransportHeader().Push(header.UDPMinimumSize)
-				pbl.PushBack(pkt)
-			}
-
-			test.setup(t, rt.Stack())
-
-			nWritten, err := rt.WritePackets(nil, pbl, stack.NetworkHeaderParams{})
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			got := int(rt.Stats().IP.PacketsSent.Value())
-			if got != test.expectSent {
-				t.Errorf("sent %d packets, but expected to send %d", got, test.expectSent)
-			}
-			if got != nWritten {
-				t.Errorf("sent %d packets, WritePackets returned %d", got, nWritten)
 			}
 		})
 	}
@@ -1840,7 +1877,9 @@ type limitedEP struct {
 }
 
 // MTU implements LinkEndpoint.MTU.
-func (*limitedEP) MTU() uint32 { return 0 }
+func (*limitedEP) MTU() uint32 {
+	return header.IPv6MinimumMTU
+}
 
 // Capabilities implements LinkEndpoint.Capabilities.
 func (*limitedEP) Capabilities() stack.LinkEndpointCapabilities { return 0 }
diff --git a/pkg/tcpip/stack/iptables.go b/pkg/tcpip/stack/iptables.go
index b6ef04d32..4a521eca9 100644
--- a/pkg/tcpip/stack/iptables.go
+++ b/pkg/tcpip/stack/iptables.go
@@ -289,8 +289,6 @@ const (
 // which address and nicName can be gathered. Currently, address is only
 // needed for prerouting and nicName is only needed for output.
 //
-// TODO(gvisor.dev/issue/170): Dropped packets should be counted.
-//
 // Precondition: pkt.NetworkHeader is set.
 func (it *IPTables) Check(hook Hook, pkt *PacketBuffer, gso *GSO, r *Route, preroutingAddr tcpip.Address, nicName string) bool {
 	if pkt.NetworkProtocolNumber != header.IPv4ProtocolNumber && pkt.NetworkProtocolNumber != header.IPv6ProtocolNumber {
diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go
index 821d3feb9..204bfc433 100644
--- a/pkg/tcpip/stack/nic.go
+++ b/pkg/tcpip/stack/nic.go
@@ -1289,6 +1289,7 @@ func (n *NIC) DeliverNetworkPacket(remote, local tcpip.LinkAddress, protocol tcp
 		address := n.primaryAddress(protocol)
 		if ok := ipt.Check(Prerouting, pkt, nil, nil, address.Address, ""); !ok {
 			// iptables is telling us to drop the packet.
+			n.stack.stats.IP.IPTablesPreroutingDropped.Increment()
 			return
 		}
 	}
diff --git a/pkg/tcpip/tcpip.go b/pkg/tcpip/tcpip.go
index b2ddb24ec..464608dee 100644
--- a/pkg/tcpip/tcpip.go
+++ b/pkg/tcpip/tcpip.go
@@ -1474,6 +1474,18 @@ type IPStats struct {
 	// MalformedFragmentsReceived is the total number of IP Fragments that were
 	// dropped due to the fragment failing validation checks.
 	MalformedFragmentsReceived *StatCounter
+
+	// IPTablesPreroutingDropped is the total number of IP packets dropped
+	// in the Prerouting chain.
+	IPTablesPreroutingDropped *StatCounter
+
+	// IPTablesInputDropped is the total number of IP packets dropped in
+	// the Input chain.
+	IPTablesInputDropped *StatCounter
+
+	// IPTablesOutputDropped is the total number of IP packets dropped in
+	// the Output chain.
+	IPTablesOutputDropped *StatCounter
 }
 
 // TCPStats collects TCP-specific stats.