All of lore.kernel.org
 help / color / mirror / Atom feed
* Processing PLDM FRU information with entity manager
@ 2020-05-19  3:40 Deepak Kodihalli
  2020-05-20 16:28 ` Brad Bishop
  2020-05-26 12:56 ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Deepak Kodihalli
  0 siblings, 2 replies; 27+ messages in thread
From: Deepak Kodihalli @ 2020-05-19  3:40 UTC (permalink / raw)
  To: Brad Bishop, James Feist, Thomaiyar, Richard Marian, Bhat, Sumanth
  Cc: openbmc

Hi,

IBM systems have a requirement to consume FRU information sent down via 
the host firmware and then relay that onto D-Bus (and then onto 
Redfish). The host firmware will send down FRU information using PLDM.

We wanted to use entity manager to enable transforming the PLDM FRU data 
to D-Bus properties that fall under D-Bus inventory interfaces such as 
the xyz.openbmc_project.Inventory.Decorator.Asset interface. I have an 
update to the PLDM design doc to capture this flow [1], and some D-Bus 
interfaces [2] proposed on Gerrit. Would appreciate feedback on the 
same. The high level idea is that the pldm daemon will host raw PLDM FRU 
information on D-Bus, and via JSON configs, entity manager can convert 
those to D-Bus inventory objects (which then can be found by bmcweb).

 From an entity manager perspective, I had few questions :
- I see there is provision for persistence, but it looks like applying 
the persisted information works only if "D-Bus probes" succeed. We have 
a requirement to make the host-sent inventory information available even 
when the host is powered off. Now if the host has sent this, then powers 
off, and then BMC reboots, the BMC will no longer have the raw PLDM FRU 
information on D-Bus and hence the entity manager probe on the same will 
fail. Question is, can the probes be made optional when reading the 
persisted config (system.json)?

- How are hierarchical relationships between FRUs supposed to be 
represented? Is that based on D-Bus pathnames? Or making use of 
something like the D-Bus Associations interface? Any thoughts on how 
representing such parent-child relation can be achieved via entity 
manager configs?

[1] https://gerrit.openbmc-project.xyz/#/c/openbmc/docs/+/32532/
[2] 
https://gerrit.openbmc-project.xyz/#/c/openbmc/phosphor-dbus-interfaces/+/32533/

Thanks,
Deepak

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Processing PLDM FRU information with entity manager
  2020-05-19  3:40 Processing PLDM FRU information with entity manager Deepak Kodihalli
@ 2020-05-20 16:28 ` Brad Bishop
  2020-05-20 16:56   ` James Feist
  2020-05-26 12:56 ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Deepak Kodihalli
  1 sibling, 1 reply; 27+ messages in thread
From: Brad Bishop @ 2020-05-20 16:28 UTC (permalink / raw)
  To: Deepak Kodihalli, James Feist, Thomaiyar, Richard Marian, Bhat, Sumanth
  Cc: openbmc

On Tue, 2020-05-19 at 09:10 +0530, Deepak Kodihalli wrote:

Hi Deepak

> I see there is provision for persistence, but it looks like
> applying the persisted information works only if "D-Bus probes"
> succeed.

This prompted me to take a closer look - as far as I can tell
system.json is for debugging purposes only.  Maybe James could confirm.

Given this we should probably have an application layer other than
entity-manager layer be responsible for maintaining the PLDM FRU
objects on the DBus at the correct time. 

> the BMC will no longer have the raw PLDM FRU information on D-Bus

I think we have to fix this.  It doesn't feel right that the PLDM FRU
DBus objects come and go off the bus as the system is powered on/off.

> How are hierarchical relationships between FRUs supposed to be 
> represented? Is that based on D-Bus pathnames? Or making use of 
> something like the D-Bus Associations interface? Any thoughts on how 
> representing such parent-child relation can be achieved via entity 
> manager configs?

I'm also hoping James or Richard will let us know what their thoughts
are for doing this.

-brad

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Processing PLDM FRU information with entity manager
  2020-05-20 16:28 ` Brad Bishop
@ 2020-05-20 16:56   ` James Feist
  2020-05-20 17:35     ` Brad Bishop
  0 siblings, 1 reply; 27+ messages in thread
From: James Feist @ 2020-05-20 16:56 UTC (permalink / raw)
  To: Brad Bishop, Deepak Kodihalli, Thomaiyar, Richard Marian, Bhat, Sumanth
  Cc: openbmc

On 5/20/2020 9:28 AM, Brad Bishop wrote:
> On Tue, 2020-05-19 at 09:10 +0530, Deepak Kodihalli wrote:
> 
> Hi Deepak
> 
>> I see there is provision for persistence, but it looks like
>> applying the persisted information works only if "D-Bus probes"
>> succeed.

There is a PowerState parameter that tells when it can be detected that 
can be used. PowerState="On" things only get removed if detected once 
power is back online.

> 
> This prompted me to take a closer look - as far as I can tell
> system.json is for debugging purposes only.  Maybe James could confirm.

No, the modifiable things are persisted there. Such as thresholds.

> 
> Given this we should probably have an application layer other than
> entity-manager layer be responsible for maintaining the PLDM FRU
> objects on the DBus at the correct time.
> 
>> the BMC will no longer have the raw PLDM FRU information on D-Bus
> 
> I think we have to fix this.  It doesn't feel right that the PLDM FRU
> DBus objects come and go off the bus as the system is powered on/off.
> 
>> How are hierarchical relationships between FRUs supposed to be
>> represented? Is that based on D-Bus pathnames? Or making use of
>> something like the D-Bus Associations interface? Any thoughts on how
>> representing such parent-child relation can be achieved via entity
>> manager configs?
> 
> I'm also hoping James or Richard will let us know what their thoughts
> are for doing this.

I need a better example. FRUs are independent things, so there is no 
hierarchy. Are you talking about Chassis level things? Those are just 
handled by having the right type.

> 
> -brad
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Processing PLDM FRU information with entity manager
  2020-05-20 16:56   ` James Feist
@ 2020-05-20 17:35     ` Brad Bishop
  2020-05-20 17:46       ` James Feist
  0 siblings, 1 reply; 27+ messages in thread
From: Brad Bishop @ 2020-05-20 17:35 UTC (permalink / raw)
  To: James Feist, Deepak Kodihalli, Thomaiyar, Richard Marian, Bhat, Sumanth
  Cc: openbmc

On Wed, 2020-05-20 at 09:56 -0700, James Feist wrote:

> I need a better example. FRUs are independent things, so there is no 
> hierarchy.

A drive plugs into a riser which plugs into the motherboard.  Isn't
that a hierarchy?

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Processing PLDM FRU information with entity manager
  2020-05-20 17:35     ` Brad Bishop
@ 2020-05-20 17:46       ` James Feist
  0 siblings, 0 replies; 27+ messages in thread
From: James Feist @ 2020-05-20 17:46 UTC (permalink / raw)
  To: Brad Bishop, Deepak Kodihalli, Thomaiyar, Richard Marian, Bhat, Sumanth
  Cc: openbmc

On 5/20/2020 10:35 AM, Brad Bishop wrote:
> On Wed, 2020-05-20 at 09:56 -0700, James Feist wrote:
> 
>> I need a better example. FRUs are independent things, so there is no
>> hierarchy.
> 
> A drive plugs into a riser which plugs into the motherboard.  Isn't
> that a hierarchy?
> 

For a FRU, no. FRUs are independent items. For inventory, yes. Assuming 
you mean inventory then? If so, then the paths do not have any hierarchy 
today. I assume it might be possible by creating an i2c device tree of 
sorts and mapping back what bus is which item.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager)
  2020-05-19  3:40 Processing PLDM FRU information with entity manager Deepak Kodihalli
  2020-05-20 16:28 ` Brad Bishop
@ 2020-05-26 12:56 ` Deepak Kodihalli
  2020-05-27 13:50   ` D-Bus interface to provide data to entity manager Thomaiyar, Richard Marian
  2020-05-28 12:03   ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Patrick Williams
  1 sibling, 2 replies; 27+ messages in thread
From: Deepak Kodihalli @ 2020-05-26 12:56 UTC (permalink / raw)
  To: Brad Bishop, James Feist, Thomaiyar, Richard Marian, Bhat,
	Sumanth, Patrick Williams
  Cc: openbmc

On 19/05/20 9:10 am, Deepak Kodihalli wrote:
> Hi,
> 
> IBM systems have a requirement to consume FRU information sent down via 
> the host firmware and then relay that onto D-Bus (and then onto 
> Redfish). The host firmware will send down FRU information using PLDM.
> 
> We wanted to use entity manager to enable transforming the PLDM FRU data 
> to D-Bus properties that fall under D-Bus inventory interfaces such as 
> the xyz.openbmc_project.Inventory.Decorator.Asset interface. I have an 
> update to the PLDM design doc to capture this flow [1], and some D-Bus 
> interfaces [2] proposed on Gerrit. Would appreciate feedback on the 
> same. The high level idea is that the pldm daemon will host raw PLDM FRU 
> information on D-Bus, and via JSON configs, entity manager can convert 
> those to D-Bus inventory objects (which then can be found by bmcweb).
> 
>  From an entity manager perspective, I had few questions :
> - I see there is provision for persistence, but it looks like applying 
> the persisted information works only if "D-Bus probes" succeed. We have 
> a requirement to make the host-sent inventory information available even 
> when the host is powered off. Now if the host has sent this, then powers 
> off, and then BMC reboots, the BMC will no longer have the raw PLDM FRU 
> information on D-Bus and hence the entity manager probe on the same will 
> fail. Question is, can the probes be made optional when reading the 
> persisted config (system.json)?
> 
> - How are hierarchical relationships between FRUs supposed to be 
> represented? Is that based on D-Bus pathnames? Or making use of 
> something like the D-Bus Associations interface? Any thoughts on how 
> representing such parent-child relation can be achieved via entity 
> manager configs?
> 
> [1] https://gerrit.openbmc-project.xyz/#/c/openbmc/docs/+/32532/
> [2] 
> https://gerrit.openbmc-project.xyz/#/c/openbmc/phosphor-dbus-interfaces/+/32533/ 
> 
> 
> Thanks,
> Deepak
> 

I've got some feedback on the proposal above, which is primarily 
directed at/impacts how the PLDM daemon provides FRU information to the 
entity manager daemon. Wanted to discuss the same here.

Very briefly, the proposal was :
a) The PLDM daemon will parse PLDM FRU format data and host the same on 
D-Bus as a set of PLDM FRU properties (similar to how the FruDevice 
daemon hosts properties under xyz.openbmc_project.FruDevice).
b) Apply EM system/board specific configurations on a) to be able to 
create specific inventory D-Bus objects (this is how EM works today).


To do a) above, there are 3 options:

1) Propose a D-Bus interface in phosphor-dbus-interfaces. This was [2] 
in my original email above. The concern raised by Patrick here is that 
this interface is very specific to a protocol (PLDM in this case), 
whereas the phosphor D-Bus interfaces are mostly abstract and protocol 
agnostic.

In my opinion, PLDM is also a data model, so PLDM specific D-Bus 
interfaces can enable two apps that are PLDM aware (for eg a PLDM 
requester app talking to the PLDM daemon) to talk to each other. I do 
see other protocol specific D-Bus interfaces as well (for eg related to 
SMBIOS). So I don't really understand the concern. The protocol specific 
interfaces do not preclude other abstract interfaces.



2) Propose a generic/abstract 'FRU properties' kind of D-Bus interface. 
This is something that both the PLDM daemon and FRU device daemon could 
use to host FRU information on D-Bus, and to provide the same as an 
intermediate FRU format data to apps like EM. The suggestion on the docs 
commit above [2] was to have an interface like {Enum[Protocol], 
array[byte]}. I think instead this could be a dict[string, 
variant[string, int64]], for a FRU property to value mapping.

While this sounds interesting, are the maintainers of EM and FruDevice 
interested in such an interface? Based on how this interface is 
designed, it might imply changes to FruDevice and Entity Manager. I 
might be interested in chasing this based on the feedback received, and 
if this will really have users other than the PLDM daemon.



3) If everyone thinks option 1 is a bad idea, and if the interest in 
option 2 is limited, I could do this based on how the FruDevice daemon 
and EM interact today, which is based on kind of a private D-Bus 
interface between the two apps. I don't think the Fru device daemon is 
tied up to EM though, it could even be in its own repository.


Thanks,
Deepak

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-26 12:56 ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Deepak Kodihalli
@ 2020-05-27 13:50   ` Thomaiyar, Richard Marian
  2020-05-27 13:58     ` Deepak Kodihalli
  2020-05-28 12:03   ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Patrick Williams
  1 sibling, 1 reply; 27+ messages in thread
From: Thomaiyar, Richard Marian @ 2020-05-27 13:50 UTC (permalink / raw)
  To: Deepak Kodihalli, Brad Bishop, James Feist, Bhat, Sumanth,
	Patrick Williams
  Cc: openbmc

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

I always view D-Bus interface as a specification / API which can work 
with different producers / consumers (correct me, if that's not what we 
intend with D-Bus interface here). Problem with Option 1 is, it will end 
up in having multiple producers exposing different interface, and 
thereby consumers(user interface facing applications) of D-Bus must be 
aware about all the D-Bus interfaces and always requires change.

Consider, we want to expose ChassisType, then irrespective of PLDM FRU / 
IPMI FRU / Proprietary FRU, Consumer applications must read it in the 
same manner. Having different interface / property types, requires 
update in both the end. PLDM FRU / IPMI FRU can be in common form 
(except few nit's /OEM's). We need to make sure it is represented in 
that angle. As of today phosphor-dbus-interfaces doesn't have FRU 
interface, but it has inventory related interfaces (exposed by 
Entity-Manager), which is what Redfish uses (i.e. Indirectly the 
FruDevice exposed interface is hidden by Entity-manager, and inventory 
interface exposed by entity-manager is used).

As of today, entity-manager doesn't add Inventory interface 
automatically for Add-on cards (which doesn't have any json 
configuration), but needs exposure (say PLDM based Add on card devices 
will be of this type), but shouldn't be hard to add it anyway.

Now the question is do we want to expose FRU as a separate interafce to 
be used in User facing application, or shall we just use Inventory based 
interface itself ?If inventory itself can be used, then let's go ahead 
and add more fields to those if missing anything / correct the same 
accordingly.

