Merge "Disable exposing remote_ips, when only the lrp prefix is sufficient"

This commit is contained in:
Zuul 2024-02-22 11:42:43 +00:00 committed by Gerrit Code Review
commit 1b371dfc03
9 changed files with 225 additions and 52 deletions

@ -34,6 +34,13 @@ agent_opts = [
'expose_ipv6_gua_tenant_networks flag and all tenant '
'network IPs will be exposed.',
default=False),
cfg.StrOpt('advertisement_method_tenant_networks',
help='The NB driver is capable of advertising the tenant '
'networks either per host or per subnet. '
'So either per /32 or /128 or per subnet like /24. '
'Choose "host" as value for this option to advertise per '
'host or choose "subnet" to announce per subnet prefix.',
default='host', choices=['host', 'subnet']),
cfg.BoolOpt('expose_ipv6_gua_tenant_networks',
help='Expose only VM IPv6 IPs on tenant networks if they are '
'GUA. The expose_tenant_networks parameter takes '

@ -152,9 +152,12 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
events.update({watcher.ChassisRedirectCreateEvent(self),
watcher.ChassisRedirectDeleteEvent(self),
watcher.LogicalSwitchPortSubnetAttachEvent(self),
watcher.LogicalSwitchPortSubnetDetachEvent(self),
watcher.LogicalSwitchPortTenantCreateEvent(self),
watcher.LogicalSwitchPortTenantDeleteEvent(self)})
watcher.LogicalSwitchPortSubnetDetachEvent(self)})
if CONF.advertisement_method_tenant_networks == 'host':
events.update({
watcher.LogicalSwitchPortTenantCreateEvent(self),
watcher.LogicalSwitchPortTenantDeleteEvent(self)
})
return events
@lockutils.synchronized('nbbgp')
@ -603,6 +606,10 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
self._withdraw_remote_ip(ips, ips_info)
def _expose_remote_ip(self, ips, ips_info):
if CONF.advertisement_method_tenant_networks == 'subnet':
# Ip should already be exported via cr-lrp subnet announcement.
return
ips_to_expose = ips
if not CONF.expose_tenant_networks:
# This means CONF.expose_ipv6_gua_tenant_networks is enabled
@ -621,6 +628,9 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
ips_to_expose, self.chassis)
def _withdraw_remote_ip(self, ips, ips_info):
if CONF.advertisement_method_tenant_networks == 'subnet':
return
ips_to_withdraw = ips
if not CONF.expose_tenant_networks:
# This means CONF.expose_ipv6_gua_tenant_networks is enabled
@ -665,6 +675,10 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
"and they have not been properly exposed", ips)
return
if CONF.advertisement_method_tenant_networks == 'subnet':
# Networks have been exposed via self._expose_router_lsp
return
ports = self.nb_idl.get_active_lsp(subnet_info['network'])
for port in ports:
ips = port.addresses[0].split(' ')[1:]
@ -695,6 +709,11 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
return
self._withdraw_router_lsp(ips, subnet_info, cr_lrp_info)
if CONF.advertisement_method_tenant_networks == 'subnet':
# Expose the routes per prefix, rather than per port.
return
ports = self.nb_idl.get_active_lsp(subnet_info['network'])
for port in ports:
ips = port.addresses[0].split(' ')[1:]
@ -712,6 +731,12 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
def _expose_router_lsp(self, ips, subnet_info, cr_lrp_info):
if not self._expose_tenant_networks:
return True
# Fix ips to be the network address, instead of the lrp address
# so the cleanup will not remove them, since they match what's
# in the kernel
ips = driver_utils.get_prefixes_from_ips(ips)
success = True
for ip in ips:
if not CONF.expose_tenant_networks:
@ -727,15 +752,15 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
cr_lrp_info.get('bridge_device'),
cr_lrp_info.get('bridge_vlan'),
self.ovn_routing_tables, cr_lrp_info.get('ips')):
self._exposed_ips.setdefault(
subnet_info['associated_router'], {}).update(
logical_switch = cr_lrp_info['provider_switch']
self._exposed_ips.setdefault(logical_switch, {}).update(
{ip: {
'bridge_device': cr_lrp_info.get('bridge_device'),
'bridge_vlan': cr_lrp_info.get('bridge_vlan')}})
if self.ovn_local_lrps.get(subnet_info['network']):
self.ovn_local_lrps[subnet_info['network']].append(ip)
else:
self.ovn_local_lrps[subnet_info['network']] = [ip]
self.ovn_local_lrps.setdefault(
subnet_info['network'], []).append(ip)
else:
success = False
@ -748,6 +773,12 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
def _withdraw_router_lsp(self, ips, subnet_info, cr_lrp_info):
if not self._expose_tenant_networks:
return
# Fix ips to be the network address, instead of the lrp address
# so the cleanup will not remove them, since they match what's
# in the kernel
ips = driver_utils.get_prefixes_from_ips(ips)
for ip in ips:
if (not CONF.expose_tenant_networks and
not driver_utils.is_ipv6_gua(ip)):
@ -762,10 +793,9 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
cr_lrp_info.get('bridge_device'),
cr_lrp_info.get('bridge_vlan'),
self.ovn_routing_tables, cr_lrp_info.get('ips')):
if self._exposed_ips.get(
subnet_info['associated_router'], {}).get(ip):
self._exposed_ips[
subnet_info['associated_router']].pop(ip)
logical_switch = cr_lrp_info['provider_switch']
self._exposed_ips.get(logical_switch, {}).pop(ip, None)
else:
return False
except Exception as e:

