From 1342d4bac1535342349ce52a583c29d7b5b1babd Mon Sep 17 00:00:00 2001 From: Gobinath Date: Tue, 9 Apr 2019 11:53:10 +0530 Subject: [PATCH] Synchronize Flow and Group Remove events Description: The flow and group remove events have to be synchronized too along with the add/update events which are already synchronized.If the flow/group remove events aren't synchronized, it could result in the stale flows when there are multiple flow updates and deleted (like in case of bulk evacuation) Fix: Used the existing nodeconfigurator framework to synchronize the remove events too (with nodeId as key) Change-Id: I9b386e5c8db2c35e721d539fa11558a292214ec0 Signed-off-by: Gobinath --- .../frm/impl/BundleFlowForwarder.java | 22 ++++++++------ .../applications/frm/impl/FlowForwarder.java | 29 +++++++++++-------- .../applications/frm/impl/GroupForwarder.java | 20 +++++++------ 3 files changed, 41 insertions(+), 30 deletions(-) diff --git a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/BundleFlowForwarder.java b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/BundleFlowForwarder.java index 46d866d73d..ed0d251225 100644 --- a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/BundleFlowForwarder.java +++ b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/BundleFlowForwarder.java @@ -83,21 +83,25 @@ public class BundleFlowForwarder implements BundleMessagesCommiter { @Override public void remove(final InstanceIdentifier identifier, final Flow flow, final InstanceIdentifier nodeIdent, final BundleId bundleId) { - final List messages = new ArrayList<>(1); - String node = nodeIdent.firstKeyOf(Node.class).getId().getValue(); - BundleInnerMessage bundleInnerMessage = new BundleRemoveFlowCaseBuilder() + final NodeId nodeId = getNodeIdFromNodeIdentifier(nodeIdent); + nodeConfigurator.enqueueJob(nodeId.getValue(), () -> { + final List messages = new ArrayList<>(1); + String node = nodeIdent.firstKeyOf(Node.class).getId().getValue(); + BundleInnerMessage bundleInnerMessage = new BundleRemoveFlowCaseBuilder() .setRemoveFlowCaseData(new RemoveFlowCaseDataBuilder(flow).build()).build(); - Message message = new MessageBuilder().setNode(new NodeRef(nodeIdent.firstIdentifierOf(Node.class))) + Message message = new MessageBuilder().setNode(new NodeRef(nodeIdent.firstIdentifierOf(Node.class))) .setBundleInnerMessage(bundleInnerMessage).build(); - messages.add(message); - AddBundleMessagesInput addBundleMessagesInput = new AddBundleMessagesInputBuilder() + messages.add(message); + AddBundleMessagesInput addBundleMessagesInput = new AddBundleMessagesInputBuilder() .setNode(new NodeRef(nodeIdent.firstIdentifierOf(Node.class))).setBundleId(bundleId) .setFlags(BUNDLE_FLAGS).setMessages(new MessagesBuilder().setMessage(messages).build()).build(); - final ListenableFuture> resultFuture = forwardingRulesManager + final ListenableFuture> resultFuture = forwardingRulesManager .getSalBundleService().addBundleMessages(addBundleMessagesInput); - LOG.trace("Pushing flow remove message {} to bundle {} for device {}", addBundleMessagesInput, + LOG.trace("Pushing flow remove message {} to bundle {} for device {}", addBundleMessagesInput, bundleId.getValue(), node); - LoggingFutures.addErrorLogging(resultFuture, LOG, "removeBundleFlow"); + LoggingFutures.addErrorLogging(resultFuture, LOG, "removeBundleFlow"); + return resultFuture; + }); } @Override diff --git a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/FlowForwarder.java b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/FlowForwarder.java index 689ffe48c0..b64fe2b794 100644 --- a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/FlowForwarder.java +++ b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/FlowForwarder.java @@ -128,18 +128,23 @@ public class FlowForwarder extends AbstractListeningCommiter { if (bundleId != null) { provider.getBundleFlowListener().remove(identifier, removeDataObj, nodeIdent, bundleId); } else { - final RemoveFlowInputBuilder builder = new RemoveFlowInputBuilder(removeDataObj); - builder.setFlowRef(new FlowRef(identifier)); - builder.setNode(new NodeRef(nodeIdent.firstIdentifierOf(Node.class))); - builder.setFlowTable(new FlowTableRef(nodeIdent.child(Table.class, tableKey))); - - // This method is called only when a given flow object has been - // removed from datastore. So FRM always needs to set strict flag - // into remove-flow input so that only a flow entry associated with - // a given flow object is removed. - builder.setTransactionUri(new Uri(provider.getNewTransactionId())).setStrict(Boolean.TRUE); - LoggingFutures.addErrorLogging(provider.getSalFlowService().removeFlow(builder.build()), LOG, - "removeFlow"); + final NodeId nodeId = getNodeIdFromNodeIdentifier(nodeIdent); + nodeConfigurator.enqueueJob(nodeId.getValue(), () -> { + final RemoveFlowInputBuilder builder = new RemoveFlowInputBuilder(removeDataObj); + builder.setFlowRef(new FlowRef(identifier)); + builder.setNode(new NodeRef(nodeIdent.firstIdentifierOf(Node.class))); + builder.setFlowTable(new FlowTableRef(nodeIdent.child(Table.class, tableKey))); + + // This method is called only when a given flow object has been + // removed from datastore. So FRM always needs to set strict flag + // into remove-flow input so that only a flow entry associated with + // a given flow object is removed. + builder.setTransactionUri(new Uri(provider.getNewTransactionId())).setStrict(Boolean.TRUE); + final ListenableFuture> resultFuture = + provider.getSalFlowService().removeFlow(builder.build()); + LoggingFutures.addErrorLogging(resultFuture, LOG, "removeFlow"); + return resultFuture; + }); } } } diff --git a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/GroupForwarder.java b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/GroupForwarder.java index 3dd51cad18..0d5848d80c 100644 --- a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/GroupForwarder.java +++ b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/GroupForwarder.java @@ -109,20 +109,22 @@ public class GroupForwarder extends AbstractListeningCommiter { if (bundleId != null) { provider.getBundleGroupListener().remove(identifier, removeDataObj, nodeIdent, bundleId); } else { - final Group group = removeDataObj; - final RemoveGroupInputBuilder builder = new RemoveGroupInputBuilder(group); final NodeId nodeId = getNodeIdFromNodeIdentifier(nodeIdent); + nodeConfigurator.enqueueJob(nodeId.getValue(), () -> { + final Group group = removeDataObj; + final RemoveGroupInputBuilder builder = new RemoveGroupInputBuilder(group); + builder.setNode(new NodeRef(nodeIdent.firstIdentifierOf(Node.class))); + builder.setGroupRef(new GroupRef(identifier)); + builder.setTransactionUri(new Uri(provider.getNewTransactionId())); - builder.setNode(new NodeRef(nodeIdent.firstIdentifierOf(Node.class))); - builder.setGroupRef(new GroupRef(identifier)); - builder.setTransactionUri(new Uri(provider.getNewTransactionId())); - - final ListenableFuture> resultFuture = + final ListenableFuture> resultFuture = this.provider.getSalGroupService().removeGroup(builder.build()); - Futures.addCallback(resultFuture, + Futures.addCallback(resultFuture, new RemoveGroupCallBack(removeDataObj.getGroupId().getValue(), nodeId), MoreExecutors.directExecutor()); - LoggingFutures.addErrorLogging(resultFuture, LOG, "removeGroup"); + LoggingFutures.addErrorLogging(resultFuture, LOG, "removeGroup"); + return resultFuture; + }); } } -- 2.36.6