James, Deepak, Patrick - your thoughts ?


regards,

Richard


Say, we want to expose Manufacturer Name, then it can be produced by 
PLDM FRU application, IPMI Fru based application or even any proprietary 
application and consumed by applications like Redfish / any other 
proprietary one. In this way applications can get the data of what ever 
it is required. I don't find any data which is different in terms of 
PLDM FRU / IPMI FRU (ofcourse OEM fields will be there, but that can't 
be unique), but we need to implement things in common form though.

Say, ChassisType in IPMI FRU is single byte field, whereas in PLDM FRU 
it will be of string. But we need to represent the same in well 
established form (say SMBIOS System /Chassis Type enums). i.e. Producers 
(IPMI FRU must change it from one byte type to enum), and PLDM FRU from 
string to proper enum. Redfish will use this one and accordingly map it 
to the schema


On 5/26/2020 6:26 PM, Deepak Kodihalli wrote:
> On 19/05/20 9:10 am, Deepak Kodihalli wrote:
>> Hi,
>>
>> IBM systems have a requirement to consume FRU information sent down 
>> via the host firmware and then relay that onto D-Bus (and then onto 
>> Redfish). The host firmware will send down FRU information using PLDM.
>>
>> We wanted to use entity manager to enable transforming the PLDM FRU 
>> data to D-Bus properties that fall under D-Bus inventory interfaces 
>> such as the xyz.openbmc_project.Inventory.Decorator.Asset interface. 
>> I have an update to the PLDM design doc to capture this flow [1], and 
>> some D-Bus interfaces [2] proposed on Gerrit. Would appreciate 
>> feedback on the same. The high level idea is that the pldm daemon 
>> will host raw PLDM FRU information on D-Bus, and via JSON configs, 
>> entity manager can convert those to D-Bus inventory objects (which 
>> then can be found by bmcweb).
>>
>>  From an entity manager perspective, I had few questions :
>> - I see there is provision for persistence, but it looks like 
>> applying the persisted information works only if "D-Bus probes" 
>> succeed. We have a requirement to make the host-sent inventory 
>> information available even when the host is powered off. Now if the 
>> host has sent this, then powers off, and then BMC reboots, the BMC 
>> will no longer have the raw PLDM FRU information on D-Bus and hence 
>> the entity manager probe on the same will fail. Question is, can the 
>> probes be made optional when reading the persisted config (system.json)?
>>
>> - How are hierarchical relationships between FRUs supposed to be 
>> represented? Is that based on D-Bus pathnames? Or making use of 
>> something like the D-Bus Associations interface? Any thoughts on how 
>> representing such parent-child relation can be achieved via entity 
>> manager configs?
>>
>> [1] https://gerrit.openbmc-project.xyz/#/c/openbmc/docs/+/32532/
>> [2] 
>> https://gerrit.openbmc-project.xyz/#/c/openbmc/phosphor-dbus-interfaces/+/32533/ 
>>
>>
>> Thanks,
>> Deepak
>>
>
> I've got some feedback on the proposal above, which is primarily 
> directed at/impacts how the PLDM daemon provides FRU information to 
> the entity manager daemon. Wanted to discuss the same here.
>
> Very briefly, the proposal was :
> a) The PLDM daemon will parse PLDM FRU format data and host the same 
> on D-Bus as a set of PLDM FRU properties (similar to how the FruDevice 
> daemon hosts properties under xyz.openbmc_project.FruDevice).
> b) Apply EM system/board specific configurations on a) to be able to 
> create specific inventory D-Bus objects (this is how EM works today).
>
>
> To do a) above, there are 3 options:
>
> 1) Propose a D-Bus interface in phosphor-dbus-interfaces. This was [2] 
> in my original email above. The concern raised by Patrick here is that 
> this interface is very specific to a protocol (PLDM in this case), 
> whereas the phosphor D-Bus interfaces are mostly abstract and protocol 
> agnostic.
>
> In my opinion, PLDM is also a data model, so PLDM specific D-Bus 
> interfaces can enable two apps that are PLDM aware (for eg a PLDM 
> requester app talking to the PLDM daemon) to talk to each other. I do 
> see other protocol specific D-Bus interfaces as well (for eg related 
> to SMBIOS). So I don't really understand the concern. The protocol 
> specific interfaces do not preclude other abstract interfaces.
>
>
>
> 2) Propose a generic/abstract 'FRU properties' kind of D-Bus 
> interface. This is something that both the PLDM daemon and FRU device 
> daemon could use to host FRU information on D-Bus, and to provide the 
> same as an intermediate FRU format data to apps like EM. The 
> suggestion on the docs commit above [2] was to have an interface like 
> {Enum[Protocol], array[byte]}. I think instead this could be a 
> dict[string, variant[string, int64]], for a FRU property to value 
> mapping.
>
> While this sounds interesting, are the maintainers of EM and FruDevice 
> interested in such an interface? Based on how this interface is 
> designed, it might imply changes to FruDevice and Entity Manager. I 
> might be interested in chasing this based on the feedback received, 
> and if this will really have users other than the PLDM daemon.
>
>
>
> 3) If everyone thinks option 1 is a bad idea, and if the interest in 
> option 2 is limited, I could do this based on how the FruDevice daemon 
> and EM interact today, which is based on kind of a private D-Bus 
> interface between the two apps. I don't think the Fru device daemon is 
> tied up to EM though, it could even be in its own repository.
>
>
> Thanks,
> Deepak

[-- Attachment #2: Type: text/html, Size: 11285 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-27 13:50   ` D-Bus interface to provide data to entity manager Thomaiyar, Richard Marian
@ 2020-05-27 13:58     ` Deepak Kodihalli
  2020-05-27 15:25       ` Thomaiyar, Richard Marian
  0 siblings, 1 reply; 27+ messages in thread
From: Deepak Kodihalli @ 2020-05-27 13:58 UTC (permalink / raw)
  To: Thomaiyar, Richard Marian, Brad Bishop, James Feist, Bhat,
	Sumanth, Patrick Williams
  Cc: openbmc

On 27/05/20 7:20 pm, Thomaiyar, Richard Marian wrote:
> I always view D-Bus interface as a specification / API which can work 
> with different producers / consumers (correct me, if that's not what we 
> intend with D-Bus interface here). Problem with Option 1 is, it will end 
> up in having multiple producers exposing different interface, and 
> thereby consumers(user interface facing applications) of D-Bus must be 
> aware about all the D-Bus interfaces and always requires change.
> 
> Consider, we want to expose ChassisType, then irrespective of PLDM FRU / 
> IPMI FRU / Proprietary FRU, Consumer applications must read it in the 
> same manner. Having different interface / property types, requires 
> update in both the end. PLDM FRU / IPMI FRU can be in common form 
> (except few nit's /OEM's). We need to make sure it is represented in 
> that angle. As of today phosphor-dbus-interfaces doesn't have FRU 
> interface, but it has inventory related interfaces (exposed by 
> Entity-Manager), which is what Redfish uses (i.e. Indirectly the 
> FruDevice exposed interface is hidden by Entity-manager, and inventory 
> interface exposed by entity-manager is used).
> 
> As of today, entity-manager doesn't add Inventory interface 
> automatically for Add-on cards (which doesn't have any json 
> configuration), but needs exposure (say PLDM based Add on card devices 
> will be of this type), but shouldn't be hard to add it anyway.
> 
> Now the question is do we want to expose FRU as a separate interafce to 
> be used in User facing application, or shall we just use Inventory based 
> interface itself ?If inventory itself can be used, then let's go ahead 
> and add more fields to those if missing anything / correct the same 
> accordingly.
> 
> James, Deepak, Patrick - your thoughts ?

I guess there is a difference between FRU and inventory. If inventory 
interfaces could be used directly, why wouldn't the FruDevice or PLDM 
apps directly host inventory objects, why even use EM?

I believe these apps (FruDevice, PLDM daemon) operate at a per FRU 
level, and rely on something like EM to make one or more inventory 
objects based on the FRU data. So that was my option 2, a generic FRU 
properties interface. I'm just not sure at the moment the 
impact/interest of doing something like that and then aligning FruDevice 
and EM to the same.

Thanks,
Deepak

> 
> regards,
> 
> Richard

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-27 13:58     ` Deepak Kodihalli
@ 2020-05-27 15:25       ` Thomaiyar, Richard Marian
  2020-05-27 15:46         ` Deepak Kodihalli
  0 siblings, 1 reply; 27+ messages in thread
From: Thomaiyar, Richard Marian @ 2020-05-27 15:25 UTC (permalink / raw)
  To: Deepak Kodihalli, Brad Bishop, James Feist, Bhat, Sumanth,
	Patrick Williams
  Cc: openbmc


On 5/27/2020 7:28 PM, Deepak Kodihalli wrote:
> On 27/05/20 7:20 pm, Thomaiyar, Richard Marian wrote:
>> I always view D-Bus interface as a specification / API which can work 
>> with different producers / consumers (correct me, if that's not what 
>> we intend with D-Bus interface here). Problem with Option 1 is, it 
>> will end up in having multiple producers exposing different 
>> interface, and thereby consumers(user interface facing applications) 
>> of D-Bus must be aware about all the D-Bus interfaces and always 
>> requires change.
>>
>> Consider, we want to expose ChassisType, then irrespective of PLDM 
>> FRU / IPMI FRU / Proprietary FRU, Consumer applications must read it 
>> in the same manner. Having different interface / property types, 
>> requires update in both the end. PLDM FRU / IPMI FRU can be in common 
>> form (except few nit's /OEM's). We need to make sure it is 
>> represented in that angle. As of today phosphor-dbus-interfaces 
>> doesn't have FRU interface, but it has inventory related interfaces 
>> (exposed by Entity-Manager), which is what Redfish uses (i.e. 
>> Indirectly the FruDevice exposed interface is hidden by 
>> Entity-manager, and inventory interface exposed by entity-manager is 
>> used).
>>
>> As of today, entity-manager doesn't add Inventory interface 
>> automatically for Add-on cards (which doesn't have any json 
>> configuration), but needs exposure (say PLDM based Add on card 
>> devices will be of this type), but shouldn't be hard to add it anyway.
>>
>> Now the question is do we want to expose FRU as a separate interafce 
>> to be used in User facing application, or shall we just use Inventory 
>> based interface itself ?If inventory itself can be used, then let's 
>> go ahead and add more fields to those if missing anything / correct 
>> the same accordingly.
>>
>> James, Deepak, Patrick - your thoughts ?
>
> I guess there is a difference between FRU and inventory. If inventory 
> interfaces could be used directly, why wouldn't the FruDevice or PLDM 
> apps directly host inventory objects, why even use EM?
>
[Richard]: Inventory.Decorator is used in Redfish. i.e. Today Frudevice 
& it's interface exposed is consumed by Entity-Manager alone & Entity 
manager exposes all the needed configuration along with 
Inventory.Decorator.Asset interface. (As you stated, inventory does more 
than what FRU has to expose, but i am trying to see can we use 
Inventory.Decorator itself to expose these needed FRU information?). 
James F ? (Maintainer of both EM & bmcweb)

Current Behavior:

Say - Baseboard fru information is exposed by FruDevice, and Entity 
Manager exposes the sensor or any configuration required for platform A 
or Platform B (based on fru device data). Entity-manager also expose 
Inventory interface which hides the manufacturer name exposed by FRU, 
but exposes the it as Manufacturer in the Inventory interface and 
Redfish uses this).

In case of exposing the PLDM FRU

Option 1:

If we need to rely on PLDM FRU, then Entity-Manager will probe the 
FruDevice data exposed by PLDM FRU daemon, and expose the inventory 
(both for sensor configuration and decorator Asset), we don't change any 
upper layer application. We can follow the same design for the PLDM FRU 
daemon.

Note: Currently, it doesn't have a mechanism to expose all the FRU 
devices as Inventory.Decorator interface automatically when there is no 
json configuration for it in EM. This needs to be fixed (say for PLDM 
based Add-on card).

Option 2:

Define new FRU interface (common for both PLDM, IPMI etc.), similar to 
inventory and this itself will be used by both Entity-manager & user 
level interface application (say Redfish).

(Problem with option 2 is it requires changes in Currenr behavior, and 
redfish etc, may need to update it's code to use the common FRU 
interface rather than inventory.Decorator).

Regards,

Richard

> I believe these apps (FruDevice, PLDM daemon) operate at a per FRU 
> level, and rely on something like EM to make one or more inventory 
> objects based on the FRU data. So that was my option 2, a generic FRU 
> properties interface. I'm just not sure at the moment the 
> impact/interest of doing something like that and then aligning 
> FruDevice and EM to the same.
>
> Thanks,
> Deepak
>
>>
>> regards,
>>
>> Richard

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-27 15:25       ` Thomaiyar, Richard Marian
@ 2020-05-27 15:46         ` Deepak Kodihalli
  2020-05-27 17:19           ` Thomaiyar, Richard Marian
  0 siblings, 1 reply; 27+ messages in thread
From: Deepak Kodihalli @ 2020-05-27 15:46 UTC (permalink / raw)
  To: Thomaiyar, Richard Marian, Brad Bishop, James Feist, Bhat,
	Sumanth, Patrick Williams
  Cc: openbmc

On 27/05/20 8:55 pm, Thomaiyar, Richard Marian wrote:
> 
> On 5/27/2020 7:28 PM, Deepak Kodihalli wrote:
>> On 27/05/20 7:20 pm, Thomaiyar, Richard Marian wrote:
>>> I always view D-Bus interface as a specification / API which can work 
>>> with different producers / consumers (correct me, if that's not what 
>>> we intend with D-Bus interface here). Problem with Option 1 is, it 
>>> will end up in having multiple producers exposing different 
>>> interface, and thereby consumers(user interface facing applications) 
>>> of D-Bus must be aware about all the D-Bus interfaces and always 
>>> requires change.
>>>
>>> Consider, we want to expose ChassisType, then irrespective of PLDM 
>>> FRU / IPMI FRU / Proprietary FRU, Consumer applications must read it 
>>> in the same manner. Having different interface / property types, 
>>> requires update in both the end. PLDM FRU / IPMI FRU can be in common 
>>> form (except few nit's /OEM's). We need to make sure it is 
>>> represented in that angle. As of today phosphor-dbus-interfaces 
>>> doesn't have FRU interface, but it has inventory related interfaces 
>>> (exposed by Entity-Manager), which is what Redfish uses (i.e. 
>>> Indirectly the FruDevice exposed interface is hidden by 
>>> Entity-manager, and inventory interface exposed by entity-manager is 
>>> used).
>>>
>>> As of today, entity-manager doesn't add Inventory interface 
>>> automatically for Add-on cards (which doesn't have any json 
>>> configuration), but needs exposure (say PLDM based Add on card 
>>> devices will be of this type), but shouldn't be hard to add it anyway.
>>>
>>> Now the question is do we want to expose FRU as a separate interafce 
>>> to be used in User facing application, or shall we just use Inventory 
>>> based interface itself ?If inventory itself can be used, then let's 
>>> go ahead and add more fields to those if missing anything / correct 
>>> the same accordingly.
>>>
>>> James, Deepak, Patrick - your thoughts ?
>>
>> I guess there is a difference between FRU and inventory. If inventory 
>> interfaces could be used directly, why wouldn't the FruDevice or PLDM 
>> apps directly host inventory objects, why even use EM?
>>
> [Richard]: Inventory.Decorator is used in Redfish. i.e. Today Frudevice 
> & it's interface exposed is consumed by Entity-Manager alone & Entity 
> manager exposes all the needed configuration along with 
> Inventory.Decorator.Asset interface. (As you stated, inventory does more 
> than what FRU has to expose, but i am trying to see can we use 
> Inventory.Decorator itself to expose these needed FRU information?). 
> James F ? (Maintainer of both EM & bmcweb)
> 
> Current Behavior:
> 
> Say - Baseboard fru information is exposed by FruDevice, and Entity 
> Manager exposes the sensor or any configuration required for platform A 
> or Platform B (based on fru device data). Entity-manager also expose 
> Inventory interface which hides the manufacturer name exposed by FRU, 
> but exposes the it as Manufacturer in the Inventory interface and 
> Redfish uses this).
> 
> In case of exposing the PLDM FRU
> 
> Option 1:
> 
> If we need to rely on PLDM FRU, then Entity-Manager will probe the 
> FruDevice data exposed by PLDM FRU daemon, and expose the inventory 
> (both for sensor configuration and decorator Asset), we don't change any 
> upper layer application. We can follow the same design for the PLDM FRU 
> daemon.

