openstack/ovn-bgp-agent
Code Issues Proposed changes

Consolidate use of IP_VERSION_TO_FAMILY conversion

Browse Source 
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
This commit is contained in:
Jakub Libosvar 2024-01-11 20:43:44 +00:00
parent 9852a8069f
commit 1902d71c50
7 changed files with 72 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
ovn_bgp_agent/constants.py
View File

@ -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

23
ovn_bgp_agent/privileged/linux_net.py
View File

@ -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))

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

@ -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

10
ovn_bgp_agent/tests/functional/utils/test_linux_net.py
View File

@ -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)

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

@ -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},

17
ovn_bgp_agent/utils/common.py Normal file
View File

@ -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}

40
ovn_bgp_agent/utils/linux_net.py
View File

@ -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)