Implement vlan-untagged, vlan-tagged, vlan-native in interface config
Existing vlan config is ambiguous and different apps could have different interpretation.
In this commit, we introduce three more vlan configs and hope this will eventually replace the original one.
Change-Id: If8dd985cc3a420601073797eb617ffd1adf90d1d
diff --git a/cli/src/main/java/org/onosproject/cli/net/InterfacesListCommand.java b/cli/src/main/java/org/onosproject/cli/net/InterfacesListCommand.java
index 832462e..9df7f91 100644
--- a/cli/src/main/java/org/onosproject/cli/net/InterfacesListCommand.java
+++ b/cli/src/main/java/org/onosproject/cli/net/InterfacesListCommand.java
@@ -38,6 +38,9 @@
private static final String IP_FORMAT = " ips=";
private static final String MAC_FORMAT = " mac=";
private static final String VLAN_FORMAT = " vlan=";
+ private static final String VLAN_UNTAGGED = " vlanUntagged=";
+ private static final String VLAN_TAGGED = " vlanTagged=";
+ private static final String VLAN_NATIVE = " vlanNative=";
private static final String NO_NAME = "(unamed)";
@@ -70,6 +73,21 @@
formatStringBuilder.append(intf.vlan().toString());
}
+ if (!intf.vlanUntagged().equals(VlanId.NONE)) {
+ formatStringBuilder.append(VLAN_UNTAGGED);
+ formatStringBuilder.append(intf.vlanUntagged().toString());
+ }
+
+ if (!intf.vlanTagged().isEmpty()) {
+ formatStringBuilder.append(VLAN_TAGGED);
+ formatStringBuilder.append(intf.vlanTagged().toString());
+ }
+
+ if (!intf.vlanNative().equals(VlanId.NONE)) {
+ formatStringBuilder.append(VLAN_NATIVE);
+ formatStringBuilder.append(intf.vlanNative().toString());
+ }
+
String name = (intf.name().equals(Interface.NO_INTERFACE_NAME)) ?
NO_NAME : intf.name();
diff --git a/incubator/api/src/main/java/org/onosproject/incubator/net/config/basics/InterfaceConfig.java b/incubator/api/src/main/java/org/onosproject/incubator/net/config/basics/InterfaceConfig.java
index 67f1012..d4c5733 100644
--- a/incubator/api/src/main/java/org/onosproject/incubator/net/config/basics/InterfaceConfig.java
+++ b/incubator/api/src/main/java/org/onosproject/incubator/net/config/basics/InterfaceConfig.java
@@ -20,6 +20,7 @@
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.annotations.Beta;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import org.onlab.packet.IpPrefix;
@@ -29,6 +30,8 @@
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.config.Config;
import org.onosproject.net.host.InterfaceIpAddress;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.util.Iterator;
import java.util.List;
@@ -42,10 +45,15 @@
*/
@Beta
public class InterfaceConfig extends Config<ConnectPoint> {
+ private static Logger log = LoggerFactory.getLogger(InterfaceConfig.class);
+
public static final String NAME = "name";
public static final String IPS = "ips";
public static final String MAC = "mac";
public static final String VLAN = "vlan";
+ public static final String VLAN_UNTAGGED = "vlan-untagged";
+ public static final String VLAN_TAGGED = "vlan-tagged";
+ public static final String VLAN_NATIVE = "vlan-native";
private static final String CONFIG_VALUE_ERROR = "Error parsing config value";
private static final String INTF_NULL_ERROR = "Interface cannot be null";
@@ -54,7 +62,8 @@
@Override
public boolean isValid() {
for (JsonNode node : array) {
- if (!hasOnlyFields((ObjectNode) node, NAME, IPS, MAC, VLAN)) {
+ if (!hasOnlyFields((ObjectNode) node, NAME, IPS, MAC, VLAN,
+ VLAN_UNTAGGED, VLAN_TAGGED, VLAN_NATIVE)) {
return false;
}
@@ -62,16 +71,41 @@
if (!(isString(obj, NAME, FieldPresence.OPTIONAL) &&
isMacAddress(obj, MAC, FieldPresence.OPTIONAL) &&
- isIntegralNumber(obj, VLAN, FieldPresence.OPTIONAL, 0, VlanId.MAX_VLAN))) {
+ isIntegralNumber(obj, VLAN, FieldPresence.OPTIONAL, 0, VlanId.MAX_VLAN) &&
+ isIntegralNumber(obj, VLAN_UNTAGGED, FieldPresence.OPTIONAL, 0, VlanId.MAX_VLAN) &&
+ isIntegralNumber(obj, VLAN_NATIVE, FieldPresence.OPTIONAL, 0, VlanId.MAX_VLAN))) {
return false;
}
-
for (JsonNode ipNode : node.path(IPS)) {
if (!ipNode.isTextual() || IpPrefix.valueOf(ipNode.asText()) == null) {
return false;
}
}
+
+ checkArgument(!hasField(obj, VLAN_TAGGED) ||
+ (node.path(VLAN_TAGGED).isArray() && node.path(VLAN_TAGGED).size() >= 1),
+ "%s must be an array with at least one element", VLAN_TAGGED);
+
+ for (JsonNode vlanNode : node.path(VLAN_TAGGED)) {
+ checkArgument(vlanNode.isInt() &&
+ vlanNode.intValue() >= 0 && vlanNode.intValue() <= VlanId.MAX_VLAN,
+ "Invalid VLAN ID %s in %s", vlanNode.intValue(), VLAN_TAGGED);
+ }
+
+ checkArgument(!hasField(obj, VLAN_UNTAGGED) ||
+ !(hasField(obj, VLAN_TAGGED) || hasField(obj, VLAN_NATIVE)),
+ "%s and %s should not be used when %s is set", VLAN_TAGGED, VLAN_NATIVE, VLAN_UNTAGGED);
+
+ checkArgument(!hasField(obj, VLAN_TAGGED) || !hasField(obj, VLAN_UNTAGGED),
+ "%s should not be used when %s is set", VLAN_UNTAGGED, VLAN_TAGGED);
+
+ checkArgument(!hasField(obj, VLAN_NATIVE) || hasField(obj, VLAN_TAGGED),
+ "%s should not be used alone without %s", VLAN_NATIVE, VLAN_TAGGED);
+
+ checkArgument(!hasField(obj, VLAN_NATIVE) || !hasField(obj, VLAN_TAGGED) ||
+ !getVlans(obj, VLAN_TAGGED).contains(getVlan(obj, VLAN_NATIVE)),
+ "%s cannot be one of the VLANs configured in %s", VLAN_NATIVE, VLAN_TAGGED);
}
return true;
}
@@ -94,9 +128,13 @@
String mac = intfNode.path(MAC).asText();
MacAddress macAddr = mac.isEmpty() ? null : MacAddress.valueOf(mac);
- VlanId vlan = getVlan(intfNode);
+ VlanId vlan = getVlan(intfNode, VLAN);
+ VlanId vlanUntagged = getVlan(intfNode, VLAN_UNTAGGED);
+ Set<VlanId> vlanTagged = getVlans(intfNode, VLAN_TAGGED);
+ VlanId vlanNative = getVlan(intfNode, VLAN_NATIVE);
- interfaces.add(new Interface(name, subject, ips, macAddr, vlan));
+ interfaces.add(new Interface(name, subject, ips, macAddr, vlan,
+ vlanUntagged, vlanTagged, vlanNative));
}
} catch (IllegalArgumentException e) {
throw new ConfigException(CONFIG_VALUE_ERROR, e);
@@ -125,13 +163,25 @@
intfNode.put(MAC, intf.mac().toString());
}
- if (!intf.ipAddresses().isEmpty()) {
+ if (!intf.ipAddressesList().isEmpty()) {
intfNode.set(IPS, putIps(intf.ipAddressesList()));
}
if (!intf.vlan().equals(VlanId.NONE)) {
intfNode.put(VLAN, intf.vlan().toString());
}
+
+ if (!intf.vlanUntagged().equals(VlanId.NONE)) {
+ intfNode.put(VLAN_UNTAGGED, intf.vlanUntagged().toString());
+ }
+
+ if (!intf.vlanTagged().isEmpty()) {
+ intfNode.set(VLAN_UNTAGGED, putVlans(intf.vlanTagged()));
+ }
+
+ if (!intf.vlanNative().equals(VlanId.NONE)) {
+ intfNode.put(VLAN_NATIVE, intf.vlanNative().toString());
+ }
}
/**
@@ -153,14 +203,48 @@
}
}
- private VlanId getVlan(JsonNode node) {
+ /**
+ * Extracts VLAN ID from given path of given json node.
+ *
+ * @param node JSON node
+ * @param path path
+ * @return VLAN ID
+ */
+ private VlanId getVlan(JsonNode node, String path) {
VlanId vlan = VlanId.NONE;
- if (!node.path(VLAN).isMissingNode()) {
- vlan = VlanId.vlanId(Short.valueOf(node.path(VLAN).asText()));
+ if (!node.path(path).isMissingNode()) {
+ vlan = VlanId.vlanId(Short.valueOf(node.path(path).asText()));
}
return vlan;
}
+ /**
+ * Extracts a set of VLAN ID from given path of given json node.
+ *
+ * @param node JSON node
+ * @param path path
+ * @return a set of VLAN ID
+ */
+ private Set<VlanId> getVlans(JsonNode node, String path) {
+ ImmutableSet.Builder<VlanId> vlanIdBuilder = ImmutableSet.builder();
+
+ JsonNode vlansNode = node.get(path);
+ if (vlansNode != null) {
+ vlansNode.forEach(vlanNode ->
+ vlanIdBuilder.add(VlanId.vlanId(vlanNode.shortValue())));
+ }
+
+ return vlanIdBuilder.build();
+ }
+
+ private ArrayNode putVlans(Set<VlanId> vlans) {
+ ArrayNode vlanArray = mapper.createArrayNode();
+
+ vlans.forEach(vlan -> vlanArray.add(vlan.toShort()));
+
+ return vlanArray;
+ }
+
private List<InterfaceIpAddress> getIps(JsonNode node) {
List<InterfaceIpAddress> ips = Lists.newArrayList();
diff --git a/incubator/api/src/main/java/org/onosproject/incubator/net/intf/Interface.java b/incubator/api/src/main/java/org/onosproject/incubator/net/intf/Interface.java
index 6472680..c6c3ea4 100644
--- a/incubator/api/src/main/java/org/onosproject/incubator/net/intf/Interface.java
+++ b/incubator/api/src/main/java/org/onosproject/incubator/net/intf/Interface.java
@@ -17,6 +17,7 @@
import com.google.common.annotations.Beta;
import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import org.onlab.packet.MacAddress;
import org.onlab.packet.VlanId;
@@ -42,7 +43,11 @@
private final ConnectPoint connectPoint;
private final List<InterfaceIpAddress> ipAddresses;
private final MacAddress macAddress;
+ // TODO: Deprecate this due to ambiguity
private final VlanId vlan;
+ private final VlanId vlanUntagged;
+ private final Set<VlanId> vlanTagged;
+ private final VlanId vlanNative;
/**
* Creates new Interface with the provided configuration.
@@ -56,11 +61,36 @@
public Interface(String name, ConnectPoint connectPoint,
List<InterfaceIpAddress> ipAddresses,
MacAddress macAddress, VlanId vlan) {
+ this(name, connectPoint, ipAddresses, macAddress, vlan, null, null, null);
+ }
+
+ /**
+ * Creates new Interface with the provided configuration.
+ *
+ * @param name name of the interface
+ * @param connectPoint the connect point this interface maps to
+ * @param ipAddresses list of IP addresses
+ * @param macAddress MAC address
+ * @param vlan VLAN ID
+ * @param vlanUntagged untagged VLAN.
+ * Cannot be used with vlanTagged or vlanNative.
+ * @param vlanTagged tagged VLANs.
+ * Cannot be used with vlanUntagged.
+ * @param vlanNative native vLAN. Optional.
+ * Can only be used when vlanTagged is specified. Cannot be used with vlanUntagged.
+ */
+ public Interface(String name, ConnectPoint connectPoint,
+ List<InterfaceIpAddress> ipAddresses,
+ MacAddress macAddress, VlanId vlan,
+ VlanId vlanUntagged, Set<VlanId> vlanTagged, VlanId vlanNative) {
this.name = name == null ? NO_INTERFACE_NAME : name;
this.connectPoint = checkNotNull(connectPoint);
this.ipAddresses = ipAddresses == null ? Lists.newArrayList() : ipAddresses;
this.macAddress = macAddress == null ? MacAddress.NONE : macAddress;
this.vlan = vlan == null ? VlanId.NONE : vlan;
+ this.vlanUntagged = vlanUntagged == null ? VlanId.NONE : vlanUntagged;
+ this.vlanTagged = vlanTagged == null ? ImmutableSet.of() : ImmutableSet.copyOf(vlanTagged);
+ this.vlanNative = vlanNative == null ? VlanId.NONE : vlanNative;
}
/**
@@ -120,6 +150,33 @@
return vlan;
}
+ /**
+ * Retrieves the VLAN ID that will be assign to untagged packets.
+ *
+ * @return the VLAN ID
+ */
+ public VlanId vlanUntagged() {
+ return vlanUntagged;
+ }
+
+ /**
+ * Retrieves the set of VLAN IDs that will be allowed on this interface.
+ *
+ * @return the VLAN ID
+ */
+ public Set<VlanId> vlanTagged() {
+ return vlanTagged;
+ }
+
+ /**
+ * Retrieves the set of VLAN IDs that will be allowed on this interface.
+ *
+ * @return the VLAN ID
+ */
+ public VlanId vlanNative() {
+ return vlanNative;
+ }
+
@Override
public boolean equals(Object other) {
if (!(other instanceof Interface)) {
@@ -132,12 +189,16 @@
Objects.equals(connectPoint, otherInterface.connectPoint) &&
Objects.equals(ipAddresses, otherInterface.ipAddresses) &&
Objects.equals(macAddress, otherInterface.macAddress) &&
- Objects.equals(vlan, otherInterface.vlan);
+ Objects.equals(vlan, otherInterface.vlan) &&
+ Objects.equals(vlanUntagged, otherInterface.vlanUntagged) &&
+ Objects.equals(vlanTagged, otherInterface.vlanTagged) &&
+ Objects.equals(vlanNative, otherInterface.vlanNative);
}
@Override
public int hashCode() {
- return Objects.hash(connectPoint, name, ipAddresses, macAddress, vlan);
+ return Objects.hash(connectPoint, name, ipAddresses, macAddress, vlan,
+ vlanUntagged, vlanTagged, vlanNative);
}
@Override
@@ -148,6 +209,9 @@
.add("ipAddresses", ipAddresses)
.add("macAddress", macAddress)
.add("vlan", vlan)
+ .add("vlanUntagged", vlanUntagged)
+ .add("vlanTagged", vlanTagged)
+ .add("vlanNative", vlanNative)
.toString();
}
}