Support Cisco ASA subinterface vlan declaration

[arifogel]

Fixes #3629

[batfish-bot]

This change is 

[codecov]

Codecov Report

Merging #3631 into master will decrease coverage by 0.9%.
The diff coverage is 85%.

@@             Coverage Diff              @@
##             master    #3631      +/-   ##
============================================
- Coverage     74.27%   73.36%   -0.91%     
+ Complexity    25348    24601     -747     
============================================
  Files          2110     2110              
  Lines        104780   101786    -2994     
  Branches      13054    12064     -990     
============================================
- Hits          77822    74674    -3148     
- Misses        21526    21738     +212     
+ Partials       5432     5374      -58

Impacted Files	Coverage Δ	Complexity Δ
...org/batfish/grammar/cisco/CiscoCombinedParser.java	89.47% <100%> (+0.58%)	10 <0> (+1)	⬆️
...tfish/representation/cisco/CiscoConfiguration.java	84.01% <100%> (-2.65%)	529 <0> (-98)
...fish/grammar/cisco/CiscoControlPlaneExtractor.java	66.38% <100%> (-8.5%)	1430 <3> (-515)
...va/org/batfish/representation/cisco/Interface.java	90.95% <100%> (-1.24%)	125 <2> (-94)
...batfish/grammar/cisco/parsing/CiscoBaseParser.java	72.72% <72.72%> (+10.22%)	4 <4> (+2)	⬆️
...atfish/dataplane/protocols/OspfProtocolHelper.java	28.3% <0%> (-47.17%)	10% <0%> (-18%)
...java/org/batfish/dataplane/ibdp/VirtualRouter.java	63.03% <0%> (-23.35%)	247% <0%> (-70%)
...ar/cumulus_nclu/parsing/CumulusNcluBaseParser.java	53.84% <0%> (-20.35%)	5% <0%> (ø)
...c/main/java/org/batfish/dataplane/rib/RibTree.java	78.72% <0%> (-10.64%)	23% <0%> (-1%)
... and 12 more

[sfraint]

Reviewed 11 of 11 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @arifogel and @dhalperi)

a discussion (no related file):
I guess this isn't an issue specific to vlan parsing, but if vlan ### is also valid at the top-level, could this cause ambiguity issues when a top-level vlan declaration occurs immediately after an interface declaration?
e.g.

interface foo
  vlan 1
  ip address ...
vlan 1001

---

[arifogel]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dhalperi and @sfraint)

a discussion (no related file):

Previously, sfraint (Spencer Fraint) wrote…

I guess this isn't an issue specific to vlan parsing, but if vlan ### is also valid at the top-level, could this cause ambiguity issues when a top-level vlan declaration occurs immediately after an interface declaration?
e.g.

interface foo
  vlan 1
  ip address ...
vlan 1001

It's not valid at the top level in ASA. I could add a guard there. We desperately need to split off these parsers :(

---

[arifogel]

Reviewable status: 9 of 12 files reviewed, 1 unresolved discussion (waiting on @dhalperi and @sfraint)

a discussion (no related file):

Previously, arifogel (Ari Fogel) wrote…

It's not valid at the top level in ASA. I could add a guard there. We desperately need to split off these parsers :(

I added a guard so there is no ambiguity.

---

[sfraint]

Reviewed 3 of 3 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @arifogel and @dhalperi)

a discussion (no related file):

Previously, arifogel (Ari Fogel) wrote…

I added a guard so there is no ambiguity.

👍

what about on a non ASA device if it encounters something similar:

interface foo
  ip address ...
vlan 1001

---

[arifogel]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dhalperi and @sfraint)

a discussion (no related file):

Previously, sfraint (Spencer Fraint) wrote…

👍

what about on a non ASA device if it encounters something similar:

interface foo
  ip address ...
vlan 1001

This will not work properly, unfortunately. To fix, we need to either split off ASA parser, or split off all ASA interface stanzas into separate top-level s_interface_asa rule.
Let's defer, as this is unlikely to be a problem in practice.

---

[sfraint]

Reviewed 1 of 1 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dhalperi)