Set trunk parent port as access port in ovs to avoid loop

A non-vlan-transparent trunk parent port (tpt) should only forward
untagged frames. Earlier it was configured to forward anything (trunk
mode in ovs). This patch changes the trunk mode to access mode and
sets the trunk parent's tag explicitly to 0.

Change-Id: I4bcfe53fe87d7c9218dd0db9d7224bb323709a21
Closes-Bug: #2048785
(cherry picked from commit 27601f8)
(cherry picked from commit 2de488a)

Author: rubasov

doc/source/contributor/internals/openvswitch_agent.rst

@@ -494,7 +494,7 @@ The IDs used for bridge and port names are truncated.
          |          tbr-trunk-id          |
          |                                |
          | tpt-parent-id   spt-subport-id |
-         |                   (tag 100)    |
+         |  (tag 0)          (tag 100)    |
          +-----+-----------------+--------+
                |                 |
                |                 |
@@ -514,27 +514,27 @@ spi-subport-id: int bridge side of the patch port that implements a subport.
 Trunk creation
 ++++++++++++++
 
-A VM is spawned passing to Nova the port-id of a parent port associated with
-a trunk. Neutron will pass to Nova the bridge where to plug the vif as part of the vif details.
-The os-vif driver creates the trunk bridge tbr-trunk-id if it does not exist in plug().
-It will create the tap interface tap1 and plug it into tbr-trunk-id setting the parent port ID in the external-ids.
-The OVS agent will be monitoring the creation of ports on the trunk bridges. When it detects
-that a new port has been created on the trunk bridge, it will do the following:
+A VM is spawned passing to Nova the port-id of a parent port associated
+with a trunk. Neutron will pass to Nova the bridge where to plug the
+vif as part of the vif details.  The os-vif driver creates the trunk
+bridge tbr-trunk-id if it does not exist in plug().  It will create the
+tap interface tap1 and plug it into tbr-trunk-id setting the parent port
+ID in the external-ids.  The trunk driver will wire the parent port via
+a patch port to connect the trunk bridge to the integration bridge:
 
 ::
 
- ovs-vsctl add-port tbr-trunk-id tpt-parent-id -- set Interface tpt-parent-id type=patch options:peer=tpi-parent-id
- ovs-vsctl add-port br-int tpi-parent-id tag=3 -- set Interface tpi-parent-id type=patch options:peer=tpt-parent-id
+ ovs-vsctl add-port tbr-trunk-id tpt-parent-id -- set Interface tpt-parent-id type=patch options:peer=tpi-parent-id -- set Port tpt-parent-id vlan_mode=access tag=0
+ ovs-vsctl add-port br-int tpi-parent-id -- set Interface tpi-parent-id type=patch options:peer=tpt-parent-id
 
 
-A patch port is created to connect the trunk bridge to the integration bridge.
-tpt-parent-id, the trunk bridge side of the patch is not associated to any
-tag. It will carry untagged traffic.
-tpi-parent-id, the br-int side the patch port is tagged with VLAN 3. We assume that the
-trunk is on network1 that on this host is associated with VLAN 3.
-The OVS agent will set the trunk ID in the external-ids of tpt-parent-id and tpi-parent-id.
-If the parent port is associated with one or more subports the agent will process them as
-described in the next paragraph.
+tpt-parent-id, the trunk bridge side of the patch will carry untagged
+traffic (vlan_mode=access tag=0).  The OVS agent will be monitoring the
+creation of ports on the integration bridge.  tpi-parent-id, the br-int
+side the patch port is tagged with VLAN 3 by ovs-agent.  We assume that
+the trunk is on network1 that on this host is associated with VLAN 3.
+If the parent port is associated with one or more subports the agent
+will process them as described in the next paragraph.
 
 Subport creation
 ++++++++++++++++

neutron/services/trunk/drivers/openvswitch/agent/trunk_manager.py

@@ -91,7 +91,7 @@ def __init__(self, trunk_id, port_id, port_mac=None):
             self.DEV_PREFIX, port_id)
         self._transaction = None
 
-    def plug(self, br_int):
+    def plug(self, br_int, tag=0):
         """Plug patch ports between trunk bridge and given bridge.
 
         The method plugs one patch port on the given bridge side using
@@ -124,6 +124,9 @@ def plug(self, br_int):
                                    self.patch_port_trunk_name))
             txn.add(ovsdb.db_set('Interface', self.patch_port_trunk_name,
                                  *patch_trunk_attrs))
+            txn.add(ovsdb.db_set('Port', self.patch_port_trunk_name,
+                                 ('vlan_mode', 'access'),
+                                 ('tag', tag)))
 
     def unplug(self, bridge):
         """Unplug the trunk from bridge.
@@ -168,12 +171,7 @@ def plug(self, br_int):
         :param br_int: an integration bridge where peer endpoint of patch port
                        will be created.
         """
-        ovsdb = self.bridge.ovsdb
-        with ovsdb.transaction() as txn:
-            super(SubPort, self).plug(br_int)
-            txn.add(ovsdb.db_set(
-                "Port", self.patch_port_trunk_name,
-                ("tag", self.segmentation_id)))
+        super(SubPort, self).plug(br_int, tag=self.segmentation_id)
 
     def unplug(self, bridge):
         """Unplug the sub port from the bridge.

neutron/tests/functional/services/trunk/drivers/openvswitch/agent/test_trunk_manager.py

@@ -55,6 +55,14 @@ def test_plug(self):
                       self.trunk.bridge.get_port_name_list())
         self.assertIn(self.trunk.patch_port_int_name,
                       self.br_int.get_port_name_list())
+        self.assertEqual(
+            'access',
+            self.trunk.bridge.db_get_val(
+                'Port', self.trunk.patch_port_trunk_name, 'vlan_mode'))
+        self.assertEqual(
+            0,
+            self.trunk.bridge.db_get_val(
+                'Port', self.trunk.patch_port_trunk_name, 'tag'))
 
     def test_plug_failure_doesnt_create_ports(self):
         with mock.patch.object(

releasenotes/notes/bug-2048785-trunk-parent-vlan-mode-9280ff2d45403bde.yaml

@@ -0,0 +1,8 @@
+---
+issues:
+  - |
+    The fix of `bug 2048785 <https://bugs.launchpad.net/neutron/+bug/2048785>`_
+    only fixes newly created trunk parent ports. If the fix of already existing
+    trunks is needed, then either delete and re-create the affected trunks
+    or set tpt ports' vlan_mode and tag manually:
+    ``ovs-vsctl set Port tpt-... vlan_mode=access tag=0``