From ddb7a38802f6b870ee6e7a57c01abbde11685667 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Wed, 13 Mar 2024 12:04:15 -0700 Subject: [PATCH] netstack: remove several layers of interface nesting of LinkEndpoints runsc's NICs will just have an fdbased LinkEndpoint to write to unless the user asks for packet logging. Before, a write from the NIC to its endpoint would go something like: - sniffer.endpoint.WritePackets - packetsocket.endpoint.WritePackets - nested.Endpoint.WritePackets - fdbased.endpoint.WritePackets In addition to the extra work done in some of these steps, and the slowness of interface calls in a hot path, directly calling into an fdbased.endpoint opens up additional optimizations (see other CLs in this chain). For example, if the qdisc knows it's talking to an fdbased.endpoint, it can pass in persistent structs (iovecs, mmsghdrs) rather than re-allocating them in every call to WritePackets. PiperOrigin-RevId: 615499448 --- pkg/tcpip/stack/nic.go | 17 ++++++++++++++ pkg/tcpip/stack/stack.go | 4 ++++ runsc/boot/BUILD | 1 - runsc/boot/loader.go | 8 ++++--- runsc/boot/network.go | 51 +++++++++++++++++++++------------------- runsc/sandbox/network.go | 3 +++ 6 files changed, 56 insertions(+), 28 deletions(-) diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go index 73836285b..7e9348182 100644 --- a/pkg/tcpip/stack/nic.go +++ b/pkg/tcpip/stack/nic.go @@ -79,6 +79,13 @@ type nic struct { qDisc QueueingDiscipline gro groDispatcher + + // deliverLinkPackets specifies whether this NIC delivers packets to + // packet sockets. It is immutable. + // + // deliverLinkPackets is off by default because some users already + // deliver link packets by explicitly calling nic.DeliverLinkPackets. + deliverLinkPackets bool } // makeNICStats initializes the NIC statistics and associates them to the global @@ -174,6 +181,7 @@ func newNIC(stack *Stack, id tcpip.NICID, ep LinkEndpoint, opts NICOptions) *nic linkAddrResolvers: make(map[tcpip.NetworkProtocolNumber]*linkResolver), duplicateAddressDetectors: make(map[tcpip.NetworkProtocolNumber]DuplicateAddressDetector), qDisc: qDisc, + deliverLinkPackets: opts.DeliverLinkPackets, } nic.linkResQueue.init(nic) @@ -396,6 +404,11 @@ func (n *nic) writeRawPacketWithLinkHeaderInPayload(pkt *PacketBuffer) tcpip.Err func (n *nic) writeRawPacket(pkt *PacketBuffer) tcpip.Error { // Always an outgoing packet. pkt.PktType = tcpip.PacketOutgoing + + if n.deliverLinkPackets { + n.DeliverLinkPacket(pkt.NetworkProtocolNumber, pkt) + } + if err := n.qDisc.WritePacket(pkt); err != nil { if _, ok := err.(*tcpip.ErrNoBufferSpace); ok { n.stats.txPacketsDroppedNoBufferSpace.Increment() @@ -735,6 +748,10 @@ func (n *nic) DeliverNetworkPacket(protocol tcpip.NetworkProtocolNumber, pkt *Pa pkt.RXChecksumValidated = n.NetworkLinkEndpoint.Capabilities()&CapabilityRXChecksumOffload != 0 + if n.deliverLinkPackets { + n.DeliverLinkPacket(protocol, pkt) + } + n.gro.dispatch(pkt, protocol, networkEndpoint) } diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index b305c7adb..4d3f74566 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -851,6 +851,10 @@ type NICOptions struct { // GROTimeout specifies the GRO timeout. Zero bypasses GRO. GROTimeout time.Duration + + // DeliverLinkPackets specifies whether the NIC is responsible for + // delivering raw packets to packet sockets. + DeliverLinkPackets bool } // CreateNICWithOptions creates a NIC with the provided id, LinkEndpoint, and diff --git a/runsc/boot/BUILD b/runsc/boot/BUILD index 4893321bc..4c92deff8 100644 --- a/runsc/boot/BUILD +++ b/runsc/boot/BUILD @@ -107,7 +107,6 @@ go_library( "//pkg/tcpip/link/ethernet", "//pkg/tcpip/link/fdbased", "//pkg/tcpip/link/loopback", - "//pkg/tcpip/link/packetsocket", "//pkg/tcpip/link/qdisc/fifo", "//pkg/tcpip/link/sniffer", "//pkg/tcpip/link/xdp", diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index b029a0dc3..c92f75aa7 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -62,7 +62,6 @@ import ( "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/link/ethernet" "gvisor.dev/gvisor/pkg/tcpip/link/loopback" - "gvisor.dev/gvisor/pkg/tcpip/link/packetsocket" "gvisor.dev/gvisor/pkg/tcpip/link/sniffer" "gvisor.dev/gvisor/pkg/tcpip/network/arp" "gvisor.dev/gvisor/pkg/tcpip/network/ipv4" @@ -1406,8 +1405,11 @@ func (f *sandboxNetstackCreator) CreateStack() (inet.Stack, error) { n := &Network{Stack: s.(*netstack.Stack).Stack} nicID := tcpip.NICID(f.uniqueID.UniqueID()) link := DefaultLoopbackLink - linkEP := packetsocket.New(ethernet.New(loopback.New())) - opts := stack.NICOptions{Name: link.Name} + linkEP := ethernet.New(loopback.New()) + opts := stack.NICOptions{ + Name: link.Name, + DeliverLinkPackets: true, + } if err := n.createNICWithAddrs(nicID, linkEP, opts, link.Addresses); err != nil { return nil, err diff --git a/runsc/boot/network.go b/runsc/boot/network.go index 3506aa382..b60c7784e 100644 --- a/runsc/boot/network.go +++ b/runsc/boot/network.go @@ -32,7 +32,6 @@ import ( "gvisor.dev/gvisor/pkg/tcpip/link/ethernet" "gvisor.dev/gvisor/pkg/tcpip/link/fdbased" "gvisor.dev/gvisor/pkg/tcpip/link/loopback" - "gvisor.dev/gvisor/pkg/tcpip/link/packetsocket" "gvisor.dev/gvisor/pkg/tcpip/link/qdisc/fifo" "gvisor.dev/gvisor/pkg/tcpip/link/sniffer" "gvisor.dev/gvisor/pkg/tcpip/link/xdp" @@ -170,6 +169,9 @@ type CreateLinksAndRoutesArgs struct { // PCAP indicates that FilePayload also contains a PCAP log file. PCAP bool + // LogPackets indicates that packets should be logged. + LogPackets bool + // NATBlob indicates whether FilePayload also contains an iptables NAT // ruleset. NATBlob bool @@ -249,12 +251,13 @@ func (n *Network) CreateLinksAndRoutes(args *CreateLinksAndRoutesArgs, _ *struct nicID++ nicids[link.Name] = nicID - linkEP := packetsocket.New(ethernet.New(loopback.New())) + linkEP := ethernet.New(loopback.New()) log.Infof("Enabling loopback interface %q with id %d on addresses %+v", link.Name, nicID, link.Addresses) opts := stack.NICOptions{ - Name: link.Name, - GROTimeout: link.GvisorGROTimeout, + Name: link.Name, + GROTimeout: link.GvisorGROTimeout, + DeliverLinkPackets: true, } if err := n.createNICWithAddrs(nicID, linkEP, opts, link.Addresses); err != nil { return err @@ -319,21 +322,20 @@ func (n *Network) CreateLinksAndRoutes(args *CreateLinksAndRoutesArgs, _ *struct return err } - // Wrap linkEP in a sniffer to enable packet logging. - var sniffEP stack.LinkEndpoint + // Setup packet logging if requested. if args.PCAP { newFD, err := unix.Dup(int(args.FilePayload.Files[fdOffset].Fd())) if err != nil { return fmt.Errorf("failed to dup pcap FD: %v", err) } const packetTruncateSize = 4096 - sniffEP, err = sniffer.NewWithWriter(packetsocket.New(linkEP), os.NewFile(uintptr(newFD), "pcap-file"), packetTruncateSize) + linkEP, err = sniffer.NewWithWriter(linkEP, os.NewFile(uintptr(newFD), "pcap-file"), packetTruncateSize) if err != nil { return fmt.Errorf("failed to create PCAP logger: %v", err) } fdOffset++ - } else { - sniffEP = sniffer.New(packetsocket.New(linkEP)) + } else if args.LogPackets { + linkEP = sniffer.New(linkEP) } var qDisc stack.QueueingDiscipline @@ -341,16 +343,17 @@ func (n *Network) CreateLinksAndRoutes(args *CreateLinksAndRoutesArgs, _ *struct case config.QDiscNone: case config.QDiscFIFO: log.Infof("Enabling FIFO QDisc on %q", link.Name) - qDisc = fifo.New(sniffEP, runtime.GOMAXPROCS(0), 1000) + qDisc = fifo.New(linkEP, runtime.GOMAXPROCS(0), 1000) } log.Infof("Enabling interface %q with id %d on addresses %+v (%v) w/ %d channels", link.Name, nicID, link.Addresses, mac, link.NumChannels) opts := stack.NICOptions{ - Name: link.Name, - QDisc: qDisc, - GROTimeout: link.GvisorGROTimeout, + Name: link.Name, + QDisc: qDisc, + GROTimeout: link.GvisorGROTimeout, + DeliverLinkPackets: true, } - if err := n.createNICWithAddrs(nicID, sniffEP, opts, link.Addresses); err != nil { + if err := n.createNICWithAddrs(nicID, linkEP, opts, link.Addresses); err != nil { return err } @@ -398,6 +401,7 @@ func (n *Network) CreateLinksAndRoutes(args *CreateLinksAndRoutesArgs, _ *struct } } + // Setup packet logging if requested. mac := tcpip.LinkAddress(link.LinkAddress) linkEP, err := xdp.New(&xdp.Options{ FD: fd, @@ -411,21 +415,19 @@ func (n *Network) CreateLinksAndRoutes(args *CreateLinksAndRoutesArgs, _ *struct return err } - // Wrap linkEP in a sniffer to enable packet logging. - var sniffEP stack.LinkEndpoint if args.PCAP { newFD, err := unix.Dup(int(args.FilePayload.Files[fdOffset].Fd())) if err != nil { return fmt.Errorf("failed to dup pcap FD: %v", err) } const packetTruncateSize = 4096 - sniffEP, err = sniffer.NewWithWriter(packetsocket.New(linkEP), os.NewFile(uintptr(newFD), "pcap-file"), packetTruncateSize) + linkEP, err = sniffer.NewWithWriter(linkEP, os.NewFile(uintptr(newFD), "pcap-file"), packetTruncateSize) if err != nil { return fmt.Errorf("failed to create PCAP logger: %v", err) } fdOffset++ - } else { - sniffEP = sniffer.New(packetsocket.New(linkEP)) + } else if args.LogPackets { + linkEP = sniffer.New(linkEP) } var qDisc stack.QueueingDiscipline @@ -433,16 +435,17 @@ func (n *Network) CreateLinksAndRoutes(args *CreateLinksAndRoutesArgs, _ *struct case config.QDiscNone: case config.QDiscFIFO: log.Infof("Enabling FIFO QDisc on %q", link.Name) - qDisc = fifo.New(sniffEP, runtime.GOMAXPROCS(0), 1000) + qDisc = fifo.New(linkEP, runtime.GOMAXPROCS(0), 1000) } log.Infof("Enabling interface %q with id %d on addresses %+v (%v) w/ %d channels", link.Name, nicID, link.Addresses, mac, link.NumChannels) opts := stack.NICOptions{ - Name: link.Name, - QDisc: qDisc, - GROTimeout: link.GvisorGROTimeout, + Name: link.Name, + QDisc: qDisc, + GROTimeout: link.GvisorGROTimeout, + DeliverLinkPackets: true, } - if err := n.createNICWithAddrs(nicID, sniffEP, opts, link.Addresses); err != nil { + if err := n.createNICWithAddrs(nicID, linkEP, opts, link.Addresses); err != nil { return err } diff --git a/runsc/sandbox/network.go b/runsc/sandbox/network.go index afbbde5c0..660acbecb 100644 --- a/runsc/sandbox/network.go +++ b/runsc/sandbox/network.go @@ -526,6 +526,9 @@ func removeAddress(source netlink.Link, ipAndMask string) error { } func pcapAndNAT(args *boot.CreateLinksAndRoutesArgs, conf *config.Config) error { + // Possibly enable packet logging. + args.LogPackets = conf.LogPackets + // Pass PCAP log file if present. if conf.PCAP != "" { args.PCAP = true