Skip to content

ospfd: RFC 4222 R4 per-neighbor LSA gap pacing#22374

Open
raviravindran123 wants to merge 6 commits into
FRRouting:masterfrom
raviravindran123:ospf-rfc-4222-lsa-pacing
Open

ospfd: RFC 4222 R4 per-neighbor LSA gap pacing#22374
raviravindran123 wants to merge 6 commits into
FRRouting:masterfrom
raviravindran123:ospf-rfc-4222-lsa-pacing

Conversation

@raviravindran123

Copy link
Copy Markdown

Implements RFC 4222 Recommendation 4: per-neighbor LSA gap pacing.

When enabled on an interface, outbound LSAs are queued per-neighbor
and metered at a configurable inter-packet gap rather than sent
back-to-back. The gap adapts using MIMD: grows multiplicatively when
the unacknowledged LSA count exceeds a high-watermark, shrinks
multiplicatively when it falls below a low-watermark.

New commands (all interface-scoped):
ip ospf lsa-pacing
ip ospf lsa-pacing min-gap max-gap
ip ospf lsa-pacing initial-gap
ip ospf lsa-pacing factor
ip ospf lsa-pacing adjust-interval
ip ospf lsa-pacing max-lsas-per-update
ip ospf lsa-pacing low-watermark high-watermark

Defaults: min/initial gap 20 ms, max 1000 ms, factor 2,
adjust-interval 1000 ms, high-water 100, low-water 2, max-lsas 1.

This feature is independent of R5 adjacency pacing (also on this
branch) — neither gates the other.

Topotests added under tests/topotests/ospf_rfc4222_r4/:

  • CLI persist/reload
  • Gap metering (functional)
  • Opaque LSA pacing
  • MIMD adaptation under congestion

@frrbot frrbot Bot added documentation ospf tests Topotests, make check, etc labels Jun 15, 2026
@raviravindran123
raviravindran123 force-pushed the ospf-rfc-4222-lsa-pacing branch 2 times, most recently from f1761fd to 3447e2f Compare June 15, 2026 23:33
@raviravindran123

Copy link
Copy Markdown
Author

@greptile

@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown

Greptile Summary

This PR implements RFC 4222 Recommendation 4 (per-neighbor LSA gap pacing) in ospfd, alongside R5 adjacency pacing. When enabled on an interface, outbound LSAs are queued per-neighbor and metered at a configurable inter-packet gap that adapts via MIMD based on unACKed LSA watermarks.

  • R4 LSA pacing: new per-neighbor r4_send_queue, a paced send timer (ospf_r4_nbr_send_timer), MIMD gap adjustment (pace_maybe_adjust_gap), and per-packet sent_lsas tracking for the ls_rxmt_unacked counter.
  • R5 adjacency pacing: per-interface ospf_adj_pacing struct with static/dynamic modes, MIMD limit adjustment via ospf_adj_dyn_adjust_timer, and throttle hooks in nsm_twoway_received/nsm_adj_ok.
  • CLI and topotests: seven new interface-scoped ip ospf lsa-pacing sub-commands with persistence/reload and functional MIMD congestion tests.

Confidence Score: 2/5

Not safe to merge as-is: the ls_rxmt_unacked counter permanently accumulates on LSA re-origination, and disabling R4 pacing on a live interface silently drops LSAs not yet recorded in the retransmit list.

The ls_rxmt_unacked counter is the sole feedback signal driving both MIMD algorithms. When ospf_ls_retransmit_add supersedes an older LSA version it discards the old node without decrementing the counter, so after each re-origination cycle the counter can only grow. In a production network with frequent topology changes this permanently disables the gap-shrink and adjacency-increase halves of both algorithms. Separately, toggling no ip ospf lsa-pacing while a flood is in progress abandons LSAs queued for initial delivery that have no retransmit record, causing LSDB divergence.

ospfd/ospf_flood.c (ospf_ls_retransmit_add around line 1143) and ospfd/ospf_interface.c (ospf_rec4_recompute_effective) need fixes before this is safe to enable in production.

Important Files Changed

