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.

Sign up for GitHub

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

Jump to bottom

bgp_process work #15572

Merged
merged 11 commits into from
Apr 16, 2024
Merged

bgp_process work #15572

merged 11 commits into from
Apr 16, 2024

Conversation

donaldsharp
Copy link
Member

See individual commits. But setup for best path reordering. Code cleanup etc.

@frrbot frrbot bot added bgp tests Topotests, make check, etc labels Mar 18, 2024
ton31337
ton31337 reviewed Mar 18, 2024
View reviewed changes
bgpd/bgp_route.h
@@ -327,6 +327,7 @@ struct bgp_path_info {
#define BGP_PATH_ACCEPT_OWN (1 << 16)
#define BGP_PATH_MPLSVPN_LABEL_NH (1 << 17)
#define BGP_PATH_MPLSVPN_NH_LABEL_BIND (1 << 18)
#define BGP_PATH_UNSORTED (1 << 19)
Copy link
Member

Choose a reason for hiding this comment

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

Does it mean: that if bgp_path_info is not evaluated yet as the best path, it's marked as UNSORTED, otherwise - SORTED. But it's not related to the actual sorting of the paths in bgp_dest?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes this is just a tag that tracks whether or not a path_info has had best path run over it at all.. Unsorted was just choosen as a name because I think of it that way

riw777
riw777 approved these changes Mar 19, 2024
View reviewed changes
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

@donaldsharp donaldsharp force-pushed the best_path_stuff_sigh branch 2 times, most recently from 0369cb8 to b494b7a Compare March 20, 2024 20:55
@github-actions github-actions bot added size/XL and removed size/L labels Mar 20, 2024
@ton31337
Copy link
Member

Just a curious question: is there any performance degradation or not at all?

@donaldsharp
Copy link
Member Author

I have not done that testing yet. My expectation though is that it should be neutral at this point in time. My next set of improvements is to use the sorted order to do faster multipath calculations.

@donaldsharp donaldsharp force-pushed the best_path_stuff_sigh branch from b494b7a to 3b6459a Compare March 21, 2024 13:03
@mwinter-osr
Copy link
Member

CI:rerun Rerun after fixing git access on CI infra

@donaldsharp donaldsharp force-pushed the best_path_stuff_sigh branch from 3b6459a to 034a318 Compare March 22, 2024 18:43
donaldsharp added 10 commits April 1, 2024 10:24
@donaldsharp
tests: Explicitly call out bgp timers for bgp_evpn_mh test
4d4c396 
Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp
tests: teste_ospf_rte_calc.py uses bgp add pytest mark
829a2e9 
Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp
bgpd: Modify update_evpn_type5_route_entry to include path_info pointer
f4fd5c8 
Modify update_evpn_type5_route_entry to return a pointer to the
struct bgp_path_info modified in this function.  This code
merely follows the standards used in other bgp_evpn.c code
where the update function returns the pointer to the path
info.

Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp
bgpd: Fix indentation problem in bgp_recalculate_afi_safi_bestpaths
1389316 
This is seriously indented.  Let's make it a bit better.

Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp
bgpd: Add a path_info_flags dumper for bgp
19fc4e7 
Add a debug function to allow developers to dump flags
associated with a bgp_path_info in a human readable format.

Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp
bgpd: Add BGP_PATH_UNSORTED for future commits
6c8bfaa 
Add a new flag BGP_PATH_UNSORTED to keep track
of sorted -vs- unsorted path_info's.  Add some
ability to the system to understand when that
flag is set.

Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp
bgpd: Call bgp_process when bgp_path_info_delete is called
04936ab 
bgp_damp.c has an instance of bgp_path_info_delete is called.
Thus setting up the path info for deletion, but since it never
calls bgp_process, it can never be deleted.  This means that in
a dampening situation, after a withdrawal the path_info would
stick around.  Schedule the path for deletion.

Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp
bgpd: Add pi to bgp_process
ab49fc9 
This will allow a consistency of approach to adding/removing
pi's to from the workqueue for processing as well as properly
handling the dest->info pi list more appropriately.

Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp
bgpd: bgp_best_selection is inherently pi based
fca8059 
Currently evpn code calls bgp_best_selection for local
decisions for local tables to figure out what to do.
This is also pi based so let's note that the pi has
been changed before calling bgp_best_selection.

Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp
bgpd: Do not reap, schedule for deletion
6ebb7ad 
Do not reap instead let's schedule for deletion
and let best_path_selection take care of the deletion
as it should.

Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp donaldsharp force-pushed the best_path_stuff_sigh branch from 034a318 to f099883 Compare April 1, 2024 14:26
@donaldsharp
bgpd: Sort the bgp_path_info's
f3575f6 
Currently bgp_path_info's are stored in reverse order
received.  Sort them by the best path ordering.

This will allow for optimizations in the future on
how multipath is done.

Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp donaldsharp force-pushed the best_path_stuff_sigh branch from f099883 to f3575f6 Compare April 1, 2024 18:54
@riw777 riw777 merged commit 1c04344 into FRRouting:master Apr 16, 2024
9 checks passed
@dgsudharsan dgsudharsan mentioned this pull request Dec 17, 2024
Merged
11 tasks
lguohan pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Dec 19, 2024
@dgsudharsan
[FRR]BGP Fixes and optimizations for handling scale scenarios (#21199)
6d876a4 
Why I did it
Added patches from FRR to support scale of BGP neighbors to 256/514. Below are the patches

Patch	FRR Pull request
0069-lib-nexthop-code-should-use-uint16_t-for-nexthop-cou.patch	FRRouting/frr#16967
0070-Allow-16-bit-size-for-nexthops.patch	FRRouting/frr#17023
0071-zebra-Only-notify-dplane-work-pthread-when-needed.patch	FRRouting/frr#17062
0072-Fix-up-improper-handling-of-nexthops-for-nexthop-tra.patch	FRRouting/frr#17076
0073-remove-in6addr-cmp.patch	FRRouting/frr#17312
0074-bgp-best-port-reordering.patch	FRRouting/frr#15572
0075-bgp-mp-info-changes.patch	FRRouting/frr#16961
0076-Optimizations-and-problem-fixing-for-large-scale-ecmp-from-bgp.patch	FRRouting/frr#17229
dgsudharsan added a commit to dgsudharsan/sonic-buildimage that referenced this pull request Dec 23, 2024
@dgsudharsan
[FRR]BGP Fixes and optimizations for handling scale scenarios (sonic-…
3b51033 
…net#21199)

Why I did it
Added patches from FRR to support scale of BGP neighbors to 256/514. Below are the patches

Patch	FRR Pull request
0069-lib-nexthop-code-should-use-uint16_t-for-nexthop-cou.patch	FRRouting/frr#16967
0070-Allow-16-bit-size-for-nexthops.patch	FRRouting/frr#17023
0071-zebra-Only-notify-dplane-work-pthread-when-needed.patch	FRRouting/frr#17062
0072-Fix-up-improper-handling-of-nexthops-for-nexthop-tra.patch	FRRouting/frr#17076
0073-remove-in6addr-cmp.patch	FRRouting/frr#17312
0074-bgp-best-port-reordering.patch	FRRouting/frr#15572
0075-bgp-mp-info-changes.patch	FRRouting/frr#16961
0076-Optimizations-and-problem-fixing-for-large-scale-ecmp-from-bgp.patch	FRRouting/frr#17229
@dgsudharsan dgsudharsan mentioned this pull request Dec 23, 2024
Open
11 tasks
github-actions bot pushed a commit to bradh352/sonic-buildimage that referenced this pull request Jan 2, 2025
@dgsudharsan @bradh352
[FRR]BGP Fixes and optimizations for handling scale scenarios (sonic-…
8878242 
…net#21199)

Why I did it
Added patches from FRR to support scale of BGP neighbors to 256/514. Below are the patches

Patch	FRR Pull request
0069-lib-nexthop-code-should-use-uint16_t-for-nexthop-cou.patch	FRRouting/frr#16967
0070-Allow-16-bit-size-for-nexthops.patch	FRRouting/frr#17023
0071-zebra-Only-notify-dplane-work-pthread-when-needed.patch	FRRouting/frr#17062
0072-Fix-up-improper-handling-of-nexthops-for-nexthop-tra.patch	FRRouting/frr#17076
0073-remove-in6addr-cmp.patch	FRRouting/frr#17312
0074-bgp-best-port-reordering.patch	FRRouting/frr#15572
0075-bgp-mp-info-changes.patch	FRRouting/frr#16961
0076-Optimizations-and-problem-fixing-for-large-scale-ecmp-from-bgp.patch	FRRouting/frr#17229
github-actions bot pushed a commit to bradh352/sonic-buildimage that referenced this pull request Jan 2, 2025
@dgsudharsan @bradh352
[FRR]BGP Fixes and optimizations for handling scale scenarios (sonic-…
93f4bbc 
…net#21199)

Why I did it
Added patches from FRR to support scale of BGP neighbors to 256/514. Below are the patches

Patch	FRR Pull request
0069-lib-nexthop-code-should-use-uint16_t-for-nexthop-cou.patch	FRRouting/frr#16967
0070-Allow-16-bit-size-for-nexthops.patch	FRRouting/frr#17023
0071-zebra-Only-notify-dplane-work-pthread-when-needed.patch	FRRouting/frr#17062
0072-Fix-up-improper-handling-of-nexthops-for-nexthop-tra.patch	FRRouting/frr#17076
0073-remove-in6addr-cmp.patch	FRRouting/frr#17312
0074-bgp-best-port-reordering.patch	FRRouting/frr#15572
0075-bgp-mp-info-changes.patch	FRRouting/frr#16961
0076-Optimizations-and-problem-fixing-for-large-scale-ecmp-from-bgp.patch	FRRouting/frr#17229
github-actions bot pushed a commit to bradh352/sonic-buildimage that referenced this pull request Jan 2, 2025
@dgsudharsan @bradh352
[FRR]BGP Fixes and optimizations for handling scale scenarios (sonic-…
99e9c3d 
…net#21199)

Why I did it
Added patches from FRR to support scale of BGP neighbors to 256/514. Below are the patches

Patch	FRR Pull request
0069-lib-nexthop-code-should-use-uint16_t-for-nexthop-cou.patch	FRRouting/frr#16967
0070-Allow-16-bit-size-for-nexthops.patch	FRRouting/frr#17023
0071-zebra-Only-notify-dplane-work-pthread-when-needed.patch	FRRouting/frr#17062
0072-Fix-up-improper-handling-of-nexthops-for-nexthop-tra.patch	FRRouting/frr#17076
0073-remove-in6addr-cmp.patch	FRRouting/frr#17312
0074-bgp-best-port-reordering.patch	FRRouting/frr#15572
0075-bgp-mp-info-changes.patch	FRRouting/frr#16961
0076-Optimizations-and-problem-fixing-for-large-scale-ecmp-from-bgp.patch	FRRouting/frr#17229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ton31337 ton31337 ton31337 approved these changes

@riw777 riw777 riw777 approved these changes

Assignees
No one assigned
Labels
bgp master size/XL tests Topotests, make check, etc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@donaldsharp @ton31337 @mwinter-osr @riw777