-
Notifications
You must be signed in to change notification settings - Fork 229
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
Enhancemets to Cumulus interface definitions. #5668
Enhancemets to Cumulus interface definitions. #5668
Conversation
…d "interface". Make AFI (ipv4.unicast) optional in FRR configs by updating grammar files used for FRR.
…ure/frr_enhancements
…t IP assignment for interfaces using "ip address".
Codecov Report
@@ Coverage Diff @@
## master #5668 +/- ##
============================================
+ Coverage 73.80% 73.82% +0.01%
- Complexity 32934 32963 +29
============================================
Files 2669 2674 +5
Lines 131845 131960 +115
Branches 15714 15724 +10
============================================
+ Hits 97313 97413 +100
- Misses 26855 26861 +6
- Partials 7677 7686 +9
|
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.
Reviewed 3 of 5 files at r1, 2 of 3 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @raveranj)
projects/batfish/src/main/antlr4/org/batfish/grammar/cumulus_interfaces/CumulusInterfacesParser.g4, line 32 at r2 (raw file):
IFACE interface_name ( si_inet
If what follow iface
and interface
is the same, please just use one rule with (IFACE | INTERFACE)
at the beginning. It will be much more maintainable, and won't require changes to CumulusInterfacesConfigurationBuilder
projects/batfish/src/test/java/org/batfish/grammar/cumulus_concatenated/CumulusConcatenatedGrammarTest.java, line 637 at r2 (raw file):
@Test public void testInterfaceDefinition() throws IOException {
nit: whitespace
projects/batfish/src/test/java/org/batfish/grammar/cumulus_concatenated/CumulusConcatenatedGrammarTest.java, line 649 at r2 (raw file):
.getExportPolicySources() .size(), equalTo(1));
For better error messages, ditch .size()
and use Matcher
hasSize
instead of equalTo
.
projects/batfish/src/test/java/org/batfish/grammar/cumulus_concatenated/CumulusConcatenatedGrammarTest.java, line 637 at r2 (raw file): Previously, arifogel (Ari Fogel) wrote…
Done. |
projects/batfish/src/main/antlr4/org/batfish/grammar/cumulus_interfaces/CumulusInterfacesParser.g4, line 32 at r2 (raw file): Previously, arifogel (Ari Fogel) wrote…
Done. De-duped to (IFACE | INTERFACE). |
projects/batfish/src/test/java/org/batfish/grammar/cumulus_concatenated/CumulusConcatenatedGrammarTest.java, line 649 at r2 (raw file): Previously, arifogel (Ari Fogel) wrote…
Done. |
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.
Reviewed 2 of 3 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved
BlockReservedAddressesAtInternet: allow documentation IPs (batfish#5666) This is crucial to keeping examples working. Instead, nudge away from the doc subnets when picking flows. TBD how we'll resolve permanently, but this is a good middle. BDDUtils: move isAssignment internally (batfish#5667) No need to materialize the inner nodes as BDD objects, reducing overhead and making free on the outer BDDs effective. CompletionMetadataUtils: memoize work (batfish#5670) fix a typo on the README.md (batfish#5671) AWS: Handle case where entire VPC is owned by one covering subnet (batfish#5672) Batfish: clean up function names and postprocess in only one place (batfish#5675) Interface: add setZoneName to builder, relax address setting (batfish#5674) Bazel: upgrade to new 3.0.0 release (batfish#5677) checkstyle: allow underscores in variable names (batfish#5678) PAN: compute interface bandwidths (batfish#5676) PaloAlto ethernet interfaces have bandwidths that are either 1Gb/s or 10Gb/s based on interface number. Add setting, tests, and bandwidth inheritance to subinterfaces. PAN: fix 10GbE interface bandwidths (batfish#5680) It's subinterface 21, not interface 21. https://knowledgebase.paloaltonetworks.com/KCSArticleDetail?id=kA10g000000ClssCAC Enhancemets to Cumulus interface definitions. (batfish#5668) * Add support in Batfish to parse FRR interface definitions with keyword "interface". Making AFI optional for Cumulus. (batfish#5669) * Relax AFI for BGP statements in Cumulus. Batfish: aggregate interfaces should ignore VRF (batfish#5681) PAN: support aggregate-ethernet interfaces (batfish#5682) Fix batfish#5598 More changes. Removed extraneous files. Fixed bug with neighbor inheriting localAs from peer-group. Fixed formatting.
No description provided.