Logging improvements to improve efficiency - specifically in critical path. 38/338/1
authorMadhavan Kasthurirangan <mkasthur@cisco.com>
Mon, 13 May 2013 21:29:21 +0000 (14:29 -0700)
committerMadhavan Kasthurirangan <mkasthur@cisco.com>
Mon, 13 May 2013 21:29:21 +0000 (14:29 -0700)
Signed-off-by: Madhavan Kasthurirangan <mkasthur@cisco.com>
14 files changed:
opendaylight/arphandler/src/main/java/org/opendaylight/controller/arphandler/internal/ArpHandler.java
opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTracker.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/MessageReadWriteService.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SecureMessageReadWriteService.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/DataPacketMuxDemux.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/OFStatisticsManager.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/vendorextension/v6extension/V6FlowMod.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/vendorextension/v6extension/V6Match.java
opendaylight/routing/dijkstra_implementation/src/main/java/org/opendaylight/controller/routing/dijkstra_implementation/internal/DijkstraImplementation.java
opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/packet/LLDP.java
opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/packet/Packet.java
opendaylight/samples/simpleforwarding/src/main/java/org/opendaylight/controller/samples/simpleforwarding/internal/SimpleForwardingImpl.java

index 3730a71..26a86c5 100644 (file)
@@ -156,9 +156,11 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
          * This is to avoid continuous flooding
          */
         if (Arrays.equals(sourceMAC, getControllerMAC())) {
-            logger.debug(
+            if (logger.isDebugEnabled()) {
+              logger.debug(
                     "Receive the self originated packet (srcMAC {}) --> DROP",
                     HexEncode.bytesToHexString(sourceMAC));
+            }
             return;
         }
 
@@ -167,18 +169,16 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
             subnet = switchManager.getSubnetByNetworkAddress(sourceIP);
         }
         if (subnet == null) {
-            logger.debug("can't find subnet matching {}, drop packet", sourceIP
-                    .toString());
+            logger.debug("can't find subnet matching {}, drop packet",sourceIP);
             return;
         }
-        logger.debug("Found {} matching {}", subnet.toString(), sourceIP
-                .toString());
+        logger.debug("Found {} matching {}", subnet, sourceIP);
         /*
          * Make sure that the host is a legitimate member of this subnet
          */
         if (!subnet.hasNodeConnector(p)) {
             logger.debug("{} showing up on {} does not belong to {}",
-                    new Object[] { sourceIP.toString(), p, subnet.toString() });
+                    new Object[] { sourceIP, p, subnet });
             return;
         }
 
@@ -364,18 +364,17 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
     }
 
     public void find(InetAddress networkAddress) {
-        logger.debug("Received find IP {}", networkAddress.toString());
+        logger.debug("Received find IP {}", networkAddress);
 
         Subnet subnet = null;
         if (switchManager != null) {
             subnet = switchManager.getSubnetByNetworkAddress(networkAddress);
         }
         if (subnet == null) {
-            logger.debug("can't find subnet matching IP {}", networkAddress
-                    .toString());
+            logger.debug("can't find subnet matching IP {}", networkAddress);
             return;
         }
-        logger.debug("found subnet {}", subnet.toString());
+        logger.debug("found subnet {}", subnet);
 
         // send a broadcast ARP Request to this interface
         sendBcastARPRequest(networkAddress, subnet);
@@ -394,7 +393,7 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
         }
         if (subnet == null) {
             logger.debug("can't find subnet matching {}", host
-                    .getNetworkAddress().toString());
+                    .getNetworkAddress());
             return;
         }
         sendUcastARPRequest(host, subnet);
@@ -419,11 +418,10 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
             subnet = switchManager.getSubnetByNetworkAddress(dIP);
         }
         if (subnet == null) {
-            logger.debug("can't find subnet matching {}, drop packet", dIP
-                    .toString());
+            logger.debug("can't find subnet matching {}, drop packet", dIP);
             return;
         }
