openstack/ovn-bgp-agent
Code Issues Proposed changes

Avoid usage of NDB in ovn_bgp_driver, and ovs and wire utils

Browse Source 
This patch continues with the work on avoiding NDB usage in favor
of IPRoute

Partial-Bug: #2022357
Change-Id: Id94c0d911ab9bb320a88b87e834ecf2fc423e041
This commit is contained in:
Luis Tomas Bolivar 2023-06-12 11:23:51 +02:00
parent 7115d6bf93
commit 1e95409666
13 changed files with 176 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py
View File

@ -14,7 +14,6 @@
import collections
import ipaddress
import pyroute2
import threading
from oslo_concurrency import lockutils
@ -168,44 +167,46 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
bridge_mappings = self.ovs_idl.get_ovn_bridge_mappings()
# 2) Get macs for bridge mappings
extra_routes = {}
with pyroute2.NDB() as ndb:
for bridge_index, bridge_mapping in enumerate(bridge_mappings, 1):
network = bridge_mapping.split(":")[0]
bridge = bridge_mapping.split(":")[1]
self.ovn_bridge_mappings[network] = bridge
if not extra_routes.get(bridge):
extra_routes[bridge] = (
linux_net.ensure_routing_table_for_bridge(
self.ovn_routing_tables, bridge,
CONF.bgp_vrf_table_id))
vlan_tags = self.sb_idl.get_network_vlan_tag_by_network_name(
network)
for bridge_index, bridge_mapping in enumerate(bridge_mappings, 1):
network = bridge_mapping.split(":")[0]
bridge = bridge_mapping.split(":")[1]
self.ovn_bridge_mappings[network] = bridge
for vlan_tag in vlan_tags:
linux_net.ensure_vlan_device_for_network(bridge,
vlan_tag)
if not extra_routes.get(bridge):
extra_routes[bridge] = (
linux_net.ensure_routing_table_for_bridge(
self.ovn_routing_tables, bridge,
CONF.bgp_vrf_table_id))
vlan_tags = self.sb_idl.get_network_vlan_tag_by_network_name(
network)
linux_net.ensure_arp_ndp_enabled_for_bridge(bridge,
bridge_index,
vlan_tags)
for vlan_tag in vlan_tags:
linux_net.ensure_vlan_device_for_network(bridge,
vlan_tag)
if self.ovs_flows.get(bridge):
continue
self.ovs_flows[bridge] = {
'mac': ndb.interfaces[bridge]['address'],
'in_port': set([])}
# 3) Get in_port for bridge mappings (br-ex, br-ex2)
self.ovs_flows[bridge]['in_port'] = (
ovs.get_ovs_patch_ports_info(bridge))
linux_net.ensure_arp_ndp_enabled_for_bridge(bridge,
bridge_index,
vlan_tags)
# 4) Add/Remove flows for each bridge mappings
ovs.ensure_mac_tweak_flows(bridge,
self.ovs_flows[bridge]['mac'],
self.ovs_flows[bridge]['in_port'],
constants.OVS_RULE_COOKIE)
ovs.remove_extra_ovs_flows(self.ovs_flows, bridge,
constants.OVS_RULE_COOKIE)
if self.ovs_flows.get(bridge):
continue
mac = linux_net.get_interface_address(bridge)
self.ovs_flows[bridge] = {
'mac': mac,
'in_port': set([])}
# 3) Get in_port for bridge mappings (br-ex, br-ex2)
self.ovs_flows[bridge]['in_port'] = (
ovs.get_ovs_patch_ports_info(bridge))
# 4) Add/Remove flows for each bridge mappings
ovs.ensure_mac_tweak_flows(bridge,
self.ovs_flows[bridge]['mac'],
self.ovs_flows[bridge]['in_port'],
constants.OVS_RULE_COOKIE)
ovs.remove_extra_ovs_flows(self.ovs_flows, bridge,
constants.OVS_RULE_COOKIE)
LOG.debug("Syncing current routes.")
exposed_ips = linux_net.get_exposed_ips(CONF.bgp_nic)

29
ovn_bgp_agent/drivers/openstack/utils/ovs.py
View File

