Make deeper comparison for best path selection.

Superficial equals caused best path states to never equal,
even if they should. This resulted in more writes to LocRib,
as all paths were new.

Change-Id: Ie1b694acdddac2bd617bc1086dc63c8c8eef049d
Signed-off-by: Dana Kutenicsova <dkutenic@cisco.com>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractRouteEntry.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractRouteEntry.java
index c92a78f220f65a2b37688003e3a1c71bdb6ba1c4..c38c4c64d91e729b61b10a1aac9abdbcb61003f5 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractRouteEntry.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractRouteEntry.java
@@ -8,7 +8,6 @@
 package org.opendaylight.protocol.bgp.rib.impl;
 
 import com.google.common.primitives.UnsignedInteger;
-import java.util.Objects;
 import javax.annotation.concurrent.NotThreadSafe;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
@@ -87,8 +86,7 @@ abstract class AbstractRouteEntry {
 
         // Get the newly-selected best path.
         final BestPath newBestPath = selector.result();
-        // FIXME: run deeper comparison
-        final boolean ret = !Objects.equals(this.bestPath, newBestPath);
+        final boolean ret = !newBestPath.equals(this.bestPath);
         LOG.trace("Previous best {}, current best {}, result {}", this.bestPath, newBestPath, ret);
         this.bestPath = newBestPath;
         return ret;

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPath.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPath.java
index b9d0177d59248e0339fc8a4787f925d5b7d1644e..5607b2515235b651b8171eb868b56ea0fd6f14b3 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPath.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPath.java
@@ -7,6 +7,8 @@
  */
 package org.opendaylight.protocol.bgp.rib.impl;
 
+import com.google.common.base.MoreObjects;
+import com.google.common.base.MoreObjects.ToStringHelper;
 import com.google.common.base.Preconditions;
 import com.google.common.primitives.UnsignedInteger;
 import javax.annotation.Nonnull;
@@ -21,10 +23,48 @@ final class BestPath {
     }
 
     UnsignedInteger getRouterId() {
-        return routerId;
+        return this.routerId;
     }
 
     BestPathState getState() {
-        return state;
+        return this.state;
+    }
+
+    private ToStringHelper addToStringAttributes(final ToStringHelper toStringHelper) {
+        toStringHelper.add("routerId", this.routerId);
+        toStringHelper.add("state", this.state);
+        return toStringHelper;
+    }
+
+    @Override
+    public String toString() {
+        return addToStringAttributes(MoreObjects.toStringHelper(this)).toString();
+    }
+
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + this.routerId.hashCode();
+        result = prime * result + this.state.hashCode();
+        return result;
+    }
+
+    @Override
+    public boolean equals(final Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (!(obj instanceof BestPath)) {
+            return false;
+        }
+        final BestPath other = (BestPath) obj;
+        if (!this.routerId.equals(other.routerId)) {
+            return false;
+        }
+        if (!this.state.equals(other.state)) {
+            return false;
+        }
+        return true;
     }
 }

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPathSelector.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPathSelector.java
index df2781a87a6f11249aa2036c859ec0649ce2fd59..4fb938f570d87d2a125355fe854d7b4990083982 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPathSelector.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPathSelector.java
@@ -61,7 +61,7 @@ final class BestPathSelector {
              */
             final BestPathState state = new BestPathState(attrs);
             if (this.bestOriginatorId == null || !selectPath(originatorId, state)) {
-                LOG.trace("Selecting path from router {} state {}", routerId, state);
+                LOG.trace("Selecting path from router {}", routerId);
                 this.bestOriginatorId = originatorId;
                 this.bestRouterId = routerId;
                 this.bestState = state;

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPathState.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPathState.java
index 1aa5b83f0597a6c7d37937fd5f43a46757a4b31c..fc3d2d7509c8505d8b7cc2cd9aefa2dbf49df587 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPathState.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BestPathState.java
@@ -7,6 +7,8 @@
  */
 package org.opendaylight.protocol.bgp.rib.impl;
 
+import com.google.common.base.MoreObjects;
+import com.google.common.base.MoreObjects.ToStringHelper;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
@@ -60,7 +62,7 @@ final class BestPathState {
         case "incomplete":
             return BgpOrigin.Incomplete;
         default:
-            throw new IllegalArgumentException("Unhandleed origin value " + originStr);
+            throw new IllegalArgumentException("Unhandled origin value " + originStr);
         }
     }
 
@@ -160,4 +162,61 @@ final class BestPathState {
     ContainerNode getAttributes() {
         return this.attributes;
     }
+
+    private ToStringHelper addToStringAttributes(final ToStringHelper toStringHelper) {
+        toStringHelper.add("attributes", this.attributes);
+        toStringHelper.add("localPref", this.localPref);
+        toStringHelper.add("multiExitDisc", this.multiExitDisc);
+        toStringHelper.add("origin", this.origin);
+        toStringHelper.add("resolved", this.resolved);
+        return toStringHelper;
+    }
+
+    @Override
+    public String toString() {
+        return addToStringAttributes(MoreObjects.toStringHelper(this)).toString();
+    }
+
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + this.attributes.hashCode();
+        result = prime * result + ((this.localPref == null) ? 0 : this.localPref.hashCode());
+        result = prime * result + ((this.multiExitDisc == null) ? 0 : this.multiExitDisc.hashCode());
+        result = prime * result + ((this.origin == null) ? 0 : this.origin.hashCode());
+        return result;
+    }
+
+    @Override
+    public boolean equals(final Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (!(obj instanceof BestPathState)) {
+            return false;
+        }
+        final BestPathState other = (BestPathState) obj;
+        if (!this.attributes.equals(other.attributes)) {
+            return false;
+        }
+        if (this.localPref == null) {
+            if (other.localPref != null) {
+                return false;
+            }
+        } else if (!this.localPref.equals(other.localPref)) {
+            return false;
+        }
+        if (this.multiExitDisc == null) {
+            if (other.multiExitDisc != null) {
+                return false;
+            }
+        } else if (!this.multiExitDisc.equals(other.multiExitDisc)) {
+            return false;
+        }
+        if (this.origin != other.origin) {
+            return false;
+        }
+        return true;
+    }
 }

