From f5640789cf4c6b934e7f76b3091211b46b6d01af Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Fri, 7 Jun 2024 09:33:00 +0200 Subject: [PATCH] Ensure cr-lrp permanent ip nei entry is added on NB DB driver There wire provider port function had two definitions for the same parameter (mac and lladdr). This patch is merging them and ensuring it gets used on the NB DB Driver so that the PERMANENT ip nei entries get added for the router gateway ports (cr-lrp) Closes-Bug: #2068699 Change-Id: I9649cd185b100c9941887e3440bad8d39881f92c (cherry picked from commit a4e307e6f53794532284f02c91c0ff7a96fb242f) --- .../drivers/openstack/nb_ovn_bgp_driver.py | 4 +- .../drivers/openstack/ovn_bgp_driver.py | 4 +- ovn_bgp_agent/drivers/openstack/utils/wire.py | 16 +++--- .../openstack/test_nb_ovn_bgp_driver.py | 13 ++--- .../drivers/openstack/test_ovn_bgp_driver.py | 52 ++++++++++++------- .../unit/drivers/openstack/utils/test_wire.py | 5 +- ovn_bgp_agent/utils/linux_net.py | 6 ++- 7 files changed, 61 insertions(+), 39 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 46a5b309..165a0603 100644 --- a/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py @@ -554,10 +554,10 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): return None, None, None net_id = nat_entry.external_ids.get(constants.OVN_FIP_NET_EXT_ID_KEY) if not net_id: - return nat_entry.external_ip, nat_entry.external_mac, None + return nat_entry.external_ip, nat_entry.external_mac[0], None else: ls_name = "neutron-{}".format(net_id) - return nat_entry.external_ip, nat_entry.external_mac, ls_name + return nat_entry.external_ip, nat_entry.external_mac[0], ls_name @lockutils.synchronized('nbbgp') def expose_fip(self, ip, mac, logical_switch, row): diff --git a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py index b037d671..2c705966 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py @@ -335,7 +335,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): if wire_utils.wire_provider_port( self.ovn_routing_tables_routes, self.ovs_flows, port_ips, bridge_device, bridge_vlan, localnet, - self.ovn_routing_tables, proxy_cidrs, lladdr=lladdr): + self.ovn_routing_tables, proxy_cidrs, mac=lladdr): # Expose the IP now that it is connected bgp_utils.announce_ips(port_ips) return True @@ -413,7 +413,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): return wire_utils.unwire_provider_port( self.ovn_routing_tables_routes, port_ips, bridge_device, bridge_vlan, self.ovn_routing_tables, proxy_cidrs, - lladdr=lladdr) + mac=lladdr) except Exception as e: LOG.exception("Unexpected exception while unwiring provider port: " "%s", e) diff --git a/ovn_bgp_agent/drivers/openstack/utils/wire.py b/ovn_bgp_agent/drivers/openstack/utils/wire.py index 38894aa8..69f39791 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/wire.py +++ b/ovn_bgp_agent/drivers/openstack/utils/wire.py @@ -555,13 +555,13 @@ def _cleanup_wiring_evpn(ovs_flows, routing_tables_routes): def wire_provider_port(routing_tables_routes, ovs_flows, port_ips, bridge_device, bridge_vlan, localnet, routing_table, - proxy_cidrs, lladdr=None, mac=None, ovn_idl=None): + proxy_cidrs, mac=None, ovn_idl=None): if CONF.exposing_method == constants.EXPOSE_METHOD_UNDERLAY: return _wire_provider_port_underlay(routing_tables_routes, ovs_flows, port_ips, bridge_device, bridge_vlan, localnet, routing_table, proxy_cidrs, - lladdr=lladdr) + lladdr=mac) elif CONF.exposing_method == constants.EXPOSE_METHOD_VRF: return _wire_provider_port_evpn(routing_tables_routes, ovs_flows, port_ips, bridge_device, @@ -576,17 +576,17 @@ def wire_provider_port(routing_tables_routes, ovs_flows, port_ips, def unwire_provider_port(routing_tables_routes, port_ips, bridge_device, - bridge_vlan, routing_table, proxy_cidrs, lladdr=None, + bridge_vlan, routing_table, proxy_cidrs, mac=None, ovn_idl=None): if CONF.exposing_method == constants.EXPOSE_METHOD_UNDERLAY: return _unwire_provider_port_underlay(routing_tables_routes, port_ips, bridge_device, bridge_vlan, routing_table, proxy_cidrs, - lladdr=lladdr) + lladdr=mac) elif CONF.exposing_method == constants.EXPOSE_METHOD_VRF: return _unwire_provider_port_evpn(routing_tables_routes, port_ips, bridge_device, bridge_vlan, - lladdr) + mac) elif CONF.exposing_method == constants.EXPOSE_METHOD_OVN: # We need to remove thestatic mac binding added due to proxy-arp issue # in core ovn that would reply on the incomming traffic from the LR, @@ -618,7 +618,8 @@ def _wire_provider_port_underlay(routing_tables_routes, ovs_flows, port_ips, linux_net.add_ip_rule(ip, routing_table[bridge_device], dev=dev, lladdr=lladdr) else: - linux_net.add_ip_rule(ip, routing_table[bridge_device]) + linux_net.add_ip_rule(ip, routing_table[bridge_device], + dev=bridge_device) except agent_exc.InvalidPortIP: LOG.exception("Invalid IP to create a rule for port on the " "provider network: %s", ip) @@ -699,7 +700,8 @@ def _unwire_provider_port_underlay(routing_tables_routes, port_ips, return False else: try: - linux_net.del_ip_rule(ip, routing_table[bridge_device]) + linux_net.del_ip_rule(ip, routing_table[bridge_device], + dev=bridge_device) except agent_exc.InvalidPortIP: LOG.exception("Invalid IP to delete a rule for the " "provider port: %s", ip) 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 6264eb4d..6a56e133 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 @@ -728,12 +728,12 @@ class TestNBOVNBGPDriver(test_base.TestCase): nat_entry = fakes.create_object({ 'external_ids': {constants.OVN_FIP_NET_EXT_ID_KEY: 'net1'}, 'external_ip': 'fake-ip', - 'external_mac': 'fake-mac'}) + 'external_mac': ['fake-mac']}) self.nb_idl.get_nat_by_logical_port.return_value = nat_entry ret = self.nb_bgp_driver.get_port_external_ip_and_ls('fake-port') - expected_result = (nat_entry.external_ip, nat_entry.external_mac, + expected_result = (nat_entry.external_ip, nat_entry.external_mac[0], "neutron-net1") self.assertEqual(ret, expected_result) @@ -748,13 +748,13 @@ class TestNBOVNBGPDriver(test_base.TestCase): nat_entry = fakes.create_object({ 'external_ids': {}, 'external_ip': 'fake-ip', - 'external_mac': 'fake-mac'}) + 'external_mac': ['fake-mac']}) self.nb_idl.get_nat_by_logical_port.return_value = nat_entry ret = self.nb_bgp_driver.get_port_external_ip_and_ls('fake-port') - self.assertEqual(ret, - (nat_entry.external_ip, nat_entry.external_mac, None)) + self.assertEqual( + ret, (nat_entry.external_ip, nat_entry.external_mac[0], None)) def test_expose_fip(self): ip = '10.0.0.1' @@ -773,7 +773,8 @@ class TestNBOVNBGPDriver(test_base.TestCase): ret = self.nb_bgp_driver.expose_fip(ip, mac, logical_switch, row) mock_get_ls_localnet_info.assert_called_once_with(logical_switch) - mock_expose_provider_port.assert_called_once_with([ip], mac, 'test-ls', + mock_expose_provider_port.assert_called_once_with([ip], mac, + 'test-ls', 'br-ex', 100, 'fake-localnet') self.assertTrue(ret) 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 64d4b747..dbf38769 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 @@ -298,7 +298,7 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_ips_dev.assert_called_once_with( CONF.bgp_nic, [self.ipv4]) mock_add_rule.assert_called_once_with( - self.ipv4, 'fake-table') + self.ipv4, 'fake-table', dev='fake-bridge') mock_add_route.assert_called_once_with( mock.ANY, self.ipv4, 'fake-table', self.bridge, vlan=10) @@ -340,7 +340,7 @@ class TestOVNBGPDriver(test_base.TestCase): self.assertEqual(False, ret) mock_add_ips_dev.assert_not_called() mock_add_rule.assert_called_once_with( - self.ipv4, 'fake-table') + self.ipv4, 'fake-table', dev='fake-bridge') mock_add_route.assert_not_called() mock_ensure_mac_tweak.assert_not_called() @@ -518,7 +518,7 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_ips_dev.assert_called_once_with( CONF.bgp_nic, [self.ipv4]) mock_del_rule.assert_called_once_with( - self.ipv4, 'fake-table') + self.ipv4, 'fake-table', dev='fake-bridge') mock_del_route.assert_called_once_with( mock.ANY, self.ipv4, 'fake-table', self.bridge, vlan=10) @@ -1009,8 +1009,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock.call(CONF.bgp_nic, [self.ipv6])] mock_del_ip_dev.assert_has_calls(expected_calls) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_del_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1066,8 +1068,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_add_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1149,8 +1153,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_add_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1218,7 +1224,7 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, [self.fip]) mock_add_rule.assert_called_once_with( - self.fip, 'fake-table') + self.fip, 'fake-table', dev='fake-bridge') mock_add_route.assert_called_once_with( mock.ANY, self.fip, 'fake-table', self.bridge, vlan=10) @@ -1296,8 +1302,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_add_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1436,8 +1444,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_del_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1474,8 +1484,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_del_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1509,7 +1521,7 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_ip_dev.assert_called_once_with( CONF.bgp_nic, [self.fip]) mock_del_rule.assert_called_once_with( - self.fip, 'fake-table') + self.fip, 'fake-table', dev='fake-bridge') mock_del_route.assert_called_once_with( mock.ANY, self.fip, 'fake-table', self.bridge, vlan=10) @@ -1555,8 +1567,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_del_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py index dc89e4bb..fd82b1d6 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py @@ -574,7 +574,7 @@ class TestWire(test_base.TestCase): wire.unwire_provider_port(routing_tables_routes, port_ips, bridge_device, bridge_vlan, routing_table, - proxy_cidrs, lladdr='boo') + proxy_cidrs, mac='boo') mock_evpn.assert_called_once_with(routing_tables_routes, port_ips, bridge_device, bridge_vlan, 'boo') @@ -757,6 +757,9 @@ class TestWire(test_base.TestCase): CONF.set_override( 'advertisement_method_tenant_networks', constants.ADVERTISEMENT_METHOD_SUBNET) + self.addCleanup( + CONF.clear_override, + 'advertisement_method_tenant_networks') routing_tables_routes = {} ip = '10.0.0.1/24' bridge_device = 'fake-bridge' diff --git a/ovn_bgp_agent/utils/linux_net.py b/ovn_bgp_agent/utils/linux_net.py index a1b01b1c..4dc25740 100644 --- a/ovn_bgp_agent/utils/linux_net.py +++ b/ovn_bgp_agent/utils/linux_net.py @@ -701,8 +701,10 @@ def del_ip_rule(ip, table, dev=None, lladdr=None): ovn_bgp_agent.privileged.linux_net.rule_delete(rule) - if lladdr: - del_ip_nei(ip, lladdr, dev) + # TODO(jayjahns): The lladdr in all delete operations is blank, so + # we need to investigate how to ensure that exists in order to + # add the if comparison. + del_ip_nei(ip, lladdr, dev) def del_ip_nei(ip, lladdr, dev):