Committed by
Brian O'Connor
Instruction related fixes
- Fixed PushHeaderInstructions bug, where half-baked Ethernet instace was used only to hold ethernetType. (ONOS-987) Conflicts: core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/GroupModBuilder.java web/api/src/main/java/org/onosproject/codec/impl/InstructionCodec.java web/api/src/test/java/org/onosproject/codec/impl/InstructionJsonMatcher.java web/api/src/test/java/org/onosproject/rest/FlowsResourceTest.java Change-Id: I330a862c8a18206250befbd4e22ee6d189beed83
Showing
4 changed files
with
31 additions
and
21 deletions
| ... | @@ -148,7 +148,7 @@ public final class Instructions { | ... | @@ -148,7 +148,7 @@ public final class Instructions { |
| 148 | */ | 148 | */ |
| 149 | public static Instruction pushMpls() { | 149 | public static Instruction pushMpls() { |
| 150 | return new PushHeaderInstructions(L2SubType.MPLS_PUSH, | 150 | return new PushHeaderInstructions(L2SubType.MPLS_PUSH, |
| 151 | - new Ethernet().setEtherType(Ethernet.MPLS_UNICAST)); | 151 | + Ethernet.MPLS_UNICAST); |
| 152 | } | 152 | } |
| 153 | 153 | ||
| 154 | /** | 154 | /** |
| ... | @@ -157,7 +157,18 @@ public final class Instructions { | ... | @@ -157,7 +157,18 @@ public final class Instructions { |
| 157 | */ | 157 | */ |
| 158 | public static Instruction popMpls() { | 158 | public static Instruction popMpls() { |
| 159 | return new PushHeaderInstructions(L2SubType.MPLS_POP, | 159 | return new PushHeaderInstructions(L2SubType.MPLS_POP, |
| 160 | - new Ethernet().setEtherType(Ethernet.MPLS_UNICAST)); | 160 | + Ethernet.MPLS_UNICAST); |
| 161 | + } | ||
| 162 | + | ||
| 163 | + /** | ||
| 164 | + * Creates a mpls header instruction. | ||
| 165 | + * | ||
| 166 | + * @param etherType Ethernet type to set | ||
| 167 | + * @return a L2 modification. | ||
| 168 | + */ | ||
| 169 | + public static Instruction popMpls(Short etherType) { | ||
| 170 | + checkNotNull(etherType, "Ethernet type cannot be null"); | ||
| 171 | + return new PushHeaderInstructions(L2SubType.MPLS_POP, etherType); | ||
| 161 | } | 172 | } |
| 162 | 173 | ||
| 163 | /* | 174 | /* | ... | ... |
| ... | @@ -19,7 +19,6 @@ import static com.google.common.base.MoreObjects.toStringHelper; | ... | @@ -19,7 +19,6 @@ import static com.google.common.base.MoreObjects.toStringHelper; |
| 19 | 19 | ||
| 20 | import java.util.Objects; | 20 | import java.util.Objects; |
| 21 | 21 | ||
| 22 | -import org.onlab.packet.Ethernet; | ||
| 23 | import org.onlab.packet.MacAddress; | 22 | import org.onlab.packet.MacAddress; |
| 24 | import org.onlab.packet.VlanId; | 23 | import org.onlab.packet.VlanId; |
| 25 | 24 | ||
| ... | @@ -134,15 +133,15 @@ public abstract class L2ModificationInstruction implements Instruction { | ... | @@ -134,15 +133,15 @@ public abstract class L2ModificationInstruction implements Instruction { |
| 134 | L2ModificationInstruction { | 133 | L2ModificationInstruction { |
| 135 | 134 | ||
| 136 | private final L2SubType subtype; | 135 | private final L2SubType subtype; |
| 137 | - private final Ethernet ethernetType; | 136 | + private final short ethernetType; // uint16_t |
| 138 | 137 | ||
| 139 | - public PushHeaderInstructions(L2SubType subType, Ethernet ethernetType) { | 138 | + PushHeaderInstructions(L2SubType subType, short ethernetType) { |
| 140 | this.subtype = subType; | 139 | this.subtype = subType; |
| 141 | this.ethernetType = ethernetType; | 140 | this.ethernetType = ethernetType; |
| 142 | } | 141 | } |
| 143 | 142 | ||
| 144 | - public Ethernet ethernetType() { | 143 | + public int ethernetType() { |
| 145 | - return ethernetType; | 144 | + return Short.toUnsignedInt(ethernetType); |
| 146 | } | 145 | } |
| 147 | 146 | ||
| 148 | @Override | 147 | @Override |
| ... | @@ -152,12 +151,14 @@ public abstract class L2ModificationInstruction implements Instruction { | ... | @@ -152,12 +151,14 @@ public abstract class L2ModificationInstruction implements Instruction { |
| 152 | 151 | ||
| 153 | @Override | 152 | @Override |
| 154 | public String toString() { | 153 | public String toString() { |
| 155 | - return toStringHelper(subtype().toString()).toString(); | 154 | + return toStringHelper(subtype().toString()) |
| 155 | + .add("ethernetType", String.format("0x%04x", ethernetType())) | ||
| 156 | + .toString(); | ||
| 156 | } | 157 | } |
| 157 | 158 | ||
| 158 | @Override | 159 | @Override |
| 159 | public int hashCode() { | 160 | public int hashCode() { |
| 160 | - return Objects.hash(type(), subtype); | 161 | + return Objects.hash(type(), subtype, ethernetType); |
| 161 | } | 162 | } |
| 162 | 163 | ||
| 163 | @Override | 164 | @Override |
| ... | @@ -167,9 +168,8 @@ public abstract class L2ModificationInstruction implements Instruction { | ... | @@ -167,9 +168,8 @@ public abstract class L2ModificationInstruction implements Instruction { |
| 167 | } | 168 | } |
| 168 | if (obj instanceof PushHeaderInstructions) { | 169 | if (obj instanceof PushHeaderInstructions) { |
| 169 | PushHeaderInstructions that = (PushHeaderInstructions) obj; | 170 | PushHeaderInstructions that = (PushHeaderInstructions) obj; |
| 170 | - return Objects.equals(this.type(), that.type()) && | 171 | + return Objects.equals(subtype, that.subtype) && |
| 171 | - Objects.equals(subtype, that.subtype); | 172 | + Objects.equals(this.ethernetType, that.ethernetType); |
| 172 | - | ||
| 173 | } | 173 | } |
| 174 | return false; | 174 | return false; |
| 175 | } | 175 | } | ... | ... |
| ... | @@ -20,6 +20,7 @@ import java.util.List; | ... | @@ -20,6 +20,7 @@ import java.util.List; |
| 20 | import org.junit.Test; | 20 | import org.junit.Test; |
| 21 | import org.onosproject.net.PortNumber; | 21 | import org.onosproject.net.PortNumber; |
| 22 | import org.onosproject.net.flow.instructions.Instruction; | 22 | import org.onosproject.net.flow.instructions.Instruction; |
| 23 | +import org.onosproject.net.flow.instructions.Instructions; | ||
| 23 | import org.onlab.packet.IpAddress; | 24 | import org.onlab.packet.IpAddress; |
| 24 | import org.onlab.packet.MacAddress; | 25 | import org.onlab.packet.MacAddress; |
| 25 | import org.onlab.packet.VlanId; | 26 | import org.onlab.packet.VlanId; |
| ... | @@ -30,8 +31,6 @@ import static org.hamcrest.CoreMatchers.equalTo; | ... | @@ -30,8 +31,6 @@ import static org.hamcrest.CoreMatchers.equalTo; |
| 30 | import static org.hamcrest.MatcherAssert.assertThat; | 31 | import static org.hamcrest.MatcherAssert.assertThat; |
| 31 | import static org.hamcrest.Matchers.hasSize; | 32 | import static org.hamcrest.Matchers.hasSize; |
| 32 | import static org.hamcrest.Matchers.is; | 33 | import static org.hamcrest.Matchers.is; |
| 33 | -import static org.onosproject.net.flow.instructions.L0ModificationInstruction.L0SubType; | ||
| 34 | -import static org.onosproject.net.flow.instructions.L0ModificationInstruction.ModLambdaInstruction; | ||
| 35 | 34 | ||
| 36 | /** | 35 | /** |
| 37 | * Unit tests for the DefaultTrafficTreatment class. | 36 | * Unit tests for the DefaultTrafficTreatment class. |
| ... | @@ -48,7 +47,7 @@ public class DefaultTrafficTreatmentTest { | ... | @@ -48,7 +47,7 @@ public class DefaultTrafficTreatmentTest { |
| 48 | public void testTreatmentBuilderConstructors() { | 47 | public void testTreatmentBuilderConstructors() { |
| 49 | final TrafficTreatment treatment1 = | 48 | final TrafficTreatment treatment1 = |
| 50 | DefaultTrafficTreatment.builder() | 49 | DefaultTrafficTreatment.builder() |
| 51 | - .add(new ModLambdaInstruction(L0SubType.LAMBDA, (short) 4)) | 50 | + .add(Instructions.modL0Lambda((short) 4)) |
| 52 | .build(); | 51 | .build(); |
| 53 | final TrafficTreatment treatment2 = | 52 | final TrafficTreatment treatment2 = |
| 54 | DefaultTrafficTreatment.builder(treatment1).build(); | 53 | DefaultTrafficTreatment.builder(treatment1).build(); |
| ... | @@ -61,7 +60,7 @@ public class DefaultTrafficTreatmentTest { | ... | @@ -61,7 +60,7 @@ public class DefaultTrafficTreatmentTest { |
| 61 | @Test | 60 | @Test |
| 62 | public void testBuilderMethods() { | 61 | public void testBuilderMethods() { |
| 63 | final Instruction instruction1 = | 62 | final Instruction instruction1 = |
| 64 | - new ModLambdaInstruction(L0SubType.LAMBDA, (short) 4); | 63 | + Instructions.modL0Lambda((short) 4); |
| 65 | 64 | ||
| 66 | final TrafficTreatment.Builder builder1 = | 65 | final TrafficTreatment.Builder builder1 = |
| 67 | DefaultTrafficTreatment.builder() | 66 | DefaultTrafficTreatment.builder() |
| ... | @@ -95,15 +94,15 @@ public class DefaultTrafficTreatmentTest { | ... | @@ -95,15 +94,15 @@ public class DefaultTrafficTreatmentTest { |
| 95 | public void testEquals() { | 94 | public void testEquals() { |
| 96 | final TrafficTreatment treatment1 = | 95 | final TrafficTreatment treatment1 = |
| 97 | DefaultTrafficTreatment.builder() | 96 | DefaultTrafficTreatment.builder() |
| 98 | - .add(new ModLambdaInstruction(L0SubType.LAMBDA, (short) 4)) | 97 | + .add(Instructions.modL0Lambda((short) 4)) |
| 99 | .build(); | 98 | .build(); |
| 100 | final TrafficTreatment sameAsTreatment1 = | 99 | final TrafficTreatment sameAsTreatment1 = |
| 101 | DefaultTrafficTreatment.builder() | 100 | DefaultTrafficTreatment.builder() |
| 102 | - .add(new ModLambdaInstruction(L0SubType.LAMBDA, (short) 4)) | 101 | + .add(Instructions.modL0Lambda((short) 4)) |
| 103 | .build(); | 102 | .build(); |
| 104 | final TrafficTreatment treatment2 = | 103 | final TrafficTreatment treatment2 = |
| 105 | DefaultTrafficTreatment.builder() | 104 | DefaultTrafficTreatment.builder() |
| 106 | - .add(new ModLambdaInstruction(L0SubType.LAMBDA, (short) 2)) | 105 | + .add(Instructions.modL0Lambda((short) 2)) |
| 107 | .build(); | 106 | .build(); |
| 108 | new EqualsTester() | 107 | new EqualsTester() |
| 109 | .addEqualityGroup(treatment1, sameAsTreatment1) | 108 | .addEqualityGroup(treatment1, sameAsTreatment1) | ... | ... |
providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowModBuilderVer13.java
| ... | @@ -230,12 +230,12 @@ public class FlowModBuilderVer13 extends FlowModBuilder { | ... | @@ -230,12 +230,12 @@ public class FlowModBuilderVer13 extends FlowModBuilder { |
| 230 | PushHeaderInstructions pushHeaderInstructions = | 230 | PushHeaderInstructions pushHeaderInstructions = |
| 231 | (PushHeaderInstructions) l2m; | 231 | (PushHeaderInstructions) l2m; |
| 232 | return factory().actions().pushMpls(EthType.of(pushHeaderInstructions | 232 | return factory().actions().pushMpls(EthType.of(pushHeaderInstructions |
| 233 | - .ethernetType().getEtherType())); | 233 | + .ethernetType())); |
| 234 | case MPLS_POP: | 234 | case MPLS_POP: |
| 235 | PushHeaderInstructions popHeaderInstructions = | 235 | PushHeaderInstructions popHeaderInstructions = |
| 236 | (PushHeaderInstructions) l2m; | 236 | (PushHeaderInstructions) l2m; |
| 237 | return factory().actions().popMpls(EthType.of(popHeaderInstructions | 237 | return factory().actions().popMpls(EthType.of(popHeaderInstructions |
| 238 | - .ethernetType().getEtherType())); | 238 | + .ethernetType())); |
| 239 | case MPLS_LABEL: | 239 | case MPLS_LABEL: |
| 240 | ModMplsLabelInstruction mplsLabel = | 240 | ModMplsLabelInstruction mplsLabel = |
| 241 | (ModMplsLabelInstruction) l2m; | 241 | (ModMplsLabelInstruction) l2m; | ... | ... |
-
Please register or login to post a comment