Supports capability of T3 to trace multiple actions with priority in a group
T3 sets an egress packet of a device in the trace after handling
OUTPUT action of a group, so actions placed after OUTPUT were ignored.
This was fixed by sorting the list of instructions with priority.
Change-Id: I071f9356e53924f90a06eb9f184e0c762b3975d4
(cherry picked from commit 80a6276388034a79c54e403c93f02094485a45d5)
diff --git a/apps/t3/app/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java b/apps/t3/app/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java
index df73a61..51332da 100644
--- a/apps/t3/app/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java
+++ b/apps/t3/app/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java
@@ -1117,7 +1117,16 @@
Builder builder, List<PortNumber> outputPorts,
ConnectPoint in, List<ConnectPoint> completePath) {
List<Instruction> groupInstructionlist = new ArrayList<>();
- for (Instruction instruction : instructions) {
+ // sort instructions according to priority (larger Instruction.Type ENUM constant first)
+ // which enables to treat other actions before the OUTPUT action
+ //TODO improve the priority scheme according to the OpenFlow ActionSet spec
+ List<Instruction> instructionsSorted = new ArrayList<>();
+ instructionsSorted.addAll(instructions);
+ instructionsSorted.sort((instr1, instr2) -> {
+ return Integer.compare(instr2.type().ordinal(), instr1.type().ordinal());
+ });
+
+ for (Instruction instruction : instructionsSorted) {
log.debug("Considering Instruction {}", instruction);
//if the instruction is not group we need to update the packet or add the output
//to the possible outputs for this packet
diff --git a/apps/t3/app/src/test/java/org/onosproject/t3/impl/T3TestObjects.java b/apps/t3/app/src/test/java/org/onosproject/t3/impl/T3TestObjects.java
index 8501400..2ec39b4 100644
--- a/apps/t3/app/src/test/java/org/onosproject/t3/impl/T3TestObjects.java
+++ b/apps/t3/app/src/test/java/org/onosproject/t3/impl/T3TestObjects.java
@@ -199,6 +199,39 @@
static final ConnectPoint GROUP_FLOW_OUT_CP = ConnectPoint.deviceConnectPoint(GROUP_FLOW_DEVICE + "/" + 2);
+ // Group multiple action order test
+ static final DeviceId ACTION_ORDER_DEVICE = DeviceId.deviceId("ActionOrderDevice");
+ private static final VlanId ACTION_ORDER_VLAN_ID = VlanId.vlanId("999");
+ static final MplsLabel ACTION_ORDER_MPLS_LABEL = MplsLabel.mplsLabel("999");
+ private static final TrafficTreatment ACTION_ORDER_FLOW_TREATMENT = DefaultTrafficTreatment.builder()
+ .pushVlan()
+ .setVlanId(ACTION_ORDER_VLAN_ID)
+ .group(GROUP_ID)
+ .build();
+ private static final FlowRule ACTION_ORDER_FLOW = DefaultFlowEntry.builder().forDevice(ACTION_ORDER_DEVICE)
+ .forTable(0)
+ .withPriority(100)
+ .withSelector(SINGLE_FLOW_SELECTOR)
+ .withTreatment(ACTION_ORDER_FLOW_TREATMENT)
+ .fromApp(new DefaultApplicationId(0, "TestApp"))
+ .makePermanent()
+ .build();
+ static final FlowEntry ACTION_ORDER_FLOW_ENTRY = new DefaultFlowEntry(ACTION_ORDER_FLOW);
+ private static final TrafficTreatment ACTION_ORDER_GROUP_TREATMENT = DefaultTrafficTreatment.builder()
+ // make lower order actions come first
+ .setOutput(PortNumber.portNumber(2))
+ .setMpls(ACTION_ORDER_MPLS_LABEL)
+ .pushMpls()
+ .popVlan()
+ .build();
+ private static final GroupBucket ACTION_ORDER_BUCKET = DefaultGroupBucket
+ .createSelectGroupBucket(ACTION_ORDER_GROUP_TREATMENT);
+ private static final GroupBuckets ACTION_ORDER_BUCKETS = new GroupBuckets(ImmutableList.of(ACTION_ORDER_BUCKET));
+ static final Group ACTION_ORDER_GROUP = new DefaultGroup(
+ GROUP_ID, ACTION_ORDER_DEVICE, Group.Type.SELECT, ACTION_ORDER_BUCKETS);
+ static final ConnectPoint ACTION_ORDER_IN_CP = ConnectPoint.deviceConnectPoint(ACTION_ORDER_DEVICE + "/" + 1);
+ static final ConnectPoint ACTION_ORDER_OUT_CP = ConnectPoint.deviceConnectPoint(ACTION_ORDER_DEVICE + "/" + 2);
+
//topology
static final DeviceId TOPO_FLOW_DEVICE = DeviceId.deviceId("SingleFlowDevice1");
diff --git a/apps/t3/app/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java b/apps/t3/app/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java
index 8ac76c8..c00d373 100644
--- a/apps/t3/app/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java
+++ b/apps/t3/app/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java
@@ -47,6 +47,7 @@
import org.onosproject.net.flow.TrafficSelector;
import org.onosproject.net.flow.criteria.Criterion;
import org.onosproject.net.flow.criteria.EthTypeCriterion;
+import org.onosproject.net.flow.criteria.MplsCriterion;
import org.onosproject.net.flow.criteria.VlanIdCriterion;
import org.onosproject.net.group.Group;
import org.onosproject.net.group.GroupServiceAdapter;
@@ -235,6 +236,27 @@
}
/**
+ * Test a single flow rule that points to a group with multiple actions
+ * that need to be executed in the order specified in the OpenFlow spec.
+ */
+ @Test
+ public void testGroupMultipleActionsOrdered() {
+
+ StaticPacketTrace traceSuccess = testSuccess(
+ PACKET_OK, ACTION_ORDER_IN_CP, ACTION_ORDER_DEVICE, ACTION_ORDER_OUT_CP, 1, 1);
+
+ assertEquals("Packet should not have VLAN ID",
+ VlanId.NONE,
+ ((VlanIdCriterion) traceSuccess.getGroupOuputs(ACTION_ORDER_DEVICE)
+ .get(0).getFinalPacket().getCriterion(Criterion.Type.VLAN_VID)).vlanId());
+ assertEquals("Packet should have MPLS label",
+ ACTION_ORDER_MPLS_LABEL,
+ ((MplsCriterion) traceSuccess.getGroupOuputs(ACTION_ORDER_DEVICE)
+ .get(0).getFinalPacket().getCriterion(Criterion.Type.MPLS_LABEL)).label());
+
+ }
+
+ /**
* Test path through a 3 device topology.
*/
@Test
@@ -457,6 +479,8 @@
return ImmutableList.of(DUAL_HOME_FLOW_ENTRY);
} else if (deviceId.equals(DUAL_HOME_DEVICE_2) || deviceId.equals(DUAL_HOME_DEVICE_3)) {
return ImmutableList.of(DUAL_HOME_OUT_FLOW_ENTRY);
+ } else if (deviceId.equals(ACTION_ORDER_DEVICE)) {
+ return ImmutableList.of(ACTION_ORDER_FLOW_ENTRY);
}
return ImmutableList.of();
}
@@ -489,6 +513,8 @@
return ImmutableList.of(NO_BUCKET_GROUP);
} else if (deviceId.equals(DUAL_HOME_DEVICE_1)) {
return ImmutableList.of(DUAL_HOME_GROUP);
+ } else if (deviceId.equals(ACTION_ORDER_DEVICE)) {
+ return ImmutableList.of(ACTION_ORDER_GROUP);
}
return ImmutableList.of();
}
@@ -509,7 +535,8 @@
connectPoint.equals(HARDWARE_DEVICE_OUT_CP) ||
connectPoint.equals(HARDWARE_DEVICE_10_OUT_CP) ||
connectPoint.equals(DEFERRED_CP_2_OUT) ||
- connectPoint.equals(DUAL_LINK_3_CP_3_OUT)) {
+ connectPoint.equals(DUAL_LINK_3_CP_3_OUT) ||
+ connectPoint.equals(ACTION_ORDER_OUT_CP)) {
return ImmutableSet.of(H1);
}
if (connectPoint.equals(DUAL_HOME_CP_2_2) || connectPoint.equals(DUAL_HOME_CP_3_2)) {