Bug 2987 - Set-Vlan action on OF1.3 Sends 2 remove flows (similar to addFlow)

Change-Id: I408e5d7931e9fab0c02b0b6eb84f7e8d8ed6bf9d
Signed-off-by: Claudio D. Gasparini <cgaspari@cisco.com>
Signed-off-by: Martin Bobak <mbobak@cisco.com>
(cherry picked from commit 8d4984a0e75093877652ae31b1e5dfb4d7a49fb9)

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/FlowRemoveService.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/FlowRemoveService.java
deleted file mode 100644 (file)
index 6a0bdf4..0000000
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/FlowRemoveService.java
+++ /dev/null
@@ -1,31 +0,0 @@
-/**
- * Copyright (c) 2015 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.impl.services;
-
-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.FlowConvertor;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.service.rev130819.RemoveFlowInput;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.service.rev130819.RemoveFlowOutput;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.FlowModInputBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.OfHeader;
-
-final class FlowRemoveService extends AbstractSimpleService<RemoveFlowInput, RemoveFlowOutput> {
-    FlowRemoveService(final RequestContextStack requestContextStack, final DeviceContext deviceContext) {
-        super(requestContextStack, deviceContext, RemoveFlowOutput.class);
-    }
-
-    @Override
-    protected OfHeader buildRequest(final Xid xid, final RemoveFlowInput input) {
-        final FlowModInputBuilder ofFlowModInput = FlowConvertor.toFlowModInput(input, getVersion(),
-            getDatapathId());
-        ofFlowModInput.setXid(xid.getValue());
-        return ofFlowModInput.build();
-    }
-}

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java
index 2ff2091068d77826d7cac3676a7930e093b0c417..2f638baa92d88453ab066a39db282cac6c813255 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java
@@ -47,10 +47,10 @@ public class SalFlowServiceImpl implements SalFlowService {
     private static final Logger LOG = LoggerFactory.getLogger(SalFlowServiceImpl.class);
     private final FlowService<UpdateFlowOutput> flowUpdate;
     private final FlowService<AddFlowOutput> flowAdd;
-    private final FlowRemoveService flowRemove;
+    private final FlowService<RemoveFlowOutput> flowRemove;
 
     public SalFlowServiceImpl(final RequestContextStack requestContextStack, final DeviceContext deviceContext) {
-        flowRemove = new FlowRemoveService(requestContextStack, deviceContext);
+        flowRemove = new FlowService(requestContextStack, deviceContext, RemoveFlowOutput.class);
         flowAdd = new FlowService<>(requestContextStack, deviceContext, AddFlowOutput.class);
         flowUpdate = new FlowService<>(requestContextStack, deviceContext, UpdateFlowOutput.class);
     }
@@ -93,7 +93,7 @@ public class SalFlowServiceImpl implements SalFlowService {
     public Future<RpcResult<RemoveFlowOutput>> removeFlow(final RemoveFlowInput input) {
         LOG.trace("Calling remove flow for flow with ID ={}.", input.getFlowRef());
 
-        final ListenableFuture<RpcResult<RemoveFlowOutput>> future = flowRemove.handleServiceCall(input);
+        final ListenableFuture<RpcResult<RemoveFlowOutput>> future = flowRemove.processFlowModInputBuilders(flowRemove.toFlowModInputs(input));
         Futures.addCallback(future, new FutureCallback<RpcResult<RemoveFlowOutput>>() {
             @Override
             public void onSuccess(final RpcResult<RemoveFlowOutput> result) {

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/OFRpcTaskFactory.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/OFRpcTaskFactory.java
index c7f9662ecfb2ef3a5a9229e551197b85a4ea4b3f..1ef7abe356bf0a27e49de9aff850f2b0b46d6451 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/OFRpcTaskFactory.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/OFRpcTaskFactory.java
@@ -700,19 +700,15 @@ public abstract class OFRpcTaskFactory {
                 ListenableFuture<RpcResult<UpdateFlowOutput>> result = SettableFuture.create();
 
                 // Convert the AddFlowInput to FlowModInput
-                FlowModInputBuilder ofFlowModInput = FlowConvertor.toFlowModInput(getInput(),
+                List<FlowModInputBuilder> ofFlowModInputs = FlowConvertor.toFlowModInputs(getInput(),
                         getVersion(), getSession().getFeatures().getDatapathId());
-                final Long xId = getSession().getNextXid();
-                ofFlowModInput.setXid(xId);
-
-                Future<RpcResult<UpdateFlowOutput>> resultFromOFLib =
-                        getMessageService().flowMod(ofFlowModInput.build(), getCookie());
-                result = JdkFutureAdapters.listenInPoolThread(resultFromOFLib);
 
+                result = chainFlowMods(ofFlowModInputs, 0, getTaskContext(), getCookie());
                 result = OFRpcTaskUtil.chainFutureBarrier(this, result);
-                OFRpcTaskUtil.hookFutureNotification(this, result,
-                        getRpcNotificationProviderService(), createFlowRemovedNotification(getInput()));
 
+                OFRpcTaskUtil.hookFutureNotification(this, result,
+                        getRpcNotificationProviderService(),
+                        createFlowRemovedNotification(getInput()));
                 return result;
             }
         }

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/FlowConvertor.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/FlowConvertor.java
index aca8cd23537be4a91cf051d24983bc2690f9fbef..ee375d8b227e6146e9ed9ce7591209a28ac4ab8f 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/FlowConvertor.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/FlowConvertor.java
@@ -134,6 +134,7 @@ public class FlowConvertor {
      * default match entries - empty
      */
     public static final List<MatchEntry> DEFAULT_MATCH_ENTRIES = new ArrayList<MatchEntry>();
