phosphor-host-ipmid and phosphor-net-ipmid architecture
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
Quoted 7 lines · 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?
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.
Quoted 4 lines
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.
Quoted 8 lines
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!
Quoted 4 lines
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?
Quoted 4 lines
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.
Quoted 12 lines
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_PRIOand then had some higher priorities such asMFR_PRIO, andOEM_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.
Quoted 8 lines
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
Quoted 12 lines · Brad Bishop
Quoted 7 lines · 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?
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.
Quoted 13 lines
Quoted 4 lines
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.
Quoted 4 lines
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.
Quoted 7 lines
Quoted 4 lines
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. :)
Quoted 5 lines
Quoted 4 lines
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).
Quoted 9 lines
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.
Quoted 30 lines
Quoted 4 lines
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.
Quoted 12 lines
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_PRIOand then had some higher priorities such asMFR_PRIO, andOEM_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.
Quoted 15 lines
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.
Quoted 8 lines
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
Quoted 18 lines · Xo Wang
Quoted 12 lines · Brad Bishop
Quoted 7 lines · 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?
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.
Quoted 8 lines
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.
Quoted 33 lines
Quoted 14 lines
Quoted 5 lines
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.
Quoted 4 lines
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.
Quoted 7 lines
Quoted 4 lines
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. :)
Quoted 9 lines
Quoted 4 lines
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.
Quoted 8 lines
Quoted 5 lines
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.
Quoted 22 lines · Brad Bishop
Quoted 22 lines · Xo Wang
Quoted 12 lines · Brad Bishop
Quoted 7 lines · 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?
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.
Quoted 6 lines
Quoted 4 lines
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.
Quoted 38 lines
Quoted 38 lines
Quoted 13 lines
Quoted 4 lines
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.
Quoted 4 lines
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.
Quoted 7 lines
Quoted 4 lines
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. :)
Quoted 14 lines
Quoted 4 lines
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?
Quoted 6 lines
Quoted 2 lines
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
Quoted 15 lines · Xo Wang
Quoted 12 lines · Brad Bishop
Quoted 7 lines · 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?
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.
Quoted 8 lines
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.
Quoted 18 lines
Quoted 18 lines
Quoted 4 lines
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.
Quoted 8 lines
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.
Quoted 4 lines
Quoted 2 lines
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.
Quoted 15 lines
Quoted 5 lines
Quoted 4 lines
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).
Quoted 9 lines
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?
Quoted 6 lines
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
Quoted 11 lines · Brad Bishop
Quoted 7 lines · 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?
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.
Quoted 8 lines
Quoted 4 lines
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.
Quoted 15 lines
Quoted 7 lines
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!
Quoted 4 lines
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).
Quoted 8 lines
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.
Quoted 29 lines
Quoted 3 lines
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.
Quoted 12 lines
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_PRIOand then had some higher priorities such asMFR_PRIO, andOEM_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.
Quoted 6 lines
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
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
Quoted 45 lines · 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_PRIOand then had some higher priorities such asMFR_PRIO, andOEM_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
Quoted 5 lines · tomjose
Hey Vernon,
1) There are few reasons for a different approach for net-ipmid
Quoted 7 lines
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. :)
Quoted 7 lines
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.
Quoted 6 lines
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.
Quoted 11 lines
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.
Quoted 3 lines
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.
Quoted 6 lines
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).
Quoted 52 lines
Regards,
TomQuoted 44 lines · 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 atDEFAULT_PRIOand then had some higher priorities such asMFR_PRIO, andOEM_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
Quoted 12 lines
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.
Quoted 3 lines
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.
Quoted 12 lines
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.
Quoted 10 lines
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.
Quoted 5 lines
Similarly System interface commands are compiled into a separate
library which is not loaded in the
net-ipmid.
Quoted 7 lines
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.
Quoted 53 lines
Regards,
TomQuoted 45 lines · 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_PRIOand then had some higher priorities such asMFR_PRIO, andOEM_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
Quoted 12 lines · On Thursday 19 October 2017 09:38 AM, [email protected] wrote:
Quoted 12 lines
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 29 lines
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.Quoted 12 lines
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.Quoted 10 lines
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.
Quoted 1 line
Not quite true, see above.
The privilege levels are for session based interfaces. I don't know any other than LAN.
Quoted 69 lines
Quoted 5 lines
Similarly System interface commands are compiled into a separate
library which is not loaded in the
net-ipmid.Quoted 7 lines
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.
Quoted 6 lines
Regards,
TomQuoted 46 lines
Quoted 45 lines · 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_PRIOand then had some higher priorities such asMFR_PRIO, andOEM_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