Skip to content

CENNSO-3792: Dynamic per-server port pool: structurally safe RADIUS id reuse - #249

Merged
RoadRunnr merged 11 commits into
feature/next-majorfrom
feature/client-dynamic-port-pool
Jun 5, 2026
Merged

CENNSO-3792: Dynamic per-server port pool: structurally safe RADIUS id reuse#249
RoadRunnr merged 11 commits into
feature/next-majorfrom
feature/client-dynamic-port-pool

Conversation

@RoadRunnr

Copy link
Copy Markdown
Member

Summary

Replaces the eradius client's blind rotating Identifier allocator (next_port_and_req_id/3 + static socket array) with a dynamic per-server pool of connected UDP sockets. Each socket is dedicated + gen_udp:connected to one server (OS-assigned source port; kernel filters replies to the peer), issues request ids 0..255 exactly once, then is retired: held open for reqid_reuse_timeout (the bound source port is the quarantine), drained, and closed.

  • Per-server pool of K active "filler" sockets (no_ports, default 10), round-robin; rolls to a fresh socket every 256 ids.
  • Per-server cap max_ports_per_server (default 256) → {error, no_ports} backpressure instead of a silent Identifier collision.
  • Monitors reclaim sockets on exit (cooldown close or crash); reconfigure closes/rebuilds pools and drops removed servers' pools; pool sockets are temporary (manager-managed).

Why

Under a slow RADIUS server the old allocator re-issued live {port, id} pairs, causing the client hang (mode A, mitigated in #248), premature reuse vs the server's duplicate-detection window (mode B), and delayed-reply misrouting (mode C). Rotating the source port makes reuse structurally safe: a reused id always lands on a fresh (client IP, port) key, which the server sees as a distinct client. Builds on #248 (the bounded call timeout guarantees in-flight drains within the cooldown).

Behavioral change to note

With K active fillers round-robined, a single RADIUS server now sees the client across up to K source ports concurrently (the old code pinned a peer to one source port, rotating every 256). This is intended (throughput / mailbox spread) and safe. eradius_metrics_SUITE was updated to sum the server-side per-nas_ip counters accordingly.

Test plan

  • Socket: connect + ReqId-keyed pending; retire holds-then-closes; waits for in-flight pending.
  • Manager: pool rolls at 256 + retires; cap → {error, no_ports}; 'DOWN' reclaims cooling slots; reconfigure resets pools.
  • Integration: across an id wrap the source port changes — no {srcport, id} pair repeats within the cooldown.
  • Full rebar3 ct green on OTP 28.3 (153 tests); rebar3 fmt clean.

Follow-ups (not in this PR)

  • Observability: spec calls for active/cooling/total gauges + no_ports/open/close counters via metrics_callback. This PR ships a minimal no_ports_rejections state counter + warning log; full metric wiring is deferred.
  • Pre-existing: eradius_client:send_request_loop matches {error, close} but the socket returns {error, closed}, so a closed socket surfaces as a generic error rather than socket_down. Predates this work; worth a small follow-up.

Design spec: docs/superpowers/specs/2026-06-04-eradius-dynamic-port-pool-design.md

🤖 Generated with Claude Code

@RoadRunnr RoadRunnr changed the title Dynamic per-server port pool: structurally safe RADIUS id reuse CENNSO-3792: Dynamic per-server port pool: structurally safe RADIUS id reuse Jun 4, 2026
RoadRunnr and others added 11 commits June 4, 2026 12:47
The socket now connects to its server (OS-assigned source port, kernel
filters replies to the peer) and keys its pending map on ReqId alone.
Groundwork for the per-server port pool.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A retired socket holds its (bound) source port for reqid_reuse_timeout
(the quarantine), keeps serving in-flight requests, then exits normally
once cooldown elapsed AND pending is empty. Force-closed at 2x cooldown.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
no_ports now means K (concurrent active fillers per server). New opts
default to 256 sockets/server cap and a 30s reqid reuse cooldown.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replaces the blind rotating counter + static socket array with per-server
pools of K connected 'filler' sockets. Each filler issues ids 0..255 then
is retired (cooled+closed by the socket); a fresh one is opened on demand.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Monitors on filler/cooling sockets let the manager drop a socket from
its pool when it closes (post-cooldown) or crashes, keeping the cap
accounting accurate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address change closes all pool sockets (and demonitors) then resets;
removed servers have their pools dropped. Legacy socket-array/idcounters
test assertions rewritten against the pool model; no_ports_one_wraps
removed (superseded by pool_rolls_and_retires).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds rotation_uses_fresh_source_ports (across a 256-id wrap the client
sends from a fresh OS source port; no {srcport,id} pair repeats).

With the K-filler pool a RADIUS server sees the client across several
source ports, so eradius_metrics_SUITE sums the server-side per-nas_ip
request/reply counters instead of expecting a single label-set.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- socket_sup: restart => temporary. Pool sockets are manager-managed
  (opened on demand, monitored, reclaimed on exit); auto-restart would
  orphan a socket the manager never learns about.
- Refresh the eradius_client_mngr 'Socket pool' moduledoc for the
  per-server dynamic pool (no_ports=K, reqid_reuse_timeout, max_ports_per_server).
- Add max_ports_per_server/reqid_reuse_timeout to client_config() type;
  tighten no_ports to pos_integer().
- Remove now-unused test-only get_socket_count/1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The active filler list is a FIFO round-robin (take head, re-enqueue at
tail; new fillers appended; retired head dropped). Model it with the
queue module: queue:out/in for the rotation, queue:filter for by-pid
removal on DOWN. No behaviour change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A filler's pid is unique, so removing it from the active queue on DOWN is
a single-element delete; queue:delete_with expresses that and short-circuits
instead of rebuilding the whole queue like queue:filter.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@RoadRunnr
RoadRunnr force-pushed the feature/client-dynamic-port-pool branch from 3ec66e2 to d3a6f68 Compare June 4, 2026 10:48
@RoadRunnr
RoadRunnr marked this pull request as ready for review June 5, 2026 07:57
@RoadRunnr
RoadRunnr requested a review from a team as a code owner June 5, 2026 07:57
@RoadRunnr
RoadRunnr merged commit 589ddfd into feature/next-major Jun 5, 2026
4 checks passed
@RoadRunnr
RoadRunnr deleted the feature/client-dynamic-port-pool branch June 5, 2026 07:58
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