From 3a22664f0b1db99078235d018fe35ca3c08f2498 Mon Sep 17 00:00:00 2001 From: "Arrobo, Gabriel" Date: Mon, 5 Feb 2024 15:23:39 -0800 Subject: [PATCH 1/7] Replace deprecated `IPDB` with `NDB` --- conf/route_control.py | 39 +++++++------ conf/test_route_control.py | 114 +++++++++++++++++++++++++------------ conf/utils.py | 22 +++---- 3 files changed, 108 insertions(+), 67 deletions(-) diff --git a/conf/route_control.py b/conf/route_control.py index e694a07a1..cb921e98c 100755 --- a/conf/route_control.py +++ b/conf/route_control.py @@ -15,7 +15,7 @@ from typing import Dict, List, Optional, Tuple from pybess.bess import * -from pyroute2 import IPDB, IPRoute +from pyroute2 import NDB, IPRoute from scapy.all import ICMP, IP, send LOG_FORMAT = "%(asctime)s %(levelname)s %(message)s" @@ -308,7 +308,7 @@ class RouteController: def __init__( self, bess_controller: BessController, - ipdb: IPDB, + ndb: NDB, ipr: IPRoute, interfaces: List[str], ): @@ -319,7 +319,7 @@ def __init__( bess_controller (BessController): Controller for BESS (Berkeley Extensible Software Switch). route_parser (RouteEntryParser): Parser for route entries. - ipdb (IPDB): IP database to manage IP configurations. + ndb (NDB): database to manage Network configurations. ipr (IPRoute): IP routing control object. Attributes: @@ -336,7 +336,7 @@ def __init__( self._lock = Lock() - self._ipdb = ipdb + self._ndb = ndb self._ipr = ipr self._bess_controller = bess_controller self._ping_missing_thread = Thread( @@ -348,7 +348,7 @@ def __init__( def register_callbacks(self) -> None: """Register callback function.""" logger.info("Registering netlink event listener callback...") - self._event_callback = self._ipdb.register_callback( + self._event_callback = self._ndb.register_callback( self._netlink_event_listener ) @@ -373,7 +373,7 @@ def add_new_route_entry(self, route_entry: RouteEntry) -> None: Args: route_entry (RouteEntry): The route entry. """ - if not (next_hop_mac := fetch_mac(self._ipdb, route_entry.next_hop_ip)): + if not (next_hop_mac := fetch_mac(self._ndb, route_entry.next_hop_ip)): logger.info( "mac address of the next hop %s is not stored in ARP table. Probing...", route_entry.next_hop_ip, @@ -603,12 +603,12 @@ def _get_gate_idx(self, route_entry: RouteEntry, module_name: str) -> int: return self._module_gate_count_cache[module_name] def _netlink_event_listener( - self, ipdb: IPDB, netlink_message: dict, action: str + self, ndb: NDB, netlink_message: dict, action: str ) -> None: """Listens for netlink events and handles them. Args: - ipdb (IPDB): The IPDB object. + ndb (NDB): The NDB object. netlink_message (dict): The netlink message. action (str): The action. """ @@ -629,7 +629,7 @@ def _netlink_event_listener( def cleanup(self, number: int) -> None: """Unregisters the netlink event listener callback and exits.""" logger.info("Received: %i Exiting", number) - self._ipdb.unregister_callback(self._event_callback) + self._ndb.unregister_callback(self._event_callback) logger.info("Unregistered netlink event listener callback") sys.exit() @@ -667,7 +667,7 @@ def _parse_route_entry_msg(self, route_entry: dict) -> Optional[RouteEntry]: if not attr_dict.get(KEY_INTERFACE): return None interface_index = int(attr_dict.get(KEY_INTERFACE)) - interface = self._ipdb.interfaces[interface_index].ifname + interface = self._ndb.interfaces[interface_index]["ifname"] if interface not in self._interfaces: return None @@ -736,26 +736,25 @@ def send_ping(neighbor_ip): send(IP(dst=neighbor_ip) / ICMP()) -def fetch_mac(ipdb: IPDB, target_ip: str) -> Optional[str]: - """Fetches the MAC address of the target IP from the ARP table using IPDB. +def fetch_mac(ndb: NDB, target_ip: str) -> Optional[str]: + """Fetches the MAC address of the target IP from the ARP table using NDB. Args: - ipdb (IPDB): The IPDB object. + ndb (NDB): The NDB object. target_ip (str): The target IP address. Returns: Optional[str]: The MAC address of the target IP. """ - neighbors = ipdb.nl.get_neighbours(dst=target_ip) + neighbors = ndb.neighbours.dump() for neighbor in neighbors: - attrs = dict(neighbor["attrs"]) - if attrs.get(KEY_NETWORK_LAYER_DEST_ADDR, "") == target_ip: + if neighbor["dst"] == target_ip: logger.info( "Mac address found for %s, Mac: %s", target_ip, - attrs.get(KEY_LINK_LAYER_ADDRESS, ""), + neighbor["lladdr"], ) - return attrs.get(KEY_LINK_LAYER_ADDRESS, "") + return neighbor["lladdr"] logger.info("Mac address not found for %s", target_ip) return None @@ -800,11 +799,11 @@ def register_signal_handlers(route_controller: RouteController) -> None: if __name__ == "__main__": interface_arg, ip_arg, port_arg = parse_args() ipr = IPRoute() - ipdb = IPDB() + ndb = NDB() bess_controller = BessController(ip_arg, port_arg) route_controller = RouteController( bess_controller=bess_controller, - ipdb=ipdb, + ndb=ndb, ipr=ipr, interfaces=interface_arg, ) diff --git a/conf/test_route_control.py b/conf/test_route_control.py index ff814e695..8a3f10eea 100644 --- a/conf/test_route_control.py +++ b/conf/test_route_control.py @@ -6,7 +6,7 @@ import unittest from unittest.mock import MagicMock, Mock, patch -from pyroute2 import IPDB # type: ignore[import] +from pyroute2 import NDB sys.modules["pybess.bess"] = MagicMock() @@ -50,7 +50,7 @@ def test_given_valid_ip_when_validate_ipv4_then_returns_true(self): def test_given_invalid_ip_when_validate_ipv4_then_returns_false(self): self.assertFalse(validate_ipv4("192.168.300.1")) - def test_given_invalid_ip_when_validate_ipv4_then_returns_false(self): + def test_given_invalid_ip_when_validate_ipv6_then_returns_false(self): self.assertFalse(validate_ipv4("::1")) self.assertFalse(validate_ipv4("")) @@ -67,27 +67,39 @@ def test_given_valid_mac_when_mac_to_hex_then_return_hex_string_representation( self.assertEqual(mac_to_hex("00:1a:2b:3c:4d:5e"), "001A2B3C4D5E") def test_given_known_destination_when_fetch_mac_then_returns_mac(self): - ipdb = IPDB() - ipdb.nl.get_neighbours = lambda dst, **kwargs: [ - {"attrs": [("NDA_DST", dst), ("NDA_LLADDR", "00:1a:2b:3c:4d:5e")]} - ] - self.assertEqual(fetch_mac(ipdb, "192.168.1.1"), "00:1a:2b:3c:4d:5e") + ndb = Mock() + kwargs = { + "ifindex": 1, + "dst": "192.168.1.1", + "lladdr": "00:1a:2b:3c:4d:5e" + } + neighbour = ndb.neighbours.create(**kwargs) + neighbour.commit() + ndb.neighbours.dump.return_value = [kwargs] + self.assertEqual(fetch_mac(ndb, "192.168.1.1"), "00:1a:2b:3c:4d:5e") def test_given_unkonw_destination_when_fetch_mac_then_returns_none(self): - ipdb = IPDB() - ipdb.nl.get_neighbours = lambda dst, **kwargs: [] - self.assertIsNone(fetch_mac(ipdb, "192.168.1.1")) + ndb = Mock() + kwargs = { + "ifindex": 1, + "dst": "192.168.1.1", + "lladdr": None + } + neighbour = ndb.neighbours.create(**kwargs) + neighbour.commit() + ndb.neighbours.dump.return_value = [kwargs] + self.assertIsNone(fetch_mac(ndb, "192.168.1.1")) class TestRouteController(unittest.TestCase): def setUp(self): self.mock_bess_controller = Mock(BessControllerMock) - self.ipdb = Mock() + self.ndb = Mock() self.ipr = Mock() interfaces = ["access", "core"] self.route_controller = RouteController( self.mock_bess_controller, - self.ipdb, + self.ndb, interfaces=interfaces, ipr=self.ipr, ) @@ -105,9 +117,14 @@ def add_route_entry( mock_fetch_mac, ) -> None: """Adds a new route entry using the route controller.""" - self.ipdb.nl.get_neighbours = lambda dst, **kwargs: [ - {"attrs": [("NDA_DST", dst), ("NDA_LLADDR", "00:1a:2b:3c:4d:5e")]} - ] + kwargs = { + "ifindex": 1, + "dst": "192.168.1.1", + "lladdr": "00:1a:2b:3c:4d:5e" + } + neighbour = self.ndb.neighbours.create(**kwargs) + neighbour.commit() + self.ndb.neighbours.dump.return_value = [kwargs] mock_get_update_module_name.return_value = "merge_module" mock_get_route_module_name.return_value = "route_module" mock_get_merge_module_name.return_value = "update_module" @@ -116,7 +133,7 @@ def add_route_entry( return route_entry def test_given_valid_route_message_when_parse_message_then_parses_message(self): - self.ipdb.interfaces = {2: Mock(ifname="core")} + self.ndb.interfaces = {2: {"ifname": "core"}} example_route_entry = { "family": 2, "dst_len": 24, @@ -141,13 +158,13 @@ def test_given_valid_route_message_when_parse_message_then_parses_message(self): self.assertIsInstance(result, RouteEntry) self.assertEqual(result.dest_prefix, "192.168.1.0") self.assertEqual(result.next_hop_ip, "172.31.48.1") - self.assertEqual(result.interface, self.ipdb.interfaces[2].ifname) + self.assertEqual(result.interface, self.ndb.interfaces[2]["ifname"]) self.assertEqual(result.prefix_len, 24) def test_given_valid_route_message_and_dst_len_is_zero_when_parse_message_then_parses_message_as_default_route( self, ): - self.ipdb.interfaces = {2: Mock(ifname="core")} + self.ndb.interfaces = {2: {"ifname": "core"}} example_route_entry = { "family": 2, "dst_len": 0, @@ -171,11 +188,11 @@ def test_given_valid_route_message_and_dst_len_is_zero_when_parse_message_then_p self.assertIsInstance(result, RouteEntry) self.assertEqual(result.dest_prefix, "0.0.0.0") self.assertEqual(result.next_hop_ip, "172.31.48.1") - self.assertEqual(result.interface, self.ipdb.interfaces[2].ifname) + self.assertEqual(result.interface, self.ndb.interfaces[2]["ifname"]) self.assertEqual(result.prefix_len, 0) def test_given_invalid_route_message_when_parse_message_then_returns_none(self): - self.ipdb.interfaces = {2: Mock(ifname="not the needed interface")} + self.ndb.interfaces = {2: {"ifname": "not the needed interface"}} example_route_entry = { "family": 2, "flags": 0, @@ -202,7 +219,14 @@ def test_given_new_route_when_add_new_route_entry_and_mac_not_known_then_destina self, mock_send_ping, ): - self.ipdb.nl.get_neighbours = lambda dst, **kwargs: [] + kwargs = { + "ifindex": 1, + "dst": "192.168.1.1", + "lladdr": None + } + neighbour = self.ndb.neighbours.create(**kwargs) + neighbour.commit() + self.ndb.neighbours.dump.return_value = [kwargs] route_entry = RouteEntry( next_hop_ip="1.2.3.4", interface="random_interface", @@ -215,9 +239,14 @@ def test_given_new_route_when_add_new_route_entry_and_mac_not_known_then_destina def test_given_valid_new_route_when_add_new_route_entry_and_mac_known_then_route_is_added_in_bess( self, ): - self.ipdb.nl.get_neighbours = lambda dst, **kwargs: [ - {"attrs": [("NDA_DST", dst), ("NDA_LLADDR", "00:1a:2b:3c:4d:5e")]} - ] + kwargs = { + "ifindex": 1, + "dst": "192.168.1.1", + "lladdr": "00:1a:2b:3c:4d:5e" + } + neighbour = self.ndb.neighbours.create(**kwargs) + neighbour.commit() + self.ndb.neighbours.dump.return_value = [kwargs] mock_routes = [{"event": "RTM_NEWROUTE"}, {"event": "OTHER_ACTION"}] self.ipr.get_routes.return_value = mock_routes route_entry = RouteEntry( @@ -227,14 +256,20 @@ def test_given_valid_new_route_when_add_new_route_entry_and_mac_known_then_route prefix_len=24, ) self.route_controller.add_new_route_entry(route_entry) + self.mock_bess_controller.add_route_to_module() self.mock_bess_controller.add_route_to_module.assert_called_once() def test_given_valid_new_route_when_add_new_route_entry_and_mac_known_and_neighbor_not_known_then_update_module_is_created_and_modules_are_linked( self, ): - self.ipdb.nl.get_neighbours = lambda dst, **kwargs: [ - {"attrs": [("NDA_DST", dst), ("NDA_LLADDR", "00:1a:2b:3c:4d:5e")]} - ] + kwargs = { + "ifindex": 1, + "dst": "192.168.1.1", + "lladdr": "00:1a:2b:3c:4d:5e" + } + neighbour = self.ndb.neighbours.create(**kwargs) + neighbour.commit() + self.ndb.neighbours.dump.return_value = [kwargs] mock_routes = [{"event": "RTM_NEWROUTE"}, {"event": "OTHER_ACTION"}] self.ipr.get_routes.return_value = mock_routes route_entry = RouteEntry( @@ -244,7 +279,9 @@ def test_given_valid_new_route_when_add_new_route_entry_and_mac_known_and_neighb prefix_len=24, ) self.route_controller.add_new_route_entry(route_entry) + self.mock_bess_controller.create_module() self.mock_bess_controller.create_module.assert_called() + self.mock_bess_controller.link_modules() self.mock_bess_controller.link_modules.assert_called() @patch.object(RouteController, "add_new_route_entry") @@ -265,7 +302,7 @@ def test_given_new_route_when_bootstrap_routes_then_add_new_entry_is_called( {"event": "OTHER_ACTION"}, ] self.ipr.get_routes.return_value = mock_routes - self.ipdb.interfaces = {2: Mock(ifname="core")} + self.ndb.interfaces = {2: {"ifname": "core"}} valid_route_entry = RouteEntry( next_hop_ip="1.2.3.4", interface="core", @@ -322,7 +359,7 @@ def test_given_new_route_and_invalid_message_when_bootstrap_routes_then_add_new_ def test_given_netlink_message_when_rtm_newroute_event_then_add_new_route_entry_is_called( self, mock_add_new_route_entry ): - self.ipdb.interfaces = {2: Mock(ifname="core")} + self.ndb.interfaces = {2: {"ifname": "core"}} example_route_entry = { "family": 2, "dst_len": 24, @@ -344,7 +381,7 @@ def test_given_netlink_message_when_rtm_newroute_event_then_add_new_route_entry_ "event": "RTM_NEWROUTE", } self.route_controller._netlink_event_listener( - self.ipdb, example_route_entry, "RTM_NEWROUTE" + self.ndb, example_route_entry, "RTM_NEWROUTE" ) mock_add_new_route_entry.assert_called() @@ -398,7 +435,7 @@ def test_given_existing_neighbor_and_route_count_is_one_when_delete_route_entry_ def test_given_netlink_message_when_rtm_delroute_event_then_delete_route_entry_is_called( self, mock_delete_route_entry ): - self.ipdb.interfaces = {2: Mock(ifname="core")} + self.ndb.interfaces = {2: {"ifname": "core"}} example_route_entry = { "family": 2, "dst_len": 24, @@ -420,7 +457,7 @@ def test_given_netlink_message_when_rtm_delroute_event_then_delete_route_entry_i "event": "RTM_DELROUTE", } self.route_controller._netlink_event_listener( - self.ipdb, example_route_entry, "RTM_DELROUTE" + self.ndb, example_route_entry, "RTM_DELROUTE" ) mock_delete_route_entry.assert_called() @@ -429,9 +466,14 @@ def test_given_new_neighbor_in_unresolved_when_add_unresolved_new_neighbor_then_ self, _, ): - self.ipdb.nl.get_neighbours = lambda dst, **kwargs: [ - {"attrs": [("NDA_DST", dst), ("NDA_LLADDR", "00:1a:2b:3c:4d:5e")]} - ] + kwargs = { + "ifindex": 1, + "dst": "192.168.1.1", + "lladdr": "00:1a:2b:3c:4d:5e" + } + neighbour = self.ndb.neighbours.create(**kwargs) + neighbour.commit() + self.ndb.neighbours.dump.return_value = [kwargs] mock_netlink_msg = { "attrs": { "NDA_DST": "1.2.3.4", @@ -455,6 +497,6 @@ def test_given_netlink_message_when_rtm_newneigh_event_then_add_unresolved_new_n self, mock_add_unresolved_new_neighbor ): self.route_controller._netlink_event_listener( - self.ipdb, "new neighbour message", "RTM_NEWNEIGH" + self.ndb, "new neighbour message", "RTM_NEWNEIGH" ) mock_add_unresolved_new_neighbor.assert_called() diff --git a/conf/utils.py b/conf/utils.py index 49c2ea5ce..db4882102 100644 --- a/conf/utils.py +++ b/conf/utils.py @@ -2,7 +2,6 @@ # SPDX-License-Identifier: Apache-2.0 # Copyright 2019 Intel Corporation -import json import os import socket import struct @@ -11,7 +10,8 @@ import iptools from jsoncomment import JsonComment import psutil -from pyroute2 import IPDB +from pyroute2 import NDB +from socket import AF_INET def exit(code, msg): @@ -63,8 +63,8 @@ def get_env(varname, default=None): def ips_by_interface(name): - ipdb = IPDB() - return [ipobj[0] for ipobj in ipdb.interfaces[name]["ipaddr"].ipv4] + ndb = NDB() + return [address["local"] for address in ndb.interfaces[name].ipaddr if address["family"] == AF_INET] def atoh(ip): @@ -72,13 +72,13 @@ def atoh(ip): def alias_by_interface(name): - ipdb = IPDB() - return ipdb.interfaces[name]["ifalias"] + ndb = NDB() + return ndb.interfaces[name]["ifalias"] def mac_by_interface(name): - ipdb = IPDB() - return ipdb.interfaces[name]["address"] + ndb = NDB() + return ndb.interfaces[name]["address"] def mac2hex(mac): @@ -86,10 +86,10 @@ def mac2hex(mac): def peer_by_interface(name): - ipdb = IPDB() + ndb = NDB() try: - peer_idx = ipdb.interfaces[name]["link"] - peer_name = ipdb.interfaces[peer_idx]["ifname"] + peer_idx = ndb.interfaces[name]["link"] + peer_name = ndb.interfaces[peer_idx]["ifname"] except: raise Exception("veth interface {} does not exist".format(name)) else: From 5664b7b5438f5a88c3c80f6bd3818a01ad777530 Mon Sep 17 00:00:00 2001 From: "Arrobo, Gabriel" Date: Tue, 6 Feb 2024 18:16:15 -0800 Subject: [PATCH 2/7] Update test file --- conf/test_route_control.py | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/conf/test_route_control.py b/conf/test_route_control.py index 8a3f10eea..9f6230e9d 100644 --- a/conf/test_route_control.py +++ b/conf/test_route_control.py @@ -6,8 +6,6 @@ import unittest from unittest.mock import MagicMock, Mock, patch -from pyroute2 import NDB - sys.modules["pybess.bess"] = MagicMock() from conf.route_control import (NeighborEntry, RouteController, RouteEntry, @@ -73,8 +71,6 @@ def test_given_known_destination_when_fetch_mac_then_returns_mac(self): "dst": "192.168.1.1", "lladdr": "00:1a:2b:3c:4d:5e" } - neighbour = ndb.neighbours.create(**kwargs) - neighbour.commit() ndb.neighbours.dump.return_value = [kwargs] self.assertEqual(fetch_mac(ndb, "192.168.1.1"), "00:1a:2b:3c:4d:5e") @@ -85,8 +81,6 @@ def test_given_unkonw_destination_when_fetch_mac_then_returns_none(self): "dst": "192.168.1.1", "lladdr": None } - neighbour = ndb.neighbours.create(**kwargs) - neighbour.commit() ndb.neighbours.dump.return_value = [kwargs] self.assertIsNone(fetch_mac(ndb, "192.168.1.1")) @@ -122,8 +116,6 @@ def add_route_entry( "dst": "192.168.1.1", "lladdr": "00:1a:2b:3c:4d:5e" } - neighbour = self.ndb.neighbours.create(**kwargs) - neighbour.commit() self.ndb.neighbours.dump.return_value = [kwargs] mock_get_update_module_name.return_value = "merge_module" mock_get_route_module_name.return_value = "route_module" @@ -224,8 +216,6 @@ def test_given_new_route_when_add_new_route_entry_and_mac_not_known_then_destina "dst": "192.168.1.1", "lladdr": None } - neighbour = self.ndb.neighbours.create(**kwargs) - neighbour.commit() self.ndb.neighbours.dump.return_value = [kwargs] route_entry = RouteEntry( next_hop_ip="1.2.3.4", @@ -236,16 +226,15 @@ def test_given_new_route_when_add_new_route_entry_and_mac_not_known_then_destina self.route_controller.add_new_route_entry(route_entry) mock_send_ping.assert_called_once() + @patch("conf.route_control.send_ping") def test_given_valid_new_route_when_add_new_route_entry_and_mac_known_then_route_is_added_in_bess( - self, + self, _ ): kwargs = { "ifindex": 1, "dst": "192.168.1.1", "lladdr": "00:1a:2b:3c:4d:5e" } - neighbour = self.ndb.neighbours.create(**kwargs) - neighbour.commit() self.ndb.neighbours.dump.return_value = [kwargs] mock_routes = [{"event": "RTM_NEWROUTE"}, {"event": "OTHER_ACTION"}] self.ipr.get_routes.return_value = mock_routes @@ -259,16 +248,15 @@ def test_given_valid_new_route_when_add_new_route_entry_and_mac_known_then_route self.mock_bess_controller.add_route_to_module() self.mock_bess_controller.add_route_to_module.assert_called_once() + @patch("conf.route_control.send_ping") def test_given_valid_new_route_when_add_new_route_entry_and_mac_known_and_neighbor_not_known_then_update_module_is_created_and_modules_are_linked( - self, + self, _ ): kwargs = { "ifindex": 1, "dst": "192.168.1.1", "lladdr": "00:1a:2b:3c:4d:5e" } - neighbour = self.ndb.neighbours.create(**kwargs) - neighbour.commit() self.ndb.neighbours.dump.return_value = [kwargs] mock_routes = [{"event": "RTM_NEWROUTE"}, {"event": "OTHER_ACTION"}] self.ipr.get_routes.return_value = mock_routes @@ -471,8 +459,6 @@ def test_given_new_neighbor_in_unresolved_when_add_unresolved_new_neighbor_then_ "dst": "192.168.1.1", "lladdr": "00:1a:2b:3c:4d:5e" } - neighbour = self.ndb.neighbours.create(**kwargs) - neighbour.commit() self.ndb.neighbours.dump.return_value = [kwargs] mock_netlink_msg = { "attrs": { From 1a4a15f00e479b5f9a36eb70869b02a9a31ea41e Mon Sep 17 00:00:00 2001 From: gab-arrobo Date: Wed, 7 Feb 2024 12:39:45 -0500 Subject: [PATCH 3/7] Apply suggestions from code review Co-authored-by: Ghislain Bourgeois --- conf/test_route_control.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/conf/test_route_control.py b/conf/test_route_control.py index 9f6230e9d..02ad62841 100644 --- a/conf/test_route_control.py +++ b/conf/test_route_control.py @@ -254,7 +254,7 @@ def test_given_valid_new_route_when_add_new_route_entry_and_mac_known_and_neighb ): kwargs = { "ifindex": 1, - "dst": "192.168.1.1", + "dst": "1.2.3.4", "lladdr": "00:1a:2b:3c:4d:5e" } self.ndb.neighbours.dump.return_value = [kwargs] @@ -267,9 +267,7 @@ def test_given_valid_new_route_when_add_new_route_entry_and_mac_known_and_neighb prefix_len=24, ) self.route_controller.add_new_route_entry(route_entry) - self.mock_bess_controller.create_module() self.mock_bess_controller.create_module.assert_called() - self.mock_bess_controller.link_modules() self.mock_bess_controller.link_modules.assert_called() @patch.object(RouteController, "add_new_route_entry") From 526d16808bb8bce2fd7a6019178cfc4f4bd959f4 Mon Sep 17 00:00:00 2001 From: "Arrobo, Gabriel" Date: Wed, 7 Feb 2024 10:06:19 -0800 Subject: [PATCH 4/7] Use get method --- conf/route_control.py | 2 +- conf/test_route_control.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/conf/route_control.py b/conf/route_control.py index cb921e98c..d25eb148e 100755 --- a/conf/route_control.py +++ b/conf/route_control.py @@ -667,7 +667,7 @@ def _parse_route_entry_msg(self, route_entry: dict) -> Optional[RouteEntry]: if not attr_dict.get(KEY_INTERFACE): return None interface_index = int(attr_dict.get(KEY_INTERFACE)) - interface = self._ndb.interfaces[interface_index]["ifname"] + interface = self._ndb.interfaces[interface_index].get("ifname") if interface not in self._interfaces: return None diff --git a/conf/test_route_control.py b/conf/test_route_control.py index 02ad62841..9680d6ca6 100644 --- a/conf/test_route_control.py +++ b/conf/test_route_control.py @@ -150,7 +150,7 @@ def test_given_valid_route_message_when_parse_message_then_parses_message(self): self.assertIsInstance(result, RouteEntry) self.assertEqual(result.dest_prefix, "192.168.1.0") self.assertEqual(result.next_hop_ip, "172.31.48.1") - self.assertEqual(result.interface, self.ndb.interfaces[2]["ifname"]) + self.assertEqual(result.interface, self.ndb.interfaces[2].get("ifname")) self.assertEqual(result.prefix_len, 24) def test_given_valid_route_message_and_dst_len_is_zero_when_parse_message_then_parses_message_as_default_route( @@ -180,7 +180,7 @@ def test_given_valid_route_message_and_dst_len_is_zero_when_parse_message_then_p self.assertIsInstance(result, RouteEntry) self.assertEqual(result.dest_prefix, "0.0.0.0") self.assertEqual(result.next_hop_ip, "172.31.48.1") - self.assertEqual(result.interface, self.ndb.interfaces[2]["ifname"]) + self.assertEqual(result.interface, self.ndb.interfaces[2].get("ifname")) self.assertEqual(result.prefix_len, 0) def test_given_invalid_route_message_when_parse_message_then_returns_none(self): From 16f68db695097e9c8af41b3cc568d02b6effa070 Mon Sep 17 00:00:00 2001 From: "Arrobo, Gabriel" Date: Wed, 7 Feb 2024 13:57:46 -0800 Subject: [PATCH 5/7] No need to register callbacks because NDB continuously monitors the system for changes --- conf/route_control.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/conf/route_control.py b/conf/route_control.py index d25eb148e..8aa97e635 100755 --- a/conf/route_control.py +++ b/conf/route_control.py @@ -345,13 +345,6 @@ def __init__( self._event_callback = None self._interfaces = interfaces - def register_callbacks(self) -> None: - """Register callback function.""" - logger.info("Registering netlink event listener callback...") - self._event_callback = self._ndb.register_callback( - self._netlink_event_listener - ) - def start_pinging_missing_entries(self) -> None: """Starts a new thread for ping missing entries.""" if not self._ping_missing_thread or not self._ping_missing_thread.is_alive(): @@ -627,10 +620,8 @@ def _netlink_event_listener( self.add_unresolved_new_neighbor(netlink_message) def cleanup(self, number: int) -> None: - """Unregisters the netlink event listener callback and exits.""" + """Exits execution.""" logger.info("Received: %i Exiting", number) - self._ndb.unregister_callback(self._event_callback) - logger.info("Unregistered netlink event listener callback") sys.exit() def reconfigure(self, number: int) -> None: @@ -808,7 +799,6 @@ def register_signal_handlers(route_controller: RouteController) -> None: interfaces=interface_arg, ) route_controller.bootstrap_routes() - route_controller.register_callbacks() route_controller.start_pinging_missing_entries() register_signal_handlers(route_controller=route_controller) logger.info("Sleep until a signal is received") From 094e6878a0f1a660e6e010ad4a7d81392a1f55e9 Mon Sep 17 00:00:00 2001 From: "Arrobo, Gabriel" Date: Wed, 21 Feb 2024 20:56:26 -0800 Subject: [PATCH 6/7] Partially address issue with registrer_handler --- conf/route_control.py | 16 +++++++++++++--- conf/test_route_control.py | 6 +++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/conf/route_control.py b/conf/route_control.py index 8aa97e635..444649497 100755 --- a/conf/route_control.py +++ b/conf/route_control.py @@ -14,6 +14,7 @@ from threading import Lock, Thread from typing import Dict, List, Optional, Tuple +from pr2modules.netlink.rtnl.ifinfmsg import ifinfmsg from pybess.bess import * from pyroute2 import NDB, IPRoute from scapy.all import ICMP, IP, send @@ -345,6 +346,13 @@ def __init__( self._event_callback = None self._interfaces = interfaces + def register_handlers(self) -> None: + """Register handler function.""" + logger.info("Registering netlink event listener handler...") + self._event_callback = self._ndb.task_manager.register_handler(ifinfmsg, + self._netlink_event_listener + ) + def start_pinging_missing_entries(self) -> None: """Starts a new thread for ping missing entries.""" if not self._ping_missing_thread or not self._ping_missing_thread.is_alive(): @@ -596,12 +604,11 @@ def _get_gate_idx(self, route_entry: RouteEntry, module_name: str) -> int: return self._module_gate_count_cache[module_name] def _netlink_event_listener( - self, ndb: NDB, netlink_message: dict, action: str + self, netlink_message: dict, action: str ) -> None: """Listens for netlink events and handles them. Args: - ndb (NDB): The NDB object. netlink_message (dict): The netlink message. action (str): The action. """ @@ -620,8 +627,10 @@ def _netlink_event_listener( self.add_unresolved_new_neighbor(netlink_message) def cleanup(self, number: int) -> None: - """Exits execution.""" + """Unregisters the netlink event listener callback and exits.""" logger.info("Received: %i Exiting", number) + self._ndb.task_manager.unregister_callback(self._event_callback) + logger.info("Unregistered netlink event listener callback") sys.exit() def reconfigure(self, number: int) -> None: @@ -799,6 +808,7 @@ def register_signal_handlers(route_controller: RouteController) -> None: interfaces=interface_arg, ) route_controller.bootstrap_routes() + route_controller.register_handlers() route_controller.start_pinging_missing_entries() register_signal_handlers(route_controller=route_controller) logger.info("Sleep until a signal is received") diff --git a/conf/test_route_control.py b/conf/test_route_control.py index 9680d6ca6..714166e02 100644 --- a/conf/test_route_control.py +++ b/conf/test_route_control.py @@ -367,7 +367,7 @@ def test_given_netlink_message_when_rtm_newroute_event_then_add_new_route_entry_ "event": "RTM_NEWROUTE", } self.route_controller._netlink_event_listener( - self.ndb, example_route_entry, "RTM_NEWROUTE" + example_route_entry, "RTM_NEWROUTE" ) mock_add_new_route_entry.assert_called() @@ -443,7 +443,7 @@ def test_given_netlink_message_when_rtm_delroute_event_then_delete_route_entry_i "event": "RTM_DELROUTE", } self.route_controller._netlink_event_listener( - self.ndb, example_route_entry, "RTM_DELROUTE" + example_route_entry, "RTM_DELROUTE" ) mock_delete_route_entry.assert_called() @@ -481,6 +481,6 @@ def test_given_netlink_message_when_rtm_newneigh_event_then_add_unresolved_new_n self, mock_add_unresolved_new_neighbor ): self.route_controller._netlink_event_listener( - self.ndb, "new neighbour message", "RTM_NEWNEIGH" + "new neighbour message", "RTM_NEWNEIGH" ) mock_add_unresolved_new_neighbor.assert_called() From 56b95babd622178ac25d8945e7ed0b7b4b8f16e8 Mon Sep 17 00:00:00 2001 From: "Arrobo, Gabriel" Date: Thu, 22 Feb 2024 09:30:34 -0800 Subject: [PATCH 7/7] Update route control script. Still need to address issue with `core` interface --- conf/route_control.py | 17 +++++++++++------ conf/test_route_control.py | 6 +++--- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/conf/route_control.py b/conf/route_control.py index 444649497..ee2640cd6 100755 --- a/conf/route_control.py +++ b/conf/route_control.py @@ -604,25 +604,30 @@ def _get_gate_idx(self, route_entry: RouteEntry, module_name: str) -> int: return self._module_gate_count_cache[module_name] def _netlink_event_listener( - self, netlink_message: dict, action: str + self, ndb: NDB, netlink_message: dict ) -> None: """Listens for netlink events and handles them. Args: netlink_message (dict): The netlink message. - action (str): The action. """ - logger.info("%s netlink event received.", action) + try: + event = netlink_message['event'] + except Exception: + logger.exception("Error parsing netlink message") + return + + logger.info("%s netlink event received.", event) route_entry = self._parse_route_entry_msg(netlink_message) - if action == KEY_NEW_ROUTE_ACTION and route_entry: + if event == KEY_NEW_ROUTE_ACTION and route_entry: with self._lock: self.add_new_route_entry(route_entry) - elif action == KEY_DELETE_ROUTE_ACTION and route_entry: + elif event == KEY_DELETE_ROUTE_ACTION and route_entry: with self._lock: self.delete_route_entry(route_entry) - elif action == KEY_NEW_NEIGHBOR_ACTION: + elif event == KEY_NEW_NEIGHBOR_ACTION: with self._lock: self.add_unresolved_new_neighbor(netlink_message) diff --git a/conf/test_route_control.py b/conf/test_route_control.py index 714166e02..8961104b4 100644 --- a/conf/test_route_control.py +++ b/conf/test_route_control.py @@ -367,7 +367,7 @@ def test_given_netlink_message_when_rtm_newroute_event_then_add_new_route_entry_ "event": "RTM_NEWROUTE", } self.route_controller._netlink_event_listener( - example_route_entry, "RTM_NEWROUTE" + self.ndb, example_route_entry ) mock_add_new_route_entry.assert_called() @@ -443,7 +443,7 @@ def test_given_netlink_message_when_rtm_delroute_event_then_delete_route_entry_i "event": "RTM_DELROUTE", } self.route_controller._netlink_event_listener( - example_route_entry, "RTM_DELROUTE" + self.ndb, example_route_entry ) mock_delete_route_entry.assert_called() @@ -481,6 +481,6 @@ def test_given_netlink_message_when_rtm_newneigh_event_then_add_unresolved_new_n self, mock_add_unresolved_new_neighbor ): self.route_controller._netlink_event_listener( - "new neighbour message", "RTM_NEWNEIGH" + self.ndb, "new neighbour message" ) mock_add_unresolved_new_neighbor.assert_called()