Yes, the question remains as to what kind of D-Bus interface does the 
PLDM daemon use. Make it's own, similar to xyz.openbmc_project.FruDevice 
(which the FruDevice daemon makes), or use a PLDM specific one (for 
which I have a patch on Gerrit), or use a common interface that can be 
used by both FruDevice and PLDM.

> Note: Currently, it doesn't have a mechanism to expose all the FRU 
> devices as Inventory.Decorator interface automatically when there is no 
> json configuration for it in EM. This needs to be fixed (say for PLDM 
> based Add-on card).
> 
> Option 2:
> 
> Define new FRU interface (common for both PLDM, IPMI etc.), similar to 
> inventory and this itself will be used by both Entity-manager & user 
> level interface application (say Redfish).

I didn't intend this to be something that describes inventory, but 
rather a common (to PLDM, FruDevice, other apps) alternative to 
xyz.openbmc_project.FruDevice, which is what the FruDevice daemon hosts 
on D-Bus today. EM can continue to make objects that implement 
xyz.openbmc_project.Inventory.Foo interfaces, since they represent 
inventory information.


> (Problem with option 2 is it requires changes in Currenr behavior, and 
> redfish etc, may need to update it's code to use the common FRU 
> interface rather than inventory.Decorator).
> 
> Regards,
> 
> Richard
> 
>> I believe these apps (FruDevice, PLDM daemon) operate at a per FRU 
>> level, and rely on something like EM to make one or more inventory 
>> objects based on the FRU data. So that was my option 2, a generic FRU 
>> properties interface. I'm just not sure at the moment the 
>> impact/interest of doing something like that and then aligning 
>> FruDevice and EM to the same.
>>
>> Thanks,
>> Deepak
>>
>>>
>>> regards,
>>>
>>> Richard

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-27 15:46         ` Deepak Kodihalli
@ 2020-05-27 17:19           ` Thomaiyar, Richard Marian
  2020-05-28 12:09             ` Patrick Williams
  0 siblings, 1 reply; 27+ messages in thread
From: Thomaiyar, Richard Marian @ 2020-05-27 17:19 UTC (permalink / raw)
  To: Deepak Kodihalli, Brad Bishop, James Feist, Bhat, Sumanth,
	Patrick Williams
  Cc: openbmc


On 5/27/2020 9:16 PM, Deepak Kodihalli wrote:
> On 27/05/20 8:55 pm, Thomaiyar, Richard Marian wrote:
>>
>> On 5/27/2020 7:28 PM, Deepak Kodihalli wrote:
>>> On 27/05/20 7:20 pm, Thomaiyar, Richard Marian wrote:
>>>> I always view D-Bus interface as a specification / API which can 
>>>> work with different producers / consumers (correct me, if that's 
>>>> not what we intend with D-Bus interface here). Problem with Option 
>>>> 1 is, it will end up in having multiple producers exposing 
>>>> different interface, and thereby consumers(user interface facing 
>>>> applications) of D-Bus must be aware about all the D-Bus interfaces 
>>>> and always requires change.
>>>>
>>>> Consider, we want to expose ChassisType, then irrespective of PLDM 
>>>> FRU / IPMI FRU / Proprietary FRU, Consumer applications must read 
>>>> it in the same manner. Having different interface / property types, 
>>>> requires update in both the end. PLDM FRU / IPMI FRU can be in 
>>>> common form (except few nit's /OEM's). We need to make sure it is 
>>>> represented in that angle. As of today phosphor-dbus-interfaces 
>>>> doesn't have FRU interface, but it has inventory related interfaces 
>>>> (exposed by Entity-Manager), which is what Redfish uses (i.e. 
>>>> Indirectly the FruDevice exposed interface is hidden by 
>>>> Entity-manager, and inventory interface exposed by entity-manager 
>>>> is used).
>>>>
>>>> As of today, entity-manager doesn't add Inventory interface 
>>>> automatically for Add-on cards (which doesn't have any json 
>>>> configuration), but needs exposure (say PLDM based Add on card 
>>>> devices will be of this type), but shouldn't be hard to add it anyway.
>>>>
>>>> Now the question is do we want to expose FRU as a separate 
>>>> interafce to be used in User facing application, or shall we just 
>>>> use Inventory based interface itself ?If inventory itself can be 
>>>> used, then let's go ahead and add more fields to those if missing 
>>>> anything / correct the same accordingly.
>>>>
>>>> James, Deepak, Patrick - your thoughts ?
>>>
>>> I guess there is a difference between FRU and inventory. If 
>>> inventory interfaces could be used directly, why wouldn't the 
>>> FruDevice or PLDM apps directly host inventory objects, why even use 
>>> EM?
>>>
>> [Richard]: Inventory.Decorator is used in Redfish. i.e. Today 
>> Frudevice & it's interface exposed is consumed by Entity-Manager 
>> alone & Entity manager exposes all the needed configuration along 
>> with Inventory.Decorator.Asset interface. (As you stated, inventory 
>> does more than what FRU has to expose, but i am trying to see can we 
>> use Inventory.Decorator itself to expose these needed FRU 
>> information?). James F ? (Maintainer of both EM & bmcweb)
>>
>> Current Behavior:
>>
>> Say - Baseboard fru information is exposed by FruDevice, and Entity 
>> Manager exposes the sensor or any configuration required for platform 
>> A or Platform B (based on fru device data). Entity-manager also 
>> expose Inventory interface which hides the manufacturer name exposed 
>> by FRU, but exposes the it as Manufacturer in the Inventory interface 
>> and Redfish uses this).
>>
>> In case of exposing the PLDM FRU
>>
>> Option 1:
>>
>> If we need to rely on PLDM FRU, then Entity-Manager will probe the 
>> FruDevice data exposed by PLDM FRU daemon, and expose the inventory 
>> (both for sensor configuration and decorator Asset), we don't change 
>> any upper layer application. We can follow the same design for the 
>> PLDM FRU daemon.
>
> Yes, the question remains as to what kind of D-Bus interface does the 
> PLDM daemon use. Make it's own, similar to 
> xyz.openbmc_project.FruDevice (which the FruDevice daemon makes), or 
> use a PLDM specific one (for which I have a patch on Gerrit), or use a 
> common interface that can be used by both FruDevice and PLDM.

[Richard]: If we are choosing this option, then we can use the existing 
Frudevice interface and use the PRODUCT_XYZ which is currently exposed. 
Almost all the PLDM Fru content will match the IPMI FRU, except few like 
SKU, version, description, Engineering_change_level & Vendor IANA (which 
we can expose as new properties in the same interface) ??

i.e. PLDM PartNumber is nothing but PRODUCT_PART_NUMBER in IPMI Fru etc.

>
>> Note: Currently, it doesn't have a mechanism to expose all the FRU 
>> devices as Inventory.Decorator interface automatically when there is 
>> no json configuration for it in EM. This needs to be fixed (say for 
>> PLDM based Add-on card).
>>
>> Option 2:
>>
>> Define new FRU interface (common for both PLDM, IPMI etc.), similar 
>> to inventory and this itself will be used by both Entity-manager & 
>> user level interface application (say Redfish).
>
> I didn't intend this to be something that describes inventory, but 
> rather a common (to PLDM, FruDevice, other apps) alternative to 
> xyz.openbmc_project.FruDevice, which is what the FruDevice daemon 
> hosts on D-Bus today. EM can continue to make objects that implement 
> xyz.openbmc_project.Inventory.Foo interfaces, since they represent 
> inventory information.
>
[Richard]: Yes, there will be 2 daemons for FRU Devices, 1. 
xyz.openbmc_project.FruDevice and 2. xyz.openbmc_project.PLDM.FruDevice, 
both exposing xyz.openbmc_project.FruDevice which will be consumed by 
Entity-manager and accordingly expose the Inventory.Decorator.Asset for 
use in Redfish.

Regards,

Richard

>
>> (Problem with option 2 is it requires changes in Currenr behavior, 
>> and redfish etc, may need to update it's code to use the common FRU 
>> interface rather than inventory.Decorator).
>>
>> Regards,
>>
>> Richard
>>
>>> I believe these apps (FruDevice, PLDM daemon) operate at a per FRU 
>>> level, and rely on something like EM to make one or more inventory 
>>> objects based on the FRU data. So that was my option 2, a generic 
>>> FRU properties interface. I'm just not sure at the moment the 
>>> impact/interest of doing something like that and then aligning 
>>> FruDevice and EM to the same.
>>>
>>> Thanks,
>>> Deepak
>>>
>>>>
>>>> regards,
>>>>
>>>> Richard
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager)
  2020-05-26 12:56 ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Deepak Kodihalli
  2020-05-27 13:50   ` D-Bus interface to provide data to entity manager Thomaiyar, Richard Marian
@ 2020-05-28 12:03   ` Patrick Williams
  2020-05-28 12:24     ` D-Bus interface to provide data to entity manager Deepak Kodihalli
  2020-05-28 15:43     ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Brad Bishop
  1 sibling, 2 replies; 27+ messages in thread
From: Patrick Williams @ 2020-05-28 12:03 UTC (permalink / raw)
  To: Deepak Kodihalli
  Cc: Brad Bishop, James Feist, Thomaiyar, Richard Marian, Bhat,
	Sumanth, openbmc

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

On Tue, May 26, 2020 at 06:26:27PM +0530, Deepak Kodihalli wrote:
> On 19/05/20 9:10 am, Deepak Kodihalli wrote:
> To do a) above, there are 3 options:
> 
> 1) Propose a D-Bus interface in phosphor-dbus-interfaces. This was [2] 
> in my original email above. The concern raised by Patrick here is that 
> this interface is very specific to a protocol (PLDM in this case), 
> whereas the phosphor D-Bus interfaces are mostly abstract and protocol 
> agnostic.
> 
> In my opinion, PLDM is also a data model, so PLDM specific D-Bus 
> interfaces can enable two apps that are PLDM aware (for eg a PLDM 
> requester app talking to the PLDM daemon) to talk to each other. I do 
> see other protocol specific D-Bus interfaces as well (for eg related to 
> SMBIOS). So I don't really understand the concern. The protocol specific 
> interfaces do not preclude other abstract interfaces.

After thinking about it for a bit, I think this is my preference with
the design caveat that these are only consumed by processes which are
"FRU-to-Inventory" transformers.  I would suggest putting these
interfaces under the 'Inventory/' namespace somewhere to hopefully make
this clearer.

We have two implementations of these "FRU-to-Inventory" transformers: EM
and PIM.  Both of them have a form of dbus backdoor in order to get the
information they need to create the Inventory objects they host.  PIM uses
`Inventory/Manager`[1] and EM uses an undocumented `FruDevice` interface
between it and 'fru-device'.  Both of these implementations do
processing on the data they get (though, very minimal in the case of
PIM) and create `Inventory/Item`s as a result.

What I am worried about, and Richard seconded in his response, is the
details of PLDM (or any other protocol) starting to leak into other
processes.  We don't want people to notice that there is some piece of
information that isn't currently exposed via Inventory but happens to be
available in PLDM and start coding towards consuming it.  Hence, my
request that the design document the caveat I listed above.  We want to
limit the scope of applications that need to understand specific
protocols.

My suggestion would be to put these new proposed PLDM interfaces under
`Inventory/Source/PLDM`.  Anything under `Source` becomes one of these
"FRU-to-Inventory" transformational interfaces.

> 2) Propose a generic/abstract 'FRU properties' kind of D-Bus interface. 
> This is something that both the PLDM daemon and FRU device daemon could 
> use to host FRU information on D-Bus, and to provide the same as an 
> intermediate FRU format data to apps like EM. The suggestion on the docs 
> commit above [2] was to have an interface like {Enum[Protocol], 
> array[byte]}. I think instead this could be a dict[string, 
> variant[string, int64]], for a FRU property to value mapping.

