From bc5704cfef8779b6ebe364521f40ff169d92b91e Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Tue, 6 Feb 2024 14:11:05 +0800 Subject: [PATCH 1/3] mctpd: dbus interface rework This change implements the dbus interface rework specified in https://github.com/CodeConstruct/mctp/issues/40. Essentially: this uses more standard bus, object and path names, and moves away from the xyz.openbmc_project namespace, and use au.com.codeconstruct (all lowercase) there instead, as we're not specificially an OpenBMC project. We also put collections of things (networks and endpoints) under a specifically-named object path, so we can introduce new collections alongside (interfaces) without compatibility issues This means: - the bus owner name is now au.com.codeconstruct.MCTP1 - interfaces are namespaced and versioned: - au.com.codeconstruct.MCTP.Endpoint1 - au.com.codeconstruct.MCTP.BusOwner1 - the top-level entrypoint path is versioned, as `/au/com/codeconstruct/mctp1` - the endpoint object tree is structured as `/au/com/codeconstruct/mctp1/networks//endpoints/` Signed-off-by: Jeremy Kerr --- CHANGELOG.md | 4 ++ README.md | 10 ++--- conf/mctpd-dbus.conf | 6 +-- conf/mctpd.service | 2 +- docs/endpoint-recovery.md | 60 +++++++++++++++-------------- docs/mctpd.md | 81 +++++++++++++++++++++++---------------- meson.build | 2 +- meson_options.txt | 2 +- src/mctpd.c | 32 +++++++++------- tests/conftest.py | 2 +- tests/mctp_test_utils.py | 10 ++--- tests/test_mctpd.py | 8 ++-- 12 files changed, 122 insertions(+), 97 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2ffd67..ee01e0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). 3. The `tests` option has changed type from `feature` to `boolean`. Tests are enabled by default. +4. The dbus interface has undergone a major rework, using standard prefixes + and version interface, bus owner and entry-point object names. See + docs/mctpd.md for full details on the new interface. + ### Fixed 1. mctpd: EID assignments now work in the case where a new endpoint has a diff --git a/README.md b/README.md index 0dc2fc7..033ed7c 100644 --- a/README.md +++ b/README.md @@ -73,12 +73,12 @@ coordinate the local setup and the supervision of the mctpd process. An example configuration is in [`conf/mctpd.conf`](conf/mctpd.conf). The `mctpd` daemon will expose a dbus interface, claiming the bus name -`xyz.openbmc_project.MCTP` and object path `/xyz/openbmc_project/mctp`. This +`au.com.codeconstruct.MCTP1` and object path `/au/com/codeconstruct/mctp1`. This provides a few functions for configuring remote endpoints: - # busctl introspect xyz.openbmc_project.MCTP /xyz/openbmc_project/mctp + # busctl introspect au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1/ NAME TYPE SIGNATURE RESULT/VALUE FLAGS - au.com.CodeConstruct.MCTP interface - - - + au.com.codeconstruct.MCTP interface - - - .AssignEndpoint method say yisb - .AssignEndpointStatic method sayy yisb - .LearnEndpoint method say yisb - @@ -88,7 +88,7 @@ Results of mctpd enumeration are also represented as dbus objects, using the OpenBMC-specified MCTP endpoint format. Each endpoint appears on the bus at the object path: - /xyz/openbmc_project/mctp// + /au/com/codeconstruct/mctp/networks//endpoints/ where `mctpd` exposes three dbus interfaces for each: @@ -103,7 +103,7 @@ where `mctpd` exposes three dbus interfaces for each: This interface is defined by the Common.UUID phosphor-dbus specification. - - `au.com.CodeConstruct.MCTP.EndPoint`: Additional control methods for the + - `au.com.codeconstruct.MCTP1.Endpoint1`: Additional control methods for the endpoint - for example, `Remove` Testing diff --git a/conf/mctpd-dbus.conf b/conf/mctpd-dbus.conf index e35eaec..e97bfe0 100644 --- a/conf/mctpd-dbus.conf +++ b/conf/mctpd-dbus.conf @@ -3,8 +3,8 @@ "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd"> - - - + + + diff --git a/conf/mctpd.service b/conf/mctpd.service index 5f1eb6c..62410c1 100644 --- a/conf/mctpd.service +++ b/conf/mctpd.service @@ -6,7 +6,7 @@ After=mctp-local.target [Service] Type=dbus -BusName=xyz.openbmc_project.MCTP +BusName=au.com.codeconstruct.MCTP1 ExecStart=/usr/sbin/mctpd [Install] diff --git a/docs/endpoint-recovery.md b/docs/endpoint-recovery.md index 63180c1..d67ab4a 100644 --- a/docs/endpoint-recovery.md +++ b/docs/endpoint-recovery.md @@ -144,20 +144,21 @@ issues. This may include applications in addition to the process which requested `mctpd` currently structures its D-Bus objects as follows: ``` -root@cc-nvme-mi:~# busctl tree xyz.openbmc_project.MCTP -`-/xyz - `-/xyz/openbmc_project - `-/xyz/openbmc_project/mctp - `-/xyz/openbmc_project/mctp/1 - |-/xyz/openbmc_project/mctp/1/12 - |-/xyz/openbmc_project/mctp/1/8 - `-/xyz/openbmc_project/mctp/1/9 +root@cc-nvme-mi:~# busctl tree au.com.codeconstruct.MCTP1 +└─/au + └─/au/com + └─/au/com/codeconstruct + └─/au/com/codeconstruct/mctp1 + ├─/au/com/codeconstruct/mctp1/networks/1 + │ └─/au/com/codeconstruct/mctp1/networks/1/endpoints/12 + ├─/au/com/codeconstruct/mctp1/networks/1/endpoints/8 + └─/au/com/codeconstruct/mctp1/networks/1/endpoints/9 ``` ``` -root@cc-nvme-mi:~# busctl introspect xyz.openbmc_project.MCTP /xyz/openbmc_project/mctp +root@cc-nvme-mi:~# busctl introspect au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1 NAME TYPE SIGNATURE RESULT/VALUE FLAGS -au.com.CodeConstruct.MCTP interface - - - +au.com.codeconstruct.Endpoint1 interface - - - .AssignEndpoint method say yisb - .LearnEndpoint method say yisb - .SetupEndpoint method say yisb - @@ -178,7 +179,7 @@ org.freedesktop.DBus.Properties interface - - - ``` ``` -root@cc-nvme-mi:~# busctl introspect xyz.openbmc_project.MCTP /xyz/openbmc_project/mctp/1 +root@cc-nvme-mi:~# busctl introspect au.com.codeconstruct.MCTP1 /au/com/codecontrust/mctp1/networks/1 NAME TYPE SIGNATURE RESULT/VALUE FLAGS org.freedesktop.DBus.Introspectable interface - - - .Introspect method - s - @@ -193,9 +194,9 @@ org.freedesktop.DBus.Properties interface - - - ``` ``` -root@cc-nvme-mi:~# busctl introspect xyz.openbmc_project.MCTP /xyz/openbmc_project/mctp/1/9 +root@cc-nvme-mi:~# busctl introspect au.com.codeconstruct.MCTP1 /au/com/codecontrust/mctp1/networks/1/endpoints/9 NAME TYPE SIGNATURE RESULT/VALUE FLAGS -au.com.CodeConstruct.MCTP.Endpoint interface - - - +au.com.codeconstruct.MCTP.Endpoint1 interface - - - .Remove method - - - .SetMTU method u - - org.freedesktop.DBus.Introspectable interface - - - @@ -217,16 +218,16 @@ xyz.openbmc_project.MCTP.Endpoint interface - - ``` The problem we have deals with specific endpoints. To satisfy the third scenario -the object `/xyz/openbmc_project/mctp/1/9` would be removed. +the object `/au/com/codeconstruct/mctp1/networks/1/endpoints/9` would be removed. For scenarios 1 and 2 we need to add capabilities on the endpoint object. The -`au.com.CodeConstruct.MCTP.Endpoint` already exposes endpoint control APIs, and +`au.com.codeconstruct.MCTP.Endpoint` already exposes endpoint control APIs, and given we're not removing capabilities we can change it. ## Proposed Design The approach is to add a `.Recover` method and a `.Connectivity` property to the -`au.com.CodeConstruct.MCTP.Endpoint` interface. `.Recover` takes no arguments, +`au.com.codeconstruct.MCTP.Endpoint1` interface. `.Recover` takes no arguments, produces no result, and returns immediately. `.Connectivity` takes one of two values: @@ -346,11 +347,11 @@ The general strategy for tracking endpoint lifecycles is [as follows] data-structures ``` - type='signal',sender='xyz.openbmc_project.MCTP',path_namespace='/xyz/openbmc_project/mctp' + type='signal',sender='xyz.openbmc_project.MCTP',path_namespace='/au/com/codeconstruct/mctp1' ``` 2. Issue a call to `GetManagedObjects` `org.freedesktop.DBus.ObjectManager` on - `/xyz/openbmc_project/mctp` for initial population of local data-structures + `/au/com/codeconstruct/mctp1` for initial population of local data-structures 3. Issue @@ -360,7 +361,8 @@ The general strategy for tracking endpoint lifecycles is [as follows] registered as an MCTP endpoint. 2. The FRU data is decoded and `SetupEndpoint` on the - `au.com.CodeConstruct.MCTP` interface of the `/xyz/openbmc_project/mctp` + `au.com.codeconstruct.MCTP.BusOwner1` interface of the + `/au/com/codeconstruct/mctp1` object hosted by `mctpd` is invoked. 3. The device was not previously configured and has no programmed static EID. @@ -372,8 +374,8 @@ The general strategy for tracking endpoint lifecycles is [as follows] interfaces): ``` - "/xyz/openbmc_project/mctp/1/9": { - "au.com.CodeConstruct.MCTP.Endpoint": { }, + "/au.com.codeconstruct/mctp1/networks/1/endpoints/9": { + "au.com.codeconstruct.MCTP.Endpoint1": { }, "xyz.openbmc_project.Common.UUID": { "UUID": "..." }, @@ -394,16 +396,18 @@ The general strategy for tracking endpoint lifecycles is [as follows] 7. MCTP endpoint 9 stops responding to NVMe-MI commands from `nvmesensor`. 8. `nvmesensor` calls the `Recover` method on the - `au.com.CodeConstruct.MCTP.Endpoint` of the `/xyz/openbmc_project/mctp/1/9` + `au.com.codeconstruct.MCTP.Endpoint1` of the + `/au/com/codeconstruct/mctp1/networks/1/endpoints/9` D-Bus object hosted by `mctpd`. This may occur via e.g. failure to regularly poll the drive CTEMP. 9. `mctpd` [emits a `PropertiesChanged` signal][dbus-spec-standard-interfaces-properties] - from `/xyz/openbmc_project/mctp/1/9` with the following contents: + from `/au/com/codeconstruct/mctp/networks/1/enpoints/9` with the following + contents: ``` [ - "au.com.CodeConstruct.MCTP.Endpoint", + "au.com.codeconstruct.MCTP.Endpoint1", { "Connectivity": "Degraded" }, { } ] @@ -417,12 +421,12 @@ The general strategy for tracking endpoint lifecycles is [as follows] 12. The device fails to respond to all (retried) queries issued inside `Treclaim`. -13. `mctpd` removes `/xyz/openbmc_project/mctp/1/9` from D-Bus, emitting an - `InterfacesRemoved` signal with the following content +13. `mctpd` removes `/au/com/codeconstruct/mctp1/networks/1/endpoints/9` from + D-Bus, emitting an `InterfacesRemoved` signal with the following content ``` - "/xyz/openbmc_project/mctp/1/9": [ - "au.com.CodeConstruct.MCTP.Endpoint", + "/au/com/codeconstruct/mctp1/networks/1/endpoints/9": [ + "au.com.codeconstruct.MCTP.Endpoint1", "xyz.openbmc_project.MCTP.Endpoint", ... ] diff --git a/docs/mctpd.md b/docs/mctpd.md index 0ff827b..2e1f967 100644 --- a/docs/mctpd.md +++ b/docs/mctpd.md @@ -2,20 +2,28 @@ ## D-Bus -`mctpd` provides a D-Bus path of `/xyz/openbmc_project/mctp`. For each known MCTP endpoint, `mctpd` -will populate an object `/xyz/openbmc_project/mctp//`. The objects have interface -`xyz.openbmc_project.MCTP.Endpoint`, as per -[OpenBMC documentation](https://github.com/openbmc/phosphor-dbus-interfaces/tree/master/yaml/xyz/openbmc_project/MCTP). +`mctpd` provides a D-Bus service named `au.com.codeconstruct.MCTP1`, and a base +object path of `/au/com/codeconstruct/mctp1`. For each known MCTP endpoint, +`mctpd` will populate an object at +`/au/com/codeconstruct/mctp1/networks//endpoints/`. The objects have +interface `xyz.openbmc_project.MCTP.Endpoint`, as per [OpenBMC +documentation](https://github.com/openbmc/phosphor-dbus-interfaces/tree/master/yaml/xyz/openbmc_project/MCTP). -As well as the standard interfaces, `mctpd` provides methods to add and configure MCTP endpoints. -These are provided by the `au.com.CodeConstruct.MCTP` D-Bus interface. +As well as those standard interfaces, `mctpd` provides methods to add and +configure MCTP endpoints. These are provided by the `au.com.codeconstruct.MCTP1` +D-Bus interface. -### `.SetupEndpoint` +## Bus-owner methods: `au.com.codeconstruct.MCTP.BusOwner1` interface -This method is the normal method used to add a MCTP endpoint. -The endpoint is identified by MCTP network interface, and physical address. -`mctpd` will query for the endpoint's current EID, and assign an EID to the endpoint if needed. -`mctpd` will add local MCTP routes and neighbour table entries for endpoints as they are added. +This interface exposes bus-owner level functions. + +### `.SetupEndpoint`: `say` → `yisb` + +This method is the normal method used to add a MCTP endpoint. The endpoint is +identified by MCTP network interface, and physical address. `mctpd` will query +for the endpoint's current EID, and assign an EID to the endpoint if needed. +`mctpd` will add local MCTP routes and neighbour table entries for endpoints as +they are added. `SetupEndpoint ` @@ -29,23 +37,27 @@ new (bool) - true if a new EID was assigned `` is an interface such as `mctpi2c6`. -`` depends on the transport type - for i2c it is a 1 byte client address (7-bit, the same as other Linux tools like `i2cdetect`). +`` depends on the transport type - for i2c it is a 1 byte client address +(7-bit, the same as other Linux tools like `i2cdetect`). An example: ```shell -busctl call xyz.openbmc_project.MCTP /xyz/openbmc_project/mctp \ - au.com.CodeConstruct.MCTP SetupEndpoint say mctpi2c6 1 0x1d +busctl call au.com.codeconstruct.MCTP1 \ + /au/com/codeconstruct/mctp1 \ + au.com.codeconstruct.MCTP1 \ + SetupEndpoint say mctpi2c6 1 0x1d ``` `1` is the length of the hwaddr array. -### `.AssignEndpoint` +### `.AssignEndpoint`: `say` → `yisb` -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`. +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`. -### `.AssignEndpointStatic` +### `.AssignEndpointStatic`: `sayy` → `yisb` Similar to AssignEndpoint, but takes an additional EID argument: @@ -59,35 +71,36 @@ This call will fail if the endpoint already has an EID, and that EID is different from `static-EID`, or if `static-EID` is already assigned to another endpoint. -### `.LearnEndpoint` +### `.LearnEndpoint`: `say` → `yisb` -Like SetupEndpoint but will not assign EIDs, will only query endpoints for a current EID. -The `new` return value is set to `false` for an already known endpoint, or `true` when an -endpoint's EID is newly discovered. +Like SetupEndpoint but will not assign EIDs, will only query endpoints for a +current EID. The `new` return value is set to `false` for an already known +endpoint, or `true` when an endpoint's EID is newly discovered. -## Endpoint Methods +## Endpoint methods: the `au.com.codeconstruct.MCTP.Endpoint1` interface -Each endpoint object has methods to configure it, with `au.com.CodeConstruct.MCTP.Endpoint` -interface on each endpoint. +Each endpoint object has methods to configure it, through the +`au.com.codeconstruct.MCTP.Endpoint1` interface on each endpoint. -## `.SetMTU` +## `.SetMTU`: `u` -Sets the MTU (maximum transmission unit) on the route for that endpoint. This must be within -the MTU range allowed for the network device. For i2c that is [68, 254]. +Sets the MTU (maximum transmission unit) on the route for that endpoint. This +must be within the MTU range allowed for the network device. For i2c that is +[68, 254]. -If a route-specific MTU has not been set (or set to 0), Linux will use the per-interface -MTU, configurable with `mctp link set mtu `. +If a route-specific MTU has not been set (or set to 0), Linux will use the +per-interface MTU, configurable with `mctp link set mtu `. An example, setting MTU of 80: ```shell -busctl call xyz.openbmc_project.MCTP /xyz/openbmc_project/mctp/1/11 \ - au.com.CodeConstruct.MCTP.Endpoint SetMTU u 80 +busctl call au.com.codeconstruct.MCTP1 \ + /au/com/codeconstruct/mctp1/networks/1/endpoints/11 \ + au.com.codeconstruct.MCTP.Endpoint1 \ + SetMTU u 80 ``` ## `.Remove` Removes the MCTP endpoint from `mctpd`, and deletes routes and neighbour entries. - - diff --git a/meson.build b/meson.build index 55533d4..592c76d 100644 --- a/meson.build +++ b/meson.build @@ -27,7 +27,7 @@ conf.set10('MCTPD_RECOVER_NIL_UUID', ) conf.set10('MCTPD_WRITABLE_CONNECTIVITY', get_option('unsafe-writable-connectivity'), - description: 'Allow writes to the Connectivity member of the au.com.CodeConstruct.MCTP.Endpoint interface on endpoint objects') + description: 'Allow writes to the Connectivity member of the au.com.codeconstruct.MCTP.Endpoint1 interface on endpoint objects') conf.set_quoted('MCTPD_CONF_FILE_DEFAULT', join_paths(get_option('prefix'), get_option('sysconfdir'), 'mctpd.conf'), diff --git a/meson_options.txt b/meson_options.txt index b634a5d..b8323f5 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -2,5 +2,5 @@ option('unsafe-recover-nil-uuid', type: 'boolean', value: false) option('unsafe-writable-connectivity', type: 'boolean', value: false, - description: 'Allow writes to the Connectivity member of the au.com.CodeConstruct.MCTP.Endpoint interface on endpoint objects') + description: 'Allow writes to the Connectivity member of the au.com.codeconstruct.MCTP.Endpoint1 interface on endpoint objects') option('tests', type: 'boolean', value: true) diff --git a/src/mctpd.c b/src/mctpd.c index 27fa9ff..4502661 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -40,11 +40,11 @@ #define max(a, b) ((a) > (b) ? (a) : (b)) #define min(a, b) ((a) < (b) ? (a) : (b)) -#define MCTP_DBUS_PATH "/xyz/openbmc_project/mctp" -#define CC_MCTP_DBUS_IFACE "au.com.CodeConstruct.MCTP" -#define CC_MCTP_DBUS_IFACE_ENDPOINT "au.com.CodeConstruct.MCTP.Endpoint" -#define CC_MCTP_DBUS_IFACE_TESTING "au.com.CodeConstruct.MCTPTesting" -#define MCTP_DBUS_IFACE "xyz.openbmc_project.MCTP" +#define MCTP_DBUS_PATH "/au/com/codeconstruct/mctp1" +#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_TESTING "au.com.codeconstruct.MCTPTesting" +#define MCTP_DBUS_NAME "au.com.codeconstruct.MCTP1" #define MCTP_DBUS_IFACE_ENDPOINT "xyz.openbmc_project.MCTP.Endpoint" #define OPENBMC_IFACE_COMMON_UUID "xyz.openbmc_project.Common.UUID" @@ -397,8 +397,9 @@ static int peer_from_path(ctx *ctx, const char* path, peer **ret_peer) int rc; *ret_peer = NULL; - rc = sd_bus_path_decode_many(path, MCTP_DBUS_PATH "/%/%", - &netstr, &eidstr); + rc = sd_bus_path_decode_many(path, + MCTP_DBUS_PATH "/networks/%/endpoints/%", + &netstr, &eidstr); if (rc == 0) return -ENOENT; if (rc < 0) @@ -434,7 +435,7 @@ static int path_from_peer(const peer *peer, char ** ret_path) { return -ENOMEM; /* can't use sd_bus_path_encode_many() since it escapes leading digits */ - snprintf(buf, l, "%s/%d/%d", MCTP_DBUS_PATH, + snprintf(buf, l, "%s/networks/%d/endpoints/%d", MCTP_DBUS_PATH, peer->net, peer->eid); *ret_path = buf; return 0; @@ -2919,7 +2920,7 @@ static char* net_path(int net) } /* can't use sd_bus_path_encode_many() since it escapes leading digits */ - snprintf(buf, l, "%s/%d", MCTP_DBUS_PATH, net); + snprintf(buf, l, "%s/networks/%d", MCTP_DBUS_PATH, net); return buf; } @@ -3112,7 +3113,7 @@ static int setup_bus(ctx *ctx) mix non-fallback and fallback vtables on MCTP_DBUS_PATH */ rc = sd_bus_add_fallback_vtable(ctx->bus, NULL, MCTP_DBUS_PATH, - CC_MCTP_DBUS_IFACE, + CC_MCTP_DBUS_IFACE_BUSOWNER, bus_mctpd_vtable, bus_mctpd_find, ctx); @@ -3174,14 +3175,17 @@ static int setup_bus(ctx *ctx) } -int request_dbus(ctx *ctx) { +int request_dbus(ctx *ctx) +{ int rc; - rc = sd_bus_request_name(ctx->bus, MCTP_DBUS_IFACE, 0); + rc = sd_bus_request_name(ctx->bus, MCTP_DBUS_NAME, 0); if (rc < 0) { - warnx("Failed requesting name %s", MCTP_DBUS_IFACE); + warnx("Failed requesting dbus name %s", MCTP_DBUS_NAME); + return rc; } - return rc; + + return 0; } diff --git a/tests/conftest.py b/tests/conftest.py index 80ea65d..ca5fbb6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -901,7 +901,7 @@ async def mctpd_proc(self, nursery, task_status = trio.TASK_STATUS_IGNORED): s = trio.Semaphore(initial_value = 0) def name_owner_changed(name, new_owner, old_owner): - if name == 'xyz.openbmc_project.MCTP': + if name == 'au.com.codeconstruct.MCTP1': s.release() await interface.on_name_owner_changed(name_owner_changed) diff --git a/tests/mctp_test_utils.py b/tests/mctp_test_utils.py index ae48066..1b40bdc 100644 --- a/tests/mctp_test_utils.py +++ b/tests/mctp_test_utils.py @@ -1,14 +1,14 @@ async def mctpd_mctp_obj(dbus): obj = await dbus.get_proxy_object( - 'xyz.openbmc_project.MCTP', - '/xyz/openbmc_project/mctp' + 'au.com.codeconstruct.MCTP1', + '/au/com/codeconstruct/mctp1' ) - return await obj.get_interface('au.com.CodeConstruct.MCTP') + return await obj.get_interface('au.com.codeconstruct.MCTP.BusOwner1') async def mctpd_mctp_endpoint_obj(dbus, path): obj = await dbus.get_proxy_object( - 'xyz.openbmc_project.MCTP', + 'au.com.codeconstruct.MCTP1', path, ) - return await obj.get_interface('au.com.CodeConstruct.MCTP.Endpoint') + return await obj.get_interface('au.com.codeconstruct.MCTP.Endpoint1') diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index ae27938..fd212de 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -11,10 +11,10 @@ # - C: Connection # - P: Path # - I: Interface -MCTPD_C = 'xyz.openbmc_project.MCTP' -MCTPD_MCTP_P = '/xyz/openbmc_project/mctp' -MCTPD_MCTP_I = 'au.com.CodeConstruct.MCTP' -MCTPD_ENDPOINT_I = 'au.com.CodeConstruct.MCTP.Endpoint' +MCTPD_C = 'au.com.codeconstruct.MCTP1' +MCTPD_MCTP_P = '/au/com/codeconstruct/mctp1' +MCTPD_MCTP_I = 'au.com.codeconstruct.MCTP.BusOwner1' +MCTPD_ENDPOINT_I = 'au.com.codeconstruct.MCTP.Endpoint1' DBUS_OBJECT_MANAGER_I = 'org.freedesktop.DBus.ObjectManager' DBUS_PROPERTIES_I = 'org.freedesktop.DBus.Properties' From 9b4eabc3a5e4af5d3f39cd59ddfcc059fe39b961 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 30 May 2024 14:10:09 +0800 Subject: [PATCH 2/3] mctpd: use .DRAFT suffix on BusOwner1 interface We'll be modifying this for the new Role-based interfaces which exist on the link/interface objects, so use a temporary name for now. Signed-off-by: Jeremy Kerr --- docs/endpoint-recovery.md | 4 ++-- docs/mctpd.md | 2 +- src/mctpd.c | 2 +- tests/mctp_test_utils.py | 2 +- tests/test_mctpd.py | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/endpoint-recovery.md b/docs/endpoint-recovery.md index d67ab4a..d5bd109 100644 --- a/docs/endpoint-recovery.md +++ b/docs/endpoint-recovery.md @@ -361,7 +361,7 @@ The general strategy for tracking endpoint lifecycles is [as follows] registered as an MCTP endpoint. 2. The FRU data is decoded and `SetupEndpoint` on the - `au.com.codeconstruct.MCTP.BusOwner1` interface of the + `au.com.codeconstruct.MCTP.BusOwner1.DRAFT` interface of the `/au/com/codeconstruct/mctp1` object hosted by `mctpd` is invoked. @@ -402,7 +402,7 @@ The general strategy for tracking endpoint lifecycles is [as follows] poll the drive CTEMP. 9. `mctpd` [emits a `PropertiesChanged` signal][dbus-spec-standard-interfaces-properties] - from `/au/com/codeconstruct/mctp/networks/1/enpoints/9` with the following + from `/au/com/codeconstruct/mctp/networks/1/endpoints/9` with the following contents: ``` diff --git a/docs/mctpd.md b/docs/mctpd.md index 2e1f967..3cc65da 100644 --- a/docs/mctpd.md +++ b/docs/mctpd.md @@ -13,7 +13,7 @@ As well as those standard interfaces, `mctpd` provides methods to add and configure MCTP endpoints. These are provided by the `au.com.codeconstruct.MCTP1` D-Bus interface. -## Bus-owner methods: `au.com.codeconstruct.MCTP.BusOwner1` interface +## Bus-owner methods: `au.com.codeconstruct.MCTP.BusOwner1.DRAFT` interface This interface exposes bus-owner level functions. diff --git a/src/mctpd.c b/src/mctpd.c index 4502661..cbbbb89 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -41,7 +41,7 @@ #define min(a, b) ((a) < (b) ? (a) : (b)) #define MCTP_DBUS_PATH "/au/com/codeconstruct/mctp1" -#define CC_MCTP_DBUS_IFACE_BUSOWNER "au.com.codeconstruct.MCTP.BusOwner1" +#define CC_MCTP_DBUS_IFACE_BUSOWNER "au.com.codeconstruct.MCTP.BusOwner1.DRAFT" #define CC_MCTP_DBUS_IFACE_ENDPOINT "au.com.codeconstruct.MCTP.Endpoint1" #define CC_MCTP_DBUS_IFACE_TESTING "au.com.codeconstruct.MCTPTesting" #define MCTP_DBUS_NAME "au.com.codeconstruct.MCTP1" diff --git a/tests/mctp_test_utils.py b/tests/mctp_test_utils.py index 1b40bdc..ffd31cc 100644 --- a/tests/mctp_test_utils.py +++ b/tests/mctp_test_utils.py @@ -4,7 +4,7 @@ async def mctpd_mctp_obj(dbus): 'au.com.codeconstruct.MCTP1', '/au/com/codeconstruct/mctp1' ) - return await obj.get_interface('au.com.codeconstruct.MCTP.BusOwner1') + return await obj.get_interface('au.com.codeconstruct.MCTP.BusOwner1.DRAFT') async def mctpd_mctp_endpoint_obj(dbus, path): obj = await dbus.get_proxy_object( diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index fd212de..5272c09 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -13,7 +13,7 @@ # - I: Interface MCTPD_C = 'au.com.codeconstruct.MCTP1' MCTPD_MCTP_P = '/au/com/codeconstruct/mctp1' -MCTPD_MCTP_I = 'au.com.codeconstruct.MCTP.BusOwner1' +MCTPD_MCTP_I = 'au.com.codeconstruct.MCTP.BusOwner1.DRAFT' MCTPD_ENDPOINT_I = 'au.com.codeconstruct.MCTP.Endpoint1' DBUS_OBJECT_MANAGER_I = 'org.freedesktop.DBus.ObjectManager' DBUS_PROPERTIES_I = 'org.freedesktop.DBus.Properties' From d441a008da42df1d81d2454d5300648a5776d782 Mon Sep 17 00:00:00 2001 From: Thu Nguyen Date: Thu, 6 Jun 2024 23:43:09 +0000 Subject: [PATCH 3/3] mctpd: enumerate /networks and /endpoints object paths Enumerate the parent D-Bus object path `.../mctp1/networks` for the network paths `.../mctp1/networks/` and `.../mctp1/networks//endpoints` for the endpoints path `.../mctp1/networks//endpoints/`. Tested: Check the MCTP D-Bus interface: ``` busctl tree au.com.codeconstruct.MCTP1 `- /au `- /au/com `- /au/com/codeconstruct `- /au/com/codeconstruct/mctp1 `- /au/com/codeconstruct/mctp1/networks `- /au/com/codeconstruct/mctp1/networks/1 `- /au/com/codeconstruct/mctp1/networks/1/endpoints `- /au/com/codeconstruct/mctp1/networks/1/endpoints/8 ``` Signed-off-by: Thu Nguyen Signed-off-by: Jeremy Kerr --- src/mctpd.c | 77 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 64 insertions(+), 13 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index cbbbb89..cb57065 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -41,6 +41,7 @@ #define min(a, b) ((a) < (b) ? (a) : (b)) #define MCTP_DBUS_PATH "/au/com/codeconstruct/mctp1" +#define MCTP_DBUS_PATH_NETWORKS "/au/com/codeconstruct/mctp1/networks" #define CC_MCTP_DBUS_IFACE_BUSOWNER "au.com.codeconstruct.MCTP.BusOwner1.DRAFT" #define CC_MCTP_DBUS_IFACE_ENDPOINT "au.com.codeconstruct.MCTP.Endpoint1" #define CC_MCTP_DBUS_IFACE_TESTING "au.com.codeconstruct.MCTPTesting" @@ -2908,6 +2909,20 @@ static int bus_endpoint_find_uuid(sd_bus *bus, const char *path, return 0; } +static char* root_endpoints_path(int net) +{ + size_t l; + char *buf = NULL; + + l = strlen(MCTP_DBUS_PATH) + 30; + buf = malloc(l); + if (!buf) { + return NULL; + } + snprintf(buf, l, "%s/networks/%d/endpoints", MCTP_DBUS_PATH, net); + return buf; +} + static char* net_path(int net) { size_t l; @@ -3005,15 +3020,28 @@ static int mctpd_dbus_enumerate(sd_bus *bus, const char* path, // NULL terminator num_nodes = 1; - // .../mctp object + // .../mctp1 object + num_nodes++; + // .../mctp1/networks object num_nodes++; + // .../mctp1/networks/ + for (i = 0; i < ctx->num_nets; i++) { + num_nodes++; + for (size_t t = 0; t < 256; t++) { + if (ctx->nets[i].peeridx[t] != -1) { + // .../mctp1/networks//endpoints object + num_nodes++; + break; + } + } + } + + // .../mctp1/networks//endpoints/ object for (i = 0; i < ctx->size_peers; i++) if (ctx->peers[i].published) num_nodes++; - num_nodes += ctx->num_nets; - nodes = malloc(sizeof(*nodes) * num_nodes); if (!nodes) { rc = -ENOMEM; @@ -3021,6 +3049,7 @@ static int mctpd_dbus_enumerate(sd_bus *bus, const char* path, } j = 0; + // .../mctp1 nodes[j] = strdup(MCTP_DBUS_PATH); if (!nodes[j]) { rc = -ENOMEM; @@ -3028,6 +3057,38 @@ static int mctpd_dbus_enumerate(sd_bus *bus, const char* path, } j++; + // .../mctp1/networks + nodes[j] = strdup(MCTP_DBUS_PATH_NETWORKS); + if (!nodes[j]) { + rc = -ENOMEM; + goto out; + } + j++; + + for (i = 0; i < ctx->num_nets; i++) { + // .../mctp1/networks/ + nodes[j] = net_path(ctx->nets[i].net); + if (nodes[j] == NULL) { + rc = -ENOMEM; + goto out; + } + j++; + + for (size_t t = 0; t < 256; t++) { + if (ctx->nets[i].peeridx[t] == -1) { + continue; + } + // .../mctp1/networks//endpoints object + nodes[j] = root_endpoints_path(ctx->nets[i].net); + if (nodes[j] == NULL) { + rc = -ENOMEM; + goto out; + } + j++; + break; + } + } + // Peers for (i = 0; i < ctx->size_peers; i++) { peer *peer = &ctx->peers[i]; @@ -3041,16 +3102,6 @@ static int mctpd_dbus_enumerate(sd_bus *bus, const char* path, j++; } - // Nets - for (i = 0; i < ctx->num_nets; i++) { - nodes[j] = net_path(ctx->nets[i].net); - if (nodes[j] == NULL) { - rc = -ENOMEM; - goto out; - } - j++; - } - // NULL terminator nodes[j] = NULL; j++;