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

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

[Vernon Mauery]

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

   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.

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

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


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

Re: phosphor-host-ipmid and phosphor-net-ipmid architecture

[Brad Bishop]

> On Oct 11, 2017, at 6:05 PM, Vernon Mauery <vernon.mauery@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

Re: phosphor-host-ipmid and phosphor-net-ipmid architecture

[Xo Wang]

On Thu, Oct 12, 2017 at 9:03 AM, Brad Bishop
<bradleyb@fuzziesquirrel.com> wrote:
>
>> On Oct 11, 2017, at 6:05 PM, Vernon Mauery <vernon.mauery@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

Re: phosphor-host-ipmid and phosphor-net-ipmid architecture

[Brad Bishop]

> On Oct 12, 2017, at 12:25 PM, Xo Wang <xow@google.com> wrote:
> 
> On Thu, Oct 12, 2017 at 9:03 AM, Brad Bishop
> <bradleyb@fuzziesquirrel.com> wrote:
>> 
>>> On Oct 11, 2017, at 6:05 PM, Vernon Mauery <vernon.mauery@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.

Another possible advantage for multiple queues might be cgroups prioritization.

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

What about three queues here?  (bt, ssif and rmcp)?  It would require
some rework of the bt/ssif/kcs -> host-ipmid interface to support multiple
host-ipmid instances. 

Re: phosphor-host-ipmid and phosphor-net-ipmid architecture

[Vernon Mauery]

On 12-Oct-2017 12:03 PM, Brad Bishop wrote:
> 
> > On Oct 11, 2017, at 6:05 PM, Vernon Mauery <vernon.mauery@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.

Okay, that makes sense. This is just a mindset change for me. :) In my 
head I was separating the providers by command group. I just need to 
make sure that we are also separating them by interface if that matters 
so they can only get registered in a single queue.

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

I guess it is weird to me that the network commands don't seem to get 
packaged up and sent across dbus, while they do for bt/kcs. Why don't 
the bt/kcs commands get processed in the same process instead of getting 
sent to ipmid. From the perspective of the command handlers in ipmid, 
they cannot tell if the channel is internal (on-bmc ipmitool) or the 
bt/kcs interface.

Having multiple queues only makes sense to me if there is one for every 
channel. Alternately, one queue that passes the channel information in 
with each command.

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

If any of the ipmi commands have any state involved, then that state 
needs to either be duplicated between two queues or somehow 
synchronized. Neither situation is ideal if the size of the shared data 
is large. A single queue would not have this problem.

But now that you mention the advantages of the multiple queues, it seems 
that neither situation is ideal. So maybe I will have to think some more 
on this one.

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

The only thing I can think of is if an integrator likes 75% of the 
commands in one of the base provider libraries but still wants to 
override 25% of the commands (or even just one).

Pushing the configuration to the build is good. Maybe adding some way to 
enable/disable each command in the base providers at build time. 
Otherwise integrators would need to maintain a patch that disables the 
command the hard way.

That said, this may not really be a huge deal since it is more likely 
that any commands integrators will be adding would be OEM commands that 
don't collide anyway.

--Vernon

Re: phosphor-host-ipmid and phosphor-net-ipmid architecture

[Vernon Mauery]

On 12-Oct-2017 09:25 AM, Xo Wang wrote:
> On Thu, Oct 12, 2017 at 9:03 AM, Brad Bishop
> <bradleyb@fuzziesquirrel.com> wrote:
> >
> >> On Oct 11, 2017, at 6:05 PM, Vernon Mauery <vernon.mauery@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.

What about a ipmi queue library or something that handles loading the 
appropriate provider libraries (based on some path) and then manages the 
incoming messages. We have two separate queue implementations between 
net-ipmi and host-ipmi; we could simplify this so the behavior is the 
same between the two.

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

I will probably start with changes like this that pull me gently into 
the 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.

We have two kcs interfaces that in theory are supposed to have different 
behavior: one is from the host and can have reasonable latency, the 
other is the kcs that the BIOS would use during an SMI and needs to have 
minimal latency. That being said, I am not sure if this feature is still 
used at this time. You said you were looking for a use case. :)

