From dbf0e7198f8b828ef3ceb848c936c473688b9155 Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Mon, 26 Jun 2023 09:50:02 +0200 Subject: [PATCH] Avoid vlan device leaking If provider network are recreated the vlan devices created for them are leaked. With this patch we ensure those devices are deleted as part of the resync action Closes-Bug: #2020653 Change-Id: I57775fdeaa3ea6dfa35c0ca44b226fa8b44e0cd3 --- .../drivers/openstack/nb_ovn_bgp_driver.py | 3 ++- .../drivers/openstack/ovn_bgp_driver.py | 3 +++ ovn_bgp_agent/drivers/openstack/utils/ovn.py | 18 ++++++++++++++++ ovn_bgp_agent/drivers/openstack/utils/wire.py | 21 +++++++++++++++---- ovn_bgp_agent/privileged/linux_net.py | 18 ++++++++++++++++ .../functional/privileged/test_linux_net.py | 10 +++++++++ .../openstack/test_nb_ovn_bgp_driver.py | 11 +++++++++- .../drivers/openstack/test_ovn_bgp_driver.py | 5 ++++- .../unit/drivers/openstack/utils/test_ovn.py | 21 +++++++++++++++++++ ovn_bgp_agent/utils/linux_net.py | 4 ++++ 10 files changed, 107 insertions(+), 7 deletions(-) diff --git a/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py index 6e4cfc89..46a95e50 100644 --- a/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py @@ -154,7 +154,8 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): self._ensure_port_exposed(port) # remove extra wiring leftovers - wire_utils.cleanup_wiring(self.ovn_bridge_mappings, + wire_utils.cleanup_wiring(self.nb_idl, + self.ovn_bridge_mappings, self.ovs_flows, self._exposed_ips, self.ovn_routing_tables, diff --git a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py index aa771d45..d7c13d42 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py @@ -258,6 +258,9 @@ class OVNBGPDriver(driver_api.AgentDriverBase): self.ovn_routing_tables_routes, extra_routes) + wire_utils.delete_vlan_devices_leftovers(self.sb_idl, + self.ovn_bridge_mappings) + def _ensure_cr_lrp_associated_ports_exposed(self, cr_lrp_port, exposed_ips, ovn_ip_rules): ips, patch_port_row = self.sb_idl.get_cr_lrp_nat_addresses_info( diff --git a/ovn_bgp_agent/drivers/openstack/utils/ovn.py b/ovn_bgp_agent/drivers/openstack/utils/ovn.py index 528cc4bf..04dd5681 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/ovn.py +++ b/ovn_bgp_agent/drivers/openstack/utils/ovn.py @@ -181,6 +181,15 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): super(OvsdbNbOvnIdl, self).__init__(connection) self.idl._session.reconnect.set_probe_interval(60000) + def get_network_vlan_tags(self): + tags = [] + cmd = self.db_find_rows('Logical_Switch_Port', ('type', '=', + constants.OVN_LOCALNET_VIF_PORT_TYPE)) + for row in cmd.execute(check_error=True): + if row.tag: + tags.append(row.tag[0]) + return tags + def get_network_vlan_tag_by_network_name(self, network_name): tags = [] cmd = self.db_find_rows('Logical_Switch_Port', ('type', '=', @@ -340,6 +349,15 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): return row.options.get('network_name'), row.tag return None, None + def get_network_vlan_tags(self): + tags = [] + cmd = self.db_find_rows('Port_Binding', ('type', '=', + constants.OVN_LOCALNET_VIF_PORT_TYPE)) + for row in cmd.execute(check_error=True): + if row.tag: + tags.append(row.tag[0]) + return tags + def get_network_vlan_tag_by_network_name(self, network_name): tags = [] cmd = self.db_find_rows('Port_Binding', ('type', '=', diff --git a/ovn_bgp_agent/drivers/openstack/utils/wire.py b/ovn_bgp_agent/drivers/openstack/utils/wire.py index 8ddbb424..5675c50e 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/wire.py +++ b/ovn_bgp_agent/drivers/openstack/utils/wire.py @@ -63,17 +63,17 @@ def _ensure_base_wiring_config_underlay(idl, bridge_mappings, routing_tables): return ovn_bridge_mappings, flows_info -def cleanup_wiring(bridge_mappings, ovs_flows, exposed_ips, routing_tables, - routing_tables_routes): +def cleanup_wiring(idl, bridge_mappings, ovs_flows, exposed_ips, + routing_tables, routing_tables_routes): if CONF.exposing_method == constants.EXPOSE_METHOD_UNDERLAY: - return _cleanup_wiring_underlay(bridge_mappings, ovs_flows, + return _cleanup_wiring_underlay(idl, bridge_mappings, ovs_flows, exposed_ips, routing_tables, routing_tables_routes) elif CONF.exposing_method == constants.EXPOSE_METHOD_OVN: raise NotImplementedError() -def _cleanup_wiring_underlay(bridge_mappings, ovs_flows, exposed_ips, +def _cleanup_wiring_underlay(idl, bridge_mappings, ovs_flows, exposed_ips, routing_tables, routing_tables_routes): current_ips = linux_net.get_exposed_ips(CONF.bgp_nic) expected_ips = [ip for ip_dict in exposed_ips.values() @@ -107,6 +107,19 @@ def _cleanup_wiring_underlay(bridge_mappings, ovs_flows, exposed_ips, linux_net.delete_bridge_ip_routes(routing_tables, routing_tables_routes, extra_routes) + # delete leaked vlan devices from previous vlan provider networks + delete_vlan_devices_leftovers(idl, bridge_mappings) + + +def delete_vlan_devices_leftovers(idl, bridge_mappings): + vlan_tags = idl.get_network_vlan_tags() + ovs_devices = set(bridge_mappings.values()) + for ovs_device in ovs_devices: + vlans = linux_net.get_bridge_vlans(ovs_device) + for vlan in vlans: + if vlan and vlan not in vlan_tags: + linux_net.delete_vlan_device_for_network(ovs_device, vlan) + def wire_provider_port(routing_tables_routes, ovs_flows, port_ips, bridge_device, bridge_vlan, localnet, routing_table, diff --git a/ovn_bgp_agent/privileged/linux_net.py b/ovn_bgp_agent/privileged/linux_net.py index 03d2bdea..216586f1 100644 --- a/ovn_bgp_agent/privileged/linux_net.py +++ b/ovn_bgp_agent/privileged/linux_net.py @@ -440,6 +440,24 @@ def get_link_device(device_name): return device +@ovn_bgp_agent.privileged.default.entrypoint +def get_bridge_vlans(device_name): + index = _get_link_id(device_name, raise_exception=False) + if not index: + LOG.debug("OVS Bridge %s deleted, no need to get information about " + "associated vlan devices", device_name) + + vlan_devices = get_link_devices(link=index) + vlans = [] + for vlan_device in vlan_devices: + ifla_linkinfo = get_attr(vlan_device, 'IFLA_LINKINFO') + if ifla_linkinfo: + ifla_data = get_attr(ifla_linkinfo, 'IFLA_INFO_DATA') + if ifla_data: + vlans.append(get_attr(ifla_data, 'IFLA_VLAN_ID')) + return vlans + + @tenacity.retry( retry=tenacity.retry_if_exception_type( netlink_exceptions.NetlinkDumpInterrupted), diff --git a/ovn_bgp_agent/tests/functional/privileged/test_linux_net.py b/ovn_bgp_agent/tests/functional/privileged/test_linux_net.py index 9b90643e..e7969005 100644 --- a/ovn_bgp_agent/tests/functional/privileged/test_linux_net.py +++ b/ovn_bgp_agent/tests/functional/privileged/test_linux_net.py @@ -356,6 +356,16 @@ class IpLinkTestCase(_LinuxNetTestCase): constants.LINK_UP) test_utils.wait_until_true(fn, timeout=5) + def test_get_bridge_vlan_devices(self): + vlan_id = random.randint(2, 4094) + linux_net.create_interface(self.dev_name, 'dummy') + linux_net.create_interface(self.dev_name2, 'vlan', + physical_interface=self.dev_name, + vlan_id=vlan_id) + + vlan_devices = linux_net.get_bridge_vlans(self.dev_name) + self.assertEqual(vlan_devices[0], vlan_id) + class IpAddressTestCase(_LinuxNetTestCase): diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py b/ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py index 9d4b8710..04d842e8 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py @@ -108,6 +108,8 @@ class TestNBOVNBGPDriver(test_base.TestCase): mock_ensure_ovn_dev.assert_called_once_with( CONF.bgp_nic, CONF.bgp_vrf) + @mock.patch.object(linux_net, 'delete_vlan_device_for_network') + @mock.patch.object(linux_net, 'get_bridge_vlans') @mock.patch.object(linux_net, 'get_extra_routing_table_for_bridge') @mock.patch.object(linux_net, 'delete_bridge_ip_routes') @mock.patch.object(linux_net, 'delete_ip_rules') @@ -125,7 +127,8 @@ class TestNBOVNBGPDriver(test_base.TestCase): mock_ensure_arp, mock_nic_address, mock_get_patch_ports, mock_ensure_mac, mock_remove_flows, mock_exposed_ips, mock_get_ip_rules, mock_del_exposed_ips, mock_del_ip_rules, - mock_del_ip_routes, mock_get_extra_route): + mock_del_ip_routes, mock_get_extra_route, + mock_get_bridge_vlans, mock_delete_vlan_dev): self.mock_ovs_idl.get_ovn_bridge_mappings.return_value = [ 'net0:bridge0', 'net1:bridge1'] self.nb_idl.get_network_vlan_tag_by_network_name.side_effect = ( @@ -149,6 +152,9 @@ class TestNBOVNBGPDriver(test_base.TestCase): mock_nic_address.return_value = self.mac mock_get_patch_ports.return_value = [1, 2] + self.nb_idl.get_network_vlan_tags.return_value = [10, 11] + mock_get_bridge_vlans.side_effect = [[10, 12], [11]] + self.nb_bgp_driver.sync() expected_calls = [mock.call({}, 'bridge0', CONF.bgp_vrf_table_id), @@ -177,6 +183,9 @@ class TestNBOVNBGPDriver(test_base.TestCase): mock_del_ip_rules.assert_called_once_with(fake_ip_rules) mock_del_ip_routes.assert_called_once() + bridge = set(self.nb_bgp_driver.ovn_bridge_mappings.values()).pop() + mock_delete_vlan_dev.assert_called_once_with(bridge, 12) + def test__ensure_port_exposed_fip(self): port0 = fakes.create_object({ 'name': 'port-0', diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_bgp_driver.py b/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_bgp_driver.py index 97268159..13356c01 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_bgp_driver.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_bgp_driver.py @@ -102,6 +102,7 @@ class TestOVNBGPDriver(test_base.TestCase): mock_ensure_ovn_dev.assert_called_once_with( CONF.bgp_nic, CONF.bgp_vrf) + @mock.patch.object(wire_utils, 'delete_vlan_devices_leftovers') @mock.patch.object(linux_net, 'delete_bridge_ip_routes') @mock.patch.object(linux_net, 'delete_ip_rules') @mock.patch.object(linux_net, 'delete_exposed_ips') @@ -119,7 +120,7 @@ class TestOVNBGPDriver(test_base.TestCase): mock_ensure_vlan_network, mock_nic_address, mock_exposed_ips, mock_get_ip_rules, mock_get_patch_ports, mock_ensure_mac, mock_remove_flows, mock_del_exposed_ips, mock_del_ip_rules, - mock_del_ip_routes): + mock_del_ip_routes, mock_vlan_leftovers): self.mock_ovs_idl.get_ovn_bridge_mappings.return_value = [ 'net0:bridge0', 'net1:bridge1'] self.sb_idl.get_network_vlan_tag_by_network_name.side_effect = ( @@ -183,6 +184,8 @@ class TestOVNBGPDriver(test_base.TestCase): {'bridge0': ['fake-route'], 'bridge1': ['fake-route']}) mock_get_ip_rules.assert_called_once_with(mock.ANY) + mock_vlan_leftovers.assert_called_once_with( + self.sb_idl, self.bgp_driver.ovn_bridge_mappings) @mock.patch.object(linux_net, 'get_ip_version') def test__ensure_cr_lrp_associated_ports_exposed(self, mock_ip_version): diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py index 8894a5c5..76f54829 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py @@ -39,6 +39,19 @@ class TestOvsdbNbOvnIdl(test_base.TestCase): self.nb_idl.db_find_rows = mock.Mock() self.nb_idl.lookup = mock.Mock() + def test_get_network_vlan_tags(self): + tag = [123] + lsp = fakes.create_object({'name': 'port-0', + 'tag': tag}) + self.nb_idl.db_find_rows.return_value.execute.return_value = [ + lsp] + ret = self.nb_idl.get_network_vlan_tags() + + self.assertEqual(tag, ret) + self.nb_idl.db_find_rows.assert_called_once_with( + 'Logical_Switch_Port', + ('type', '=', constants.OVN_LOCALNET_VIF_PORT_TYPE)) + def test_get_network_vlan_tag_by_network_name(self): network_name = 'net0' tag = [123] @@ -322,6 +335,14 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): def test_get_network_name_and_tag_not_in_bridge_mappings(self): self._test_get_network_name_and_tag(network_in_bridge_map=False) + def test_get_netweork_vlan_tags(self): + tag = [1001] + row = fakes.create_object({'tag': tag}) + self.sb_idl.db_find_rows.return_value.execute.return_value = [row, ] + + ret = self.sb_idl.get_network_vlan_tags() + self.assertEqual(tag, ret) + def _test_get_network_vlan_tag_by_network_name(self, match=True): network = 'public' if match else 'spongebob' tag = [1001] diff --git a/ovn_bgp_agent/utils/linux_net.py b/ovn_bgp_agent/utils/linux_net.py index 665a9a1b..24c1f464 100644 --- a/ovn_bgp_agent/utils/linux_net.py +++ b/ovn_bgp_agent/utils/linux_net.py @@ -339,6 +339,10 @@ def delete_vlan_device_for_network(bridge, vlan_tag): delete_device(vlan_device_name) +def get_bridge_vlans(bridge): + return ovn_bgp_agent.privileged.linux_net.get_bridge_vlans(bridge) + + def enable_proxy_ndp(device): flag = "net.ipv6.conf.{}.proxy_ndp".format(device) ovn_bgp_agent.privileged.linux_net.set_kernel_flag(flag, 1)