T3: Handling double rules on table 10 for HW
Change-Id: I700138780ef72caf5a58507dfd37178f1e2237f3
(cherry picked from commit 8314b07a7bb73775eb6f00d5f132a7c31f1853f6)
diff --git a/src/main/java/org/onosproject/t3/cli/TroubleshootTraceCommand.java b/src/main/java/org/onosproject/t3/cli/TroubleshootTraceCommand.java
index 7387c5f..587c748 100644
--- a/src/main/java/org/onosproject/t3/cli/TroubleshootTraceCommand.java
+++ b/src/main/java/org/onosproject/t3/cli/TroubleshootTraceCommand.java
@@ -29,6 +29,7 @@
import org.onosproject.net.flow.TrafficSelector;
import org.onosproject.net.flow.TrafficTreatment;
import org.onosproject.net.group.GroupBucket;
+import org.onosproject.t3.api.GroupsInDevice;
import org.onosproject.t3.api.StaticPacketTrace;
import org.onosproject.t3.api.TroubleshootService;
@@ -227,26 +228,29 @@
//Prints the groups for a given trace and a specified level of verbosity
private void printGroups(StaticPacketTrace trace, boolean verbose, ConnectPoint connectPoint) {
- print("Groups");
- trace.getGroupOuputs(connectPoint.deviceId()).forEach(output -> {
- if (output.getOutput().equals(connectPoint)) {
- output.getGroups().forEach(group -> {
- if (verbose) {
- print(GROUP_FORMAT, Integer.toHexString(group.id().id()), group.state(), group.type(),
- group.bytes(), group.packets(), group.appId().name(), group.referenceCount());
- int i = 0;
- for (GroupBucket bucket : group.buckets().buckets()) {
- print(GROUP_BUCKET_FORMAT, Integer.toHexString(group.id().id()), ++i,
- bucket.bytes(), bucket.packets(),
- bucket.treatment().allInstructions());
+ List<GroupsInDevice> groupsInDevice = trace.getGroupOuputs(connectPoint.deviceId());
+ if (groupsInDevice != null) {
+ print("Groups");
+ groupsInDevice.forEach(output -> {
+ if (output.getOutput().equals(connectPoint)) {
+ output.getGroups().forEach(group -> {
+ if (verbose) {
+ print(GROUP_FORMAT, Integer.toHexString(group.id().id()), group.state(), group.type(),
+ group.bytes(), group.packets(), group.appId().name(), group.referenceCount());
+ int i = 0;
+ for (GroupBucket bucket : group.buckets().buckets()) {
+ print(GROUP_BUCKET_FORMAT, Integer.toHexString(group.id().id()), ++i,
+ bucket.bytes(), bucket.packets(),
+ bucket.treatment().allInstructions());
+ }
+ } else {
+ print(" groupId=%s", group.id());
}
- } else {
- print(" groupId=%s", group.id());
- }
- });
- print("Outgoing Packet %s", output.getFinalPacket());
- }
- });
+ });
+ print("Outgoing Packet %s", output.getFinalPacket());
+ }
+ });
+ }
}
private String printTreatment(TrafficTreatment treatment) {
diff --git a/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java b/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java
index 5d6fabb..bbec3ea 100644
--- a/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java
+++ b/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java
@@ -73,6 +73,10 @@
import static org.onlab.packet.EthType.EtherType;
import static org.onosproject.net.flow.TrafficSelector.Builder;
import static org.onosproject.net.flow.instructions.Instructions.GroupInstruction;
+import static org.onosproject.net.flow.instructions.L2ModificationInstruction.ModEtherInstruction;
+import static org.onosproject.net.flow.instructions.L2ModificationInstruction.ModMplsHeaderInstruction;
+import static org.onosproject.net.flow.instructions.L2ModificationInstruction.ModMplsLabelInstruction;
+import static org.onosproject.net.flow.instructions.L2ModificationInstruction.ModVlanIdInstruction;
import static org.slf4j.LoggerFactory.getLogger;
/**
@@ -380,7 +384,6 @@
if (flowEntry == null && isOfdpaHardware) {
log.debug("Ofdpa Hw setup, no flow rule means table miss");
- //Handling Hardware Specifics
if (((IndexTableId) tableId).id() == 27) {
//Apparently a miss but Table 27 on OFDPA is a fixed table
packet = handleOfdpa27FixedTable(trace, packet);
@@ -409,12 +412,12 @@
tableId = nextTableIdEntry.table();
}
-
} else if (flowEntry == null) {
trace.addResultMessage("Packet has no match on table " + tableId + " in device " +
in.deviceId() + ". Dropping");
return trace;
} else {
+
//IF the table has a transition
if (flowEntry.treatment().tableTransition() != null) {
//update the next table we transitions to
@@ -439,6 +442,37 @@
deferredInstructions.clear();
}
+ //On table 10 OFDPA needs two rules to apply the vlan if none and then to transition to the next table.
+ if (needsSecondTable10Flow(flowEntry, isOfdpaHardware)) {
+
+ //Let's get the packet vlanId instruction
+ VlanIdCriterion packetVlanIdCriterion =
+ (VlanIdCriterion) packet.getCriterion(Criterion.Type.VLAN_VID);
+
+ //Let's get the flow entry vlan mod instructions
+ ModVlanIdInstruction entryModVlanIdInstruction = (ModVlanIdInstruction) flowEntry.treatment()
+ .immediate().stream()
+ .filter(instruction -> instruction instanceof ModVlanIdInstruction)
+ .findFirst().orElse(null);
+
+ //If the entry modVlan is not null we need to make sure that the packet has been updated and there
+ // is a flow rule that matches on same criteria and with updated vlanId
+ if (entryModVlanIdInstruction != null) {
+
+ FlowEntry secondVlanFlow = getSecondFlowEntryOnTable10(packet, in,
+ packetVlanIdCriterion, entryModVlanIdInstruction);
+
+ //We found the flow that we expected
+ if (secondVlanFlow != null) {
+ flows.add(secondVlanFlow);
+ } else {
+ trace.addResultMessage("Missing forwarding rule for tagged packet on " + in);
+ return trace;
+ }
+ }
+
+ }
+
}
}
@@ -449,19 +483,9 @@
//Adding all the flows to the trace
trace.addFlowsForDevice(in.deviceId(), ImmutableList.copyOf(flows));
- log.debug("Flows traversed by {}", packet);
- flows.forEach(entry -> {
- log.debug("Flow {}", entry);
- });
-
- log.debug("Output Flows for {}", packet);
- outputFlows.forEach(entry -> {
- log.debug("Output Flow {}", entry);
- });
List<PortNumber> outputPorts = new ArrayList<>();
//TODO optimization
-
//outputFlows contains also last rule of device, so we need filtering for OUTPUT instructions.
List<FlowEntry> outputFlowEntries = outputFlows.stream().filter(flow -> flow.treatment()
.allInstructions().stream().filter(instruction -> instruction.type()
@@ -503,19 +527,53 @@
}
packet = builder.build();
- log.debug("Groups hit by packet {}", packet);
- groups.forEach(group -> {
- log.debug("Group {}", group);
- });
- log.debug("Output ports for packet {}", packet);
- outputPorts.forEach(port -> {
- log.debug("Port {}", port);
- });
- log.info("Output Packet {}", packet);
+ log.debug("Output Packet {}", packet);
return trace;
}
+ private boolean needsSecondTable10Flow(FlowEntry flowEntry, boolean isOfdpaHardware) {
+ return isOfdpaHardware && flowEntry.table().equals(IndexTableId.of(10))
+ && flowEntry.selector().getCriterion(Criterion.Type.VLAN_VID) != null
+ && ((VlanIdCriterion) flowEntry.selector().getCriterion(Criterion.Type.VLAN_VID))
+ .vlanId().equals(VlanId.NONE);
+ }
+
+ /**
+ * Method that finds a flow rule on table 10 that matches the packet and the VLAN of the already
+ * found rule on table 10. This is because OFDPA needs two rules on table 10, first to apply the rule,
+ * second to transition to following table
+ *
+ * @param packet the incoming packet
+ * @param in the input connect point
+ * @param packetVlanIdCriterion the vlan criterion from the packet
+ * @param entryModVlanIdInstruction the entry vlan instruction
+ * @return the second flow entry that matched
+ */
+ private FlowEntry getSecondFlowEntryOnTable10(TrafficSelector packet, ConnectPoint in,
+ VlanIdCriterion packetVlanIdCriterion,
+ ModVlanIdInstruction entryModVlanIdInstruction) {
+ FlowEntry secondVlanFlow = null;
+ //Check the packet has been update from the first rule.
+ if (packetVlanIdCriterion.vlanId().equals(entryModVlanIdInstruction.vlanId())) {
+ //find a rule on the same table that matches the vlan and
+ // also all the other elements of the flow such as input port
+ secondVlanFlow = Lists.newArrayList(flowRuleService.getFlowEntries(in.deviceId()).iterator())
+ .stream()
+ .filter(entry -> {
+ return entry.table().equals(IndexTableId.of(10));
+ })
+ .filter(entry -> {
+ VlanIdCriterion criterion = (VlanIdCriterion) entry.selector()
+ .getCriterion(Criterion.Type.VLAN_VID);
+ return criterion != null && match(packet, entry)
+ && criterion.vlanId().equals(entryModVlanIdInstruction.vlanId());
+ }).findFirst().orElse(null);
+
+ }
+ return secondVlanFlow;
+ }
+
/**
* Handles table 27 in Ofpda which is a fixed table not visible to any controller that handles Mpls Labels.
@@ -699,8 +757,8 @@
L2ModificationInstruction l2Instruction = (L2ModificationInstruction) instruction;
switch (l2Instruction.subtype()) {
case VLAN_ID:
- L2ModificationInstruction.ModVlanIdInstruction vlanIdInstruction =
- (L2ModificationInstruction.ModVlanIdInstruction) instruction;
+ ModVlanIdInstruction vlanIdInstruction =
+ (ModVlanIdInstruction) instruction;
VlanId id = vlanIdInstruction.vlanId();
criterion = Criteria.matchVlanId(id);
break;
@@ -708,13 +766,13 @@
criterion = Criteria.matchVlanId(VlanId.NONE);
break;
case MPLS_PUSH:
- L2ModificationInstruction.ModMplsHeaderInstruction mplsEthInstruction =
- (L2ModificationInstruction.ModMplsHeaderInstruction) instruction;
+ ModMplsHeaderInstruction mplsEthInstruction =
+ (ModMplsHeaderInstruction) instruction;
criterion = Criteria.matchEthType(mplsEthInstruction.ethernetType().toShort());
break;
case MPLS_POP:
- L2ModificationInstruction.ModMplsHeaderInstruction mplsPopInstruction =
- (L2ModificationInstruction.ModMplsHeaderInstruction) instruction;
+ ModMplsHeaderInstruction mplsPopInstruction =
+ (ModMplsHeaderInstruction) instruction;
criterion = Criteria.matchEthType(mplsPopInstruction.ethernetType().toShort());
//When popping MPLS we remove label and BOS
@@ -730,19 +788,19 @@
break;
case MPLS_LABEL:
- L2ModificationInstruction.ModMplsLabelInstruction mplsLabelInstruction =
- (L2ModificationInstruction.ModMplsLabelInstruction) instruction;
+ ModMplsLabelInstruction mplsLabelInstruction =
+ (ModMplsLabelInstruction) instruction;
criterion = Criteria.matchMplsLabel(mplsLabelInstruction.label());
newSelector.matchMplsBos(true);
break;
case ETH_DST:
- L2ModificationInstruction.ModEtherInstruction modEtherDstInstruction =
- (L2ModificationInstruction.ModEtherInstruction) instruction;
+ ModEtherInstruction modEtherDstInstruction =
+ (ModEtherInstruction) instruction;
criterion = Criteria.matchEthDst(modEtherDstInstruction.mac());
break;
case ETH_SRC:
- L2ModificationInstruction.ModEtherInstruction modEtherSrcInstruction =
- (L2ModificationInstruction.ModEtherInstruction) instruction;
+ ModEtherInstruction modEtherSrcInstruction =
+ (ModEtherInstruction) instruction;
criterion = Criteria.matchEthSrc(modEtherSrcInstruction.mac());
break;
default:
diff --git a/src/test/java/org/onosproject/t3/impl/T3TestObjects.java b/src/test/java/org/onosproject/t3/impl/T3TestObjects.java
index 9009c8e..abbe77e 100644
--- a/src/test/java/org/onosproject/t3/impl/T3TestObjects.java
+++ b/src/test/java/org/onosproject/t3/impl/T3TestObjects.java
@@ -344,6 +344,67 @@
static final FlowEntry HARDWARE_ETH_FLOW_ENTRY = new DefaultFlowEntry(HARDWARE_ETH_FLOW);
+ //HW Double Rule on 10
+
+ static final DeviceId HARDWARE_DEVICE_10 = DeviceId.deviceId("HardwareDevice10");
+
+ static final ConnectPoint HARDWARE_DEVICE_10_IN_CP = ConnectPoint.deviceConnectPoint(HARDWARE_DEVICE_10 + "/" + 1);
+
+ static final ConnectPoint HARDWARE_DEVICE_10_OUT_CP = ConnectPoint.deviceConnectPoint(HARDWARE_DEVICE_10 + "/" + 2);
+
+ private static final TrafficSelector HARDWARE_10_FLOW_SELECTOR = DefaultTrafficSelector.builder()
+ .matchInPort(PortNumber.portNumber(1))
+ .matchIPSrc(IpPrefix.valueOf("127.0.0.1/32"))
+ .matchIPDst(IpPrefix.valueOf("127.0.0.2/32"))
+ .matchVlanId(VlanId.NONE)
+ .build();
+
+ private static final TrafficTreatment HARDWARE_10_TRANSITION_FLOW_TREATMENT = DefaultTrafficTreatment.builder()
+ .setVlanId(VlanId.vlanId("10"))
+ .transition(20)
+ .build();
+
+ private static final FlowRule HARDWARE_DEVICE_10_FLOW = DefaultFlowEntry.builder().forDevice(HARDWARE_DEVICE_10)
+ .forTable(10)
+ .withPriority(100)
+ .withSelector(HARDWARE_10_FLOW_SELECTOR)
+ .withTreatment(HARDWARE_10_TRANSITION_FLOW_TREATMENT)
+ .fromApp(new DefaultApplicationId(0, "TestApp"))
+ .makePermanent()
+ .build();
+
+ static final FlowEntry HARDWARE_10_FLOW_ENTRY = new DefaultFlowEntry(HARDWARE_DEVICE_10_FLOW);
+
+ private static final TrafficSelector HARDWARE_10_SECOND_SELECTOR = DefaultTrafficSelector.builder()
+ .matchInPort(PortNumber.portNumber(1))
+ .matchVlanId(VlanId.vlanId("10"))
+ .matchIPSrc(IpPrefix.valueOf("127.0.0.1/32"))
+ .matchIPDst(IpPrefix.valueOf("127.0.0.2/32"))
+ .matchEthType(EthType.EtherType.IPV4.ethType().toShort())
+ .build();
+
+ private static final FlowRule HARDWARE_10_SECOND_FLOW = DefaultFlowEntry.builder().forDevice(HARDWARE_DEVICE_10)
+ .forTable(10)
+ .withPriority(100)
+ .withSelector(HARDWARE_10_SECOND_SELECTOR)
+ .withTreatment(HARDWARE_10_TRANSITION_FLOW_TREATMENT)
+ .fromApp(new DefaultApplicationId(0, "TestApp"))
+ .makePermanent()
+ .build();
+
+ static final FlowEntry HARDWARE_10_SECOND_FLOW_ENTRY = new DefaultFlowEntry(HARDWARE_10_SECOND_FLOW);
+
+ private static final FlowRule HARDWARE_10_OUTPUT_FLOW = DefaultFlowEntry.builder().forDevice(HARDWARE_DEVICE_10)
+ .forTable(20)
+ .withPriority(100)
+ .withSelector(SINGLE_FLOW_SELECTOR)
+ .withTreatment(OUTPUT_FLOW_TREATMENT)
+ .fromApp(new DefaultApplicationId(0, "TestApp"))
+ .makePermanent()
+ .build();
+
+ static final FlowEntry HARDWARE_10_OUTPUT_FLOW_ENTRY = new DefaultFlowEntry(HARDWARE_10_OUTPUT_FLOW);
+
//Dual Links
// - (1) Device 1 (2-3) - (1-4) Device 2 (2-3) - (1-2) Device 3 (3) -
static final DeviceId DUAL_LINK_1 = DeviceId.deviceId("DualLink1");
@@ -495,6 +556,7 @@
.matchEthType(EthType.EtherType.IPV4.ethType().toShort())
.matchIPSrc(IpPrefix.valueOf("127.0.0.1/32"))
.matchIPDst(IpPrefix.valueOf("127.0.0.2/32"))
+ .matchVlanId(VlanId.NONE)
.build();
static final TrafficSelector PACKET_OK_TOPO = DefaultTrafficSelector.builder()
diff --git a/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java b/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java
index 5426278..9767730 100644
--- a/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java
+++ b/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java
@@ -262,6 +262,20 @@
}
/**
+ * Test that HW has two rules on table 10 for untagged packets.
+ */
+ @Test
+ public void hardwareTable10Test() throws Exception {
+
+ StaticPacketTrace traceSuccess = testSuccess(PACKET_OK, HARDWARE_DEVICE_10_IN_CP,
+ HARDWARE_DEVICE_10, HARDWARE_DEVICE_10_OUT_CP, 1, 1);
+
+ assertTrue("Second flow rule is absent", traceSuccess.getFlowsForDevice(HARDWARE_DEVICE_10)
+ .contains(HARDWARE_10_SECOND_FLOW_ENTRY));
+
+ }
+
+ /**
* Test dual links between 3 topology elements.
*/
@Test
@@ -348,6 +362,9 @@
return ImmutableList.of(DUAL_LINK_3_FLOW_ENTRY, DUAL_LINK_3_FLOW_ENTRY_2);
} else if (deviceId.equals(DEFERRED_1)) {
return ImmutableList.of(DEFERRED_FLOW_ENTRY, DEFERRED_CLEAR_FLOW_ENTRY);
+ } else if (deviceId.equals(HARDWARE_DEVICE_10)) {
+ return ImmutableList.of(HARDWARE_10_FLOW_ENTRY, HARDWARE_10_SECOND_FLOW_ENTRY,
+ HARDWARE_10_OUTPUT_FLOW_ENTRY);
}
return ImmutableList.of();
}
@@ -356,7 +373,7 @@
private class TestDriverService extends DriverServiceAdapter {
@Override
public Driver getDriver(DeviceId deviceId) {
- if (deviceId.equals(HARDWARE_DEVICE)) {
+ if (deviceId.equals(HARDWARE_DEVICE) || deviceId.equals(HARDWARE_DEVICE_10)) {
return new DefaultDriver("ofdpa", ImmutableList.of(),
"test", "test", "test", new HashMap<>(), new HashMap<>());
}