ampere-openbmc/phosphor-host-ipmid

[Emily Shaffer]

+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>
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>
> *Date: *Friday, January 25, 2019 at 7:11 AM
> *To: *Hien Pham <hpham at amperecomputing.com>
> *Cc: *Hoang Nguyen <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>
> 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>
> *Date: *Thursday, January 24, 2019 at 5:26 AM
> *To: *Hoang Nguyen <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) 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/20190125/454d11fd/attachment.html>