From 55d41b3ce28b914f8477dc25d1b7f81f5fd9c52f Mon Sep 17 00:00:00 2001 From: Tomas Slusny Date: Wed, 29 Jun 2016 08:54:36 +0200 Subject: [PATCH] Bug 5540 - PacketOutConvertor - Reworked PacketOutConvertor to use new ConvertorManager design - Updated tests and usage of PacketOutConvertor accordingly Change-Id: I9df612a7d9c3b9604c8119f86d2266723066e68a Signed-off-by: Tomas Slusny --- .../services/PacketProcessingServiceImpl.java | 11 +- .../md/core/sal/ModelDrivenSwitchImpl.java | 20 +-- .../core/sal/convertor/ConvertorManager.java | 1 + .../sal/convertor/PacketOutConvertor.java | 125 ++++++++++-------- .../data/PacketOutConvertorData.java | 44 ++++++ .../sal/convertor/PacketOutConvertorTest.java | 20 ++- 6 files changed, 150 insertions(+), 71 deletions(-) create mode 100644 openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/data/PacketOutConvertorData.java diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/PacketProcessingServiceImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/PacketProcessingServiceImpl.java index 85e198ef24..348495c44a 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/PacketProcessingServiceImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/PacketProcessingServiceImpl.java @@ -7,12 +7,16 @@ */ package org.opendaylight.openflowplugin.impl.services; +import java.util.Optional; import java.util.concurrent.Future; import org.opendaylight.openflowplugin.api.openflow.device.DeviceContext; import org.opendaylight.openflowplugin.api.openflow.device.RequestContextStack; import org.opendaylight.openflowplugin.api.openflow.device.Xid; +import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.ConvertorManager; import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.PacketOutConvertor; +import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.data.PacketOutConvertorData; import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.OfHeader; +import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.PacketOutInput; import org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.PacketProcessingService; import org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.TransmitPacketInput; import org.opendaylight.yangtools.yang.common.RpcResult; @@ -30,6 +34,11 @@ public final class PacketProcessingServiceImpl extends AbstractVoidService result = ConvertorManager.getInstance().convert(input, data); + return result.orElse(PacketOutConvertor.defaultResult(getVersion())); } } diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/ModelDrivenSwitchImpl.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/ModelDrivenSwitchImpl.java index aee9dffc1e..40c845c7e0 100644 --- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/ModelDrivenSwitchImpl.java +++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/ModelDrivenSwitchImpl.java @@ -8,21 +8,21 @@ package org.opendaylight.openflowplugin.openflow.md.core.sal; import com.google.common.base.Optional; -import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction; - import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; - import java.math.BigInteger; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction; import org.opendaylight.controller.sal.binding.api.NotificationProviderService; import org.opendaylight.openflowplugin.api.OFConstants; import org.opendaylight.openflowplugin.api.openflow.md.core.SwitchConnectionDistinguisher; -import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.PacketOutConvertor; import org.opendaylight.openflowplugin.api.openflow.md.core.session.IMessageDispatchService; -import org.opendaylight.openflowplugin.openflow.md.core.session.OFSessionUtil; import org.opendaylight.openflowplugin.api.openflow.md.core.session.SessionContext; +import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.ConvertorManager; +import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.PacketOutConvertor; +import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.data.PacketOutConvertorData; +import org.opendaylight.openflowplugin.openflow.md.core.session.OFSessionUtil; import org.opendaylight.openflowplugin.openflow.md.core.session.SwitchConnectionCookieOFImpl; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.service.rev130819.AddFlowInput; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.service.rev130819.AddFlowOutput; @@ -219,8 +219,11 @@ public class ModelDrivenSwitchImpl extends AbstractModelDrivenSwitch { public Future> transmitPacket(final TransmitPacketInput input) { LOG.debug("TransmitPacket - {}", input); // Convert TransmitPacket to PacketOutInput - PacketOutInput message = PacketOutConvertor.toPacketOutInput(input, version, sessionContext.getNextXid(), - sessionContext.getFeatures().getDatapathId()); + final PacketOutConvertorData data = new PacketOutConvertorData(version); + data.setDatapathId(sessionContext.getFeatures().getDatapathId()); + data.setXid(sessionContext.getNextXid()); + + final java.util.Optional message = ConvertorManager.getInstance().convert(input, data); SwitchConnectionDistinguisher cookie = null; ConnectionCookie connectionCookie = input.getConnectionCookie(); @@ -229,7 +232,8 @@ public class ModelDrivenSwitchImpl extends AbstractModelDrivenSwitch { } LOG.debug("Calling the transmitPacket RPC method"); - return messageService.packetOut(message, cookie); + return messageService.packetOut(message + .orElse(PacketOutConvertor.defaultResult(version)), cookie); } @Override diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/ConvertorManager.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/ConvertorManager.java index 3d26f331f1..d8cf176b20 100644 --- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/ConvertorManager.java +++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/ConvertorManager.java @@ -50,6 +50,7 @@ public class ConvertorManager { INSTANCE.registerConvertor(new GroupConvertor()); INSTANCE.registerConvertor(new GroupDescStatsResponseConvertor()); INSTANCE.registerConvertor(new GroupStatsResponseConvertor()); + INSTANCE.registerConvertor(new PacketOutConvertor()); } // Actual convertor keys diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PacketOutConvertor.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PacketOutConvertor.java index 50f834a46d..3db52261d8 100644 --- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PacketOutConvertor.java +++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PacketOutConvertor.java @@ -1,14 +1,14 @@ -/** +/* * Copyright (c) 2013 Cisco Systems, Inc. and others. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v1.0 which accompanies this distribution, * and is available at http://www.eclipse.org/legal/epl-v10.html */ + package org.opendaylight.openflowplugin.openflow.md.core.sal.convertor; import com.google.common.collect.Iterables; -import java.math.BigInteger; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -17,6 +17,8 @@ import org.opendaylight.controller.sal.common.util.Arguments; import org.opendaylight.openflowplugin.api.OFConstants; import org.opendaylight.openflowplugin.api.openflow.md.util.OpenflowVersion; import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.action.data.ActionConvertorData; +import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.common.ParametrizedConvertor; +import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.data.PacketOutConvertorData; import org.opendaylight.openflowplugin.openflow.md.util.InventoryDataServiceUtil; import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeConnectorRef; import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.node.NodeConnectorKey; @@ -33,62 +35,93 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.PathArgument; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public final class PacketOutConvertor { +/** + * Converts a MD-SAL packet out data into the OF library packet out input. + * + * Example usage: + * 
+ * {@code
+ * PacketOutConvertorData data = new PacketOutConvertorData(version);
+ * data.setDatapathId(datapathId);
+ * data.setXid(xid);
+ * Optional ofPacketInput = ConvertorManager.getInstance().convert(salPacket, data);
+ * }
+ *
+ */ +public class PacketOutConvertor implements ParametrizedConvertor { private static final Logger LOG = LoggerFactory.getLogger(PacketOutConvertor.class); - private PacketOutConvertor() { - + /** + * Create default empty meter mot input builder. + * Use this method, if result from convertor is empty. + * + * @param version Openflow version + * @return default empty meter mod input builder + */ + public static PacketOutInput defaultResult(short version) { + return new PacketOutInputBuilder() + .setVersion(version) + .build(); } - // Get all the data for the PacketOut from the Yang/SAL-Layer + private static PortNumber getPortNumber(final PathArgument pathArgument, final Short ofVersion) { + // FIXME VD P! find InstanceIdentifier helper + InstanceIdentifier.IdentifiableItem item = Arguments.checkInstanceOf(pathArgument, + InstanceIdentifier.IdentifiableItem.class); + NodeConnectorKey key = Arguments.checkInstanceOf(item.getKey(), NodeConnectorKey.class); + Long port = InventoryDataServiceUtil.portNumberfromNodeConnectorId( + OpenflowVersion.get(ofVersion), key.getId()); + return new PortNumber(port); + } - /** - * @param version openflow version - * @param inputPacket input packet - * @param datapathid datapath id - * @param xid tx id - * @return PacketOutInput required by OF Library - */ - public static PacketOutInput toPacketOutInput(final TransmitPacketInput inputPacket, final short version, final Long xid, - final BigInteger datapathid) { + @Override + public Class getType() { + return TransmitPacketInput.class; + } - LOG.trace("toPacketOutInput for datapathId:{}, xid:{}", datapathid, xid); + @Override + public PacketOutInput convert(TransmitPacketInput source, PacketOutConvertorData data) { + LOG.trace("toPacketOutInput for datapathId:{}, xid:{}", data.getDatapathId(), data.getXid()); // Build Port ID from TransmitPacketInput.Ingress - PortNumber inPortNr = null; + PortNumber inPortNr; Long bufferId = OFConstants.OFP_NO_BUFFER; Iterable inArgs = null; PacketOutInputBuilder builder = new PacketOutInputBuilder(); - if (inputPacket.getIngress() != null) { - inArgs = inputPacket.getIngress().getValue().getPathArguments(); + + if (source.getIngress() != null) { + inArgs = source.getIngress().getValue().getPathArguments(); } + if (inArgs != null && Iterables.size(inArgs) >= 3) { - inPortNr = getPortNumber(Iterables.get(inArgs, 2), version); + inPortNr = getPortNumber(Iterables.get(inArgs, 2), data.getVersion()); } else { // The packetOut originated from the controller inPortNr = new PortNumber(0xfffffffdL); } // Build Buffer ID to be NO_OFP_NO_BUFFER - if (inputPacket.getBufferId() != null) { - bufferId = inputPacket.getBufferId(); + if (source.getBufferId() != null) { + bufferId = source.getBufferId(); } PortNumber outPort = null; - NodeConnectorRef outRef = inputPacket.getEgress(); + NodeConnectorRef outRef = source.getEgress(); Iterable outArgs = outRef.getValue().getPathArguments(); + if (Iterables.size(outArgs) >= 3) { - outPort = getPortNumber(Iterables.get(outArgs, 2), version); + outPort = getPortNumber(Iterables.get(outArgs, 2), data.getVersion()); } else { // TODO : P4 search for some normal exception - new Exception("PORT NR not exist in Egress"); + // new Exception("PORT NR not exist in Egress"); + LOG.error("PORT NR not exist in Egress"); } - List actions = null; - List inputActions = - inputPacket.getAction(); + List actions = new ArrayList<>(); + List inputActions = source.getAction(); + if (inputActions != null) { - final ActionConvertorData actionConvertorData = new ActionConvertorData(version); - actionConvertorData.setDatapathId(datapathid); + final ActionConvertorData actionConvertorData = new ActionConvertorData(data.getVersion()); + actionConvertorData.setDatapathId(data.getDatapathId()); final Optional> convertedActions = ConvertorManager.getInstance().convert( inputActions, actionConvertorData); @@ -96,44 +129,24 @@ public final class PacketOutConvertor { actions = convertedActions.orElse(Collections.emptyList()); } else { - actions = new ArrayList<>(); // TODO VD P! wait for way to move Actions (e.g. augmentation) ActionBuilder aBuild = new ActionBuilder(); - - OutputActionCaseBuilder outputActionCaseBuilder = - new OutputActionCaseBuilder(); - - OutputActionBuilder outputActionBuilder = - new OutputActionBuilder(); - + OutputActionCaseBuilder outputActionCaseBuilder = new OutputActionCaseBuilder(); + OutputActionBuilder outputActionBuilder = new OutputActionBuilder(); outputActionBuilder.setPort(outPort); outputActionBuilder.setMaxLength(OFConstants.OFPCML_NO_BUFFER); - outputActionCaseBuilder.setOutputAction(outputActionBuilder.build()); - aBuild.setActionChoice(outputActionCaseBuilder.build()); - actions.add(aBuild.build()); } builder.setAction(actions); - builder.setData(inputPacket.getPayload()); - builder.setVersion(version); - builder.setXid(xid); + builder.setData(source.getPayload()); + builder.setVersion(data.getVersion()); + builder.setXid(data.getXid()); builder.setInPort(inPortNr); builder.setBufferId(bufferId); - // -------------------------------------------------------- return builder.build(); } - - private static PortNumber getPortNumber(final PathArgument pathArgument, final Short ofVersion) { - // FIXME VD P! find InstanceIdentifier helper - InstanceIdentifier.IdentifiableItem item = Arguments.checkInstanceOf(pathArgument, - InstanceIdentifier.IdentifiableItem.class); - NodeConnectorKey key = Arguments.checkInstanceOf(item.getKey(), NodeConnectorKey.class); - Long port = InventoryDataServiceUtil.portNumberfromNodeConnectorId( - OpenflowVersion.get(ofVersion), key.getId()); - return new PortNumber(port); - } -} +} \ No newline at end of file diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/data/PacketOutConvertorData.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/data/PacketOutConvertorData.java new file mode 100644 index 0000000000..1ae204a8ff --- /dev/null +++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/data/PacketOutConvertorData.java @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2016 Cisco Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ + +package org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.data; + +/** + * Convertor data used in {@link org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.PacketOutConvertor} + * containing Openflow version and XID + */ +public class PacketOutConvertorData extends VersionDatapathIdConvertorData { + private Long xid; + + /** + * Instantiates a new Packet out convertor data. + * + * @param version the version + */ + public PacketOutConvertorData(short version) { + super(version); + } + + /** + * Gets xid. + * + * @return the xid + */ + public Long getXid() { + return xid; + } + + /** + * Sets xid. + * + * @param xid the xid + */ + public void setXid(Long xid) { + this.xid = xid; + } +} \ No newline at end of file diff --git a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PacketOutConvertorTest.java b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PacketOutConvertorTest.java index c5ef62043e..cce8c34e94 100644 --- a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PacketOutConvertorTest.java +++ b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PacketOutConvertorTest.java @@ -20,6 +20,7 @@ import org.opendaylight.openflowjava.protocol.api.util.EncodeConstants; import org.opendaylight.openflowplugin.api.OFConstants; import org.opendaylight.openflowplugin.api.openflow.md.util.OpenflowVersion; import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.action.data.ActionConvertorData; +import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.data.PacketOutConvertorData; import org.opendaylight.openflowplugin.openflow.md.util.InventoryDataServiceUtil; import org.opendaylight.openflowplugin.openflow.md.util.OpenflowPortsUtil; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Uri; @@ -86,13 +87,12 @@ public class PacketOutConvertorTest { Short version = (short) 0x04; Long xid = null; - PacketOutInput message = PacketOutConvertor.toPacketOutInput( - transmitPacketInput, version, null, null); + PacketOutConvertorData data = new PacketOutConvertorData(version); + PacketOutInput message = convert(transmitPacketInput, data); //FIXME : this has to be fixed along with actions changed in openflowjava - Assert.assertEquals(PacketOutConvertorTest.buildActionForNullTransmitPacketInputAction(nodeConnKey, - version), message.getAction()); + Assert.assertEquals(buildActionForNullTransmitPacketInputAction(nodeConnKey, version), message.getAction()); Assert.assertEquals(OFConstants.OFP_NO_BUFFER, message.getBufferId()); Assert.assertEquals(new PortNumber(0xfffffffdL), message.getInPort()); @@ -166,8 +166,10 @@ public class PacketOutConvertorTest { OpenflowPortsUtil.init(); - PacketOutInput message = PacketOutConvertor.toPacketOutInput( - transmitPacketInput, version, xid, datapathId); + PacketOutConvertorData data = new PacketOutConvertorData(version); + data.setXid(xid); + data.setDatapathId(datapathId); + PacketOutInput message = convert(transmitPacketInput, data); Assert.assertEquals(transmitPacketInput.getBufferId(), message.getBufferId()); @@ -176,6 +178,7 @@ public class PacketOutConvertorTest { Assert.assertEquals((Object) version, Short.valueOf(message.getVersion())); Assert.assertEquals(xid, message.getXid()); + ActionConvertorData actionConvertorData = new ActionConvertorData(version); actionConvertorData.setDatapathId(datapathId); @@ -277,4 +280,9 @@ public class PacketOutConvertorTest { .child(Node.class, key).build(); return new NodeRef(path); } + + private PacketOutInput convert(TransmitPacketInput transmitPacketInput, PacketOutConvertorData data) { + Optional messageOptional = ConvertorManager.getInstance().convert(transmitPacketInput, data); + return messageOptional.orElse(PacketOutConvertor.defaultResult(data.getVersion())); + } } -- 2.36.6