Fixed bug in NetUtils.isMulticastMACAddr() caused by sign extension.

This patch fixes a bug that returned false incorrectly when the MSB was set
in the MAC address.

Change-Id: I8d04534962b0a1a585cf0217969a6415f360140b
Signed-off-by: Shigeru Yasuda <s-yasuda@da.jp.nec.com>

diff --git a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/NetUtils.java b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/NetUtils.java
index dc341625af642395872a3e2860a9d0fcc6f1ce3b..6b303f09f11955a053f7bcf740bf9f647b5df2ce 100644 (file)
--- a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/NetUtils.java
+++ b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/NetUtils.java
@@ -355,9 +355,7 @@ public abstract class NetUtils {
      */
     public static boolean isMulticastMACAddr(byte[] MACAddress) {
         if (MACAddress.length == MACAddrLengthInBytes && !isBroadcastMACAddr(MACAddress)) {
-            if (MACAddress[0] % 2 == 1) {
-                return true;
-            }
+            return (MACAddress[0] & 1) != 0;
         }
         return false;
     }

diff --git a/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/utils/NetUtilsTest.java b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/utils/NetUtilsTest.java
index a2b12782ac2986f24cfd6fbe8f295f5ef105712d..e5e0a0941b7db0ccb6b647b313096f64def5fa90 100644 (file)
--- a/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/utils/NetUtilsTest.java
+++ b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/utils/NetUtilsTest.java
@@ -468,4 +468,35 @@ public class NetUtilsTest {
         Assert.assertEquals(32768, NetUtils.getUnsignedShort((short) 0x8000));
         Assert.assertEquals(65535, NetUtils.getUnsignedShort((short) 0xffff));
     }
+
+    @Test
+    public void testMulticastMACAddr() {
+        byte[] empty = new byte[0];
+        Assert.assertFalse(NetUtils.isUnicastMACAddr(empty));
+        Assert.assertFalse(NetUtils.isMulticastMACAddr(empty));
+
+        byte[] bcast = {
+            (byte)0xff, (byte)0xff, (byte)0xff,
+            (byte)0xff, (byte)0xff, (byte)0xff,
+        };
+        Assert.assertFalse(NetUtils.isUnicastMACAddr(bcast));
+        Assert.assertFalse(NetUtils.isMulticastMACAddr(bcast));
+
+        byte[] firstOctet = {
+            (byte)0x00, (byte)0x20, (byte)0x80, (byte)0xfe,
+        };
+        for (int len = 1; len <= 10; len++) {
+            byte[] ba = new byte[len];
+            boolean valid = (len == 6);
+            for (byte first: firstOctet) {
+                ba[0] = first;
+                Assert.assertFalse(NetUtils.isMulticastMACAddr(ba));
+                Assert.assertEquals(valid, NetUtils.isUnicastMACAddr(ba));
+
+                ba[0] |= (byte)0x01;
+                Assert.assertEquals(valid, NetUtils.isMulticastMACAddr(ba));
+                Assert.assertFalse(NetUtils.isUnicastMACAddr(ba));
+            }
+        }
+    }
 }