Filename Overview
ospfd/ospf_packet.c Core of the R4 pacing implementation: per-neighbor send queue, paced send timer, MIMD gap adjustment, ospf_count_sent_lsa tracking; redundant retransmit-add on the retransmit path can reset ls_rxmt_list entry timing on concurrent re-origination.
ospfd/ospf_flood.c Adds R4 pacing hooks in ospf_flood_through_interface and ospf_ls_retransmit_delete; the unchanged ospf_ls_retransmit_add does not decrement ls_rxmt_unacked when superseding an older LSA, causing the MIMD counter to permanently drift upward.
ospfd/ospf_interface.c Adds R4 effective-param caching (ospf_rec4_recompute_effective) and R5 adjacency pacing init; disabling R4 pacing on a live interface does not drain r4_send_queue, silently abandoning unflooded LSAs on the initial-flood path.
ospfd/ospf_nsm.c Implements R5 adjacency pacing: queue init/flush, MIMD dynamic adjustment, throttle hooks in nsm_twoway_received and nsm_adj_ok, in_progress tracking in nsm_change_state; logic is sound.
ospfd/ospf_vty.c Adds CLI for R4 LSA-pacing and R5 adjacency-pacing; config-write correctly guards sub-parameters behind gap_pacing_enable.
ospfd/ospf_interface.h Adds R4 pacing parameter defaults and runtime fields to ospf_if_params and ospf_interface; R5 adjacency pacing structs also added.
ospfd/ospf_neighbor.h Adds per-neighbor R4 pacing fields (lsu_gap_ms, next_send_ms, r4_send_queue, ls_rxmt_unacked) and R5 queue linkage.
ospfd/ospf_lsdb.h Adds counted_sent bool to ospf_lsdb_linked_node for unACKed LSA tracking; minimal, correct addition.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Flood as ospf_flood_through_interface
    participant Q as r4_send_queue
    participant Timer as ospf_r4_nbr_send_timer
    participant Send as ospf_ls_upd_queue_send
    participant Write as ospf_write
    participant Count as ospf_count_sent_lsa
    participant RxmtDB as nbr->ls_rxmt
    participant MIMD as pace_maybe_adjust_gap
    Flood->>Q: ospf_r4_nbr_enqueue(nbr, lsa)
    Note over Q: skips ls_retransmit_add
    Timer->>Send: ospf_ls_upd_queue_send(oi, r4_send_queue, nbr)
    Send->>RxmtDB: ospf_ls_retransmit_add(dst_nbr, lsa)
    Send->>Write: ospf_packet_add(oi, op)
    Write->>Count: ospf_count_sent_lsa(oi, dst, lsa)
    Count->>RxmtDB: "lookup counted_sent=false, ls_rxmt_unacked++"
    Note over MIMD: ACK arrives
    MIMD->>RxmtDB: ospf_ls_retransmit_delete, ls_rxmt_unacked--
    MIMD->>MIMD: "pace_maybe_adjust_gap U<=L shrink gap"
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Flood as ospf_flood_through_interface
    participant Q as r4_send_queue
    participant Timer as ospf_r4_nbr_send_timer
    participant Send as ospf_ls_upd_queue_send
    participant Write as ospf_write
    participant Count as ospf_count_sent_lsa
    participant RxmtDB as nbr->ls_rxmt
    participant MIMD as pace_maybe_adjust_gap
    Flood->>Q: ospf_r4_nbr_enqueue(nbr, lsa)
    Note over Q: skips ls_retransmit_add
    Timer->>Send: ospf_ls_upd_queue_send(oi, r4_send_queue, nbr)
    Send->>RxmtDB: ospf_ls_retransmit_add(dst_nbr, lsa)
    Send->>Write: ospf_packet_add(oi, op)
    Write->>Count: ospf_count_sent_lsa(oi, dst, lsa)
    Count->>RxmtDB: "lookup counted_sent=false, ls_rxmt_unacked++"
    Note over MIMD: ACK arrives
    MIMD->>RxmtDB: ospf_ls_retransmit_delete, ls_rxmt_unacked--
    MIMD->>MIMD: "pace_maybe_adjust_gap U<=L shrink gap"
Loading

Comments Outside Diff (1)

  1. ospfd/ospf_flood.c, line 1142-1163 (link)

    P1 ls_rxmt_unacked leaks on LSA version supersession

    When ospf_ls_retransmit_add replaces an older LSA version with a newer one (the ospf_lsa_more_recent(old, lsa) < 0 branch), it frees the old ospf_lsdb_linked_node but never checks whether that node had counted_sent = true. If it did, nbr->ls_rxmt_unacked is NOT decremented, even though the tracked retransmit entry no longer exists.

    Concrete failure: each time a re-originated LSA instance (new sequence number) supersedes an older one that was already counted-sent, the counter grows by one and is never reclaimed — not even when the new version is ACKed (which calls ospf_ls_retransmit_delete once for the new version only). After enough re-originations the MIMD watermark U > L is permanently satisfied, so the gap never shrinks to its minimum and R5 dynamic adjacency pacing always perceives congestion.

    Fix: in the if (old) block (around line 1143) check ls_rxmt_node->counted_sent and decrement nbr->ls_rxmt_unacked before calling ospf_lsdb_delete.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: ospfd/ospf_flood.c
    Line: 1142-1163
    
    Comment:
    **`ls_rxmt_unacked` leaks on LSA version supersession**
    
    When `ospf_ls_retransmit_add` replaces an older LSA version with a newer one (the `ospf_lsa_more_recent(old, lsa) < 0` branch), it frees the old `ospf_lsdb_linked_node` but never checks whether that node had `counted_sent = true`. If it did, `nbr->ls_rxmt_unacked` is NOT decremented, even though the tracked retransmit entry no longer exists.
    
    Concrete failure: each time a re-originated LSA instance (new sequence number) supersedes an older one that was already counted-sent, the counter grows by one and is never reclaimed — not even when the new version is ACKed (which calls `ospf_ls_retransmit_delete` once for the new version only). After enough re-originations the MIMD watermark `U > L` is permanently satisfied, so the gap never shrinks to its minimum and R5 dynamic adjacency pacing always perceives congestion.
    
    Fix: in the `if (old)` block (around line 1143) check `ls_rxmt_node->counted_sent` and decrement `nbr->ls_rxmt_unacked` before calling `ospf_lsdb_delete`.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
ospfd/ospf_flood.c:1142-1163
**`ls_rxmt_unacked` leaks on LSA version supersession**

When `ospf_ls_retransmit_add` replaces an older LSA version with a newer one (the `ospf_lsa_more_recent(old, lsa) < 0` branch), it frees the old `ospf_lsdb_linked_node` but never checks whether that node had `counted_sent = true`. If it did, `nbr->ls_rxmt_unacked` is NOT decremented, even though the tracked retransmit entry no longer exists.

Concrete failure: each time a re-originated LSA instance (new sequence number) supersedes an older one that was already counted-sent, the counter grows by one and is never reclaimed — not even when the new version is ACKed (which calls `ospf_ls_retransmit_delete` once for the new version only). After enough re-originations the MIMD watermark `U > L` is permanently satisfied, so the gap never shrinks to its minimum and R5 dynamic adjacency pacing always perceives congestion.

Fix: in the `if (old)` block (around line 1143) check `ls_rxmt_node->counted_sent` and decrement `nbr->ls_rxmt_unacked` before calling `ospf_lsdb_delete`.

Reviews (2): Last reviewed commit: "tests: add RFC4222 R4 LSA pacing topotes..." | Re-trigger Greptile

Comment thread ospfd/ospf_packet.c
Comment thread ospfd/ospf_packet.c Outdated
Comment thread ospfd/ospf_packet.c Outdated
@raviravindran123
raviravindran123 force-pushed the ospf-rfc-4222-lsa-pacing branch 4 times, most recently from c9f07fc to d4fabd3 Compare June 16, 2026 20:53
@raviravindran123
raviravindran123 marked this pull request as ready for review July 7, 2026 17:56
@raviravindran123
raviravindran123 force-pushed the ospf-rfc-4222-lsa-pacing branch from d4fabd3 to e830773 Compare July 15, 2026 23:41
@raviravindran123

Copy link
Copy Markdown
Author

Force-pushed an update addressing a defect found in further testing of the paced retransmit path:

  • The retransmit timer could enqueue duplicate copies of an unacked LSA whenever queue residence exceeded the retransmit interval (unbounded queue growth under congestion — precisely the regime R4 pacing targets). Each retransmission-list entry now tracks its queued copy (r4_qnode); the timer reschedules instead of duplicating, ACKs and newer instances purge stale queued copies in O(1), and disabling pacing at runtime hands never-transmitted queued LSAs to the regular retransmission machinery instead of dropping them.
  • Moved the live pacing-parameter propagation hunks from the tests commit into the ospfd commit where they belong; the tests commit is now purely tests.
  • Added a heavy-load regression topotest (test_ospf_lsa_pacing_congested.py::test_congested_heavy_load_no_duplicate_retransmits): 20 LSAs paced at 500 ms under a 13 s ACK blackout with a 3 s retransmit interval — asserts the adjacency holds throughout and the retransmission list converges to zero promptly once ACKs resume.

@riw777

riw777 commented Jul 17, 2026

Copy link
Copy Markdown
Member

Can we get the linter error fixed? Thanks!

@raviravindran123
raviravindran123 force-pushed the ospf-rfc-4222-lsa-pacing branch from e830773 to d9e4ab3 Compare July 20, 2026 15:47
@frrbot frrbot Bot added the bugfix label Jul 20, 2026
Implement RFC 4222 recommendation 5: per-interface adjacency pacing to
limit the number of simultaneous OSPF adjacency formations and avoid
database-exchange bursts on bandwidth-constrained interfaces.

Two modes are provided:

