From d6a39734c4934aa98141074c67389e80ed130a94 Mon Sep 17 00:00:00 2001 From: Ghanan Gowripalan Date: Thu, 28 Jan 2021 20:30:02 -0800 Subject: [PATCH] Avoid locking when route doesn't require resolution When a route does not need to resolve a remote link address to send a packet, avoid having to obtain the pending packets queue's lock. PiperOrigin-RevId: 354456280 --- pkg/tcpip/stack/nic.go | 58 ++++++++++++++++++++---------- pkg/tcpip/stack/pending_packets.go | 18 ++-------- 2 files changed, 42 insertions(+), 34 deletions(-) diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go index 16e55085d..e56a624fe 100644 --- a/pkg/tcpip/stack/nic.go +++ b/pkg/tcpip/stack/nic.go @@ -309,25 +309,47 @@ func (n *NIC) WritePacket(r *Route, gso *GSO, protocol tcpip.NetworkProtocolNumb return err } +func (n *NIC) writePacketBuffer(r RouteInfo, gso *GSO, protocol tcpip.NetworkProtocolNumber, pkt pendingPacketBuffer) (int, tcpip.Error) { + switch pkt := pkt.(type) { + case *PacketBuffer: + if err := n.writePacket(r, gso, protocol, pkt); err != nil { + return 0, err + } + return 1, nil + case *PacketBufferList: + return n.writePackets(r, gso, protocol, *pkt) + default: + panic(fmt.Sprintf("unrecognized pending packet buffer type = %T", pkt)) + } +} + func (n *NIC) enqueuePacketBuffer(r *Route, gso *GSO, protocol tcpip.NetworkProtocolNumber, pkt pendingPacketBuffer) (int, tcpip.Error) { - // As per relevant RFCs, we should queue packets while we wait for link - // resolution to complete. - // - // RFC 1122 section 2.3.2.2 (for IPv4): - // The link layer SHOULD save (rather than discard) at least - // one (the latest) packet of each set of packets destined to - // the same unresolved IP address, and transmit the saved - // packet when the address has been resolved. - // - // RFC 4861 section 7.2.2 (for IPv6): - // While waiting for address resolution to complete, the sender MUST, for - // each neighbor, retain a small queue of packets waiting for address - // resolution to complete. The queue MUST hold at least one packet, and MAY - // contain more. However, the number of queued packets per neighbor SHOULD - // be limited to some small value. When a queue overflows, the new arrival - // SHOULD replace the oldest entry. Once address resolution completes, the - // node transmits any queued packets. - return n.linkResQueue.enqueue(r, gso, protocol, pkt) + routeInfo, _, err := r.resolvedFields(nil) + switch err.(type) { + case nil: + return n.writePacketBuffer(routeInfo, gso, protocol, pkt) + case *tcpip.ErrWouldBlock: + // As per relevant RFCs, we should queue packets while we wait for link + // resolution to complete. + // + // RFC 1122 section 2.3.2.2 (for IPv4): + // The link layer SHOULD save (rather than discard) at least + // one (the latest) packet of each set of packets destined to + // the same unresolved IP address, and transmit the saved + // packet when the address has been resolved. + // + // RFC 4861 section 7.2.2 (for IPv6): + // While waiting for address resolution to complete, the sender MUST, for + // each neighbor, retain a small queue of packets waiting for address + // resolution to complete. The queue MUST hold at least one packet, and + // MAY contain more. However, the number of queued packets per neighbor + // SHOULD be limited to some small value. When a queue overflows, the new + // arrival SHOULD replace the oldest entry. Once address resolution + // completes, the node transmits any queued packets. + return n.linkResQueue.enqueue(r, gso, protocol, pkt) + default: + return 0, err + } } // WritePacketToRemote implements NetworkInterface. diff --git a/pkg/tcpip/stack/pending_packets.go b/pkg/tcpip/stack/pending_packets.go index c6adfb27b..1c651e216 100644 --- a/pkg/tcpip/stack/pending_packets.go +++ b/pkg/tcpip/stack/pending_packets.go @@ -114,20 +114,6 @@ func (f *packetsPendingLinkResolution) dequeue(ch <-chan struct{}, linkAddr tcpi } } -func (f *packetsPendingLinkResolution) writePacketBuffer(r RouteInfo, gso *GSO, proto tcpip.NetworkProtocolNumber, pkt pendingPacketBuffer) (int, tcpip.Error) { - switch pkt := pkt.(type) { - case *PacketBuffer: - if err := f.nic.writePacket(r, gso, proto, pkt); err != nil { - return 0, err - } - return 1, nil - case *PacketBufferList: - return f.nic.writePackets(r, gso, proto, *pkt) - default: - panic(fmt.Sprintf("unrecognized pending packet buffer type = %T", pkt)) - } -} - // enqueue a packet to be sent once link resolution completes. // // If the maximum number of pending resolutions is reached, the packets @@ -151,7 +137,7 @@ func (f *packetsPendingLinkResolution) enqueue(r *Route, gso *GSO, proto tcpip.N // The route resolved immediately, so we don't need to wait for link // resolution to send the packet. f.mu.Unlock() - return f.writePacketBuffer(routeInfo, gso, proto, pkt) + return f.nic.writePacketBuffer(routeInfo, gso, proto, pkt) case *tcpip.ErrWouldBlock: // We need to wait for link resolution to complete. default: @@ -225,7 +211,7 @@ func (f *packetsPendingLinkResolution) dequeuePackets(packets []pendingPacket, l for _, p := range packets { if success { p.routeInfo.RemoteLinkAddress = linkAddr - _, _ = f.writePacketBuffer(p.routeInfo, p.gso, p.proto, p.pkt) + _, _ = f.nic.writePacketBuffer(p.routeInfo, p.gso, p.proto, p.pkt) } else { f.incrementOutgoingPacketErrors(p.proto, p.pkt)