From 19031be731df48b3287b7620a238166600dbb518 Mon Sep 17 00:00:00 2001
From: Saurav Das <saurav.das@bigswitch.com>
Date: Tue, 14 May 2013 11:26:23 -0700
Subject: [PATCH] A few things:   1. Moving debug event for switch
 connect/disconnect to Controller (from LinkDiscovery) to be consistent with
 Shudong's changes   2. Bug fix in OFChannelHandler that was causing NPE   3.
 Bug fix in DebugEvents that was ignoring flushNow directive in event registry
   4. Updated unit test

---
 .../core/internal/Controller.java             | 28 ++++++++++++---
 .../core/internal/OFChannelHandler.java       |  2 +-
 .../debugevent/DebugEvent.java                |  2 +-
 .../internal/LinkDiscoveryManager.java        | 12 -------
 .../debugevent/DebugEventTest.java            | 36 ++++++++++++++-----
 5 files changed, 54 insertions(+), 26 deletions(-)

diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
index a11d6793f..f9f44f460 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
@@ -69,6 +69,9 @@ import net.floodlightcontroller.counter.ICounterStoreService;
 import net.floodlightcontroller.debugcounter.IDebugCounterService;
 import net.floodlightcontroller.debugcounter.IDebugCounterService.CounterType;
 import net.floodlightcontroller.debugevent.IDebugEventService;
+import net.floodlightcontroller.debugevent.NullDebugEvent;
+import net.floodlightcontroller.debugevent.IDebugEventService.EventType;
+import net.floodlightcontroller.debugevent.IDebugEventService.MaxEventsRegistered;
 import net.floodlightcontroller.packet.Ethernet;
 import net.floodlightcontroller.packet.IPv4;
 import net.floodlightcontroller.perfmon.IPktInProcessingTimeService;
@@ -155,6 +158,8 @@ public class Controller implements IFloodlightProviderService,
     protected int openFlowPort = 6633;
     protected int workerThreads = 0;
 
+    // Event IDs for debug events
+    private int SWITCH_EVENT = -1;
 
     // This controller's current role that modules can use/query to decide
     // if they should operate in master or slave mode.
@@ -931,6 +936,7 @@ public class Controller implements IFloodlightProviderService,
             IOFSwitch oldSw = this.activeSwitches.put(dpid, sw);
             // Update event history
             addSwitchEvent(dpid, EvAction.SWITCH_CONNECTED, "None");
+            debugEvents.updateEvent(SWITCH_EVENT, new Object[] {sw.getId(), "connected"});
 
             if (oldSw == sw)  {
                 // Note == for object equality, not .equals for value
@@ -1110,6 +1116,7 @@ public class Controller implements IFloodlightProviderService,
             //       in switchActivated(). Should we have events on the
             //       slave as well?
             addSwitchEvent(dpid, EvAction.SWITCH_DISCONNECTED, "None");
+            debugEvents.updateEvent(SWITCH_EVENT, new Object[] {dpid, "disconnected"});
             counters.switchDisconnected.increment();
             IOFSwitch oldSw = this.activeSwitches.get(dpid);
             if (oldSw != sw) {
@@ -1361,7 +1368,6 @@ public class Controller implements IFloodlightProviderService,
             }
             evSwitch.reason = reason;
             evSwitch = evHistSwitch.put(evSwitch, actn);
-            debugEvents.flushEvents();
         }
 
     }
@@ -1773,9 +1779,6 @@ public class Controller implements IFloodlightProviderService,
         }
     }
 
-
-
-
     void switchActivated(IOFSwitch sw) {
         this.switchManager.switchActivated(sw);
     }
@@ -2239,6 +2242,23 @@ public class Controller implements IFloodlightProviderService,
             throw new FloodlightModuleException("Error while setting up sync service", e);
         }
         this.counters.createCounters(debugCounters);
