Improve Xconnect to support BSOD use case
- Push ACL flow to ignore xconnect VLAN
- Add pair port to the L2FG internally
- Remove the ETH_TYPE restriction in OFDPA processVersatile
- Remove the NOACTION warning in OFDPA processVersatile
- Do not push bridging flow for hosts that have wrong VLAN tag
Known issue:
- flooding issue on the pair port
- tagged host will be learnt before xconnect config is pushed (will be ignored by SR)
Change-Id: I30e4f46e54daa0f7bd349419df100523dceb2c4c
diff --git a/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index 3982731..4c56db5 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -114,7 +114,10 @@
srManager.getPairLocalPort(pairDeviceId).ifPresent(pairRemotePort -> {
// NOTE: Since the pairLocalPort is trunk port, use assigned vlan of original port
// when the host is untagged
- VlanId vlanId = Optional.ofNullable(srManager.getInternalVlanId(location)).orElse(hostVlanId);
+ VlanId vlanId = vlanForPairPort(hostVlanId, location);
+ if (vlanId == null) {
+ return;
+ }
processBridgingRule(pairDeviceId, pairRemotePort, hostMac, vlanId, false);
ips.forEach(ip -> processRoutingRule(pairDeviceId, pairRemotePort, hostMac, vlanId,
@@ -158,7 +161,10 @@
if (pairDeviceId.isPresent() && pairLocalPort.isPresent()) {
// NOTE: Since the pairLocalPort is trunk port, use assigned vlan of original port
// when the host is untagged
- VlanId vlanId = Optional.ofNullable(srManager.getInternalVlanId(location)).orElse(hostVlanId);
+ VlanId vlanId = vlanForPairPort(hostVlanId, location);
+ if (vlanId == null) {
+ return;
+ }
processBridgingRule(pairDeviceId.get(), pairLocalPort.get(), hostMac, vlanId, true);
ips.forEach(ip ->
@@ -362,7 +368,10 @@
srManager.getPairLocalPort(pairDeviceId).ifPresent(pairRemotePort -> {
// NOTE: Since the pairLocalPort is trunk port, use assigned vlan of original port
// when the host is untagged
- VlanId vlanId = Optional.ofNullable(srManager.getInternalVlanId(location)).orElse(hostVlanId);
+ VlanId vlanId = vlanForPairPort(hostVlanId, location);
+ if (vlanId == null) {
+ return;
+ }
ipsToRemove.forEach(ip ->
processRoutingRule(pairDeviceId, pairRemotePort, hostMac, vlanId, ip, true)
@@ -526,6 +535,28 @@
}
/**
+ * Returns VLAN ID to be used to program redirection flow on pair port.
+ *
+ * @param hostVlanId host VLAN ID
+ * @param location host location
+ * @return VLAN ID to be used; Or null if host VLAN does not match the interface config
+ */
+ VlanId vlanForPairPort(VlanId hostVlanId, ConnectPoint location) {
+ VlanId internalVlan = srManager.getInternalVlanId(location);
+ Set<VlanId> taggedVlan = srManager.interfaceService.getTaggedVlanId(location);
+
+ if (!hostVlanId.equals(VlanId.NONE) && taggedVlan.contains(hostVlanId)) {
+ return hostVlanId;
+ } else if (hostVlanId.equals(VlanId.NONE) && internalVlan != null) {
+ return internalVlan;
+ } else {
+ log.warn("VLAN mismatch. hostVlan={}, location={}, internalVlan={}, taggedVlan={}",
+ hostVlanId, location, internalVlan, taggedVlan);
+ return null;
+ }
+ }
+
+ /**
* Update forwarding objective for unicast bridging and unicast routing.
* Also check the validity of updated interface configuration on VLAN.
*
diff --git a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
index 03ef5b5..8d64517 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
@@ -45,7 +45,12 @@
*/
public interface SegmentRoutingService {
/**
- * VLAN cross-connect priority.
+ * VLAN cross-connect ACL priority.
+ */
+ int XCONNECT_ACL_PRIORITY = 60000;
+
+ /**
+ * VLAN cross-connect Bridging priority.
*/
int XCONNECT_PRIORITY = 1000;
diff --git a/app/src/main/java/org/onosproject/segmentrouting/XConnectHandler.java b/app/src/main/java/org/onosproject/segmentrouting/XConnectHandler.java
index aa60684..96c1a38 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/XConnectHandler.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/XConnectHandler.java
@@ -17,7 +17,9 @@
package org.onosproject.segmentrouting;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import org.onlab.packet.MacAddress;
+import org.onlab.packet.VlanId;
import org.onlab.util.KryoNamespace;
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.DeviceId;
@@ -43,7 +45,6 @@
import org.onosproject.store.serializers.KryoNamespaces;
import org.onosproject.store.service.ConsistentMap;
import org.onosproject.store.service.Serializer;
-import org.onosproject.store.service.StorageService;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -59,18 +60,15 @@
private static final String CONFIG_NOT_FOUND = "XConnect config not found";
private static final String NOT_MASTER = "Not master controller";
private final SegmentRoutingManager srManager;
- private final StorageService storageService;
private final ConsistentMap<XConnectStoreKey, NextObjective> xConnectNextObjStore;
- private final KryoNamespace.Builder xConnectKryo;
- protected XConnectHandler(SegmentRoutingManager srManager) {
+ XConnectHandler(SegmentRoutingManager srManager) {
this.srManager = srManager;
- this.storageService = srManager.storageService;
- xConnectKryo = new KryoNamespace.Builder()
+ KryoNamespace.Builder xConnectKryo = new KryoNamespace.Builder()
.register(KryoNamespaces.API)
.register(XConnectStoreKey.class)
.register(NextObjContext.class);
- xConnectNextObjStore = storageService
+ xConnectNextObjStore = srManager.storageService
.<XConnectStoreKey, NextObjective>consistentMapBuilder()
.withName("onos-xconnect-nextobj-store")
.withSerializer(Serializer.using(xConnectKryo.build()))
@@ -91,9 +89,7 @@
return;
}
- config.getXconnects(deviceId).forEach(key -> {
- populateXConnect(key, config.getPorts(key));
- });
+ config.getXconnects(deviceId).forEach(key -> populateXConnect(key, config.getPorts(key)));
}
/**
@@ -101,12 +97,10 @@
*
* @param event network config added event
*/
- protected void processXConnectConfigAdded(NetworkConfigEvent event) {
+ void processXConnectConfigAdded(NetworkConfigEvent event) {
log.info("Processing XConnect CONFIG_ADDED");
XConnectConfig config = (XConnectConfig) event.config().get();
- config.getXconnects().forEach(key -> {
- populateXConnect(key, config.getPorts(key));
- });
+ config.getXconnects().forEach(key -> populateXConnect(key, config.getPorts(key)));
}
/**
@@ -114,7 +108,7 @@
*
* @param event network config updated event
*/
- protected void processXConnectConfigUpdated(NetworkConfigEvent event) {
+ void processXConnectConfigUpdated(NetworkConfigEvent event) {
log.info("Processing XConnect CONFIG_UPDATED");
XConnectConfig prevConfig = (XConnectConfig) event.prevConfig().get();
XConnectConfig config = (XConnectConfig) event.config().get();
@@ -130,15 +124,10 @@
!config.getPorts(key).equals(prevConfig.getPorts(key)))
.collect(Collectors.toSet());
- pendingRemove.forEach(key -> {
- revokeXConnect(key, prevConfig.getPorts(key));
- });
- pendingAdd.forEach(key -> {
- populateXConnect(key, config.getPorts(key));
- });
- pendingUpdate.forEach(key -> {
- updateXConnect(key, prevConfig.getPorts(key), config.getPorts(key));
- });
+ pendingRemove.forEach(key -> revokeXConnect(key, prevConfig.getPorts(key)));
+ pendingAdd.forEach(key -> populateXConnect(key, config.getPorts(key)));
+ pendingUpdate.forEach(key ->
+ updateXConnect(key, prevConfig.getPorts(key), config.getPorts(key)));
}
/**
@@ -146,7 +135,7 @@
*
* @param event network config removed event
*/
- protected void processXConnectConfigRemoved(NetworkConfigEvent event) {
+ void processXConnectConfigRemoved(NetworkConfigEvent event) {
log.info("Processing XConnect CONFIG_REMOVED");
XConnectConfig prevConfig = (XConnectConfig) event.prevConfig().get();
prevConfig.getXconnects().forEach(key -> {
@@ -183,8 +172,11 @@
log.info("Abort populating XConnect {}: {}", key, NOT_MASTER);
return;
}
+
+ ports = addPairPort(key.deviceId(), ports);
populateFilter(key, ports);
populateFwd(key, populateNext(key, ports));
+ populateAcl(key);
}
/**
@@ -213,7 +205,7 @@
* @param ports XConnect ports
*/
private NextObjective populateNext(XConnectStoreKey key, Set<PortNumber> ports) {
- NextObjective nextObj = null;
+ NextObjective nextObj;
if (xConnectNextObjStore.containsKey(key)) {
nextObj = xConnectNextObjStore.get(key).value();
log.debug("NextObj for {} found, id={}", key, nextObj.id());
@@ -229,7 +221,7 @@
}
/**
- * Populates forwarding objectives for given XConnect.
+ * Populates bridging forwarding objectives for given XConnect.
*
* @param key XConnect store key
* @param nextObj next objective
@@ -244,6 +236,20 @@
}
/**
+ * Populates ACL forwarding objectives for given XConnect.
+ *
+ * @param key XConnect store key
+ */
+ private void populateAcl(XConnectStoreKey key) {
+ ForwardingObjective.Builder aclObjBuilder = aclObjBuilder(key.vlanId());
+ ObjectiveContext aclContext = new DefaultObjectiveContext(
+ (objective) -> log.debug("XConnect AclObj for {} populated", key),
+ (objective, error) ->
+ log.warn("Failed to populate XConnect AclObj for {}: {}", key, error));
+ srManager.flowObjectiveService.forward(key.deviceId(), aclObjBuilder.add(aclContext));
+ }
+
+ /**
* Revokes XConnect groups and flows for given key.
*
* @param key XConnect key
@@ -255,6 +261,7 @@
return;
}
+ ports = addPairPort(key.deviceId(), ports);
revokeFilter(key, ports);
if (xConnectNextObjStore.containsKey(key)) {
NextObjective nextObj = xConnectNextObjStore.get(key).value();
@@ -263,6 +270,7 @@
} else {
log.warn("NextObj for {} does not exist in the store.", key);
}
+ revokeAcl(key);
}
/**
@@ -316,7 +324,7 @@
}
/**
- * Revokes forwarding objectives for given XConnect.
+ * Revokes bridging forwarding objectives for given XConnect.
*
* @param key XConnect store key
* @param nextObj next objective
@@ -347,6 +355,21 @@
}
/**
+ * Revokes ACL forwarding objectives for given XConnect.
+ *
+ * @param key XConnect store key
+ */
+ private void revokeAcl(XConnectStoreKey key) {
+ ForwardingObjective.Builder aclObjBuilder = aclObjBuilder(key.vlanId());
+ ObjectiveContext aclContext = new DefaultObjectiveContext(
+ (objective) -> log.debug("XConnect AclObj for {} populated", key),
+ (objective, error) ->
+ log.warn("Failed to populate XConnect AclObj for {}: {}", key, error));
+ srManager.flowObjectiveService
+ .forward(key.deviceId(), aclObjBuilder.remove(aclContext));
+ }
+
+ /**
* Updates XConnect groups and flows for given key.
*
* @param key XConnect key
@@ -355,14 +378,15 @@
*/
private void updateXConnect(XConnectStoreKey key, Set<PortNumber> prevPorts,
Set<PortNumber> ports) {
+ // NOTE: ACL flow doesn't include port information. No need to update it.
+ // Pair port is built-in and thus not going to change. No need to update it.
+
// remove old filter
- prevPorts.stream().filter(port -> !ports.contains(port)).forEach(port -> {
- revokeFilter(key, ImmutableSet.of(port));
- });
+ prevPorts.stream().filter(port -> !ports.contains(port)).forEach(port ->
+ revokeFilter(key, ImmutableSet.of(port)));
// install new filter
- ports.stream().filter(port -> !prevPorts.contains(port)).forEach(port -> {
- populateFilter(key, ImmutableSet.of(port));
- });
+ ports.stream().filter(port -> !prevPorts.contains(port)).forEach(port ->
+ populateFilter(key, ImmutableSet.of(port)));
CompletableFuture<ObjectiveError> fwdFuture = new CompletableFuture<>();
CompletableFuture<ObjectiveError> nextFuture = new CompletableFuture<>();
@@ -394,12 +418,10 @@
*
* @param deviceId device ID
*/
- protected void removeDevice(DeviceId deviceId) {
+ void removeDevice(DeviceId deviceId) {
xConnectNextObjStore.entrySet().stream()
.filter(entry -> entry.getKey().deviceId().equals(deviceId))
- .forEach(entry -> {
- xConnectNextObjStore.remove(entry.getKey());
- });
+ .forEach(entry -> xConnectNextObjStore.remove(entry.getKey()));
log.debug("{} is removed from xConnectNextObjStore", deviceId);
}
@@ -427,11 +449,11 @@
}
/**
- * Creates a forwarding objective builder for XConnect.
+ * Creates a bridging forwarding objective builder for XConnect.
*
* @param key XConnect key
* @param nextId next ID of the broadcast group for this XConnect key
- * @return next objective builder
+ * @return forwarding objective builder
*/
private ForwardingObjective.Builder fwdObjBuilder(XConnectStoreKey key, int nextId) {
/*
@@ -453,6 +475,28 @@
}
/**
+ * Creates an ACL forwarding objective builder for XConnect.
+ *
+ * @param vlanId cross connect VLAN id
+ * @return forwarding objective builder
+ */
+ private ForwardingObjective.Builder aclObjBuilder(VlanId vlanId) {
+ TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
+ sbuilder.matchVlanId(vlanId);
+
+ TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
+
+ ForwardingObjective.Builder fob = DefaultForwardingObjective.builder();
+ fob.withFlag(ForwardingObjective.Flag.VERSATILE)
+ .withSelector(sbuilder.build())
+ .withTreatment(tbuilder.build())
+ .withPriority(SegmentRoutingService.XCONNECT_ACL_PRIORITY)
+ .fromApp(srManager.appId)
+ .makePermanent();
+ return fob;
+ }
+
+ /**
* Creates a filtering objective builder for XConnect.
*
* @param key XConnect key
@@ -468,6 +512,19 @@
return fob.permit().fromApp(srManager.appId);
}
+ /**
+ * Add pair port to the given set of port.
+ *
+ * @param deviceId device Id
+ * @param ports ports specified in the xconnect config
+ * @return port specified in the xconnect config plus the pair port (if configured)
+ */
+ private Set<PortNumber> addPairPort(DeviceId deviceId, Set<PortNumber> ports) {
+ Set<PortNumber> newPorts = Sets.newHashSet(ports);
+ srManager.getPairLocalPort(deviceId).ifPresent(newPorts::add);
+ return newPorts;
+ }
+
// TODO: Lambda closure in DefaultObjectiveContext cannot be serialized properly
// with Kryo 3.0.3. It will be fixed in 3.0.4. By then we can use
// DefaultObjectiveContext again.
diff --git a/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java b/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
index 6783da3..30462fb 100644
--- a/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
+++ b/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
@@ -123,7 +123,8 @@
private static final HostLocation HOST_LOC61 = new HostLocation(CP61, 0);
// Interface VLAN
private static final VlanId INTF_VLAN_UNTAGGED = VlanId.vlanId((short) 10);
- private static final Set<VlanId> INTF_VLAN_TAGGED = Sets.newHashSet(VlanId.vlanId((short) 20));
+ private static final VlanId INTF_VLAN_TAGGED_1 = VlanId.vlanId((short) 20);
+ private static final Set<VlanId> INTF_VLAN_TAGGED = Sets.newHashSet(INTF_VLAN_TAGGED_1);
private static final VlanId INTF_VLAN_NATIVE = VlanId.vlanId((short) 30);
private static final Set<VlanId> INTF_VLAN_PAIR = Sets.newHashSet(VlanId.vlanId((short) 10),
VlanId.vlanId((short) 20), VlanId.vlanId((short) 30));
@@ -855,4 +856,15 @@
assertNotNull(BRIDGING_TABLE.get(new MockBridgingTableKey(DEV1, HOST_MAC, INTF_VLAN_UNTAGGED)));
assertNotNull(BRIDGING_TABLE.get(new MockBridgingTableKey(DEV2, HOST_MAC, INTF_VLAN_UNTAGGED)));
}
+
+ @Test
+ public void testVlanForPairPort() {
+ assertEquals(INTF_VLAN_UNTAGGED, hostHandler.vlanForPairPort(VlanId.NONE, CP11));
+ assertEquals(INTF_VLAN_NATIVE, hostHandler.vlanForPairPort(VlanId.NONE, CP13));
+ assertEquals(INTF_VLAN_TAGGED_1, hostHandler.vlanForPairPort(INTF_VLAN_TAGGED_1, CP13));
+ assertNull(hostHandler.vlanForPairPort(INTF_VLAN_UNTAGGED, CP11));
+ assertNull(hostHandler.vlanForPairPort(INTF_VLAN_UNTAGGED, CP13));
+ assertNull(hostHandler.vlanForPairPort(VlanId.NONE, CP51));
+ assertNull(hostHandler.vlanForPairPort(INTF_VLAN_UNTAGGED, CP51));
+ }
}