From d35aae0e21d45c4aee0bf0cf6c769946d482a6a0 Mon Sep 17 00:00:00 2001
From: Srinivasan Ramasubramanian <srini@bigswitch.com>
Date: Fri, 1 Jun 2012 08:17:10 -0700
Subject: [PATCH] Update to attachment point flapping logic.  Use active time
 instead of last seen to order the entities.  Corresponding changes to unit
 tests.

---
 .../devicemanager/internal/Device.java        | 21 ++++-----
 .../internal/DeviceManagerImpl.java           |  9 ++--
 .../internal/DeviceManagerImplTest.java       | 46 ++++++++++++++++++-
 3 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/src/main/java/net/floodlightcontroller/devicemanager/internal/Device.java b/src/main/java/net/floodlightcontroller/devicemanager/internal/Device.java
index 326ad13f1..9795363ae 100755
--- a/src/main/java/net/floodlightcontroller/devicemanager/internal/Device.java
+++ b/src/main/java/net/floodlightcontroller/devicemanager/internal/Device.java
@@ -260,16 +260,12 @@ public class Device implements IDevice {
             Integer port = cur.getSwitchPort();
             if (dpid == null || port == null ||
                 !deviceManager.isValidAttachmentPoint(dpid, port) ||
-                ///*||
                 (prev != null && 
-                topology.isBroadcastDomainPort(prev.getSwitchDPID().longValue(), 
-                                               prev.getSwitchPort().shortValue()) == false &&
-                topology.isBroadcastDomainPort(dpid.longValue(), port.shortValue()) &&
                 topology.isConsistent(prev.getSwitchDPID().longValue(),
-                                                       prev.getSwitchPort().shortValue(),
-                                                       dpid.longValue(),
-                                                       port.shortValue()))
-                )//*/
+                                      prev.getSwitchPort().shortValue(),
+                                      dpid.longValue(),
+                                      port.shortValue()))
+                )
                 continue;
             long curCluster = 
                     topology.getL2DomainId(cur.switchDPID);
@@ -287,9 +283,12 @@ public class Device implements IDevice {
                 !(dpid.equals(prev.getSwitchDPID()) &&
                   port.equals(prev.getSwitchPort())) &&
                 !topology.isInSameBroadcastDomain(dpid.longValue(),
-                		port.shortValue(),
-                        prev.getSwitchDPID().longValue(),
-                        prev.getSwitchPort().shortValue())) {
+                                                  port.shortValue(),
+                                                  prev.getSwitchDPID().longValue(),
+                                                  prev.getSwitchPort().shortValue()) &&
+                !topology.isConsistent(prev.getSwitchDPID().longValue(), 
+                                       prev.getSwitchPort().shortValue(), 
+                                       dpid.longValue(), port.shortValue())) {
                 long curActive = 
                         deviceManager.apComparator.
                             getEffTS(cur, cur.getActiveSince());
diff --git a/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java b/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java
index b989baf0b..f610260b2 100755
--- a/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java
+++ b/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java
@@ -276,9 +276,12 @@ public class DeviceManagerImpl implements
                 r = d1ClusterId.compareTo(d2ClusterId);
             }
             if (r != 0) return r;
-            
-            long e1ts = getEffTS(e1, e1.getLastSeenTimestamp());
-            long e2ts = getEffTS(e2, e2.getLastSeenTimestamp());
+
+            // the ordering of active times is a more
+            // representative of the causal relationship
+            // than lastSeen time.
+            long e1ts = getEffTS(e1, e1.getActiveSince());
+            long e2ts = getEffTS(e2, e2.getActiveSince());
             return Long.valueOf(e1ts).compareTo(e2ts);
         }
         
diff --git a/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java b/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java
index 3833a0d47..2672d4dad 100644
--- a/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java
+++ b/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java
@@ -280,8 +280,15 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
 
         expect(mockTopology.isAttachmentPointPort(anyLong(), 
                                        anyShort())).andReturn(true).anyTimes();
+        expect(mockTopology.isConsistent(10L, (short)1, 10L, (short)1)).
+        andReturn(true).anyTimes();
+        expect(mockTopology.isConsistent(1L, (short)1, 1L, (short)1)).
+        andReturn(true).anyTimes();
+        expect(mockTopology.isConsistent(50L, (short)3, 50L, (short)3)).
+        andReturn(true).anyTimes();
+
         deviceManager.topology = mockTopology;
