[AETHER-514] Support MODIFY on SIMPLE next
Add additional debug prints and removes the generation
of the hashed and next_vlan flows
Change-Id: Iecee9e86dd8443ae0b67c5fad193311f929503a2
diff --git a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/FabricPipeliner.java b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/FabricPipeliner.java
index eac2116..79045f1 100644
--- a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/FabricPipeliner.java
+++ b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/FabricPipeliner.java
@@ -50,6 +50,7 @@
import java.util.stream.Collectors;
import static java.lang.String.format;
+import static org.onosproject.net.flowobjective.NextObjective.Type.SIMPLE;
import static org.onosproject.pipelines.fabric.impl.behaviour.FabricUtils.outputPort;
import static org.slf4j.LoggerFactory.getLogger;
@@ -128,9 +129,12 @@
return;
}
- if (obj.op() == Objective.Operation.MODIFY) {
- // TODO: support MODIFY operation
- log.warn("MODIFY operation not yet supported for NextObjective, will return failure :(");
+ if (obj.op() == Objective.Operation.MODIFY && obj.type() != SIMPLE) {
+ log.warn("MODIFY operation not yet supported for NextObjective {}, will return failure :(",
+ obj.type());
+ if (log.isTraceEnabled()) {
+ log.trace("Objective {}", obj);
+ }
fail(obj, ObjectiveError.UNSUPPORTED);
return;
}
@@ -182,10 +186,16 @@
if (flowRules.isEmpty()) {
return;
}
+
+ if (log.isTraceEnabled()) {
+ log.trace("Objective {} -> Flows {}", objective, flowRules);
+ }
+
final FlowRuleOperations.Builder ops = FlowRuleOperations.builder();
switch (objective.op()) {
case ADD:
case ADD_TO_EXISTING:
+ case MODIFY:
flowRules.forEach(ops::add);
break;
case REMOVE:
@@ -193,7 +203,7 @@
flowRules.forEach(ops::remove);
break;
default:
- log.warn("Unsupported Objective operation '{}'", objective.op());
+ log.warn("Unsupported Objective operation {}", objective.op());
return;
}
flowRuleService.apply(ops.build());
@@ -203,6 +213,11 @@
if (groups.isEmpty()) {
return;
}
+
+ if (log.isTraceEnabled()) {
+ log.trace("Objective {} -> Groups {}", objective, groups);
+ }
+
switch (objective.op()) {
case ADD:
groups.forEach(groupService::addGroup);
@@ -223,6 +238,14 @@
group.appCookie(), group.appId())
);
break;
+ case MODIFY:
+ // Modify is only supported for simple next objective
+ // Replace group bucket directly
+ groups.forEach(group -> groupService.setBucketsForGroup(
+ deviceId, group.appCookie(), group.buckets(),
+ group.appCookie(), group.appId())
+ );
+ break;
default:
log.warn("Unsupported Objective operation {}", objective.op());
}
diff --git a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/NextObjectiveTranslator.java b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/NextObjectiveTranslator.java
index cb79e9b..1e0e52d 100644
--- a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/NextObjectiveTranslator.java
+++ b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/NextObjectiveTranslator.java
@@ -199,7 +199,7 @@
obj.nextTreatments(), true);
if (forceSimple && treatments.size() > 1) {
- log.warn("Forcing SIMPLE behavior for NextObjective with {} treatments []",
+ log.warn("Forcing SIMPLE behavior for NextObjective with {} treatments {}",
treatments.size(), obj);
}
@@ -494,10 +494,12 @@
}
private boolean isGroupModifyOp(NextObjective obj) {
- // If operation is ADD_TO_EXIST or REMOVE_FROM_EXIST, it means we modify
+ // If operation is ADD_TO_EXIST, REMOVE_FROM_EXIST or MODIFY, it means we modify
// group buckets only, no changes for flow rules.
+ // FIXME Please note that for MODIFY op this could not apply in future if we extend the scope of MODIFY
return obj.op() == Objective.Operation.ADD_TO_EXISTING ||
- obj.op() == Objective.Operation.REMOVE_FROM_EXISTING;
+ obj.op() == Objective.Operation.REMOVE_FROM_EXISTING ||
+ obj.op() == Objective.Operation.MODIFY;
}
private boolean isXconnect(NextObjective obj) {