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

[Brad Bishop]

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

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

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

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