Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)

Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)

[Ambrozewicz, Adrian]

Hi,

Currently bmcweb exposes sensors as part of Chassis subnodes in Sensors, 
Power and Thermal schemas. All of them lists sensors as arrays of 
generic properties distinguished by Id/Name etc. On the other hand - for 
well-defined metrics Redfish specifies concrete schemas like 
ProcessorMetrics and MemoryMetrics. They define designated Redfish 
properties with given name and value type.

I'm starting to explore ways to implement these schemas in bmcweb, while 
retaining interoperability with TelemetryService. This requirement 
implicates, that source of these properties should implement 
xyz.openbmc_project.Sensor.Value interface and comply with OpenBMC D-Bus 
sensors architecture, which enforces predefined paths and units for 
various types of sensors.

Question of extending xyz.openbmc_project.Sensor.Value interface (so it 
allows for more types or nested paths, if necessary) is something  I 
know should be considered, but seems like more or less straightforward 
to address.

There is bigger issue I see now - mapping D-Bus sensors to concrete 
Redfish properties. Mapping sensors at inventory level is sorted out 
with use of xyz.openbmc_project.Association.Definitions interface. 
However - I don't see (or know of) any method to link given D-Bus sensor 
with it's designated place in Redfish schema.
I've done some PoC implementation of ProcessorMetrics, which relies on 
new D-Bus interface with 'Mapping' property (eg. 'TemperatureCelsius' or 
'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node implementation 
queries D-Bus for all sensors associated with given CPU and populates 
properties if proper mapping was found.

Is my PoC approach described above feasible for OpenBMC? What are your 
thoughts? I would like to start a discussion and hear your proposals 
about possible alternatives.

Regards,
Adrian

Re: Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 2033 bytes --]

On Wed, Apr 07, 2021 at 02:24:55PM +0200, Ambrozewicz, Adrian wrote:
> Question of extending xyz.openbmc_project.Sensor.Value interface (so it 
> allows for more types or nested paths, if necessary) is something  I 
> know should be considered, but seems like more or less straightforward 
> to address.

I suspect this would be the more difficult direction to go down.  There
is already enough code that looks for sensors at specific paths that
you'd have to track down and fix up.  Also, there has been some concern
by some maintainers in other cases about having information in the paths
have meaning and prefering to reduce the reliance on that.

> There is bigger issue I see now - mapping D-Bus sensors to concrete 
> Redfish properties. Mapping sensors at inventory level is sorted out 
> with use of xyz.openbmc_project.Association.Definitions interface. 
> However - I don't see (or know of) any method to link given D-Bus sensor 
> with it's designated place in Redfish schema.

I think associations are the right approach to link sensors with
inventory items.  There is a design document underway to define some of
those assocations for inventory items and it seems like your needs
should be an extension to that.

https://gerrit.openbmc-project.xyz/c/openbmc/docs/+/41468/4/architecture/dbus-inventory.md

> I've done some PoC implementation of ProcessorMetrics, which relies on 
> new D-Bus interface with 'Mapping' property (eg. 'TemperatureCelsius' or 
> 'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node implementation 
> queries D-Bus for all sensors associated with given CPU and populates 
> properties if proper mapping was found.

I'm not really grasping what the contents of this mapping property are.
Generally we want to stay away from free-form strings having programatic
uses.  Maybe if you can define these mappings as enumerations?

What is the additional information you need besides the assocation from
a sensor to its inventory item?

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)

[Ed Tanous]

On Wed, Apr 7, 2021 at 5:26 AM Ambrozewicz, Adrian
<adrian.ambrozewicz@linux.intel.com> wrote:
>
> Hi,
>
> Currently bmcweb exposes sensors as part of Chassis subnodes in Sensors,
> Power and Thermal schemas. All of them lists sensors as arrays of
> generic properties distinguished by Id/Name etc. On the other hand - for
> well-defined metrics Redfish specifies concrete schemas like
> ProcessorMetrics and MemoryMetrics. They define designated Redfish
> properties with given name and value type.
>
> I'm starting to explore ways to implement these schemas in bmcweb, while
> retaining interoperability with TelemetryService. This requirement
> implicates, that source of these properties should implement
> xyz.openbmc_project.Sensor.Value interface and comply with OpenBMC D-Bus
> sensors architecture, which enforces predefined paths and units for
> various types of sensors.
>
> Question of extending xyz.openbmc_project.Sensor.Value interface (so it
> allows for more types or nested paths, if necessary) is something  I
> know should be considered, but seems like more or less straightforward
> to address.

I'm not following this statement.  From my perspective, this is
something we've already solved at a dbus level.  To attach a "sensor"
to a given inventory item, set up the association back to said
inventory item.  Today, we only connect things to Redfish "Chassis",
because that's currently the only node with sensors, but that's
certainly easy to change and there's nothing about the API that
prevents doing that.

Beyond that, because MemoryMetrics and ProcessorMetrics schemas call
out specific sensor names, I suspect we need to come up with a well
defined set of names;  If you want to populate MemoryMetrics, you
would expose sensors, with associations back to the dimm in question,
and have a sensor called, say,
/xyz/openbmc_project/remaining_block_percentage, and map that to the
RemainingSpareBlockPercentage property.

