From bae2cb2a18ed30c1e95511a38c4da5dd8cbb6c12 Mon Sep 17 00:00:00 2001 From: Charles Chan Date: Wed, 18 Jan 2017 19:35:20 -0800 Subject: [PATCH] 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 --- .../cli/net/InterfacesListCommand.java | 18 ++++ .../net/config/basics/InterfaceConfig.java | 102 ++++++++++++++++-- .../incubator/net/intf/Interface.java | 68 +++++++++++- 3 files changed, 177 insertions(+), 11 deletions(-) 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 832462eb75..9df7f912e8 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 @@ public class InterfacesListCommand extends AbstractShellCommand { 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 @@ public class InterfacesListCommand extends AbstractShellCommand { 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 67f1012e01..d4c5733010 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.JsonNode; 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.incubator.net.intf.Interface; 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 @@ import static com.google.common.base.Preconditions.checkNotNull; */ @Beta public class InterfaceConfig extends Config { + 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 @@ public class InterfaceConfig extends Config { @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 @@ public class InterfaceConfig extends Config { 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 @@ public class InterfaceConfig extends Config { 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 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 @@ public class InterfaceConfig extends Config { 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 @@ public class InterfaceConfig extends Config { } } - 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 getVlans(JsonNode node, String path) { + ImmutableSet.Builder 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 vlans) { + ArrayNode vlanArray = mapper.createArrayNode(); + + vlans.forEach(vlan -> vlanArray.add(vlan.toShort())); + + return vlanArray; + } + private List getIps(JsonNode node) { List 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 64726801de..c6c3ea4c63 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 @@ package org.onosproject.incubator.net.intf; 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 @@ public class Interface { private final ConnectPoint connectPoint; private final List ipAddresses; private final MacAddress macAddress; + // TODO: Deprecate this due to ambiguity private final VlanId vlan; + private final VlanId vlanUntagged; + private final Set vlanTagged; + private final VlanId vlanNative; /** * Creates new Interface with the provided configuration. @@ -56,11 +61,36 @@ public class Interface { public Interface(String name, ConnectPoint connectPoint, List 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 ipAddresses, + MacAddress macAddress, VlanId vlan, + VlanId vlanUntagged, Set 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 @@ public class Interface { 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 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 @@ public class Interface { 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 @@ public class Interface { .add("ipAddresses", ipAddresses) .add("macAddress", macAddress) .add("vlan", vlan) + .add("vlanUntagged", vlanUntagged) + .add("vlanTagged", vlanTagged) + .add("vlanNative", vlanNative) .toString(); } }