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 <[email protected]>

Author: JunYe1993

src/mctpd.c

@@ -2927,23 +2927,59 @@ static int method_learn_endpoint(sd_bus_message *call, void *data,
 // and routable.
 static int query_peer_properties(struct peer *peer)
 {
+	const unsigned int max_retries = 4;
 	int rc;
 
-	rc = query_get_peer_msgtypes(peer);
-	if (rc < 0) {
-		// Warn here, it's a mandatory command code.
-		// It might be too noisy if some devices don't implement it.
-		warnx("Error getting endpoint types for %s. Ignoring error %d %s",
-		      peer_tostr(peer), rc, strerror(-rc));
-		rc = 0;
+	for (unsigned int i = 0; i < max_retries; i++) {
+		rc = query_get_peer_msgtypes(peer);
+
+		// Success
+		if (rc == 0)
+			break;
+
+		// On timeout, retry
+		if (rc == -ETIMEDOUT) {
+			if (peer->ctx->verbose)
+				warnx("Retrying to get endpoint types for %s. Attempt %u",
+				      peer_tostr(peer), i + 1);
+			rc = 0;
+			continue;
+		}
+
+		// On other errors, warn and ignore
+		if (rc < 0) {
+			if (peer->ctx->verbose)
+				warnx("Error getting endpoint types for %s. Ignoring error %d %s",
+				      peer_tostr(peer), -rc, strerror(-rc));
+			rc = 0;
+			break;
+		}
 	}
 
-	rc = query_get_peer_uuid(peer);
-	if (rc < 0) {
-		if (peer->ctx->verbose)
-			warnx("Error getting UUID for %s. Ignoring error %d %s",
-			      peer_tostr(peer), rc, strerror(-rc));
-		rc = 0;
+	for (unsigned int i = 0; i < max_retries; i++) {
+		rc = query_get_peer_uuid(peer);
+
+		// Success
+		if (rc == 0)
+			break;
+
+		// On timeout, retry
+		if (rc == -ETIMEDOUT) {
+			if (peer->ctx->verbose)
+				warnx("Retrying to get peer UUID for %s. Attempt %u",
+				      peer_tostr(peer), i + 1);
+			rc = 0;
+			continue;
+		}
+
+		// On other errors, warn and ignore
+		if (rc < 0) {
+			if (peer->ctx->verbose)
+				warnx("Error getting UUID for %s. Ignoring error %d %s",
+				      peer_tostr(peer), -rc, strerror(-rc));
+			rc = 0;
+			break;
+		}
 	}
 
 	// TODO: emit property changed? Though currently they are all const.

tests/mctpenv/__init__.py

@@ -318,14 +318,16 @@ def to_buf(self):
         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, timeout_opcodes = set(), retry_count = 2):
         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.timeout_opcodes = timeout_opcodes
+        self.retry_count = retry_count
 
         # keyed by (type, type-specific-instance)
         self.commands = {}
@@ -368,6 +370,11 @@ async def handle_mctp_control(self, sock, addr, data):
             # Use IID from request, zero Rq and D bits
             hdr = [iid, opcode]
 
+            if opcode in self.timeout_opcodes:
+                if self.retry_count > 0:
+                    self.retry_count -= 1
+                    return
+
             if opcode == 1:
                 # Set Endpoint ID
                 (op, eid) = data[2:]
@@ -435,6 +442,9 @@ async def send_control(self, sock, cmd):
 
         return await cmd.wait()
 
+    def response_timeout_control(self, opcode):
+        self.timeout_opcodes.add(opcode)
+
 class Network:
     def __init__(self):
         self.endpoints = []
@@ -1197,13 +1207,18 @@ async def handle_control(self, nursery):
             else:
                 print(f"unknown op {op}")
 
+import subprocess
+
 class MctpdWrapper(MctpProcessWrapper):
     def __init__(self, bus, sysnet, binary=None, config=None):
         super().__init__(sysnet)
         self.bus = bus
         self.binary = binary or './test-mctpd'
         self.config = config
 
+        self.stdout_logs = []
+        self.stderr_logs = []
+
     async def start_mctpd(self, nursery):
         nursery.start_soon(self.handle_control, nursery)
         (send_chan, self.proc_rc_recv_chan) = trio.open_memory_channel(1)
@@ -1253,10 +1268,13 @@ def name_owner_changed(name, new_owner, old_owner):
             config_file = None
             command = [self.binary, '-v']
 
+        import subprocess
         proc = await trio.lowlevel.open_process(
                 command = command,
                 pass_fds = (1, 2, self.sock_remote.fileno()),
                 env = env,
+                stdout = subprocess.PIPE,
+                stderr = subprocess.PIPE,
             )
         self.sock_remote.close()
 
@@ -1269,6 +1287,13 @@ def name_owner_changed(name, new_owner, old_owner):
         # process after the test has run.
         task_status.started(proc)
 
+        async def read_stream(stream, storage):
+            async for data in stream:
+                storage.append(data.decode(errors="replace"))
+
+        nursery.start_soon(read_stream, proc.stdout, self.stdout_logs)
+        nursery.start_soon(read_stream, proc.stderr, self.stderr_logs)
+
         proc_rc = await proc.wait()
 
         if config_file:

tests/test_mctpd.py

@@ -1376,3 +1376,30 @@ async def test_register_vdm_type_support_errors(dbus, mctpd):
     with pytest.raises(asyncdbus.errors.DBusError) as ex:
         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):
+
+    # activate mctpd
+    mctpd = MctpdWrapper(dbus, sysnet)
+    await mctpd.start_mctpd(nursery)
+
+    iface = mctpd.system.interfaces[0]
+    ep = mctpd.network.endpoints[0]
+
+    mctp = await mctpd_mctp_iface_obj(dbus, iface)
+
+    # add a bridged endpoint to ep
+    fake_ep = Endpoint(iface, b'\x12\x34', types=[0, 2])
+    fake_ep.response_timeout_control(5)
+    ep.add_bridged_ep(fake_ep)
+    mctpd.network.add_endpoint(fake_ep)
+
+    # call assign_endpoint on ep, which will allocate a pool for fake_ep
+    mctp = await mctpd_mctp_iface_obj(dbus, iface)
+    await mctp.call_setup_endpoint(ep.lladdr)
+
+    assert any("Retrying to get endpoint types" in l for l in mctpd.stderr_logs)
+
+    # exit mctpd
+    res = await mctpd.stop_mctpd()
+    assert res == 0