zebra: Static routes async notification do not need this test #13875
Conversation
| */ | ||
| if (re->type == ZEBRA_ROUTE_STATIC && |
There was a problem hiding this comment.
did you want to remove this test for the "update" clause above?
There was a problem hiding this comment.
ah I should shouldn't I give me a bit
donaldsharp
force-pushed
the
static_dplane_issues
branch
from
372907d to
994caa7
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: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 i386 part 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO1U18I386-12617/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 1 Topotests Ubuntu 18.04 arm8 part 7: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 7: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12617/artifact/TOPO7U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 7: No useful log foundTopotests Ubuntu 18.04 arm8 part 1: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 1: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12617/artifact/TOPO1U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 1: No useful log foundTopotests Ubuntu 18.04 amd64 part 7: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7U18AMD64-12617/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 7 Topotests 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-12617/artifact/TOPO8U18AMD64/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 8: No useful log foundTopotests Ubuntu 18.04 i386 part 7: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7U18I386-12617/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 7 Topotests debian 10 amd64 part 7: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7DEB10AMD64-12617/test Topology Tests failed for Topotests debian 10 amd64 part 7 Topotests debian 10 amd64 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO8DEB10AMD64-12617/test Topology Tests failed for Topotests debian 10 amd64 part 8 Topotests 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-12617/artifact/TP1U1804AMD64/TopotestDetails/Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TP1U1804AMD64-12617/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-12617/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 8 Topotests debian 10 amd64 part 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO1DEB10AMD64-12617/test Topology Tests failed for Topotests debian 10 amd64 part 1 Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-12617/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests Ubuntu 18.04 amd64 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO8U18ARM64-12617/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 8 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 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO1U18I386-12622/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 1 Topotests Ubuntu 18.04 arm8 part 1: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 1: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12622/artifact/TOPO1U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 1: No useful log foundTopotests Ubuntu 18.04 arm8 part 7: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 7: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12622/artifact/TOPO7U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 7: No useful log foundTopotests Ubuntu 18.04 i386 part 7: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7U18I386-12622/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 7 Topotests Ubuntu 18.04 amd64 part 7: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7U18AMD64-12622/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 7 Topotests 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-12622/artifact/TOPO8U18AMD64/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 8: No useful log foundTopotests debian 10 amd64 part 7: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7DEB10AMD64-12622/test Topology Tests failed for Topotests debian 10 amd64 part 7 Topotests debian 10 amd64 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO8DEB10AMD64-12622/test Topology Tests failed for Topotests debian 10 amd64 part 8 Topotests 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-12622/artifact/TP1U1804AMD64/TopotestDetails/Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TP1U1804AMD64-12622/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-12622/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 8 Topotests debian 10 amd64 part 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO1DEB10AMD64-12622/test Topology Tests failed for Topotests debian 10 amd64 part 1 Topotests Ubuntu 18.04 amd64 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO8U18ARM64-12622/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 8 Successful on other platforms/tests
|
When using asic_offload with an asynchronous notification the rib_route_match_ctx function is testing for distance and tag being correct against the re. Normal route notification for static routes is this(well really all routes): a) zebra dplane generates a ctx to send to the dplane for route install b) dplane installs it in the kernel c) if the dplane_fpm_nl.c module is being used it installs it. d) The context's success code is set to it worked and passes the context back up to zebra for processing. e) Zebra master receives this and checks the distance and tag are correct for static routes and accepts the route and marks it installed. If the operator is using a wait for install mechansim where the dplane is asynchronously sending the result back up at a future time *and* it is using the dplane_fpm_nl.c code where it uses the rt_netlink.c route parsing code, then there is no way to set distance as that we do not pass distance to the kernel. As such static routes were never being properly handled since the re and context would not match and the route would still be marked as queued. Modify the code such that the asynchronous path notification for static routes ignores the distance and tag's as that there is no way to test for this data from that path at this point in time. Signed-off-by: Donald Sharp <[email protected]>
donaldsharp
force-pushed
the
static_dplane_issues
branch
from
994caa7 to
d0123a9
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: SuccessfulBasic Tests: FailedTopotests 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-12644/artifact/TOPO8U18AMD64/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 8: No useful log foundTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-12644/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Successful on other platforms/tests
|
|
ci:rerun |
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 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18AMD64-12651/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 6 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-12651/ This is a comment from an automated CI system. |
When using asic_offload with an asynchronous notification the rib_route_match_ctx function is testing for distance and tag being correct against the re. This is no longer necessary.
Normal route notification for static routes is this(well really all routes): a) zebra dplane generates a ctx to send to the dplane for route install b) dplane installs it in the kernel
c) if the dplane_fpm_nl.c module is being used it installs it. d) The context's success code is set to it worked and passes the context back up to zebra for processing.
e) Zebra master receives this and checks the distance and tag are correct for static routes and accepts the route and marks it installed.
If the operator is using a wait for install mechansim where the dplane is asynchronously sending the result back up at a future time and it is using the dplane_fpm_nl.c code where it uses the rt_netlink.c route parsing code, then there is no way to set distance as that we do not pass distance to the kernel.
As such static routes were never being properly handled since the re and context would not match and the route would still be marked as queued.
This code is historical in nature and is no longer necessary. rib_route_match_ctx is only ever used with dplane notifications. Additionally static routes are now handled more intelligently from staticd and the distance changes are held in staticd not zebra, thus it can be removed.