go
/
gvisor
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Deprecate Failed state in favor of Unreachable state 

... as per RFC 7048. The Failed state is an internal state that is not
specified by any RFC; replacing it with the Unreachable state enables us to
expose this state while keeping our terminology consistent with RFC 4861 and
RFC 7048.

Unreachable state replaces all internal references for Failed state. However
unlike the Failed state, change events are dispatched when moving into
Unreachable state. This gives developers insight into whether a neighbor entry
failed address resolution or whether it was explicitly removed.

The Failed state will be removed entirely once all references to it are
removed. This is done to avoid a Fuchsia roll failure.

Updates #4667

PiperOrigin-RevId: 356554104
This commit is contained in:
Sam Balana 2021-02-09 11:45:01 -08:00 committed by gVisor bot
parent 2b978d8743
commit d0c0549e60
5 changed files with 197 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
pkg/tcpip/stack/neighbor_cache.go
View File

@ -25,9 +25,13 @@ const neighborCacheSize = 512 // max entries per interface
// NeighborStats holds metrics for the neighbor table.
type NeighborStats struct {
// FailedEntryLookups counts the number of lookups performed on an entry in
// Failed state.
// FailedEntryLookups is deprecated; UnreachableEntryLookups should be used
// instead.
FailedEntryLookups *tcpip.StatCounter
// UnreachableEntryLookups counts the number of lookups performed on an
// entry in Unreachable state.
UnreachableEntryLookups *tcpip.StatCounter
}
// neighborCache maps IP addresses to link addresses. It uses the Least
@ -143,7 +147,7 @@ func (n *neighborCache) entry(remoteAddr, localAddr tcpip.Address, onResolve fun
onResolve(LinkResolutionResult{LinkAddress: entry.mu.neigh.LinkAddr, Success: true})
}
return entry.mu.neigh, nil, nil
case Unknown, Incomplete, Failed:
case Unknown, Incomplete, Unreachable:
if onResolve != nil {
entry.mu.onResolve = append(entry.mu.onResolve, onResolve)
}

3
pkg/tcpip/stack/neighbor_cache_test.go
View File

