NX-OS: Allow ICMP ACL lines without l4 options (#8267)

Fixes #8264
batfish · Apr 26, 2022 · 538dff4 · 538dff4
1 parent 4aebc34
commit 538dff4
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 2 deletions.
diff --git a/projects/batfish/src/main/antlr4/org/batfish/grammar/cisco_nxos/CiscoNxos_ip_access_list.g4 b/projects/batfish/src/main/antlr4/org/batfish/grammar/cisco_nxos/CiscoNxos_ip_access_list.g4
@@ -213,8 +213,8 @@ packet_length
 aclla_icmp
 :
   ICMP acllal3_src_address acllal3_dst_address
-  // NX-OS allows multiple l3 options but only one l4 option, interleaved in any order.
-  acllal3_option* acllal4icmp_option acllal3_option*
+  // NX-OS allows multiple l3 options but at most l4 option, interleaved in any order.
+  acllal3_option* acllal4icmp_option? acllal3_option*
   NEWLINE
 ;
 

diff --git a/projects/batfish/src/test/java/org/batfish/grammar/cisco_nxos/CiscoNxosGrammarTest.java b/projects/batfish/src/test/java/org/batfish/grammar/cisco_nxos/CiscoNxosGrammarTest.java
@@ -178,6 +178,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -487,6 +488,10 @@ public final class CiscoNxosGrammarTest {
     return _aclToBdd.toPermitAndDenyBdds(aclLine).getMatchBdd();
   }
 
+  private @Nonnull BDD toIcmpIfBDD() {
+    return toIfBDD(AclLineMatchExprs.and(matchFragmentOffset(0), matchIpProtocol(IpProtocol.ICMP)));
+  }
+
   private @Nonnull BDD toIcmpIfBDD(AclLineMatchExpr aclLineMatchExpr) {
     return toIfBDD(
         AclLineMatchExprs.and(
@@ -3487,6 +3492,7 @@ public void testIpAccessListConversion() throws IOException {
       assertThat(
           acl.getLines().stream().map(this::toIfBDD).collect(ImmutableList.toImmutableList()),
           contains(
+              toIcmpIfBDD(),
               toIcmpIfBDD(matchIcmpType(0)),
               toIcmpIfBDD(matchIcmp(1, 2)),
               toIcmpIfBDD(matchIcmp(IcmpCode.COMMUNICATION_ADMINISTRATIVELY_PROHIBITED)),
@@ -3979,11 +3985,14 @@ public void testIpAccessListExtraction() {
     }
     {
       IpAccessList acl = vc.getIpAccessLists().get("acl_icmp");
+      // check first line (with null L4 options), and then the rest
+      assertThat(((ActionIpAccessListLine) acl.getLines().get(10L)).getL4Options(), nullValue());
       assertThat(
           acl.getLines().values().stream()
               .filter(ActionIpAccessListLine.class::isInstance) // filter ICMPv6
               .map(ActionIpAccessListLine.class::cast)
               .map(ActionIpAccessListLine::getL4Options)
+              .filter(Objects::nonNull)
               .map(IcmpOptions.class::cast)
               .map(icmpOptions -> immutableEntry(icmpOptions.getType(), icmpOptions.getCode()))
               .collect(ImmutableList.toImmutableList()),

diff --git a/...batfish/src/test/resources/org/batfish/grammar/cisco_nxos/testconfigs/nxos_ip_access_list b/...batfish/src/test/resources/org/batfish/grammar/cisco_nxos/testconfigs/nxos_ip_access_list
@@ -121,6 +121,8 @@ ip access-list acl_common_ip_options_ttl
 
 !!! ICMP options
 ip access-list acl_icmp
+  ! without any options
+  permit icmp any any
   ! match ICMP type 0
   ! type can be 0-255
   permit icmp any any 0