+    private static final Integer PUSH_VLAN = 0x8100;
 
     private static final Ordering<org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.instruction.list.Instruction> INSTRUCTION_ORDERING =
             Ordering.from(OrderComparator.<org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.instruction.list.Instruction>build());
@@ -177,7 +178,7 @@ public class FlowConvertor {
         }
     }
 
-    public static FlowModInputBuilder toFlowModInput(Flow flow, short version, BigInteger datapathid) {
+    private static FlowModInputBuilder toFlowModInput(Flow flow, short version, BigInteger datapathid) {
 
         FlowModInputBuilder flowMod = new FlowModInputBuilder();
         salToOFFlowCookie(flow, flowMod);
@@ -532,10 +533,8 @@ public class FlowConvertor {
 
                         pushVlanActionBuilder.setCfi(new VlanCfi(1))
                                 .setVlanId(setVlanIdActionCase.getSetVlanIdAction().getVlanId())
-                                .setEthernetType(sourceFlow.getMatch().getEthernetMatch()
-                                        .getEthernetType().getType().getValue().intValue())
-                                .setTag(sourceFlow.getMatch().getEthernetMatch()
-                                        .getEthernetType().getType().getValue().intValue());
+                                .setEthernetType(PUSH_VLAN)
+                                .setTag(PUSH_VLAN);
                         pushVlanActionCaseBuilder.setPushVlanAction(pushVlanActionBuilder.build());
                         PushVlanActionCase injectedAction = pushVlanActionCaseBuilder.build();
 

diff --git a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/FlowConvertorTest.java b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/FlowConvertorTest.java
index 5261bd1ce5784db5c848e2ca0c1114caa12d3f5d..383cd9c47b4f6355479023a7cbf94f4ec1fe1f18 100644 (file)
--- a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/FlowConvertorTest.java
+++ b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/FlowConvertorTest.java
@@ -48,6 +48,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.instruction
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.FlowModCommand;\r
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.FlowModFlags;\r
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.FlowModInput;\r
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.FlowModInputBuilder;\r
 \r
 /**\r
  * @author michal.polkorab\r
@@ -56,7 +57,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731
 public class FlowConvertorTest {\r
 \r
     /**\r
-     * Tests {@link FlowConvertor#toFlowModInput(Flow, short, BigInteger)}\r
+     * Tests {@link FlowConvertor#toFlowModInputs(Flow, short, BigInteger)}\r
      */\r
     @Test\r
     public void test() {\r
@@ -76,28 +77,28 @@ public class FlowConvertorTest {
         flowBuilder.setMatch(null);\r
         RemoveFlowInput flow = flowBuilder.build();\r
 \r
-        FlowModInput flowMod = FlowConvertor\r
-                .toFlowModInput(flow, EncodeConstants.OF13_VERSION_ID, new BigInteger("42")).build();\r
+        List<FlowModInputBuilder> flowMod = FlowConvertor\r
+                .toFlowModInputs(flow, EncodeConstants.OF13_VERSION_ID, new BigInteger("42"));\r
 \r
-        Assert.assertEquals("Wrong version", 4, flowMod.getVersion().intValue());\r
-        Assert.assertEquals("Wrong cookie", 4, flowMod.getCookie().intValue());\r
-        Assert.assertEquals("Wrong cookie mask", 5, flowMod.getCookieMask().intValue());\r
-        Assert.assertEquals("Wrong table id", 6, flowMod.getTableId().getValue().intValue());\r
-        Assert.assertEquals("Wrong command", FlowModCommand.OFPFCDELETESTRICT, flowMod.getCommand());\r
-        Assert.assertEquals("Wrong idle timeout", 50, flowMod.getIdleTimeout().intValue());\r
-        Assert.assertEquals("Wrong hard timeout", 500, flowMod.getHardTimeout().intValue());\r
-        Assert.assertEquals("Wrong priority", 40, flowMod.getPriority().intValue());\r
-        Assert.assertEquals("Wrong buffer id", 18, flowMod.getBufferId().intValue());\r
-        Assert.assertEquals("Wrong out port", 65535, flowMod.getOutPort().getValue().intValue());\r
-        Assert.assertEquals("Wrong out group", 5000, flowMod.getOutGroup().intValue());\r
-        Assert.assertEquals("Wrong flags", new FlowModFlags(false, false, false, false, false), flowMod.getFlags());\r
+        Assert.assertEquals("Wrong version", 4, flowMod.get(0).getVersion().intValue());\r
+        Assert.assertEquals("Wrong cookie", 4, flowMod.get(0).getCookie().intValue());\r
+        Assert.assertEquals("Wrong cookie mask", 5, flowMod.get(0).getCookieMask().intValue());\r
+        Assert.assertEquals("Wrong table id", 6, flowMod.get(0).getTableId().getValue().intValue());\r
+        Assert.assertEquals("Wrong command", FlowModCommand.OFPFCDELETESTRICT, flowMod.get(0).getCommand());\r
+        Assert.assertEquals("Wrong idle timeout", 50, flowMod.get(0).getIdleTimeout().intValue());\r
+        Assert.assertEquals("Wrong hard timeout", 500, flowMod.get(0).getHardTimeout().intValue());\r
+        Assert.assertEquals("Wrong priority", 40, flowMod.get(0).getPriority().intValue());\r
+        Assert.assertEquals("Wrong buffer id", 18, flowMod.get(0).getBufferId().intValue());\r
+        Assert.assertEquals("Wrong out port", 65535, flowMod.get(0).getOutPort().getValue().intValue());\r
+        Assert.assertEquals("Wrong out group", 5000, flowMod.get(0).getOutGroup().intValue());\r
+        Assert.assertEquals("Wrong flags", new FlowModFlags(false, false, false, false, false), flowMod.get(0).getFlags());\r
         Assert.assertEquals("Wrong match", "org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.oxm.rev150225.OxmMatchType",\r
-                flowMod.getMatch().getType().getName());\r
-        Assert.assertEquals("Wrong match entries size", 0, flowMod.getMatch().getMatchEntry().size());\r
+                flowMod.get(0).getMatch().getType().getName());\r
+        Assert.assertEquals("Wrong match entries size", 0, flowMod.get(0).getMatch().getMatchEntry().size());\r
     }\r
 \r
     /**\r
-     * Tests {@link FlowConvertor#toFlowModInput(Flow, short, BigInteger)}\r
+     * Tests {@link FlowConvertor#toFlowModInputs(Flow, short, BigInteger)}\r
      */\r
     @Test\r
     public void testOnlyModifyStrictCommand() {\r
@@ -105,15 +106,15 @@ public class FlowConvertorTest {
         flowBuilder.setStrict(true);\r
         UpdatedFlow flow = flowBuilder.build();\r
 \r
-        FlowModInput flowMod = FlowConvertor\r
-                .toFlowModInput(flow, EncodeConstants.OF10_VERSION_ID, new BigInteger("42")).build();\r
+        List<FlowModInputBuilder> flowMod = FlowConvertor\r
+                .toFlowModInputs(flow, EncodeConstants.OF10_VERSION_ID, new BigInteger("42"));\r
 \r
-        Assert.assertEquals("Wrong version", 1, flowMod.getVersion().intValue());\r
-        Assert.assertEquals("Wrong command", FlowModCommand.OFPFCMODIFYSTRICT, flowMod.getCommand());\r
+        Assert.assertEquals("Wrong version", 1, flowMod.get(0).getVersion().intValue());\r
+        Assert.assertEquals("Wrong command", FlowModCommand.OFPFCMODIFYSTRICT, flowMod.get(0).getCommand());\r
     }\r
 \r
     /**\r
-     * Tests {@link FlowConvertor#toFlowModInput(Flow, short, BigInteger)}\r
+     * Tests {@link FlowConvertor#toFlowModInputs(Flow, short, BigInteger)}\r
      */\r
     @Test\r
     public void testInstructionsTranslation() {\r
@@ -176,19 +177,19 @@ public class FlowConvertorTest {
         flowBuilder.setInstructions(instructionsBuilder.build());\r
         AddFlowInput flow = flowBuilder.build();\r
 \r
-        FlowModInput flowMod = FlowConvertor\r
-                .toFlowModInput(flow, EncodeConstants.OF10_VERSION_ID, new BigInteger("42")).build();\r
+        List<FlowModInputBuilder> flowMod = FlowConvertor\r
+                .toFlowModInputs(flow, EncodeConstants.OF10_VERSION_ID, new BigInteger("42"));\r
 \r
-        Assert.assertEquals("Wrong version", 1, flowMod.getVersion().intValue());\r
-        Assert.assertEquals("Wrong command", FlowModCommand.OFPFCADD, flowMod.getCommand());\r
-        Assert.assertEquals("Wrong instructions size", 6, flowMod.getInstruction().size());\r
+        Assert.assertEquals("Wrong version", 1, flowMod.get(0).getVersion().intValue());\r
+        Assert.assertEquals("Wrong command", FlowModCommand.OFPFCADD, flowMod.get(0).getCommand());\r
+        Assert.assertEquals("Wrong instructions size", 6, flowMod.get(0).getInstruction().size());\r
         org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.instruction.rev130731.instructions\r
-        .grouping.Instruction instruction = flowMod.getInstruction().get(0);\r
+        .grouping.Instruction instruction = flowMod.get(0).getInstruction().get(0);\r
         Assert.assertEquals("Wrong type", "org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common"\r
                 + ".instruction.rev130731.instruction.grouping.instruction.choice.GotoTableCase", instruction.getInstructionChoice().getImplementedInterface().getName());\r
         GotoTableCase gotoTableCase = (GotoTableCase) instruction.getInstructionChoice();\r
         Assert.assertEquals("Wrong table id", 1, gotoTableCase.getGotoTable().getTableId().intValue());\r
-        instruction = flowMod.getInstruction().get(1);\r
+        instruction = flowMod.get(0).getInstruction().get(1);\r
         Assert.assertEquals("Wrong type", "org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common"\r
                 + ".instruction.rev130731.instruction.grouping.instruction.choice.WriteMetadataCase", instruction.getInstructionChoice().getImplementedInterface().getName());\r
         WriteMetadataCase writeMetadataCase = (WriteMetadataCase) instruction.getInstructionChoice();\r
@@ -197,20 +198,20 @@ public class FlowConvertorTest {
         Assert.assertArrayEquals("Wrong metadata mask", new byte[]{0, 0, 0, 0, 0, 0, 0, 3},\r
                 writeMetadataCase.getWriteMetadata().getMetadataMask());\r
         \r
-        instruction = flowMod.getInstruction().get(2);\r
+        instruction = flowMod.get(0).getInstruction().get(2);\r
         Assert.assertEquals("Wrong type", "org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common"\r
                 + ".instruction.rev130731.instruction.grouping.instruction.choice.WriteActionsCase", instruction.getInstructionChoice().getImplementedInterface().getName());\r
         WriteActionsCase writeActionsCase = (WriteActionsCase) instruction.getInstructionChoice();\r
         Assert.assertEquals("Wrong actions size", 0, writeActionsCase.getWriteActions().getAction().size());\r
-        instruction = flowMod.getInstruction().get(3);\r
+        instruction = flowMod.get(0).getInstruction().get(3);\r
         Assert.assertEquals("Wrong type", "org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common"\r
                 + ".instruction.rev130731.instruction.grouping.instruction.choice.ApplyActionsCase", instruction.getInstructionChoice().getImplementedInterface().getName());\r
         ApplyActionsCase applyActionsCase =  (ApplyActionsCase) instruction.getInstructionChoice();\r
         Assert.assertEquals("Wrong actions size", 0, applyActionsCase.getApplyActions().getAction().size());\r
-        instruction = flowMod.getInstruction().get(4);\r
+        instruction = flowMod.get(0).getInstruction().get(4);\r
         Assert.assertEquals("Wrong type", "org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common"\r
                 + ".instruction.rev130731.instruction.grouping.instruction.choice.ClearActionsCase", instruction.getInstructionChoice().getImplementedInterface().getName());\r
-        instruction = flowMod.getInstruction().get(5);\r
+        instruction = flowMod.get(0).getInstruction().get(5);\r
         Assert.assertEquals("Wrong type", "org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common"\r
                 + ".instruction.rev130731.instruction.grouping.instruction.choice.MeterCase", instruction.getInstructionChoice().getImplementedInterface().getName());\r
         MeterCase meterCase = (MeterCase) instruction.getInstructionChoice();\r