[SDFAB-1147] Encode P4Runtime messages with canonical byte string

In ONOS, we still use non-canonical byte strings when sending P4Runtime
write requests.

Ref: https://p4.org/p4-spec/p4runtime/v1.3.0/P4Runtime-Spec.html#sec-bytestrings

Change-Id: I27d7977660bea462de82ebe2a4cb5d14500d3b69
(cherry picked from commit 3821399aee849c2a6a334fdb16bfa6b5f8f6dcc5)
diff --git a/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java b/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java
index b7e806b..1ee881b 100644
--- a/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java
+++ b/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java
@@ -185,7 +185,8 @@
 
         assertEquals(false, response.isSuccess());
         assertEquals(1, response.all().size());
-        assertEquals("Wrong size for param 'port' of action 'set_egress_port', expected 2 bytes, but found 13",
+        assertEquals("Wrong size for param 'port' of action 'set_egress_port', " +
+                        "expected no more than 2 bytes, but found 13",
                 response.all().iterator().next().explanation());
     }
 
@@ -244,7 +245,7 @@
             Action.Param param = action.getParamsList().get(0);
             assertEquals(1, param.getParamId());
             byte outPort = (byte) (member.getMemberId() - BASE_MEM_ID);
-            ByteString bs = ByteString.copyFrom(new byte[]{0, outPort});
+            ByteString bs = ByteString.copyFrom(new byte[]{outPort});
             assertEquals(bs, param.getValue());
         }
     }
diff --git a/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/codec/TableEntryEncoderTest.java b/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/codec/TableEntryEncoderTest.java
index 29f2936..a5ccecf 100644
--- a/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/codec/TableEntryEncoderTest.java
+++ b/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/codec/TableEntryEncoderTest.java
@@ -256,7 +256,7 @@
 
         // Ternary match.
         byte[] encodedTernaryMatchValue = tableEntryMsg.getMatch(0).getTernary().getValue().toByteArray();
-        assertThat(encodedTernaryMatchValue, is(ethAddr.asArray()));
+        assertThat(encodedTernaryMatchValue, is(ethAddr.canonical().asArray()));
 
         Action actionMsg = tableEntryMsg.getAction().getAction();
 
@@ -386,7 +386,7 @@
 
         // Ternary match.
         byte[] encodedTernaryMatchValue = tableEntryMsg.getMatch(0).getTernary().getValue().toByteArray();
-        assertThat(encodedTernaryMatchValue, is(ethAddr.asArray()));
+        assertThat(encodedTernaryMatchValue, is(ethAddr.canonical().asArray()));
 
         // no action
         assertThat(tableEntryMsg.hasAction(), is(false));
