From 7115d6bf93a9c294b4903f7cf080fca5a811b862 Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Mon, 12 Jun 2023 10:17:59 +0200 Subject: [PATCH] Add new privileged rule methods implementations This patch add the following new privileged methods implementations: * rule_create * rule_delete These new method use pyroute2 IPRoute class, replacing the use of the NDB class. Partial-Bug: #2022357 Change-Id: I7cb004b7523d957b3849621a0387288e758b8520 --- ovn_bgp_agent/privileged/linux_net.py | 77 ++++++++----------- .../functional/privileged/test_linux_net.py | 40 ++++++++++ .../tests/unit/privileged/test_linux_net.py | 25 ------ 3 files changed, 74 insertions(+), 68 deletions(-) diff --git a/ovn_bgp_agent/privileged/linux_net.py b/ovn_bgp_agent/privileged/linux_net.py index f495a869..745bf9e9 100644 --- a/ovn_bgp_agent/privileged/linux_net.py +++ b/ovn_bgp_agent/privileged/linux_net.py @@ -216,57 +216,22 @@ def delete_exposed_ips(ips, nic): @ovn_bgp_agent.privileged.default.entrypoint def rule_create(rule): - with pyroute2.NDB() as ndb: - try: - ndb.rules[rule] - except KeyError: - LOG.debug("Creating ip rule with: %s", rule) - try: - ndb.rules.create(rule).commit() - except ValueError: - # FIXME: There is an issue with NDB and ip rules - # Remove try/except once the next is fixed: - # https://github.com/svinota/pyroute2/issues/967 - pass + _run_iproute_rule('add', **rule) @ovn_bgp_agent.privileged.default.entrypoint def rule_delete(rule): - with pyroute2.NDB() as ndb: - try: - ndb.rules[rule].remove().commit() - LOG.debug("Deleting ip rule with: %s", rule) - except KeyError: - LOG.debug("Rule already deleted: %s", rule) - except ValueError: - # FIXME: There is an issue with NDB and ip rules - # Remove except once the next is fixed: - # https://github.com/svinota/pyroute2/issues/967 - # fixed on pyroute2 0.7.2 version, remove it when that version - # is the minimal one supported - pass + _run_iproute_rule('del', **rule) @ovn_bgp_agent.privileged.default.entrypoint def delete_ip_rules(ip_rules): - with pyroute2.NDB() as ndb: - for rule_ip, rule_info in ip_rules.items(): - rule = {'dst': rule_ip.split("/")[0], - 'dst_len': rule_ip.split("/")[1], - 'table': rule_info['table'], - 'family': rule_info['family']} - try: - with ndb.rules[rule] as r: - r.remove() - except KeyError: - LOG.debug("Rule {} already deleted".format(rule)) - except pyroute_netlink.exceptions.NetlinkError: - # FIXME: There is a issue with NDB and ip rules deletion: - # https://github.com/svinota/pyroute2/issues/771 - # fixed on pyroute2 0.7.2 version, remove it when that version - # is the minimal one supported - LOG.debug("This should not happen, skipping: NetlinkError " - "deleting rule %s", rule) + for rule_ip, rule_info in ip_rules.items(): + rule = {'dst': rule_ip.split("/")[0], + 'dst_len': int(rule_ip.split("/")[1]), + 'table': int(rule_info['table']), + 'family': rule_info['family']} + _run_iproute_rule('del', **rule) @ovn_bgp_agent.privileged.default.entrypoint @@ -429,6 +394,16 @@ def _translate_ip_route_exception(e, kwargs): raise e +def _translate_ip_rule_exception(e, kwargs): + if e.code == errno.EEXIST: # Already exists + LOG.debug("Rule %s already exists.", kwargs) + return + if e.code == errno.ENOENT: # Not found + LOG.debug("Rule already deleted: %s", kwargs) + return + raise e + + def get_attr(pyroute2_obj, attr_name): """Get an attribute in a pyroute object @@ -543,6 +518,14 @@ def _run_iproute_route(command, **kwargs): _translate_ip_route_exception(e, kwargs) +def _run_iproute_rule(command, **kwargs): + try: + with iproute.IPRoute() as ip: + ip.rule(command, **kwargs) + except netlink_exceptions.NetlinkError as e: + _translate_ip_rule_exception(e, kwargs) + + @ovn_bgp_agent.privileged.default.entrypoint def create_interface(ifname, kind, **kwargs): ifname = ifname[:15] @@ -620,3 +603,11 @@ def list_ip_routes(ip_version, device=None, table=None, **kwargs): kwargs['table'] = int(table) with iproute.IPRoute() as ip: return make_serializable(ip.route('show', **kwargs)) + + +@ovn_bgp_agent.privileged.default.entrypoint +def list_ip_rules(ip_version, **kwargs): + """List all IP rules""" + with iproute.IPRoute() as ip: + return make_serializable(ip.get_rules( + family=_IP_VERSION_FAMILY_MAP[ip_version], **kwargs)) 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 bcdb32de..dc3f0486 100644 --- a/ovn_bgp_agent/tests/functional/privileged/test_linux_net.py +++ b/ovn_bgp_agent/tests/functional/privileged/test_linux_net.py @@ -14,6 +14,7 @@ import functools import random +import socket import netaddr from neutron_lib import constants as n_const @@ -36,6 +37,7 @@ IP_ADDRESS_SCOPE = {rtnl.rtscopes['RT_SCOPE_UNIVERSE']: 'global', rtnl.rtscopes['RT_SCOPE_SITE']: 'site', rtnl.rtscopes['RT_SCOPE_LINK']: 'link', rtnl.rtscopes['RT_SCOPE_HOST']: 'host'} +_IP_VERSION_FAMILY_MAP = {4: socket.AF_INET, 6: socket.AF_INET6} def set_up(ifname): @@ -523,3 +525,41 @@ class IpRouteTestCase(_LinuxNetTestCase): self.assertEqual(rtnl.rtypes['RTN_UNREACHABLE'], routes[0]['type']) self.assertEqual(4278198272, linux_net.get_attr(routes[0], 'RTA_PRIORITY')) + + +class IpRuleTestCase(_LinuxNetTestCase): + + def _test_add_and_delete_ip_rule(self, ip_version, cidrs): + table = random.randint(10, 250) + rules_added = [] + for cidr in cidrs: + _ip = netaddr.IPNetwork(cidr) + rule = {'dst': str(_ip.ip), + 'dst_len': _ip.netmask.netmask_bits(), + 'table': table, + 'family': _IP_VERSION_FAMILY_MAP[ip_version]} + rules_added.append(rule) + linux_net.rule_create(rule) + # recreate the last rule, to ensure recreation does not fail + linux_net.rule_create(rule) + rules = linux_net.list_ip_rules(ip_version, table=table) + self.assertEqual(len(cidrs), len(rules)) + for idx, rule in enumerate(rules): + _ip = netaddr.IPNetwork(cidrs[idx]) + self.assertEqual(str(_ip.ip), linux_net.get_attr(rule, 'FRA_DST')) + self.assertEqual(_ip.netmask.netmask_bits(), rule['dst_len']) + + for rule in rules_added: + linux_net.rule_delete(rule) + # remove again the last rule to ensure it does not fail + linux_net.rule_delete(rule) + rules = linux_net.list_ip_rules(ip_version, table=table) + self.assertEqual(0, len(rules)) + + def test_add_and_delete_ip_rule_v4(self): + cidrs = ['192.168.0.0/24', '172.90.0.0/16', '10.0.0.0/8'] + self._test_add_and_delete_ip_rule(n_const.IP_VERSION_4, cidrs) + + def test_add_and_delete_ip_rule_v6(self): + cidrs = ['2001:db8::/64', 'fe80::/10'] + self._test_add_and_delete_ip_rule(n_const.IP_VERSION_6, cidrs) diff --git a/ovn_bgp_agent/tests/unit/privileged/test_linux_net.py b/ovn_bgp_agent/tests/unit/privileged/test_linux_net.py index e9e2a249..fedfddf5 100644 --- a/ovn_bgp_agent/tests/unit/privileged/test_linux_net.py +++ b/ovn_bgp_agent/tests/unit/privileged/test_linux_net.py @@ -47,31 +47,6 @@ class TestPrivilegedLinuxNet(test_base.TestCase): self.dev = 'ethfake' self.mac = 'aa:bb:cc:dd:ee:ff' - def test_rule_create(self): - fake_rule = mock.MagicMock() - self.fake_ndb.rules.__getitem__.side_effect = KeyError - priv_linux_net.rule_create(fake_rule) - self.fake_ndb.rules.create.assert_called_once_with(fake_rule) - - def test_rule_create_existing(self): - fake_rule = mock.MagicMock() - self.fake_ndb.rules.__getitem__.return_value = fake_rule - priv_linux_net.rule_create(fake_rule) - self.fake_ndb.rules.create.assert_not_called() - - def test_rule_delete(self): - fake_rule = mock.MagicMock() - rules_dict = {'fake-rule': fake_rule} - self.fake_ndb.rules = rules_dict - priv_linux_net.rule_delete('fake-rule') - fake_rule.remove.assert_called_once() - - def test_rule_delete_keyerror(self): - fake_rule = mock.MagicMock() - self.fake_ndb.rules.__getitem__.side_effect = KeyError - priv_linux_net.rule_delete(fake_rule) - fake_rule.__enter__().remove.assert_not_called() - def test_set_kernel_flag(self): priv_linux_net.set_kernel_flag('net.ipv6.conf.fake', 1) self.mock_exc.assert_called_once_with(