From d748832cdc8a58bba10bfb0ed6069e5d3f4b9b09 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Mon, 5 Jan 2026 14:03:41 +0800 Subject: [PATCH 01/10] tests/mctpenv: fix invalid var reference F821 Undefined name `a` --> tests/mctpenv/__init__.py:348:52 | 346 | await self.handle_mctp_control(sock, addr, data) 347 | else: 348 | print(f"unknown MCTP message type {a.type}") | ^ 349 | else: 350 | for br_ep in self.bridged_eps: | Signed-off-by: Jeremy Kerr --- tests/mctpenv/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mctpenv/__init__.py b/tests/mctpenv/__init__.py index 9e6b724..10ae989 100644 --- a/tests/mctpenv/__init__.py +++ b/tests/mctpenv/__init__.py @@ -345,7 +345,7 @@ async def handle_mctp_message(self, sock, addr, data): if addr.type == 0: await self.handle_mctp_control(sock, addr, data) else: - print(f"unknown MCTP message type {a.type}") + print(f"unknown MCTP message type {addr.type}") else: for br_ep in self.bridged_eps: if addr.eid == br_ep.eid: From 4a347b52bab916bfafa3c27f92d57993c3cc3da8 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Mon, 5 Jan 2026 14:04:20 +0800 Subject: [PATCH 02/10] tests: remove unused imports Fix ruff lint F401. Signed-off-by: Jeremy Kerr --- tests/conftest.py | 2 -- tests/test_mctp.py | 2 -- tests/test_mctpd.py | 1 - 3 files changed, 5 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 1dc86f6..d13fdec 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,4 @@ -import sys - import pytest import asyncdbus import trio.testing diff --git a/tests/test_mctp.py b/tests/test_mctp.py index b7d4b18..af7c286 100644 --- a/tests/test_mctp.py +++ b/tests/test_mctp.py @@ -1,6 +1,4 @@ -from mctpenv import MctpWrapper - async def test_link_simple(mctp): proc = await mctp.run(["link"]) assert proc.returncode == 0 diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index 55bf13f..d3d18b8 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -1,6 +1,5 @@ import pytest import trio -import uuid import asyncdbus from mctp_test_utils import ( From cc916112d3020c9af68a19b6faa9ee01918c6a64 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Mon, 5 Jan 2026 14:05:19 +0800 Subject: [PATCH 03/10] tests: remove unnecessary semicolon Fix ruff lint E703. Signed-off-by: Jeremy Kerr --- tests/mctpenv/__init__.py | 8 ++++---- tests/test_mctpd.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/mctpenv/__init__.py b/tests/mctpenv/__init__.py index 10ae989..80e8ea0 100644 --- a/tests/mctpenv/__init__.py +++ b/tests/mctpenv/__init__.py @@ -126,7 +126,7 @@ def net(self): return self.gw[0] elif self.iface is not None: return self.iface.net - raise ValueError("no gw or iface"); + raise ValueError("no gw or iface") def __str__(self): s = f"{self.start_eid}-{self.end_eid} -> " @@ -746,7 +746,7 @@ async def handle_send(self, addr, data): typ = header['type'] if not header['flags'] & netlink.NLM_F_REQUEST: - print("error: not a request?"); + print("error: not a request?") return if typ == rtnl.RTM_GETLINK: @@ -1108,10 +1108,10 @@ async def _notify_route(self, route, typ): await self._send_msg(buf) async def notify_newroute(self, route): - await self._notify_route(route, rtnl.RTM_NEWROUTE); + await self._notify_route(route, rtnl.RTM_NEWROUTE) async def notify_delroute(self, route): - await self._notify_route(route, rtnl.RTM_DELROUTE); + await self._notify_route(route, rtnl.RTM_DELROUTE) class TimerSocket(BaseSocket): diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index d3d18b8..80bfd07 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -715,7 +715,7 @@ async def test_network_learn_endpoint_absent(dbus, mctpd): """ Change a network id, while we have an active endpoint on that net """ async def test_change_network(dbus, mctpd): - iface = mctpd.system.interfaces[0]; + iface = mctpd.system.interfaces[0] ep = mctpd.network.endpoints[0] net = await mctpd_mctp_network_obj(dbus, 1) From 0489e54f5198b4307ee83ed6850765b5334ceaae Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Mon, 5 Jan 2026 14:05:38 +0800 Subject: [PATCH 04/10] tests: use 'not in' for set membership test Signed-off-by: Jeremy Kerr --- tests/mctpenv/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mctpenv/__init__.py b/tests/mctpenv/__init__.py index 80e8ea0..c858698 100644 --- a/tests/mctpenv/__init__.py +++ b/tests/mctpenv/__init__.py @@ -427,7 +427,7 @@ async def send_control(self, sock, cmd): addr.set_ext(self.iface.ifindex, self.lladdr) key = (typ, cmd.iid) - assert not key in self.commands + assert key not in self.commands self.commands[key] = cmd From 5495ef4daff9bb0d1a5191dd03f86ced46227014 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Mon, 5 Jan 2026 14:12:19 +0800 Subject: [PATCH 05/10] tests: remove unused variables Fix ruff lint F841. Signed-off-by: Jeremy Kerr --- tests/mctpenv/__init__.py | 9 ++++----- tests/test_mctpd.py | 9 +++------ 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/tests/mctpenv/__init__.py b/tests/mctpenv/__init__.py index c858698..fa9a0dd 100644 --- a/tests/mctpenv/__init__.py +++ b/tests/mctpenv/__init__.py @@ -545,7 +545,7 @@ async def run(self): while True: try: data = await self.sock.recv(1024) - except ConnectionResetError as ex: + except ConnectionResetError: break if len(data) == 0: @@ -553,7 +553,7 @@ async def run(self): try: await self.recv_msg(data) - except BrokenPipeError as ex: + except BrokenPipeError: break async def recv_msg(self, data): @@ -1039,8 +1039,7 @@ def _parse_route(self, msg): gw = msg.get_attr('RTA_GATEWAY') start_eid = msg.get_attr('RTA_DST') extent_eid = msg['dst_len'] - # todo: RTAX metrics - mtu = 0 + # todo: RTAX metrics: MTU if ifindex: iface = self.system.find_interface_by_ifindex(ifindex) @@ -1137,7 +1136,7 @@ async def run(self): math.floor(trio.current_time() * 1000000)) await self.sock.send(data) self.delay = sys.maxsize - except (ConnectionResetError, BrokenPipeError) as ex: + except (ConnectionResetError, BrokenPipeError): break diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index 80bfd07..4f4e376 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -227,7 +227,6 @@ def ep_removed(ep_path, interfaces): async def test_recover_endpoint_reset(dbus, mctpd, autojump_clock): iface = mctpd.system.interfaces[0] dev = mctpd.network.endpoints[0] - mctp = await dbus.get_proxy_object(MCTPD_C, MCTPD_MCTP_P) mctp_iface = await mctpd_mctp_iface_obj(dbus, iface) (eid, net, path, new) = await mctp_iface.call_setup_endpoint(dev.lladdr) @@ -407,8 +406,6 @@ async def test_assign_endpoint_static_varies(dbus, mctpd): for a basic Get Endpoint ID command""" async def test_get_endpoint_id(dbus, mctpd, routed_ep): ep = routed_ep - iface = mctpd.system.interfaces[0] - mctp = await mctpd_mctp_iface_obj(dbus, iface) cmd = MCTPControlCommand(True, 0, 0x02) rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) @@ -448,7 +445,7 @@ async def handle_mctp_control(self, sock, src_addr, msg): mctp = await mctpd_mctp_iface_obj(dbus, iface) with pytest.raises(asyncdbus.errors.DBusError) as ex: - rc = await mctp.call_learn_endpoint(ep.lladdr) + await mctp.call_learn_endpoint(ep.lladdr) assert str(ex.value) == "Request failed" @@ -479,7 +476,7 @@ async def handle_mctp_control(self, sock, src_addr, msg): mctp = await mctpd_mctp_iface_obj(dbus, iface) with pytest.raises(asyncdbus.errors.DBusError) as ex: - rc = await mctp.call_setup_endpoint(ep.lladdr) + await mctp.call_setup_endpoint(ep.lladdr) assert str(ex.value) == "Endpoint returned failure to Set Endpoint ID" @@ -710,7 +707,7 @@ async def test_network_learn_endpoint_absent(dbus, mctpd): net = await mctpd_mctp_network_obj(dbus, iface.net) - with pytest.raises(asyncdbus.errors.DBusError) as ex: + with pytest.raises(asyncdbus.errors.DBusError): await net.call_learn_endpoint(10) """ Change a network id, while we have an active endpoint on that net """ From cbc6707198b86b51d342a84b9891847c6257be4a Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Mon, 5 Jan 2026 14:16:14 +0800 Subject: [PATCH 06/10] tests: use `is None` for comparision against None Fix ruff lint E711. Signed-off-by: Jeremy Kerr --- tests/test_mctp.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_mctp.py b/tests/test_mctp.py index af7c286..ddca8ba 100644 --- a/tests/test_mctp.py +++ b/tests/test_mctp.py @@ -43,7 +43,7 @@ async def test_route_add_single_direct(mctp): assert len(mctp.system.routes) == 1 route = mctp.system.routes[0] assert route.iface.name == "mctp0" - assert route.gw == None + assert route.gw is None assert route.start_eid == 9 assert route.end_eid == 9 assert route.mtu == 0 @@ -54,7 +54,7 @@ async def test_route_add_single_gw(mctp): assert len(mctp.system.routes) == 1 route = mctp.system.routes[0] - assert route.iface == None + assert route.iface is None assert route.gw[0] == 1 assert route.gw[1] == 9 assert route.start_eid == 10 From 0bb91aacf44696dd78f0d751f9319525a068d82d Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Mon, 5 Jan 2026 14:24:32 +0800 Subject: [PATCH 07/10] tests: avoid 'import *' Signed-off-by: Jeremy Kerr --- tests/test_mctpd_endpoint.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test_mctpd_endpoint.py b/tests/test_mctpd_endpoint.py index 82ecf28..a2b2ce4 100644 --- a/tests/test_mctpd_endpoint.py +++ b/tests/test_mctpd_endpoint.py @@ -1,7 +1,13 @@ import pytest import asyncdbus -from mctp_test_utils import * -from mctpenv import * +from mctp_test_utils import ( + mctpd_mctp_iface_control_obj, + mctpd_mctp_endpoint_control_obj +) +from mctpenv import ( + Endpoint, MCTPControlCommand, Network, PhysicalBinding, Sysnet, System +) + """Simple endpoint setup. From b8809f38b618e37782b44fc6e83ec10d8541b49c Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Mon, 5 Jan 2026 15:03:49 +0800 Subject: [PATCH 08/10] tests: move docstrings to their correct locations Docstrings should be within the block they document. Move and reformat with no leading space, and multi-line strings should have their closing-triple-quote on its own line. Signed-off-by: Jeremy Kerr --- tests/conftest.py | 10 +- tests/test_mctpd.py | 232 +++++++++++++++++++---------------- tests/test_mctpd_endpoint.py | 10 +- 3 files changed, 136 insertions(+), 116 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index d13fdec..560227f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,13 +5,13 @@ import mctpenv -"""Simple system & network. - -Contains one interface (lladdr 0x10, local EID 8), and one endpoint (lladdr -0x1d), that reports support for MCTP control and PLDM. -""" @pytest.fixture async def sysnet(): + """Simple system & network. + + Contains one interface (lladdr 0x10, local EID 8), and one endpoint (lladdr + 0x1d), that reports support for MCTP control and PLDM. + """ return await mctpenv.default_sysnet() @pytest.fixture diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index 4f4e376..68ee53f 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -50,53 +50,53 @@ async def _introspect_path_recursive(dbus, path, node_set): return dups -""" Test that the dbus object tree is sensible: we can introspect all -objects, and that there are no duplicates -""" async def test_enumerate(dbus, mctpd): + """Test that the dbus object tree is sensible: we can introspect all + objects, and that there are no duplicates + """ dups = await _introspect_path_recursive(dbus, '/', set()) assert not dups -""" Test the SetupEndpoint dbus call +async def test_setup_endpoint(dbus, mctpd): + """Test the SetupEndpoint dbus call -Using the default system & network ojects, call SetupEndpoint on our mock -endpoint. We expect the dbus call to return the endpoint details, and -the new kernel neighbour and route entries. + Using the default system & network ojects, call SetupEndpoint on our mock + endpoint. We expect the dbus call to return the endpoint details, and + the new kernel neighbour and route entries. -We have a few things provided by the test infrastructure: + We have a few things provided by the test infrastructure: - - dbus is the dbus connection, call the mctpd_mctp_iface_obj helper to - get the MCTP dbus interface object + - dbus is the dbus connection, call the mctpd_mctp_iface_obj helper to + get the MCTP dbus interface object - - mctpd is our wrapper for the mctpd process and mock MCTP environment. This - has two properties that represent external state: + - mctpd is our wrapper for the mctpd process and mock MCTP environment. + This has two properties that represent external state: - mctp.system: the local system info - containing MCTP interfaces - (mctp.system.interfaces), addresses (.addresses), neighbours (.neighbours) - and routes (.routes). These may be updated by the running mctpd process - during tests, over the simlated netlink socket. + mctp.system: the local system info - containing MCTP interfaces + (mctp.system.interfaces), addresses (.addresses), neighbours + (.neighbours) and routes (.routes). These may be updated by the running + mctpd process during tests, over the simlated netlink socket. - mctp.network: the set of remote MCTP endpoints connected to the system. Each - endpoint has a physical address (.lladdr) and an EID (.eid), and a tiny - MCTP control protocol implementation, which the mctpd process will - interact with over simulated AF_MCTP sockets. + mctp.network: the set of remote MCTP endpoints connected to the system. + Each endpoint has a physical address (.lladdr) and an EID (.eid), and a + tiny MCTP control protocol implementation, which the mctpd process will + interact with over simulated AF_MCTP sockets. -By default, we get some minimal defaults for .system and .network: + By default, we get some minimal defaults for .system and .network: - - The system has one interface ('mctp0'), assigned local EID 8. This is - similar to a MCTP-over-i2c interface, in that physical addresses are - a single byte. + - The system has one interface ('mctp0'), assigned local EID 8. This is + similar to a MCTP-over-i2c interface, in that physical addresses are + a single byte. - - The network has one endpoint (lladdr 0x1d) connected to mctp0, with no EID - assigned. It also has a random UUID, and advertises support for MCTP - Control Protocol and PLDM (but note that it doesn't actually implement - any PLDM!). + - The network has one endpoint (lladdr 0x1d) connected to mctp0, with no + EID assigned. It also has a random UUID, and advertises support for MCTP + Control Protocol and PLDM (but note that it doesn't actually implement + any PLDM!). -But these are only defaults; .system and .network can be altered as required -for each test. -""" -async def test_setup_endpoint(dbus, mctpd): + But these are only defaults; .system and .network can be altered as + required for each test. + """ # shortcuts to the default system/network configuration iface = mctpd.system.interfaces[0] ep = mctpd.network.endpoints[0] @@ -120,17 +120,18 @@ async def test_setup_endpoint(dbus, mctpd): # we should have a route for the new endpoint too assert len(mctpd.system.routes) == 2 -""" Test that we correctly handle address conflicts on EID assignment. +async def test_setup_endpoint_conflict(dbus, mctpd): + """Test that we correctly handle address conflicts on EID assignment. -We have the following scenario: + We have the following scenario: - 1. A configured peer at physaddr 1, EID A, allocated by mctpd - 2. A non-configured peer at physaddr 2, somehow carrying a default EID also A - 3. Attempt to enumerate physaddr 2 + 1. A configured peer at physaddr 1, EID A, allocated by mctpd + 2. A non-configured peer at physaddr 2, somehow carrying a default EID + also A + 3. Attempt to enumerate physaddr 2 -At (3), we should reconfigure the EID to B. -""" -async def test_setup_endpoint_conflict(dbus, mctpd): + At (3), we should reconfigure the EID to B. + """ iface = mctpd.system.interfaces[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) @@ -145,8 +146,8 @@ async def test_setup_endpoint_conflict(dbus, mctpd): (eid2, _, _, _) = await mctp.call_setup_endpoint(ep2.lladdr) assert eid1 != eid2 -""" Test neighbour removal """ async def test_remove_endpoint(dbus, mctpd): + """Test neighbour removal""" iface = mctpd.system.interfaces[0] ep1 = mctpd.network.endpoints[0] @@ -312,9 +313,10 @@ def ep_added(ep_path, content): assert not expected.cancelled_caught -""" Test that we get the correct EID allocated (and the usual route/neigh setup) -on an AssignEndpointStatic call """ async def test_assign_endpoint_static(dbus, mctpd): + """Test that we get the correct EID allocated (and the usual route/neigh + setup) on an AssignEndpointStatic call + """ iface = mctpd.system.interfaces[0] dev = mctpd.network.endpoints[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) @@ -334,9 +336,10 @@ async def test_assign_endpoint_static(dbus, mctpd): assert neigh.eid == static_eid assert len(mctpd.system.routes) == 2 -""" Test that we can repeat an AssignEndpointStatic call with the same static -EID""" async def test_assign_endpoint_static_allocated(dbus, mctpd): + """Test that we can repeat an AssignEndpointStatic call with the same + static EID + """ iface = mctpd.system.interfaces[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) dev = mctpd.network.endpoints[0] @@ -359,8 +362,8 @@ async def test_assign_endpoint_static_allocated(dbus, mctpd): assert eid == static_eid assert not new -""" Test that we cannot assign a conflicting static EID """ async def test_assign_endpoint_static_conflict(dbus, mctpd): + """Test that we cannot assign a conflicting static EID""" iface = mctpd.system.interfaces[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) dev1 = mctpd.network.endpoints[0] @@ -381,9 +384,10 @@ async def test_assign_endpoint_static_conflict(dbus, mctpd): assert str(ex.value) == "Address in use" -""" Test that we cannot re-assign a static EID to an endpoint that already has -a different EID allocated""" async def test_assign_endpoint_static_varies(dbus, mctpd): + """Test that we cannot re-assign a static EID to an endpoint that already + has a different EID allocated + """ iface = mctpd.system.interfaces[0] dev = mctpd.network.endpoints[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) @@ -402,9 +406,10 @@ async def test_assign_endpoint_static_varies(dbus, mctpd): assert str(ex.value) == "Already assigned a different EID" -""" Test that the mctpd control protocol responder support has support -for a basic Get Endpoint ID command""" async def test_get_endpoint_id(dbus, mctpd, routed_ep): + """Test that the mctpd control protocol responder support has support for a + basic Get Endpoint ID command + """ ep = routed_ep cmd = MCTPControlCommand(True, 0, 0x02) @@ -417,19 +422,21 @@ async def test_get_endpoint_id(dbus, mctpd, routed_ep): # EID matches the system assert rsp[3] == mctpd.system.addresses[0].eid -""" Test that instance ID is populated correctly on control protocol responses -""" async def test_response_iid(mctpd): + """Test that instance ID is populated correctly on control protocol + responses + """ peer = mctpd.network.endpoints[0] for iid in [0, 1, 30, 31]: cmd = MCTPControlCommand(True, iid, 0x02) rsp = await peer.send_control(mctpd.network.mctp_socket, cmd) assert rsp[0] == iid -""" During a LearnEndpoint's Get Endpoint ID exchange, return a response -from a different command; in this case Get Message Type Support, which happens -to be the same length as a the expected Get Endpoint ID response.""" async def test_learn_endpoint_invalid_response_command(dbus, mctpd): + """During a LearnEndpoint's Get Endpoint ID exchange, return a response + from a different command; in this case Get Message Type Support, which + happens to be the same length as a the expected Get Endpoint ID response. + """ class BusyEndpoint(Endpoint): async def handle_mctp_control(self, sock, src_addr, msg): flags, opcode = msg[0:2] @@ -449,10 +456,11 @@ async def handle_mctp_control(self, sock, src_addr, msg): assert str(ex.value) == "Request failed" -""" During a SetupEndpoint's Set Endpoint ID exchange, return a response -that indicates that the EID has been set, but report an invalid (0) EID -in the response.""" async def test_setup_endpoint_invalid_set_eid_response(dbus, mctpd): + """During a SetupEndpoint's Set Endpoint ID exchange, return a response + that indicates that the EID has been set, but report an invalid (0) EID in + the response. + """ class InvalidEndpoint(Endpoint): async def handle_mctp_control(self, sock, src_addr, msg): flags, opcode = msg[0:2] @@ -480,10 +488,11 @@ async def handle_mctp_control(self, sock, src_addr, msg): assert str(ex.value) == "Endpoint returned failure to Set Endpoint ID" -""" During a SetupEndpoint's Set Endpoint ID exchange, return a response -that indicates that the EID has been set, but report a different set EID -in the response.""" async def test_setup_endpoint_vary_set_eid_response(dbus, mctpd): + """During a SetupEndpoint's Set Endpoint ID exchange, return a response + that indicates that the EID has been set, but report a different set EID in + the response. + """ class VaryEndpoint(Endpoint): async def handle_mctp_control(self, sock, src_addr, msg): flags, opcode = msg[0:2] @@ -510,10 +519,11 @@ async def handle_mctp_control(self, sock, src_addr, msg): assert eid == ep.eid -""" During a SetupEndpoint's Set Endpoint ID exchange, return a response -that indicates that the EID has been set, but report a different set EID -in the response, which conflicts with another endpoint""" async def test_setup_endpoint_conflicting_set_eid_response(dbus, mctpd): + """During a SetupEndpoint's Set Endpoint ID exchange, return a response + that indicates that the EID has been set, but report a different set EID in + the response, which conflicts with another endpoint + """ class ConflictingEndpoint(Endpoint): def __init__(self, iface, lladdr, conflict_eid): @@ -550,8 +560,8 @@ async def handle_mctp_control(self, sock, src_addr, msg): assert "already used" in str(ex.value) -""" Ensure a response with an invalid IID is discarded """ async def test_learn_endpoint_invalid_response_iid(dbus, mctpd): + """Ensure a response with an invalid IID is discarded""" class InvalidIIDEndpoint(Endpoint): async def handle_mctp_control(self, sock, src_addr, msg): # bump IID @@ -571,8 +581,8 @@ async def handle_mctp_control(self, sock, src_addr, msg): assert str(ex.value) == "Request failed" -""" Ensure we're parsing Get Message Type Support responses""" async def test_query_message_types(dbus, mctpd): + """Ensure we're parsing Get Message Type Support responses""" iface = mctpd.system.interfaces[0] ep = mctpd.network.endpoints[0] ep_types = [0, 1, 5] @@ -592,8 +602,8 @@ async def test_query_message_types(dbus, mctpd): assert ep_types == query_types -""" Network1.LocalEIDs should reflect locally-assigned EID state """ async def test_network_local_eids_single(dbus, mctpd): + """Network1.LocalEIDs should reflect locally-assigned EID state""" iface = mctpd.system.interfaces[0] net = await mctpd_mctp_network_obj(dbus, iface.net) @@ -684,8 +694,8 @@ def ep_removed(ep_path, interfaces): await removed.acquire() assert not expected.cancelled_caught -""" Bridged EP can be discovered via Network1.LearnEndpoint """ async def test_bridged_learn_endpoint(dbus, mctpd): + """Bridged EP can be discovered via Network1.LearnEndpoint""" iface = mctpd.system.interfaces[0] ep = mctpd.network.endpoints[0] br_ep = Endpoint(iface, bytes(), eid = 10, types = [0, 2]) @@ -710,8 +720,8 @@ async def test_network_learn_endpoint_absent(dbus, mctpd): with pytest.raises(asyncdbus.errors.DBusError): await net.call_learn_endpoint(10) -""" Change a network id, while we have an active endpoint on that net """ async def test_change_network(dbus, mctpd): + """Change a network id, while we have an active endpoint on that net""" iface = mctpd.system.interfaces[0] ep = mctpd.network.endpoints[0] @@ -736,8 +746,8 @@ async def test_change_network(dbus, mctpd): ) assert ep is not None -""" Delete our only interface """ async def test_del_interface_last(dbus, mctpd): + """Delete our only interface""" iface = mctpd.system.interfaces[0] await mctpd.system.del_interface(iface) @@ -749,8 +759,8 @@ async def test_del_interface_last(dbus, mctpd): with pytest.raises(asyncdbus.errors.DBusError): await mctpd_mctp_network_obj(dbus, iface.net) -""" Delete an interface with peers attached, ensure all are gone """ async def test_del_interface_with_peers(dbus, mctpd): + """Delete an interface with peers attached, ensure all are gone""" net = mctpd.system.interfaces[0].net iface = mctpd.system.Interface( 'mctp1', 2, net, bytes([0x10]), 68, 254, True, @@ -787,8 +797,8 @@ async def test_del_interface_with_peers(dbus, mctpd): ep = await mctpd_mctp_endpoint_common_obj(dbus, path) assert str(ex.value).startswith("Unknown object") -""" Remove and re-add an interface """ async def test_add_interface(dbus, mctpd): + """Remove and re-add an interface""" net = 1 # Create a new netdevice iface = mctpd.system.Interface('mctpnew', 10, net, bytes([]), 68, 254, True) @@ -871,8 +881,8 @@ async def test_interface_rename_with_peers(dbus, mctpd): ep_obj = await dbus.get_proxy_object(MCTPD_C, ep_path) assert ep_obj is not None -""" Test that we use the minimum EID from the dynamic_eid_range config """ async def test_config_dyn_eid_range_min(nursery, dbus, sysnet): + """Test that we use the minimum EID from the dynamic_eid_range config""" (min_dyn_eid, max_dyn_eid) = (20, 254) config = f""" [bus-owner] @@ -895,8 +905,8 @@ async def test_config_dyn_eid_range_min(nursery, dbus, sysnet): res = await mctpd.stop_mctpd() assert res == 0 -""" Test that we use the maximum EID from the dynamic_eid_range config """ async def test_config_dyn_eid_range_max(nursery, dbus, sysnet): + """Test that we use the maximum EID from the dynamic_eid_range config""" (min_dyn_eid, max_dyn_eid) = (20, 21) config = f""" [bus-owner] @@ -928,14 +938,14 @@ async def test_config_dyn_eid_range_max(nursery, dbus, sysnet): res = await mctpd.stop_mctpd() assert res == 0 -""" Test bridge endpoint dynamic EID assignment and downstream -endpoint EID allocation - -Tests that: -- Bridge endpoint can be assigned a dynamic EID -- Downstream endpoints get contiguous EIDs after bridge's own eid -""" async def test_assign_dynamic_bridge_eid(dbus, mctpd): + """ Test bridge endpoint dynamic EID assignment and downstream + endpoint EID allocation + + Tests that: + - Bridge endpoint can be assigned a dynamic EID + - Downstream endpoints get contiguous EIDs after bridge's own eid + """ iface = mctpd.system.interfaces[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) ep = mctpd.network.endpoints[0] @@ -953,9 +963,10 @@ async def test_assign_dynamic_bridge_eid(dbus, mctpd): assert new assert ep.allocated_pool == (eid + 1, pool_size) -""" Test that static allocations are not permitted, if they would conflict -with a bridge pool""" async def test_bridge_ep_conflict_static(dbus, mctpd): + """Test that static allocations are not permitted, if they would conflict + with a bridge pool + """ iface = mctpd.system.interfaces[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) ep = mctpd.network.endpoints[0] @@ -986,9 +997,10 @@ async def test_bridge_ep_conflict_static(dbus, mctpd): assert eid == static_eid -""" Test that learnt allocations (ie, pre-assigned device EIDs) are not -permitted, if they would conflict with a bridge pool """ async def test_bridge_ep_conflict_learn(dbus, mctpd): + """Test that learnt allocations (ie, pre-assigned device EIDs) are not + permitted, if they would conflict with a bridge pool + """ iface = mctpd.system.interfaces[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) ep = mctpd.network.endpoints[0] @@ -1017,9 +1029,10 @@ async def test_bridge_ep_conflict_learn(dbus, mctpd): assert eid == dev_eid -""" Test that learnt allocations (ie, pre-assigned device EIDs) are not -permitted through SetupEndpoint, if they would conflict with a bridge pool """ async def test_bridge_ep_conflict_setup(dbus, mctpd): + """Test that learnt allocations (ie, pre-assigned device EIDs) are not + permitted through SetupEndpoint, if they would conflict with a bridge pool + """ iface = mctpd.system.interfaces[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) ep = mctpd.network.endpoints[0] @@ -1042,9 +1055,10 @@ async def test_bridge_ep_conflict_setup(dbus, mctpd): (eid, _, _, _) = await mctp.call_setup_endpoint(dev.lladdr) assert eid not in pool_range -""" Test that mctpd will reassign a bridge endpoints (pre-configured) EID -if necessary to satisfy the bridge pool allocation""" async def test_bridge_setup_reassign(dbus, mctpd): + """Test that mctpd will reassign a bridge endpoints (pre-configured) EID if + necessary to satisfy the bridge pool allocation + """ iface = mctpd.system.interfaces[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) @@ -1069,9 +1083,10 @@ async def test_bridge_setup_reassign(dbus, mctpd): assert br.allocated_pool is not None assert br.allocated_pool[0] == eid + 1 -""" Test that we truncate the requested pool size to - the max_pool_size config """ async def test_assign_dynamic_eid_limited_pool(nursery, dbus, sysnet): + """Test that we truncate the requested pool size to the max_pool_size + config + """ max_pool_size = 1 config = f""" [bus-owner] @@ -1105,9 +1120,10 @@ async def test_assign_dynamic_eid_limited_pool(nursery, dbus, sysnet): res = await mctpd.stop_mctpd() assert res == 0 -""" Test that a limited pool is assigned if we run out of space for a full -allocation""" async def test_bridge_pool_assign_limited(nursery, dbus, sysnet): + """Test that a limited pool is assigned if we run out of space for a full + allocation + """ (min_dyn_eid, max_dyn_eid) = (8, 13) config = f""" [bus-owner] @@ -1148,9 +1164,10 @@ async def test_bridge_pool_assign_limited(nursery, dbus, sysnet): res = await mctpd.stop_mctpd() assert res == 0 -"""During Allocate Endpoint ID exchange, return completion code failure -to indicate no pool has been assigned to the bridge""" async def test_assign_dynamic_eid_allocation_failure(dbus, mctpd): + """During Allocate Endpoint ID exchange, return completion code failure + to indicate no pool has been assigned to the bridge + """ class BridgeEndpoint(Endpoint): async def handle_mctp_control(self, sock, src_addr, msg): flags, opcode = msg[0:2] @@ -1186,9 +1203,10 @@ async def handle_mctp_control(self, sock, src_addr, msg): bridge_obj = await dbus.get_proxy_object(MCTPD_C, path) await bridge_obj.get_interface(MCTPD_ENDPOINT_BRIDGE_I) -""" Test assigning a non-bridge endpoint, when we don't have capacity for -the speculatively-allocated bridge range""" async def test_assign_without_bridge_range(dbus, sysnet, nursery): + """Test assigning a non-bridge endpoint, when we don't have capacity for + the speculatively-allocated bridge range + """ (dyn_eid_min, dyn_eid_max) = (10, 20) max_pool_size = (dyn_eid_max - dyn_eid_min) + 1 config = f""" @@ -1211,10 +1229,11 @@ async def test_assign_without_bridge_range(dbus, sysnet, nursery): res = await mctpd.stop_mctpd() assert res == 0 -""" Test that we can still allocate a bridge pool even though we may not have -the maximum EID range available. The bridge pool's full allocation is still -possible, since it is smaller than the configured max""" async def test_bridge_pool_range_limited(dbus, sysnet, nursery): + """Test that we can still allocate a bridge pool even though we may not have + the maximum EID range available. The bridge pool's full allocation is still + possible, since it is smaller than the configured max + """ # configure for: # 10: bridge A # 11-13: bridge A pool @@ -1283,8 +1302,8 @@ async def test_get_message_types(dbus, mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 04 00 01 f4 f3 f2 f1' -""" Test RegisterVDMTypeSupport when no responders are registered """ async def test_register_vdm_type_support_empty(mctpd, routed_ep): + """Test RegisterVDMTypeSupport when no responders are registered""" ep = routed_ep # Verify error response when no VDM is registered @@ -1292,8 +1311,8 @@ async def test_register_vdm_type_support_empty(mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 06 02' -""" Test RegisterVDMTypeSupport when a single PCIe VDM is registered """ async def test_register_vdm_type_support_pcie_only(dbus, mctpd, routed_ep): + """Test RegisterVDMTypeSupport when a single PCIe VDM is registered""" ep = routed_ep mctp = await mctpd_mctp_base_iface_obj(dbus) @@ -1316,8 +1335,8 @@ async def test_register_vdm_type_support_pcie_only(dbus, mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 06 02' -""" Test RegisterVDMTypeSupport when a single IANA VDM is registered """ async def test_register_vdm_type_support_iana_only(dbus, mctpd, routed_ep): + """Test RegisterVDMTypeSupport when a single IANA VDM is registered""" ep = routed_ep mctp = await mctpd_mctp_base_iface_obj(dbus) @@ -1335,8 +1354,9 @@ async def test_register_vdm_type_support_iana_only(dbus, mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 06 00 ff 01 12 34 ab cd 56 78' -""" Test RegisterVDMTypeSupport when both IANA and PCI types are registered """ async def test_register_vdm_type_support_both(dbus, mctpd, routed_ep): + """Test RegisterVDMTypeSupport when both IANA and PCI types are registered + """ ep = routed_ep mctp = await mctpd_mctp_base_iface_obj(dbus) @@ -1365,8 +1385,8 @@ async def test_register_vdm_type_support_both(dbus, mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 06 00 ff 00 ab cd 56 78' -""" Test RegisterVDMTypeSupport with dbus disconnect """ async def test_register_vdm_type_support_dbus_disconnect(mctpd, routed_ep): + """Test RegisterVDMTypeSupport with dbus disconnect""" ep = routed_ep # Verify error response when no VDM is registered @@ -1409,8 +1429,8 @@ async def test_register_vdm_type_support_dbus_disconnect(mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 05 00 01 00' -""" Test RegisterVDMTypeSupport error handling """ async def test_register_vdm_type_support_errors(dbus, mctpd): + """Test RegisterVDMTypeSupport error handling""" mctp = await mctpd_mctp_base_iface_obj(dbus) # Verify DBus call fails with invalid format 0x02 diff --git a/tests/test_mctpd_endpoint.py b/tests/test_mctpd_endpoint.py index a2b2ce4..bb29d52 100644 --- a/tests/test_mctpd_endpoint.py +++ b/tests/test_mctpd_endpoint.py @@ -36,15 +36,15 @@ async def sysnet(iface): return Sysnet(system, network) -""" Test if mctpd is running as an endpoint """ async def test_endpoint_role(dbus, mctpd): + """Test if mctpd is running as an endpoint""" obj = await mctpd_mctp_iface_control_obj(dbus, mctpd.system.interfaces[0]) role = await obj.get_role() assert str(role) == "Endpoint" -""" mctpd returns null EID on no EID """ async def test_respond_get_eid_with_no_eid(dbus, mctpd): + """mctpd returns null EID on no EID""" bo = mctpd.network.endpoints[0] assert len(mctpd.system.addresses) == 0 @@ -55,8 +55,8 @@ async def test_respond_get_eid_with_no_eid(dbus, mctpd): assert rsp.hex(' ') == '00 02 00 00 02 00' -""" Test if mctpd accepts Set EID when no EID """ async def test_accept_set_eid(dbus, mctpd): + """Test if mctpd accepts Set EID when no EID""" bo = mctpd.network.endpoints[0] assert len(mctpd.system.addresses) == 0 @@ -119,8 +119,8 @@ class TestDiscovery: async def iface(self): return System.Interface("mctp0", 1, 1, bytes([0x1D]), 68, 254, True, PhysicalBinding.PCIE_VDM) - """ Test simple Discovery sequence """ async def test_simple_discovery_sequence(self, dbus, mctpd): + """Test simple Discovery sequence""" bo = mctpd.network.endpoints[0] assert len(mctpd.system.addresses) == 0 @@ -152,8 +152,8 @@ class TestUnsupportedDiscovery: async def iface(self): return System.Interface("mctp0", 1, 1, bytes([0x1D]), 68, 254, True, PhysicalBinding.SMBUS) - """ Discovery command on unsupported interface """ async def test_simple(self, dbus, mctpd): + """Discovery command on unsupported interface""" bo = mctpd.network.endpoints[0] # BMC response ERROR_UNSUPPORTED_CMD to Prepare for Discovery From 37753e020698e0ea17939fc6010b9502ef8bf80d Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Mon, 5 Jan 2026 15:16:45 +0800 Subject: [PATCH 09/10] tests: apply ruff format Now that we have lints passing, add a basic ruff.toml configuration, and reformat accordingly. We leave the quote-style as-is, but may unify this later. Signed-off-by: Jeremy Kerr --- tests/conftest.py | 12 +- tests/mctp_test_utils.py | 47 +++--- tests/mctpenv/__init__.py | 283 ++++++++++++++++++++++------------- tests/ruff.toml | 3 + tests/test_mctp.py | 15 +- tests/test_mctpd.py | 277 ++++++++++++++++++++++------------ tests/test_mctpd_endpoint.py | 103 ++++++++++--- 7 files changed, 483 insertions(+), 257 deletions(-) create mode 100644 tests/ruff.toml diff --git a/tests/conftest.py b/tests/conftest.py index 560227f..cbd53cb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,10 +1,10 @@ - import pytest import asyncdbus import trio.testing import mctpenv + @pytest.fixture async def sysnet(): """Simple system & network. @@ -14,27 +14,32 @@ async def sysnet(): """ return await mctpenv.default_sysnet() + @pytest.fixture async def dbus(): async with asyncdbus.MessageBus().connect() as bus: yield bus + @pytest.fixture def config(): return None + @pytest.fixture async def mctpd(nursery, dbus, sysnet, config): - m = mctpenv.MctpdWrapper(dbus, sysnet, config = config) + m = mctpenv.MctpdWrapper(dbus, sysnet, config=config) await m.start_mctpd(nursery) yield m res = await m.stop_mctpd() assert res == 0 + @pytest.fixture async def mctp(nursery, sysnet): return mctpenv.MctpWrapper(nursery, sysnet) + @pytest.fixture def autojump_clock(): """ @@ -42,6 +47,7 @@ def autojump_clock(): """ return trio.testing.MockClock(autojump_threshold=0.01) + @pytest.fixture async def routed_ep(mctpd): """ @@ -52,7 +58,7 @@ async def routed_ep(mctpd): iface = mctpd.system.interfaces[0] ep.eid = 9 - route = mctpd.system.Route(ep.eid, 1, iface = iface) + route = mctpd.system.Route(ep.eid, 1, iface=iface) neigh = mctpd.system.Neighbour(iface, ep.lladdr, ep.eid) await mctpd.system.add_route(route) await mctpd.system.add_neighbour(neigh) diff --git a/tests/mctp_test_utils.py b/tests/mctp_test_utils.py index 7370536..338c128 100644 --- a/tests/mctp_test_utils.py +++ b/tests/mctp_test_utils.py @@ -1,52 +1,53 @@ async def mctpd_mctp_base_iface_obj(dbus): obj = await dbus.get_proxy_object( - 'au.com.codeconstruct.MCTP1', - '/au/com/codeconstruct/mctp1' - ) + 'au.com.codeconstruct.MCTP1', '/au/com/codeconstruct/mctp1' + ) return await obj.get_interface('au.com.codeconstruct.MCTP1') + async def mctpd_mctp_iface_obj(dbus, iface): obj = await dbus.get_proxy_object( - 'au.com.codeconstruct.MCTP1', - '/au/com/codeconstruct/mctp1/interfaces/' + iface.name - ) + 'au.com.codeconstruct.MCTP1', + '/au/com/codeconstruct/mctp1/interfaces/' + iface.name, + ) return await obj.get_interface('au.com.codeconstruct.MCTP.BusOwner1') + async def mctpd_mctp_iface_control_obj(dbus, iface): obj = await dbus.get_proxy_object( - "au.com.codeconstruct.MCTP1", - "/au/com/codeconstruct/mctp1/interfaces/" + iface.name, - ) + "au.com.codeconstruct.MCTP1", + "/au/com/codeconstruct/mctp1/interfaces/" + iface.name, + ) return await obj.get_interface("au.com.codeconstruct.MCTP.Interface1") + async def mctpd_mctp_endpoint_obj(dbus, path, iface): obj = await dbus.get_proxy_object( - 'au.com.codeconstruct.MCTP1', - path, - ) + 'au.com.codeconstruct.MCTP1', + path, + ) return await obj.get_interface(iface) + async def mctpd_mctp_network_obj(dbus, net): obj = await dbus.get_proxy_object( - 'au.com.codeconstruct.MCTP1', - f'/au/com/codeconstruct/mctp1/networks/{net:d}' - ) + 'au.com.codeconstruct.MCTP1', + f'/au/com/codeconstruct/mctp1/networks/{net:d}', + ) iface = await obj.get_interface('au.com.codeconstruct.MCTP.Network1') # fixup autogenerated snake-case names iface.get_local_eids = iface.get_local_ei_ds iface.set_local_eids = iface.set_local_ei_ds return iface + async def mctpd_mctp_endpoint_control_obj(dbus, path): return await mctpd_mctp_endpoint_obj( - dbus, - path, - 'au.com.codeconstruct.MCTP.Endpoint1' - ) + dbus, path, 'au.com.codeconstruct.MCTP.Endpoint1' + ) + async def mctpd_mctp_endpoint_common_obj(dbus, path): return await mctpd_mctp_endpoint_obj( - dbus, - path, - 'xyz.openbmc_project.MCTP.Endpoint' - ) + dbus, path, 'xyz.openbmc_project.MCTP.Endpoint' + ) diff --git a/tests/mctpenv/__init__.py b/tests/mctpenv/__init__.py index fa9a0dd..e5e0d35 100644 --- a/tests/mctpenv/__init__.py +++ b/tests/mctpenv/__init__.py @@ -1,4 +1,3 @@ - import array import enum import errno @@ -23,13 +22,14 @@ MAX_SOCKADDR_SIZE = 56 + # can be serialised into a NLMSG_ERROR class NetlinkError(Exception): - def __init__(self, errno, msg = None): + def __init__(self, errno, msg=None): self.errno = errno self.msg = msg - def to_nlmsg(self, seq = 0): + def to_nlmsg(self, seq=0): resp = netlink.nlmsgerr() resp['header']['sequence_number'] = seq resp['header']['pid'] = 0 @@ -39,6 +39,7 @@ def to_nlmsg(self, seq = 0): resp['attrs'] = [['NLMSGERR_ATTR_MSG', self.msg]] return resp + class PhysicalBinding(enum.Enum): UNSPEC = 0x00 SMBUS = 0x01 @@ -52,14 +53,25 @@ class PhysicalBinding(enum.Enum): UCIE = 0x09 VENDOR = 0xFF + class System: class Interface: """Interface constructor. Initial mtu is set to max_mtu. """ - def __init__(self, name, ifindex, net, lladdr, min_mtu, max_mtu, - up = False, phys_binding = PhysicalBinding.UNSPEC): + + def __init__( + self, + name, + ifindex, + net, + lladdr, + min_mtu, + max_mtu, + up=False, + phys_binding=PhysicalBinding.UNSPEC, + ): self.name = name self.ifindex = ifindex self.net = net @@ -98,8 +110,7 @@ def __str__(self): return f"{self.eid} -> {lladdrstr} {self.iface.name}" class Route: - def __init__(self, start_eid, extent_eid, iface = None, gw = None, - mtu = 0): + def __init__(self, start_eid, extent_eid, iface=None, gw=None, mtu=0): if (iface is None) and (gw is None): raise ValueError("neither interface or gateway are set") elif (iface is not None) and (gw is not None): @@ -151,8 +162,9 @@ async def add_route(self, route): await self.nl.notify_newroute(route) async def del_route(self, route): - route = self.lookup_route_exact(route.net(), route.start_eid, - route.end_eid) + route = self.lookup_route_exact( + route.net(), route.start_eid, route.end_eid + ) if not route: raise NetlinkError(errno.ENOENT) @@ -232,8 +244,11 @@ def lookup_route(self, net, eid): def lookup_route_exact(self, net, start_eid, end_eid): for rt in self.routes: - if (rt.net() == net and rt.start_eid == start_eid - and rt.end_eid == end_eid): + if ( + rt.net() == net + and rt.start_eid == start_eid + and rt.end_eid == end_eid + ): return rt return None @@ -289,6 +304,7 @@ def dump(self): for n in self.neighbours: print(f" {n}") + class MCTPCommand: def __init__(self): self.send_channel, self.receive_channel = trio.open_memory_channel(0) @@ -301,10 +317,11 @@ async def wait(self): async with self.receive_channel as chan: return await chan.receive() + class MCTPControlCommand(MCTPCommand): MSGTYPE = 0 - def __init__(self, rq, iid, cmd, data = bytes()): + def __init__(self, rq, iid, cmd, data=bytes()): super().__init__() self.rq = rq self.iid = iid @@ -317,15 +334,16 @@ def to_buf(self): flags = flags | 0x80 return bytes([flags, self.cmd]) + self.data + class Endpoint: - def __init__(self, iface, lladdr, ep_uuid = None, eid = 0, types = None): + def __init__(self, iface, lladdr, ep_uuid=None, eid=0, types=None): self.iface = iface self.lladdr = lladdr self.uuid = ep_uuid or uuid.uuid1() self.eid = eid self.types = types or [0] self.bridged_eps = [] - self.allocated_pool = None # or (start, size) + self.allocated_pool = None # or (start, size) # keyed by (type, type-specific-instance) self.commands = {} @@ -354,7 +372,7 @@ async def handle_mctp_message(self, sock, addr, data): async def handle_mctp_control(self, sock, addr, data): flags, opcode = data[0:2] rq = flags & 0x80 - iid = flags & 0x1f + iid = flags & 0x1F if not rq: cmd = self.commands.pop((0, iid), None) @@ -363,7 +381,6 @@ async def handle_mctp_control(self, sock, addr, data): await cmd.complete(data) else: - raddr = MCTPSockAddr.for_ep_resp(self, addr, sock.addr_ext) # Use IID from request, zero Rq and D bits hdr = [iid, opcode] @@ -408,18 +425,26 @@ async def handle_mctp_control(self, sock, addr, data): else: self.allocated_pool = (pool_start, pool_size) # Assign sequential EIDs starting from pool_start - for (n, ep) in enumerate(self.bridged_eps[:pool_size]): + for n, ep in enumerate(self.bridged_eps[:pool_size]): ep.eid = self.allocated_pool[0] + n - data = bytes(hdr + [0x00, alloc_status, - self.allocated_pool[1], self.allocated_pool[0]]) + data = bytes( + hdr + + [ + 0x00, + alloc_status, + self.allocated_pool[1], + self.allocated_pool[0], + ] + ) await sock.send(raddr, data) else: - await sock.send(raddr, bytes(hdr + [0x05])) # unsupported command + await sock.send( + raddr, bytes(hdr + [0x05]) + ) # unsupported command async def send_control(self, sock, cmd): - typ = cmd.MSGTYPE # todo: tag 0 implied addr = MCTPSockAddr(self.iface.net, self.eid, typ, 0x80) @@ -435,6 +460,7 @@ async def send_control(self, sock, cmd): return await cmd.wait() + class Network: def __init__(self): self.endpoints = [] @@ -455,13 +481,12 @@ def register_mctp_socket(self, socket): assert self.mctp_socket is None self.mctp_socket = socket + # MCTP-capable pyroute2 objects class ifinfmsg_mctp(rtnl.ifinfmsg.ifinfmsg): class af_spec(netlink.nla): prefix = 'IFLA_' - nla_map = ( - (AF_MCTP, 'AF_MCTP', 'af_spec_mctp'), - ) + nla_map = ((AF_MCTP, 'AF_MCTP', 'af_spec_mctp'),) class af_spec_mctp(netlink.nla): prefix = 'IFLA_MCTP_' @@ -474,6 +499,7 @@ class af_spec_mctp(netlink.nla): class l2addr(netlink.nla_base): fields = [('value', 's')] + class ifaddrmsg_mctp(rtnl.ifaddrmsg.ifaddrmsg): nla_map = ( ('IFA_UNSPEC', 'hex'), @@ -487,6 +513,7 @@ class ifaddrmsg_mctp(rtnl.ifaddrmsg.ifaddrmsg): ('IFA_FLAGS', 'uint32'), ) + class ndmsg_mctp(rtnl.ndmsg.ndmsg): nla_map = ( ('NDA_UNSPEC', 'none'), @@ -504,6 +531,7 @@ class ndmsg_mctp(rtnl.ndmsg.ndmsg): class lladdr(netlink.nla_base): fields = [('value', 'c')] + class rtmsg_mctp(rtnl.rtmsg.rtmsg): nla_map = ( ('RTA_UNSPEC', 'none'), @@ -535,6 +563,7 @@ class rtmsg_mctp(rtnl.rtmsg.rtmsg): class gateway(netlink.nla_base): fields = [('net', 'I'), ('eid', 'B'), ('__pad', '3x')] + class BaseSocket: msg_fmt = "@I" @@ -563,25 +592,25 @@ async def recv_msg(self, data): # send op addr = data[:MAX_SOCKADDR_SIZE] addrlen = int.from_bytes( - data[MAX_SOCKADDR_SIZE:MAX_SOCKADDR_SIZE+4], - byteorder = sys.byteorder - ) - data = data[MAX_SOCKADDR_SIZE+4:] + data[MAX_SOCKADDR_SIZE : MAX_SOCKADDR_SIZE + 4], + byteorder=sys.byteorder, + ) + data = data[MAX_SOCKADDR_SIZE + 4 :] addr = addr[:addrlen] await self.handle_send(addr, data) elif typ == 2: # setsockopt op level, optname, optval = data[0:4], data[4:8], data[20:] - level = int.from_bytes(level, byteorder = sys.byteorder) - optname = int.from_bytes(optname, byteorder = sys.byteorder) + level = int.from_bytes(level, byteorder=sys.byteorder) + optname = int.from_bytes(optname, byteorder=sys.byteorder) await self.handle_setsockopt(level, optname, optval) elif typ == 3: # bind addr = data[:MAX_SOCKADDR_SIZE] addrlen = int.from_bytes( - data[MAX_SOCKADDR_SIZE:MAX_SOCKADDR_SIZE+4], - byteorder = sys.byteorder - ) + data[MAX_SOCKADDR_SIZE : MAX_SOCKADDR_SIZE + 4], + byteorder=sys.byteorder, + ) addr = addr[:addrlen] await self.handle_bind(addr) @@ -598,9 +627,10 @@ async def send(self, addr, data): async def handle_bind(self, addr): pass + class MCTPSockAddr: base_addr_fmt = "@HHiBBBB" - ext_addr_fmt = "@iB3c" # just the header here, we append the lladdr data + ext_addr_fmt = "@iB3c" # just the header here, we append the lladdr data @classmethod def parse(cls, data, ext): @@ -614,9 +644,9 @@ def parse(cls, data, ext): a = cls(net, eid, type, tag) if ext and addrlen >= extlen + baselen: - ext = data[baselen:baselen + extlen] + ext = data[baselen : baselen + extlen] parts = struct.unpack(cls.ext_addr_fmt, ext) - lladdr = data[baselen + extlen: baselen + extlen + parts[1]] + lladdr = data[baselen + extlen : baselen + extlen + parts[1]] a.set_ext(parts[0], lladdr) return a @@ -640,16 +670,28 @@ def set_ext(self, ifindex, lladdr): self.ifindex = ifindex self.lladdr = lladdr - def to_buf(self): - data = struct.pack(self.base_addr_fmt, - AF_MCTP, 0, self.net, self.eid, self.type, self.tag, 0) + data = struct.pack( + self.base_addr_fmt, + AF_MCTP, + 0, + self.net, + self.eid, + self.type, + self.tag, + 0, + ) if self.is_ext: # pad to MAX_ADDR_LEN lladdr_data = self.lladdr + bytes([0] * (32 - len(self.lladdr))) - data += struct.pack(self.ext_addr_fmt, - self.ifindex, len(self.lladdr), - b'\0', b'\0', b'\0') + data += struct.pack( + self.ext_addr_fmt, + self.ifindex, + len(self.lladdr), + b'\0', + b'\0', + b'\0', + ) data += lladdr_data return data @@ -684,7 +726,7 @@ async def handle_send(self, addr, data): async def handle_setsockopt(self, level, optname, optval): if level == 285 and optname == 1: - val = int.from_bytes(optval, byteorder = sys.byteorder) + val = int.from_bytes(optval, byteorder=sys.byteorder) self.addr_ext = bool(val) async def handle_bind(self, addr): @@ -698,6 +740,7 @@ async def send(self, addr, data): buf = struct.pack("@I", 0) + addrbuf + struct.pack("@I", addrlen) + data await self.sock.send(buf) + class NLSocket(BaseSocket): addr_fmt = "@HHII" @@ -738,7 +781,7 @@ async def _nlmsg_ack(self, req): await self._send_msg(resp.data) async def handle_send(self, addr, data): - addr = addr[:struct.calcsize(self.addr_fmt)] + addr = addr[: struct.calcsize(self.addr_fmt)] addr = struct.unpack(self.addr_fmt, addr) msg = netlink.nlmsg(data) msg.decode() @@ -783,33 +826,39 @@ async def _send_msg(self, buf): await self.send(addr, buf) def _format_link(self, msg, iface): - msg['index'] = iface.ifindex - msg['family'] = 0 - msg['type'] = ARPHRD_MCTP - msg['flags'] = ( - rtnl.ifinfmsg.IFF_RUNNING | - (rtnl.ifinfmsg.IFF_UP | rtnl.ifinfmsg.IFF_LOWER_UP - if iface.up else 0) - ) + msg['index'] = iface.ifindex + msg['family'] = 0 + msg['type'] = ARPHRD_MCTP + msg['flags'] = rtnl.ifinfmsg.IFF_RUNNING | ( + rtnl.ifinfmsg.IFF_UP | rtnl.ifinfmsg.IFF_LOWER_UP if iface.up else 0 + ) - msg['attrs'] = [ - ['IFLA_IFNAME', iface.name], - ['IFLA_ADDRESS', iface.lladdr], - ['IFLA_MTU', iface.mtu], - ['IFLA_MIN_MTU', iface.min_mtu], - ['IFLA_MAX_MTU', iface.max_mtu], - ['IFLA_AF_SPEC', { - 'attrs': [['AF_MCTP', { - 'attrs': [ - ['IFLA_MCTP_NET', iface.net], - [ - 'IFLA_MCTP_PHYS_BINDING', - iface.phys_binding.value, - ], - ], - }]], - }], - ] + msg['attrs'] = [ + ['IFLA_IFNAME', iface.name], + ['IFLA_ADDRESS', iface.lladdr], + ['IFLA_MTU', iface.mtu], + ['IFLA_MIN_MTU', iface.min_mtu], + ['IFLA_MAX_MTU', iface.max_mtu], + [ + 'IFLA_AF_SPEC', + { + 'attrs': [ + [ + 'AF_MCTP', + { + 'attrs': [ + ['IFLA_MCTP_NET', iface.net], + [ + 'IFLA_MCTP_PHYS_BINDING', + iface.phys_binding.value, + ], + ], + }, + ] + ], + }, + ], + ] async def _handle_getlink(self, msg): dump = bool(msg['header']['flags'] & netlink.NLM_F_DUMP) @@ -823,7 +872,9 @@ async def _handle_getlink(self, msg): ifaces = self.system.interfaces for iface in ifaces: - resp = self._create_resp(ifinfmsg_mctp, msg, rtnl.RTM_NEWLINK, flags) + resp = self._create_resp( + ifinfmsg_mctp, msg, rtnl.RTM_NEWLINK, flags + ) self._format_link(resp, iface) resp.encode() buf.extend(resp.data) @@ -866,8 +917,9 @@ async def _handle_getaddr(self, msg): addrs = self.system.addresses for addr in addrs: - resp = self._create_resp(ifaddrmsg_mctp, msg, - rtnl.RTM_NEWADDR, flags) + resp = self._create_resp( + ifaddrmsg_mctp, msg, rtnl.RTM_NEWADDR, flags + ) self._format_addr(resp, addr) resp.encode() buf.extend(resp.data) @@ -1019,17 +1071,25 @@ def _format_route(self, msg, route): msg['src_len'] = 0 msg['attrs'] = [ ['RTA_DST', route.start_eid], - ['RTA_METRICS', { - 'attrs': [['RTAX_MTU', route.mtu]], - }], + [ + 'RTA_METRICS', + { + 'attrs': [['RTAX_MTU', route.mtu]], + }, + ], ] if route.iface: msg['attrs'].append(['RTA_OIF', route.iface.ifindex]) elif route.gw: - msg['attrs'].append(['RTA_GATEWAY', { - "net": route.gw[0], - "eid": route.gw[1], - }]) + msg['attrs'].append( + [ + 'RTA_GATEWAY', + { + "net": route.gw[0], + "eid": route.gw[1], + }, + ] + ) def _parse_route(self, msg): msg = rtmsg_mctp(msg.data) @@ -1048,7 +1108,7 @@ def _parse_route(self, msg): gw = (gw['net'], gw['eid']) iface = None - return System.Route(start_eid, extent_eid, iface = iface, gw = gw) + return System.Route(start_eid, extent_eid, iface=iface, gw=gw) async def _handle_getroute(self, msg): dump = bool(msg['header']['flags'] & netlink.NLM_F_DUMP) @@ -1132,8 +1192,9 @@ async def run(self): # timed out if scope.cancelled_caught: - data = struct.pack('@Q', - math.floor(trio.current_time() * 1000000)) + data = struct.pack( + '@Q', math.floor(trio.current_time() * 1000000) + ) await self.sock.send(data) self.delay = sys.maxsize except (ConnectionResetError, BrokenPipeError): @@ -1142,11 +1203,14 @@ async def run(self): async def send_fd(sock, fd): fdarray = array.array("i", [fd]) - await sock.sendmsg([b'x'], [ + await sock.sendmsg( + [b'x'], + [ (socket.SOL_SOCKET, socket.SCM_RIGHTS, fdarray), - ] + ], ) + class MctpProcessWrapper: def __init__(self, sysnet): self.system = sysnet.system @@ -1155,9 +1219,8 @@ def __init__(self, sysnet): def socketpair(self): return trio.socket.socketpair( - trio.socket.AF_UNIX, - trio.socket.SOCK_SEQPACKET - ) + trio.socket.AF_UNIX, trio.socket.SOCK_SEQPACKET + ) async def handle_control(self, nursery): while True: @@ -1195,6 +1258,7 @@ async def handle_control(self, nursery): else: print(f"unknown op {op}") + class MctpdWrapper(MctpProcessWrapper): def __init__(self, bus, sysnet, binary=None, config=None): super().__init__(sysnet) @@ -1217,8 +1281,9 @@ async def stop_mctpd(self): async def wait_mctpd(self): return await self.proc_rc_recv_chan.receive() - async def mctpd_proc(self, nursery, send_chan, - task_status = trio.TASK_STATUS_IGNORED): + async def mctpd_proc( + self, nursery, send_chan, task_status=trio.TASK_STATUS_IGNORED + ): # We want to start the mctpd process, but not return before it's # ready to interact with our test via dbus. # @@ -1226,12 +1291,12 @@ async def mctpd_proc(self, nursery, send_chan, # the Name Owner Changed signal that indicates that it has registered # itself. busobj = await self.bus.get_proxy_object( - 'org.freedesktop.DBus', - '/org/freedesktop/DBus' - ) + 'org.freedesktop.DBus', '/org/freedesktop/DBus' + ) interface = await busobj.get_interface('org.freedesktop.DBus') - s = trio.Semaphore(initial_value = 0) + s = trio.Semaphore(initial_value=0) + def name_owner_changed(name, new_owner, old_owner): if name == 'au.com.codeconstruct.MCTP1': s.release() @@ -1252,10 +1317,10 @@ def name_owner_changed(name, new_owner, old_owner): command = [self.binary, '-v'] proc = await trio.lowlevel.open_process( - command = command, - pass_fds = (1, 2, self.sock_remote.fileno()), - env = env, - ) + command=command, + pass_fds=(1, 2, self.sock_remote.fileno()), + env=env, + ) self.sock_remote.close() # wait for name to appear, cancel NameOwnerChanged listener @@ -1274,6 +1339,7 @@ def name_owner_changed(name, new_owner, old_owner): await send_chan.send(proc_rc) + class MctpWrapper(MctpProcessWrapper): def __init__(self, nursery, sysnet): super().__init__(sysnet) @@ -1289,13 +1355,13 @@ async def run(self, args): self.nursery.start_soon(self.handle_control, self.nursery) proc = await trio.run_process( - command = command, - pass_fds = (1, 2, self.sock_remote.fileno()), - env = env, - capture_stdout = True, - capture_stderr = True, - check = False, - ) + command=command, + pass_fds=(1, 2, self.sock_remote.fileno()), + env=env, + capture_stdout=True, + capture_stderr=True, + check=False, + ) self.sock_remote.close() # everything is text @@ -1307,6 +1373,7 @@ async def run(self, args): Sysnet = namedtuple('SysNet', ['system', 'network']) + async def default_sysnet(): system = System() iface = System.Interface('mctp0', 1, 1, bytes([0x10]), 68, 254, True) @@ -1314,17 +1381,20 @@ async def default_sysnet(): await system.add_address(System.Address(iface, 8)) network = Network() - network.add_endpoint(Endpoint(iface, bytes([0x1d]), types = [0, 1])) + network.add_endpoint(Endpoint(iface, bytes([0x1D]), types=[0, 1])) return Sysnet(system, network) + async def sighandler(): with trio.open_signal_receiver(signal.SIGINT) as sigs: async for sig in sigs: return + async def main(): import asyncdbus + binary = None if len(sys.argv) > 1: binary = sys.argv[1] @@ -1336,5 +1406,6 @@ async def main(): await mctpd.start_mctpd(nursery) await mctpd.wait_mctpd() + if __name__ == '__main__': trio.run(main) diff --git a/tests/ruff.toml b/tests/ruff.toml new file mode 100644 index 0000000..cc89e9e --- /dev/null +++ b/tests/ruff.toml @@ -0,0 +1,3 @@ +line-length = 80 +[format] +quote-style = 'preserve' diff --git a/tests/test_mctp.py b/tests/test_mctp.py index ddca8ba..29d0ce0 100644 --- a/tests/test_mctp.py +++ b/tests/test_mctp.py @@ -1,41 +1,45 @@ - async def test_link_simple(mctp): proc = await mctp.run(["link"]) assert proc.returncode == 0 assert 'dev mctp0' in proc.stdout + async def test_route_single_direct(mctp): - rt = mctp.system.Route(9, 0, iface = mctp.system.interfaces[0]) + rt = mctp.system.Route(9, 0, iface=mctp.system.interfaces[0]) await mctp.system.add_route(rt) proc = await mctp.run(["route"]) assert proc.returncode == 0 assert proc.stdout.strip() == 'eid min 9 max 9 net 1 dev mctp0 mtu 0' + async def test_route_range_direct(mctp): - rt = mctp.system.Route(9, 1, iface = mctp.system.interfaces[0]) + rt = mctp.system.Route(9, 1, iface=mctp.system.interfaces[0]) await mctp.system.add_route(rt) proc = await mctp.run(["route"]) assert proc.returncode == 0 assert proc.stdout.strip() == 'eid min 9 max 10 net 1 dev mctp0 mtu 0' + async def test_route_single_gw(mctp): - rt = mctp.system.Route(10, 0, gw = (1, 9)) + rt = mctp.system.Route(10, 0, gw=(1, 9)) await mctp.system.add_route(rt) proc = await mctp.run(["route"]) assert proc.returncode == 0 assert proc.stdout.strip() == 'eid min 10 max 10 net 1 gw 9 mtu 0' + async def test_route_range_gw(mctp): - rt = mctp.system.Route(10, 1, gw = (1, 9)) + rt = mctp.system.Route(10, 1, gw=(1, 9)) await mctp.system.add_route(rt) proc = await mctp.run(["route"]) assert proc.returncode == 0 assert proc.stdout.strip() == 'eid min 10 max 11 net 1 gw 9 mtu 0' + async def test_route_add_single_direct(mctp): proc = await mctp.run(["route", "add", "9", "via", "mctp0"]) assert proc.returncode == 0 @@ -48,6 +52,7 @@ async def test_route_add_single_direct(mctp): assert route.end_eid == 9 assert route.mtu == 0 + async def test_route_add_single_gw(mctp): proc = await mctp.run(["route", "add", "10", "gw", "9"]) assert proc.returncode == 0 diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index 68ee53f..08a1020 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -7,7 +7,7 @@ mctpd_mctp_network_obj, mctpd_mctp_endpoint_common_obj, mctpd_mctp_endpoint_control_obj, - mctpd_mctp_base_iface_obj + mctpd_mctp_base_iface_obj, ) from mctpenv import Endpoint, MCTPSockAddr, MCTPControlCommand, MctpdWrapper @@ -26,6 +26,7 @@ MCTPD_TRECLAIM = 5 + async def _introspect_path_recursive(dbus, path, node_set): node_set.add(path) dups = set() @@ -120,6 +121,7 @@ async def test_setup_endpoint(dbus, mctpd): # we should have a route for the new endpoint too assert len(mctpd.system.routes) == 2 + async def test_setup_endpoint_conflict(dbus, mctpd): """Test that we correctly handle address conflicts on EID assignment. @@ -140,12 +142,13 @@ async def test_setup_endpoint_conflict(dbus, mctpd): (eid1, _, _, _) = await mctp.call_setup_endpoint(ep1.lladdr) # endpoint configured with eid1 already - ep2 = Endpoint(iface, bytes([0x1e]), eid=eid1) + ep2 = Endpoint(iface, bytes([0x1E]), eid=eid1) mctpd.network.add_endpoint(ep2) (eid2, _, _, _) = await mctp.call_setup_endpoint(ep2.lladdr) assert eid1 != eid2 + async def test_remove_endpoint(dbus, mctpd): """Test neighbour removal""" iface = mctpd.system.interfaces[0] @@ -154,12 +157,13 @@ async def test_remove_endpoint(dbus, mctpd): mctp = await mctpd_mctp_iface_obj(dbus, iface) (_, _, path, _) = await mctp.call_setup_endpoint(ep1.lladdr) - assert(len(mctpd.system.neighbours) == 1) + assert len(mctpd.system.neighbours) == 1 ep = await mctpd_mctp_endpoint_control_obj(dbus, path) await ep.call_remove() - assert(len(mctpd.system.neighbours) == 0) + assert len(mctpd.system.neighbours) == 0 + async def test_recover_endpoint_present(dbus, mctpd): iface = mctpd.system.interfaces[0] @@ -170,7 +174,8 @@ async def test_recover_endpoint_present(dbus, mctpd): ep = await dbus.get_proxy_object(MCTPD_C, path) ep_props = await ep.get_interface(DBUS_PROPERTIES_I) - recovered = trio.Semaphore(initial_value = 0) + recovered = trio.Semaphore(initial_value=0) + def ep_connectivity_changed(iface, changed, invalidated): if iface == MCTPD_ENDPOINT_I and 'Connectivity' in changed: if 'Available' == changed['Connectivity'].value: @@ -188,6 +193,7 @@ def ep_connectivity_changed(iface, changed, invalidated): # to transition 'Connectivity' to 'Available', which is a test failure. assert not expected.cancelled_caught + async def test_recover_endpoint_removed(dbus, mctpd, autojump_clock): iface = mctpd.system.interfaces[0] dev = mctpd.network.endpoints[0] @@ -198,7 +204,8 @@ async def test_recover_endpoint_removed(dbus, mctpd, autojump_clock): ep = await dbus.get_proxy_object(MCTPD_C, path) ep_props = await ep.get_interface(DBUS_PROPERTIES_I) - degraded = trio.Semaphore(initial_value = 0) + degraded = trio.Semaphore(initial_value=0) + def ep_connectivity_changed(iface, changed, invalidated): if iface == MCTPD_ENDPOINT_I and 'Connectivity' in changed: if 'Degraded' == changed['Connectivity'].value: @@ -208,7 +215,8 @@ def ep_connectivity_changed(iface, changed, invalidated): mctp_objmgr = await mctp.get_interface(DBUS_OBJECT_MANAGER_I) - removed = trio.Semaphore(initial_value = 0) + removed = trio.Semaphore(initial_value=0) + def ep_removed(ep_path, interfaces): if ep_path == path and MCTPD_ENDPOINT_I in interfaces: removed.release() @@ -225,6 +233,7 @@ def ep_removed(ep_path, interfaces): assert not expected.cancelled_caught + async def test_recover_endpoint_reset(dbus, mctpd, autojump_clock): iface = mctpd.system.interfaces[0] dev = mctpd.network.endpoints[0] @@ -234,7 +243,8 @@ async def test_recover_endpoint_reset(dbus, mctpd, autojump_clock): ep = await dbus.get_proxy_object(MCTPD_C, path) ep_props = await ep.get_interface(DBUS_PROPERTIES_I) - recovered = trio.Semaphore(initial_value = 0) + recovered = trio.Semaphore(initial_value=0) + def ep_connectivity_changed(iface, changed, invalidated): if iface == MCTPD_ENDPOINT_I and 'Connectivity' in changed: if 'Available' == changed['Connectivity'].value: @@ -260,6 +270,7 @@ def ep_connectivity_changed(iface, changed, invalidated): assert not expected.cancelled_caught + async def test_recover_endpoint_exchange(dbus, mctpd, autojump_clock): iface = mctpd.system.interfaces[0] dev = mctpd.network.endpoints[0] @@ -270,7 +281,8 @@ async def test_recover_endpoint_exchange(dbus, mctpd, autojump_clock): ep = await dbus.get_proxy_object(MCTPD_C, path) ep_props = await ep.get_interface(DBUS_PROPERTIES_I) - degraded = trio.Semaphore(initial_value = 0) + degraded = trio.Semaphore(initial_value=0) + def ep_connectivity_changed(iface, changed, invalidated): if iface == MCTPD_ENDPOINT_I and 'Connectivity' in changed: if 'Degraded' == changed['Connectivity'].value: @@ -280,14 +292,16 @@ def ep_connectivity_changed(iface, changed, invalidated): mctp_objmgr = await mctp.get_interface(DBUS_OBJECT_MANAGER_I) - removed = trio.Semaphore(initial_value = 0) + removed = trio.Semaphore(initial_value=0) + def ep_removed(ep_path, interfaces): if ep_path == path and MCTPD_ENDPOINT_I in interfaces: removed.release() await mctp_objmgr.on_interfaces_removed(ep_removed) - added = trio.Semaphore(initial_value = 0) + added = trio.Semaphore(initial_value=0) + def ep_added(ep_path, content): if MCTPD_ENDPOINT_I in content: added.release() @@ -304,7 +318,7 @@ def ep_added(ep_path, content): await trio.sleep(1) # Add a new the endpoint device at the same physical address (different UUID) - mctpd.network.add_endpoint(Endpoint(dev.iface, dev.lladdr, types = dev.types)) + mctpd.network.add_endpoint(Endpoint(dev.iface, dev.lladdr, types=dev.types)) with trio.move_on_after(2 * MCTPD_TRECLAIM) as expected: await added.acquire() @@ -313,6 +327,7 @@ def ep_added(ep_path, content): assert not expected.cancelled_caught + async def test_assign_endpoint_static(dbus, mctpd): """Test that we get the correct EID allocated (and the usual route/neigh setup) on an AssignEndpointStatic call @@ -323,8 +338,7 @@ async def test_assign_endpoint_static(dbus, mctpd): static_eid = 12 (eid, _, _, new) = await mctp.call_assign_endpoint_static( - dev.lladdr, - static_eid + dev.lladdr, static_eid ) assert eid == static_eid @@ -336,6 +350,7 @@ async def test_assign_endpoint_static(dbus, mctpd): assert neigh.eid == static_eid assert len(mctpd.system.routes) == 2 + async def test_assign_endpoint_static_allocated(dbus, mctpd): """Test that we can repeat an AssignEndpointStatic call with the same static EID @@ -362,13 +377,14 @@ async def test_assign_endpoint_static_allocated(dbus, mctpd): assert eid == static_eid assert not new + async def test_assign_endpoint_static_conflict(dbus, mctpd): """Test that we cannot assign a conflicting static EID""" iface = mctpd.system.interfaces[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) dev1 = mctpd.network.endpoints[0] - dev2 = Endpoint(iface, bytes([0x1e])) + dev2 = Endpoint(iface, bytes([0x1E])) mctpd.network.add_endpoint(dev2) # dynamic EID assigment for dev1 @@ -384,6 +400,7 @@ async def test_assign_endpoint_static_conflict(dbus, mctpd): assert str(ex.value) == "Address in use" + async def test_assign_endpoint_static_varies(dbus, mctpd): """Test that we cannot re-assign a static EID to an endpoint that already has a different EID allocated @@ -394,8 +411,7 @@ async def test_assign_endpoint_static_varies(dbus, mctpd): static_eid = 12 (eid, _, _, new) = await mctp.call_assign_endpoint_static( - dev.lladdr, - static_eid + dev.lladdr, static_eid ) assert eid == static_eid @@ -406,6 +422,7 @@ async def test_assign_endpoint_static_varies(dbus, mctpd): assert str(ex.value) == "Already assigned a different EID" + async def test_get_endpoint_id(dbus, mctpd, routed_ep): """Test that the mctpd control protocol responder support has support for a basic Get Endpoint ID command @@ -422,6 +439,7 @@ async def test_get_endpoint_id(dbus, mctpd, routed_ep): # EID matches the system assert rsp[3] == mctpd.system.addresses[0].eid + async def test_response_iid(mctpd): """Test that instance ID is populated correctly on control protocol responses @@ -432,22 +450,24 @@ async def test_response_iid(mctpd): rsp = await peer.send_control(mctpd.network.mctp_socket, cmd) assert rsp[0] == iid + async def test_learn_endpoint_invalid_response_command(dbus, mctpd): """During a LearnEndpoint's Get Endpoint ID exchange, return a response from a different command; in this case Get Message Type Support, which happens to be the same length as a the expected Get Endpoint ID response. """ + class BusyEndpoint(Endpoint): async def handle_mctp_control(self, sock, src_addr, msg): flags, opcode = msg[0:2] if opcode != 2: return await super().handle_mctp_control(sock, src_addr, msg) dst_addr = MCTPSockAddr.for_ep_resp(self, src_addr, sock.addr_ext) - msg = bytes([flags & 0x1f, 0x05, 0x00, 0x02, 0x00, 0x01]) + msg = bytes([flags & 0x1F, 0x05, 0x00, 0x02, 0x00, 0x01]) await sock.send(dst_addr, msg) iface = mctpd.system.interfaces[0] - ep = BusyEndpoint(iface, bytes([0x1e]), eid = 15) + ep = BusyEndpoint(iface, bytes([0x1E]), eid=15) mctpd.network.add_endpoint(ep) mctp = await mctpd_mctp_iface_obj(dbus, iface) @@ -456,11 +476,13 @@ async def handle_mctp_control(self, sock, src_addr, msg): assert str(ex.value) == "Request failed" + async def test_setup_endpoint_invalid_set_eid_response(dbus, mctpd): """During a SetupEndpoint's Set Endpoint ID exchange, return a response that indicates that the EID has been set, but report an invalid (0) EID in the response. """ + class InvalidEndpoint(Endpoint): async def handle_mctp_control(self, sock, src_addr, msg): flags, opcode = msg[0:2] @@ -468,18 +490,20 @@ async def handle_mctp_control(self, sock, src_addr, msg): return await super().handle_mctp_control(sock, src_addr, msg) dst_addr = MCTPSockAddr.for_ep_resp(self, src_addr, sock.addr_ext) self.eid = msg[3] - msg = bytes([ - flags & 0x1f, # Rsp - 0x01, # opcode: Set Endpoint ID - 0x00, # cc: success - 0x00, # assignment accepted, no pool - 0x00, # set EID: invalid - 0x00, # pool size: 0 - ]) + msg = bytes( + [ + flags & 0x1F, # Rsp + 0x01, # opcode: Set Endpoint ID + 0x00, # cc: success + 0x00, # assignment accepted, no pool + 0x00, # set EID: invalid + 0x00, # pool size: 0 + ] + ) await sock.send(dst_addr, msg) iface = mctpd.system.interfaces[0] - ep = InvalidEndpoint(iface, bytes([0x1e]), eid = 0) + ep = InvalidEndpoint(iface, bytes([0x1E]), eid=0) mctpd.network.add_endpoint(ep) mctp = await mctpd_mctp_iface_obj(dbus, iface) @@ -488,11 +512,13 @@ async def handle_mctp_control(self, sock, src_addr, msg): assert str(ex.value) == "Endpoint returned failure to Set Endpoint ID" + async def test_setup_endpoint_vary_set_eid_response(dbus, mctpd): """During a SetupEndpoint's Set Endpoint ID exchange, return a response that indicates that the EID has been set, but report a different set EID in the response. """ + class VaryEndpoint(Endpoint): async def handle_mctp_control(self, sock, src_addr, msg): flags, opcode = msg[0:2] @@ -500,18 +526,20 @@ async def handle_mctp_control(self, sock, src_addr, msg): return await super().handle_mctp_control(sock, src_addr, msg) dst_addr = MCTPSockAddr.for_ep_resp(self, src_addr, sock.addr_ext) self.eid = msg[3] + 1 - msg = bytes([ - flags & 0x1f, # Rsp - 0x01, # opcode: Set Endpoint ID - 0x00, # cc: success - 0x00, # assignment accepted, no pool - self.eid, # set EID: valid, but not what was assigned - 0x00, # pool size: 0 - ]) + msg = bytes( + [ + flags & 0x1F, # Rsp + 0x01, # opcode: Set Endpoint ID + 0x00, # cc: success + 0x00, # assignment accepted, no pool + self.eid, # set EID: valid, but not what was assigned + 0x00, # pool size: 0 + ] + ) await sock.send(dst_addr, msg) iface = mctpd.system.interfaces[0] - ep = VaryEndpoint(iface, bytes([0x1e])) + ep = VaryEndpoint(iface, bytes([0x1E])) mctpd.network.add_endpoint(ep) mctp = await mctpd_mctp_iface_obj(dbus, iface) @@ -519,6 +547,7 @@ async def handle_mctp_control(self, sock, src_addr, msg): assert eid == ep.eid + async def test_setup_endpoint_conflicting_set_eid_response(dbus, mctpd): """During a SetupEndpoint's Set Endpoint ID exchange, return a response that indicates that the EID has been set, but report a different set EID in @@ -537,14 +566,16 @@ async def handle_mctp_control(self, sock, src_addr, msg): dst_addr = MCTPSockAddr.for_ep_resp(self, src_addr, sock.addr_ext) # reject reality, use a conflicting eid self.eid = self.conflict_eid - msg = bytes([ - flags & 0x1f, # Rsp - 0x01, # opcode: Set Endpoint ID - 0x00, # cc: success - 0x00, # assignment accepted, no pool - self.eid, # set EID: valid, but not what was assigned - 0x00, # pool size: 0 - ]) + msg = bytes( + [ + flags & 0x1F, # Rsp + 0x01, # opcode: Set Endpoint ID + 0x00, # cc: success + 0x00, # assignment accepted, no pool + self.eid, # set EID: valid, but not what was assigned + 0x00, # pool size: 0 + ] + ) await sock.send(dst_addr, msg) iface = mctpd.system.interfaces[0] @@ -553,26 +584,28 @@ async def handle_mctp_control(self, sock, src_addr, msg): (eid1, _, _, _) = await mctp.call_setup_endpoint(ep1.lladdr) assert eid1 == ep1.eid - ep2 = ConflictingEndpoint(iface, bytes([0x1f]), ep1.eid) + ep2 = ConflictingEndpoint(iface, bytes([0x1F]), ep1.eid) mctpd.network.add_endpoint(ep2) with pytest.raises(asyncdbus.errors.DBusError) as ex: await mctp.call_setup_endpoint(ep2.lladdr) assert "already used" in str(ex.value) + async def test_learn_endpoint_invalid_response_iid(dbus, mctpd): """Ensure a response with an invalid IID is discarded""" + class InvalidIIDEndpoint(Endpoint): async def handle_mctp_control(self, sock, src_addr, msg): # bump IID flags = msg[0] - iid_mask = 0x1d + iid_mask = 0x1D flags = (flags & ~iid_mask) | ((flags + 1) & iid_mask) msg = bytes([flags]) + msg[1:] return await super().handle_mctp_control(sock, src_addr, msg) iface = mctpd.system.interfaces[0] - ep = InvalidIIDEndpoint(iface, bytes([0x1e]), eid = 15) + ep = InvalidIIDEndpoint(iface, bytes([0x1E]), eid=15) mctpd.network.add_endpoint(ep) mctp = await mctpd_mctp_iface_obj(dbus, iface) @@ -581,6 +614,7 @@ async def handle_mctp_control(self, sock, src_addr, msg): assert str(ex.value) == "Request failed" + async def test_query_message_types(dbus, mctpd): """Ensure we're parsing Get Message Type Support responses""" iface = mctpd.system.interfaces[0] @@ -602,6 +636,7 @@ async def test_query_message_types(dbus, mctpd): assert ep_types == query_types + async def test_network_local_eids_single(dbus, mctpd): """Network1.LocalEIDs should reflect locally-assigned EID state""" iface = mctpd.system.interfaces[0] @@ -611,6 +646,7 @@ async def test_network_local_eids_single(dbus, mctpd): assert eids == [8] + async def test_network_local_eids_multiple(dbus, mctpd): iface = mctpd.system.interfaces[0] await mctpd.system.add_address(mctpd.system.Address(iface, 9)) @@ -620,6 +656,7 @@ async def test_network_local_eids_multiple(dbus, mctpd): assert eids == [8, 9] + async def test_network_local_eids_none(dbus, mctpd): iface = mctpd.system.interfaces[0] await mctpd.system.del_address(mctpd.system.Address(iface, 8)) @@ -629,6 +666,7 @@ async def test_network_local_eids_none(dbus, mctpd): assert eids == [] + async def test_concurrent_recovery_setup(dbus, mctpd, autojump_clock): iface = mctpd.system.interfaces[0] mctp_i = await mctpd_mctp_iface_obj(dbus, iface) @@ -637,7 +675,7 @@ async def test_concurrent_recovery_setup(dbus, mctpd, autojump_clock): # reach the allocation boundary. split = 19 for i in range(split): - pep = Endpoint(iface, bytes([0x1e + i])) + pep = Endpoint(iface, bytes([0x1E + i])) mctpd.network.add_endpoint(pep) (_, _, path, _) = await mctp_i.call_setup_endpoint(pep.lladdr) @@ -649,17 +687,20 @@ async def test_concurrent_recovery_setup(dbus, mctpd, autojump_clock): # Set up a match for Connectivity transitioning to Degraded on the endpoint # for which we request recovery - degraded = trio.Semaphore(initial_value = 0) + degraded = trio.Semaphore(initial_value=0) + def ep_connectivity_changed(iface, changed, invalidated): if iface == MCTPD_ENDPOINT_I and 'Connectivity' in changed: if 'Degraded' == changed['Connectivity'].value: degraded.release() + await ep_props.on_properties_changed(ep_connectivity_changed) # Set up a match for the recovery endpoint object being removed from DBus mctp_p = await dbus.get_proxy_object(MCTPD_C, MCTPD_MCTP_P) mctp_objmgr = await mctp_p.get_interface(DBUS_OBJECT_MANAGER_I) - removed = trio.Semaphore(initial_value = 0) + removed = trio.Semaphore(initial_value=0) + def ep_removed(ep_path, interfaces): if ep_path == path and MCTPD_ENDPOINT_I in interfaces: removed.release() @@ -683,7 +724,7 @@ def ep_removed(ep_path, interfaces): # Now that we're asynchronously waiting for the endpoint recovery process # to complete, force a realloc() of the peer object array by adding a new # peer, which will invalidate the recovering peer's pointer - pep = Endpoint(iface, bytes([0x1e + split])) + pep = Endpoint(iface, bytes([0x1E + split])) mctpd.network.add_endpoint(pep) (_, _, _, new) = await mctp_i.call_setup_endpoint(pep.lladdr) assert new @@ -694,24 +735,30 @@ def ep_removed(ep_path, interfaces): await removed.acquire() assert not expected.cancelled_caught + async def test_bridged_learn_endpoint(dbus, mctpd): """Bridged EP can be discovered via Network1.LearnEndpoint""" iface = mctpd.system.interfaces[0] ep = mctpd.network.endpoints[0] - br_ep = Endpoint(iface, bytes(), eid = 10, types = [0, 2]) + br_ep = Endpoint(iface, bytes(), eid=10, types=[0, 2]) ep.add_bridged_ep(br_ep) mctpd.network.add_endpoint(br_ep) - await mctpd.system.add_route(mctpd.system.Route(br_ep.eid, 1, iface = iface)) + await mctpd.system.add_route(mctpd.system.Route(br_ep.eid, 1, iface=iface)) # static neighbour; no gateway route support at present - await mctpd.system.add_neighbour(mctpd.system.Neighbour(iface, ep.lladdr, br_ep.eid)) + await mctpd.system.add_neighbour( + mctpd.system.Neighbour(iface, ep.lladdr, br_ep.eid) + ) net = await mctpd_mctp_network_obj(dbus, iface.net) (path, new) = await net.call_learn_endpoint(br_ep.eid) - assert path == f'/au/com/codeconstruct/mctp1/networks/1/endpoints/{br_ep.eid}' + assert ( + path == f'/au/com/codeconstruct/mctp1/networks/1/endpoints/{br_ep.eid}' + ) assert new + async def test_network_learn_endpoint_absent(dbus, mctpd): iface = mctpd.system.interfaces[0] @@ -720,6 +767,7 @@ async def test_network_learn_endpoint_absent(dbus, mctpd): with pytest.raises(asyncdbus.errors.DBusError): await net.call_learn_endpoint(10) + async def test_change_network(dbus, mctpd): """Change a network id, while we have an active endpoint on that net""" iface = mctpd.system.interfaces[0] @@ -738,14 +786,18 @@ async def test_change_network(dbus, mctpd): # and nothing at 1 with pytest.raises(asyncdbus.errors.DBusError) as ex: await mctpd_mctp_network_obj(dbus, 1) - assert str(ex.value) == "Unknown object '/au/com/codeconstruct/mctp1/networks/1'." + assert ( + str(ex.value) + == "Unknown object '/au/com/codeconstruct/mctp1/networks/1'." + ) # endpoint should be present under 2/ - ep = await mctpd_mctp_endpoint_common_obj(dbus, - '/au/com/codeconstruct/mctp1/networks/2/endpoints/8' + ep = await mctpd_mctp_endpoint_common_obj( + dbus, '/au/com/codeconstruct/mctp1/networks/2/endpoints/8' ) assert ep is not None + async def test_del_interface_last(dbus, mctpd): """Delete our only interface""" iface = mctpd.system.interfaces[0] @@ -759,11 +811,18 @@ async def test_del_interface_last(dbus, mctpd): with pytest.raises(asyncdbus.errors.DBusError): await mctpd_mctp_network_obj(dbus, iface.net) + async def test_del_interface_with_peers(dbus, mctpd): """Delete an interface with peers attached, ensure all are gone""" net = mctpd.system.interfaces[0].net iface = mctpd.system.Interface( - 'mctp1', 2, net, bytes([0x10]), 68, 254, True, + 'mctp1', + 2, + net, + bytes([0x10]), + 68, + 254, + True, ) await mctpd.system.add_interface(iface) @@ -797,6 +856,7 @@ async def test_del_interface_with_peers(dbus, mctpd): ep = await mctpd_mctp_endpoint_common_obj(dbus, path) assert str(ex.value).startswith("Unknown object") + async def test_add_interface(dbus, mctpd): """Remove and re-add an interface""" net = 1 @@ -807,12 +867,11 @@ async def test_add_interface(dbus, mctpd): mctp = await mctpd_mctp_iface_obj(dbus, iface) # Add an endpoint on the interface - mctpd.network.add_endpoint(Endpoint(iface, bytes([]), types = [0, 1])) + mctpd.network.add_endpoint(Endpoint(iface, bytes([]), types=[0, 1])) static_eid = 30 (eid, _, _, new) = await mctp.call_assign_endpoint_static( - bytes([]), - static_eid + bytes([]), static_eid ) assert eid == static_eid assert new @@ -833,20 +892,20 @@ async def test_add_interface(dbus, mctpd): mctp = await mctpd_mctp_iface_obj(dbus, iface) # Add an endpoint on the interface - mctpd.network.add_endpoint(Endpoint(iface, bytes([]), types = [0, 1])) + mctpd.network.add_endpoint(Endpoint(iface, bytes([]), types=[0, 1])) # Old route should still be gone assert mctpd.system.lookup_route(net, static_eid) is None static_eid = 40 (eid, _, _, new) = await mctp.call_assign_endpoint_static( - bytes([]), - static_eid + bytes([]), static_eid ) assert eid == static_eid assert new assert mctpd.system.lookup_route(net, static_eid).iface == iface + async def test_interface_rename(dbus, mctpd): iface = mctpd.system.interfaces[0] iface_obj = await mctpd_mctp_iface_obj(dbus, iface) @@ -859,6 +918,7 @@ async def test_interface_rename(dbus, mctpd): iface_obj = await mctpd_mctp_iface_obj(dbus, iface) assert iface_obj.path.endswith(new_name) + async def test_interface_rename_with_peers(dbus, mctpd): iface = mctpd.system.interfaces[0] ep = mctpd.network.endpoints[0] @@ -881,6 +941,7 @@ async def test_interface_rename_with_peers(dbus, mctpd): ep_obj = await dbus.get_proxy_object(MCTPD_C, ep_path) assert ep_obj is not None + async def test_config_dyn_eid_range_min(nursery, dbus, sysnet): """Test that we use the minimum EID from the dynamic_eid_range config""" (min_dyn_eid, max_dyn_eid) = (20, 254) @@ -891,7 +952,7 @@ async def test_config_dyn_eid_range_min(nursery, dbus, sysnet): # since we're specifying per-test config, we create the wrapper directly # rather than using the fixture. - mctpd = MctpdWrapper(dbus, sysnet, config = config) + mctpd = MctpdWrapper(dbus, sysnet, config=config) await mctpd.start_mctpd(nursery) iface = mctpd.system.interfaces[0] @@ -905,6 +966,7 @@ async def test_config_dyn_eid_range_min(nursery, dbus, sysnet): res = await mctpd.stop_mctpd() assert res == 0 + async def test_config_dyn_eid_range_max(nursery, dbus, sysnet): """Test that we use the maximum EID from the dynamic_eid_range config""" (min_dyn_eid, max_dyn_eid) = (20, 21) @@ -913,14 +975,14 @@ async def test_config_dyn_eid_range_max(nursery, dbus, sysnet): dynamic_eid_range = [{min_dyn_eid}, {max_dyn_eid}] """ - mctpd = MctpdWrapper(dbus, sysnet, config = config) + mctpd = MctpdWrapper(dbus, sysnet, config=config) await mctpd.start_mctpd(nursery) iface = mctpd.system.interfaces[0] mctp = await mctpd_mctp_iface_obj(dbus, iface) - mctpd.network.add_endpoint(Endpoint(iface, bytes([0x01]), types = [0, 1])) - mctpd.network.add_endpoint(Endpoint(iface, bytes([0x02]), types = [0, 1])) + mctpd.network.add_endpoint(Endpoint(iface, bytes([0x01]), types=[0, 1])) + mctpd.network.add_endpoint(Endpoint(iface, bytes([0x02]), types=[0, 1])) for i in range(0, 2): ep = mctpd.network.endpoints[i] @@ -938,8 +1000,9 @@ async def test_config_dyn_eid_range_max(nursery, dbus, sysnet): res = await mctpd.stop_mctpd() assert res == 0 + async def test_assign_dynamic_bridge_eid(dbus, mctpd): - """ Test bridge endpoint dynamic EID assignment and downstream + """Test bridge endpoint dynamic EID assignment and downstream endpoint EID allocation Tests that: @@ -963,6 +1026,7 @@ async def test_assign_dynamic_bridge_eid(dbus, mctpd): assert new assert ep.allocated_pool == (eid + 1, pool_size) + async def test_bridge_ep_conflict_static(dbus, mctpd): """Test that static allocations are not permitted, if they would conflict with a bridge pool @@ -997,6 +1061,7 @@ async def test_bridge_ep_conflict_static(dbus, mctpd): assert eid == static_eid + async def test_bridge_ep_conflict_learn(dbus, mctpd): """Test that learnt allocations (ie, pre-assigned device EIDs) are not permitted, if they would conflict with a bridge pool @@ -1029,6 +1094,7 @@ async def test_bridge_ep_conflict_learn(dbus, mctpd): assert eid == dev_eid + async def test_bridge_ep_conflict_setup(dbus, mctpd): """Test that learnt allocations (ie, pre-assigned device EIDs) are not permitted through SetupEndpoint, if they would conflict with a bridge pool @@ -1055,6 +1121,7 @@ async def test_bridge_ep_conflict_setup(dbus, mctpd): (eid, _, _, _) = await mctp.call_setup_endpoint(dev.lladdr) assert eid not in pool_range + async def test_bridge_setup_reassign(dbus, mctpd): """Test that mctpd will reassign a bridge endpoints (pre-configured) EID if necessary to satisfy the bridge pool allocation @@ -1066,8 +1133,7 @@ async def test_bridge_setup_reassign(dbus, mctpd): ep = mctpd.network.endpoints[0] static_eid = 10 (eid, _, _, _) = await mctp.call_assign_endpoint_static( - ep.lladdr, - static_eid + ep.lladdr, static_eid ) assert eid == static_eid @@ -1083,6 +1149,7 @@ async def test_bridge_setup_reassign(dbus, mctpd): assert br.allocated_pool is not None assert br.allocated_pool[0] == eid + 1 + async def test_assign_dynamic_eid_limited_pool(nursery, dbus, sysnet): """Test that we truncate the requested pool size to the max_pool_size config @@ -1093,7 +1160,7 @@ async def test_assign_dynamic_eid_limited_pool(nursery, dbus, sysnet): max_pool_size = {max_pool_size} """ - mctpd = MctpdWrapper(dbus, sysnet, config = config) + mctpd = MctpdWrapper(dbus, sysnet, config=config) await mctpd.start_mctpd(nursery) iface = mctpd.system.interfaces[0] @@ -1120,6 +1187,7 @@ async def test_assign_dynamic_eid_limited_pool(nursery, dbus, sysnet): res = await mctpd.stop_mctpd() assert res == 0 + async def test_bridge_pool_assign_limited(nursery, dbus, sysnet): """Test that a limited pool is assigned if we run out of space for a full allocation @@ -1130,7 +1198,7 @@ async def test_bridge_pool_assign_limited(nursery, dbus, sysnet): dynamic_eid_range = [{min_dyn_eid}, {max_dyn_eid}] """ - mctpd = MctpdWrapper(dbus, sysnet, config = config) + mctpd = MctpdWrapper(dbus, sysnet, config=config) await mctpd.start_mctpd(nursery) iface = mctpd.system.interfaces[0] @@ -1148,8 +1216,7 @@ async def test_bridge_pool_assign_limited(nursery, dbus, sysnet): dev2 = Endpoint(iface, bytes([0x09])) mctpd.network.add_endpoint(dev2) (eid, _, path, new) = await mctp.call_assign_endpoint_static( - dev2.lladdr, - 10 + dev2.lladdr, 10 ) assert new @@ -1164,10 +1231,12 @@ async def test_bridge_pool_assign_limited(nursery, dbus, sysnet): res = await mctpd.stop_mctpd() assert res == 0 + async def test_assign_dynamic_eid_allocation_failure(dbus, mctpd): """During Allocate Endpoint ID exchange, return completion code failure to indicate no pool has been assigned to the bridge """ + class BridgeEndpoint(Endpoint): async def handle_mctp_control(self, sock, src_addr, msg): flags, opcode = msg[0:2] @@ -1175,18 +1244,20 @@ async def handle_mctp_control(self, sock, src_addr, msg): return await super().handle_mctp_control(sock, src_addr, msg) dst_addr = MCTPSockAddr.for_ep_resp(self, src_addr, sock.addr_ext) - msg = bytes([ - flags & 0x1f, # Rsp - 0x08, # opcode: Allocate Endpoint ID - 0x01, # cc: failure - 0x01, # allocation rejected - 0x00, # pool size - 0x00, # pool start - ]) + msg = bytes( + [ + flags & 0x1F, # Rsp + 0x08, # opcode: Allocate Endpoint ID + 0x01, # cc: failure + 0x01, # allocation rejected + 0x00, # pool size + 0x00, # pool start + ] + ) await sock.send(dst_addr, msg) iface = mctpd.system.interfaces[0] - ep = BridgeEndpoint(iface, bytes([0x1e])) + ep = BridgeEndpoint(iface, bytes([0x1E])) mctpd.network.add_endpoint(ep) # Set up downstream endpoints as undiscovered EID 0 for i in range(0, 2): @@ -1203,6 +1274,7 @@ async def handle_mctp_control(self, sock, src_addr, msg): bridge_obj = await dbus.get_proxy_object(MCTPD_C, path) await bridge_obj.get_interface(MCTPD_ENDPOINT_BRIDGE_I) + async def test_assign_without_bridge_range(dbus, sysnet, nursery): """Test assigning a non-bridge endpoint, when we don't have capacity for the speculatively-allocated bridge range @@ -1215,7 +1287,7 @@ async def test_assign_without_bridge_range(dbus, sysnet, nursery): max_pool_size = {max_pool_size} """ - mctpd = MctpdWrapper(dbus, sysnet, config = config) + mctpd = MctpdWrapper(dbus, sysnet, config=config) await mctpd.start_mctpd(nursery) iface = mctpd.system.interfaces[0] @@ -1229,6 +1301,7 @@ async def test_assign_without_bridge_range(dbus, sysnet, nursery): res = await mctpd.stop_mctpd() assert res == 0 + async def test_bridge_pool_range_limited(dbus, sysnet, nursery): """Test that we can still allocate a bridge pool even though we may not have the maximum EID range available. The bridge pool's full allocation is still @@ -1250,7 +1323,7 @@ async def test_bridge_pool_range_limited(dbus, sysnet, nursery): max_pool_size = {max_pool_size} """ - mctpd = MctpdWrapper(dbus, sysnet, config = config) + mctpd = MctpdWrapper(dbus, sysnet, config=config) await mctpd.start_mctpd(nursery) iface = mctpd.system.interfaces[0] @@ -1272,6 +1345,7 @@ async def test_bridge_pool_range_limited(dbus, sysnet, nursery): res = await mctpd.stop_mctpd() assert res == 0 + async def test_get_message_types(dbus, mctpd, routed_ep): ep = routed_ep @@ -1289,7 +1363,7 @@ async def test_get_message_types(dbus, mctpd, routed_ep): await mctp.call_register_type_support(0x0, [0xF1F2F3F4]) assert str(ex.value) == "Invalid message type 0" with pytest.raises(asyncdbus.errors.DBusError) as ex: - await mctp.call_register_type_support(0x7e, [0xF1F2F3F4]) + await mctp.call_register_type_support(0x7E, [0xF1F2F3F4]) assert str(ex.value) == "Invalid message type 126" # Verify get message type response includes spdm @@ -1302,6 +1376,7 @@ async def test_get_message_types(dbus, mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 04 00 01 f4 f3 f2 f1' + async def test_register_vdm_type_support_empty(mctpd, routed_ep): """Test RegisterVDMTypeSupport when no responders are registered""" ep = routed_ep @@ -1311,6 +1386,7 @@ async def test_register_vdm_type_support_empty(mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 06 02' + async def test_register_vdm_type_support_pcie_only(dbus, mctpd, routed_ep): """Test RegisterVDMTypeSupport when a single PCIe VDM is registered""" ep = routed_ep @@ -1335,6 +1411,7 @@ async def test_register_vdm_type_support_pcie_only(dbus, mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 06 02' + async def test_register_vdm_type_support_iana_only(dbus, mctpd, routed_ep): """Test RegisterVDMTypeSupport when a single IANA VDM is registered""" ep = routed_ep @@ -1354,9 +1431,9 @@ async def test_register_vdm_type_support_iana_only(dbus, mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 06 00 ff 01 12 34 ab cd 56 78' + async def test_register_vdm_type_support_both(dbus, mctpd, routed_ep): - """Test RegisterVDMTypeSupport when both IANA and PCI types are registered - """ + """Test RegisterVDMTypeSupport when both IANA and PCI types are registered""" ep = routed_ep mctp = await mctpd_mctp_base_iface_obj(dbus) @@ -1385,6 +1462,7 @@ async def test_register_vdm_type_support_both(dbus, mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 06 00 ff 00 ab cd 56 78' + async def test_register_vdm_type_support_dbus_disconnect(mctpd, routed_ep): """Test RegisterVDMTypeSupport with dbus disconnect""" ep = routed_ep @@ -1429,6 +1507,7 @@ async def test_register_vdm_type_support_dbus_disconnect(mctpd, routed_ep): rsp = await ep.send_control(mctpd.network.mctp_socket, cmd) assert rsp.hex(' ') == '00 05 00 01 00' + async def test_register_vdm_type_support_errors(dbus, mctpd): """Test RegisterVDMTypeSupport error handling""" mctp = await mctpd_mctp_base_iface_obj(dbus) @@ -1456,15 +1535,17 @@ async def test_register_vdm_type_support_errors(dbus, mctpd): await mctp.call_register_vdm_type_support(0x00, v_type, 0x0001) assert str(ex.value) == "VDM type already registered" + async def test_query_peer_properties_retry_timeout(nursery, dbus, sysnet): class LossyEndpoint(Endpoint): """An endpoint object that may drop a specific number (timeout_count) of MCTP Control Protocol requests. """ + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.timeout_count = 0 - self.timeout_opcode = 0x05 # Get Message Type Support + self.timeout_opcode = 0x05 # Get Message Type Support async def handle_mctp_control(self, sock, addr, data): rq = data[0] & 0x80 @@ -1484,7 +1565,7 @@ async def handle_mctp_control(self, sock, addr, data): # define expected message types # add a normal endpoint to network expected_types = [0, 1, 2] - ep = LossyEndpoint(iface, bytes([0x1a]), eid=15, types=expected_types) + ep = LossyEndpoint(iface, bytes([0x1A]), eid=15, types=expected_types) mctpd.network.add_endpoint(ep) # call setup_endpoint on ep, which will allocate a object path for it @@ -1495,8 +1576,8 @@ async def handle_mctp_control(self, sock, addr, data): objtypes.sort() assert objtypes == expected_types - ep.lladdr = bytes([0x1b]) # change lladdr to force retry - ep.timeout_count = 2 # timeout twice before responding + ep.lladdr = bytes([0x1B]) # change lladdr to force retry + ep.timeout_count = 2 # timeout twice before responding # call setup_endpoint again, which will trigger query of peer properties (eid, net, path, new) = await mctp.call_setup_endpoint(ep.lladdr) @@ -1507,8 +1588,8 @@ async def handle_mctp_control(self, sock, addr, data): objtypes.sort() assert objtypes == expected_types - ep.lladdr = bytes([0x1c]) # change lladdr to force retry - ep.timeout_count = 5 # timeout five times before responding + ep.lladdr = bytes([0x1C]) # change lladdr to force retry + ep.timeout_count = 5 # timeout five times before responding # call setup_endpoint again, which will trigger query of peer properties (eid, net, path, new) = await mctp.call_setup_endpoint(ep.lladdr) @@ -1516,7 +1597,7 @@ async def handle_mctp_control(self, sock, addr, data): # timeout five times does prevent us from getting the correct message types objep = await mctpd_mctp_endpoint_common_obj(dbus, path) objtypes = list(await objep.get_supported_message_types()) - expected_types = [] # exceeded retry limit, so no types known + expected_types = [] # exceeded retry limit, so no types known assert objtypes == expected_types # exit mctpd diff --git a/tests/test_mctpd_endpoint.py b/tests/test_mctpd_endpoint.py index bb29d52..0785cae 100644 --- a/tests/test_mctpd_endpoint.py +++ b/tests/test_mctpd_endpoint.py @@ -2,10 +2,15 @@ import asyncdbus from mctp_test_utils import ( mctpd_mctp_iface_control_obj, - mctpd_mctp_endpoint_control_obj + mctpd_mctp_endpoint_control_obj, ) from mctpenv import ( - Endpoint, MCTPControlCommand, Network, PhysicalBinding, Sysnet, System + Endpoint, + MCTPControlCommand, + Network, + PhysicalBinding, + Sysnet, + System, ) @@ -15,6 +20,7 @@ that reports support for MCTP control and PLDM. """ + @pytest.fixture def config(): return """ @@ -62,15 +68,22 @@ async def test_accept_set_eid(dbus, mctpd): assert len(mctpd.system.addresses) == 0 # no EID yet - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02)) + rsp = await bo.send_control( + mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02) + ) assert rsp.hex(' ') == '00 02 00 00 02 00' # set EID = 42 - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x01, bytes([0x00, 0x42]))) + rsp = await bo.send_control( + mctpd.network.mctp_socket, + MCTPControlCommand(True, 0, 0x01, bytes([0x00, 0x42])), + ) assert rsp.hex(' ') == '00 01 00 00 42 00' # get EID, expect receive 42 back - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02)) + rsp = await bo.send_control( + mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02) + ) assert rsp.hex(' ') == '00 02 00 42 02 00' @@ -84,40 +97,69 @@ async def test_accept_multiple_set_eids_for_single_interface(dbus, mctpd): assert len(mctpd.system.interfaces) == 1 # no EID yet - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02)) + rsp = await bo.send_control( + mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02) + ) assert rsp.hex(' ') == '00 02 00 00 02 00' # set EID = 42 first_eid = 42 - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x01, bytes([0x00, first_eid]))) + rsp = await bo.send_control( + mctpd.network.mctp_socket, + MCTPControlCommand(True, 0, 0x01, bytes([0x00, first_eid])), + ) assert rsp.hex(' ') == f'00 01 00 00 {first_eid:02x} 00' # get EID, expect receive 42 back - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02)) + rsp = await bo.send_control( + mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02) + ) assert rsp.hex(' ') == f'00 02 00 {first_eid:02x} 02 00' # set EID = 66 second_eid = 66 - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x01, bytes([0x00, second_eid]))) + rsp = await bo.send_control( + mctpd.network.mctp_socket, + MCTPControlCommand(True, 0, 0x01, bytes([0x00, second_eid])), + ) assert rsp.hex(' ') == f'00 01 00 00 {second_eid:02x} 00' # get EID, expect receive 66 back - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02)) + rsp = await bo.send_control( + mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02) + ) assert rsp.hex(' ') == f'00 02 00 {second_eid:02x} 02 00' # expect previous EID removed on D-Bus with pytest.raises(asyncdbus.errors.DBusError) as ex: - await mctpd_mctp_endpoint_control_obj(dbus, f"/au/com/codeconstruct/mctp1/networks/1/endpoints/{first_eid}") - assert str(ex.value) == f"Unknown object '/au/com/codeconstruct/mctp1/networks/1/endpoints/{first_eid}'." + await mctpd_mctp_endpoint_control_obj( + dbus, + f"/au/com/codeconstruct/mctp1/networks/1/endpoints/{first_eid}", + ) + assert ( + str(ex.value) + == f"Unknown object '/au/com/codeconstruct/mctp1/networks/1/endpoints/{first_eid}'." + ) # expect new EID on D-Bus - assert await mctpd_mctp_endpoint_control_obj(dbus, f"/au/com/codeconstruct/mctp1/networks/1/endpoints/{second_eid}") + assert await mctpd_mctp_endpoint_control_obj( + dbus, f"/au/com/codeconstruct/mctp1/networks/1/endpoints/{second_eid}" + ) class TestDiscovery: @pytest.fixture async def iface(self): - return System.Interface("mctp0", 1, 1, bytes([0x1D]), 68, 254, True, PhysicalBinding.PCIE_VDM) + return System.Interface( + "mctp0", + 1, + 1, + bytes([0x1D]), + 68, + 254, + True, + PhysicalBinding.PCIE_VDM, + ) async def test_simple_discovery_sequence(self, dbus, mctpd): """Test simple Discovery sequence""" @@ -126,36 +168,53 @@ async def test_simple_discovery_sequence(self, dbus, mctpd): assert len(mctpd.system.addresses) == 0 # no EID yet - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02)) + rsp = await bo.send_control( + mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x02) + ) assert rsp.hex(' ') == '00 02 00 00 02 00' # BMC response to Prepare for Discovery - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x0B)) + rsp = await bo.send_control( + mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x0B) + ) assert rsp.hex(' ') == '00 0b 00' # BMC response to Endpoint Discovery - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x0C)) + rsp = await bo.send_control( + mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x0C) + ) assert rsp.hex(' ') == '00 0c 00' # set EID = 42 eid = 42 - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x01, bytes([0x00, eid]))) + rsp = await bo.send_control( + mctpd.network.mctp_socket, + MCTPControlCommand(True, 0, 0x01, bytes([0x00, eid])), + ) assert rsp.hex(' ') == f'00 01 00 00 {eid:02x} 00' # BMC should contains two object paths: bus owner and itself - assert await mctpd_mctp_endpoint_control_obj(dbus, f"/au/com/codeconstruct/mctp1/networks/1/endpoints/{bo.eid}") - assert await mctpd_mctp_endpoint_control_obj(dbus, f"/au/com/codeconstruct/mctp1/networks/1/endpoints/{eid}") + assert await mctpd_mctp_endpoint_control_obj( + dbus, f"/au/com/codeconstruct/mctp1/networks/1/endpoints/{bo.eid}" + ) + assert await mctpd_mctp_endpoint_control_obj( + dbus, f"/au/com/codeconstruct/mctp1/networks/1/endpoints/{eid}" + ) class TestUnsupportedDiscovery: @pytest.fixture async def iface(self): - return System.Interface("mctp0", 1, 1, bytes([0x1D]), 68, 254, True, PhysicalBinding.SMBUS) + return System.Interface( + "mctp0", 1, 1, bytes([0x1D]), 68, 254, True, PhysicalBinding.SMBUS + ) async def test_simple(self, dbus, mctpd): """Discovery command on unsupported interface""" bo = mctpd.network.endpoints[0] # BMC response ERROR_UNSUPPORTED_CMD to Prepare for Discovery - rsp = await bo.send_control(mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x0B)) + rsp = await bo.send_control( + mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x0B) + ) assert rsp.hex(' ') == '00 0b 05' From 0ea4900b7015921be63d58d6f10ec4f06e3a9d73 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Mon, 5 Jan 2026 15:29:35 +0800 Subject: [PATCH 10/10] workflows: Add ruff lint & format to default pr action Signed-off-by: Jeremy Kerr --- .github/workflows/pr.yml | 6 ++++++ tests/requirements.txt | 1 + 2 files changed, 7 insertions(+) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 663fd7a..218a817 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -29,6 +29,12 @@ jobs: - name: Test mctp run: meson test -C build --verbose + - name: Check python tests + run: ruff check tests/ + + - name: Format python tests + run: ruff format --diff tests/ + - uses: actions/upload-artifact@v4 if: always() with: diff --git a/tests/requirements.txt b/tests/requirements.txt index f46f09d..80b07fc 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -4,3 +4,4 @@ pytest==8.3.2 pytest-trio==0.8.0 trio==0.31.0 pytest-tap==3.5 +ruff==0.14.10