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

bgpd, zebra, tests: disable rtadv when bgp instance unconfiguration. #18364

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dmytroshytyi-6WIND
Copy link
Contributor

If bgpd instance is removed, then the rtadv is not disabled.

Like it was done when a peer is unconfigured,
do the same at BGP unconfiguration: unregister rtadv before deleting a peer.

Add a rtadv json fields. Create a topotest to check that newly added json fields are present.

pguibert6WIND and others added 2 commits March 11, 2025 10:27
If a peer uses radv for an interface, and bgp instance is removed,
then the radv service is not disabled on the interface.

Fix this by doing the same at BGP unconfiguration. Like it has been
done when a peer is unconfigured, call the radv unregistration before
deleting the peer.

Fixes: b3a3290 ("bgpd: turn off RAs when numbered peers are deleted")
Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Dmytro Shytyi <[email protected]>
When configured Graceful-Restart, skipping unconfig notification,
similarly as it is done in 95098d9
("bgpd: Do not send Deconfig/Shutdown message when restarting")

Signed-off-by: Dmytro Shytyi <[email protected]>
Add to "show interface json" output multiple rtadv parameters.

if_dump_vty() calls => hook_call(zebra_if_extra_info, vty, ifp);

if_dump_vty_json() now do the same call, with additional parameter:
hook_call(zebra_if_extra_info, vty, json_if, ifp);

Signed-off-by: Dmytro Shytyi <[email protected]>
Verify the new rtadv "show interface json" fields

Signed-off-by: Dmytro Shytyi <[email protected]>
@dmytroshytyi-6WIND
Copy link
Contributor Author

ci:rerun

@riw777 riw777 self-requested a review March 11, 2025 15:25
#if defined(HAVE_RTADV)
if (json_if && rtadv->AdvSendAdvertisements) {
json_object_int_add(json_if, "ndAdvertisedReachableTimeMilliseconds",
rtadv->AdvReachableTime);
Copy link
Member

@pguibert6WIND pguibert6WIND Mar 11, 2025

Choose a reason for hiding this comment

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

Msecs instead of Milliseconds (git grep Msecs on bgpd)

json_object_int_add(json_if, "ndAdvertisedReachableTimeMilliseconds",
rtadv->AdvReachableTime);
json_object_int_add(json_if, "ndAdvertisedRetransmitIntervalMilliseconds",
rtadv->AdvRetransTimer);
Copy link
Member

Choose a reason for hiding this comment

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

Msecs instead of Milliseconds

interval = rtadv->MaxRtrAdvInterval;
if (interval % 1000)
json_object_int_add(json_if, "ndRouterAdvertisementsIntervalMilliseconds",
interval);
Copy link
Member

Choose a reason for hiding this comment

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

Msecs instead of Milliseconds

else
json_object_int_add(json_if, "ndRouterAdvertisementsIntervalSeconds",
interval / 1000);

Copy link
Member

Choose a reason for hiding this comment

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

Secs instead of Seconds

"ndRouterAdvertisementsWithHomeAgentFlagBitSet",
true);
if (rtadv->HomeAgentLifetime != -1)
json_object_int_add(json_if, "homeAgentLifetimeSeconds",
Copy link
Member

Choose a reason for hiding this comment

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

Secs instead of Seconds

"homeAgentLifetimeTracksRaLifetime", true);

json_object_int_add(json_if, "homeAgenttPreferenceIs",
rtadv->HomeAgentLifetime);
Copy link
Member

Choose a reason for hiding this comment

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

typo: homeAgentPreferenceIs
'homeAgentPreference' is enough

@@ -1792,6 +1792,65 @@ static int nd_dump_vty(struct vty *vty, struct interface *ifp)
vty_out(vty,
" ND router advertisements with Adv. Interval option.\n");
}

#if defined(HAVE_RTADV)
Copy link
Member

Choose a reason for hiding this comment

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

useless define


if (rtadv->AdvHomeAgentFlag) {
json_object_boolean_add(json_if,
"ndRouterAdvertisementsWithHomeAgentFlagBitSet",
Copy link
Member

Choose a reason for hiding this comment

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

ndRouterAdvertisementsWithHomeAgentFlagBit is enough


if (rtadv->AdvDefaultLifetime != -1)
json_object_int_add(json_if, "ndRouterAdvertisementsLiveForSeconds",
rtadv->AdvDefaultLifetime);
Copy link
Member

Choose a reason for hiding this comment

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

ndRouterAdvertisementsLiveForSecs

Copy link
Member

@pguibert6WIND pguibert6WIND left a comment

Choose a reason for hiding this comment

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

some changes need to be done

@donaldsharp
Copy link
Member

we appear to have 2 open commits from 6wind in relation to this exact problem. Which one is the one we should be lookiing at?

@pguibert6WIND
Copy link
Member

we appear to have 2 open commits from 6wind in relation to this exact problem. Which one is the one we should be lookiing at?

I just closed #18328.
this is that one to look at.

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.

3 participants