- Static: a fixed concurrency limit configured per interface.
- Dynamic: an AIMD-based limit that adjusts automatically using the
  total unacknowledged LSA count (U) across the interface as a
  congestion signal. When U rises above a high-water mark the limit
  is halved; when U falls below a low-water mark the limit is
  incremented by one.

Neighbors that cannot immediately enter ExStart are queued per
interface in FIFO order. When a slot opens, ospf_adj_pacing_kick()
restarts the next queued neighbor. The dynamic limit is re-evaluated
on LSA acknowledgement and on the retransmit timer.

Signed-off-by: Ravi Ravindran <rravindran@labn.net>
Add topotests for RFC 4222 recommendation 5 adjacency pacing covering
three test files:

- test_ospf_adj_pacing_config.py: verifies static and dynamic pacing
  configuration persistence, interface flap behavior, FRR daemon
  restart, no-command cleanup, threshold validation, and defaults.

- test_ospf_broadcast_router_dynamic_pacing.py: verifies dynamic
  pacing on a 7-router broadcast topology with DR election; tests
  AIMD congestion detection under LSA flood and queue-kick behavior
  when the dynamic limit increases after congestion clears.

- test_ospf_broadcast_router_static_pacing.py: verifies static pacing
  on the same broadcast topology.

Router configurations cover point-to-point (r1-r3) and broadcast
(r1_broadcast through r7_broadcast) topologies with hello/dead timers
tuned for fast convergence.

Signed-off-by: Ravi Ravindran <rravindran@labn.net>
Add developer and user documentation for RFC 4222 recommendation 5
adjacency pacing.

doc/developer/ospf-adj-pacing.rst: describes the static and dynamic
pacing modes, the AIMD algorithm, code paths in ospf_nsm.c, and how
unacknowledged LSA counting works across broadcast and point-to-point
segments.

doc/developer/ospf.rst: add ospf-adj-pacing to the OSPF developer
documentation toctree.

doc/user/ospfd.rst: add four clicmd entries for the adjacency pacing
interface commands: static limit, dynamic mode, dynamic thresholds,
and no-form.

Signed-off-by: Ravi Ravindran <rravindran@labn.net>
Add 7 test cases to test_ospf_adj_pacing_config.py covering static
pacing configuration that was missing from initial test coverage:

- config persistence (write memory / running-config)
- persistence across ospfd restart
- persistence across interface flap
- no-command removes static config
- boundary values (limit=1 minimum, limit=65535 maximum)
- transition from static to dynamic mode
- transition from dynamic to static mode

Signed-off-by: Ravi Ravindran <rravindran@labn.net>
Implement RFC 4222 recommendation 4: rate-limit outbound LS Update
packets on a per-neighbor basis using a configurable inter-packet gap.

When enabled on an interface, outbound LSAs destined for a neighbor are
queued in a per-neighbor send list (nbr->r4_send_queue) rather than
written immediately. A per-neighbor timer (ospf_r4_nbr_send_timer)
drains the queue at the configured rate, sending at most
rec4_max_lsas LSAs per LS Update packet.

The inter-packet gap is adapted using a multiplicative-increase /
multiplicative-decrease (MIMD) algorithm based on the per-neighbor
unacknowledged LSA count U:

  U > H:  G = min(G * F, Gmax)  -- gap grows, send rate slows
  U == 0: G = Gmin              -- immediate reset to floor
  U <= L: G = max(G / F, Gmin)  -- gap shrinks, send rate rises

Defaults: initial/min gap 20 ms, max 1000 ms, factor 2,
adjust-interval 1000 ms, high-water 100, low-water 2,
max-lsas-per-update 1.

New interface commands:
  ip ospf lsa-pacing
  ip ospf lsa-pacing initial-gap <ms>
  ip ospf lsa-pacing min-gap <ms> max-gap <ms>
  ip ospf lsa-pacing factor <F>
  ip ospf lsa-pacing adjust-interval <ms>
  ip ospf lsa-pacing max-lsas-per-update <N>
  ip ospf lsa-pacing low-watermark <L> high-watermark <H>

Signed-off-by: Ravi Ravindran <rravindran@labn.net>
@raviravindran123
raviravindran123 force-pushed the ospf-rfc-4222-lsa-pacing branch 2 times, most recently from 336a899 to 9427408 Compare July 21, 2026 19:08
Add RFC4222 R4 coverage for LSA pacing behavior under functional, broadcast, opaque, CLI, and congested conditions.

Signed-off-by: Ravi Ravindran <rravindran@labn.net>
@raviravindran123
raviravindran123 force-pushed the ospf-rfc-4222-lsa-pacing branch from 9427408 to b62e86d Compare July 21, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants