Using D-Bus well-known service names and deprecating mapper service lookups

[Andrew Geissler]

On Thu, Apr 11, 2019 at 12:03 AM Andrew Jeffery <andrew at aj.id.au> wrote:
>
> Hello,
>
> Recently Andrew Geissler, William and myself have been running into limitations of the mapper's service lookups. In my opinion we should move to use well known service names and avoiding reliance on the mapper to determine who to talk to.

One thing to ask ourselves is what are the advantages to mapper and
the GetObject concept (I think we all agree(?) that features like
getting the subtree and associations are still useful).
For GetObject the main thing I see as an advantage is actually
relevant in my patch you linked. My code wants to talk to the host.
The interface to talk to the host is currently provided by ipmid but
at some point in the future that may move to the pldm daemon. Using
mapper to get the object that provides my interface abstract that from
me. Will the pldm daemon use the same well-know name as ipmid does
now? I'm not so sure about that. I know ipmid does currently implement
two well known names, so we could potentially move it.

Another feature we've thought about would be if we some day support
multi-BMC distributed systems. Having something like mapper
abstracting cross BMC communication could be useful (i.e. process A
just knows it needs to talk to interface B, mapper could route process
A through a dbus proxy in another BMC without process A having any
knowledge of that). Not really a fully a formed thought but just
something we kicked around a bit when talking about distributed BMC's.

>
> My issues with using the mapper for service dependencies are:
>
> 1. Ambiguities in the way `mapper wait` functions
> 2. Race conditions between bus name announcement, mapper introspection and inbound service lookups
> 3. Increased IPC traffic (decreasing performance).
>
> Expanding on 1, `mapper wait` takes a path name argument and waits until the path is available on a bus connection. The problem here is that multiple connections can (and do) implement objects under a given path, so the successful exit of `mapper wait` with the expectation that a given service is available can be an undefined result, as it's possible the *wrong* service was used to resolve the wait.
>
> Where this hits home is that we currently have a systemd service, mapper-wait at .service, that allows units to depend on the presence of a service providing a required path. With the ambiguity outlined above it's possible that dependent services are started earlier than they should be due to `mapper wait` falsely exiting success with respect to the dependent's actual requirement.
>
> A quick grep of the openbmc tree gives an idea of the potential problem size:
>
> $ git ls-files -- recipes-phosphor/ | grep '\.service$' | wc -l
> 98
> $ git ls-files -- recipes-phosphor/ | grep '\.service$' | xargs grep -li mapper-wait | wc -l
> 32
>
> So about a 1/3 generic services are potentially exposed.

Yes, this has been hounding me for a while now. We had a discussion in
IRC a few months ago about having mapper stop doing introspection and
move to using the InterfacesAdded signal. This would improve
performance (introspection is resource intensive) and it would
severely limit (if not fix) the race condition issues. I took a stab
at a patch (https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-objmgr/+/17837)
but have been running into some weird issues I haven't gotten to the
bottom of yet.

>
> Point 2 leads to patches like [1] where we wind up implementing a fallback to a well-known service name if we get issues with a mapper lookup. If this is a valid approach then we should just avoid looking up the service altogether; implementing this fallback approach everywhere seems tedious and error-prone, and the lookup is a waste of resources if the fallback works regardless.
>
> By moving to well known service names we can exploit more of systemd's D-Bus integration with units of Type=dbus, and depending either on unit names aliased to D-Bus service names, or on D-Bus service activation, and receive reliable behaviour for D-Bus-based dependencies.
>
> Point 3 is something William has complained about, and I think it's a good point - avoiding IPC where we can is a good thing no matter what we're doing.
>
> I'm kicking this thread off because it's something that's bothered me for a while. I've complained about it to various people in private but have never taken it any further. My impression is that there's support for the idea that we move away from the mapper for service lookups (though we can't eliminate it entirely, we still need the interface reverse mapping feature), and I want to gauge how much support it has and whether there are solid counter-arguments.

Thanks for getting this started. Definitely something that's been
bothering me more and more lately as some of the issues you've noted
above have become more prevalent. I agree that I'd like to transition
away form GetObject usage in our process's. I'd be interested in the
history from Brad on the reasons this was initially created and
whether they are still relevant though.

>
> Cheers,
>
> Andrew
>
> [1] https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-state-manager/+/20460