Fix 'INPORT' keyword in port field 00/58600/11
authorJozef Bacigal <jozef.bacigal@pantheon.tech>
Fri, 9 Jun 2017 10:30:27 +0000 (12:30 +0200)
committerJozef Bacigal <jozef.bacigal@pantheon.tech>
Thu, 15 Jun 2017 12:13:27 +0000 (14:13 +0200)
- also fix NPE in serialization by using wrong port number

Enumeration from opeflow java yang files is correct used keyword 'IN_PORT'.
Openflowplugin with the wrong translation (getString instead of getName) used the
keyword 'INPORT'. This fix use the correct name from enumeration but allow use
the legacy 'INPORT' keyword for next release.
Also prevent to throw NPE by using wrong keyword in port field.

Change-Id: If8b3d301d73169fd158a546a233928ab0c65d1c3
Fix: Bug 2095
Signed-off-by: Jozef Bacigal <jozef.bacigal@pantheon.tech>
openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/protocol/serialization/actions/OutputActionSerializer.java
openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/protocol/serialization/match/InPortEntrySerializer.java
openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/AddressNormalizationUtil.java
openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/util/AddressNormalizationUtilTest.java
openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/util/InventoryDataServiceUtil.java
openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/util/OpenflowPortsUtil.java
openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/translator/MultipartReplyTranslatorThirdTest.java
openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/util/OpenflowPortsUtilTest.java

