Skip to content

pimd: fix elected BSR not updating when priority changes#22465

Merged
Jafaral merged 2 commits into
FRRouting:masterfrom
enkechen-panw:pim-bsr-priority
Jun 24, 2026
Merged

pimd: fix elected BSR not updating when priority changes#22465
Jafaral merged 2 commits into
FRRouting:masterfrom
enkechen-panw:pim-bsr-priority

Conversation

@enkechen-panw

Copy link
Copy Markdown
Contributor

When the elected BSR changes its own priority, the change was silently
ignored. This happened because pim_cand_bsr_trigger() returned early
when current_bsr matched our own address, without checking if the
priority had changed.

Fix by updating current_bsr_prio and regenerating BSM immediately when
the elected BSR's priority changes.

@frrbot frrbot Bot added bugfix pim tests Topotests, make check, etc labels Jun 24, 2026
@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a bug where an elected BSR silently ignored its own priority increases. The root cause was pim_cand_bsr_trigger() returning early when current_bsr == run_addr without checking if the priority had diverged from cand_bsr_prio.

  • Core fix (pim_bsm.c): When the elected BSR's address matches our own and state is BSR_ELECTED, the code now detects a changed cand_bsr_prio, syncs current_bsr_prio, and calls pim_bsm_generate() to rebuild and resend fragments with the updated priority — using the correct regeneration path that frees stale fragments rather than replaying them.
  • Test (test_pim_cand_rp_bsr.py): A new test step raises r2's priority from 250 → 255 and validates the change on both r2's own state and r1's received BSM, confirming the on-wire packet is properly regenerated; test_pim_bsr_election_fallback_r2 is updated to reflect the new baseline priority of 255.

Confidence Score: 5/5

The change is safe to merge: it adds a narrowly scoped guard inside pim_cand_bsr_trigger that only fires when the router is BSR_ELECTED and the two priority fields have diverged, then delegates to the existing pim_bsm_generate() path which has its own assertf(state == BSR_ELECTED) guard.

The fix is minimal and correct. It updates current_bsr_prio before calling pim_bsm_generate(), so the rebuilt fragment carries the new priority. The priority-decrease path is still handled by the early-return in pim_cand_bsr_apply and is unaffected by this change. The test validates the on-wire behaviour by checking that a downstream neighbour (r1) also reflects the updated priority.

No files require special attention.

Important Files Changed

Filename Overview
pimd/pim_bsm.c Adds a priority-change guard inside pim_cand_bsr_trigger; correctly updates current_bsr_prio and calls pim_bsm_generate() (which rebuilds fragments with the new priority and then calls pim_bsm_changed()) only when state == BSR_ELECTED and the priority has actually diverged.
tests/topotests/pim_cand_rp_bsr/test_pim_cand_rp_bsr.py Adds test_pim_elected_bsr_priority_change which raises r2's priority to 255 and verifies both r2's state and r1's received BSM reflect the new priority; updates the priority baseline in test_pim_bsr_election_fallback_r2.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Op as Operator
    participant BSR as r2 (BSR_ELECTED)
    participant r1 as r1 (Neighbour)

    Note over BSR: current_bsr_prio = 250, cand_bsr_prio = 250

    Op->>BSR: configure priority 255
    BSR->>BSR: pim_cand_bsr_apply()
    Note over BSR: current_bsr_prio(250) NOT > cand_bsr_prio(255), does NOT early-return

    BSR->>BSR: pim_cand_bsr_trigger()
    Note over BSR: current_bsr == run_addr, state == BSR_ELECTED, priorities differ

    BSR->>BSR: "current_bsr_prio = 255"
    BSR->>BSR: "pim_bsm_generate() - free stale frags, rebuild bsr_prio=255, pim_bsm_changed()"

    BSR-->>r1: "BSM (bsr_prio=255)"
    Note over r1: Updates current_bsr_prio = 255
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 Op as Operator
    participant BSR as r2 (BSR_ELECTED)
    participant r1 as r1 (Neighbour)

    Note over BSR: current_bsr_prio = 250, cand_bsr_prio = 250

    Op->>BSR: configure priority 255
    BSR->>BSR: pim_cand_bsr_apply()
    Note over BSR: current_bsr_prio(250) NOT > cand_bsr_prio(255), does NOT early-return

    BSR->>BSR: pim_cand_bsr_trigger()
    Note over BSR: current_bsr == run_addr, state == BSR_ELECTED, priorities differ

    BSR->>BSR: "current_bsr_prio = 255"
    BSR->>BSR: "pim_bsm_generate() - free stale frags, rebuild bsr_prio=255, pim_bsm_changed()"

    BSR-->>r1: "BSM (bsr_prio=255)"
    Note over r1: Updates current_bsr_prio = 255
Loading

Reviews (4): Last reviewed commit: "tests: pim_cand_rp_bsr: add elected BSR ..." | Re-trigger Greptile

Comment thread pimd/pim_bsm.c Outdated
@enkechen-panw enkechen-panw marked this pull request as draft June 24, 2026 02:24
When the elected BSR changes its own priority, the change was silently
ignored. This happened because pim_cand_bsr_trigger() returned early
when current_bsr matched our own address, without checking if the
priority had changed.

Fix by updating current_bsr_prio and regenerating BSM immediately when
the elected BSR's priority changes.

Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
@enkechen-panw

Copy link
Copy Markdown
Contributor Author

@greptileai review

Add test_pim_elected_bsr_priority_change to verify that when an
elected BSR changes its own priority, the change takes effect
immediately.

Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
@enkechen-panw

Copy link
Copy Markdown
Contributor Author

@greptileai review

@enkechen-panw enkechen-panw marked this pull request as ready for review June 24, 2026 02:44
@Jafaral

Jafaral commented Jun 24, 2026

Copy link
Copy Markdown
Member

@merifyio backport stable/10.7 stable/10.6

@Jafaral Jafaral merged commit 90a2db7 into FRRouting:master Jun 24, 2026
23 checks passed
@enkechen-panw enkechen-panw deleted the pim-bsr-priority branch June 24, 2026 05:59
@enkechen-panw

Copy link
Copy Markdown
Contributor Author

Hi, @Jafaral: Did you try to trigger backport? There was a typo in the command: "@merifyio" vs "@Mergifyio"?

@Jafaral

Jafaral commented Jun 24, 2026

Copy link
Copy Markdown
Member

@Mergifyio backport stable/10.7 stable/10.6

@mergify

mergify Bot commented Jun 24, 2026

Copy link
Copy Markdown

backport stable/10.7 stable/10.6

✅ Backports have been created

Details

@Jafaral

Jafaral commented Jun 24, 2026

Copy link
Copy Markdown
Member

Hi, @Jafaral: Did you try to trigger backport? There was a typo in the command: "@merifyio" vs "@Mergifyio"?

Thanks! I fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants