Adding next-hop interface support to static routes for FRR #5698
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 10 of 10 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @kylehoferamzn)
.gitignore, line 47 at r1 (raw file):
# Ignore Tokens *.tokens
nit: can you send that as a separate PR, as not to conflate changes?
projects/batfish/src/main/java/org/batfish/representation/cumulus/StaticRoute.java, line 79 at r1 (raw file):
* supported by the Batfish VI model - "null_interface" */ public String CanonicalizeInterfaceName(String nextHopInterface) {
- why public?
- I think we have checks that require methods to start w/ lower case letter.
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @kylehoferamzn and @progwriter)
.gitignore, line 47 at r1 (raw file):
Previously, progwriter (Victor Heorhiadi) wrote…
nit: can you send that as a separate PR, as not to conflate changes?
NACK - this is not checked in on purpose, since users need to know that those files existing will mess everything up.
|
.gitignore, line 47 at r1 (raw file): Previously, dhalperi (Dan Halperin) wrote…
not ignored on purpose, that is. |
|
"NACK - this is not checked in on purpose, since users need to know that those files existing will mess everything up." Fair play - I'll remove. Re: CanonicalizeInterfaceName Good eye, I worked for years on a Java project where it was this way. Public vs Private - I wasn't sure if this method would get re-used in the future, it seemed somewhat useful. I'll switch it to private and if someone wants to use it in the future they can open it up/refactor. |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
Codecov Report
@@ Coverage Diff @@
## master #5698 +/- ##
============================================
+ Coverage 71.26% 71.37% +0.11%
- Complexity 33497 33637 +140
============================================
Files 2785 2786 +1
Lines 139421 139541 +120
Branches 16810 16781 -29
============================================
+ Hits 99357 99603 +246
+ Misses 32196 32073 -123
+ Partials 7868 7865 -3
|
There was a problem hiding this comment.
Reviewed 5 of 5 files at r3.
Reviewable status:
Previous pull request: #5691
This adds support for syntax such as:
ip route x.x.x.x/x Null0
ip route x.x.x.x/x eth0
ip route x.x.x.x/x blackhole
ip route x.x.x.x/x Eth0
Does NOT support
ip route x.x.x.x/x y.y.y.y/y Null0although that is valid syntax, the parser will continue to error out on that as it did previously.