Merge pull request #14242 from tamird:checklocks-neighbors

PiperOrigin-RevId: 968757123
This commit is contained in:
gVisor bot
2026-08-21 16:29:30 -07:00
3 changed files with 44 additions and 19 deletions
+3 -1
View File
@@ -45,7 +45,9 @@ type dynamicCacheEntry struct {
type neighborCacheMu struct {
neighborCacheRWMutex `state:"nosave"`
cache map[tcpip.Address]*neighborEntry
// +checklocks:neighborCacheRWMutex
cache map[tcpip.Address]*neighborEntry
// +checklocks:neighborCacheRWMutex
dynamic dynamicCacheEntry
}
+25 -7
View File
@@ -879,7 +879,10 @@ func TestNeighborCacheAddStaticEntryThenOverflow(t *testing.T) {
State: Static,
UpdatedAt: c.clock.NowMonotonic(),
}
if diff := cmp.Diff(want, e.mu.neigh, cmp.AllowUnexported(tcpip.MonotonicTime{})); diff != "" {
e.mu.RLock()
gotNeighbor := e.mu.neigh
e.mu.RUnlock()
if diff := cmp.Diff(want, gotNeighbor, cmp.AllowUnexported(tcpip.MonotonicTime{})); diff != "" {
t.Errorf("c.linkRes.neigh.entry(%s, \"\", nil) mismatch (-want, +got):\n%s", entry.Addr, diff)
}
@@ -1248,7 +1251,10 @@ func TestNeighborCacheReplace(t *testing.T) {
State: Delay,
UpdatedAt: clock.NowMonotonic(),
}
if diff := cmp.Diff(want, e.mu.neigh, cmp.AllowUnexported(tcpip.MonotonicTime{})); diff != "" {
e.mu.RLock()
gotNeighbor := e.mu.neigh
e.mu.RUnlock()
if diff := cmp.Diff(want, gotNeighbor, cmp.AllowUnexported(tcpip.MonotonicTime{})); diff != "" {
t.Errorf("linkRes.neigh.entry(%s, '', nil) mismatch (-want, +got):\n%s", entry.Addr, diff)
}
}
@@ -1267,7 +1273,10 @@ func TestNeighborCacheReplace(t *testing.T) {
State: Reachable,
UpdatedAt: clock.NowMonotonic(),
}
if diff := cmp.Diff(want, e.mu.neigh, cmp.AllowUnexported(tcpip.MonotonicTime{})); diff != "" {
e.mu.RLock()
gotNeighbor := e.mu.neigh
e.mu.RUnlock()
if diff := cmp.Diff(want, gotNeighbor, cmp.AllowUnexported(tcpip.MonotonicTime{})); diff != "" {
t.Errorf("linkRes.neigh.entry(%s, '', nil) mismatch (-want, +got):\n%s", entry.Addr, diff)
}
}
@@ -1305,7 +1314,10 @@ func TestNeighborCacheResolutionFailed(t *testing.T) {
State: Reachable,
UpdatedAt: clock.NowMonotonic(),
}
if diff := cmp.Diff(want, got.mu.neigh, cmp.AllowUnexported(tcpip.MonotonicTime{})); diff != "" {
got.mu.RLock()
gotNeighbor := got.mu.neigh
got.mu.RUnlock()
if diff := cmp.Diff(want, gotNeighbor, cmp.AllowUnexported(tcpip.MonotonicTime{})); diff != "" {
t.Errorf("linkRes.neigh.entry(%s, '', nil) mismatch (-want, +got):\n%s", entry.Addr, diff)
}
@@ -1477,8 +1489,11 @@ func TestNeighborCacheRetryResolution(t *testing.T) {
if _, ok := err.(*tcpip.ErrWouldBlock); !ok {
t.Fatalf("got linkRes.neigh.entry(%s, '', _) = %v, want = %s", entry.Addr, err, &tcpip.ErrWouldBlock{})
}
if incompleteEntry.mu.neigh.State != Incomplete {
t.Fatalf("got entry.State = %s, want = %s", incompleteEntry.mu.neigh.State, Incomplete)
incompleteEntry.mu.RLock()
gotState := incompleteEntry.mu.neigh.State
incompleteEntry.mu.RUnlock()
if gotState != Incomplete {
t.Fatalf("got entry.State = %s, want = %s", gotState, Incomplete)
}
{
@@ -1545,7 +1560,10 @@ func TestNeighborCacheRetryResolution(t *testing.T) {
State: Reachable,
UpdatedAt: clock.NowMonotonic(),
}
if diff := cmp.Diff(gotEntry.mu.neigh, wantEntry, cmp.AllowUnexported(tcpip.MonotonicTime{})); diff != "" {
gotEntry.mu.RLock()
gotNeighbor := gotEntry.mu.neigh
gotEntry.mu.RUnlock()
if diff := cmp.Diff(gotNeighbor, wantEntry, cmp.AllowUnexported(tcpip.MonotonicTime{})); diff != "" {
t.Fatalf("neighbor entry mismatch (-got, +want):\n%s", diff)
}
}
+16 -11
View File
@@ -90,17 +90,22 @@ type timer struct {
type neighborEntryMu struct {
neighborEntryRWMutex `state:"nosave"`
// +checklocks:neighborEntryRWMutex
neigh NeighborEntry
// done is closed when address resolution is complete. It is nil iff s is
// incomplete and resolution is not yet in progress.
// +checklocks:neighborEntryRWMutex
done chan struct{} `state:"nosave"`
// onResolve is called with the result of address resolution.
// +checklocks:neighborEntryRWMutex
onResolve []func(LinkResolutionResult) `state:"nosave"`
// +checklocks:neighborEntryRWMutex
isRouter bool
// +checklocks:neighborEntryRWMutex
timer timer
}
@@ -163,7 +168,7 @@ func newStaticNeighborEntry(cache *neighborCache, addr tcpip.Address, linkAddr t
// notifyCompletionLocked notifies those waiting for address resolution, with
// the link address if resolution completed successfully.
//
// Precondition: e.mu MUST be locked.
// +checklocks:e.mu.neighborEntryRWMutex
func (e *neighborEntry) notifyCompletionLocked(err tcpip.Error) {
res := LinkResolutionResult{LinkAddress: e.mu.neigh.LinkAddr, Err: err}
for _, callback := range e.mu.onResolve {
@@ -185,7 +190,7 @@ func (e *neighborEntry) notifyCompletionLocked(err tcpip.Error) {
// keyword but allows tests that use manual clocks to deterministically
// wait for this work to complete.
e.cache.nic.stack.clock.AfterFunc(0, func() {
e.cache.nic.linkResQueue.dequeue(ch, e.mu.neigh.LinkAddr, err)
e.cache.nic.linkResQueue.dequeue(ch, res.LinkAddress, err)
})
}
}
@@ -193,7 +198,7 @@ func (e *neighborEntry) notifyCompletionLocked(err tcpip.Error) {
// dispatchAddEventLocked signals to stack's NUD Dispatcher that the entry has
// been added.
//
// Precondition: e.mu MUST be locked.
// +checklocks:e.mu.neighborEntryRWMutex
func (e *neighborEntry) dispatchAddEventLocked() {
if nudDisp := e.cache.nic.stack.nudDisp; nudDisp != nil {
nudDisp.OnNeighborAdded(e.cache.nic.id, e.mu.neigh)
@@ -203,7 +208,7 @@ func (e *neighborEntry) dispatchAddEventLocked() {
// dispatchChangeEventLocked signals to stack's NUD Dispatcher that the entry
// has changed state or link-layer address.
//
// Precondition: e.mu MUST be locked.
// +checklocks:e.mu.neighborEntryRWMutex
func (e *neighborEntry) dispatchChangeEventLocked() {
if nudDisp := e.cache.nic.stack.nudDisp; nudDisp != nil {
nudDisp.OnNeighborChanged(e.cache.nic.id, e.mu.neigh)
@@ -213,7 +218,7 @@ func (e *neighborEntry) dispatchChangeEventLocked() {
// dispatchRemoveEventLocked signals to stack's NUD Dispatcher that the entry
// has been removed.
//
// Precondition: e.mu MUST be locked.
// +checklocks:e.mu.neighborEntryRWMutex
func (e *neighborEntry) dispatchRemoveEventLocked() {
if nudDisp := e.cache.nic.stack.nudDisp; nudDisp != nil {
nudDisp.OnNeighborRemoved(e.cache.nic.id, e.mu.neigh)
@@ -223,7 +228,7 @@ func (e *neighborEntry) dispatchRemoveEventLocked() {
// cancelTimerLocked cancels the currently scheduled action, if there is one.
// Entries in Unknown, Stale, or Static state do not have a scheduled action.
//
// Precondition: e.mu MUST be locked.
// +checklocks:e.mu.neighborEntryRWMutex
func (e *neighborEntry) cancelTimerLocked() {
if e.mu.timer.timer != nil {
e.mu.timer.timer.Stop()
@@ -235,7 +240,7 @@ func (e *neighborEntry) cancelTimerLocked() {
// removeLocked prepares the entry for removal.
//
// Precondition: e.mu MUST be locked.
// +checklocks:e.mu.neighborEntryRWMutex
func (e *neighborEntry) removeLocked() {
e.mu.neigh.UpdatedAt = e.cache.nic.stack.clock.NowMonotonic()
e.dispatchRemoveEventLocked()
@@ -255,7 +260,7 @@ func (e *neighborEntry) removeLocked() {
//
// Follows the logic defined in RFC 4861 section 7.3.3.
//
// Precondition: e.mu MUST be locked.
// +checklocks:e.mu.neighborEntryRWMutex
func (e *neighborEntry) setStateLocked(next NeighborState) {
e.cancelTimerLocked()
@@ -363,7 +368,7 @@ func (e *neighborEntry) setStateLocked(next NeighborState) {
//
// Follows the logic defined in RFC 4861 section 7.3.3.
//
// Precondition: e.mu MUST be locked.
// +checklocks:e.mu.neighborEntryRWMutex
func (e *neighborEntry) handlePacketQueuedLocked(localAddr tcpip.Address) {
switch e.mu.neigh.State {
case Unknown, Unreachable:
@@ -442,7 +447,7 @@ func (e *neighborEntry) handlePacketQueuedLocked(localAddr tcpip.Address) {
//
// Follows the logic defined in RFC 4861 section 7.2.3.
//
// Precondition: e.mu MUST be locked.
// +checklocks:e.mu.neighborEntryRWMutex
func (e *neighborEntry) handleProbeLocked(remoteLinkAddr tcpip.LinkAddress) {
// Probes MUST be silently discarded if the target address is tentative, does
// not exist, or not bound to the NIC as per RFC 4861 section 7.2.3. These
@@ -505,7 +510,7 @@ func (e *neighborEntry) handleProbeLocked(remoteLinkAddr tcpip.LinkAddress) {
// Generated Addresses (CGA), as defined in RFC 3972. This ensures that the
// claimed source of an NDP message is the owner of the claimed address.
//
// Precondition: e.mu MUST be locked.
// +checklocks:e.mu.neighborEntryRWMutex
func (e *neighborEntry) handleConfirmationLocked(linkAddr tcpip.LinkAddress, flags ReachabilityConfirmationFlags) {
switch e.mu.neigh.State {
case Incomplete: