ospfd: use rib metric as the base for set metric +/- #13166
Conversation
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-10497/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests debian 10 amd64 part 6: Failed (click for details)Topotests debian 10 amd64 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10497/artifact/TOPO6DEB10AMD64/TopotestLogs/ Topotests debian 10 amd64 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10497/artifact/TOPO6DEB10AMD64/TopotestDetails/ Topotests debian 10 amd64 part 6: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-10497/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests debian 10 amd64 part 6: Failed (click for details)Topotests debian 10 amd64 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10497/artifact/TOPO6DEB10AMD64/TopotestLogs/ Topotests debian 10 amd64 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10497/artifact/TOPO6DEB10AMD64/TopotestDetails/ Topotests debian 10 amd64 part 6: No useful log found |
|
I think the only danger here would be routing loops caused by changing metrics ... I can't seem to find one, though, so I think this is fine from a operational perspective. |
|
Just skimming over the commit here... Does this also cover the Type-7 LSA? I saw the AS-External (Type-5) function modified, but didn't look close enough to see if that is reused for NSSA-External LSAs |
Jafaral
force-pushed
the
ospf-external-metric
branch
from
fcbe1ac to
75caddc
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 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-10797/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-10797/test Topology Tests failed for Topotests debian 10 amd64 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-10797/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log foundTopotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-10797/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9 Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-10797/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-10797/test Topology Tests failed for Topotests debian 10 amd64 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-10797/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log foundTopotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-10797/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9 |
Jafaral
force-pushed
the
ospf-external-metric
branch
from
75caddc to
7c66ae8
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 debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-10876/test Topology Tests failed for Topotests debian 10 amd64 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-10876/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: 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-10876/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-10876/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9 Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-10876/test Topology Tests failed for Topotests debian 10 amd64 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-10876/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: 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-10876/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-10876/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9 |
Jafaral
force-pushed
the
ospf-external-metric
branch
from
7c66ae8 to
dc9195e
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-10888/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
Jafaral
force-pushed
the
ospf-external-metric
branch
from
dc9195e to
f207ab2
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 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-10898/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-10898/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 |
|
ci:rerun |
| ei->route_map_set.metric = OSPF_LS_INFINITY; | ||
| if ((uint32_t)ROUTEMAP_METRIC(ei) < ei->min_metric) | ||
| ROUTEMAP_METRIC(ei) = ei->min_metric; | ||
| if ((uint32_t)ROUTEMAP_METRIC(ei) > ei->max_metric) |
There was a problem hiding this comment.
Can it be higher than OSPF_LS_INFINITY?
There was a problem hiding this comment.
currently no, but now that you say it, I want to allow higher values and trim excess here since the added route map rule might be used with other protocol in the future.
There was a problem hiding this comment.
I double checked. The code is already doing the right thing. I.e, in the context of OSPF the limit is always trimmed to OSPF_LS_INFINITY. The value in the route map is still stored as-is.
| static void *route_set_min_metric_compile(const char *arg) | ||
| { | ||
|
|
||
| uint32_t *min_metric; |
There was a problem hiding this comment.
Why not just uint32_t min_metric?
There was a problem hiding this comment.
just being consistent with other functions in vicinity! :)
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-10911/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
When using route maps with external routes in OSPF as follows: ``` set metric +10 ``` The current behavior is to use the default ospf metric as the base and then add to 10 to it. The behavior isn't useful as-is. A value 30 (20 dfeault + 10) can be set directly instead. the behavior is also not consistent with bgp. bgp does use the rib metric in this case as the base. The current behavior also doesn't allow the metric to accumulate when crossing different routing domains such as vrfs causing the metric to reset every time the route enters a new vrf with a new ospf network. This PR changes the behavior such that the rib metric is used as a base for ospf exteral routes when used with `set metric -/+` Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Jafaral
force-pushed
the
ospf-external-metric
branch
from
f207ab2 to
08a0d90
Compare
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Jafaral
force-pushed
the
ospf-external-metric
branch
from
08a0d90 to
f24dfcc
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: FailedUbuntu 18.04 ppc64le build: Failed (click for details)Ubuntu 18.04 ppc64le build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10935/artifact/U1804PPC64LEBUILD/frr.xref.xz/frr.xref.xz Ubuntu 18.04 ppc64le build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10935/artifact/U1804PPC64LEBUILD/config.log/config.log.gz Ubuntu 18.04 ppc64le build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-10935/artifact/U1804PPC64LEBUILD/config.status/config.status Ubuntu 18.04 ppc64le build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10935/artifact/U1804PPC64LEBUILD/ErrorLog/ Ubuntu 18.04 ppc64le build: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsUbuntu 18.04 ppc64le build: Failed (click for details)Ubuntu 18.04 ppc64le build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10935/artifact/U1804PPC64LEBUILD/frr.xref.xz/frr.xref.xz Ubuntu 18.04 ppc64le build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10935/artifact/U1804PPC64LEBUILD/config.log/config.log.gz Ubuntu 18.04 ppc64le build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-10935/artifact/U1804PPC64LEBUILD/config.status/config.status Ubuntu 18.04 ppc64le build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10935/artifact/U1804PPC64LEBUILD/ErrorLog/ Ubuntu 18.04 ppc64le 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-10934/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
|
no results ... trying again |
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-10944/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-10944/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 |
|
ci:rerun failed |
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-10954/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
When using route maps with external routes in OSPF as follows:
The current behavior is to use the default ospf metric as the base and then. and add to 10 to it.
The behavior isn't useful as-is and since a value 30 (20, the ospf default metric +10) can be set
directly instead. the behavior is also not consistent with bgp for example where bgp does use the
rib metric in this case as the base. The current behavior also doesn't allow the metric to accumulate
when crossing different routing domains such as vrfs causing the metric to reset every time the
route enters a new vrf with a new ospf network.
This PR changes the behavior such so that the rib metric is as a base for external routes
when used with route maps set metric -/+
This PR also adds two new route map commands:
to set the minimum and the maximum metric for the route.