diff options
| -rw-r--r-- | pkg/tcpip/link/pipe/pipe.go | 17 | ||||
| -rw-r--r-- | pkg/tcpip/stack/nic.go | 9 | ||||
| -rw-r--r-- | pkg/tcpip/stack/registration.go | 21 | ||||
| -rw-r--r-- | pkg/tcpip/stack/route.go | 22 | ||||
| -rw-r--r-- | pkg/tcpip/stack/stack.go | 38 |
5 files changed, 58 insertions, 49 deletions
diff --git a/pkg/tcpip/link/pipe/pipe.go b/pkg/tcpip/link/pipe/pipe.go index 12e246e21..d6e83a414 100644 --- a/pkg/tcpip/link/pipe/pipe.go +++ b/pkg/tcpip/link/pipe/pipe.go @@ -55,7 +55,22 @@ func (e *Endpoint) WritePacket(r stack.RouteInfo, _ *stack.GSO, proto tcpip.Netw // remote address from the perspective of the other end of the pipe // (e.linked). Similarly, the remote address from the perspective of this // endpoint is the local address on the other end. - e.linked.dispatcher.DeliverNetworkPacket(r.LocalLinkAddress /* remote */, r.RemoteLinkAddress /* local */, proto, stack.NewPacketBuffer(stack.PacketBufferOptions{ + // + // Deliver the packet in a new goroutine to escape this goroutine's stack and + // avoid a deadlock when a packet triggers a response which leads the stack to + // try and take a lock it already holds. + // + // As of writing, a deadlock may occur when performing link resolution as the + // neighbor table will send a solicitation while holding a lock and the + // response advertisement will be sent in the same stack that sent the + // solictation. When the response is received, the stack attempts to take the + // same lock it already took before sending the solicitation, leading to a + // deadlock. Basically, we attempt to lock the same lock twice in the same + // call stack. + // + // TODO(gvisor.dev/issue/5289): don't use a new goroutine once we support send + // and receive queues. + go e.linked.dispatcher.DeliverNetworkPacket(r.LocalLinkAddress /* remote */, r.RemoteLinkAddress /* local */, proto, stack.NewPacketBuffer(stack.PacketBufferOptions{ Data: buffer.NewVectorisedView(pkt.Size(), pkt.Views()), })) diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go index c8d0323cb..9e106a161 100644 --- a/pkg/tcpip/stack/nic.go +++ b/pkg/tcpip/stack/nic.go @@ -547,6 +547,15 @@ func (n *NIC) removeAddress(addr tcpip.Address) *tcpip.Error { return tcpip.ErrBadLocalAddress } +func (n *NIC) getNeighborLinkAddress(addr, localAddr tcpip.Address, linkRes LinkAddressResolver, onResolve func(tcpip.LinkAddress, bool)) (tcpip.LinkAddress, <-chan struct{}, *tcpip.Error) { + if n.neigh != nil { + entry, ch, err := n.neigh.entry(addr, localAddr, linkRes, onResolve) + return entry.LinkAddr, ch, err + } + + return n.stack.linkAddrCache.get(tcpip.FullAddress{NIC: n.ID(), Addr: addr}, linkRes, localAddr, n, onResolve) +} + func (n *NIC) neighbors() ([]NeighborEntry, *tcpip.Error) { if n.neigh == nil { return nil, tcpip.ErrNotSupported diff --git a/pkg/tcpip/stack/registration.go b/pkg/tcpip/stack/registration.go index 0f6ec92c9..68c113b6a 100644 --- a/pkg/tcpip/stack/registration.go +++ b/pkg/tcpip/stack/registration.go @@ -835,27 +835,6 @@ type LinkAddressCache interface { // AddLinkAddress adds a link address to the cache. AddLinkAddress(nicID tcpip.NICID, addr tcpip.Address, linkAddr tcpip.LinkAddress) - - // GetLinkAddress finds the link address corresponding to the remote address - // (e.g. IP -> MAC). - // - // Returns a link address for the remote address, if readily available. - // - // Returns ErrWouldBlock if the link address is not readily available, along - // with a notification channel for the caller to block on. Triggers address - // resolution asynchronously. - // - // If onResolve is provided, it will be called either immediately, if - // resolution is not required, or when address resolution is complete, with - // the resolved link address and whether resolution succeeded. After any - // callbacks have been called, the returned notification channel is closed. - // - // If specified, the local address must be an address local to the interface - // the neighbor cache belongs to. The local address is the source address of - // a packet prompting NUD/link address resolution. - // - // TODO(gvisor.dev/issue/5151): Don't return the link address. - GetLinkAddress(nicID tcpip.NICID, addr, localAddr tcpip.Address, protocol tcpip.NetworkProtocolNumber, onResolve func(tcpip.LinkAddress, bool)) (tcpip.LinkAddress, <-chan struct{}, *tcpip.Error) } // RawFactory produces endpoints for writing various types of raw packets. diff --git a/pkg/tcpip/stack/route.go b/pkg/tcpip/stack/route.go index ac163904c..8dfde488b 100644 --- a/pkg/tcpip/stack/route.go +++ b/pkg/tcpip/stack/route.go @@ -51,10 +51,6 @@ type Route struct { // outgoingNIC is the interface this route uses to write packets. outgoingNIC *NIC - // linkCache is set if link address resolution is enabled for this protocol on - // the route's NIC. - linkCache LinkAddressCache - // linkRes is set if link address resolution is enabled for this protocol on // the route's NIC. linkRes LinkAddressResolver @@ -191,7 +187,6 @@ func makeRouteInner(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr if r.outgoingNIC.LinkEndpoint.Capabilities()&CapabilityResolutionRequired != 0 { if linkRes, ok := r.outgoingNIC.stack.linkAddrResolvers[r.NetProto]; ok { r.linkRes = linkRes - r.linkCache = r.outgoingNIC.stack } } @@ -338,19 +333,8 @@ func (r *Route) Resolve(afterResolve func()) (<-chan struct{}, *tcpip.Error) { r.Release() } - if neigh := r.outgoingNIC.neigh; neigh != nil { - _, ch, err := neigh.entry(nextAddr, linkAddressResolutionRequestLocalAddr, r.linkRes, finishResolution) - if err != nil { - return ch, err - } - return nil, nil - } - - _, ch, err := r.linkCache.GetLinkAddress(r.outgoingNIC.ID(), nextAddr, linkAddressResolutionRequestLocalAddr, r.NetProto, finishResolution) - if err != nil { - return ch, err - } - return nil, nil + _, ch, err := r.outgoingNIC.getNeighborLinkAddress(nextAddr, linkAddressResolutionRequestLocalAddr, r.linkRes, finishResolution) + return ch, err } // local returns true if the route is a local route. @@ -373,7 +357,7 @@ func (r *Route) isResolutionRequiredRLocked() bool { return false } - return (r.outgoingNIC.neigh != nil && r.linkRes != nil) || r.linkCache != nil + return r.linkRes != nil } func (r *Route) isValidForOutgoing() bool { diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index 114643b03..281bb7a9d 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -1518,19 +1518,41 @@ func (s *Stack) AddLinkAddress(nicID tcpip.NICID, addr tcpip.Address, linkAddr t // that AddLinkAddress for a particular address has been called. } -// GetLinkAddress implements LinkAddressCache.GetLinkAddress. +// GetLinkAddress finds the link address corresponding to the remote address. +// +// Returns a link address for the remote address, if readily available. +// +// Returns ErrNotSupported if the stack is not configured with a link address +// resolver for the specified network protocol. +// +// Returns ErrWouldBlock if the link address is not readily available, along +// with a notification channel for the caller to block on. Triggers address +// resolution asynchronously. +// +// If onResolve is provided, it will be called either immediately, if +// resolution is not required, or when address resolution is complete, with +// the resolved link address and whether resolution succeeded. After any +// callbacks have been called, the returned notification channel is closed. +// +// If specified, the local address must be an address local to the interface +// the neighbor cache belongs to. The local address is the source address of +// a packet prompting NUD/link address resolution. +// +// TODO(gvisor.dev/issue/5151): Don't return the link address. func (s *Stack) GetLinkAddress(nicID tcpip.NICID, addr, localAddr tcpip.Address, protocol tcpip.NetworkProtocolNumber, onResolve func(tcpip.LinkAddress, bool)) (tcpip.LinkAddress, <-chan struct{}, *tcpip.Error) { s.mu.RLock() - nic := s.nics[nicID] - if nic == nil { - s.mu.RUnlock() + nic, ok := s.nics[nicID] + s.mu.RUnlock() + if !ok { return "", nil, tcpip.ErrUnknownNICID } - s.mu.RUnlock() - fullAddr := tcpip.FullAddress{NIC: nicID, Addr: addr} - linkRes := s.linkAddrResolvers[protocol] - return s.linkAddrCache.get(fullAddr, linkRes, localAddr, nic, onResolve) + linkRes, ok := s.linkAddrResolvers[protocol] + if !ok { + return "", nil, tcpip.ErrNotSupported + } + + return nic.getNeighborLinkAddress(addr, localAddr, linkRes, onResolve) } // Neighbors returns all IP to MAC address associations. |