-        
+
         Entity entity1 = new Entity(1L, null, null, 1L, 1, new Date());
         Entity entity2 = new Entity(1L, null, null, 10L, 1, new Date());
         Entity entity3 = new Entity(1L, null, 1, 10L, 1, new Date());
@@ -420,6 +427,12 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
         
         expect(mockTopology.isAttachmentPointPort(anyLong(), 
                                        anyShort())).andReturn(true).anyTimes();
+        expect(mockTopology.isConsistent(1L, (short)1, 5L, (short)1)).
+        andReturn(false).anyTimes();
+        expect(mockTopology.isConsistent(5L, (short)1, 10L, (short)1)).
+        andReturn(false).anyTimes();
+        expect(mockTopology.isConsistent(10L, (short)1, 50L, (short)1)).
+        andReturn(false).anyTimes();
         
         replay(mockTopology);
         
@@ -513,6 +526,8 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
         
         expect(mockTopology.isAttachmentPointPort(anyLong(), 
                                        anyShort())).andReturn(true).anyTimes();
+        expect(mockTopology.isConsistent(5L, (short)1, 50L, (short)1)).
+        andReturn(false).anyTimes();
         
         replay(mockTopology);
         
@@ -742,6 +757,9 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
                                        andReturn(false).anyTimes();
         expect(mockTopology.getL2DomainId(1L)).andReturn(1L).anyTimes();
         expect(mockTopology.getL2DomainId(5L)).andReturn(5L).anyTimes();
+        expect(mockTopology.isConsistent(1L, (short)1, 5L, (short)1)).
+        andReturn(false).anyTimes();
+
         replay(mockTopology);
         deviceManager.topology = mockTopology;
         
@@ -823,6 +841,16 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
         		anyLong(), anyShort())).andReturn(false).anyTimes();
         expect(mockTopology.getL2DomainId(anyLong())).
                     andReturn(1L).anyTimes();
+        expect(mockTopology.isConsistent(1L, (short)1, 1L, (short)1)).
+        andReturn(true).anyTimes();
+        expect(mockTopology.isConsistent(1L, (short)1, 5L, (short)1)).
+        andReturn(false).anyTimes();
+        expect(mockTopology.isConsistent(5L, (short)1, 10L, (short)1)).
+        andReturn(false).anyTimes();
+        expect(mockTopology.isConsistent(10L, (short)1, 1L, (short)1)).
+        andReturn(false).anyTimes();
+        expect(mockTopology.isConsistent(5L, (short)1, 1L, (short)1)).
+        andReturn(false).anyTimes();
         replay(mockTopology);
         deviceManager.topology = mockTopology;
         
@@ -919,9 +947,23 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
                 andReturn(1L).anyTimes();
         expect(mockTopology.getL2DomainId(5L)).
                 andReturn(5L).anyTimes();
+        expect(mockTopology.isConsistent(1L, (short)1, 1L, (short)2)).
+        andReturn(false).anyTimes();
+        expect(mockTopology.isConsistent(1L, (short)2, 5L, (short)1)).
+        andReturn(false).anyTimes();
+        expect(mockTopology.isConsistent(5L, (short)1, 5L, (short)2)).
+        andReturn(false).anyTimes();
+        expect(mockTopology.isConsistent(1L, (short)2, 1L, (short)1)).
+        andReturn(false).anyTimes();
+        expect(mockTopology.isConsistent(1L, (short)1, 5L, (short)1)).
+        andReturn(false).anyTimes();
+        expect(mockTopology.isConsistent(1L, (short)1, 5L, (short)2)).
+        andReturn(false).anyTimes();
+        expect(mockTopology.isConsistent(5L, (short)2, 5L, (short)1)).
+        andReturn(false).anyTimes();
         replay(mockTopology);
         deviceManager.topology = mockTopology;
-        
+
         Entity entity1 = new Entity(1L, null, null, 1L, 1, c.getTime());
         Entity entity2 = new Entity(1L, null, null, 1L, 2, c.getTime());
         Entity entity3 = new Entity(1L, null, null, 5L, 1, c.getTime());
-- 
GitLab