@ -17,7 +17,6 @@ from ovs.db import idl
from ovsdbapp.backend.ovs_idl import connection
from ovsdbapp.backend.ovs_idl import idlutils
from ovsdbapp.schema.open_vswitch import impl_idl as idl_ovs
import pyroute2
import tenacity
from ovn_bgp_agent import constants
@ -144,21 +143,19 @@ def ensure_evpn_ovs_flow(bridge, cookie, mac, output_port, port_dst, net,
strip_vlan_opt = 'strip_vlan,' if strip_vlan else ''
ip_version = linux_net.get_ip_version(net)
with pyroute2.NDB() as ndb:
if ip_version == constants.IP_VERSION_6:
flow = (
"cookie={},priority=1000,ipv6,in_port={},dl_src:{},"
"ipv6_src={} actions=mod_dl_dst:{},{}output={}".format(
cookie, ovs_ofport, mac, net,
ndb.interfaces[port_dst]['address'], strip_vlan_opt,
vrf_ofport))
else:
flow = (
"cookie={},priority=1000,ip,in_port={},dl_src:{},nw_src={}"
"actions=mod_dl_dst:{},{}output={}".format(
cookie, ovs_ofport, mac, net,
ndb.interfaces[port_dst]['address'], strip_vlan_opt,
vrf_ofport))
port_dst_mac = linux_net.get_interface_address(port_dst)
if ip_version == constants.IP_VERSION_6:
flow = (
"cookie={},priority=1000,ipv6,in_port={},dl_src:{},"
"ipv6_src={} actions=mod_dl_dst:{},{}output={}".format(
cookie, ovs_ofport, mac, net, port_dst_mac, strip_vlan_opt,
vrf_ofport))
else:
flow = (
"cookie={},priority=1000,ip,in_port={},dl_src:{},nw_src={}"
"actions=mod_dl_dst:{},{}output={}".format(
cookie, ovs_ofport, mac, net, port_dst_mac, strip_vlan_opt,
vrf_ofport))
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd(
'ovs-ofctl', ['add-flow', bridge, flow])

10
ovn_bgp_agent/drivers/openstack/utils/wire.py
View File

@ -12,8 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import pyroute2
from oslo_config import cfg
from oslo_log import log as logging
@ -55,13 +53,11 @@ def _ensure_base_wiring_config_underlay(idl, bridge_mappings, routing_tables):
bridge_index,
vlan_tags)
if not flows_info.get(bridge):
with pyroute2.NDB() as ndb:
flows_info[bridge] = {
'mac': ndb.interfaces[bridge]['address'],
'in_port': set([])}
mac = linux_net.get_interface_address(bridge)
flows_info[bridge] = {'mac': mac, 'in_port': set([])}
flows_info[bridge]['in_port'] = ovs.get_ovs_patch_ports_info(
bridge)
ovs.ensure_mac_tweak_flows(bridge, flows_info[bridge]['mac'],
ovs.ensure_mac_tweak_flows(bridge, mac,
flows_info[bridge]['in_port'],
constants.OVS_RULE_COOKIE)
return ovn_bridge_mappings, flows_info

8
ovn_bgp_agent/exceptions.py
View File

@ -75,3 +75,11 @@ class IpAddressAlreadyExists(RuntimeError):
def __init__(self, message=None, ip=None, device=None):
message = message or self.message % {'ip': ip, 'device': device}
super(IpAddressAlreadyExists, self).__init__(message)
class NetworkInterfaceNotFound(RuntimeError):
message = _("Network interface %(device)s not found")
def __init__(self, message=None, device=None):
message = message or self.message % {'device': device}
super(NetworkInterfaceNotFound, self).__init__(message)

30
ovn_bgp_agent/privileged/linux_net.py
View File

@ -31,23 +31,14 @@ import tenacity
from ovn_bgp_agent import constants
from ovn_bgp_agent import exceptions as agent_exc
from ovn_bgp_agent.utils import linux_net as l_net
import ovn_bgp_agent.privileged.linux_net
from ovn_bgp_agent.utils import linux_net as l_net
LOG = logging.getLogger(__name__)
_IP_VERSION_FAMILY_MAP = {4: socket.AF_INET, 6: socket.AF_INET6}
class NetworkInterfaceNotFound(RuntimeError):
message = 'Network interface %(device)s not found'
def __init__(self, message=None, device=None):
message = message or self.message % {'device': device}
super(NetworkInterfaceNotFound, self).__init__(message)
class InterfaceAlreadyExists(RuntimeError):
message = "Interface %(device)s already exists."
@ -90,7 +81,7 @@ def set_device_state(device, state):
def ensure_vrf(vrf_name, vrf_table):
try:
set_device_state(vrf_name, constants.LINK_UP)
except NetworkInterfaceNotFound:
except agent_exc.NetworkInterfaceNotFound:
create_interface(vrf_name, 'vrf', vrf_table=vrf_table,
state=constants.LINK_UP)
@ -99,7 +90,7 @@ def ensure_vrf(vrf_name, vrf_table):
def ensure_bridge(bridge_name):
try:
set_device_state(bridge_name, constants.LINK_UP)
except NetworkInterfaceNotFound:
except agent_exc.NetworkInterfaceNotFound:
create_interface(bridge_name, 'bridge', br_stp_state=0,
state=constants.LINK_UP)
@ -108,7 +99,7 @@ def ensure_bridge(bridge_name):
def ensure_vxlan(vxlan_name, vni, local_ip, dstport):
try:
set_device_state(vxlan_name, constants.LINK_UP)
except NetworkInterfaceNotFound:
except agent_exc.NetworkInterfaceNotFound:
# FIXME: Perhaps we need to set neigh_suppress on
create_interface(vxlan_name, 'vxlan',
vxlan_id=vni,
@ -122,7 +113,7 @@ def ensure_vxlan(vxlan_name, vni, local_ip, dstport):
def ensure_veth(veth_name, veth_peer):
try:
set_device_state(veth_name, constants.LINK_UP)
except NetworkInterfaceNotFound:
except agent_exc.NetworkInterfaceNotFound:
create_interface(veth_name, 'veth', peer=veth_peer,
state=constants.LINK_UP)
set_device_state(veth_peer, constants.LINK_UP)
@ -132,7 +123,7 @@ def ensure_veth(veth_name, veth_peer):
def ensure_dummy_device(device):
try:
set_device_state(device, constants.LINK_UP)
except NetworkInterfaceNotFound:
except agent_exc.NetworkInterfaceNotFound:
create_interface(device, 'dummy', state=constants.LINK_UP)
@ -141,7 +132,7 @@ def ensure_vlan_device_for_network(bridge, vlan_tag):
vlan_device_name = '{}.{}'.format(bridge, vlan_tag)
try:
set_device_state(vlan_device_name, constants.LINK_UP)
except NetworkInterfaceNotFound:
except agent_exc.NetworkInterfaceNotFound:
create_interface(vlan_device_name, 'vlan',
physical_interface=bridge,
vlan_id=vlan_tag,
@ -174,7 +165,7 @@ def set_master_for_device(device, master):
def delete_device(device):
try:
delete_interface(device)
except NetworkInterfaceNotFound:
except agent_exc.NetworkInterfaceNotFound:
LOG.debug("Interfaces %s already deleted.", device)
@ -364,7 +355,7 @@ def create_routing_table_for_bridge(table_number, bridge):
def _translate_ip_device_exception(e, device):
if e.code == errno.ENODEV:
raise NetworkInterfaceNotFound(device=device)
raise agent_exc.NetworkInterfaceNotFound(device=device)
if e.code == errno.EOPNOTSUPP:
raise InterfaceOperationNotSupported(device=device)
if e.code == errno.EINVAL:
@ -450,7 +441,7 @@ def _get_link_id(ifname, raise_exception=True):
link_id = ip.link_lookup(ifname=ifname)
if not link_id or len(link_id) < 1:
if raise_exception:
raise NetworkInterfaceNotFound(device=ifname)
raise agent_exc.NetworkInterfaceNotFound(device=ifname)
LOG.debug('Interface %(dev)s not found', {'dev': ifname})
return None
return link_id[0]
@ -540,6 +531,7 @@ def create_interface(ifname, kind, **kwargs):
_translate_ip_device_exception(e, ifname)
@ovn_bgp_agent.privileged.default.entrypoint
def delete_interface(ifname, **kwargs):
_run_iproute_link('del', ifname, **kwargs)

4
ovn_bgp_agent/tests/functional/privileged/test_linux_net.py
View File

@ -396,11 +396,11 @@ class IpAddressTestCase(_LinuxNetTestCase):
linux_net.delete_ip_address(ip_address, self.dev_name)
def test_add_ip_address_no_device(self):
self.assertRaises(linux_net.NetworkInterfaceNotFound,
self.assertRaises(agent_exc.NetworkInterfaceNotFound,
linux_net.add_ip_address, '240.0.0.1', self.dev_name)
def test_delete_ip_address_no_device(self):
self.assertRaises(linux_net.NetworkInterfaceNotFound,
self.assertRaises(agent_exc.NetworkInterfaceNotFound,
linux_net.delete_ip_address, '240.0.0.1',
self.dev_name)

0
ovn_bgp_agent/tests/functional/utils/__init__.py Normal file
View File

52
ovn_bgp_agent/tests/functional/utils/test_linux_net.py Normal file
View File

@ -0,0 +1,52 @@
# Copyright 2023 Red Hat, Inc.
#
# 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 oslo_utils import uuidutils
from ovn_bgp_agent import exceptions as agent_exc
from ovn_bgp_agent.privileged import linux_net as priv_linux_net
from ovn_bgp_agent.tests.functional import base as base_functional
from ovn_bgp_agent.tests.functional.privileged import test_linux_net as \
test_priv_linux_net
from ovn_bgp_agent.utils import linux_net
class GetInterfaceAddressTestCase(base_functional.BaseFunctionalTestCase):
def _delete_interfaces(self, dev_names):
for dev_name in dev_names:
try:
priv_linux_net.delete_interface(dev_name)
except Exception:
pass
def _get_device(self, device_name):
devices = test_priv_linux_net.get_devices_info()
for device in devices.values():
if device['name'] == device_name:
return device
def test_get_interface_address(self):
dev_names = list(map(lambda x: uuidutils.generate_uuid()[:15],
range(5)))
self.addCleanup(self._delete_interfaces, dev_names)
for dev_name in dev_names:
priv_linux_net.create_interface(dev_name, 'dummy')
device = self._get_device(dev_name)
mac = linux_net.get_interface_address(dev_name)
self.assertEqual(device['mac'], mac)
def test_get_interface_address_no_interface(self):
self.assertRaises(agent_exc.NetworkInterfaceNotFound,
linux_net.get_interface_address, 'no_interface_name')

14
ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py
View File

@ -64,10 +64,6 @@ class TestNBOVNBGPDriver(test_base.TestCase):
self.conf_ovsdb_connection = 'tcp:127.0.0.1:6642'
# Mock pyroute2.NDB context manager object
self.mock_ndb = mock.patch.object(linux_net.pyroute2, 'NDB').start()
self.fake_ndb = self.mock_ndb().__enter__()
@mock.patch.object(linux_net, 'ensure_vrf')
@mock.patch.object(frr, 'vrf_leak')
@mock.patch.object(linux_net, 'ensure_ovn_device')
@ -123,14 +119,15 @@ class TestNBOVNBGPDriver(test_base.TestCase):
@mock.patch.object(ovs, 'remove_extra_ovs_flows')
@mock.patch.object(ovs, 'ensure_mac_tweak_flows')
@mock.patch.object(ovs, 'get_ovs_patch_ports_info')
@mock.patch.object(linux_net, 'get_interface_address')
@mock.patch.object(linux_net, 'ensure_arp_ndp_enabled_for_bridge')
@mock.patch.object(linux_net, 'ensure_vlan_device_for_network')
@mock.patch.object(linux_net, 'ensure_routing_table_for_bridge')
def test_sync(self, mock_routing_bridge, mock_ensure_vlan_network,
mock_ensure_arp, 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_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):
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 = (
@ -151,6 +148,7 @@ class TestNBOVNBGPDriver(test_base.TestCase):
mock_ensure_port_exposed = mock.patch.object(
self.nb_bgp_driver, '_ensure_port_exposed').start()
mock_routing_bridge.return_value = ['fake-route']
mock_nic_address.return_value = self.mac
mock_get_patch_ports.return_value = [1, 2]
self.nb_bgp_driver.sync()

13
ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_bgp_driver.py
View File

@ -75,10 +75,6 @@ class TestOVNBGPDriver(test_base.TestCase):
self.loadbalancer_vip_port: {'ips': [self.ipv4, self.ipv6],
'gateway_port': self.cr_lrp0}}
# Mock pyroute2.NDB context manager object
self.mock_ndb = mock.patch.object(linux_net.pyroute2, 'NDB').start()
self.fake_ndb = self.mock_ndb().__enter__()
@mock.patch.object(linux_net, 'ensure_ovn_device')
@mock.patch.object(linux_net, 'ensure_vrf')
@mock.patch.object(frr, 'vrf_leak')
@ -116,14 +112,16 @@ class TestOVNBGPDriver(test_base.TestCase):
@mock.patch.object(ovs, 'get_ovs_patch_ports_info')
@mock.patch.object(linux_net, 'get_ovn_ip_rules')
@mock.patch.object(linux_net, 'get_exposed_ips')
@mock.patch.object(linux_net, 'get_interface_address')
@mock.patch.object(linux_net, 'ensure_vlan_device_for_network')
@mock.patch.object(linux_net, 'ensure_routing_table_for_bridge')
@mock.patch.object(linux_net, 'ensure_arp_ndp_enabled_for_bridge')
def test_sync(
self, mock_ensure_arp, mock_routing_bridge,
mock_ensure_vlan_network, 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_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):
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 = (
@ -142,6 +140,7 @@ class TestOVNBGPDriver(test_base.TestCase):
mock_ensure_cr_port_exposed = mock.patch.object(
self.bgp_driver, '_ensure_cr_lrp_associated_ports_exposed').start()
mock_routing_bridge.return_value = ['fake-route']
mock_nic_address.return_value = self.mac
mock_get_patch_ports.return_value = [1, 2]
self.bgp_driver.sync()

10
ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovs.py
View File

@ -36,9 +36,6 @@ class TestOVS(test_base.TestCase):
self.cookie = 'fake-cookie'
self.cookie_id = 'cookie=%s/-1' % self.cookie
self.mac = 'aa:bb:cc:dd:ee:ff'
self.fake_ndb = mock.Mock(interfaces={})
mock_ndb = mock.patch('pyroute2.NDB').start()
mock_ndb.return_value.__enter__.return_value = self.fake_ndb
def _test_get_bridge_flows(self, has_filter=False):
port_iface = '1'
@ -104,14 +101,15 @@ class TestOVS(test_base.TestCase):
mock_flows.assert_called_once_with(self.bridge, self.cookie_id)
@mock.patch.object(ovs_utils, 'get_device_port_at_ovs')
@mock.patch.object(linux_net, 'get_interface_address')
@mock.patch.object(linux_net, 'get_ip_version')
def _test_ensure_evpn_ovs_flow(self, mock_ip_version, mock_ofport,
ip_version, strip_vlan=False):
def _test_ensure_evpn_ovs_flow(self, mock_ip_version, mock_nic_address,
mock_ofport, ip_version, strip_vlan=False):
address = '00:00:00:00:00:00'
mock_ip_version.return_value = ip_version
mock_nic_address.return_value = address
port = 'fake-port'
port_dst = 'fake-port-dst'
self.fake_ndb.interfaces[port_dst] = {'address': address}
ovs_port = constants.OVS_PATCH_PROVNET_PORT_PREFIX + 'fake-port'
port_iface = '1'
ovs_port_iface = '2'

20
ovn_bgp_agent/tests/unit/utils/test_linux_net.py
View File

@ -33,6 +33,11 @@ class TestLinuxNet(test_base.TestCase):
self.mock_ndb = mock.patch.object(linux_net.pyroute2, 'NDB').start()
self.fake_ndb = self.mock_ndb().__enter__()
# Mock pyroute2.IPRoute context manager object
self.mock_ipr = mock.patch.object(linux_net.pyroute2,
'IPRoute').start()
self.fake_ipr = self.mock_ipr().__enter__()
# Helper variables used accross many tests
self.ip = '10.10.1.16'
self.ipv6 = '2002::1234:abcd:ffff:c0a8:101'
@ -65,6 +70,21 @@ class TestLinuxNet(test_base.TestCase):
ret = linux_net.get_interface_index('fake-nic')
self.assertEqual(7, ret)
def test_get_interface_address(self):
device_idx = 7
self.fake_ipr.link_lookup.return_value = [device_idx]
fake_link = mock.MagicMock()
fake_link.get_attr.return_value = self.mac
self.fake_ipr.get_links.return_value = [fake_link]
ret = linux_net.get_interface_address('fake-nic')
self.assertEqual(self.mac, ret)
def test_get_interface_address_index_error(self):
self.fake_ipr.link_lookup.return_value = ''
self.assertRaises(agent_exc.NetworkInterfaceNotFound,
linux_net.get_interface_address, 'fake-nic')
@mock.patch('ovn_bgp_agent.privileged.linux_net.ensure_vrf')
def test_ensure_vrf(self, mock_ensure_vrf):
linux_net.ensure_vrf('fake-vrf', 10)

16
ovn_bgp_agent/utils/linux_net.py
View File

@ -59,6 +59,22 @@ def get_interface_index(nic):
return ndb.interfaces[nic]['index']
@tenacity.retry(
retry=tenacity.retry_if_exception_type(
netlink_exceptions.NetlinkDumpInterrupted),
wait=tenacity.wait_exponential(multiplier=0.02, max=1),
stop=tenacity.stop_after_delay(8),
reraise=True)
def get_interface_address(nic):
try:
with pyroute2.IPRoute() as ipr:
idx = ipr.link_lookup(ifname=nic)[0]
mac = ipr.get_links(idx)[0].get_attr('IFLA_ADDRESS')
return mac
except IndexError:
raise agent_exc.NetworkInterfaceNotFound(device=nic)
def ensure_vrf(vrf_name, vrf_table):
ovn_bgp_agent.privileged.linux_net.ensure_vrf(vrf_name, vrf_table)