From 7866ab85147039eca3d040c48400a3812f89f2dd Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 30 Apr 2026 13:18:48 +0800 Subject: [PATCH 1/8] mctpd: remove req_type argument from endpoint query functions We're a MCTP Control Protocol implementation, all our messages will be the control protocol type. Signed-off-by: Jeremy Kerr --- src/mctpd.c | 46 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index 523cda5f..5c3a163c 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -1747,9 +1747,8 @@ static int endpoint_query_addr(struct ctx *ctx, /* Queries an endpoint peer. Addressing is standard eid/net. */ -static int endpoint_query_peer(const struct peer *peer, uint8_t req_type, - const void *req, size_t req_len, uint8_t **resp, - size_t *resp_len, +static int endpoint_query_peer(const struct peer *peer, const void *req, + size_t req_len, uint8_t **resp, size_t *resp_len, struct sockaddr_mctp_ext *resp_addr) { struct sockaddr_mctp_ext addr = { 0 }; @@ -1763,7 +1762,7 @@ static int endpoint_query_peer(const struct peer *peer, uint8_t req_type, addr.smctp_base.smctp_network = peer->net; addr.smctp_base.smctp_addr.s_addr = peer->eid; - addr.smctp_base.smctp_type = req_type; + addr.smctp_base.smctp_type = MCTP_CTRL_HDR_MSG_TYPE; addr.smctp_base.smctp_tag = MCTP_TAG_OWNER; return endpoint_query_addr(peer->ctx, &addr, false, req, req_len, resp, @@ -1773,8 +1772,8 @@ static int endpoint_query_peer(const struct peer *peer, uint8_t req_type, /* Queries an endpoint using physical addressing, null EID. */ static int endpoint_query_phys(struct ctx *ctx, const dest_phys *dest, - uint8_t req_type, const void *req, - size_t req_len, uint8_t **resp, size_t *resp_len, + const void *req, size_t req_len, uint8_t **resp, + size_t *resp_len, struct sockaddr_mctp_ext *resp_addr) { struct sockaddr_mctp_ext addr = { 0 }; @@ -1793,7 +1792,7 @@ static int endpoint_query_phys(struct ctx *ctx, const dest_phys *dest, addr.smctp_halen = dest->hwaddr_len; memcpy(addr.smctp_haddr, dest->hwaddr, dest->hwaddr_len); - addr.smctp_base.smctp_type = req_type; + addr.smctp_base.smctp_type = MCTP_CTRL_HDR_MSG_TYPE; addr.smctp_base.smctp_tag = MCTP_TAG_OWNER; return endpoint_query_addr(ctx, &addr, true, req, req_len, resp, @@ -1829,8 +1828,8 @@ static int endpoint_send_set_endpoint_id(const struct peer *peer, req.operation = mctp_ctrl_cmd_set_eid_set_eid; // TODO: do we want Force? req.eid = peer->eid; - rc = endpoint_query_phys(peer->ctx, dest, MCTP_CTRL_HDR_MSG_TYPE, &req, - sizeof(req), &buf, &buf_size, &addr); + rc = endpoint_query_phys(peer->ctx, dest, &req, sizeof(req), &buf, + &buf_size, &addr); if (rc < 0) goto out; @@ -2448,12 +2447,11 @@ static int query_get_endpoint_id(struct ctx *ctx, const dest_phys *dest, MCTP_CTRL_CMD_GET_ENDPOINT_ID); if (peer) - rc = endpoint_query_peer(peer, MCTP_CTRL_HDR_MSG_TYPE, &req, - sizeof(req), &buf, &buf_size, &addr); + rc = endpoint_query_peer(peer, &req, sizeof(req), &buf, + &buf_size, &addr); else - rc = endpoint_query_phys(ctx, dest, MCTP_CTRL_HDR_MSG_TYPE, - &req, sizeof(req), &buf, &buf_size, - &addr); + rc = endpoint_query_phys(ctx, dest, &req, sizeof(req), &buf, + &buf_size, &addr); if (rc < 0) goto out; @@ -2558,8 +2556,8 @@ static int query_get_peer_msgtypes(struct peer *peer) mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_MESSAGE_TYPE_SUPPORT); - rc = endpoint_query_peer(peer, MCTP_CTRL_HDR_MSG_TYPE, &req, - sizeof(req), &buf, &buf_size, &addr); + rc = endpoint_query_peer(peer, &req, sizeof(req), &buf, &buf_size, + &addr); if (rc < 0) goto out; @@ -2617,8 +2615,8 @@ static int query_get_peer_vdm_types(struct peer *peer) free(buf); buf = NULL; } - rc = endpoint_query_peer(peer, MCTP_CTRL_HDR_MSG_TYPE, &req, - sizeof(req), &buf, &buf_size, &addr); + rc = endpoint_query_peer(peer, &req, sizeof(req), &buf, + &buf_size, &addr); if (rc < 0) break; @@ -2725,8 +2723,8 @@ static int query_get_peer_uuid_by_phys(struct ctx *ctx, const dest_phys *dest, mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_ENDPOINT_UUID); - rc = endpoint_query_phys(ctx, dest, MCTP_CTRL_HDR_MSG_TYPE, &req, - sizeof(req), &buf, &buf_size, &addr); + rc = endpoint_query_phys(ctx, dest, &req, sizeof(req), &buf, &buf_size, + &addr); if (rc < 0) goto out; @@ -2765,8 +2763,8 @@ static int query_get_peer_uuid(struct peer *peer) mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_ENDPOINT_UUID); - rc = endpoint_query_peer(peer, MCTP_CTRL_HDR_MSG_TYPE, &req, - sizeof(req), &buf, &buf_size, &addr); + rc = endpoint_query_peer(peer, &req, sizeof(req), &buf, &buf_size, + &addr); if (rc < 0) goto out; @@ -5495,8 +5493,8 @@ static int endpoint_send_allocate_endpoint_ids( req.alloc_eid_op = (uint8_t)(op & 0x03); req.pool_size = eid_pool_size; req.start_eid = eid_start; - rc = endpoint_query_peer(peer, MCTP_CTRL_HDR_MSG_TYPE, &req, - sizeof(req), &buf, &buf_size, &addr); + rc = endpoint_query_peer(peer, &req, sizeof(req), &buf, &buf_size, + &addr); if (rc < 0) goto out; From c0eb02dafc056faacfdd4644eceb5db6b9d7150c Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 30 Apr 2026 13:54:23 +0800 Subject: [PATCH 2/8] mctpd: Use a struct for command / response data Our arguments for endpoint_query_* are getting large; create a struct to represent the command/response pair, and pass that around instead. Signed-off-by: Jeremy Kerr --- src/mctpd.c | 251 ++++++++++++++++++++++++++++------------------------ 1 file changed, 133 insertions(+), 118 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index 5c3a163c..223280e5 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -292,6 +292,31 @@ struct ctx { uint64_t endpoint_poll; }; +/* data for a command + response. + * + * req is populated by the caller. resp is allocated during command submission, + * and must be freed by the caller. + */ +struct mctp_ctrl_cmd { + /* populated by caller */ + void *req; + size_t req_len; + + /* populated on response RX */ + void *resp; + size_t resp_len; + struct sockaddr_mctp_ext resp_addr; +}; + +/* frees response data, but not cmd itself, as this will typically be on the + * stack + */ +static void mctp_ctrl_cmd_free(struct mctp_ctrl_cmd *cmd) +{ + free(cmd->resp); + cmd->resp = NULL; +} + static int emit_endpoint_added(const struct peer *peer); static int emit_endpoint_removed(const struct peer *peer); static int emit_interface_added(struct link *link); @@ -1595,9 +1620,9 @@ static const char *peer_cmd_prefix(const char *peer, uint8_t cmd) /* Common checks for responses: that we have enough data for a response, * the expected IID and opcode, and that the response indicated success. */ -static int mctp_ctrl_validate_response(uint8_t *buf, size_t rsp_size, +static int mctp_ctrl_validate_response(struct mctp_ctrl_cmd *cmd, size_t exp_size, const char *peer, - uint8_t iid, uint8_t cmd) + uint8_t iid, uint8_t cmd_id) { struct mctp_ctrl_resp *rsp; @@ -1607,38 +1632,39 @@ static int mctp_ctrl_validate_response(uint8_t *buf, size_t rsp_size, } /* Error responses only need to include the completion code */ - if (rsp_size < MCTP_CTRL_ERROR_RESP_LEN) { + if (cmd->resp_len < MCTP_CTRL_ERROR_RESP_LEN) { warnx("%s: Wrong reply length (%zu bytes)", - peer_cmd_prefix(peer, cmd), rsp_size); + peer_cmd_prefix(peer, cmd_id), cmd->resp_len); return -ENOMSG; } /* we have enough for the smallest common response message */ - rsp = (void *)buf; + rsp = cmd->resp; if ((rsp->ctrl_hdr.rq_dgram_inst & RQDI_IID_MASK) != iid) { warnx("%s: Wrong IID (0x%02x, expected 0x%02x)", - peer_cmd_prefix(peer, cmd), + peer_cmd_prefix(peer, cmd_id), rsp->ctrl_hdr.rq_dgram_inst & RQDI_IID_MASK, iid); return -ENOMSG; } - if (rsp->ctrl_hdr.command_code != cmd) { + if (rsp->ctrl_hdr.command_code != cmd_id) { warnx("%s: Wrong opcode (0x%02x) in response", - peer_cmd_prefix(peer, cmd), rsp->ctrl_hdr.command_code); + peer_cmd_prefix(peer, cmd_id), + rsp->ctrl_hdr.command_code); return -ENOMSG; } if (rsp->completion_code) { warnx("%s: Command failed, completion code 0x%02x", - peer_cmd_prefix(peer, cmd), rsp->completion_code); + peer_cmd_prefix(peer, cmd_id), rsp->completion_code); return -ECONNREFUSED; } /* Non-error responses must be full sized */ - if (rsp_size < exp_size) { + if (cmd->resp_len < exp_size) { warnx("%s: Wrong reply length (%zu bytes)", - peer_cmd_prefix(peer, cmd), rsp_size); + peer_cmd_prefix(peer, cmd_id), cmd->resp_len); return -ENOMSG; } @@ -1651,9 +1677,7 @@ static int mctp_ctrl_validate_response(uint8_t *buf, size_t rsp_size, * Extended addressing is used optionally, depending on ext_addr arg. */ static int endpoint_query_addr(struct ctx *ctx, const struct sockaddr_mctp_ext *req_addr, - bool ext_addr, const void *req, size_t req_len, - uint8_t **resp, size_t *resp_len, - struct sockaddr_mctp_ext *resp_addr) + bool ext_addr, struct mctp_ctrl_cmd *cmd) { size_t req_addr_len; int sd = -1, val; @@ -1662,8 +1686,8 @@ static int endpoint_query_addr(struct ctx *ctx, uint8_t *buf = NULL; - *resp = NULL; - *resp_len = 0; + cmd->resp = NULL; + cmd->resp_len = 0; sd = mctp_ops.mctp.socket(); if (sd < 0) { @@ -1688,23 +1712,25 @@ static int endpoint_query_addr(struct ctx *ctx, req_addr_len = sizeof(struct sockaddr_mctp); } - if (req_len == 0) { + if (cmd->req_len == 0) { bug_warn("zero length request"); rc = -EPROTO; goto out; } - rc = mctp_ops.mctp.sendto(sd, req, req_len, 0, + rc = mctp_ops.mctp.sendto(sd, cmd->req, cmd->req_len, 0, (struct sockaddr *)req_addr, req_addr_len); if (rc < 0) { rc = -errno; if (ctx->verbose) { warnx("%s: sendto(%s) %zu bytes failed. %s", __func__, - ext_addr_tostr(req_addr), req_len, strerror(-rc)); + ext_addr_tostr(req_addr), cmd->req_len, + strerror(-rc)); } goto out; } - if ((size_t)rc != req_len) { - bug_warn("incorrect sendto %zd, expected %zu", rc, req_len); + if ((size_t)rc != cmd->req_len) { + bug_warn("incorrect sendto %zd, expected %zu", rc, + cmd->req_len); rc = -EPROTO; goto out; } @@ -1718,15 +1744,15 @@ static int endpoint_query_addr(struct ctx *ctx, goto out; } - rc = read_message(ctx, sd, &buf, &buf_size, resp_addr); + rc = read_message(ctx, sd, &buf, &buf_size, &cmd->resp_addr); if (rc < 0) { goto out; } - if (resp_addr->smctp_base.smctp_type != + if (cmd->resp_addr.smctp_base.smctp_type != req_addr->smctp_base.smctp_type) { warnx("Mismatching response type %d for request type %d. dest %s", - resp_addr->smctp_base.smctp_type, + cmd->resp_addr.smctp_base.smctp_type, req_addr->smctp_base.smctp_type, ext_addr_tostr(req_addr)); rc = -ENOMSG; @@ -1738,8 +1764,8 @@ static int endpoint_query_addr(struct ctx *ctx, if (rc) { free(buf); } else { - *resp = buf; - *resp_len = buf_size; + cmd->resp = buf; + cmd->resp_len = buf_size; } return rc; @@ -1747,9 +1773,8 @@ static int endpoint_query_addr(struct ctx *ctx, /* Queries an endpoint peer. Addressing is standard eid/net. */ -static int endpoint_query_peer(const struct peer *peer, const void *req, - size_t req_len, uint8_t **resp, size_t *resp_len, - struct sockaddr_mctp_ext *resp_addr) +static int endpoint_query_peer(const struct peer *peer, + struct mctp_ctrl_cmd *cmd) { struct sockaddr_mctp_ext addr = { 0 }; @@ -1765,16 +1790,13 @@ static int endpoint_query_peer(const struct peer *peer, const void *req, addr.smctp_base.smctp_type = MCTP_CTRL_HDR_MSG_TYPE; addr.smctp_base.smctp_tag = MCTP_TAG_OWNER; - return endpoint_query_addr(peer->ctx, &addr, false, req, req_len, resp, - resp_len, resp_addr); + return endpoint_query_addr(peer->ctx, &addr, false, cmd); } /* Queries an endpoint using physical addressing, null EID. */ static int endpoint_query_phys(struct ctx *ctx, const dest_phys *dest, - const void *req, size_t req_len, uint8_t **resp, - size_t *resp_len, - struct sockaddr_mctp_ext *resp_addr) + struct mctp_ctrl_cmd *cmd) { struct sockaddr_mctp_ext addr = { 0 }; @@ -1795,8 +1817,7 @@ static int endpoint_query_phys(struct ctx *ctx, const dest_phys *dest, addr.smctp_base.smctp_type = MCTP_CTRL_HDR_MSG_TYPE; addr.smctp_base.smctp_tag = MCTP_TAG_OWNER; - return endpoint_query_addr(ctx, &addr, true, req, req_len, resp, - resp_len, resp_addr); + return endpoint_query_addr(ctx, &addr, true, cmd); } /* returns -ECONNREFUSED if the endpoint returns failure. @@ -1808,15 +1829,13 @@ static int endpoint_send_set_endpoint_id(const struct peer *peer, mctp_eid_t *new_eidp, uint8_t *req_pool_size) { - struct sockaddr_mctp_ext addr; - struct mctp_ctrl_cmd_set_eid req = { 0 }; struct mctp_ctrl_resp_set_eid *resp = NULL; - int rc; - uint8_t *buf = NULL; - size_t buf_size; + struct mctp_ctrl_cmd_set_eid req = { 0 }; uint8_t iid, stat, alloc, pool_size = 0; const dest_phys *dest = &peer->phys; + struct mctp_ctrl_cmd cmd = { 0 }; mctp_eid_t new_eid; + int rc; rc = -1; @@ -1828,18 +1847,20 @@ static int endpoint_send_set_endpoint_id(const struct peer *peer, req.operation = mctp_ctrl_cmd_set_eid_set_eid; // TODO: do we want Force? req.eid = peer->eid; - rc = endpoint_query_phys(peer->ctx, dest, &req, sizeof(req), &buf, - &buf_size, &addr); + + cmd.req = &req; + cmd.req_len = sizeof(req); + rc = endpoint_query_phys(peer->ctx, dest, &cmd); if (rc < 0) goto out; - rc = mctp_ctrl_validate_response(buf, buf_size, sizeof(*resp), + rc = mctp_ctrl_validate_response(&cmd, sizeof(*resp), dest_phys_tostr(dest), iid, MCTP_CTRL_CMD_SET_ENDPOINT_ID); if (rc) goto out; - resp = (void *)buf; + resp = (void *)cmd.resp; stat = resp->status >> 4 & 0x3; new_eid = resp->eid_set; @@ -1886,7 +1907,7 @@ static int endpoint_send_set_endpoint_id(const struct peer *peer, rc = 0; out: - free(buf); + mctp_ctrl_cmd_free(&cmd); return rc; } @@ -2433,11 +2454,9 @@ static int query_get_endpoint_id(struct ctx *ctx, const dest_phys *dest, mctp_eid_t *ret_eid, uint8_t *ret_ep_type, uint8_t *ret_media_spec, struct peer *peer) { - struct sockaddr_mctp_ext addr; struct mctp_ctrl_cmd_get_eid req = { 0 }; struct mctp_ctrl_resp_get_eid *resp = NULL; - uint8_t *buf = NULL; - size_t buf_size; + struct mctp_ctrl_cmd cmd = { 0 }; uint8_t iid; int rc; @@ -2445,29 +2464,29 @@ static int query_get_endpoint_id(struct ctx *ctx, const dest_phys *dest, mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_ENDPOINT_ID); + cmd.req = &req; + cmd.req_len = sizeof(req); if (peer) - rc = endpoint_query_peer(peer, &req, sizeof(req), &buf, - &buf_size, &addr); + rc = endpoint_query_peer(peer, &cmd); else - rc = endpoint_query_phys(ctx, dest, &req, sizeof(req), &buf, - &buf_size, &addr); + rc = endpoint_query_phys(ctx, dest, &cmd); if (rc < 0) goto out; - rc = mctp_ctrl_validate_response(buf, buf_size, sizeof(*resp), + rc = mctp_ctrl_validate_response(&cmd, sizeof(*resp), dest_phys_tostr(dest), iid, MCTP_CTRL_CMD_GET_ENDPOINT_ID); if (rc) goto out; - resp = (void *)buf; + resp = cmd.resp; *ret_eid = resp->eid; *ret_ep_type = resp->eid_type; *ret_media_spec = resp->medium_data; out: - free(buf); + mctp_ctrl_cmd_free(&cmd); return rc; } @@ -2540,11 +2559,10 @@ static int get_endpoint_peer(struct ctx *ctx, sd_bus_error *berr, static int query_get_peer_msgtypes(struct peer *peer) { - struct sockaddr_mctp_ext addr; - struct mctp_ctrl_cmd_get_msg_type_support req; struct mctp_ctrl_resp_get_msg_type_support *resp = NULL; - uint8_t *buf = NULL; - size_t buf_size, expect_size; + struct mctp_ctrl_cmd_get_msg_type_support req; + struct mctp_ctrl_cmd cmd = { 0 }; + size_t expect_size; uint8_t iid; int rc; @@ -2555,23 +2573,24 @@ static int query_get_peer_msgtypes(struct peer *peer) mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_MESSAGE_TYPE_SUPPORT); + cmd.req = &req; + cmd.req_len = sizeof(req); - rc = endpoint_query_peer(peer, &req, sizeof(req), &buf, &buf_size, - &addr); + rc = endpoint_query_peer(peer, &cmd); if (rc < 0) goto out; rc = mctp_ctrl_validate_response( - buf, buf_size, sizeof(*resp), peer_tostr_short(peer), iid, + &cmd, sizeof(*resp), peer_tostr_short(peer), iid, MCTP_CTRL_CMD_GET_MESSAGE_TYPE_SUPPORT); if (rc) goto out; - resp = (void *)buf; + resp = cmd.resp; expect_size = sizeof(*resp) + resp->msg_type_count; - if (buf_size != expect_size) { + if (cmd.resp_len != expect_size) { warnx("%s: bad reply length. got %zu, expected %zu, %d entries. dest %s", - __func__, buf_size, expect_size, resp->msg_type_count, + __func__, cmd.resp_len, expect_size, resp->msg_type_count, peer_tostr(peer)); rc = -ENOMSG; goto out; @@ -2586,20 +2605,20 @@ static int query_get_peer_msgtypes(struct peer *peer) memcpy(peer->message_types, (void *)(resp + 1), resp->msg_type_count); rc = 0; out: - free(buf); + mctp_ctrl_cmd_free(&cmd); return rc; } static int query_get_peer_vdm_types(struct peer *peer) { struct vdm_type_support *cur_vdm_type, *new_vdm, *vdm_types = NULL; - size_t buf_size, expect_size, new_size, num_vdm_types = 0; struct mctp_ctrl_resp_get_vdm_support *resp = NULL; + size_t expect_size, new_size, num_vdm_types = 0; struct mctp_ctrl_cmd_get_vdm_support req; - uint8_t selector, iid, fmt, *buf = NULL; struct mctp_vdm_pcie_data *vdm_pcie; struct mctp_vdm_iana_data *vdm_iana; - struct sockaddr_mctp_ext addr; + uint8_t selector, iid, fmt; + struct mctp_ctrl_cmd cmd; int rc; req.ctrl_hdr.command_code = MCTP_CTRL_CMD_GET_VENDOR_MESSAGE_SUPPORT; @@ -2611,23 +2630,22 @@ static int query_get_peer_vdm_types(struct peer *peer) mctp_ctrl_msg_hdr_init_req( &req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_VENDOR_MESSAGE_SUPPORT); - if (buf) { - free(buf); - buf = NULL; - } - rc = endpoint_query_peer(peer, &req, sizeof(req), &buf, - &buf_size, &addr); + + memset(&cmd, 0, sizeof(cmd)); + cmd.req = &req; + cmd.req_len = sizeof(req); + rc = endpoint_query_peer(peer, &cmd); if (rc < 0) break; expect_size = sizeof(*resp); rc = mctp_ctrl_validate_response( - buf, buf_size, expect_size, peer_tostr_short(peer), iid, + &cmd, expect_size, peer_tostr_short(peer), iid, MCTP_CTRL_CMD_GET_VENDOR_MESSAGE_SUPPORT); if (rc) break; - resp = (void *)buf; + resp = cmd.resp; fmt = resp->vendor_id_format; if (fmt == MCTP_GET_VDM_SUPPORT_PCIE_FORMAT_ID) { expect_size = sizeof(*resp) + @@ -2641,9 +2659,9 @@ static int query_get_peer_vdm_types(struct peer *peer) rc = -ENOMSG; break; } - if (buf_size != expect_size) { + if (cmd.resp_len != expect_size) { warnx("%s: bad reply length. got %zu, expected %zu dest %s", - __func__, buf_size, expect_size, + __func__, cmd.resp_len, expect_size, peer_tostr(peer)); rc = -ENOMSG; break; @@ -2689,9 +2707,10 @@ static int query_get_peer_vdm_types(struct peer *peer) rc = -EPROTO; break; } + mctp_ctrl_cmd_free(&cmd); } - free(buf); + mctp_ctrl_cmd_free(&cmd); free(vdm_types); return rc; } @@ -2710,11 +2729,9 @@ static int peer_set_uuid(struct peer *peer, const uint8_t uuid[16]) static int query_get_peer_uuid_by_phys(struct ctx *ctx, const dest_phys *dest, uint8_t uuid[16]) { - struct sockaddr_mctp_ext addr; - struct mctp_ctrl_cmd_get_uuid req; struct mctp_ctrl_resp_get_uuid *resp = NULL; - uint8_t *buf = NULL; - size_t buf_size; + struct mctp_ctrl_cmd_get_uuid req; + struct mctp_ctrl_cmd cmd = { 0 }; uint8_t iid; int rc; @@ -2722,33 +2739,32 @@ static int query_get_peer_uuid_by_phys(struct ctx *ctx, const dest_phys *dest, mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_ENDPOINT_UUID); + cmd.req = &req; + cmd.req_len = sizeof(req); - rc = endpoint_query_phys(ctx, dest, &req, sizeof(req), &buf, &buf_size, - &addr); + rc = endpoint_query_phys(ctx, dest, &cmd); if (rc < 0) goto out; - rc = mctp_ctrl_validate_response(buf, buf_size, sizeof(*resp), + rc = mctp_ctrl_validate_response(&cmd, sizeof(*resp), dest_phys_tostr(dest), iid, MCTP_CTRL_CMD_GET_ENDPOINT_UUID); if (rc) goto out; - resp = (void *)buf; + resp = cmd.resp; memcpy(uuid, resp->uuid, 16); out: - free(buf); + mctp_ctrl_cmd_free(&cmd); return rc; } static int query_get_peer_uuid(struct peer *peer) { - struct sockaddr_mctp_ext addr; - struct mctp_ctrl_cmd_get_uuid req; struct mctp_ctrl_resp_get_uuid *resp = NULL; - uint8_t *buf = NULL; - size_t buf_size; + struct mctp_ctrl_cmd_get_uuid req; + struct mctp_ctrl_cmd cmd = { 0 }; uint8_t iid; int rc; @@ -2762,27 +2778,27 @@ static int query_get_peer_uuid(struct peer *peer) mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_ENDPOINT_UUID); + cmd.req = &req; + cmd.req_len = sizeof(req); - rc = endpoint_query_peer(peer, &req, sizeof(req), &buf, &buf_size, - &addr); + rc = endpoint_query_peer(peer, &cmd); if (rc < 0) goto out; - rc = mctp_ctrl_validate_response(buf, buf_size, sizeof(*resp), + rc = mctp_ctrl_validate_response(&cmd, sizeof(*resp), peer_tostr_short(peer), iid, MCTP_CTRL_CMD_GET_ENDPOINT_UUID); if (rc) goto out; - resp = (void *)buf; - + resp = cmd.resp; rc = peer_set_uuid(peer, resp->uuid); if (rc < 0) goto out; rc = 0; out: - free(buf); + mctp_ctrl_cmd_free(&cmd); return rc; } @@ -5479,11 +5495,9 @@ static int endpoint_send_allocate_endpoint_ids( mctp_ctrl_cmd_allocate_eids_op op, uint8_t *allocated_pool_size, mctp_eid_t *allocated_pool_start) { - struct sockaddr_mctp_ext addr; - struct mctp_ctrl_cmd_allocate_eids req = { 0 }; struct mctp_ctrl_resp_allocate_eids *resp = NULL; - uint8_t *buf = NULL; - size_t buf_size; + struct mctp_ctrl_cmd_allocate_eids req = { 0 }; + struct mctp_ctrl_cmd cmd = { 0 }; uint8_t iid, stat; int rc; @@ -5493,19 +5507,20 @@ static int endpoint_send_allocate_endpoint_ids( req.alloc_eid_op = (uint8_t)(op & 0x03); req.pool_size = eid_pool_size; req.start_eid = eid_start; - rc = endpoint_query_peer(peer, &req, sizeof(req), &buf, &buf_size, - &addr); + cmd.req = &req; + cmd.req_len = sizeof(req); + rc = endpoint_query_peer(peer, &cmd); if (rc < 0) goto out; - rc = mctp_ctrl_validate_response(buf, buf_size, sizeof(*resp), + rc = mctp_ctrl_validate_response(&cmd, sizeof(*resp), peer_tostr_short(peer), iid, MCTP_CTRL_CMD_ALLOCATE_ENDPOINT_IDS); if (rc) goto out; - resp = (void *)buf; + resp = cmd.resp; if (!resp) { warnx("Invalid response buffer"); return -ENOMEM; @@ -5540,7 +5555,7 @@ static int endpoint_send_allocate_endpoint_ids( } out: - free(buf); + mctp_ctrl_cmd_free(&cmd); return rc; } @@ -5572,17 +5587,15 @@ static int peer_reschedule_poll(sd_event_source *source, uint64_t usec) static int peer_endpoint_poll(sd_event_source *s, uint64_t usec, void *userdata) { - struct sockaddr_mctp_ext resp_addr = { 0 }; struct mctp_ctrl_resp_get_eid *resp = NULL; struct sockaddr_mctp_ext req_addr = { 0 }; struct mctp_ctrl_cmd_get_eid req = { 0 }; mctp_eid_t pool_start, idx, ret_eid = 0; struct ep_poll_ctx *pctx = userdata; struct peer *bridge = pctx->bridge; + struct mctp_ctrl_cmd cmd = { 0 }; sd_event_source *source = NULL; struct peer *peer = NULL; - uint8_t *buf = NULL; - size_t buf_size; struct net *n; uint8_t iid; int rc = 0; @@ -5626,15 +5639,17 @@ static int peer_endpoint_poll(sd_event_source *s, uint64_t usec, void *userdata) mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_ENDPOINT_ID); - rc = endpoint_query_addr(bridge->ctx, &req_addr, false, &req, - sizeof(req), &buf, &buf_size, &resp_addr); + cmd.req = &req; + cmd.req_len = sizeof(req); + + rc = endpoint_query_addr(bridge->ctx, &req_addr, false, &cmd); if (rc < 0) { - free(buf); + mctp_ctrl_cmd_free(&cmd); peer_reschedule_poll(source, bridge->ctx->endpoint_poll); return 0; } - resp = (void *)buf; + resp = cmd.resp; if (!resp) { warnx("Invalid response buffer"); return -ENOMEM; @@ -5662,7 +5677,7 @@ static int peer_endpoint_poll(sd_event_source *s, uint64_t usec, void *userdata) rc = setup_added_peer(peer); if (rc < 0) { - free(buf); + mctp_ctrl_cmd_free(&cmd); peer_reschedule_poll(source, bridge->ctx->endpoint_poll); return 0; } @@ -5672,7 +5687,7 @@ static int peer_endpoint_poll(sd_event_source *s, uint64_t usec, void *userdata) sd_event_source_unref(source); bridge->bridge_ep_poll.sources[idx] = NULL; free(pctx); - free(buf); + mctp_ctrl_cmd_free(&cmd); return rc; } From 436105ff079947e32a2932d0cdfc54a231b20d2e Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 30 Apr 2026 14:11:10 +0800 Subject: [PATCH 3/8] mctpd: use helper to init cmds from a typed request Signed-off-by: Jeremy Kerr --- src/mctpd.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index 223280e5..d0cbdf52 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -308,6 +308,12 @@ struct mctp_ctrl_cmd { struct sockaddr_mctp_ext resp_addr; }; +#define mctp_ctrl_cmd_init_from_req_type(cmd, req) \ + do { \ + (cmd)->req = &req; \ + (cmd)->req_len = sizeof(req); \ + } while (0) + /* frees response data, but not cmd itself, as this will typically be on the * stack */ @@ -1848,8 +1854,7 @@ static int endpoint_send_set_endpoint_id(const struct peer *peer, mctp_ctrl_cmd_set_eid_set_eid; // TODO: do we want Force? req.eid = peer->eid; - cmd.req = &req; - cmd.req_len = sizeof(req); + mctp_ctrl_cmd_init_from_req_type(&cmd, req); rc = endpoint_query_phys(peer->ctx, dest, &cmd); if (rc < 0) goto out; @@ -2464,8 +2469,7 @@ static int query_get_endpoint_id(struct ctx *ctx, const dest_phys *dest, mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_ENDPOINT_ID); - cmd.req = &req; - cmd.req_len = sizeof(req); + mctp_ctrl_cmd_init_from_req_type(&cmd, req); if (peer) rc = endpoint_query_peer(peer, &cmd); @@ -2573,8 +2577,7 @@ static int query_get_peer_msgtypes(struct peer *peer) mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_MESSAGE_TYPE_SUPPORT); - cmd.req = &req; - cmd.req_len = sizeof(req); + mctp_ctrl_cmd_init_from_req_type(&cmd, req); rc = endpoint_query_peer(peer, &cmd); if (rc < 0) @@ -2632,8 +2635,7 @@ static int query_get_peer_vdm_types(struct peer *peer) MCTP_CTRL_CMD_GET_VENDOR_MESSAGE_SUPPORT); memset(&cmd, 0, sizeof(cmd)); - cmd.req = &req; - cmd.req_len = sizeof(req); + mctp_ctrl_cmd_init_from_req_type(&cmd, req); rc = endpoint_query_peer(peer, &cmd); if (rc < 0) break; @@ -2739,8 +2741,7 @@ static int query_get_peer_uuid_by_phys(struct ctx *ctx, const dest_phys *dest, mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_ENDPOINT_UUID); - cmd.req = &req; - cmd.req_len = sizeof(req); + mctp_ctrl_cmd_init_from_req_type(&cmd, req); rc = endpoint_query_phys(ctx, dest, &cmd); if (rc < 0) @@ -2778,8 +2779,7 @@ static int query_get_peer_uuid(struct peer *peer) mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_ENDPOINT_UUID); - cmd.req = &req; - cmd.req_len = sizeof(req); + mctp_ctrl_cmd_init_from_req_type(&cmd, req); rc = endpoint_query_peer(peer, &cmd); if (rc < 0) @@ -5507,8 +5507,7 @@ static int endpoint_send_allocate_endpoint_ids( req.alloc_eid_op = (uint8_t)(op & 0x03); req.pool_size = eid_pool_size; req.start_eid = eid_start; - cmd.req = &req; - cmd.req_len = sizeof(req); + mctp_ctrl_cmd_init_from_req_type(&cmd, req); rc = endpoint_query_peer(peer, &cmd); if (rc < 0) goto out; @@ -5639,8 +5638,7 @@ static int peer_endpoint_poll(sd_event_source *s, uint64_t usec, void *userdata) mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_ENDPOINT_ID); - cmd.req = &req; - cmd.req_len = sizeof(req); + mctp_ctrl_cmd_init_from_req_type(&cmd, req); rc = endpoint_query_addr(bridge->ctx, &req_addr, false, &cmd); if (rc < 0) { From 6cce2281575740527496a6f178e57282617ca9c0 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 30 Apr 2026 15:10:00 +0800 Subject: [PATCH 4/8] mctpd: implement retries through command submission logic We have a number of separate retry loops when querying peer properties. Instead, use a common retry loop in the endpoint_query_addr path, which can be disabled by a new 'disable_retry' member on struct mctp_ctrl_cmd. This now enables retries for all commands, except the Get Endpoint ID probe during a recover. Signed-off-by: Jeremy Kerr --- src/mctpd.c | 157 ++++++++++++++++++++-------------------------------- 1 file changed, 59 insertions(+), 98 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index d0cbdf52..c4a213b8 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -301,6 +301,7 @@ struct mctp_ctrl_cmd { /* populated by caller */ void *req; size_t req_len; + bool disable_retry; /* populated on response RX */ void *resp; @@ -1685,7 +1686,9 @@ static int endpoint_query_addr(struct ctx *ctx, const struct sockaddr_mctp_ext *req_addr, bool ext_addr, struct mctp_ctrl_cmd *cmd) { + unsigned int max_retries = 4; size_t req_addr_len; + unsigned int retry; int sd = -1, val; ssize_t rc; size_t buf_size; @@ -1694,6 +1697,8 @@ static int endpoint_query_addr(struct ctx *ctx, cmd->resp = NULL; cmd->resp_len = 0; + if (cmd->disable_retry) + max_retries = 1; sd = mctp_ops.mctp.socket(); if (sd < 0) { @@ -1723,37 +1728,47 @@ static int endpoint_query_addr(struct ctx *ctx, rc = -EPROTO; goto out; } - rc = mctp_ops.mctp.sendto(sd, cmd->req, cmd->req_len, 0, - (struct sockaddr *)req_addr, req_addr_len); - if (rc < 0) { - rc = -errno; - if (ctx->verbose) { - warnx("%s: sendto(%s) %zu bytes failed. %s", __func__, - ext_addr_tostr(req_addr), cmd->req_len, - strerror(-rc)); + + for (retry = 0; retry < max_retries; retry++) { + rc = mctp_ops.mctp.sendto(sd, cmd->req, cmd->req_len, 0, + (struct sockaddr *)req_addr, + req_addr_len); + if (rc < 0) { + rc = -errno; + if (ctx->verbose) { + warnx("%s: sendto(%s) %zu bytes failed. %s", + __func__, ext_addr_tostr(req_addr), + cmd->req_len, strerror(-rc)); + } + break; + } + if ((size_t)rc != cmd->req_len) { + bug_warn("incorrect sendto %zd, expected %zu", rc, + cmd->req_len); + rc = -EPROTO; + break; } - goto out; - } - if ((size_t)rc != cmd->req_len) { - bug_warn("incorrect sendto %zd, expected %zu", rc, - cmd->req_len); - rc = -EPROTO; - goto out; - } - rc = wait_fd_timeout(sd, EPOLLIN, ctx->mctp_timeout); - if (rc < 0) { - if (rc == -ETIMEDOUT && ctx->verbose) { - warnx("%s: receive timed out from %s", __func__, + rc = wait_fd_timeout(sd, EPOLLIN, ctx->mctp_timeout); + if (rc == 0) + break; + + if (rc == -ETIMEDOUT) { + warnx("receive timed out from %s, attempt %d/%d", + ext_addr_tostr(req_addr), retry + 1, max_retries); + } else { + warnx("receive error from %s", ext_addr_tostr(req_addr)); + break; } - goto out; } + if (rc || retry == max_retries) + goto out; + rc = read_message(ctx, sd, &buf, &buf_size, &cmd->resp_addr); - if (rc < 0) { + if (rc < 0) goto out; - } if (cmd->resp_addr.smctp_base.smctp_type != req_addr->smctp_base.smctp_type) { @@ -2457,7 +2472,8 @@ static void set_berr(struct ctx *ctx, int errcode, sd_bus_error *berr) static int query_get_endpoint_id(struct ctx *ctx, const dest_phys *dest, mctp_eid_t *ret_eid, uint8_t *ret_ep_type, - uint8_t *ret_media_spec, struct peer *peer) + uint8_t *ret_media_spec, struct peer *peer, + bool retry) { struct mctp_ctrl_cmd_get_eid req = { 0 }; struct mctp_ctrl_resp_get_eid *resp = NULL; @@ -2470,6 +2486,7 @@ static int query_get_endpoint_id(struct ctx *ctx, const dest_phys *dest, mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, MCTP_CTRL_CMD_GET_ENDPOINT_ID); mctp_ctrl_cmd_init_from_req_type(&cmd, req); + cmd.disable_retry = !retry; if (peer) rc = endpoint_query_peer(peer, &cmd); @@ -2510,7 +2527,7 @@ static int get_endpoint_peer(struct ctx *ctx, sd_bus_error *berr, *ret_peer = NULL; rc = query_get_endpoint_id(ctx, dest, &eid, &ep_type, &medium_spec, - /*peer=*/NULL); + /*peer=*/NULL, /*retry=*/true); if (rc) return rc; @@ -2876,7 +2893,7 @@ static int method_setup_endpoint(sd_bus_message *call, void *data, /* Get Endpoint ID */ rc = query_get_endpoint_id(ctx, dest, &eid, &ep_type, &medium_spec, - /*peer=*/NULL); + /*peer=*/NULL, true); if (rc) goto err; @@ -3145,34 +3162,13 @@ 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; bool supports_vdm = false; int rc; - 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_msgtypes(peer); + if (rc < 0 && peer->ctx->verbose) { + errno = -rc; + warn("Error getting endpoint types for %s", peer_tostr(peer)); } for (unsigned int i = 0; i < peer->num_message_types; i++) { @@ -3184,54 +3180,18 @@ static int query_peer_properties(struct peer *peer) } if (supports_vdm) { - for (unsigned int i = 0; i < max_retries; i++) { - rc = query_get_peer_vdm_types(peer); - - if (rc == 0) - break; - - // On timeout, retry - if (rc == -ETIMEDOUT) { - if (peer->ctx->verbose) - warnx("Retrying to get vendor message types for %s. Attempt %u", - peer_tostr(peer), i + 1); - rc = 0; - continue; - } - - if (rc < 0) { - warnx("Error getting vendor message types for %s. Ignoring error %d %s", - peer_tostr(peer), rc, strerror(-rc)); - rc = 0; - break; - } + rc = query_get_peer_vdm_types(peer); + if (rc < 0 && peer->ctx->verbose) { + errno = -rc; + warn("Error getting vendor message types for %s", + peer_tostr(peer)); } } - 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; - } + rc = query_get_peer_uuid(peer); + if (rc < 0 && peer->ctx->verbose) { + errno = -rc; + warn("Error getting UUID for %s", peer_tostr(peer)); } // TODO: emit property changed? Though currently they are all const. @@ -3531,7 +3491,8 @@ static int peer_endpoint_recover(sd_event_source *s, uint64_t usec, */ rc = query_get_endpoint_id(ctx, &peer->phys, &peer->recovery.eid, &peer->recovery.endpoint_type, - &peer->recovery.medium_spec, /*peer=*/NULL); + &peer->recovery.medium_spec, /*peer=*/NULL, + /*retry=*/false); if (rc < 0) { goto reschedule; } @@ -3749,7 +3710,7 @@ static int method_net_learn_endpoint(sd_bus_message *call, void *data, } rc = query_get_endpoint_id(peer->ctx, &dest, &ret_eid, &ret_ep_type, - &ret_medium_spec, peer); + &ret_medium_spec, peer, true); if (rc) { warnx("Error getting endpoint id for %s. error %d %s", peer_tostr(peer), rc, strerror(-rc)); From d53236ffe4bf38302ec0329c5307d41115ef00ce Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Tue, 12 May 2026 15:14:40 +0800 Subject: [PATCH 5/8] mctpd: query_peer_properties should not prevent endpoint setup We're currently reporting a failure from a Get Endpoint UUID command through query_peer_properties, which will prevent endpoint setup. Instead, don't return an error if the Get Endpoint UUID command fails; this makes query_peer_properties() infallible, so change the return type to void, and remove caller error handling. Reported-by: Freddie Jheng Signed-off-by: Jeremy Kerr --- src/mctpd.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index c4a213b8..8f5eba28 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -337,7 +337,7 @@ static int add_peer_from_addr(struct ctx *ctx, struct peer **ret_peer); static int remove_peer(struct peer *peer); static int remove_bridged_peers(struct peer *bridge); -static int query_peer_properties(struct peer *peer); +static void query_peer_properties(struct peer *peer); static int setup_added_peer(struct peer *peer); static void add_peer_route(struct peer *peer); static int publish_peer(struct peer *peer); @@ -3160,7 +3160,7 @@ static int method_learn_endpoint(sd_bus_message *call, void *data, // Query various properties of a peer. // To be called when a new peer is discovered/assigned, once an EID is known // and routable. -static int query_peer_properties(struct peer *peer) +static void query_peer_properties(struct peer *peer) { bool supports_vdm = false; int rc; @@ -3195,7 +3195,6 @@ static int query_peer_properties(struct peer *peer) } // TODO: emit property changed? Though currently they are all const. - return rc; } static int peer_neigh_update(struct peer *peer, uint16_t type) @@ -3276,12 +3275,9 @@ static int setup_added_peer(struct peer *peer) if (!is_eid_in_bridge_pool(n, peer->ctx, peer->eid)) add_peer_route(peer); - rc = query_peer_properties(peer); - if (rc < 0) - goto out; + query_peer_properties(peer); rc = publish_peer(peer); -out: if (rc < 0) { remove_peer(peer); } From 1bb5340cfc159368ed4183f1aab94158df1a9bc9 Mon Sep 17 00:00:00 2001 From: Freddie Jheng Date: Wed, 29 Apr 2026 16:07:39 +0800 Subject: [PATCH 6/8] tests: mctpd: Add test for IID behaviour on retries Add a test to verify that Control Protocol IIDs are re-used on a command retry. [This is based on an originial commit from Freddie Jheng , containing the test cases from that commit. Minor modifications from Jeremy Kerr ] Signed-off-by: Freddie Jheng Signed-off-by: Jeremy Kerr --- tests/test_mctpd.py | 48 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index 4f3d484b..9c73cf85 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -1765,6 +1765,54 @@ async def handle_mctp_control(self, sock, addr, data): assert res == 0 +async def test_query_peer_properties_same_iid_on_retry(nursery, dbus, sysnet): + """Verify that retries for query_peer_properties reuse the same IID. + + Per DSP0236 Table 10, a retry is a retransmission of the same MCTP control + message and must use the same instance ID (IID) within MT4. + """ + + class IIDTrackingEndpoint(Endpoint): + """Drop the first Get Message Type Support request, record all IIDs seen.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.seen_iids = [] + self.drop_next = True + + async def handle_mctp_control(self, sock, addr, data): + rq = data[0] & 0x80 + opcode = data[1] + iid = data[0] & 0x1F + if rq and opcode == 0x05: # Get Message Type Support + self.seen_iids.append(iid) + if self.drop_next: + self.drop_next = False + return # simulate timeout + return await super().handle_mctp_control(sock, addr, data) + + mctpd = MctpdWrapper(dbus, sysnet) + await mctpd.start_mctpd(nursery) + + iface = mctpd.system.interfaces[0] + mctp = await mctpd_mctp_iface_obj(dbus, iface) + + ep = IIDTrackingEndpoint(iface, bytes([0x1A]), eid=15, types=[0, 1, 2]) + mctpd.network.add_endpoint(ep) + + await mctp.call_setup_endpoint(ep.lladdr) + + # Two Get Message Type Support requests: initial + one retry + assert len(ep.seen_iids) == 2 + # Both must carry the same IID + assert ep.seen_iids[0] == ep.seen_iids[1], ( + f"IID changed across retry: {ep.seen_iids[0]:#04x} -> {ep.seen_iids[1]:#04x}" + ) + + res = await mctpd.stop_mctpd() + assert res == 0 + + async def test_bridged_endpoint_poll(dbus, sysnet, nursery, autojump_clock): """Test that we use endpoint poll interval from the config and that we discover bridged endpoints via polling From f2df9a513d1c658361e19e0731dc253dd0a071d7 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Tue, 12 May 2026 15:10:05 +0800 Subject: [PATCH 7/8] tests: mctpd: Test that optional query command failures do not prevent setup The three peer query commands (Get Message Type Support, Get VDM Support, and Get Endpoint UUID) may be unimplemented, but that shouldn't prevent endpoint setup. Add a test for failures of these three commands. In this case, "failure" may take two forms: the command response reports ERROR_UNSUPPORTED_CMD, or the command is dropped altogether (not ideal, but a lot of endpoints seem to do this). Check that either behaviour is accepted. Signed-off-by: Jeremy Kerr --- tests/test_mctpd.py | 75 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index 9c73cf85..2bff0b41 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -1765,6 +1765,81 @@ async def handle_mctp_control(self, sock, addr, data): assert res == 0 +async def test_query_peer_properties_unimplemented(dbus, mctpd): + """Test that any of the optional commands being unimplemented do not + prevent endpoint setup.""" + + class CommandUnimplementedEndpoint(Endpoint): + """Always returns Command Not Implemented for the specified opcode""" + + def __init__(self, opcode, *args, **kwargs): + self.opcode = opcode + super().__init__(*args, **kwargs) + + async def handle_mctp_control(self, sock, addr, data): + flags, opcode = data[0:2] + rq = flags & 0x80 + if rq and opcode == self.opcode: + raddr = MCTPSockAddr.for_ep_resp(self, addr, sock.addr_ext) + cc = 0x05 # unsupported command + resp = bytes([flags & 0x1F, opcode, cc]) + await sock.send(raddr, resp) + return + return await super().handle_mctp_control(sock, addr, data) + + class CommandDropEndpoint(Endpoint): + """Always ignores commands of the specified opcode""" + + def __init__(self, opcode, *args, **kwargs): + self.opcode = opcode + super().__init__(*args, **kwargs) + + async def handle_mctp_control(self, sock, addr, data): + rq = data[0] & 0x80 + opcode = data[1] + if rq and opcode == self.opcode: + return + return await super().handle_mctp_control(sock, addr, data) + + optional_commands = [ + (0x03, "Get Endpoint UUID"), + (0x04, "Get Message Type Support"), + (0x06, "Get Vendor Defined Message Support"), + ] + + iface = mctpd.system.interfaces[0] + mctp = await mctpd_mctp_iface_obj(dbus, iface) + # Setting VDM types will trigger a Get VDM Support query + vdm_types = [ + VDMType(VDMType.TYPE_PCI, 0x1234, 0x5678), + VDMType(VDMType.TYPE_IANA, 0xABCDEF12, 0x3456), + ] + for idx, cmd in enumerate(optional_commands): + lladdr = 0xA1 + (idx * 2) + + ep = CommandUnimplementedEndpoint( + cmd[0], iface, bytes([lladdr]), vdm_msg_types=vdm_types + ) + mctpd.network.add_endpoint(ep) + + try: + await mctp.call_setup_endpoint(ep.lladdr) + except Exception: + raise AssertionError( + f"query failed with unimplemented {cmd[1]} command" + ) + + ep = CommandDropEndpoint( + cmd[0], iface, bytes([lladdr + 1]), vdm_msg_types=vdm_types + ) + mctpd.network.add_endpoint(ep) + + try: + await mctp.call_setup_endpoint(ep.lladdr) + except Exception: + raise AssertionError(f"query failed with dropped {cmd[1]} command") + + async def test_query_peer_properties_same_iid_on_retry(nursery, dbus, sysnet): """Verify that retries for query_peer_properties reuse the same IID. From d616ab59a8294759bd136a1068984329ee88a442 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 30 Apr 2026 16:23:41 +0800 Subject: [PATCH 8/8] mctpd: don't retry when doing a bridge poll Avoid multiple timeouts while polling downstream bridge endpoints. We'll continue to poll on a command timeout anyway. Signed-off-by: Jeremy Kerr --- src/mctpd.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mctpd.c b/src/mctpd.c index 8f5eba28..093bd504 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -5596,6 +5596,8 @@ static int peer_endpoint_poll(sd_event_source *s, uint64_t usec, void *userdata) MCTP_CTRL_CMD_GET_ENDPOINT_ID); mctp_ctrl_cmd_init_from_req_type(&cmd, req); + /* don't retry here, we're just probing */ + cmd.disable_retry = true; rc = endpoint_query_addr(bridge->ctx, &req_addr, false, &cmd); if (rc < 0) {