mctpd: Automatic Discovery Notify for endpoint role

docs/mctpd.md

@@ -245,6 +245,53 @@ reports as a bridge.
 
 Bridge endpoints should be initialised with `AssignEndpoint` instead.
 
+### Endpoint interface: `au.com.codeconstruct.MCTP.Endpoint1` interface
+
+When the interface `Role` is `Endpoint`, the MCTP interface object also hosts
+the `au.com.codeconstruct.MCTP.Endpoint1` D-Bus interface. This exposes
+endpoint-role functions on the per-interface D-Bus path.
+
+```
+NAME                                  TYPE      SIGNATURE RESULT/VALUE FLAGS
+au.com.codeconstruct.MCTP.Interface1  interface -         -            -
+.NetworkId                            property  u         1            emits-change
+.Role                                 property  s         "Endpoint"   emits-change writable
+au.com.codeconstruct.MCTP.Endpoint1   interface -         -            -
+.BusOwner                             property  ay        -            writable
+```
+
+#### `.BusOwner`: `ay`
+
+Sets the physical address of the bus owner for this interface. For transport
+types that know the bus owner physaddr (or do not need one to address the bus
+owner), this would be populated automatically by the transport binding. For
+other cases, this property allows the address to be supplied by an external
+management application.
+
+On transport types that require a Discovery Notify (PCIe and I3C), setting the
+bus owner address will automatically trigger a Discovery Notify to the bus
+owner if the interface is in Endpoint role and not yet discovered.
+
+For transport types that do not use physical addressing for bus owner
+communication (such as USB and serial), this property has no effect;
+Discovery Notify is only triggered on transports that require it (PCIe VDM
+and I3C).
+
+`BusOwner <hwaddr>`
+
+ - `<hwaddr>` Physical hardware address of the bus owner. For MCTP-over-I3C
+   this is the 6-byte Provisional ID (PID) of the bus owner.
+
+An example for MCTP-over-I3C, setting the bus owner's 6-byte PID
+`00 6c 90 01 23 45`:
+
+```shell
+busctl set-property au.com.codeconstruct.MCTP1 \
+    /au/com/codeconstruct/mctp1/interfaces/mctpi3c0 \
+    au.com.codeconstruct.MCTP.Endpoint1 \
+    BusOwner ay 6 0x00 0x6c 0x90 0x01 0x23 0x45
+```
+
 ## Network objects: `/au/com/codeconstruct/mctp1/networks/<net>`
 
 These objects represent MCTP networks which have been added use `mctp link`

src/mctpd.c

@@ -151,8 +151,12 @@ struct link {
 	char *path;
 	sd_bus_slot *slot_iface;
 	sd_bus_slot *slot_busowner;
+	sd_bus_slot *slot_endpoint;
 	sd_event_source *role_defer;
 
+	/* Physical address of the bus owner. */
+	dest_phys bus_owner;
+
 	struct ctx *ctx;
 };
 
