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'