Several fixes for Segment Routing
1. In getSubnetNextObjectiveId, do not create a broadcast group if it does not exist
The creation is done when device is up and when config is added
2. Check if broadcast group exists before creating one
3. Put subnet in a Set instead of a List to avoid duplicated elements
Change-Id: Ifafd8e783a9d98aa62b5e299f560897458b90bb0
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index 895c446..c4a91c7 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -29,7 +29,6 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
import java.util.Set;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
@@ -451,7 +450,7 @@
// If both target switch and dest switch are edge routers, then set IP
// rule for both subnet and router IP.
if (config.isEdgeDevice(targetSw) && config.isEdgeDevice(destSw)) {
- List<Ip4Prefix> subnets = config.getSubnets(destSw);
+ Set<Ip4Prefix> subnets = config.getSubnets(destSw);
log.debug("* populateEcmpRoutingRulePartial in device {} towards {} for subnets {}",
targetSw, destSw, subnets);
result = rulePopulator.populateIpRuleForSubnet(targetSw,
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DeviceConfiguration.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DeviceConfiguration.java
index 5c10eb6..6b51ff3 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DeviceConfiguration.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DeviceConfiguration.java
@@ -15,6 +15,7 @@
*/
package org.onosproject.segmentrouting;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import org.onlab.packet.Ip4Address;
import org.onlab.packet.Ip4Prefix;
@@ -344,12 +345,12 @@
* @param deviceId device identifier
* @return list of ip prefixes or null if not found
*/
- public List<Ip4Prefix> getSubnets(DeviceId deviceId) {
+ public Set<Ip4Prefix> getSubnets(DeviceId deviceId) {
SegmentRouterInfo srinfo = deviceConfigMap.get(deviceId);
if (srinfo != null) {
log.trace("getSubnets for device{} is {}", deviceId,
srinfo.subnets.values());
- return new ArrayList<>(srinfo.subnets.values());
+ return ImmutableSet.copyOf(srinfo.subnets.values());
}
return null;
}
@@ -424,7 +425,7 @@
*/
public boolean inSameSubnet(DeviceId deviceId, Ip4Address hostIp) {
- List<Ip4Prefix> subnets = getSubnets(deviceId);
+ Set<Ip4Prefix> subnets = getSubnets(deviceId);
if (subnets == null) {
return false;
}
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 6e4de12..0d7d4c9 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -141,7 +141,7 @@
* @return true if all rules are set successfully, false otherwise
*/
public boolean populateIpRuleForSubnet(DeviceId deviceId,
- List<Ip4Prefix> subnets,
+ Set<Ip4Prefix> subnets,
DeviceId destSw,
Set<DeviceId> nextHops) {
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 a5ac678..012f134 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -375,7 +375,7 @@
return null;
}
// vlan assignment is expensive but done only once
- List<Ip4Prefix> configuredSubnets = deviceConfiguration.getSubnets(deviceId);
+ Set<Ip4Prefix> configuredSubnets = deviceConfiguration.getSubnets(deviceId);
Set<Short> assignedVlans = new HashSet<>();
Set<Ip4Prefix> unassignedSubnets = new HashSet<>();
for (Ip4Prefix sub : configuredSubnets) {
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
index 2606f48..69a0d86 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
@@ -334,8 +334,8 @@
}
/**
- * Returns the next objective associated with the neighborset.
- * If there is no next objective for this neighborset, this API
+ * Returns the next objective associated with the subnet.
+ * If there is no next objective for this subnet, this API
* would create a next objective and return.
*
* @param prefix subnet information
@@ -344,31 +344,8 @@
public int getSubnetNextObjectiveId(IpPrefix prefix) {
Integer nextId = subnetNextObjStore.
get(new SubnetNextObjectiveStoreKey(deviceId, prefix));
- if (nextId == null) {
- log.trace("getSubnetNextObjectiveId in device{}: Next objective id "
- + "not found for {} and creating", deviceId, prefix);
- log.trace("getSubnetNextObjectiveId: subnetNextObjStore contents for device {}: {}",
- deviceId,
- subnetNextObjStore.entrySet()
- .stream()
- .filter((subnetStoreEntry) ->
- (subnetStoreEntry.getKey().deviceId().equals(deviceId)))
- .collect(Collectors.toList()));
- createGroupsFromSubnetConfig();
- nextId = subnetNextObjStore.
- get(new SubnetNextObjectiveStoreKey(deviceId, prefix));
- if (nextId == null) {
- log.warn("subnetNextObjStore: unable to create next objective");
- return -1;
- } else {
- log.debug("subnetNextObjStore in device{}: Next objective id {} "
- + "created for {}", deviceId, nextId, prefix);
- }
- } else {
- log.trace("subnetNextObjStore in device{}: Next objective id {} "
- + "found for {}", deviceId, nextId, prefix);
- }
- return nextId;
+
+ return (nextId != null) ? nextId : -1;
}
/**
@@ -541,6 +518,15 @@
// Construct a broadcast group for each subnet
subnetPortMap.forEach((subnet, ports) -> {
+ SubnetNextObjectiveStoreKey key =
+ new SubnetNextObjectiveStoreKey(deviceId, subnet);
+
+ if (subnetNextObjStore.containsKey(key)) {
+ log.debug("Broadcast group for device {} and subnet {} exists",
+ deviceId, subnet);
+ return;
+ }
+
int nextId = flowObjectiveService.allocateNextId();
NextObjective.Builder nextObjBuilder = DefaultNextObjective
@@ -558,8 +544,7 @@
log.debug("createGroupFromSubnetConfig: Submited "
+ "next objective {} in device {}",
nextId, deviceId);
- SubnetNextObjectiveStoreKey key =
- new SubnetNextObjectiveStoreKey(deviceId, subnet);
+
subnetNextObjStore.put(key, nextId);
});
}