From 84eaa4206eadcd05e39e625bf9fbbacf27e89c59 Mon Sep 17 00:00:00 2001 From: Faizan Ali Date: Wed, 23 Apr 2025 01:23:36 +0530 Subject: [PATCH 1/8] Add ALLOCATE_ENDPOINT_ID control message support Add MCTP control message structures for the ALLOCATE_ENDPOINT_ID command to support bridge endpoint EID pool allocation. Signed-off-by: Faizan Ali --- src/mctp-control-spec.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/mctp-control-spec.h b/src/mctp-control-spec.h index e3f2f62..d1293f8 100644 --- a/src/mctp-control-spec.h +++ b/src/mctp-control-spec.h @@ -183,6 +183,28 @@ struct mctp_ctrl_resp_resolve_endpoint_id { // ... uint8_t physical_address[N] } __attribute__((__packed__)); +typedef enum { + mctp_ctrl_cmd_alloc_eid_alloc_eid = 0, + mctp_ctrl_cmd_alloc_eid_force_alloc = 1, + mctp_ctrl_cmd_alloc_eid_get_alloc_info = 2, + mctp_ctrl_cmd_alloc_eid_reserved = 3 +} mctp_ctrl_cmd_alloc_eid_op; + +struct mctp_ctrl_cmd_alloc_eid { + struct mctp_ctrl_msg_hdr ctrl_hdr; + uint8_t alloc_eid_op; + uint8_t pool_size; + uint8_t start_eid; +} __attribute__((__packed__)); + +struct mctp_ctrl_resp_alloc_eid { + struct mctp_ctrl_msg_hdr ctrl_hdr; + uint8_t completion_code; + uint8_t status; + uint8_t eid_pool_size; + uint8_t eid_set; +} __attribute__((__packed__)); + #define MCTP_CTRL_HDR_MSG_TYPE 0 #define MCTP_CTRL_HDR_FLAG_REQUEST (1 << 7) #define MCTP_CTRL_HDR_FLAG_DGRAM (1 << 6) From f0f5f050192e549cce95e9696e339a04bfa79a11 Mon Sep 17 00:00:00 2001 From: Faizan Ali Date: Thu, 24 Apr 2025 01:55:51 +0530 Subject: [PATCH 2/8] Add mctp bridge endpoint support Add support for MCTP bridge endpoints that can allocate pools of EIDs for downstream endpoints. We assume each AssignEndpoint d-bus call will be for an MCTP bridge, with this we allocate/reserve a max_pool_size eid range contiguous to bridge's own eid. Later this pool size is updated based on SET_ENDPOINT_ID command response. - for static eid assignment via AssignEndpointStatic d-bus call, add check if eid is part of any other bridge's pool range. [Fixup and requested change from Jeremy Kerr ] Signed-off-by: Faizan Ali Signed-off-by: Jeremy Kerr --- src/mctpd.c | 184 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 155 insertions(+), 29 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index 3d7f186..dce5f38 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -185,6 +186,10 @@ struct peer { uint8_t endpoint_type; uint8_t medium_spec; } recovery; + + // Pool size + uint8_t pool_size; + uint8_t pool_start; }; struct ctx { @@ -232,7 +237,7 @@ static int emit_interface_removed(struct link *link); static int emit_net_added(struct ctx *ctx, struct net *net); static int emit_net_removed(struct ctx *ctx, struct net *net); static int add_peer(struct ctx *ctx, const dest_phys *dest, mctp_eid_t eid, - uint32_t net, struct peer **ret_peer); + uint32_t net, struct peer **ret_peer, bool net_learn); static int add_peer_from_addr(struct ctx *ctx, const struct sockaddr_mctp_ext *addr, struct peer **ret_peer); @@ -1484,7 +1489,7 @@ static int endpoint_query_phys(struct ctx *ctx, const dest_phys *dest, } /* returns -ECONNREFUSED if the endpoint returns failure. */ -static int endpoint_send_set_endpoint_id(const struct peer *peer, +static int endpoint_send_set_endpoint_id(struct peer *peer, mctp_eid_t *new_eidp) { struct sockaddr_mctp_ext addr; @@ -1552,9 +1557,20 @@ static int endpoint_send_set_endpoint_id(const struct peer *peer, alloc = resp->status & 0x3; if (alloc != 0) { - // TODO for bridges - warnx("%s requested allocation pool, unimplemented", - dest_phys_tostr(dest)); + peer->pool_size = resp->eid_pool_size; + if (peer->ctx->verbose) { + fprintf(stderr, + "%s requested allocation of pool size = %d\n", + dest_phys_tostr(dest), peer->pool_size); + } + if (peer->pool_size > peer->ctx->max_pool_size) { + warnx("Truncate: requested pool size > max pool size config"); + peer->pool_size = peer->ctx->max_pool_size; + } + } else { + // reset previous assumed pool + peer->pool_size = 0; + peer->pool_start = 0; } rc = 0; @@ -1563,10 +1579,27 @@ static int endpoint_send_set_endpoint_id(const struct peer *peer, return rc; } +// Checks if given EID belongs to any bridge's pool range +static bool is_eid_in_bridge_pool(struct net *n, struct ctx *ctx, + mctp_eid_t eid) +{ + for (int i = ctx->dyn_eid_min; i <= eid; i++) { + struct peer *peer = n->peers[i]; + if (peer && peer->pool_size > 0) { + if (eid >= peer->pool_start && + eid < peer->pool_start + peer->pool_size) { + return true; + } + i += peer->pool_size; + } + } + return false; +} + /* Returns the newly added peer. * Error is -EEXISTS if it exists */ static int add_peer(struct ctx *ctx, const dest_phys *dest, mctp_eid_t eid, - uint32_t net, struct peer **ret_peer) + uint32_t net, struct peer **ret_peer, bool net_learn) { struct peer *peer, **tmp; struct net *n; @@ -1584,6 +1617,14 @@ static int add_peer(struct ctx *ctx, const dest_phys *dest, mctp_eid_t eid, } *ret_peer = peer; return 0; + } else { + /* only LearnEndpoint methods of au.com.codeconstruct.MCTP.Network1 + * interface will approve peer structure if eid belongs to a bridge + * pool space else never allow. + */ + if (!net_learn & is_eid_in_bridge_pool(n, ctx, eid)) { + return -EADDRNOTAVAIL; + } } if (ctx->num_peers == MAX_PEER_SIZE) @@ -1627,7 +1668,7 @@ static int add_peer_from_addr(struct ctx *ctx, phys.hwaddr_len = addr->smctp_halen; return add_peer(ctx, &phys, addr->smctp_base.smctp_addr.s_addr, - addr->smctp_base.smctp_network, ret_peer); + addr->smctp_base.smctp_network, ret_peer, true); } static int check_peer_struct(const struct peer *peer, const struct net *n) @@ -1796,11 +1837,66 @@ static int peer_set_mtu(struct ctx *ctx, struct peer *peer, uint32_t mtu) return rc; } +struct eid_allocation { + mctp_eid_t start; + unsigned int extent; /* 0 = only the start EID */ +}; + +/* Allocate an unused dynamic EID for a peer, optionally with an associated + * bridge range (of size @bridged_len). + * + * We try to find the first allocation that contains the base EID plus the + * full range. If no space for that exists, we return the largest + * possible range. If the requested range is 0, then the first available + * (single) EID will suit as a match, the returned alloc->extent will be zero. + * + * It is up to the caller to check whether this range is suitable, and + * actually reserve that EID (& range) if so. + * + * returns 0 on success (with @alloc populated), non-zero on failure. + */ +static int allocate_eid(struct ctx *ctx, struct net *net, + unsigned int bridged_len, struct eid_allocation *alloc) +{ + struct eid_allocation cur = { 0 }, best = { 0 }; + mctp_eid_t eid; + + for (eid = ctx->dyn_eid_min; eid <= ctx->dyn_eid_max; eid++) { + if (net->peers[eid]) { + // reset our current candidate allocation + cur.start = 0; + eid += net->peers[eid]->pool_size; + continue; + } + + // start a new candidate allocation + if (!cur.start) + cur.start = eid; + cur.extent = eid - cur.start; + + // if this suits, we're done + if (cur.extent == bridged_len) { + *alloc = cur; + return 0; + } + + if (cur.extent > best.extent) + best = cur; + } + + if (best.start) { + *alloc = best; + return 0; + } + + return -1; +} + static int endpoint_assign_eid(struct ctx *ctx, sd_bus_error *berr, const dest_phys *dest, struct peer **ret_peer, - mctp_eid_t static_eid) + mctp_eid_t static_eid, bool assign_bridge) { - mctp_eid_t e, new_eid; + mctp_eid_t new_eid; struct net *n = NULL; struct peer *peer = NULL; uint32_t net; @@ -1819,28 +1915,49 @@ static int endpoint_assign_eid(struct ctx *ctx, sd_bus_error *berr, } if (static_eid) { - rc = add_peer(ctx, dest, static_eid, net, &peer); + rc = add_peer(ctx, dest, static_eid, net, &peer, false); if (rc < 0) return rc; new_eid = static_eid; } else { - /* Find an unused dynamic EID */ - for (e = ctx->dyn_eid_min; e <= ctx->dyn_eid_max; e++) { - if (n->peers[e]) - continue; - rc = add_peer(ctx, dest, e, net, &peer); - if (rc < 0) - return rc; - break; - } - if (e > ctx->dyn_eid_max) { - warnx("Ran out of EIDs for net %d, allocating %s", net, - dest_phys_tostr(dest)); + struct eid_allocation alloc; + unsigned int alloc_size = 0; + + if (assign_bridge) + alloc_size = ctx->max_pool_size; + + rc = allocate_eid(ctx, n, alloc_size, &alloc); + if (rc) { + warnx("Cannot allocate any EID (+pool %d) on net %d for %s", + alloc_size, net, dest_phys_tostr(dest)); sd_bus_error_setf(berr, SD_BUS_ERROR_FAILED, "Ran out of EIDs"); return -EADDRNOTAVAIL; } + + /* Only allow complete pools for now. In future we could reserve + * this range, in the assumption that the subsequent pool + * request (in the Set Endpoint ID response) will fit in this + * reservation. + */ + if (alloc.extent < alloc_size) { + warnx("Cannot allocate sufficient EIDs (+pool %d) on net %d for %s" + " (largest span %d at %d)", + alloc_size, net, dest_phys_tostr(dest), + alloc.extent, alloc.start); + alloc.extent = 0; + } + + new_eid = alloc.start; + + rc = add_peer(ctx, dest, new_eid, net, &peer, false); + if (rc < 0) + return rc; + + peer->pool_size = alloc.extent; + if (peer->pool_size) + peer->pool_start = new_eid + 1; } rc = endpoint_send_set_endpoint_id(peer, &new_eid); @@ -1854,6 +1971,10 @@ static int endpoint_assign_eid(struct ctx *ctx, sd_bus_error *berr, } if (new_eid != peer->eid) { + // avoid allocation for any different EID in response + warnx("Mismatch of requested from received EID, resetting the pool"); + peer->pool_size = 0; + peer->pool_start = 0; rc = change_peer_eid(peer, new_eid); if (rc == -EEXIST) { sd_bus_error_setf( @@ -2017,7 +2138,7 @@ static int get_endpoint_peer(struct ctx *ctx, sd_bus_error *berr, return 0; } /* New endpoint */ - rc = add_peer(ctx, dest, eid, net, &peer); + rc = add_peer(ctx, dest, eid, net, &peer, false); if (rc < 0) return rc; } @@ -2256,7 +2377,7 @@ static int method_setup_endpoint(sd_bus_message *call, void *data, } /* Set Endpoint ID */ - rc = endpoint_assign_eid(ctx, berr, dest, &peer, 0); + rc = endpoint_assign_eid(ctx, berr, dest, &peer, 0, false); if (rc < 0) goto err; @@ -2309,7 +2430,7 @@ static int method_assign_endpoint(sd_bus_message *call, void *data, peer->net, peer_path, 0); } - rc = endpoint_assign_eid(ctx, berr, dest, &peer, 0); + rc = endpoint_assign_eid(ctx, berr, dest, &peer, 0, true); if (rc < 0) goto err; @@ -2317,6 +2438,10 @@ static int method_assign_endpoint(sd_bus_message *call, void *data, if (!peer_path) goto err; + if (peer->pool_size > 0) { + //TODO: Implement Allocate EndpointID + } + return sd_bus_reply_method_return(call, "yisb", peer->eid, peer->net, peer_path, 1); err: @@ -2381,7 +2506,7 @@ static int method_assign_endpoint_static(sd_bus_message *call, void *data, } } - rc = endpoint_assign_eid(ctx, berr, dest, &peer, eid); + rc = endpoint_assign_eid(ctx, berr, dest, &peer, eid, false); if (rc < 0) { goto err; } @@ -2791,7 +2916,8 @@ static int peer_endpoint_recover(sd_event_source *s, uint64_t usec, * after which we immediately return as there's no old peer state left to * maintain. */ - return endpoint_assign_eid(ctx, NULL, &phys, &peer, 0); + return endpoint_assign_eid(ctx, NULL, &phys, &peer, 0, + false); } /* Confirmation of the same device, apply its already allocated EID */ @@ -2952,7 +3078,7 @@ static int method_net_learn_endpoint(sd_bus_message *call, void *data, return sd_bus_reply_method_return(call, "sb", path_from_peer(peer), false); - rc = add_peer(ctx, &dest, eid, net->net, &peer); + rc = add_peer(ctx, &dest, eid, net->net, &peer, true); if (rc) { warnx("can't add peer: %s", strerror(-rc)); goto err; @@ -3752,7 +3878,7 @@ static int add_local_eid(struct ctx *ctx, uint32_t net, int eid) } } - rc = add_peer(ctx, &local_phys, eid, net, &peer); + rc = add_peer(ctx, &local_phys, eid, net, &peer, false); if (rc < 0) { bug_warn("Error adding local eid %d net %d", eid, net); return rc; From b4abc00358732dc3484a822202d3a5ce7a70e52c Mon Sep 17 00:00:00 2001 From: Faizan Ali Date: Thu, 24 Apr 2025 17:57:25 +0530 Subject: [PATCH 3/8] Implement ALLOCATE_ENDPOINT_ID support for bridges Add implementation for the MCTP ALLOCATE_ENDPOINT_ID control command to enable bridges to allocate EID pools for downstream endpoints. Update gateway route for downstream EIDs Signed-off-by: Faizan Ali --- src/mctpd.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 147 insertions(+), 1 deletion(-) diff --git a/src/mctpd.c b/src/mctpd.c index dce5f38..ba9e3c6 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -259,6 +259,7 @@ static int del_local_eid(struct ctx *ctx, uint32_t net, int eid); static int add_net(struct ctx *ctx, uint32_t net); static void del_net(struct net *net); static int add_interface(struct ctx *ctx, int ifindex); +static int endpoint_allocate_eid(struct peer *peer); static const sd_bus_vtable bus_endpoint_obmc_vtable[]; static const sd_bus_vtable bus_endpoint_cc_vtable[]; @@ -2439,7 +2440,18 @@ static int method_assign_endpoint(sd_bus_message *call, void *data, goto err; if (peer->pool_size > 0) { - //TODO: Implement Allocate EndpointID + rc = endpoint_allocate_eid(peer); + if (rc < 0) { + warnx("Failed to allocate downstream EIDs"); + } else { + if (peer->ctx->verbose) { + fprintf(stderr, + "Downstream EIDs assigned from %d to %d : pool size %d\n", + peer->pool_start, + peer->pool_start + peer->pool_size - 1, + peer->pool_size); + } + } } return sd_bus_reply_method_return(call, "yisb", peer->eid, peer->net, @@ -2634,6 +2646,20 @@ static int peer_route_update(struct peer *peer, uint16_t type) return mctp_nl_route_add(peer->ctx->nl, peer->eid, 0, peer->phys.ifindex, NULL, peer->mtu); } else if (type == RTM_DELROUTE) { + if (peer->pool_size > 0) { + int rc = 0; + struct mctp_fq_addr gw_addr = { 0 }; + gw_addr.net = peer->net; + gw_addr.eid = peer->eid; + rc = mctp_nl_route_del(peer->ctx->nl, peer->pool_start, + peer->pool_size - 1, + peer->phys.ifindex, &gw_addr); + if (rc < 0) + warnx("failed to delete route for peer pool eids %d-%d %s", + peer->pool_start, + peer->pool_start + peer->pool_size - 1, + strerror(-rc)); + } return mctp_nl_route_del(peer->ctx->nl, peer->eid, 0, peer->phys.ifindex, NULL); } @@ -4360,6 +4386,126 @@ static void free_config(struct ctx *ctx) free(ctx->config_filename); } +static int endpoint_send_allocate_endpoint_id(struct peer *peer, + mctp_eid_t eid_start, + uint8_t eid_pool_size, + mctp_ctrl_cmd_alloc_eid_op oper, + uint8_t *allocated_pool_size, + mctp_eid_t *allocated_pool_start) +{ + struct sockaddr_mctp_ext addr; + struct mctp_ctrl_cmd_alloc_eid req = { 0 }; + struct mctp_ctrl_resp_alloc_eid *resp = NULL; + uint8_t *buf = NULL; + size_t buf_size; + uint8_t iid, stat; + int rc; + + iid = mctp_next_iid(peer->ctx); + mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid, + MCTP_CTRL_CMD_ALLOCATE_ENDPOINT_IDS); + req.alloc_eid_op = (uint8_t)(oper & 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); + if (rc < 0) + goto out; + + rc = mctp_ctrl_validate_response(buf, buf_size, sizeof(*resp), + peer_tostr_short(peer), iid, + MCTP_CTRL_CMD_ALLOCATE_ENDPOINT_IDS); + + if (rc) + goto out; + + resp = (void *)buf; + if (!resp) { + warnx("Invalid response buffer"); + return -ENOMEM; + } + + stat = resp->status & 0x03; + if (stat == 0x00) { + if (peer->ctx->verbose) { + fprintf(stderr, "Allocation accepted\n"); + } + if (resp->eid_pool_size != eid_pool_size || + resp->eid_set != eid_start) { + warnx("Unexpected pool start %d pool size %d", + resp->eid_set, resp->eid_pool_size); + rc = -1; + goto out; + } + *allocated_pool_size = eid_pool_size; + *allocated_pool_start = eid_start; + } else { + if (stat == 0x1) + warnx("Allocation was rejected: already allocated by other bus" + " pool size %d, pool start %d", + resp->eid_pool_size, resp->eid_set); + rc = -1; + goto out; + } + + if (peer->ctx->verbose) { + fprintf(stderr, "Allocated size of %d, starting from EID %d\n", + resp->eid_pool_size, resp->eid_set); + } + +out: + free(buf); + return rc; +} + +static int endpoint_allocate_eid(struct peer *peer) +{ + uint8_t allocated_pool_size = 0; + mctp_eid_t allocated_pool_start = 0; + int rc = 0; + + if (peer->pool_start >= peer->ctx->dyn_eid_max || + peer->pool_start <= 0) { + warnx("Invalid pool start %d", peer->pool_start); + return -1; + } + rc = endpoint_send_allocate_endpoint_id( + peer, peer->pool_start, peer->pool_size, + mctp_ctrl_cmd_alloc_eid_alloc_eid, &allocated_pool_size, + &allocated_pool_start); + if (rc) { + //reset peer pool + peer->pool_size = 0; + peer->pool_start = 0; + return rc; + } + peer->pool_size = allocated_pool_size; + peer->pool_start = allocated_pool_start; + + // add gateway route for all bridge's downstream eids + if (peer->pool_size > 0) { + struct mctp_fq_addr gw_addr = { 0 }; + gw_addr.net = peer->net; + gw_addr.eid = peer->eid; + rc = mctp_nl_route_add(peer->ctx->nl, peer->pool_start, + peer->pool_size - 1, peer->phys.ifindex, + &gw_addr, peer->mtu); + if (rc < 0) { + warnx("Failed to add gateway route for EID %d: %s", + gw_addr.eid, strerror(-rc)); + // If the route already exists, continue polling + if (rc == -EEXIST) { + rc = 0; + } else { + return rc; + } + } + // TODO: Polling logic for downstream EID + } + + return rc; +} + int main(int argc, char **argv) { struct ctx ctxi = { 0 }, *ctx = &ctxi; From b5cdc3631d4cb8f59683349513a80cc24e8f8978 Mon Sep 17 00:00:00 2001 From: Faizan Ali Date: Wed, 25 Jun 2025 17:56:52 +0530 Subject: [PATCH 4/8] Update mctp document with mctp bridge support * updated mctpd.md with new mctp bridge support for dynamic eid assignment from AssignEndpoint d-bus call Signed-off-by: Faizan Ali --- docs/mctpd.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/mctpd.md b/docs/mctpd.md index b4a5cf0..3bab1d6 100644 --- a/docs/mctpd.md +++ b/docs/mctpd.md @@ -126,7 +126,11 @@ busctl call au.com.codeconstruct.MCTP1 \ Similar to SetupEndpoint, but will always assign an EID rather than querying for existing ones. Will return `new = false` when an endpoint is already known to -`mctpd`. +`mctpd`. If the endpoint is an MCTP bridge (indicated by requesting a pool size +in its Set Endpoint ID response), this method attempts to allocate a contiguous +range of EIDs for the bridge's downstream endpoints. If sufficient contiguous EIDs +are not available within the dynamic allocation pool for the network, only the +bridge's own EID will be assigned, and downstream EID allocation will fail. #### `.AssignEndpointStatic`: `ayy` → `yisb` From b039eac47a39817f52acf80bd7d70c66ed15f49c Mon Sep 17 00:00:00 2001 From: Faizan Ali Date: Tue, 1 Jul 2025 01:45:11 +0530 Subject: [PATCH 5/8] Add tests for dynamic bridge eid and downstream eid assignment Add new test for validating AssignEndpoint D-Bus method to verify bridge endpoint EID allocation being contiguous to its downstream eids. Add Allocate Endpoint control message support with new endpoint property for allocated pool size also assign dynamic eid contiguous to bridge during Allocate Endpoint control message. Signed-off-by: Faizan Ali --- tests/mctpenv/__init__.py | 26 +++++++++++++++++++++++++- tests/test_mctpd.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/tests/mctpenv/__init__.py b/tests/mctpenv/__init__.py index 8f82bf9..523c4c1 100644 --- a/tests/mctpenv/__init__.py +++ b/tests/mctpenv/__init__.py @@ -303,6 +303,9 @@ def __init__(self, iface, lladdr, ep_uuid = None, eid = 0, types = None): self.eid = eid self.types = types or [0] self.bridged_eps = [] + self.pool_size = 0 + self.allocated_pool = None # or (start, size) + # keyed by (type, type-specific-instance) self.commands = {} @@ -348,7 +351,12 @@ async def handle_mctp_control(self, sock, addr, data): # Set Endpoint ID (op, eid) = data[2:] self.eid = eid - data = bytes(hdr + [0x00, 0x00, self.eid, 0x00]) + pool_size = len(self.bridged_eps) + alloc_status = 0x00 + # request a pool if we have one + if pool_size: + alloc_status |= 0x01 + data = bytes(hdr + [0x00, alloc_status, self.eid, pool_size]) await sock.send(raddr, data) elif opcode == 2: @@ -367,6 +375,22 @@ async def handle_mctp_control(self, sock, addr, data): data = bytes(hdr + [0x00, len(types)] + types) await sock.send(raddr, data) + elif opcode == 8: + # Allocate Endpoint IDs + (_, _, _, pool_size, pool_start) = data + alloc_status = 0x00 + if self.allocated_pool is not None: + alloc_status = 0x01 + 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]): + ep.eid = self.allocated_pool[0] + n + + 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 diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index c9b2aaa..50d89b1 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -926,3 +926,39 @@ 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): + iface = mctpd.system.interfaces[0] + mctp = await mctpd_mctp_iface_obj(dbus, iface) + ep = mctpd.network.endpoints[0] + pool_size = 2 + + # Set up bridged endpoints as undiscovered EID 0 + for i in range(pool_size): + br_ep = Endpoint(iface, bytes(), types=[0, 2]) + ep.add_bridged_ep(br_ep) + mctpd.network.add_endpoint(br_ep) + + mctp = await mctpd_mctp_iface_obj(dbus, iface) + + # dynamic EID assigment for dev1 + (eid, _, path, new) = await mctp.call_assign_endpoint(ep.lladdr) + + assert new + assert ep.allocated_pool == (eid + 1, pool_size) + + # check if we can assign non-bridged endpoint dev2, eid from + #bridge's already assigned pool + dev2 = Endpoint(iface, bytes([0x1e])) + mctpd.network.add_endpoint(dev2) + with pytest.raises(asyncdbus.errors.DBusError) as ex: + await mctp.call_assign_endpoint_static(dev2.lladdr, ep.eid + 1) + + assert str(ex.value) == "Request failed" From e7d4478b803a42254cbb132240daa77affd1ceaa Mon Sep 17 00:00:00 2001 From: Faizan Ali Date: Fri, 18 Jul 2025 04:21:11 +0530 Subject: [PATCH 6/8] Add interface au.com.codeconstruct.MCTP.Bridge1 New endpoint object interface au.com.codeconstruct.MCTP.Bridge1 which will capture details of bridge type endpoint such as pool start, pool end. Update test framework with new test methods to validate bridge pool assignemnt. Signed-off-by: Faizan Ali --- docs/mctpd.md | 16 ++++++ src/mctpd.c | 54 ++++++++++++++++++++ tests/test_mctpd.py | 117 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+) diff --git a/docs/mctpd.md b/docs/mctpd.md index 3bab1d6..4806b0e 100644 --- a/docs/mctpd.md +++ b/docs/mctpd.md @@ -219,6 +219,22 @@ busctl call au.com.codeconstruct.MCTP1 \ Removes the MCTP endpoint from `mctpd`, and deletes routes and neighbour entries. +### MCTP bridge interface: `au.com.codeconstruct.MCTP.Bridge1` interface +For any endpoint which also happens to be an MCTP Bridge, if dynamic eid is +assgined to it via d-bus method `.AssignEndpoint`, such endpoint's pool +allocation details would be reflected into `au.com.codeconstruct.MCTP.Bridge1` +interface of bridge's endpoint object. + +### `.PoolEnd`: `y` + +A constant property representing last EID in the contiguous range allocated +for downstream endpoints. + +### `.PoolStart`: `y` + +A constant property representing first EID in the contiguous range allocated +for downstream endpoints. + ## Configuration `mctpd` reads configuration data from a TOML file, typically `/etc/mctpd.conf`. diff --git a/src/mctpd.c b/src/mctpd.c index ba9e3c6..d396d98 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -46,6 +46,7 @@ #define MCTP_DBUS_PATH_LINKS "/au/com/codeconstruct/mctp1/interfaces" #define CC_MCTP_DBUS_IFACE_BUSOWNER "au.com.codeconstruct.MCTP.BusOwner1" #define CC_MCTP_DBUS_IFACE_ENDPOINT "au.com.codeconstruct.MCTP.Endpoint1" +#define CC_MCTP_DBUS_IFACE_BRIDGE "au.com.codeconstruct.MCTP.Bridge1" #define CC_MCTP_DBUS_IFACE_TESTING "au.com.codeconstruct.MCTPTesting" #define MCTP_DBUS_NAME "au.com.codeconstruct.MCTP1" #define MCTP_DBUS_IFACE_ENDPOINT "xyz.openbmc_project.MCTP.Endpoint" @@ -152,6 +153,7 @@ struct peer { bool published; sd_bus_slot *slot_obmc_endpoint; sd_bus_slot *slot_cc_endpoint; + sd_bus_slot *slot_bridge; sd_bus_slot *slot_uuid; char *path; @@ -263,6 +265,7 @@ static int endpoint_allocate_eid(struct peer *peer); static const sd_bus_vtable bus_endpoint_obmc_vtable[]; static const sd_bus_vtable bus_endpoint_cc_vtable[]; +static const sd_bus_vtable bus_endpoint_bridge[]; static const sd_bus_vtable bus_endpoint_uuid_vtable[]; __attribute__((format(printf, 1, 2))) static void bug_warn(const char *fmt, ...) @@ -1770,6 +1773,7 @@ static void free_peers(struct ctx *ctx) free(peer->path); sd_bus_slot_unref(peer->slot_obmc_endpoint); sd_bus_slot_unref(peer->slot_cc_endpoint); + sd_bus_slot_unref(peer->slot_bridge); sd_bus_slot_unref(peer->slot_uuid); free(peer); } @@ -2823,6 +2827,8 @@ static int unpublish_peer(struct peer *peer) peer->slot_obmc_endpoint = NULL; sd_bus_slot_unref(peer->slot_cc_endpoint); peer->slot_cc_endpoint = NULL; + sd_bus_slot_unref(peer->slot_bridge); + peer->slot_bridge = NULL; sd_bus_slot_unref(peer->slot_uuid); peer->slot_uuid = NULL; peer->published = false; @@ -3207,6 +3213,28 @@ static int bus_endpoint_get_prop(sd_bus *bus, const char *path, return rc; } +static int bus_bridge_get_prop(sd_bus *bus, const char *path, + const char *interface, const char *property, + sd_bus_message *reply, void *userdata, + sd_bus_error *berr) +{ + struct peer *peer = userdata; + int rc; + + if (strcmp(property, "PoolStart") == 0) { + rc = sd_bus_message_append(reply, "y", peer->pool_start); + } else if (strcmp(property, "PoolEnd") == 0) { + uint8_t pool_end = peer->pool_start + peer->pool_size - 1; + rc = sd_bus_message_append(reply, "y", pool_end); + } else { + warnx("Unknown bridge property '%s' for %s iface %s", property, + path, interface); + rc = -ENOENT; + } + + return rc; +} + static int bus_network_get_prop(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, @@ -3406,6 +3434,21 @@ static const sd_bus_vtable bus_endpoint_cc_vtable[] = { SD_BUS_VTABLE_END }; +static const sd_bus_vtable bus_endpoint_bridge[] = { + SD_BUS_VTABLE_START(0), + SD_BUS_PROPERTY("PoolStart", + "y", + bus_bridge_get_prop, + 0, + SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("PoolEnd", + "y", + bus_bridge_get_prop, + 0, + SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_VTABLE_END +}; + static const sd_bus_vtable bus_link_vtable[] = { SD_BUS_VTABLE_START(0), SD_BUS_WRITABLE_PROPERTY("Role", @@ -4500,6 +4543,17 @@ static int endpoint_allocate_eid(struct peer *peer) return rc; } } + sd_bus_add_object_vtable(peer->ctx->bus, &peer->slot_bridge, + peer->path, CC_MCTP_DBUS_IFACE_BRIDGE, + bus_endpoint_bridge, peer); + rc = sd_bus_emit_interfaces_added(peer->ctx->bus, peer->path, + CC_MCTP_DBUS_IFACE_BRIDGE, + NULL); + if (rc < 0) { + warnx("Failed to emit add %s signal for endpoint %d : %s", + CC_MCTP_DBUS_IFACE_BRIDGE, peer->eid, + strerror(-rc)); + } // TODO: Polling logic for downstream EID } diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index 50d89b1..37933c3 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -20,6 +20,7 @@ MCTPD_MCTP_P = '/au/com/codeconstruct/mctp1' MCTPD_MCTP_I = 'au.com.codeconstruct.MCTP.BusOwner1' MCTPD_ENDPOINT_I = 'au.com.codeconstruct.MCTP.Endpoint1' +MCTPD_ENDPOINT_BRIDGE_I = 'au.com.codeconstruct.MCTP.Bridge1' DBUS_OBJECT_MANAGER_I = 'org.freedesktop.DBus.ObjectManager' DBUS_PROPERTIES_I = 'org.freedesktop.DBus.Properties' @@ -962,3 +963,119 @@ async def test_assign_dynamic_bridge_eid(dbus, mctpd): await mctp.call_assign_endpoint_static(dev2.lladdr, ep.eid + 1) assert str(ex.value) == "Request failed" + +""" 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): + max_pool_size = 1 + config = f""" + [bus-owner] + max_pool_size = {max_pool_size} + """ + + mctpd = MctpdWrapper(dbus, sysnet, config = config) + await mctpd.start_mctpd(nursery) + + iface = mctpd.system.interfaces[0] + ep = mctpd.network.endpoints[0] + mctp = await mctpd_mctp_iface_obj(dbus, iface) + + # Set up bridged endpoints as undiscovered EID 0 + for i in range(0, 2): + br_ep = Endpoint(iface, bytes(), types=[0, 2]) + ep.add_bridged_ep(br_ep) + mctpd.network.add_endpoint(br_ep) + + # dynamic EID assigment for dev1 + (eid, _, path, new) = await mctp.call_assign_endpoint(ep.lladdr) + + assert new + + bridge_obj = await dbus.get_proxy_object(MCTPD_C, path) + props_iface = await bridge_obj.get_interface(DBUS_PROPERTIES_I) + pool_end = await props_iface.call_get(MCTPD_ENDPOINT_BRIDGE_I, "PoolEnd") + pool_size = pool_end.value - eid + assert pool_size == max_pool_size + + res = await mctpd.stop_mctpd() + assert res == 0 + +""" Test that no pool is assigned for requested pool size from + unavailable pool space""" +async def test_assign_dynamic_unavailable_pool(nursery, dbus, sysnet): + (min_dyn_eid, max_dyn_eid) = (8, 12) + config = f""" + [bus-owner] + dynamic_eid_range = [{min_dyn_eid}, {max_dyn_eid}] + """ + + mctpd = MctpdWrapper(dbus, sysnet, config = config) + await mctpd.start_mctpd(nursery) + + iface = mctpd.system.interfaces[0] + ep = mctpd.network.endpoints[0] + mctp = await mctpd_mctp_iface_obj(dbus, iface) + + # Set up bridged endpoints as undiscovered EID 0 + for i in range(0, 2): + br_ep = Endpoint(iface, bytes(), types=[0, 2]) + ep.add_bridged_ep(br_ep) + mctpd.network.add_endpoint(br_ep) + + # consume middle eid from the range to dev2 + dev2 = Endpoint(iface, bytes([0x09])) + mctpd.network.add_endpoint(dev2) + (eid, _, path, new) = await mctp.call_assign_endpoint_static( + dev2.lladdr, + 10 + ) + assert new + + # dynamic EID assigment for dev1 + (eid, _, path, new) = await mctp.call_assign_endpoint(ep.lladdr) + assert new + # Interface should not be present for unavailable pool space + with pytest.raises(asyncdbus.errors.InterfaceNotFoundError): + bridge_obj = await dbus.get_proxy_object(MCTPD_C, path) + await bridge_obj.get_interface(MCTPD_ENDPOINT_BRIDGE_I) + + 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): + class BridgeEndpoint(Endpoint): + async def handle_mctp_control(self, sock, src_addr, msg): + flags, opcode = msg[0:2] + if opcode != 0x8: + 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 + ]) + await sock.send(dst_addr, msg) + + iface = mctpd.system.interfaces[0] + ep = BridgeEndpoint(iface, bytes([0x1e])) + mctpd.network.add_endpoint(ep) + # Set up downstream endpoints as undiscovered EID 0 + for i in range(0, 2): + br_ep = Endpoint(iface, bytes(), types=[0, 2]) + ep.add_bridged_ep(br_ep) + mctpd.network.add_endpoint(br_ep) + mctp = await mctpd_mctp_iface_obj(dbus, iface) + + # dynamic EID assigment for dev1 + (eid, _, path, new) = await mctp.call_assign_endpoint(ep.lladdr) + assert new + # Interface should not be present for failed pool allocation + with pytest.raises(asyncdbus.errors.InterfaceNotFoundError): + bridge_obj = await dbus.get_proxy_object(MCTPD_C, path) + await bridge_obj.get_interface(MCTPD_ENDPOINT_BRIDGE_I) From e7e72dd993fc155b539c49420e2c5d5fbee925f9 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Wed, 20 Aug 2025 17:44:02 +0800 Subject: [PATCH 7/8] tests: mctpd: split dynamic bridge test into smaller components Currently, test_assign_dynamic_bridge_eid test both the bridge assignment, and conflicts against static EIDs. Instead, split this into two smaller tests, which provide a base for future bridge-conflict tests. Signed-off-by: Jeremy Kerr --- tests/test_mctpd.py | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index 37933c3..48b6cd2 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -943,26 +943,48 @@ async def test_assign_dynamic_bridge_eid(dbus, mctpd): # Set up bridged endpoints as undiscovered EID 0 for i in range(pool_size): - br_ep = Endpoint(iface, bytes(), types=[0, 2]) + br_ep = Endpoint(iface, bytes(), types=[0]) ep.add_bridged_ep(br_ep) mctpd.network.add_endpoint(br_ep) - mctp = await mctpd_mctp_iface_obj(dbus, iface) - # dynamic EID assigment for dev1 (eid, _, path, new) = await mctp.call_assign_endpoint(ep.lladdr) assert new assert ep.allocated_pool == (eid + 1, pool_size) - # check if we can assign non-bridged endpoint dev2, eid from - #bridge's already assigned pool - dev2 = Endpoint(iface, bytes([0x1e])) - mctpd.network.add_endpoint(dev2) - with pytest.raises(asyncdbus.errors.DBusError) as ex: - await mctp.call_assign_endpoint_static(dev2.lladdr, ep.eid + 1) +""" Test that static allocations are not permitted, if they would conflict +with a bridge pool""" +async def test_bridge_ep_conflict_static(dbus, mctpd): + iface = mctpd.system.interfaces[0] + mctp = await mctpd_mctp_iface_obj(dbus, iface) + ep = mctpd.network.endpoints[0] + n_bridged = 3 - assert str(ex.value) == "Request failed" + # add downstream devices + for i in range(n_bridged): + br_ep = Endpoint(iface, bytes()) + ep.add_bridged_ep(br_ep) + + (eid, _, path, new) = await mctp.call_assign_endpoint(ep.lladdr) + assert ep.allocated_pool == (eid + 1, n_bridged) + + # ensure no static assignment can be made from the bridged range + for i in range(n_bridged): + dev = Endpoint(iface, bytes([0x30 + i])) + mctpd.network.add_endpoint(dev) + with pytest.raises(asyncdbus.errors.DBusError): + await mctp.call_assign_endpoint_static(dev.lladdr, ep.eid + 1 + i) + + # ... but we're okay with the EID following + dev = Endpoint(iface, bytes([0x30 + n_bridged])) + mctpd.network.add_endpoint(dev) + static_eid = ep.eid + 1 + n_bridged + (eid, _, _, _) = await mctp.call_assign_endpoint_static( + dev.lladdr, static_eid + ) + + assert eid == static_eid """ Test that we truncate the requested pool size to the max_pool_size config """ From d6d11e37302518b126c9ed277825427c5a254792 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Wed, 20 Aug 2025 18:11:46 +0800 Subject: [PATCH 8/8] tests: mctpd: add bridge conflict-from-learnt-eid test In addition to the static assignments, we want to ensure that LearnEndpoint does not result in EID conflicts. Signed-off-by: Jeremy Kerr --- tests/test_mctpd.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index 48b6cd2..5403dae 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -986,6 +986,37 @@ 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): + iface = mctpd.system.interfaces[0] + mctp = await mctpd_mctp_iface_obj(dbus, iface) + ep = mctpd.network.endpoints[0] + n_bridged = 3 + + # add downstream devices + for i in range(n_bridged): + br_ep = Endpoint(iface, bytes()) + ep.add_bridged_ep(br_ep) + + (eid, _, path, new) = await mctp.call_assign_endpoint(ep.lladdr) + assert ep.allocated_pool == (eid + 1, n_bridged) + + # ensure no learnt assignment can be made from the bridged range + for i in range(n_bridged): + dev = Endpoint(iface, bytes([0x30 + i]), eid=ep.eid + 1 + i) + mctpd.network.add_endpoint(dev) + with pytest.raises(asyncdbus.errors.DBusError): + await mctp.call_learn_endpoint(dev.lladdr) + + # ... but we're okay with the EID following + dev_eid = ep.eid + 1 + n_bridged + dev = Endpoint(iface, bytes([0x30 + n_bridged]), eid=dev_eid) + mctpd.network.add_endpoint(dev) + (eid, _, _, _) = await mctp.call_learn_endpoint(dev.lladdr) + + assert eid == dev_eid + """ 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):