BVS-195, notify listeners for device classification first, then listeners for reconcile.

src/main/java/net/floodlightcontroller/devicemanager/IDeviceService.java

@@ -30,6 +30,23 @@ import net.floodlightcontroller.core.module.IFloodlightService;
  * from the {@link FloodlightContext} rather than from {@link IDeviceManager}.
  */
 public interface IDeviceService extends IFloodlightService {
+
+    /**
+     * IDeviceListeners do one of two actions on devices in response to the device events:
+     * 1. Reclassify device
+     * 2. Reconcile flows for the device.
+     *
+     * We must make sure all device classification are done before flow reconciliation.
+     * So all listeners are grouped into two groups based on their types.
+     * Reclassification group is notified first, then the reconciliation group.
+     *
+     * When a module registers as a listener, it must specify the type.
+     */
+    enum ListenerType {
+        DeviceClassifier, // Listener should only do device classification change.
+        DeviceReconciler, // Listener should do reconciliation only
+    }
+
     /**
      * Fields used in devices for indexes and querying
      * @see IDeviceService#addIndex
@@ -189,8 +206,9 @@ public interface IDeviceService extends IFloodlightService {
      * Adds a listener to listen for IDeviceManagerServices notifications
      * 
      * @param listener The listener that wants the notifications
+     * @param type     The type of the listener
      */
-    public void addListener(IDeviceListener listener);
+    public void addListener(IDeviceListener listener, ListenerType type);
     
     /**
      * Specify points in the network where attachment points are not to
@@ -198,8 +216,8 @@ public interface IDeviceService extends IFloodlightService {
      * @param sw
      * @param port
      */
-	public void addSuppressAPs(long swId, short port);
+    public void addSuppressAPs(long swId, short port);
 
-	public void removeSuppressAPs(long swId, short port);
+    public void removeSuppressAPs(long swId, short port);
 
 }

src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java

