Skip to content

bgpd: skip peers not activated for AFI/SAFI in bgp_gr_check_all_eors()#22295

Merged
ton31337 merged 1 commit into
FRRouting:masterfrom
shashanka-ks:bgpd-gr-check-all-eors-afc-lift
Jun 10, 2026
Merged

bgpd: skip peers not activated for AFI/SAFI in bgp_gr_check_all_eors()#22295
ton31337 merged 1 commit into
FRRouting:masterfrom
shashanka-ks:bgpd-gr-check-all-eors-afc-lift

Conversation

@shashanka-ks

Copy link
Copy Markdown
Contributor

bgp_gr_check_all_eors() walks every peer in bgp->peer and -- for any peer with PEER_STATUS_GR_WAIT_EOR set but no PEER_STATUS_EOR_RECEIVED -- splits into two code paths based on bgp->gr_multihop_peer_exists.

An existing !afc check filtered out peers that do not have this AFI/SAFI configured/activated, but it sat after the no-multihop-mix branch's early 'return false' -- so it was only ever reached when gr_multihop_peer_exists was true. The no-multihop-mix branch instead returned false on the first peer that lacked EOR_RECEIVED -- including peers that have no activated AF at all and are therefore physically incapable of ever sending an EOR for the AFI/SAFI in question.

In topologies where the BGP config defines neighbors that are never 'activate'd under any address family, this caused bgp_gr_check_all_eors() to return FALSE on every incoming EOR receipt, permanently blocking the GR fast-cancel path and forcing the deferral to always run to the select-defer-time safety-timer expiry.

Move the !afc check above the branch split so both branches see it; the post-split copy becomes unreachable and is removed.

bgp_gr_check_all_eors() walks every peer in bgp->peer and -- for any
peer with PEER_STATUS_GR_WAIT_EOR set but no PEER_STATUS_EOR_RECEIVED
-- splits into two code paths based on bgp->gr_multihop_peer_exists.

An existing !afc check filtered out peers that do not have this
AFI/SAFI configured/activated, but it sat after the no-multihop-mix
branch's early 'return false' -- so it was only ever reached when
gr_multihop_peer_exists was true. The no-multihop-mix branch instead
returned false on the first peer that lacked EOR_RECEIVED -- including
peers that have no activated AF at all and are therefore physically
incapable of ever sending an EOR for the AFI/SAFI in question.

In topologies where the BGP config defines neighbors that are never
'activate'd under any address family, this caused bgp_gr_check_all_eors()
to return FALSE on every incoming EOR receipt, permanently blocking
the GR fast-cancel path and forcing the deferral to always run to the
select-defer-time safety-timer expiry.

Move the !afc check above the branch split so both branches see it;
the post-split copy becomes unreachable and is removed.

Signed-off-by: Shashanka K S <shashankak@nvidia.com>
@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a bug in bgp_gr_check_all_eors() where peers with PEER_STATUS_GR_WAIT_EOR set but no activated address family for the given AFI/SAFI could cause the function to permanently return false, blocking the GR fast-cancel path and forcing all deferred route selection to wait until the safety timer expired.

  • The !afc[afi][safi] guard that correctly skips non-activated peers was only reachable on the gr_multihop_peer_exists == true branch; the gr_multihop_peer_exists == false branch returned false immediately on the first such peer, never seeing the guard.
  • The fix moves the !afc check above the branch split so both code paths skip non-activated peers, and removes the now-unreachable copy from the multihop branch.

Confidence Score: 5/5

The change is a surgical one-function fix that correctly moves an existing guard to an earlier position, with no new logic introduced and no regressions expected in the common case.

The moved !afc[afi][safi] check was already present and correct in the multihop branch; this PR extends its reach to the no-multihop branch where it was absent. The debug log message, the continue path, and the surrounding peer-iteration logic are all unchanged. No new state is introduced and the removed code is provably dead after the move.

No files require special attention.

Important Files Changed

Filename Overview
bgpd/bgp_fsm.c Moves the !afc[afi][safi] guard before the gr_multihop_peer_exists branch split so non-activated peers are skipped in both code paths; removes the now-dead copy from the multihop branch. Logic is correct and the change is minimal.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["for each peer in bgp->peer"] --> B{peer_is_config_node &&\nnot SHUTDOWN &&\nGRACEFUL_RESTART?}
    B -- No --> A
    B -- Yes --> C{PEER_STATUS_GR_WAIT_EOR\nset?}
    C -- No --> A
    C -- Yes --> D{PEER_STATUS_EOR_RECEIVED\nset?}
    D -- Yes --> A
    D -- No --> E{"!afc[afi][safi]?\n(peer not activated for AF)"}
    E -- Yes: skip --> A
    E -- No --> F{gr_multihop_peer_exists?}
    F -- No --> G["return false\n(still waiting for EOR)"]
    F -- Yes --> H{peer is multihop?}
    H -- Yes --> I[mark eor_rcvd_from_all_mh_peers = false]
    H -- No --> J["return false\n(direct peer, still waiting)"]
    I --> A
    A -- done --> K["return eor_rcvd_from_all_mh_peers"]
Loading

Reviews (1): Last reviewed commit: "bgpd: lift !afc check in bgp_gr_check_al..." | Re-trigger Greptile

@shashanka-ks shashanka-ks changed the title bgpd: lift !afc check in bgp_gr_check_all_eors bgpd: skip peers not activated for AFI/SAFI in bgp_gr_check_all_eors() Jun 9, 2026
@ton31337

Copy link
Copy Markdown
Member

@Mergifyio backport stable/10.7 stable/10.6 stable/10.5 stable/10.4 stable/10.3

@mergify

mergify Bot commented Jun 10, 2026

Copy link
Copy Markdown

backport stable/10.7 stable/10.6 stable/10.5 stable/10.4 stable/10.3

✅ Backports have been created

Details

Cherry-pick of e6b40ba has failed:

On branch mergify/bp/stable/10.5/pr-22295
Your branch is up to date with 'origin/stable/10.5'.

You are currently cherry-picking commit e6b40baa5.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bgpd/bgp_fsm.c

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Cherry-pick of e6b40ba has failed:

On branch mergify/bp/stable/10.4/pr-22295
Your branch is up to date with 'origin/stable/10.4'.

You are currently cherry-picking commit e6b40baa5.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bgpd/bgp_fsm.c

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Cherry-pick of e6b40ba has failed:

On branch mergify/bp/stable/10.3/pr-22295
Your branch is up to date with 'origin/stable/10.3'.

You are currently cherry-picking commit e6b40baa5.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bgpd/bgp_fsm.c

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

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