Add node callback to flow capable listener from ovsdb southbound

Use Bridge events in the ovsdb southbound to callback into flowCapableListener.
This callback can be used together (or instead of) the mdsal callback. Also,
protect the nodeCache by using a synchronization object.

Patch 2 (code review):
  - Use Constants.OPENFLOW_NODE_PREFIX instead of duplicate internal copy
  - Make code consistent when removing node form local cache (sync access)
  - Remove check for dpid value (not needed)

Patch 3 (Sonar issue):
  - No need to check for null before an instanceof; the instanceof keyword returns
    false when given a null argument.

Change-Id: Id93cd5f17366e499b7edf2db29164d6f6e586eda
Signed-off-by: Flavio Fernandes <ffernand@redhat.com>

diff --git a/integrationtest/src/test/java/org/opendaylight/ovsdb/integrationtest/neutron/NeutronIT.java b/integrationtest/src/test/java/org/opendaylight/ovsdb/integrationtest/neutron/NeutronIT.java
index 33089f58da225c6664235c588298c399db192508..4c95b10c5b2d4d419ca1d461853389eddf4cf601 100644 (file)
--- a/integrationtest/src/test/java/org/opendaylight/ovsdb/integrationtest/neutron/NeutronIT.java
+++ b/integrationtest/src/test/java/org/opendaylight/ovsdb/integrationtest/neutron/NeutronIT.java
@@ -24,6 +24,7 @@ import org.opendaylight.ovsdb.integrationtest.OvsdbIntegrationTestBase;
 import org.opendaylight.ovsdb.lib.notation.Row;
 import org.opendaylight.ovsdb.lib.notation.UUID;
 import org.opendaylight.ovsdb.lib.notation.Version;
+import org.opendaylight.ovsdb.openstack.netvirt.api.Action;
 import org.opendaylight.ovsdb.openstack.netvirt.api.ConfigurationService;
 import org.opendaylight.ovsdb.openstack.netvirt.api.Constants;
 import org.opendaylight.ovsdb.openstack.netvirt.api.BridgeConfigurationManager;
@@ -331,5 +332,10 @@ public class NeutronIT extends OvsdbIntegrationTestBase {
         public void initializeOFFlowRules(Node openflowNode) {
 
         }
+
+        @Override
+        public void notifyFlowCapableNodeEvent(Long dpid, Action action) {
+
+        }
     }
 }

diff --git a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/FlowCapableNodeDataChangeListener.java b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/FlowCapableNodeDataChangeListener.java
index 02b649ed45568f812afb42063e393dc3252f801a..0b2b17286de6f778a304d6be27f30bd57d98fc49 100644 (file)
--- a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/FlowCapableNodeDataChangeListener.java
+++ b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/FlowCapableNodeDataChangeListener.java
@@ -18,9 +18,10 @@ import org.opendaylight.controller.md.sal.binding.api.DataChangeListener;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncDataBroker;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeEvent;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
-import org.opendaylight.ovsdb.utils.mdsal.node.NodeUtils;
+import org.opendaylight.ovsdb.openstack.netvirt.api.Action;
 import org.opendaylight.ovsdb.openstack.netvirt.api.NetworkingProviderManager;
 import org.opendaylight.ovsdb.openstack.netvirt.api.NodeCacheManager;
+import org.opendaylight.ovsdb.utils.mdsal.node.NodeUtils;
 import org.opendaylight.ovsdb.utils.servicehelper.ServiceHelper;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNode;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes;
