CORD-1583 More bug fixes in dual-ToR scenarios
- reentrant lock was not being used correctly
- fixHashGroup in group handler was not updating global store correctly
- linkUp was not being noted in seenLinks if configuration came after switches connected
- serialization error in global objective store due to missing kryo for Sets
- damaged routepath computation was not taking pair-devs into account
- switch failures were leading to improper ecmpSpg graph updates, and missed hash-group changes
- implemented more next-objective verification as group sub-system can go out-of-sync with objective-store
Change-Id: If3cfdd715e9b69820894b49def31f75ceb748863
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java
index 4f16cff..24ababc 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java
@@ -42,6 +42,7 @@
import org.onosproject.net.flow.criteria.VlanIdCriterion;
import org.onosproject.net.flow.instructions.Instruction;
import org.onosproject.net.flow.instructions.Instructions;
+import org.onosproject.net.flow.instructions.Instructions.GroupInstruction;
import org.onosproject.net.flow.instructions.L2ModificationInstruction;
import org.onosproject.net.flowobjective.DefaultNextObjective;
import org.onosproject.net.flowobjective.FlowObjectiveStore;
@@ -1497,7 +1498,6 @@
List<Deque<GroupKey>> allActiveKeys = appKryo.deserialize(next.data());
List<TrafficTreatment> bucketsToCreate = Lists.newArrayList();
List<Integer> indicesToRemove = Lists.newArrayList();
- // XXX verify empty group
for (TrafficTreatment bkt : nextObjective.next()) {
PortNumber portNumber = readOutPortFromTreatment(bkt);
int label = readLabelFromTreatment(bkt);
@@ -1544,6 +1544,100 @@
removeBucket(chainsToRemove, nextObjective);
}
+ if (bucketsToCreate.isEmpty() && indicesToRemove.isEmpty()) {
+ // flowObjective store record is in-sync with nextObjective passed-in
+ // Nevertheless groupStore may not be in sync due to bug in the store
+ // - see CORD-1844. XXX When this bug is fixed, the rest of this verify
+ // method will not be required.
+ GroupKey hashGroupKey = allActiveKeys.get(0).peekFirst();
+ Group hashGroup = groupService.getGroup(deviceId, hashGroupKey);
+ int actualGroupSize = hashGroup.buckets().buckets().size();
+ int objGroupSize = nextObjective.next().size();
+ if (actualGroupSize != objGroupSize) {
+ log.warn("Mismatch detected in device:{}, nextId:{}, nextObjective-size"
+ + ":{} group-size:{} .. correcting", deviceId, nextObjective.id(),
+ objGroupSize, actualGroupSize);
+ }
+ if (actualGroupSize > objGroupSize) {
+ List<GroupBucket> bucketsToRemove = Lists.newArrayList();
+ //check every bucket in the actual group
+ for (GroupBucket bucket : hashGroup.buckets().buckets()) {
+ GroupInstruction g = (GroupInstruction) bucket.treatment()
+ .allInstructions().iterator().next();
+ GroupId gidToCheck = g.groupId(); // the group pointed to
+ boolean matches = false;
+ for (Deque<GroupKey> validChain : allActiveKeys) {
+ if (validChain.size() < 2) {
+ continue;
+ }
+ GroupKey pointedGroupKey = validChain.stream()
+ .collect(Collectors.toList()).get(1);
+ Group pointedGroup = groupService.getGroup(deviceId, pointedGroupKey);
+ if (pointedGroup != null && gidToCheck.equals(pointedGroup.id())) {
+ matches = true;
+ break;
+ }
+ }
+ if (!matches) {
+ log.warn("Removing bucket pointing to groupId:{}", gidToCheck);
+ bucketsToRemove.add(bucket);
+ }
+ }
+ // remove buckets for which there was no record in the obj store
+ if (bucketsToRemove.isEmpty()) {
+ log.warn("Mismatch detected but could not determine which"
+ + "buckets to remove");
+ } else {
+ GroupBuckets removeBuckets = new GroupBuckets(bucketsToRemove);
+ groupService.removeBucketsFromGroup(deviceId, hashGroupKey,
+ removeBuckets, hashGroupKey,
+ nextObjective.appId());
+ }
+ } else if (actualGroupSize < objGroupSize) {
+ // should also add buckets not in group-store but in obj-store
+ List<GroupBucket> bucketsToAdd = Lists.newArrayList();
+ //check every bucket in the obj
+ for (Deque<GroupKey> validChain : allActiveKeys) {
+ if (validChain.size() < 2) {
+ continue;
+ }
+ GroupKey pointedGroupKey = validChain.stream()
+ .collect(Collectors.toList()).get(1);
+ Group pointedGroup = groupService.getGroup(deviceId, pointedGroupKey);
+ if (pointedGroup == null) {
+ // group should exist, otherwise cannot be added as bucket
+ continue;
+ }
+ boolean matches = false;
+ for (GroupBucket bucket : hashGroup.buckets().buckets()) {
+ GroupInstruction g = (GroupInstruction) bucket.treatment()
+ .allInstructions().iterator().next();
+ GroupId gidToCheck = g.groupId(); // the group pointed to
+ if (pointedGroup.id().equals(gidToCheck)) {
+ matches = true;
+ break;
+ }
+ }
+ if (!matches) {
+ log.warn("Adding bucket pointing to groupId:{}", pointedGroup);
+ TrafficTreatment t = DefaultTrafficTreatment.builder()
+ .group(pointedGroup.id())
+ .build();
+ bucketsToAdd.add(DefaultGroupBucket.createSelectGroupBucket(t));
+ }
+ }
+ if (bucketsToAdd.isEmpty()) {
+ log.warn("Mismatch detected but could not determine which "
+ + "buckets to add");
+ } else {
+ GroupBuckets addBuckets = new GroupBuckets(bucketsToAdd);
+ groupService.addBucketsToGroup(deviceId, hashGroupKey,
+ addBuckets, hashGroupKey,
+ nextObjective.appId());
+ }
+ }
+ }
+
pass(nextObjective);
}
@@ -1733,7 +1827,6 @@
private class InnerGroupListener implements GroupListener {
@Override
public void event(GroupEvent event) {
- log.trace("received group event of type {}", event.type());
switch (event.type()) {
case GROUP_ADDED:
processPendingAddGroupsOrNextObjs(event.subject().appCookie(), true);