--Vernon

Re: phosphor-host-ipmid and phosphor-net-ipmid architecture

[Vernon Mauery]

On 12-Oct-2017 12:37 PM, Brad Bishop wrote:
> 
> > On Oct 12, 2017, at 12:25 PM, Xo Wang <xow@google.com> wrote:
> > 
> > On Thu, Oct 12, 2017 at 9:03 AM, Brad Bishop
> > <bradleyb@fuzziesquirrel.com> wrote:
> >> 
> >>> On Oct 11, 2017, at 6:05 PM, Vernon Mauery <vernon.mauery@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.
> 
> Another possible advantage for multiple queues might be cgroups prioritization.

I think multiple queues might work out fine then. They do seem to have 
some compelling advantages. But I would like to at least consolidate the 
queueing portion of the code so all the queues have the same bugs.

> >>>  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).
> 
> What about three queues here?  (bt, ssif and rmcp)?  It would require
> some rework of the bt/ssif/kcs -> host-ipmid interface to support multiple
> host-ipmid instances. 

I think we could have as many queues as we want. I would vote for one 
per interface so that the interface information can be passed in via the 
context data to the handler.

--Vernon

Re: phosphor-host-ipmid and phosphor-net-ipmid architecture

[tomjose]

Hey Vernon,

1)  There are few reasons for a different approach for net-ipmid

     The primary design direction when developing net-ipmid was to reuse 
the provider libraries
     for the commands that are common across the interfaces. It was a 
conscious decision
     to have two processes for network & BT, keeping in mind to have 
small applications
     rather than having monolithic ones.

    The network handler would not be minimal like btbridge or KCSbridge, 
since there are net-ipmid
    specific functionalities like session setup, serial over LAN..etc. 
The only difference i see in your
    idea is we would relay the common commands on the dbus so that ipmid 
would handle.

2) The phosphor-host-ipmid was developed as part of the prototyping done 
for Barreleye. So i agree that
     we have quite a number of shortcomings, that it is not in alignment 
with the modern C++ practices.
     We have added more code and haven't got to refactor that. We have a 
story to refactor phosphor-host-ipmid.
     The plan was to handle so some of these suggestions as part of that.

    The support is in place to register each command's privilege. The 
support would be completed once the
    IPMI user account management changes are in place. The command's 
privilege would matter only on
    the LAN interface which is session based. In net-ipmid every command 
would be checked against the
    session's privilege, so that a session with USER privilege would be 
restricted from running a command
    that needs ADMIN privilege. Since KCS and BT are sessionless and 
unauthenticated, this would not matter.

    Similarly System interface commands are compiled into a separate 
library which is not loaded in the
    net-ipmid.

3) This is something we wanted to do, we can discuss what is the best 
possible approach. This would help with
     command like Get/Set System Boot Configuration where each 
implementer would get the flexibility
     to implement the OEM parameters.

  Are you working on a OEM provider library?

Regards,
Tom




On Thursday 12 October 2017 03:35 AM, Vernon Mauery 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?
>     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.
>
>     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.
>
>     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).
>
>     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.
>     
> 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).
>
>
> 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
>

Re: phosphor-host-ipmid and phosphor-net-ipmid architecture

[Vernon Mauery]

On 13-Oct-2017 12:27 PM, tomjose wrote:
>
>Hey Vernon,
>
>1)  There are few reasons for a different approach for net-ipmid
>
>    The primary design direction when developing net-ipmid was to 
>reuse the provider libraries
>    for the commands that are common across the interfaces. It was a 
>conscious decision
>    to have two processes for network & BT, keeping in mind to have 
>small applications
>    rather than having monolithic ones.

Okay. If this was a conscious design decision, I don't see a reason to 
expend effort to undo it just to have my patch rejected. :)

>   The network handler would not be minimal like btbridge or 
>KCSbridge, since there are net-ipmid
>   specific functionalities like session setup, serial over LAN..etc. 
>The only difference i see in your
>   idea is we would relay the common commands on the dbus so that 
>ipmid would handle.
>
>2) The phosphor-host-ipmid was developed as part of the prototyping 
>done for Barreleye. So i agree that
>    we have quite a number of shortcomings, that it is not in 
>alignment with the modern C++ practices.
>    We have added more code and haven't got to refactor that. We have 
>a story to refactor phosphor-host-ipmid.
>    The plan was to handle so some of these suggestions as part of that.

I admit I have not looked that much at the net-ipmid (only enough to see 
that it was a different queue implementation from host-ipmid). Making 
some of the queueing common and updating to current c++ standards would 
be great.

>   The support is in place to register each command's privilege. The 
>support would be completed once the
>   IPMI user account management changes are in place. The command's 
>privilege would matter only on
>   the LAN interface which is session based. In net-ipmid every 
>command would be checked against the
>   session's privilege, so that a session with USER privilege would be 
>restricted from running a command
>   that needs ADMIN privilege. Since KCS and BT are sessionless and 
>unauthenticated, this would not matter.
>
>   Similarly System interface commands are compiled into a separate 
>library which is not loaded in the
>   net-ipmid.

If this support is already in the net-ipmid, great.

>3) This is something we wanted to do, we can discuss what is the best 
>possible approach. This would help with
>    command like Get/Set System Boot Configuration where each 
>implementer would get the flexibility
>    to implement the OEM parameters.
>
> Are you working on a OEM provider library?

I am working on some firmware update commands (which I guess by the spec 
are technically OEM commands) and we have some other people working on 
other commands (sensor/sdr related).

>Regards,
>Tom
>
>
>
>
>On Thursday 12 October 2017 03:35 AM, Vernon Mauery 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?
>>    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.
>>
>>    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.
>>
>>    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).
>>
>>    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.
>>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).
>>
>>
>>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
>>
>

Re: phosphor-host-ipmid and phosphor-net-ipmid architecture

[tomjose]

On Thursday 19 October 2017 09:38 AM, brendanhiggins@google.com wrote:
>> Hey Vernon,
>>
>> 1)  There are few reasons for a different approach for net-ipmid
>>
>>       The primary design direction when developing net-ipmid was to reuse
>> the provider libraries
>>       for the commands that are common across the interfaces. It was a
>> conscious decision
>>       to have two processes for network & BT, keeping in mind to have
>> small applications
>>       rather than having monolithic ones.
>>
>>      The network handler would not be minimal like btbridge or KCSbridge,
>> since there are net-ipmid
>>      specific functionalities like session setup, serial over LAN..etc.
> Unfortunately there is more common functionality than just handlers. For
> example, Firmware Firewall is to be applied to all interfaces. Another example,
> bridging allows messages to be taken from one interface, like the LAN Interface,
> and applied on some other interface, like the System Interface.

Firmware firewalling applies only for the system interface as per the 
IPMI specification.
There is a whitelist of IPMI commands, only those commands can be 
executed when
the machine is set to restricted mode. The list though is setup at build 
time.

>
> I talked about this extensively with Corey Minyard (see discussion under [RFC v1
> 1/4] ipmi_bmc: framework for BT IPMI on BMCs). I was trying to create a common
> interface that the userland could get IPMI messages from as well as a common way
> to handle IPMI messages in the kernel. Unfortunately, the various concepts in
> IPMI are just really too tightly coupled.
>
>> The only difference i see in your
>>      idea is we would relay the common commands on the dbus so that ipmid
>> would handle.
>>
>> 2) The phosphor-host-ipmid was developed as part of the prototyping done
>> for Barreleye. So i agree that
>>       we have quite a number of shortcomings, that it is not in alignment
>> with the modern C++ practices.
>>       We have added more code and haven't got to refactor that. We have a
>> story to refactor phosphor-host-ipmid.
>>       The plan was to handle so some of these suggestions as part of that.
>>
>>      The support is in place to register each command's privilege. The
>> support would be completed once the
>>      IPMI user account management changes are in place. The command's
>> privilege would matter only on
>>      the LAN interface which is session based. In net-ipmid every command
>> would be checked against the
>>      session's privilege, so that a session with USER privilege would be
>> restricted from running a command
>>      that needs ADMIN privilege. Since KCS and BT are sessionless and
>> unauthenticated, this would not matter.
> Not quite true, see above.
The privilege levels are for session based interfaces. I don't know any
other than LAN.

