From: Asad Ahmed Date: Mon, 6 May 2013 17:47:04 +0000 (-0700) Subject: Fixed problem with incorrect subnet validation with container flow X-Git-Tag: releasepom-0.1.0~486 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=d0b8fb866390a5ce2c2ceac424bc9e5307b74ab0 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 --- 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 736314caed..71e0087090 100644 --- 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 a15fecfdc6..691ddc9356 100644 --- 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 77d1fe2164..2d16afbd3d 100644 --- 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"))); + + } }