Skip to content

Commit 2e60e4e

Browse files
committed
mctp: Add retry for one-time peer property queries on timeout
The function `query_peer_properties()` is called once during peer initialization to query basic information after the EID becomes routable. To improve reliability, this change adds a retry mechanism when the query fails with `-ETIMEDOUT`. Since these queries are one-time initialization steps, a single successful attempt is sufficient, and retrying enhances stability under transient MCTP bus contention or multi-master timing issues. Testing: add stress test for peer initialization under multi-master ``` while true; do echo "Restarting mctpd.service..." systemctl restart mctpd.service # Wait a few seconds to allow service to initialize sleep 20 done ``` After the 30 loops, the script checks mctpd.service journal for expected retry messages to verify robustness under transient MCTP bus contention. ``` root@bmc:~# journalctl -xeu mctpd.service | grep Retrying Oct 29 00:35:21 bmc mctpd[31801]: mctpd: Retrying to get endpoint types for peer eid 10 net 1 phys physaddr if 4 hw len 1 0x20 state 1. Attempt 1 Oct 29 00:39:00 bmc mctpd[32065]: mctpd: Retrying to get endpoint types for peer eid 10 net 1 phys physaddr if 4 hw len 1 0x20 state 1. Attempt 1 Oct 29 00:39:01 bmc mctpd[32065]: mctpd: Retrying to get endpoint types for peer eid 10 net 1 phys physaddr if 4 hw len 1 0x20 state 1. Attempt 2 Oct 29 00:45:08 bmc mctpd[32360]: mctpd: Retrying to get endpoint types for peer eid 10 net 1 phys physaddr if 4 hw len 1 0x20 state 1. Attempt 1 ``` Signed-off-by: Daniel Hsu <Daniel-Hsu@quantatw.com>
1 parent 8b019a3 commit 2e60e4e

4 files changed

Lines changed: 103 additions & 14 deletions

File tree