>
> There is bigger issue I see now - mapping D-Bus sensors to concrete
> Redfish properties. Mapping sensors at inventory level is sorted out
> with use of xyz.openbmc_project.Association.Definitions interface.
> However - I don't see (or know of) any method to link given D-Bus sensor
> with it's designated place in Redfish schema.

This is by design.  Dbus should largely have no reliance on any
specific protocol, and we shouldn't be building interfaces that
require daemons pushing data on the bus to have any knowledge of
Redfish, IPMI, PLDM, or protocol of next year.  This generally means
that some dbus->redfish transforms are not as efficient or clean as
they could be, but it keeps that logic quarantined into bmcweb.  I
suspect bmcweb will need logic to translate redfish URI + property
name to dbus path, and this logic will be non-trivial.

> I've done some PoC implementation of ProcessorMetrics, which relies on
> new D-Bus interface with 'Mapping' property (eg. 'TemperatureCelsius' or
> 'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node implementation
> queries D-Bus for all sensors associated with given CPU and populates
> properties if proper mapping was found.

Do you have a pointer to it?  It'd be good to see the code you're thinking of.

With that said, I would be against this approach.  This would require
clients to hardcode in, say, BlocksWritten, as a mapping, which means
that as we have more than just redfish, now each client needs to
hardcode the Redfish representation, the PLDM representation, and the
IPMI representation of the same data.  That seems messy.

>
> Is my PoC approach described above feasible for OpenBMC? What are your
> thoughts? I would like to start a discussion and hear your proposals
> about possible alternatives.
>
> Regards,
> Adrian

Re: Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)

[Ambrozewicz, Adrian]

W dniu 4/9/2021 o 17:27, Ed Tanous pisze:
> On Wed, Apr 7, 2021 at 5:26 AM Ambrozewicz, Adrian
> <adrian.ambrozewicz@linux.intel.com> wrote:
>>
>> Hi,
>>
>> Currently bmcweb exposes sensors as part of Chassis subnodes in Sensors,
>> Power and Thermal schemas. All of them lists sensors as arrays of
>> generic properties distinguished by Id/Name etc. On the other hand - for
>> well-defined metrics Redfish specifies concrete schemas like
>> ProcessorMetrics and MemoryMetrics. They define designated Redfish
>> properties with given name and value type.
>>
>> I'm starting to explore ways to implement these schemas in bmcweb, while
>> retaining interoperability with TelemetryService. This requirement
>> implicates, that source of these properties should implement
>> xyz.openbmc_project.Sensor.Value interface and comply with OpenBMC D-Bus
>> sensors architecture, which enforces predefined paths and units for
>> various types of sensors.
>>
>> Question of extending xyz.openbmc_project.Sensor.Value interface (so it
>> allows for more types or nested paths, if necessary) is something  I
>> know should be considered, but seems like more or less straightforward
>> to address.
>
> I'm not following this statement.  From my perspective, this is
> something we've already solved at a dbus level.  To attach a "sensor"
> to a given inventory item, set up the association back to said
> inventory item.  Today, we only connect things to Redfish "Chassis",
> because that's currently the only node with sensors, but that's
> certainly easy to change and there's nothing about the API that
> prevents doing that.
>
That is correct. Technically we have all the pieces to integrate sensor 
with Redfish in general (Associations, standard sensor types and APIs). 
Challenge lies below.

> Beyond that, because MemoryMetrics and ProcessorMetrics schemas call
> out specific sensor names, I suspect we need to come up with a well
> defined set of names;  If you want to populate MemoryMetrics, you
> would expose sensors, with associations back to the dimm in question,
> and have a sensor called, say,
> /xyz/openbmc_project/remaining_block_percentage, and map that to the
> RemainingSpareBlockPercentage property.
>
Is the path you've provided just a shortcut for full D-bus sensor path, 
or do you have something else in mind? I will assume the former.

