Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RemoteLUN!=0 is not supported #56

Closed
lukeyeager opened this issue May 11, 2023 · 5 comments · Fixed by #58
Closed

RemoteLUN!=0 is not supported #56

lukeyeager opened this issue May 11, 2023 · 5 comments · Fixed by #58

Comments

@lukeyeager
Copy link
Contributor

lukeyeager commented May 11, 2023

I am testing out a new version of a BMC. This library is giving different readings from ipmitool. In the changelog for this version, they claim that ipmitool>=1.8.18 is required because "the issues with the older ipmitool was that the LUN of the sensor was not passed around with the sensor number".

Might this library have a similar issue as the older versions of ipmitool which would explain the discrepancy? I would be very willing to believe that the behavior exhibited by this BMC and by ipmitool is not according to spec.

@lukeyeager lukeyeager closed this as not planned Won't fix, can't repro, duplicate, stale May 11, 2023
@lukeyeager lukeyeager reopened this May 11, 2023
@lukeyeager
Copy link
Contributor Author

lukeyeager commented May 11, 2023

Hmm. I still get the same value when using ipmitool 1.8.16 from ubuntu 16.04:

ipmitool 1.8.18 reading: 68
ipmitool 1.8.16 reading: 68
gebn/bmc        reading: 76

So now I'm less sure that this is related to that particular entry in the changelog.

My other theory is related to this comment. It looks to me (from a glance at ipmitool -vvv ... | grep 'command : 0x23) that ipmitool uses a lot of calls to Get Sensor Reading Factors to produce its result. Whereas this library references but does not implement GetSensorReadingFactorsRsp. Maybe the factors in the FSR are different from those returned by the Get Sensor Reading Factors query?

FWIW, here is the FSR for the sensor in question:
&ipmi.FullSensorRecord{BaseLayer:layers.BaseLayer{Contents:[]uint8{0x20, 0x1, 0x30, 0x9, 0x1, 0x7f, 0x68, 0x7, 0x1, 0x80, 0x72, 0x80, 0x72, 0x18, 0x18, 0x0, 0x6, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xff, 0x0, 0x0, 0xff, 0xff, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xcb, 0x50, 0x57, 0x52, 0x5f, 0x47, 0x42, 0x5f, 0x53, 0x58, 0x4d, 0x31}, Payload:[]uint8{}}, SensorRecordKey:ipmi.SensorRecordKey{OwnerAddress:0x20, Channel:0x0, OwnerLUN:0x1, Number:0x30}, ConversionFactors:ipmi.ConversionFactors{M:4, B:0, BExp:0, RExp:0}, IsContainerEntity:false, Entity:0x9, Instance:0x1, Ignore:false, SensorType:0x7, OutputType:0x1, AnalogDataFormat:0x0, RateUnit:0x0, IsPercentage:false, BaseUnit:0x6, ModifierUnit:0x0, Linearisation:0x0, Tolerance:0x0, Accuracy:0, AccuracyExp:0x0, Direction:0x0, NominalReadingSpecified:false, NormalMinSpecified:false, NormalMaxSpecified:false, NominalReading:0x0, NormalMin:0x0, NormalMax:0x0, SensorMin:0x0, SensorMax:0xff, Identity:"<REDACTED>"}

@lukeyeager
Copy link
Contributor Author

lukeyeager commented May 11, 2023

I think this line might be the problem: https://github.com/gebn/bmc/blob/455e4875b2/v2session.go#L155

But, when I hack it up to set RemoteLUN=1 when c.Name() == "Get Sensor Reading" I get an error - sensor reading is not available. How confident are you that this section is right? https://github.com/gebn/bmc/blob/455e4875b2/pkg/ipmi/message.go#L215

I only ask because if you've always hard-coded it to 0 then you may have never had the opportunity to test it. I'm trying to find the spot in the IPMI 2.0 spec which defines how these fields should be set, but haven't found it yet.

@lukeyeager
Copy link
Contributor Author

I have a patch locally that works. I added RemoteLUN() LUN to the ipmi.Command interface. All commands simply return LUNBMC except for GetSensorReadingCmd. It works on a new BMC and also on an old BMC. I am working on approval to submit patches to this repo. If you'd like to implement it yourself based on my description that's fine by me. Otherwise I'll send a PR whenever I'm approved to.

@lukeyeager lukeyeager changed the title reader.Read() returning different values from 'ipmitool sdr get' RemoteLUN!=0 is not supported May 12, 2023
@gebn
Copy link
Owner

gebn commented May 29, 2023

Thanks for this. So in summary, you have an FSR specifying the sensor owner is 0x1(OEM 1), and because we always assume 0x0(BMC command/event request message) in outbound messages, our Get Sensor Reading call retrieves the correct sensor number but for the wrong LUN?

But, when I hack it up to set RemoteLUN=1 when c.Name() == "Get Sensor Reading" I get an error - sensor reading is not available. How confident are you that this section is right? https://github.com/gebn/bmc/blob/455e4875b2/pkg/ipmi/message.go#L215

Can you confirm message serialising is absolved? Non-zero LUNs should be covered by the unit tests.

Maybe the factors in the FSR are different from those returned by the Get Sensor Reading Factors query?

This definitely shouldn't be the case for a linear sensor like this one. I think IPMItool's behaviour is somewhat brute-force and paranoid, probably due to some poorly-implemented BMCs that are hopefully no longer in use. Does IPMItool show the FSR and reading factors responses match here?

I'll take a look at your PR.

@lukeyeager
Copy link
Contributor Author

Your summary is correct. Here's a snippet from ipmitool -I lanplus ... sdr get <mysensor> -vvv showing that the RemoteLUN for a "Get Sensor Reading" (command 0x2d) is 1:

>> Sending IPMI command payload
>>    netfn   : 0x04
>>    command : 0x2d
BUILDING A v2 COMMAND
Local RqAddr 0x20 transit 0:0 target 0x20:0 bridgePossible 1
<< IPMI Response Session Header
<<   Authtype                : RMCP+
<<   Payload type            : IPMI (0)
<<   Session ID              : 0xa0a2a3a4
<<   Sequence                : 0x00000005
<<   IPMI Msg/Payload Length : 32
<< IPMI Response Message Header
<<   Rq Addr    : 81
<<   NetFn      : 05
<<   Rq LUN     : 0
<<   Rs Addr    : 20
<<   Rq Seq     : 05
<<   Rs Lun     : 1
<<   Command    : 2d
<<   Compl Code : 0x00

But, when I hack it up to set RemoteLUN=1 when c.Name() == "Get Sensor Reading" I get an error - sensor reading is not available.

Can you confirm message serialising is absolved? Non-zero LUNs should be covered by the unit tests.

I can't reproduce that error now. I'm not sure what I did wrong when I was hacking things up in my first attempt.

Maybe the factors in the FSR are different from those returned by the Get Sensor Reading Factors query?

This definitely shouldn't be the case for a linear sensor like this one.

You're right - that was a bad theory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants