phosphor-host-ipmid and phosphor-net-ipmid architecture

[Xo Wang]

On Thu, Oct 12, 2017 at 9:03 AM, Brad Bishop
<bradleyb at fuzziesquirrel.com> wrote:
>
>> On Oct 11, 2017, at 6:05 PM, Vernon Mauery <vernon.mauery at linux.intel.com> wrote:
>>
>> I am working on an ipmi provider library and had a few questions and
>> observations.
>>
>> 1) Why are there separate ipmi message queues for the host and network?
>
> iirc it was so that you could have a different set of providers
> registered for each channel or even different, channel specific
> implementations for the same command.
>

The reasoning here also derives from the "build the distro as you like
it" approach. If you don't want a network interface to the BMC, then
you can simply not build it or its handlers.

Of course we could write a single queue service for both host and
network that can be configured and built to support only one, but that
puts the onus on architecting and testing that service to ensure the
all three configurations all work independently.

>>   It seems awkward that for the host, the ipmi request comes from a
>>   different process (btbridge, or in our case kcsbridge), while for the
>>   network (RMCP+), the messages are handled directly in the same
>>   process.
>
> Is it still awkward if you accept the two queue approach?  The
> additional layer is needed for the bt/kcs -> host-ipmid
> abstraction.  No such abstraction is needed for network.
>
>>
>>   It seems that the network handler could just as easily package the
>>   command up and send it to ipmid the same way that btbridge does.
>>
>> 2) Can we modify the signature of the handlers so that they can behave
>>   in a more intelligent manner? It would be nice if they were handed a
>>   gsl::span<uint8_t> instead of a void* and a length. This allows for
>>   a no-copy, bounds-checked way of passing buffers.
>
> sounds good to me!
>

We can use more intelligent (safer) data passing throughout phosphor code. :)

>>
>>   It would be nice to know what channel something came in on. We might
>>   want to be able to change behavior based on the incoming channel (as
>>   some channels are more secure than others).
>
> I think the separate message queues solves this need; however, doing
> this need not be mutually exclusive of having separate queues if that
> enables another use case we don’t support today.
>
> I’m not totally stuck on two queues.  Especially if some alternative
> maintains the same level of flexibility.  The motivation for a single
> queue isn’t clear yet to me though - without more discussion it sounds
> like we’d end up with the same capability we already have…so why bother?
>

One set of cases is where a host might have multiple interfaces to the
BMC (e.g. BT and SSIF).

I can't immediately think of why a provider might want to respond to a
command differently based on what host interface it came on, but maybe
somebody else can.

>>
>>   It would be nice to know what IPMI privilege the command came in
>>   with (ADMIN for session-less commands) so that the command handler
>>   can behave appropriately based on the user.
>
> Makes sense.
>
>>
>> 3) When registering commands, it would be nice of the list also
>>   maintained a priority so that commands could be easily overridden.
>>   Currently the only way to override a command is to make sure that
>>   your library gets loaded first (and this is done via the library
>>   name). If we had default ipmi commands loaded at DEFAULT_PRIO and
>>   then had some higher priorities such as MFR_PRIO, and OEM_PRIO, or
>>   something like that, we could have integrators further on down the
>>   line able to easily add a new provider library and piecemeal override
>>   individual command. An alternate (or addition) might be the addition
>>   of a unregister command method to remove an existing command so it
>>   could be replaced with a new one (or just straight up removed).
>
> The intent is that everything in OpenBMC is overridable by an integrator.
> We provide a reasonable ‘reference’ implementation but can be replaced
> with something else via a bitbake layer.  At image construction time.
>
> I think you are proposing a setup where an image has multiple providers
> for the same command, and the ‘reference’ has a low priority.  In their
> bitbake layer, an integrator could _add_ another provider with higher
> priority, which would be selected to handle the command since it has
> the higher priority.
>
> That makes sense, but why wouldn’t an integrator just _replace_ the
> reference in an image with the custom integrator provider?  This type
> of thing is exactly why we are a bitbake distro.
>
> I hope you don’t have a good answer to this :-) as one of my goals is
> to try move runtime complexity into the build process as much as we can.
>
>>
>>
>> I am happy to work on changes that I would like to see and submit
>> patches for review, but I wanted to know if there was some sort of
>> historical or other reason that would prevent my work from getting
>> rejected before I actually do the work.
>>
>> --Vernon

cheers
xo