diff --git a/src/mctpd.c b/src/mctpd.c index a1d2728..bdb3329 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -2237,7 +2237,6 @@ static void free_peers(struct ctx *ctx) static int change_peer_eid(struct peer *peer, mctp_eid_t new_eid) { struct net *n = NULL; - int rc; if (!mctp_eid_is_valid_unicast(new_eid)) return -EINVAL; @@ -2256,15 +2255,17 @@ static int change_peer_eid(struct peer *peer, mctp_eid_t new_eid) if (n->peers[new_eid]) return -EEXIST; - /* publish & unpublish will update peer->path */ + /* The object path contains the EID, so the peer is left unpublished + * and the caller is responsible for publishing: publish_peer + * registers the UUID vtable only when peer->uuid is already known, + * so publishing before query_peer_properties has run would + * permanently hide the UUID interface (the assign-mismatch paths + * reach here before the properties are queried). */ unpublish_peer(peer); n->peers[new_eid] = n->peers[peer->eid]; n->peers[peer->eid] = NULL; peer->eid = new_eid; add_peer_route(peer); - rc = publish_peer(peer); - if (rc) - return rc; return 0; } @@ -3813,6 +3814,15 @@ static int peer_endpoint_recover(sd_event_source *s, uint64_t usec, if (rc < 0) { goto reclaim; } + /* change_peer_eid() leaves the peer unpublished; the + * object path moves with the EID. The properties were + * queried when the peer was first set up, so + * publishing here re-registers the full interface + * set. */ + rc = publish_peer(peer); + if (rc < 0) { + goto reschedule; + } } } diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index 7853794..88cba6b 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -8,6 +8,7 @@ mctpd_mctp_network_obj, mctpd_mctp_endpoint_common_obj, mctpd_mctp_endpoint_control_obj, + mctpd_mctp_endpoint_obj, mctpd_mctp_base_iface_obj, ) from mctpenv import ( @@ -732,6 +733,46 @@ async def handle_mctp_control(self, sock, src_addr, msg): assert eid == ep.eid +async def test_assign_endpoint_vary_set_eid_uuid(dbus, mctpd): + """An endpoint that holds its current EID and reports it in the Set + Endpoint ID response takes the EID-change path, where the UUID query only + completes after the object would first be published; the endpoint object + must still expose the UUID interface, same as a plain enrollment. + """ + + class VaryEndpoint(Endpoint): + async def handle_mctp_control(self, sock, src_addr, msg): + flags, opcode = msg[0:2] + if opcode != 1: + return await super().handle_mctp_control(sock, src_addr, msg) + dst_addr = MCTPSockAddr.for_ep_resp(self, src_addr, sock.addr_ext) + self.eid = msg[3] + 1 + msg = bytes( + [ + flags & 0x1F, # Rsp + 0x01, # opcode: Set Endpoint ID + 0x00, # cc: success + 0x00, # assignment accepted, no pool + self.eid, # set EID: valid, but not what was assigned + 0x00, # pool size: 0 + ] + ) + await sock.send(dst_addr, msg) + + iface = mctpd.system.interfaces[0] + ep = VaryEndpoint(iface, bytes([0x1E])) + mctpd.network.add_endpoint(ep) + mctp = await mctpd_mctp_iface_obj(dbus, iface) + + eid, _, path, _ = await mctp.call_assign_endpoint(ep.lladdr) + + assert eid == ep.eid + uuid_i = await mctpd_mctp_endpoint_obj( + dbus, path, "xyz.openbmc_project.Common.UUID" + ) + assert await uuid_i.get_uuid() == str(ep.uuid) + + async def test_setup_endpoint_conflicting_set_eid_response(dbus, mctpd): """During a SetupEndpoint's Set Endpoint ID exchange, return a response that indicates that the EID has been set, but report a different set EID in