Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sharpd: fix deleting nhid when suppressing nexthop from nh group #14937

Merged
merged 2 commits into from
Dec 6, 2023

Conversation

pguibert6WIND
Copy link
Member

When no nexthops are in a nexthop group, two successive events are sent: NHG_DEL and NHG_ADD, but only the NHG_DEL one is necessary. Fixes this by returning in the nhg_add() function.

Fixes: 82beaf6 ("sharpd: fix deleting nhid when suppressing nexthop from nh group")

@ton31337
Copy link
Member

ton31337 commented Dec 5, 2023

@pguibert6WIND commit hygiene is missing yet.

When no nexthops are in a nexthop group, two successive events are
sent: NHG_DEL and NHG_ADD, but only the NHG_DEL one is necessary.
Fixes this by returning in the nhg_add() function.

Fixes: 82beaf6 ("sharpd: fix deleting nhid when suppressing nexthop from nh group")

Signed-off-by: Philippe Guibert <[email protected]>
@ton31337
Copy link
Member

ton31337 commented Dec 5, 2023

@Mergifyio backport stable/9.1 stable/9.0

Copy link

mergify bot commented Dec 5, 2023

backport stable/9.1 stable/9.0

✅ Backports have been created

@pguibert6WIND pguibert6WIND force-pushed the fix_nhid_suppress branch 2 times, most recently from 1c7748e to 82c1d82 Compare December 5, 2023 07:25
Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

LGTM

After removing the nexthop of a nexthop group, when the nexthop group
is removed, a nhg removal failure happens:

> ubuntu2204(config)# nexthop-group CCC
> ubuntu2204(config-nh-group)#  nexthop 192.0.2.211 loop1
> ubuntu2204(config-nh-group)# no  nexthop 192.0.2.211 loop1
> [..]
> 2023/12/05 08:59:22 SHARP: [H3QKG-WH8ZV] Removed nhg 179687505
> ubuntu2204(config-nh-group)# exi
> ubuntu2204(config)# no nexthop-group CCC
> [..]
> 2023/12/05 08:59:27 SHARP: [N030J-V0SFN] Failed removal of nhg 179687505

The NHG_DEL message is sent twice at the nexthop deletion, and at the
nexthop-group deletion. Avoid sending it twice.

Fixes: 82beaf6 ("sharpd: fix deleting nhid when suppressing nexthop from nh group")

Signed-off-by: Philippe Guibert <[email protected]>
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
Copy link
Member

riw777 commented Dec 5, 2023

Doing a partial rerun ... I don't think the BGP failure on the Debian part 6 has anything to do with this code ...

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.

3 participants