diff options
author | Ghanan Gowripalan <ghanan@google.com> | 2021-01-15 18:47:10 -0800 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-01-15 18:49:22 -0800 |
commit | cd75bb163f46bbe238945b98d50c7b33e60d4490 (patch) | |
tree | 59e62d48d8dc0dd0663c1559087d288a714dcc38 | |
parent | 2814a032be7b34e4cc0c0607dba8030e74e11208 (diff) |
Resolve known link address on route creation
If a Route is being created through a link that requires link address
resolution and a remote address that has a known mapping to a link
address, populate the link address when the route is created.
This removes the need for neighbor/link address caches to perform this
check.
Fixes #5149
PiperOrigin-RevId: 352122401
-rw-r--r-- | pkg/tcpip/stack/linkaddrcache.go | 9 | ||||
-rw-r--r-- | pkg/tcpip/stack/linkaddrcache_test.go | 17 | ||||
-rw-r--r-- | pkg/tcpip/stack/neighbor_cache.go | 14 | ||||
-rw-r--r-- | pkg/tcpip/stack/neighbor_cache_test.go | 28 | ||||
-rw-r--r-- | pkg/tcpip/stack/route.go | 72 | ||||
-rw-r--r-- | pkg/tcpip/stack/stack.go | 3 | ||||
-rw-r--r-- | pkg/tcpip/stack/stack_test.go | 3 |
7 files changed, 49 insertions, 97 deletions
diff --git a/pkg/tcpip/stack/linkaddrcache.go b/pkg/tcpip/stack/linkaddrcache.go index 792f4f170..b600a1cab 100644 --- a/pkg/tcpip/stack/linkaddrcache.go +++ b/pkg/tcpip/stack/linkaddrcache.go @@ -182,15 +182,6 @@ func (c *linkAddrCache) getOrCreateEntryLocked(k tcpip.FullAddress) *linkAddrEnt // get reports any known link address for k. func (c *linkAddrCache) get(k tcpip.FullAddress, linkRes LinkAddressResolver, localAddr tcpip.Address, nic NetworkInterface, onResolve func(tcpip.LinkAddress, bool)) (tcpip.LinkAddress, <-chan struct{}, *tcpip.Error) { - if linkRes != nil { - if addr, ok := linkRes.ResolveStaticAddress(k.Addr); ok { - if onResolve != nil { - onResolve(addr, true) - } - return addr, nil, nil - } - } - c.cache.Lock() defer c.cache.Unlock() entry := c.getOrCreateEntryLocked(k) diff --git a/pkg/tcpip/stack/linkaddrcache_test.go b/pkg/tcpip/stack/linkaddrcache_test.go index 03b2f2d6f..d7ac6cf5f 100644 --- a/pkg/tcpip/stack/linkaddrcache_test.go +++ b/pkg/tcpip/stack/linkaddrcache_test.go @@ -273,20 +273,3 @@ func TestCacheResolutionTimeout(t *testing.T) { t.Errorf("got getBlocking(_, %#v, _) = (%s, %s), want = (_, %s)", e.addr, a, err, tcpip.ErrTimeout) } } - -// TestStaticResolution checks that static link addresses are resolved immediately and don't -// send resolution requests. -func TestStaticResolution(t *testing.T) { - c := newLinkAddrCache(1<<63-1, time.Millisecond, 1) - linkRes := &testLinkAddressResolver{cache: c, delay: time.Minute} - - addr := tcpip.Address("broadcast") - want := tcpip.LinkAddress("mac_broadcast") - got, _, err := c.get(tcpip.FullAddress{Addr: addr}, linkRes, "", nil, nil) - if err != nil { - t.Errorf("c.get(%q)=%q, got error: %v", string(addr), string(got), err) - } - if got != want { - t.Errorf("c.get(%q)=%q, want %q", string(addr), string(got), string(want)) - } -} diff --git a/pkg/tcpip/stack/neighbor_cache.go b/pkg/tcpip/stack/neighbor_cache.go index c15f10e76..acee72572 100644 --- a/pkg/tcpip/stack/neighbor_cache.go +++ b/pkg/tcpip/stack/neighbor_cache.go @@ -127,20 +127,6 @@ func (n *neighborCache) getOrCreateEntry(remoteAddr tcpip.Address, linkRes LinkA // // TODO(gvisor.dev/issue/5151): Don't return the neighbor entry. func (n *neighborCache) entry(remoteAddr, localAddr tcpip.Address, linkRes LinkAddressResolver, onResolve func(tcpip.LinkAddress, bool)) (NeighborEntry, <-chan struct{}, *tcpip.Error) { - // TODO(gvisor.dev/issue/5149): Handle static resolution in route.Resolve. - if linkAddr, ok := linkRes.ResolveStaticAddress(remoteAddr); ok { - e := NeighborEntry{ - Addr: remoteAddr, - LinkAddr: linkAddr, - State: Static, - UpdatedAtNanos: 0, - } - if onResolve != nil { - onResolve(linkAddr, true) - } - return e, nil, nil - } - entry := n.getOrCreateEntry(remoteAddr, linkRes) entry.mu.Lock() defer entry.mu.Unlock() diff --git a/pkg/tcpip/stack/neighbor_cache_test.go b/pkg/tcpip/stack/neighbor_cache_test.go index a2ed6ae2a..b96a56612 100644 --- a/pkg/tcpip/stack/neighbor_cache_test.go +++ b/pkg/tcpip/stack/neighbor_cache_test.go @@ -1750,34 +1750,6 @@ func TestNeighborCacheRetryResolution(t *testing.T) { } } -// TestNeighborCacheStaticResolution checks that static link addresses are -// resolved immediately and don't send resolution requests. -func TestNeighborCacheStaticResolution(t *testing.T) { - config := DefaultNUDConfigurations() - clock := faketime.NewManualClock() - neigh := newTestNeighborCache(nil, config, clock) - store := newTestEntryStore() - linkRes := &testNeighborResolver{ - clock: clock, - neigh: neigh, - entries: store, - delay: typicalLatency, - } - - got, _, err := neigh.entry(testEntryBroadcastAddr, "", linkRes, nil) - if err != nil { - t.Fatalf("unexpected error from neigh.entry(%s, '', _, nil, nil): %s", testEntryBroadcastAddr, err) - } - want := NeighborEntry{ - Addr: testEntryBroadcastAddr, - LinkAddr: testEntryBroadcastLinkAddr, - State: Static, - } - if diff := cmp.Diff(got, want, entryDiffOpts()...); diff != "" { - t.Errorf("neigh.entry(%s, '', _, nil, nil) mismatch (-got, +want):\n%s", testEntryBroadcastAddr, diff) - } -} - func BenchmarkCacheClear(b *testing.B) { b.StopTimer() config := DefaultNUDConfigurations() diff --git a/pkg/tcpip/stack/route.go b/pkg/tcpip/stack/route.go index 8dfde488b..1ff7b3a37 100644 --- a/pkg/tcpip/stack/route.go +++ b/pkg/tcpip/stack/route.go @@ -116,6 +116,7 @@ func constructAndValidateRoute(netProto tcpip.NetworkProtocolNumber, addressEndp r := makeRoute( netProto, + gateway, localAddr, remoteAddr, outgoingNIC, @@ -125,20 +126,12 @@ func constructAndValidateRoute(netProto tcpip.NetworkProtocolNumber, addressEndp multicastLoop, ) - // If the route requires us to send a packet through some gateway, do not - // broadcast it. - if len(gateway) > 0 { - r.NextHop = gateway - } else if subnet := addressEndpoint.Subnet(); subnet.IsBroadcast(remoteAddr) { - r.ResolveWith(header.EthernetBroadcastAddress) - } - return r } // makeRoute initializes a new route. It takes ownership of the provided // AssignableAddressEndpoint. -func makeRoute(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr tcpip.Address, outgoingNIC, localAddressNIC *NIC, localAddressEndpoint AssignableAddressEndpoint, handleLocal, multicastLoop bool) *Route { +func makeRoute(netProto tcpip.NetworkProtocolNumber, gateway, localAddr, remoteAddr tcpip.Address, outgoingNIC, localAddressNIC *NIC, localAddressEndpoint AssignableAddressEndpoint, handleLocal, multicastLoop bool) *Route { if localAddressNIC.stack != outgoingNIC.stack { panic(fmt.Sprintf("cannot create a route with NICs from different stacks")) } @@ -164,7 +157,44 @@ func makeRoute(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr tcpip } } - return makeRouteInner(netProto, localAddr, remoteAddr, outgoingNIC, localAddressNIC, localAddressEndpoint, loop) + r := makeRouteInner(netProto, localAddr, remoteAddr, outgoingNIC, localAddressNIC, localAddressEndpoint, loop) + if r.Loop&PacketOut == 0 { + // Packet will not leave the stack, no need for a gateway or a remote link + // address. + return r + } + + if r.outgoingNIC.LinkEndpoint.Capabilities()&CapabilityResolutionRequired != 0 { + if linkRes, ok := r.outgoingNIC.stack.linkAddrResolvers[r.NetProto]; ok { + r.linkRes = linkRes + } + } + + if len(gateway) > 0 { + r.NextHop = gateway + return r + } + + if r.linkRes == nil { + return r + } + + if linkAddr, ok := r.linkRes.ResolveStaticAddress(r.RemoteAddress); ok { + r.ResolveWith(linkAddr) + return r + } + + if subnet := localAddressEndpoint.Subnet(); subnet.IsBroadcast(remoteAddr) { + r.ResolveWith(header.EthernetBroadcastAddress) + return r + } + + if r.RemoteAddress == r.LocalAddress { + // Local link address is already known. + r.ResolveWith(r.LocalLinkAddress) + } + + return r } func makeRouteInner(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr tcpip.Address, outgoingNIC, localAddressNIC *NIC, localAddressEndpoint AssignableAddressEndpoint, loop PacketLooping) *Route { @@ -184,12 +214,6 @@ func makeRouteInner(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr r.mu.localAddressEndpoint = localAddressEndpoint r.mu.Unlock() - if r.outgoingNIC.LinkEndpoint.Capabilities()&CapabilityResolutionRequired != 0 { - if linkRes, ok := r.outgoingNIC.stack.linkAddrResolvers[r.NetProto]; ok { - r.linkRes = linkRes - } - } - return r } @@ -300,14 +324,13 @@ func (r *Route) Resolve(afterResolve func()) (<-chan struct{}, *tcpip.Error) { return nil, nil } + // Increment the route's reference count because finishResolution retains a + // reference to the route and releases it when called. + r.acquireLocked() + r.mu.Unlock() + nextAddr := r.NextHop if nextAddr == "" { - // Local link address is already known. - if r.RemoteAddress == r.LocalAddress { - r.mu.remoteLinkAddress = r.LocalLinkAddress - r.mu.Unlock() - return nil, nil - } nextAddr = r.RemoteAddress } @@ -318,11 +341,6 @@ func (r *Route) Resolve(afterResolve func()) (<-chan struct{}, *tcpip.Error) { linkAddressResolutionRequestLocalAddr = r.LocalAddress } - // Increment the route's reference count because finishResolution retains a - // reference to the route and releases it when called. - r.acquireLocked() - r.mu.Unlock() - finishResolution := func(linkAddress tcpip.LinkAddress, ok bool) { if ok { r.ResolveWith(linkAddress) diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index 281bb7a9d..6d761b125 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -1329,6 +1329,7 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n if addressEndpoint := s.getAddressEP(nic, localAddr, remoteAddr, netProto); addressEndpoint != nil { return makeRoute( netProto, + "", /* gateway */ localAddr, remoteAddr, nic, /* outboundNIC */ @@ -1518,7 +1519,7 @@ func (s *Stack) AddLinkAddress(nicID tcpip.NICID, addr tcpip.Address, linkAddr t // that AddLinkAddress for a particular address has been called. } -// GetLinkAddress finds the link address corresponding to the remote address. +// GetLinkAddress finds the link address corresponding to a neighbor's address. // // Returns a link address for the remote address, if readily available. // diff --git a/pkg/tcpip/stack/stack_test.go b/pkg/tcpip/stack/stack_test.go index 82ee066e6..7e935ddff 100644 --- a/pkg/tcpip/stack/stack_test.go +++ b/pkg/tcpip/stack/stack_test.go @@ -3572,9 +3572,10 @@ func TestOutgoingSubnetBroadcast(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { s := stack.New(stack.Options{ - NetworkProtocols: []stack.NetworkProtocolFactory{ipv4.NewProtocol, ipv6.NewProtocol}, + NetworkProtocols: []stack.NetworkProtocolFactory{arp.NewProtocol, ipv4.NewProtocol, ipv6.NewProtocol}, }) ep := channel.New(0, defaultMTU, "") + ep.LinkEPCapabilities |= stack.CapabilityResolutionRequired if err := s.CreateNIC(nicID1, ep); err != nil { t.Fatalf("CreateNIC(%d, _): %s", nicID1, err) } |