From 99c5e1f54142426bb240916a492499437cb1a425 Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Tue, 12 Dec 2023 12:46:53 +0100 Subject: [PATCH] Ensure NDB Proxy gets added for provider IPs too NDB proxy is needed with newer ovn versions for proper redirection of egress traffic for IPv6. This patch is adding it Closes-Bug: #2046254 Change-Id: I055e332deebce25d08d596d469f3ad2bdb800579 --- .../drivers/openstack/nb_ovn_bgp_driver.py | 95 +++++++++---------- .../drivers/openstack/ovn_bgp_driver.py | 4 + ovn_bgp_agent/drivers/openstack/utils/wire.py | 16 ++-- .../openstack/test_nb_ovn_bgp_driver.py | 5 +- 4 files changed, 58 insertions(+), 62 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 b883224d..3697128f 100644 --- a/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py @@ -281,6 +281,8 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): def _expose_provider_port(self, port_ips, mac, logical_switch, bridge_device, bridge_vlan, localnet, proxy_cidrs=None): + if proxy_cidrs is None: + proxy_cidrs = [] # Connect to OVN try: if wire_utils.wire_provider_port( @@ -302,6 +304,8 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): def _withdraw_provider_port(self, port_ips, logical_switch, bridge_device, bridge_vlan, proxy_cidrs=None): + if proxy_cidrs is None: + proxy_cidrs = [] # Withdraw IP before disconnecting it bgp_utils.withdraw_ips(port_ips) @@ -393,44 +397,36 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): LOG.debug("Adding BGP route for logical port with ip %s", ips) localnet = self.ovn_provider_ls[logical_switch]['localnet'] - if cidrs and port_type in [constants.OVN_VIRTUAL_VIF_PORT_TYPE, - constants.OVN_CR_LRP_PORT_TYPE]: - # NOTE: For Amphora Load Balancer with IPv6 VIP on the provider - # network, we need a NDP Proxy so that the traffic from the - # amphora can properly be redirected back - if not self._expose_provider_port(ips, mac, logical_switch, - bridge_device, bridge_vlan, - localnet, cidrs): - return [] - if router and port_type == constants.OVN_CR_LRP_PORT_TYPE: - # Store information about local CR-LRPs that will later be used - # to expose networks - self.ovn_local_cr_lrps[router] = { - 'bridge_device': bridge_device, - 'bridge_vlan': bridge_vlan, - 'provider_switch': logical_switch, - 'ips': ips, - } - # Expose associated subnets - ports = self.nb_idl.get_active_local_lrps([router]) - for port in ports: - ips = port.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY, - "").split() - subnet_info = { - 'associated_router': port.external_ids.get( - constants.OVN_DEVICE_ID_EXT_ID_KEY), - 'network': port.external_ids.get( - constants.OVN_LS_NAME_EXT_ID_KEY), - 'address_scopes': driver_utils.get_addr_scopes(port)} - self._expose_subnet(ips, subnet_info) + if not self._expose_provider_port(ips, mac, logical_switch, + bridge_device, bridge_vlan, + localnet, cidrs): + return [] + + if router and port_type == constants.OVN_CR_LRP_PORT_TYPE: + # Store information about local CR-LRPs that will later be used + # to expose networks + self.ovn_local_cr_lrps[router] = { + 'bridge_device': bridge_device, + 'bridge_vlan': bridge_vlan, + 'provider_switch': logical_switch, + 'ips': ips, + } + # Expose associated subnets + ports = self.nb_idl.get_active_local_lrps([router]) + for port in ports: + ips = port.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY, + "").split() + subnet_info = { + 'associated_router': port.external_ids.get( + constants.OVN_DEVICE_ID_EXT_ID_KEY), + 'network': port.external_ids.get( + constants.OVN_LS_NAME_EXT_ID_KEY), + 'address_scopes': driver_utils.get_addr_scopes(port)} + self._expose_subnet(ips, subnet_info) + + # add missing routes/ips for OVN loadbalancers + self._expose_lbs([router]) - # add missing routes/ips for OVN loadbalancers - self._expose_lbs([router]) - else: - if not self._expose_provider_port(ips, mac, logical_switch, - bridge_device, bridge_vlan, - localnet): - return [] LOG.debug("Added BGP route for logical port with ip %s", ips) return ips @@ -456,21 +452,18 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): # This means it is not a provider network return - proxy_cidr = None - - if ips_info['type'] in [constants.OVN_VIRTUAL_VIF_PORT_TYPE, - constants.OVN_CR_LRP_PORT_TYPE]: - for n_cidr in ips_info['cidrs']: - if linux_net.get_ip_version(n_cidr) == constants.IP_VERSION_6: - if not self.nb_idl.ls_has_virtual_ports(logical_switch): - proxy_cidr = n_cidr + proxy_cidr = [] + if ips_info['cidrs']: + if not (self.nb_idl.ls_has_virtual_ports(logical_switch) or + self.nb_idl.get_active_lsp_on_chassis(self.chassis)): + for n_cidr in ips_info['cidrs']: + if (linux_net.get_ip_version(n_cidr) == + constants.IP_VERSION_6): + proxy_cidr.append(n_cidr) LOG.debug("Deleting BGP route for logical port with ip %s", ips) - if proxy_cidr: - self._withdraw_provider_port(ips, logical_switch, bridge_device, - bridge_vlan, [proxy_cidr]) - else: - self._withdraw_provider_port(ips, logical_switch, bridge_device, - bridge_vlan) + self._withdraw_provider_port(ips, logical_switch, bridge_device, + bridge_vlan, proxy_cidr) + if ips_info.get('router'): # It is a Logical Router Port (CR-LRP) # Withdraw associated subnets diff --git a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py index bb257b3c..ea5e7847 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py @@ -325,6 +325,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase): def _expose_provider_port(self, port_ips, provider_datapath, bridge_device=None, bridge_vlan=None, lladdr=None, proxy_cidrs=None): + if proxy_cidrs is None: + proxy_cidrs = [] if not bridge_device and not bridge_vlan: bridge_device, bridge_vlan = self._get_bridge_for_datapath( provider_datapath) @@ -416,6 +418,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase): def _withdraw_provider_port(self, port_ips, provider_datapath, bridge_device=None, bridge_vlan=None, lladdr=None, proxy_cidrs=None): + if proxy_cidrs is None: + proxy_cidrs = [] # Withdraw IP before disconnecting it bgp_utils.withdraw_ips(port_ips) diff --git a/ovn_bgp_agent/drivers/openstack/utils/wire.py b/ovn_bgp_agent/drivers/openstack/utils/wire.py index 939d76a3..8e046093 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/wire.py +++ b/ovn_bgp_agent/drivers/openstack/utils/wire.py @@ -495,11 +495,10 @@ def _wire_provider_port_underlay(routing_tables_routes, ovs_flows, port_ips, linux_net.add_ip_route(routing_tables_routes, ip, routing_table[bridge_device], bridge_device, vlan=bridge_vlan) - if proxy_cidrs: - # add proxy ndp config for ipv6 - for n_cidr in proxy_cidrs: - if linux_net.get_ip_version(n_cidr) == constants.IP_VERSION_6: - linux_net.add_ndp_proxy(n_cidr, bridge_device, bridge_vlan) + # add proxy ndp config for ipv6 + for n_cidr in proxy_cidrs: + if linux_net.get_ip_version(n_cidr) == constants.IP_VERSION_6: + linux_net.add_ndp_proxy(n_cidr, bridge_device, bridge_vlan) # NOTE(ltomasbo): This is needed as the patch ports are not created # until the first VM/FIP in that provider network is created in a node try: @@ -556,10 +555,9 @@ def _unwire_provider_port_underlay(routing_tables_routes, port_ips, linux_net.del_ip_route(routing_tables_routes, ip, routing_table[bridge_device], bridge_device, vlan=bridge_vlan) - if proxy_cidrs: - for n_cidr in proxy_cidrs: - if linux_net.get_ip_version(n_cidr) == constants.IP_VERSION_6: - linux_net.del_ndp_proxy(n_cidr, bridge_device, bridge_vlan) + for n_cidr in proxy_cidrs: + if linux_net.get_ip_version(n_cidr) == constants.IP_VERSION_6: + linux_net.del_ndp_proxy(n_cidr, bridge_device, bridge_vlan) return True 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 75a51695..405ddb87 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 @@ -495,7 +495,7 @@ class TestNBOVNBGPDriver(test_base.TestCase): ips_info['cidrs']) else: mock_expose_provider_port.assert_called_once_with( - ips, 'fake-mac', 'test-ls', 'br-ex', 10, 'fake-localnet') + ips, 'fake-mac', 'test-ls', 'br-ex', 10, 'fake-localnet', []) if (ips_info.get('router') and ips_info['type'] == constants.OVN_CR_LRP_PORT_TYPE): @@ -559,6 +559,7 @@ class TestNBOVNBGPDriver(test_base.TestCase): self.nb_bgp_driver, '_get_ls_localnet_info').start() mock_ip_version.return_value = constants.IP_VERSION_6 self.nb_idl.ls_has_virtual_ports.return_value = False + self.nb_idl.get_active_lsp_on_chassis.return_value = False if provider: mock_get_ls_localnet_info.return_value = ('fake-localnet', 'br-ex', 10) @@ -606,7 +607,7 @@ class TestNBOVNBGPDriver(test_base.TestCase): ips, 'test-ls', 'br-ex', 10, ips_info['cidrs']) else: mock_withdraw_provider_port.assert_called_once_with( - ips, 'test-ls', 'br-ex', 10) + ips, 'test-ls', 'br-ex', 10, []) if ips_info.get('router'): mock_withdraw_subnet.assert_called_once_with(