From 6c793e7e369978867a8c6e2f3c6537ee2c923ba4 Mon Sep 17 00:00:00 2001
From: Randy Sharo <rsharo@users.noreply.github.com>
Date: Wed, 13 Jul 2016 18:42:21 -0400
Subject: [PATCH] Changed ArrayList<> types to ImmutableList<> where
 applicable. OpenFlow messages are now cached in
 Forwarding.deleteFlowsByDescriptor().

---
 .../firewall/Firewall.java                    | 24 +++++------
 .../forwarding/Forwarding.java                | 41 +++++++++++--------
 2 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/src/main/java/net/floodlightcontroller/firewall/Firewall.java b/src/main/java/net/floodlightcontroller/firewall/Firewall.java
index 6c01ed1f2..4ecc605c4 100644
--- a/src/main/java/net/floodlightcontroller/firewall/Firewall.java
+++ b/src/main/java/net/floodlightcontroller/firewall/Firewall.java
@@ -70,6 +70,8 @@ import net.floodlightcontroller.storage.StorageException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.ImmutableList;
+
 /**
  * Stateless firewall implemented as a Google Summer of Code project.
  * Configuration done through REST API
@@ -340,14 +342,14 @@ IFloodlightModule {
 
 	@Override
 	public void enableFirewall(boolean enabled) {
-		
 		if(this.enabled != enabled) {
 			logger.info("Setting firewall to {}", enabled);
 			this.enabled = enabled;
-		
-			List<Masked<U64>> changes = new ArrayList<Masked<U64>>();
-			changes.add(Masked.of(DEFAULT_COOKIE, AppCookie.getAppFieldMask()));
-		
+
+			List<Masked<U64>> changes = ImmutableList.of(
+						Masked.of(DEFAULT_COOKIE, AppCookie.getAppFieldMask())
+					);
+
 			// Add announcement that all firewall decisions changed
 			routingService.handleRoutingDecisionChange(changes);
 		}
@@ -440,17 +442,16 @@ IFloodlightModule {
 		storageSource.insertRow(TABLE_NAME, entry);
 		
 		U64 singleRuleMask = AppCookie.getAppFieldMask().or(AppCookie.getUserFieldMask());
-		List<Masked<U64>> changes = new ArrayList<Masked<U64>>();
+		ImmutableList.Builder<Masked<U64>> changesBuilder = ImmutableList.builder();
 		Iterator<FirewallRule> iter = this.rules.iterator();
 		while (iter.hasNext()) {
 			FirewallRule r = iter.next();
 			if (r.priority >= rule.priority) {
-				// 
-				changes.add(Masked.of(AppCookie.makeCookie(APP_ID, r.ruleid), singleRuleMask));
+				changesBuilder.add(Masked.of(AppCookie.makeCookie(APP_ID, r.ruleid), singleRuleMask));
 			}
 		}
-		changes.add(Masked.of(RULE_MISS_COOKIE, singleRuleMask));
-		routingService.handleRoutingDecisionChange(changes);
+		changesBuilder.add(Masked.of(RULE_MISS_COOKIE, singleRuleMask));
+		routingService.handleRoutingDecisionChange(changesBuilder.build());
 	}
 
 	@Override
@@ -472,8 +473,7 @@ IFloodlightModule {
 				AppCookie.makeCookie(APP_ID, ruleid),
 				AppCookie.getAppFieldMask().or(AppCookie.getUserFieldMask()));
 		
-		List<Masked<U64>> changes = new ArrayList<Masked<U64>>();
-		changes.add(delDescriptor);
+		List<Masked<U64>> changes = ImmutableList.of(delDescriptor);
 		
 		//Add announcement that rule is added
 		// should we try to delete the flow even if not found in this.rules
diff --git a/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java b/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java
index e7d3699c5..87ffc5500 100644
--- a/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java
+++ b/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java
@@ -86,6 +86,8 @@ import org.projectfloodlight.openflow.types.OFVlanVidMatch;
 import org.projectfloodlight.openflow.types.TableId;
 import org.projectfloodlight.openflow.types.U64;
 import org.projectfloodlight.openflow.types.VlanVid;
+import org.python.google.common.collect.ImmutableList;
+import org.python.google.common.collect.Maps;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -179,18 +181,18 @@ public class Forwarding extends ForwardingBase implements IFloodlightModule, IOF
 	 * @return A collection of masked cookies suitable for flow-mod operations
 	 */
 	protected Collection<Masked<U64>> convertRoutingDecisionDescriptors(Iterable<Masked<U64>> maskedDescriptors) {
-		if(maskedDescriptors == null){
+		if(maskedDescriptors == null) {
 			return null;
 		}
-		List<Masked<U64>> result = new ArrayList<Masked<U64>>();
 
+		ImmutableList.Builder<Masked<U64>> resultBuilder = ImmutableList.builder();
 		for (Masked<U64> maskedDescriptor : maskedDescriptors) {
 			long user_mask = AppCookie.extractUser(maskedDescriptor.getMask()) & DECISION_MASK;
 			long user_value = AppCookie.extractUser(maskedDescriptor.getValue()) & user_mask;
 
 			// TODO combine in any other cookie fields you need here.
 
-			result.add(
+			resultBuilder.add(
 					Masked.of(
 							AppCookie.makeCookie(FORWARDING_APP_ID, (int)user_value),
 							AppCookie.getAppFieldMask().or(U64.of(user_mask))
@@ -198,7 +200,7 @@ public class Forwarding extends ForwardingBase implements IFloodlightModule, IOF
 			);
 		}
 
-		return result;
+		return resultBuilder.build();
 	}
 
 	/**
@@ -211,25 +213,32 @@ public class Forwarding extends ForwardingBase implements IFloodlightModule, IOF
 		Collection<Masked<U64>> masked_cookies = convertRoutingDecisionDescriptors(descriptors);
 
 		if (masked_cookies != null && !masked_cookies.isEmpty()) {
+			Map<OFVersion, List<OFMessage>> cache = Maps.newHashMap();
+
 			for (DatapathId dpid : switchService.getAllSwitchDpids()) {
 				IOFSwitch sw = switchService.getActiveSwitch(dpid);
 				if (sw == null) {
 					continue;
 				}
 
-				// Would like to swap these for loops and only build the message set once,
-				// but doing so would assume all switches are using the same OF protocol version.
-				List<OFMessage> msgs = new ArrayList<OFMessage>();
-				for (Masked<U64> masked_cookie : masked_cookies) {
-					msgs.add(
-						sw.getOFFactory().buildFlowDelete()
-							.setCookie(masked_cookie.getValue())
-							.setCookieMask(masked_cookie.getMask())
-								.build()
-						);
+				OFVersion ver = sw.getOFFactory().getVersion();
+				if (cache.containsKey(ver)) {
+					sw.write(cache.get(ver));
+				} else {
+					ImmutableList.Builder<OFMessage> msgsBuilder = ImmutableList.builder();
+					for (Masked<U64> masked_cookie : masked_cookies) {
+						msgsBuilder.add(
+							sw.getOFFactory().buildFlowDelete()
+								.setCookie(masked_cookie.getValue())
+								.setCookieMask(masked_cookie.getMask())
+									.build()
+							);
+					}
+
+					List<OFMessage> msgs = msgsBuilder.build();
+					sw.write(msgs);
+					cache.put(ver, msgs);
 				}
-
-				sw.write(msgs);
 			}
 		}
 	}
-- 
GitLab