-
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
Fix Juniper ethernet switching parsing #5860
Conversation
arifogel
commented
Jun 4, 2020
- support access vlan specified as integer
- change VS model to look more like configs
- move guts of switching conversion to JuniperConfiguration
- support access vlan specified as integer - change VS model to look more like configs - move guts of switching conversion to JuniperConfiguration
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 9 of 9 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @arifogel)
projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java, line 4283 at r1 (raw file):
Quoted 19 lines of code…
if (ctx.range() != null) { IntegerSpace range = IntegerSpace.unionOf(toRange(ctx.range()).toArray(new SubRange[] {})); _currentInterfaceOrRange.getEthernetSwitching().getVlanMembers().add(new VlanRange(range)); } else if (ctx.name != null) { String name = ctx.name.getText(); _configuration.referenceStructure(VLAN, name, INTERFACE_VLAN, getLine(ctx.name.getStart())); _currentInterfaceOrRange.getEthernetSwitching().getVlanMembers().add(new VlanReference(name)); } }
double-checking: the config can say members a 1 b
?
projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java, line 4284 at r1 (raw file):
_currentInterfaceOrRange.getEthernetSwitching().getVlanMembers().add(new VlanReference(name)); } }
else {
// ife_vlan has ALL selected
}
?
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.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @arifogel and @dhalperi)
projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java, line 4283 at r1 (raw file):
Previously, dhalperi (Dan Halperin) wrote…
if (ctx.range() != null) { IntegerSpace range = IntegerSpace.unionOf(toRange(ctx.range()).toArray(new SubRange[] {})); _currentInterfaceOrRange.getEthernetSwitching().getVlanMembers().add(new VlanRange(range)); } else if (ctx.name != null) { String name = ctx.name.getText(); _configuration.referenceStructure(VLAN, name, INTERFACE_VLAN, getLine(ctx.name.getStart())); _currentInterfaceOrRange.getEthernetSwitching().getVlanMembers().add(new VlanReference(name)); } }
double-checking: the config can say
members a 1 b
?
It can have e.g. members [a 1-2,5 b]
, which is really 3 separate lines
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.
Reviewable status: 6 of 11 files reviewed, 1 unresolved discussion (waiting on @dhalperi)
projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java, line 4284 at r1 (raw file):
Previously, dhalperi (Dan Halperin) wrote…
else { // ife_vlan has ALL selected }
?
done
Codecov Report
@@ Coverage Diff @@
## master #5860 +/- ##
============================================
- Coverage 71.99% 71.98% -0.01%
- Complexity 33882 33894 +12
============================================
Files 2795 2799 +4
Lines 139542 139588 +46
Branches 16765 16770 +5
============================================
+ Hits 100462 100482 +20
- Misses 31210 31230 +20
- Partials 7870 7876 +6
|
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 10 of 10 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
- support access vlan specified as integer - change VS model to look more like configs - move guts of switching conversion to JuniperConfiguration