D-Bus interface to provide data to entity manager

[Deepak Kodihalli]

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