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

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

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
>>
>