ampere-openbmc/phosphor-host-ipmid

[Hoang Nguyen]

Hi Emily,

Sorry for late response.

I added new document for FRU manager package.

https://github.com/ampere-openbmc/docs/tree/master/fru-manager

Please take a look about the design of new package.

Best regards,
Hoang

From: Emily Shaffer <emilyshaffer at google.com>
Date: Saturday, January 26, 2019 at 8:18 AM
To: Hoang Nguyen <hnguyen at amperecomputing.com>
Cc: Hien Pham <hpham at amperecomputing.com>, OpenBMC Maillist <openbmc at lists.ozlabs.org>
Subject: Re: ampere-openbmc/phosphor-host-ipmid

[NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.] ________________________________________________________________________________________________________________________
+OpenBMC mailing list

Hoang,

Thanks for the overview. I think that we would probably like to see your design for review - it sounds interesting and there have been other complaints regarding FRU parser.  If you want to provide more detail to the entire list I'm sure we will be interested to hear, and also very interested to take a look at your code and consider adoption.

Thanks!
Emily

On Fri, Jan 25, 2019 at 1:23 AM Hoang Nguyen <hnguyen at amperecomputing.com<mailto:hnguyen at amperecomputing.com>> wrote:
Hi Emily,

FYI, now I don’t have too much time to develop new feature of IPMI at OpenBMC, but I can follow the IPMI at OpenBMC activity.
I’ll push the code for reviewing if you’re interesting in my design.

Firstly, I’ll overview about my design. And I’ll continue in detail if you agree.

The package name: FRU manager

The reason for new package:
+ Current package doesn’t support Multirecord FRU parsing
+ Sometime, using “IPMI fru write…” command will crash the FRU EEPROM image -> nothing from D-Bus can inform this info

New package structure:
FRU manager package has 2 parts: Manager and Parser
+ Manager:
    - Initial the D-Bus interface + all properties of Chassis/Board/Product/Multirecord areas (with default value)
    - Provide the D-Bus methods for other packages (Parser as an example) to update the real data of FRU to D-Bus
    - Provide methods to update the FRU device directly (in case of FRU recovering)

+ Parser:
    - Access the FRU device directly to parse the FRU data and request to update these date to D-Bus (using Manager supported methods)

New services to handle new package:
Add more 2 services:
+ FRU manager service: start Manager part to handle all FRU request on D-Bus
+ FRU parser service: start Parser part to parse FRU data and update to D-Bus

Improvement points:
+ Handle Multirecord data
+ In case of FRU data is not correct, the FRU parser service will be failed, all data of FRU on D-Bus are “default value”
   -> We can identify the issue and recovering the FRU image


Best regards,
Hoang

From: Emily Shaffer <emilyshaffer at google.com<mailto:emilyshaffer at google.com>>
Date: Friday, January 25, 2019 at 7:11 AM
To: Hien Pham <hpham at amperecomputing.com<mailto:hpham at amperecomputing.com>>
Cc: Hoang Nguyen <hnguyen at amperecomputing.com<mailto:hnguyen at amperecomputing.com>>
Subject: Re: ampere-openbmc/phosphor-host-ipmid

[NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.] ________________________________________________________________________________________________________________________
Hien, Hoang,

Thanks so much for replying to my cold email :)

I want to take a look and understand better the approach you guys used. We've added more support to some of the FRU functionality you were missing - specifically multirecord FRU has seen development in the past month or two - but it looks like you made a broader change.  If you have the engineer time to do so, I think it'd be valuable for you to push either your code or a design proposal (which maybe you already have written) for the OpenBMC community to take a look at (via gerrit) - I'd like to get the rest of the community's feedback too.

It sounds like you aren't able to dedicate a lot of resources right now so I definitely understand if you don't want to put up a review; if you won't have the time, could you let me know? I'll ask the other IPMI maintainers to take a look at your approach too and we can decide if it's something we want to bring over ourselves.  (But it's more cohesive for the community if you're able to push the review from your end :) )

Thanks,
Emily

On Wed, Jan 23, 2019 at 11:53 PM Hien Pham <hpham at amperecomputing.com<mailto:hpham at amperecomputing.com>> wrote:

Dear Emily,



I'm Hien Plam (Ampere OpenBMC CLA manager).  Nice to hear that you care the change we did.



No worry, we committed to contribute back and willing to submit the change to the upstream anyway (if you see the change are applicable).  Please let us know if you are interesting in our FRU refactor (Hoang's explanation below).  If yes, we can process  next step to upstream the changes.



Thank and best regards,

________________________________
From: Hoang Nguyen
Sent: Thursday, January 24, 2019 2:02:23 PM
To: Emily Shaffer
Cc: Hien Pham
Subject: Re: ampere-openbmc/phosphor-host-ipmid


Hi Emily,



First at all, I’d like to inform that now we’re suspending the OpenBMC activity due to the business strategy.

But we’ll come back with OpenBMC soon (I’m sure about it).



Regarding to your interesting of IPMI implementations (FRU interaction in phosphor-host-ipmid), I’ve not push these code because I’m using another design for the communication between IPMI implementation (phosphor-host-ipmid) and FRU access handler (ipmi-fru-parser).



At current design, I found some limitation, such as:

  *   ipmi-fru-parser cannot parse the multi-record area
  *   ipmi-fru-parser takes many responsibilities:

     *   Handle FRU device
     *   Implement IPMI FRU commands

  *   many modules can access the FRU EEPROM device directly



So I changed the design, include:

  1.  Disable the ipmi-fru-parser
  2.  Create new module (like FRU manager package) to handle the FRU device.

Overview about this package, it can:

  *   Access (read/write) the FRU device directly.
  *   Parse the multi-record area and public all info to D-Bus interface
  *   Provide some methods (using D-Bus interface) to support the read/write FRU device request from other packages.

  1.  Update all relate packages (phosphor-host-ipmid is example):

  *   Stop accessing FRU device directly and start using FRU manager package by calling supported methods
  *   Move IPMI FRU commands to phosphor-host-ipmid



With this design, each package takes only one responsibility; prevent the issue of FRU device accessing from many packages.



Finally, because this solution is specific for our system, so I did not pushed it.

(but it always follow the common specification)



Best regards,

Hoang



From: Emily Shaffer <emilyshaffer at google.com<mailto:emilyshaffer at google.com>>
Date: Thursday, January 24, 2019 at 5:26 AM
To: Hoang Nguyen <hnguyen at amperecomputing.com<mailto:hnguyen at amperecomputing.com>>
Subject: ampere-openbmc/phosphor-host-ipmid



[NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.] ________________________________________________________________________________________________________________________

Hi Hoang,



I'm a maintainer of the IPMI stack for OpenBMC (github.com/openbmc<http://github.com/openbmc>) and came across your forked project today.



I noticed that Ampere executed a CLA with OpenBMC in July 2018, but I don't see that you're contributing your IPMI implementations back to OpenBMC. It actually looks like in general Ampere has written some code that we want, especially regarding FRU interaction in phosphor-host-ipmid, so I'm curious why it is that you haven't pushed any code back up to us.



Can I facilitate getting Ampere to contribute back to the OpenBMC community?



Thanks,

Emily Shaffer
(Github/Freenode: nasamuffin)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20190213/9b033087/attachment-0001.htm>