Describe the bug
When more than 8 temperature sensors are present for a DCMI sensor category, ipmitool dcmi sensors duplicates the 8th sensor entry
IPMITOOL Version
$ ipmitool -V
ipmitool version 1.8.19.59.gd62a996
To Reproduce
Steps to reproduce the behavior:
- Build the latest code from the current master branch.
- Connect to a BMC that reports more than 8 sensors in a DCMI temperature sensor category.
- Run
ipmitool -I lanplus -H <bmc-ip> -U <user> -P <password> dcmi sensors - Observe the abnormal reply:
Baseboard: 10 temperature sensors found:
Record ID 0x000a: TEMP01 | 22 degrees C | ok
Record ID 0x0014: TEMP02 | 20 degrees C | ok
Record ID 0x0015: TEMP03 | 20 degrees C | ok
Record ID 0x0017: TEMP04 | 22 degrees C | ok
Record ID 0x0018: TEMP05 | 47 degrees C | ok
Record ID 0x0019: TEMP06 | 45 degrees C | ok
Record ID 0x001a: TEMP07 | 46 degrees C | ok
Record ID 0x001b: TEMP08 | 45 degrees C | ok
Record ID 0x001b: TEMP08 | 45 degrees C | ok
Record ID 0x001c: TEMP09 | 21 degrees C | ok
Record ID 0x001d: TEMP10 | 21 degrees C | ok
Expected behavior
The sensor list should continue from the 9th sensor without duplicating the 8th entry:
Record ID 0x001a: TEMP07
Record ID 0x001b: TEMP08
Record ID 0x001c: TEMP09
Record ID 0x001d: TEMP10
No duplicate sensor entries should be reported.
Additional context
While investigating the issue, I noticed that the duplicated entry seems to be related to the offset used in ipmi_dcmi_prnt_discvry_snsr(), which is passed directly as the DCMI request's Entity Instance Start field:
// ipmi_dcmi_discvry_snsr()
msg_data[0] = IPMI_DCMI; /* Group Extension Identification */
msg_data[1] = 0x01; /* Senser Type = Temp (01h) */
msg_data[2] = isnsr; /* Sensor Number */
msg_data[3] = 0x00; /* Entity Instance, set to read all instances */
msg_data[4] = offset; /* Entity instance start */
According to DCMI v1.5, Table 6-14, DCMI Entity Instance values are defined as:
0x01 ... n
and Table 6-15 defines request byte 5 as:
Entity Instance Start
Used with Entity Instance 00h for # of instance exceeding one IPMI Response
Since offset is used as the request's Entity Instance Start, the first request currently uses:
Entity Instance = 00h
Entity Instance Start = 00h
while the DCMI Entity Instance numbering appears to start from 1.
In my test environment, initializing the offset to 1 instead of 0 avoids the duplicated 8th entry:
~/ipmitool (master)$ git diff
diff --git a/lib/ipmi_dcmi.c b/lib/ipmi_dcmi.c
index 90db253..b1702d6 100644
--- a/lib/ipmi_dcmi.c
+++ b/lib/ipmi_dcmi.c
@@ -1355,7 +1355,7 @@ ipmi_dcmi_prnt_discvry_snsr(struct ipmi_intf * intf, uint8_t isnsr)
struct ipmi_rs * rsp; /* ipmi response */
uint8_t records = 0;
int8_t instances = 0;
- uint8_t offset = 0;
+ uint8_t offset = 1;
uint16_t record_id = 0;
uint8_t id_buff[16]; /* enough for 8 record IDs */
rsp = ipmi_dcmi_discvry_snsr(intf, isnsr, 0);
After this change, the sensor list continues from TEMP09 instead of repeating TEMP08.
I can submit a pull request if this is considered the correct fix.
**Describe the bug**
When more than 8 temperature sensors are present for a DCMI sensor category, `ipmitool dcmi sensors` duplicates the 8th sensor entry
**IPMITOOL Version**
```none
$ ipmitool -V
ipmitool version 1.8.19.59.gd62a996
```
**To Reproduce**
Steps to reproduce the behavior:
1. Build the latest code from the current master branch.
2. Connect to a BMC that reports more than 8 sensors in a DCMI temperature sensor category.
3. Run `ipmitool -I lanplus -H <bmc-ip> -U <user> -P <password> dcmi sensors`
4. Observe the abnormal reply:
```none
Baseboard: 10 temperature sensors found:
Record ID 0x000a: TEMP01 | 22 degrees C | ok
Record ID 0x0014: TEMP02 | 20 degrees C | ok
Record ID 0x0015: TEMP03 | 20 degrees C | ok
Record ID 0x0017: TEMP04 | 22 degrees C | ok
Record ID 0x0018: TEMP05 | 47 degrees C | ok
Record ID 0x0019: TEMP06 | 45 degrees C | ok
Record ID 0x001a: TEMP07 | 46 degrees C | ok
Record ID 0x001b: TEMP08 | 45 degrees C | ok
Record ID 0x001b: TEMP08 | 45 degrees C | ok
Record ID 0x001c: TEMP09 | 21 degrees C | ok
Record ID 0x001d: TEMP10 | 21 degrees C | ok
```
**Expected behavior**
The sensor list should continue from the 9th sensor without duplicating the 8th entry:
```none
Record ID 0x001a: TEMP07
Record ID 0x001b: TEMP08
Record ID 0x001c: TEMP09
Record ID 0x001d: TEMP10
```
No duplicate sensor entries should be reported.
**Additional context**
While investigating the issue, I noticed that the duplicated entry seems to be related to the `offset` used in `ipmi_dcmi_prnt_discvry_snsr()`, which is passed directly as the DCMI request's `Entity Instance Start` field:
```cpp
// ipmi_dcmi_discvry_snsr()
msg_data[0] = IPMI_DCMI; /* Group Extension Identification */
msg_data[1] = 0x01; /* Senser Type = Temp (01h) */
msg_data[2] = isnsr; /* Sensor Number */
msg_data[3] = 0x00; /* Entity Instance, set to read all instances */
msg_data[4] = offset; /* Entity instance start */
```
According to DCMI v1.5, Table 6-14, DCMI Entity Instance values are defined as:
```none
0x01 ... n
```
and Table 6-15 defines request byte 5 as:
> Entity Instance Start
> Used with Entity Instance 00h for # of instance exceeding one IPMI Response
Since offset is used as the request's `Entity Instance Start`, the first request currently uses:
```none
Entity Instance = 00h
Entity Instance Start = 00h
```
while the DCMI Entity Instance numbering appears to start from 1.
In my test environment, initializing the `offset` to 1 instead of 0 avoids the duplicated 8th entry:
```diff
~/ipmitool (master)$ git diff
diff --git a/lib/ipmi_dcmi.c b/lib/ipmi_dcmi.c
index 90db253..b1702d6 100644
--- a/lib/ipmi_dcmi.c
+++ b/lib/ipmi_dcmi.c
@@ -1355,7 +1355,7 @@ ipmi_dcmi_prnt_discvry_snsr(struct ipmi_intf * intf, uint8_t isnsr)
struct ipmi_rs * rsp; /* ipmi response */
uint8_t records = 0;
int8_t instances = 0;
- uint8_t offset = 0;
+ uint8_t offset = 1;
uint16_t record_id = 0;
uint8_t id_buff[16]; /* enough for 8 record IDs */
rsp = ipmi_dcmi_discvry_snsr(intf, isnsr, 0);
```
After this change, the sensor list continues from TEMP09 instead of repeating TEMP08.
I can submit a pull request if this is considered the correct fix.