From: Alessandro Boch Date: Thu, 15 Aug 2013 16:36:38 +0000 (-0700) Subject: Fix bug in Static Route configuration path X-Git-Tag: releasepom-0.1.0~197 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=53bf3293a370a093faa114402bcae1f8938c28c7;hp=8c9a1cf398348546e6e5ba74a2012ed942a2fddb Fix bug in Static Route configuration path ISSUE: Multiple equivalent static route configurations with different names not detected CHANGE: In addStaticRoute() perform proper equality check on existing StaticRoute objects (using available StaticRoute compareTo() method) instead of checking on StaticRouteConfig objects which can never be positive as same name config is detected at the beginning of method Change-Id: I51d2e1a641f612f5e79eb11063a868739a275209 Signed-off-by: Alessandro Boch --- diff --git a/opendaylight/forwarding/staticrouting/src/main/java/org/opendaylight/controller/forwarding/staticrouting/internal/StaticRoutingImplementation.java b/opendaylight/forwarding/staticrouting/src/main/java/org/opendaylight/controller/forwarding/staticrouting/internal/StaticRoutingImplementation.java index ec6da38769..eac854c106 100644 --- a/opendaylight/forwarding/staticrouting/src/main/java/org/opendaylight/controller/forwarding/staticrouting/internal/StaticRoutingImplementation.java +++ b/opendaylight/forwarding/staticrouting/src/main/java/org/opendaylight/controller/forwarding/staticrouting/internal/StaticRoutingImplementation.java @@ -97,6 +97,7 @@ public class StaticRoutingImplementation implements IfNewHostNotify, } } + @Override public ConcurrentMap getStaticRouteConfigs() { return staticRouteConfigs; } @@ -258,11 +259,13 @@ public class StaticRoutingImplementation implements IfNewHostNotify, } private void notifyHostUpdate(HostNodeConnector host, boolean added) { - if (host == null) + if (host == null) { return; + } for (StaticRoute s : staticRoutes.values()) { - if (s.getType() == StaticRoute.NextHopType.SWITCHPORT) + if (s.getType() == StaticRoute.NextHopType.SWITCHPORT) { continue; + } if (s.getNextHopAddress().equals(host.getNetworkAddress())) { if (added) { s.setHost(host); @@ -285,8 +288,9 @@ public class StaticRoutingImplementation implements IfNewHostNotify, } public boolean isIPv4AddressValid(String cidr) { - if (cidr == null) + if (cidr == null) { return false; + } String values[] = cidr.split("/"); Pattern ipv4Pattern = Pattern @@ -322,6 +326,7 @@ public class StaticRoutingImplementation implements IfNewHostNotify, return 0; } + @Override public StaticRoute getBestMatchStaticRoute(InetAddress ipAddress) { ByteBuffer bblongestPrefix = null; try { @@ -349,10 +354,9 @@ public class StaticRoutingImplementation implements IfNewHostNotify, return longestPrefixRoute; } + @Override public Status addStaticRoute(StaticRouteConfig config) { - Status status; - - status = config.isValid(); + Status status = config.isValid(); if (!status.isSuccess()) { return status; } @@ -361,22 +365,29 @@ public class StaticRoutingImplementation implements IfNewHostNotify, "A valid Static Route configuration with this name " + "already exists. Please use a different name"); } - for (StaticRouteConfig s : staticRouteConfigs.values()) { - if (s.equals(config)) { + + // Update database + StaticRoute sRoute = new StaticRoute(config); + + for (Map.Entry entry : staticRoutes.entrySet()) { + if (entry.getValue().compareTo(sRoute) == 0) { return new Status(StatusCode.CONFLICT, - "This conflicts with an existing Static Route " + - "Configuration. Please check the configuration " + - "and try again"); + "This conflicts with an existing Static Route " + + "Configuration. Please check the configuration " + + "and try again"); } } + staticRoutes.put(config.getName(), sRoute); + // Update config databse staticRouteConfigs.put(config.getName(), config); - StaticRoute sRoute = new StaticRoute(config); - staticRoutes.put(config.getName(), sRoute); + + // Notify checkAndUpdateListeners(sRoute, true); return status; } + @Override public Status removeStaticRoute(String name) { staticRouteConfigs.remove(name); StaticRoute sRoute = staticRoutes.remove(name); @@ -424,8 +435,9 @@ public class StaticRoutingImplementation implements IfNewHostNotify, allocateCaches(); retrieveCaches(); this.executor = Executors.newFixedThreadPool(1); - if (staticRouteConfigs.isEmpty()) + if (staticRouteConfigs.isEmpty()) { loadConfiguration(); + } /* * Slow probe to identify any gateway that might have silently appeared diff --git a/opendaylight/forwarding/staticrouting/src/test/java/org/opendaylight/controller/forwarding/staticrouting/StaticRouteTest.java b/opendaylight/forwarding/staticrouting/src/test/java/org/opendaylight/controller/forwarding/staticrouting/StaticRouteTest.java index 4a497a058c..25b2dacde1 100644 --- a/opendaylight/forwarding/staticrouting/src/test/java/org/opendaylight/controller/forwarding/staticrouting/StaticRouteTest.java +++ b/opendaylight/forwarding/staticrouting/src/test/java/org/opendaylight/controller/forwarding/staticrouting/StaticRouteTest.java @@ -83,9 +83,9 @@ public class StaticRouteTest { Assert.assertFalse(staticRoute1.equals(staticRoute3)); Assert.assertFalse(staticRoute1.equals(staticRoute4)); - Assert.assertTrue(staticRoute1.compareTo(staticRoute2) == 0 ? true : false); - Assert.assertFalse(staticRoute1.compareTo(staticRoute3) == 0 ? true : false); - Assert.assertTrue(staticRoute1.compareTo(staticRoute4) == 0 ? true : false); + Assert.assertTrue(staticRoute1.compareTo(staticRoute2) == 0); + Assert.assertFalse(staticRoute1.compareTo(staticRoute3) == 0); + Assert.assertTrue(staticRoute1.compareTo(staticRoute4) == 0); }