[CORD-197] Properly handles a flow with empty instruction
Change-Id: Ia465fdc8df1dca7a46249cd4cd8d41faf8461c3a
diff --git a/core/api/src/main/java/org/onosproject/net/flow/DefaultTrafficTreatment.java b/core/api/src/main/java/org/onosproject/net/flow/DefaultTrafficTreatment.java
index bf650c0..26ab1bd 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/DefaultTrafficTreatment.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/DefaultTrafficTreatment.java
@@ -234,6 +234,7 @@
switch (instruction.type()) {
case DROP:
+ case NOACTION:
case OUTPUT:
case GROUP:
case L0MODIFICATION:
@@ -259,9 +260,23 @@
return this;
}
+ /**
+ * Add a NOACTION when DROP instruction is explicitly specified.
+ *
+ * @return the traffic treatment builder
+ */
@Override
public Builder drop() {
- return add(Instructions.createDrop());
+ return add(Instructions.createNoAction());
+ }
+
+ /**
+ * Add a NOACTION when no instruction is specified.
+ *
+ * @return the traffic treatment builder
+ */
+ private Builder noAction() {
+ return add(Instructions.createNoAction());
}
@Override
@@ -459,14 +474,10 @@
@Override
public TrafficTreatment build() {
- //Don't add DROP instruction by default when instruction
- //set is empty. This will be handled in DefaultSingleTablePipeline
- //driver.
-
- //if (deferred.size() == 0 && immediate.size() == 0
- // && table == null && !clear) {
- // drop();
- //}
+ if (deferred.size() == 0 && immediate.size() == 0
+ && table == null && !clear) {
+ noAction();
+ }
return new DefaultTrafficTreatment(deferred, immediate, table, clear, meta, meter);
}
diff --git a/core/api/src/main/java/org/onosproject/net/flow/instructions/Instruction.java b/core/api/src/main/java/org/onosproject/net/flow/instructions/Instruction.java
index 6f2cac6..d01ea29 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/instructions/Instruction.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/instructions/Instruction.java
@@ -27,9 +27,18 @@
/**
* Signifies that the traffic should be dropped.
*/
+ @Deprecated
DROP,
/**
+ * Signifies that the traffic requires no action.
+ *
+ * In OF10, the behavior of NOACTION is DROP.
+ * In OF13, the behavior depends on current Action Set.
+ */
+ NOACTION,
+
+ /**
* Signifies that the traffic should be output to a port.
*/
OUTPUT,
diff --git a/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java b/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java
index ee2a74f..c9f1068 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java
@@ -68,11 +68,21 @@
*
* @return drop instruction
*/
+ @Deprecated
public static DropInstruction createDrop() {
return new DropInstruction();
}
/**
+ * Creates a no action instruction.
+ *
+ * @return no action instruction
+ */
+ public static NoActionInstruction createNoAction() {
+ return new NoActionInstruction();
+ }
+
+ /**
* Creates a group instruction.
*
* @param groupId Group Id
@@ -450,6 +460,7 @@
/**
* Drop instruction.
*/
+ @Deprecated
public static final class DropInstruction implements Instruction {
private DropInstruction() {}
@@ -482,6 +493,40 @@
}
/**
+ * No Action instruction.
+ */
+ public static final class NoActionInstruction implements Instruction {
+
+ private NoActionInstruction() {}
+
+ @Override
+ public Type type() {
+ return Type.NOACTION;
+ }
+
+ @Override
+ public String toString() {
+ return toStringHelper(type().toString()).toString();
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(type().ordinal());
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj instanceof NoActionInstruction) {
+ return true;
+ }
+ return false;
+ }
+ }
+
+ /**
* Output Instruction.
*/
public static final class OutputInstruction implements Instruction {
diff --git a/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java b/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
index 968db0e..4734672 100644
--- a/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
+++ b/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
@@ -339,6 +339,7 @@
Criterion.Type.class,
DefaultTrafficTreatment.class,
Instructions.DropInstruction.class,
+ Instructions.NoActionInstruction.class,
Instructions.OutputInstruction.class,
Instructions.GroupInstruction.class,
Instructions.TableTypeTransition.class,
diff --git a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowEntryBuilder.java b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowEntryBuilder.java
index f238bdb..cf91860 100644
--- a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowEntryBuilder.java
+++ b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowEntryBuilder.java
@@ -221,11 +221,6 @@
private TrafficTreatment buildTreatment() {
TrafficTreatment.Builder builder = DefaultTrafficTreatment.builder();
- // If this is a drop rule
- if (instructions.size() == 0) {
- builder.drop();
- return builder.build();
- }
for (OFInstruction in : instructions) {
switch (in.getType()) {
case GOTO_TABLE:
diff --git a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowModBuilderVer10.java b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowModBuilderVer10.java
index c9de450..f77819d 100644
--- a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowModBuilderVer10.java
+++ b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowModBuilderVer10.java
@@ -142,7 +142,7 @@
for (Instruction i : treatment.immediate()) {
switch (i.type()) {
case DROP:
- log.warn("Saw drop action; assigning drop action");
+ case NOACTION:
return Collections.emptyList();
case L2MODIFICATION:
act = buildL2Modification(i);
diff --git a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowModBuilderVer13.java b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowModBuilderVer13.java
index 0a53a2d..cc26575 100644
--- a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowModBuilderVer13.java
+++ b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowModBuilderVer13.java
@@ -215,6 +215,7 @@
for (Instruction i : treatments) {
switch (i.type()) {
case DROP:
+ case NOACTION:
return Collections.emptyList();
case L0MODIFICATION:
actions.add(buildL0Modification(i));