Repository navigation
Read each sensor from its owner: Send Message bridging, LUN and Device SDRs (#84, #105) - #192
Conversation
SendMessage carries any command to a controller behind the BMC (Send Message with response tracking, IPMI 2.0 sections 6.13.3 and 22.7): the IPMB message is addressed to the controller, keeps the remote console as requester and the sequence number of the Send Message request, as ipmitool lanplus does. The empty response to Send Message itself does not end the exchange: the response of the controller follows under the same tag, or comes in the data of that response on some BMCs. To let a command wait past such a reply, the reply matching of IpmiMessageHandler moves to IpmiCommandCoder.answers(), which SendMessage overrides. GetSensorReading and GetSensorReadingFactors take the LUN of the sensor owner. GetDeviceSdrInfo, GetDeviceSdr and ReserveDeviceSdrRepository read the SDRs a Sensor Device holds itself (section 35). Refs #84, #105 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sensors() sent every Get Sensor Reading to LUN 0 of the BMC, which answered for its own sensor of that number when the record named another owner: the BIOS sensors of Dell iDRACs (owner B1h, a software ID) got the states of unrelated BMC sensors, and the fan, power supply, NIC and drive sensors of an HPE iLO 5 (satellite controllers 44h, 2Ch, 28h) got CBh or another sensor's reading. Now a sensor of the BMC is read at the LUN of its record, a sensor of another controller through the BMC with Send Message, and a sensor of system software not at all (UNAVAILABLE, with its problem). A controller that does not answer costs its own sensors only: a lost reply to Send Message does not lose the session, and the walk does not wait for that controller again. A BMC that refuses Get SDR Repository Info and reports Device SDRs without SDR Repository in Get Device ID is walked with Get Device SDR, as ipmitool does. Live: iLO 5 bridged at User level (50 sensors), the GIGABYTE AMI BMC bridges to the Intel ME (2Ch, channel 6) at Administrator only. No lab BMC has a sensor at another LUN or lacks an SDR repository: those paths are covered by the fake BMC, which now models a controller behind the BMC, the request LUN and Device SDRs. Fixes #84 Fixes #105 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sensors.md explains how each owner is read and the privilege Send Message needs (Operator by the specification, User on iLO 5, Administrator on an AMI BMC); low-level-api.md shows SendMessage to the Intel ME; the supported commands, the privilege tables, the CLI page, troubleshooting and upgrading.md follow. Refs #84, #105 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 318a8a2493
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A Send Message request without reply kept the session whatever went wrong, so a BMC that stopped answering while only bridged sensors were left gave a list of unavailable sensors instead of a lost session and its replay. SendMessage now records whether the BMC answered Send Message itself; only then is a missing response that of the controller. Without the count of the Device SDRs (Get Device SDR Info refused or malformed), "record not present" to the first record ends the walk with no record instead of failing it; the same holds for a repository whose Get SDR Repository Info is refused. GetDeviceSdrInfo rejects a dynamic population without its change indicator, and README.md gives the privilege the sensors of other controllers need. Refs #84, #105 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db5145001b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The acknowledgment of the BMC is reset each time SendMessage is encoded, so that the last try decides: a BMC that acknowledged an earlier try and then stopped answering loses the session instead of passing for a silent controller. A bridged non-linear sensor whose controller answers Get Sensor Reading but not Get Sensor Reading Factors loses its conversion only: the controller is no longer taken for silent for the rest of the walk. GetDeviceSdrInfo takes the LUN to address, for the sensor count of each LUN (IPMI 2.0 table 35-2). Refs #84, #105 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3548401fea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| acknowledged = true; | ||
| return reply.getData() != null; | ||
| } | ||
| return command.answers(reply); |
There was a problem hiding this comment.
When a satellite responds after its request has timed out and the 6-bit tag is later reused during a long sensor walk, this branch accepts the stale reply for another SendMessage whenever the carried command and network function match. Because it ignores the reply's responder address and LUN, a delayed Get Sensor Reading response from one controller can become another controller's sensor value; match the carried response to the target controller and LUN as well.
AGENTS.md reference: AGENTS.md:L42-L42
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changed. The spec (IPMI 2.0 section 6.13.4) lets the BMC answer a bridged request with its own address (20h) or the controller's, and asks the console to accept both. The iLO 5 in our lab answers with 20h for every controller, so the responder address cannot tell two controllers apart there, and a LUN check would rely on BMCs preserving fields the spec does not ask them to keep. A stale reply would also have to outlive the 5-second IPMB pending-response expiry of the BMC and a full cycle of the 48 tags, the same window every command has, direct ones included. If that window ever matters, the fix belongs in the tag allocation of the message queue, not in Send Message.
🤖 Addressed by Claude Code
…ation A controller that does not answer at one LUN no longer hides the sensors of its other LUNs for the rest of the walk, and the problem names the LUN when it is not 0. A device with a dynamic sensor population may lose its records between Get Device SDR Info and the first Get Device SDR: "record not present" to the first record then ends the walk with no record, as when the count is unknown. low-level-api.md says that only a reply lost after the BMC acknowledged Send Message keeps the session. Refs #84, #105 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex please review the last commit that you haven't reviewed yet |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Fixes #84. Fixes #105.
Each sensor is now read from the owner its SDR record names (IPMI 2.0 section 5.4), where the library sent every Get Sensor Reading to LUN 0 of the BMC.
20h, channel 0)CBhUNAVAILABLE, no states,problem()says whyProtocol core
SendMessage<R>(App34h) carries any command to a controller behind the BMC with response tracking (40h | channel), the IPMB message addressed to the controller, requester81hand the same sequence number as the Send Message request, asipmitoollanplus does. The BMC answers Send Message at once and forwards the controller's response under that sequence number; some BMCs put it in their response to Send Message instead: both are accepted.describeCompletionCode()names the Send Message codes80h-83h.IpmiCommandCoder.answers(IpmiLanMessage): the reply matching ofIpmiMessageHandlermoved to the coder, so thatSendMessagecan wait past the empty response to Send Message itself.GetSensorReadingandGetSensorReadingFactors.GetDeviceSdrInfo(S/E20h),GetDeviceSdr(S/E21h,80hrecord changed decoded as a canceled reservation),ReserveDeviceSdrRepository(S/E22h).Client
sensors()routes each reading as in the table above. A controller that does not answer costs its own sensors only: a lost reply to Send Message never marks the session lost, and the other sensors of that controller are not requested again during the same operation.ipmitoolrule) is walked with Get Device SDR.Privilege
Send Message to another channel needs Operator (table G-1, note 2). Live: the HPE iLO 5 allows it at User, the AMI-based GIGABYTE BMC only at Administrator (
D4hat User and Operator). Below the level the BMC requires, the bridged sensors come back without reading with theD4hinproblem(); the BMC's own sensors are unaffected. Documented insensors.md,configuration.md,cli.md,supported-commands.md.Live results (lab, 2026-10-16)
Survey of the SDR owners on 36 cards:
B1h(system software) on every card.mainreturned for them the state of the BMC sensor with the same number (e.g.Memory Mirrored=VCore PG); the branch returns themUNAVAILABLEwithThe sensor belongs to system software (owner ID B1h), not to a controller: it has no reading.44h(ChasMC: fans, power supplies),2Ch(embedded NIC) and28h(embedded RAID: drive status).maingotCBhor another sensor's value; the branch reads them all through the BMC at User level (separate-reply style), with consistent values (PS 1 output 80 W ≈ 12 V × 6.6 A, input 100 W ≈ 227 V × 0.4 A).2Chon channel 6 works as Administrator (Get Device ID: manufacturer 343, firmware 4.44, reply from2Ch).20h, channel 0, LUN 0: unchanged path.C1hto Get Device SDR Info).Tests
The fake BMC models a controller behind it (
satellite(), reply separate, embedded or never,83hfor an unknown address), the request LUN, and Device SDRs. New tests: each sensor read from its owner (both reply styles), a silent and an absent controller, a BMC walked through its Device SDRs. Mutation-checked: ignoring the empty Send Message response, losing the session on a bridged timeout, dropping the LUN and skipping the Device SDR rule each fail a test.🤖 Generated with Claude Code