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: Clean address-family config on daemon restart #17716

Merged
merged 1 commit into from
Jan 1, 2025

Conversation

ykholod
Copy link

@ykholod ykholod commented Dec 23, 2024

When stopping and restarting BGP daemon part of the configuration remains. It should be cleared.
Particulary those are address-family parametes, like: distance, ead-es-frag, disable-ead-evi-rx, disable-ead-evi-tx.

@ykholod ykholod changed the title BGP: Clean address-family config on daemon restart bgpd: Clean address-family config on daemon restart Dec 23, 2024
@ykholod
Copy link
Author

ykholod commented Dec 27, 2024

Fixing issue #17463
Please feel free to comment

bgpd/bgp_route.c Outdated
@@ -15778,6 +15778,30 @@ static int bgp_distance_unset(struct vty *vty, const char *distance_str,
return CMD_SUCCESS;
}

int bgp_address_family_distance_clean()
Copy link
Member

Choose a reason for hiding this comment

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

Please use the same pattern as bgp_static_delete(). More/less identical except for some adjustments.

Copy link
Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

I meant not only void(), but the structure, macros, logic inside also.

bgpd/bgp_route.h Outdated
@@ -831,6 +831,7 @@ extern void bgp_redistribute_withdraw(struct bgp *, afi_t, int, unsigned short);

extern void bgp_static_add(struct bgp *);
extern void bgp_static_delete(struct bgp *);
extern void bgp_address_family_distance_clean();
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 name it also ..._delete()?

Copy link
Author

Choose a reason for hiding this comment

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

Done

bgpd/bgpd.c Outdated
@@ -4237,6 +4237,14 @@ int bgp_delete(struct bgp *bgp)
}
}

// Clean BGP address family parameters
Copy link
Member

Choose a reason for hiding this comment

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

Please drop this comment or replace with /* */.

Copy link
Author

Choose a reason for hiding this comment

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

Done

bgpd/bgp_route.c Outdated
struct bgp_dest *dest = NULL;
struct bgp_distance *bdistance = NULL;

for (afi=AFI_IP;afi<AFI_MAX;afi++) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's use FOREACH_AFI_SAFI macro, see bgp_static_delete().

Copy link
Author

Choose a reason for hiding this comment

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

Done

bgpd/bgp_route.h Outdated
@@ -831,6 +831,7 @@ extern void bgp_redistribute_withdraw(struct bgp *, afi_t, int, unsigned short);

extern void bgp_static_add(struct bgp *);
extern void bgp_static_delete(struct bgp *);
extern void bgp_address_family_distance_delete();
Copy link
Member

Choose a reason for hiding this comment

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

Please use extern void bgp_address_family_distance_delete(void);

Copy link
Author

Choose a reason for hiding this comment

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

Done

bgpd/bgp_route.c Outdated
@@ -15778,6 +15778,28 @@ static int bgp_distance_unset(struct vty *vty, const char *distance_str,
return CMD_SUCCESS;
}

void bgp_address_family_distance_delete()
Copy link
Member

Choose a reason for hiding this comment

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

void bgp_address_family_distance_delete(void), please.

Copy link
Author

Choose a reason for hiding this comment

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

Done

bgpd/bgp_route.c Outdated
XFREE(MTYPE_AS_LIST, bdistance->access_list);
bgp_distance_free(bdistance);

bgp_dest_set_bgp_path_info(dest, NULL);
Copy link
Member

Choose a reason for hiding this comment

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

We should use bgp_dest_set_bgp_distance_info() here.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@ton31337
Copy link
Member

Also, please take a look at frrbot and Verify Source warnings.

When stopping and restarting BGP daemon part of the configuration
remains. It should be cleared.
Particulary those are address-family parametes, like: distance,
ead-es-frag, disable-ead-evi-rx, disable-ead-evi-tx.

Signed-off-by: Yaroslav Kholod <[email protected]>
@ykholod
Copy link
Author

ykholod commented Dec 30, 2024

Sorry for the mess.

@ykholod ykholod requested a review from ton31337 December 30, 2024 12:40
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

@ton31337 ton31337 merged commit f3daeda into FRRouting:master Jan 1, 2025
11 checks passed
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