batfish · dhalperi · Jun 4, 2020 · Jun 4, 2020 · Jun 4, 2020 · Jun 4, 2020
diff --git a/projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java b/projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java
@@ -135,6 +135,7 @@
 import org.batfish.datamodel.IcmpType;
 import org.batfish.datamodel.IkeAuthenticationMethod;
 import org.batfish.datamodel.IkeHashingAlgorithm;
+import org.batfish.datamodel.IntegerSpace;
 import org.batfish.datamodel.Ip;
 import org.batfish.datamodel.Ip6;
 import org.batfish.datamodel.IpProtocol;
@@ -782,6 +783,8 @@
 import org.batfish.representation.juniper.TcpNoFlag;
 import org.batfish.representation.juniper.TcpSynFin;
 import org.batfish.representation.juniper.Vlan;
+import org.batfish.representation.juniper.VlanRange;
+import org.batfish.representation.juniper.VlanReference;
 import org.batfish.representation.juniper.Zone;
 
 public class ConfigurationBuilder extends FlatJuniperParserBaseListener {
@@ -2358,11 +2361,7 @@ public void enterI_unit(I_unitContext ctx) {
 
   @Override
   public void enterIf_ethernet_switching(If_ethernet_switchingContext ctx) {
-    if (_currentInterfaceOrRange.getSwitchportMode() == SwitchportMode.NONE) {
-      // JunOS docs say family ethernet-switching are access mode by default.
-      //   use enter rule so that an inner port-mode or interface-mode command will override.
-      _currentInterfaceOrRange.setSwitchportMode(SwitchportMode.ACCESS);
-    }
+    _currentInterfaceOrRange.initEthernetSwitching();
   }
 
   @Override
@@ -4252,9 +4251,9 @@ public void exitIfe_filter(Ife_filterContext ctx) {
   @Override
   public void exitIfe_interface_mode(Ife_interface_modeContext ctx) {
     if (ctx.ACCESS() != null) {
-      _currentInterfaceOrRange.setSwitchportMode(SwitchportMode.ACCESS);
+      _currentInterfaceOrRange.getEthernetSwitching().setSwitchportMode(SwitchportMode.ACCESS);
     } else if (ctx.TRUNK() != null) {
-      _currentInterfaceOrRange.setSwitchportMode(SwitchportMode.TRUNK);
+      _currentInterfaceOrRange.getEthernetSwitching().setSwitchportMode(SwitchportMode.TRUNK);
     } else {
       todo(ctx);
     }
@@ -4268,29 +4267,19 @@ public void exitIfe_native_vlan_id(Ife_native_vlan_idContext ctx) {
   @Override
   public void exitIfe_port_mode(Ife_port_modeContext ctx) {
     if (ctx.TRUNK() != null) {
-      _currentInterfaceOrRange.setSwitchportMode(SwitchportMode.TRUNK);
+      _currentInterfaceOrRange.getEthernetSwitching().setSwitchportMode(SwitchportMode.TRUNK);
     }
   }
 
   @Override
   public void exitIfe_vlan(Ife_vlanContext ctx) {
-    if (_currentInterfaceOrRange.getSwitchportMode() == SwitchportMode.TRUNK) {
-      if (ctx.range() != null) {
-        List<SubRange> subRanges = toRange(ctx.range());
-        subRanges.forEach(subRange -> _currentInterfaceOrRange.getAllowedVlans().add(subRange));
-      } else if (ctx.name != null) {
-        String name = ctx.name.getText();
-        _currentInterfaceOrRange.getAllowedVlanNames().add(name);
-        _configuration.referenceStructure(VLAN, name, INTERFACE_VLAN, getLine(ctx.name.getStart()));
-      }
+    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) {
-      // SwitchPortMode here can be ACCESS or NONE, overwrite both with ACCESS(considered default)
-      _currentInterfaceOrRange.setSwitchportMode(SwitchportMode.ACCESS);
       String name = ctx.name.getText();
-      _currentInterfaceOrRange.setAccessVlan(name);
       _configuration.referenceStructure(VLAN, name, INTERFACE_VLAN, getLine(ctx.name.getStart()));
-    } else {
-      todo(ctx, "Unexpected vlan members range on non-trunk interface");
+      _currentInterfaceOrRange.getEthernetSwitching().getVlanMembers().add(new VlanReference(name));
     }
   }
 

diff --git a/projects/batfish/src/main/java/org/batfish/representation/juniper/EthernetSwitching.java b/projects/batfish/src/main/java/org/batfish/representation/juniper/EthernetSwitching.java
@@ -0,0 +1,42 @@
+package org.batfish.representation.juniper;
+
+import java.io.Serializable;
+import java.util.LinkedList;
+import java.util.List;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import javax.annotation.ParametersAreNonnullByDefault;
+import org.batfish.datamodel.SwitchportMode;
+
+/** Interface settings for the ethernet-switching family. */
+@ParametersAreNonnullByDefault
+public final class EthernetSwitching implements Serializable {
+
+  public EthernetSwitching() {
+    _vlanMembers = new LinkedList<>();
+  }
+
+  public @Nullable Integer getNativeVlan() {
+    return _nativeVlan;
+  }
+
+  public void setNativeVlan(int nativeVlan) {
+    _nativeVlan = nativeVlan;
+  }
+
+  public @Nullable SwitchportMode getSwitchportMode() {
+    return _switchportMode;
+  }
+
+  public void setSwitchportMode(SwitchportMode switchportMode) {
+    _switchportMode = switchportMode;
+  }
+
+  public @Nonnull List<VlanMember> getVlanMembers() {
+    return _vlanMembers;
+  }
+
+  private @Nullable Integer _nativeVlan;
+  private @Nullable SwitchportMode _switchportMode;
+  private final @Nonnull List<VlanMember> _vlanMembers;
+}
diff --git a/projects/batfish/src/main/java/org/batfish/representation/juniper/Interface.java b/projects/batfish/src/main/java/org/batfish/representation/juniper/Interface.java
@@ -18,7 +18,6 @@
 import org.batfish.datamodel.IsoAddress;
 import org.batfish.datamodel.SubRange;
 import org.batfish.datamodel.SwitchportEncapsulationType;
-import org.batfish.datamodel.SwitchportMode;
 import org.batfish.datamodel.VrrpGroup;
 
 public class Interface implements Serializable {
@@ -43,6 +42,16 @@ public static double getDefaultBandwidthByName(String name) {
     return _ospfNeighbors;
   }
 
+  public @Nullable EthernetSwitching getEthernetSwitching() {
+    return _ethernetSwitching;
+  }
+
+  public void initEthernetSwitching() {
+    if (_ethernetSwitching == null) {
+      _ethernetSwitching = new EthernetSwitching();
+    }
+  }
+
   /** Represents the type of interface for OSPF */
   public enum OspfInterfaceType {
     /** This is not an explicit type -- assumed by default */
@@ -57,7 +66,6 @@ public enum OspfInterfaceType {
     P2P
   }
 
-  private String _accessVlan;
   private boolean _active;
   private Set<Ip> _additionalArpIps;
   private final Set<ConcreteInterfaceAddress> _allAddresses;
@@ -69,6 +77,7 @@ public enum OspfInterfaceType {
   private double _bandwidth;
   private String _description;
   private boolean _defined;
+  private @Nullable EthernetSwitching _ethernetSwitching;
   private @Nullable String _incomingFilter;
   private @Nullable List<String> _incomingFilterList;
   private transient boolean _inherited;
@@ -93,7 +102,6 @@ public enum OspfInterfaceType {
   private ConcreteInterfaceAddress _primaryAddress;
   @Nullable private String _redundantParentInterface;
   private String _routingInstance;
-  private SwitchportMode _switchportMode;
   private SwitchportEncapsulationType _switchportTrunkEncapsulation;
   private final SortedMap<String, Interface> _units;
   private final SortedMap<Integer, VrrpGroup> _vrrpGroups;
@@ -109,7 +117,6 @@ public Interface(String name) {
     _name = name;
     _ospfInterfaceType = OspfInterfaceType.BROADCAST;
     _ospfNeighbors = new HashSet<>();
-    _switchportMode = SwitchportMode.NONE;
     _switchportTrunkEncapsulation = SwitchportEncapsulationType.DOT1Q;
     _allowedVlans = new LinkedList<>();
     _allowedVlanNames = new LinkedList<>();
@@ -122,10 +129,6 @@ public String get8023adInterface() {
     return _agg8023adInterface;
   }
 
-  public String getAccessVlan() {
-    return _accessVlan;
-  }
-
   public boolean getActive() {
     return _active;
   }
@@ -263,14 +266,6 @@ public String getRoutingInstance() {
     return _routingInstance;
   }
 
-  public SwitchportMode getSwitchportMode() {
-    return _switchportMode;
-  }
-
-  public SwitchportEncapsulationType getSwitchportTrunkEncapsulation() {
-    return _switchportTrunkEncapsulation;
-  }
-
   public Map<String, Interface> getUnits() {
     return _units;
   }
@@ -338,10 +333,6 @@ public void set8023adInterface(String interfaceName) {
     _agg8023adInterface = interfaceName;
   }
 
-  public void setAccessVlan(String vlan) {
-    _accessVlan = vlan;
-  }
-
   public void setActive(boolean active) {
     _active = active;
   }
@@ -452,14 +443,6 @@ public void setRoutingInstance(String routingInstance) {
     _routingInstance = routingInstance;
   }
 
-  public void setSwitchportMode(SwitchportMode switchportMode) {
-    _switchportMode = switchportMode;
-  }
-
-  public void setSwitchportTrunkEncapsulation(SwitchportEncapsulationType encapsulation) {
-    _switchportTrunkEncapsulation = encapsulation;
-  }
-
   public void setTcpMss(@Nullable Integer tcpMss) {
     _tcpMss = tcpMss;
   }

diff --git a/projects/batfish/src/main/java/org/batfish/representation/juniper/JuniperConfiguration.java b/projects/batfish/src/main/java/org/batfish/representation/juniper/JuniperConfiguration.java
@@ -315,10 +315,13 @@ private static String communityRegexToJavaRegex(String regex) {
 
   private LogicalSystem _masterLogicalSystem;
 
+  private final Map<String, VlanReference> _indirectAccessPorts;
+
   public JuniperConfiguration() {
     _logicalSystems = new TreeMap<>();
     _masterLogicalSystem = new LogicalSystem("");
     _nodeDevices = new TreeMap<>();
+    _indirectAccessPorts = new HashMap<>();
   }
 
   @Nonnull
@@ -1787,44 +1790,83 @@ private org.batfish.datamodel.Interface toInterface(Interface iface) {
     }
     newIface.setAllAddresses(iface.getAllAddresses());
     newIface.setActive(iface.getActive());
-    if (iface.getSwitchportMode() == SwitchportMode.ACCESS && iface.getAccessVlan() != null) {
-      Vlan vlan = _masterLogicalSystem.getNamedVlans().get(iface.getAccessVlan());
-      if (vlan != null) {
-        newIface.setAccessVlan(vlan.getVlanId());
-      }
-    }
-    if (iface.getSwitchportMode() == SwitchportMode.TRUNK) {
-      IntegerSpace.Builder vlanIdsBuilder = IntegerSpace.builder();
-      Stream.concat(
-              iface.getAllowedVlanNames().stream()
-                  .map(_masterLogicalSystem.getNamedVlans()::get)
-                  .filter(Objects::nonNull) // named vlan must exist
-                  .map(Vlan::getVlanId)
-                  .filter(Objects::nonNull) // named vlan must have assigned numeric id
-                  .map(SubRange::new),
-              iface.getAllowedVlans().stream())
-          .forEach(vlanIdsBuilder::including);
-
-      newIface.setAllowedVlans(vlanIdsBuilder.build());
-      // default is no native vlan, untagged are dropped.
-      // https://www.juniper.net/documentation/en_US/junos/topics/reference/configuration-statement/native-vlan-id-edit-interfaces-qfx-series.html
-      newIface.setNativeVlan(iface.getNativeVlan());
-    }
-
-    newIface.setSwitchportMode(iface.getSwitchportMode());
-    if (iface.getSwitchportMode() != SwitchportMode.NONE) {
+    EthernetSwitching es = iface.getEthernetSwitching();
+    if (_indirectAccessPorts.containsKey(name)) {
       newIface.setSwitchport(true);
+      newIface.setSwitchportMode(SwitchportMode.ACCESS);
+      newIface.setAccessVlan(
+          _masterLogicalSystem
+              .getNamedVlans()
+              .get(_indirectAccessPorts.get(name).getName())
+              .getVlanId());
+    } else if (es != null) {
+      newIface.setSwitchport(true);
+      if (es.getSwitchportMode() == null || es.getSwitchportMode() == SwitchportMode.ACCESS) {
+        newIface.setSwitchportMode(SwitchportMode.ACCESS);
+        newIface.setAccessVlan(computeAccessVlan(iface.getName(), es.getVlanMembers()));
+      }
+      if (es.getSwitchportMode() == SwitchportMode.TRUNK) {
+        newIface.setSwitchportTrunkEncapsulation(SwitchportEncapsulationType.DOT1Q);
+        newIface.setAllowedVlans(vlanMembersToIntegerSpace(es.getVlanMembers()));
+        // default is no native vlan, untagged are dropped.
+        // https://www.juniper.net/documentation/en_US/junos/topics/reference/configuration-statement/native-vlan-id-edit-interfaces-qfx-series.html
+        newIface.setNativeVlan(es.getNativeVlan());
+        newIface.setSwitchportMode(SwitchportMode.TRUNK);
+      }
+    } else {
+      newIface.setSwitchportMode(SwitchportMode.NONE);
+      newIface.setSwitchport(false);
     }
-
-    SwitchportEncapsulationType swe = iface.getSwitchportTrunkEncapsulation();
-    if (swe == null) {
-      swe = SwitchportEncapsulationType.DOT1Q;
-    }
-    newIface.setSwitchportTrunkEncapsulation(swe);
     newIface.setBandwidth(iface.getBandwidth());
     return newIface;
   }
 
+  private int computeAccessVlan(String ifaceName, List<VlanMember> vlanMembers) {
+    if (vlanMembers.isEmpty()) {
+      // TODO: verify default
+      return 1;
+    }
+    if (vlanMembers.size() > 1) {
+      _w.redFlag(
+          String.format(
+              "Cannot assign more than one access vlan: %s to interface '%s'",
+              vlanMembers, ifaceName));
+      return 1;
+    }
+    IntegerSpace members = vlanMembersToIntegerSpace(vlanMembers);
+    if (!members.isSingleton()) {
+      _w.redFlag(
+          String.format(
+              "Cannot assign more than one access vlan: %s to interface '%s'",
+              vlanMembers, ifaceName));
+      return 1;
+    }
+    return members.singletonValue();
+  }
+
+  private @Nonnull IntegerSpace vlanMembersToIntegerSpace(Collection<VlanMember> vlanMembers) {
+    IntegerSpace.Builder builder = IntegerSpace.builder();
+    vlanMembers.stream().map(this::toIntegerSpace).forEach(builder::including);
+    return builder.build();
+  }
+
+  private @Nonnull IntegerSpace toIntegerSpace(VlanMember vlanMember) {
+    if (vlanMember instanceof VlanReference) {
+      VlanReference vlanReference = (VlanReference) vlanMember;
+      Vlan vlan = _masterLogicalSystem.getNamedVlans().get(vlanReference.getName());
+      return vlan == null
+          ? IntegerSpace.EMPTY
+          : vlan.getVlanId() == null ? IntegerSpace.EMPTY : IntegerSpace.of(vlan.getVlanId());
+    } else if (vlanMember instanceof VlanRange) {
+      return ((VlanRange) vlanMember).getRange();
+    } else {
+      _w.redFlag(
+          String.format(
+              "Unsupported vlan member type: %s", vlanMember.getClass().getCanonicalName()));
+      return IntegerSpace.EMPTY;
+    }
+  }
+
   @Nullable
   private OspfNetworkType toOspfNetworkType(Interface.OspfInterfaceType type) {
     switch (type) {
@@ -3598,11 +3640,22 @@ private void convertInterfaces() {
           continue;
         }
         Interface i = optionalInterface.get();
-        if (i.getSwitchportMode() == SwitchportMode.ACCESS) {
-          i.setAccessVlan(vlan.getName());
-        } else if (i.getSwitchportMode() == SwitchportMode.TRUNK) {
-          i.getAllowedVlanNames().add(vlan.getName());
+        EthernetSwitching es = i.getEthernetSwitching();
+        if (es != null && (es.getSwitchportMode() != null || !es.getVlanMembers().isEmpty())) {
+          _w.redFlag(
+              String.format(
+                  "Cannot assign '%s' as interface of vlan '%s' since it is already has vlan configuration under family ethernet-switching",
+                  memberIfName, vlanId));
+          continue;
+        }
+        if (_indirectAccessPorts.containsKey(memberIfName)) {
+          _w.redFlag(
+              String.format(
+                  "Cannot assign '%s' as interface of vlan '%s' since it is already interface of vlan '%s'",
+                  memberIfName, vlanId, _indirectAccessPorts.get(memberIfName).getName()));
+          continue;
         }
+        _indirectAccessPorts.put(memberIfName, new VlanReference(vlan.getName()));
       }
     }
 

diff --git a/projects/batfish/src/main/java/org/batfish/representation/juniper/VlanMember.java b/projects/batfish/src/main/java/org/batfish/representation/juniper/VlanMember.java
@@ -0,0 +1,8 @@
+package org.batfish.representation.juniper;
+
+import java.io.Serializable;
+import javax.annotation.ParametersAreNonnullByDefault;
+
+/** A member vlan of a layer-2 interface. */
+@ParametersAreNonnullByDefault
+public interface VlanMember extends Serializable {}