Fix findbugs/sonar violations in sfc-openflow-renderer

- Method call passes null for non-null parameter
- Method ignores exceptional return value
- Consider using Locale parameterized version of invoked method
- Field isn't final but should be
- Method invokes toString() method on a String
- Inefficient use of keySet iterator instead of entrySet iterator
- Should be a static inner class
- Private method is never called
- Method invokes inefficient Number constructor; use static valueOf instead
- Dead store to local variable
- Write to static field from instance method
- Unread public/protected field
- Field not initialized in constructor but dereferenced without null check
- Useless object created
- Collapsible "if" statements should be merged

Also enabled findbugs enforcement.

Change-Id: If2f867adce5b7a65984c889c0dbb099b7af7ae53
Signed-off-by: Tom Pantelis <[email protected]>

diff --git a/sfc-renderers/sfc-openflow-renderer/pom.xml b/sfc-renderers/sfc-openflow-renderer/pom.xml
index 82f0e9ff8d1204d191f8ddad6775ec217dbe5391..41f2d89cad9377b7995155891c0c62efc2807c29 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/pom.xml
+++ b/sfc-renderers/sfc-openflow-renderer/pom.xml
@@ -81,6 +81,13 @@
           <propertyExpansion>checkstyle.violationSeverity=error</propertyExpansion>
         </configuration>
       </plugin>
