Improve SegmentRoutingAppConfig
- Separate host and subnet suppression
- Use port instead of interface to specify which SR should ignore
Change-Id: Ie6491950cddf0860924565f081504b4f4d788179
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index f77804f..da8faa7 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -473,7 +473,7 @@
for (Port port : srManager.deviceService.getPorts(deviceId)) {
ConnectPoint cp = new ConnectPoint(deviceId, port.number());
// TODO: Handles dynamic port events when we are ready for dynamic config
- if (!srManager.deviceConfiguration.excludedPorts().contains(cp) &&
+ if (!srManager.deviceConfiguration.suppressSubnet().contains(cp) &&
port.isEnabled()) {
Ip4Prefix portSubnet = config.getPortSubnet(deviceId, port.number());
VlanId assignedVlan = (portSubnet == null)
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index ac9b291..ec2ea06 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -991,7 +991,7 @@
Set<IpAddress> ips = event.subject().ipAddresses();
log.info("Host {}/{} is added at {}:{}", mac, vlanId, deviceId, port);
- if (!deviceConfiguration.excludedPorts()
+ if (!deviceConfiguration.suppressHost()
.contains(new ConnectPoint(deviceId, port))) {
// Populate bridging table entry
log.debug("Populate L2 table entry for host {} at {}:{}",
@@ -1020,7 +1020,7 @@
Set<IpAddress> ips = event.subject().ipAddresses();
log.debug("Host {}/{} is removed from {}:{}", mac, vlanId, deviceId, port);
- if (!deviceConfiguration.excludedPorts()
+ if (!deviceConfiguration.suppressHost()
.contains(new ConnectPoint(deviceId, port))) {
// Revoke bridging table entry
ForwardingObjective.Builder fob =
@@ -1051,7 +1051,7 @@
log.debug("Host {}/{} is moved from {}:{} to {}:{}",
mac, vlanId, prevDeviceId, prevPort, newDeviceId, newPort);
- if (!deviceConfiguration.excludedPorts()
+ if (!deviceConfiguration.suppressHost()
.contains(new ConnectPoint(prevDeviceId, prevPort))) {
// Revoke previous bridging table entry
ForwardingObjective.Builder prevFob =
@@ -1069,7 +1069,7 @@
});
}
- if (!deviceConfiguration.excludedPorts()
+ if (!deviceConfiguration.suppressHost()
.contains(new ConnectPoint(newDeviceId, newPort))) {
// Populate new bridging table entry
ForwardingObjective.Builder newFob =
@@ -1099,7 +1099,7 @@
Set<IpAddress> newIps = event.subject().ipAddresses();
log.debug("Host {}/{} is updated", mac, vlanId);
- if (!deviceConfiguration.excludedPorts()
+ if (!deviceConfiguration.suppressHost()
.contains(new ConnectPoint(prevDeviceId, prevPort))) {
// Revoke previous IP table entry
prevIps.forEach(ip -> {
@@ -1110,7 +1110,7 @@
});
}
- if (!deviceConfiguration.excludedPorts()
+ if (!deviceConfiguration.suppressHost()
.contains(new ConnectPoint(newDeviceId, newPort))) {
// Populate new IP table entry
newIps.forEach(ip -> {
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java
index 932b8ad..1beed32 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java
@@ -18,7 +18,6 @@
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.SetMultimap;
-import com.google.common.collect.Sets;
import org.onlab.packet.Ip4Address;
import org.onlab.packet.Ip4Prefix;
import org.onlab.packet.IpPrefix;
@@ -30,6 +29,7 @@
import org.onosproject.incubator.net.intf.Interface;
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.config.NetworkConfigRegistry;
+import org.onosproject.net.config.NetworkConfigService;
import org.onosproject.net.host.InterfaceIpAddress;
import org.onosproject.net.DeviceId;
import org.onosproject.net.PortNumber;
@@ -57,7 +57,8 @@
private final List<Integer> allSegmentIds = new ArrayList<>();
private final Map<DeviceId, SegmentRouterInfo> deviceConfigMap = new ConcurrentHashMap<>();
private final Map<VlanId, List<ConnectPoint>> xConnects = new ConcurrentHashMap<>();
- private final Set<ConnectPoint> excludedPorts = Sets.newConcurrentHashSet();
+ private ApplicationId appId;
+ private NetworkConfigService cfgService;
private SegmentRoutingAppConfig appConfig;
private class SegmentRouterInfo {
@@ -85,6 +86,9 @@
*/
public DeviceConfiguration(ApplicationId appId,
NetworkConfigRegistry cfgService) {
+ this.appId = appId;
+ this.cfgService = cfgService;
+
// Read config from device subject, excluding gatewayIps and subnets.
Set<DeviceId> deviceSubjects =
cfgService.getSubjects(DeviceId.class, SegmentRoutingDeviceConfig.class);
@@ -103,15 +107,18 @@
allSegmentIds.add(info.nodeSid);
});
- // Read excluded port names from config
+ // Read SegmentRoutingAppConfig
appConfig = cfgService.getConfig(appId, SegmentRoutingAppConfig.class);
- Set<String> excludePorts = (appConfig != null) ?
- appConfig.excludePorts() : ImmutableSet.of();
// Read gatewayIps and subnets from port subject.
Set<ConnectPoint> portSubjects =
cfgService.getSubjects(ConnectPoint.class, InterfaceConfig.class);
portSubjects.forEach(subject -> {
+ // Do not process excluded ports
+ if (suppressSubnet().contains(subject)) {
+ return;
+ }
+
InterfaceConfig config =
cfgService.getConfig(subject, InterfaceConfig.class);
Set<Interface> networkInterfaces;
@@ -122,12 +129,6 @@
return;
}
networkInterfaces.forEach(networkInterface -> {
- // Do not process excluded ports
- if (excludePorts.contains(networkInterface.name())) {
- excludedPorts.add(subject);
- return;
- }
-
VlanId vlanId = networkInterface.vlan();
ConnectPoint connectPoint = networkInterface.connectPoint();
DeviceId dpid = connectPoint.deviceId();
@@ -366,7 +367,7 @@
ImmutableSet.Builder<Ip4Prefix> builder = ImmutableSet.builder();
builder.addAll(srinfo.subnets.values());
- if (deviceId.equals(appConfig.vRouterId())) {
+ if (appConfig != null && deviceId.equals(appConfig.vRouterId())) {
builder.add(Ip4Prefix.valueOf("0.0.0.0/0"));
}
return builder.build();
@@ -491,12 +492,11 @@
return srinfo != null && srinfo.adjacencySids.containsKey(sid);
}
- /**
- * Returns a set of excluded ports.
- *
- * @return excluded ports
- */
- public Set<ConnectPoint> excludedPorts() {
- return excludedPorts;
+ public Set<ConnectPoint> suppressSubnet() {
+ return (appConfig != null) ? appConfig.suppressSubnet() : ImmutableSet.of();
+ }
+
+ public Set<ConnectPoint> suppressHost() {
+ return (appConfig != null) ? appConfig.suppressHost() : ImmutableSet.of();
}
}
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfig.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfig.java
index 62b31b9..2932800 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfig.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfig.java
@@ -21,8 +21,11 @@
import com.google.common.collect.ImmutableSet;
import org.onlab.packet.MacAddress;
import org.onosproject.core.ApplicationId;
+import org.onosproject.net.ConnectPoint;
import org.onosproject.net.DeviceId;
import org.onosproject.net.config.Config;
+
+import java.util.Optional;
import java.util.Set;
import static com.google.common.base.MoreObjects.toStringHelper;
@@ -33,23 +36,25 @@
public class SegmentRoutingAppConfig extends Config<ApplicationId> {
private static final String VROUTER_MACS = "vRouterMacs";
private static final String VROUTER_ID = "vRouterId";
- private static final String EXCLUDE_PORTS = "excludePorts";
+ private static final String SUPPRESS_SUBNET = "suppressSubnet";
+ private static final String SUPPRESS_HOST = "suppressHost";
@Override
public boolean isValid() {
- return hasOnlyFields(VROUTER_MACS, VROUTER_ID, EXCLUDE_PORTS) &&
+ return hasOnlyFields(VROUTER_MACS, VROUTER_ID, SUPPRESS_SUBNET, SUPPRESS_HOST) &&
vRouterMacs() != null && vRouterId() != null &&
- excludePorts() != null;
+ suppressSubnet() != null && suppressHost() != null;
}
/**
* Gets vRouters from the config.
*
- * @return a set of vRouter MAC addresses
+ * @return Set of vRouter MAC addresses, empty is not specified,
+ * or null if not valid
*/
public Set<MacAddress> vRouterMacs() {
if (!object.has(VROUTER_MACS)) {
- return null;
+ return ImmutableSet.of();
}
ImmutableSet.Builder<MacAddress> builder = ImmutableSet.builder();
@@ -96,15 +101,16 @@
/**
* Gets vRouter device ID.
*
- * @return vRouter device ID, or null if not valid
+ * @return Optional vRouter device ID,
+ * empty is not specified or null if not valid
*/
- public DeviceId vRouterId() {
+ public Optional<DeviceId> vRouterId() {
if (!object.has(VROUTER_ID)) {
- return null;
+ return Optional.empty();
}
try {
- return DeviceId.deviceId(object.path(VROUTER_ID).asText());
+ return Optional.of(DeviceId.deviceId(object.path(VROUTER_ID).asText()));
} catch (IllegalArgumentException e) {
return null;
}
@@ -126,42 +132,95 @@
}
/**
- * Gets names of ports that are ignored by SegmentRouting.
+ * Gets names of ports to which SegmentRouting does not push subnet rules.
*
- * @return set of port names
+ * @return Set of port names, empty if not specified, or null
+ * if not valid
*/
- public Set<String> excludePorts() {
- if (!object.has(EXCLUDE_PORTS)) {
- return null;
+ public Set<ConnectPoint> suppressSubnet() {
+ if (!object.has(SUPPRESS_SUBNET)) {
+ return ImmutableSet.of();
}
- ImmutableSet.Builder<String> builder = ImmutableSet.builder();
- ArrayNode arrayNode = (ArrayNode) object.path(EXCLUDE_PORTS);
+ ImmutableSet.Builder<ConnectPoint> builder = ImmutableSet.builder();
+ ArrayNode arrayNode = (ArrayNode) object.path(SUPPRESS_SUBNET);
for (JsonNode jsonNode : arrayNode) {
String portName = jsonNode.asText(null);
if (portName == null) {
return null;
}
- builder.add(portName);
+ try {
+ builder.add(ConnectPoint.deviceConnectPoint(portName));
+ } catch (IllegalArgumentException e) {
+ return null;
+ }
}
return builder.build();
}
/**
- * Sets names of ports that are ignored by SegmentRouting.
+ * Sets names of ports to which SegmentRouting does not push subnet rules.
*
- * @param excludePorts names of ports that are ignored by SegmentRouting
+ * @param suppressSubnet names of ports to which SegmentRouting does not push
+ * subnet rules
* @return this {@link SegmentRoutingAppConfig}
*/
- public SegmentRoutingAppConfig setExcludePorts(Set<String> excludePorts) {
- if (excludePorts == null) {
- object.remove(EXCLUDE_PORTS);
+ public SegmentRoutingAppConfig setSuppressSubnet(Set<ConnectPoint> suppressSubnet) {
+ if (suppressSubnet == null) {
+ object.remove(SUPPRESS_SUBNET);
} else {
ArrayNode arrayNode = mapper.createArrayNode();
- excludePorts.forEach(portName -> {
- arrayNode.add(portName);
+ suppressSubnet.forEach(connectPoint -> {
+ arrayNode.add(connectPoint.deviceId() + "/" + connectPoint.port());
});
- object.set(EXCLUDE_PORTS, arrayNode);
+ object.set(SUPPRESS_SUBNET, arrayNode);
+ }
+ return this;
+ }
+
+ /**
+ * Gets names of ports to which SegmentRouting does not push host rules.
+ *
+ * @return Set of port names, empty if not specified, or null
+ * if not valid
+ */
+ public Set<ConnectPoint> suppressHost() {
+ if (!object.has(SUPPRESS_HOST)) {
+ return ImmutableSet.of();
+ }
+
+ ImmutableSet.Builder<ConnectPoint> builder = ImmutableSet.builder();
+ ArrayNode arrayNode = (ArrayNode) object.path(SUPPRESS_HOST);
+ for (JsonNode jsonNode : arrayNode) {
+ String portName = jsonNode.asText(null);
+ if (portName == null) {
+ return null;
+ }
+ try {
+ builder.add(ConnectPoint.deviceConnectPoint(portName));
+ } catch (IllegalArgumentException e) {
+ return null;
+ }
+ }
+ return builder.build();
+ }
+
+ /**
+ * Sets names of ports to which SegmentRouting does not push host rules.
+ *
+ * @param suppressHost names of ports to which SegmentRouting does not push
+ * host rules
+ * @return this {@link SegmentRoutingAppConfig}
+ */
+ public SegmentRoutingAppConfig setSuppressHost(Set<ConnectPoint> suppressHost) {
+ if (suppressHost == null) {
+ object.remove(SUPPRESS_HOST);
+ } else {
+ ArrayNode arrayNode = mapper.createArrayNode();
+ suppressHost.forEach(connectPoint -> {
+ arrayNode.add(connectPoint.deviceId() + "/" + connectPoint.port());
+ });
+ object.set(SUPPRESS_HOST, arrayNode);
}
return this;
}
@@ -170,7 +229,9 @@
public String toString() {
return toStringHelper(this)
.add("vRouterMacs", vRouterMacs())
- .add("excludePorts", excludePorts())
+ .add("vRouterId", vRouterId())
+ .add("suppressSubnet", suppressSubnet())
+ .add("suppressHost", suppressHost())
.toString();
}
}