Read FRU of host through ipmi in Entity manager.

[Ed Tanous]

On Tue, Sep 22, 2020 at 12:52 AM Andrei Kartashev <a.kartashev at yadro.com> wrote:
>
>
> >     Minor amendment again.  I'd much rather the IPMBSensor daemon
> > simply
> >     create the same interface that fru device does, rather than
> > adding
> >     IPMB logic to FruDevice.  In this way, platforms that don't have
> > IPMB
> >     don't need to include the feature at all.  Also, all the IO can
> > be
> >     managed in one place.
> >
> > https://github.com/openbmc/dbus-sensors/blob/master/src/IpmbSensor.cpp
> >     Ideally, your IPMB device would also have an SDR that details
> > what
> >     FRUs and sensors exist, so that the inventory can be read and
> > posted
> >     to DBus at startup.  If they don't then we might need a static
> > mapping
> >     from an EM config once the device on the other end is detected
> > via get
> >     device ID.
> >
> > I agree with Ed here, all ipmb related interfaces should be
> > implemented here.
> >
>
>
> I disagree here.
> First of all, IPMBSensor located in dbus-sensors package and it is
> suppose to handle SENSORS.
> FRU is not sensor, it would be big illogical
> mistake to put FruDevice code there.

WIth respect, dbus-sensors is a repo that's not aptly named these
days.  Ideally when this first started we would've called it
dbus-entities, but alas, that's history, and unlikely that we'll
change it (although I'd support you if you wanted to propose that).
There should be no expectation that dbus-sensors contains only
sensors, nor that dbus-sensors is the only place a sensor can be put
to dbus from, neither of which are true in the codebase today.

> Then there are already number of places in OpenBMC and related projects
> uses FRU and implements encoding/decoding by its own. I already spend
> lot of time debugging different behaviour of FruDevice and ipmitool...
> You propose to fragment FRU handling code even more and I against this.

Lets centralize the logic in a library, and move the other
implementations over to it.  Then we have a single source of FRU
parsing that all can use.  This is likely to be a problem in another
spot now that NVMe has adopted IPMI for their VPD format, we'll need
to duplicate the logic there as well, so we'd be solving future
problems (hopefully).

> We at least should then extract data-source independent code from
> FruDevice to a kind of library to use from different daemons. But I
> prefer to do opposite - extract impb i/o code to library and use it
> from both IPMBSensor and FruDevice.

My issue with that is that we're birfurcating the control of the IPMB
device in multiple places.  The easiest example of a problem this
causes is an IPMB device that needs the sensors to stop scanning while
it's doing a firmware update.  Having the control of the device
centralized in a single process like that avoids cluttering dbus, and
makes it more likely that IPMB code can be reused between the
use-cases.

Also, keep in mind that IPMBSensor is far from done, nor what I'd
really like to see.  In the next few years, I very much expect that it
will need to parse the SDR as well to automatically populate sensors.
Doing that in 2 places (FRUDevice and IPMBSensor) is inefficient, when
they could simply share the SDR parsing code in a single place.

>
> --
> Best regards,
> Andrei Kartashev
>
>