index d241db34cb6c072eebb11cf564814f66265ad242..176e991e31cec76f34f1a4dc9fc50aa7a823bdc6 100644 (file)
@@ -18,13 +18,19 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.acti
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.output.action._case.OutputAction;
 
 public class OutputActionSerializer extends AbstractActionSerializer {
+
     @Override
     public void serialize(Action action, ByteBuf outBuffer) {
         super.serialize(action, outBuffer);
         final OutputAction outputAction = OutputActionCase.class.cast(action).getOutputAction();
-        outBuffer.writeInt(InventoryDataServiceUtil.portNumberfromNodeConnectorId(
+        Long value = InventoryDataServiceUtil.portNumberfromNodeConnectorId(
                 OpenflowVersion.OF13,
-                outputAction.getOutputNodeConnector().getValue()).intValue());
+                outputAction.getOutputNodeConnector().getValue());
+        if (value == null) {
+            throw new IllegalArgumentException("Not a valid port number: "
+                    + outputAction.getOutputNodeConnector().getValue());
+        }
+        outBuffer.writeInt(value.intValue());
         outBuffer.writeShort(MoreObjects.firstNonNull(outputAction.getMaxLength(), 0));
         outBuffer.writeZero(ActionConstants.OUTPUT_PADDING);
     }
index 9d2afa6309fb7e4d39e9eff904833888e08a5594..8a13baa68bc17a42ccc9b93e07241727e37f6ce9 100644 (file)
@@ -21,9 +21,13 @@ public class InPortEntrySerializer extends AbstractMatchEntrySerializer {
     @Override
     public void serialize(Match match, ByteBuf outBuffer) {
         super.serialize(match, outBuffer);
-        outBuffer.writeInt(InventoryDataServiceUtil.portNumberfromNodeConnectorId(
+        Long value = InventoryDataServiceUtil.portNumberfromNodeConnectorId(
                 OpenflowVersion.OF13,
-                match.getInPort().getValue()).intValue());
+                match.getInPort().getValue());
+        if (value == null) {
+            throw new IllegalArgumentException("Not a valid port number: " + match.getInPort().getValue());
+        }
+        outBuffer.writeInt(value.intValue());
     }
 
     @Override
index b61e2b6cb69b1db333c58030f03323bcdf99853d..ee11155af092c2e97df42cfb0f4a4b3839ed9946 100644 (file)
@@ -50,8 +50,10 @@ public class AddressNormalizationUtil {
             return null;
         }
 
-        return OpenflowPortsUtil.getProtocolAgnosticPortUri(protocolVersion, InventoryDataServiceUtil
-                .portNumberfromNodeConnectorId(OpenflowVersion.get(protocolVersion), port.getValue()));
+        Long portValue = InventoryDataServiceUtil
+                .portNumberfromNodeConnectorId(OpenflowVersion.get(protocolVersion), port.getValue());
+
+        return portValue == null ? null : OpenflowPortsUtil.getProtocolAgnosticPortUri(protocolVersion, portValue);
     }
 
     /**
index 0c190369f4dd3964e7114286b564b50253c61ed3..8dcfe0e5c4b071ac218e13d8a145e47cfc88c10e 100644 (file)
@@ -26,7 +26,7 @@ public class AddressNormalizationUtilTest {
     @Test
     public void normalizeProtocolAgnosticPortOF10() throws Exception {
         final Uri left = new Uri("openflow:1:INPORT");
-        final Uri right = new Uri("INPORT");
+        final Uri right = new Uri("IN_PORT");
 
         assertEquals(
                 right,
index 80556f37d7b81489b3dc1fbe1c1f02f9fb1b9b49..65b8bd00c4eac2810e1e6eac105455f677854d43 100644 (file)
@@ -13,6 +13,8 @@ import java.util.List;
 import java.util.concurrent.ExecutionException;
 
 import com.google.common.base.Splitter;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.apache.commons.lang3.StringUtils;
 import org.opendaylight.controller.md.sal.binding.api.ReadTransaction;
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
@@ -130,6 +132,7 @@ public abstract class InventoryDataServiceUtil {
         return new NodeConnectorId(OFConstants.OF_URI_PREFIX + datapathid + ":" + (logicalName == null ? portNo : logicalName));
     }
 
+    @Nullable
     public static Long portNumberfromNodeConnectorId(final OpenflowVersion ofVersion, final NodeConnectorId ncId) {
         return portNumberfromNodeConnectorId(ofVersion, ncId.getValue());
     }
@@ -145,10 +148,10 @@ public abstract class InventoryDataServiceUtil {
         return splitStringList.get(splitStringList.size()-1);
     }
 
-    public static Long portNumberfromNodeConnectorId(final OpenflowVersion ofVersion, final String ncId) {
+    @Nullable
+    public static Long portNumberfromNodeConnectorId(final OpenflowVersion ofVersion, @Nonnull final String ncId) {
         String portNoString = portNoStringfromNodeConnectorID(ncId);
-        Long portNo = OpenflowPortsUtil.getPortFromLogicalName(ofVersion, portNoString);
-        return portNo;
+        return OpenflowPortsUtil.getPortFromLogicalName(ofVersion, portNoString);
     }
 
 
index 19b1b25e778403bafe96b6c4f3eab72412866d00..3e69463127a3c39dd9ff57ea2cbb2277c766da98 100644 (file)
@@ -9,6 +9,8 @@
 package org.opendaylight.openflowplugin.openflow.md.util;
 
 import com.google.common.collect.ImmutableBiMap;
+import java.util.Objects;
+import javax.annotation.Nullable;
 import org.opendaylight.openflowjava.protocol.api.util.BinContent;
 import org.opendaylight.openflowplugin.api.OFConstants;
 import org.opendaylight.openflowplugin.api.openflow.md.util.OpenflowVersion;
@@ -17,6 +19,8 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.P
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.OutputPortValues;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.PortNumberValues;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.PortNumberValuesV10;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Class which integrates the port constants defined and used by MDSAL and the ports defined in openflow java
@@ -26,36 +30,40 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev13
  * @author Kamal Rameshan on 5/2/14.
  */
 public class OpenflowPortsUtil {
+
+    private static final Logger LOG = LoggerFactory.getLogger(OpenflowPortsUtil.class);
     private static final ImmutableBiMap<Short, ImmutableBiMap<String, Long>> versionPortMap;
     private static final ImmutableBiMap<Short, ImmutableBiMap<Long, String>> versionInversePortMap;
 
+    private static boolean inportWarnignAlreadyFired = false;
+
     static {
         // v1.0 ports
         final ImmutableBiMap<String, Long> ofv10ports = new ImmutableBiMap.Builder<String, Long>()
-                .put(OutputPortValues.MAX.toString(), (long) PortNumberValuesV10.MAX.getIntValue()) //0xff00
-                .put(OutputPortValues.INPORT.toString(), (long) PortNumberValuesV10.INPORT.getIntValue()) //0xfff8
-                .put(OutputPortValues.TABLE.toString(), (long) PortNumberValuesV10.TABLE.getIntValue()) //0xfff9
-                .put(OutputPortValues.NORMAL.toString(), (long) PortNumberValuesV10.NORMAL.getIntValue()) //0xfffa
-                .put(OutputPortValues.FLOOD.toString(), (long) PortNumberValuesV10.FLOOD.getIntValue()) //0xfffb
-                .put(OutputPortValues.ALL.toString(), (long) PortNumberValuesV10.ALL.getIntValue()) //0xfffc
-                .put(OutputPortValues.CONTROLLER.toString(), (long) PortNumberValuesV10.CONTROLLER.getIntValue()) //0xfffd
-                .put(OutputPortValues.LOCAL.toString(), (long) PortNumberValuesV10.LOCAL.getIntValue()) //0xfffe
-                .put(OutputPortValues.NONE.toString(), (long) PortNumberValuesV10.NONE.getIntValue()) //0xffff
+                .put(OutputPortValues.MAX.getName(), (long) PortNumberValuesV10.MAX.getIntValue()) //0xff00
+                .put(OutputPortValues.INPORT.getName(), (long) PortNumberValuesV10.INPORT.getIntValue()) //0xfff8
+                .put(OutputPortValues.TABLE.getName(), (long) PortNumberValuesV10.TABLE.getIntValue()) //0xfff9
+                .put(OutputPortValues.NORMAL.getName(), (long) PortNumberValuesV10.NORMAL.getIntValue()) //0xfffa
+                .put(OutputPortValues.FLOOD.getName(), (long) PortNumberValuesV10.FLOOD.getIntValue()) //0xfffb
+                .put(OutputPortValues.ALL.getName(), (long) PortNumberValuesV10.ALL.getIntValue()) //0xfffc
+                .put(OutputPortValues.CONTROLLER.getName(), (long) PortNumberValuesV10.CONTROLLER.getIntValue()) //0xfffd
+                .put(OutputPortValues.LOCAL.getName(), (long) PortNumberValuesV10.LOCAL.getIntValue()) //0xfffe
+                .put(OutputPortValues.NONE.getName(), (long) PortNumberValuesV10.NONE.getIntValue()) //0xffff
                 .build();
 
         // openflow 1.3 reserved ports.
         // PortNumberValues are defined in OFJava yang. And yang maps an int to all enums. Hence we need to create longs from (-ve) ints
         // TODO: do we need to define these ports in yang?
         final ImmutableBiMap<String, Long> ofv13ports = new ImmutableBiMap.Builder<String, Long>()
-                .put(OutputPortValues.MAX.toString(), BinContent.intToUnsignedLong(PortNumberValues.MAX.getIntValue())) //0xffffff00
-                .put(OutputPortValues.INPORT.toString(), BinContent.intToUnsignedLong(PortNumberValues.INPORT.getIntValue())) //0xfffffff8
-                .put(OutputPortValues.TABLE.toString(), BinContent.intToUnsignedLong(PortNumberValues.TABLE.getIntValue())) //0xfffffff9
-                .put(OutputPortValues.NORMAL.toString(), BinContent.intToUnsignedLong(PortNumberValues.NORMAL.getIntValue())) //0xfffffffa
-                .put(OutputPortValues.FLOOD.toString(), BinContent.intToUnsignedLong(PortNumberValues.FLOOD.getIntValue())) //0xfffffffb
-                .put(OutputPortValues.ALL.toString(), BinContent.intToUnsignedLong(PortNumberValues.ALL.getIntValue())) //0xfffffffc
-                .put(OutputPortValues.CONTROLLER.toString(), BinContent.intToUnsignedLong(PortNumberValues.CONTROLLER.getIntValue())) //0xfffffffd
-                .put(OutputPortValues.LOCAL.toString(), BinContent.intToUnsignedLong(PortNumberValues.LOCAL.getIntValue())) //0xfffffffe
-                .put(OutputPortValues.ANY.toString(), BinContent.intToUnsignedLong(PortNumberValues.ANY.getIntValue())) //0xffffffff
+                .put(OutputPortValues.MAX.getName(), BinContent.intToUnsignedLong(PortNumberValues.MAX.getIntValue())) //0xffffff00
+                .put(OutputPortValues.INPORT.getName(), BinContent.intToUnsignedLong(PortNumberValues.INPORT.getIntValue())) //0xfffffff8
+                .put(OutputPortValues.TABLE.getName(), BinContent.intToUnsignedLong(PortNumberValues.TABLE.getIntValue())) //0xfffffff9
+                .put(OutputPortValues.NORMAL.getName(), BinContent.intToUnsignedLong(PortNumberValues.NORMAL.getIntValue())) //0xfffffffa
+                .put(OutputPortValues.FLOOD.getName(), BinContent.intToUnsignedLong(PortNumberValues.FLOOD.getIntValue())) //0xfffffffb
+                .put(OutputPortValues.ALL.getName(), BinContent.intToUnsignedLong(PortNumberValues.ALL.getIntValue())) //0xfffffffc
+                .put(OutputPortValues.CONTROLLER.getName(), BinContent.intToUnsignedLong(PortNumberValues.CONTROLLER.getIntValue())) //0xfffffffd
+                .put(OutputPortValues.LOCAL.getName(), BinContent.intToUnsignedLong(PortNumberValues.LOCAL.getIntValue())) //0xfffffffe
+                .put(OutputPortValues.ANY.getName(), BinContent.intToUnsignedLong(PortNumberValues.ANY.getIntValue())) //0xffffffff
                 .build();
 
         versionPortMap = new ImmutableBiMap.Builder<Short, ImmutableBiMap<String, Long>>()
@@ -79,8 +87,22 @@ public class OpenflowPortsUtil {
                 : getPortLogicalName(ofVersion.getVersion(), portNumber);
     }
 
-    public static Long getPortFromLogicalName(final OpenflowVersion ofVersion, final String logicalNameOrPort) {
-        Long port = versionPortMap.get(ofVersion.getVersion()).get(logicalNameOrPort);
+    @Nullable
+    static Long getPortFromLogicalName(final OpenflowVersion ofVersion, final String logicalNameOrPort) {
+
+        //The correct keyword defined in openflow specification in IN_PORT so we need to allow to use it
+        //for legacy reasons we can't just simply drop the misspelled INPORT
+        //TODO: Consider to remove 'INPORT' keyword
+        Long port;
+        if (Objects.equals(logicalNameOrPort, "INPORT")) {
+            if (!inportWarnignAlreadyFired) {
+                LOG.warn("Using '{}' in port field is not recommended use 'IN_PORT' instead", logicalNameOrPort);
+                inportWarnignAlreadyFired = true;
+            }
+            port = versionPortMap.get(ofVersion.getVersion()).get(OutputPortValues.INPORT.getName());
+        } else {
+            port = versionPortMap.get(ofVersion.getVersion()).get(logicalNameOrPort);
+        }
         if (port == null) {
             try {
                 port = Long.decode(logicalNameOrPort);
index 47628e7005eabb6cf1bae22b793983963207bb93..c16ba9d832f3569630ab36d426574b158f21062c 100644 (file)
@@ -371,7 +371,7 @@ public class MultipartReplyTranslatorThirdTest {
         Assert.assertEquals("Wrong duration sec", 6, stat.getDuration().getSecond().getValue().intValue());\r
         Assert.assertEquals("Wrong duration n sec", 7, stat.getDuration().getNanosecond().getValue().intValue());\r
         stat = statUpdate.getQueueIdAndStatisticsMap().get(1);\r
-        Assert.assertEquals("Wrong port number", "openflow:42:INPORT", stat.getNodeConnectorId().getValue());\r
+        Assert.assertEquals("Wrong port number", "openflow:42:IN_PORT", stat.getNodeConnectorId().getValue());\r
         Assert.assertEquals("Wrong queue-id", 20, stat.getQueueId().getValue().intValue());\r
         Assert.assertEquals("Wrong tx packets", 30, stat.getTransmittedPackets().getValue().intValue());\r
         Assert.assertEquals("Wrong tx bytes", 40, stat.getTransmittedBytes().getValue().intValue());\r
index 54e12ecdf39274c1c52cb3876e22c84ba4f106ac..73aa5374965741f918f159935afb6d7edf4c11a5 100644 (file)
@@ -32,26 +32,26 @@ public class OpenflowPortsUtilTest {
     @BeforeClass
     public static void setupClass() {
         mapOF10Ports = new HashMap<String, Long>();
-        mapOF10Ports.put(OutputPortValues.MAX.toString(), 65280L);
-        mapOF10Ports.put(OutputPortValues.INPORT.toString(), 65528L);
-        mapOF10Ports.put(OutputPortValues.TABLE.toString(), 65529L);
-        mapOF10Ports.put(OutputPortValues.NORMAL.toString(), 65530L);
-        mapOF10Ports.put(OutputPortValues.FLOOD.toString(), 65531L);
-        mapOF10Ports.put(OutputPortValues.ALL.toString(), 65532L);
-        mapOF10Ports.put(OutputPortValues.CONTROLLER.toString(), 65533L);
-        mapOF10Ports.put(OutputPortValues.LOCAL.toString(), 65534L);
-        mapOF10Ports.put(OutputPortValues.NONE.toString(), 65535L);
+        mapOF10Ports.put(OutputPortValues.MAX.getName(), 65280L);
+        mapOF10Ports.put(OutputPortValues.INPORT.getName(), 65528L);
+        mapOF10Ports.put(OutputPortValues.TABLE.getName(), 65529L);
+        mapOF10Ports.put(OutputPortValues.NORMAL.getName(), 65530L);
+        mapOF10Ports.put(OutputPortValues.FLOOD.getName(), 65531L);
+        mapOF10Ports.put(OutputPortValues.ALL.getName(), 65532L);
+        mapOF10Ports.put(OutputPortValues.CONTROLLER.getName(), 65533L);
+        mapOF10Ports.put(OutputPortValues.LOCAL.getName(), 65534L);
+        mapOF10Ports.put(OutputPortValues.NONE.getName(), 65535L);
 
         mapOF13Ports = new HashMap<String, Long>();
-        mapOF13Ports.put(OutputPortValues.MAX.toString(), 4294967040L);
-        mapOF13Ports.put(OutputPortValues.INPORT.toString(), 4294967288L);
-        mapOF13Ports.put(OutputPortValues.TABLE.toString(), 4294967289L);
-        mapOF13Ports.put(OutputPortValues.NORMAL.toString(), 4294967290L);
-        mapOF13Ports.put(OutputPortValues.FLOOD.toString(), 4294967291L);
-        mapOF13Ports.put(OutputPortValues.ALL.toString(), 4294967292L);
-        mapOF13Ports.put(OutputPortValues.CONTROLLER.toString(), 4294967293L);
-        mapOF13Ports.put(OutputPortValues.LOCAL.toString(), 4294967294L);
-        mapOF13Ports.put(OutputPortValues.ANY.toString(), 4294967295L);
+        mapOF13Ports.put(OutputPortValues.MAX.getName(), 4294967040L);
+        mapOF13Ports.put(OutputPortValues.INPORT.getName(), 4294967288L);
+        mapOF13Ports.put(OutputPortValues.TABLE.getName(), 4294967289L);
+        mapOF13Ports.put(OutputPortValues.NORMAL.getName(), 4294967290L);
+        mapOF13Ports.put(OutputPortValues.FLOOD.getName(), 4294967291L);
+        mapOF13Ports.put(OutputPortValues.ALL.getName(), 4294967292L);
+        mapOF13Ports.put(OutputPortValues.CONTROLLER.getName(), 4294967293L);
+        mapOF13Ports.put(OutputPortValues.LOCAL.getName(), 4294967294L);
+        mapOF13Ports.put(OutputPortValues.ANY.getName(), 4294967295L);
 
         mapVersionToPorts = new HashMap<OpenflowVersion, Map<String, Long>>();
         mapVersionToPorts.put(OpenflowVersion.OF10, mapOF10Ports);
@@ -88,59 +88,62 @@ public class OpenflowPortsUtilTest {
     @Test
     public void testGetPortLogicalName() {
 
-        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.MAX.toString());
-        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.INPORT.toString());
-        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.TABLE.toString());
-        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.NORMAL.toString());
-        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.FLOOD.toString());
-        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.ALL.toString());
-        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.CONTROLLER.toString());
-        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.LOCAL.toString());
-        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.NONE.toString());
-
-        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.MAX.toString());
-        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.INPORT.toString());
-        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.TABLE.toString());
-        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.NORMAL.toString());
-        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.FLOOD.toString());
-        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.ALL.toString());
-        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.CONTROLLER.toString());
-        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.LOCAL.toString());
-        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.ANY.toString());
+        String s = OutputPortValues.INPORT.getName();
+        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.MAX.getName());
+        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.INPORT.getName());
+        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.TABLE.getName());
+        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.NORMAL.getName());
+        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.FLOOD.getName());
+        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.ALL.getName());
+        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.CONTROLLER.getName());
+        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.LOCAL.getName());
+        matchGetLogicalName(OpenflowVersion.OF10, OutputPortValues.NONE.getName());
+
+        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.MAX.getName());
+        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.INPORT.getName());
+        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.TABLE.getName());
+        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.NORMAL.getName());
+        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.FLOOD.getName());
+        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.ALL.getName());
+        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.CONTROLLER.getName());
+        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.LOCAL.getName());
+        matchGetLogicalName(OpenflowVersion.OF13, OutputPortValues.ANY.getName());
 
         Assert.assertNull("Invalid port number should return a null",
                 OpenflowPortsUtil.getPortLogicalName(OpenflowVersion.OF10, 99999L));
 
         Assert.assertNull("Invalid port number should return a null",
                 OpenflowPortsUtil.getPortLogicalName(OpenflowVersion.OF13, 99999L));
