From f1b810b815f018e77c5198b9e489d55b025c74c7 Mon Sep 17 00:00:00 2001 From: Anil Vishnoi Date: Wed, 17 May 2017 14:46:19 -0700 Subject: [PATCH] Bug 8497 - Provide config knob to disable the Forwarding Rule Manager reconciliation Resolves: Bug 8497, Bug 7957 Change-Id: Ib798d9282f019e0c8dd520c15d8794678ee9af3d Signed-off-by: Anil Vishnoi --- .../frm/ForwardingRulesManager.java | 18 ++++++++++--- .../frm/impl/AbstractListeningCommiter.java | 2 +- .../frm/impl/DeviceMastership.java | 1 - .../frm/impl/FlowNodeReconciliationImpl.java | 12 ++++++--- .../frm/impl/ForwardingRulesManagerImpl.java | 27 ++++++++++++++++--- .../blueprint/forwardingrules-manager.xml | 18 +++++++++++++ .../test/java/test/mock/FlowListenerTest.java | 2 +- .../java/test/mock/GroupListenerTest.java | 2 +- .../java/test/mock/MeterListenerTest.java | 2 +- .../test/java/test/mock/NodeListenerTest.java | 2 +- .../test/mock/TableFeaturesListenerTest.java | 2 +- .../main/resources/initial/openflowplugin.cfg | 21 +++++++++++++++ .../impl/OpenFlowPluginProviderImpl.java | 1 - 13 files changed, 93 insertions(+), 17 deletions(-) diff --git a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/ForwardingRulesManager.java b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/ForwardingRulesManager.java index bef847c025..0dcd3aa176 100644 --- a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/ForwardingRulesManager.java +++ b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/ForwardingRulesManager.java @@ -119,10 +119,22 @@ public interface ForwardingRulesManager extends AutoCloseable { ForwardingRulesCommiter getTableFeaturesCommiter(); /** - * Returns the config-subsystem/fallback configuration of FRM - * @return ForwardingRulesManagerConfig + * Check if reconciliation is disabled by user. + * @return true if reconciliation is disabled, else false */ - ForwardingRulesManagerConfig getConfiguration(); + boolean isReconciliationDisabled(); + + /** + * Check if stale marking is enabled for switch reconciliation. + * @return true if stale marking is enabled, else false + */ + boolean isStaleMarkingEnabled(); + + /** + * Return number of reconciliation retry are allowed. + * @return number of retries. + */ + int getReconciliationRetryCount(); /** * Method checks if *this* instance of openflowplugin is owner of diff --git a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/AbstractListeningCommiter.java b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/AbstractListeningCommiter.java index 5a77d9e1cd..c3df34644c 100644 --- a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/AbstractListeningCommiter.java +++ b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/AbstractListeningCommiter.java @@ -64,7 +64,7 @@ public abstract class AbstractListeningCommiter implement } } else{ - if (provider.getConfiguration().isStaleMarkingEnabled()) { + if (provider.isStaleMarkingEnabled()) { LOG.info("Stale-Marking ENABLED and switch {} is NOT connected, storing stale entities", nodeIdent.toString()); // Switch is NOT connected diff --git a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/DeviceMastership.java b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/DeviceMastership.java index 182b95343c..82aec1cba4 100644 --- a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/DeviceMastership.java +++ b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/DeviceMastership.java @@ -77,7 +77,6 @@ public class DeviceMastership implements ClusterSingletonService, AutoCloseable public void setDeviceOperationalStatus(boolean inOperDS) { isDeviceInOperDS.set(inOperDS); if(canReconcile()) { - LOG.info("Triggering reconciliation for device {}", nodeId.getValue()); reconcliationAgent.reconcileConfiguration(fcnIID); } } diff --git a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/FlowNodeReconciliationImpl.java b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/FlowNodeReconciliationImpl.java index 52522f7230..c530713c59 100644 --- a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/FlowNodeReconciliationImpl.java +++ b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/FlowNodeReconciliationImpl.java @@ -106,8 +106,14 @@ public class FlowNodeReconciliationImpl implements FlowNodeReconciliation { @Override public void reconcileConfiguration(InstanceIdentifier connectedNode) { + if (provider.isReconciliationDisabled()) { + LOG.debug("Reconciliation is disabled by user. Skipping reconciliation of node : {}", connectedNode + .firstKeyOf(Node.class)); + return; + } if (provider.isNodeOwner(connectedNode)) { - if (provider.getConfiguration().isStaleMarkingEnabled()) { + LOG.info("Triggering reconciliation for device {}", connectedNode.firstKeyOf(Node.class)); + if (provider.isStaleMarkingEnabled()) { LOG.info("Stale-Marking is ENABLED and proceeding with deletion of stale-marked entities on switch {}", connectedNode.toString()); reconciliationPreProcess(connectedNode); @@ -164,7 +170,7 @@ public class FlowNodeReconciliationImpl implements FlowNodeReconciliation { Map> groupFutures = new HashMap<>(); while ((!(toBeInstalledGroups.isEmpty()) || !(suspectedGroups.isEmpty())) && - (counter <= provider.getConfiguration().getReconciliationRetryCount())) { //also check if the counter has not crossed the threshold + (counter <= provider.getReconciliationRetryCount())) { //also check if the counter has not crossed the threshold if (toBeInstalledGroups.isEmpty() && !suspectedGroups.isEmpty()) { LOG.error("These Groups are pointing to node-connectors that are not up yet {}", suspectedGroups.toString()); @@ -249,7 +255,7 @@ public class FlowNodeReconciliationImpl implements FlowNodeReconciliation { if (!toBeInstalledGroups.isEmpty()) { for (Group group : toBeInstalledGroups) { LOG.error("Installing the group {} finally although the port is not up after checking for {} times " - , group.getGroupId().toString(), provider.getConfiguration().getReconciliationRetryCount()); + , group.getGroupId().toString(), provider.getReconciliationRetryCount()); addGroup(groupFutures, group); } } diff --git a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/ForwardingRulesManagerImpl.java b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/ForwardingRulesManagerImpl.java index d055ca260d..9bfa943639 100644 --- a/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/ForwardingRulesManagerImpl.java +++ b/applications/forwardingrules-manager/src/main/java/org/opendaylight/openflowplugin/applications/frm/impl/ForwardingRulesManagerImpl.java @@ -14,6 +14,7 @@ import com.google.common.base.Preconditions; import com.google.common.util.concurrent.CheckedFuture; import java.util.Objects; import java.util.concurrent.atomic.AtomicLong; +import javax.annotation.Nonnull; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; @@ -62,6 +63,9 @@ public class ForwardingRulesManagerImpl implements ForwardingRulesManager { private final ForwardingRulesManagerConfig forwardingRulesManagerConfig; private final ClusterSingletonServiceProvider clusterSingletonServiceProvider; private final NotificationProviderService notificationService; + private final boolean disableReconciliation; + private final boolean staleMarkingEnabled; + private final int reconciliationRetryCount; private ForwardingRulesCommiter flowListener; private ForwardingRulesCommiter groupListener; @@ -75,7 +79,10 @@ public class ForwardingRulesManagerImpl implements ForwardingRulesManager { final RpcConsumerRegistry rpcRegistry, final ForwardingRulesManagerConfig config, final ClusterSingletonServiceProvider clusterSingletonService, - final NotificationProviderService notificationService) { + final NotificationProviderService notificationService, + final boolean disableReconciliation, + final boolean staleMarkingEnabled, + final int reconciliationRetryCount) { this.dataService = Preconditions.checkNotNull(dataBroker, "DataBroker can not be null!"); this.forwardingRulesManagerConfig = Preconditions.checkNotNull(config, "Configuration for FRM cannot be null"); this.clusterSingletonServiceProvider = Preconditions.checkNotNull(clusterSingletonService, @@ -93,6 +100,10 @@ public class ForwardingRulesManagerImpl implements ForwardingRulesManager { "RPC SalMeterService not found."); this.salTableService = Preconditions.checkNotNull(rpcRegistry.getRpcService(SalTableService.class), "RPC SalTableService not found."); + + this.disableReconciliation = disableReconciliation; + this.staleMarkingEnabled = staleMarkingEnabled; + this.reconciliationRetryCount = reconciliationRetryCount; } @Override @@ -217,8 +228,18 @@ public class ForwardingRulesManagerImpl implements ForwardingRulesManager { } @Override - public ForwardingRulesManagerConfig getConfiguration() { - return forwardingRulesManagerConfig; + public boolean isReconciliationDisabled() { + return this.disableReconciliation; + } + + @Override + public boolean isStaleMarkingEnabled() { + return this.staleMarkingEnabled; + } + + @Override + public int getReconciliationRetryCount() { + return this.reconciliationRetryCount; } @Override diff --git a/applications/forwardingrules-manager/src/main/resources/org/opendaylight/blueprint/forwardingrules-manager.xml b/applications/forwardingrules-manager/src/main/resources/org/opendaylight/blueprint/forwardingrules-manager.xml index c7d81bdc02..34a6dc8f45 100644 --- a/applications/forwardingrules-manager/src/main/resources/org/opendaylight/blueprint/forwardingrules-manager.xml +++ b/applications/forwardingrules-manager/src/main/resources/org/opendaylight/blueprint/forwardingrules-manager.xml @@ -1,5 +1,6 @@ @@ -11,6 +12,19 @@ + + + + + + + + + + + @@ -18,5 +32,9 @@ + + + + \ No newline at end of file diff --git a/applications/forwardingrules-manager/src/test/java/test/mock/FlowListenerTest.java b/applications/forwardingrules-manager/src/test/java/test/mock/FlowListenerTest.java index 997c556f75..5246750de9 100644 --- a/applications/forwardingrules-manager/src/test/java/test/mock/FlowListenerTest.java +++ b/applications/forwardingrules-manager/src/test/java/test/mock/FlowListenerTest.java @@ -74,7 +74,7 @@ public class FlowListenerTest extends FRMTest { rpcProviderRegistryMock, getConfig(), clusterSingletonService, - notificationService); + notificationService, false, false, 5); forwardingRulesManager.start(); // TODO consider tests rewrite (added because of complicated access) forwardingRulesManager.setDeviceMastershipManager(deviceMastershipManager); diff --git a/applications/forwardingrules-manager/src/test/java/test/mock/GroupListenerTest.java b/applications/forwardingrules-manager/src/test/java/test/mock/GroupListenerTest.java index 91f437c955..7c990531d3 100644 --- a/applications/forwardingrules-manager/src/test/java/test/mock/GroupListenerTest.java +++ b/applications/forwardingrules-manager/src/test/java/test/mock/GroupListenerTest.java @@ -64,7 +64,7 @@ public class GroupListenerTest extends FRMTest { rpcProviderRegistryMock, getConfig(), clusterSingletonService, - notificationService); + notificationService, false, false, 5); forwardingRulesManager.start(); // TODO consider tests rewrite (added because of complicated access) forwardingRulesManager.setDeviceMastershipManager(deviceMastershipManager); diff --git a/applications/forwardingrules-manager/src/test/java/test/mock/MeterListenerTest.java b/applications/forwardingrules-manager/src/test/java/test/mock/MeterListenerTest.java index 1afff7b914..6fca053ef6 100644 --- a/applications/forwardingrules-manager/src/test/java/test/mock/MeterListenerTest.java +++ b/applications/forwardingrules-manager/src/test/java/test/mock/MeterListenerTest.java @@ -64,7 +64,7 @@ public class MeterListenerTest extends FRMTest { rpcProviderRegistryMock, getConfig(), clusterSingletonService, - notificationService); + notificationService, false, false, 5); forwardingRulesManager.start(); // TODO consider tests rewrite (added because of complicated access) forwardingRulesManager.setDeviceMastershipManager(deviceMastershipManager); diff --git a/applications/forwardingrules-manager/src/test/java/test/mock/NodeListenerTest.java b/applications/forwardingrules-manager/src/test/java/test/mock/NodeListenerTest.java index 4834f9f840..cfae719021 100644 --- a/applications/forwardingrules-manager/src/test/java/test/mock/NodeListenerTest.java +++ b/applications/forwardingrules-manager/src/test/java/test/mock/NodeListenerTest.java @@ -46,7 +46,7 @@ public class NodeListenerTest extends FRMTest { rpcProviderRegistryMock, getConfig(), clusterSingletonService, - notificationService); + notificationService, false ,false ,5); forwardingRulesManager.start(); } diff --git a/applications/forwardingrules-manager/src/test/java/test/mock/TableFeaturesListenerTest.java b/applications/forwardingrules-manager/src/test/java/test/mock/TableFeaturesListenerTest.java index 1060e795bd..52e0b85169 100644 --- a/applications/forwardingrules-manager/src/test/java/test/mock/TableFeaturesListenerTest.java +++ b/applications/forwardingrules-manager/src/test/java/test/mock/TableFeaturesListenerTest.java @@ -59,7 +59,7 @@ public class TableFeaturesListenerTest extends FRMTest { rpcProviderRegistryMock, getConfig(), clusterSingletonService, - notificationService); + notificationService, false, false , 5); forwardingRulesManager.start(); // TODO consider tests rewrite (added because of complicated access) forwardingRulesManager.setDeviceMastershipManager(deviceMastershipManager); diff --git a/openflowplugin-blueprint-config/src/main/resources/initial/openflowplugin.cfg b/openflowplugin-blueprint-config/src/main/resources/initial/openflowplugin.cfg index 0adf815b8a..a3759234c5 100644 --- a/openflowplugin-blueprint-config/src/main/resources/initial/openflowplugin.cfg +++ b/openflowplugin-blueprint-config/src/main/resources/initial/openflowplugin.cfg @@ -97,3 +97,24 @@ # then to format supported by device, and reversed when deserializing. # # use-single-layer-serialization=true + +############################################################################# +# # +# Forwarding Rule Manager Application Configuration # +# # +############################################################################# + +# Disable the default switch reconciliation mechanism +# disable-reconciliation=false + +# Enable stale marking for switch reconciliation. Once user enable this feature +# forwarding rule manager will keep track of any change to the config data store +# while the switch is disconnected from controller. Once switch reconnect to the +# controller it will apply those changes to the switch and do the reconciliation +# of other configuration as well. +# NOTE: This option will be effective only if disable_reconciliation=false. +# stale-marking-enabled=false + +# Number of time forwarding rules manager should retry to reconcile any specific +# configuration. +# reconciliation-retry-count=5 diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/OpenFlowPluginProviderImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/OpenFlowPluginProviderImpl.java index 3b53c96815..c919099948 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/OpenFlowPluginProviderImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/OpenFlowPluginProviderImpl.java @@ -457,7 +457,6 @@ public class OpenFlowPluginProviderImpl implements OpenFlowPluginProvider, OpenF modifiable = false; break; default: - LOG.warn("Unsupported configuration property '{}={}'", key, sValue); return; } -- 2.36.6