BUG-8893 patch port mysteriously deleted

This happens sporadically when br-int exists prior to ODL
managing the OVS node. If the event notifying ElanOvsdbNodeListener
of the existence of the br-int bridge happens BEFORE the event
notifying about the OVS node the following occurs:

1. Netvirt gets event for br-int
2. Writes patch ports
3. Netvirt gets event for the ovs node
4. Creates br-int...which already exists and this over-writes the
   patch ports

Fix:
a) clean up code so that br-int is configured with a controller
   and patch ports only once we receive the notification for br-int
   in operational.
b) when receiving a notification for the OVSDB node in operational,
   check if br-int is in operational. If it is not, create it in
   config.
c) when receiving a notification for br-int in operational, check if it
   is also in config. If it is not, copy br-int fields from operational
   to config. The proceed to step (a).

Change-Id: I11739ccc847e65c4179ff1f73493159bd4effcd2
Signed-off-by: Josh <jhershbe@redhat.com>

diff --git a/docs/user-guide/bridge-configuration.rst b/docs/user-guide/bridge-configuration.rst
new file mode 100644 (file)
index 0000000..51340fd
--- /dev/null
+++ b/docs/user-guide/bridge-configuration.rst
@@ -0,0 +1,40 @@
+Bridge Configuration
+====================
+
+.. contents:: Table of Contents
+   :depth: 2
+
+The following describes OVS bridge configurations supported by OpenDaylight.
+
+The "br-int" Bridge
+-------------------
+If the br-int bridge is not configured prior to the ovsdb manager connection with ODL,
+ODL will create it. If br-int exists prior to the ovsdb manager connection, ODL will retain
+any existing configurations on br-int. Note that if you choose to create br-int prior to
+connecting to ODL, ``disable-in-band`` MUST be set to true and any flows configured may interfere
+with the flows ODL will create. ODL will add the following configuration items to br-int:
+
+1. ODL will set itself as br-int's controller
+2. Any provider network configuration (see section "Provider Networks" below)
+
+It is important to note that once the ovsdb manager connection is established with ODL, ODL
+"owns" br-int and other applications should not modify its settings.
+
+Provider Networks
+-----------------
+Provider networks should be configured prior to OVSDB connecting to ODL. These are configured
+in the Open_vSwitch table’s other_Config column and have the format ``<physnet>:<connector>``
+where ``<physnet>`` is the name of the provider network and ``<connector>`` is one of the following
+three options:
+
+1. The name of a local interface (ODL will add this port to br-int)
+2. The name of a bridge on OpenVSwitch (ODL will create patch ports between br-int and this bridge)
+3. The name of a port already present on br-int (ODL will use that port)
+
+For example, assume your provider network is called extnet and it is attached to the eth0 interface
+on your host you can set this in OVSDB using the following command:
+
+``sudo ovs-vsctl set Open_vSwitch . Other_Config:provider_mappings=extnet:eth0``
+
+If instead of ``eth0`` the provider network is accesable via on OVS bridge called ``br-int``, ``eth0`` in the
+above command would be substituted with ``br-int``.

diff --git a/docs/user-guide/index.rst b/docs/user-guide/index.rst
index 235a9c1e87d952c76cf2b27eabd6fa7ae0dcc370..24bd7d5c69d625ea800bae628b565e6f92ff9f24 100644 (file)
--- a/docs/user-guide/index.rst
+++ b/docs/user-guide/index.rst
@@ -7,3 +7,4 @@ NetVirt User Guide
    ../specs/index
    l3vpn-service_-user-guide
    support
+   bridge-configuration

diff --git a/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanBridgeManager.java b/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanBridgeManager.java
index 5e03a991a4297f76f63ac1b4167f6b7efd314cee..794347ef2e16d9102427baf9093cb5b93af1dd0e 100644 (file)
--- a/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanBridgeManager.java
+++ b/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanBridgeManager.java
@@ -12,6 +12,7 @@ import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
 
 import java.math.BigInteger;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -21,6 +22,7 @@ import java.util.Random;
 
 import javax.inject.Inject;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.genius.interfacemanager.globals.IfmConstants;
 import org.opendaylight.genius.interfacemanager.interfaces.IInterfaceManager;
 import org.opendaylight.genius.itm.globals.ITMConstants;
