From 1e5ba78870fbf9de38764e0b39d7602cc32f7a25 Mon Sep 17 00:00:00 2001 From: Bence Romsics Date: Tue, 9 Jan 2024 14:18:45 +0100 Subject: [PATCH] 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 27601f8eead444283e4d1c258298ac5afaff377f) --- .../internals/openvswitch_agent.rst | 34 +++++++++---------- .../openvswitch/agent/trunk_manager.py | 12 +++---- .../openvswitch/agent/test_trunk_manager.py | 8 +++++ ...unk-parent-vlan-mode-9280ff2d45403bde.yaml | 8 +++++ 4 files changed, 38 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/bug-2048785-trunk-parent-vlan-mode-9280ff2d45403bde.yaml diff --git a/doc/source/contributor/internals/openvswitch_agent.rst b/doc/source/contributor/internals/openvswitch_agent.rst index 5fb35684156..e0d47d85885 100644 --- a/doc/source/contributor/internals/openvswitch_agent.rst +++ b/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 ++++++++++++++++ diff --git a/neutron/services/trunk/drivers/openvswitch/agent/trunk_manager.py b/neutron/services/trunk/drivers/openvswitch/agent/trunk_manager.py index ff37fc946f3..bb9559b739f 100644 --- a/neutron/services/trunk/drivers/openvswitch/agent/trunk_manager.py +++ b/neutron/services/trunk/drivers/openvswitch/agent/trunk_manager.py @@ -91,7 +91,7 @@ class TrunkParentPort(object): 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 @@ class TrunkParentPort(object): 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. @@ -167,12 +170,7 @@ class SubPort(TrunkParentPort): :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. diff --git a/neutron/tests/functional/services/trunk/drivers/openvswitch/agent/test_trunk_manager.py b/neutron/tests/functional/services/trunk/drivers/openvswitch/agent/test_trunk_manager.py index b3e14a6c32d..8f7052ec652 100644 --- a/neutron/tests/functional/services/trunk/drivers/openvswitch/agent/test_trunk_manager.py +++ b/neutron/tests/functional/services/trunk/drivers/openvswitch/agent/test_trunk_manager.py @@ -54,6 +54,14 @@ class TrunkParentPortTestCase(base.BaseSudoTestCase): 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( diff --git a/releasenotes/notes/bug-2048785-trunk-parent-vlan-mode-9280ff2d45403bde.yaml b/releasenotes/notes/bug-2048785-trunk-parent-vlan-mode-9280ff2d45403bde.yaml new file mode 100644 index 00000000000..725acc3203f --- /dev/null +++ b/releasenotes/notes/bug-2048785-trunk-parent-vlan-mode-9280ff2d45403bde.yaml @@ -0,0 +1,8 @@ +--- +issues: + - | + The fix of `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``