I don't like arbitrary string-based properties like this.  They aren't
documented and effectively become a private API between the processes.
In order for someone to make a new implementation they have to reverse
engineer the code.

> Thanks,
> Deepak

1. https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Inventory/Manager.interface.yaml

-- 
Patrick Williams

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-27 17:19           ` Thomaiyar, Richard Marian
@ 2020-05-28 12:09             ` Patrick Williams
  0 siblings, 0 replies; 27+ messages in thread
From: Patrick Williams @ 2020-05-28 12:09 UTC (permalink / raw)
  To: Thomaiyar, Richard Marian
  Cc: Deepak Kodihalli, Brad Bishop, James Feist, Bhat, Sumanth, openbmc

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

I already responded with my overall design preference but had one point
in the chain I wanted to discuss a bit more.

On Wed, May 27, 2020 at 10:49:38PM +0530, Thomaiyar, Richard Marian wrote:
> 
> [Richard]: If we are choosing this option, then we can use the existing 
> Frudevice interface and use the PRODUCT_XYZ which is currently exposed. 
> Almost all the PLDM Fru content will match the IPMI FRU, except few like 
> SKU, version, description, Engineering_change_level & Vendor IANA (which 
> we can expose as new properties in the same interface) ??
> 
> i.e. PLDM PartNumber is nothing but PRODUCT_PART_NUMBER in IPMI Fru etc.

The current FruDevice interface is effectively a private dbus API
between EM and `fru-device` (which is also in the EM repo anyhow) and it
doesn't follow either our dbus practices nor widely accepted ones.  Here
are a few reasons:

    - FruDevice instances do not have the same properties.  Two
      different IPMI FRU types create a different set of properties, so
      you end up with dbus objects with a variety of interface layouts.

    - The properties exposed are not documented and differ stylistically
      from our existing, documented dbus objects.

Right now, if we need to implement another FruDevice provider, such as
will likely happen as Facebook implements our multi-host system, we have
to reverse engineer the code in `fru-device`.  Ideally this would be
refactored into a set of documented dbus interfaces under
`Inventory/Source/IPMI` (or some equally reasonable name) so others
could implement as needed.

-- 
Patrick Williams

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-28 12:03   ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Patrick Williams
@ 2020-05-28 12:24     ` Deepak Kodihalli
  2020-05-28 16:42       ` Thomaiyar, Richard Marian
  2020-05-28 15:43     ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Brad Bishop
  1 sibling, 1 reply; 27+ messages in thread
From: Deepak Kodihalli @ 2020-05-28 12:24 UTC (permalink / raw)
  To: Patrick Williams
  Cc: Brad Bishop, James Feist, Thomaiyar, Richard Marian, Bhat,
	Sumanth, openbmc

On 28/05/20 5:33 pm, Patrick Williams wrote:
> On Tue, May 26, 2020 at 06:26:27PM +0530, Deepak Kodihalli wrote:
>> On 19/05/20 9:10 am, Deepak Kodihalli wrote:
>> To do a) above, there are 3 options:
>>
>> 1) Propose a D-Bus interface in phosphor-dbus-interfaces. This was [2]
>> in my original email above. The concern raised by Patrick here is that
>> this interface is very specific to a protocol (PLDM in this case),
>> whereas the phosphor D-Bus interfaces are mostly abstract and protocol
>> agnostic.
>>
>> In my opinion, PLDM is also a data model, so PLDM specific D-Bus
>> interfaces can enable two apps that are PLDM aware (for eg a PLDM
>> requester app talking to the PLDM daemon) to talk to each other. I do
>> see other protocol specific D-Bus interfaces as well (for eg related to
>> SMBIOS). So I don't really understand the concern. The protocol specific
>> interfaces do not preclude other abstract interfaces.
> 
> After thinking about it for a bit, I think this is my preference with
> the design caveat that these are only consumed by processes which are
> "FRU-to-Inventory" transformers.  I would suggest putting these
> interfaces under the 'Inventory/' namespace somewhere to hopefully make
> this clearer.
> 
> We have two implementations of these "FRU-to-Inventory" transformers: EM
> and PIM.  Both of them have a form of dbus backdoor in order to get the
> information they need to create the Inventory objects they host.  PIM uses
> `Inventory/Manager`[1] and EM uses an undocumented `FruDevice` interface
> between it and 'fru-device'.  Both of these implementations do
> processing on the data they get (though, very minimal in the case of
> PIM) and create `Inventory/Item`s as a result.
> 
> What I am worried about, and Richard seconded in his response, is the
> details of PLDM (or any other protocol) starting to leak into other
> processes.  We don't want people to notice that there is some piece of
> information that isn't currently exposed via Inventory but happens to be
> available in PLDM and start coding towards consuming it.  Hence, my
> request that the design document the caveat I listed above.  We want to
> limit the scope of applications that need to understand specific
> protocols.
> 
> My suggestion would be to put these new proposed PLDM interfaces under
> `Inventory/Source/PLDM`.  Anything under `Source` becomes one of these
> "FRU-to-Inventory" transformational interfaces.


Makes sense to me. This makes the intent clearer. I've updated my commit 
on Gerrit. Please re-review.

Thanks,
Deepak

> 1. https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Inventory/Manager.interface.yaml

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager)
  2020-05-28 12:03   ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Patrick Williams
  2020-05-28 12:24     ` D-Bus interface to provide data to entity manager Deepak Kodihalli
@ 2020-05-28 15:43     ` Brad Bishop
  2020-05-28 18:21       ` D-Bus interface to provide data to entity manager James Feist
  1 sibling, 1 reply; 27+ messages in thread
From: Brad Bishop @ 2020-05-28 15:43 UTC (permalink / raw)
  To: Patrick Williams, Deepak Kodihalli
  Cc: Bhat, Sumanth, openbmc, Thomaiyar, Richard Marian, James Feist

On Thu, 2020-05-28 at 07:03 -0500, Patrick Williams wrote:
> 
> After thinking about it for a bit, I think this is my preference with
> the design caveat that these are only consumed by processes which are
> "FRU-to-Inventory" transformers.  I would suggest putting these
> interfaces under the 'Inventory/' namespace somewhere to hopefully
> make this clearer.

> My suggestion would be to put these new proposed PLDM interfaces under
> `Inventory/Source/PLDM`.  Anything under `Source` becomes one of these
> "FRU-to-Inventory" transformational interfaces.

FWIW I'm also happy with this direction.  It would be great to get a
quick "me too" from the author of fru-device and entity-manager to avoid
wasting time later 😉

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-28 12:24     ` D-Bus interface to provide data to entity manager Deepak Kodihalli
@ 2020-05-28 16:42       ` Thomaiyar, Richard Marian
  2020-05-28 18:05         ` Patrick Williams
  0 siblings, 1 reply; 27+ messages in thread
From: Thomaiyar, Richard Marian @ 2020-05-28 16:42 UTC (permalink / raw)
  To: Deepak Kodihalli, Patrick Williams
  Cc: Brad Bishop, James Feist, Bhat, Sumanth, openbmc


On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
> On 28/05/20 5:33 pm, Patrick Williams wrote:
>> On Tue, May 26, 2020 at 06:26:27PM +0530, Deepak Kodihalli wrote:
>>> On 19/05/20 9:10 am, Deepak Kodihalli wrote:
>>> To do a) above, there are 3 options:
>>>
>>> 1) Propose a D-Bus interface in phosphor-dbus-interfaces. This was [2]
>>> in my original email above. The concern raised by Patrick here is that
>>> this interface is very specific to a protocol (PLDM in this case),
>>> whereas the phosphor D-Bus interfaces are mostly abstract and protocol
>>> agnostic.
>>>
>>> In my opinion, PLDM is also a data model, so PLDM specific D-Bus
>>> interfaces can enable two apps that are PLDM aware (for eg a PLDM
>>> requester app talking to the PLDM daemon) to talk to each other. I do
>>> see other protocol specific D-Bus interfaces as well (for eg related to
>>> SMBIOS). So I don't really understand the concern. The protocol 
>>> specific
>>> interfaces do not preclude other abstract interfaces.
>>
>> After thinking about it for a bit, I think this is my preference with
>> the design caveat that these are only consumed by processes which are
>> "FRU-to-Inventory" transformers.  I would suggest putting these
>> interfaces under the 'Inventory/' namespace somewhere to hopefully make
>> this clearer.
>>
>> We have two implementations of these "FRU-to-Inventory" transformers: EM
>> and PIM.  Both of them have a form of dbus backdoor in order to get the
>> information they need to create the Inventory objects they host.  PIM 
>> uses
>> `Inventory/Manager`[1] and EM uses an undocumented `FruDevice` interface
>> between it and 'fru-device'.  Both of these implementations do
>> processing on the data they get (though, very minimal in the case of
>> PIM) and create `Inventory/Item`s as a result.
>>
>> What I am worried about, and Richard seconded in his response, is the
>> details of PLDM (or any other protocol) starting to leak into other
>> processes.  We don't want people to notice that there is some piece of
>> information that isn't currently exposed via Inventory but happens to be
>> available in PLDM and start coding towards consuming it.  Hence, my
>> request that the design document the caveat I listed above.  We want to
>> limit the scope of applications that need to understand specific
>> protocols.
>>
>> My suggestion would be to put these new proposed PLDM interfaces under
>> `Inventory/Source/PLDM`.  Anything under `Source` becomes one of these
>> "FRU-to-Inventory" transformational interfaces.
>
[Richard]:

Patrick / Deepak

Why do we need to have 2 different interfaces to represent the same 
information for FRU-to-inventory transformational (say ProductName). 
This will make inventory manager to be updated for every FRU producer?. 
Many of the properties are common, and we can form a common interface 
for that, and rest can be maintained in it's specific interface. I 
understand that current FRU to Entity-manager interface seems to be 
private, but we must make a common interface to represent like Product 
Name, PartNumer, Serial Number etc. (instead of maintaining it in 
different interface saying IPMI / PLDM Source, with different types). 
How about?

Inventory/Source/General/Fru  (Maintain common things here Product Name. 
This can be used by Inventory manager to advertise it (instead of 
searching it in multiple interfaces/properties))

Inventory/Source/PLDM   (Maintain PLDM specific stuff's here, say SKU, 
Engineering change level, IANA etc).

Inventory/Source/IPMI (Maintain IPMI FRU specific here)

Regards,

Richard

> Makes sense to me. This makes the intent clearer. I've updated my 
> commit on Gerrit. Please re-review.
>
> Thanks,
> Deepak
>
>> 1. 
>> https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Inventory/Manager.interface.yaml
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-28 16:42       ` Thomaiyar, Richard Marian
@ 2020-05-28 18:05         ` Patrick Williams
  2020-05-28 18:31           ` James Feist
  2020-05-29  5:09           ` Deepak Kodihalli
  0 siblings, 2 replies; 27+ messages in thread
From: Patrick Williams @ 2020-05-28 18:05 UTC (permalink / raw)
  To: Thomaiyar, Richard Marian
  Cc: Deepak Kodihalli, Brad Bishop, James Feist, Bhat, Sumanth, openbmc

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

On Thu, May 28, 2020 at 10:12:19PM +0530, Thomaiyar, Richard Marian wrote:
> 
> On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
> > On 28/05/20 5:33 pm, Patrick Williams wrote:

> Why do we need to have 2 different interfaces to represent the same
> information for FRU-to-inventory transformational (say ProductName). This
> will make inventory manager to be updated for every FRU producer?. Many of
> the properties are common, and we can form a common interface for that, and
> rest can be maintained in it's specific interface. I understand that current
> FRU to Entity-manager interface seems to be private, but we must make a
> common interface to represent like Product Name, PartNumer, Serial Number
> etc. (instead of maintaining it in different interface saying IPMI / PLDM
> Source, with different types). How about?

Yes, I am in favor of common interfaces for this where ever possible.

Is there someone that knows the existing FruDevice implementation well
enough that can be included in this work to propose common interfaces
where it is appropriate?

> Inventory/Source/General/Fru  (Maintain common things here Product Name.
> This can be used by Inventory manager to advertise it (instead of searching
> it in multiple interfaces/properties))

Minor tweak here of 'Source/Common'?  When we have an existing Inventory
interface for this information should we mimic what is already in
Inventory?  At some point are we trying to be too common that we're
effectively reimplementing Inventory instances under a different name?