@@ -34,6 +36,8 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.re
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.DatapathTypeNetdev;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbBridgeAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbNodeAugmentation;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbTerminationPointAugmentation;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbTerminationPointAugmentationBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.bridge.attributes.BridgeOtherConfigs;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.bridge.attributes.BridgeOtherConfigsBuilder;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NetworkTopology;
@@ -41,7 +45,10 @@ import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyKey;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeBuilder;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeKey;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.node.TerminationPoint;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.node.TerminationPointBuilder;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -140,22 +147,21 @@ public class ElanBridgeManager implements IElanBridgeManager {
      * @param node A node
      * @param generateIntBridgeMac whether or not the int bridge's mac should be set to a random value
      */
+    @SuppressWarnings("checkstyle:IllegalCatch")
     public void processNodePrep(Node node, boolean generateIntBridgeMac) {
         if (isOvsdbNode(node)) {
-            ensureBridgesExist(node, generateIntBridgeMac);
-
-            //if br-int already exists, we can add provider networks
-            Node brIntNode = southboundUtils.readBridgeNode(node, INTEGRATION_BRIDGE);
-            if (brIntNode != null) {
-                if (!addControllerToBridge(node, INTEGRATION_BRIDGE)) {
-                    LOG.error("Failed to set controller to existing integration bridge {}", brIntNode);
+            if (southboundUtils.readBridgeNode(node, INTEGRATION_BRIDGE) == null) {
+                LOG.debug("OVSDB node in operational does not have br-int, create one {}", node.getNodeId().getValue());
+                try {
+                    createIntegrationBridgeConfig(node, generateIntBridgeMac);
+                } catch (RuntimeException e) {
+                    LOG.error("Error creating bridge on " + node, e);
                 }
-
-                prepareIntegrationBridge(node, brIntNode);
             }
             return;
         }
 
+        //Assume "node" is a bridge node, extract the OVSDB node from operational
         Node ovsdbNode = southboundUtils.readOvsdbNode(node);
         if (ovsdbNode == null) {
             LOG.error("Node is neither bridge nor ovsdb {}", node);
@@ -169,6 +175,11 @@ public class ElanBridgeManager implements IElanBridgeManager {
     }
 
     private void prepareIntegrationBridge(Node ovsdbNode, Node brIntNode) {
+        if (southboundUtils.getBridgeFromConfig(ovsdbNode, INTEGRATION_BRIDGE) == null) {
+            LOG.debug("br-int in operational but not config, copying into config");
+            copyBridgeToConfig(brIntNode);
+        }
+
         Map<String, String> providerMappings = getOpenvswitchOtherConfigMap(ovsdbNode, PROVIDER_MAPPINGS_KEY);
 
         for (String value : providerMappings.values()) {
@@ -190,6 +201,37 @@ public class ElanBridgeManager implements IElanBridgeManager {
 
         }
 
+        if (!addControllerToBridge(ovsdbNode, INTEGRATION_BRIDGE)) {
+            LOG.error("Failed to set controller to existing integration bridge {}", brIntNode);
+        }
+
+    }
+
+    private void copyBridgeToConfig(Node brIntNode) {
+        NodeBuilder bridgeNodeBuilder = new NodeBuilder(brIntNode);
+
+        //termination points need to be masssaged to remove the ifindex field
+        //which are not allowed in the config data store
+        List<TerminationPoint> terminationPoints = brIntNode.getTerminationPoint();
+        if (terminationPoints != null) {
+            List<TerminationPoint> newTerminationPoints = new ArrayList<>();
+            for (TerminationPoint tp : terminationPoints) {
+                OvsdbTerminationPointAugmentation ovsdbTerminationPointAugmentation =
+                                                        tp.getAugmentation(OvsdbTerminationPointAugmentation.class);
+                TerminationPointBuilder tpBuilder = new TerminationPointBuilder(tp);
+                if (ovsdbTerminationPointAugmentation != null) {
+                    OvsdbTerminationPointAugmentationBuilder tpAugmentationBuilder =
+                                new OvsdbTerminationPointAugmentationBuilder(ovsdbTerminationPointAugmentation);
+                    tpAugmentationBuilder.setIfindex(null);
+                    tpBuilder.addAugmentation(OvsdbTerminationPointAugmentation.class, tpAugmentationBuilder.build());
+                }
+                newTerminationPoints.add(tpBuilder.build());
+            }
+            bridgeNodeBuilder.setTerminationPoint(newTerminationPoints);
+        }
+
+        InstanceIdentifier<Node> brNodeIid = SouthboundUtils.createInstanceIdentifier(brIntNode.getNodeId());
+        this.mdsalUtils.put(LogicalDatastoreType.CONFIGURATION, brNodeIid, bridgeNodeBuilder.build());
     }
 
     private void patchBridgeToBrInt(Node intBridgeNode, Node exBridgeNode, String physnetBridgeName) {
@@ -207,16 +249,7 @@ public class ElanBridgeManager implements IElanBridgeManager {
         }
     }
 
-    @SuppressWarnings("checkstyle:IllegalCatch")
-    private void ensureBridgesExist(Node ovsdbNode, boolean generateIntBridgeMac) {
-        try {
-            createIntegrationBridge(ovsdbNode, generateIntBridgeMac);
-        } catch (RuntimeException e) {
-            LOG.error("Error creating bridge on " + ovsdbNode, e);
-        }
-    }
-
-    private boolean createIntegrationBridge(Node ovsdbNode, boolean generateIntBridgeMac) {
+    private boolean createIntegrationBridgeConfig(Node ovsdbNode, boolean generateIntBridgeMac) {
         // Make sure iface-type exist in Open_vSwitch table prior to br-int creation
         // in order to allow mixed topology of both DPDK and non-DPDK OVS nodes
         if (!ifaceTypesExist(ovsdbNode)) {
@@ -224,7 +257,7 @@ public class ElanBridgeManager implements IElanBridgeManager {
             return false;
         }
 
-        LOG.debug("ElanBridgeManager.createIntegrationBridge, skipping if exists");
+        LOG.debug("ElanBridgeManager.createIntegrationBridgeConfig, skipping if exists");
         if (!addBridge(ovsdbNode, INTEGRATION_BRIDGE,
                 generateIntBridgeMac ? generateRandomMac() : null)) {
             LOG.warn("Integration Bridge Creation failed");