From 7d9d8c05414697ce27e159a9f64c57d12dab4b2a Mon Sep 17 00:00:00 2001 From: Lorand Jakab Date: Tue, 20 Jun 2017 11:53:15 +0300 Subject: [PATCH] Bug 8679: Fix widest negative prefix calculation And add integration tests to avoid regressions. Change-Id: I6cadaca03d360f475c7c88fd3cb58dd39814d23f Signed-off-by: Lorand Jakab --- .../MappingServiceIntegrationTest.java | 97 +++++++++++++------ .../implementation/MappingSystem.java | 15 +++ .../lisp/util/LispAddressUtil.java | 10 ++ .../mapcache/MultiTableMapCache.java | 3 +- .../mapcache/SimpleMapCache.java | 4 +- 5 files changed, 97 insertions(+), 32 deletions(-) diff --git a/integrationtest/src/test/java/org/opendaylight/lispflowmapping/integrationtest/MappingServiceIntegrationTest.java b/integrationtest/src/test/java/org/opendaylight/lispflowmapping/integrationtest/MappingServiceIntegrationTest.java index 34e403c50..68caed3e6 100644 --- a/integrationtest/src/test/java/org/opendaylight/lispflowmapping/integrationtest/MappingServiceIntegrationTest.java +++ b/integrationtest/src/test/java/org/opendaylight/lispflowmapping/integrationtest/MappingServiceIntegrationTest.java @@ -68,6 +68,7 @@ import org.opendaylight.lispflowmapping.lisp.type.MappingData; import org.opendaylight.lispflowmapping.lisp.util.ByteUtil; import org.opendaylight.lispflowmapping.lisp.util.LispAddressStringifier; import org.opendaylight.lispflowmapping.lisp.util.LispAddressUtil; +import org.opendaylight.lispflowmapping.lisp.util.MappingRecordUtil; import org.opendaylight.lispflowmapping.type.sbplugin.IConfigLispSouthboundPlugin; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpPrefix; @@ -424,6 +425,9 @@ public class MappingServiceIntegrationTest extends AbstractMdsalTestBase { insertMappings(); testMultipleMappings(); + + // https://bugs.opendaylight.org/show_bug.cgi?id=8679 + testNegativePrefix(); } private void testRepeatedSmr() throws SocketTimeoutException, UnknownHostException { @@ -574,14 +578,8 @@ public class MappingServiceIntegrationTest extends AbstractMdsalTestBase { * Tests a negative mapping from an intersection of gaps in northbound and southbound. */ private void testGapIntersection() throws UnknownHostException { - final InstanceIdType iid = new InstanceIdType(1L); - // request an Eid from a gap between mappings - final MapRequest mapRequest = new MapRequestBuilder().setSmrInvoked(false).setEidItem(Lists.newArrayList( - new EidItemBuilder().setEid(LispAddressUtil.asIpv4PrefixBinaryEid("1.1.127.10/32", iid)) - .build())) - .build(); - final MapReply mapReply = lms.handleMapRequest(mapRequest); + final MapReply mapReply = lms.handleMapRequest(newMapRequest(1L, "1.1.127.10/32")); // expected negative mapping final Address resultNegMapping = new Ipv4PrefixBinaryBuilder() @@ -591,38 +589,67 @@ public class MappingServiceIntegrationTest extends AbstractMdsalTestBase { .getAddress()); } - private void insertMappings() { + private void testNegativePrefix() { + // First, we test with one mapping in NB and one mapping in SB cleanUP(); - mapService.setLookupPolicy(IMappingService.LookupPolicy.NB_AND_SB); - final InstanceIdType iid = new InstanceIdType(1L); - final String prefixNbLeft = "1.2.0.0/16"; - final String prefixNbRight = "1.1.128.0/17"; - final String prefixSbLeft = "1.1.32.0/19"; - final String prefixSbRight = "1.0.0.0/8"; + insertNBMappings(1L, "192.0.2.0/24"); + insertSBMappings(1L, "10.0.0.0/32"); - final MappingRecord mapRecordNbLeft = newMappingRecord(prefixNbLeft, iid); - final MappingRecord mapRecordNbRight = newMappingRecord(prefixNbRight, iid); - final MappingRecord mapRecordSbLeft = newMappingRecord(prefixSbLeft, iid); - final MappingRecord mapRecordSbRight = newMappingRecord(prefixSbRight, iid); + restartSocket(); + sleepForSeconds(2); - /* set auth */ - final Eid eid = LispAddressUtil.asIpv4PrefixBinaryEid("0.0.0.0/0", iid); - mapService.addAuthenticationKey(eid, NULL_AUTH_KEY); + MapReply mapReply = lms.handleMapRequest(newMapRequest(1L, "11.1.1.1/32")); + Eid expectedNegativePrefix = LispAddressUtil.asIpv4PrefixBinaryEid(1L, "11.0.0.0/8"); + assertEquals(expectedNegativePrefix, mapReply.getMappingRecordItem().get(0).getMappingRecord().getEid()); + assertTrue(MappingRecordUtil.isNegativeMapping(mapReply.getMappingRecordItem().get(0).getMappingRecord())); + + // Second, we test with two mappings in NB only + cleanUP(); + + insertNBMappings(1L, "192.167.0.0/16", "192.169.0.0/16"); + + restartSocket(); + sleepForSeconds(2); + + mapReply = lms.handleMapRequest(newMapRequest(1L, "192.168.0.1/32")); + expectedNegativePrefix = LispAddressUtil.asIpv4PrefixBinaryEid(1L, "192.168.0.0/16"); + MappingRecord mr = mapReply.getMappingRecordItem().get(0).getMappingRecord(); + assertEquals(expectedNegativePrefix, mr.getEid()); + assertTrue(MappingRecordUtil.isNegativeMapping(mr)); + } + + private void insertMappings() { + cleanUP(); + mapService.setLookupPolicy(IMappingService.LookupPolicy.NB_AND_SB); - mapService.addMapping(MappingOrigin.Northbound, mapRecordNbLeft.getEid(), null, - new MappingData(mapRecordNbLeft)); - mapService.addMapping(MappingOrigin.Northbound, mapRecordNbRight.getEid(), null, - new MappingData(mapRecordNbRight)); - mapService.addMapping(MappingOrigin.Southbound, mapRecordSbLeft.getEid(), null, - new MappingData(mapRecordSbLeft, System.currentTimeMillis())); - mapService.addMapping(MappingOrigin.Southbound, mapRecordSbRight.getEid(), null, - new MappingData(mapRecordSbRight, System.currentTimeMillis())); + insertNBMappings(1L, "1.2.0.0/16", "1.1.128.0/17"); + insertSBMappings(1L, "1.1.32.0/19", "1.0.0.0/8"); restartSocket(); sleepForSeconds(2); } + private void insertNBMappings(long iid, String ... prefixes) { + final InstanceIdType iiType = new InstanceIdType(iid); + for (String prefix : prefixes) { + MappingRecord record = newMappingRecord(prefix, iiType); + mapService.addMapping(MappingOrigin.Northbound, record.getEid(), null, new MappingData(record)); + } + } + + private void insertSBMappings(long iid, String ... prefixes) { + final InstanceIdType iiType = new InstanceIdType(iid); + Eid eid = LispAddressUtil.asIpv4PrefixBinaryEid("0.0.0.0/0", iiType); + mapService.addAuthenticationKey(eid, NULL_AUTH_KEY); + + for (String prefix : prefixes) { + MappingRecord record = newMappingRecord(prefix, iiType); + mapService.addMapping(MappingOrigin.Southbound, record.getEid(), null, + new MappingData(record, System.currentTimeMillis())); + } + } + /** * Creates a new MappingRecord object. * @@ -637,11 +664,21 @@ public class MappingServiceIntegrationTest extends AbstractMdsalTestBase { .setLocatorRecord(Lists.newArrayList(new LocatorRecordBuilder() .setRloc(LispAddressUtil.asIpv4Rloc("2.2.2.2")) .setLocatorId("loc_id") - .setPriority((short) 1).build())) + .setPriority((short) 1) + .setWeight((short) 1).build())) .setTimestamp(System.currentTimeMillis()) .setRecordTtl(1440).build(); } + private MapRequest newMapRequest(long iid, String prefix) { + final InstanceIdType iidt = new InstanceIdType(iid); + return new MapRequestBuilder() + .setSmrInvoked(false) + .setEidItem(Lists.newArrayList( + new EidItemBuilder().setEid(LispAddressUtil.asIpv4PrefixBinaryEid(prefix, iidt)).build())) + .build(); + } + /** * TEST SCENARIO A */ diff --git a/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/MappingSystem.java b/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/MappingSystem.java index 0661c5f49..f175952ca 100644 --- a/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/MappingSystem.java +++ b/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/MappingSystem.java @@ -435,15 +435,30 @@ public class MappingSystem implements IMappingSystem { @Override public Eid getWidestNegativePrefix(Eid key) { + if (!MaskUtil.isMaskable(key.getAddress())) { + LOG.warn("Widest negative prefix only makes sense for maskable addresses!"); + return null; + } + + // We assume that ILispMapCache#getWidestNegativeMapping() returns null for positive mappings, and 0/0 + // for empty cache. Eid nbPrefix = pmc.getWidestNegativeMapping(key); if (nbPrefix == null) { + LOG.trace("getWidestNegativePrefix NB: positive mapping, returning null"); return null; } + if (LOG.isTraceEnabled()) { + LOG.trace("getWidestNegativePrefix NB: {}", LispAddressStringifier.getString(nbPrefix)); + } Eid sbPrefix = smc.getWidestNegativeMapping(key); if (sbPrefix == null) { + LOG.trace("getWidestNegativePrefix SB: positive mapping, returning null"); return null; } + if (LOG.isTraceEnabled()) { + LOG.trace("getWidestNegativePrefix SB: {}", LispAddressStringifier.getString(sbPrefix)); + } // since prefixes overlap, just return the more specific (larger mask) if (LispAddressUtil.getIpPrefixMask(nbPrefix) < LispAddressUtil.getIpPrefixMask(sbPrefix)) { diff --git a/mappingservice/lisp-proto/src/main/java/org/opendaylight/lispflowmapping/lisp/util/LispAddressUtil.java b/mappingservice/lisp-proto/src/main/java/org/opendaylight/lispflowmapping/lisp/util/LispAddressUtil.java index d968036b3..f9a642c75 100644 --- a/mappingservice/lisp-proto/src/main/java/org/opendaylight/lispflowmapping/lisp/util/LispAddressUtil.java +++ b/mappingservice/lisp-proto/src/main/java/org/opendaylight/lispflowmapping/lisp/util/LispAddressUtil.java @@ -471,6 +471,11 @@ public final class LispAddressUtil { return asIpv4PrefixBinaryEid(prefix, null); } + public static Eid asIpv4PrefixBinaryEid(long vni, final String prefix) { + InstanceIdType iid = new InstanceIdType(vni); + return asIpv4PrefixBinaryEid(prefix, iid); + } + public static Eid asIpv4PrefixBinaryEid(final String prefix, final InstanceIdType iiType) { String address = MaskUtil.getPrefixAddress(prefix); short mask = Short.valueOf(MaskUtil.getPrefixMask(prefix)); @@ -527,6 +532,11 @@ public final class LispAddressUtil { return asIpv6PrefixBinaryEid(prefix, null); } + public static Eid asIpv6PrefixBinaryEid(long vni, final String prefix) { + InstanceIdType iid = new InstanceIdType(vni); + return asIpv6PrefixBinaryEid(prefix, iid); + } + public static Eid asIpv6PrefixBinaryEid(final String prefix, final InstanceIdType iiType) { String address = MaskUtil.getPrefixAddress(prefix); short mask = Short.valueOf(MaskUtil.getPrefixMask(prefix)); diff --git a/mappingservice/mapcache/src/main/java/org/opendaylight/lispflowmapping/mapcache/MultiTableMapCache.java b/mappingservice/mapcache/src/main/java/org/opendaylight/lispflowmapping/mapcache/MultiTableMapCache.java index 413b5eae6..ebc9da441 100644 --- a/mappingservice/mapcache/src/main/java/org/opendaylight/lispflowmapping/mapcache/MultiTableMapCache.java +++ b/mappingservice/mapcache/src/main/java/org/opendaylight/lispflowmapping/mapcache/MultiTableMapCache.java @@ -127,11 +127,12 @@ public class MultiTableMapCache implements IMapCache { return getMappingLpmSD(srcEid, dstEid, table); } + // Returns null for positive mappings, and 0/0 for empty cache. @Override public Eid getWidestNegativeMapping(Eid key) { ILispDAO table = getVniTable(key); if (table == null) { - return null; + return MaskUtil.normalize(key, (short) 0); } return table.getWidestNegativePrefix(key); } diff --git a/mappingservice/mapcache/src/main/java/org/opendaylight/lispflowmapping/mapcache/SimpleMapCache.java b/mappingservice/mapcache/src/main/java/org/opendaylight/lispflowmapping/mapcache/SimpleMapCache.java index 9d298bab3..494129a5c 100644 --- a/mappingservice/mapcache/src/main/java/org/opendaylight/lispflowmapping/mapcache/SimpleMapCache.java +++ b/mappingservice/mapcache/src/main/java/org/opendaylight/lispflowmapping/mapcache/SimpleMapCache.java @@ -164,10 +164,12 @@ public class SimpleMapCache implements ILispMapCache { return null; } + // Returns null for positive mappings, and 0/0 for empty cache. + @Override public Eid getWidestNegativeMapping(Eid eid) { ILispDAO table = getVniTable(eid); if (table == null) { - return null; + return MaskUtil.normalize(eid, (short) 0); } return table.getWidestNegativePrefix(MaskUtil.normalize(eid)); } -- 2.36.6