Committed by
Gerrit Code Review
More readable toString for BMv2 extension selectors and treatments
Also, added a test for serialization Change-Id: I77e80fa7597b552c71e80c9d39d03549e0325778
Showing
4 changed files
with
104 additions
and
20 deletions
... | @@ -39,6 +39,11 @@ | ... | @@ -39,6 +39,11 @@ |
39 | <groupId>org.apache.felix</groupId> | 39 | <groupId>org.apache.felix</groupId> |
40 | <artifactId>org.apache.felix.scr.annotations</artifactId> | 40 | <artifactId>org.apache.felix.scr.annotations</artifactId> |
41 | </dependency> | 41 | </dependency> |
42 | + <dependency> | ||
43 | + <groupId>org.onosproject</groupId> | ||
44 | + <artifactId>onos-core-serializers</artifactId> | ||
45 | + <version>${project.version}</version> | ||
46 | + </dependency> | ||
42 | </dependencies> | 47 | </dependencies> |
43 | 48 | ||
44 | </project> | 49 | </project> |
... | \ No newline at end of file | ... | \ No newline at end of file | ... | ... |
... | @@ -29,9 +29,10 @@ import org.onosproject.bmv2.api.utils.Bmv2TranslatorUtils; | ... | @@ -29,9 +29,10 @@ import org.onosproject.bmv2.api.utils.Bmv2TranslatorUtils; |
29 | import org.onosproject.net.flow.AbstractExtension; | 29 | import org.onosproject.net.flow.AbstractExtension; |
30 | import org.onosproject.net.flow.criteria.ExtensionSelector; | 30 | import org.onosproject.net.flow.criteria.ExtensionSelector; |
31 | import org.onosproject.net.flow.criteria.ExtensionSelectorType; | 31 | import org.onosproject.net.flow.criteria.ExtensionSelectorType; |
32 | +import org.onosproject.store.serializers.KryoNamespaces; | ||
32 | 33 | ||
33 | import java.nio.ByteBuffer; | 34 | import java.nio.ByteBuffer; |
34 | -import java.util.HashMap; | 35 | +import java.util.Collections; |
35 | import java.util.Map; | 36 | import java.util.Map; |
36 | 37 | ||
37 | import static com.google.common.base.Preconditions.*; | 38 | import static com.google.common.base.Preconditions.*; |
... | @@ -46,9 +47,8 @@ import static org.onosproject.bmv2.api.utils.Bmv2TranslatorUtils.fitByteSequence | ... | @@ -46,9 +47,8 @@ import static org.onosproject.bmv2.api.utils.Bmv2TranslatorUtils.fitByteSequence |
46 | @Beta | 47 | @Beta |
47 | public final class Bmv2ExtensionSelector extends AbstractExtension implements ExtensionSelector { | 48 | public final class Bmv2ExtensionSelector extends AbstractExtension implements ExtensionSelector { |
48 | 49 | ||
49 | - private final KryoNamespace appKryo = new KryoNamespace.Builder() | 50 | + private static final KryoNamespace APP_KRYO = new KryoNamespace.Builder() |
50 | - .register(HashMap.class) | 51 | + .register(KryoNamespaces.API) |
51 | - .register(Bmv2MatchParam.class) | ||
52 | .register(Bmv2ExactMatchParam.class) | 52 | .register(Bmv2ExactMatchParam.class) |
53 | .register(Bmv2TernaryMatchParam.class) | 53 | .register(Bmv2TernaryMatchParam.class) |
54 | .register(Bmv2LpmMatchParam.class) | 54 | .register(Bmv2LpmMatchParam.class) |
... | @@ -83,12 +83,12 @@ public final class Bmv2ExtensionSelector extends AbstractExtension implements Ex | ... | @@ -83,12 +83,12 @@ public final class Bmv2ExtensionSelector extends AbstractExtension implements Ex |
83 | 83 | ||
84 | @Override | 84 | @Override |
85 | public byte[] serialize() { | 85 | public byte[] serialize() { |
86 | - return appKryo.serialize(parameterMap); | 86 | + return APP_KRYO.serialize(parameterMap); |
87 | } | 87 | } |
88 | 88 | ||
89 | @Override | 89 | @Override |
90 | public void deserialize(byte[] data) { | 90 | public void deserialize(byte[] data) { |
91 | - this.parameterMap = appKryo.deserialize(data); | 91 | + this.parameterMap = APP_KRYO.deserialize(data); |
92 | } | 92 | } |
93 | 93 | ||
94 | @Override | 94 | @Override |
... | @@ -110,9 +110,31 @@ public final class Bmv2ExtensionSelector extends AbstractExtension implements Ex | ... | @@ -110,9 +110,31 @@ public final class Bmv2ExtensionSelector extends AbstractExtension implements Ex |
110 | 110 | ||
111 | @Override | 111 | @Override |
112 | public String toString() { | 112 | public String toString() { |
113 | - return MoreObjects.toStringHelper(this) | 113 | + MoreObjects.ToStringHelper helper = MoreObjects.toStringHelper(this); |
114 | - .add("parameterMap", parameterMap) | 114 | + parameterMap.forEach((name, param) -> { |
115 | - .toString(); | 115 | + switch (param.type()) { |
116 | + case EXACT: | ||
117 | + Bmv2ExactMatchParam e = (Bmv2ExactMatchParam) param; | ||
118 | + helper.add(name, e.value()); | ||
119 | + break; | ||
120 | + case TERNARY: | ||
121 | + Bmv2TernaryMatchParam t = (Bmv2TernaryMatchParam) param; | ||
122 | + helper.add(name, t.value() + "&&&" + t.mask()); | ||
123 | + break; | ||
124 | + case LPM: | ||
125 | + Bmv2LpmMatchParam l = (Bmv2LpmMatchParam) param; | ||
126 | + helper.add(name, l.value() + "/" + String.valueOf(l.prefixLength())); | ||
127 | + break; | ||
128 | + case VALID: | ||
129 | + Bmv2ValidMatchParam v = (Bmv2ValidMatchParam) param; | ||
130 | + helper.add(name, v.flag() ? "VALID" : "NOT_VALID"); | ||
131 | + break; | ||
132 | + default: | ||
133 | + helper.add(name, param); | ||
134 | + break; | ||
135 | + } | ||
136 | + }); | ||
137 | + return helper.toString(); | ||
116 | } | 138 | } |
117 | 139 | ||
118 | /** | 140 | /** |
... | @@ -121,7 +143,7 @@ public final class Bmv2ExtensionSelector extends AbstractExtension implements Ex | ... | @@ -121,7 +143,7 @@ public final class Bmv2ExtensionSelector extends AbstractExtension implements Ex |
121 | * @return a BMv2 extension treatment | 143 | * @return a BMv2 extension treatment |
122 | */ | 144 | */ |
123 | public static Bmv2ExtensionSelector empty() { | 145 | public static Bmv2ExtensionSelector empty() { |
124 | - return new Bmv2ExtensionSelector(null); | 146 | + return new Bmv2ExtensionSelector(Collections.emptyMap()); |
125 | } | 147 | } |
126 | 148 | ||
127 | /** | 149 | /** | ... | ... |
... | @@ -29,11 +29,14 @@ import org.onosproject.bmv2.api.utils.Bmv2TranslatorUtils; | ... | @@ -29,11 +29,14 @@ import org.onosproject.bmv2.api.utils.Bmv2TranslatorUtils; |
29 | import org.onosproject.net.flow.AbstractExtension; | 29 | import org.onosproject.net.flow.AbstractExtension; |
30 | import org.onosproject.net.flow.instructions.ExtensionTreatment; | 30 | import org.onosproject.net.flow.instructions.ExtensionTreatment; |
31 | import org.onosproject.net.flow.instructions.ExtensionTreatmentType; | 31 | import org.onosproject.net.flow.instructions.ExtensionTreatmentType; |
32 | +import org.onosproject.store.serializers.KryoNamespaces; | ||
32 | 33 | ||
33 | import java.nio.ByteBuffer; | 34 | import java.nio.ByteBuffer; |
34 | import java.util.ArrayList; | 35 | import java.util.ArrayList; |
36 | +import java.util.Collections; | ||
35 | import java.util.List; | 37 | import java.util.List; |
36 | import java.util.Map; | 38 | import java.util.Map; |
39 | +import java.util.StringJoiner; | ||
37 | 40 | ||
38 | import static com.google.common.base.Preconditions.checkArgument; | 41 | import static com.google.common.base.Preconditions.checkArgument; |
39 | import static com.google.common.base.Preconditions.checkNotNull; | 42 | import static com.google.common.base.Preconditions.checkNotNull; |
... | @@ -47,16 +50,25 @@ import static org.onosproject.net.flow.instructions.ExtensionTreatmentType.Exten | ... | @@ -47,16 +50,25 @@ import static org.onosproject.net.flow.instructions.ExtensionTreatmentType.Exten |
47 | @Beta | 50 | @Beta |
48 | public final class Bmv2ExtensionTreatment extends AbstractExtension implements ExtensionTreatment { | 51 | public final class Bmv2ExtensionTreatment extends AbstractExtension implements ExtensionTreatment { |
49 | 52 | ||
50 | - private final KryoNamespace appKryo = new KryoNamespace.Builder().build(); | 53 | + private static final KryoNamespace APP_KRYO = new KryoNamespace.Builder() |
54 | + .register(KryoNamespaces.API) | ||
55 | + .register(Bmv2ExtensionTreatment.class) | ||
56 | + .register(Bmv2Action.class) | ||
57 | + .build(); | ||
58 | + | ||
59 | + private List<String> parameterNames; | ||
51 | private Bmv2Action action; | 60 | private Bmv2Action action; |
52 | 61 | ||
53 | /** | 62 | /** |
54 | * Creates a new extension treatment for the given BMv2 action. | 63 | * Creates a new extension treatment for the given BMv2 action. |
64 | + * The list of action parameters name is also required for visualization purposes (i.e. nicer toString()). | ||
55 | * | 65 | * |
56 | * @param action an action | 66 | * @param action an action |
67 | + * @param parameterNames a list of strings | ||
57 | */ | 68 | */ |
58 | - private Bmv2ExtensionTreatment(Bmv2Action action) { | 69 | + private Bmv2ExtensionTreatment(Bmv2Action action, List<String> parameterNames) { |
59 | this.action = action; | 70 | this.action = action; |
71 | + this.parameterNames = parameterNames; | ||
60 | } | 72 | } |
61 | 73 | ||
62 | /** | 74 | /** |
... | @@ -75,12 +87,14 @@ public final class Bmv2ExtensionTreatment extends AbstractExtension implements E | ... | @@ -75,12 +87,14 @@ public final class Bmv2ExtensionTreatment extends AbstractExtension implements E |
75 | 87 | ||
76 | @Override | 88 | @Override |
77 | public byte[] serialize() { | 89 | public byte[] serialize() { |
78 | - return appKryo.serialize(action); | 90 | + return APP_KRYO.serialize(this); |
79 | } | 91 | } |
80 | 92 | ||
81 | @Override | 93 | @Override |
82 | public void deserialize(byte[] data) { | 94 | public void deserialize(byte[] data) { |
83 | - action = appKryo.deserialize(data); | 95 | + Bmv2ExtensionTreatment other = APP_KRYO.deserialize(data); |
96 | + action = other.action; | ||
97 | + parameterNames = other.parameterNames; | ||
84 | } | 98 | } |
85 | 99 | ||
86 | @Override | 100 | @Override |
... | @@ -102,8 +116,12 @@ public final class Bmv2ExtensionTreatment extends AbstractExtension implements E | ... | @@ -102,8 +116,12 @@ public final class Bmv2ExtensionTreatment extends AbstractExtension implements E |
102 | 116 | ||
103 | @Override | 117 | @Override |
104 | public String toString() { | 118 | public String toString() { |
119 | + StringJoiner stringJoiner = new StringJoiner(", ", "(", ")"); | ||
120 | + for (int i = 0; i < parameterNames.size(); i++) { | ||
121 | + stringJoiner.add(parameterNames.get(i) + "=" + action.parameters().get(i)); | ||
122 | + } | ||
105 | return MoreObjects.toStringHelper(this) | 123 | return MoreObjects.toStringHelper(this) |
106 | - .add("action", action) | 124 | + .addValue(action.name() + stringJoiner.toString()) |
107 | .toString(); | 125 | .toString(); |
108 | } | 126 | } |
109 | 127 | ||
... | @@ -113,7 +131,7 @@ public final class Bmv2ExtensionTreatment extends AbstractExtension implements E | ... | @@ -113,7 +131,7 @@ public final class Bmv2ExtensionTreatment extends AbstractExtension implements E |
113 | * @return a BMv2 extension treatment | 131 | * @return a BMv2 extension treatment |
114 | */ | 132 | */ |
115 | public static Bmv2ExtensionTreatment empty() { | 133 | public static Bmv2ExtensionTreatment empty() { |
116 | - return new Bmv2ExtensionTreatment(null); | 134 | + return new Bmv2ExtensionTreatment(null, Collections.emptyList()); |
117 | } | 135 | } |
118 | 136 | ||
119 | /** | 137 | /** |
... | @@ -232,6 +250,7 @@ public final class Bmv2ExtensionTreatment extends AbstractExtension implements E | ... | @@ -232,6 +250,7 @@ public final class Bmv2ExtensionTreatment extends AbstractExtension implements E |
232 | "invalid number of parameters", actionName); | 250 | "invalid number of parameters", actionName); |
233 | 251 | ||
234 | List<ImmutableByteSequence> newParameters = new ArrayList<>(parameters.size()); | 252 | List<ImmutableByteSequence> newParameters = new ArrayList<>(parameters.size()); |
253 | + List<String> parameterNames = new ArrayList<>(parameters.size()); | ||
235 | 254 | ||
236 | for (String parameterName : parameters.keySet()) { | 255 | for (String parameterName : parameters.keySet()) { |
237 | Bmv2RuntimeDataModel runtimeData = actionModel.runtimeData(parameterName); | 256 | Bmv2RuntimeDataModel runtimeData = actionModel.runtimeData(parameterName); |
... | @@ -242,13 +261,14 @@ public final class Bmv2ExtensionTreatment extends AbstractExtension implements E | ... | @@ -242,13 +261,14 @@ public final class Bmv2ExtensionTreatment extends AbstractExtension implements E |
242 | ImmutableByteSequence newSequence = fitByteSequence(parameters.get(parameterName), bitWidth); | 261 | ImmutableByteSequence newSequence = fitByteSequence(parameters.get(parameterName), bitWidth); |
243 | int idx = actionModel.runtimeDatas().indexOf(runtimeData); | 262 | int idx = actionModel.runtimeDatas().indexOf(runtimeData); |
244 | newParameters.add(idx, newSequence); | 263 | newParameters.add(idx, newSequence); |
264 | + parameterNames.add(idx, parameterName); | ||
245 | } catch (Bmv2TranslatorUtils.ByteSequenceFitException e) { | 265 | } catch (Bmv2TranslatorUtils.ByteSequenceFitException e) { |
246 | throw new IllegalArgumentException(e.getMessage() + | 266 | throw new IllegalArgumentException(e.getMessage() + |
247 | " [" + actionName + "->" + runtimeData.name() + "]"); | 267 | " [" + actionName + "->" + runtimeData.name() + "]"); |
248 | } | 268 | } |
249 | } | 269 | } |
250 | 270 | ||
251 | - return new Bmv2ExtensionTreatment(new Bmv2Action(actionName, newParameters)); | 271 | + return new Bmv2ExtensionTreatment(new Bmv2Action(actionName, newParameters), parameterNames); |
252 | } | 272 | } |
253 | 273 | ||
254 | 274 | ... | ... |
... | @@ -18,6 +18,7 @@ package org.onosproject.bmv2.api.runtime; | ... | @@ -18,6 +18,7 @@ package org.onosproject.bmv2.api.runtime; |
18 | 18 | ||
19 | import com.eclipsesource.json.Json; | 19 | import com.eclipsesource.json.Json; |
20 | import com.eclipsesource.json.JsonObject; | 20 | import com.eclipsesource.json.JsonObject; |
21 | +import com.google.common.testing.EqualsTester; | ||
21 | import org.junit.Before; | 22 | import org.junit.Before; |
22 | import org.junit.Test; | 23 | import org.junit.Test; |
23 | import org.onlab.packet.MacAddress; | 24 | import org.onlab.packet.MacAddress; |
... | @@ -30,7 +31,7 @@ import java.io.InputStreamReader; | ... | @@ -30,7 +31,7 @@ import java.io.InputStreamReader; |
30 | import static org.hamcrest.MatcherAssert.assertThat; | 31 | import static org.hamcrest.MatcherAssert.assertThat; |
31 | import static org.hamcrest.Matchers.is; | 32 | import static org.hamcrest.Matchers.is; |
32 | 33 | ||
33 | -public class Bmv2ExtensionBuilderTest { | 34 | +public class Bmv2ExtensionsTest { |
34 | 35 | ||
35 | private Bmv2Configuration config; | 36 | private Bmv2Configuration config; |
36 | 37 | ||
... | @@ -42,7 +43,7 @@ public class Bmv2ExtensionBuilderTest { | ... | @@ -42,7 +43,7 @@ public class Bmv2ExtensionBuilderTest { |
42 | } | 43 | } |
43 | 44 | ||
44 | @Test | 45 | @Test |
45 | - public void testExtensionSelector() throws Exception { | 46 | + public void testExtensionSelectorBuilder() throws Exception { |
46 | 47 | ||
47 | Bmv2ExtensionSelector extSelectorExact = Bmv2ExtensionSelector.builder() | 48 | Bmv2ExtensionSelector extSelectorExact = Bmv2ExtensionSelector.builder() |
48 | .forConfiguration(config) | 49 | .forConfiguration(config) |
... | @@ -85,7 +86,7 @@ public class Bmv2ExtensionBuilderTest { | ... | @@ -85,7 +86,7 @@ public class Bmv2ExtensionBuilderTest { |
85 | } | 86 | } |
86 | 87 | ||
87 | @Test | 88 | @Test |
88 | - public void testExtensionTreatment() throws Exception { | 89 | + public void testExtensionTreatmentBuilder() throws Exception { |
89 | 90 | ||
90 | Bmv2ExtensionTreatment treatment = Bmv2ExtensionTreatment.builder() | 91 | Bmv2ExtensionTreatment treatment = Bmv2ExtensionTreatment.builder() |
91 | .forConfiguration(config) | 92 | .forConfiguration(config) |
... | @@ -97,4 +98,40 @@ public class Bmv2ExtensionBuilderTest { | ... | @@ -97,4 +98,40 @@ public class Bmv2ExtensionBuilderTest { |
97 | 98 | ||
98 | // TODO add more tests, e.g. check for byte sequences content and size. | 99 | // TODO add more tests, e.g. check for byte sequences content and size. |
99 | } | 100 | } |
101 | + | ||
102 | + @Test | ||
103 | + public void testExtensionSelectorSerialization() throws Exception { | ||
104 | + | ||
105 | + Bmv2ExtensionSelector original = Bmv2ExtensionSelector.builder() | ||
106 | + .forConfiguration(config) | ||
107 | + .matchExact("standard_metadata", "ingress_port", (short) 255) | ||
108 | + .matchLpm("ethernet", "etherType", 512, 4) | ||
109 | + .matchTernary("ethernet", "dstAddr", 1024L, 512L) | ||
110 | + .matchValid("ethernet", "srcAddr", true) | ||
111 | + .build(); | ||
112 | + | ||
113 | + Bmv2ExtensionSelector other = Bmv2ExtensionSelector.empty(); | ||
114 | + other.deserialize(original.serialize()); | ||
115 | + | ||
116 | + new EqualsTester() | ||
117 | + .addEqualityGroup(original, other) | ||
118 | + .testEquals(); | ||
119 | + } | ||
120 | + | ||
121 | + @Test | ||
122 | + public void testExtensionTreatmentSerialization() throws Exception { | ||
123 | + | ||
124 | + Bmv2ExtensionTreatment original = Bmv2ExtensionTreatment.builder() | ||
125 | + .forConfiguration(config) | ||
126 | + .setActionName("set_egress_port") | ||
127 | + .addParameter("port", 1) | ||
128 | + .build(); | ||
129 | + | ||
130 | + Bmv2ExtensionTreatment other = Bmv2ExtensionTreatment.empty(); | ||
131 | + other.deserialize(original.serialize()); | ||
132 | + | ||
133 | + new EqualsTester() | ||
134 | + .addEqualityGroup(original, other) | ||
135 | + .testEquals(); | ||
136 | + } | ||
100 | } | 137 | } |
... | \ No newline at end of file | ... | \ No newline at end of file | ... | ... |
-
Please register or login to post a comment