[AETHER-537][AETHER-1031][AETHER-540] Several bugfixes for SR (p2)
This patch includes:
- new method to update the indirect nexthops when the
internal vlan is changed
- fix update l2 interface scenario (tagged <-> untagged)
- fix a race condition during the removal of fwd objectives
- new method to signal the driver when a port update is happening
- minor fixes in the log messages
- fix two issues in the update of the l3 unicast. One is related
to the vlan to be used during the nexthop moves. Another issue
was happening when there was a mismatch between hostVlan and
vlan configuration on a specific port
Change-Id: I149a03c09daafc015ea4bf131b0968761d02273c
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index de3fd73..7a65e95 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -1405,19 +1405,21 @@
return CompletableFuture.completedFuture(null);
}
- void updateBridging(DeviceId deviceId, PortNumber portNum, MacAddress hostMac,
- VlanId vlanId, boolean popVlan, boolean install) {
+ CompletableFuture<Objective> updateBridging(DeviceId deviceId, PortNumber portNum, MacAddress hostMac,
+ VlanId vlanId, boolean popVlan, boolean install) {
if (shouldProgram(deviceId)) {
- srManager.routingRulePopulator.updateBridging(deviceId, portNum, hostMac, vlanId, popVlan, install);
+ return srManager.routingRulePopulator.updateBridging(deviceId, portNum, hostMac, vlanId, popVlan, install);
}
+ return CompletableFuture.completedFuture(null);
}
- void updateFwdObj(DeviceId deviceId, PortNumber portNumber, IpPrefix prefix, MacAddress hostMac,
- VlanId vlanId, boolean popVlan, boolean install) {
+ CompletableFuture<Objective> updateFwdObj(DeviceId deviceId, PortNumber portNumber, IpPrefix prefix,
+ MacAddress hostMac, VlanId vlanId, boolean popVlan, boolean install) {
if (shouldProgram(deviceId)) {
- srManager.routingRulePopulator.updateFwdObj(deviceId, portNumber, prefix, hostMac,
+ return srManager.routingRulePopulator.updateFwdObj(deviceId, portNumber, prefix, hostMac,
vlanId, popVlan, install);
}
+ return CompletableFuture.completedFuture(null);
}
/**
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/impl/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index 46231b0..07e619f 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -208,12 +208,17 @@
}
});
+ // NOTE: that we use the nexthop vlanId to retrieve the nextId
+ // while the vlanId used to program the L3 unicast chain
+ // is derived from the port configuration. In case of
+ // a tagged interface we use host vlanId. Host vlan should
+ // be part of the tags configured for that port. See the
+ // code in DefaultGroupHandler.updateL3UcastGroupBucket
int nextId = srManager.getMacVlanNextObjectiveId(location.deviceId(), hostMac, hostVlanId, null, false);
if (nextId != -1) {
- VlanId vlanId = Optional.ofNullable(srManager.getInternalVlanId(location)).orElse(hostVlanId);
log.debug(" Updating next objective for device {}, host {}/{}, port {}, nextid {}",
- location.deviceId(), hostMac, vlanId, location.port(), nextId);
- srManager.updateMacVlanTreatment(location.deviceId(), hostMac, vlanId,
+ location.deviceId(), hostMac, hostVlanId, location.port(), nextId);
+ srManager.updateMacVlanTreatment(location.deviceId(), hostMac, hostVlanId,
location.port(), nextId);
}
@@ -759,8 +764,21 @@
return;
}
- hosts.forEach(host -> hostWorkers.execute(() -> processIntfVlanUpdatedEventInternal(
- host, deviceId, portNum, vlanId, popVlan, install), host.id().hashCode()));
+ List<CompletableFuture<Void>> hostFutures = Lists.newArrayList();
+ hosts.forEach(host -> hostFutures.add(hostWorkers.submit(() -> processIntfVlanUpdatedEventInternal(
+ host, deviceId, portNum, vlanId, popVlan, install), host.id().hashCode())));
+ // Let's wait for the completion of all hosts
+ try {
+ CompletableFuture.allOf(hostFutures.toArray(new CompletableFuture[0]))
+ .thenApply(objectives -> hostFutures.stream()
+ .map(CompletableFuture::join)
+ .collect(Collectors.toList())
+ )
+ .get();
+ } catch (InterruptedException | ExecutionException e) {
+ log.warn("Exception caught when executing processIntfVlanUpdatedEvent futures");
+ hostFutures.forEach(future -> future.cancel(false));
+ }
}
private void processIntfVlanUpdatedEventInternal(Host host, DeviceId deviceId, PortNumber portNum,
@@ -768,30 +786,43 @@
MacAddress mac = host.mac();
VlanId hostVlanId = host.vlan();
+ List<CompletableFuture<Objective>> futures = Lists.newArrayList();
if (!install) {
// Do not check the host validity. Just remove all rules corresponding to the vlan id
// Revoke forwarding objective for bridging to the host
- srManager.defaultRoutingHandler.updateBridging(deviceId, portNum, mac, vlanId, popVlan, false);
+ futures.add(srManager.defaultRoutingHandler.updateBridging(deviceId, portNum, mac, vlanId,
+ popVlan, false));
// Revoke forwarding objective and corresponding simple Next objective
// for each Host and IP address connected to given port
host.ipAddresses().forEach(ipAddress ->
- srManager.routingRulePopulator.updateFwdObj(deviceId, portNum, ipAddress.toIpPrefix(),
- mac, vlanId, popVlan, false)
- );
+ futures.add(srManager.defaultRoutingHandler.updateFwdObj(deviceId, portNum, ipAddress.toIpPrefix(),
+ mac, vlanId, popVlan, false)));
} else {
// Check whether the host vlan is valid for new interface configuration
if ((!popVlan && hostVlanId.equals(vlanId)) ||
(popVlan && hostVlanId.equals(VlanId.NONE))) {
- srManager.defaultRoutingHandler.updateBridging(deviceId, portNum, mac, vlanId, popVlan, true);
+ futures.add(srManager.defaultRoutingHandler.updateBridging(deviceId, portNum, mac, vlanId,
+ popVlan, true));
// Update Forwarding objective and corresponding simple Next objective
// for each Host and IP address connected to given port
host.ipAddresses().forEach(ipAddress ->
- srManager.routingRulePopulator.updateFwdObj(deviceId, portNum, ipAddress.toIpPrefix(),
- mac, vlanId, popVlan, true)
- );
+ futures.add(srManager.defaultRoutingHandler.updateFwdObj(deviceId, portNum, ipAddress.toIpPrefix(),
+ mac, vlanId, popVlan, true)));
}
}
+ // Let's wait for the completion of update procedure
+ try {
+ CompletableFuture.allOf(futures.toArray(new CompletableFuture[0]))
+ .thenApply(objectives -> futures.stream()
+ .map(CompletableFuture::join)
+ .collect(Collectors.toList())
+ )
+ .get();
+ } catch (InterruptedException | ExecutionException e) {
+ log.warn("Exception caught when executing processIntfVlanUpdatedEventInternal futures");
+ futures.forEach(future -> future.cancel(false));
+ }
}
/**
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java b/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
index d9372fe..c0ccfb6 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
@@ -22,6 +22,7 @@
import org.onlab.packet.MacAddress;
import org.onlab.packet.VlanId;
import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.Host;
import org.onosproject.net.HostLocation;
import org.onosproject.net.PortNumber;
import org.onosproject.net.host.HostEvent;
@@ -264,20 +265,24 @@
if (!batchedSubnets.isEmpty()) {
// For each new location, if NextObj exists for the host, update with new location ..
Sets.difference(newLocations, prevLocations).forEach(newLocation -> {
- int nextId = srManager.getMacVlanNextObjectiveId(newLocation.deviceId(),
- hostMac, hostVlanId, null, false);
- VlanId vlanId = Optional.ofNullable(srManager.getInternalVlanId(newLocation)).orElse(hostVlanId);
-
- if (nextId != -1) {
- //Update the nextId group bucket
- log.debug("HostMoved. NextId exists, update L3 Ucast Group Bucket {}, {}, {} --> {}",
- newLocation, hostMac, vlanId, nextId);
- srManager.updateMacVlanTreatment(newLocation.deviceId(), hostMac, vlanId,
- newLocation.port(), nextId);
- } else {
- log.debug("HostMoved. NextId does not exist for this location {}, host {}/{}",
- newLocation, hostMac, vlanId);
- }
+ // NOTE: that we use the nexthop vlanId to retrieve the nextId
+ // while the vlanId used to program the L3 unicast chain
+ // is derived from the port configuration. In case of
+ // a tagged interface we use host vlanId. Host vlan should
+ // be part of the tags configured for that port. See the
+ // code in DefaultGroupHandler.updateL3UcastGroupBucket
+ int nextId = srManager.getMacVlanNextObjectiveId(newLocation.deviceId(),
+ hostMac, hostVlanId, null, false);
+ if (nextId != -1) {
+ //Update the nextId group bucket
+ log.debug("HostMoved. NextId exists, update L3 Ucast Group Bucket {}, {}, {} --> {}",
+ newLocation, hostMac, hostVlanId, nextId);
+ srManager.updateMacVlanTreatment(newLocation.deviceId(), hostMac, hostVlanId,
+ newLocation.port(), nextId);
+ } else {
+ log.debug("HostMoved. NextId does not exist for this location {}, host {}/{}",
+ newLocation, hostMac, hostVlanId);
+ }
});
}
@@ -327,6 +332,40 @@
}
+ public void processIntfVlanUpdatedEvent(DeviceId deviceId, PortNumber portNum) {
+ log.info("processIntfVlanUpdatedEvent {}/{}", deviceId, portNum);
+
+ // Verify there are hosts attached to the port
+ ConnectPoint connectPoint = new ConnectPoint(deviceId, portNum);
+ Set<Host> hosts = srManager.hostService.getConnectedHosts(connectPoint);
+ if (hosts == null || hosts.size() == 0) {
+ log.debug("processIntfVlanUpdatedEvent: No hosts connected to {}", connectPoint);
+ return;
+ }
+
+ // Iterate over the hosts and update if needed the vlan of the nextobjective
+ hosts.forEach(host -> {
+ // Verify if there is a nexthop and only then update the l3 indirect unicast chain
+ int nextId = srManager.getMacVlanNextObjectiveId(deviceId, host.mac(), host.vlan(),
+ null, false);
+ if (nextId != -1) {
+ //Update the nextId group bucket
+ log.debug("intfVlanUpdated. NextId exists, update L3 Ucast Group Bucket {}, {}, {} --> {}",
+ connectPoint, host.mac(), host.vlan(), nextId);
+ // NOTE: that we use the nexthop vlanId to retrieve the nextId
+ // while the vlanId used to program the L3 unicast chain
+ // is derived from the port configuration. In case of
+ // a tagged interface we use host vlanId. Host vlan should
+ // be part of the tags configured for that port. See the
+ // code in DefaultGroupHandler.updateL3UcastGroupBucket
+ srManager.updateMacVlanTreatment(deviceId, host.mac(), host.vlan(), portNum, nextId);
+ } else {
+ log.debug("intfVlanUpdated. NextId does not exist for this location {}, host {}/{}",
+ connectPoint, host.mac(), host.vlan());
+ }
+ });
+ }
+
private boolean isReady() {
return Objects.nonNull(srManager.deviceConfiguration) &&
Objects.nonNull(srManager.defaultRoutingHandler);
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index c3ecfb9..751bbcc 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -67,6 +67,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;
@@ -78,7 +79,6 @@
import static org.onlab.packet.ICMP6.ROUTER_ADVERTISEMENT;
import static org.onlab.packet.ICMP6.ROUTER_SOLICITATION;
import static org.onlab.packet.IPv6.PROTOCOL_ICMP6;
-import static org.onosproject.segmentrouting.SegmentRoutingService.DEFAULT_PRIORITY;
/**
* Populator of segment routing flow rules.
@@ -95,7 +95,9 @@
// used for signalling the driver to remove vlan table and tmac entry also
private static final long CLEANUP_DOUBLE_TAGGED_HOST_ENTRIES = 1;
- private static final long DOUBLE_TAGGED_METADATA_MASK = 0xffffffffffffffffL;
+ // used for signalling the driver when not remove tmac entries
+ private static final long INTERFACE_CONFIG_UPDATE = 2;
+ private static final long METADATA_MASK = 0xffffffffffffffffL;
/**
* Creates a RoutingRulePopulator object.
@@ -232,7 +234,7 @@
mbuilder.matchVlanId(nativeVlan);
tbuilder.immediate().popVlan();
} else {
- log.warn("Untagged host {}/{} is not allowed on {} without untagged or native" +
+ log.warn("Untagged host {}/{} is not allowed on {} without untagged or native " +
"vlan config", mac, hostVlanId, connectPoint);
return null;
}
@@ -272,10 +274,11 @@
* @param vlanId Vlan ID configured on the switch port
* @param popVlan true to pop Vlan tag at TrafficTreatment, false otherwise
* @param install true to populate the objective, false to revoke
+ * @return a completable future that completes when the update of the bridging rule completes
*/
// TODO Refactor. There are a lot of duplications between this method, populateBridging,
// revokeBridging and bridgingFwdObjBuilder.
- void updateBridging(DeviceId deviceId, PortNumber portNum, MacAddress hostMac,
+ CompletableFuture<Objective> updateBridging(DeviceId deviceId, PortNumber portNum, MacAddress hostMac,
VlanId vlanId, boolean popVlan, boolean install) {
// Create host selector
TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
@@ -297,6 +300,7 @@
int portNextObjId = srManager.getPortNextObjectiveId(deviceId, portNum,
tbuilder.build(), mbuilder.build(), install);
+ CompletableFuture<Objective> future = new CompletableFuture<>();
if (portNextObjId != -1) {
ForwardingObjective.Builder fob = DefaultForwardingObjective.builder()
.withFlag(ForwardingObjective.Flag.SPECIFIC)
@@ -307,14 +311,21 @@
.makePermanent();
ObjectiveContext context = new DefaultObjectiveContext(
- (objective) -> log.debug("Brigding rule for {}/{} {}", hostMac, vlanId,
- install ? "populated" : "revoked"),
- (objective, error) -> log.warn("Failed to {} bridging rule for {}/{}: {}",
- install ? "populate" : "revoke", hostMac, vlanId, error));
+ (objective) -> {
+ log.debug("Brigding rule for {}/{} {}", hostMac, vlanId, install ? "populated" : "revoked");
+ future.complete(objective);
+ },
+ (objective, error) -> {
+ log.warn("Failed to {} bridging rule for {}/{}: {}", install ? "populate" : "revoke",
+ hostMac, vlanId, error);
+ future.complete(null);
+ });
srManager.flowObjectiveService.forward(deviceId, install ? fob.add(context) : fob.remove(context));
} else {
log.warn("Failed to retrieve next objective for {}/{}", hostMac, vlanId);
+ return CompletableFuture.completedFuture(null);
}
+ return future;
}
/**
@@ -1053,31 +1064,34 @@
if (taggedVlans.size() != 0) {
// Filter for tagged vlans
if (!srManager.interfaceService.getTaggedVlanId(connectPoint).stream().allMatch(taggedVlanId ->
- processSinglePortFiltersInternal(deviceId, portnum, false, taggedVlanId, install))) {
+ processSinglePortFiltersInternal(deviceId, portnum, false, taggedVlanId,
+ install, false))) {
return false;
}
if (nativeVlan != null) {
// Filter for native vlan
- if (!processSinglePortFiltersInternal(deviceId, portnum, true, nativeVlan, install)) {
+ if (!processSinglePortFiltersInternal(deviceId, portnum, true, nativeVlan,
+ install, false)) {
return false;
}
}
} else if (untaggedVlan != null) {
// Filter for untagged vlan
- if (!processSinglePortFiltersInternal(deviceId, portnum, true, untaggedVlan, install)) {
+ if (!processSinglePortFiltersInternal(deviceId, portnum, true, untaggedVlan,
+ install, false)) {
return false;
}
} else if (!hasIPConfiguration(connectPoint)) {
// Filter for unconfigured upstream port, using INTERNAL_VLAN
if (!processSinglePortFiltersInternal(deviceId, portnum, true,
srManager.getDefaultInternalVlan(),
- install)) {
+ install, false)) {
return false;
}
// Filter for receiveing pseudowire traffic
if (!processSinglePortFiltersInternal(deviceId, portnum, false,
srManager.getPwTransportVlan(),
- install)) {
+ install, false)) {
return false;
}
}
@@ -1095,14 +1109,19 @@
*/
void updateSinglePortFilters(DeviceId deviceId, PortNumber portNum,
boolean pushVlan, VlanId vlanId, boolean install) {
- if (!processSinglePortFiltersInternal(deviceId, portNum, pushVlan, vlanId, install)) {
+ // NOTE: update port filters is used only when the vlan configuration
+ // has been updated. Port configuration is never removed here.
+ // For pipeline sharing the TMAC entry among different vlans
+ // there is no need to push or remove the TMAC entry
+ if (!processSinglePortFiltersInternal(deviceId, portNum, pushVlan, vlanId, install, true)) {
log.warn("Failed to update FilteringObjective for {}/{} with vlan {}",
deviceId, portNum, vlanId);
}
}
private boolean processSinglePortFiltersInternal(DeviceId deviceId, PortNumber portnum,
- boolean pushVlan, VlanId vlanId, boolean install) {
+ boolean pushVlan, VlanId vlanId, boolean install,
+ boolean update) {
boolean doTMAC = true;
if (!pushVlan) {
@@ -1119,7 +1138,7 @@
}
}
- FilteringObjective.Builder fob = buildFilteringObjective(deviceId, portnum, pushVlan, vlanId, doTMAC);
+ FilteringObjective.Builder fob = buildFilteringObjective(deviceId, portnum, pushVlan, vlanId, doTMAC, update);
if (fob == null) {
// error encountered during build
return false;
@@ -1140,7 +1159,8 @@
}
private FilteringObjective.Builder buildFilteringObjective(DeviceId deviceId, PortNumber portnum,
- boolean pushVlan, VlanId vlanId, boolean doTMAC) {
+ boolean pushVlan, VlanId vlanId,
+ boolean doTMAC, boolean update) {
MacAddress deviceMac;
try {
deviceMac = config.getDeviceMac(deviceId);
@@ -1175,6 +1195,13 @@
tBuilder.wipeDeferred();
}
+ // NOTE: Some switch hardware share the same tmac flow among different vlans.
+ // We use this metadata to let the driver know that there is still a vlan
+ // configuration associated to that port
+ if (update) {
+ tBuilder.writeMetadata(INTERFACE_CONFIG_UPDATE, METADATA_MASK);
+ }
+
fob.withMeta(tBuilder.build());
fob.permit().fromApp(srManager.appId);
@@ -1257,9 +1284,9 @@
// special metadata for driver
if (cleanupDoubleTaggedRules) {
- tBuilder.writeMetadata(CLEANUP_DOUBLE_TAGGED_HOST_ENTRIES, DOUBLE_TAGGED_METADATA_MASK);
+ tBuilder.writeMetadata(CLEANUP_DOUBLE_TAGGED_HOST_ENTRIES, METADATA_MASK);
} else {
- tBuilder.writeMetadata(0, DOUBLE_TAGGED_METADATA_MASK);
+ tBuilder.writeMetadata(0, METADATA_MASK);
}
// NOTE: Some switch hardware share the same filtering flow among different ports.
@@ -1677,9 +1704,11 @@
.fromApp(srManager.appId)
.makePermanent();
ObjectiveContext context = new DefaultObjectiveContext(
- (objective) -> log.debug("Vlan broadcast rule for {} populated", vlanId),
+ (objective) -> log.debug("Vlan broadcast rule for {} {}", vlanId, install ?
+ "populated" : "removed"),
(objective, error) ->
- log.warn("Failed to populate vlan broadcast rule for {}: {}", vlanId, error));
+ log.warn("Failed to {} vlan broadcast rule for {}: {}", install ? "populate" : "remove",
+ vlanId, error));
if (install) {
srManager.flowObjectiveService.forward(deviceId, fob.add(context));
@@ -1705,9 +1734,11 @@
* @param vlanId Vlan ID of the port
* @param popVlan true to pop vlan tag in TrafficTreatment
* @param install true to populate the forwarding objective, false to revoke
+ * @return a completable future that completes when the fwdobj completes. In case of removal,
+ * the completable future completes when also the nextobj completes.
*/
- void updateFwdObj(DeviceId deviceId, PortNumber portNumber, IpPrefix prefix, MacAddress hostMac,
- VlanId vlanId, boolean popVlan, boolean install) {
+ CompletableFuture<Objective> updateFwdObj(DeviceId deviceId, PortNumber portNumber, IpPrefix prefix,
+ MacAddress hostMac, VlanId vlanId, boolean popVlan, boolean install) {
ForwardingObjective.Builder fob;
TrafficSelector.Builder sbuilder = buildIpSelectorFromIpPrefix(prefix);
MacAddress deviceMac;
@@ -1715,7 +1746,7 @@
deviceMac = config.getDeviceMac(deviceId);
} catch (DeviceConfigNotFoundException e) {
log.warn(e.getMessage() + " Aborting updateFwdObj.");
- return;
+ return CompletableFuture.completedFuture(null);
}
TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
@@ -1738,9 +1769,10 @@
// the next-objective does not exist, then a new one should not be created
int portNextObjId = srManager.getPortNextObjectiveId(deviceId, portNumber,
tbuilder.build(), mbuilder.build(), install);
+ CompletableFuture<Objective> future = new CompletableFuture<>();
if (portNextObjId == -1) {
// Warning log will come from getPortNextObjective method
- return;
+ return CompletableFuture.completedFuture(null);
}
fob = DefaultForwardingObjective.builder().withSelector(sbuilder.build())
@@ -1748,10 +1780,15 @@
.withPriority(getPriorityFromPrefix(prefix)).withFlag(ForwardingObjective.Flag.SPECIFIC);
ObjectiveContext context = new DefaultObjectiveContext(
- (objective) -> log.debug("IP rule for route {} {}", prefix, install ? "installed" : "revoked"),
- (objective, error) ->
- log.warn("Failed to {} IP rule for route {}: {}",
- install ? "install" : "revoke", prefix, error));
+ (objective) -> {
+ log.debug("IP rule for route {} {}", prefix, install ? "installed" : "revoked");
+ future.complete(objective);
+ },
+ (objective, error) -> {
+ log.warn("Failed to {} IP rule for route {}: {}", install ? "install" : "revoke",
+ prefix, error);
+ future.complete(null);
+ });
srManager.flowObjectiveService.forward(deviceId, install ? fob.add(context) : fob.remove(context));
if (!install) {
@@ -1761,11 +1798,19 @@
if (grpHandler == null) {
log.warn("updateFwdObj: groupHandler for device {} not found", deviceId);
} else {
- // Remove L3UG for the given port and host
- grpHandler.removeGroupFromPort(portNumber, tbuilder.build(), mbuilder.build());
+ // Before moving forward we have to be sure flow has been removed;
+ try {
+ future.get();
+ } catch (InterruptedException | ExecutionException e) {
+ log.warn("Exception caught when executing IP rule removal for route {}", prefix);
+ }
+ // Remove L3UG for the given port and host and return the future
+ // Flow future has been already consumed (normally or exceptionally)
+ return grpHandler.removeGroupFromPort(portNumber, tbuilder.build(), mbuilder.build());
}
}
}
+ return future;
}
/**
@@ -1793,42 +1838,6 @@
}
/**
- * Returns a forwarding objective builder for egress forwarding rules.
- * <p>
- * The forwarding objective installs flow rules to egress pipeline to push
- * two vlan headers with given inner, outer vlan ids and outer tpid.
- *
- * @param portNumber port where the next hop attaches to
- * @param dummyVlanId vlan ID of the packet to match
- * @param innerVlan inner vlan ID of the next hop
- * @param outerVlan outer vlan ID of the next hop
- * @param outerTpid outer TPID of the next hop
- * @return forwarding objective builder
- */
- private ForwardingObjective.Builder egressFwdObjBuilder(PortNumber portNumber, VlanId dummyVlanId,
- VlanId innerVlan, VlanId outerVlan, EthType outerTpid) {
- TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
- sbuilder.matchVlanId(dummyVlanId);
- TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
- tbuilder.setOutput(portNumber).setVlanId(innerVlan);
-
- if (outerTpid.equals(EthType.EtherType.QINQ.ethType())) {
- tbuilder.pushVlan(outerTpid);
- } else {
- tbuilder.pushVlan();
- }
-
- tbuilder.setVlanId(outerVlan);
- return DefaultForwardingObjective.builder()
- .withSelector(sbuilder.build())
- .withTreatment(tbuilder.build())
- .fromApp(srManager.appId)
- .makePermanent()
- .withPriority(DEFAULT_PRIORITY)
- .withFlag(ForwardingObjective.Flag.EGRESS);
- }
-
- /**
* Populates IP rules for a route that has double-tagged next hop.
*
* @param deviceId device ID of the device that next hop attaches to
@@ -1884,7 +1893,6 @@
ForwardingObjective.Builder fwdBuilder;
log.debug("Revoking direct routing entry for double-tagged host route {} at {}:{}",
prefix, deviceId, outPort);
-
try {
fwdBuilder = routingFwdObjBuilder(deviceId, prefix, hostMac, outerVlan, outPort, innerVlan, outerTpid,
true, true);
@@ -1901,7 +1909,6 @@
int nextId = fwdBuilder.remove().nextId();
DefaultObjectiveContext context = new DefaultObjectiveContext(objective -> {
log.debug("Direct routing rule for double-tagged host route {} revoked. nextId={}", prefix, nextId);
-
// Try to remove next objective as well
ImmutablePair<TrafficTreatment, TrafficSelector> treatmentAndMeta;
try {
@@ -1910,12 +1917,10 @@
log.error(e.getMessage() + " Aborting revokeDoubleTaggedRoute");
return;
}
-
if (treatmentAndMeta == null) {
// Warning log will come from getTreatmentAndMeta method
return;
}
-
DefaultGroupHandler groupHandler = srManager.getGroupHandler(deviceId);
if (groupHandler == null) {
log.warn("Failed to revoke direct routing rule for double-tagged host route {}: " +
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index 4e15cad..9e98218 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -146,9 +146,11 @@
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;
@@ -1314,7 +1316,7 @@
* @param nextId of Next Obj which needs to be updated.
*/
public void updateMacVlanTreatment(DeviceId deviceId, MacAddress hostMac,
- VlanId hostVlanId, PortNumber port, int nextId) {
+ VlanId hostVlanId, PortNumber port, int nextId) {
// Check if we are the king of this device
// just one instance should perform this update
if (!defaultRoutingHandler.shouldProgram(deviceId)) {
@@ -2104,6 +2106,9 @@
DeviceId deviceId = intf.connectPoint().deviceId();
PortNumber portNum = intf.connectPoint().port();
+ // We need to do nexthop update al least one time for each
+ // interface config change. There is no difference when it is done;
+ boolean updateNexthop = false;
removeSubnetConfig(prevIntf.connectPoint(),
Sets.difference(new HashSet<>(prevIntf.ipAddressesList()),
@@ -2116,8 +2121,12 @@
// Update filtering objective and L2IG group bucket
updatePortVlanTreatment(deviceId, portNum, prevIntf.vlanNative(), false);
} else {
- // RemoveVlanNative
+ // RemoveVlanNative - affected scenarios:
+ // (T,N)->U; (T*,N)->U; (T,N)->(T,N); (T,N)->T
updateVlanConfigInternal(deviceId, portNum, prevIntf.vlanNative(), true, false);
+ // Update the nexthops of the indirect routes
+ updateNexthop = true;
+ routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
}
}
@@ -2125,16 +2134,27 @@
&& !prevIntf.vlanUntagged().equals(intf.vlanUntagged())
&& !prevIntf.vlanUntagged().equals(intf.vlanNative())) {
if (intf.vlanTagged().contains(prevIntf.vlanUntagged())) {
- // Update filtering objective and L2IG group bucket
+ // Update filtering objective and L2IG group bucket - affected scenarios:
+ // U->(T*,N); U->T*
updatePortVlanTreatment(deviceId, portNum, prevIntf.vlanUntagged(), false);
+ if (!updateNexthop) {
+ updateNexthop = true;
+ routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
+ }
} else {
- // RemoveVlanUntagged
+ // RemoveVlanUntagged - affected scenarios:
+ // U->U; U->(T,N); U->T
updateVlanConfigInternal(deviceId, portNum, prevIntf.vlanUntagged(), true, false);
+ if (!updateNexthop) {
+ updateNexthop = true;
+ routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
+ }
}
}
if (!prevIntf.vlanTagged().isEmpty() && !intf.vlanTagged().equals(prevIntf.vlanTagged())) {
- // RemoveVlanTagged
+ // RemoveVlanTagged - affected scenarios:
+ // T->U; T->T; (T,N*)->U; (T,N)->(T,N)
Sets.difference(prevIntf.vlanTagged(), intf.vlanTagged()).stream()
.filter(i -> !intf.vlanUntagged().equals(i))
.filter(i -> !intf.vlanNative().equals(i))
@@ -2149,30 +2169,45 @@
// Update filtering objective and L2IG group bucket
updatePortVlanTreatment(deviceId, portNum, intf.vlanNative(), true);
} else {
- // AddVlanNative
+ // AddVlanNative - affected scenarios
+ // U->(T,N); U->(T*,N); T->(T,N)
updateVlanConfigInternal(deviceId, portNum, intf.vlanNative(), true, true);
+ if (!updateNexthop) {
+ updateNexthop = true;
+ routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
+ }
}
}
if (!intf.vlanTagged().isEmpty() && !intf.vlanTagged().equals(prevIntf.vlanTagged())) {
- // AddVlanTagged
+ // AddVlanTagged - affected scenarios
+ // U->T; U->(T,N*); T->T; (T,N)->(T,N)
Sets.difference(intf.vlanTagged(), prevIntf.vlanTagged()).stream()
.filter(i -> !prevIntf.vlanUntagged().equals(i))
.filter(i -> !prevIntf.vlanNative().equals(i))
.forEach(vlanId -> updateVlanConfigInternal(
deviceId, portNum, vlanId, false, true)
);
+
}
if (!intf.vlanUntagged().equals(VlanId.NONE)
&& !prevIntf.vlanUntagged().equals(intf.vlanUntagged())
&& !prevIntf.vlanNative().equals(intf.vlanUntagged())) {
if (prevIntf.vlanTagged().contains(intf.vlanUntagged())) {
- // Update filtering objective and L2IG group bucket
+ // Update filtering objective and L2IG group bucket - affected scenarios
+ // (T*,N)->U; T*->U
updatePortVlanTreatment(deviceId, portNum, intf.vlanUntagged(), true);
+ if (!updateNexthop) {
+ routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
+ }
} else {
- // AddVlanUntagged
+ // AddVlanUntagged - affected scenarios
+ // U->U; (T,N)->U; T->U
updateVlanConfigInternal(deviceId, portNum, intf.vlanUntagged(), true, true);
+ if (!updateNexthop) {
+ routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
+ }
}
}
addSubnetConfig(prevIntf.connectPoint(),
@@ -2198,11 +2233,6 @@
if (getVlanNextObjectiveId(deviceId, vlanId) != -1) {
// Update L2IG bucket of the port
grpHandler.updateL2InterfaceGroupBucket(portNum, vlanId, pushVlan);
- // Update bridging and unicast routing rule for each host
- hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
- vlanId, !pushVlan, false));
- hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
- vlanId, pushVlan, true));
} else {
log.warn("Failed to retrieve next objective for vlan {} in device {}:{}", vlanId, deviceId, portNum);
}
@@ -2226,8 +2256,9 @@
if (nextId != -1 && !install) {
// Remove L2 Bridging rule and L3 Unicast rule to the host
- hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
- vlanId, pushVlan, install));
+ ScheduledFuture<?> future = hostEventExecutor.schedule(
+ () -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum, vlanId, pushVlan, install),
+ 0, TimeUnit.SECONDS);
// Remove broadcast forwarding rule and corresponding L2FG for VLAN
// only if there is no port configured on that VLAN ID
if (!getVlanPortMap(deviceId).containsKey(vlanId)) {
@@ -2239,6 +2270,12 @@
// Remove a single port from L2FG
grpHandler.updateGroupFromVlanConfiguration(vlanId, portNum, nextId, install);
}
+ // Before moving forward we have to be sure that processIntfVlanUpdatedEvent completed;
+ try {
+ future.get();
+ } catch (InterruptedException | ExecutionException e) {
+ log.warn("Exception caught when executing updateVlanConfigInternal future");
+ }
// Remove L2IG of the port
grpHandler.removePortNextObjective(deviceId, portNum, vlanId, pushVlan);
} else if (install) {
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java b/impl/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
index 9c3b713..76ca8b7 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
@@ -38,6 +38,7 @@
import org.onosproject.net.flowobjective.DefaultObjectiveContext;
import org.onosproject.net.flowobjective.FlowObjectiveService;
import org.onosproject.net.flowobjective.NextObjective;
+import org.onosproject.net.flowobjective.Objective;
import org.onosproject.net.flowobjective.ObjectiveContext;
import org.onosproject.net.link.LinkService;
import org.onosproject.segmentrouting.DefaultRoutingHandler;
@@ -60,6 +61,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
@@ -1475,12 +1477,14 @@
* @param portNum the outgoing port on the device
* @param treatment the actions applied on the packets (should include outport)
* @param meta optional data to pass to the driver
+ * @return a completable future that completes when the port has been removed
*/
- public void removeGroupFromPort(PortNumber portNum, TrafficTreatment treatment,
- TrafficSelector meta) {
+ public CompletableFuture<Objective> removeGroupFromPort(PortNumber portNum, TrafficTreatment treatment,
+ TrafficSelector meta) {
PortNextObjectiveStoreKey key = new PortNextObjectiveStoreKey(
deviceId, portNum, treatment, meta);
Integer nextId = portNextObjStore.get(key);
+ CompletableFuture<Objective> future = new CompletableFuture<>();
NextObjective.Builder nextObjBuilder = DefaultNextObjective
.builder().withId(nextId)
@@ -1490,12 +1494,14 @@
.withMeta(meta);
ObjectiveContext context = new DefaultObjectiveContext(
- (objective) ->
- log.info("removeGroupFromPort installed "
- + "NextObj {} on {}", nextId, deviceId),
+ (objective) -> {
+ log.info("removeGroupFromPort done " + "NextObj {} on {}", nextId, deviceId);
+ future.complete(objective);
+ },
(objective, error) -> {
log.warn("removeGroupFromPort failed to install NextObj {} on {}: {}", nextId, deviceId, error);
srManager.invalidateNextObj(objective.id());
+ future.complete(null);
}
);
NextObjective nextObj = nextObjBuilder.remove(context);
@@ -1504,6 +1510,7 @@
+ "for port {}", nextId, deviceId, portNum);
portNextObjStore.remove(key);
+ return future;
}
/**
@@ -1627,6 +1634,10 @@
hostMac, hostVlanId, connectPoint);
return;
}
+ } else {
+ log.warn("Tagged nexthop {}/{} is not allowed on {} without VLAN listed"
+ + " in tagged vlan", hostMac, hostVlanId, connectPoint);
+ return;
}
log.debug(" update L3Ucast : deviceMac {}, port {}, host {}/{}, nextid {}, Treatment {} Meta {}",