-        logger.debug("Found {} matching {}", subnet.toString(), dIP.toString());
+        logger.debug("Found {} matching {}", subnet, dIP);
         /*
          * unknown destination host, initiate ARP request
          */
index f416f29..f5123eb 100644 (file)
@@ -394,8 +394,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
         arphost.setHostIP(networkAddr);
         arphost.setSent_count((short) 1);
         ARPPendingList.add(arphost);
-        logger.debug("Host Added to ARPPending List, IP: {}",
-                networkAddr.toString());
+        logger.debug("Host Added to ARPPending List, IP: {}", networkAddr);
     }
 
     private void removePendingARPFromList(int index) {
@@ -434,7 +433,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
                  */
                 removePendingARPFromList(i);
                 logger.debug("Host Removed from ARPPending List, IP: {}",
-                        networkAddr.toString());
+                          networkAddr);
                 return;
             }
         }
@@ -452,7 +451,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
                  */
                 failedARPReqList.remove(i);
                 logger.debug("Host Removed from FailedARPReqList List, IP: {}",
-                        networkAddr.toString());
+                        networkAddr);
                 return;
             }
         }
@@ -708,7 +707,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
             for (String switchName : hierarchy) {
                 buf.append(switchName + "/");
             }
-            logger.debug("{} -> {}", getContainerName(), buf.toString());
+            logger.debug("{} -> {}", getContainerName(), buf);
             num++;
         }
     }
@@ -991,13 +990,15 @@ public class HostTracker implements IfIptoHost, IfHostListener,
                      * Use the services of arphandler to check if host is still
                      * there
                      */
-                    logger.trace(
-                            "ARP Probing ({}) for {}({})",
-                            new Object[] {
-                                    arp_cntdown,
-                                    host.getNetworkAddress().getHostAddress(),
-                                    HexEncode.bytesToHexString(host
-                                            .getDataLayerAddressBytes()) });
+                    if (logger.isTraceEnabled()) {
+                      logger.trace(
+                              "ARP Probing ({}) for {}({})",
+                              new Object[] {
+                                      arp_cntdown,
+                                      host.getNetworkAddress().getHostAddress(),
+                                      HexEncode.bytesToHexString(host
+                                              .getDataLayerAddressBytes()) });
+                    }
                     host.setArpSendCountDown(arp_cntdown);
                     hostFinder.probe(host);
                 }
@@ -1160,8 +1161,10 @@ public class HostTracker implements IfIptoHost, IfHostListener,
         switch (type) {
         case REMOVED:
             long sid = (Long) node.getID();
-            logger.debug("Received removedSwitch for sw id {}",
-                    HexEncode.longToHexString(sid));
+            if (logger.isDebugEnabled()) {
+              logger.debug("Received removedSwitch for sw id {}",
+                      HexEncode.longToHexString(sid));
+            }
             for (Entry<InetAddress, HostNodeConnector> entry : hostsDB
                     .entrySet()) {
                 HostNodeConnector host = entry.getValue();
index bfa6f0b..2863070 100644 (file)
@@ -70,7 +70,7 @@ public class Controller implements IController, CommandProvider {
                         ISwitch existingSwitch = switches.get(sid);
                         if (existingSwitch != null) {
                             logger.info("Replacing existing {} with New {}",
-                                    existingSwitch.toString(), sw.toString());
+                                    existingSwitch, sw);
                             disconnectSwitch(existingSwitch);
                         }
                         switches.put(sid, sw);
@@ -175,20 +175,18 @@ public class Controller implements IController, CommandProvider {
     public void addMessageListener(OFType type, IMessageListener listener) {
         IMessageListener currentListener = this.messageListeners.get(type);
         if (currentListener != null) {
-            logger.warn("{} is already listened by {}", type.toString(),
-                    currentListener.toString());
+            logger.warn("{} is already listened by {}", type,
+                    currentListener);
         }
         this.messageListeners.put(type, listener);
-        logger.debug("{} is now listened by {}", type.toString(),
-                listener.toString());
+        logger.debug("{} is now listened by {}", type, listener);
     }
 
     @Override
     public void removeMessageListener(OFType type, IMessageListener listener) {
         IMessageListener currentListener = this.messageListeners.get(type);
         if ((currentListener != null) && (currentListener == listener)) {
-            logger.debug("{} listener {} is Removed", type.toString(),
-                    listener.toString());
+            logger.debug("{} listener {} is Removed", type, listener);
             this.messageListeners.remove(type);
         }
     }
@@ -197,19 +195,17 @@ public class Controller implements IController, CommandProvider {
     public void addSwitchStateListener(ISwitchStateListener listener) {
         if (this.switchStateListener != null) {
             logger.warn("Switch events are already listened by {}",
-                    this.switchStateListener.toString());
+                    this.switchStateListener);
         }
         this.switchStateListener = listener;
-        logger.debug("Switch events are now listened by {}",
-                listener.toString());
+        logger.debug("Switch events are now listened by {}", listener);
     }
 
     @Override
     public void removeSwitchStateListener(ISwitchStateListener listener) {
         if ((this.switchStateListener != null)
                 && (this.switchStateListener == listener)) {
-            logger.debug("SwitchStateListener {} is Removed",
-                    listener.toString());
+            logger.debug("SwitchStateListener {} is Removed", listener);
             this.switchStateListener = null;
         }
     }
@@ -242,7 +238,7 @@ public class Controller implements IController, CommandProvider {
         if (((SwitchHandler) sw).isOperational()) {
             Long sid = sw.getId();
             if (this.switches.remove(sid, sw)) {
-                logger.warn("{} is Disconnected", sw.toString());
+                logger.warn("{} is Disconnected", sw);
                 notifySwitchDeleted(sw);
             }
         }
index 3dd99e6..d2dab0f 100644 (file)
@@ -84,7 +84,7 @@ public class MessageReadWriteService implements IMessageReadWrite {
                 this.clientSelectionKey = this.socket.register(this.selector,
                         SelectionKey.OP_WRITE, this);
             }
-            logger.trace("Message sent: {}", msg.toString());
+            logger.trace("Message sent: {}", msg);
         }
     }
 
index 9a05c3f..91606f4 100644 (file)
@@ -320,13 +320,13 @@ public class SwitchHandler implements ISwitch {
         }
 
         if (msgs == null) {
-            logger.debug("{} is down", toString());
+            logger.debug("{} is down", this);
             // the connection is down, inform core
             reportSwitchStateChange(false);
             return;
         }
         for (OFMessage msg : msgs) {
-            logger.trace("Message received: {}", msg.toString());
+            logger.trace("Message received: {}", msg);
             this.lastMsgReceivedTimeStamp = System.currentTimeMillis();
             OFType type = msg.getType();
             switch (type) {
@@ -418,7 +418,7 @@ public class SwitchHandler implements ISwitch {
                             // send a probe to see if the switch is still alive
                             logger.debug(
                                     "Send idle probe (Echo Request) to {}",
-                                    toString());
+                                    this);
                             probeSent = true;
                             OFMessage echo = factory
                                     .getMessage(OFType.ECHO_REQUEST);
@@ -462,7 +462,9 @@ public class SwitchHandler implements ISwitch {
                 || e instanceof InterruptedException
                 || e instanceof SocketException || e instanceof IOException
                 || e instanceof ClosedSelectorException) {
-            logger.debug("Caught exception {}", e.getMessage());
+            if (logger.isDebugEnabled()) {
+              logger.debug("Caught exception {}", e.getMessage());
+            }
         } else {
             logger.warn("Caught exception ", e);
         }
@@ -729,7 +731,7 @@ public class SwitchHandler implements ISwitch {
                     if (!transmitQ.isEmpty()) {
                         PriorityMessage pmsg = transmitQ.poll();
                         msgReadWriteService.asyncSend(pmsg.msg);
-                        logger.trace("Message sent: {}", pmsg.toString());
+                        logger.trace("Message sent: {}", pmsg);
                         /*
                          * If syncReply is set to true, wait for the response
                          * back.
@@ -883,8 +885,10 @@ public class SwitchHandler implements ISwitch {
                 // if result is not null, this means the switch can't handle
                 // this message
                 // the result if OFError already
-                logger.debug("Send {} failed --> {}", msg.getType().toString(),
-                        ((OFError) result).toString());
+                if (logger.isDebugEnabled()) {
+                  logger.debug("Send {} failed --> {}", msg.getType(),
+                               ((OFError) result));
+                }
             }
             return result;
         } catch (Exception e) {
index a8ebcb0..7614a4d 100644 (file)
@@ -195,13 +195,15 @@ public class DataPacketMuxDemux implements IContainerListener,
                         .get(GlobalConstants.DEFAULT.toString());
                 if (defaultOutService != null) {
                     defaultOutService.receiveDataPacket(dataPacket);
-                    logger.trace(
-                            "Dispatched to apps a frame of size: {} on container: {}: {}",
-                            new Object[] {
+                    if (logger.isTraceEnabled()) {
+                      logger.trace(
+                              "Dispatched to apps a frame of size: {} on " +
+                              "container: {}: {}", new Object[] {
                                     ofPacket.getPacketData().length,
                                     GlobalConstants.DEFAULT.toString(),
                                     HexEncode.bytesToHexString(dataPacket
                                             .getPacketData()) });
+                    }
                 }
                 // Now check the mapping between nodeConnector and
                 // Container and later on optimally filter based on
@@ -215,15 +217,15 @@ public class DataPacketMuxDemux implements IContainerListener,
                         if (s != null) {
                             // TODO add filtering on a per-flowSpec base
                             s.receiveDataPacket(dataPacket);
-                            logger.trace(
-                                    "Dispatched to apps a frame of size: {} on container: {}: {}",
-                                    new Object[] {
+                            if (logger.isTraceEnabled()) {
+                              logger.trace(
+                                      "Dispatched to apps a frame of size: {}" +
+                                      " on container: {}: {}", new Object[] {
                                             ofPacket.getPacketData().length,
-                                            GlobalConstants.DEFAULT.toString(),
-                                            HexEncode
-                                                    .bytesToHexString(dataPacket
-                                                            .getPacketData()) });
-
+                                            container,
+                                            HexEncode.bytesToHexString(dataPacket
+                                                    .getPacketData()) });
+                            }
                         }
                     }
                 }
index 9dc8b3b..d6100e3 100644 (file)
@@ -176,7 +176,7 @@ public class OFStatisticsManager implements IOFStatisticsManager,
         descriptionListeners = new HashSet<IStatisticsListener>();
 
         configStatsPollIntervals();
-        
+
         // Initialize managed timers
         statisticsTimer = new Timer();
         statisticsTimerTask = new TimerTask() {
@@ -703,8 +703,10 @@ public class OFStatisticsManager implements IOFStatisticsManager,
         ByteBuffer data = ByteBuffer.allocate(length);
         stat.writeTo(data);
         data.rewind();
-        log.trace("getV6ReplyStatistics: Buffer BYTES ARE {}",
-                HexString.toHexString(data.array()));
+        if (log.isTraceEnabled()) {
+            log.trace("getV6ReplyStatistics: Buffer BYTES ARE {}",
+                    HexString.toHexString(data.array()));
+        }
 
         int vendor = data.getInt(); // first 4 bytes is vendor id.
         if (vendor != V6StatsRequest.NICIRA_VENDOR_ID) {
index 9921e82..2da6b9b 100644 (file)
@@ -674,7 +674,7 @@ public class V6Match extends OFMatch implements Cloneable {
                 data.put(ipv6ext_dstport_msg);
             }
         }
-        logger.trace("{}", this.toString());
+        logger.trace("{}", this);
     }
 
     private void readInPort(ByteBuffer data, int nxmLen, boolean hasMask) {
index d878b23..26ae862 100644 (file)
@@ -184,16 +184,14 @@ public class DijkstraImplementation implements IRouting, ITopologyManagerAware {
         try {
             path = mtp.getMaxThroughputPath(src, dst);
         } catch (IllegalArgumentException ie) {
-            log.debug("A vertex is yet not known between {} {}",
-                    src.toString(), dst.toString());
+            log.debug("A vertex is yet not known between {} {}", src, dst);
             return null;
         }
         Path res;
         try {
             res = new Path(path);
         } catch (ConstructionException e) {
-            log.debug("A vertex is yet not known between {} {}",
-                    src.toString(), dst.toString());
+            log.debug("A vertex is yet not known between {} {}", src, dst);
             return null;
         }
         return res;
@@ -208,16 +206,14 @@ public class DijkstraImplementation implements IRouting, ITopologyManagerAware {
         try {
             path = spt.getPath(src, dst);
         } catch (IllegalArgumentException ie) {
-            log.debug("A vertex is yet not known between {} {}",
-                    src.toString(), dst.toString());
+            log.debug("A vertex is yet not known between {} {}", src, dst);
             return null;
         }
         Path res;
         try {
             res = new Path(path);
         } catch (ConstructionException e) {
-            log.debug("A vertex is yet not known between {} {}",
-                    src.toString(), dst.toString());
+            log.debug("A vertex is yet not known between {} {}", src, dst);
             return null;
         }
         return res;
@@ -343,7 +339,9 @@ public class DijkstraImplementation implements IRouting, ITopologyManagerAware {
         if (props != null)
             props.remove(bw);
 
-        log.debug("edgeUpdate: {} bw: {}", e.toString(), bw.getValue());
+        if (log.isDebugEnabled()) {
+          log.debug("edgeUpdate: {} bw: {}", e, bw.getValue());
+        }
 
         Short baseBW = Short.valueOf((short) 0);
         boolean add = (type == UpdateType.ADDED) ? true : false;
index 5e7a214..562d03b 100644 (file)
@@ -171,8 +171,10 @@ public class LLDP extends Packet {
         int lldpOffset = bitOffset; // LLDP start
         int lldpSize = size; // LLDP size
 
-        logger.trace("LLDP: {} (offset {} bitsize {})", new Object[] {
-                HexEncode.bytesToHexString(data), lldpOffset, lldpSize });
+        if (logger.isTraceEnabled()) {
+          logger.trace("LLDP: {} (offset {} bitsize {})", new Object[] {
+                  HexEncode.bytesToHexString(data), lldpOffset, lldpSize });
+        }
         /*
          * Deserialize the TLVs until we reach the end of the packet
          */
@@ -212,8 +214,10 @@ public class LLDP extends Packet {
             throw new PacketException(e.getMessage());
         }
 
-        logger.trace("LLDP: serialized: {}",
-                HexEncode.bytesToHexString(serializedBytes));
+        if (logger.isTraceEnabled()) {
+          logger.trace("LLDP: serialized: {}",
+                  HexEncode.bytesToHexString(serializedBytes));
+        }
         return serializedBytes;
     }
 
index 95f0ca8..446ec3e 100644 (file)
@@ -109,10 +109,12 @@ public abstract class Packet {
              * Store the raw read value, checks the payload type and set the
              * payloadClass accordingly
              */
-            logger.trace("{}: {}: {} (offset {} bitsize {})",
-                    new Object[] { this.getClass().getSimpleName(), hdrField,
-                            HexEncode.bytesToHexString(hdrFieldBytes),
-                            startOffset, numBits });
+            if (logger.isTraceEnabled()) {
+              logger.trace("{}: {}: {} (offset {} bitsize {})",
+                      new Object[] { this.getClass().getSimpleName(), hdrField,
+                              HexEncode.bytesToHexString(hdrFieldBytes),
+                              startOffset, numBits });
+            }
 
             this.setHeaderField(hdrField, hdrFieldBytes);
         }
@@ -189,8 +191,10 @@ public abstract class Packet {
         }
         postSerializeCustomOperation(headerBytes);
 
-        logger.trace("{}: {}", this.getClass().getSimpleName(),
-                HexEncode.bytesToHexString(headerBytes));
+        if (logger.isTraceEnabled()) {
+          logger.trace("{}: {}", this.getClass().getSimpleName(),
+                  HexEncode.bytesToHexString(headerBytes));
+        }
         return headerBytes;
     }
 
index 9e838d2..c2fee97 100644 (file)
@@ -285,12 +285,13 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
                         //short tag = container.getTag((Long)nextNode.getNodeID());
                         short tag = 0;
                         if (tag != 0) {
-                            log.debug("adding SET_VLAN " + tag
-                                    + "  for traffic leaving " + currNode + "/"
-                                    + outPort + "toward switch " + nextNode);
+                            log.debug("adding SET_VLAN {} for traffic " +
+                                    "leaving {}/{} toward switch {}",
+                                    new Object[] { tag, currNode, outPort,
+                                    nextNode});
                             actions.add(new SetVlanId(tag));
                         } else {
-                            log.debug("No tag assigned to switch " + nextNode);
+                            log.debug("No tag assigned to switch {}", nextNode);
                         }
                     }
                 }
@@ -319,12 +320,12 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
                     //short tag = container.getTag((Long)currNode.getNodeID());
                     short tag = 0;
                     if (tag != 0) {
-                        log.debug("adding MATCH VLAN " + tag
-                                + "  for traffic entering " + currNode + "/"
-                                + inPort);
+                        log.debug("adding MATCH VLAN {} for traffic entering" +
+                                "  {}/{}",
+                                new Object[] {tag, currNode, inPort});
                         match.setField(MatchType.DL_VLAN, tag);
                     } else {
-                        log.debug("No tag assigned to switch " + currNode);
+                        log.debug("No tag assigned to switch {}", currNode);
                     }
                 }
             }