+      <plugin>
+        <groupId>org.codehaus.mojo</groupId>
+        <artifactId>findbugs-maven-plugin</artifactId>
+        <configuration>
+          <failOnError>true</failOnError>
+        </configuration>
+      </plugin>
       <plugin>
         <groupId>org.opendaylight.yangtools</groupId>
         <artifactId>yang-maven-plugin</artifactId>

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfAbstractDataListener.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfAbstractDataListener.java
index 4dbac19675e1866fc4946f003941492fc3659daf..8ba7fd4d6e62b80580bf284ad8084e4f200204a0 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfAbstractDataListener.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfAbstractDataListener.java
@@ -18,32 +18,17 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
 public abstract class SfcOfAbstractDataListener<T extends DataObject> implements DataTreeChangeListener<T> {
 
-    private DataBroker dataBroker;
-    private InstanceIdentifier<T> instanceIdentifier;
     private ListenerRegistration<SfcOfAbstractDataListener<T>> dataChangeListenerRegistration;
 
-    public void setDataBroker(DataBroker dataBroker) {
-        this.dataBroker = dataBroker;
-    }
-
-    public void setIID(InstanceIdentifier<T> instanceIdentifier) {
-        this.instanceIdentifier = instanceIdentifier;
-    }
-
-    public DataBroker getDataBroker() {
-        return dataBroker;
-    }
-
-    public void registerAsDataChangeListener(LogicalDatastoreType datastoreType) {
+    public void registerAsDataChangeListener(DataBroker dataBroker, LogicalDatastoreType datastoreType,
+            InstanceIdentifier<T> instanceIdentifier) {
         dataChangeListenerRegistration = dataBroker.registerDataTreeChangeListener(new DataTreeIdentifier<>(
                 datastoreType, instanceIdentifier), this);
     }
 
-    public void registerAsDataChangeListener() {
-        registerAsDataChangeListener(LogicalDatastoreType.CONFIGURATION);
-    }
-
     public void closeDataChangeListener() {
-        dataChangeListenerRegistration.close();
+        if (dataChangeListenerRegistration != null) {
+            dataChangeListenerRegistration.close();
+        }
     }
 }

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfRendererDataListener.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfRendererDataListener.java
index 4c7d6abfd80f0830087aee7d4f8a875c5a8646b7..c647519d3b443a886214f071d65f97c5a95f678d 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfRendererDataListener.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfRendererDataListener.java
@@ -40,9 +40,8 @@ public class SfcOfRendererDataListener extends SfcOfAbstractDataListener<SfcOfRe
 
     public SfcOfRendererDataListener(DataBroker dataBroker, SfcOfFlowProgrammerInterface sfcOfFlowProgrammer,
             SfcSynchronizer sfcSynchronizer) {
-        setDataBroker(dataBroker);
-        setIID(InstanceIdentifier.builder(SfcOfRendererConfig.class).build());
-        registerAsDataChangeListener(LogicalDatastoreType.CONFIGURATION);
+        registerAsDataChangeListener(dataBroker, LogicalDatastoreType.CONFIGURATION,
+                InstanceIdentifier.builder(SfcOfRendererConfig.class).build());
         this.sfcOfFlowProgrammer = sfcOfFlowProgrammer;
         this.sfcSynchronizer = sfcSynchronizer;
         threadExecutor = Executors.newSingleThreadExecutor();
@@ -112,7 +111,7 @@ public class SfcOfRendererDataListener extends SfcOfAbstractDataListener<SfcOfRe
         UpdateOpenFlowTableOffsets updateThread = new UpdateOpenFlowTableOffsets(config.getSfcOfTableOffset(),
                 config.getSfcOfAppEgressTableOffset());
 
-        threadExecutor.submit(updateThread);
+        threadExecutor.execute(updateThread);
     }
 
     /**
@@ -129,7 +128,7 @@ public class SfcOfRendererDataListener extends SfcOfAbstractDataListener<SfcOfRe
             return new TableId((short) (tableOffset + maxTable));
         } catch (IllegalArgumentException e) {
             LOG.error("SfcOfRendererDataListener::verifyMaxTableId invalid table offset [{}] maxTable [{}]",
-                    tableOffset, maxTable);
+                    tableOffset, maxTable, e);
             return null;
         }
     }

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfSfgDataListener.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfSfgDataListener.java
index 3e0eaee857c79ee06cdcfb85f7f4d36e36a763cc..79d2219b18d8befb6fba7fe82095c861e21f88d5 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfSfgDataListener.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/listeners/SfcOfSfgDataListener.java
@@ -60,9 +60,8 @@ public class SfcOfSfgDataListener extends SfcOfAbstractDataListener<ServiceFunct
         this.sfcOfFlowProgrammer = sfcOfFlowProgrammer;
         this.sfcOfProviderUtils = sfcOfProviderUtils;
 
-        setDataBroker(dataBroker);
-        setIID(SfcInstanceIdentifiers.SFG_ENTRY_IID);
-        registerAsDataChangeListener(LogicalDatastoreType.CONFIGURATION);
+        registerAsDataChangeListener(dataBroker, LogicalDatastoreType.CONFIGURATION,
+                SfcInstanceIdentifiers.SFG_ENTRY_IID);
     }
 
     @Override

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcIpv4PacketInHandler.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcIpv4PacketInHandler.java
index 573c6da3f85b5bfd4659cba0580ceefbbcbf2bc9..5facc9301b53544a7f10602550a8e298d919174c 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcIpv4PacketInHandler.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcIpv4PacketInHandler.java
@@ -13,8 +13,9 @@ import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.Map;
-import java.util.Set;
+import java.util.Map.Entry;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.model.match.types.rev131026.match.Metadata;
@@ -171,13 +172,13 @@ public class SfcIpv4PacketInHandler implements PacketProcessingListener, AutoClo
 
         // Configure the uplink packet
         if (ulPathId >= 0) {
-            this.flowProgrammer.setFlowRspId(new Long(ulPathId));
+            this.flowProgrammer.setFlowRspId(Long.valueOf(ulPathId));
             this.flowProgrammer.configurePathMapperAclFlow(nodeName, pktSrcIpStr, pktDstIpStr, ulPathId);
         }
 
         // Configure the downlink packet
         if (dlPathId >= 0) {
-            this.flowProgrammer.setFlowRspId(new Long(dlPathId));
+            this.flowProgrammer.setFlowRspId(Long.valueOf(dlPathId));
             this.flowProgrammer.configurePathMapperAclFlow(nodeName, pktDstIpStr, pktSrcIpStr, dlPathId);
         }
     }
@@ -289,13 +290,14 @@ public class SfcIpv4PacketInHandler implements PacketProcessingListener, AutoClo
      */
     private void purgePktInBuffer() {
         long currentMillis = System.currentTimeMillis();
-        Set<String> keySet = pktInBuffer.keySet();
-        for (String key : keySet) {
-            Long bufferedTime = pktInBuffer.get(key);
+        Iterator<Entry<String, Long>> iter = pktInBuffer.entrySet().iterator();
+        while (iter.hasNext()) {
+            Entry<String, Long> entry = iter.next();
+            Long bufferedTime = entry.getValue();
             if (currentMillis - bufferedTime.longValue() > maxBufferTime) {
                 // This also removes the entry from the pktInBuffer map and
                 // doesnt invalidate iteration
-                keySet.remove(key);
+                iter.remove();
             }
         }
     }

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerImpl.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerImpl.java
index 48f14e2034f45999a8a6cf91a0f5224f70f60ae1..97bd1744838369cdec731f2a67847d04903c74fa 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerImpl.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerImpl.java
@@ -12,6 +12,7 @@ import com.google.common.net.InetAddresses;
 import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Locale;
 import java.util.Optional;
 import java.util.Set;
 import java.util.StringJoiner;
@@ -254,7 +255,7 @@ public class SfcOfFlowProgrammerImpl implements SfcOfFlowProgrammerInterface {
             return false;
         }
 
-        return cookie.toString().toUpperCase().startsWith(TRANSPORT_EGRESS_COOKIE_STR_BASE);
+        return cookie.toString().toUpperCase(Locale.getDefault()).startsWith(TRANSPORT_EGRESS_COOKIE_STR_BASE);
     }
 
     //
@@ -309,7 +310,7 @@ public class SfcOfFlowProgrammerImpl implements SfcOfFlowProgrammerInterface {
         // Action output
         List<Action> actionList = new ArrayList<>();
         String outPortStr = OUTPUT_STRING + outPort.toString();
-        actionList.add(SfcOpenflowUtils.createActionOutPort(outPortStr, order++));
+        actionList.add(SfcOpenflowUtils.createActionOutPort(outPortStr, order));
 
         InstructionsBuilder isb = SfcOpenflowUtils.wrapActionsIntoApplyActionsInstruction(actionList);
 
@@ -344,7 +345,7 @@ public class SfcOfFlowProgrammerImpl implements SfcOfFlowProgrammerInterface {
 
         // Action NORMAL
         List<Action> actionList = new ArrayList<>();
-        actionList.add(SfcOpenflowUtils.createActionNormal(order++));
+        actionList.add(SfcOpenflowUtils.createActionNormal(order));
 
         InstructionsBuilder isb = SfcOpenflowUtils.wrapActionsIntoApplyActionsInstruction(actionList);
 
@@ -696,7 +697,7 @@ public class SfcOfFlowProgrammerImpl implements SfcOfFlowProgrammerInterface {
 
         // Create and configure the FlowBuilder
         return SfcOpenflowUtils.createFlowBuilder(getTableId(TABLE_INDEX_TRANSPORT_INGRESS),
-                FLOW_PRIORITY_TRANSPORT_INGRESS, flowName.toString(), match, isb);
+                FLOW_PRIORITY_TRANSPORT_INGRESS, flowName, match, isb);
     }
 
     /**

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorMacChaining.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorMacChaining.java
index d5b1e4c346a2b34948eb73441548366e98a90bdc..f8e7a645f375de15cf565fff7933702e957b931b 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorMacChaining.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorMacChaining.java
@@ -9,7 +9,6 @@
 package org.opendaylight.sfc.ofrenderer.processors;
 
 import java.util.Iterator;
-
 import org.opendaylight.sfc.util.macchaining.SfcModelUtil;
 import org.opendaylight.sfc.util.macchaining.VirtualMacAddress;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sf.rev140701.service.function.base.SfDataPlaneLocator;
@@ -161,7 +160,7 @@ public class SfcRspProcessorMacChaining extends SfcRspTransportProcessorBase {
         VirtualMacAddress hopMac = VirtualMacAddress.getForwardAddress(entry.getPathId(), 0);
 
         String vmac = hopMac.getHop(entry.getServiceIndex()).getValue();
-        String dstSfMac = sfcProviderUtils.getSfDplMac(dstSfDpl);
+        String dstSfMac = dstSfDpl == null ? null : sfcProviderUtils.getSfDplMac(dstSfDpl);
         String nextVMac = hopMac.getHop((short)(entry.getServiceIndex() - 1)).getValue();
 
         boolean isL2Transparent = false;
@@ -245,7 +244,7 @@ public class SfcRspProcessorMacChaining extends SfcRspTransportProcessorBase {
 
         String dstMac = sfcProviderUtils.getSfDplMac(dstSfDpl);
 
-        LOG.info("SFFNodeName {}", sffNodeName.toString());
+        LOG.info("SFFNodeName {}", sffNodeName);
         this.sfcFlowProgrammer.configureMacChainingSfTransportEgressFlow(sffNodeName, dstMac, srcOfsPortStr, null);
 
     }
@@ -295,7 +294,7 @@ public class SfcRspProcessorMacChaining extends SfcRspTransportProcessorBase {
         VirtualMacAddress hopMac = VirtualMacAddress.getForwardAddress(entry.getPathId(), 0);
         String vmac = hopMac.getHop(entry.getServiceIndex()).getValue();
 
-        LOG.info("SFFNodeName {}", sffNodeName.toString());
+        LOG.info("SFFNodeName {}", sffNodeName);
 
         this.sfcFlowProgrammer.configureMacChainingSfTransportEgressFlow(sffNodeName, dstMac, srcOfsPortStr, vmac);
     }

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorMpls.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorMpls.java
index a034e4b5afdc57d73a8701764b92bae0695b85c5..6980ca943cd21b11b2bade88d8e58ae24750b216 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorMpls.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorMpls.java
@@ -9,7 +9,7 @@
 package org.opendaylight.sfc.ofrenderer.processors;
 
 import java.util.Iterator;
-
+import java.util.concurrent.atomic.AtomicLong;
 import org.opendaylight.sfc.ofrenderer.processors.SffGraph.SffGraphEntry;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sf.rev140701.service.function.base.SfDataPlaneLocator;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sf.rev140701.service.functions.ServiceFunction;
@@ -23,7 +23,7 @@ import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sl.rev14070
 public class SfcRspProcessorMpls extends SfcRspTransportProcessorBase {
     private static final int MPLS_LABEL_INCR_HOP = 1;
     private static final int MPLS_LABEL_INCR_RSP = 100;
-    private static int lastMplsLabel;
+    private static final AtomicLong LAST_MPLS_LABEL = new AtomicLong(0);
 
     /**
      * Set the RSP path egress DPL and SFF Hop Ingress DPLs for the VLAN
@@ -32,8 +32,7 @@ public class SfcRspProcessorMpls extends SfcRspTransportProcessorBase {
     @Override
     public void setRspTransports() {
         int hopIncrement = MPLS_LABEL_INCR_HOP;
-        int transportData = lastMplsLabel + MPLS_LABEL_INCR_RSP;
-        lastMplsLabel = transportData;
+        long transportData = LAST_MPLS_LABEL.addAndGet(MPLS_LABEL_INCR_RSP);
 
         Iterator<SffGraph.SffGraphEntry> sffGraphIter = sffGraph.getGraphEntryIterator();
         while (sffGraphIter.hasNext()) {
@@ -50,7 +49,7 @@ public class SfcRspProcessorMpls extends SfcRspTransportProcessorBase {
             DataPlaneLocatorBuilder dpl = new DataPlaneLocatorBuilder();
             dpl.setTransport(rsp.getTransportType());
             MplsBuilder mplsBuilder = new MplsBuilder();
-            mplsBuilder.setMplsLabel((long) transportData);
+            mplsBuilder.setMplsLabel(transportData);
             dpl.setLocatorType(mplsBuilder.build());
 
             if (entry.getDstSff().equals(SffGraph.EGRESS)) {

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorVlan.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorVlan.java
index 256b5fc900a11f522a8a798b9229d1cad946d39d..37eba4f69c4a2e839dbf436f3bff461b9f7670d8 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorVlan.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorVlan.java
@@ -9,7 +9,7 @@
 package org.opendaylight.sfc.ofrenderer.processors;
 
 import java.util.Iterator;
-
+import java.util.concurrent.atomic.AtomicInteger;
 import org.opendaylight.sfc.ofrenderer.processors.SffGraph.SffGraphEntry;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sf.rev140701.service.function.base.SfDataPlaneLocator;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sf.rev140701.service.functions.ServiceFunction;
@@ -22,7 +22,7 @@ import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sl.rev14070
 public class SfcRspProcessorVlan extends SfcRspTransportProcessorBase {
     private static final int VLAN_ID_INCR_HOP = 1;
     private static final int VLAN_ID_INCR_RSP = 100;
-    private static int lastVlanId = 0;
+    private static final AtomicInteger LAST_VLAN_ID = new AtomicInteger(0);
 
     /**
      * Set the RSP path egress DPL and SFF Hop Ingress DPLs for the VLAN
@@ -31,8 +31,7 @@ public class SfcRspProcessorVlan extends SfcRspTransportProcessorBase {
     @Override
     public void setRspTransports() {
         int hopIncrement = VLAN_ID_INCR_HOP;
-        int transportData = lastVlanId + VLAN_ID_INCR_RSP;
-        lastVlanId = transportData;
+        int transportData = LAST_VLAN_ID.addAndGet(VLAN_ID_INCR_RSP);
 
         Iterator<SffGraph.SffGraphEntry> sffGraphIter = sffGraph.getGraphEntryIterator();
         while (sffGraphIter.hasNext()) {

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspTransportProcessorBase.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspTransportProcessorBase.java
index 9fe056d2799a84cf4d4fd308f30369edeb401b35..55542bf93512037e1d5d3137179f43f7e7eacde3 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspTransportProcessorBase.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspTransportProcessorBase.java
@@ -10,8 +10,8 @@ package org.opendaylight.sfc.ofrenderer.processors;
 
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Optional;
-
 import org.opendaylight.sfc.genius.util.appcoexistence.SfcTableIndexMapper;
 import org.opendaylight.sfc.ofrenderer.openflow.SfcOfFlowProgrammerInterface;
 import org.opendaylight.sfc.ofrenderer.utils.SfcOfBaseProviderUtils;
@@ -177,13 +177,12 @@ public abstract class SfcRspTransportProcessorBase {
             // may be null if its EGRESS
             ServiceFunctionForwarder dstSff = sfcProviderUtils.getServiceFunctionForwarder(entry.getDstSff(),
                     entry.getPathId());
-            if (dstSff != null) {
-                // Set the SFF-SFF Hop DPL
-                if (!setSffHopDataPlaneLocators(srcSff, dstSff)) {
-                    throw new SfcRenderingException("Unable to get SFF HOP DPLs srcSff [" + entry.getSrcSff()
-                            + "] dstSff [" + entry.getDstSff() + "] transport [" + rsp.getTransportType() + "] pathId ["
-                            + entry.getPathId() + "]");
-                }
+
+            // Set the SFF-SFF Hop DPL
+            if (dstSff != null && !setSffHopDataPlaneLocators(srcSff, dstSff)) {
+                throw new SfcRenderingException("Unable to get SFF HOP DPLs srcSff [" + entry.getSrcSff()
+                    + "] dstSff [" + entry.getDstSff() + "] transport [" + rsp.getTransportType() + "] pathId ["
+                    + entry.getPathId() + "]");
             }
 
             if (entry.getDstSff().equals(SffGraph.EGRESS)) {
@@ -360,29 +359,17 @@ public abstract class SfcRspTransportProcessorBase {
         if (lhs.getImplementedInterface() != rhs.getImplementedInterface()) {
             return false;
         }
-        String type = lhs.getImplementedInterface().getSimpleName().toLowerCase();
+        String type = lhs.getImplementedInterface().getSimpleName().toLowerCase(Locale.getDefault());
 
         switch (type) {
             case IP:
-                if (((Ip) lhs).getPort().getValue().intValue() == ((Ip) rhs).getPort().getValue().intValue()) {
-                    if (((Ip) lhs).getIp().toString().equals(((Ip) rhs).getIp().toString())) {
-                        return true;
-                    }
-                }
-                break;
+                return ((Ip) lhs).getPort().getValue().intValue() == ((Ip) rhs).getPort().getValue().intValue()
+                    && ((Ip) lhs).getIp().toString().equals(((Ip) rhs).getIp().toString());
             case MAC:
-                if (((Mac) lhs).getVlanId() != null && ((Mac) rhs).getVlanId() != null) {
-                    if (((Mac) lhs).getVlanId().intValue() == ((Mac) rhs).getVlanId().intValue()) {
-                        return true;
-                    }
-                }
-                break;
+                return ((Mac) lhs).getVlanId() != null && ((Mac) rhs).getVlanId() != null
+                    && ((Mac) lhs).getVlanId().intValue() == ((Mac) rhs).getVlanId().intValue();
             case MPLS:
-                if (((Mpls) lhs).getMplsLabel().longValue() == ((Mpls) rhs).getMplsLabel().longValue()) {
-                    return true;
-                }
-                break;
-
+                return ((Mpls) lhs).getMplsLabel().longValue() == ((Mpls) rhs).getMplsLabel().longValue();
             case FUNCTION:
             case LISP:
             default:

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SffGraph.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SffGraph.java
index ed845eb3da298051f0ba42fe0612ad66870bb883..f5a54c5d0b54c98c8a386d0f32f7d2cf543bacfe 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SffGraph.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SffGraph.java
@@ -14,7 +14,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.common.rev151017.SfName;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.common.rev151017.SffDataPlaneLocatorName;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.common.rev151017.SffName;
@@ -36,7 +35,7 @@ public class SffGraph {
     /**
      * Internal class to hold each SffGraph entry.
      */
-    public class SffGraphEntry {
+    public static class SffGraphEntry {
 
         private final SffName srcSff;
         private final SffName dstSff;
@@ -93,15 +92,14 @@ public class SffGraph {
 
         @Override
         public String toString() {
-            StringBuffer returnValue = new StringBuffer(
-                    "SffGraphEntry [srcSff=" + srcSff + ", dstSff=" + dstSff + ", sf=" + sf + ", prevSf=" + prevSf
-                            + ", sfg=" + sfg + ", pathId=" + pathId + ", serviceIndex=" + serviceIndex);
+            String str = "SffGraphEntry [srcSff=" + srcSff + ", dstSff=" + dstSff + ", sf=" + sf + ", prevSf=" + prevSf
+                    + ", sfg=" + sfg + ", pathId=" + pathId + ", serviceIndex=" + serviceIndex;
             if (getSrcDpnId() != null || getDstDpnId() != null) {
-                returnValue.append(
-                        ", Logical SFF params: src dpnid [" + getSrcDpnId() + "], dst dpnid [" + getDstDpnId() + "]");
+                str += ", Logical SFF params: src dpnid [" + getSrcDpnId() + "], dst dpnid [" + getDstDpnId() + "]";
             }
-            returnValue.append("]");
-            return returnValue.toString();
+
+            str += "]";
+            return str;
         }
 
         public DpnIdType getDstDpnId() {
@@ -133,7 +131,7 @@ public class SffGraph {
      * Internal class to hold the ingress and egress DPLs for an SFF for a
      * particular pathId.
      */
-    public class SffDataPlaneLocators {
+    public static class SffDataPlaneLocators {
 
         private final SffName sffName;
         private final long pathId;
@@ -323,9 +321,7 @@ public class SffGraph {
         Set<Long> dplKeys = this.getSffDplKeys();
         for (Long key : dplKeys) {
             Map<SffName, SffDataPlaneLocators> sffDpls = this.getSffDplsForPath(key);
-            Set<SffName> sffDplKeys = sffDpls.keySet();
-            for (SffName sffDplKey : sffDplKeys) {
-                SffDataPlaneLocators sffDpl = sffDpls.get(sffDplKey);
+            for (SffDataPlaneLocators sffDpl: sffDpls.values()) {
                 // TODO need to get the locator details for any/all transport
                 LOG.info(
                         "SFF [{}] pathId [{}] IngressDpl [{}] EgressDpl [{}] IngressHopDpl Transport [{}] Vlan ID [{}]",

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/SfcOfBaseProviderUtils.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/SfcOfBaseProviderUtils.java
index 736a186650414192818f9a94fd012067cf91342b..90a6ea2ee7c7cba3cf70c655a41a9051ff78797c 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/SfcOfBaseProviderUtils.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/SfcOfBaseProviderUtils.java
@@ -9,7 +9,6 @@
 package org.opendaylight.sfc.ofrenderer.utils;
 
 import java.util.List;
-
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.common.rev151017.SfName;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.common.rev151017.SffDataPlaneLocatorName;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.common.rev151017.SffName;
@@ -83,11 +82,10 @@ public abstract class SfcOfBaseProviderUtils {
 
         List<SffDataPlaneLocator> sffDataPlanelocatorList = sff.getSffDataPlaneLocator();
         for (SffDataPlaneLocator sffDataPlanelocator : sffDataPlanelocatorList) {
-            if (sffDataPlanelocator.getName() != null) {
-                if (sffDataPlanelocator.getName().getValue().equals(dplName.getValue())) {
-                    sffDpl = sffDataPlanelocator;
-                    break;
-                }
+            if (sffDataPlanelocator.getName() != null
+                    && sffDataPlanelocator.getName().getValue().equals(dplName.getValue())) {
+                sffDpl = sffDataPlanelocator;
+                break;
             }
         }
 
@@ -196,10 +194,8 @@ public abstract class SfcOfBaseProviderUtils {
         Class<? extends DataContainer> implementedInterface = sffLocatorType.getImplementedInterface();
 
         // Mac/IP and possibly VLAN
-        if (implementedInterface.equals(Mac.class)) {
-            if (((MacAddressLocator) sffLocatorType).getMac() != null) {
-                sfMac = ((MacAddressLocator) sffLocatorType).getMac().getValue();
-            }
+        if (implementedInterface.equals(Mac.class) && ((MacAddressLocator) sffLocatorType).getMac() != null) {
+            sfMac = ((MacAddressLocator) sffLocatorType).getMac().getValue();
         }
 
         return sfMac;
@@ -310,19 +306,15 @@ public abstract class SfcOfBaseProviderUtils {
         String macStr = null;
         OfsPort ofsPort = getSffPortInfoFromDpl(dpl);
 
-        if (ofsPort != null) {
-            if (ofsPort.getMacAddress() != null) {
-                macStr = ofsPort.getMacAddress().getValue();
-            }
+        if (ofsPort != null && ofsPort.getMacAddress() != null) {
+            macStr = ofsPort.getMacAddress().getValue();
         }
 
-        if (macStr == null) {
-            if (dpl.getDataPlaneLocator().getTransport()
-                    .equals(org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sl.rev140701.Mac.class)) {
-                MacAddress mac = ((MacAddressLocator) dpl.getDataPlaneLocator().getLocatorType()).getMac();
-                if (mac != null) {
-                    macStr = mac.getValue();
-                }
+        if (macStr == null && dpl.getDataPlaneLocator().getTransport()
+                .equals(org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sl.rev140701.Mac.class)) {
+            MacAddress mac = ((MacAddressLocator) dpl.getDataPlaneLocator().getLocatorType()).getMac();
+            if (mac != null) {
+                macStr = mac.getValue();
             }
         }
 
@@ -346,13 +338,11 @@ public abstract class SfcOfBaseProviderUtils {
 
         // If the SFF DPL wasnt augmented, check if the DPL is of type mac, and
         // return that mac address
-        if (macStr == null) {
-            if (sffDpl.getDataPlaneLocator().getTransport()
-                    .equals(org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sl.rev140701.Mac.class)) {
-                MacAddress mac = ((MacAddressLocator) sffDpl.getDataPlaneLocator().getLocatorType()).getMac();
-                if (mac != null) {
-                    macStr = mac.getValue();
-                }
+        if (macStr == null && sffDpl.getDataPlaneLocator().getTransport()
+                .equals(org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sl.rev140701.Mac.class)) {
+            MacAddress mac = ((MacAddressLocator) sffDpl.getDataPlaneLocator().getLocatorType()).getMac();
+            if (mac != null) {
+                macStr = mac.getValue();
             }
         }
 

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/SfcOfProviderUtils.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/SfcOfProviderUtils.java
index 04f9cabc02fc39d6a72a3536f9a2c9c042555240..39b65a30e2e07aeb8ff67ecd587afbbd3af94470 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/SfcOfProviderUtils.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/SfcOfProviderUtils.java
@@ -13,9 +13,6 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.sfc.ovs.provider.SfcOvsUtil;
 import org.opendaylight.sfc.provider.api.SfcDataStoreAPI;
@@ -32,7 +29,6 @@ import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sff.rev1407
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sfg.rev150214.service.function.groups.ServiceFunctionGroup;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sft.rev140701.service.function.types.ServiceFunctionType;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Uri;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbBridgeAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbNodeAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbTerminationPointAugmentation;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NetworkTopology;
@@ -51,7 +47,7 @@ public class SfcOfProviderUtils extends SfcOfBaseProviderUtils {
 
     // Since this class can be called by multiple threads,
     // store these objects per RSP id to avoid collisions
-    private class RspContext {
+    private static class RspContext {
 
         // store the SFs and SFFs internally so we dont have to
         // query the DataStore repeatedly for the same thing
@@ -67,7 +63,6 @@ public class SfcOfProviderUtils extends SfcOfBaseProviderUtils {
     }
 
     private final Map<Long, RspContext> rspIdToContext;
-    protected static ExecutorService executor = Executors.newFixedThreadPool(10);
 
     public SfcOfProviderUtils() {
         rspIdToContext = new HashMap<>();
@@ -200,19 +195,6 @@ public class SfcOfProviderUtils extends SfcOfBaseProviderUtils {
         return ofPort;
     }
 
-    // internal support method for getPortNumberFromName() and related methods
-    private OvsdbTerminationPointAugmentation extractTerminationPointAugmentation(Node bridgeNode, String portName) {
-        if (bridgeNode.getAugmentation(OvsdbBridgeAugmentation.class) != null) {
-            List<OvsdbTerminationPointAugmentation> tpAugmentations = extractTerminationPointAugmentations(bridgeNode);
-            for (OvsdbTerminationPointAugmentation ovsdbTerminationPointAugmentation : tpAugmentations) {
-                if (ovsdbTerminationPointAugmentation.getName().equals(portName)) {
-                    return ovsdbTerminationPointAugmentation;
-                }
-            }
-        }
-        return null;
-    }
-
     // internal support method for getPortNumberFromName() and related methods
     public List<OvsdbTerminationPointAugmentation> extractTerminationPointAugmentations(Node node) {
         List<OvsdbTerminationPointAugmentation> tpAugmentations = new ArrayList<>();

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/operdsupdate/OperDsUpdateHandlerLSFFImpl.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/operdsupdate/OperDsUpdateHandlerLSFFImpl.java
index 1372c12afd16b561f6acef80c8cc604317280307..24a1d3eeb2ab0c074cfe4a25a002dbf652800246 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/operdsupdate/OperDsUpdateHandlerLSFFImpl.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/utils/operdsupdate/OperDsUpdateHandlerLSFFImpl.java
@@ -144,10 +144,6 @@ public class OperDsUpdateHandlerLSFFImpl implements OperDsUpdateHandlerInterface
                 continue;
             }
             ServiceFunctionForwarderStateKey sffKey = new ServiceFunctionForwarderStateKey(sffName);
-            RspsForDpnidBuilder builder = new RspsForDpnidBuilder();
-            List<Rsps> values = new ArrayList<>();
-            values.add(new RspsBuilder().setKey(new RspsKey(new SfpName(rsp.getName().getValue()))).build());
-            builder.setRsps(values);
 
             InstanceIdentifier<Rsps> dpnidIif = InstanceIdentifier.builder(ServiceFunctionForwardersState.class)
                     .child(ServiceFunctionForwarderState.class, sffKey).augmentation(SffLogicalSffAugmentation.class)
@@ -204,7 +200,7 @@ public class OperDsUpdateHandlerLSFFImpl implements OperDsUpdateHandlerInterface
      *            The transaction to submit
      */
     private void commitChangesAsync(WriteTransaction trans) {
-        threadPoolExecutorService.submit(() -> {
+        threadPoolExecutorService.execute(() -> {
             ListenableFuture<Void> submitFuture = trans.submit();
             try {
                 submitFuture.get();