D-Bus sensor path is composed of : 
/xyz/openbmc_project/{namespace/type}/{name}. Do you propose to encode 
sensor name in such a way, that bmcweb (and IPMI,PLDM, whatever) would 
be able to identify exact meaning of the value if they need to? Bear in 
mind that we have several issues to consider here:
- multiple items (eg. Processors) exposing the same data, names would 
need to be unique in that regard
- certain properties are singular (CPU-wide) while other are buried down 
the hierarchy (mentioned UnhaltedCycles of Core#12).

This would lead to names like: 
/xyz/openbmc_project/sensors/counter/cpu1_core12_unhaltedcycles .

Don't get me wrong - this seems like quite flexible approach, however it 
lacks standardization. If I understood response form Patrick correctly - 
there is urge to avoid storing information in D-Bus path. Do you think 
that would be acceptable to introduce this logic (name-encoding) in bmcweb?

>>
>> There is bigger issue I see now - mapping D-Bus sensors to concrete
>> Redfish properties. Mapping sensors at inventory level is sorted out
>> with use of xyz.openbmc_project.Association.Definitions interface.
>> However - I don't see (or know of) any method to link given D-Bus sensor
>> with it's designated place in Redfish schema.
>
> This is by design.  Dbus should largely have no reliance on any
> specific protocol, and we shouldn't be building interfaces that
> require daemons pushing data on the bus to have any knowledge of
> Redfish, IPMI, PLDM, or protocol of next year.  This generally means
> that some dbus->redfish transforms are not as efficient or clean as
> they could be, but it keeps that logic quarantined into bmcweb.  I
> suspect bmcweb will need logic to translate redfish URI + property
> name to dbus path, and this logic will be non-trivial.
>
Point taken - mapping data (in whatever form it will be available) 
should be generic and domain agnostic.

>> I've done some PoC implementation of ProcessorMetrics, which relies on
>> new D-Bus interface with 'Mapping' property (eg. 'TemperatureCelsius' or
>> 'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node implementation
>> queries D-Bus for all sensors associated with given CPU and populates
>> properties if proper mapping was found.
>
> Do you have a pointer to it?  It'd be good to see the code you're  > thinking of.
>
Code itself is boilerplate D-Bus sensor service with one extra property. 
New property contains data encoded in the string, which you already 
proposed to be part of the name. Should you have need to work on some 
shared code to work out PoC in the flesh - let me know.

> With that said, I would be against this approach.  This would require
> clients to hardcode in, say, BlocksWritten, as a mapping, which means
> that as we have more than just redfish, now each client needs to
> hardcode the Redfish representation, the PLDM representation, and the
> IPMI representation of the same data.  That seems messy.
>
>>
>> Is my PoC approach described above feasible for OpenBMC? What are your
>> thoughts? I would like to start a discussion and hear your proposals
>> about possible alternatives.
>>
>> Regards,
>> Adrian

Re: Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)

[Ambrozewicz, Adrian]

W dniu 4/9/2021 o 14:05, Patrick Williams pisze:
> On Wed, Apr 07, 2021 at 02:24:55PM +0200, Ambrozewicz, Adrian wrote:
>> Question of extending xyz.openbmc_project.Sensor.Value interface (so it
>> allows for more types or nested paths, if necessary) is something  I
>> know should be considered, but seems like more or less straightforward
>> to address.
> 
> I suspect this would be the more difficult direction to go down.  There
> is already enough code that looks for sensors at specific paths that
> you'd have to track down and fix up.  Also, there has been some concern
> by some maintainers in other cases about having information in the paths
> have meaning and prefering to reduce the reliance on that.
> 

Please see message from Ed, as he's supposedly proposing to follow that 
path. I don't have strong opinions on one or the other approach.

>> There is bigger issue I see now - mapping D-Bus sensors to concrete
>> Redfish properties. Mapping sensors at inventory level is sorted out
>> with use of xyz.openbmc_project.Association.Definitions interface.
>> However - I don't see (or know of) any method to link given D-Bus sensor
>> with it's designated place in Redfish schema.
> 
> I think associations are the right approach to link sensors with
> inventory items.  There is a design document underway to define some of
> those assocations for inventory items and it seems like your needs
> should be an extension to that.
> 
> https://gerrit.openbmc-project.xyz/c/openbmc/docs/+/41468/4/architecture/dbus-inventory.md
> 
Thanks for the link. I linked my sensor to concrete CPU with 
Association, similarly to already existing bmcweb Processors node 
implementation. Will your extension be able to provide even 'deeper' 
associations? For ProcessorMetrics we would need to map for CPU_X\Core_Y.

I've read the design, however one thing is not clear for me. My current 
understanding was that for each association there would need to exist 
some D-bus object at given path somewhere. Would i need my CPU inventory 
service to also expose separate objects for each core for my association 
to be 'legal', or could we represent some virtual hierarchy with no 
actual D-Bus object in the system?

>> I've done some PoC implementation of ProcessorMetrics, which relies on
>> new D-Bus interface with 'Mapping' property (eg. 'TemperatureCelsius' or
>> 'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node implementation
>> queries D-Bus for all sensors associated with given CPU and populates
>> properties if proper mapping was found.
> 
> I'm not really grasping what the contents of this mapping property are.
> Generally we want to stay away from free-form strings having programatic
> uses.  Maybe if you can define these mappings as enumerations?
> 
> What is the additional information you need besides the assocation from
> a sensor to its inventory item?
> 

In given example I would like my sensor to be source of information for 
property defined by ProcessorMetrics schema. In the example I've used 
property associated with given Core, thus CoreMetrics/12/UnhaltedCycles 
maps directly to ProcessorMetrics sub-property. Enumerations could be 
not enough as we have multiple informations to represent:
- association with given processor (done by ProcessorMetrics)
- association with given core (could that be handled by your proposed 
design?)
- linking to given property

Would the enumeration be used for the last element, while leaving 
hierarchy problem to Associations?

Re: Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 3485 bytes --]

On Tue, Apr 27, 2021 at 01:52:51PM +0200, Ambrozewicz, Adrian wrote:
> W dniu 4/9/2021 o 14:05, Patrick Williams pisze:
> > On Wed, Apr 07, 2021 at 02:24:55PM +0200, Ambrozewicz, Adrian wrote:
> > 
> > I suspect this would be the more difficult direction to go down.  There
> > is already enough code that looks for sensors at specific paths that
> > you'd have to track down and fix up.  Also, there has been some concern
> > by some maintainers in other cases about having information in the paths
> > have meaning and prefering to reduce the reliance on that.
> > 
> 
> Please see message from Ed, as he's supposedly proposing to follow that 
> path. I don't have strong opinions on one or the other approach.

I suspect you are not signing up to change all the existing code.  I'll
look at Ed's reply though.

> I've read the design, however one thing is not clear for me. My current 
> understanding was that for each association there would need to exist 
> some D-bus object at given path somewhere. Would i need my CPU inventory 
> service to also expose separate objects for each core for my association 
> to be 'legal', or could we represent some virtual hierarchy with no 
> actual D-Bus object in the system?

Yes.  You would need an inventory object for each entity you want to
attach sensors or metrics to.  This doesn't seem like it should really
be an issue.  Other people have been working on adding CPU Cores already
and there is the xyz.openbmc_project.Inventory.Item.CpuCore defined.

> >> I've done some PoC implementation of ProcessorMetrics, which relies on
> >> new D-Bus interface with 'Mapping' property (eg. 'TemperatureCelsius' or
> >> 'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node implementation
> >> queries D-Bus for all sensors associated with given CPU and populates
> >> properties if proper mapping was found.
> > 
> > I'm not really grasping what the contents of this mapping property are.
> > Generally we want to stay away from free-form strings having programatic
> > uses.  Maybe if you can define these mappings as enumerations?
> > 
> > What is the additional information you need besides the assocation from
> > a sensor to its inventory item?
> 
> In given example I would like my sensor to be source of information for 
> property defined by ProcessorMetrics schema. In the example I've used 
> property associated with given Core, thus CoreMetrics/12/UnhaltedCycles 
> maps directly to ProcessorMetrics sub-property. Enumerations could be 
> not enough as we have multiple informations to represent:
> - association with given processor (done by ProcessorMetrics)
> - association with given core (could that be handled by your proposed 
> design?)
> - linking to given property
> 
> Would the enumeration be used for the last element, while leaving 
> hierarchy problem to Associations?

"UnhaltedCycles" is not a sensor, just to be clear.  IPMI might have
called these kinds of things sensors but we do not.  Sensors for us
measure physical properties.  This is just a property (or maybe a
"metric") but it doesn't belong in the sensors namespace or modeled with
a Sensor.Value.

I don't know why the "linking to a given property" would be a dbus
representation.  Metric service should know which properties from dbus
map to some metric entity, right?  For a one-user piece of information,
I don't see a good reason to put this on dbus.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)

[Ed Tanous]

On Tue, Apr 27, 2021 at 4:38 AM Ambrozewicz, Adrian
<adrian.ambrozewicz@linux.intel.com> wrote:
>
> W dniu 4/9/2021 o 17:27, Ed Tanous pisze:
> > On Wed, Apr 7, 2021 at 5:26 AM Ambrozewicz, Adrian
> > <adrian.ambrozewicz@linux.intel.com> wrote:
> >>
> >> Hi,
> >>
> >> Currently bmcweb exposes sensors as part of Chassis subnodes in Sensors,
> >> Power and Thermal schemas. All of them lists sensors as arrays of
> >> generic properties distinguished by Id/Name etc. On the other hand - for
> >> well-defined metrics Redfish specifies concrete schemas like
> >> ProcessorMetrics and MemoryMetrics. They define designated Redfish
> >> properties with given name and value type.
> >>
> >> I'm starting to explore ways to implement these schemas in bmcweb, while
> >> retaining interoperability with TelemetryService. This requirement
> >> implicates, that source of these properties should implement
> >> xyz.openbmc_project.Sensor.Value interface and comply with OpenBMC D-Bus
> >> sensors architecture, which enforces predefined paths and units for
> >> various types of sensors.
> >>
> >> Question of extending xyz.openbmc_project.Sensor.Value interface (so it
> >> allows for more types or nested paths, if necessary) is something  I
> >> know should be considered, but seems like more or less straightforward
> >> to address.
> >
> > I'm not following this statement.  From my perspective, this is
> > something we've already solved at a dbus level.  To attach a "sensor"
> > to a given inventory item, set up the association back to said
> > inventory item.  Today, we only connect things to Redfish "Chassis",
> > because that's currently the only node with sensors, but that's
> > certainly easy to change and there's nothing about the API that
> > prevents doing that.
> >
> That is correct. Technically we have all the pieces to integrate sensor
> with Redfish in general (Associations, standard sensor types and APIs).
> Challenge lies below.
>
> > Beyond that, because MemoryMetrics and ProcessorMetrics schemas call
> > out specific sensor names, I suspect we need to come up with a well
> > defined set of names;  If you want to populate MemoryMetrics, you
> > would expose sensors, with associations back to the dimm in question,
> > and have a sensor called, say,
> > /xyz/openbmc_project/remaining_block_percentage, and map that to the
> > RemainingSpareBlockPercentage property.
> >
> Is the path you've provided just a shortcut for full D-bus sensor path,
> or do you have something else in mind? I will assume the former.
>
> D-Bus sensor path is composed of :
> /xyz/openbmc_project/{namespace/type}/{name}. Do you propose to encode
> sensor name in such a way, that bmcweb (and IPMI,PLDM, whatever) would
> be able to identify exact meaning of the value if they need to? Bear in
> mind that we have several issues to consider here:

I missed the units in my first email.  That should've read;

xyz/openbmc_project/utilization/remaining_block_percentage

> - multiple items (eg. Processors) exposing the same data, names would
> need to be unique in that regard

This is solved by having an association back to the processor in
question.  Names would need to be unique, but not unique
per-processor, although you bring up a good point.
As I think through this more, I suspect Sensor.Value isn't a good fit
for this kind of data.  I suspect we should come up with some specific
interfaces for exposing this kind of telemetry in a standard way.

> - certain properties are singular (CPU-wide) while other are buried down
> the hierarchy (mentioned UnhaltedCycles of Core#12).

Also solved by associations.  If it's associated with a core, it's the
property for that core, if it's associated with the processor object,
it's a property of that socket.

>
> This would lead to names like:
> /xyz/openbmc_project/sensors/counter/cpu1_core12_unhaltedcycles .
>
> Don't get me wrong - this seems like quite flexible approach, however it
> lacks standardization. If I understood response form Patrick correctly -
> there is urge to avoid storing information in D-Bus path. Do you think
> that would be acceptable to introduce this logic (name-encoding) in bmcweb?

No, having read Patricks email, I agree with him.  What I proposed was
only a strawman, and hadn't been fully thought through.

>
> >>
> >> There is bigger issue I see now - mapping D-Bus sensors to concrete
> >> Redfish properties. Mapping sensors at inventory level is sorted out
> >> with use of xyz.openbmc_project.Association.Definitions interface.
> >> However - I don't see (or know of) any method to link given D-Bus sensor
> >> with it's designated place in Redfish schema.
> >
> > This is by design.  Dbus should largely have no reliance on any
> > specific protocol, and we shouldn't be building interfaces that
> > require daemons pushing data on the bus to have any knowledge of
> > Redfish, IPMI, PLDM, or protocol of next year.  This generally means
> > that some dbus->redfish transforms are not as efficient or clean as
> > they could be, but it keeps that logic quarantined into bmcweb.  I
> > suspect bmcweb will need logic to translate redfish URI + property
> > name to dbus path, and this logic will be non-trivial.
> >
> Point taken - mapping data (in whatever form it will be available)
> should be generic and domain agnostic.
>
> >> I've done some PoC implementation of ProcessorMetrics, which relies on
> >> new D-Bus interface with 'Mapping' property (eg. 'TemperatureCelsius' or
> >> 'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node implementation
> >> queries D-Bus for all sensors associated with given CPU and populates
> >> properties if proper mapping was found.
> >
> > Do you have a pointer to it?  It'd be good to see the code you're  > thinking of.
> >
> Code itself is boilerplate D-Bus sensor service with one extra property.
> New property contains data encoded in the string, which you already
> proposed to be part of the name. Should you have need to work on some
> shared code to work out PoC in the flesh - let me know.

Text descriptions of code are a lot less useful than code I can look
at and run.  With that said, if you don't want to publish it
somewhere, no worries.

>
> > With that said, I would be against this approach.  This would require
> > clients to hardcode in, say, BlocksWritten, as a mapping, which means
> > that as we have more than just redfish, now each client needs to
> > hardcode the Redfish representation, the PLDM representation, and the
> > IPMI representation of the same data.  That seems messy.
> >
> >>
> >> Is my PoC approach described above feasible for OpenBMC? What are your
> >> thoughts? I would like to start a discussion and hear your proposals
> >> about possible alternatives.
> >>
> >> Regards,
> >> Adrian

Re: Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)

[Ed Tanous]

On Tue, May 11, 2021 at 7:53 AM Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Tue, Apr 27, 2021 at 01:52:51PM +0200, Ambrozewicz, Adrian wrote:
> > W dniu 4/9/2021 o 14:05, Patrick Williams pisze:
> > > On Wed, Apr 07, 2021 at 02:24:55PM +0200, Ambrozewicz, Adrian wrote:
> > >
> > > I suspect this would be the more difficult direction to go down.  There
> > > is already enough code that looks for sensors at specific paths that
> > > you'd have to track down and fix up.  Also, there has been some concern
> > > by some maintainers in other cases about having information in the paths
> > > have meaning and prefering to reduce the reliance on that.
> > >
> >
> > Please see message from Ed, as he's supposedly proposing to follow that
> > path. I don't have strong opinions on one or the other approach.
>
> I suspect you are not signing up to change all the existing code.  I'll
> look at Ed's reply though.
>
> > I've read the design, however one thing is not clear for me. My current
> > understanding was that for each association there would need to exist
> > some D-bus object at given path somewhere. Would i need my CPU inventory
> > service to also expose separate objects for each core for my association
> > to be 'legal', or could we represent some virtual hierarchy with no
> > actual D-Bus object in the system?
>
> Yes.  You would need an inventory object for each entity you want to
> attach sensors or metrics to.  This doesn't seem like it should really
> be an issue.  Other people have been working on adding CPU Cores already
> and there is the xyz.openbmc_project.Inventory.Item.CpuCore defined.
>
> > >> I've done some PoC implementation of ProcessorMetrics, which relies on
> > >> new D-Bus interface with 'Mapping' property (eg. 'TemperatureCelsius' or
> > >> 'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node implementation
> > >> queries D-Bus for all sensors associated with given CPU and populates
> > >> properties if proper mapping was found.
> > >
> > > I'm not really grasping what the contents of this mapping property are.
> > > Generally we want to stay away from free-form strings having programatic
> > > uses.  Maybe if you can define these mappings as enumerations?
> > >
> > > What is the additional information you need besides the assocation from
> > > a sensor to its inventory item?
> >
> > In given example I would like my sensor to be source of information for
> > property defined by ProcessorMetrics schema. In the example I've used
> > property associated with given Core, thus CoreMetrics/12/UnhaltedCycles
> > maps directly to ProcessorMetrics sub-property. Enumerations could be
> > not enough as we have multiple informations to represent:
> > - association with given processor (done by ProcessorMetrics)
> > - association with given core (could that be handled by your proposed
> > design?)
> > - linking to given property
> >
> > Would the enumeration be used for the last element, while leaving
> > hierarchy problem to Associations?
>
> "UnhaltedCycles" is not a sensor, just to be clear.  IPMI might have
> called these kinds of things sensors but we do not.  Sensors for us
> measure physical properties.  This is just a property (or maybe a
> "metric") but it doesn't belong in the sensors namespace or modeled with
> a Sensor.Value.
>

This somewhat brings up a good point, what is a "sensor" on dbus?  I
would've assumed that these would be well represented as sensors, as
they do measure physical properties.  I hadn't assumed that they had
this limitation because we do have the
/xyz/openbmc_project/utilization namespace defined already.  If we're
going down the path of "must be physical" it would seem like
utilization should be moved out of the sensors interface?  Or am I
taking your statement too literally?

Not reusing sensor seems like it would lead to a lot of code
duplication, as every API would now need to understand everything
about every "publishes real time telemetry" type interface, and every
time we add a new one, we'd (probably) have to update the code to add
the new type.  That doesn't really seem maintainable to me for every
type of telemetry we might want;  If a sensor isn't the right place to
put it, how would we solve the "I want to publish all telemetry
values" type use cases?  Maybe namespace the interface itself, so we
can use the arg0namespace feature in match expressions?  I'm thinking
out loud at this point...

> I don't know why the "linking to a given property" would be a dbus
> representation.  Metric service should know which properties from dbus
> map to some metric entity, right?  For a one-user piece of information,
> I don't see a good reason to put this on dbus.

I think the issue here is how do you know that a specific value
relates to say, the processor utilization, or the ram utilization, or
the smart statistics?

>
> --
> Patrick Williams

Re: Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)

[Ambrozewicz, Adrian]

W dniu 5/11/2021 o 18:26, Ed Tanous pisze:
> On Tue, May 11, 2021 at 7:53 AM Patrick Williams <patrick@stwcx.xyz> wrote:
>>
>> On Tue, Apr 27, 2021 at 01:52:51PM +0200, Ambrozewicz, Adrian wrote:
>>> W dniu 4/9/2021 o 14:05, Patrick Williams pisze:
>>>> On Wed, Apr 07, 2021 at 02:24:55PM +0200, Ambrozewicz, Adrian wrote:
>>>>
>>>> I suspect this would be the more difficult direction to go down.  There
>>>> is already enough code that looks for sensors at specific paths that
>>>> you'd have to track down and fix up.  Also, there has been some concern
>>>> by some maintainers in other cases about having information in the paths
>>>> have meaning and prefering to reduce the reliance on that.
>>>>
>>>
>>> Please see message from Ed, as he's supposedly proposing to follow that
>>> path. I don't have strong opinions on one or the other approach.
>>
>> I suspect you are not signing up to change all the existing code.  I'll
>> look at Ed's reply though.
>>

Crisis averted - paths and names dropped out from scope :)

>>> I've read the design, however one thing is not clear for me. My current
>>> understanding was that for each association there would need to exist
>>> some D-bus object at given path somewhere. Would i need my CPU inventory
>>> service to also expose separate objects for each core for my association
>>> to be 'legal', or could we represent some virtual hierarchy with no
>>> actual D-Bus object in the system?
>>
>> Yes.  You would need an inventory object for each entity you want to
>> attach sensors or metrics to.  This doesn't seem like it should really
>> be an issue.  Other people have been working on adding CPU Cores already
>> and there is the xyz.openbmc_project.Inventory.Item.CpuCore defined.
>>

Thanks for pointing that out. It seems like logical path to follow. Do 
you have some pointers to some reviews or discussion? CpuCore as of now 
is empty.

>>>>> I've done some PoC implementation of ProcessorMetrics, which relies on
>>>>> new D-Bus interface with 'Mapping' property (eg. 'TemperatureCelsius' or
>>>>> 'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node implementation
>>>>> queries D-Bus for all sensors associated with given CPU and populates
>>>>> properties if proper mapping was found.
>>>>
>>>> I'm not really grasping what the contents of this mapping property are.
>>>> Generally we want to stay away from free-form strings having programatic
>>>> uses.  Maybe if you can define these mappings as enumerations?
>>>>
>>>> What is the additional information you need besides the assocation from
>>>> a sensor to its inventory item?
>>>
>>> In given example I would like my sensor to be source of information for
>>> property defined by ProcessorMetrics schema. In the example I've used
>>> property associated with given Core, thus CoreMetrics/12/UnhaltedCycles
>>> maps directly to ProcessorMetrics sub-property. Enumerations could be
>>> not enough as we have multiple informations to represent:
>>> - association with given processor (done by ProcessorMetrics)
>>> - association with given core (could that be handled by your proposed
>>> design?)
>>> - linking to given property
>>>
>>> Would the enumeration be used for the last element, while leaving
>>> hierarchy problem to Associations?
>>
>> "UnhaltedCycles" is not a sensor, just to be clear.  IPMI might have
>> called these kinds of things sensors but we do not.  Sensors for us
>> measure physical properties.  This is just a property (or maybe a
>> "metric") but it doesn't belong in the sensors namespace or modeled with
>> a Sensor.Value.

Up to this point we've established, that sensors/metrics would be linked 
to given item by association. That leaves figuring out how to 'glue' 
together Redfish property with given D-Bus entity.

> 
> This somewhat brings up a good point, what is a "sensor" on dbus?  I
> would've assumed that these would be well represented as sensors, as
> they do measure physical properties.  I hadn't assumed that they had
> this limitation because we do have the
> /xyz/openbmc_project/utilization namespace defined already.  If we're
> going down the path of "must be physical" it would seem like
> utilization should be moved out of the sensors interface?  Or am I
> taking your statement too literally?
> 

Agreed. I suppose we should forget about 'old ways' and previous meaning 
of IPMI sensor. BTW Redfish specifies such properties as 'metrics'.

What are the limitations of Sensor.Value interface when it comes to 
representing values in ProcessorMetrics and similar schemas?

ProcessorMetrics uses such units for its metrics:
- bytes
- % (already available as 'utilization')
- MHz
- Cel (altready available as 'temperature')
- count (number of events/occurrences)

I suppose that we could just extend namespaces and units of Sensor.Value 
to cover them and call it a day. We would retain compatibility with 
existing sensors code, TelemetryService etc. (I agree with below comment 
by Ed). I am aware, that when more schemas and metric types comes this 
list will grow, but do we have other alternative?

What are your thoughts? If Sensor.Value is not the way, then how would 
we define the next interface?

> Not reusing sensor seems like it would lead to a lot of code
> duplication, as every API would now need to understand everything
> about every "publishes real time telemetry" type interface, and every
> time we add a new one, we'd (probably) have to update the code to add
> the new type.  That doesn't really seem maintainable to me for every
> type of telemetry we might want;  If a sensor isn't the right place to
> put it, how would we solve the "I want to publish all telemetry
> values" type use cases?  Maybe namespace the interface itself, so we
> can use the arg0namespace feature in match expressions?  I'm thinking
> out loud at this point...
> 
>> I don't know why the "linking to a given property" would be a dbus
>> representation.  Metric service should know which properties from dbus
>> map to some metric entity, right?  For a one-user piece of information,
>> I don't see a good reason to put this on dbus.
> 
> I think the issue here is how do you know that a specific value
> relates to say, the processor utilization, or the ram utilization, or
> the smart statistics?

Yes, this is the gap we still need to address. Perhaps idea with with an 
well defined enum is not a bad one?

Taking an example of ProcessorMetrics\CoreMetrics[]\UnhaltedCycles
We would have an D-Bus sensor with given interfaces:

xyz.openbmc_project.Association.Definitions
.Associations	{"cpucore", "all_sensors", "/xyz.../core/5"}

xyz.openbmc_project.Sensor.Value
.Unit		"Count" # new unit
.Value		123123

xyz.openbmc_project.CpuCore.Metrics # New imagined interface with enum
.Type		"UnhaltedCycles"

> 
>>
>> --
>> Patrick Williams

Thanks a lot for your input, it seems like we're going in good direction.

Re: Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas)

[Ambrozewicz, Adrian]

Guys,

Part of the problem can be solved with existing solutions. At least If I 
understood you correctly. There are still some questions or opens to 
cover. I would appreciate if you found time to respond.

Regards,
Adrian

W dniu 5/12/2021 o 13:17, Ambrozewicz, Adrian pisze:
> W dniu 5/11/2021 o 18:26, Ed Tanous pisze:
>> On Tue, May 11, 2021 at 7:53 AM Patrick Williams <patrick@stwcx.xyz> 
>> wrote:
>>>
>>> On Tue, Apr 27, 2021 at 01:52:51PM +0200, Ambrozewicz, Adrian wrote:
>>>> W dniu 4/9/2021 o 14:05, Patrick Williams pisze:
>>>>> On Wed, Apr 07, 2021 at 02:24:55PM +0200, Ambrozewicz, Adrian wrote:
>>>>>
>>>>> I suspect this would be the more difficult direction to go down.  
>>>>> There
>>>>> is already enough code that looks for sensors at specific paths that
>>>>> you'd have to track down and fix up.  Also, there has been some 
>>>>> concern
>>>>> by some maintainers in other cases about having information in the 
>>>>> paths
>>>>> have meaning and prefering to reduce the reliance on that.
>>>>>
>>>>
>>>> Please see message from Ed, as he's supposedly proposing to follow that
>>>> path. I don't have strong opinions on one or the other approach.
>>>
>>> I suspect you are not signing up to change all the existing code.  I'll
>>> look at Ed's reply though.
>>>
> 
> Crisis averted - paths and names dropped out from scope :)
> 
>>>> I've read the design, however one thing is not clear for me. My current
>>>> understanding was that for each association there would need to exist
>>>> some D-bus object at given path somewhere. Would i need my CPU 
>>>> inventory
>>>> service to also expose separate objects for each core for my 
>>>> association
>>>> to be 'legal', or could we represent some virtual hierarchy with no
>>>> actual D-Bus object in the system?
>>>
>>> Yes.  You would need an inventory object for each entity you want to
>>> attach sensors or metrics to.  This doesn't seem like it should really
>>> be an issue.  Other people have been working on adding CPU Cores already
>>> and there is the xyz.openbmc_project.Inventory.Item.CpuCore defined.
>>>
> 
> Thanks for pointing that out. It seems like logical path to follow. Do 
> you have some pointers to some reviews or discussion? CpuCore as of now 
> is empty.
> 
>>>>>> I've done some PoC implementation of ProcessorMetrics, which 
>>>>>> relies on
>>>>>> new D-Bus interface with 'Mapping' property (eg. 
>>>>>> 'TemperatureCelsius' or
>>>>>> 'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node 
>>>>>> implementation
>>>>>> queries D-Bus for all sensors associated with given CPU and populates
>>>>>> properties if proper mapping was found.
>>>>>
>>>>> I'm not really grasping what the contents of this mapping property 
>>>>> are.
>>>>> Generally we want to stay away from free-form strings having 
>>>>> programatic
>>>>> uses.  Maybe if you can define these mappings as enumerations?
>>>>>
>>>>> What is the additional information you need besides the assocation 
>>>>> from
>>>>> a sensor to its inventory item?
>>>>
>>>> In given example I would like my sensor to be source of information for
>>>> property defined by ProcessorMetrics schema. In the example I've used
>>>> property associated with given Core, thus CoreMetrics/12/UnhaltedCycles
>>>> maps directly to ProcessorMetrics sub-property. Enumerations could be
>>>> not enough as we have multiple informations to represent:
>>>> - association with given processor (done by ProcessorMetrics)
>>>> - association with given core (could that be handled by your proposed
>>>> design?)
>>>> - linking to given property
>>>>
>>>> Would the enumeration be used for the last element, while leaving
>>>> hierarchy problem to Associations?
>>>
>>> "UnhaltedCycles" is not a sensor, just to be clear.  IPMI might have
>>> called these kinds of things sensors but we do not.  Sensors for us
>>> measure physical properties.  This is just a property (or maybe a
>>> "metric") but it doesn't belong in the sensors namespace or modeled with
>>> a Sensor.Value.
> 
> Up to this point we've established, that sensors/metrics would be linked 
> to given item by association. That leaves figuring out how to 'glue' 
> together Redfish property with given D-Bus entity.
> 
>>
>> This somewhat brings up a good point, what is a "sensor" on dbus?  I
>> would've assumed that these would be well represented as sensors, as
>> they do measure physical properties.  I hadn't assumed that they had
>> this limitation because we do have the
>> /xyz/openbmc_project/utilization namespace defined already.  If we're
>> going down the path of "must be physical" it would seem like
>> utilization should be moved out of the sensors interface?  Or am I
>> taking your statement too literally?
>>
> 
> Agreed. I suppose we should forget about 'old ways' and previous meaning 
> of IPMI sensor. BTW Redfish specifies such properties as 'metrics'.
> 
> What are the limitations of Sensor.Value interface when it comes to 
> representing values in ProcessorMetrics and similar schemas?
> 
> ProcessorMetrics uses such units for its metrics:
> - bytes
> - % (already available as 'utilization')
> - MHz
> - Cel (altready available as 'temperature')
> - count (number of events/occurrences)
> 
> I suppose that we could just extend namespaces and units of Sensor.Value 
> to cover them and call it a day. We would retain compatibility with 
> existing sensors code, TelemetryService etc. (I agree with below comment 
> by Ed). I am aware, that when more schemas and metric types comes this 
> list will grow, but do we have other alternative?
> 
> What are your thoughts? If Sensor.Value is not the way, then how would 
> we define the next interface?
> 
>> Not reusing sensor seems like it would lead to a lot of code
>> duplication, as every API would now need to understand everything
>> about every "publishes real time telemetry" type interface, and every
>> time we add a new one, we'd (probably) have to update the code to add
>> the new type.  That doesn't really seem maintainable to me for every
>> type of telemetry we might want;  If a sensor isn't the right place to
>> put it, how would we solve the "I want to publish all telemetry
>> values" type use cases?  Maybe namespace the interface itself, so we
>> can use the arg0namespace feature in match expressions?  I'm thinking
>> out loud at this point...
>>
>>> I don't know why the "linking to a given property" would be a dbus
>>> representation.  Metric service should know which properties from dbus
>>> map to some metric entity, right?  For a one-user piece of information,
>>> I don't see a good reason to put this on dbus.
>>
>> I think the issue here is how do you know that a specific value
>> relates to say, the processor utilization, or the ram utilization, or
>> the smart statistics?
> 
> Yes, this is the gap we still need to address. Perhaps idea with with an 
> well defined enum is not a bad one?
> 
> Taking an example of ProcessorMetrics\CoreMetrics[]\UnhaltedCycles
> We would have an D-Bus sensor with given interfaces:
> 
> xyz.openbmc_project.Association.Definitions
> .Associations    {"cpucore", "all_sensors", "/xyz.../core/5"}
> 
> xyz.openbmc_project.Sensor.Value
> .Unit        "Count" # new unit
> .Value        123123
> 
> xyz.openbmc_project.CpuCore.Metrics # New imagined interface with enum
> .Type        "UnhaltedCycles"
> 
>>
>>>
>>> -- 
>>> Patrick Williams
> 
> Thanks a lot for your input, it seems like we're going in good direction.
>