Skip to content

bgpd: output 'graceful-restart' value for peer group in 'write' command#20338

Merged
riw777 merged 2 commits intoFRRouting:masterfrom
hedrok:fix-bgp-gr-per-neighbor-output
Jan 6, 2026
Merged

bgpd: output 'graceful-restart' value for peer group in 'write' command#20338
riw777 merged 2 commits intoFRRouting:masterfrom
hedrok:fix-bgp-gr-per-neighbor-output

Conversation

@hedrok
Copy link
Contributor

@hedrok hedrok commented Dec 28, 2025

Revert of d34b973 (merged in #18305)

The reverted commit prevented output of graceful-restart value for peer groups.

Setting option graceful-restart for peer group is allowed, it is used by peers that are in this group. I've modified test to demonstrate this and to catch regressions.

Dear @Pdoijode @vivek-cumulus I'm sure you had reasons to add this commit, but description contains only

bgpd: Ensure incorrect GR config isn't displayed for peer-groups

Could you please describe which problem it tried to solve?..

@frrbot frrbot bot added the tests Topotests, make check, etc label Dec 28, 2025
Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good ... waiting on comment/discussion about why this was changed in the first place to make certain this is correct

neighbor 192.168.34.4 timers 1 3
neighbor 192.168.34.4 timers connect 1
neighbor 192.168.34.4 graceful-restart
neighbor PG peer-group
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we leave the 192.168.34.4 peer as it was before? And add a peer-group additionally (separate topotest is fine too).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course.
Changed test so that original test is untouched except adding peer-group + starting back r3 at the end of test.
Added new test method that checks only this issue.

@hedrok hedrok force-pushed the fix-bgp-gr-per-neighbor-output branch from 2b1622a to c64cb20 Compare January 5, 2026 19:43
@github-actions github-actions bot added the rebase PR needs rebase label Jan 5, 2026
hedrok added 2 commits January 5, 2026 21:45
Check that for peer groups value of graceful-restart is written by
`write` command.

Signed-off-by: Kyrylo Yatsenko <hedrok@gmail.com>
…ups"

This reverts commit d34b973.

Signed-off-by: Kyrylo Yatsenko <hedrok@gmail.com>
@hedrok hedrok force-pushed the fix-bgp-gr-per-neighbor-output branch from c64cb20 to a8bf21b Compare January 5, 2026 19:56
@hedrok hedrok requested a review from ton31337 January 5, 2026 19:59
@hedrok
Copy link
Contributor Author

hedrok commented Jan 5, 2026

ci:rerun

@Pdoijode
Copy link
Contributor

Pdoijode commented Jan 5, 2026

@hedrok, Since FRR supports GR for peer-groups, commit d34b973 seems incorrect. Reverting it is ok.

Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@riw777 riw777 merged commit e0fe5e4 into FRRouting:master Jan 6, 2026
18 checks passed
@hedrok hedrok deleted the fix-bgp-gr-per-neighbor-output branch January 6, 2026 15:19
@hedrok hedrok mentioned this pull request Jan 6, 2026
11 tasks
@ton31337
Copy link
Member

@Mergifyio backport stable/10.5 stable/10.4 stable/10.3 stable/10.2

@mergify
Copy link

mergify bot commented Jan 22, 2026

backport stable/10.5 stable/10.4 stable/10.3 stable/10.2

✅ Backports have been created

Details

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

Labels

master rebase PR needs rebase size/M tests Topotests, make check, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants