From d253552ad26f4920ea9f1f53e5435f291f391790 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 10f14c7c9b4..4bdb71e5086 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -586,13 +586,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 cf21a4d9791281605ab3cebbbf3de3cb8631b5bc 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 neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py Closes-Bug: #2053274 Change-Id: I7ff300e9634e5e3fc68d70540392109fd8b9babc (cherry picked from commit 47b4d14955451c320ce612e4e2e7d0a896e2fe36) (cherry picked from commit 101898fde869a5f530580d1c69c52cdc4a33cd98) (cherry picked from commit 5ffec5d17a0aa03afb6353c39865b42290b678c1) --- neutron/agent/ovn/metadata/agent.py | 37 +++++++----- .../ovn/mech_driver/ovsdb/ovn_client.py | 11 ++-- .../unit/agent/ovn/metadata/test_agent.py | 58 +++++++++---------- 3 files changed, 55 insertions(+), 51 deletions(-) diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index 4bdb71e5086..dda2cd4aeec 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -93,7 +93,7 @@ def run(self, event, row, old): net_name = ovn_utils.get_network_name_from_datapath( row.datapath) LOG.info(self.LOG_MSG, row.logical_port, net_name) - 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 @@ -341,12 +341,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): @@ -361,12 +361,12 @@ def sync(self): 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] @@ -376,8 +376,8 @@ def sync(self): # resync all network namespaces based on the associated datapaths, # even those that are already running. This is to make sure # everything within each namespace is up to date. - 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): @@ -548,7 +548,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 @@ -556,11 +556,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 @@ -589,6 +591,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 7354673f2d4..3e7bc5c01f3 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 @@ -78,6 +78,7 @@ "security_group_ids", "address4_scope_id", "address6_scope_id", + "mtu", ], ) @@ -366,6 +367,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 = binding_prof.get('vtep-logical-switch') port_type = 'vtep' @@ -482,10 +484,10 @@ def _get_port_options(self, port): ovn_const.VIF_DETAILS_PF_MAC_ADDRESS in binding_prof): 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: ( binding_prof.get( ovn_const.VIF_DETAILS_PF_MAC_ADDRESS)), @@ -525,7 +527,7 @@ def _get_port_options(self, port): return OvnPortInfo(port_type, options, addresses, port_security, parent_name, tag, dhcpv4_options, dhcpv6_options, cidrs.strip(), device_owner, sg_ids, - address4_scope_id, address6_scope_id + address4_scope_id, address6_scope_id, mtu ) def sync_ha_chassis_group(self, context, network_id, txn): @@ -633,7 +635,8 @@ def get_external_ids_from_port(self, port): port_info.security_group_ids, ovn_const.OVN_REV_NUM_EXT_ID_KEY: str( utils.get_revision_number( - port, ovn_const.TYPE_PORTS))} + port, ovn_const.TYPE_PORTS)), + ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY: port_info.mtu} return port_info, external_ids def create_port(self, context, port): diff --git a/neutron/tests/unit/agent/ovn/metadata/test_agent.py b/neutron/tests/unit/agent/ovn/metadata/test_agent.py index 8d6b10b89c1..05a2da79717 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,27 +119,23 @@ 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') - 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', @@ -167,11 +158,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. @@ -399,7 +387,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') as ip_addr_add,\ mock.patch.object( @@ -416,7 +405,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( @@ -426,6 +419,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(