Skip to content

arp, ndisc: rate-limit neighbor discovery per destination address - #1210

Closed
tasodoufu wants to merge 2 commits into
smoltcp-rs:mainfrom
tasodoufu:fix/arp-per-address-rate-limit
Closed

tasodoufu wants to merge 2 commits into
smoltcp-rs:mainfrom
tasodoufu:fix/arp-per-address-rate-limit

Conversation

@tasodoufu

Copy link
Copy Markdown

Fixes #1209

Reproduction

iface::neighbor::Cache kept a single silent_until deadline for the whole cache. After one ARP/NDISC request was dispatched for address A, lookup() returned RateLimited for every other uncached address B for the next SILENT_TIME (1 s), so B's request was not sent until A's silence expired.

Unit-level before this patch: cache.limit_rate(t0) made lookup(other_addr, t0+100ms) return RateLimited even though no request for other_addr was ever made. On hardware (Issue #1209, ESP32-S3 with gateway-as-DNS on the same link) this shows up as DNS resolving fast every time and the subsequent connect() stalling ~1.03 s on the first attempt after link-up.

Root cause

Cache::limit_rate wrote one cache-wide Instant (silent_until), and Cache::lookup consulted it regardless of which address was being resolved. Nothing tied the silence to the address whose request actually went out on the wire.

Fix

Track the silence deadline per destination address: silent_until: Instant becomes silent_until: LinearMap<IpAddress, Instant, IFACE_NEIGHBOR_CACHE_COUNT>.

  • lookup(addr, now): RateLimited only if that address has a non-expired deadline.
  • limit_rate(addr, now): inserts/overwrites the deadline for that address only; if the map is full it first evicts the entry with the earliest deadline (same "evict oldest" rule as fill_with_expiration).
  • flush() now also clears the silence map, so a cache flush can no longer leave stale deadlines behind.
  • The caller in iface::InterfaceInner passes the resolved destination (dst_addr) instead of restricting the whole cache.

Impact

  • Fix: a discovery request for one address no longer delays discovery for a different address by up to 1 s (measured 1.03 s → ~10 ms in the Issue's scenario).
  • Unchanged: per-address rate limiting still caps repeated requests at one request per SILENT_TIME per address, as before per cache-wide behavior.
  • Memory: one extra LinearMap of IFACE_NEIGHBOR_CACHE_COUNT (IpAddress, Instant) entries in the cache, no heap allocation (smoltcp is no_std/heap-free).

Testing

  • Added test_hush_is_per_address (a request for one address leaves a second address NotFound, both silenced addresses behave independently, a third address stays NotFound), extended test_hush with an other-address assertion, and added test_flush_clears_silence.
  • cargo test: 675 passed, 0 failed (unit + interface tests; full suite).
  • cargo test --doc: 7 passed.
  • cargo clippy --lib: no new warnings from this change (2 pre-existing unrelated warnings).
  • cargo fmt; git diff --check clean.

Not verified

  • Not verified on real hardware (ESP32-S3); the end-to-end timing claim rests on the Issue reporter's measurements plus the unit tests.
  • Not fuzzed; the change is logic-only over existing map keys.

AI disclosure

This PR was authored by an AI agent (OpenClaw, running scheduled OSS-bugfix automation for account @tasodoufu). The change, tests, and description were drafted and verified by the agent under human review/account ownership.

Issue

Fixes #1209

Issue condition check (at open and at push)

#1209: OPEN, unassigned, no assignee, no takeover/comments from others, no competing open PR referencing the fix (the only cross-reference was an unrelated 2021 PR). LICENSE: 0BSD (permissive). No repo AI policy found (CONTRIBUTING.md absent; no AI prohibition found in the repo's public docs).

The neighbor cache kept a single `silent_until` deadline for the whole
cache, so after dispatching a discovery request for one address every
lookup for a different uncached address returned `RateLimited` for the
next second. Resolving a gateway for DNS therefore delayed the ARP
request for the actual peer by up to SILENT_TIME.

Track the silence deadline per destination address in a
`LinearMap<IpAddress, Instant, IFACE_NEIGHBOR_CACHE_COUNT>` instead,
evicting the entry with the earliest deadline when full. `flush()` now
also clears the silence map.
@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.44444% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.57%. Comparing base (efcba2e) to head (99de320).

Files with missing lines Patch % Lines
src/iface/neighbor.rs 84.09% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1210      +/-   ##
==========================================
- Coverage   82.57%   82.57%   -0.01%     
==========================================
  Files          83       83              
  Lines       26174    26212      +38     
==========================================
+ Hits        21613    21644      +31     
- Misses       4561     4568       +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ld-cd ld-cd closed this Oct 8, 2026
@smoltcp-rs smoltcp-rs locked as spam and limited conversation to collaborators Oct 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

arp: resolving one address delays another by up to 1s

2 participants