diff --git a/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/ActionCodec.java b/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/ActionCodec.java
index daf91f5..cbf112c 100644
--- a/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/ActionCodec.java
+++ b/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/ActionCodec.java
@@ -49,11 +49,14 @@
         for (PiActionParam p : piAction.parameters()) {
             final P4InfoOuterClass.Action.Param paramInfo = browser.actionParams(actionId)
                     .getByName(p.id().toString());
-            final ByteString paramValue = ByteString.copyFrom(p.value().asReadOnlyBuffer());
-            if (!browser.isTypeString(paramInfo.getTypeName())) {
+            final ByteString paramValue;
+            if (browser.isTypeString(paramInfo.getTypeName())) {
+                paramValue = ByteString.copyFrom(p.value().asReadOnlyBuffer());
+            } else {
+                paramValue = ByteString.copyFrom(p.value().canonical().asReadOnlyBuffer());
                 // Check size only if the param type is not a sdn_string
                 assertSize(format("param '%s' of action '%s'", p.id(), piAction.id()),
-                           paramValue, paramInfo.getBitwidth());
+                        paramValue, paramInfo.getBitwidth());
             }
             actionMsgBuilder.addParams(P4RuntimeOuterClass.Action.Param.newBuilder()
                                                .setParamId(paramInfo.getId())
diff --git a/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/FieldMatchCodec.java b/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/FieldMatchCodec.java
index 48baa17..4b8fe50 100644
--- a/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/FieldMatchCodec.java
+++ b/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/FieldMatchCodec.java
@@ -74,8 +74,11 @@
         switch (piFieldMatch.type()) {
             case EXACT:
                 PiExactFieldMatch fieldMatch = (PiExactFieldMatch) piFieldMatch;
-                ByteString exactValue = ByteString.copyFrom(fieldMatch.value().asReadOnlyBuffer());
-                if (!isSdnString) {
+                ByteString exactValue;
+                if (isSdnString) {
+                    exactValue = ByteString.copyFrom(fieldMatch.value().asReadOnlyBuffer());
+                } else {
+                    exactValue = ByteString.copyFrom(fieldMatch.value().canonical().asReadOnlyBuffer());
                     assertSize(VALUE_OF_PREFIX + entityName, exactValue, fieldBitwidth);
                 }
                 return messageBuilder.setExact(
@@ -86,8 +89,8 @@
                         .build();
             case TERNARY:
                 PiTernaryFieldMatch ternaryMatch = (PiTernaryFieldMatch) piFieldMatch;
-                ByteString ternaryValue = ByteString.copyFrom(ternaryMatch.value().asReadOnlyBuffer());
-                ByteString ternaryMask = ByteString.copyFrom(ternaryMatch.mask().asReadOnlyBuffer());
+                ByteString ternaryValue = ByteString.copyFrom(ternaryMatch.value().canonical().asReadOnlyBuffer());
+                ByteString ternaryMask = ByteString.copyFrom(ternaryMatch.mask().canonical().asReadOnlyBuffer());
                 if (isSdnString) {
                     sdnStringUnsupported(entityName, piFieldMatch.type());
                 }
@@ -102,7 +105,7 @@
                         .build();
             case LPM:
                 PiLpmFieldMatch lpmMatch = (PiLpmFieldMatch) piFieldMatch;
-                ByteString lpmValue = ByteString.copyFrom(lpmMatch.value().asReadOnlyBuffer());
+                ByteString lpmValue = ByteString.copyFrom(lpmMatch.value().canonical().asReadOnlyBuffer());
                 int lpmPrefixLen = lpmMatch.prefixLength();
                 if (isSdnString) {
                     sdnStringUnsupported(entityName, piFieldMatch.type());
@@ -117,8 +120,8 @@
                         .build();
             case RANGE:
                 PiRangeFieldMatch rangeMatch = (PiRangeFieldMatch) piFieldMatch;
-                ByteString rangeHighValue = ByteString.copyFrom(rangeMatch.highValue().asReadOnlyBuffer());
-                ByteString rangeLowValue = ByteString.copyFrom(rangeMatch.lowValue().asReadOnlyBuffer());
+                ByteString rangeHighValue = ByteString.copyFrom(rangeMatch.highValue().canonical().asReadOnlyBuffer());
+                ByteString rangeLowValue = ByteString.copyFrom(rangeMatch.lowValue().canonical().asReadOnlyBuffer());
                 if (isSdnString) {
                     sdnStringUnsupported(entityName, piFieldMatch.type());
                 }
@@ -132,8 +135,11 @@
                         .build();
             case OPTIONAL:
                 PiOptionalFieldMatch optionalMatch = (PiOptionalFieldMatch) piFieldMatch;
-                ByteString optionalValue = ByteString.copyFrom(optionalMatch.value().asReadOnlyBuffer());
-                if (!isSdnString) {
+                ByteString optionalValue;
+                if (isSdnString) {
+                    optionalValue = ByteString.copyFrom(optionalMatch.value().asReadOnlyBuffer());
+                } else {
+                    optionalValue = ByteString.copyFrom(optionalMatch.value().canonical().asReadOnlyBuffer());
                     assertSize(VALUE_OF_PREFIX + entityName, optionalValue, fieldBitwidth);
                 }
                 return messageBuilder.setOptional(
diff --git a/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/PacketMetadataCodec.java b/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/PacketMetadataCodec.java
index ab29e62..24aa00e 100644
--- a/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/PacketMetadataCodec.java
+++ b/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/PacketMetadataCodec.java
@@ -25,6 +25,8 @@
 import p4.config.v1.P4InfoOuterClass;
 import p4.v1.P4RuntimeOuterClass;
 
+import java.nio.ByteBuffer;
+
 import static org.onlab.util.ImmutableByteSequence.copyAndFit;
 import static org.onlab.util.ImmutableByteSequence.copyFrom;
 
@@ -41,12 +43,18 @@
             PiPacketMetadata piEntity, P4InfoOuterClass.Preamble ctrlPktMetaPreamble,
             PiPipeconf pipeconf, P4InfoBrowser browser)
             throws P4InfoBrowser.NotFoundException {
-        final int metadataId = browser
+        P4InfoOuterClass.ControllerPacketMetadata.Metadata packetMetadata = browser
                 .packetMetadatas(ctrlPktMetaPreamble.getId())
-                .getByName(piEntity.id().id()).getId();
+                .getByName(piEntity.id().id());
+        final ByteBuffer value;
+        if (browser.isTypeString(packetMetadata.getTypeName())) {
+            value = piEntity.value().asReadOnlyBuffer();
+        } else {
+            value = piEntity.value().canonical().asReadOnlyBuffer();
+        }
         return P4RuntimeOuterClass.PacketMetadata.newBuilder()
-                .setMetadataId(metadataId)
-                .setValue(ByteString.copyFrom(piEntity.value().asReadOnlyBuffer()))
+                .setMetadataId(packetMetadata.getId())
+                .setValue(ByteString.copyFrom(value))
                 .build();
     }
 
diff --git a/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/Utils.java b/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/Utils.java
index 2895b83..af1c159 100644
--- a/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/Utils.java
+++ b/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/Utils.java
@@ -34,9 +34,9 @@
             throws CodecException {
 
         int byteWidth = (int) Math.ceil((float) bitWidth / 8);
-        if (value.size() != byteWidth) {
+        if (value.size() > byteWidth) {
             throw new CodecException(format(
-                    "Wrong size for %s, expected %d bytes, but found %d",
+                    "Wrong size for %s, expected no more than %d bytes, but found %d",
                     entityDescr, byteWidth, value.size()));
         }
     }
diff --git a/utils/misc/src/main/java/org/onlab/util/ImmutableByteSequence.java b/utils/misc/src/main/java/org/onlab/util/ImmutableByteSequence.java
index 32e9d0b..41c6ba5 100644
--- a/utils/misc/src/main/java/org/onlab/util/ImmutableByteSequence.java
+++ b/utils/misc/src/main/java/org/onlab/util/ImmutableByteSequence.java
@@ -326,7 +326,7 @@
      * @return an integer value
      */
     public int size() {
-        return this.value.capacity();
+        return this.value.limit() - this.value.position();
     }
 
     /**
@@ -547,6 +547,22 @@
     }
 
     /**
+     * Returns a new ImmutableByteSequence with same content as this one, but with leading zero bytes stripped.
+     *
+     * @return new ImmutableByteSequence
+     */
+    public ImmutableByteSequence canonical() {
+        ByteBuffer newByteBuffer = this.value.duplicate();
+        ImmutableByteSequence canonicalBs = new ImmutableByteSequence(newByteBuffer);
+        canonicalBs.value.rewind();
+        while (canonicalBs.value.hasRemaining() && canonicalBs.value.get() == 0) {
+            // Make style check happy
+        }
+        canonicalBs.value.position(canonicalBs.value.position() - 1);
+        return canonicalBs;
+    }
+
+    /**
      * Signals a trim exception during byte sequence creation.
      */
     public static class ByteSequenceTrimException extends Exception {
diff --git a/utils/misc/src/test/java/org/onlab/util/ImmutableByteSequenceTest.java b/utils/misc/src/test/java/org/onlab/util/ImmutableByteSequenceTest.java
index c83c6b2..f9c2a0b 100644
--- a/utils/misc/src/test/java/org/onlab/util/ImmutableByteSequenceTest.java
+++ b/utils/misc/src/test/java/org/onlab/util/ImmutableByteSequenceTest.java
@@ -376,4 +376,52 @@
         assertThat("Invalid bitwise XOR result",
                    xorBs.asReadOnlyBuffer().getLong(), is(long1 ^ long2));
     }
+
+    @Test
+    public void testCanonical() {
+        ImmutableByteSequence bs = ImmutableByteSequence.copyFrom(0x000000ff);
+        ImmutableByteSequence canonicalBs = bs.canonical();
+        assertThat("Incorrect size", canonicalBs.size(), is(1));
+        ByteBuffer bb = canonicalBs.asReadOnlyBuffer();
+        assertThat("Incorrect byte buffer position", bb.position(), is(3));
+
+        bs = ImmutableByteSequence.copyFrom(0x100000ff);
+        canonicalBs = bs.canonical();
+        assertThat("Incorrect size", canonicalBs.size(), is(4));
+        bb = canonicalBs.asReadOnlyBuffer();
+        assertThat("Incorrect byte buffer position", bb.position(), is(0));
+
+        bs = ImmutableByteSequence.copyFrom(0x00000000ff0000ffL);
+        canonicalBs = bs.canonical();
+        assertThat("Incorrect size", canonicalBs.size(), is(4));
+        bb = canonicalBs.asReadOnlyBuffer();
+        assertThat("Incorrect byte buffer position", bb.position(), is(4));
+
+        bs = ImmutableByteSequence.copyFrom(0);
+        canonicalBs = bs.canonical();
+        assertThat("Incorrect size", canonicalBs.size(), is(1));
+        bb = canonicalBs.asReadOnlyBuffer();
+        assertThat("Incorrect byte buffer position", bb.position(), is(3));
+
+        bs = ImmutableByteSequence.copyFrom(0L);
+        canonicalBs = bs.canonical();
+        assertThat("Incorrect size", canonicalBs.size(), is(1));
+        bb = canonicalBs.asReadOnlyBuffer();
+        assertThat("Incorrect byte buffer position", bb.position(), is(7));
+
+        new EqualsTester()
+                .addEqualityGroup(
+                        ImmutableByteSequence.copyFrom(0x000000ff).canonical(),
+                        ImmutableByteSequence.copyFrom((short) 0x00ff).canonical())
+                .addEqualityGroup(
+                        ImmutableByteSequence.copyFrom(0x000001ff).canonical(),
+                        ImmutableByteSequence.copyFrom(0x00000000000001ffL).canonical())
+                .addEqualityGroup(
+                        ImmutableByteSequence.copyFrom(0xc00001ff).canonical(),
+                        ImmutableByteSequence.copyFrom(0x00000000c00001ffL).canonical())
+                .addEqualityGroup(
+                        ImmutableByteSequence.copyFrom(0).canonical(),
+                        ImmutableByteSequence.copyFrom(0L).canonical())
+                .testEquals();
+    }
 }