In this commit:
Removing dependence on hashing for unique groupkeys in ofdpa driver.
Group-store no longer removes groups from store if a group-operation fails due to GROUP_EXISTS.
Group-store also checks for unique group-id when given by app.
Group-provider now logs warning before making call to core.
Change-Id: I4a1dcb887cb74cd6e245df0c82c90a50d8f3898a
diff --git a/core/store/dist/src/main/java/org/onosproject/store/group/impl/DistributedGroupStore.java b/core/store/dist/src/main/java/org/onosproject/store/group/impl/DistributedGroupStore.java
index 250dfb9..520c65d 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/group/impl/DistributedGroupStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/group/impl/DistributedGroupStore.java
@@ -537,6 +537,19 @@
// Get a new group identifier
id = new DefaultGroupId(getFreeGroupIdValue(groupDesc.deviceId()));
} else {
+ // we need to use the identifier passed in by caller, but check if
+ // already used
+ Group existing = getGroup(groupDesc.deviceId(),
+ new DefaultGroupId(groupDesc.givenGroupId()));
+ if (existing != null) {
+ log.warn("Group already exists with the same id: 0x{} in dev:{} "
+ + "but with different key: {} (request gkey: {})",
+ Integer.toHexString(groupDesc.givenGroupId()),
+ groupDesc.deviceId(),
+ existing.appCookie(),
+ groupDesc.appCookie());
+ return;
+ }
id = new DefaultGroupId(groupDesc.givenGroupId());
}
// Create a group entry object
@@ -621,7 +634,8 @@
// Check if a group is existing with the provided key
Group oldGroup = getGroup(deviceId, oldAppCookie);
if (oldGroup == null) {
- log.warn("updateGroupDescriptionInternal: Group not found...strange");
+ log.warn("updateGroupDescriptionInternal: Group not found...strange. "
+ + "GroupKey:{} DeviceId:{}", oldAppCookie, deviceId);
return;
}
@@ -940,6 +954,19 @@
log.warn("Current extraneous groups in device:{} are: {}",
deviceId,
getExtraneousGroups(deviceId));
+ if (operation.buckets().equals(existing.buckets())) {
+ if (existing.state() == GroupState.PENDING_ADD) {
+ log.info("GROUP_EXISTS: GroupID and Buckets match for group in pending "
+ + "add state - moving to ADDED for group {} in device {}",
+ existing.id(), deviceId);
+ addOrUpdateGroupEntry(existing);
+ return;
+ } else {
+ log.warn("GROUP EXISTS: Group ID matched but buckets did not. "
+ + "Operation: {} Existing: {}", operation.buckets(),
+ existing.buckets());
+ }
+ }
}
switch (operation.opType()) {
case ADD:
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/OFDPA2GroupHandler.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/OFDPA2GroupHandler.java
index 7a42d93..3674aa1 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/OFDPA2GroupHandler.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/OFDPA2GroupHandler.java
@@ -36,6 +36,8 @@
import org.onosproject.net.group.GroupKey;
import org.onosproject.net.group.GroupListener;
import org.onosproject.net.group.GroupService;
+import org.onosproject.store.service.AtomicCounter;
+import org.onosproject.store.service.StorageService;
import org.slf4j.Logger;
import java.util.ArrayDeque;
@@ -87,6 +89,7 @@
private final Logger log = getLogger(getClass());
private ServiceDirectory serviceDirectory;
protected GroupService groupService;
+ protected StorageService storageService;
private DeviceId deviceId;
private FlowObjectiveStore flowObjectiveStore;
@@ -96,7 +99,7 @@
Executors.newScheduledThreadPool(2, groupedThreads("onos/pipeliner", "ofdpa2-%d"));
// index number for group creation
- private AtomicInteger l3VpnIndex = new AtomicInteger(0);
+ private AtomicCounter nextIndex;
// local stores for port-vlan mapping
protected Map<PortNumber, VlanId> port2Vlan = new ConcurrentHashMap<>();
@@ -111,6 +114,11 @@
this.flowObjectiveStore = context.store();
this.serviceDirectory = context.directory();
this.groupService = serviceDirectory.get(GroupService.class);
+ this.storageService = serviceDirectory.get(StorageService.class);
+ this.nextIndex = storageService.atomicCounterBuilder()
+ .withName("group-id-index-counter")
+ .build()
+ .asAtomicCounter();
pendingNextObjectives = CacheBuilder.newBuilder()
.expireAfterWrite(20, TimeUnit.SECONDS)
@@ -129,6 +137,10 @@
groupService.addListener(new InnerGroupListener());
}
+ //////////////////////////////////////
+ // Group Creation
+ //////////////////////////////////////
+
protected void addGroup(NextObjective nextObjective) {
switch (nextObjective.type()) {
case SIMPLE:
@@ -190,8 +202,8 @@
// break up simple next objective to GroupChain objects
GroupInfo groupInfo = createL2L3Chain(treatment, nextObj.id(),
- nextObj.appId(), false,
- nextObj.meta());
+ nextObj.appId(), false,
+ nextObj.meta());
if (groupInfo == null) {
log.error("Could not process nextObj={} in dev:{}", nextObj.id(), deviceId);
return;
@@ -213,26 +225,6 @@
groupService.addGroup(groupInfo.innerGrpDesc);
}
- private void updatePendingNextObjective(GroupKey key, OfdpaNextGroup value) {
- List<OfdpaNextGroup> nextList = new CopyOnWriteArrayList<OfdpaNextGroup>();
- nextList.add(value);
- List<OfdpaNextGroup> ret = pendingNextObjectives.asMap()
- .putIfAbsent(key, nextList);
- if (ret != null) {
- ret.add(value);
- }
- }
-
- private void updatePendingGroups(GroupKey gkey, GroupChainElem gce) {
- Set<GroupChainElem> gceSet = Collections.newSetFromMap(
- new ConcurrentHashMap<GroupChainElem, Boolean>());
- gceSet.add(gce);
- Set<GroupChainElem> retval = pendingGroups.putIfAbsent(gkey, gceSet);
- if (retval != null) {
- retval.add(gce);
- }
- }
-
/**
* Creates a simple L2 Interface Group.
*
@@ -324,8 +316,8 @@
* error in processing the chain
*/
private GroupInfo createL2L3Chain(TrafficTreatment treatment, int nextId,
- ApplicationId appId, boolean mpls,
- TrafficSelector meta) {
+ ApplicationId appId, boolean mpls,
+ TrafficSelector meta) {
// for the l2interface group, get vlan and port info
// for the outer group, get the src/dst mac, and vlan info
TrafficTreatment.Builder outerTtb = DefaultTrafficTreatment.builder();
@@ -403,7 +395,7 @@
// assemble information for ofdpa l2interface group
int l2groupId = L2_INTERFACE_TYPE | (vlanid.toShort() << 16) | (int) portNum;
- // a globally unique groupkey that is different for ports in the same devices
+ // a globally unique groupkey that is different for ports in the same device,
// but different for the same portnumber on different devices. Also different
// for the various group-types created out of the same next objective.
int l2gk = l2InterfaceGroupKey(deviceId, vlanid, portNum);
@@ -413,10 +405,10 @@
GroupDescription outerGrpDesc = null;
if (mpls) {
// outer group is MPLSInteface
- int mplsgroupId = MPLS_INTERFACE_TYPE | (int) portNum;
- // using mplsinterfacemask in groupkey to differentiate from l2interface
- int mplsgk = MPLS_INTERFACE_TYPE | (SUBTYPE_MASK & (deviceId.hashCode() << 8 | (int) portNum));
- final GroupKey mplsgroupkey = new DefaultGroupKey(OFDPA2Pipeline.appKryo.serialize(mplsgk));
+ int mplsInterfaceIndex = getNextAvailableIndex();
+ int mplsgroupId = MPLS_INTERFACE_TYPE | (SUBTYPE_MASK & mplsInterfaceIndex);
+ final GroupKey mplsgroupkey = new DefaultGroupKey(
+ OFDPA2Pipeline.appKryo.serialize(mplsInterfaceIndex));
outerTtb.group(new DefaultGroupId(l2groupId));
// create the mpls-interface group description to wait for the
// l2 interface group to be processed
@@ -435,11 +427,10 @@
mplsgroupkey, nextId);
} else {
// outer group is L3Unicast
- int l3GroupIdHash = Objects.hash(srcMac, dstMac, portNum);
- int l3groupId = L3_UNICAST_TYPE | (TYPE_MASK & l3GroupIdHash);
- int l3GroupKeyHash = Objects.hash(deviceId, srcMac, dstMac, portNum);
- int l3gk = L3_UNICAST_TYPE | (TYPE_MASK & l3GroupKeyHash);
- final GroupKey l3groupkey = new DefaultGroupKey(OFDPA2Pipeline.appKryo.serialize(l3gk));
+ int l3unicastIndex = getNextAvailableIndex();
+ int l3groupId = L3_UNICAST_TYPE | (TYPE_MASK & l3unicastIndex);
+ final GroupKey l3groupkey = new DefaultGroupKey(
+ OFDPA2Pipeline.appKryo.serialize(l3unicastIndex));
outerTtb.group(new DefaultGroupId(l2groupId));
// create the l3unicast group description to wait for the
// l2 interface group to be processed
@@ -558,7 +549,7 @@
// assemble info for l2 flood group
// since there can be only one flood group for a vlan, its index is always the same - 0
Integer l2floodgroupId = L2_FLOOD_TYPE | (vlanId.toShort() << 16);
- int l2floodgk = L2_FLOOD_TYPE | nextObj.id() << 12;
+ int l2floodgk = getNextAvailableIndex();
final GroupKey l2floodgroupkey = new DefaultGroupKey(OFDPA2Pipeline.appKryo.serialize(l2floodgk));
// collection of group buckets pointing to all the l2 interface groups
List<GroupBucket> l2floodBuckets = new ArrayList<>();
@@ -633,8 +624,10 @@
.createSelectGroupBucket(ttb.build());
l3ecmpGroupBuckets.add(sbucket);
}
- int l3ecmpGroupId = L3_ECMP_TYPE | nextObj.id() << 12;
- GroupKey l3ecmpGroupKey = new DefaultGroupKey(OFDPA2Pipeline.appKryo.serialize(l3ecmpGroupId));
+ int l3ecmpIndex = getNextAvailableIndex();
+ int l3ecmpGroupId = L3_ECMP_TYPE | (TYPE_MASK & l3ecmpIndex);
+ GroupKey l3ecmpGroupKey = new DefaultGroupKey(
+ OFDPA2Pipeline.appKryo.serialize(l3ecmpIndex));
GroupDescription l3ecmpGroupDesc =
new DefaultGroupDescription(
deviceId,
@@ -682,8 +675,8 @@
* @param unsentGroups a list to store GroupInfo for each bucket-group-chain
*/
private void createHashBucketChains(NextObjective nextObj,
- List<Deque<GroupKey>> allGroupKeys,
- List<GroupInfo> unsentGroups) {
+ List<Deque<GroupKey>> allGroupKeys,
+ List<GroupInfo> unsentGroups) {
// break up hashed next objective to multiple groups
Collection<TrafficTreatment> buckets = nextObj.next();
@@ -742,9 +735,10 @@
onelabelGroupInfo.outerGrpDesc.givenGroupId()));
GroupBucket l3vpnGrpBkt =
DefaultGroupBucket.createIndirectGroupBucket(l3vpnTtb.build());
- int l3vpngroupId = MPLS_L3VPN_SUBTYPE | l3VpnIndex.incrementAndGet();
- int l3vpngk = MPLS_L3VPN_SUBTYPE | nextObj.id() << 12 | l3VpnIndex.get();
- GroupKey l3vpngroupkey = new DefaultGroupKey(OFDPA2Pipeline.appKryo.serialize(l3vpngk));
+ int l3vpnIndex = getNextAvailableIndex();
+ int l3vpngroupId = MPLS_L3VPN_SUBTYPE | (SUBTYPE_MASK & l3vpnIndex);
+ GroupKey l3vpngroupkey = new DefaultGroupKey(
+ OFDPA2Pipeline.appKryo.serialize(l3vpnIndex));
GroupDescription l3vpnGroupDesc =
new DefaultGroupDescription(
deviceId,
@@ -783,6 +777,10 @@
}
}
+ //////////////////////////////////////
+ // Group Editing
+ //////////////////////////////////////
+
/**
* Adds a bucket to the top level group of a group-chain, and creates the chain.
*
@@ -954,6 +952,30 @@
flowObjectiveStore.removeNextGroup(nextObjective.id());
}
+ //////////////////////////////////////
+ // Helper Methods and Classes
+ //////////////////////////////////////
+
+ private void updatePendingNextObjective(GroupKey key, OfdpaNextGroup value) {
+ List<OfdpaNextGroup> nextList = new CopyOnWriteArrayList<OfdpaNextGroup>();
+ nextList.add(value);
+ List<OfdpaNextGroup> ret = pendingNextObjectives.asMap()
+ .putIfAbsent(key, nextList);
+ if (ret != null) {
+ ret.add(value);
+ }
+ }
+
+ private void updatePendingGroups(GroupKey gkey, GroupChainElem gce) {
+ Set<GroupChainElem> gceSet = Collections.newSetFromMap(
+ new ConcurrentHashMap<GroupChainElem, Boolean>());
+ gceSet.add(gce);
+ Set<GroupChainElem> retval = pendingGroups.putIfAbsent(gkey, gceSet);
+ if (retval != null) {
+ retval.add(gce);
+ }
+ }
+
/**
* Processes next element of a group chain. Assumption is that if this
* group points to another group, the latter has already been created
@@ -1002,6 +1024,17 @@
}
}
+ private class InnerGroupListener implements GroupListener {
+ @Override
+ public void event(GroupEvent event) {
+ log.trace("received group event of type {}", event.type());
+ if (event.type() == GroupEvent.Type.GROUP_ADDED) {
+ GroupKey key = event.subject().appCookie();
+ processPendingGroupsOrNextObjectives(key, true);
+ }
+ }
+ }
+
private void processPendingGroupsOrNextObjectives(GroupKey key, boolean added) {
//first check for group chain
Set<GroupChainElem> gceSet = pendingGroups.remove(key);
@@ -1046,6 +1079,10 @@
? null : ((VlanIdCriterion) criterion).vlanId();
}
+ private int getNextAvailableIndex() {
+ return (int) nextIndex.incrementAndGet();
+ }
+
/**
* Returns a hash as the L2 Interface Group Key.
*
@@ -1065,17 +1102,6 @@
return L2_INTERFACE_TYPE | (TYPE_MASK & hash << 6) | portLowerBits;
}
- private class InnerGroupListener implements GroupListener {
- @Override
- public void event(GroupEvent event) {
- log.trace("received group event of type {}", event.type());
- if (event.type() == GroupEvent.Type.GROUP_ADDED) {
- GroupKey key = event.subject().appCookie();
- processPendingGroupsOrNextObjectives(key, true);
- }
- }
- }
-
/**
* Utility class for moving group information around.
*/
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/OFDPA2Pipeline.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/OFDPA2Pipeline.java
index 72fe8aa..4ccf671 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/OFDPA2Pipeline.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/OFDPA2Pipeline.java
@@ -1030,26 +1030,30 @@
obj.context().ifPresent(context -> context.onError(obj, error));
}
-
@Override
public List<String> getNextMappings(NextGroup nextGroup) {
List<String> mappings = new ArrayList<>();
List<Deque<GroupKey>> gkeys = appKryo.deserialize(nextGroup.data());
for (Deque<GroupKey> gkd : gkeys) {
Group lastGroup = null;
- String gchain = "";
+ StringBuffer gchain = new StringBuffer();
for (GroupKey gk : gkd) {
Group g = groupService.getGroup(deviceId, gk);
- gchain += " 0x" + Integer.toHexString(g.id().id()) + " -->";
+ if (g == null) {
+ gchain.append(" ERROR").append(" -->");
+ continue;
+ }
+ gchain.append(" 0x").append(Integer.toHexString(g.id().id()))
+ .append(" -->");
lastGroup = g;
}
// add port information for last group in group-chain
for (Instruction i: lastGroup.buckets().buckets().get(0).treatment().allInstructions()) {
if (i instanceof OutputInstruction) {
- gchain += " port:" + ((OutputInstruction) i).port();
+ gchain.append(" port:").append(((OutputInstruction) i).port());
}
}
- mappings.add(gchain);
+ mappings.add(gchain.toString());
}
return mappings;
}
diff --git a/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java b/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
index 162c89d..1835cf6 100644
--- a/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
+++ b/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
@@ -337,11 +337,11 @@
GroupMsgErrorCode.values()[(code.ordinal())];
GroupOperation failedOperation = GroupOperation
.createFailedGroupOperation(operation, failureCode);
+ log.warn("Received a group mod error {}", msg);
providerService.groupOperationFailed(deviceId,
failedOperation);
pendingGroupOperations.remove(pendingGroupId);
pendingXidMaps.remove(pendingGroupId);
- log.warn("Received a group mod error {}", msg);
} else {
log.error("Cannot find pending group operation with group ID: {}",
pendingGroupId);