@ -72,9 +72,9 @@ class OVNBGPStretchedL2Driver(driver_api.AgentDriverBase):
self.ovs_idl.start(CONF.ovsdb_connection)
# Base BGP configuration
# Ensure FRR is configured to leak the routes
bgp_utils.ensure_base_bgp_configuration(
template=frr.LEAK_VRF_KERNEL_TEMPLATE)
# Ensure FRR is configured to leak only kernel routes by default
frr.set_default_redistribute(['kernel'])
bgp_utils.ensure_base_bgp_configuration()
# Clear vrf routing table
if CONF.clear_vrf_routes_on_startup:
@ -114,8 +114,7 @@ class OVNBGPStretchedL2Driver(driver_api.AgentDriverBase):
LOG.debug("Ensuring VRF configuration for advertising routes")
# Base BGP configuration
# Ensure FRR is configured to leak the routes
bgp_utils.ensure_base_bgp_configuration(
template=frr.LEAK_VRF_KERNEL_TEMPLATE)
bgp_utils.ensure_base_bgp_configuration()
@lockutils.synchronized("bgp")
def sync(self):

@ -41,6 +41,10 @@ def ensure_base_bgp_configuration(template=frr.LEAK_VRF_TEMPLATE):
# Create VRF
linux_net.ensure_vrf(CONF.bgp_vrf, CONF.bgp_vrf_table_id)
# If we expose subnet routes, we should add kernel routes too.
if CONF.advertisement_method_tenant_networks == 'subnet':
frr.set_default_redistribute(['connected', 'kernel'])
# Ensure FRR is configure to leak the routes
frr.vrf_leak(CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id,
template=template)

@ -49,3 +49,13 @@ def check_name_prefix(entity, prefix):
def is_pf_lb(lb):
return check_name_prefix(lb, constants.OVN_LB_PF_NAME_PREFIX)
def get_prefixes_from_ips(ips: 'list[str]') -> 'list[str]':
'''Return the network address for any given ip (with mask)
For a list like ['192.168.0.1/24'] it will return ['192.168.0.0/24']
'''
return ['/'.join([ipaddress.ip_network(ip, strict=False)[0].compressed,
ip.split('/')[-1]])
for ip in ips]

