Skip to content

Commit

Permalink
Interface: switch allowedVlans to IntegerSpace (#2616)
Browse files Browse the repository at this point in the history 
* Interface: switch allowedVlans to IntegerSpace
  • Loading branch information
@dhalperi
dhalperi committed Nov 10, 2018
1 parent 6331f06 commit 6b3a908
Show file tree
Hide file tree
Showing 18 changed files with 615 additions and 58 deletions.
26 changes: 9 additions & 17 deletions projects/batfish-common-protocol/src/main/java/org/batfish/common/topology/TopologyUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import org.batfish.datamodel.Ip;
import org.batfish.datamodel.IpSpace;
import org.batfish.datamodel.Route;
import org.batfish.datamodel.SubRange;
import org.batfish.datamodel.SwitchportMode;
import org.batfish.datamodel.Topology;
import org.batfish.datamodel.Vrf;
Expand All @@ -52,10 +51,7 @@ private static void addLayer2TrunkEdges(
&& i2.getSwitchportMode() == SwitchportMode.TRUNK) {
i1.getAllowedVlans()
.stream()
.flatMapToInt(SubRange::asStream)
.filter(
i1AllowedVlan ->
i2.getAllowedVlans().stream().anyMatch(sr -> sr.includes(i1AllowedVlan)))
.filter(i2.getAllowedVlans()::contains)
.forEach(
allowedVlan ->
edges.add(new Layer2Edge(node1, allowedVlan, node2, allowedVlan, allowedVlan)));
Expand Down Expand Up @@ -86,13 +82,11 @@ private static void computeAugmentedLayer2SelfEdges(
.add(i.getName());
i.getAllowedVlans()
.stream()
.flatMapToInt(SubRange::asStream)
.forEach(
allowedVlan -> {
switchportsByVlan
.computeIfAbsent(allowedVlan, n -> ImmutableList.builder())
.add(i.getName());
});
allowedVlan ->
switchportsByVlan
.computeIfAbsent(allowedVlan, n -> ImmutableList.builder())
.add(i.getName()));
}
if (i.getSwitchportMode() == SwitchportMode.ACCESS) {
switchportsByVlan
Expand Down Expand Up @@ -213,13 +207,11 @@ private static void computeLayer2SelfEdges(
.add(i.getName());
i.getAllowedVlans()
.stream()
.flatMapToInt(SubRange::asStream)
.forEach(
allowedVlan -> {
switchportsByVlan
.computeIfAbsent(allowedVlan, n -> ImmutableList.builder())
.add(i.getName());
});
allowedVlan ->
switchportsByVlan
.computeIfAbsent(allowedVlan, n -> ImmutableList.builder())
.add(i.getName()));
}
if (i.getSwitchportMode() == SwitchportMode.ACCESS) {
switchportsByVlan
Expand Down
12 changes: 7 additions & 5 deletions projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Interface.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ private static InterfaceType computeVyosInterfaceType(String name) {

private SortedSet<Ip> _additionalArpIps;

private List<SubRange> _allowedVlans;
private IntegerSpace _allowedVlans;

private SortedSet<InterfaceAddress> _allAddresses;

Expand Down Expand Up @@ -713,7 +713,7 @@ public Interface(String name, Configuration owner, @Nonnull InterfaceType interf
super(name);
_active = true;
_autoState = true;
_allowedVlans = ImmutableList.of();
_allowedVlans = IntegerSpace.EMPTY;
_allAddresses = ImmutableSortedSet.of();
_channelGroupMembers = ImmutableSortedSet.of();
_declaredNames = ImmutableSortedSet.of();
Expand All @@ -731,7 +731,9 @@ public Interface(String name, Configuration owner, @Nonnull InterfaceType interf
}

public void addAllowedRanges(List<SubRange> ranges) {
_allowedVlans = ImmutableList.<SubRange>builder().addAll(_allowedVlans).addAll(ranges).build();
IntegerSpace.Builder b = IntegerSpace.builder().including(_allowedVlans);
ranges.forEach(b::including);
_allowedVlans = b.build();
}

@Override
Expand Down Expand Up @@ -831,7 +833,7 @@ public SortedSet<Ip> getAdditionalArpIps() {

@JsonProperty(PROP_ALLOWED_VLANS)
@JsonPropertyDescription("Ranges of allowed VLANs when switchport mode is TRUNK")
public List<SubRange> getAllowedVlans() {
public IntegerSpace getAllowedVlans() {
return _allowedVlans;
}

Expand Down Expand Up @@ -1202,7 +1204,7 @@ public void setAdditionalArpIps(Iterable<Ip> additionalArpIps) {
}

@JsonProperty(PROP_ALLOWED_VLANS)
public void setAllowedVlans(List<SubRange> allowedVlans) {
public void setAllowedVlans(IntegerSpace allowedVlans) {
_allowedVlans = allowedVlans;
}

Expand Down
9 changes: 5 additions & 4 deletions ...s/batfish-common-protocol/src/test/java/org/batfish/common/topology/TopologyUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.batfish.datamodel.Configuration;
import org.batfish.datamodel.Configuration.Builder;
import org.batfish.datamodel.ConfigurationFormat;
import org.batfish.datamodel.IntegerSpace;
import org.batfish.datamodel.Interface;
import org.batfish.datamodel.InterfaceAddress;
import org.batfish.datamodel.InterfaceType;
Expand Down Expand Up @@ -115,7 +116,7 @@ public void testComputeLayer2Topology() {
Interface c1i4 = _ib.setName(c1i4Name).build();
c1i4.setSwitchport(true);
c1i4.setSwitchportMode(SwitchportMode.TRUNK);
c1i4.setAllowedVlans(ImmutableList.of(new SubRange(1, 3)));
c1i4.setAllowedVlans(IntegerSpace.of(new SubRange(1, 3)));
c1i4.setNativeVlan(0);
Interface c1i5 = _ib.setName(c1i5Name).build();
c1i5.setSwitchport(true);
Expand All @@ -133,7 +134,7 @@ public void testComputeLayer2Topology() {
Interface c2i4 = _ib.setName(c2i4Name).build();
c2i4.setSwitchport(true);
c2i4.setSwitchportMode(SwitchportMode.TRUNK);
c2i4.setAllowedVlans(ImmutableList.of(new SubRange(1, 2)));
c2i4.setAllowedVlans(IntegerSpace.of(new SubRange(1, 2)));
c2i4.setNativeVlan(0);

Configuration c3 = _cb.setHostname(c3Name).build();
Expand Down Expand Up @@ -255,7 +256,7 @@ public void testComputeLayer3Topology() {
Interface c1i4 = _ib.setName(c1i4Name).setAddresses(null).build();
c1i4.setSwitchport(true);
c1i4.setSwitchportMode(SwitchportMode.TRUNK);
c1i4.setAllowedVlans(ImmutableList.of(new SubRange(1, 3)));
c1i4.setAllowedVlans(IntegerSpace.of(new SubRange(1, 3)));
c1i4.setNativeVlan(0);
Interface c1i5 = _ib.setName(c1i5Name).build();
c1i5.setSwitchport(true);
Expand Down Expand Up @@ -303,7 +304,7 @@ public void testComputeLayer3Topology() {
Interface c2i4 = _ib.setName(c2i4Name).setAddresses(null).build();
c2i4.setSwitchport(true);
c2i4.setSwitchportMode(SwitchportMode.TRUNK);
c2i4.setAllowedVlans(ImmutableList.of(new SubRange(1, 2)));
c2i4.setAllowedVlans(IntegerSpace.of(new SubRange(1, 2)));
c2i4.setNativeVlan(0);
Interface c2Vlan1 =
_ib.setName(vlan1Name)
Expand Down
10 changes: 5 additions & 5 deletions ...tfish-common-protocol/src/test/java/org/batfish/datamodel/matchers/InterfaceMatchers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
import java.util.SortedSet;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.batfish.datamodel.IntegerSpace;
import org.batfish.datamodel.Interface;
import org.batfish.datamodel.InterfaceAddress;
import org.batfish.datamodel.Ip;
import org.batfish.datamodel.SourceNat;
import org.batfish.datamodel.SubRange;
import org.batfish.datamodel.SwitchportMode;
import org.batfish.datamodel.Vrf;
import org.batfish.datamodel.eigrp.EigrpInterfaceSettings;
Expand Down Expand Up @@ -73,15 +73,15 @@ public static Matcher<Interface> hasAllAddresses(
/**
* Provides a matcher that matches if the provided value matches the interface's Allowed VLANs.
*/
public static HasAllowedVlans hasAllowedVlans(List<SubRange> value) {
public static HasAllowedVlans hasAllowedVlans(IntegerSpace value) {
return hasAllowedVlans(equalTo(value));
}

/**
* Provides a matcher that matches if the provided {@code subMatcher} matches the interface's
* Allowed VLANs.
*/
public static HasAllowedVlans hasAllowedVlans(Matcher<? super List<SubRange>> subMatcher) {
public static HasAllowedVlans hasAllowedVlans(Matcher<? super IntegerSpace> subMatcher) {
return new HasAllowedVlans(subMatcher);
}

Expand Down Expand Up @@ -139,7 +139,7 @@ public static Matcher<Interface> hasDescription(String expectedDescription) {
}

/**
* Provides a matcher that matches if the provided {@link subMatcher} matches the {@link
* Provides a matcher that matches if the provided {@code subMatcher} matches the {@link
* Interface}'s hsrpGroup with the specified {@code number}.
*/
public static @Nonnull Matcher<Interface> hasHsrpGroup(
Expand Down Expand Up @@ -190,7 +190,7 @@ public static HasOspfArea hasOspfArea(Matcher<OspfArea> subMatcher) {
}

/**
* Provides a matcher that matches if the the interface's OSPF area ID is {@link expectedArea}.
* Provides a matcher that matches if the the interface's OSPF area ID is {@code expectedArea}.
*/
public static @Nonnull Matcher<Interface> hasOspfAreaName(long expectedArea) {
return new HasOspfAreaName(equalTo(expectedArea));
Expand Down
8 changes: 4 additions & 4 deletions ...h-common-protocol/src/test/java/org/batfish/datamodel/matchers/InterfaceMatchersImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
import java.util.SortedSet;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.batfish.datamodel.IntegerSpace;
import org.batfish.datamodel.Interface;
import org.batfish.datamodel.InterfaceAddress;
import org.batfish.datamodel.Ip;
import org.batfish.datamodel.IpAccessList;
import org.batfish.datamodel.SourceNat;
import org.batfish.datamodel.SubRange;
import org.batfish.datamodel.SwitchportMode;
import org.batfish.datamodel.Vrf;
import org.batfish.datamodel.eigrp.EigrpInterfaceSettings;
Expand Down Expand Up @@ -55,13 +55,13 @@ protected Set<InterfaceAddress> featureValueOf(Interface actual) {
}
}

static final class HasAllowedVlans extends FeatureMatcher<Interface, List<SubRange>> {
HasAllowedVlans(@Nonnull Matcher<? super List<SubRange>> subMatcher) {
static final class HasAllowedVlans extends FeatureMatcher<Interface, IntegerSpace> {
HasAllowedVlans(@Nonnull Matcher<? super IntegerSpace> subMatcher) {
super(subMatcher, "an Interface with allowedVlans:", "allowedVlans");
}

@Override
protected List<SubRange> featureValueOf(Interface actual) {
protected IntegerSpace featureValueOf(Interface actual) {
return actual.getAllowedVlans();
}
}
Expand Down
23 changes: 12 additions & 11 deletions projects/batfish/src/main/java/org/batfish/main/Batfish.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
import org.batfish.datamodel.FlowTrace;
import org.batfish.datamodel.GenericConfigObject;
import org.batfish.datamodel.HeaderSpace;
import org.batfish.datamodel.IntegerSpace;
import org.batfish.datamodel.Interface;
import org.batfish.datamodel.InterfaceType;
import org.batfish.datamodel.Ip;
Expand Down Expand Up @@ -999,34 +1000,34 @@ private void disableUnusableVlanInterfaces(Map<String, Configuration> configurat
}
// Update vlanMemberCounts:
for (Interface iface : nonVlanInterfaces) {
List<SubRange> vlans = new ArrayList<>();
IntegerSpace.Builder vlans = IntegerSpace.builder();
if (iface.getSwitchportMode() == SwitchportMode.TRUNK) { // vlan trunked interface
Collection<SubRange> allowed = iface.getAllowedVlans();
IntegerSpace allowed = iface.getAllowedVlans();
if (!allowed.isEmpty()) {
// Explicit list of allowed VLANs
vlans.addAll(allowed);
vlans.including(allowed);
} else {
// No explicit list, so all VLANs are allowed.
vlanInterfaces.keySet().forEach(v -> vlans.add(new SubRange(v, v)));
vlanInterfaces.keySet().forEach(vlans::including);
}
// Add the native VLAN as well.
vlanNumber = iface.getNativeVlan();
vlans.add(new SubRange(vlanNumber, vlanNumber));
vlans.including(vlanNumber);
} else if (iface.getSwitchportMode() == SwitchportMode.ACCESS) { // access mode ACCESS
vlanNumber = iface.getAccessVlan();
vlans.add(new SubRange(vlanNumber, vlanNumber));
vlans.including(vlanNumber);
// Any other Switch Port mode is unsupported
} else if (iface.getSwitchportMode() != SwitchportMode.NONE) {
_logger.warnf(
"WARNING: Unsupported switch port mode %s, assuming no VLANs allowed: \"%s:%s\"\n",
iface.getSwitchportMode(), hostname, iface.getName());
}

for (SubRange sr : vlans) {
for (int vlanId = sr.getStart(); vlanId <= sr.getEnd(); ++vlanId) {
vlanMemberCounts.compute(vlanId, (k, v) -> (v == null) ? 1 : (v + 1));
}
}
vlans
.build()
.stream()
.forEach(
vlanId -> vlanMemberCounts.compute(vlanId, (k, v) -> (v == null) ? 1 : (v + 1)));
}
// Disable all "normal" vlan interfaces with zero member counts:
SubRange normalVlanRange = c.getNormalVlanRange();
Expand Down
3 changes: 2 additions & 1 deletion projects/batfish/src/main/java/org/batfish/representation/juniper/JuniperConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.batfish.datamodel.IkePhase1Key;
import org.batfish.datamodel.IkePhase1Policy;
import org.batfish.datamodel.IkePhase1Proposal;
import org.batfish.datamodel.IntegerSpace;
import org.batfish.datamodel.InterfaceAddress;
import org.batfish.datamodel.Ip;
import org.batfish.datamodel.IpAccessList;
Expand Down Expand Up @@ -1393,7 +1394,7 @@ private org.batfish.datamodel.Interface toInterface(Interface iface) {
newIface.setAccessVlan(vlan.getVlanId());
}
}
newIface.setAllowedVlans(iface.getAllowedVlans());
newIface.setAllowedVlans(IntegerSpace.unionOf(iface.getAllowedVlans()));
newIface.setNativeVlan(iface.getNativeVlan());
newIface.setSwitchportMode(iface.getSwitchportMode());
SwitchportEncapsulationType swe = iface.getSwitchportTrunkEncapsulation();
Expand Down
3 changes: 2 additions & 1 deletion projects/batfish/src/test/java/org/batfish/grammar/flatjuniper/FlatJuniperGrammarTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@
import org.batfish.datamodel.HeaderSpace;
import org.batfish.datamodel.IkeAuthenticationMethod;
import org.batfish.datamodel.IkeHashingAlgorithm;
import org.batfish.datamodel.IntegerSpace;
import org.batfish.datamodel.InterfaceAddress;
import org.batfish.datamodel.Ip;
import org.batfish.datamodel.IpAccessList;
Expand Down Expand Up @@ -1461,7 +1462,7 @@ public void testInterfaceVlan() throws IOException {
// Expecting an Interface in TRUNK mode with VLANs 1-5
assertThat(c, hasInterface("ge-0/3/0.0", hasSwitchPortMode(SwitchportMode.TRUNK)));
assertThat(
c, hasInterface("ge-0/3/0.0", hasAllowedVlans(ImmutableList.of(new SubRange("1-5")))));
c, hasInterface("ge-0/3/0.0", hasAllowedVlans(IntegerSpace.of(new SubRange("1-5")))));
}

@Test
Expand Down

0 comments on commit 6b3a908

Please sign in to comment.