@@ -389,6 +393,7 @@ 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[];
+static const sd_bus_vtable bus_link_endpoint_vtable[];
 
 __attribute__((format(printf, 1, 2))) static void bug_warn(const char *fmt, ...)
 {
@@ -4255,6 +4260,138 @@ static int method_register_vdm_type_support(sd_bus_message *call, void *data,
 	return rc;
 }
 
+static bool link_supports_discovery_notify(const struct link *link)
+{
+	return (link->phys_binding == MCTP_PHYS_BINDING_PCIE_VDM ||
+		link->phys_binding == MCTP_PHYS_BINDING_I3C) &&
+	       link->role == ENDPOINT_ROLE_ENDPOINT &&
+	       link->discovered != DISCOVERY_DISCOVERED;
+}
+
+static void link_attempt_discovery_notify(struct link *link)
+{
+	struct mctp_ctrl_resp_discovery_notify *resp = NULL;
+	struct mctp_ctrl_cmd_discovery_notify req = { 0 };
+	struct mctp_ctrl_cmd cmd = { 0 };
+	struct ctx *ctx = link->ctx;
+	unsigned int retry;
+	uint8_t iid;
+	int rc;
+
+	if (!link_supports_discovery_notify(link))
+		return;
+
+	for (retry = 0; retry < 4; retry++) {
+		iid = mctp_next_iid(ctx);
+		mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid,
+					   MCTP_CTRL_CMD_DISCOVERY_NOTIFY);
+		mctp_ctrl_cmd_init_from_req_type(&cmd, req);
+
+		rc = endpoint_query_phys(ctx, &link->bus_owner, &cmd);
+		if (rc < 0) {
+			mctp_ctrl_cmd_free(&cmd);
+			break;
+		}
+
+		rc = mctp_ctrl_validate_response(
+			&cmd, sizeof(*resp), dest_phys_tostr(&link->bus_owner),
+			iid, MCTP_CTRL_CMD_DISCOVERY_NOTIFY);
+		mctp_ctrl_cmd_free(&cmd);
+		if (rc == 0)
+			return;
+		/* retry on non-fatal completion code errors */
+		if (rc != -EBUSY)
+			break;
+	}
+
+	if (rc < 0)
+		warnx("Discovery Notify on %s failed: %s", link->path,
+		      strerror(-rc));
+}
+
+static int bus_link_endpoint_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 link *link = userdata;
+	int rc;
+
+	if (strcmp(property, "BusOwner") == 0) {
+		rc = sd_bus_message_append_array(reply, 'y',
+						 link->bus_owner.hwaddr,
+						 link->bus_owner.hwaddr_len);
+	} else {
+		warnx("Unknown property '%s' for %s iface %s", property, path,
+		      interface);
+		rc = -ENOENT;
+	}
+
+	return rc;
+}
+
+static int bus_link_endpoint_set_prop(sd_bus *bus, const char *path,
+				      const char *interface,
+				      const char *property,
+				      sd_bus_message *value, void *userdata,
+				      sd_bus_error *berr)
+{
+	struct link *link = userdata;
+	struct ctx *ctx = link->ctx;
+	dest_phys dest = { 0 };
+	int rc;
+
+	if (strcmp(property, "BusOwner") != 0) {
+		warnx("Unknown property '%s' for %s iface %s", property, path,
+		      interface);
+		rc = -ENOENT;
+		goto out;
+	}
+
+	dest.ifindex = link->ifindex;
+	if (dest.ifindex <= 0) {
+		sd_bus_error_setf(berr, SD_BUS_ERROR_INVALID_ARGS,
+				  "Unknown MCTP interface");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	rc = message_read_hwaddr(value, &dest);
+	if (rc < 0)
+		goto out;
+
+	rc = validate_dest_phys(ctx, &dest);
+	if (rc < 0) {
+		sd_bus_error_setf(berr, SD_BUS_ERROR_INVALID_ARGS,
+				  "Bad physaddr");
+		goto out;
+	}
+
+	link->bus_owner = dest;
+	if (link->ctx->verbose)
+		warnx("BusOwner for %s set to %s", link->path,
+		      dest_phys_tostr(&link->bus_owner));
+	link_attempt_discovery_notify(link);
+	rc = 0;
+out:
+	set_berr(ctx, rc, berr);
+	return rc;
+}
+
+// clang-format off
+static const sd_bus_vtable bus_link_endpoint_vtable[] = {
+	SD_BUS_VTABLE_START(0),
+	SD_BUS_WRITABLE_PROPERTY("BusOwner",
+		"ay",
+		bus_link_endpoint_get_prop,
+		bus_link_endpoint_set_prop,
+		0,
+		0),
+	SD_BUS_VTABLE_END,
+};
+// clang-format on
+
 // clang-format off
 static const sd_bus_vtable bus_link_owner_vtable[] = {
 	SD_BUS_VTABLE_START(0),
@@ -4467,14 +4604,22 @@ static int link_set_role(sd_event_source *ev, void *userdata)
 	sd_event_source_unref(link->role_defer);
 	link->role_defer = NULL;
 
-	if (link->role != ENDPOINT_ROLE_BUS_OWNER)
-		return 0;
-
-	rc = sd_bus_add_object_vtable(link->ctx->bus, &link->slot_busowner,
-				      link->path, CC_MCTP_DBUS_IFACE_BUSOWNER,
-				      bus_link_owner_vtable, link);
-	if (rc)
-		warnx("adding link owner vtable failed: %d", rc);
+	if (link->role == ENDPOINT_ROLE_BUS_OWNER) {
+		rc = sd_bus_add_object_vtable(link->ctx->bus,
+					      &link->slot_busowner, link->path,
+					      CC_MCTP_DBUS_IFACE_BUSOWNER,
+					      bus_link_owner_vtable, link);
+		if (rc)
+			warnx("adding link owner vtable failed: %d", rc);
+	} else if (link->role == ENDPOINT_ROLE_ENDPOINT) {
+		rc = sd_bus_add_object_vtable(link->ctx->bus,
+					      &link->slot_endpoint, link->path,
+					      CC_MCTP_DBUS_IFACE_ENDPOINT,
+					      bus_link_endpoint_vtable, link);
+		if (rc)
+			warnx("adding link endpoint vtable failed: %d", rc);
+		link_attempt_discovery_notify(link);
+	}
 
 	return 0;
 }
@@ -4965,6 +5110,7 @@ static void free_link(struct link *link)
 	sd_event_source_disable_unref(link->role_defer);
 	sd_bus_slot_unref(link->slot_iface);
 	sd_bus_slot_unref(link->slot_busowner);
+	sd_bus_slot_unref(link->slot_endpoint);
 	free(link->path);
 	free(link->sysfs_path);
 	free(link);
@@ -5034,6 +5180,8 @@ static int rename_interface(struct ctx *ctx, struct link *link, int ifindex)
 	link->slot_iface = NULL;
 	sd_bus_slot_unref(link->slot_busowner);
 	link->slot_busowner = NULL;
+	sd_bus_slot_unref(link->slot_endpoint);
+	link->slot_endpoint = NULL;
 	free(link->path);
 
 	/* set new path and re-add */
@@ -5047,6 +5195,11 @@ static int rename_interface(struct ctx *ctx, struct link *link, int ifindex)
 					 link->path,
 					 CC_MCTP_DBUS_IFACE_BUSOWNER,
 					 bus_link_owner_vtable, link);