@@ -35,8 +36,8 @@ import org.slf4j.LoggerFactory;
 public class FlowCapableNodeDataChangeListener implements DataChangeListener, AutoCloseable {
     private static final Logger LOG = LoggerFactory.getLogger(FlowCapableNodeDataChangeListener.class);
     private ListenerRegistration<DataChangeListener> registration;
+    private final Object nodeCacheLock = new Object();
     private List<Node> nodeCache = Lists.newArrayList();
-    private NetworkingProviderManager networkingProviderManager = null;
     private PipelineOrchestrator pipelineOrchestrator = null;
     private NodeCacheManager nodeCacheManager = null;
 
@@ -65,10 +66,14 @@ public class FlowCapableNodeDataChangeListener implements DataChangeListener, Au
 
         for (InstanceIdentifier instanceIdentifier : changes.getRemovedPaths()) {
             DataObject originalDataObject = changes.getOriginalData().get(instanceIdentifier);
-            LOG.info(">>>>> removed iiD: {} - dataObject {}", instanceIdentifier, originalDataObject);
-            if (originalDataObject != null && originalDataObject instanceof Node){
+            if (originalDataObject instanceof Node) {
                 Node node = (Node) originalDataObject;
-                notifyNodeRemoved(NodeUtils.getOpenFlowNode(node.getId().getValue()));
+                String openflowId = node.getId().getValue();
+                LOG.info(">>>>> removed iiD: {} - NodeKey: {}", instanceIdentifier, openflowId);
+                Node openFlowNode = NodeUtils.getOpenFlowNode(openflowId);
+                if (removeNodeFromCache(openFlowNode)) {
+                    notifyNodeRemoved(openFlowNode);
+                }
             }
         }
 
@@ -78,10 +83,10 @@ public class FlowCapableNodeDataChangeListener implements DataChangeListener, Au
             LOG.info(">>>>> created iiD: {} - first: {} - NodeKey: {}",
                     iID, iID.firstIdentifierOf(Node.class), openflowId);
             Node openFlowNode = NodeUtils.getOpenFlowNode(openflowId);
-            if (nodeCache.contains(openFlowNode)) {
-                notifyNodeUpdated(openFlowNode);
-            } else {
+            if (addNodeToCache(openFlowNode)) {
                 notifyNodeCreated(openFlowNode);
+            } else {
+                notifyNodeUpdated(openFlowNode);
             }
         }
 
@@ -91,17 +96,62 @@ public class FlowCapableNodeDataChangeListener implements DataChangeListener, Au
             LOG.info(">>>>> updated iiD: {} - first: {} - NodeKey: {}",
                     iID, iID.firstIdentifierOf(Node.class), openflowId);
             Node openFlowNode = NodeUtils.getOpenFlowNode(openflowId);
-            if (nodeCache.contains(openFlowNode)) {
-                notifyNodeUpdated(openFlowNode);
+            if (addNodeToCache(openFlowNode)) {
+                notifyNodeCreated(openFlowNode);
             } else {
+                notifyNodeUpdated(openFlowNode);
+            }
+        }
+    }
+
+    public void notifyFlowCapableNodeEvent (String openFlowId, Action action) {
+        LOG.debug("Notification of flow capable node {}, action {}", openFlowId, action);
+        checkMemberInitialization();
+
+        Node openFlowNode = NodeUtils.getOpenFlowNode(openFlowId);
+        if (action == Action.DELETE) {
+            notifyNodeRemoved(openFlowNode);
+        } else {
+            if (addNodeToCache(openFlowNode)) {
                 notifyNodeCreated(openFlowNode);
+            } else {
+                notifyNodeUpdated(openFlowNode);
+            }
+        }
+    }
+
+    /**
+     * This method returns the true if node was added to the nodeCache. If param node
+     * is already in the cache, this method is expected to return false.
+     *
+     * @param openFlowNode the node to be added to the cache, if needed
+     * @return whether new node entry was added to cache
+     */
+    private Boolean addNodeToCache (Node openFlowNode) {
+        synchronized (nodeCacheLock) {
+            if (nodeCache.contains(openFlowNode)) {
+                return false;
             }
+            return nodeCache.add(openFlowNode);
+        }
+    }
+
+    /**
+     * This method returns the true if node was removed from the nodeCache. If param node
+     * is not in the cache, this method is expected to return false.
+     *
+     * @param openFlowNode the node to be removed from the cache, if needed
+     * @return whether new node entry was removed from cache
+     */
+    private Boolean removeNodeFromCache (Node openFlowNode) {
+        synchronized (nodeCacheLock) {
+            return nodeCache.remove(openFlowNode);
         }
     }
 
     private void notifyNodeUpdated (Node openFlowNode) {
         final String openflowId = openFlowNode.getId().getValue();
-        LOG.info("notifyNodeUpdated: Node {} from Controller's inventory Service", openflowId);
+        LOG.debug("notifyNodeUpdated: Node {} from Controller's inventory Service", openflowId);
 
         // TODO: will do something amazing here, someday
     }
@@ -110,8 +160,6 @@ public class FlowCapableNodeDataChangeListener implements DataChangeListener, Au
         final String openflowId = openFlowNode.getId().getValue();
         LOG.info("notifyNodeCreated: Node {} from Controller's inventory Service", openflowId);
 
-        nodeCache.add(openFlowNode);
-
         if (pipelineOrchestrator != null) {
             pipelineOrchestrator.enqueue(openflowId);
         }
@@ -124,14 +172,12 @@ public class FlowCapableNodeDataChangeListener implements DataChangeListener, Au
         LOG.info("notifyNodeRemoved: Node {} from Controller's inventory Service",
                 openFlowNode.getId().getValue());
 
-        nodeCache.remove(openFlowNode);
-
         if (nodeCacheManager != null) {
             nodeCacheManager.nodeRemoved(openFlowNode.getId().getValue());
         }
     }
 
-    private void checkMemberInitialization() {
+    private void checkMemberInitialization () {
         /**
          * Obtain local ref to members, if needed. Having these local saves us from calling getGlobalInstance
          * upon every event.

diff --git a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/MdsalConsumer.java b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/MdsalConsumer.java
index 8c57d8dcf1fb300bc3d9cc0f745b32688512201c..8626b0762d10c6e0293657b85ab6a106b3fe1176 100644 (file)
--- a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/MdsalConsumer.java
+++ b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/MdsalConsumer.java
@@ -13,9 +13,11 @@ package org.opendaylight.ovsdb.openstack.netvirt.providers.openflow13;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.sal.binding.api.BindingAwareBroker.ConsumerContext;
 import org.opendaylight.controller.sal.binding.api.NotificationProviderService;
+import org.opendaylight.ovsdb.openstack.netvirt.api.Action;
 
 public interface MdsalConsumer {
     public ConsumerContext getConsumerContext();
     public DataBroker getDataBroker();
     public NotificationProviderService getNotificationService();
+    public void notifyFlowCapableNodeCreateEvent(String openFlowId, Action action);
 }

diff --git a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/MdsalConsumerImpl.java b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/MdsalConsumerImpl.java
index 1f16f1569718ef77a66586c160fec0f084c8ef8b..2d3a00ab38a696bbfa0de4a79435bd0c16bc9323 100644 (file)
--- a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/MdsalConsumerImpl.java
+++ b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/MdsalConsumerImpl.java
@@ -18,6 +18,7 @@ import org.opendaylight.controller.sal.binding.api.BindingAwareBroker.ProviderCo
 import org.opendaylight.controller.sal.binding.api.BindingAwareConsumer;
 import org.opendaylight.controller.sal.binding.api.BindingAwareProvider;
 import org.opendaylight.controller.sal.binding.api.NotificationProviderService;
+import org.opendaylight.ovsdb.openstack.netvirt.api.Action;
 import org.osgi.framework.BundleContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -79,4 +80,11 @@ public class MdsalConsumerImpl implements BindingAwareConsumer, MdsalConsumer, B
     public void onSessionInitiated(ProviderContext session) {
         notificationService = session.getSALService(NotificationProviderService.class);
     }
+
+    @Override
+    public void notifyFlowCapableNodeCreateEvent(String openFlowId, Action action) {
+        if (flowCapableNodeChangeListener != null) {
+            flowCapableNodeChangeListener.notifyFlowCapableNodeEvent(openFlowId, action);
+        }
+    }
 }

diff --git a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/OF13Provider.java b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/OF13Provider.java
index 59a1b77a2bbf571a9bb87b9fc60d051d0b2b1252..525348b4d57f21a451d0825cd67c2a62ed985047 100644 (file)
--- a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/OF13Provider.java
+++ b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/OF13Provider.java
@@ -107,7 +107,6 @@ public class OF13Provider implements NetworkingProvider {
     private static final short TABLE_0_DEFAULT_INGRESS = 0;
     private static final short TABLE_1_ISOLATE_TENANT = 10;
     private static final short TABLE_2_LOCAL_FORWARD = 20;
-    private static final String OPENFLOW = "openflow:";
     private static Long groupId = 1L;
 
     private volatile ConfigurationService configurationService;
@@ -835,7 +834,8 @@ public class OF13Provider implements NetworkingProvider {
             String bridgeName = configurationService.getExternalBridgeName();
             String brUuid = this.getInternalBridgeUUID(node, bridgeName);
             if (brUuid == null) {
-                logger.error("Unable to spot Bridge Identifier for {} in {}", bridgeName, node);
+                // Note: it is okay for certain nodes to not have br-ex configured; not an error
+                logger.info("Unable to spot Bridge Identifier for {} in {}", bridgeName, node);
                 return 0L;
             }
 
@@ -1387,7 +1387,7 @@ public class OF13Provider implements NetworkingProvider {
 
     private void writeNormalRule(Long dpidLong) {
 
-        String nodeName = OPENFLOW + dpidLong;
+        String nodeName = Constants.OPENFLOW_NODE_PREFIX + dpidLong;
 
         MatchBuilder matchBuilder = new MatchBuilder();
         NodeBuilder nodeBuilder = createNodeBuilder(nodeName);
@@ -1844,7 +1844,7 @@ public class OF13Provider implements NetworkingProvider {
             InstructionBuilder ib,
             Long dpidLong, Long port ,
             List<Instruction> instructions) {
-        NodeConnectorId ncid = new NodeConnectorId(OPENFLOW + dpidLong + ":" + port);
+        NodeConnectorId ncid = new NodeConnectorId(Constants.OPENFLOW_NODE_PREFIX + dpidLong + ":" + port);
         logger.debug("createOutputGroupInstructions() Node Connector ID is - Type=openflow: DPID={} port={} existingInstructions={}", dpidLong, port, instructions);
 
         List<Action> actionList = Lists.newArrayList();
@@ -2000,7 +2000,7 @@ public class OF13Provider implements NetworkingProvider {
     protected boolean removeOutputPortFromGroup(NodeBuilder nodeBuilder, InstructionBuilder ib,
             Long dpidLong, Long port , List<Instruction> instructions) {
 
-        NodeConnectorId ncid = new NodeConnectorId(OPENFLOW + dpidLong + ":" + port);
+        NodeConnectorId ncid = new NodeConnectorId(Constants.OPENFLOW_NODE_PREFIX + dpidLong + ":" + port);
         logger.debug("removeOutputPortFromGroup() Node Connector ID is - Type=openflow: DPID={} port={} existingInstructions={}", dpidLong, port, instructions);
 
         List<Action> actionList = Lists.newArrayList();
@@ -2132,6 +2132,11 @@ public class OF13Provider implements NetworkingProvider {
         }
     }
 
+    @Override
+    public void notifyFlowCapableNodeEvent(Long dpid, org.opendaylight.ovsdb.openstack.netvirt.api.Action action) {
+        mdsalConsumer.notifyFlowCapableNodeCreateEvent(Constants.OPENFLOW_NODE_PREFIX + dpid, action);
+    }
+
     public static NodeBuilder createNodeBuilder(String nodeId) {
         NodeBuilder builder = new NodeBuilder();
         builder.setId(new NodeId(nodeId));

diff --git a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/SouthboundHandler.java b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/SouthboundHandler.java
index f2aa19bfe4b66ef1402522e2a795c94506ef23d4..5b6d3be8b444af4c304430fb1dd01c2f4ba5f838 100644 (file)
--- a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/SouthboundHandler.java
+++ b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/SouthboundHandler.java
@@ -15,6 +15,7 @@ import org.opendaylight.ovsdb.lib.notation.UUID;
 import org.opendaylight.ovsdb.openstack.netvirt.api.Action;
 import org.opendaylight.ovsdb.openstack.netvirt.api.BridgeConfigurationManager;
 import org.opendaylight.ovsdb.openstack.netvirt.api.ConfigurationService;
+import org.opendaylight.ovsdb.openstack.netvirt.api.NetworkingProvider;
 import org.opendaylight.ovsdb.openstack.netvirt.api.NetworkingProviderManager;
 import org.opendaylight.ovsdb.openstack.netvirt.api.NodeCacheListener;
 import org.opendaylight.ovsdb.openstack.netvirt.api.TenantNetworkManager;
@@ -22,9 +23,11 @@ import org.opendaylight.ovsdb.openstack.netvirt.impl.NeutronL3Adapter;
 import org.opendaylight.ovsdb.plugin.api.OvsdbConfigurationService;
 import org.opendaylight.ovsdb.plugin.api.OvsdbConnectionService;
 import org.opendaylight.ovsdb.plugin.api.OvsdbInventoryListener;
+import org.opendaylight.ovsdb.schema.openvswitch.Bridge;
 import org.opendaylight.ovsdb.schema.openvswitch.Interface;
 import org.opendaylight.ovsdb.schema.openvswitch.OpenVSwitch;
 import org.opendaylight.ovsdb.schema.openvswitch.Port;
+import org.opendaylight.ovsdb.utils.mdsal.node.StringConvertor;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
 
 import org.slf4j.Logger;
@@ -227,6 +230,18 @@ public class SouthboundHandler extends AbstractHandler
             } catch (Exception e) {
                 logger.error("Error fetching Interface Rows for node " + node, e);
             }
+        } else if (tableName.equalsIgnoreCase(ovsdbConfigurationService.getTableName(node, Bridge.class))) {
+            logger.debug("Processing update of {}:{} node: {}, bridge uuid: {}, row: {}", tableName, action, node, uuid, row);
+            Bridge bridge = ovsdbConfigurationService.getTypedRow(node, Bridge.class, row);
+            final Set<String> dpids = bridge.getDatapathIdColumn().getData();
+            if (dpids != null &&
+                    (bridge.getName().equals(configurationService.getIntegrationBridgeName()) ||
+                            bridge.getName().equals(configurationService.getExternalBridgeName()))) {
+                NetworkingProvider networkingProvider = networkingProviderManager.getProvider(node);
+                for (String dpid : dpids) {
+                    networkingProvider.notifyFlowCapableNodeEvent(StringConvertor.dpidStringToLong(dpid), action);
+                }
+            }
         }
     }
 

diff --git a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/api/NetworkingProvider.java b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/api/NetworkingProvider.java
index 403597fbe826e1ddb02fcbc4645fcf0589594c49..c188c4b6265b6e999e15bda0bc5292afc7e2b2c4 100644 (file)
--- a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/api/NetworkingProvider.java
+++ b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/api/NetworkingProvider.java
@@ -63,4 +63,10 @@ public interface NetworkingProvider {
      */
     public void initializeOFFlowRules(Node openflowNode);
 
+    /**
+     * Generate event to announce flow capable node.
+     * @param dpid the data path id of the node
+     * @param action the type of update for the given dpid
+     */
+    public void notifyFlowCapableNodeEvent(Long dpid, Action action);
 }