Skip to contentSkip to conversation

Discussion on IPMI provider libraries

DISCUSSION tomjose 3 participants 6 messages Nov 7, 2016 – Nov 16, 2016

lore
Share & tools
lore

tomjose

author
#0 lore raw

Hello Brendan,

The discussion is with respect to issue https://github.com/openbmc/openbmc/issues/430.

The goal of this issue is to share ipmi provider libraries between phosphor-host-ipmid and phosphor-net-ipmid.

The plan is to provide a framework in net-ipmid to register IPMI provider API's. Every IPMI command has a privilege level which would be compared against the session's privilege level before the command is executed. For supporting IPMI on LAN channel, the privilege for each command need to be assigned.

This is the present signature of the callback command API's.

typedef ipmi_ret_t (*ipmid_callback_t)(ipmi_netfn_t, ipmi_cmd_t, ipmi_request_t,
ipmi_response_t,
ipmi_data_len_t, ipmi_context_t);

I plan to add additional parameter for the command privilege level. I heard you had interest around this area to modify callback API to suit your specific needs.

Let me know your thoughts.

Regards,
Tom

#1 lore raw

Sharing the provider libraries makes sense; my first area of concern is the API; I am currently working on a change to the API (see
https://gerrit.openbmc-project.xyz/#/c/841/ for details); I would prefer you do not make any changes to the current API, but understand if the need arises before my change is ready.

Could you elaborate on how you plan on enforcing privilege? Having each provider check privilege level seems like a leaky abstraction to me; I think it would make more sense to have privilege managed by the host-ipmid and the net-ipmid.

As far as the actual details concerning phosphor-net-ipmid: I do not have strong opinions on the matter as Google has no intention of using IPMI over LAN at this time, but would welcome discussion on the matter nonetheless.

Cheers

author
#2 lore raw
Quoted 1 line · Brendan Higgins

Sharing the provider libraries makes sense; my first area of concern is the API; I am currently working on a change to the API (see https://gerrit.openbmc-project.xyz/#/c/841/ <https://gerrit.openbmc-project.xyz/#/c/841/> for details); I would prefer you do not make any changes to the current API, but understand if the need arises before my change is ready.

From what i have noticed in the patch, there is support for ipmid_callback_t handlers as it is now. So the change in API is to accommodate the OEM group ? So do you have plans to change the callback signatures for the standard commands already implemented in host-ipmid?

Quoted 2 lines

Could you elaborate on how you plan on enforcing privilege? Having each provider check privilege level seems like a leaky abstraction to me; I think it would make more sense to have privilege managed by the host-ipmid and the net-ipmid.

Table G - Command Number Assignments and Privilege Levels in the IPMI specification gives more details on this.

Each command is assigned a privilege level( Callback, User, Operator, Admin) which means that the command can be executed only on a session with this privilege or higher. So if a command needs be executed on net-ipmid path, one of the attribute needed for net-ipmid is the command's privilege level.
The privilege provided by each command is a registration parameter and it is consumed only by net-ipmid.

As part of the same issue, i am separating commands that need to be executed from system interface as a separate library.

The provider libraries is now copied into /usr/lib/host-ipmid. The plan is to have the /usr/lib/ipmid-providers as the default install location for all providers and then symlink into /usr/lib/host-ipmid and /usr/lib/net-ipmid depending on whether the provider library is needed in out-of-band or in-band path.

Quoted 4 lines

As far as the actual details concerning phosphor-net-ipmid: I do not have strong opinions on the matter as Google has no intention of using IPMI over LAN at this time, but would welcome discussion on the matter nonetheless.

Cheers

#3 lore raw
Quoted 6 lines

From what i have noticed in the patch, there is support for ipmid_callback_t handlers as it is now. So the change in API is to accommodate the OEM group ?
So do you have plans to change the callback signatures for the standard commands already implemented in host-ipmid?

The support for ipmid_callback_t is only for backwards compatibility. Part of the change in the API was to accommodate OEM/Group extensions, yes; however, the intention is also to provide a cleaner API in general.

The privilege provided by each command is a registration parameter and it

Quoted 1 line

is consumed only by net-ipmid.

That's fine, but in that case it should not go in the callback; it should be maintained and enforced by net-ipmid when it looks up a handler.

Cheers

#4 lore raw
Quoted 6 lines · Brendan Higgins
Quoted 2 lines

The privilege provided by each command is a registration parameter and it is consumed only by net-ipmid.

That's fine, but in that case it should not go in the callback; it should be maintained and enforced by net-ipmid when it looks up a handler.

Neither net-ipmid nor host-ipmid intrinsically know all of the IPMI commands that will or may be registered. This is especially true for OEM commands where the privilege level is determined by the command.

Are the privilege levels defined by the IPMI spec? If so, I don't see anything incorrect about each provider having it. If not, it is something that we have defined at build time, correct? Would it be acceptable to have multiple symlink locations for net-ipmid providers? ie. /usr/lib/phosphor-net-ipmid/user/ , /usr/lib/phosphor-net-ipmid/admin/ , etc. I suspect we would need to break libraries up more because currently a single library provides commands at different privilege levels.

Signature · 1 line
Patrick Williams
author
#5 lore raw
Quoted 9 lines · Patrick Williams
Quoted 5 lines · Brendan Higgins
Quoted 2 lines

The privilege provided by each command is a registration parameter and it is consumed only by net-ipmid.

That's fine, but in that case it should not go in the callback; it should be maintained and enforced by net-ipmid when it looks up a handler.

Neither net-ipmid nor host-ipmid intrinsically know all of the IPMI commands that will or may be registered. This is especially true for OEM commands where the privilege level is determined by the command.

Agree.

Quoted 10 lines

Are the privilege levels defined by the IPMI spec? If so, I don't see anything incorrect about each provider having it. If not, it is something that we have defined at build time, correct? Would it be acceptable to have multiple symlink locations for net-ipmid providers? ie. /usr/lib/phosphor-net-ipmid/user/ , /usr/lib/phosphor-net-ipmid/admin/ , etc. I suspect we would need to break libraries up more because currently a single library provides commands at different privilege levels.

My earlier mail got stripped off when Brendan replied which had details about the privilege levels.

The Privilege levels are defined by the IPMI specification. Table G - Command Number Assignments and Privilege Levels in the IPMI specification gives more details on this.

Every command has a privilege level assigned to the command( Admin, User, Operator or Callback) or the command is system interface only.

The plan is to break down the system interface commands into a separate library. The apphandler library in phospor-host-ipmid had support for Get Message Flags, Set BMC Global Enables and Read Event Message Buffer which are System Interface commands only. So that net-ipmid would not load the system interface commands. In a similar way if an OEM command need not be loaded for net-ipmid, then no symlink would be provided for that library in /usr/lib/phosphor-net-ipmid.

I have pushed a patch for separating system interface commands: https://gerrit.openbmc-project.xyz/#/c/1020/

I did not completely understand the rationale behind having multiple symlink locations for net-ipmid providers. The phosphor-net-ipmid would load the provider libraries at starting and net-ipmid would allow multiple sessions with different privilege levels. There is a 'Set Session Privilege Level' command which would change the privilege level to a higher( based on the maximum privilege supported for the user) or lower privilege level.

So the suggestion for multiple symlink locations for net-ipmid providers(net-ipmid/user.. net-ipmid/admin) is a way for net-ipmid to figure out the privilege level of the command instead of a registration parameter(privilege level) in the callback handler?