Skip to content

fix: clear NAT / IPv6 firewall tables through the hash delete path on reconfig - #233

Merged
w180112 merged 3 commits into
masterfrom
fix/nat-reset-wait-grace-period
Oct 24, 2026
Merged

w180112 merged 3 commits into
masterfrom
fix/nat-reset-wait-grace-period

Conversation

@w180112

@w180112 w180112 commented Oct 24, 2026

Copy link
Copy Markdown
Member

Summary

  • Replace rte_hash_reset / rte_ring_reset with per-entry rte_hash_del_key for clearing the NAT and IPv6 firewall tables on reconfig. rte_hash_reset / rte_ring_reset are not thread-safe against a concurrent GC reader; the previous code only got away with it because the caller happened to mark entries FREE, drain the defer queue, and stop feeding new packets first, with enough timing slack in between. The delete path walks the same per-entry route as GC, so it is safe to run concurrently with GC by construction, not by timing. The gate is still closed first, followed by one RCU grace period before the slots are freed — this rte_rcu_qsbr_synchronize call is defensive. On the measured call path, the data plane has already stopped feeding this subscriber for >=1.5 ms (processing one packet takes about 1.6 us) before the table clear even starts, so there is no reachable race on today's call paths. Please do not read this as "fixes a race that happens today."
  • Split the one-time table initialization into *_init_entries and *_enqueue_all_entries_to_ring, and remove the unused FILLING state.
  • Add unit tests for the new delete-based clearing functions; no existing test cases were modified.

What the measurements actually show

  • The reachable defect on current master: clearing used rte_hash_reset / rte_ring_reset, which makes the ring's "used" counter jump to MAX and then refill over ~3-5 ms. During that refill window, /metrics can report up to ~260k NAT entries / tens of thousands of IPv6 firewall entries for the affected subscriber even though no such entries exist. Dense on-node sampling: master showed this false reading in 60/60 reconfig rounds across three independent runs; this branch showed it in 0/60 rounds.
  • There is no evidence of in-flight-packet-vs-clear or GC-vs-clear races on any call path reachable today (dial-triggered reconfig, subscriber-count decrease, etc.) — this was confirmed both by code review of every caller of the table-clear routines and by targeted timing instrumentation on a master build. The grace period in this PR exists to keep that guarantee if a future caller clears the table closer to the gate close, not because such a caller exists now.

e2e additions

  • New Step 155b (phase36, RSS) and Step 190b (phase41, software distributor): drop and re-add a subscriber under continuous new-flow load while sampling NAT / IPv6 firewall usage on the node at ~1 ms resolution, and assert that no sample ever exceeds the pre-drop high-water mark. Verdicts are lib predicates (e2e_reconfig_race_verdict, e2e_nat_entries_unique, e2e_reconfig_race_node_verdict) with fixtures, and are exercised by dedicated drills (reconfig_race_trigger_skipped, reconfig_race_sampler_sparse) to confirm the checks actually fail when they should.
  • A new guard (e2e_test/phases/bras_pool_lib.sh) that fails loudly, with the offending log line quoted, whenever the lab BRAS (dpdk-bras) reports "IP pool exhausted". Once the pool is exhausted, every subsequent PPPoE dial in the run is rejected for a reason that has nothing to do with the node under test, which previously made unrelated failures look like node bugs.

Dependency on an out-of-repo fix (bench only, not part of this PR)

Full e2e runs depend on a fix to the lab BRAS (dpdk-bras) that makes its IP address and IPv6 PD allocation recycle properly instead of binding addresses to monotonically increasing session IDs. That fix lives in the separate dpdk-bras repo and has already been built and deployed to the bench BRAS (10.215); it is not part of this PR. Without it, a full e2e run exhausts the address pool during the VALIDATE phase.

Test results

  • make test: 2458 tests passed.
  • make all: builds clean.
  • Full e2e: 208/208 passed. Local validation: 248/248. VALIDATE: 29/29 caught, 0 polluted.

🤖 Generated with Claude Code

w180112 and others added 3 commits October 23, 2026 08:55
Subscriber re-initialization (ppp_init_config_by_user) flushed the
per-subscriber NAT and IPv6 firewall tables (rte_hash_reset /
rte_ring_reset) while data lcores could still be inserting flows
(packets already past the gate) or running the amortized GC, which
ignores the gate and deletes keys / reclaims slots. DPDK requires no
concurrent users during a reset of these structures.

Close the data-plane gates, wait one ppp_ccb_rcu grace period so
in-flight packets finish, mark every entry free, wait a second grace
period so in-flight GC scans finish, then reset the tables.

The per-subscriber insert spinlocks and the six PPP timers are now
initialized once in pppd_construct_ccb_elements; re-initialization no
longer overwrites a lock a data lcore may hold, and stops timers with
rte_timer_stop (which unlinks a pending timer) instead of
rte_timer_init (which would leave it linked).

Comments on nat_table_reset / ipv6_firewall_table_reset now state the
runtime precondition.

Tests: test_ppp_init_config_by_user asserts two grace periods are
started, held insert locks stay locked, and a pending PPP timer is
unlinked from the timer list.

Verified: make test 2413/2413, make all, full e2e 198/198 with 25/25
validation drills.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… reconfig

Replace rte_hash_reset / rte_ring_reset (not safe against concurrent data
lcores) with per-entry rte_hash_del_key, the same path the GC uses, so one RCU
grace period after closing the gates is enough. Split table init into
*_init_entries + *_enqueue_all_entries_to_ring, drop the unused FILLING state,
and add unit tests for the delete-all helpers.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ss pool

Add Step 155b (RSS) and Step 190b (software distributor): drop and re-add a
subscriber under new-flow load while sampling the NAT / IPv6 firewall usage
every millisecond on the node. The reset-based clearing let the usage read up
to ~260k entries for a few milliseconds while the free ring was refilled; the
delete path never shows a value above the pre-reconfig fill. The sampler is
positive-controlled and every verdict is a lib predicate with fixtures and
drills.

Also fail loudly when dpdk-bras reports "IP pool exhausted": from that point
every PPPoE dial is turned away for reasons unrelated to the node.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@w180112
w180112 merged commit 61f0f8b into master Oct 24, 2026
9 checks passed
@w180112
w180112 deleted the fix/nat-reset-wait-grace-period branch October 24, 2026 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant