Re: D-Bus interface to provide data to entity manager

["Thomaiyar, Richard Marian" <richard.marian.thomaiyar@linux.intel.com>]

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
>