diff --git a/pkg/tcpip/stack/neighbor_cache.go b/pkg/tcpip/stack/neighbor_cache.go index b0468d8d2..be5c66bfe 100644 --- a/pkg/tcpip/stack/neighbor_cache.go +++ b/pkg/tcpip/stack/neighbor_cache.go @@ -294,19 +294,6 @@ func (n *neighborCache) handleConfirmation(addr tcpip.Address, linkAddr tcpip.Li } } -// handleUpperLevelConfirmation processes a confirmation of reachablity from -// some protocol that operates at a layer above the IP/link layer. -func (n *neighborCache) handleUpperLevelConfirmation(addr tcpip.Address) { - n.mu.RLock() - entry, ok := n.mu.cache[addr] - n.mu.RUnlock() - if ok { - entry.mu.Lock() - entry.handleUpperLevelConfirmationLocked() - entry.mu.Unlock() - } -} - func (n *neighborCache) init(nic *nic, r LinkAddressResolver) { *n = neighborCache{ nic: nic, diff --git a/pkg/tcpip/stack/neighbor_entry.go b/pkg/tcpip/stack/neighbor_entry.go index da7837af9..63eb19646 100644 --- a/pkg/tcpip/stack/neighbor_entry.go +++ b/pkg/tcpip/stack/neighbor_entry.go @@ -589,11 +589,12 @@ func (e *neighborEntry) handleConfirmationLocked(linkAddr tcpip.LinkAddress, fla } } -// handleUpperLevelConfirmationLocked processes an incoming upper-level protocol +// handleUpperLevelConfirmation processes an incoming upper-level protocol // (e.g. TCP acknowledgements) reachability confirmation. -// -// Precondition: e.mu MUST be locked. -func (e *neighborEntry) handleUpperLevelConfirmationLocked() { +func (e *neighborEntry) handleUpperLevelConfirmation() { + e.mu.Lock() + defer e.mu.Unlock() + switch e.mu.neigh.State { case Stale, Delay, Probe: e.setStateLocked(Reachable) diff --git a/pkg/tcpip/stack/neighbor_entry_test.go b/pkg/tcpip/stack/neighbor_entry_test.go index 583a6e8fa..12eb8ae6d 100644 --- a/pkg/tcpip/stack/neighbor_entry_test.go +++ b/pkg/tcpip/stack/neighbor_entry_test.go @@ -1415,12 +1415,12 @@ func TestEntryDelayToReachableWhenUpperLevelConfirmation(t *testing.T) { t.Fatalf("staleToDelay(...) = %s", err) } - e.mu.Lock() - e.handleUpperLevelConfirmationLocked() + e.handleUpperLevelConfirmation() + e.mu.RLock() if e.mu.neigh.State != Reachable { t.Errorf("got e.mu.neigh.State = %q, want = %q", e.mu.neigh.State, Reachable) } - e.mu.Unlock() + e.mu.RUnlock() // No probes should have been sent. runImmediatelyScheduledJobs(clock) diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go index d7258dd95..9dce040a9 100644 --- a/pkg/tcpip/stack/nic.go +++ b/pkg/tcpip/stack/nic.go @@ -30,10 +30,6 @@ type linkResolver struct { neigh neighborCache } -func (l *linkResolver) confirmReachable(addr tcpip.Address) { - l.neigh.handleUpperLevelConfirmation(addr) -} - var _ NetworkInterface = (*nic)(nil) var _ NetworkDispatcher = (*nic)(nil) diff --git a/pkg/tcpip/stack/route.go b/pkg/tcpip/stack/route.go index 8d77ab078..cd97df596 100644 --- a/pkg/tcpip/stack/route.go +++ b/pkg/tcpip/stack/route.go @@ -581,7 +581,7 @@ func (r *Route) IsOutboundBroadcast() bool { // "Reachable" is defined as having full-duplex communication between the // local and remote ends of the route. func (r *Route) ConfirmReachable() { - if r.linkRes != nil { - r.linkRes.confirmReachable(r.nextHop()) + if entry := r.getCachedNeighborEntry(); entry != nil { + entry.handleUpperLevelConfirmation() } } diff --git a/pkg/tcpip/tests/integration/link_resolution_test.go b/pkg/tcpip/tests/integration/link_resolution_test.go index 8fa0cc3da..7b2663c59 100644 --- a/pkg/tcpip/tests/integration/link_resolution_test.go +++ b/pkg/tcpip/tests/integration/link_resolution_test.go @@ -1391,8 +1391,20 @@ func TestTCPConfirmNeighborReachability(t *testing.T) { if err := listenerEP.Bind(listenerAddr); err != nil { t.Fatalf("listenerEP.Bind(%#v): %s", listenerAddr, err) } - if err := listenerEP.Listen(1); err != nil { - t.Fatalf("listenerEP.Listen(1): %s", err) + // A backlog of 1 results in SYN cookies being used for all passive + // connections to make sure the only spot in the accept queue is not + // taken by a connection that never completes the handshake. We use + // a backlog of 2 to make sure SYN cookies are not used. + // + // We avoid SYN cookies to make sure that an accepted endpoint is able + // to confirm the neighbor's reachability through the cached neighbor + // entry in the endpoint's route. When SYN cookies are used, the + // accepted endpoint is constructed when the handshake has already been + // established and such an endpoint's route will not have a cached + // neighbor entry as it was not used to send any of packets for the + // handshake. + if err := listenerEP.Listen(2); err != nil { + t.Fatalf("listenerEP.Listen(2): %s", err) } { err := clientEP.Connect(listenerAddr)