From a21f516d73fa204ffdd5c6ac1bf5d93429fc6ad5 Mon Sep 17 00:00:00 2001
From: Alex Reimers <alex@bigswitch.com>
Date: Mon, 3 Dec 2012 22:19:13 -0800
Subject: [PATCH] FLOODLIGHT-30 - When deleting all entries put in by the
 static flow pusher for a specific switch or all switches   use a single
 delete based on matching the static flow pusher cookie instead of deleting
 each   flow individually.   Note: this means that flows mods are no longer
 hashed to have unique cookies.

---
 .../staticflowentry/StaticFlowEntries.java    |   6 +-
 .../StaticFlowEntryPusher.java                | 195 ++++++++++++------
 2 files changed, 136 insertions(+), 65 deletions(-)

diff --git a/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntries.java b/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntries.java
index ba286195f..733ed600e 100644
--- a/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntries.java
+++ b/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntries.java
@@ -66,12 +66,14 @@ public class StaticFlowEntries {
      */
     public static long computeEntryCookie(OFFlowMod fm, int userCookie, String name) {
         // flow-specific hash is next 20 bits LOOK! who knows if this 
+        /*
         int prime = 211;
         int flowHash = 2311;
         for (int i=0; i < name.length(); i++)
             flowHash = flowHash * prime + (int)name.charAt(i);
-        
-        return AppCookie.makeCookie(StaticFlowEntryPusher.STATIC_FLOW_APP_ID, flowHash);
+        */
+        // For now we use 0 so we can do a mass delete by cookie
+        return AppCookie.makeCookie(StaticFlowEntryPusher.STATIC_FLOW_APP_ID, 0);
     }
     
     /**
diff --git a/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java b/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java
index 4ed59d700..2a70610fe 100644
--- a/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java
+++ b/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java
@@ -9,7 +9,6 @@ import java.util.Iterator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -35,12 +34,13 @@ import net.floodlightcontroller.storage.IStorageSourceService;
 import net.floodlightcontroller.storage.IStorageSourceListener;
 
 import net.floodlightcontroller.storage.StorageException;
+
+import org.openflow.protocol.OFBarrierRequest;
 import org.openflow.protocol.OFFlowMod;
 import org.openflow.protocol.OFFlowRemoved;
 import org.openflow.protocol.OFMatch;
 import org.openflow.protocol.OFMessage;
 import org.openflow.protocol.OFType;
-import org.openflow.protocol.factory.BasicFactory;
 import org.openflow.util.HexString;
 import org.openflow.util.U16;
 import org.slf4j.Logger;
@@ -90,19 +90,17 @@ public class StaticFlowEntryPusher
             COLUMN_DL_SRC, COLUMN_DL_DST, COLUMN_DL_VLAN, COLUMN_DL_VLAN_PCP,
             COLUMN_DL_TYPE, COLUMN_NW_TOS, COLUMN_NW_PROTO, COLUMN_NW_SRC,
             COLUMN_NW_DST, COLUMN_TP_DST, COLUMN_TP_SRC, COLUMN_ACTIONS };
- 
+
 
     protected IFloodlightProviderService floodlightProvider;
     protected IStorageSourceService storageSource;
     protected IRestApiService restApi;
 
-    // Map<DPID, Map<Name, FlowMod>> ; FlowMod can be null to indicate non-active
+    // Map<DPID, Map<Name, FlowMod>>; FlowMod can be null to indicate non-active
     protected Map<String, Map<String, OFFlowMod>> entriesFromStorage;
     // Entry Name -> DPID of Switch it's on
     protected Map<String, String> entry2dpid;
 
-    private BasicFactory ofMessageFactory;
-
     // Class to sort FlowMod's by priority, from lowest to highest
     class FlowModSorter implements Comparator<String> {
         private String dpid;
@@ -234,17 +232,13 @@ public class StaticFlowEntryPusher
      * @param row
      * @param entries
      */
-
-    void parseRow(Map<String, Object> row,
-            Map<String, Map<String, OFFlowMod>> entries) {
+    void parseRow(Map<String, Object> row, Map<String, Map<String, OFFlowMod>> entries) {
         String switchName = null;
         String entryName = null;
 
         StringBuffer matchString = new StringBuffer();
-        if (ofMessageFactory == null) // lazy init
-            ofMessageFactory = new BasicFactory();
 
-        OFFlowMod flowMod = (OFFlowMod) ofMessageFactory
+        OFFlowMod flowMod = (OFFlowMod) floodlightProvider.getOFMessageFactory()
                 .getMessage(OFType.FLOW_MOD);
 
         if (!row.containsKey(COLUMN_SWITCH) || !row.containsKey(COLUMN_NAME)) {
@@ -265,29 +259,29 @@ public class StaticFlowEntryPusher
             for (String key : row.keySet()) {
                 if (row.get(key) == null)
                     continue;
-                if ( key.equals(COLUMN_SWITCH) || key.equals(COLUMN_NAME)
+                if (key.equals(COLUMN_SWITCH) || key.equals(COLUMN_NAME)
                         || key.equals("id"))
                     continue; // already handled
                 // explicitly ignore timeouts and wildcards
-                if ( key.equals(COLUMN_HARD_TIMEOUT) || key.equals(COLUMN_IDLE_TIMEOUT) ||
+                if (key.equals(COLUMN_HARD_TIMEOUT) || key.equals(COLUMN_IDLE_TIMEOUT) ||
                         key.equals(COLUMN_WILDCARD))
                     continue;
-                if ( key.equals(COLUMN_ACTIVE)) {
-                    if  (! Boolean.valueOf((String) row.get(COLUMN_ACTIVE))) {
+                if (key.equals(COLUMN_ACTIVE)) {
+                    if  (!Boolean.valueOf((String) row.get(COLUMN_ACTIVE))) {
                         log.debug("skipping inactive entry {} for switch {}",
                                 entryName, switchName);
                         entries.get(switchName).put(entryName, null);  // mark this an inactive
                         return;
                     }
-                } else if ( key.equals(COLUMN_ACTIONS)){
+                } else if (key.equals(COLUMN_ACTIONS)){
                     StaticFlowEntries.parseActionString(flowMod, (String) row.get(COLUMN_ACTIONS), log);
-                } else if ( key.equals(COLUMN_COOKIE)) {
+                } else if (key.equals(COLUMN_COOKIE)) {
                     flowMod.setCookie(
                             StaticFlowEntries.computeEntryCookie(flowMod, 
                                     Integer.valueOf((String) row.get(COLUMN_COOKIE)), 
                                     entryName)
                         );
-                } else if ( key.equals(COLUMN_PRIORITY)) {
+                } else if (key.equals(COLUMN_PRIORITY)) {
                     flowMod.setPriority(U16.t(Integer.valueOf((String) row.get(COLUMN_PRIORITY))));
                 } else { // the rest of the keys are for OFMatch().fromString()
                     if (matchString.length() > 0)
@@ -296,13 +290,14 @@ public class StaticFlowEntryPusher
                 }
             }
         } catch (ClassCastException e) {
-            if (entryName != null && switchName != null)
-                log.debug(
-                        "skipping entry {} on switch {} with bad data : "
+            if (entryName != null && switchName != null) {
+                log.warn(
+                        "Skipping entry {} on switch {} with bad data : "
                                 + e.getMessage(), entryName, switchName);
-            else
-                log.debug("skipping entry with bad data: {} :: {} ",
+            } else {
+                log.warn("Skipping entry with bad data: {} :: {} ",
                         e.getMessage(), e.getStackTrace());
+            }
         }
 
         OFMatch ofMatch = new OFMatch();
@@ -328,7 +323,6 @@ public class StaticFlowEntryPusher
 
     @Override
     public void removedSwitch(IOFSwitch sw) {
-        log.debug("removedSwitch {}", sw);
         // do NOT delete from our internal state; we're tracking the rules,
         // not the switches
     }
@@ -338,28 +332,26 @@ public class StaticFlowEntryPusher
         // no-op
     }
 
-    /**
-     * This handles both rowInsert() and rowUpdate()
-     */
-    
     @Override
     public void rowsModified(String tableName, Set<Object> rowKeys) {
+        // This handles both rowInsert() and rowUpdate()
         log.debug("Modifying Table {}", tableName);
-
         HashMap<String, Map<String, OFFlowMod>> entriesToAdd = 
             new HashMap<String, Map<String, OFFlowMod>>();
         // build up list of what was added 
-        for(Object key: rowKeys) {
+        for (Object key: rowKeys) {
             IResultSet resultSet = storageSource.getRow(tableName, key);
-            for (Iterator<IResultSet> it = resultSet.iterator(); it.hasNext();) {
+            Iterator<IResultSet> it = resultSet.iterator();
+            while (it.hasNext()) {
                 Map<String, Object> row = it.next().getRow();
                 parseRow(row, entriesToAdd);
-            }            
+            }
         }
         // batch updates by switch and blast them out
         for (String dpid : entriesToAdd.keySet()) {
             if (!entriesFromStorage.containsKey(dpid))
                 entriesFromStorage.put(dpid, new HashMap<String, OFFlowMod>());
+            
             List<OFMessage> outQueue = new ArrayList<OFMessage>();
             for(String entry : entriesToAdd.get(dpid).keySet()) {
                 OFFlowMod newFlowMod = entriesToAdd.get(dpid).get(entry);
@@ -385,12 +377,12 @@ public class StaticFlowEntryPusher
     @Override
     public void rowsDeleted(String tableName, Set<Object> rowKeys) {
         if (log.isDebugEnabled()) {
-            log.debug("deleting from Table {}", tableName);
+            log.debug("Deleting from table {}", tableName);
         }
         
         for(Object obj : rowKeys) {
             if (!(obj instanceof String)) {
-                log.debug("tried to delete non-string key {}; ignoring", obj);
+                log.debug("Tried to delete non-string key {}; ignoring", obj);
                 continue;
             }
             deleteStaticFlowEntry((String) obj);
@@ -400,17 +392,18 @@ public class StaticFlowEntryPusher
     @LogMessageDoc(level="ERROR",
             message="inconsistent internal state: no switch has rule {rule}",
             explanation="Inconsistent internat state discovered while " +
-            		"deleting a static flow rule",
+                    "deleting a static flow rule",
             recommendation=LogMessageDoc.REPORT_CONTROLLER_BUG)
-    private boolean deleteStaticFlowEntry(String entryName) {
-        String dpid = entry2dpid.get(entryName);
+    private void deleteStaticFlowEntry(String entryName) {
+        String dpid = entry2dpid.remove(entryName);
+        
         if (log.isDebugEnabled()) {
             log.debug("Deleting flow {} for switch {}", entryName, dpid);
         }
         if (dpid == null) {
-            log.error("inconsistent internal state: no switch has rule {}",
-                    entryName);
-            return false;
+            // assume state has been cleared by deleteFlowsForSwitch() or
+            // deleteAllFlows()
+            return;
         }
         
         // send flow_mod delete
@@ -423,11 +416,11 @@ public class StaticFlowEntryPusher
         } else { 
             log.debug("Tried to delete non-existent entry {} for switch {}", 
                     entryName, dpid);
-            return false;
+            return;
         }
         
         writeFlowModToSwitch(HexString.toLong(dpid), flowMod);
-        return true;
+        return;
     }
     
     /**
@@ -499,6 +492,29 @@ public class StaticFlowEntryPusher
         return StaticFlowName;
     }
     
+    public Command handleFlowRemoved(IOFSwitch sw, OFMessage msg, FloodlightContext cntx) {
+        OFFlowRemoved flowRemoved = (OFFlowRemoved) msg;
+        long cookie = flowRemoved.getCookie();
+        /**
+         * This is just to sanity check our assumption that static flows 
+         * never expire.
+         */
+        if (AppCookie.extractApp(cookie) == STATIC_FLOW_APP_ID) {
+            if (flowRemoved.getReason() != 
+                    OFFlowRemoved.OFFlowRemovedReason.OFPRR_DELETE)
+                log.error("Got a FlowRemove message for a infinite " +
+                          "timeout flow: {} from switch {}", msg, sw);
+            return Command.STOP;    // only for us
+        }
+        
+        return Command.CONTINUE;
+    }
+    
+    public Command handleBarrierReply(IOFSwitch sw, OFMessage msg, FloodlightContext cntx) {
+        
+        return Command.CONTINUE;
+    }
+    
     @Override
     @LogMessageDoc(level="ERROR",
         message="Got a FlowRemove message for a infinite " +
@@ -509,24 +525,12 @@ public class StaticFlowEntryPusher
     public Command receive(IOFSwitch sw, OFMessage msg, FloodlightContext cntx) {
         switch (msg.getType()) {
         case FLOW_REMOVED:
-            break;
+            return handleFlowRemoved(sw, msg, cntx);
+        case BARRIER_REPLY:
+            return handleBarrierReply(sw, msg, cntx);
         default:
             return Command.CONTINUE;
         }
-        OFFlowRemoved flowRemoved = (OFFlowRemoved) msg;
-        long cookie = flowRemoved.getCookie();
-        /**
-         * This is just to sanity check our assumption that static flows 
-         * never expire.
-         */
-        if( AppCookie.extractApp(cookie) == STATIC_FLOW_APP_ID) {
-            if (flowRemoved.getReason() != 
-                    OFFlowRemoved.OFFlowRemovedReason.OFPRR_DELETE)
-                log.error("Got a FlowRemove message for a infinite " +
-                		  "timeout flow: {} from switch {}", msg, sw);
-            return Command.STOP;    // only for us
-        } else
-            return Command.CONTINUE;
     }
 
     @Override
@@ -583,6 +587,7 @@ public class StaticFlowEntryPusher
     @Override
     public void startUp(FloodlightModuleContext context) {        
         floodlightProvider.addOFMessageListener(OFType.FLOW_REMOVED, this);
+        floodlightProvider.addOFMessageListener(OFType.BARRIER_REPLY, this);
         floodlightProvider.addOFSwitchListener(this);
         floodlightProvider.addHAListener(this);
         
@@ -613,23 +618,88 @@ public class StaticFlowEntryPusher
     @Override
     public void deleteFlow(String name) {
         storageSource.deleteRowAsync(TABLE_NAME, name);
-        // TODO - What if there is a delay in storage?
     }
     
     @Override
     public void deleteAllFlows() {
+        // Send a delete for each switch
+        for (String dpid : entry2dpid.values()) {
+            sendDeleteByCookie(HexString.toLong(dpid));
+        }
+        
+        // Delete it from the DB
         for (String entry : entry2dpid.keySet()) {
             deleteFlow(entry);
         }
+        
+        // Clear our map
+        entry2dpid.clear();
+        
+        // Clear our book keeping map
+        for (Map<String, OFFlowMod> eMap : entriesFromStorage.values()) {
+            eMap.clear();
+        }
+        
     }
     
     @Override
     public void deleteFlowsForSwitch(long dpid) {
         String sDpid = HexString.toHexString(dpid);
+        sendDeleteByCookie(dpid);
+        
+        // Clear all internal flows for this switch
+        Map<String, OFFlowMod> sMap = entriesFromStorage.get(sDpid);
+        if (sMap != null) {
+            for (String entryName : sMap.keySet()) {
+                entry2dpid.remove(entryName);
+                // Delete from DB
+                deleteFlow(entryName);
+            }
+            sMap.clear();
+        } else {
+            log.warn("Map of storage entries for switch {} was null", sDpid);
+        }
+    }
+    
+    /**
+     * Deletes all flows installed by static flow pusher on a given switch.
+     * We send a delete flow mod with the static flow pusher app ID in the cookie.
+     * Since OF1.0 doesn't support masking based on the cookie we have to 
+     * disable having flow specific cookies.
+     * @param dpid
+     */
+    public void sendDeleteByCookie(long dpid) {
+        if (log.isDebugEnabled())
+            log.debug("Delete all static flows on switch {}",
+                    HexString.toHexString(dpid));
         
-        for (Entry<String, String> e : entry2dpid.entrySet()) {
-            if (e.getValue().equals(sDpid))
-                deleteFlow(e.getKey());
+        IOFSwitch sw = floodlightProvider.getSwitches().get(dpid);
+        if (sw == null) {
+            log.warn("Tried to delete static flows for non-existant switch {}",
+                    HexString.toHexString(dpid));
+            return;
+        }
+        
+        OFFlowMod fm = (OFFlowMod) floodlightProvider.getOFMessageFactory().
+                getMessage(OFType.FLOW_MOD);
+        OFMatch ofm = new OFMatch();
+        fm.setMatch(ofm);
+        fm.setCookie(AppCookie.makeCookie(StaticFlowEntryPusher.STATIC_FLOW_APP_ID, 0));
+        fm.setCommand(OFFlowMod.OFPFC_DELETE);
+        
+        OFBarrierRequest ofbr = (OFBarrierRequest) floodlightProvider.getOFMessageFactory().
+                getMessage(OFType.BARRIER_REQUEST);
+        
+        List<OFMessage> mList = new ArrayList<OFMessage>(2);
+        mList.add(fm);
+        mList.add(ofbr);
+        try {
+            sw.write(mList, null);
+            sw.flush();
+        } catch (IOException e1) {
+            log.error("Error deleting all flows for switch {}:\n {}", 
+                    HexString.toHexString(dpid), e1.getMessage());
+            return;
         }
     }
     
@@ -675,5 +745,4 @@ public class StaticFlowEntryPusher
             Map<String, String> removedControllerNodeIPs) {
         // ignore
     }
-     
 }
-- 
GitLab