#1697 Wrong sensor number may be in ipmidirect events

[David McKinley]

When the ipmidirect plugin creates RDRs for sensors found in SDRs, it creates a "virtual sensor number" to use for the sensor in the RDR. Typically, this is the same value as the sensor number found in the SDR, but if there are multiple sensors found in the SDR with the same sensor number (could happen if they have different LUNs, for example) then the "virtual sensor number" used in the RDR will be changed for one of them.

The problem is that when events are generated (both regular sensor events and sensor enable change events), the code puts the SDR sensor number in the HPI event instead of the virtual sensor number. It should use the virtual sensor number, since that is how the sensor is known to the HPI user.

[Anton Pak]

There is one more issue with the numbers.
IPMI Direct allows sensor virtual number be in range [0,255).
So the maximum number of IPMI sensors for a FRU resource is 256.
However IPMI limit is 768.

[Anton Pak]

And I witnessed ATCA board with 200 sensors.
So the limit is likely to be reached in the near future.

[David McKinley]

Hi Anton,

Poking around in the code a little more, I see where the limitation comes from - in CreateSensorNum() in ipmi_resource.cpp. With a cursory look, it doesn't appear that it would be very difficult to raise the number of sensors supported. In fact, it looks to me like the m_sensor_num array in the resource object is not used for anything other than this assignment of a virtual sensor number. So, it could either be expanded to 768, or maybe even 1024 entries - or, if is not seen as desirable to use that much memory, with a bit more coding, it could be turned into a bit map of 1024 bits that simply flag which sensor numbers have been assigned. Then it would take just 32 bytes, rather than the 256 words (1K bytes) it is using now.

David

From: Anton Pak [mailto:avpak@users.sf.net]
Sent: Monday, June 08, 2015 1:32 PM
To: [openhpi:bugs]
Subject: [openhpi:bugs] #1697 Wrong sensor number may be in ipmidirect events

And I witnessed ATCA board with 200 sensors.
So the limit is likely to be reached in the near future.

---

HYPERLINK "http://sourceforge.net/p/openhpi/bugs/1697"[bugs:#1697] Wrong sensor number may be in ipmidirect events

Status: open
3.6.0: Future
Labels: IPMI Direct plugin
Created: Wed Feb 22, 2012 06:58 PM UTC by David McKinley
Last Updated: Fri Jul 13, 2012 05:21 AM UTC
Owner: nobody

When the ipmidirect plugin creates RDRs for sensors found in SDRs, it creates a "virtual sensor number" to use for the sensor in the RDR. Typically, this is the same value as the sensor number found in the SDR, but if there are multiple sensors found in the SDR with the same sensor number (could happen if they have different LUNs, for example) then the "virtual sensor number" used in the RDR will be changed for one of them.

The problem is that when events are generated (both regular sensor events and sensor enable change events), the code puts the SDR sensor number in the HPI event instead of the virtual sensor number. It should use the virtual sensor number, since that is how the sensor is known to the HPI user.

---

Sent from sourceforge.net because you indicated interest in HYPERLINK "https://sourceforge.net/p/openhpi/bugs/1697"https://sourceforge.net/p/openhpi/bugs/1697/

To unsubscribe from further messages, please visit HYPERLINK "https://sourceforge.net/auth/subscriptions"https://sourceforge.net/auth/subscriptions/

[dr_mohan]

Thanks to David for the following comments

Ignoring the issue Anton brought up in the bug report about the possibility of having more than 256 IPMI sensors in a FRU, the fix is quite simple. Although I agree that a limit of 256 would be at least a theoretical problem, I wonder if any current ipmi-based platforms actually have more sensors than that in a FRU. That's a lot of sensors. Also, I'm not sure where that limitation comes from, though perhaps Anton knows. The data type for the m_virtual_num variable is 'unsigned int'.

Regardless of that, however: in ipmi_sensor.cpp, at lines 706 and 744 in the 3.0 source, there are these statements in the CreateEnableChangeEvent() and CreateEvent() functions, respectively:

706: se->SensorNum = m_num;
744: s->SensorNum = m_num;

They should be changed to:

706: se->SensorNum = m_virtual_num;
744: s.SensorNum = m_virtual_num;

[dr_mohan]

Memory of the systems are always going up. Looks like the array size could be increased to 768 or 1024 and the above two lines could be changed.

[dr_mohan]

Could some one who has hardware to test ipmidirect plugin test the attached patch and post the results? Let's know if the patch looks good.