[VOL-3824] Wrong PCP for DHCP traffic
Change-Id: I51fb14e5c966025db7283b41cc6dcea2f262875b
diff --git a/app/src/main/java/org/opencord/olt/impl/OltFlowService.java b/app/src/main/java/org/opencord/olt/impl/OltFlowService.java
index bf99fe4..429dd95 100644
--- a/app/src/main/java/org/opencord/olt/impl/OltFlowService.java
+++ b/app/src/main/java/org/opencord/olt/impl/OltFlowService.java
@@ -241,7 +241,8 @@
int techProfileId = tagInformation != null ? tagInformation.getTechnologyProfileId() : NONE_TP_ID;
VlanId cTag = tagInformation != null ? tagInformation.getPonCTag() : VlanId.NONE;
VlanId unitagMatch = tagInformation != null ? tagInformation.getUniTagMatch() : VlanId.ANY;
- Byte vlanPcp = tagInformation != null ? (byte) tagInformation.getUsPonCTagPriority() : null;
+ Byte vlanPcp = tagInformation != null && tagInformation.getUsPonCTagPriority() != NO_PCP
+ ? (byte) tagInformation.getUsPonCTagPriority() : null;
if (enableDhcpV4) {
diff --git a/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java b/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java
index 73f9b14..033945e 100644
--- a/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java
+++ b/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java
@@ -64,6 +64,7 @@
public class OltFlowTest extends TestBase {
private OltFlowService oltFlowService;
PortNumber uniPortNumber = PortNumber.portNumber(1);
+ PortNumber uniPortNumber2 = PortNumber.portNumber(2);
PortNumber nniPortNumber = PortNumber.portNumber(65535);
UniTagInformation.Builder tagInfoBuilder = new UniTagInformation.Builder();
@@ -79,6 +80,21 @@
.setIsIgmpRequired(true)
.build();
+ UniTagInformation.Builder tagInfoBuilderNoPcp = new UniTagInformation.Builder();
+ UniTagInformation uniTagInfoNoPcp = tagInfoBuilderNoPcp.setUniTagMatch(VlanId.vlanId((short) 35))
+ .setPonCTag(VlanId.vlanId((short) 34))
+ .setPonSTag(VlanId.vlanId((short) 7))
+ .setDsPonCTagPriority(-1)
+ .setUsPonSTagPriority(-1)
+ .setUsPonCTagPriority(-1)
+ .setDsPonSTagPriority(-1)
+ .setTechnologyProfileId(64)
+ .setDownstreamBandwidthProfile(dsBpId)
+ .setUpstreamBandwidthProfile(usBpId)
+ .setIsDhcpRequired(true)
+ .setIsIgmpRequired(true)
+ .build();
+
UniTagInformation.Builder tagInfoBuilder2 = new UniTagInformation.Builder();
UniTagInformation uniTagInfoNoDhcpNoIgmp = tagInfoBuilder2
.setUniTagMatch(VlanId.vlanId((short) 35))
@@ -114,11 +130,18 @@
usMeterId, uniTagInfo,
false, true);
assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 2;
+
+ // Ensure upstream flow has no pcp unless properly specified.
+ oltFlowService.processDhcpFilteringObjectives(DEVICE_ID_1, uniPortNumber2,
+ usMeterId, uniTagInfoNoPcp,
+ true, true);
+ assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 3;
+
// ensure upstream flows are not added if uniTagInfo is missing dhcp requirement
oltFlowService.processDhcpFilteringObjectives(DEVICE_ID_1, uniPortNumber,
usMeterId, uniTagInfoNoDhcpNoIgmp,
true, true);
- assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 2;
+ assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 3;
// ensure downstream traps don't succeed without global config for nni ports
oltFlowService.processDhcpFilteringObjectives(DEVICE_ID_1, nniPortNumber,
@@ -127,7 +150,7 @@
oltFlowService.processDhcpFilteringObjectives(DEVICE_ID_1, nniPortNumber,
null, null,
false, false);
- assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 2;
+ assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 3;
// do global config for nni ports and now it should succeed
oltFlowService.enableDhcpOnNni = true;
oltFlowService.processDhcpFilteringObjectives(DEVICE_ID_1, nniPortNumber,
@@ -136,21 +159,21 @@
oltFlowService.processDhcpFilteringObjectives(DEVICE_ID_1, nniPortNumber,
null, null,
false, false);
- assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 4;
+ assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 5;
// turn on DHCPv6 and we should get 2 flows
oltFlowService.enableDhcpV6 = true;
oltFlowService.processDhcpFilteringObjectives(DEVICE_ID_1, uniPortNumber,
usMeterId, uniTagInfo,
true, true);
- assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 6;
+ assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 7;
// turn off DHCPv4 and it's only v6
oltFlowService.enableDhcpV4 = false;
oltFlowService.processDhcpFilteringObjectives(DEVICE_ID_1, uniPortNumber,
usMeterId, uniTagInfo,
true, true);
- assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 7;
+ assert oltFlowService.flowObjectiveService.getPendingFlowObjectives().size() == 8;
// cleanup
oltFlowService.flowObjectiveService.clearQueue();
@@ -335,6 +358,16 @@
filterForCriterion(filteringObjective.conditions(), Criterion.Type.VLAN_VID);
PortCriterion portCriterion = (PortCriterion) filteringObjective.key();
+ filteringObjective.meta().allInstructions().forEach(instruction -> {
+ if (instruction.type().equals(Instruction.Type.L2MODIFICATION)) {
+ L2ModificationInstruction l2Instruction = (L2ModificationInstruction) instruction;
+ if (l2Instruction.subtype().equals(L2ModificationInstruction.L2SubType.VLAN_PCP)) {
+ //this, given the uniTagInfo we provide, should not be present
+ assert false;
+ }
+ }
+ });
+
if (ethType.ethType().equals(EthType.EtherType.LLDP.ethType()) ||
portCriterion.port().equals(nniPortNumber)) {
@@ -344,7 +377,8 @@
} else {
assert meter.meterId().equals(usMeterId) || meter.meterId().equals(dsMeterId);
assert writeMetadata != null;
- assert vlanIdCriterion == null || vlanIdCriterion.vlanId() == uniTagInfo.getUniTagMatch();
+ assert vlanIdCriterion == null || vlanIdCriterion.vlanId() == uniTagInfo.getUniTagMatch()
+ || vlanIdCriterion.vlanId() == uniTagInfoNoPcp.getUniTagMatch();
}
}