src/mctpd.c

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2835,23 +2835,59 @@ static int method_learn_endpoint(sd_bus_message *call, void *data,
28352835
// and routable.
28362836
static int query_peer_properties(struct peer *peer)
28372837
{
2838+
const unsigned int max_retries = 4;
28382839
int rc;
28392840

2840-
rc = query_get_peer_msgtypes(peer);
2841-
if (rc < 0) {
2842-
// Warn here, it's a mandatory command code.
2843-
// It might be too noisy if some devices don't implement it.
2844-
warnx("Error getting endpoint types for %s. Ignoring error %d %s",
2845-
peer_tostr(peer), rc, strerror(-rc));
2846-
rc = 0;
2841+
for (unsigned int i = 0; i < max_retries; i++) {
2842+
rc = query_get_peer_msgtypes(peer);
2843+
2844+
// Success
2845+
if (rc == 0)
2846+
break;
2847+
2848+
// On timeout, retry
2849+
if (rc == -ETIMEDOUT) {
2850+
if (peer->ctx->verbose)
2851+
warnx("Retrying to get endpoint types for %s. Attempt %u",
2852+
peer_tostr(peer), i + 1);
2853+
rc = 0;
2854+
continue;
2855+
}
2856+
2857+
// On other errors, warn and ignore
2858+
if (rc < 0) {
2859+
if (peer->ctx->verbose)
2860+
warnx("Error getting endpoint types for %s. Ignoring error %d %s",
2861+
peer_tostr(peer), -rc, strerror(-rc));
2862+
rc = 0;
2863+
break;
2864+
}
28472865
}
28482866

2849-
rc = query_get_peer_uuid(peer);
2850-
if (rc < 0) {
2851-
if (peer->ctx->verbose)
2852-
warnx("Error getting UUID for %s. Ignoring error %d %s",
2853-
peer_tostr(peer), rc, strerror(-rc));
2854-
rc = 0;
2867+
for (unsigned int i = 0; i < max_retries; i++) {
2868+
rc = query_get_peer_uuid(peer);
2869+
2870+
// Success
2871+
if (rc == 0)
2872+
break;
2873+
2874+
// On timeout, retry
2875+
if (rc == -ETIMEDOUT) {
2876+
if (peer->ctx->verbose)
2877+
warnx("Retrying to get peer UUID for %s. Attempt %u",
2878+
peer_tostr(peer), i + 1);
2879+
rc = 0;
2880+
continue;
2881+
}
2882+
2883+
// On other errors, warn and ignore
2884+
if (rc < 0) {
2885+
if (peer->ctx->verbose)
2886+
warnx("Error getting UUID for %s. Ignoring error %d %s",
2887+
peer_tostr(peer), -rc, strerror(-rc));
2888+
rc = 0;
2889+
break;
2890+
}
28552891
}
28562892

28572893
// TODO: emit property changed? Though currently they are all const.
2.63 KB
Binary file not shown.

tests/mctpenv/__init__.py

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,14 +317,16 @@ def to_buf(self):
317317
return bytes([flags, self.cmd]) + self.data
318318

319319
class Endpoint:
320-
def __init__(self, iface, lladdr, ep_uuid = None, eid = 0, types = None):
320+
def __init__(self, iface, lladdr, ep_uuid = None, eid = 0, types = None, timeout_opcodes = set(), retry_count = 2):
321321
self.iface = iface
322322
self.lladdr = lladdr
323323
self.uuid = ep_uuid or uuid.uuid1()
324324
self.eid = eid
325325
self.types = types or [0]
326326
self.bridged_eps = []
327327
self.allocated_pool = None # or (start, size)
328+
self.timeout_opcodes = timeout_opcodes
329+
self.retry_count = retry_count
328330

329331
# keyed by (type, type-specific-instance)
330332
self.commands = {}
@@ -367,6 +369,11 @@ async def handle_mctp_control(self, sock, addr, data):
367369
# Use IID from request, zero Rq and D bits
368370
hdr = [iid, opcode]
369371

372+
if opcode in self.timeout_opcodes:
373+
if self.retry_count > 0:
374+
self.retry_count -= 1
375+
return
376+
370377
if opcode == 1:
371378
# Set Endpoint ID
372379
(op, eid) = data[2:]
@@ -434,6 +441,9 @@ async def send_control(self, sock, cmd):
434441

435442
return await cmd.wait()
436443

444+
def response_timeout_control(self, opcode):
445+
self.timeout_opcodes.add(opcode)
446+
437447
class Network:
438448
def __init__(self):
439449
self.endpoints = []
@@ -1161,13 +1171,19 @@ async def handle_control(self, nursery):
11611171
else:
11621172
print(f"unknown op {op}")
11631173

1174+
import subprocess
1175+
11641176
class MctpdWrapper(MctpProcessWrapper):
11651177
def __init__(self, bus, sysnet, binary=None, config=None):
11661178
super().__init__(sysnet)
11671179
self.bus = bus
11681180
self.binary = binary or './test-mctpd'
11691181
self.config = config
11701182

1183+
# 你要 assert 用的 log
1184+
self.stdout_logs = []
1185+
self.stderr_logs = []
1186+
11711187
async def start_mctpd(self, nursery):
11721188
nursery.start_soon(self.handle_control, nursery)
11731189
(send_chan, self.proc_rc_recv_chan) = trio.open_memory_channel(1)
@@ -1217,10 +1233,13 @@ def name_owner_changed(name, new_owner, old_owner):
12171233
config_file = None
12181234
command = [self.binary, '-v']
12191235

1236+
import subprocess
12201237
proc = await trio.lowlevel.open_process(
12211238
command = command,
12221239
pass_fds = (1, 2, self.sock_remote.fileno()),
12231240
env = env,
1241+
stdout = subprocess.PIPE,
1242+
stderr = subprocess.PIPE,
12241243
)
12251244
self.sock_remote.close()
12261245

@@ -1233,6 +1252,13 @@ def name_owner_changed(name, new_owner, old_owner):
12331252
# process after the test has run.
12341253
task_status.started(proc)
12351254

1255+
async def read_stream(stream, storage):
1256+
async for data in stream:
1257+
storage.append(data.decode(errors="replace"))
1258+
1259+
nursery.start_soon(read_stream, proc.stdout, self.stdout_logs)
1260+
nursery.start_soon(read_stream, proc.stderr, self.stderr_logs)
1261+
12361262
proc_rc = await proc.wait()
12371263

12381264
if config_file:

tests/test_mctpd.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,3 +1289,30 @@ async def test_get_message_types(dbus, mctpd):
12891289
cmd = MCTPControlCommand(True, 0, 0x04, bytes([0x05]))
12901290
rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
12911291
assert rsp.hex(' ') == '00 04 00 01 f4 f3 f2 f1'
1292+
1293+
async def test_query_peer_properties_retry_timeout(nursery, dbus, sysnet):
1294+
1295+
# activate mctpd
1296+
mctpd = MctpdWrapper(dbus, sysnet)
1297+
await mctpd.start_mctpd(nursery)
1298+
1299+
iface = mctpd.system.interfaces[0]
1300+
ep = mctpd.network.endpoints[0]
1301+
1302+
mctp = await mctpd_mctp_iface_obj(dbus, iface)
1303+
1304+
# add a bridged endpoint to ep
1305+
fake_ep = Endpoint(iface, b'\x12\x34', types=[0, 2])
1306+
fake_ep.response_timeout_control(5)
1307+
ep.add_bridged_ep(fake_ep)
1308+
mctpd.network.add_endpoint(fake_ep)
1309+
1310+
# call assign_endpoint on ep, which will allocate a pool for fake_ep
1311+
mctp = await mctpd_mctp_iface_obj(dbus, iface)
1312+
await mctp.call_setup_endpoint(ep.lladdr)
1313+
1314+
assert any("Retrying to get endpoint types" in l for l in mctpd.stderr_logs)
1315+
1316+
# exit mctpd
1317+
res = await mctpd.stop_mctpd()
1318+
assert res == 0

0 commit comments

Comments
 (0)