From 094826e7d9380ecd006945764878f3df6a25d3ab Mon Sep 17 00:00:00 2001 From: Michal Rehak Date: Fri, 27 Jun 2014 22:21:07 +0200 Subject: [PATCH 1/1] 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 --- .../core/sal/convertor/ActionConvertor.java | 20 +++--- .../GroupStatsResponseConvertor.java | 2 +- .../openflow/md/util/OpenflowPortsUtil.java | 34 ++++++++++ .../md/util/OpenflowPortsUtilTest.java | 62 ++++++++++++++++++- 4 files changed, 106 insertions(+), 12 deletions(-) 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 e19d50c488..e09f1ad78b 100644 --- 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 4f8b1728a9..944b3748c1 100644 --- 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 9865fc90ca..ca02d0ad7b 100644 --- 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 a9921f8ee3..d3be8d505f 100644 --- 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 mapOF13Ports; private static Map> 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)); + } + } -- 2.36.6