@@ -351,10 +352,11 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
             pos.put(inPort, po);
             this.rulesDB.put(key, pos);
             if (!inPort.getType().equals(NodeConnectorIDType.ALL)) {
-                log.debug("Adding Match(inPort=" + inPort + ",DIP="
-                        + host.getNetworkAddress().getHostAddress()
-                        + ") Action(outPort=" + outPort + ") to node "
-                        + currNode);
+                log.debug("Adding Match(inPort = {} , DIP = {})" +
+                        " Action(outPort= {}) to node {}",
+                        new Object[] { inPort,
+                        host.getNetworkAddress().getHostAddress(),
+                        outPort, currNode});
                 if ((removed_po != null)
                         && (!po.getFlow().getMatch().equals(
                                 removed_po.getFlow().getMatch()))) {
@@ -365,10 +367,10 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
                 }
 
             } else {
-                log.debug("Adding policy Match(DIP="
-                        + host.getNetworkAddress().getHostAddress()
-                        + ") Action(outPort=" + outPort + ") to node "
-                        + currNode);
+                log.debug("Adding policyMatch(DIP = {}) Action(outPort= {}) " + 
+                        "to node {}", new Object[] {
+                        host.getNetworkAddress().getHostAddress(), outPort,
+                        currNode});
             }
         }
     }
