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: add multiple paths support for draft ietf bgp4v2 in nlriTable #14889

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

fdumontet6WIND
Copy link
Contributor

There is no support for dumping multiple paths for the same prefix.
The current implementation only takes the first available entry.

Fix this by walking over the list of available paths, ordered by peer.
The nlri index is set gradually for each path.

@frrbot frrbot bot added bgp tests Topotests, make check, etc labels Nov 28, 2023
@fdumontet6WIND fdumontet6WIND force-pushed the snmpv2 branch 4 times, most recently from d91c4e9 to 6fe1c69 Compare November 28, 2023 11:25
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

@fdumontet6WIND
Copy link
Contributor Author

ci:rerun

tests/topotests/bgp_snmp_bgp4v2mib/r3/bgpd.conf Outdated Show resolved Hide resolved
tests/topotests/bgp_snmp_bgp4v2mib/r3/bgpd.conf Outdated Show resolved Hide resolved
tests/topotests/bgp_snmp_bgp4v2mib/rr/bgpd.conf Outdated Show resolved Hide resolved
bgpd/bgp_snmp_bgp4v2.c Outdated Show resolved Hide resolved
bgpd/bgp_snmp_bgp4v2.c Outdated Show resolved Hide resolved
bgpd/bgp_snmp_bgp4v2.c Outdated Show resolved Hide resolved
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.

Also styling warning:

			
warning: 1 line adds whitespace errors.
Report for bgp_snmp_bgp4v2.c | 2 issues

< ERROR: trailing whitespace
< 717: FILE: /tmp/f1-31970/bgp_snmp_bgp4v2.c:717:

bgpd/bgp_snmp_bgp4v2.c Outdated Show resolved Hide resolved
@fdumontet6WIND
Copy link
Contributor Author

Test nor working with ARM CPU. need to change the test paradigm.

There is no support for dumping multiple paths for the same prefix.
The current implementation only takes the first available entry.

Fix this by walking over the list of available paths, ordered by peer.
The nlri index is set gradually for each path.

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

ton31337 commented Dec 5, 2023

ci:rerun not all checks started up

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

@fdumontet6WIND fdumontet6WIND force-pushed the snmpv2 branch 2 times, most recently from cd32b14 to 797dc3f Compare December 5, 2023 13:16
multi path support by snmp implies change in configuration and expected
tests results.

ipv6 trap test output is now ordered to avoid radom result due to
timeline.

Signed-off-by: Francois Dumontet <[email protected]>
Signed-off-by: Philippe Guibert <[email protected]>
@fdumontet6WIND
Copy link
Contributor Author

ci:rerun

@fdumontet6WIND
Copy link
Contributor Author

ci:rerun failure not related to pull request

@ton31337 ton31337 merged commit 6b79b56 into FRRouting:master Dec 8, 2023
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bgp master rebase PR needs rebase size/XL tests Topotests, make check, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants