DBus ObjectManager Interface usages within the project

[Ed Tanous]

We've had a couple cases in the project where patches have needed to be
slowed down because of inconsistencies in our use of object manager, so
IMO, the time has come to make our usage of ObjectManager consistent,
documented, and drop support for the (not upstream) daemons that don't
follow the rules.  As part of this, we will port forward all the upstream
daemons to be consistent, and do our best to avoid bugs, but this email is
intended to inform those that have non-upstream daemons about the change so
they can resolve their implementations.

The basics:
ObjectManager DBus interface will now be required for any daemon
implementing a Sensor.Value interface or Inventory.Item interface.

Daemons producing sensors will be required to place their ObjectManager at
/xyz/openbmc_project/sensors
Daemons producing inventory items will be required to place their
ObjectManager at /xyz/openbmc_project/inventory.

Both of these interfaces will be required to be published before claiming a
well known name on dbus, to allow for the possibility of caching
implementations at some point in future.

Functionally, this gets a little complicated because the
sdbusplus::asio::object_manager bindings in their default invocation create
an ObjectManager at the root object /, so we need to execute this in a few
steps.

0. Send this email out, outlining the problem, and warn the community that
if this is an assumption you make in your downstream daemons, those
assumptions will need to change.  Get consensus from maintainers.
1. Update the phosphor-dbus-interfaces documentation for both Sensor.Value
and Inventory.Item to call out the required-ness of ObjectManager, and the
explicit dbus path where it's required to be placed.
2. Identify all the asio daemons that need changes, and publish changes
that move the object_manager to the appropriate path.  This is mostly going
to be a tree-wide grep for sdbusplus::asio::object_server, and look for
daemonst that don't make use of the add_manager() API.  Anyone directly
calling the Sensor or Invertory ObjectManager interfaces will need to port
to the new paths, but luckily, this isn't a very common operation, and I
beleive bmcweb and phosphor-ipmid-host might be the only direct users.  In
bmcweb, there is actually a convoluted piece of code that uses the mapper
to sort out the location that the ObjectManager exists at so Redfish
sensors should remain consistent, and we don't yet have code that relies on
ObjectManager for Inventory items.  I believe phosphor-ipmi-host has a
similar piece of code that should protect us in this case, but I defer to
those maintainers.

A starter list of daemons that need updating is:
All daemons in dbus-sensors
peci-pcie
entity-manager
libpeci
phosphor-u-boot-env-manager
pfr-manager
smbios-mdr
s2600-misc
dbus-sensors


4. Change the defaults of sdbusplus::asio::object_server to not init an
ObjectManager interface at the root path /.

5. Unwind the various complexities of searching for ObjectManager
interfaces, and rely on the assumptions above that should net us some
speedups in at least the sensors API, completely avoiding an extra mapper
call.

6. Get system owners to retest their platforms, with the tree changes in,
and fix any bugs we may have accidentally caused.

This was discussed pretty heavily on discord in the #development topic, and
some of the nitty gritty details on why this is needed is available there,
as well as I'm happy to discuss here.  This is one of those nasty
spaghetti-code things that we've lived with for a while;  Hopefully if we
can get this behind us, we can get some good real world performance
improvements.

Thoughts?

-Ed
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20220712/dd4c091c/attachment.htm>