From c63ebef2d58e15f4388cf064066f77b503a2f841 Mon Sep 17 00:00:00 2001 From: LIU Yulong Date: Mon, 29 Nov 2021 12:27:23 +0800 Subject: [PATCH] Add tag to port more earlier During some ml2 ovs agent port processing performance test, we noticed that some ports are missing tag before it really done processing. While ovs treats those ports without tag as trunk port, so some packets will be flooded to it. In large scale cloud, if too many port added to the bridge, the ovs-vswitchd will consume a huge amount of CPU cores if ports are not bound in a short time. So, in the port_bound function of ovs-agent, we set the port tag to it after a local_vlan id is allocated. Because after that, setup security groups (setup_port_filters) and bind devices in DB (update_device_list) are really time-consuming. And also fix a potential bug, port is processed as created first, but no tag in ovsdb, so openflow security group will not be processed successfully [1]. It must be done in a update event during next loop, after port bound and ovsdb set the required value. This patch can also fix some upstream test failures of waiting too long time to ping some cases. [1] https://github.com/openstack/neutron/blob/master/neutron/agent/linux/openvswitch_firewall/firewall.py#L112 Closes-Bug: #1952567 Change-Id: I3533f0d416d32f8d0888ad58f975960d89a985d9 --- .../openvswitch/agent/ovs_neutron_agent.py | 14 +++++++++++++- .../openvswitch/agent/test_ovs_neutron_agent.py | 16 ++++++++++------ .../drivers/openvswitch/agent/test_ovs_tunnel.py | 11 +++++++---- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 614e41b4a8c..1b52efd187d 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1117,6 +1117,16 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, self.available_local_vlans.add(lvm.vlan) + def _set_port_vlan(self, port, vlan): + ovsdb = self.int_br.ovsdb + with self.int_br.ovsdb.transaction() as txn: + # When adding the port's tag, + # also clear port's vlan_mode and trunks, + # which were set to make sure all packets are dropped. + txn.add(ovsdb.db_set('Port', port.port_name, ('tag', vlan))) + txn.add(ovsdb.db_clear('Port', port.port_name, 'vlan_mode')) + txn.add(ovsdb.db_clear('Port', port.port_name, 'trunks')) + def port_bound(self, port, net_uuid, network_type, physical_network, segmentation_id, fixed_ips, device_owner, @@ -1157,10 +1167,12 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, vlan_mapping = {'net_uuid': net_uuid, 'network_type': network_type, - 'physical_network': str(physical_network)} + 'physical_network': str(physical_network), + 'tag': str(lvm.vlan)} if segmentation_id is not None: vlan_mapping['segmentation_id'] = str(segmentation_id) port_other_config.update(vlan_mapping) + self._set_port_vlan(port, lvm.vlan) self.int_br.set_db_attribute("Port", port.port_name, "other_config", port_other_config) return True diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index e087a57806a..e11f08abda4 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -179,18 +179,21 @@ class TestOvsNeutronAgent(object): self.agent.vlan_manager.add( net_uuid, old_local_vlan, None, None, None) with mock.patch.object(self.agent, 'int_br', autospec=True) as int_br: - int_br.db_get_val.return_value = db_get_val - int_br.set_db_attribute.return_value = True - needs_binding = self.agent.port_bound( - port, net_uuid, 'local', None, None, - fixed_ips, DEVICE_OWNER_COMPUTE, False) + with mock.patch.object(self.agent, '_set_port_vlan') as set_vlan: + int_br.db_get_val.return_value = db_get_val + int_br.set_db_attribute.return_value = True + needs_binding = self.agent.port_bound( + port, net_uuid, 'local', None, None, + fixed_ips, DEVICE_OWNER_COMPUTE, False) if db_get_val is None: int_br.assert_not_called() self.assertFalse(needs_binding) else: vlan_mapping = {'net_uuid': net_uuid, 'network_type': 'local', - 'physical_network': 'None'} + 'physical_network': 'None', + 'tag': str(new_local_vlan)} + set_vlan.assert_called_once_with(port, new_local_vlan) int_br.set_db_attribute.assert_called_once_with( "Port", mock.ANY, "other_config", vlan_mapping) self.assertTrue(needs_binding) @@ -3102,6 +3105,7 @@ class TestOvsDvrNeutronAgent(object): self.agent = self.mod_agent.OVSNeutronAgent(self._bridge_classes(), ext_manager, cfg.CONF) self.agent.tun_br = self.br_tun_cls(br_name='br-tun') + self.agent._set_port_vlan = mock.Mock() self.agent.sg_agent = mock.Mock() def _setup_for_dvr_test(self): diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py index 0e3b0c0d89b..0bce429767b 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py @@ -503,7 +503,8 @@ class TunnelTest(object): vlan_mapping = {'segmentation_id': str(LS_ID), 'physical_network': 'None', 'net_uuid': NET_UUID, - 'network_type': 'gre'} + 'network_type': 'gre', + 'tag': str(LV_ID)} self.mock_int_bridge_expected += [ mock.call.db_get_val('Port', 'port', 'other_config'), mock.call.set_db_attribute('Port', VIF_PORT.port_name, @@ -514,9 +515,11 @@ class TunnelTest(object): a.vlan_manager.add(NET_UUID, *self.LVM_DATA) a.local_dvr_map = {} self.ovs_bridges[self.INT_BRIDGE].db_get_val.return_value = {} - a.port_bound(VIF_PORT, NET_UUID, 'gre', None, LS_ID, - FIXED_IPS, VM_DEVICE_OWNER, False) - self._verify_mock_calls() + with mock.patch.object(a, "_set_port_vlan") as set_vlan: + a.port_bound(VIF_PORT, NET_UUID, 'gre', None, LS_ID, + FIXED_IPS, VM_DEVICE_OWNER, False) + self._verify_mock_calls() + set_vlan.assert_called_once_with(VIF_PORT, LV_ID) def test_port_unbound(self): with mock.patch.object(self.mod_agent.OVSNeutronAgent,