Bug 206640

Summary: ipmi_register_smi implementation does not match definition since 2.6.9-42
Product: Red Hat Enterprise Linux 4 Reporter: BYTRON <bugzilla>
Component: kernelAssignee: Peter Martuccelli <peterm>
Status: CLOSED WONTFIX QA Contact: Brian Brock <bbrock>
Severity: medium Docs Contact:
Priority: medium    
Version: 4.4   
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-06-20 15:56:34 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description BYTRON 2006-09-15 13:37:51 UTC
Hi guys,

Since upgrading one of our machines to U4 a third party kernel module failed to
build.

linux-2.6.9/include/linux/ipmi_smi.h defines the following: -

int ipmi_register_smi(struct ipmi_smi_handlers *handlers,
                      void                     *send_info,
                      unsigned char            version_major,
                      unsigned char            version_minor,
                      ipmi_smi_t               *intf);

A patch present in kernel-2.6.9-34.0.2 (linux-2.6.9-openipmi-update.patch)
modifies this to be: -

int ipmi_register_smi(struct ipmi_smi_handlers *handlers,
                      void                     *send_info,
                      unsigned char            version_major,
                      unsigned char            version_minor,
                      unsigned char            slave_addr,
                      ipmi_smi_t               *intf);

It seems like the definition of this function has stayed at the 6 arg version in
the transition to U4, but the implementation in drivers/char/ipmi/ipmi_msghandler.c

Changes from this (correct) in kernel-2.6.9-34.0.2: -

int ipmi_register_smi(struct ipmi_smi_handlers *handlers,
                      void                     *send_info,
                      unsigned char            version_major,
                      unsigned char            version_minor,
                      unsigned char            slave_addr,
                      ipmi_smi_t               *intf)

To this (broken) in kernel-2.6.9-42 & kernel-2.6.9-42.0.2: -

int ipmi_register_smi(struct ipmi_smi_handlers *handlers,
                      void                     *send_info,
                      unsigned char            version_major,
                      unsigned char            version_minor,
                      unsigned char            slave_addr)

It looks like our third party driver author has developed using the 6 arg
version, hence the module not building since the move to U4; not to mention the
implementation no longer matching the definition.

Comment 1 Peter Martuccelli 2006-09-15 14:36:28 UTC
I will check on the U4 code base to see if the differences exits between the
msghandler and the smi include files.  A quick look at the patch files and a
tree I had used for U4 development does not show any discrepancy between the two
files, they both have the args defined as follows:

int ipmi_register_smi(struct ipmi_smi_handlers *handlers,
                      void                     *send_info,
                      unsigned char            version_major,
                      unsigned char            version_minor,
                      unsigned char            slave_addr)

The patch went into -35, so I would expect that the E4 security branch with a
revision of 2.6.9-34.0.2.EL would not have the change, (and still be using the
six argument call).

I would say that the six argument version was not carried into U4 at this time,
and that the implementation and the definition do indeed match.  The third party
developer will need to make the required changes for U4 GA to make the call
using the five argument interface.  This is a gray area as the exported symbols
are for the IPMI drivers private interface, as such they are not preserved
across updates.

Comment 2 BYTRON 2006-09-18 09:06:44 UTC
I’ve just double checked this with a fresh tree and your right, the definition
and implementation do indeed match, I must have had some old files laying around :-S

Regarding the change from 6 args to 5 args, our third party developer is
claiming that their module failing to build is RedHats fault for changing the
interface between the updates. This is not my opinion, as the rest of the world
seemed to cope fine with the transition to U4.

Am I correct in assuming that if they have developed their module using the
public interface rather than the private interface they would not have run into
this problem when U4 was released?

If you have any further reasons why they should not be using that private
interface (and the time to post them) I’d be grateful to have the information
for the next time I speak with them.


Thanks for your help,
Adam

Comment 3 Peter Martuccelli 2006-11-13 16:50:53 UTC
I would say we have the following reasons why the third party driver developer
will need to change their code:

1 - IPMI is viewed as a subsystem, changes are made to the exported symbols to
keep the IPMI drivers in sync, and to keep RHEL in sync with upstream in regards
to important bug and security fixes.  We have preserved the /proc interface, so
the user space API code will work without any changes.

2 - The third party driver code will not work upstream without making the
required changes.

3 - The driver code should be submitted upstream, (2.6.19), to the OpenIPMI site
and/or to LKML, to preserve the IPMI additions/modifications.  This will allow
RHEL IPMI to work out of the box on your systems.

I hope this is of help to you.  





Comment 4 BYTRON 2007-01-23 09:37:22 UTC
OK, thanks for your help. I'm happy to have this bug closed (but it won't let me
do it).

Comment 5 Jiri Pallich 2012-06-20 15:56:34 UTC
Thank you for submitting this issue for consideration in Red Hat Enterprise Linux. The release for which you requested us to review is now End of Life. 
Please See https://access.redhat.com/support/policy/updates/errata/

If you would like Red Hat to re-consider your feature request for an active release, please re-open the request via appropriate support channels and provide additional supporting details about the importance of this issue.