+        registerControllerDebugEvents();
+    }
+
+    private void registerControllerDebugEvents() {
+        if (debugEvents == null) {
+            debugEvents = new NullDebugEvent();
+            return;
+        }
+        try {
+            SWITCH_EVENT = debugEvents.registerEvent(
+                               "controller", "switchevent", true,
+                               "Switch connected, disconnected or port changed",
+                               EventType.ALWAYS_LOG, 100,
+                               "Sw=%dpid, reason=%s", null);
+        } catch (MaxEventsRegistered e) {
+            e.printStackTrace();
+        }
     }
 
     @Override
diff --git a/src/main/java/net/floodlightcontroller/core/internal/OFChannelHandler.java b/src/main/java/net/floodlightcontroller/core/internal/OFChannelHandler.java
index 169e539c2..2c47dacf5 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/OFChannelHandler.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/OFChannelHandler.java
@@ -1360,7 +1360,7 @@ class OFChannelHandler
     public void channelIdle(ChannelHandlerContext ctx, IdleStateEvent e)
             throws Exception {
         OFMessage m = BasicFactory.getInstance().getMessage(OFType.ECHO_REQUEST);
-        e.getChannel().write(m);
+        e.getChannel().write(Collections.singletonList(m));
     }
 
     @Override
diff --git a/src/main/java/net/floodlightcontroller/debugevent/DebugEvent.java b/src/main/java/net/floodlightcontroller/debugevent/DebugEvent.java
index 1595380ce..1a43b970c 100644
--- a/src/main/java/net/floodlightcontroller/debugevent/DebugEvent.java
+++ b/src/main/java/net/floodlightcontroller/debugevent/DebugEvent.java
@@ -269,7 +269,7 @@ public class DebugEvent implements IFloodlightModule, IDebugEventService {
             }
             le.nextIndex++;
 
-            if (le.nextIndex >= le.maxCapacity) {
+            if (le.nextIndex >= le.maxCapacity || le.flushNow) {
                 // flush this buffer now
                 DebugEventHistory de = allEvents[eventId];
                 if (de.einfo.enabled) {
diff --git a/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java b/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java
index 2b7d72deb..d9f111171 100644
--- a/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java
+++ b/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java
@@ -145,7 +145,6 @@ public class LinkDiscoveryManager implements IOFMessageListener,
 
     // Event Ids for debug events
     private int LINK_EVENT = -1;
-    private int SWITCH_EVENT = -1;
 
     protected IFloodlightProviderService floodlightProvider;
     protected IStorageSourceService storageSource;
@@ -1648,9 +1647,6 @@ public class LinkDiscoveryManager implements IOFMessageListener,
 
     @Override
     public void switchRemoved(long sw) {
-        // Update event history - TODO move to controller.java
-        debugEvents.updateEvent(SWITCH_EVENT, new Object[] {sw, "disconnected"});
-
         List<Link> eraseList = new ArrayList<Link>();
         lock.writeLock().lock();
         try {
@@ -1690,8 +1686,6 @@ public class LinkDiscoveryManager implements IOFMessageListener,
                 processNewPort(sw.getId(), p);
             }
         }
-        // Update event history - TODO move to controller.java
-        debugEvents.updateEvent(SWITCH_EVENT, new Object[] {sw.getId(), "connected"});
         LDUpdate update = new LDUpdate(sw.getId(), null,
                                        UpdateOperation.SWITCH_UPDATED);
         updates.add(update);
@@ -2177,12 +2171,6 @@ public class LinkDiscoveryManager implements IOFMessageListener,
             return;
         }
         try {
-            SWITCH_EVENT = debugEvents.registerEvent(
-                               getName(), "switchevent", true,
-                               "Switch connected, disconnected or port changed",
-                               EventType.ALWAYS_LOG, 100,
-                               "Sw=%dpid, reason=%s", null);
-
             LINK_EVENT = debugEvents.registerEvent(
                                getName(), "linkevent", false,
                                "Direct OpenFlow links discovered or timed-out",
diff --git a/src/test/java/net/floodlightcontroller/debugevent/DebugEventTest.java b/src/test/java/net/floodlightcontroller/debugevent/DebugEventTest.java
index 67d18288f..d5fbedb95 100644
--- a/src/test/java/net/floodlightcontroller/debugevent/DebugEventTest.java
+++ b/src/test/java/net/floodlightcontroller/debugevent/DebugEventTest.java
@@ -25,30 +25,50 @@ public class DebugEventTest extends FloodlightTestCase {
     @Test
     public void testRegisterAndUpdateEvent() {
         assertEquals(0, debugEvent.currentEvents.size());
-        int eventId = -1;
+        int eventId1 = -1, eventId2 = -1 ;
         try {
-            eventId = debugEvent.registerEvent("dbgevtest", "switchevent", true,
+            eventId1 = debugEvent.registerEvent("dbgevtest", "switchevent", true,
                                                "switchtest", EventType.ALWAYS_LOG,
                                                100, "Sw=%dpid, reason=%s", null);
+            eventId2 = debugEvent.registerEvent("dbgevtest", "pktinevent", false,
+                                               "pktintest", EventType.ALWAYS_LOG,
+                                               100, "Sw=%d, reason=%s", null);
         } catch (MaxEventsRegistered e) {
             e.printStackTrace();
         }
 
-        assertEquals(1, debugEvent.currentEvents.size());
-        assertEquals(eventId, debugEvent.moduleEvents.get("dbgevtest").
+        assertEquals(2, debugEvent.currentEvents.size());
+        assertEquals(eventId1, debugEvent.moduleEvents.get("dbgevtest").
                                              get("switchevent").intValue());
+        assertEquals(eventId2, debugEvent.moduleEvents.get("dbgevtest").
+                     get("pktinevent").intValue());
         assertEquals(true, debugEvent.containsModName("dbgevtest"));
         assertEquals(true, debugEvent.containsMEName("dbgevtest-switchevent"));
+        assertEquals(true, debugEvent.containsMEName("dbgevtest-pktinevent"));
+
+        assertEquals(0, DebugEvent.allEvents[eventId1].eventBuffer.size());
+        assertEquals(0, DebugEvent.allEvents[eventId2].eventBuffer.size());
+
+        // update is immediately flushed to global store
+        debugEvent.updateEvent(eventId1, new Object[] {1L, "connected"});
+        assertEquals(1, DebugEvent.allEvents[eventId1].eventBuffer.size());
+
+        // update is flushed only when flush is explicity called
+        debugEvent.updateEvent(eventId2, new Object[] {1L, "switch sent pkt-in"});
+        assertEquals(0, DebugEvent.allEvents[eventId2].eventBuffer.size());
 
-        assertEquals(0, DebugEvent.allEvents[eventId].eventBuffer.size());
-        debugEvent.updateEvent(eventId, new Object[] {1L, "connected"});
-        assertEquals(0, DebugEvent.allEvents[eventId].eventBuffer.size());
         debugEvent.flushEvents();
-        assertEquals(1, DebugEvent.allEvents[eventId].eventBuffer.size());
+        assertEquals(1, DebugEvent.allEvents[eventId1].eventBuffer.size());
+        assertEquals(1, DebugEvent.allEvents[eventId2].eventBuffer.size());
 
         DebugEventInfo de = debugEvent.getSingleEventHistory("dbgevtest-switchevent");
         assertEquals(1, de.events.size());
         assertEquals(true, de.events.get(0)
                          .contains("Sw=00:00:00:00:00:00:00:01, reason=connected"));
+
+        DebugEventInfo de2 = debugEvent.getSingleEventHistory("dbgevtest-pktinevent");
+        assertEquals(1, de2.events.size());
+        assertEquals(true, de2.events.get(0)
+                     .contains("Sw=1, reason=switch sent pkt-in"));
     }
 }
-- 
GitLab