Bug 2280 - MatchConvertorImpl bugs found during test creation

- changed PbbIsid mask byte[] legnth to 3

Change-Id: I7f6eb71bc9a2ea03778835f8232f8f5057a93296
Signed-off-by: Martin Bobak <mbobak@cisco.com>

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl.java
index 3b73311e9a9b0b1ecd5450c55a36cd1bf3039f37..ef88ccfaf8fd4b990832365f678c70ddc8bbb538 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl.java
@@ -971,7 +971,7 @@ public class MatchConvertorImpl implements MatchConvertor<List<MatchEntries>> {
                     pbbBuilder.setPbbIsid(isidMatchEntry.getIsid());
                     MaskMatchEntry maskMatchEntry = ofMatch.getAugmentation(MaskMatchEntry.class);
                     if (maskMatchEntry != null) {
-                        pbbBuilder.setPbbMask(ByteUtil.bytesToUnsignedInt(maskMatchEntry.getMask()));
+                        pbbBuilder.setPbbMask(ByteUtil.bytesToUnsignedMedium(maskMatchEntry.getMask()));
                     }
                     protocolMatchFieldsBuilder.setPbb(pbbBuilder.build());
                     matchBuilder.setProtocolMatchFields(protocolMatchFieldsBuilder.build());
@@ -1011,7 +1011,7 @@ public class MatchConvertorImpl implements MatchConvertor<List<MatchEntries>> {
         matchEntriesBuilder.addAugmentation(IsidMatchEntry.class, isidBuilder.build());
         if (pbb.getPbbMask() != null) {
             hasmask = true;
-            addMaskAugmentation(matchEntriesBuilder, ByteUtil.unsignedIntToBytes(pbb.getPbbMask()));
+            addMaskAugmentation(matchEntriesBuilder, ByteUtil.unsignedMediumToBytes(pbb.getPbbMask()));
         }
         matchEntriesBuilder.setHasMask(hasmask);
         return matchEntriesBuilder.build();

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/util/ByteUtil.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/util/ByteUtil.java
index bf2631fc02666897dec12e087f7d34a10ca27b9f..671c081393b8d91f7262559dea3f166992927674 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/util/ByteUtil.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/util/ByteUtil.java
@@ -8,7 +8,6 @@
 package org.opendaylight.openflowplugin.openflow.md.util;
 
 import com.google.common.base.Preconditions;
-
 import java.math.BigInteger;
 import java.util.Arrays;
 
@@ -62,7 +61,7 @@ public abstract class ByteUtil {
      * @return a long representing the unsigned int
      */
     public static final long bytesToUnsignedInt(final byte[] bytes) {
-        Preconditions.checkArgument(bytes.length == 4, "Input byte array must be exactly two bytes long.");
+        Preconditions.checkArgument(bytes.length == 4, "Input byte array must be exactly four bytes long.");
         long unsignedInt = 0;
         unsignedInt |= bytes[0] & 0xFF;
         unsignedInt <<= 8;
@@ -74,6 +73,23 @@ public abstract class ByteUtil {
         return unsignedInt;
     }
 
+    /**
+     * Converts a 3 byte array of unsigned bytes to unsigned int
+     *
+     * @param bytes an array of 4 unsigned bytes
+     * @return a long representing the unsigned int
+     */
+    public static final long bytesToUnsignedMedium(final byte[] bytes) {
+        Preconditions.checkArgument(bytes.length == 3, "Input byte array must be exactly three bytes long.");
+        long unsignedMedium = 0;
+        unsignedMedium |= bytes[0] & 0xFF;
+        unsignedMedium <<= 8;
+        unsignedMedium |= bytes[1] & 0xFF;
+        unsignedMedium <<= 8;
+        unsignedMedium |= bytes[2] & 0xFF;
+        return unsignedMedium;
+    }
+
     /**
      * Converts a 2 byte array of unsigned bytes to unsigned short
      *
@@ -104,6 +120,20 @@ public abstract class ByteUtil {
         return bytes;
     }
 
+    /**
+     * Converts unsigned integer to a 3 byte array of unsigned bytes
+     *
+     * @param unsignedInt representing the unsigned integer
+     * @return bytes an array of 3 unsigned bytes
+     */
+    public static byte[] unsignedMediumToBytes(final Long unsignedInt) {
+        byte[] bytes = new byte[3];
+        bytes[2] = (byte) (unsignedInt & 0xFF);
+        bytes[1] = (byte) ((unsignedInt >> 8) & 0xFF);
+        bytes[0] = (byte) ((unsignedInt >> 16) & 0xFF);
+        return bytes;
+    }
+
     /**
      * Converts unsigned short to a 2 byte array of unsigned bytes
      *

diff --git a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl2Test.java b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl2Test.java
index d7692f0dd408b9523f427795db9986e98e1c6731..767c61a47639bfce4fd260ac7ad7a4989513dd7b 100644 (file)
--- a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl2Test.java
+++ b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl2Test.java
@@ -652,9 +652,8 @@ public class MatchConvertorImpl2Test {
         checkEntryHeader(entry, PbbIsid.class, true);\r
         Assert.assertEquals("Wrong pbb isid", 20, entry.getAugmentation(IsidMatchEntry.class)\r
                 .getIsid().intValue());\r
-        // FIXME - fix mask computation (length should be 3 instead of 4)\r
-//        Assert.assertArrayEquals("Wrong pbb isid mask", new byte[]{0, 0, 8},\r
-//                entry.getAugmentation(MaskMatchEntry.class).getMask());\r
+        Assert.assertArrayEquals("Wrong pbb isid mask", new byte[]{0, 0, 8},\r
+                entry.getAugmentation(MaskMatchEntry.class).getMask());\r
         entry = entries.get(7);\r
         checkEntryHeader(entry, TunnelId.class, true);\r
         Assert.assertArrayEquals("Wrong tunnel id", new byte[]{0, 0, 0, 0, 0, 0, 0, 21},\r

diff --git a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImplV13Test.java b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImplV13Test.java
index 65622c485fd79c082459c0c9699f476d674cd4ea..b62ed381c0d2ead42aad0cdc698a9b34e4cbd1ad 100644 (file)
--- a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImplV13Test.java
+++ b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImplV13Test.java
@@ -497,18 +497,17 @@ public class MatchConvertorImplV13Test {
         maskBuilder.setMask(new byte[]{(byte) 255, (byte) 255, (byte) 240, 0});\r
         entriesBuilder.addAugmentation(MaskMatchEntry.class, maskBuilder.build());\r
         entries.add(entriesBuilder.build());\r
-        // FIXME - PbbIsid should check for mask length of 3 instead of 2\r
-//        entriesBuilder = new MatchEntriesBuilder();\r
-//        entriesBuilder.setOxmClass(OpenflowBasicClass.class);\r
-//        entriesBuilder.setOxmMatchField(PbbIsid.class);\r
-//        entriesBuilder.setHasMask(true);\r
-//        IsidMatchEntryBuilder isidBuilder = new IsidMatchEntryBuilder();\r
-//        isidBuilder.setIsid(23L);\r
-//        entriesBuilder.addAugmentation(IsidMatchEntry.class, isidBuilder.build());\r
-//        maskBuilder = new MaskMatchEntryBuilder();\r
-//        maskBuilder.setMask(new byte[]{0, 0, 7});\r
-//        entriesBuilder.addAugmentation(MaskMatchEntry.class, maskBuilder.build());\r
-//        entries.add(entriesBuilder.build());\r
+        entriesBuilder = new MatchEntriesBuilder();\r
+        entriesBuilder.setOxmClass(OpenflowBasicClass.class);\r
+        entriesBuilder.setOxmMatchField(PbbIsid.class);\r
+        entriesBuilder.setHasMask(true);\r
+        IsidMatchEntryBuilder isidBuilder = new IsidMatchEntryBuilder();\r
+        isidBuilder.setIsid(23L);\r
+        entriesBuilder.addAugmentation(IsidMatchEntry.class, isidBuilder.build());\r
+        maskBuilder = new MaskMatchEntryBuilder();\r
+        maskBuilder.setMask(new byte[]{0, 0, 7});\r
+        entriesBuilder.addAugmentation(MaskMatchEntry.class, maskBuilder.build());\r
+        entries.add(entriesBuilder.build());\r
         entriesBuilder = new MatchEntriesBuilder();\r
         entriesBuilder.setOxmClass(OpenflowBasicClass.class);\r
         entriesBuilder.setOxmMatchField(TunnelId.class);\r

diff --git a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/util/ByteUtilTest.java b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/util/ByteUtilTest.java
index e0e3590d236ede3942ca894a48b9902041fc025e..8fdd1a25e8af4ea4dd8a906ff2956f811abb9e23 100644 (file)
--- a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/util/ByteUtilTest.java
+++ b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/util/ByteUtilTest.java
@@ -7,13 +7,12 @@
 
 package org.opendaylight.openflowplugin.openflow.md.util;
 
-import org.junit.Test;
-
-import java.math.BigInteger;
-
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.math.BigInteger;
+import org.junit.Test;
+
 /**
  * Created by Martin Bobak mbobak@cisco.com on 6/30/14.
  */
@@ -27,11 +26,19 @@ public class ByteUtilTest {
     private static final byte[] testBytes00 = {0, 0, 0, 0};
     private static final byte[] testBytesFF = {(byte) 255, (byte) 255, (byte) 255, (byte) 255};
 
+    private static final byte[] test3Bytes = {100, 101, 102};
+    private static final byte[] test3Bytes00 = {0, 0, 0};
+    private static final byte[] test3BytesFF = {(byte) 255, (byte) 255, (byte) 255};
+
     private static final BigInteger bigInteger = new BigInteger("1684367103");
     private static final BigInteger bigIntFF = new BigInteger("4294967295");
+
+    private static final Integer mediumInteger = new Integer("6579558");
+    private static final Integer mediumIntegerFF = new Integer("16777215");
     private static final int int00 = 0;
 
     private static final int shortByteLength = 2;
+    private static final int mediumByteLength = 3;
     private static final int intByteLength = 4;
 
     @Test
@@ -81,6 +88,18 @@ public class ByteUtilTest {
         assertEquals(bigIntFF.shiftRight(16).intValue(), unsigned);
     }
 
+    @Test
+    public void testBytesToUnsignedMedium() {
+        long unsigned = ByteUtil.bytesToUnsignedMedium(test3Bytes);
+        assertEquals(mediumInteger.longValue(), unsigned);
+
+        unsigned = ByteUtil.bytesToUnsignedMedium(test3Bytes00);
+        assertEquals(0, unsigned);
+
+        unsigned = ByteUtil.bytesToUnsignedMedium(test3BytesFF);
+        assertEquals(mediumIntegerFF.longValue(), unsigned);
+    }
+
     @Test(expected = IllegalArgumentException.class)
     public void exceptionTestBytesToUnsignedShort() {
         ByteUtil.bytesToUnsignedShort(testBytes);
@@ -123,4 +142,21 @@ public class ByteUtilTest {
         bytes = ByteUtil.unsignedShortToBytes(intValue);
         assertTrue(bytes.length == shortByteLength);
     }
+
+    @Test
+    public void testUnsignedMediumToBytes() {
+        long intValue = 255;
+        byte[] bytes = ByteUtil.unsignedMediumToBytes(intValue);
+
+        assertTrue(bytes.length == mediumByteLength);
+
+        intValue += 256;
+        bytes = ByteUtil.unsignedMediumToBytes(intValue);
+        assertTrue(bytes.length == mediumByteLength);
+
+        intValue += 256;
+        bytes = ByteUtil.unsignedMediumToBytes(intValue);
+        assertTrue(bytes.length == mediumByteLength);
+    }
+
 }