+	} else if (link->role == ENDPOINT_ROLE_ENDPOINT) {
+		sd_bus_add_object_vtable(link->ctx->bus, &link->slot_endpoint,
+					 link->path,
+					 CC_MCTP_DBUS_IFACE_ENDPOINT,
+					 bus_link_endpoint_vtable, link);
 	}
 
 	emit_interface_added(link);
@@ -5420,6 +5573,11 @@ static int add_interface(struct ctx *ctx, int ifindex)
 					 link->path,
 					 CC_MCTP_DBUS_IFACE_BUSOWNER,
 					 bus_link_owner_vtable, link);
+	} else if (link->role == ENDPOINT_ROLE_ENDPOINT) {
+		sd_bus_add_object_vtable(link->ctx->bus, &link->slot_endpoint,
+					 link->path,
+					 CC_MCTP_DBUS_IFACE_ENDPOINT,
+					 bus_link_endpoint_vtable, link);
 	}
 
 	if (link->phys_binding == MCTP_PHYS_BINDING_PCIE_VDM) {
@@ -5432,6 +5590,8 @@ static int add_interface(struct ctx *ctx, int ifindex)
 		link->published = false;
 	}
 
+	link_attempt_discovery_notify(link);
+
 	return rc;
 
 err_free:

tests/mctp_test_utils.py

@@ -21,6 +21,14 @@ async def mctpd_mctp_iface_control_obj(dbus, iface):
     return await obj.get_interface("au.com.codeconstruct.MCTP.Interface1")
 
 
+async def mctpd_mctp_iface_endpoint_obj(dbus, iface):
+    obj = await dbus.get_proxy_object(
+        'au.com.codeconstruct.MCTP1',
+        '/au/com/codeconstruct/mctp1/interfaces/' + iface.name,
+    )
+    return await obj.get_interface('au.com.codeconstruct.MCTP.Endpoint1')
+
+
 async def mctpd_mctp_endpoint_obj(dbus, path, iface):
     obj = await dbus.get_proxy_object(
         'au.com.codeconstruct.MCTP1',

tests/mctpenv/__init__.py

@@ -537,6 +537,11 @@ async def handle_mctp_control(self, sock, addr, data):
                 )
                 await sock.send(raddr, data)
 
+            elif opcode == 0x0D:
+                # Discovery Notify — respond with completion_code = 0
+                data = bytes(hdr + [0x00])
+                await sock.send(raddr, data)
+
             else:
                 await sock.send(
                     raddr, bytes(hdr + [0x05])

tests/test_mctpd_endpoint.py

@@ -2,6 +2,7 @@
 import asyncdbus
 from mctp_test_utils import (
     mctpd_mctp_iface_control_obj,
+    mctpd_mctp_iface_endpoint_obj,
     mctpd_mctp_endpoint_control_obj,
 )
 from mctpenv import (
@@ -218,3 +219,34 @@ async def test_simple(self, dbus, mctpd):
             mctpd.network.mctp_socket, MCTPControlCommand(True, 0, 0x0B)
         )
         assert rsp.hex(' ') == '00 0b 05'
+
+
+class TestDiscoveryNotify:
+    """Discovery Notify for I3C endpoints with a 6-byte bus owner PID."""
+
+    # Example 6-byte I3C Provisional ID of the bus owner
+    BO_PID = bytes([0x00, 0x6C, 0x90, 0x01, 0x23, 0x45])
+
+    @pytest.fixture
+    async def iface(self):
+        return System.Interface(
+            "mctp0", 1, 1, bytes([0x1D]), 68, 254, True, PhysicalBinding.I3C
+        )
+
+    @pytest.fixture
+    async def sysnet(self, iface):
+        system = System()
+        await system.add_interface(iface)
+        network = Network()
+        network.add_endpoint(Endpoint(iface, self.BO_PID, eid=8))
+        return Sysnet(system, network)
+
+    async def test_discovery_notify_on_set_bus_owner(self, dbus, mctpd):
+        """Discovery Notify is sent automatically when the 6-byte I3C bus owner PID is set."""
+        iface = mctpd.system.interfaces[0]
+        bo = mctpd.network.endpoints[0]
+
+        ep = await mctpd_mctp_iface_endpoint_obj(dbus, iface)
+        # Set the bus owner's 6-byte I3C PID; this satisfies all conditions
+        # and triggers Discovery Notify to the bus owner immediately.
+        await ep.set_bus_owner(bo.lladdr)