@ -23,6 +23,8 @@ import ovn_bgp_agent.privileged.vtysh
LOG = logging.getLogger(__name__)
DEFAULT_REDISTRIBUTE = {'connected'}
ADD_VRF_TEMPLATE = '''
vrf {{ vrf_name }}
vni {{ vni }}
@ -30,10 +32,14 @@ exit-vrf
router bgp {{ bgp_as }} vrf {{ vrf_name }}
address-family ipv4 unicast
redistribute connected
{% for redist in redistribute %}
redistribute {{ redist }}
{% endfor %}
exit-address-family
address-family ipv6 unicast
redistribute connected
{% for redist in redistribute %}
redistribute {{ redist }}
{% endfor %}
exit-address-family
address-family l2vpn evpn
advertise ipv4 unicast
@ -61,33 +67,15 @@ router bgp {{ bgp_as }}
router bgp {{ bgp_as }} vrf {{ vrf_name }}
bgp router-id {{ bgp_router_id }}
address-family ipv4 unicast
redistribute connected
{% for redist in redistribute %}
redistribute {{ redist }}
{% endfor %}
exit-address-family
address-family ipv6 unicast
redistribute connected
exit-address-family
'''
LEAK_VRF_KERNEL_TEMPLATE = '''
router bgp {{ bgp_as }}
address-family ipv4 unicast
import vrf {{ vrf_name }}
exit-address-family
address-family ipv6 unicast
import vrf {{ vrf_name }}
exit-address-family
router bgp {{ bgp_as }} vrf {{ vrf_name }}
bgp router-id {{ bgp_router_id }}
address-family ipv4 unicast
redistribute kernel
exit-address-family
address-family ipv6 unicast
redistribute kernel
{% for redist in redistribute %}
redistribute {{ redist }}
{% endfor %}
exit-address-family
'''
@ -118,6 +106,18 @@ def _run_vtysh_config_with_tempfile(vrf_config):
f.close()
def set_default_redistribute(redist_opts):
if not isinstance(redist_opts, set):
redist_opts = set(redist_opts)
if redist_opts == DEFAULT_REDISTRIBUTE:
# no update required.
return
DEFAULT_REDISTRIBUTE.clear()
DEFAULT_REDISTRIBUTE.update(redist_opts)
def vrf_leak(vrf, bgp_as, bgp_router_id=None, template=LEAK_VRF_TEMPLATE):
LOG.info("Add VRF leak for VRF %s on router bgp %s", vrf, bgp_as)
if not bgp_router_id:
@ -128,6 +128,7 @@ def vrf_leak(vrf, bgp_as, bgp_router_id=None, template=LEAK_VRF_TEMPLATE):
vrf_template = Template(template)
vrf_config = vrf_template.render(vrf_name=vrf, bgp_as=bgp_as,
redistribute=DEFAULT_REDISTRIBUTE,
bgp_router_id=bgp_router_id)
_run_vtysh_config_with_tempfile(vrf_config)
@ -141,6 +142,7 @@ def vrf_reconfigure(evpn_info, action):
vrf_name="{}{}".format(constants.OVN_EVPN_VRF_PREFIX,
evpn_info['vni']),
bgp_as=evpn_info['bgp_as'],
redistribute=DEFAULT_REDISTRIBUTE,
vni=evpn_info['vni'])
elif action == "del-vrf":
vrf_template = Template(DEL_VRF_TEMPLATE)

