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

[Deepak Kodihalli <dkodihal@linux.vnet.ibm.com>]

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
>