-- 
Patrick Williams

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-28 15:43     ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Brad Bishop
@ 2020-05-28 18:21       ` James Feist
  0 siblings, 0 replies; 27+ messages in thread
From: James Feist @ 2020-05-28 18:21 UTC (permalink / raw)
  To: Brad Bishop, Patrick Williams, Deepak Kodihalli
  Cc: Bhat, Sumanth, openbmc, Thomaiyar, Richard Marian

On 5/28/2020 8:43 AM, Brad Bishop wrote:
> On Thu, 2020-05-28 at 07:03 -0500, Patrick Williams wrote:
>>
>> After thinking about it for a bit, I think this is my preference with
>> the design caveat that these are only consumed by processes which are
>> "FRU-to-Inventory" transformers.  I would suggest putting these
>> interfaces under the 'Inventory/' namespace somewhere to hopefully
>> make this clearer.
> 
>> My suggestion would be to put these new proposed PLDM interfaces under
>> `Inventory/Source/PLDM`.  Anything under `Source` becomes one of these
>> "FRU-to-Inventory" transformational interfaces.
> 
> FWIW I'm also happy with this direction.  It would be great to get a
> quick "me too" from the author of fru-device and entity-manager to avoid
> wasting time later 😉
> 

I don't have a strong opinion on the naming of the interface. That would 
be OK by me.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-28 18:05         ` Patrick Williams
@ 2020-05-28 18:31           ` James Feist
  2020-05-29  5:11             ` Deepak Kodihalli
  2020-05-29  5:09           ` Deepak Kodihalli
  1 sibling, 1 reply; 27+ messages in thread
From: James Feist @ 2020-05-28 18:31 UTC (permalink / raw)
  To: Patrick Williams, Thomaiyar, Richard Marian
  Cc: Bhat, Sumanth, Brad Bishop, openbmc

On 5/28/2020 11:05 AM, Patrick Williams wrote:
> On Thu, May 28, 2020 at 10:12:19PM +0530, Thomaiyar, Richard Marian wrote:
>>
>> On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
>>> On 28/05/20 5:33 pm, Patrick Williams wrote:
> 
>> Why do we need to have 2 different interfaces to represent the same
>> information for FRU-to-inventory transformational (say ProductName). This
>> will make inventory manager to be updated for every FRU producer?. Many of
>> the properties are common, and we can form a common interface for that, and
>> rest can be maintained in it's specific interface. I understand that current
>> FRU to Entity-manager interface seems to be private, but we must make a
>> common interface to represent like Product Name, PartNumer, Serial Number
>> etc. (instead of maintaining it in different interface saying IPMI / PLDM
>> Source, with different types). How about?
> 
> Yes, I am in favor of common interfaces for this where ever possible.
> 
> Is there someone that knows the existing FruDevice implementation well
> enough that can be included in this work to propose common interfaces
> where it is appropriate?

Yes, I wrote it. I can review the changes, however if you read the ipmi 
fru spec, which is what fru device is based of, there are many optional 
fields based on the type of the fru. You'd probably at a minimum need an 
interface per region supported as phosphor-dbus-interfaces does not 
allow optional fields. I'm not sure how you'd deal with custom fields 
either. You'd probably need everything as a superset of the fru spec, 
and have blanks where its not populated.

https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/ipmi-platform-mgt-fru-info-storage-def-v1-0-rev-1-3-spec-update.pdf

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-28 18:05         ` Patrick Williams
  2020-05-28 18:31           ` James Feist
@ 2020-05-29  5:09           ` Deepak Kodihalli
  2020-05-29  7:17             ` Thomaiyar, Richard Marian
  1 sibling, 1 reply; 27+ messages in thread
From: Deepak Kodihalli @ 2020-05-29  5:09 UTC (permalink / raw)
  To: Patrick Williams, Thomaiyar, Richard Marian
  Cc: Brad Bishop, James Feist, Bhat, Sumanth, openbmc

On 28/05/20 11:35 pm, Patrick Williams wrote:
> On Thu, May 28, 2020 at 10:12:19PM +0530, Thomaiyar, Richard Marian wrote:
>>
>> On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
>>> On 28/05/20 5:33 pm, Patrick Williams wrote:
> 
>> Why do we need to have 2 different interfaces to represent the same
>> information for FRU-to-inventory transformational (say ProductName). This
>> will make inventory manager to be updated for every FRU producer?. Many of
>> the properties are common, and we can form a common interface for that, and
>> rest can be maintained in it's specific interface. I understand that current
>> FRU to Entity-manager interface seems to be private, but we must make a
>> common interface to represent like Product Name, PartNumer, Serial Number
>> etc. (instead of maintaining it in different interface saying IPMI / PLDM
>> Source, with different types). How about?

Richard, I have concerns with this approach. Like I mentioned in one my 
earlier emails, and Patrick also alludes to below, if you try to make 
this common (event if it's for a subset of the properties) then you 
basically arrive at the existing phosphor Inventory interfaces (eg 
Inventory.Decorator.Asset).

My question in my earlier mail was, if you do such a thing, then why do 
you even need inventory producers? FruDevice and PLDM could have hosted 
inventory on their own. If they still rely on the inventory producers 
(EM and PIM) with this "common interface" approach, then it's basically 
re-implementation/duplications of the (Inventory.Decorator.Asset like) 
interface by two processes.

The idea is for apps like FruDevice and PLDM, which are aware of a 
specific FRU format, to host data in *that* format, to be consumed 
*solely* by inventory producers (like EM and PIM).

Also note that (as James pointed out in his email), the IPMI FRU format 
distinguishes Board/Chassis/Product areas. PLDM FRU format does not. So 
there are differences. If a new FRU format is introduced, then yes we 
would expect a new interface to show up under Inventory/Source/<FruFormat>


> Yes, I am in favor of common interfaces for this where ever possible.
> 
> Is there someone that knows the existing FruDevice implementation well
> enough that can be included in this work to propose common interfaces
> where it is appropriate?
> 
>> Inventory/Source/General/Fru  (Maintain common things here Product Name.
>> This can be used by Inventory manager to advertise it (instead of searching
>> it in multiple interfaces/properties))
> 
> Minor tweak here of 'Source/Common'?  When we have an existing Inventory
> interface for this information should we mimic what is already in
> Inventory?  At some point are we trying to be too common that we're
> effectively reimplementing Inventory instances under a different name?
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-28 18:31           ` James Feist
@ 2020-05-29  5:11             ` Deepak Kodihalli
  0 siblings, 0 replies; 27+ messages in thread
From: Deepak Kodihalli @ 2020-05-29  5:11 UTC (permalink / raw)
  To: James Feist, Patrick Williams, Thomaiyar, Richard Marian
  Cc: Bhat, Sumanth, openbmc, Brad Bishop

On 29/05/20 12:01 am, James Feist wrote:
> On 5/28/2020 11:05 AM, Patrick Williams wrote:
>> On Thu, May 28, 2020 at 10:12:19PM +0530, Thomaiyar, Richard Marian 
>> wrote:
>>>
>>> On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
>>>> On 28/05/20 5:33 pm, Patrick Williams wrote:
>>
>>> Why do we need to have 2 different interfaces to represent the same
>>> information for FRU-to-inventory transformational (say ProductName). 
>>> This
>>> will make inventory manager to be updated for every FRU producer?. 
>>> Many of
>>> the properties are common, and we can form a common interface for 
>>> that, and
>>> rest can be maintained in it's specific interface. I understand that 
>>> current
>>> FRU to Entity-manager interface seems to be private, but we must make a
>>> common interface to represent like Product Name, PartNumer, Serial 
>>> Number
>>> etc. (instead of maintaining it in different interface saying IPMI / 
>>> PLDM
>>> Source, with different types). How about?
>>
>> Yes, I am in favor of common interfaces for this where ever possible.
>>
>> Is there someone that knows the existing FruDevice implementation well
>> enough that can be included in this work to propose common interfaces
>> where it is appropriate?
> 
> Yes, I wrote it. I can review the changes, however if you read the ipmi 
> fru spec, which is what fru device is based of, there are many optional 
> fields based on the type of the fru. You'd probably at a minimum need an 
> interface per region supported as phosphor-dbus-interfaces does not 
> allow optional fields. I'm not sure how you'd deal with custom fields 
> either. You'd probably need everything as a superset of the fru spec, 
> and have blanks where its not populated.


I don't think it is worth trying to make a FRU device/properties 
interface that is common to PLDM and IPMI. The two FRU formats are 
different (pls see my previous email about the same).

Thanks,
Deepak


> https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/ipmi-platform-mgt-fru-info-storage-def-v1-0-rev-1-3-spec-update.pdf 
> 
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-29  5:09           ` Deepak Kodihalli
@ 2020-05-29  7:17             ` Thomaiyar, Richard Marian
  2020-05-29  7:31               ` Deepak Kodihalli
  0 siblings, 1 reply; 27+ messages in thread
From: Thomaiyar, Richard Marian @ 2020-05-29  7:17 UTC (permalink / raw)
  To: Deepak Kodihalli, Patrick Williams
  Cc: Brad Bishop, James Feist, Bhat, Sumanth, openbmc