@ -935,6 +935,30 @@ class TestNBOVNBGPDriver(test_base.TestCase):
self.nb_bgp_driver.expose_subnet(ips, subnet_info)
mock_expose_router_lsp.assert_not_called()
def test_expose_subnet_not_per_lsp(self):
CONF.set_override('advertisement_method_tenant_networks', 'subnet')
self.addCleanup(CONF.clear_override,
'advertisement_method_tenant_networks')
ips = ['10.0.0.1/24']
subnet_info = {
'associated_router': 'router1',
'network': 'network1',
'address_scopes': {4: None, 6: None}}
mock_expose_router_lsp = mock.patch.object(
self.nb_bgp_driver, '_expose_router_lsp').start()
mock_expose_remote_ip = mock.patch.object(
self.nb_bgp_driver, '_expose_remote_ip').start()
self.nb_bgp_driver.expose_subnet(ips, subnet_info)
mock_expose_router_lsp.assert_called_once_with(ips, subnet_info,
self.router1_info)
self.nb_idl.get_active_lsp.assert_not_called()
mock_expose_remote_ip.assert_not_called()
def test_withdraw_subnet(self):
ips = ['10.0.0.1/24']
subnet_info = {
@ -1009,12 +1033,16 @@ class TestNBOVNBGPDriver(test_base.TestCase):
'network': 'network1',
'address_scopes': {4: None, 6: None}}
CONF.set_override('advertisement_method_tenant_networks', 'subnet')
self.addCleanup(CONF.clear_override,
'advertisement_method_tenant_networks')
ret = self.nb_bgp_driver._expose_router_lsp(ips, subnet_info,
self.router1_info)
self.assertTrue(ret)
mock_wire.assert_called_once_with(
mock.ANY, ips[0], self.router1_info['bridge_device'],
mock.ANY, '10.0.0.0/24', self.router1_info['bridge_device'],
self.router1_info['bridge_vlan'], mock.ANY,
self.router1_info['ips'])
@ -1027,12 +1055,16 @@ class TestNBOVNBGPDriver(test_base.TestCase):
'address_scopes': {4: None, 6: None}}
mock_wire.side_effect = Exception
CONF.set_override('advertisement_method_tenant_networks', 'subnet')
self.addCleanup(CONF.clear_override,
'advertisement_method_tenant_networks')
ret = self.nb_bgp_driver._expose_router_lsp(ips, subnet_info,
self.router1_info)
self.assertFalse(ret)
mock_wire.assert_called_once_with(
mock.ANY, ips[0], self.router1_info['bridge_device'],
mock.ANY, '10.0.0.0/24', self.router1_info['bridge_device'],
self.router1_info['bridge_vlan'], mock.ANY,
self.router1_info['ips'])
@ -1059,6 +1091,9 @@ class TestNBOVNBGPDriver(test_base.TestCase):
self.addCleanup(CONF.clear_override, 'expose_tenant_networks')
CONF.set_override('expose_ipv6_gua_tenant_networks', True)
self.addCleanup(CONF.clear_override, 'expose_ipv6_gua_tenant_networks')
CONF.set_override('advertisement_method_tenant_networks', 'subnet')
self.addCleanup(CONF.clear_override,
'advertisement_method_tenant_networks')
ips = ['10.0.0.1/24', '2002::1/64']
subnet_info = {
@ -1072,7 +1107,7 @@ class TestNBOVNBGPDriver(test_base.TestCase):
self.assertTrue(ret)
mock_wire.assert_called_once_with(
mock.ANY, ips[1], self.router1_info['bridge_device'],
mock.ANY, '2002::/64', self.router1_info['bridge_device'],
self.router1_info['bridge_vlan'], mock.ANY,
self.router1_info['ips'])
@ -1084,12 +1119,16 @@ class TestNBOVNBGPDriver(test_base.TestCase):
'network': 'network1',
'address_scopes': {4: None, 6: None}}
CONF.set_override('advertisement_method_tenant_networks', 'subnet')
self.addCleanup(CONF.clear_override,
'advertisement_method_tenant_networks')
ret = self.nb_bgp_driver._withdraw_router_lsp(ips, subnet_info,
self.router1_info)
self.assertTrue(ret)
mock_unwire.assert_called_once_with(
mock.ANY, ips[0], self.router1_info['bridge_device'],
mock.ANY, '10.0.0.0/24', self.router1_info['bridge_device'],
self.router1_info['bridge_vlan'], mock.ANY,
self.router1_info['ips'])
@ -1102,12 +1141,16 @@ class TestNBOVNBGPDriver(test_base.TestCase):
'address_scopes': {4: None, 6: None}}
mock_unwire.side_effect = Exception
CONF.set_override('advertisement_method_tenant_networks', 'subnet')
self.addCleanup(CONF.clear_override,
'advertisement_method_tenant_networks')
ret = self.nb_bgp_driver._withdraw_router_lsp(ips, subnet_info,
self.router1_info)
self.assertFalse(ret)
mock_unwire.assert_called_once_with(
mock.ANY, ips[0], self.router1_info['bridge_device'],
mock.ANY, '10.0.0.0/24', self.router1_info['bridge_device'],
self.router1_info['bridge_vlan'], mock.ANY,
self.router1_info['ips'])
@ -1121,6 +1164,10 @@ class TestNBOVNBGPDriver(test_base.TestCase):
'network': 'network1',
'address_scopes': {4: None, 6: None}}
CONF.set_override('advertisement_method_tenant_networks', 'subnet')
self.addCleanup(CONF.clear_override,
'advertisement_method_tenant_networks')
ret = self.nb_bgp_driver._withdraw_router_lsp(ips, subnet_info,
self.router1_info)
@ -1135,6 +1182,9 @@ class TestNBOVNBGPDriver(test_base.TestCase):
self.addCleanup(CONF.clear_override, 'expose_tenant_networks')
CONF.set_override('expose_ipv6_gua_tenant_networks', True)
self.addCleanup(CONF.clear_override, 'expose_ipv6_gua_tenant_networks')
CONF.set_override('advertisement_method_tenant_networks', 'subnet')
self.addCleanup(CONF.clear_override,
'advertisement_method_tenant_networks')
ips = ['10.0.0.1/24', '2002::1/64']
subnet_info = {
@ -1148,7 +1198,7 @@ class TestNBOVNBGPDriver(test_base.TestCase):
self.assertTrue(ret)
mock_unwire.assert_called_once_with(
mock.ANY, ips[1], self.router1_info['bridge_device'],
mock.ANY, '2002::/64', self.router1_info['bridge_device'],
self.router1_info['bridge_vlan'], mock.ANY,
self.router1_info['ips'])

@ -143,13 +143,18 @@ class TestOVNBGPStretchedL2Driver(test_base.TestCase):
*args):
CONF.set_override("clear_vrf_routes_on_startup", True)
mock_redistribute = mock.patch.object(
frr, "set_default_redistribute"
).start()
self.bgp_driver.start()
mock_redistribute.assert_called_with(['kernel'])
mock_vrf.assert_called_once_with(
CONF.bgp_vrf,
CONF.bgp_AS,
CONF.bgp_router_id,
template=frr.LEAK_VRF_KERNEL_TEMPLATE,
template=frr.LEAK_VRF_TEMPLATE,
)
# Assert connections were started
self.mock_ovs_idl().start.assert_called_once_with(
@ -168,13 +173,18 @@ class TestOVNBGPStretchedL2Driver(test_base.TestCase):
self, mock_vrf, mock_delete_routes, mock_ensure_ovn_device, *args):
CONF.set_override("clear_vrf_routes_on_startup", False)
mock_redistribute = mock.patch.object(
frr, "set_default_redistribute"
).start()
self.bgp_driver.start()
mock_redistribute.assert_called_with(['kernel'])
mock_vrf.assert_called_once_with(
CONF.bgp_vrf,
CONF.bgp_AS,
CONF.bgp_router_id,
template=frr.LEAK_VRF_KERNEL_TEMPLATE,
template=frr.LEAK_VRF_TEMPLATE,
)
# Assert connections were started
self.mock_ovs_idl().start.assert_called_once_with(

@ -0,0 +1,61 @@
# Copyright 2024 team.blue/nl
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from ovn_bgp_agent.drivers.openstack.utils import driver_utils
from ovn_bgp_agent.tests import base as test_base
from ovn_bgp_agent.tests import utils
class TestDriverUtils(test_base.TestCase):
def setUp(self):
super(TestDriverUtils, self).setUp()
def test_is_ipv6_gua(self):
self.assertFalse(driver_utils.is_ipv6_gua('1.1.1.1'))
self.assertFalse(driver_utils.is_ipv6_gua('fe80::1337'))
self.assertTrue(driver_utils.is_ipv6_gua('2a01:db8::1337'))
def test_check_name_prefix(self):
lb = utils.create_row(name='some-name')
self.assertTrue(driver_utils.check_name_prefix(lb, 'some'))
self.assertFalse(driver_utils.check_name_prefix(lb, 'other'))
lb = utils.create_row(no_name='aa')
self.assertFalse(driver_utils.check_name_prefix(lb, ''))
def is_pf_lb(self):
lb = utils.create_row(name='pf-floating-ip-someuuid')
self.assertTrue(driver_utils.is_pf_lb(lb))
lb = utils.create_row(name='lb-someuuid')
self.assertFalse(driver_utils.is_pf_lb(lb))
def test_get_prefixes_from_ips(self):
# IPv4
ips = ['192.168.0.1/24', '192.168.0.244/28', '172.13.37.59/27']
self.assertListEqual(driver_utils.get_prefixes_from_ips(ips),
['192.168.0.0/24', '192.168.0.240/28',
'172.13.37.32/27'])
# IPv6
ips = ['fe80::5097/64', 'ff00::13:37/112', 'fc00::1/46']
self.assertListEqual(driver_utils.get_prefixes_from_ips(ips),
['fe80::/64', 'ff00::13:0/112', 'fc00::/46'])
# combined.
ips = ['172.13.37.59/27', 'ff00::13:37/112']
self.assertListEqual(driver_utils.get_prefixes_from_ips(ips),
['172.13.37.32/27', 'ff00::13:0/112'])