From a16fd710c633de1336bcf45d90c25e6471a6ffbd Mon Sep 17 00:00:00 2001
From: Alex Reimers <alex@bigswitch.com>
Date: Thu, 30 Aug 2012 17:10:02 -0700
Subject: [PATCH] FL-82 - Simply enabling/disabling the firewall and log it.

---
 .../firewall/Firewall.java                    | 25 +++++++------------
 .../firewall/FirewallResource.java            |  4 +--
 .../firewall/IFirewallService.java            | 10 +++-----
 .../firewall/FirewallTest.java                | 14 +++++------
 4 files changed, 21 insertions(+), 32 deletions(-)

diff --git a/src/main/java/net/floodlightcontroller/firewall/Firewall.java b/src/main/java/net/floodlightcontroller/firewall/Firewall.java
index bdb5becf6..d6225b23c 100644
--- a/src/main/java/net/floodlightcontroller/firewall/Firewall.java
+++ b/src/main/java/net/floodlightcontroller/firewall/Firewall.java
@@ -263,25 +263,18 @@ public class Firewall implements IFirewallService, IOFMessageListener, IFloodlig
     }
 
     @Override
-    public void enableFirewall() {
-        // check if the firewall module is not listening for events, if not, then start listening (enable it)
+    public void enableFirewall(boolean enabled) {
+        logger.info("Setting firewall to {}", enabled);
+        this.enabled = enabled;
+        // add/remove ourself as a packetin listener
         List<IOFMessageListener> listeners = floodlightProvider.getListeners().get(OFType.PACKET_IN);
         if ((listeners != null) && (!listeners.contains(this))) {
-            // enable firewall, i.e. listen for packet-in events
-            floodlightProvider.addOFMessageListener(OFType.PACKET_IN, this);
-        }
-        this.enabled = true;
-    }
-
-    @Override
-    public void disableFirewall() {
-        // check if the firewall module is listening for events, if yes, then remove it from listeners (disable it)
-        List<IOFMessageListener> listeners = floodlightProvider.getListeners().get(OFType.PACKET_IN);
-        if ((listeners != null) && (listeners.contains(this))) {
-            // disable firewall, i.e. stop listening for packet-in events
-            floodlightProvider.removeOFMessageListener(OFType.PACKET_IN, this);
+            if (enabled) {
+                floodlightProvider.addOFMessageListener(OFType.PACKET_IN, this);
+            } else {
+                floodlightProvider.removeOFMessageListener(OFType.PACKET_IN, this);
+            }
         }
-        this.enabled = false;
     }
 
     @Override
diff --git a/src/main/java/net/floodlightcontroller/firewall/FirewallResource.java b/src/main/java/net/floodlightcontroller/firewall/FirewallResource.java
index e269414e6..5ef403fd4 100644
--- a/src/main/java/net/floodlightcontroller/firewall/FirewallResource.java
+++ b/src/main/java/net/floodlightcontroller/firewall/FirewallResource.java
@@ -23,10 +23,10 @@ public class FirewallResource extends ServerResource {
                 get(IFirewallService.class.getCanonicalName());
 
         if (op.equalsIgnoreCase("enable")) {
-            firewall.enableFirewall();
+            firewall.enableFirewall(true);
             return "{\"status\" : \"success\", \"details\" : \"firewall running\"}";
         } else if (op.equalsIgnoreCase("disable")) {
-            firewall.disableFirewall();
+            firewall.enableFirewall(false);
             return "{\"status\" : \"success\", \"details\" : \"firewall stopped\"}";
         } else if (op.equalsIgnoreCase("storageRules")) {
             return firewall.getStorageRules();
diff --git a/src/main/java/net/floodlightcontroller/firewall/IFirewallService.java b/src/main/java/net/floodlightcontroller/firewall/IFirewallService.java
index aa846c9a6..a70381505 100644
--- a/src/main/java/net/floodlightcontroller/firewall/IFirewallService.java
+++ b/src/main/java/net/floodlightcontroller/firewall/IFirewallService.java
@@ -8,14 +8,10 @@ import net.floodlightcontroller.core.module.IFloodlightService;
 public interface IFirewallService extends IFloodlightService {
 
     /**
-     * Enables the Firewall module
+     * Enables/disables the firewall.
+     * @param enable Whether to enable or disable the firewall.
      */
-    public void enableFirewall();
-
-    /**
-     * Disables the Firewall module
-     */
-    public void disableFirewall();
+    public void enableFirewall(boolean enable);
 
     /**
      * Returns all of the firewall rules
diff --git a/src/test/java/net/floodlightcontroller/firewall/FirewallTest.java b/src/test/java/net/floodlightcontroller/firewall/FirewallTest.java
index 43cf08f0f..1944eb126 100644
--- a/src/test/java/net/floodlightcontroller/firewall/FirewallTest.java
+++ b/src/test/java/net/floodlightcontroller/firewall/FirewallTest.java
@@ -209,7 +209,7 @@ public class FirewallTest extends FloodlightTestCase {
     @Test
     public void testNoRules() throws Exception {
         // enable firewall first
-        firewall.enableFirewall();
+        firewall.enableFirewall(true);
         // simulate a packet-in event
         this.setPacketIn(tcpPacket);
         firewall.receive(sw, this.packetIn, cntx);
@@ -322,7 +322,7 @@ public class FirewallTest extends FloodlightTestCase {
     @Test
     public void testSimpleAllowRule() throws Exception {
         // enable firewall first
-        firewall.enableFirewall();
+        firewall.enableFirewall(true);
 
         // add TCP rule
         FirewallRule rule = new FirewallRule();
@@ -360,7 +360,7 @@ public class FirewallTest extends FloodlightTestCase {
 
     @Test
     public void testOverlappingRules() throws Exception {
-        firewall.enableFirewall();
+        firewall.enableFirewall(true);
 
         // add TCP port 80 (destination only) allow rule
         FirewallRule rule = new FirewallRule();
@@ -403,7 +403,7 @@ public class FirewallTest extends FloodlightTestCase {
     @Test
     public void testARP() throws Exception {
         // enable firewall first
-        firewall.enableFirewall();
+        firewall.enableFirewall(true);
 
         // no rules inserted so all traffic other than broadcast and ARP-request-broadcast should be blocked
 
@@ -434,7 +434,7 @@ public class FirewallTest extends FloodlightTestCase {
     @Test
     public void testIPBroadcast() throws Exception {
         // enable firewall first
-        firewall.enableFirewall();
+        firewall.enableFirewall(true);
         
         // set subnet mask for IP broadcast
         firewall.setSubnetMask("255.255.255.0");
@@ -455,7 +455,7 @@ public class FirewallTest extends FloodlightTestCase {
     @Test
     public void testMalformedIPBroadcast() throws Exception {
         // enable firewall first
-        firewall.enableFirewall();
+        firewall.enableFirewall(true);
 
         // no rules inserted so all traffic other than broadcast and ARP-request-broadcast should be blocked
 
@@ -473,7 +473,7 @@ public class FirewallTest extends FloodlightTestCase {
     @Test
     public void testLayer2Rule() throws Exception {
         // enable firewall first
-        firewall.enableFirewall();
+        firewall.enableFirewall(true);
 
         // add L2 rule
         FirewallRule rule = new FirewallRule();
-- 
GitLab