From 1902d71c5080b37fb4b2c2270ed0bf20280177bc Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Thu, 11 Jan 2024 20:43:44 +0000 Subject: [PATCH] Consolidate use of IP_VERSION_TO_FAMILY conversion The constant mapping has been defined in several places. This patch creates a new module utils.common that defines the mapping and uses it everywhere. It also encapsulates socket family constants in the constants module and reuses the constants. Change-Id: I754d4c06007db7d68ceaa64e76f3ef2305d7850c --- ovn_bgp_agent/constants.py | 7 ++++ ovn_bgp_agent/privileged/linux_net.py | 23 +++++------ .../functional/privileged/test_linux_net.py | 11 +++-- .../tests/functional/utils/test_linux_net.py | 10 ++--- .../tests/unit/utils/test_linux_net.py | 18 ++++----- ovn_bgp_agent/utils/common.py | 17 ++++++++ ovn_bgp_agent/utils/linux_net.py | 40 +++++++++---------- 7 files changed, 72 insertions(+), 54 deletions(-) create mode 100644 ovn_bgp_agent/utils/common.py diff --git a/ovn_bgp_agent/constants.py b/ovn_bgp_agent/constants.py index 559f36a8..4d6b4c7c 100644 --- a/ovn_bgp_agent/constants.py +++ b/ovn_bgp_agent/constants.py @@ -12,6 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import socket + + OVN_VIF_PORT_TYPES = ("", "chassisredirect", "virtual") OVN_VIRTUAL_VIF_PORT_TYPE = "virtual" @@ -100,3 +103,7 @@ POLICY_ACTION_TYPES = (POLICY_ACTION_REROUTE) LR_POLICY_PRIORITY_MAX = 32767 ROUTE_DISCARD = 'discard' + +# Family constants +AF_INET = socket.AF_INET +AF_INET6 = socket.AF_INET6 diff --git a/ovn_bgp_agent/privileged/linux_net.py b/ovn_bgp_agent/privileged/linux_net.py index c7bc5ec1..8c3ce466 100644 --- a/ovn_bgp_agent/privileged/linux_net.py +++ b/ovn_bgp_agent/privileged/linux_net.py @@ -15,7 +15,6 @@ import errno import ipaddress import os -import socket import netaddr @@ -32,11 +31,11 @@ import tenacity import ovn_bgp_agent from ovn_bgp_agent import constants from ovn_bgp_agent import exceptions as agent_exc +from ovn_bgp_agent.utils import common as common_utils 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} NUD_STATES = {state[1]: state[0] for state in ndmsg.states.items()} @@ -151,7 +150,7 @@ def route_create(route): scope = route.pop('scope', 'link') route['scope'] = get_scope_name(scope) if 'family' not in route: - route['family'] = socket.AF_INET + route['family'] = constants.AF_INET _run_iproute_route('replace', **route) @@ -160,7 +159,7 @@ def route_delete(route): scope = route.pop('scope', 'link') route['scope'] = get_scope_name(scope) if 'family' not in route: - route['family'] = socket.AF_INET + route['family'] = constants.AF_INET _run_iproute_route('del', **route) @@ -250,7 +249,7 @@ def del_ip_from_dev(ip, nic): @ovn_bgp_agent.privileged.default.entrypoint def add_ip_nei(ip, lladdr, dev): ip_version = l_net.get_ip_version(ip) - family = _IP_VERSION_FAMILY_MAP[ip_version] + family = common_utils.IP_VERSION_FAMILY_MAP[ip_version] _run_iproute_neigh('replace', dev, dst=ip, @@ -262,7 +261,7 @@ def add_ip_nei(ip, lladdr, dev): @ovn_bgp_agent.privileged.default.entrypoint def del_ip_nei(ip, lladdr, dev): ip_version = l_net.get_ip_version(ip) - family = _IP_VERSION_FAMILY_MAP[ip_version] + family = common_utils.IP_VERSION_FAMILY_MAP[ip_version] _run_iproute_neigh('del', dev, dst=ip.split("/")[0], @@ -288,7 +287,7 @@ def get_neigh_entries(device, ip_version, **kwargs): 'lladdr': mac_address, 'device': device_name} """ - family = _IP_VERSION_FAMILY_MAP[ip_version] + family = common_utils.IP_VERSION_FAMILY_MAP[ip_version] dump = _run_iproute_neigh('dump', device, family=family, @@ -312,7 +311,7 @@ def add_unreachable_route(vrf_name): ifla_linkinfo = get_attr(device, 'IFLA_LINKINFO') ifla_data = get_attr(ifla_linkinfo, 'IFLA_INFO_DATA') vrf_table = get_attr(ifla_data, 'IFLA_VRF_TABLE') - for ip_version in (socket.AF_INET, socket.AF_INET6): + for ip_version in common_utils.IP_VERSION_FAMILY_MAP.values(): kwargs = {'dst': 'default', 'family': ip_version, 'table': vrf_table, @@ -551,7 +550,7 @@ def add_ip_address(ip_address, ifname): ip_version = l_net.get_ip_version(ip_address) address = str(net.ip) prefixlen = 32 if ip_version == 4 else 128 - family = _IP_VERSION_FAMILY_MAP[ip_version] + family = common_utils.IP_VERSION_FAMILY_MAP[ip_version] _run_iproute_addr('add', ifname, address=address, @@ -565,7 +564,7 @@ def delete_ip_address(ip_address, ifname): ip_version = l_net.get_ip_version(ip_address) address = str(net.ip) prefixlen = 32 if ip_version == 4 else 128 - family = _IP_VERSION_FAMILY_MAP[ip_version] + family = common_utils.IP_VERSION_FAMILY_MAP[ip_version] _run_iproute_addr("delete", ifname, address=address, @@ -592,7 +591,7 @@ def get_ip_addresses(**kwargs): @ovn_bgp_agent.privileged.default.entrypoint def list_ip_routes(ip_version, device=None, table=None, **kwargs): """List IP routes""" - kwargs['family'] = _IP_VERSION_FAMILY_MAP[ip_version] + kwargs['family'] = common_utils.IP_VERSION_FAMILY_MAP[ip_version] if device: kwargs['oif'] = _get_link_id(device) if table: @@ -606,4 +605,4 @@ 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)) + family=common_utils.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 3ce8811b..4c623db4 100644 --- a/ovn_bgp_agent/tests/functional/privileged/test_linux_net.py +++ b/ovn_bgp_agent/tests/functional/privileged/test_linux_net.py @@ -14,7 +14,6 @@ import functools import random -import socket import netaddr from neutron_lib import constants as n_const @@ -28,6 +27,7 @@ from ovn_bgp_agent import exceptions as agent_exc from ovn_bgp_agent.privileged import linux_net from ovn_bgp_agent.tests.functional import base as base_functional from ovn_bgp_agent.tests import utils as test_utils +from ovn_bgp_agent.utils import common as common_utils from ovn_bgp_agent.utils import linux_net as l_net @@ -37,7 +37,6 @@ 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): @@ -447,7 +446,7 @@ class IpRouteTestCase(_LinuxNetTestCase): continue self.assertEqual(table, route['table']) self.assertEqual( - linux_net._IP_VERSION_FAMILY_MAP[ip_version], + common_utils.IP_VERSION_FAMILY_MAP[ip_version], route['family']) ret_scope = linux_net.get_scope_name(route['scope']) self.assertEqual(scope, ret_scope) @@ -465,7 +464,7 @@ class IpRouteTestCase(_LinuxNetTestCase): proto='static'): for cidr in cidrs: ip_version = l_net.get_ip_version(cidr) - family = linux_net._IP_VERSION_FAMILY_MAP[ip_version] + family = common_utils.IP_VERSION_FAMILY_MAP[ip_version] route = {'dst': cidr, 'oif': self.device['index'], 'table': table, @@ -479,7 +478,7 @@ class IpRouteTestCase(_LinuxNetTestCase): proto=proto) for cidr in cidrs: ip_version = l_net.get_ip_version(cidr) - family = linux_net._IP_VERSION_FAMILY_MAP[ip_version] + family = common_utils.IP_VERSION_FAMILY_MAP[ip_version] route = {'dst': cidr, 'oif': self.device['index'], 'table': table, @@ -547,7 +546,7 @@ class IpRuleTestCase(_LinuxNetTestCase): rule = {'dst': str(_ip.ip), 'dst_len': _ip.netmask.netmask_bits(), 'table': table, - 'family': _IP_VERSION_FAMILY_MAP[ip_version]} + 'family': common_utils.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 diff --git a/ovn_bgp_agent/tests/functional/utils/test_linux_net.py b/ovn_bgp_agent/tests/functional/utils/test_linux_net.py index 034e474e..cfda59f8 100644 --- a/ovn_bgp_agent/tests/functional/utils/test_linux_net.py +++ b/ovn_bgp_agent/tests/functional/utils/test_linux_net.py @@ -12,8 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import socket - import netaddr from neutron_lib.utils import net as net_utils from oslo_utils import uuidutils @@ -23,12 +21,10 @@ 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 common as common_utils from ovn_bgp_agent.utils import linux_net -_IP_VERSION_FAMILY_MAP = {4: socket.AF_INET, 6: socket.AF_INET6} - - class GetInterfaceTestCase(base_functional.BaseFunctionalTestCase): def _delete_interfaces(self, dev_names): @@ -139,12 +135,12 @@ class GetRulesTestCase(base_functional.BaseFunctionalTestCase): rule = {'dst': str(_ip.ip), 'dst_len': _ip.netmask.netmask_bits(), 'table': table, - 'family': _IP_VERSION_FAMILY_MAP[ip_version]} + 'family': common_utils.IP_VERSION_FAMILY_MAP[ip_version]} dst = "{}/{}".format(str(_ip.ip), _ip.netmask.netmask_bits()) rules_added.append(rule) expected_rules[dst] = { 'table': table, - 'family': _IP_VERSION_FAMILY_MAP[ip_version]} + 'family': common_utils.IP_VERSION_FAMILY_MAP[ip_version]} self.addCleanup(self._delete_rules, rules_added) for rule in rules_added: priv_linux_net.rule_create(rule) diff --git a/ovn_bgp_agent/tests/unit/utils/test_linux_net.py b/ovn_bgp_agent/tests/unit/utils/test_linux_net.py index 6a7f7522..5357d54e 100644 --- a/ovn_bgp_agent/tests/unit/utils/test_linux_net.py +++ b/ovn_bgp_agent/tests/unit/utils/test_linux_net.py @@ -15,11 +15,10 @@ import copy import ipaddress -from socket import AF_INET -from socket import AF_INET6 from unittest import mock +from ovn_bgp_agent import constants from ovn_bgp_agent import exceptions as agent_exc from ovn_bgp_agent.tests import base as test_base from ovn_bgp_agent.utils import linux_net @@ -381,13 +380,13 @@ class TestLinuxNet(test_base.TestCase): routing_tables_routes = {self.bridge: [route]} # extra_route0 matches with the route extra_route0 = IPRouteDict({ - 'dst_len': 32, 'family': AF_INET, 'table': 20, + 'dst_len': 32, 'family': constants.AF_INET, 'table': 20, 'attrs': [('RTA_DST', self.ip), ('RTA_OIF', oif), ('RTA_GATEWAY', gateway)]}) # extra_route1 does not match with route and should be removed extra_route1 = IPRouteDict({ - 'dst_len': 32, 'family': AF_INET, 'table': 20, + 'dst_len': 32, 'family': constants.AF_INET, 'table': 20, 'attrs': [('RTA_DST', '10.10.1.17'), ('RTA_OIF', oif), ('RTA_GATEWAY', gateway)]}) @@ -398,7 +397,7 @@ class TestLinuxNet(test_base.TestCase): # Assert extra_route1 has been removed expected_route = {'dst': '10.10.1.17', 'dst_len': 32, - 'family': AF_INET, 'oif': oif, + 'family': constants.AF_INET, 'oif': oif, 'gateway': gateway, 'table': 20} mock_route_delete.assert_called_once_with(expected_route) @@ -525,7 +524,8 @@ class TestLinuxNet(test_base.TestCase): linux_net.add_ip_rule(self.ipv6, 7, dev=self.dev, lladdr=self.mac) expected_args = {'dst': self.ipv6, - 'table': 7, 'dst_len': 128, 'family': AF_INET6} + 'table': 7, 'dst_len': 128, + 'family': constants.AF_INET6} mock_rule_create.assert_called_once_with(expected_args) mock_add_ip_nei.assert_called_once_with(self.ipv6, self.mac, self.dev) @@ -555,7 +555,7 @@ class TestLinuxNet(test_base.TestCase): linux_net.del_ip_rule(self.ipv6, 7, dev=self.dev, lladdr=self.mac) expected_args = {'dst': self.ipv6, 'table': 7, - 'dst_len': 128, 'family': AF_INET6} + 'dst_len': 128, 'family': constants.AF_INET6} mock_rule_delete.assert_called_once_with(expected_args) mock_del_ip_nei.assert_called_once_with(self.ipv6, self.mac, self.dev) @@ -600,7 +600,7 @@ class TestLinuxNet(test_base.TestCase): expected_routes = { self.dev: [{'route': {'dst': self.ipv6, 'dst_len': 128, - 'family': AF_INET6, + 'family': constants.AF_INET6, 'oif': mock.ANY, 'proto': 3, 'table': 7}, @@ -715,7 +715,7 @@ class TestLinuxNet(test_base.TestCase): routes = { self.dev: [{'route': {'dst': self.ipv6, 'dst_len': 128, - 'family': AF_INET6, + 'family': constants.AF_INET6, 'oif': mock.ANY, 'proto': 3, 'table': 7}, diff --git a/ovn_bgp_agent/utils/common.py b/ovn_bgp_agent/utils/common.py new file mode 100644 index 00000000..0be263fe --- /dev/null +++ b/ovn_bgp_agent/utils/common.py @@ -0,0 +1,17 @@ +# Copyright 2024 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 ovn_bgp_agent import constants + +IP_VERSION_FAMILY_MAP = {4: constants.AF_INET, 6: constants.AF_INET6} diff --git a/ovn_bgp_agent/utils/linux_net.py b/ovn_bgp_agent/utils/linux_net.py index ff55be1b..b925d0be 100644 --- a/ovn_bgp_agent/utils/linux_net.py +++ b/ovn_bgp_agent/utils/linux_net.py @@ -17,9 +17,6 @@ import random import re import sys -from socket import AF_INET -from socket import AF_INET6 - from oslo_log import log as logging import pyroute2 from pyroute2.netlink import exceptions as netlink_exceptions @@ -222,7 +219,8 @@ def _ensure_routing_table_routes(ovn_routing_tables, bridge): ovn_bgp_agent.privileged.linux_net.route_create(r1) r2 = {'dst': 'default', 'oif': bridge_idx, - 'table': ovn_routing_tables[bridge], 'family': AF_INET6, + 'table': ovn_routing_tables[bridge], + 'family': constants.AF_INET6, 'proto': 3} ovn_bgp_agent.privileged.linux_net.route_create(r2) else: @@ -234,7 +232,7 @@ def _ensure_routing_table_routes(ovn_routing_tables, bridge): route = [ r for r in ip.get_routes( table=ovn_routing_tables[bridge], - family=AF_INET) + family=constants.AF_INET) if not r.get_attr('RTA_DST')][0] if bridge_idx == route.get_attr('RTA_OIF'): route_missing = False @@ -246,7 +244,7 @@ def _ensure_routing_table_routes(ovn_routing_tables, bridge): route_6 = [ r for r in ip.get_routes( table=ovn_routing_tables[bridge], - family=AF_INET6) + family=constants.AF_INET6) if not r.get_attr('RTA_DST')][0] if bridge_idx == route_6.get_attr('RTA_OIF'): route6_missing = False @@ -261,14 +259,14 @@ def _ensure_routing_table_routes(ovn_routing_tables, bridge): table=ovn_routing_tables[bridge], dst=dst, dst_len=dst_len, - family=AF_INET6)[0]) + family=constants.AF_INET6)[0]) else: extra_routes.append( ip.get_routes( table=ovn_routing_tables[bridge], dst=dst, dst_len=dst_len, - family=AF_INET)[0]) + family=constants.AF_INET)[0]) if route_missing: r = {'dst': 'default', 'oif': bridge_idx, @@ -277,7 +275,8 @@ def _ensure_routing_table_routes(ovn_routing_tables, bridge): ovn_bgp_agent.privileged.linux_net.route_create(r) if route6_missing: r = {'dst': 'default', 'oif': bridge_idx, - 'table': ovn_routing_tables[bridge], 'family': AF_INET6, + 'table': ovn_routing_tables[bridge], + 'family': constants.AF_INET6, 'proto': 3} ovn_bgp_agent.privileged.linux_net.route_create(r) return extra_routes @@ -307,7 +306,7 @@ def get_extra_routing_table_for_bridge(ovn_routing_tables, bridge): route = [ r for r in ip.get_routes( table=ovn_routing_tables[bridge], - family=AF_INET) + family=constants.AF_INET) if not r.get_attr('RTA_DST')][0] if bridge_idx != route.get_attr('RTA_OIF'): extra_routes.append(route) @@ -317,7 +316,7 @@ def get_extra_routing_table_for_bridge(ovn_routing_tables, bridge): route_6 = [ r for r in ip.get_routes( table=ovn_routing_tables[bridge], - family=AF_INET6) + family=constants.AF_INET6) if not r.get_attr('RTA_DST')][0] if bridge_idx != route_6.get_attr('RTA_OIF'): extra_routes.append(route_6) @@ -330,14 +329,14 @@ def get_extra_routing_table_for_bridge(ovn_routing_tables, bridge): table=ovn_routing_tables[bridge], dst=dst, dst_len=dst_len, - family=AF_INET6)[0]) + family=constants.AF_INET6)[0]) else: extra_routes.append( ip.get_routes( table=ovn_routing_tables[bridge], dst=dst, dst_len=dst_len, - family=AF_INET)[0]) + family=constants.AF_INET)[0]) return extra_routes @@ -449,7 +448,8 @@ def get_ovn_ip_rules(routing_tables): "{}/{}".format(rule.get_attr('FRA_DST'), rule['dst_len']), rule['family']) for rule in ( - ipr.get_rules(family=AF_INET) + ipr.get_rules(family=AF_INET6)) + ipr.get_rules(family=constants.AF_INET) + + ipr.get_rules(family=constants.AF_INET6)) if rule.get_attr('FRA_TABLE') in routing_tables ] for table, dst, family in rules_info: @@ -594,11 +594,11 @@ def add_ip_rule(ip, table, dev=None, lladdr=None): rule = {'dst': ip_info[0], 'table': table, 'dst_len': 32} if ip_version == constants.IP_VERSION_6: rule['dst_len'] = 128 - rule['family'] = AF_INET6 + rule['family'] = constants.AF_INET6 elif len(ip_info) == 2: rule = {'dst': ip_info[0], 'table': table, 'dst_len': int(ip_info[1])} if ip_version == constants.IP_VERSION_6: - rule['family'] = AF_INET6 + rule['family'] = constants.AF_INET6 else: raise agent_exc.InvalidPortIP(ip=ip) @@ -626,11 +626,11 @@ def del_ip_rule(ip, table, dev=None, lladdr=None): rule = {'dst': ip_info[0], 'table': table, 'dst_len': 32} if ip_version == constants.IP_VERSION_6: rule['dst_len'] = 128 - rule['family'] = AF_INET6 + rule['family'] = constants.AF_INET6 elif len(ip_info) == 2: rule = {'dst': ip_info[0], 'table': table, 'dst_len': int(ip_info[1])} if ip_version == constants.IP_VERSION_6: - rule['family'] = AF_INET6 + rule['family'] = constants.AF_INET6 else: raise agent_exc.InvalidPortIP(ip=ip) @@ -699,7 +699,7 @@ def add_ip_route(ovn_routing_tables_routes, ip_address, route_table, dev, else: route['scope'] = 253 if get_ip_version(net_ip) == constants.IP_VERSION_6: - route['family'] = AF_INET6 + route['family'] = constants.AF_INET6 del route['scope'] with pyroute2.IPRoute() as ipr: @@ -750,7 +750,7 @@ def del_ip_route(ovn_routing_tables_routes, ip_address, route_table, dev, else: route['scope'] = 253 if get_ip_version(net_ip) == constants.IP_VERSION_6: - route['family'] = AF_INET6 + route['family'] = constants.AF_INET6 del route['scope'] LOG.debug("Deleting route at table %s: %s", route_table, route)