@ -1610,12 +1610,11 @@ func TestNeighborCacheRetryResolution(t *testing.T) {
}
}
// Verify the entry is in Failed state.
wantEntries := []NeighborEntry{
{
Addr: entry.Addr,
LinkAddr: "",
State: Failed,
State: Unreachable,
},
}
if diff := cmp.Diff(linkRes.neigh.entries(), wantEntries, entryDiffOptsWithSort()...); diff != "" {

55
pkg/tcpip/stack/neighbor_entry.go
View File

@ -38,7 +38,8 @@ type NeighborEntry struct {
}
// NeighborState defines the state of a NeighborEntry within the Neighbor
// Unreachability Detection state machine, as per RFC 4861 section 7.3.2.
// Unreachability Detection state machine, as per RFC 4861 section 7.3.2 and
// RFC 7048.
type NeighborState uint8
const (
@ -61,13 +62,24 @@ const (
Delay
// Probe means a reachability confirmation is actively being sought by
// periodically retransmitting reachability probes until a reachability
// confirmation is received, or until the max amount of probes has been sent.
// confirmation is received, or until the maximum number of probes has been
// sent.
Probe
// Static describes entries that have been explicitly added by the user. They
// do not expire and are not deleted until explicitly removed.
Static
// Failed means recent attempts of reachability have returned inconclusive.
// Failed is deprecated and should no longer be used.
//
// TODO(gvisor.dev/issue/4667): Remove this once all references to Failed
// are removed from Fuchsia.
Failed
// Unreachable means reachability confirmation failed; the maximum number of
// reachability probes has been sent and no replies have been received.
//
// TODO(gvisor.dev/issue/5472): Add the following sentence when we implement
// RFC 7048: "Packets continue to be sent to the neighbor while
// re-attempting to resolve the address."
Unreachable
)
type timer struct {
@ -310,8 +322,8 @@ func (e *neighborEntry) setStateLocked(next NeighborState) {
}
if timedoutResolution || err != nil {
e.dispatchRemoveEventLocked()
e.setStateLocked(Failed)
e.setStateLocked(Unreachable)
e.dispatchChangeEventLocked()
return
}
@ -320,7 +332,7 @@ func (e *neighborEntry) setStateLocked(next NeighborState) {
}),
}
case Failed:
case Unreachable:
e.notifyCompletionLocked(false /* succeeded */)
case Unknown, Stale, Static:
@ -339,15 +351,19 @@ func (e *neighborEntry) setStateLocked(next NeighborState) {
// Precondition: e.mu MUST be locked.
func (e *neighborEntry) handlePacketQueuedLocked(localAddr tcpip.Address) {
switch e.mu.neigh.State {
case Failed:
e.cache.nic.stats.Neighbor.FailedEntryLookups.Increment()
fallthrough
case Unknown:
case Unknown, Unreachable:
prev := e.mu.neigh.State
e.mu.neigh.State = Incomplete
e.mu.neigh.UpdatedAtNanos = e.cache.nic.stack.clock.NowNanoseconds()
switch prev {
case Unknown:
e.dispatchAddEventLocked()
case Unreachable:
e.dispatchChangeEventLocked()
e.cache.nic.stats.Neighbor.UnreachableEntryLookups.Increment()
}
config := e.nudState.Config()
// Protected by e.mu.
@ -385,8 +401,8 @@ func (e *neighborEntry) handlePacketQueuedLocked(localAddr tcpip.Address) {
}
if timedoutResolution || err != nil {
e.dispatchRemoveEventLocked()
e.setStateLocked(Failed)
e.setStateLocked(Unreachable)
e.dispatchChangeEventLocked()
return
}
@ -418,7 +434,7 @@ func (e *neighborEntry) handleProbeLocked(remoteLinkAddr tcpip.LinkAddress) {
// checks MUST be done by the NetworkEndpoint.
switch e.mu.neigh.State {
case Unknown, Failed:
case Unknown:
e.mu.neigh.LinkAddr = remoteLinkAddr
e.setStateLocked(Stale)
e.dispatchAddEventLocked()
@ -447,6 +463,13 @@ func (e *neighborEntry) handleProbeLocked(remoteLinkAddr tcpip.LinkAddress) {
e.dispatchChangeEventLocked()
}
case Unreachable:
// TODO(gvisor.dev/issue/5472): Do not change the entry if the link
// address is the same, as per RFC 7048.
e.mu.neigh.LinkAddr = remoteLinkAddr
e.setStateLocked(Stale)
e.dispatchChangeEventLocked()
case Static:
// Do nothing
@ -549,7 +572,7 @@ func (e *neighborEntry) handleConfirmationLocked(linkAddr tcpip.LinkAddress, fla
}
e.mu.isRouter = flags.IsRouter
case Unknown, Failed, Static:
case Unknown, Unreachable, Static:
// Do nothing
default:
@ -571,7 +594,7 @@ func (e *neighborEntry) handleUpperLevelConfirmationLocked() {
e.dispatchChangeEventLocked()
}
case Unknown, Incomplete, Failed, Static:
case Unknown, Incomplete, Unreachable, Static:
// Do nothing
default:

144
pkg/tcpip/stack/neighbor_entry_test.go
View File

@ -70,10 +70,10 @@ func eventDiffOptsWithSort() []cmp.Option {
}
// The following unit tests exercise every state transition and verify its
// behavior with RFC 4681.
// behavior with RFC 4681 and RFC 7048.
//
// | From | To | Cause | Update | Action | Event |
// | ========== | ========== | ========================================== | ======== | ===========| ======= |
// | =========== | =========== | ========================================== | ======== | ===========| ======= |
// | Unknown | Unknown | Confirmation w/ unknown address | | | Added |
// | Unknown | Incomplete | Packet queued to unknown address | | Send probe | Added |
// | Unknown | Stale | Probe | | | Added |
@ -81,7 +81,7 @@ func eventDiffOptsWithSort() []cmp.Option {
// | Incomplete | Reachable | Solicited confirmation | LinkAddr | Notify | Changed |
// | Incomplete | Stale | Unsolicited confirmation | LinkAddr | Notify | Changed |
// | Incomplete | Stale | Probe | LinkAddr | Notify | Changed |
// | Incomplete | Failed | Max probes sent without reply | | Notify | Removed |
// | Incomplete | Unreachable | Max probes sent without reply | | Notify | Changed |
// | Reachable | Reachable | Confirmation w/ different isRouter flag | IsRouter | | |
// | Reachable | Stale | Reachable timer expired | | | Changed |
// | Reachable | Stale | Probe or confirmation w/ different address | | | Changed |
@ -100,8 +100,9 @@ func eventDiffOptsWithSort() []cmp.Option {
// | Probe | Reachable | Solicited confirmation w/o address | | Notify | Changed |
// | Probe | Stale | Probe or confirmation w/ different address | | | Changed |
// | Probe | Probe | Retransmit timer expired | | | Changed |
// | Probe | Failed | Max probes sent without reply | | Notify | Removed |
// | Failed | Incomplete | Packet queued | | Send probe | Added |
// | Probe | Unreachable | Max probes sent without reply | | Notify | Changed |
// | Unreachable | Incomplete | Packet queued | | Send probe | Changed |
// | Unreachable | Stale | Probe w/ different address | LinkAddr | | Changed |
type testEntryEventType uint8
@ -448,7 +449,7 @@ func TestEntryIncompleteToIncompleteDoesNotChangeUpdatedAt(t *testing.T) {
clock.Advance(c.RetransmitTimer)
// UpdatedAt should change after failing address resolution. Timing out after
// sending the last probe transitions the entry to Failed.
// sending the last probe transitions the entry to Unreachable.
{
wantProbes := []entryTestProbeInfo{
{
@ -478,12 +479,12 @@ func TestEntryIncompleteToIncompleteDoesNotChangeUpdatedAt(t *testing.T) {
},
},
{
EventType: entryTestRemoved,
EventType: entryTestChanged,
NICID: entryTestNICID,
Entry: NeighborEntry{
Addr: entryTestAddr1,
LinkAddr: tcpip.LinkAddress(""),
State: Incomplete,
State: Unreachable,
},
},
}
@ -755,7 +756,7 @@ func TestEntryIncompleteToStaleWhenProbe(t *testing.T) {
nudDisp.mu.Unlock()
}
func TestEntryIncompleteToFailed(t *testing.T) {
func TestEntryIncompleteToUnreachable(t *testing.T) {
c := DefaultNUDConfigurations()
c.MaxMulticastProbes = 3
e, nudDisp, linkRes, clock := entryTestSetup(c)
@ -807,12 +808,12 @@ func TestEntryIncompleteToFailed(t *testing.T) {
},
},
{
EventType: entryTestRemoved,
EventType: entryTestChanged,
NICID: entryTestNICID,
Entry: NeighborEntry{
Addr: entryTestAddr1,
LinkAddr: tcpip.LinkAddress(""),
State: Incomplete,
State: Unreachable,
},
},
}
@ -823,8 +824,8 @@ func TestEntryIncompleteToFailed(t *testing.T) {
nudDisp.mu.Unlock()
e.mu.Lock()
if e.mu.neigh.State != Failed {
t.Errorf("got e.mu.neigh.State = %q, want = %q", e.mu.neigh.State, Failed)
if e.mu.neigh.State != Unreachable {
t.Errorf("got e.mu.neigh.State = %q, want = %q", e.mu.neigh.State, Unreachable)
}
e.mu.Unlock()
}
@ -3294,7 +3295,7 @@ func TestEntryProbeToReachableWhenSolicitedConfirmationWithoutAddress(t *testing
nudDisp.mu.Unlock()
}
func TestEntryProbeToFailed(t *testing.T) {
func TestEntryProbeToUnreachable(t *testing.T) {
c := DefaultNUDConfigurations()
c.MaxMulticastProbes = 3
c.MaxUnicastProbes = 3
@ -3355,11 +3356,11 @@ func TestEntryProbeToFailed(t *testing.T) {
e.mu.Unlock()
}
// Wait for the last probe to expire, causing a transition to Failed.
// Wait for the last probe to expire, causing a transition to Unreachable.
clock.Advance(c.RetransmitTimer)
e.mu.Lock()
if e.mu.neigh.State != Failed {
t.Errorf("got e.mu.neigh.State = %q, want = %q", e.mu.neigh.State, Failed)
if e.mu.neigh.State != Unreachable {
t.Errorf("got e.mu.neigh.State = %q, want = %q", e.mu.neigh.State, Unreachable)
}
e.mu.Unlock()
@ -3401,12 +3402,12 @@ func TestEntryProbeToFailed(t *testing.T) {
},
},
{
EventType: entryTestRemoved,
EventType: entryTestChanged,
NICID: entryTestNICID,
Entry: NeighborEntry{
Addr: entryTestAddr1,
LinkAddr: entryTestLinkAddr1,
State: Probe,
State: Unreachable,
},
},
}
@ -3417,7 +3418,7 @@ func TestEntryProbeToFailed(t *testing.T) {
nudDisp.mu.Unlock()
}
func TestEntryFailedToIncomplete(t *testing.T) {
func TestEntryUnreachableToIncomplete(t *testing.T) {
c := DefaultNUDConfigurations()
c.MaxMulticastProbes = 3
e, nudDisp, linkRes, clock := entryTestSetup(c)
@ -3461,8 +3462,8 @@ func TestEntryFailedToIncomplete(t *testing.T) {
}
e.mu.Lock()
if e.mu.neigh.State != Failed {
t.Errorf("got e.mu.neigh.State = %q, want = %q", e.mu.neigh.State, Failed)
if e.mu.neigh.State != Unreachable {
t.Errorf("got e.mu.neigh.State = %q, want = %q", e.mu.neigh.State, Unreachable)
}
e.mu.Unlock()
@ -3484,16 +3485,16 @@ func TestEntryFailedToIncomplete(t *testing.T) {
},
},
{
EventType: entryTestRemoved,
EventType: entryTestChanged,
NICID: entryTestNICID,
Entry: NeighborEntry{
Addr: entryTestAddr1,
LinkAddr: tcpip.LinkAddress(""),
State: Incomplete,
State: Unreachable,
},
},
{
EventType: entryTestAdded,
EventType: entryTestChanged,
NICID: entryTestNICID,
Entry: NeighborEntry{
Addr: entryTestAddr1,
@ -3508,3 +3509,96 @@ func TestEntryFailedToIncomplete(t *testing.T) {
}
nudDisp.mu.Unlock()
}
func TestEntryUnreachableToStale(t *testing.T) {
wantProbes := []entryTestProbeInfo{
// The Incomplete-to-Incomplete state transition is tested here by
// verifying that 3 reachability probes were sent.
{
RemoteAddress: entryTestAddr1,
RemoteLinkAddress: tcpip.LinkAddress(""),
LocalAddress: entryTestAddr2,
},
{
RemoteAddress: entryTestAddr1,
RemoteLinkAddress: tcpip.LinkAddress(""),
LocalAddress: entryTestAddr2,
},
{
RemoteAddress: entryTestAddr1,
RemoteLinkAddress: tcpip.LinkAddress(""),
LocalAddress: entryTestAddr2,
},
}
c := DefaultNUDConfigurations()
c.MaxMulticastProbes = uint32(len(wantProbes))
e, nudDisp, linkRes, clock := entryTestSetup(c)
// TODO(gvisor.dev/issue/4872): Use helper functions to start entry tests in
// their expected state.
e.mu.Lock()
e.handlePacketQueuedLocked(entryTestAddr2)
if e.mu.neigh.State != Incomplete {
t.Errorf("got e.mu.neigh.State = %q, want = %q", e.mu.neigh.State, Incomplete)
}
e.mu.Unlock()
waitFor := c.RetransmitTimer * time.Duration(c.MaxMulticastProbes)
clock.Advance(waitFor)
linkRes.mu.Lock()
diff := cmp.Diff(wantProbes, linkRes.probes)
linkRes.mu.Unlock()
if diff != "" {
t.Fatalf("link address resolver probes mismatch (-want, +got):\n%s", diff)
}
e.mu.Lock()
if e.mu.neigh.State != Unreachable {
t.Errorf("got e.mu.neigh.State = %q, want = %q", e.mu.neigh.State, Unreachable)
}
e.mu.Unlock()
e.mu.Lock()
e.handleProbeLocked(entryTestLinkAddr2)
if e.mu.neigh.State != Stale {
t.Errorf("got e.mu.neigh.State = %q, want = %q", e.mu.neigh.State, Stale)
}
e.mu.Unlock()
wantEvents := []testEntryEventInfo{
{
EventType: entryTestAdded,
NICID: entryTestNICID,
Entry: NeighborEntry{
Addr: entryTestAddr1,
LinkAddr: tcpip.LinkAddress(""),
State: Incomplete,
},
},
{
EventType: entryTestChanged,
NICID: entryTestNICID,
Entry: NeighborEntry{
Addr: entryTestAddr1,
LinkAddr: tcpip.LinkAddress(""),
State: Unreachable,
},
},
{
EventType: entryTestChanged,
NICID: entryTestNICID,
Entry: NeighborEntry{
Addr: entryTestAddr1,
LinkAddr: entryTestLinkAddr2,
State: Stale,
},
},
}
nudDisp.mu.Lock()
if diff := cmp.Diff(wantEvents, nudDisp.events, eventDiffOpts()...); diff != "" {
t.Errorf("nud dispatcher events mismatch (-want, +got):\n%s", diff)
}
nudDisp.mu.Unlock()
}

7
pkg/tcpip/stack/neighborstate_string.go
View File

@ -1,4 +1,4 @@
// Copyright 2020 The gVisor Authors.
// Copyright 2021 The gVisor Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@ -30,11 +30,12 @@ func _() {
_ = x[Probe-5]
_ = x[Static-6]
_ = x[Failed-7]
_ = x[Unreachable-8]
}
const _NeighborState_name = "UnknownIncompleteReachableStaleDelayProbeStaticFailed"
const _NeighborState_name = "UnknownIncompleteReachableStaleDelayProbeStaticFailedUnreachable"
var _NeighborState_index = [...]uint8{0, 7, 17, 26, 31, 36, 41, 47, 53}
var _NeighborState_index = [...]uint8{0, 7, 17, 26, 31, 36, 41, 47, 53, 64}
func (i NeighborState) String() string {
if i >= NeighborState(len(_NeighborState_index)-1) {