Fix IpConversionUtil.extractIpv4AddressMask()

The algorithm here is wrong when prefix length is 0, as it
returns 255.255.255.255, where it should be returning 0.0.0.0.
This turns out to be an error coming from the fact we use a long
bitmask and incorrect masks to mask out the topmost long bits.

Use an int bitmask and switch to non-sign-extending shifts, which
fixes the problem.

JIRA: OPNFLWPLUG-1063
Change-Id: Idd42ba510a3a9d2032646f07b0bde8061ecd2637
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/util/AddressNormalizationUtilTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/util/AddressNormalizationUtilTest.java
index 98e23ed0af90392f23a6ccf93116216fcfbb47c3..dde1522fec2dd98c8573a8a39e5616c9c2c8a90e 100644 (file)
--- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/util/AddressNormalizationUtilTest.java
+++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/util/AddressNormalizationUtilTest.java
@@ -81,13 +81,15 @@ public class AddressNormalizationUtilTest {
 
     @Test
     public void normalizeIpv4Prefix() {
-        final Ipv4Prefix left = new Ipv4Prefix("192.168.72.1/16");
-        final Ipv4Prefix right = new Ipv4Prefix("192.168.0.0/16");
+        assertNormalizedIpv4Prefix("192.168.72.1/32", "192.168.72.1/32");
+        assertNormalizedIpv4Prefix("192.168.0.0/16", "192.168.72.1/16");
+        assertNormalizedIpv4Prefix("192.0.0.0/2", "192.168.72.1/2");
+        assertNormalizedIpv4Prefix("128.0.0.0/1", "192.168.72.1/1");
+        assertNormalizedIpv4Prefix("0.0.0.0/0", "192.168.72.1/0");
+    }
 
-        assertEquals(
-                right,
-                AddressNormalizationUtil.normalizeIpv4Prefix(left)
-        );
+    private static void assertNormalizedIpv4Prefix(final String expected, final String input) {
+        assertEquals(new Ipv4Prefix(expected), AddressNormalizationUtil.normalizeIpv4Prefix(new Ipv4Prefix(input)));
     }
 
     @Test

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/common/IpConversionUtil.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/common/IpConversionUtil.java
index 44ee4cc59019603ca7a6ff0ca9026964b221ec91..224280760bce21387e4f41529191ae4f0db717f6 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/common/IpConversionUtil.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/common/IpConversionUtil.java
@@ -78,11 +78,11 @@ public final class IpConversionUtil {
         final DottedQuad[] quads = new DottedQuad[IPV4_ADDRESS_LENGTH + 1];
 
         for (int i = 0; i <= IPV4_ADDRESS_LENGTH; ++i) {
-            final long maskBits = 0xffffffff << IPV4_ADDRESS_LENGTH - i;
+            final int maskBits = (int) (0xffffffffL << IPV4_ADDRESS_LENGTH - i);
             quads[i] = new DottedQuad(new StringBuilder(15)
-                .append((maskBits & 0x0000000000ff000000L) >> 24).append('.')
-                .append((maskBits & 0x0000000000ff0000) >> 16).append('.')
-                .append((maskBits & 0x0000000000ff00) >> 8).append('.')
+                .append(maskBits >>> 24).append('.')
+                .append(maskBits >>> 16 & 0xff).append('.')
+                .append(maskBits >>>  8 & 0xff).append('.')
                 .append(maskBits & 0xff).toString());
         }
 

diff --git a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/common/IpConversionUtilTest.java b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/common/IpConversionUtilTest.java
index 71211f05d992dbf51acd36dd70e8b58f98ae8e2d..d37119da4ce098bda0f211b181397b3c09503f72 100644 (file)
--- a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/common/IpConversionUtilTest.java
+++ b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/common/IpConversionUtilTest.java
@@ -125,6 +125,7 @@ public class IpConversionUtilTest {
 
     @Test
     public void extractIpv4AddressMaskTest() {
+        assertAddressMask("0.0.0.0", 0);
         assertAddressMask("128.0.0.0", 1);
         assertAddressMask("255.0.0.0", 8);
         assertAddressMask("255.128.0.0", 9);