summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGhanan Gowripalan <ghanan@google.com>2021-01-15 18:47:10 -0800
committergVisor bot <gvisor-bot@google.com>2021-01-15 18:49:22 -0800
commitcd75bb163f46bbe238945b98d50c7b33e60d4490 (patch)
tree59e62d48d8dc0dd0663c1559087d288a714dcc38
parent2814a032be7b34e4cc0c0607dba8030e74e11208 (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.go9
-rw-r--r--pkg/tcpip/stack/linkaddrcache_test.go17
-rw-r--r--pkg/tcpip/stack/neighbor_cache.go14
-rw-r--r--pkg/tcpip/stack/neighbor_cache_test.go28
-rw-r--r--pkg/tcpip/stack/route.go72
-rw-r--r--pkg/tcpip/stack/stack.go3
-rw-r--r--pkg/tcpip/stack/stack_test.go3
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)
}