On 5/29/2020 10:39 AM, Deepak Kodihalli wrote:
> On 28/05/20 11:35 pm, Patrick Williams wrote:
>> On Thu, May 28, 2020 at 10:12:19PM +0530, Thomaiyar, Richard Marian 
>> wrote:
>>>
>>> On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
>>>> On 28/05/20 5:33 pm, Patrick Williams wrote:
>>
>>> Why do we need to have 2 different interfaces to represent the same
>>> information for FRU-to-inventory transformational (say ProductName). 
>>> This
>>> will make inventory manager to be updated for every FRU producer?. 
>>> Many of
>>> the properties are common, and we can form a common interface for 
>>> that, and
>>> rest can be maintained in it's specific interface. I understand that 
>>> current
>>> FRU to Entity-manager interface seems to be private, but we must make a
>>> common interface to represent like Product Name, PartNumer, Serial 
>>> Number
>>> etc. (instead of maintaining it in different interface saying IPMI / 
>>> PLDM
>>> Source, with different types). How about?
>
> Richard, I have concerns with this approach. Like I mentioned in one 
> my earlier emails, and Patrick also alludes to below, if you try to 
> make this common (event if it's for a subset of the properties) then 
> you basically arrive at the existing phosphor Inventory interfaces (eg 
> Inventory.Decorator.Asset).
>
> My question in my earlier mail was, if you do such a thing, then why 
> do you even need inventory producers? FruDevice and PLDM could have 
> hosted inventory on their own. If they still rely on the inventory 
> producers (EM and PIM) with this "common interface" approach, then 
> it's basically re-implementation/duplications of the 
> (Inventory.Decorator.Asset like) interface by two processes.

[Richard]: Basically FRU information (either IPMI/PLDM) is needed for 
the inventory producers to expose configuration, which FRU will not 
have. Say, based on FRU Product name, either we will expose 4 temp 
sensor / 2 (Now along with this one, we need to inform the product name 
through Inventory.Decorator.Asset). Now from Redfish point of it, 
Inventory.Decorator is what it uses. This is what i was asking with 2 
options in earlier mail (whether to change or stick with it (recommended)).

>
> The idea is for apps like FruDevice and PLDM, which are aware of a 
> specific FRU format, to host data in *that* format, to be consumed 
> *solely* by inventory producers (like EM and PIM).
>
[Richard]: Yes, but it doesn't need to expose those in that format? Say 
Manufacturer Name, it doesn't mater whether it is coming from PLDM FRU / 
IPMI FRU. Say we have a special handling for a particular manufacture / 
product, then irrespective of inventory producers both can handle the same.

If we have 2 different interface, then inventory producer may need to be 
aware about both and probe it accordingly. FRU producers code must be 
written in such a way that for these common properties it does the 
necessary conversion (Say make manufacturer as string, irrespective of 
any format it read through). Note: Specific stuff, we need to create a 
separate interface (as phosphor-dbus-interface at present doesn't 
support dynamic property addition/deletion). (Tomorrow, if we have any 
other proprietary way of producing FRU data, we can still work with this 
approach, with less or no change for other layers).

> Also note that (as James pointed out in his email), the IPMI FRU 
> format distinguishes Board/Chassis/Product areas. PLDM FRU format does 
> not. So there are differences. If a new FRU format is introduced, then 
> yes we would expect a new interface to show up under 
> Inventory/Source/<FruFormat>
[Richard]: Fru producers should do this conversion. Say Chassis Type 
(Irrespective of what area it comes from it is same). PLDM FRU mostly 
represents the product as a whole, but technically we can point it to 
all the needed using the Fru Record set to the Entity type mapping in 
the PDR record. Accordingly it needs to be exposed.
>
>
>> Yes, I am in favor of common interfaces for this where ever possible.
>>
>> Is there someone that knows the existing FruDevice implementation well
>> enough that can be included in this work to propose common interfaces
>> where it is appropriate?
>>
>>> Inventory/Source/General/Fru  (Maintain common things here Product 
>>> Name.
>>> This can be used by Inventory manager to advertise it (instead of 
>>> searching
>>> it in multiple interfaces/properties))
>>
>> Minor tweak here of 'Source/Common'?  When we have an existing Inventory
>> interface for this information should we mimic what is already in
>> Inventory?  At some point are we trying to be too common that we're
>> effectively reimplementing Inventory instances under a different name?
>>
[Richard]: Yes, currently, FRU to inventory and inventory to upper layer 
is what used. If we want to change it, we need to go with differnt 
option of using FRU to upper layer, but many of existing code will 
require change.

Regards,

Richard

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-29  7:17             ` Thomaiyar, Richard Marian
@ 2020-05-29  7:31               ` Deepak Kodihalli
  2020-05-29  9:03                 ` Thomaiyar, Richard Marian
  0 siblings, 1 reply; 27+ messages in thread
From: Deepak Kodihalli @ 2020-05-29  7:31 UTC (permalink / raw)
  To: Thomaiyar, Richard Marian, Patrick Williams
  Cc: Brad Bishop, James Feist, Bhat, Sumanth, openbmc

On 29/05/20 12:47 pm, Thomaiyar, Richard Marian wrote:
> 
> On 5/29/2020 10:39 AM, Deepak Kodihalli wrote:
>> On 28/05/20 11:35 pm, Patrick Williams wrote:
>>> On Thu, May 28, 2020 at 10:12:19PM +0530, Thomaiyar, Richard Marian 
>>> wrote:
>>>>
>>>> On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
>>>>> On 28/05/20 5:33 pm, Patrick Williams wrote:
>>>
>>>> Why do we need to have 2 different interfaces to represent the same
>>>> information for FRU-to-inventory transformational (say ProductName). 
>>>> This
>>>> will make inventory manager to be updated for every FRU producer?. 
>>>> Many of
>>>> the properties are common, and we can form a common interface for 
>>>> that, and
>>>> rest can be maintained in it's specific interface. I understand that 
>>>> current
>>>> FRU to Entity-manager interface seems to be private, but we must make a
>>>> common interface to represent like Product Name, PartNumer, Serial 
>>>> Number
>>>> etc. (instead of maintaining it in different interface saying IPMI / 
>>>> PLDM
>>>> Source, with different types). How about?
>>
>> Richard, I have concerns with this approach. Like I mentioned in one 
>> my earlier emails, and Patrick also alludes to below, if you try to 
>> make this common (event if it's for a subset of the properties) then 
>> you basically arrive at the existing phosphor Inventory interfaces (eg 
>> Inventory.Decorator.Asset).
>>
>> My question in my earlier mail was, if you do such a thing, then why 
>> do you even need inventory producers? FruDevice and PLDM could have 
>> hosted inventory on their own. If they still rely on the inventory 
>> producers (EM and PIM) with this "common interface" approach, then 
>> it's basically re-implementation/duplications of the 
>> (Inventory.Decorator.Asset like) interface by two processes.

Richard, what is your thought on the re-implementation/duplication 
concern above? I'm not sure if you answered that and I missed.

> [Richard]: Basically FRU information (either IPMI/PLDM) is needed for 
> the inventory producers to expose configuration, which FRU will not 
> have. Say, based on FRU Product name, either we will expose 4 temp 
> sensor / 2 (Now along with this one, we need to inform the product name 
> through Inventory.Decorator.Asset). Now from Redfish point of it, 
> Inventory.Decorator is what it uses. This is what i was asking with 2 
> options in earlier mail (whether to change or stick with it (recommended)).

>>
>> The idea is for apps like FruDevice and PLDM, which are aware of a 
>> specific FRU format, to host data in *that* format, to be consumed 
>> *solely* by inventory producers (like EM and PIM).
>>
> [Richard]: Yes, but it doesn't need to expose those in that format?

Why not?

> Say Manufacturer Name, it doesn't mater whether it is coming from PLDM FRU / 
> IPMI FRU. Say we have a special handling for a particular manufacture / 
> product, then irrespective of inventory producers both can handle the same.

This is what the Inventory.Decorator.Asset interface is for.

> If we have 2 different interface, then inventory producer may need to be 
> aware about both and probe it accordingly.

No, the "FRU" properties producer needs to be concerned only about the 
format it understands.

> FRU producers code must be 
> written in such a way that for these common properties it does the 
> necessary conversion (Say make manufacturer as string, irrespective of 
> any format it read through). Note: Specific stuff, we need to create a 
> separate interface (as phosphor-dbus-interface at present doesn't 
> support dynamic property addition/deletion). (Tomorrow, if we have any 
> other proprietary way of producing FRU data, we can still work with this 
> approach, with less or no change for other layers).
> 
>> Also note that (as James pointed out in his email), the IPMI FRU 
>> format distinguishes Board/Chassis/Product areas. PLDM FRU format does 
>> not. So there are differences. If a new FRU format is introduced, then 
>> yes we would expect a new interface to show up under 
>> Inventory/Source/<FruFormat>
> [Richard]: Fru producers should do this conversion.

I'm of the opinion that the inventory producer (like EM and PIM) should 
perform this conversion. Consider 
https://github.com/openbmc/entity-manager/blob/master/configurations/Intel%20Front%20Panel.json#L55 
for example. I don't think it's up to the FruDevice/PLDM kind of apps to 
decide that this is actually a Panel. You can design it that way, but 
like I said above that means the config knowledge moves into these apps, 
which I don't think we should head towards, since then every FRU 
producer app needs to do this. This is why we have apps like EM.

> Say Chassis Type 
> (Irrespective of what area it comes from it is same). PLDM FRU mostly 
> represents the product as a whole, but technically we can point it to 
> all the needed using the Fru Record set to the Entity type mapping in 
> the PDR record. Accordingly it needs to be exposed.
>>
>>
>>> Yes, I am in favor of common interfaces for this where ever possible.
>>>
>>> Is there someone that knows the existing FruDevice implementation well
>>> enough that can be included in this work to propose common interfaces
>>> where it is appropriate?
>>>
>>>> Inventory/Source/General/Fru  (Maintain common things here Product 
>>>> Name.
>>>> This can be used by Inventory manager to advertise it (instead of 
>>>> searching
>>>> it in multiple interfaces/properties))
>>>
>>> Minor tweak here of 'Source/Common'?  When we have an existing Inventory
>>> interface for this information should we mimic what is already in
>>> Inventory?  At some point are we trying to be too common that we're
>>> effectively reimplementing Inventory instances under a different name?
>>>
> [Richard]: Yes, currently, FRU to inventory and inventory to upper layer 
> is what used. If we want to change it, we need to go with differnt 
> option of using FRU to upper layer, but many of existing code will 
> require change.


> Regards,
> 
> Richard
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-29  7:31               ` Deepak Kodihalli
@ 2020-05-29  9:03                 ` Thomaiyar, Richard Marian
  2020-05-29 10:19                   ` Deepak Kodihalli
  0 siblings, 1 reply; 27+ messages in thread
From: Thomaiyar, Richard Marian @ 2020-05-29  9:03 UTC (permalink / raw)
  To: Deepak Kodihalli, Patrick Williams
  Cc: Bhat, Sumanth, openbmc, Brad Bishop, James Feist


On 5/29/2020 1:01 PM, Deepak Kodihalli wrote:
> On 29/05/20 12:47 pm, Thomaiyar, Richard Marian wrote:
>>
>> On 5/29/2020 10:39 AM, Deepak Kodihalli wrote:
>>> On 28/05/20 11:35 pm, Patrick Williams wrote:
>>>> On Thu, May 28, 2020 at 10:12:19PM +0530, Thomaiyar, Richard Marian 
>>>> wrote:
>>>>>
>>>>> On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
>>>>>> On 28/05/20 5:33 pm, Patrick Williams wrote:
>>>>
>>>>> Why do we need to have 2 different interfaces to represent the same
>>>>> information for FRU-to-inventory transformational (say 
>>>>> ProductName). This
>>>>> will make inventory manager to be updated for every FRU producer?. 
>>>>> Many of
>>>>> the properties are common, and we can form a common interface for 
>>>>> that, and
>>>>> rest can be maintained in it's specific interface. I understand 
>>>>> that current
>>>>> FRU to Entity-manager interface seems to be private, but we must 
>>>>> make a
>>>>> common interface to represent like Product Name, PartNumer, Serial 
>>>>> Number
>>>>> etc. (instead of maintaining it in different interface saying IPMI 
>>>>> / PLDM
>>>>> Source, with different types). How about?
>>>
>>> Richard, I have concerns with this approach. Like I mentioned in one 
>>> my earlier emails, and Patrick also alludes to below, if you try to 
>>> make this common (event if it's for a subset of the properties) then 
>>> you basically arrive at the existing phosphor Inventory interfaces 
>>> (eg Inventory.Decorator.Asset).
>>>
>>> My question in my earlier mail was, if you do such a thing, then why 
>>> do you even need inventory producers? FruDevice and PLDM could have 
>>> hosted inventory on their own. If they still rely on the inventory 
>>> producers (EM and PIM) with this "common interface" approach, then 
>>> it's basically re-implementation/duplications of the 
>>> (Inventory.Decorator.Asset like) interface by two processes.
>
> Richard, what is your thought on the re-implementation/duplication 
> concern above? I'm not sure if you answered that and I missed.
[Richard]: FRU Consumers must be aware about each and every Format 
specifically, even though it conveys same meaning.
>
>> [Richard]: Basically FRU information (either IPMI/PLDM) is needed for 
>> the inventory producers to expose configuration, which FRU will not 
>> have. Say, based on FRU Product name, either we will expose 4 temp 
>> sensor / 2 (Now along with this one, we need to inform the product 
>> name through Inventory.Decorator.Asset). Now from Redfish point of 
>> it, Inventory.Decorator is what it uses. This is what i was asking 
>> with 2 options in earlier mail (whether to change or stick with it 
>> (recommended)).
>
>>>
>>> The idea is for apps like FruDevice and PLDM, which are aware of a 
>>> specific FRU format, to host data in *that* format, to be consumed 
>>> *solely* by inventory producers (like EM and PIM).
>>>
>> [Richard]: Yes, but it doesn't need to expose those in that format?
>
> Why not?
[Richard]: What's the advantage in keeping it in that format itself? 
This is used only by EM / PIM, and not by redfish directly right? Where 
the intelligence must reside in the producer or consumer (With producer, 
consumers can be in common form)
>
>> Say Manufacturer Name, it doesn't mater whether it is coming from 
>> PLDM FRU / IPMI FRU. Say we have a special handling for a particular 
>> manufacture / product, then irrespective of inventory producers both 
>> can handle the same.
>
> This is what the Inventory.Decorator.Asset interface is for.
[Richard]: Yes, That is exposed by EM / PIM in our case. Why EM / PIM 
must rely on 2 different stuff, for common things is the question here.
>
>> If we have 2 different interface, then inventory producer may need to 
>> be aware about both and probe it accordingly.
>
> No, the "FRU" properties producer needs to be concerned only about the 
> format it understands.

[Richard]: FRU property producer must know the format and produce the 
interface with data (in common form as much as possible). E.g. IPMI FRU 
Producer (say xyz.openbmc_project.FruDevice service) will read device A 
FRU, and expose the Manufacturer name (It can read the EEPROM content 
and decode it as per the IPMI FRU format, but the data it produces is 
Manufacturer name). Simiarly PLDM FRU Producer (say 
xyz.openbmc_project.PLDM.FruDevice service) will read the data using 
PLDM FRU commands, and expose the Manufacturer name. Now why this 2 
service need to have 2 different interface(one from 
Inventory.Source.PLDM & another from Inventory.Source.IPMI, to expose 
the Manufacturer name. ? Why Entity manager / PIM need to read the same 
information from 2 different interface and expose it in 
Inventory.Decorator.Asset. (It can do it with same interface). What 
Entity manager / PIM needs to do is using Object Mapper query all the 
FruDevices (IPMI / PLDM FRU), and accordingly expose the Inventory.

>> FRU producers code must be written in such a way that for these 
>> common properties it does the necessary conversion (Say make 
>> manufacturer as string, irrespective of any format it read through). 
>> Note: Specific stuff, we need to create a separate interface (as 
>> phosphor-dbus-interface at present doesn't support dynamic property 
>> addition/deletion). (Tomorrow, if we have any other proprietary way 
>> of producing FRU data, we can still work with this approach, with 
>> less or no change for other layers).
>>
>>> Also note that (as James pointed out in his email), the IPMI FRU 
>>> format distinguishes Board/Chassis/Product areas. PLDM FRU format 
>>> does not. So there are differences. If a new FRU format is 
>>> introduced, then yes we would expect a new interface to show up 
>>> under Inventory/Source/<FruFormat>
>> [Richard]: Fru producers should do this conversion.
>
> I'm of the opinion that the inventory producer (like EM and PIM) 
> should perform this conversion. Consider 
> https://github.com/openbmc/entity-manager/blob/master/configurations/Intel%20Front%20Panel.json#L55 
> for example. I don't think it's up to the FruDevice/PLDM kind of apps 
> to decide that this is actually a Panel. You can design it that way, 
> but like I said above that means the config knowledge moves into these 
> apps, which I don't think we should head towards, since then every FRU 
> producer app needs to do this. This is why we have apps like EM.
[Richard]: Exactly. What we need to make sure is create abstraction 
between Entity manager and FRU Producers as much as possible. FRU 
Producer responsibility is to read the FRU in decode the FRU data as per 
the spec and expose it in common form which Entity-manager / PIM will 
rely on.
>
>> Say Chassis Type (Irrespective of what area it comes from it is 
>> same). PLDM FRU mostly represents the product as a whole, but 
>> technically we can point it to all the needed using the Fru Record 
>> set to the Entity type mapping in the PDR record. Accordingly it 
>> needs to be exposed.
>>>
>>>
>>>> Yes, I am in favor of common interfaces for this where ever possible.
>>>>
>>>> Is there someone that knows the existing FruDevice implementation well
>>>> enough that can be included in this work to propose common interfaces
>>>> where it is appropriate?
>>>>
>>>>> Inventory/Source/General/Fru (Maintain common things here Product 
>>>>> Name.
>>>>> This can be used by Inventory manager to advertise it (instead of 
>>>>> searching
>>>>> it in multiple interfaces/properties))
>>>>
>>>> Minor tweak here of 'Source/Common'?  When we have an existing 
>>>> Inventory
>>>> interface for this information should we mimic what is already in
>>>> Inventory?  At some point are we trying to be too common that we're
>>>> effectively reimplementing Inventory instances under a different name?
>>>>
>> [Richard]: Yes, currently, FRU to inventory and inventory to upper 
>> layer is what used. If we want to change it, we need to go with 
>> differnt option of using FRU to upper layer, but many of existing 
>> code will require change.
>
>
>> Regards,
>>
>> Richard
>>
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-29  9:03                 ` Thomaiyar, Richard Marian
@ 2020-05-29 10:19                   ` Deepak Kodihalli
  2020-05-29 10:30                     ` Deepak Kodihalli
  0 siblings, 1 reply; 27+ messages in thread
From: Deepak Kodihalli @ 2020-05-29 10:19 UTC (permalink / raw)
  To: Thomaiyar, Richard Marian, Patrick Williams
  Cc: Bhat, Sumanth, openbmc, Brad Bishop, James Feist

On 29/05/20 2:33 pm, Thomaiyar, Richard Marian wrote:
> 
> On 5/29/2020 1:01 PM, Deepak Kodihalli wrote:
>> On 29/05/20 12:47 pm, Thomaiyar, Richard Marian wrote:
>>>
>>> On 5/29/2020 10:39 AM, Deepak Kodihalli wrote:
>>>> On 28/05/20 11:35 pm, Patrick Williams wrote:
>>>>> On Thu, May 28, 2020 at 10:12:19PM +0530, Thomaiyar, Richard Marian 
>>>>> wrote:
>>>>>>
>>>>>> On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
>>>>>>> On 28/05/20 5:33 pm, Patrick Williams wrote:
>>>>>
>>>>>> Why do we need to have 2 different interfaces to represent the same
>>>>>> information for FRU-to-inventory transformational (say 
>>>>>> ProductName). This
>>>>>> will make inventory manager to be updated for every FRU producer?. 
>>>>>> Many of
>>>>>> the properties are common, and we can form a common interface for 
>>>>>> that, and
>>>>>> rest can be maintained in it's specific interface. I understand 
>>>>>> that current
>>>>>> FRU to Entity-manager interface seems to be private, but we must 
>>>>>> make a
>>>>>> common interface to represent like Product Name, PartNumer, Serial 
>>>>>> Number
>>>>>> etc. (instead of maintaining it in different interface saying IPMI 
>>>>>> / PLDM
>>>>>> Source, with different types). How about?
>>>>
>>>> Richard, I have concerns with this approach. Like I mentioned in one 
>>>> my earlier emails, and Patrick also alludes to below, if you try to 
>>>> make this common (event if it's for a subset of the properties) then 
>>>> you basically arrive at the existing phosphor Inventory interfaces 
>>>> (eg Inventory.Decorator.Asset).
>>>>
>>>> My question in my earlier mail was, if you do such a thing, then why 
>>>> do you even need inventory producers? FruDevice and PLDM could have 
>>>> hosted inventory on their own. If they still rely on the inventory 
>>>> producers (EM and PIM) with this "common interface" approach, then 
>>>> it's basically re-implementation/duplications of the 
>>>> (Inventory.Decorator.Asset like) interface by two processes.
>>
>> Richard, what is your thought on the re-implementation/duplication 
>> concern above? I'm not sure if you answered that and I missed.
> [Richard]: FRU Consumers must be aware about each and every Format 
> specifically, even though it conveys same meaning.

I agree with that, but my question was about FRU producers.


>>> [Richard]: Basically FRU information (either IPMI/PLDM) is needed for 
>>> the inventory producers to expose configuration, which FRU will not 
>>> have. Say, based on FRU Product name, either we will expose 4 temp 
>>> sensor / 2 (Now along with this one, we need to inform the product 
>>> name through Inventory.Decorator.Asset). Now from Redfish point of 
>>> it, Inventory.Decorator is what it uses. This is what i was asking 
>>> with 2 options in earlier mail (whether to change or stick with it 
>>> (recommended)).
>>
>>>>
>>>> The idea is for apps like FruDevice and PLDM, which are aware of a 
>>>> specific FRU format, to host data in *that* format, to be consumed 
>>>> *solely* by inventory producers (like EM and PIM).
>>>>
>>> [Richard]: Yes, but it doesn't need to expose those in that format?
>>
>> Why not?
> [Richard]: What's the advantage in keeping it in that format itself?

The advantage I see is basically what you said on the next line.

> This is used only by EM / PIM, and not by redfish directly right? Where 
> the intelligence must reside in the producer or consumer (With producer, 
> consumers can be in common form)

>>> Say Manufacturer Name, it doesn't mater whether it is coming from 
>>> PLDM FRU / IPMI FRU. Say we have a special handling for a particular 
>>> manufacture / product, then irrespective of inventory producers both 
>>> can handle the same.
>>
>> This is what the Inventory.Decorator.Asset interface is for.
> [Richard]: Yes, That is exposed by EM / PIM in our case. Why EM / PIM 
> must rely on 2 different stuff, for common things is the question here.
>>
>>> If we have 2 different interface, then inventory producer may need to 
>>> be aware about both and probe it accordingly.
>>
>> No, the "FRU" properties producer needs to be concerned only about the 
>> format it understands.
> 
> [Richard]: FRU property producer must know the format and produce the 
> interface with data (in common form as much as possible). E.g. IPMI FRU 
> Producer (say xyz.openbmc_project.FruDevice service) will read device A 
> FRU, and expose the Manufacturer name (It can read the EEPROM content 
> and decode it as per the IPMI FRU format, but the data it produces is 
> Manufacturer name). Simiarly PLDM FRU Producer (say 
> xyz.openbmc_project.PLDM.FruDevice service) will read the data using 
> PLDM FRU commands, and expose the Manufacturer name. Now why this 2 
> service need to have 2 different interface(one from 
> Inventory.Source.PLDM & another from Inventory.Source.IPMI, to expose 
> the Manufacturer name. ? Why Entity manager / PIM need to read the same 
> information from 2 different interface and expose it in 
> Inventory.Decorator.Asset. (It can do it with same interface).

What is that interface?

> What Entity manager / PIM needs to do is using Object Mapper query all the 
> FruDevices (IPMI / PLDM FRU), and accordingly expose the Inventory
>>> FRU producers code must be written in such a way that for these 
>>> common properties it does the necessary conversion (Say make 
>>> manufacturer as string, irrespective of any format it read through). 
>>> Note: Specific stuff, we need to create a separate interface (as 
>>> phosphor-dbus-interface at present doesn't support dynamic property 
>>> addition/deletion). (Tomorrow, if we have any other proprietary way 
>>> of producing FRU data, we can still work with this approach, with 
>>> less or no change for other layers).
>>>
>>>> Also note that (as James pointed out in his email), the IPMI FRU 
>>>> format distinguishes Board/Chassis/Product areas. PLDM FRU format 
>>>> does not. So there are differences. If a new FRU format is 
>>>> introduced, then yes we would expect a new interface to show up 
>>>> under Inventory/Source/<FruFormat>
>>> [Richard]: Fru producers should do this conversion.
>>
>> I'm of the opinion that the inventory producer (like EM and PIM) 
>> should perform this conversion. Consider 
>> https://github.com/openbmc/entity-manager/blob/master/configurations/Intel%20Front%20Panel.json#L55 
>> for example. I don't think it's up to the FruDevice/PLDM kind of apps 
>> to decide that this is actually a Panel. You can design it that way, 
>> but like I said above that means the config knowledge moves into these 
>> apps, which I don't think we should head towards, since then every FRU 
>> producer app needs to do this. This is why we have apps like EM.
> [Richard]: Exactly. What we need to make sure is create abstraction 
> between Entity manager and FRU Producers as much as possible. FRU 
> Producer responsibility is to read the FRU in decode the FRU data as per 
> the spec and expose it in common form which Entity-manager / PIM will 
> rely on.

I don't see why the abstraction is necessary. There already is 
abstraction in terms of the phosphor interfaces.

>>> Say Chassis Type (Irrespective of what area it comes from it is 
>>> same). PLDM FRU mostly represents the product as a whole, but 
>>> technically we can point it to all the needed using the Fru Record 
>>> set to the Entity type mapping in the PDR record. Accordingly it 
>>> needs to be exposed.
>>>>
>>>>
>>>>> Yes, I am in favor of common interfaces for this where ever possible.
>>>>>
>>>>> Is there someone that knows the existing FruDevice implementation well
>>>>> enough that can be included in this work to propose common interfaces
>>>>> where it is appropriate?
>>>>>
>>>>>> Inventory/Source/General/Fru (Maintain common things here Product 
>>>>>> Name.
>>>>>> This can be used by Inventory manager to advertise it (instead of 
>>>>>> searching
>>>>>> it in multiple interfaces/properties))
>>>>>
>>>>> Minor tweak here of 'Source/Common'?  When we have an existing 
>>>>> Inventory
>>>>> interface for this information should we mimic what is already in
>>>>> Inventory?  At some point are we trying to be too common that we're
>>>>> effectively reimplementing Inventory instances under a different name?
>>>>>
>>> [Richard]: Yes, currently, FRU to inventory and inventory to upper 
>>> layer is what used. If we want to change it, we need to go with 
>>> differnt option of using FRU to upper layer, but many of existing 
>>> code will require change.
>>
>>
>>> Regards,
>>>
>>> Richard
>>>
>>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-29 10:19                   ` Deepak Kodihalli
@ 2020-05-29 10:30                     ` Deepak Kodihalli
  2020-05-29 10:33                       ` Thomaiyar, Richard Marian
  0 siblings, 1 reply; 27+ messages in thread
From: Deepak Kodihalli @ 2020-05-29 10:30 UTC (permalink / raw)
  To: Thomaiyar, Richard Marian, Patrick Williams
  Cc: Bhat, Sumanth, openbmc, Brad Bishop, James Feist

On 29/05/20 3:49 pm, Deepak Kodihalli wrote:
> On 29/05/20 2:33 pm, Thomaiyar, Richard Marian wrote:
>>
>> On 5/29/2020 1:01 PM, Deepak Kodihalli wrote:
>>> On 29/05/20 12:47 pm, Thomaiyar, Richard Marian wrote:
>>>>
>>>> On 5/29/2020 10:39 AM, Deepak Kodihalli wrote:
>>>>> On 28/05/20 11:35 pm, Patrick Williams wrote:
>>>>>> On Thu, May 28, 2020 at 10:12:19PM +0530, Thomaiyar, Richard 
>>>>>> Marian wrote:
>>>>>>>
>>>>>>> On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
>>>>>>>> On 28/05/20 5:33 pm, Patrick Williams wrote:
>>>>>>
>>>>>>> Why do we need to have 2 different interfaces to represent the same
>>>>>>> information for FRU-to-inventory transformational (say 
>>>>>>> ProductName). This
>>>>>>> will make inventory manager to be updated for every FRU 
>>>>>>> producer?. Many of
>>>>>>> the properties are common, and we can form a common interface for 
>>>>>>> that, and
>>>>>>> rest can be maintained in it's specific interface. I understand 
>>>>>>> that current
>>>>>>> FRU to Entity-manager interface seems to be private, but we must 
>>>>>>> make a
>>>>>>> common interface to represent like Product Name, PartNumer, 
>>>>>>> Serial Number
>>>>>>> etc. (instead of maintaining it in different interface saying 
>>>>>>> IPMI / PLDM
>>>>>>> Source, with different types). How about?
>>>>>
>>>>> Richard, I have concerns with this approach. Like I mentioned in 
>>>>> one my earlier emails, and Patrick also alludes to below, if you 
>>>>> try to make this common (event if it's for a subset of the 
>>>>> properties) then you basically arrive at the existing phosphor 
>>>>> Inventory interfaces (eg Inventory.Decorator.Asset).
>>>>>
>>>>> My question in my earlier mail was, if you do such a thing, then 
>>>>> why do you even need inventory producers? FruDevice and PLDM could 
>>>>> have hosted inventory on their own. If they still rely on the 
>>>>> inventory producers (EM and PIM) with this "common interface" 
>>>>> approach, then it's basically re-implementation/duplications of the 
>>>>> (Inventory.Decorator.Asset like) interface by two processes.
>>>
>>> Richard, what is your thought on the re-implementation/duplication 
>>> concern above? I'm not sure if you answered that and I missed.
>> [Richard]: FRU Consumers must be aware about each and every Format 
>> specifically, even though it conveys same meaning.
> 
> I agree with that, but my question was about FRU producers.
> 
> 
>>>> [Richard]: Basically FRU information (either IPMI/PLDM) is needed 
>>>> for the inventory producers to expose configuration, which FRU will 
>>>> not have. Say, based on FRU Product name, either we will expose 4 
>>>> temp sensor / 2 (Now along with this one, we need to inform the 
>>>> product name through Inventory.Decorator.Asset). Now from Redfish 
>>>> point of it, Inventory.Decorator is what it uses. This is what i was 
>>>> asking with 2 options in earlier mail (whether to change or stick 
>>>> with it (recommended)).
>>>
>>>>>
>>>>> The idea is for apps like FruDevice and PLDM, which are aware of a 
>>>>> specific FRU format, to host data in *that* format, to be consumed 
>>>>> *solely* by inventory producers (like EM and PIM).
>>>>>
>>>> [Richard]: Yes, but it doesn't need to expose those in that format?
>>>
>>> Why not?
>> [Richard]: What's the advantage in keeping it in that format itself?
> 
> The advantage I see is basically what you said on the next line.
> 
>> This is used only by EM / PIM, and not by redfish directly right? 
>> Where the intelligence must reside in the producer or consumer (With 
>> producer, consumers can be in common form)
> 
>>>> Say Manufacturer Name, it doesn't mater whether it is coming from 
>>>> PLDM FRU / IPMI FRU. Say we have a special handling for a particular 
>>>> manufacture / product, then irrespective of inventory producers both 
>>>> can handle the same.
>>>
>>> This is what the Inventory.Decorator.Asset interface is for.
>> [Richard]: Yes, That is exposed by EM / PIM in our case. Why EM / PIM 
>> must rely on 2 different stuff, for common things is the question here.
>>>
>>>> If we have 2 different interface, then inventory producer may need 
>>>> to be aware about both and probe it accordingly.
>>>
>>> No, the "FRU" properties producer needs to be concerned only about 
>>> the format it understands.
>>
>> [Richard]: FRU property producer must know the format and produce the 
>> interface with data (in common form as much as possible). E.g. IPMI 
>> FRU Producer (say xyz.openbmc_project.FruDevice service) will read 
>> device A FRU, and expose the Manufacturer name (It can read the EEPROM 
>> content and decode it as per the IPMI FRU format, but the data it 
>> produces is Manufacturer name). Simiarly PLDM FRU Producer (say 
>> xyz.openbmc_project.PLDM.FruDevice service) will read the data using 
>> PLDM FRU commands, and expose the Manufacturer name. Now why this 2 
>> service need to have 2 different interface(one from 
>> Inventory.Source.PLDM & another from Inventory.Source.IPMI, to expose 
>> the Manufacturer name. ? Why Entity manager / PIM need to read the 
>> same information from 2 different interface and expose it in 
>> Inventory.Decorator.Asset. (It can do it with same interface).
> 
> What is that interface?


@Richard - to elaborate further - on Gerrit you suggest moving things 
like Serial Number and Part Number to Inventory.Decorator.Asset. 
However, note that these are already present in 
Inventory.Decorator.Asset. Hence the question - why do want FruDevice, 
PLDM, EM, PIM to all implement Inventory.Decorator.Asset?


>> What Entity manager / PIM needs to do is using Object Mapper query all 
>> the FruDevices (IPMI / PLDM FRU), and accordingly expose the Inventory
>>>> FRU producers code must be written in such a way that for these 
>>>> common properties it does the necessary conversion (Say make 
>>>> manufacturer as string, irrespective of any format it read through). 
>>>> Note: Specific stuff, we need to create a separate interface (as 
>>>> phosphor-dbus-interface at present doesn't support dynamic property 
>>>> addition/deletion). (Tomorrow, if we have any other proprietary way 
>>>> of producing FRU data, we can still work with this approach, with 
>>>> less or no change for other layers).
>>>>
>>>>> Also note that (as James pointed out in his email), the IPMI FRU 
>>>>> format distinguishes Board/Chassis/Product areas. PLDM FRU format 
>>>>> does not. So there are differences. If a new FRU format is 
>>>>> introduced, then yes we would expect a new interface to show up 
>>>>> under Inventory/Source/<FruFormat>
>>>> [Richard]: Fru producers should do this conversion.
>>>
>>> I'm of the opinion that the inventory producer (like EM and PIM) 
>>> should perform this conversion. Consider 
>>> https://github.com/openbmc/entity-manager/blob/master/configurations/Intel%20Front%20Panel.json#L55 
>>> for example. I don't think it's up to the FruDevice/PLDM kind of apps 
>>> to decide that this is actually a Panel. You can design it that way, 
>>> but like I said above that means the config knowledge moves into 
>>> these apps, which I don't think we should head towards, since then 
>>> every FRU producer app needs to do this. This is why we have apps 
>>> like EM.
>> [Richard]: Exactly. What we need to make sure is create abstraction 
>> between Entity manager and FRU Producers as much as possible. FRU 
>> Producer responsibility is to read the FRU in decode the FRU data as 
>> per the spec and expose it in common form which Entity-manager / PIM 
>> will rely on.
> 
> I don't see why the abstraction is necessary. There already is 
> abstraction in terms of the phosphor interfaces.
> 
>>>> Say Chassis Type (Irrespective of what area it comes from it is 
>>>> same). PLDM FRU mostly represents the product as a whole, but 
>>>> technically we can point it to all the needed using the Fru Record 
>>>> set to the Entity type mapping in the PDR record. Accordingly it 
>>>> needs to be exposed.
>>>>>
>>>>>
>>>>>> Yes, I am in favor of common interfaces for this where ever possible.
>>>>>>
>>>>>> Is there someone that knows the existing FruDevice implementation 
>>>>>> well
>>>>>> enough that can be included in this work to propose common interfaces
>>>>>> where it is appropriate?
>>>>>>
>>>>>>> Inventory/Source/General/Fru (Maintain common things here Product 
>>>>>>> Name.
>>>>>>> This can be used by Inventory manager to advertise it (instead of 
>>>>>>> searching
>>>>>>> it in multiple interfaces/properties))
>>>>>>
>>>>>> Minor tweak here of 'Source/Common'?  When we have an existing 
>>>>>> Inventory
>>>>>> interface for this information should we mimic what is already in
>>>>>> Inventory?  At some point are we trying to be too common that we're
>>>>>> effectively reimplementing Inventory instances under a different 
>>>>>> name?
>>>>>>
>>>> [Richard]: Yes, currently, FRU to inventory and inventory to upper 
>>>> layer is what used. If we want to change it, we need to go with 
>>>> differnt option of using FRU to upper layer, but many of existing 
>>>> code will require change.
>>>
>>>
>>>> Regards,
>>>>
>>>> Richard
>>>>
>>>
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: D-Bus interface to provide data to entity manager
  2020-05-29 10:30                     ` Deepak Kodihalli
@ 2020-05-29 10:33                       ` Thomaiyar, Richard Marian
  0 siblings, 0 replies; 27+ messages in thread
From: Thomaiyar, Richard Marian @ 2020-05-29 10:33 UTC (permalink / raw)
  To: Deepak Kodihalli, Patrick Williams
  Cc: Bhat, Sumanth, openbmc, Brad Bishop, James Feist


On 5/29/2020 4:00 PM, Deepak Kodihalli wrote:
> On 29/05/20 3:49 pm, Deepak Kodihalli wrote:
>> On 29/05/20 2:33 pm, Thomaiyar, Richard Marian wrote:
>>>
>>> On 5/29/2020 1:01 PM, Deepak Kodihalli wrote:
>>>> On 29/05/20 12:47 pm, Thomaiyar, Richard Marian wrote:
>>>>>
>>>>> On 5/29/2020 10:39 AM, Deepak Kodihalli wrote:
>>>>>> On 28/05/20 11:35 pm, Patrick Williams wrote:
>>>>>>> On Thu, May 28, 2020 at 10:12:19PM +0530, Thomaiyar, Richard 
>>>>>>> Marian wrote:
>>>>>>>>
>>>>>>>> On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
>>>>>>>>> On 28/05/20 5:33 pm, Patrick Williams wrote:
>>>>>>>
>>>>>>>> Why do we need to have 2 different interfaces to represent the 
>>>>>>>> same
>>>>>>>> information for FRU-to-inventory transformational (say 
>>>>>>>> ProductName). This
>>>>>>>> will make inventory manager to be updated for every FRU 
>>>>>>>> producer?. Many of
>>>>>>>> the properties are common, and we can form a common interface 
>>>>>>>> for that, and
>>>>>>>> rest can be maintained in it's specific interface. I understand 
>>>>>>>> that current
>>>>>>>> FRU to Entity-manager interface seems to be private, but we 
>>>>>>>> must make a
>>>>>>>> common interface to represent like Product Name, PartNumer, 
>>>>>>>> Serial Number
>>>>>>>> etc. (instead of maintaining it in different interface saying 
>>>>>>>> IPMI / PLDM
>>>>>>>> Source, with different types). How about?
>>>>>>
>>>>>> Richard, I have concerns with this approach. Like I mentioned in 
>>>>>> one my earlier emails, and Patrick also alludes to below, if you 
>>>>>> try to make this common (event if it's for a subset of the 
>>>>>> properties) then you basically arrive at the existing phosphor 
>>>>>> Inventory interfaces (eg Inventory.Decorator.Asset).
>>>>>>
>>>>>> My question in my earlier mail was, if you do such a thing, then 
>>>>>> why do you even need inventory producers? FruDevice and PLDM 
>>>>>> could have hosted inventory on their own. If they still rely on 
>>>>>> the inventory producers (EM and PIM) with this "common interface" 
>>>>>> approach, then it's basically re-implementation/duplications of 
>>>>>> the (Inventory.Decorator.Asset like) interface by two processes.
>>>>
>>>> Richard, what is your thought on the re-implementation/duplication 
>>>> concern above? I'm not sure if you answered that and I missed.
>>> [Richard]: FRU Consumers must be aware about each and every Format 
>>> specifically, even though it conveys same meaning.
>>
>> I agree with that, but my question was about FRU producers.
>>
>>
>>>>> [Richard]: Basically FRU information (either IPMI/PLDM) is needed 
>>>>> for the inventory producers to expose configuration, which FRU 
>>>>> will not have. Say, based on FRU Product name, either we will 
>>>>> expose 4 temp sensor / 2 (Now along with this one, we need to 
>>>>> inform the product name through Inventory.Decorator.Asset). Now 
>>>>> from Redfish point of it, Inventory.Decorator is what it uses. 
>>>>> This is what i was asking with 2 options in earlier mail (whether 
>>>>> to change or stick with it (recommended)).
>>>>
>>>>>>
>>>>>> The idea is for apps like FruDevice and PLDM, which are aware of 
>>>>>> a specific FRU format, to host data in *that* format, to be 
>>>>>> consumed *solely* by inventory producers (like EM and PIM).
>>>>>>
>>>>> [Richard]: Yes, but it doesn't need to expose those in that format?
>>>>
>>>> Why not?
>>> [Richard]: What's the advantage in keeping it in that format itself?
>>
>> The advantage I see is basically what you said on the next line.
>>
>>> This is used only by EM / PIM, and not by redfish directly right? 
>>> Where the intelligence must reside in the producer or consumer (With 
>>> producer, consumers can be in common form)
>>
>>>>> Say Manufacturer Name, it doesn't mater whether it is coming from 
>>>>> PLDM FRU / IPMI FRU. Say we have a special handling for a 
>>>>> particular manufacture / product, then irrespective of inventory 
>>>>> producers both can handle the same.
>>>>
>>>> This is what the Inventory.Decorator.Asset interface is for.
>>> [Richard]: Yes, That is exposed by EM / PIM in our case. Why EM / 
>>> PIM must rely on 2 different stuff, for common things is the 
>>> question here.
>>>>
>>>>> If we have 2 different interface, then inventory producer may need 
>>>>> to be aware about both and probe it accordingly.
>>>>
>>>> No, the "FRU" properties producer needs to be concerned only about 
>>>> the format it understands.
>>>
>>> [Richard]: FRU property producer must know the format and produce 
>>> the interface with data (in common form as much as possible). E.g. 
>>> IPMI FRU Producer (say xyz.openbmc_project.FruDevice service) will 
>>> read device A FRU, and expose the Manufacturer name (It can read the 
>>> EEPROM content and decode it as per the IPMI FRU format, but the 
>>> data it produces is Manufacturer name). Simiarly PLDM FRU Producer 
>>> (say xyz.openbmc_project.PLDM.FruDevice service) will read the data 
>>> using PLDM FRU commands, and expose the Manufacturer name. Now why 
>>> this 2 service need to have 2 different interface(one from 
>>> Inventory.Source.PLDM & another from Inventory.Source.IPMI, to 
>>> expose the Manufacturer name. ? Why Entity manager / PIM need to 
>>> read the same information from 2 different interface and expose it 
>>> in Inventory.Decorator.Asset. (It can do it with same interface).
>>
>> What is that interface?
>
>
> @Richard - to elaborate further - on Gerrit you suggest moving things 
> like Serial Number and Part Number to Inventory.Decorator.Asset. 
> However, note that these are already present in 
> Inventory.Decorator.Asset. Hence the question - why do want FruDevice, 
> PLDM, EM, PIM to all implement Inventory.Decorator.Asset?
[Richard]:  No. What i meant was to use Inventory.Source.Common for 
serial Number, part number etc (Fru to Inventory) and inventory producer 
will use the inventory.Decorator.Asset for redfish exposure.
>
>
>>> What Entity manager / PIM needs to do is using Object Mapper query 
>>> all the FruDevices (IPMI / PLDM FRU), and accordingly expose the 
>>> Inventory
>>>>> FRU producers code must be written in such a way that for these 
>>>>> common properties it does the necessary conversion (Say make 
>>>>> manufacturer as string, irrespective of any format it read 
>>>>> through). Note: Specific stuff, we need to create a separate 
>>>>> interface (as phosphor-dbus-interface at present doesn't support 
>>>>> dynamic property addition/deletion). (Tomorrow, if we have any 
>>>>> other proprietary way of producing FRU data, we can still work 
>>>>> with this approach, with less or no change for other layers).
>>>>>
>>>>>> Also note that (as James pointed out in his email), the IPMI FRU 
>>>>>> format distinguishes Board/Chassis/Product areas. PLDM FRU format 
>>>>>> does not. So there are differences. If a new FRU format is 
>>>>>> introduced, then yes we would expect a new interface to show up 
>>>>>> under Inventory/Source/<FruFormat>
>>>>> [Richard]: Fru producers should do this conversion.
>>>>
>>>> I'm of the opinion that the inventory producer (like EM and PIM) 
>>>> should perform this conversion. Consider 
>>>> https://github.com/openbmc/entity-manager/blob/master/configurations/Intel%20Front%20Panel.json#L55 
>>>> for example. I don't think it's up to the FruDevice/PLDM kind of 
>>>> apps to decide that this is actually a Panel. You can design it 
>>>> that way, but like I said above that means the config knowledge 
>>>> moves into these apps, which I don't think we should head towards, 
>>>> since then every FRU producer app needs to do this. This is why we 
>>>> have apps like EM.
>>> [Richard]: Exactly. What we need to make sure is create abstraction 
>>> between Entity manager and FRU Producers as much as possible. FRU 
>>> Producer responsibility is to read the FRU in decode the FRU data as 
>>> per the spec and expose it in common form which Entity-manager / PIM 
>>> will rely on.
>>
>> I don't see why the abstraction is necessary. There already is 
>> abstraction in terms of the phosphor interfaces.
>>
>>>>> Say Chassis Type (Irrespective of what area it comes from it is 
>>>>> same). PLDM FRU mostly represents the product as a whole, but 
>>>>> technically we can point it to all the needed using the Fru Record 
>>>>> set to the Entity type mapping in the PDR record. Accordingly it 
>>>>> needs to be exposed.
>>>>>>
>>>>>>
>>>>>>> Yes, I am in favor of common interfaces for this where ever 
>>>>>>> possible.
>>>>>>>
>>>>>>> Is there someone that knows the existing FruDevice 
>>>>>>> implementation well
>>>>>>> enough that can be included in this work to propose common 
>>>>>>> interfaces
>>>>>>> where it is appropriate?
>>>>>>>
>>>>>>>> Inventory/Source/General/Fru (Maintain common things here 
>>>>>>>> Product Name.
>>>>>>>> This can be used by Inventory manager to advertise it (instead 
>>>>>>>> of searching
>>>>>>>> it in multiple interfaces/properties))
>>>>>>>
>>>>>>> Minor tweak here of 'Source/Common'?  When we have an existing 
>>>>>>> Inventory
>>>>>>> interface for this information should we mimic what is already in
>>>>>>> Inventory?  At some point are we trying to be too common that we're
>>>>>>> effectively reimplementing Inventory instances under a different 
>>>>>>> name?
>>>>>>>
>>>>> [Richard]: Yes, currently, FRU to inventory and inventory to upper 
>>>>> layer is what used. If we want to change it, we need to go with 
>>>>> differnt option of using FRU to upper layer, but many of existing 
>>>>> code will require change.
>>>>
>>>>
>>>>> Regards,
>>>>>
>>>>> Richard
>>>>>
>>>>
>>
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2020-05-29 10:34 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19  3:40 Processing PLDM FRU information with entity manager Deepak Kodihalli
2020-05-20 16:28 ` Brad Bishop
2020-05-20 16:56   ` James Feist
2020-05-20 17:35     ` Brad Bishop
2020-05-20 17:46       ` James Feist
2020-05-26 12:56 ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Deepak Kodihalli
2020-05-27 13:50   ` D-Bus interface to provide data to entity manager Thomaiyar, Richard Marian
2020-05-27 13:58     ` Deepak Kodihalli
2020-05-27 15:25       ` Thomaiyar, Richard Marian
2020-05-27 15:46         ` Deepak Kodihalli
2020-05-27 17:19           ` Thomaiyar, Richard Marian
2020-05-28 12:09             ` Patrick Williams
2020-05-28 12:03   ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Patrick Williams
2020-05-28 12:24     ` D-Bus interface to provide data to entity manager Deepak Kodihalli
2020-05-28 16:42       ` Thomaiyar, Richard Marian
2020-05-28 18:05         ` Patrick Williams
2020-05-28 18:31           ` James Feist
2020-05-29  5:11             ` Deepak Kodihalli
2020-05-29  5:09           ` Deepak Kodihalli
2020-05-29  7:17             ` Thomaiyar, Richard Marian
2020-05-29  7:31               ` Deepak Kodihalli
2020-05-29  9:03                 ` Thomaiyar, Richard Marian
2020-05-29 10:19                   ` Deepak Kodihalli
2020-05-29 10:30                     ` Deepak Kodihalli
2020-05-29 10:33                       ` Thomaiyar, Richard Marian
2020-05-28 15:43     ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Brad Bishop
2020-05-28 18:21       ` D-Bus interface to provide data to entity manager James Feist

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.