Fixed problem with incorrect subnet validation with container flow

Also added unit test cases to verify the validation

Change-Id: Ica257f6065d1d4180fccf0e878f188e71dbc21e4
Signed-off-by: Asad Ahmed <asaahmed@cisco.com>

diff --git a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/Match.java b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/Match.java
index 736314caedc2cb535a9e8160e3d056789e2d67c5..71e0087090334f12c18561e94f7c0fc5f5443126 100644 (file)
--- a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/Match.java
+++ b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/Match.java
@@ -293,10 +293,8 @@ public class Match implements Cloneable, Serializable {
                         && filterAddress instanceof Inet4Address) {
                     return true;
                 }
-                InetAddress thisMask = (InetAddress) filter.getField(type)
-                        .getMask();
-                InetAddress filterMask = (InetAddress) filter.getField(type)
-                        .getMask();
+                InetAddress thisMask = (InetAddress) thisField.getMask();
+                InetAddress filterMask = (InetAddress) filterField.getMask();
                 // thisAddress has to be in same subnet of filterAddress
                 if (NetUtils.inetAddressConflict(thisAddress, filterAddress,
                         thisMask, filterMask)) {

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 a15fecfdc6e056a606857ff05a1af350fbd074e2..691ddc93561bc0359858fa778b9e83d73b3cf2e3 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
@@ -219,23 +219,21 @@ public abstract class NetUtils {
         if (isAny(testAddress) || isAny(filterAddress)) {
             return false;
         }
-
-        // Derive the masks length. A null mask means a full mask
-        int testMaskLen = (testMask != null) ? NetUtils
-                .getSubnetMaskLength(testMask.getAddress())
-                : (testAddress instanceof Inet6Address) ? 128 : 32;
-        int filterMaskLen = (filterMask != null) ? NetUtils
-                .getSubnetMaskLength(filterMask.getAddress())
-                : (filterAddress instanceof Inet6Address) ? 128 : 32;
-
-        // Mask length check. Test mask has to be more generic than filter one
-        if (testMaskLen < filterMaskLen) {
+        
+        int testMaskLen = (testMask != null) ? NetUtils.getSubnetMaskLength(testMask.getAddress()) : 0;
+        int filterMaskLen = (filterMask != null) ? NetUtils.getSubnetMaskLength(filterMask.getAddress()) : 0;
+        
+        int testPrefixLen = (testAddress instanceof Inet6Address) ? (128 - testMaskLen) : (32 - testMaskLen);
+        int filterPrefixLen = (filterAddress instanceof Inet6Address) ? (128 - filterMaskLen) : (32 - filterMaskLen);
+        
+        // Mask length check. Test mask has to be more specific than filter one
+        if (testPrefixLen < filterPrefixLen) {
             return true;
         }
 
         // Subnet Prefix on filter mask length must be the same
-        InetAddress prefix1 = getSubnetPrefix(testAddress, filterMaskLen);
-        InetAddress prefix2 = getSubnetPrefix(filterAddress, filterMaskLen);
+        InetAddress prefix1 = getSubnetPrefix(testAddress, filterPrefixLen);
+        InetAddress prefix2 = getSubnetPrefix(filterAddress, filterPrefixLen);
         return (!prefix1.equals(prefix2));
     }
 

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 77d1fe216413d38477eaffabdc5f8f1dea9bfea4..2d16afbd3d8fd6161922c092c212ae84722f9920 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
@@ -15,7 +15,6 @@ import java.util.Arrays;
 
 import org.junit.Assert;
 import org.junit.Test;
-import org.opendaylight.controller.sal.utils.NetUtils;
 
 public class NetUtilsTest {
 
@@ -274,4 +273,49 @@ public class NetUtilsTest {
                 .isIPv6AddressValid("fe80:::0:0:0:204:61ff:fe9d/-1")); //not valid both
 
     }
+    
+    @Test
+    public void testInetAddressConflict() throws UnknownHostException {
+
+        // test a ipv4 testAddress in the same subnet as the filter
+        // the method should return false as there is no conflict
+        Assert.assertFalse(NetUtils.inetAddressConflict(
+                InetAddress.getByName("9.9.1.1"),
+                InetAddress.getByName("9.9.1.0"), null,
+                InetAddress.getByName("255.255.255.0")));
+
+        // test a ipv4 testAddress not in the same subnet as the filter
+        // the method should return true as there is a conflict
+        Assert.assertTrue(NetUtils.inetAddressConflict(
+                InetAddress.getByName("9.9.2.1"),
+                InetAddress.getByName("9.9.1.0"), null,
+                InetAddress.getByName("255.255.255.0")));
+
+        // test a ipv4 testAddress more generic than the filter
+        // the method should return true as there is a conflict
+        Assert.assertTrue(NetUtils.inetAddressConflict(
+                InetAddress.getByName("9.9.1.1"),
+                InetAddress.getByName("9.9.1.0"),
+                InetAddress.getByName("255.255.0.0"),
+                InetAddress.getByName("255.255.255.0")));
+
+        // test a ipv4 testAddress less generic than the filter and in the same
+        // subnet as the filter
+        // the method should return false as there is no conflict
+        Assert.assertFalse(NetUtils.inetAddressConflict(
+                InetAddress.getByName("9.9.1.0"),
+                InetAddress.getByName("9.9.0.0"),
+                InetAddress.getByName("255.255.255.0"),
+                InetAddress.getByName("255.255.0.0")));
+
+        // test a ipv4 testAddress less generic than the filter and not in the
+        // same subnet as the filter
+        // the method should return true as there is a conflict
+        Assert.assertTrue(NetUtils.inetAddressConflict(
+                InetAddress.getByName("9.8.1.0"),
+                InetAddress.getByName("9.9.0.0"),
+                InetAddress.getByName("255.255.255.0"),
+                InetAddress.getByName("255.255.0.0")));
+
+    }
 }