Optimized group updating during host move
In addition,
- Added a fix for host remove issue
- Added a fix for single homed (loc1) to dual homed (loc2).
Change-Id: I1a7344bd77f73d6bed8955dcdbf407b5354d0eee
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index 296fbcd..14df622 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -129,6 +129,15 @@
});
}
});
+
+ 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.port(), nextId);
+ }
}
void processHostRemovedEvent(HostEvent event) {
@@ -181,6 +190,7 @@
}
});
});
+
});
}
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
index b99f33f..cc03597 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
@@ -32,9 +32,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.util.HashMap;
import java.util.List;
-import java.util.Map;
import java.util.Collection;
import java.util.Objects;
import java.util.Optional;
@@ -234,37 +232,51 @@
log.info("processHostMovedEvent {}", event);
MacAddress hostMac = event.subject().mac();
VlanId hostVlanId = event.subject().vlan();
- // map of nextId for prev port in each device
- Map<DeviceId, Integer> nextIdMap = new HashMap<>();
+
Set<HostLocation> prevLocations = event.prevSubject().locations();
Set<HostLocation> newLocations = event.subject().locations();
- Set<ConnectPoint> connectPoints = newLocations.stream().map(l -> (ConnectPoint) l).collect(Collectors.toSet());
+ Set<ConnectPoint> connectPoints = newLocations.stream()
+ .map(l -> (ConnectPoint) l).collect(Collectors.toSet());
List<Set<IpPrefix>> batchedSubnets =
srManager.deviceConfiguration.getBatchedSubnets(event.subject().id());
+ Set<DeviceId> newDeviceIds = newLocations.stream().map(HostLocation::deviceId)
+ .collect(Collectors.toSet());
+
+ // Set of deviceIDs of the previous locations where the host was connected
+ // Used to determine if host moved to different connect points
+ // on same device or moved to a different device altogether
+ Set<DeviceId> oldDeviceIds = prevLocations.stream().map(HostLocation::deviceId)
+ .collect(Collectors.toSet());
+
+ // L3 Ucast bucket needs to be updated only once per host
+ // and only when the no. of routes with the host as next-hop is not zero
+ 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);
+ }
+ });
+ }
batchedSubnets.forEach(subnets -> {
log.debug("HostMoved. populateSubnet {}, {}", newLocations, subnets);
srManager.defaultRoutingHandler.populateSubnet(connectPoints, subnets);
subnets.forEach(prefix -> {
- Set<DeviceId> newDeviceIds = newLocations.stream().map(HostLocation::deviceId)
- .collect(Collectors.toSet());
-
- // Set of deviceIDs of the previous locations where the host was connected
- // Used to determine if host moved to different connect points
- // on same device or moved to a different device altogether
- Set<DeviceId> oldDeviceIds = prevLocations.stream().map(HostLocation::deviceId)
- .collect(Collectors.toSet());
-
// For each old location
Sets.difference(prevLocations, newLocations).forEach(prevLocation -> {
- //find next Id for each old port, add to map
- int nextId = srManager.getNextIdForHostPort(prevLocation.deviceId(), hostMac,
- hostVlanId, prevLocation.port());
- log.debug("HostMoved. NextId For Host {}, {}, {}, {} {}", prevLocation, prefix,
- hostMac, hostVlanId, nextId);
-
- nextIdMap.put(prevLocation.deviceId(), nextId);
// Remove flows for unchanged IPs only when the host moves from a switch to another.
// Otherwise, do not remove and let the adding part update the old flow
@@ -297,18 +309,11 @@
log.debug("HostMoved. populateRoute {}, {}, {}, {}", newLocation, prefix, hostMac, hostVlanId);
srManager.defaultRoutingHandler.populateRoute(newLocation.deviceId(), prefix,
hostMac, hostVlanId, newLocation.port(), false);
- } else {
- // update same flow to point to new nextObj
- VlanId vlanId = Optional.ofNullable(srManager.getInternalVlanId(newLocation)).orElse(hostVlanId);
- //use nextIdMap to send new port details to update the nextId group bucket
- log.debug("HostMoved. update L3 Ucast Group Bucket {}, {}, {} --> {}",
- newLocation, hostMac, vlanId, nextIdMap.get(newLocation.deviceId()));
- srManager.updateMacVlanTreatment(newLocation.deviceId(), hostMac, vlanId,
- newLocation.port(), nextIdMap.get(newLocation.deviceId()));
}
});
});
});
+
}
private boolean isReady() {
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index bddf2d2..e91591a 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -454,7 +454,7 @@
// if the objective is to revoke an existing rule, and for some reason
// the next-objective does not exist, then a new one should not be created
nextObjId = srManager.getMacVlanNextObjectiveId(deviceId, hostMac, hostVlanId,
- tbuilder.build(), mbuilder.build(), !revoke);
+ outPort, !revoke);
}
if (nextObjId == -1) {
// Warning log will come from getMacVlanNextObjective method
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index 4a3293c..98514b1 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -1289,18 +1289,15 @@
* @param deviceId Device ID
* @param macAddr mac of host for which Next ID is required.
* @param vlanId vlan of host for which Next ID is required.
- * @param treatment the actions to apply on the packets (should include outport)
- * @param meta metadata passed into the creation of a Next Objective if necessary
+ * @param port port with which to create the Next Obj.
* @param createIfMissing true if a next object should be created if not found
* @return next objective ID or -1 if an error occurred during retrieval or creation
*/
public int getMacVlanNextObjectiveId(DeviceId deviceId, MacAddress macAddr, VlanId vlanId,
- TrafficTreatment treatment,
- TrafficSelector meta,
- boolean createIfMissing) {
+ PortNumber port, boolean createIfMissing) {
DefaultGroupHandler ghdlr = groupHandlerMap.get(deviceId);
if (ghdlr != null) {
- return ghdlr.getMacVlanNextObjectiveId(macAddr, vlanId, treatment, meta, createIfMissing);
+ return ghdlr.getMacVlanNextObjectiveId(macAddr, vlanId, port, createIfMissing);
} else {
log.warn("getMacVlanNextObjectiveId query - groupHandler for device {}"
+ " not found", deviceId);
@@ -1309,27 +1306,6 @@
}
/**
- * Returns the next ID for the given host port from the store.
- *
- * @param deviceId Device ID
- * @param hostMac mac of host for which Next ID is required.
- * @param hostVlanId vlan of host for which Next ID is required.
- * @param port port of device for which next ID is required.
- * @return next objective ID or -1 if an error occurred during retrieval
- */
- public int getNextIdForHostPort(DeviceId deviceId, MacAddress hostMac,
- VlanId hostVlanId, PortNumber port) {
- DefaultGroupHandler ghdlr = groupHandlerMap.get(deviceId);
- if (ghdlr != null) {
- return ghdlr.getNextIdForHostPort(hostMac, hostVlanId, port);
- } else {
- log.warn("getNextIdForHostPort query - groupHandler for device {}"
- + " not found", deviceId);
- return -1;
- }
- }
-
- /**
* Updates the next objective for the given nextId .
*
* @param deviceId Device ID
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
index 716178d..58599ba 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
@@ -822,14 +822,13 @@
*
* @param macAddr the mac addr for the simple next objective
* @param vlanId the vlan for the simple next objective
- * @param treatment the actions to apply on the packets (should include outport)
- * @param meta optional metadata passed into the creation of the next objective
+ * @param port port with which to create the Next Obj.
* @param createIfMissing true if a next object should be created if not found
* @return int if found or created, -1 if there are errors during the
* creation of the next objective.
*/
- public int getMacVlanNextObjectiveId(MacAddress macAddr, VlanId vlanId, TrafficTreatment treatment,
- TrafficSelector meta, boolean createIfMissing) {
+ public int getMacVlanNextObjectiveId(MacAddress macAddr, VlanId vlanId, PortNumber port,
+ boolean createIfMissing) {
Integer nextId = macVlanNextObjStore
.get(new MacVlanNextObjectiveStoreKey(deviceId, macAddr, vlanId));
@@ -846,8 +845,55 @@
return -1;
}
+ MacAddress deviceMac;
+ try {
+ deviceMac = deviceConfig.getDeviceMac(deviceId);
+ } catch (DeviceConfigNotFoundException e) {
+ log.warn(e.getMessage() + " in getMacVlanNextObjectiveId");
+ return -1;
+ }
+
+ // since we are creating now, port cannot be null
+ if (port == null) {
+ log.debug("getMacVlanNextObjectiveId : port information cannot be null "
+ + "for device {}, host {}/{}", deviceId, macAddr, vlanId);
+ return -1;
+ }
+
+ TrafficSelector.Builder meta = DefaultTrafficSelector.builder();
+
+ TrafficTreatment.Builder treatment = DefaultTrafficTreatment.builder();
+ treatment.deferred()
+ .setEthDst(macAddr)
+ .setEthSrc(deviceMac)
+ .setOutput(port);
+
+ ConnectPoint connectPoint = new ConnectPoint(deviceId, port);
+ VlanId untaggedVlan = srManager.interfaceService.getUntaggedVlanId(connectPoint);
+ Set<VlanId> taggedVlans = srManager.interfaceService.getTaggedVlanId(connectPoint);
+ VlanId nativeVlan = srManager.interfaceService.getNativeVlanId(connectPoint);
+
+ // Adjust the meta according to VLAN configuration
+ if (taggedVlans.contains(vlanId)) {
+ treatment.setVlanId(vlanId);
+ } else if (vlanId.equals(VlanId.NONE)) {
+ if (untaggedVlan != null) {
+ meta.matchVlanId(untaggedVlan);
+ } else if (nativeVlan != null) {
+ meta.matchVlanId(nativeVlan);
+ } else {
+ log.warn("Untagged nexthop {}/{} is not allowed on {} without untagged or native vlan",
+ macAddr, vlanId, connectPoint);
+ return -1;
+ }
+ } else {
+ log.warn("Tagged nexthop {}/{} is not allowed on {} without VLAN listed"
+ + " in tagged vlan", macAddr, vlanId, connectPoint);
+ return -1;
+ }
+
/* create missing next objective */
- nextId = createGroupFromMacVlan(macAddr, vlanId, treatment, meta);
+ nextId = createGroupFromMacVlan(macAddr, vlanId, treatment.build(), meta.build());
if (nextId == null) {
log.warn("getMacVlanNextObjectiveId: unable to create next obj"
+ "for dev:{} host:{}/{}", deviceId, macAddr, vlanId);
@@ -1480,43 +1526,6 @@
}
/**
- * Returns the next ID for the given host port from the store.
- *
- * @param hostMac mac of host for which Next ID is required.
- * @param hostVlanId vlan of host for which Next ID is required.
- * @param port port of device for which next ID is required.
- * @return next objective ID or -1 if an error occurred during retrieval
- */
- public int getNextIdForHostPort(MacAddress hostMac, VlanId hostVlanId, PortNumber port) {
-
- MacAddress deviceMac;
- try {
- deviceMac = deviceConfig.getDeviceMac(deviceId);
- } catch (DeviceConfigNotFoundException e) {
- log.warn(e.getMessage() + " in getNextIdForHostPort");
- return -1;
- }
-
- TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
- tbuilder.deferred()
- .setEthDst(hostMac)
- .setEthSrc(deviceMac)
- .setVlanId(hostVlanId)
- .setOutput(port);
-
- TrafficSelector metadata =
- DefaultTrafficSelector.builder().matchVlanId(hostVlanId).build();
-
- int nextId = getMacVlanNextObjectiveId(hostMac, hostVlanId,
- tbuilder.build(), metadata, true);
-
- log.debug("getNextId : hostMac {}, deviceMac {}, port {}, hostVlanId {} Treatment {} Meta {} nextId {} ",
- hostMac, deviceMac, port, hostVlanId, tbuilder.build(), metadata, nextId);
-
- return nextId;
- }
-
- /**
* Updates the next objective for the given nextId .
*
* @param hostMac mac of host for which Next obj is to be updated.