>
>>      Similarly System interface commands are compiled into a separate
>> library which is not loaded in the
>>      net-ipmid.
>>
>> 3) This is something we wanted to do, we can discuss what is the best
>> possible approach. This would help with
>>       command like Get/Set System Boot Configuration where each
>> implementer would get the flexibility
>>       to implement the OEM parameters.
>>
>>    Are you working on a OEM provider library?
> We have written some in the past. I proposed one about a year ago. I think there
> was another one as well. Might be time to revisit that.
>
>> Regards,
>> Tom
>>
>>
>>
>>
>> On Thursday 12 October 2017 03:35 AM, Vernon Mauery 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?
>>>      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.
>>>
>>>      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.
>>>
>>>      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).
>>>
>>>      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.
>>>
>>> 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).
>>>
>>>
>>> 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
>

Re: phosphor-host-ipmid and phosphor-net-ipmid architecture

[brendanhiggins]

> Hey Vernon,
>
> 1)  There are few reasons for a different approach for net-ipmid
>
>      The primary design direction when developing net-ipmid was to reuse
> the provider libraries
>      for the commands that are common across the interfaces. It was a
> conscious decision
>      to have two processes for network & BT, keeping in mind to have
> small applications
>      rather than having monolithic ones.
>
>     The network handler would not be minimal like btbridge or KCSbridge,
> since there are net-ipmid
>     specific functionalities like session setup, serial over LAN..etc.

Unfortunately there is more common functionality than just handlers. For
example, Firmware Firewall is to be applied to all interfaces. Another example,
bridging allows messages to be taken from one interface, like the LAN Interface,
and applied on some other interface, like the System Interface.

I talked about this extensively with Corey Minyard (see discussion under [RFC v1
1/4] ipmi_bmc: framework for BT IPMI on BMCs). I was trying to create a common
interface that the userland could get IPMI messages from as well as a common way
to handle IPMI messages in the kernel. Unfortunately, the various concepts in
IPMI are just really too tightly coupled.

> The only difference i see in your
>     idea is we would relay the common commands on the dbus so that ipmid
> would handle.
>
> 2) The phosphor-host-ipmid was developed as part of the prototyping done
> for Barreleye. So i agree that
>      we have quite a number of shortcomings, that it is not in alignment
> with the modern C++ practices.
>      We have added more code and haven't got to refactor that. We have a
> story to refactor phosphor-host-ipmid.
>      The plan was to handle so some of these suggestions as part of that.
>
>     The support is in place to register each command's privilege. The
> support would be completed once the
>     IPMI user account management changes are in place. The command's
> privilege would matter only on
>     the LAN interface which is session based. In net-ipmid every command
> would be checked against the
>     session's privilege, so that a session with USER privilege would be
> restricted from running a command
>     that needs ADMIN privilege. Since KCS and BT are sessionless and
> unauthenticated, this would not matter.

Not quite true, see above.

>
>     Similarly System interface commands are compiled into a separate
> library which is not loaded in the
>     net-ipmid.
>
> 3) This is something we wanted to do, we can discuss what is the best
> possible approach. This would help with
>      command like Get/Set System Boot Configuration where each
> implementer would get the flexibility
>      to implement the OEM parameters.
>
>   Are you working on a OEM provider library?

We have written some in the past. I proposed one about a year ago. I think there
was another one as well. Might be time to revisit that.

>
> Regards,
> Tom
>
>
>
>
> On Thursday 12 October 2017 03:35 AM, Vernon Mauery 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?
> >     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.
> >
> >     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.
> >
> >     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).
> >
> >     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.
> >
> > 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).
> >
> >
> > 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