홍길동 / onos

Go to a project
Toggle navigation Toggle navigation pinning
Konstantinos Kanonakis
Committed by Gerrit Code Review
a477e341 1 parent d331afdc

Reverting to previous iteration mode, fixing case of modifying existing tags and adding extra checks 

- Reverting iteration through meta instructions to earlier mode
	Assuming instructions appear in the order they were added
- Saving explicitly provided VlanId to new variable modifiedVlan:
	modifiedVlan is the value to be used when modifying existing tag
	assignedVlan is the value to be pushed to untagged packets
	pushedVlan is the value to be pushed to tagged packets (i.e. when pushVlan == true)
- Extra checks added for the following cases:
	Do not allow to pop tag after modifying existing tag
	Do not allow to modify a tag after pushing a new tag
	Do not allow including multiple modify VLAN operations

Change-Id: I92801e3845a2ca1e88181698cb0ba3c22224acf4
Inline Side-by-side
Showing 2 changed files with 88 additions and 11 deletions
...@@ -58,6 +58,7 @@ import org.onosproject.net.flow.criteria.PortCriterion; ...@@ -58,6 +58,7 @@ import org.onosproject.net.flow.criteria.PortCriterion;
58 import org.onosproject.net.flow.criteria.VlanIdCriterion; 58 import org.onosproject.net.flow.criteria.VlanIdCriterion;
59 import org.onosproject.net.flow.instructions.Instruction; 59 import org.onosproject.net.flow.instructions.Instruction;
60 import org.onosproject.net.flow.instructions.Instructions.OutputInstruction; 60 import org.onosproject.net.flow.instructions.Instructions.OutputInstruction;
61 +import org.onosproject.net.flow.instructions.L2ModificationInstruction;
61 import org.onosproject.net.flow.instructions.L2ModificationInstruction.ModVlanIdInstruction; 62 import org.onosproject.net.flow.instructions.L2ModificationInstruction.ModVlanIdInstruction;
62 import org.onosproject.net.flowobjective.FilteringObjective; 63 import org.onosproject.net.flowobjective.FilteringObjective;
63 import org.onosproject.net.flowobjective.FlowObjectiveStore; 64 import org.onosproject.net.flowobjective.FlowObjectiveStore;
...@@ -849,7 +850,8 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour ...@@ -849,7 +850,8 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour
849 850
850 protected List<FlowRule> processVlanIdFilter(VlanIdCriterion vlanIdCriterion, 851 protected List<FlowRule> processVlanIdFilter(VlanIdCriterion vlanIdCriterion,
851 FilteringObjective filt, 852 FilteringObjective filt,
852 - VlanId assignedVlan, 853 + VlanId assignedVlan, VlanId explicitlyAssignedVlan, VlanId pushedVlan,
854 + boolean pushVlan, boolean popVlan,
853 ApplicationId applicationId) { 855 ApplicationId applicationId) {
854 List<FlowRule> rules = new ArrayList<>(); 856 List<FlowRule> rules = new ArrayList<>();
855 log.debug("adding rule for VLAN: {}", vlanIdCriterion.vlanId()); 857 log.debug("adding rule for VLAN: {}", vlanIdCriterion.vlanId());
...@@ -861,6 +863,19 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour ...@@ -861,6 +863,19 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour
861 if (vlanIdCriterion.vlanId() != VlanId.NONE) { 863 if (vlanIdCriterion.vlanId() != VlanId.NONE) {
862 selector.matchVlanId(vlanIdCriterion.vlanId()); 864 selector.matchVlanId(vlanIdCriterion.vlanId());
863 selector.matchInPort(p.port()); 865 selector.matchInPort(p.port());
866 + if (popVlan) {
867 + // Pop outer tag
868 + treatment.immediate().popVlan();
869 + }
870 + if (explicitlyAssignedVlan != null && (!popVlan || !vlanIdCriterion.vlanId().equals(assignedVlan))) {
871 + // Modify VLAN ID on single tagged packet or modify remaining tag after popping
872 + // In the first case, do not set VLAN ID again to the already existing value
873 + treatment.immediate().setVlanId(explicitlyAssignedVlan);
874 + }
875 + if (pushVlan) {
876 + // Push new tag
877 + treatment.immediate().pushVlan().setVlanId(pushedVlan);
878 + }
864 } else { 879 } else {
865 selector.matchInPort(p.port()); 880 selector.matchInPort(p.port());
866 treatment.immediate().pushVlan().setVlanId(assignedVlan); 881 treatment.immediate().pushVlan().setVlanId(assignedVlan);
...@@ -910,11 +925,61 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour ...@@ -910,11 +925,61 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour
910 } 925 }
911 926
912 VlanId assignedVlan = null; 927 VlanId assignedVlan = null;
928 + VlanId modifiedVlan = null;
929 + VlanId pushedVlan = null;
930 + boolean pushVlan = false;
931 + boolean popVlan = false;
913 if (vlanIdCriterion != null) { 932 if (vlanIdCriterion != null) {
914 - // For VLAN cross-connect packets, use the configured VLAN 933 + if (filt.meta() != null) {
915 - if (vlanIdCriterion.vlanId() != VlanId.NONE) { 934 + for (Instruction i : filt.meta().allInstructions()) {
916 - assignedVlan = vlanIdCriterion.vlanId(); 935 + if (i instanceof L2ModificationInstruction) {
936 + if (((L2ModificationInstruction) i).subtype()
937 + .equals(L2ModificationInstruction.L2SubType.VLAN_PUSH)) {
938 + pushVlan = true;
939 + } else if (((L2ModificationInstruction) i).subtype()
940 + .equals(L2ModificationInstruction.L2SubType.VLAN_POP)) {
941 + if (modifiedVlan != null) {
942 + log.error("Pop tag is not allowed after modify VLAN operation " +
943 + "in filtering objective", deviceId);
944 + fail(filt, ObjectiveError.BADPARAMS);
945 + return;
946 + }
947 + popVlan = true;
948 + }
949 + }
950 + if (i instanceof ModVlanIdInstruction) {
951 + if (pushVlan && vlanIdCriterion.vlanId() != VlanId.NONE) {
952 + // Modify VLAN should not appear after pushing a new tag
953 + if (pushedVlan != null) {
954 + log.error("Modify VLAN not allowed after push tag operation " +
955 + "in filtering objective", deviceId);
956 + fail(filt, ObjectiveError.BADPARAMS);
957 + return;
958 + }
959 + pushedVlan = ((ModVlanIdInstruction) i).vlanId();
960 + } else if (vlanIdCriterion.vlanId() == VlanId.NONE) {
961 + // For untagged packets the pushed VLAN ID will be saved in assignedVlan
962 + // just to ensure the driver works as designed for the fabric use case
963 + assignedVlan = ((ModVlanIdInstruction) i).vlanId();
964 + } else {
965 + // For tagged packets modifiedVlan will contain the modified value of existing tag
966 + if (modifiedVlan != null) {
967 + log.error("Driver does not allow multiple modify VLAN operations " +
968 + "in the same filtering objective", deviceId);
969 + fail(filt, ObjectiveError.BADPARAMS);
970 + return;
971 + }
972 + modifiedVlan = ((ModVlanIdInstruction) i).vlanId();
973 + }
974 + }
975 + }
976 + }
917 977
978 + // For VLAN cross-connect packets, use the configured VLAN unless there is an explicitly provided VLAN ID
979 + if (vlanIdCriterion.vlanId() != VlanId.NONE) {
980 + if (assignedVlan == null) {
981 + assignedVlan = vlanIdCriterion.vlanId();
982 + }
918 // For untagged packets, assign a VLAN ID 983 // For untagged packets, assign a VLAN ID
919 } else { 984 } else {
920 if (filt.meta() == null) { 985 if (filt.meta() == null) {
...@@ -923,11 +988,6 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour ...@@ -923,11 +988,6 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour
923 fail(filt, ObjectiveError.BADPARAMS); 988 fail(filt, ObjectiveError.BADPARAMS);
924 return; 989 return;
925 } 990 }
926 - for (Instruction i : filt.meta().allInstructions()) {
927 - if (i instanceof ModVlanIdInstruction) {
928 - assignedVlan = ((ModVlanIdInstruction) i).vlanId();
929 - }
930 - }
931 if (assignedVlan == null) { 991 if (assignedVlan == null) {
932 log.error("Driver requires an assigned vlan-id to tag incoming " 992 log.error("Driver requires an assigned vlan-id to tag incoming "
933 + "untagged packets. Not processing vlan filters on " 993 + "untagged packets. Not processing vlan filters on "
...@@ -936,6 +996,21 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour ...@@ -936,6 +996,21 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour
936 return; 996 return;
937 } 997 }
938 } 998 }
999 + if (pushVlan && popVlan) {
1000 + log.error("Cannot push and pop vlan in the same filtering objective");
1001 + fail(filt, ObjectiveError.BADPARAMS);
1002 + return;
1003 + }
1004 + if (popVlan && vlanIdCriterion.vlanId() == VlanId.NONE) {
1005 + log.error("Cannot pop vlan for untagged packets");
1006 + fail(filt, ObjectiveError.BADPARAMS);
1007 + return;
1008 + }
1009 + if ((pushVlan && pushedVlan == null) && vlanIdCriterion.vlanId() != VlanId.NONE) {
1010 + log.error("No VLAN ID provided for push tag operation");
1011 + fail(filt, ObjectiveError.BADPARAMS);
1012 + return;
1013 + }
939 } 1014 }
940 1015
941 if (ethCriterion == null) { 1016 if (ethCriterion == null) {
...@@ -958,7 +1033,8 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour ...@@ -958,7 +1033,8 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour
958 } else { 1033 } else {
959 for (FlowRule vlanRule : processVlanIdFilter(vlanIdCriterion, 1034 for (FlowRule vlanRule : processVlanIdFilter(vlanIdCriterion,
960 filt, 1035 filt,
961 - assignedVlan, 1036 + assignedVlan, modifiedVlan, pushedVlan,
1037 + pushVlan, popVlan,
962 applicationId)) { 1038 applicationId)) {
963 log.debug("adding VLAN filtering rule in VLAN table: {} for dev: {}", 1039 log.debug("adding VLAN filtering rule in VLAN table: {} for dev: {}",
964 vlanRule, deviceId); 1040 vlanRule, deviceId);
......
...@@ -205,7 +205,8 @@ public class SpringOpenTTPDell extends SpringOpenTTP { ...@@ -205,7 +205,8 @@ public class SpringOpenTTPDell extends SpringOpenTTP {
205 @Override 205 @Override
206 protected List<FlowRule> processVlanIdFilter(VlanIdCriterion vlanIdCriterion, 206 protected List<FlowRule> processVlanIdFilter(VlanIdCriterion vlanIdCriterion,
207 FilteringObjective filt, 207 FilteringObjective filt,
208 - VlanId assignedVlan, 208 + VlanId assignedVlan, VlanId modifiedVlan, VlanId pushedVlan,
209 + boolean popVlan, boolean pushVlan,
209 ApplicationId applicationId) { 210 ApplicationId applicationId) {
210 log.debug("For now not adding any VLAN rules " 211 log.debug("For now not adding any VLAN rules "
211 + "into Dell switches as it is ignoring"); 212 + "into Dell switches as it is ignoring");
......