Bug-835 - Reserve Ports should be logical ports, part2

- spread ofVersion parameter
- improved logical port number usage
- unit tests

Change-Id: I904baefdb62db7d9e49f1eede765bbb389aa7b03
Signed-off-by: Michal Rehak <mirehak@cisco.com>

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/ActionConvertor.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/ActionConvertor.java
index e19d50c488608f64f9a714a34c80d23b01e24391..e09f1ad78b3f790eed4ea0fe82047e374de4cceb 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/ActionConvertor.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/ActionConvertor.java
@@ -88,6 +88,7 @@ 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.set.tp.src.action._case.SetTpSrcAction;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.set.vlan.id.action._case.SetVlanIdAction;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.set.vlan.pcp.action._case.SetVlanPcpAction;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.CommonPort;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.augments.rev131002.DlAddressAction;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.augments.rev131002.DlAddressActionBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.augments.rev131002.EthertypeAction;
@@ -154,9 +155,6 @@ import java.util.List;
  */
 public final class ActionConvertor {
     private static final Logger logger = LoggerFactory.getLogger(ActionConvertor.class);
-    private static final String PREFIX_SEPARATOR = "/";
-    final private static Long MAXPortOF13 = new Long(4294967040L); // 0xffffff00
-    final private static Long MAXPortOF10 = new Long(0xff00);
 
     private ActionConvertor() {
         // NOOP
@@ -693,7 +691,7 @@ public final class ActionConvertor {
 
         OpenflowVersion ofVersion = OpenflowVersion.get(version);
         Long portNumber = InventoryDataServiceUtil.portNumberfromNodeConnectorId(ofVersion, uri.getValue());
-        if (portNumber != null && (OpenflowPortsUtil.isPortReserved(ofVersion, portNumber) || portNumber < OpenflowPortsUtil.getMaxPortForVersion(ofVersion)) ) {
+        if (OpenflowPortsUtil.checkPortValidity(ofVersion, portNumber)) {
             portAction.setPort(new PortNumber(portNumber));
         } else {
             logger.error("Invalid Port specified "+ portNumber + " for Output Action for OF version:"+ ofVersion);
@@ -719,7 +717,7 @@ public final class ActionConvertor {
         for (Action action : actionList) {
             if (action.getType().equals(
                     org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev130731.Output.class)) {
-                bucketActions.add(ofToSALOutputAction(action));
+                bucketActions.add(ofToSALOutputAction(ofVersion, action));
 
             } else if (action.getType().equals(
                     org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev130731.Group.class)) {
@@ -801,18 +799,22 @@ public final class ActionConvertor {
 
     /**
      * Method converts OF Output action object to SAL Output action object.
-     *
+     * @param ofVersion 
+     * @param ofVersion 
      * @param action
      *            org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.
      *            action.rev130731.actions.actions.list.Action
+     *
      * @return OutputAction
      */
-    public static OutputActionCase ofToSALOutputAction(Action action) {
-
+    public static OutputActionCase ofToSALOutputAction(OpenflowVersion ofVersion, Action action) {
         OutputActionBuilder outputAction = new OutputActionBuilder();
         PortAction port = action.getAugmentation(PortAction.class);
         if (port != null) {
-            outputAction.setOutputNodeConnector(new Uri(port.getPort().getValue().toString()));
+            CommonPort.PortNumber protocolAgnosticPort = OpenflowPortsUtil.getProtocolAgnosticPort(
+                    ofVersion, port.getPort().getValue());
+            String portNumberAsString = OpenflowPortsUtil.portNumberToString(protocolAgnosticPort);
+            outputAction.setOutputNodeConnector(new Uri(portNumberAsString));
         } else {
             logger.error("Provided action is not OF Output action, no associated port found!");
         }

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/GroupStatsResponseConvertor.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/GroupStatsResponseConvertor.java
index 4f8b1728a93a6da48a21915165a6d2d1446074e8..944b3748c119bd881b3e4daccce1ca4616919869 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/GroupStatsResponseConvertor.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/GroupStatsResponseConvertor.java
@@ -120,7 +120,7 @@ public class GroupStatsResponseConvertor {
     /**
      * Method convert GroupStats message from library to MD SAL defined GroupStats  
      * @param groupDesc GroupStats from library
-     * @param ofVersion TODO
+     * @param ofVersion current ofp version
      * @return GroupStats -- GroupStats defined in MD-SAL
      */
     public GroupDescStats toSALGroupDescStats(GroupDesc groupDesc, OpenflowVersion ofVersion){

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/util/OpenflowPortsUtil.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/util/OpenflowPortsUtil.java
index 9865fc90ca3da232608d3f4ff3a5bc656f8f77e7..ca02d0ad7bee3ca0f6ed0b71749819474b9e13c5 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/util/OpenflowPortsUtil.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/util/OpenflowPortsUtil.java
@@ -1,6 +1,7 @@
 package org.opendaylight.openflowplugin.openflow.md.util;
 
 import com.google.common.collect.ImmutableBiMap;
+
 import org.opendaylight.openflowjava.protocol.api.util.BinContent;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.CommonPort;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.OutputPortValues;
@@ -103,4 +104,37 @@ public class OpenflowPortsUtil {
     public static boolean isPortReserved(OpenflowVersion ofVersion, Long portNumber) {
         return versionPortMap.get(ofVersion).inverse().containsKey(portNumber);
     }
+
+    /**
+     * @param ofVersion
+     * @param portNumber
+     * @return true if port number is valid for given protocol version
+     */
+    public static boolean checkPortValidity(OpenflowVersion ofVersion, Long portNumber) {
+        boolean portIsValid = true;
+        if (portNumber == null) {
+            portIsValid = false;
+        } else if (portNumber < 0) {
+            portIsValid = false;
+        } else if (portNumber > OpenflowPortsUtil.getMaxPortForVersion(ofVersion)) {
+            if (!OpenflowPortsUtil.isPortReserved(ofVersion, portNumber)) {
+                portIsValid = false;
+            }
+        }
+        return portIsValid;
+    }
+
+    /**
+     * @param portNumber
+     * @return string containing number or logical name
+     */
+    public static String portNumberToString(CommonPort.PortNumber portNumber) {
+        String result = null;
+        if (portNumber.getUint32() != null) {
+            result = String.valueOf(portNumber.getUint32());
+        } else if (portNumber.getString() != null) {
+            result = portNumber.getString();
+        }
+        return result;
+    }
 }

diff --git a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/util/OpenflowPortsUtilTest.java b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/util/OpenflowPortsUtilTest.java
index a9921f8ee3c066bc5650a060a6f41c8f7642dd87..d3be8d505f39a80b5d15367f77a0eaec58b67285 100644 (file)
--- a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/util/OpenflowPortsUtilTest.java
+++ b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/util/OpenflowPortsUtilTest.java
@@ -4,6 +4,7 @@ import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.CommonPort;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.OutputPortValues;
 
 import java.util.HashMap;
@@ -18,6 +19,9 @@ public class OpenflowPortsUtilTest {
     private static Map<String, Long> mapOF13Ports;
     private static Map<OpenflowVersion, Map<String, Long>> mapVersionToPorts;
 
+    /**
+     * initiation before testing - once for all
+     */
     @BeforeClass
     public static void setupClass() {
         OpenflowPortsUtil.init();
@@ -50,6 +54,9 @@ public class OpenflowPortsUtilTest {
 
     }
 
+    /**
+     * tearing down initiated values after all tests done
+     */
     @AfterClass
     public static void tearDownClass() {
         OpenflowPortsUtil.close();
@@ -59,18 +66,21 @@ public class OpenflowPortsUtilTest {
     }
 
     //helper
-    private void matchGetLogicalName(OpenflowVersion version, String logicalName) {
+    private static void matchGetLogicalName(OpenflowVersion version, String logicalName) {
         Assert.assertEquals("Controller reserve port not matching to logical-name for "+ version,
                 logicalName,
                 OpenflowPortsUtil.getPortLogicalName(version, mapVersionToPorts.get(version).get(logicalName)));
     }
 
     //helper
-    private void matchGetPortfromLogicalName(OpenflowVersion version, String logicalName) {
+    private static void matchGetPortfromLogicalName(OpenflowVersion version, String logicalName) {
         Assert.assertEquals("Controller reserve port not matching to logical-name for "+ version,
                 mapVersionToPorts.get(version).get(logicalName), OpenflowPortsUtil.getPortFromLogicalName(version, logicalName));
     }
 
+    /**
+     * test for method {@link OpenflowPortsUtil#getPortLogicalName(OpenflowVersion, Long)}
+     */
     @Test
     public void testGetPortLogicalName() {
 
@@ -102,6 +112,9 @@ public class OpenflowPortsUtilTest {
     }
 
 
+    /**
+     * test for method {@link OpenflowPortsUtil#getPortFromLogicalName(OpenflowVersion, String)}
+     */
     @Test
     public void testGetPortFromLogicalName() {
 
@@ -133,4 +146,49 @@ public class OpenflowPortsUtilTest {
 
     }
 
+    /**
+     * test for method {@link OpenflowPortsUtil#checkPortValidity(OpenflowVersion, Long)} - OF-1.0
+     */
+    @Test
+    public void testCheckPortValidity10() {
+        Assert.assertFalse(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF10 , -1L));
+        Assert.assertTrue(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF10 , 0L));
+        Assert.assertTrue(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF10 , 0xFF00L));
+        Assert.assertTrue(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF10 , 0xFFF8L));
+        Assert.assertFalse(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF10 , 0xFFF0L));
+        Assert.assertTrue(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF10 , 0xFFFFL));
+        Assert.assertFalse(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF10 , 0x1FFFFL));
+    }
+
+    /**
+     * test for method {@link OpenflowPortsUtil#checkPortValidity(OpenflowVersion, Long)} - OF-1.3
+     */
+    @Test
+    public void testCheckPortValidity13() {
+        Assert.assertFalse(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF13 , -1L));
+        Assert.assertTrue(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF13 , 0L));
+        Assert.assertTrue(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF13 , 0xFFFFFF00L));
+        Assert.assertTrue(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF13 , 0xFFFFFFF8L));
+        Assert.assertFalse(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF13 , 0xFFFFFFF0L));
+        Assert.assertTrue(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF13 , 0xFFFFFFFFL));
+        Assert.assertFalse(OpenflowPortsUtil.checkPortValidity(OpenflowVersion.OF13 , 0x1FFFFFFFFL));
+    }
+
+    /**
+     * test for method {@link OpenflowPortsUtil#portNumberToString(CommonPort.PortNumber)}
+     */
+    @Test
+    public void testPortNumberToString() {
+        CommonPort.PortNumber portNumber;
+        
+        portNumber = new CommonPort.PortNumber(42L);
+        Assert.assertEquals("42", OpenflowPortsUtil.portNumberToString(portNumber));
+        
+        portNumber = new CommonPort.PortNumber(OutputPortValues.FLOOD.toString());
+        Assert.assertEquals("FLOOD", OpenflowPortsUtil.portNumberToString(portNumber));
+        
+        portNumber = new CommonPort.PortNumber((String) null);
+        Assert.assertNull(OpenflowPortsUtil.portNumberToString(portNumber));
+    }
+
 }