diff --git a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/AbstractRIBSupport.java b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/AbstractRIBSupport.java
index dac9d2a2df01d9df3d0865b0c46075e66740bc0a..63a10cc0dd23190adba2495297639fa27a12136b 100644 (file)
--- a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/AbstractRIBSupport.java
+++ b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/AbstractRIBSupport.java
@@ -179,20 +179,16 @@ public abstract class AbstractRIBSupport implements RIBSupport {
 
     @Override
     public final Collection<DataTreeCandidateNode> changedRoutes(final DataTreeCandidateNode routes) {
-        LOG.trace("Changed routes called with {} identifier {}", routes, routes.getIdentifier());
         final DataTreeCandidateNode myRoutes = routes.getModifiedChild(this.routesContainerIdentifier);
         if (myRoutes == null) {
             return Collections.emptySet();
         }
-        LOG.trace("MyRoutes {} identifier {}", myRoutes, myRoutes.getIdentifier());
         final DataTreeCandidateNode routesMap = myRoutes.getModifiedChild(this.routesListIdentifier);
         if (routesMap == null) {
             return Collections.emptySet();
         }
-        LOG.trace("RoutesMap {} identifier {}", routesMap, routesMap.getIdentifier());
         // Well, given the remote possibility of augmentation, we should perform a filter here,
         // to make sure the type matches what routeType() reports.
-        LOG.trace("Returning children {}", routesMap.getChildNodes());
         return routesMap.getChildNodes();
     }
 
@@ -254,11 +250,11 @@ public abstract class AbstractRIBSupport implements RIBSupport {
         ab.setCNextHop(null);
 
         if (!advertised.isEmpty()) {
-            MpReachNlri mb = buildReach(advertised, hop);
+            final MpReachNlri mb = buildReach(advertised, hop);
             ab.addAugmentation(Attributes1.class, new Attributes1Builder().setMpReachNlri(mb).build());
         }
         if (!withdrawn.isEmpty()) {
-            MpUnreachNlri mb = buildUnreach(withdrawn);
+            final MpUnreachNlri mb = buildUnreach(withdrawn);
             ab.addAugmentation(Attributes2.class, new Attributes2Builder().setMpUnreachNlri(mb).build());
         }