Support GetLinkAddress with neighborCache

Test: integration_test.TestGetLinkAddress
PiperOrigin-RevId: 352119404
This commit is contained in:
Ghanan Gowripalan 2021-01-15 18:12:50 -08:00 committed by gVisor bot
parent fd5b52c87f
commit 2814a032be
7 changed files with 140 additions and 49 deletions

View File

@ -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()),
}))

View File

@ -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

View File

@ -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.

View File

@ -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 {

View File

@ -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.

View File

@ -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)
}
}

View File

@ -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)
}
}
})
}
})
}
}