@@ -419,8 +421,8 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
             if ((res == null) || ((links = res.getEdges()) == null)) {
                 // Still the path that connect node to rootNode
                 // doesn't exists
-                log.debug("NO Route/Path between SW[" + node + "] --> SW["
-                        + rootNode + "] cleaning potentially existing entries");
+                log.debug("NO Route/Path between SW[{}] --> SW[{}] cleaning " +
+                        "potentially existing entries", node, rootNode);
                 key = new HostNodePair(host, node);
                 pos = this.rulesDB.get(key);
                 if (pos != null) {
@@ -436,8 +438,7 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
                 continue;
             }
 
-            log.debug("Route between SW[" + node + "] --> SW[" + rootNode
-                            + "]");
+            log.debug("Route between SW[{}] --> SW[{}]", node, rootNode);
             Integer curr;
             Node currNode = node;
             key = new HostNodePair(host, currNode);
@@ -528,8 +529,8 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
         if ((res == null) || ((links = res.getEdges()) == null)) {
             // Still the path that connect node to rootNode
             // doesn't exists
-            log.debug("NO Route/Path between SW[" + node + "] --> SW["
-                    + rootNode + "] cleaning potentially existing entries");
+            log.debug("NO Route/Path between SW[{}] --> SW[{}] cleaning " +
+                    "potentially existing entries", node, rootNode);
             key = new HostNodePair(host, node);
             pos = this.rulesDB.get(key);
             if (pos != null) {
@@ -545,7 +546,7 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
             return null;
         }
 
-        log.debug("Route between SW[" + node + "] --> SW[" + rootNode + "]");
+        log.debug("Route between SW[{}] --> SW[{}]", node, rootNode);
         Integer curr;
         Node currNode = node;
         key = new HostNodePair(host, currNode);
@@ -557,10 +558,10 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
                 continue;
             }
 
-            log.debug("Link [" + currNode + "/" + link.getHeadNodeConnector()
-                    + "] --> ["
-                    + link.getHeadNodeConnector().getNode() + "/"
-                    + link.getTailNodeConnector() + "]");
+            log.debug("Link [{}/{}] --> [{}/{}]", new Object[] {
+                    currNode, link.getHeadNodeConnector(),
+                    link.getHeadNodeConnector().getNode(),
+                    link.getTailNodeConnector()});
 
             // Index all the switches to be programmed
             switchesToProgram.add(currNode);
@@ -627,8 +628,8 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
                                 + po.toString() + " on switch " + swId);
                     }
                 } else {
-                    log.error("Cannot find a policy for SW:{" + swId
-                            + "} Host: {" + host + "}");
+                    log.error("Cannot find a policy for SW:({}) Host: ({})",
+                              swId, host);
                     /* // Now dump every single rule */
                     /* for (HostNodePair dumpkey : this.rulesDB.keySet()) { */
                     /*         po = this.rulesDB.get(dumpkey); */
@@ -688,7 +689,7 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
         for (HostNodePair key : this.rulesDB.keySet()) {
             Node node = key.getNode();
             if (targetNode == null || node.equals(targetNode)) {
-                log.debug("Work on " + node + " host " + key.getHost());
+                log.debug("Work on {} host {}", node, key.getHost());
                 pos = this.rulesDB.get(key);
                 for (Map.Entry<NodeConnector, FlowEntry> e : pos.entrySet()) {
                     po = e.getValue();
@@ -697,7 +698,7 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
                         this.frm.uninstallFlowEntry(po);
                     }
                 }
-                log.debug("Remove " + key);
+                log.debug("Remove {}", key);
                 this.rulesDB.remove(key);
             }
         }
@@ -738,8 +739,8 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
         }
         pl.add(po);
         log.debug("Adding Pruned Policy for SwId: {}", swId);
-        log.debug("Old Policy: " + po.toString());
-        log.debug("New Policy: " + new_po.toString());
+        log.debug("Old Policy: {}", po);
+        log.debug("New Policy: {}", new_po);
     }
 
     private void pruneExcessRules(Set<Node> switches) {
@@ -825,7 +826,7 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
 
         switch (type) {
         case REMOVED:
-            log.debug("Node " + node + " gone, doing a cleanup");
+            log.debug("Node {} gone, doing a cleanup", node);
             uninstallPerNodeRules(node);
             break;
         default: