D-Bus interface to provide data to entity manager
dkodihal at linux.vnet.ibm.com
Fri May 29 20:19:01 AEST 2020
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
>>>>>> 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
>>>>>> 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
>>>> 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
>> 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
>>>>>> This can be used by Inventory manager to advertise it (instead of
>>>>>> it in multiple interfaces/properties))
>>>>> Minor tweak here of 'Source/Common'? When we have an existing
>>>>> 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.
More information about the openbmc