summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--pkg/tcpip/link/pipe/pipe.go17
-rw-r--r--pkg/tcpip/stack/nic.go9
-rw-r--r--pkg/tcpip/stack/registration.go21
-rw-r--r--pkg/tcpip/stack/route.go22
-rw-r--r--pkg/tcpip/stack/stack.go38
-rw-r--r--pkg/tcpip/stack/stack_test.go21
-rw-r--r--pkg/tcpip/tests/integration/link_resolution_test.go61
7 files changed, 140 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.
diff --git a/pkg/tcpip/stack/stack_test.go b/pkg/tcpip/stack/stack_test.go
index 4a3f937e3..82ee066e6 100644
--- a/pkg/tcpip/stack/stack_test.go
+++ b/pkg/tcpip/stack/stack_test.go
@@ -4357,3 +4357,24 @@ func TestClearNeighborCacheOnNICDisable(t *testing.T) {
t.Fatalf("got len(neighbors) = %d, want = 0; neighbors = %#v", len(neighbors), neighbors)
}
}
+
+func TestGetLinkAddressErrors(t *testing.T) {
+ const (
+ nicID = 1
+ unknownNICID = nicID + 1
+ )
+
+ s := stack.New(stack.Options{
+ NetworkProtocols: []stack.NetworkProtocolFactory{ipv4.NewProtocol},
+ })
+ if err := s.CreateNIC(nicID, channel.New(0, 0, "")); err != nil {
+ t.Fatalf("CreateNIC(%d, _) = %s", nicID, err)
+ }
+
+ if addr, _, err := s.GetLinkAddress(unknownNICID, "", "", ipv4.ProtocolNumber, nil); err != tcpip.ErrUnknownNICID {
+ t.Errorf("got s.GetLinkAddress(%d, '', '', %d, nil) = (%s, _, %s), want = (_, _, %s)", unknownNICID, ipv4.ProtocolNumber, addr, err, tcpip.ErrUnknownNICID)
+ }
+ if addr, _, err := s.GetLinkAddress(nicID, "", "", ipv4.ProtocolNumber, nil); err != tcpip.ErrNotSupported {
+ t.Errorf("got s.GetLinkAddress(%d, '', '', %d, nil) = (%s, _, %s), want = (_, _, %s)", unknownNICID, ipv4.ProtocolNumber, addr, err, tcpip.ErrNotSupported)
+ }
+}
diff --git a/pkg/tcpip/tests/integration/link_resolution_test.go b/pkg/tcpip/tests/integration/link_resolution_test.go
index 3f06c2145..af32d3009 100644
--- a/pkg/tcpip/tests/integration/link_resolution_test.go
+++ b/pkg/tcpip/tests/integration/link_resolution_test.go
@@ -16,6 +16,7 @@ package integration_test
import (
"bytes"
+ "fmt"
"net"
"testing"
@@ -395,3 +396,63 @@ func TestTCPLinkResolutionFailure(t *testing.T) {
})
}
}
+
+func TestGetLinkAddress(t *testing.T) {
+ const (
+ host1NICID = 1
+ host2NICID = 4
+ )
+
+ tests := []struct {
+ name string
+ netProto tcpip.NetworkProtocolNumber
+ remoteAddr tcpip.Address
+ expectedLinkAddr bool
+ }{
+ {
+ name: "IPv4",
+ netProto: ipv4.ProtocolNumber,
+ remoteAddr: ipv4Addr2.AddressWithPrefix.Address,
+ },
+ {
+ name: "IPv6",
+ netProto: ipv6.ProtocolNumber,
+ remoteAddr: ipv6Addr2.AddressWithPrefix.Address,
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ for _, useNeighborCache := range []bool{true, false} {
+ t.Run(fmt.Sprintf("UseNeighborCache=%t", useNeighborCache), func(t *testing.T) {
+ stackOpts := stack.Options{
+ NetworkProtocols: []stack.NetworkProtocolFactory{arp.NewProtocol, ipv4.NewProtocol, ipv6.NewProtocol},
+ UseNeighborCache: useNeighborCache,
+ }
+
+ host1Stack, _ := setupStack(t, stackOpts, host1NICID, host2NICID)
+
+ for i := 0; i < 2; i++ {
+ addr, ch, err := host1Stack.GetLinkAddress(host1NICID, test.remoteAddr, "", test.netProto, func(tcpip.LinkAddress, bool) {})
+ var want *tcpip.Error
+ if i == 0 {
+ want = tcpip.ErrWouldBlock
+ }
+ if err != want {
+ t.Fatalf("got host1Stack.GetLinkAddress(%d, %s, '', %d, _) = (%s, _, %s), want = (_, _, %s)", host1NICID, test.remoteAddr, test.netProto, addr, err, want)
+ }
+
+ if i == 0 {
+ <-ch
+ continue
+ }
+
+ if addr != linkAddr2 {
+ t.Fatalf("got addr = %s, want = %s", addr, linkAddr2)
+ }
+ }
+ })
+ }
+ })
+ }
+}
generated by cgit v1.2.3 (git 2.39.1) at 2024-11-01 18:23:34 +0000