From 7f9e13053e84b82c67c12a4964fa4703ebaa571f Mon Sep 17 00:00:00 2001 From: Arthur Sfez Date: Wed, 30 Sep 2020 15:08:00 -0700 Subject: Count IP OutgoingPacketErrors in the NetworkEndpoint methods Before this change, OutgoingPacketErrors was incremented in the stack.Route methods. This was going to be a problem once IPv4/IPv6 WritePackets support fragmentation because Route.WritePackets might now know how many packets are left after an error occurs. Test: - pkg/tcpip/network/ipv4:ipv4_test - pkg/tcpip/network/ipv6:ipv6_test PiperOrigin-RevId: 334687983 --- pkg/tcpip/network/ipv4/ipv4.go | 13 ++++++++-- pkg/tcpip/network/ipv4/ipv4_test.go | 47 +++++++++++++++++++++++++++++++++---- pkg/tcpip/network/ipv6/ipv6.go | 5 ++++ pkg/tcpip/stack/route.go | 21 +++++------------ 4 files changed, 65 insertions(+), 21 deletions(-) (limited to 'pkg') diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go index 34d3f8474..1f6e14c3f 100644 --- a/pkg/tcpip/network/ipv4/ipv4.go +++ b/pkg/tcpip/network/ipv4/ipv4.go @@ -278,6 +278,7 @@ func (e *endpoint) writePacketFragments(r *stack.Route, gso *stack.GSO, mtu int, // Send out the fragment. if err := e.linkEP.WritePacket(r, gso, ProtocolNumber, fragPkt); err != nil { + r.Stats().IP.OutgoingPacketErrors.IncrementBy(uint64(n - i)) return err } r.Stats().IP.PacketsSent.Increment() @@ -349,6 +350,7 @@ func (e *endpoint) WritePacket(r *stack.Route, gso *stack.GSO, params stack.Netw return e.writePacketFragments(r, gso, int(e.linkEP.MTU()), pkt) } if err := e.linkEP.WritePacket(r, gso, ProtocolNumber, pkt); err != nil { + r.Stats().IP.OutgoingPacketErrors.Increment() return err } r.Stats().IP.PacketsSent.Increment() @@ -379,6 +381,9 @@ func (e *endpoint) WritePackets(r *stack.Route, gso *stack.GSO, pkts stack.Packe // faster WritePackets API directly. n, err := e.linkEP.WritePackets(r, gso, pkts, ProtocolNumber) r.Stats().IP.PacketsSent.IncrementBy(uint64(n)) + if err != nil { + r.Stats().IP.OutgoingPacketErrors.IncrementBy(uint64(pkts.Len() - n)) + } return n, err } r.Stats().IP.IPTablesOutputDropped.IncrementBy(uint64(len(dropped))) @@ -403,6 +408,7 @@ 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)) + r.Stats().IP.OutgoingPacketErrors.IncrementBy(uint64(pkts.Len() - n - len(dropped))) // Dropped packets aren't errors, so include them in // the return value. return n + len(dropped), err @@ -461,9 +467,12 @@ func (e *endpoint) WriteHeaderIncludedPacket(r *stack.Route, pkt *stack.PacketBu return nil } + if err := e.linkEP.WritePacket(r, nil /* gso */, ProtocolNumber, pkt); err != nil { + r.Stats().IP.OutgoingPacketErrors.Increment() + return err + } r.Stats().IP.PacketsSent.Increment() - - return e.linkEP.WritePacket(r, nil /* gso */, ProtocolNumber, pkt) + return nil } // HandlePacket is called by the link layer when new ipv4 packets arrive for diff --git a/pkg/tcpip/network/ipv4/ipv4_test.go b/pkg/tcpip/network/ipv4/ipv4_test.go index ad3bc53ae..33cd5a3eb 100644 --- a/pkg/tcpip/network/ipv4/ipv4_test.go +++ b/pkg/tcpip/network/ipv4/ipv4_test.go @@ -450,6 +450,9 @@ func TestFragmentation(t *testing.T) { if got, want := len(ep.WrittenPackets), int(r.Stats().IP.PacketsSent.Value()); got != want { t.Errorf("no errors yet got len(ep.WrittenPackets) = %d, want = %d", got, want) } + if got := r.Stats().IP.OutgoingPacketErrors.Value(); got != 0 { + t.Errorf("got r.Stats().IP.OutgoingPacketErrors.Value() = %d, want = 0", got) + } compareFragments(t, ep.WrittenPackets, source, ft.mtu) }) } @@ -465,11 +468,44 @@ func TestFragmentationErrors(t *testing.T) { payloadViewsSizes []int err *tcpip.Error allowPackets int + fragmentCount int }{ - {"NoFrag", 2000, 0, []int{1000}, tcpip.ErrAborted, 0}, - {"ErrorOnFirstFrag", 500, 0, []int{1000}, tcpip.ErrAborted, 0}, - {"ErrorOnSecondFrag", 500, 0, []int{1000}, tcpip.ErrAborted, 1}, - {"ErrorOnFirstFragMTUSmallerThanHeader", 500, 1000, []int{500}, tcpip.ErrAborted, 0}, + { + description: "NoFrag", + mtu: 2000, + transportHeaderLength: 0, + payloadViewsSizes: []int{1000}, + err: tcpip.ErrAborted, + allowPackets: 0, + fragmentCount: 1, + }, + { + description: "ErrorOnFirstFrag", + mtu: 500, + transportHeaderLength: 0, + payloadViewsSizes: []int{1000}, + err: tcpip.ErrAborted, + allowPackets: 0, + fragmentCount: 3, + }, + { + description: "ErrorOnSecondFrag", + mtu: 500, + transportHeaderLength: 0, + payloadViewsSizes: []int{1000}, + err: tcpip.ErrAborted, + allowPackets: 1, + fragmentCount: 3, + }, + { + description: "ErrorOnFirstFragMTUSmallerThanHeader", + mtu: 500, + transportHeaderLength: 1000, + payloadViewsSizes: []int{500}, + err: tcpip.ErrAborted, + allowPackets: 0, + fragmentCount: 4, + }, } for _, ft := range fragTests { @@ -488,6 +524,9 @@ func TestFragmentationErrors(t *testing.T) { if got, want := len(ep.WrittenPackets), int(r.Stats().IP.PacketsSent.Value()); err != nil && got != want { t.Errorf("got len(ep.WrittenPackets) = %d, want = %d", got, want) } + if got, want := int(r.Stats().IP.OutgoingPacketErrors.Value()), ft.fragmentCount-ft.allowPackets; got != want { + t.Errorf("got r.Stats().IP.OutgoingPacketErrors.Value() = %d, want = %d", got, want) + } }) } } diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go index 7458c3795..c8a3e0b34 100644 --- a/pkg/tcpip/network/ipv6/ipv6.go +++ b/pkg/tcpip/network/ipv6/ipv6.go @@ -424,6 +424,7 @@ func (e *endpoint) WritePacket(r *stack.Route, gso *stack.GSO, params stack.Netw } if err := e.linkEP.WritePacket(r, gso, ProtocolNumber, pkt); err != nil { + r.Stats().IP.OutgoingPacketErrors.Increment() return err } r.Stats().IP.PacketsSent.Increment() @@ -453,6 +454,9 @@ func (e *endpoint) WritePackets(r *stack.Route, gso *stack.GSO, pkts stack.Packe // faster WritePackets API directly. n, err := e.linkEP.WritePackets(r, gso, pkts, ProtocolNumber) r.Stats().IP.PacketsSent.IncrementBy(uint64(n)) + if err != nil { + r.Stats().IP.OutgoingPacketErrors.IncrementBy(uint64(pkts.Len() - n)) + } return n, err } r.Stats().IP.IPTablesOutputDropped.IncrementBy(uint64(len(dropped))) @@ -477,6 +481,7 @@ 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)) + r.Stats().IP.OutgoingPacketErrors.IncrementBy(uint64(pkts.Len() - n + len(dropped))) // Dropped packets aren't errors, so include them in // the return value. return n + len(dropped), err diff --git a/pkg/tcpip/stack/route.go b/pkg/tcpip/stack/route.go index 5ade3c832..effe30155 100644 --- a/pkg/tcpip/stack/route.go +++ b/pkg/tcpip/stack/route.go @@ -211,14 +211,13 @@ func (r *Route) WritePacket(gso *GSO, params NetworkHeaderParams, pkt *PacketBuf // WritePacket takes ownership of pkt, calculate numBytes first. numBytes := pkt.Size() - err := r.addressEndpoint.NetworkEndpoint().WritePacket(r, gso, params, pkt) - if err != nil { - r.Stats().IP.OutgoingPacketErrors.Increment() - } else { - r.nic.stats.Tx.Packets.Increment() - r.nic.stats.Tx.Bytes.IncrementBy(uint64(numBytes)) + if err := r.addressEndpoint.NetworkEndpoint().WritePacket(r, gso, params, pkt); err != nil { + return err } - return err + + r.nic.stats.Tx.Packets.Increment() + r.nic.stats.Tx.Bytes.IncrementBy(uint64(numBytes)) + return nil } // WritePackets writes a list of n packets through the given route and returns @@ -228,15 +227,8 @@ func (r *Route) WritePackets(gso *GSO, pkts PacketBufferList, params NetworkHead return 0, tcpip.ErrInvalidEndpointState } - // WritePackets takes ownership of pkt, calculate length first. - numPkts := pkts.Len() - n, err := r.addressEndpoint.NetworkEndpoint().WritePackets(r, gso, pkts, params) - if err != nil { - r.Stats().IP.OutgoingPacketErrors.IncrementBy(uint64(numPkts - n)) - } r.nic.stats.Tx.Packets.IncrementBy(uint64(n)) - writtenBytes := 0 for i, pb := 0, pkts.Front(); i < n && pb != nil; i, pb = i+1, pb.Next() { writtenBytes += pb.Size() @@ -257,7 +249,6 @@ func (r *Route) WriteHeaderIncludedPacket(pkt *PacketBuffer) *tcpip.Error { numBytes := pkt.Data.Size() if err := r.addressEndpoint.NetworkEndpoint().WriteHeaderIncludedPacket(r, pkt); err != nil { - r.Stats().IP.OutgoingPacketErrors.Increment() return err } r.nic.stats.Tx.Packets.Increment() -- cgit v1.2.3