+        Assert.assertFalse(s.equals("a"));
     }
 
 
+
     /**
      * test for method {@link OpenflowPortsUtil#getPortFromLogicalName(OpenflowVersion, String)}
      */
     @Test
     public void testGetPortFromLogicalName() {
 
-        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.MAX.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.INPORT.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.TABLE.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.NORMAL.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.FLOOD.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.ALL.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.CONTROLLER.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.LOCAL.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.NONE.toString());
-
-        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.MAX.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.INPORT.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.TABLE.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.NORMAL.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.FLOOD.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.ALL.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.CONTROLLER.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.LOCAL.toString());
-        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.ANY.toString());
+        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.MAX.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.INPORT.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.TABLE.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.NORMAL.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.FLOOD.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.ALL.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.CONTROLLER.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.LOCAL.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF10, OutputPortValues.NONE.getName());
+
+        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.MAX.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.INPORT.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.TABLE.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.NORMAL.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.FLOOD.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.ALL.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.CONTROLLER.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.LOCAL.getName());
+        matchGetPortfromLogicalName(OpenflowVersion.OF13, OutputPortValues.ANY.getName());
 
         Assert.assertNull("Invalid port logical name should return a null",
                 OpenflowPortsUtil.getPortFromLogicalName(OpenflowVersion.OF10, "abc"));
@@ -179,7 +182,7 @@ public class OpenflowPortsUtilTest {
     }
 
     /**
-     * test for method {@link OpenflowPortsUtil#portNumberToString(PortNumber)}
+     * test for method {@link OpenflowPortsUtil}
      */
     @Test
     public void testPortNumberToString() {