diff options
author | Ghanan Gowripalan <ghanan@google.com> | 2021-04-21 18:07:13 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-04-21 18:09:27 -0700 |
commit | 47bc115158397024841aa3747be7558b2c317cbb (patch) | |
tree | 462032ed4e1db726ed48fa974c7c8dbf928775c6 /pkg/tcpip/network/ipv4 | |
parent | 6f9db949d89000d920da5c97adad470cf97b1c6c (diff) |
Only carry GSO options in the packet buffer
With this change, GSO options no longer needs to be passed around as
a function argument in the write path.
This change is done in preparation for a later change that defers
segmentation, and may change GSO options for a packet as it flows
down the stack.
Updates #170.
PiperOrigin-RevId: 369774872
Diffstat (limited to 'pkg/tcpip/network/ipv4')
-rw-r--r-- | pkg/tcpip/network/ipv4/icmp.go | 1 | ||||
-rw-r--r-- | pkg/tcpip/network/ipv4/igmp.go | 2 | ||||
-rw-r--r-- | pkg/tcpip/network/ipv4/ipv4.go | 42 | ||||
-rw-r--r-- | pkg/tcpip/network/ipv4/ipv4_test.go | 31 |
4 files changed, 28 insertions, 48 deletions
diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go index 1525f15db..f663fdc0b 100644 --- a/pkg/tcpip/network/ipv4/icmp.go +++ b/pkg/tcpip/network/ipv4/icmp.go @@ -629,7 +629,6 @@ func (p *protocol) returnError(reason icmpReason, pkt *stack.PacketBuffer) tcpip icmpHdr.SetChecksum(header.ICMPv4Checksum(icmpHdr, icmpPkt.Data().AsRange().Checksum())) if err := route.WritePacket( - nil, /* gso */ stack.NetworkHeaderParams{ Protocol: header.ICMPv4ProtocolNumber, TTL: route.DefaultTTL(), diff --git a/pkg/tcpip/network/ipv4/igmp.go b/pkg/tcpip/network/ipv4/igmp.go index b1ac29294..3ce499298 100644 --- a/pkg/tcpip/network/ipv4/igmp.go +++ b/pkg/tcpip/network/ipv4/igmp.go @@ -341,7 +341,7 @@ func (igmp *igmpState) writePacket(destAddress tcpip.Address, groupAddress tcpip } sentStats := igmp.ep.stats.igmp.packetsSent - if err := igmp.ep.nic.WritePacketToRemote(header.EthernetAddressFromMulticastIPv4Address(destAddress), nil /* gso */, ProtocolNumber, pkt); err != nil { + if err := igmp.ep.nic.WritePacketToRemote(header.EthernetAddressFromMulticastIPv4Address(destAddress), ProtocolNumber, pkt); err != nil { sentStats.dropped.Increment() return false, err } diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go index a82a5790d..a0bc06465 100644 --- a/pkg/tcpip/network/ipv4/ipv4.go +++ b/pkg/tcpip/network/ipv4/ipv4.go @@ -350,7 +350,7 @@ func (e *endpoint) addIPHeader(srcAddr, dstAddr tcpip.Address, pkt *stack.Packet // fragment. It returns the number of fragments handled and the number of // fragments left to be processed. The IP header must already be present in the // original packet. -func (e *endpoint) handleFragments(r *stack.Route, gso *stack.GSO, networkMTU uint32, pkt *stack.PacketBuffer, handler func(*stack.PacketBuffer) tcpip.Error) (int, int, tcpip.Error) { +func (e *endpoint) handleFragments(r *stack.Route, networkMTU uint32, pkt *stack.PacketBuffer, handler func(*stack.PacketBuffer) tcpip.Error) (int, int, tcpip.Error) { // Round the MTU down to align to 8 bytes. fragmentPayloadSize := networkMTU &^ 7 networkHeader := header.IPv4(pkt.NetworkHeader().View()) @@ -370,7 +370,7 @@ func (e *endpoint) handleFragments(r *stack.Route, gso *stack.GSO, networkMTU ui } // WritePacket writes a packet to the given destination address and protocol. -func (e *endpoint) WritePacket(r *stack.Route, gso *stack.GSO, params stack.NetworkHeaderParams, pkt *stack.PacketBuffer) tcpip.Error { +func (e *endpoint) WritePacket(r *stack.Route, params stack.NetworkHeaderParams, pkt *stack.PacketBuffer) tcpip.Error { if err := e.addIPHeader(r.LocalAddress(), r.RemoteAddress(), pkt, params, nil /* options */); err != nil { return err } @@ -378,7 +378,7 @@ func (e *endpoint) WritePacket(r *stack.Route, gso *stack.GSO, params stack.Netw // iptables filtering. All packets that reach here are locally // generated. outNicName := e.protocol.stack.FindNICNameFromID(e.nic.ID()) - if ok := e.protocol.stack.IPTables().Check(stack.Output, pkt, gso, r, "" /* preroutingAddr */, "" /* inNicName */, outNicName); !ok { + if ok := e.protocol.stack.IPTables().Check(stack.Output, pkt, r, "" /* preroutingAddr */, "" /* inNicName */, outNicName); !ok { // iptables is telling us to drop the packet. e.stats.ip.IPTablesOutputDropped.Increment() return nil @@ -401,10 +401,10 @@ func (e *endpoint) WritePacket(r *stack.Route, gso *stack.GSO, params stack.Netw } } - return e.writePacket(r, gso, pkt, false /* headerIncluded */) + return e.writePacket(r, pkt, false /* headerIncluded */) } -func (e *endpoint) writePacket(r *stack.Route, gso *stack.GSO, pkt *stack.PacketBuffer, headerIncluded bool) tcpip.Error { +func (e *endpoint) writePacket(r *stack.Route, pkt *stack.PacketBuffer, headerIncluded bool) tcpip.Error { if r.Loop()&stack.PacketLoop != 0 { // If the packet was generated by the stack (not a raw/packet endpoint // where a packet may be written with the header included), then we can @@ -418,7 +418,7 @@ func (e *endpoint) writePacket(r *stack.Route, gso *stack.GSO, pkt *stack.Packet // Postrouting NAT can only change the source address, and does not alter the // route or outgoing interface of the packet. outNicName := e.protocol.stack.FindNICNameFromID(e.nic.ID()) - if ok := e.protocol.stack.IPTables().Check(stack.Postrouting, pkt, gso, r, "" /* preroutingAddr */, "" /* inNicName */, outNicName); !ok { + if ok := e.protocol.stack.IPTables().Check(stack.Postrouting, pkt, r, "" /* preroutingAddr */, "" /* inNicName */, outNicName); !ok { // iptables is telling us to drop the packet. e.stats.ip.IPTablesPostroutingDropped.Increment() return nil @@ -432,20 +432,20 @@ func (e *endpoint) writePacket(r *stack.Route, gso *stack.GSO, pkt *stack.Packet return err } - if packetMustBeFragmented(pkt, networkMTU, gso) { - sent, remain, err := e.handleFragments(r, gso, networkMTU, pkt, func(fragPkt *stack.PacketBuffer) tcpip.Error { + if packetMustBeFragmented(pkt, networkMTU) { + sent, remain, err := e.handleFragments(r, networkMTU, pkt, func(fragPkt *stack.PacketBuffer) tcpip.Error { // TODO(gvisor.dev/issue/3884): Evaluate whether we want to send each // fragment one by one using WritePacket() (current strategy) or if we // want to create a PacketBufferList from the fragments and feed it to // WritePackets(). It'll be faster but cost more memory. - return e.nic.WritePacket(r, gso, ProtocolNumber, fragPkt) + return e.nic.WritePacket(r, ProtocolNumber, fragPkt) }) stats.PacketsSent.IncrementBy(uint64(sent)) stats.OutgoingPacketErrors.IncrementBy(uint64(remain)) return err } - if err := e.nic.WritePacket(r, gso, ProtocolNumber, pkt); err != nil { + if err := e.nic.WritePacket(r, ProtocolNumber, pkt); err != nil { stats.OutgoingPacketErrors.Increment() return err } @@ -454,7 +454,7 @@ func (e *endpoint) writePacket(r *stack.Route, gso *stack.GSO, pkt *stack.Packet } // WritePackets implements stack.NetworkEndpoint.WritePackets. -func (e *endpoint) WritePackets(r *stack.Route, gso *stack.GSO, pkts stack.PacketBufferList, params stack.NetworkHeaderParams) (int, tcpip.Error) { +func (e *endpoint) WritePackets(r *stack.Route, pkts stack.PacketBufferList, params stack.NetworkHeaderParams) (int, tcpip.Error) { if r.Loop()&stack.PacketLoop != 0 { panic("multiple packets in local loop") } @@ -475,11 +475,11 @@ func (e *endpoint) WritePackets(r *stack.Route, gso *stack.GSO, pkts stack.Packe return 0, err } - if packetMustBeFragmented(pkt, networkMTU, gso) { + if packetMustBeFragmented(pkt, networkMTU) { // Keep track of the packet that is about to be fragmented so it can be // removed once the fragmentation is done. originalPkt := pkt - if _, _, err := e.handleFragments(r, gso, networkMTU, pkt, func(fragPkt *stack.PacketBuffer) tcpip.Error { + if _, _, err := e.handleFragments(r, networkMTU, pkt, func(fragPkt *stack.PacketBuffer) tcpip.Error { // Modify the packet list in place with the new fragments. pkts.InsertAfter(pkt, fragPkt) pkt = fragPkt @@ -495,7 +495,7 @@ func (e *endpoint) WritePackets(r *stack.Route, gso *stack.GSO, pkts stack.Packe outNicName := e.protocol.stack.FindNICNameFromID(e.nic.ID()) // iptables filtering. All packets that reach here are locally // generated. - outputDropped, natPkts := e.protocol.stack.IPTables().CheckPackets(stack.Output, pkts, gso, r, "" /* inNicName */, outNicName) + outputDropped, natPkts := e.protocol.stack.IPTables().CheckPackets(stack.Output, pkts, r, "" /* inNicName */, outNicName) stats.IPTablesOutputDropped.IncrementBy(uint64(len(outputDropped))) for pkt := range outputDropped { pkts.Remove(pkt) @@ -522,7 +522,7 @@ func (e *endpoint) WritePackets(r *stack.Route, gso *stack.GSO, pkts stack.Packe // We ignore the list of NAT-ed packets here because Postrouting NAT can only // change the source address, and does not alter the route or outgoing // interface of the packet. - postroutingDropped, _ := e.protocol.stack.IPTables().CheckPackets(stack.Postrouting, pkts, gso, r, "" /* inNicName */, outNicName) + postroutingDropped, _ := e.protocol.stack.IPTables().CheckPackets(stack.Postrouting, pkts, r, "" /* inNicName */, outNicName) stats.IPTablesPostroutingDropped.IncrementBy(uint64(len(postroutingDropped))) for pkt := range postroutingDropped { pkts.Remove(pkt) @@ -530,7 +530,7 @@ func (e *endpoint) WritePackets(r *stack.Route, gso *stack.GSO, pkts stack.Packe // The rest of the packets can be delivered to the NIC as a batch. pktsLen := pkts.Len() - written, err := e.nic.WritePackets(r, gso, pkts, ProtocolNumber) + written, err := e.nic.WritePackets(r, pkts, ProtocolNumber) stats.PacketsSent.IncrementBy(uint64(written)) stats.OutgoingPacketErrors.IncrementBy(uint64(pktsLen - written)) @@ -595,7 +595,7 @@ func (e *endpoint) WriteHeaderIncludedPacket(r *stack.Route, pkt *stack.PacketBu return &tcpip.ErrMalformedHeader{} } - return e.writePacket(r, nil /* gso */, pkt, true /* headerIncluded */) + return e.writePacket(r, pkt, true /* headerIncluded */) } // forwardPacket attempts to forward a packet to its final destination. @@ -732,7 +732,7 @@ func (e *endpoint) HandlePacket(pkt *stack.PacketBuffer) { // Loopback traffic skips the prerouting chain. inNicName := e.protocol.stack.FindNICNameFromID(e.nic.ID()) - if ok := e.protocol.stack.IPTables().Check(stack.Prerouting, pkt, nil, nil, e.MainAddress().Address, inNicName, "" /* outNicName */); !ok { + if ok := e.protocol.stack.IPTables().Check(stack.Prerouting, pkt, nil, e.MainAddress().Address, inNicName, "" /* outNicName */); !ok { // iptables is telling us to drop the packet. stats.IPTablesPreroutingDropped.Increment() return @@ -805,7 +805,7 @@ func (e *endpoint) handleValidatedPacket(h header.IPv4, pkt *stack.PacketBuffer) // iptables filtering. All packets that reach here are intended for // this machine and will not be forwarded. inNicName := e.protocol.stack.FindNICNameFromID(e.nic.ID()) - if ok := e.protocol.stack.IPTables().Check(stack.Input, pkt, nil, nil, "" /* preroutingAddr */, inNicName, "" /* outNicName */); !ok { + if ok := e.protocol.stack.IPTables().Check(stack.Input, pkt, nil, "" /* preroutingAddr */, inNicName, "" /* outNicName */); !ok { // iptables is telling us to drop the packet. stats.ip.IPTablesInputDropped.Increment() return @@ -1258,9 +1258,9 @@ func calculateNetworkMTU(linkMTU, networkHeaderSize uint32) (uint32, tcpip.Error return networkMTU - uint32(networkHeaderSize), nil } -func packetMustBeFragmented(pkt *stack.PacketBuffer, networkMTU uint32, gso *stack.GSO) bool { +func packetMustBeFragmented(pkt *stack.PacketBuffer, networkMTU uint32) bool { payload := pkt.TransportHeader().View().Size() + pkt.Data().Size() - return (gso == nil || gso.Type == stack.GSONone) && uint32(payload) > networkMTU + return pkt.GSOOptions.Type == stack.GSONone && uint32(payload) > networkMTU } // addressToUint32 translates an IPv4 address into its little endian uint32 diff --git a/pkg/tcpip/network/ipv4/ipv4_test.go b/pkg/tcpip/network/ipv4/ipv4_test.go index d49dff4d5..7d413c455 100644 --- a/pkg/tcpip/network/ipv4/ipv4_test.go +++ b/pkg/tcpip/network/ipv4/ipv4_test.go @@ -1242,7 +1242,6 @@ type fragmentInfo struct { var fragmentationTests = []struct { description string mtu uint32 - gso *stack.GSO transportHeaderLength int payloadSize int wantFragments []fragmentInfo @@ -1250,7 +1249,6 @@ var fragmentationTests = []struct { { description: "No fragmentation", mtu: 1280, - gso: nil, transportHeaderLength: 0, payloadSize: 1000, wantFragments: []fragmentInfo{ @@ -1260,7 +1258,6 @@ var fragmentationTests = []struct { { description: "Fragmented", mtu: 1280, - gso: nil, transportHeaderLength: 0, payloadSize: 2000, wantFragments: []fragmentInfo{ @@ -1271,7 +1268,6 @@ var fragmentationTests = []struct { { description: "Fragmented with the minimum mtu", mtu: header.IPv4MinimumMTU, - gso: nil, transportHeaderLength: 0, payloadSize: 100, wantFragments: []fragmentInfo{ @@ -1283,7 +1279,6 @@ var fragmentationTests = []struct { { description: "Fragmented with mtu not a multiple of 8", mtu: header.IPv4MinimumMTU + 1, - gso: nil, transportHeaderLength: 0, payloadSize: 100, wantFragments: []fragmentInfo{ @@ -1295,7 +1290,6 @@ var fragmentationTests = []struct { { description: "No fragmentation with big header", mtu: 2000, - gso: nil, transportHeaderLength: 100, payloadSize: 1000, wantFragments: []fragmentInfo{ @@ -1303,20 +1297,8 @@ var fragmentationTests = []struct { }, }, { - description: "Fragmented with gso none", - mtu: 1280, - gso: &stack.GSO{Type: stack.GSONone}, - transportHeaderLength: 0, - payloadSize: 1400, - wantFragments: []fragmentInfo{ - {offset: 0, payloadSize: 1256, more: true}, - {offset: 1256, payloadSize: 144, more: false}, - }, - }, - { description: "Fragmented with big header", mtu: 1280, - gso: nil, transportHeaderLength: 100, payloadSize: 1200, wantFragments: []fragmentInfo{ @@ -1327,7 +1309,6 @@ var fragmentationTests = []struct { { description: "Fragmented with MTU smaller than header", mtu: 300, - gso: nil, transportHeaderLength: 1000, payloadSize: 500, wantFragments: []fragmentInfo{ @@ -1350,13 +1331,13 @@ func TestFragmentationWritePacket(t *testing.T) { r := buildRoute(t, ep) pkt := testutil.MakeRandPkt(ft.transportHeaderLength, extraHeaderReserve+header.IPv4MinimumSize, []int{ft.payloadSize}, header.IPv4ProtocolNumber) source := pkt.Clone() - err := r.WritePacket(ft.gso, stack.NetworkHeaderParams{ + err := r.WritePacket(stack.NetworkHeaderParams{ Protocol: tcp.ProtocolNumber, TTL: ttl, TOS: stack.DefaultTOS, }, pkt) if err != nil { - t.Fatalf("r.WritePacket(_, _, _) = %s", err) + t.Fatalf("r.WritePacket(...): %s", err) } if got := len(ep.WrittenPackets); got != len(ft.wantFragments) { t.Errorf("got len(ep.WrittenPackets) = %d, want = %d", got, len(ft.wantFragments)) @@ -1422,7 +1403,7 @@ func TestFragmentationWritePackets(t *testing.T) { r := buildRoute(t, ep) wantTotalPackets := len(ft.wantFragments) + test.insertBefore + test.insertAfter - n, err := r.WritePackets(ft.gso, pkts, stack.NetworkHeaderParams{ + n, err := r.WritePackets(pkts, stack.NetworkHeaderParams{ Protocol: tcp.ProtocolNumber, TTL: ttl, TOS: stack.DefaultTOS, @@ -1529,7 +1510,7 @@ func TestFragmentationErrors(t *testing.T) { pkt := testutil.MakeRandPkt(ft.transportHeaderLength, extraHeaderReserve+header.IPv4MinimumSize, []int{ft.payloadSize}, header.IPv4ProtocolNumber) ep := testutil.NewMockLinkEndpoint(ft.mtu, ft.mockError, ft.allowPackets) r := buildRoute(t, ep) - err := r.WritePacket(&stack.GSO{}, stack.NetworkHeaderParams{ + err := r.WritePacket(stack.NetworkHeaderParams{ Protocol: tcp.ProtocolNumber, TTL: ttl, TOS: stack.DefaultTOS, @@ -2729,7 +2710,7 @@ func TestWriteStats(t *testing.T) { 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 { + if err := rt.WritePacket(stack.NetworkHeaderParams{}, pkt); err != nil { return nWritten, err } nWritten++ @@ -2739,7 +2720,7 @@ func TestWriteStats(t *testing.T) { }, { name: "WritePackets", writePackets: func(rt *stack.Route, pkts stack.PacketBufferList) (int, tcpip.Error) { - return rt.WritePackets(nil, pkts, stack.NetworkHeaderParams{}) + return rt.WritePackets(pkts, stack.NetworkHeaderParams{}) }, }, } |