From 0673a9bb232ad2e5db45a5ed4b4ad81275e475d6 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Sun, 25 Feb 2024 09:47:59 +0000 Subject: [PATCH 1/2] [stable-only][OVN] Set VETH interface MAC address before up Set the VETH interface MAC address of the metadata namespace before setting this interface as up. This change was introduced in [1] and should be backported to stable versions. [1]https://review.opendev.org/c/openstack/neutron/+/894026/12/neutron/agent/ovn/metadata/agent.py#710 Closes-Bug: #2055561 Change-Id: Ied8f7a5b6c0e9c295e8e040546c53bd98206ef12 (cherry picked from commit ac63999b6621ca409eff53a9f113efd3fd28c966) --- neutron/agent/ovn/metadata/agent.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index 861715d8e14..891090df048 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -649,13 +649,13 @@ def provision_datapath(self, datapath): ip1, ip2 = ip_lib.IPWrapper().add_veth( veth_name[0], veth_name[1], namespace) + # Configure the MAC address. + ip2.link.set_address(metadata_port_info.mac) + # Make sure both ends of the VETH are up ip1.link.set_up() ip2.link.set_up() - # Configure the MAC address. - ip2.link.set_address(metadata_port_info.mac) - cidrs_to_add, cidrs_to_delete = self._process_cidrs( {dev['cidr'] for dev in ip2.addr.list()}, datapath_ports_ips, From 5ffec5d17a0aa03afb6353c39865b42290b678c1 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 21 Feb 2024 15:34:13 +0000 Subject: [PATCH 2/2] [OVN] Set MTU of the VETH interfaces between OVS and metadata The VETH pair between the metadata namespace and the local OVS now has the same MTU as the network associated to this metadata service. The "LSP.external_ids" field has a new key defined: "neutron:mtu". This is the value of the network MTU. This patch does not update the previous metadata datapaths nor the existing LSP. This change will affect only to new created ports and the corresponding metadata datapaths. Conflicts: neutron/agent/ovn/metadata/agent.py Closes-Bug: #2053274 Change-Id: I7ff300e9634e5e3fc68d70540392109fd8b9babc (cherry picked from commit 47b4d14955451c320ce612e4e2e7d0a896e2fe36) (cherry picked from commit 101898fde869a5f530580d1c69c52cdc4a33cd98) --- neutron/agent/ovn/metadata/agent.py | 37 +++++----- .../ovn/mech_driver/ovsdb/ovn_client.py | 9 ++- .../unit/agent/ovn/metadata/test_agent.py | 67 ++++++++----------- 3 files changed, 56 insertions(+), 57 deletions(-) diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index 891090df048..1d2f3146f44 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -100,7 +100,7 @@ def run(self, event, row, old): with _SYNC_STATE_LOCK.read_lock(): self.log_row(row) try: - self.agent.provision_datapath(row.datapath) + self.agent.provision_datapath(row) except ConfigException: # We're now in the reader lock mode, we need to exit the # context and then use writer lock @@ -400,12 +400,12 @@ def _get_ovn_bridge(self): "br-int instead.") return 'br-int' - def get_networks_datapaths(self): - """Return a set of datapath objects of the VIF ports on the current + def get_networks_port_bindings(self): + """Return a set of Port_Binding objects of the VIF ports on the current chassis. """ ports = self.sb_idl.get_ports_on_chassis(self.chassis) - return set(p.datapath for p in self._vif_ports(ports)) + return list(self._vif_ports(ports)) @_sync_lock def sync(self, provision=True): @@ -420,12 +420,12 @@ def sync(self, provision=True): system_namespaces = tuple( ns.decode('utf-8') if isinstance(ns, bytes) else ns for ns in ip_lib.list_network_namespaces()) - net_datapaths = self.get_networks_datapaths() - metadata_namespaces = [ + net_port_bindings = self.get_networks_port_bindings() + metadata_namespaces = set( self._get_namespace_name( ovn_utils.get_network_name_from_datapath(datapath)) - for datapath in net_datapaths - ] + for datapath in (pb.datapath for pb in net_port_bindings) + ) unused_namespaces = [ns for ns in system_namespaces if ns.startswith(NS_PREFIX) and ns not in metadata_namespaces] @@ -439,8 +439,8 @@ def sync(self, provision=True): # even those that are already running. This is to make sure # everything within each namespace is up to date. if provision: - for datapath in net_datapaths: - self.provision_datapath(datapath) + for port_binding in net_port_bindings: + self.provision_datapath(port_binding) @staticmethod def _get_veth_name(datapath): @@ -611,7 +611,7 @@ def _get_provision_params(self, datapath): return net_name, datapath_ports_ips, metadata_port_info - def provision_datapath(self, datapath): + def provision_datapath(self, port_binding): """Provision the datapath so that it can serve metadata. This function will create the namespace and VETH pair if needed @@ -619,11 +619,13 @@ def provision_datapath(self, datapath): metadata port of the network. It will also remove existing IP from the namespace if they are no longer needed. - :param datapath: datapath object. - :return: The metadata namespace name for the datapath or None - if namespace was not provisioned + :param port_binding: Port_Binding object. + :return: The metadata namespace name for the Port_Binding.datapath or + None if namespace was not provisioned """ - + datapath = port_binding.datapath + mtu = int(port_binding.external_ids.get( + ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY) or '0') provision_params = self._get_provision_params(datapath) if not provision_params: return @@ -652,6 +654,11 @@ def provision_datapath(self, datapath): # Configure the MAC address. ip2.link.set_address(metadata_port_info.mac) + # Set VETH ports MTU. + if mtu: + ip1.link.set_mtu(mtu) + ip2.link.set_mtu(mtu) + # Make sure both ends of the VETH are up ip1.link.set_up() ip2.link.set_up() diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index df61890ac9f..b15525217ab 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -79,6 +79,7 @@ "address6_scope_id", "vnic_type", "capabilities", + "mtu", ], ) @@ -324,6 +325,7 @@ def _get_port_options(self, port): address6_scope_id = "" dhcpv4_options = self._get_port_dhcp_options(port, const.IP_VERSION_4) dhcpv6_options = self._get_port_dhcp_options(port, const.IP_VERSION_6) + mtu = '' if vtep_physical_switch: vtep_logical_switch = bp_info.bp_param.get('vtep-logical-switch') port_type = 'vtep' @@ -421,10 +423,10 @@ def _get_port_options(self, port): ovn_const.VIF_DETAILS_PF_MAC_ADDRESS in bp_info.bp_param): port_net = self._plugin.get_network( context, port['network_id']) + mtu = str(port_net['mtu']) options.update({ ovn_const.LSP_OPTIONS_VIF_PLUG_TYPE_KEY: 'representor', - ovn_const.LSP_OPTIONS_VIF_PLUG_MTU_REQUEST_KEY: str( - port_net['mtu']), + ovn_const.LSP_OPTIONS_VIF_PLUG_MTU_REQUEST_KEY: mtu, ovn_const.LSP_OPTIONS_VIF_PLUG_REPRESENTOR_PF_MAC_KEY: ( bp_info.bp_param.get( ovn_const.VIF_DETAILS_PF_MAC_ADDRESS)), @@ -465,7 +467,7 @@ def _get_port_options(self, port): parent_name, tag, dhcpv4_options, dhcpv6_options, cidrs.strip(), device_owner, sg_ids, address4_scope_id, address6_scope_id, - bp_info.vnic_type, bp_info.capabilities + bp_info.vnic_type, bp_info.capabilities, mtu ) def update_port_dhcp_options(self, port_info, txn): @@ -506,6 +508,7 @@ def get_external_ids_from_port(self, port): ovn_const.OVN_PORT_VNIC_TYPE_KEY: port_info.vnic_type, ovn_const.OVN_PORT_BP_CAPABILITIES_KEY: ';'.join(port_info.capabilities), + ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY: port_info.mtu, } return port_info, external_ids diff --git a/neutron/tests/unit/agent/ovn/metadata/test_agent.py b/neutron/tests/unit/agent/ovn/metadata/test_agent.py index 9bf9f0db52a..7a529d9d82b 100644 --- a/neutron/tests/unit/agent/ovn/metadata/test_agent.py +++ b/neutron/tests/unit/agent/ovn/metadata/test_agent.py @@ -28,6 +28,7 @@ from neutron.agent.linux.ip_lib import IPWrapper as ip_wrap from neutron.agent.ovn.metadata import agent from neutron.agent.ovn.metadata import driver +from neutron.common.ovn import constants as ovn_const from neutron.conf.agent.metadata import config as meta_conf from neutron.conf.agent.ovn.metadata import config as ovn_meta_conf from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf @@ -100,14 +101,8 @@ def test_sync(self): self.agent.sync() - pdp.assert_has_calls( - [ - mock.call(p.datapath) - for p in self.ports - ], - any_order=True - ) - + pdp.assert_has_calls([mock.call(p) for p in self.ports], + any_order=True) lnn.assert_called_once_with() tdp.assert_not_called() @@ -124,13 +119,8 @@ def test_sync_teardown_namespace(self): self.agent.sync() - pdp.assert_has_calls( - [ - mock.call(p.datapath) - for p in self.ports - ], - any_order=True - ) + pdp.assert_has_calls([mock.call(p) for p in self.ports], + any_order=True) lnn.assert_called_once_with() tdp.assert_called_once_with('3') @@ -149,27 +139,23 @@ def test_sync_teardown_namespace_does_not_crash_on_error(self): side_effect=Exception()) as tdp: self.agent.sync() - pdp.assert_has_calls( - [ - mock.call(p.datapath) - for p in self.ports - ], - any_order=True - ) + pdp.assert_has_calls([mock.call(p) for p in self.ports], + any_order=True) lnn.assert_called_once_with() tdp.assert_called_once_with('3') - def test_get_networks_datapaths(self): - """Test get_networks_datapaths returns only datapath objects for the - networks containing vif ports of type ''(blank) and 'external'. + def test_get_networks_port_bindings(self): + """Test get_networks_port_bindings returns only the port binding + objects for ports with VIF type empty ('') or 'external'. This test simulates that this chassis has the following ports: - * datapath '1': 1 port type '' , 1 port 'external' and - 1 port 'unknown' - * datapath '2': 1 port type '' - * datapath '3': 1 port with type 'external' - * datapath '4': 1 port with type 'unknown' - - It is expected that only datapaths '1', '2' and '3' are returned + * port0: datapath 1, type '' + * port1: datapath 1, type 'external' + * port2: datapath 1, type 'unknown' + * port3: datapath 2, type '' + * port4: datapath 3, type 'external' + * port5: datapath 4, type 'unknown' + + Only port bindings from ports 0, 1, 3, and 4 are expected. """ datapath_1 = DatapathInfo(uuid='uuid1', @@ -192,11 +178,8 @@ def test_get_networks_datapaths(self): with mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis', return_value=ports): - expected_datapaths = set([datapath_1, datapath_2, datapath_3]) - self.assertSetEqual( - expected_datapaths, - self.agent.get_networks_datapaths() - ) + self.assertEqual([ports[0], ports[1], ports[3], ports[4]], + self.agent.get_networks_port_bindings()) def test_teardown_datapath(self): """Test teardown datapath. @@ -424,7 +407,8 @@ def test_provision_datapath(self): mock.patch.object(agent.MetadataAgent, '_get_namespace_name', return_value=nemaspace_name),\ mock.patch.object(ip_link, 'set_up') as link_set_up,\ - mock.patch.object(ip_link, 'set_address') as link_set_addr,\ + mock.patch.object(ip_link, 'set_address') as link_set_addr, \ + mock.patch.object(ip_link, 'set_mtu') as link_set_mtu, \ mock.patch.object(ip_addr, 'list', return_value=[]),\ mock.patch.object( ip_addr, 'add_multiple') as ip_addr_add_multiple,\ @@ -442,7 +426,11 @@ def test_provision_datapath(self): # We need to assert that it was deleted first. self.agent.ovs_idl.list_br.return_value.execute.return_value = ( ['br-int', 'br-fake']) - self.agent.provision_datapath('fake_datapath') + mtu = 1500 + port_binding = mock.Mock( + datapath='fake_datapath', + external_ids={ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY: str(mtu)}) + self.agent.provision_datapath(port_binding) # Check that the port was deleted from br-fake self.agent.ovs_idl.del_port.assert_called_once_with( @@ -452,6 +440,7 @@ def test_provision_datapath(self): nemaspace_name) # Make sure that the two ends of the VETH pair have been set as up. self.assertEqual(2, link_set_up.call_count) + link_set_mtu.assert_has_calls([mock.call(mtu), mock.call(mtu)]) link_set_addr.assert_called_once_with('aa:bb:cc:dd:ee:ff') # Make sure that the port has been added to OVS. self.agent.ovs_idl.add_port.assert_called_once_with(