-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: fix import of evpn routes into vni/vrf/esi after nexthop update #10372
base: master
Are you sure you want to change the base?
Conversation
a2354be
to
078cfb3
Compare
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-2824/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
|
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-2825/ This is a comment from an automated CI system. |
8a086f6
to
2641ab9
Compare
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedDebian 11 amd64 build: Failed (click for details)Debian 11 amd64 build: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsDebian 11 amd64 build: Failed (click for details)Debian 11 amd64 build: No useful log found
|
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-2842/ This is a comment from an automated CI system. |
PR status? |
It is ready and needs a review |
@ton31337 needs anything additional? who can review this? we have crit infra affected by this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I understand the change in evaluate_paths. Can you go into more detail what you expect to work here?
330d979
to
f3fc37a
Compare
If the igp metric changes, I want to update nexthop on EVPN routes. bgp_evpn_import_route() can also do updates on existing routes, not only import.
|
f3fc37a
to
91936d6
Compare
To reproduce the issue:
Apply the configuration PE1 to 3.
PE2
PE3
PE3
PE1
PE3
But the routes remain the same.
Expected output that we get after the patches
|
3d3a0d1
to
e28687d
Compare
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-3329/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
|
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: IncompleteAddresssanitizer topotests part 4: Incomplete(check logs for details)Successful on other platforms/tests
|
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-3330/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Successful on other platforms/tests
|
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-8929/ This is a comment from an automated CI system. |
ci:rerun |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests debian 10 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5DEB10AMD64-8978/test Topology Tests failed for Topotests debian 10 amd64 part 5 Successful on other platforms/tests
|
a30ab4c
to
93e4dda
Compare
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 i386 part 4: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO4U18I386-8988/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 4 Successful on other platforms/tests
CLANG Static Analyzer Summary
2 Static Analyzer issues remaining.See details at |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
93e4dda
to
b9cbc32
Compare
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 arm8 part 9: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14326/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log foundTopotests Ubuntu 18.04 arm8 part 8: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 8: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14326/artifact/TOPO8U18AMD64/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 8: No useful log foundSuccessful on other platforms/tests
|
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14326/ This is a comment from an automated CI system. |
Issue is now fixed in the master. No need for the fix. |
path_valid is the previous path validity state. Setting BGP_PATH_VALID to true if path_valid is false is confusing. Clarify the logic by inverting the if else condition. Cosmetic change only. Signed-off-by: Louis Scalbert <[email protected]>
Related to issue FRRouting#10271. When importing prefixes from EVPN, any changes to the BGP nexthop associated with those prefixes, such as a change in the route metric to the peer, will not trigger a re-evaluation of the imported prefixes. In evaluate_path(), the import is only triggered when the validity of the path changes, which is not the case when, for example, the route metric to the peer changes. Request import or unimport when BGP_PATH_IGP_CHANGED is set but the path validity is unchanged. In install_evpn_route_entry_in_vrf(), do not skip VRF import when BGP_PATH_IGP_CHANGED is set. Fixes: 34ea39b ("bgpd: Check NHT change for triggering EVPN import or unimport") Signed-off-by: Louis Scalbert <[email protected]>
Add an evpn RT5 path selection to bgp_path_selection Signed-off-by: Louis Scalbert <[email protected]>
Reformat bgp_path_selection with black Signed-off-by: Louis Scalbert <[email protected]>
Reopen because the issue is still there without the fix |
b9cbc32
to
b7b5f90
Compare
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-14391/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14391/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log foundSuccessful on other platforms/tests
|
ci:rerun |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 amd64 part 1: Failed (click for details)Topotests Ubuntu 18.04 amd64 part 1: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14403/artifact/TP1U1804AMD64/TopotestDetails/Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TP1U1804AMD64-14403/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 1 Topotests Ubuntu 18.04 i386 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO8U18I386-14403/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 8 Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14403/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log foundSuccessful on other platforms/tests
|
ci:rerun |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Related to issue #10271
After an update of the BGP nexthop (e.g. update of the igp metric), the
EVPN routes are not updated in VNIs, VRFs and ESIs.
Fix the issue.
Signed-off-by: Louis Scalbert [email protected]