Fixes npe during l2 flood creation
Includes an initial implementation of fobj context handling
Change-Id: Ic6e17ba2dc8a6ac97b4b0fda91470355d2216ef3
diff --git a/apps/l2lb/src/main/java/org/onosproject/l2lb/app/L2LbManager.java b/apps/l2lb/src/main/java/org/onosproject/l2lb/app/L2LbManager.java
index e5973aa..6039099 100644
--- a/apps/l2lb/src/main/java/org/onosproject/l2lb/app/L2LbManager.java
+++ b/apps/l2lb/src/main/java/org/onosproject/l2lb/app/L2LbManager.java
@@ -47,6 +47,8 @@
import org.onosproject.net.flow.DefaultTrafficTreatment;
import org.onosproject.net.flow.TrafficSelector;
import org.onosproject.net.flow.TrafficTreatment;
+import org.onosproject.net.flow.instructions.Instruction;
+import org.onosproject.net.flow.instructions.Instructions;
import org.onosproject.net.flowobjective.DefaultNextObjective;
import org.onosproject.net.flowobjective.FlowObjectiveService;
import org.onosproject.net.flowobjective.NextObjective;
@@ -64,10 +66,12 @@
import org.onosproject.store.service.Versioned;
import org.slf4j.Logger;
+import java.util.Collection;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
+import java.util.stream.Collectors;
import static org.onlab.util.Tools.groupedThreads;
import static org.slf4j.LoggerFactory.getLogger;
@@ -507,24 +511,7 @@
public void onSuccess(Objective objective) {
NextObjective nextObj = (NextObjective) objective;
log.debug("Success {} nextobj {} for L2 load balancer {}", nextObj.op(), nextObj, l2LbId);
- // Operation done
- L2LbData oldl2LbData = new L2LbData(l2LbId);
- L2LbData newl2LbData = new L2LbData(l2LbId);
- l2LbProvExecutor.execute(() -> {
- // Other operations will not lead to a generation of an event
- switch (nextObj.op()) {
- case ADD:
- newl2LbData.setNextId(nextObj.id());
- post(new L2LbEvent(L2LbEvent.Type.INSTALLED, newl2LbData, oldl2LbData));
- break;
- case REMOVE:
- oldl2LbData.setNextId(nextObj.id());
- post(new L2LbEvent(L2LbEvent.Type.UNINSTALLED, newl2LbData, oldl2LbData));
- break;
- default:
- break;
- }
- });
+ l2LbProvExecutor.execute(() -> onSuccessHandler(nextObj, l2LbId));
}
@Override
@@ -532,25 +519,82 @@
NextObjective nextObj = (NextObjective) objective;
log.debug("Failed {} nextobj {} for L2 load balancer {} due to {}", nextObj.op(), nextObj,
l2LbId, error);
- l2LbProvExecutor.execute(() -> {
- // Init the data structure
- L2LbData l2LbData = new L2LbData(l2LbId);
- // Update the next id and send the event;
- switch (nextObj.op()) {
- case ADD:
- // If ADD is failing apps do not know the next id; let's update the store
- l2LbNextStore.remove(l2LbId);
- post(new L2LbEvent(L2LbEvent.Type.FAILED, l2LbData, l2LbData));
- break;
- case REMOVE:
- // If REMOVE is failing let's send also the info about the next id; no need to update the store
- l2LbData.setNextId(nextObj.id());
- post(new L2LbEvent(L2LbEvent.Type.FAILED, l2LbData, l2LbData));
- break;
- default:
- break;
- }
- });
+ l2LbProvExecutor.execute(() -> onErrorHandler(nextObj, l2LbId));
+ }
+ }
+
+ private void onSuccessHandler(NextObjective nextObjective, L2LbId l2LbId) {
+ // Operation done
+ L2LbData oldl2LbData = new L2LbData(l2LbId);
+ L2LbData newl2LbData = new L2LbData(l2LbId);
+ // Other operations will not lead to a generation of an event
+ switch (nextObjective.op()) {
+ case ADD:
+ newl2LbData.setNextId(nextObjective.id());
+ post(new L2LbEvent(L2LbEvent.Type.INSTALLED, newl2LbData, oldl2LbData));
+ break;
+ case REMOVE:
+ oldl2LbData.setNextId(nextObjective.id());
+ post(new L2LbEvent(L2LbEvent.Type.UNINSTALLED, newl2LbData, oldl2LbData));
+ break;
+ default:
+ break;
+ }
+ }
+
+ private void onErrorHandler(NextObjective nextObjective, L2LbId l2LbId) {
+ // There was a failure
+ L2LbData l2LbData = new L2LbData(l2LbId);
+ // send FAILED event;
+ switch (nextObjective.op()) {
+ case ADD:
+ // If ADD is failing apps do not know the next id; let's update the store
+ l2LbNextStore.remove(l2LbId);
+ l2LbResStore.remove(l2LbId);
+ l2LbStore.remove(l2LbId);
+ post(new L2LbEvent(L2LbEvent.Type.FAILED, l2LbData, l2LbData));
+ break;
+ case ADD_TO_EXISTING:
+ // If ADD_TO_EXISTING is failing let's remove the failed ports
+ Collection<PortNumber> addedPorts = nextObjective.next().stream()
+ .flatMap(t -> t.allInstructions().stream())
+ .filter(i -> i.type() == Instruction.Type.OUTPUT)
+ .map(i -> ((Instructions.OutputInstruction) i).port())
+ .collect(Collectors.toList());
+ l2LbStore.compute(l2LbId, (key, value) -> {
+ if (value != null && value.ports() != null && !value.ports().isEmpty()) {
+ value.ports().removeAll(addedPorts);
+ }
+ return value;
+ });
+ l2LbData.setNextId(nextObjective.id());
+ post(new L2LbEvent(L2LbEvent.Type.FAILED, l2LbData, l2LbData));
+ break;
+ case REMOVE_FROM_EXISTING:
+ // If REMOVE_TO_EXISTING is failing let's re-add the failed ports
+ Collection<PortNumber> removedPorts = nextObjective.next().stream()
+ .flatMap(t -> t.allInstructions().stream())
+ .filter(i -> i.type() == Instruction.Type.OUTPUT)
+ .map(i -> ((Instructions.OutputInstruction) i).port())
+ .collect(Collectors.toList());
+ l2LbStore.compute(l2LbId, (key, value) -> {
+ if (value != null && value.ports() != null) {
+ value.ports().addAll(removedPorts);
+ }
+ return value;
+ });
+ l2LbData.setNextId(nextObjective.id());
+ post(new L2LbEvent(L2LbEvent.Type.FAILED, l2LbData, l2LbData));
+ break;
+ case VERIFY:
+ case REMOVE:
+ // If ADD/REMOVE_TO_EXISTING, REMOVE and VERIFY are failing let's send
+ // also the info about the next id
+ l2LbData.setNextId(nextObjective.id());
+ post(new L2LbEvent(L2LbEvent.Type.FAILED, l2LbData, l2LbData));
+ break;
+ default:
+ break;
}
}
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 1bef276..7a51f17 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
@@ -728,6 +728,12 @@
log.debug("prepareL2UnfilteredGroup");
}
+ if (groupInfos == null || groupInfos.isEmpty()) {
+ log.warn("No buckets for Broadcast NextObj {}", nextObj);
+ fail(nextObj, ObjectiveError.GROUPMISSING);
+ return;
+ }
+
IpPrefix ipDst = readIpDstFromSelector(nextObj.meta());
if (ipDst != null) {
if (ipDst.isMulticast()) {
@@ -785,18 +791,29 @@
deviceId, Integer.toHexString(l2UnfilteredGroupId), l2UnfilterGroupKey, nextObj.id());
groupInfoBuilder.add(new GroupInfo(l2UnfilteredGroupDesc, l2UnfilteredGroupDesc));
});
-
+ // Save the current count
+ int counts = groupInfoBuilder.build().size();
// Lookup each nextId in the store and obtain the group information
nextIds.forEach(nextId -> {
- List<Deque<GroupKey>> allActiveKeys = appKryo.deserialize(flowObjectiveStore.getNextGroup(nextId).data());
- GroupKey topGroupKey = allActiveKeys.get(0).getFirst();
- GroupDescription groupDesc = groupService.getGroup(deviceId, topGroupKey);
- log.debug("Trying L2-Interface: device:{} gid:{}, gkey:{} nextid:{}",
- deviceId, Integer.toHexString(((Group) groupDesc).id().id()), topGroupKey, nextId);
- groupInfoBuilder.add(new GroupInfo(groupDesc, groupDesc));
+ NextGroup nextGroup = flowObjectiveStore.getNextGroup(nextId);
+ if (nextGroup != null) {
+ List<Deque<GroupKey>> allActiveKeys = appKryo.deserialize(nextGroup.data());
+ GroupKey topGroupKey = allActiveKeys.get(0).getFirst();
+ GroupDescription groupDesc = groupService.getGroup(deviceId, topGroupKey);
+ if (groupDesc != null) {
+ log.debug("Trying L2-Hash device:{} gid:{}, gkey:{}, nextid:{}",
+ deviceId, Integer.toHexString(((Group) groupDesc).id().id()), topGroupKey, nextId);
+ groupInfoBuilder.add(new GroupInfo(groupDesc, groupDesc));
+ } else {
+ log.error("Not found L2-Hash device:{}, gkey:{}, nextid:{}", deviceId, topGroupKey, nextId);
+ }
+ } else {
+ log.error("Not found NextGroup device:{}, nextid:{}", deviceId, nextId);
+ }
});
-
- return groupInfoBuilder.build();
+ // Compare the size before and after to detect problems during the creation
+ ImmutableList<GroupInfo> groupInfos = groupInfoBuilder.build();
+ return (counts + nextIds.size()) == groupInfos.size() ? groupInfos : ImmutableList.of();
}
private List<GroupInfo> prepareL2InterfaceGroup(NextObjective nextObj, VlanId assignedVlan) {