@@ -197,8 +197,11 @@ IFlowReconcileListener, IInfoProvider, IHAListener {
 
     /**
      * Device manager event listeners
+     * reclassifyDeviceListeners are notified first before reconcileDeviceListeners.
+     * This is to make sure devices are correctly reclassified before reconciliation.
      */
-    protected Set<IDeviceListener> deviceListeners;
+    protected Set<IDeviceListener> reclassifyDeviceListeners;
+    protected Set<IDeviceListener> reconcileDeviceListeners;
 
     /**
      * A device update event to be dispatched
@@ -523,8 +526,15 @@ IFlowReconcileListener, IInfoProvider, IHAListener {
     }
 
     @Override
-    public void addListener(IDeviceListener listener) {
-        deviceListeners.add(listener);
+    public void addListener(IDeviceListener listener, ListenerType type) {
+        switch (type) {
+            case DeviceClassifier:
+                reclassifyDeviceListeners.add(listener);
+                break;
+            case DeviceReconciler:
+                reconcileDeviceListeners.add(listener);
+                break;
+        }
     }
 
     // *************
@@ -674,7 +684,8 @@ IFlowReconcileListener, IInfoProvider, IHAListener {
                 new HashSet<EnumSet<DeviceField>>();
         addIndex(true, EnumSet.of(DeviceField.IPV4));
 
-        this.deviceListeners = new HashSet<IDeviceListener>();
+        this.reclassifyDeviceListeners = new HashSet<IDeviceListener>();
+        this.reconcileDeviceListeners = new HashSet<IDeviceListener>();
         this.suppressAPs = Collections.newSetFromMap(
                                new ConcurrentHashMap<SwitchPort, Boolean>());
 
@@ -1312,39 +1323,44 @@ IFlowReconcileListener, IInfoProvider, IHAListener {
             if (logger.isTraceEnabled()) {
                 logger.trace("Dispatching device update: {}", update);
             }
-            for (IDeviceListener listener : deviceListeners) {
-                switch (update.change) {
-                    case ADD:
-                        listener.deviceAdded(update.device);
-                        break;
-                    case DELETE:
-                        listener.deviceRemoved(update.device);
-                        break;
-                    case CHANGE:
-                        for (DeviceField field : update.fieldsChanged) {
-                            switch (field) {
-                                case IPV4:
-                                    listener.deviceIPV4AddrChanged(update.device);
-                                    break;
-                                case SWITCH:
-                                case PORT:
-                                    //listener.deviceMoved(update.device);
-                                    break;
-                                case VLAN:
-                                    listener.deviceVlanChanged(update.device);
-                                    break;
-                                default:
-                                    logger.debug("Unknown device field changed {}",
-                                                update.fieldsChanged.toString());
-                                    break;
-                            }
+            notifyListeners(reclassifyDeviceListeners, update);
+            notifyListeners(reconcileDeviceListeners, update);
+        }
+    }
+
+    protected void notifyListeners(Set<IDeviceListener> listeners, DeviceUpdate update) {
+        for (IDeviceListener listener : listeners) {
+            switch (update.change) {
+                case ADD:
+                    listener.deviceAdded(update.device);
+                    break;
+                case DELETE:
+                    listener.deviceRemoved(update.device);
+                    break;
+                case CHANGE:
+                    for (DeviceField field : update.fieldsChanged) {
+                        switch (field) {
+                            case IPV4:
+                                listener.deviceIPV4AddrChanged(update.device);
+                                break;
+                            case SWITCH:
+                            case PORT:
+                                //listener.deviceMoved(update.device);
+                                break;
+                            case VLAN:
+                                listener.deviceVlanChanged(update.device);
+                                break;
+                            default:
+                                logger.debug("Unknown device field changed {}",
+                                            update.fieldsChanged.toString());
+                                break;
                         }
-                        break;
-                }
+                    }
+                    break;
             }
         }
     }
-    
+
     /**
      * Check if the entity e has all the keyFields set. Returns false if not
      * @param e entity to check 
@@ -1713,7 +1729,10 @@ IFlowReconcileListener, IInfoProvider, IHAListener {
      * @param updates the updates to process.
      */
     protected void sendDeviceMovedNotification(Device d) {
-        for (IDeviceListener listener : deviceListeners) {
+        for (IDeviceListener listener : reclassifyDeviceListeners) {
+            listener.deviceMoved(d);
+        }
+        for (IDeviceListener listener : reconcileDeviceListeners) {
             listener.deviceMoved(d);
         }
     }

src/main/java/net/floodlightcontroller/flowcache/FlowReconcileManager.java

@@ -274,8 +274,8 @@ public class FlowReconcileManager
         
         // Get the maximum number of flows that can be reconciled.
         int reconcileCapacity = getCurrentCapacity();
-        if (logger.isTraceEnabled()) {
-            logger.trace("Reconcile capacity {} flows", reconcileCapacity);
+        if (logger.isInfoEnabled()) {
+            logger.info("Reconcile capacity {} flows", reconcileCapacity);
         }
         while (!flowQueue.isEmpty() && reconcileCapacity > 0) {
             OFMatchReconcile ofmRc = flowQueue.poll();

src/main/java/net/floodlightcontroller/routing/ForwardingBase.java

@@ -34,8 +34,6 @@ import net.floodlightcontroller.core.annotations.LogMessageDoc;
 import net.floodlightcontroller.core.annotations.LogMessageDocs;
 import net.floodlightcontroller.core.util.AppCookie;
 import net.floodlightcontroller.counter.ICounterStoreService;
-import net.floodlightcontroller.devicemanager.IDevice;
-import net.floodlightcontroller.devicemanager.IDeviceListener;
 import net.floodlightcontroller.devicemanager.IDeviceService;
 import net.floodlightcontroller.devicemanager.SwitchPort;
 import net.floodlightcontroller.packet.Ethernet;
@@ -66,7 +64,7 @@ import org.slf4j.LoggerFactory;
  */
 @LogMessageCategory("Flow Programming")
 public abstract class ForwardingBase 
-    implements IOFMessageListener, IDeviceListener {
+    implements IOFMessageListener {
     
     protected static Logger log =
             LoggerFactory.getLogger(ForwardingBase.class);
@@ -124,7 +122,6 @@ public abstract class ForwardingBase
      * Adds a listener for devicemanager and registers for PacketIns.
      */
     protected void startUp() {
-        deviceManager.addListener(this);
         floodlightProvider.addOFMessageListener(OFType.PACKET_IN, this);
     }
 
@@ -592,30 +589,6 @@ public abstract class ForwardingBase
 
     }
 
-    @Override
-    public void deviceAdded(IDevice device) {
-        // NOOP
-    }
-
-    @Override
-    public void deviceRemoved(IDevice device) {
-        // NOOP
-    }
-
-    @Override
-    public void deviceMoved(IDevice device) {
-    }
-
-    @Override
-    public void deviceIPV4AddrChanged(IDevice device) {
-
-    }
-
-    @Override
-    public void deviceVlanChanged(IDevice device) {
-
-    }
-
     @Override
     public boolean isCallbackOrderingPrereq(OFType type, String name) {
         return (type.equals(OFType.PACKET_IN) && 

src/main/java/net/floodlightcontroller/virtualnetwork/VirtualNetworkFilter.java

@@ -296,7 +296,7 @@ public class VirtualNetworkFilter
     public void startUp(FloodlightModuleContext context) {
         floodlightProvider.addOFMessageListener(OFType.PACKET_IN, this);
         restApi.addRestletRoutable(new VirtualNetworkWebRoutable());
-        deviceService.addListener(this);
+        deviceService.addListener(this, IDeviceService.ListenerType.DeviceClassifier);
     }
 
     // IOFMessageListener

src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java

@@ -277,7 +277,7 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
         IDeviceListener mockListener =
                 createStrictMock(IDeviceListener.class);
 
-        deviceManager.addListener(mockListener);
+        deviceManager.addListener(mockListener, IDeviceService.ListenerType.DeviceClassifier);
         deviceManager.entityClassifier= new MockEntityClassifier();
         deviceManager.startUp(null);
 
@@ -483,7 +483,7 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
         IDeviceListener mockListener =
                 createStrictMock(IDeviceListener.class);
 
-        deviceManager.addListener(mockListener);
+        deviceManager.addListener(mockListener, IDeviceService.ListenerType.DeviceClassifier);
 
         ITopologyService mockTopology = createMock(ITopologyService.class);
         expect(mockTopology.getL2DomainId(1L)).
@@ -591,7 +591,7 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
         IDeviceListener mockListener =
                 createMock(IDeviceListener.class);
 
-        deviceManager.addListener(mockListener);
+        deviceManager.addListener(mockListener, IDeviceService.ListenerType.DeviceClassifier);
 
         ITopologyService mockTopology = createMock(ITopologyService.class);
         expect(mockTopology.getL2DomainId(1L)).
@@ -706,7 +706,7 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
         IDeviceListener mockListener =
                 createMock(IDeviceListener.class);
 
-        deviceManager.addListener(mockListener);
+        deviceManager.addListener(mockListener, IDeviceService.ListenerType.DeviceClassifier);
 
         ITopologyService mockTopology = createMock(ITopologyService.class);
         expect(mockTopology.getL2DomainId(1L)).
@@ -1097,7 +1097,7 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
         assertEquals(d.getDeviceKey(), diter.next().getDeviceKey());
 
 
-        deviceManager.addListener(mockListener);
+        deviceManager.addListener(mockListener, IDeviceService.ListenerType.DeviceClassifier);
         replay(mockListener);
         deviceManager.entityCleanupTask.reschedule(0, null);
 
@@ -1160,7 +1160,7 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
         d = deviceManager.learnDeviceByEntity(entity1);
         assertArrayEquals(new Integer[] { 1, 2 }, d.getIPv4Addresses());
 
-        deviceManager.addListener(mockListener);
+        deviceManager.addListener(mockListener, IDeviceService.ListenerType.DeviceClassifier);
         replay(mockListener);
         deviceManager.entityCleanupTask.reschedule(0, null);
 

src/test/java/net/floodlightcontroller/devicemanager/test/MockDeviceManager.java

@@ -47,9 +47,11 @@ public class MockDeviceManager extends DeviceManagerImpl {
                                Integer ipv4Address, Long switchDPID, 
                                Integer switchPort,
                                boolean processUpdates) {
-        Set<IDeviceListener> listeners = deviceListeners;
+        Set<IDeviceListener> reclassifyListeners = reclassifyDeviceListeners;
+        Set<IDeviceListener> reconcileListeners = reconcileDeviceListeners;
         if (!processUpdates) {
-            deviceListeners = Collections.<IDeviceListener>emptySet();
+            reclassifyDeviceListeners = Collections.<IDeviceListener>emptySet();
+            reconcileDeviceListeners = Collections.<IDeviceListener>emptySet();
         }
         
         if (vlan != null && vlan.shortValue() <= 0)
@@ -59,7 +61,8 @@ public class MockDeviceManager extends DeviceManagerImpl {
         IDevice res =  learnDeviceByEntity(new Entity(macAddress, vlan, 
                                                       ipv4Address, switchDPID, 
                                                       switchPort, new Date()));
-        deviceListeners = listeners;
+        reclassifyDeviceListeners = reclassifyListeners;
+        reconcileDeviceListeners = reconcileListeners;
         return res;
     }