ipmi_exporter icon indicating copy to clipboard operation
ipmi_exporter copied to clipboard

bmcInfoFirmwareRevisionRegex is missing the patch part of semantic versionning

Open fbouynot opened this issue 1 year ago • 2 comments

Hi, we're working with the following supermicro BMC output:

$ sudo ipmitool bmc info 
Device ID                 : 32
Device Revision           : 1
Firmware Revision         : 3.10
IPMI Version              : 2.0
Manufacturer ID           : 10876
Manufacturer Name         : Supermicro
Product ID                : 6970 (0x1b3a)
Product Name              : Unknown (0x1B3A)
Device Available          : yes
Provides Device SDRs      : no
Additional Device Support :
    Sensor Device
    SDR Repository Device
    SEL Device
    FRU Inventory Device
    IPMB Event Receiver
    IPMB Event Generator
    Chassis Device
Aux Firmware Rev Info     : 
    0x38
    0x01
    0x00
    0x00

We can see it is matched like by the following regex: https://github.com/prometheus-community/ipmi_exporter/blob/master/freeipmi/freeipmi.go#L45C2-L45C30

It does match only major.minor: image

Although I would like to get major.minor.patch: 3.10.38 here We can see the patch version is the first number printed after the Aux Firmware Rev Info line, we tried on multiples servers with different versions.

I could build an updated regex that would match the patch part if it exists, like this: image

^Firmware Revision\s*:\s*(?P<value>[0-9.]*).*$(?:\n(?:^(?!Aux Firmware Rev Info).*$\n)*Aux Firmware Rev Info.*\n\s*0x(?P<patch>[0-9]*))?

Although I'm not sure how to change the file https://github.com/prometheus-community/ipmi_exporter/blob/master/freeipmi/freeipmi.go to do a PR myself, because it's not only changing the regex since there is a new var, it would require to change at least GetBMCInfoFirmwareRevision too.

Could someone work on this feature request, or maybe help me to understand how to work on this by myself if you don't have time to do this?

fbouynot avatar Jun 26 '24 14:06 fbouynot

Hello, and thank you for your suggestion. I am open to making this data available, though I have some reservations about the exact approach.

First, this may be besides the point, but I'd like to point out:

Although I would like to get major.minor.patch: 3.10.38 here

The first byte of the Aux Firmware Rev Info is 0x38, a hex value that would translate to version 3.10.56. Just as a precaution, if this is really the value you are looking for.

Next, I am not sure what the semantics of those values really are, and I suspect it differs from vendor to vendor? At any rate, if anything, these values should be exported as a new metric, but not be mixed with the existing Firmware Revision value. I suppose one could export each byte as separate metric or such, to make using the values easy.

bitfehler avatar Jul 09 '24 12:07 bitfehler

Hello,

The first byte of the Aux Firmware Rev Info is 0x38, a hex value that would translate to version 3.10.56. Just as a precaution, if this is really the value you are looking for.

I think you are right, but supermicro is doing supermicro things : the hexadecimal value here is appended as a decimal patch number. Capture d’écran du 2024-07-09 17-48-41 Capture d’écran du 2024-07-09 17-48-58

these values should be exported as a new metric, but not be mixed with the existing Firmware Revision value

I agree with you, your approach looks safer.

Is this something you can do? Or do you have time to give me something that could help me to start? (doc or similar PR)

Thank you

fbouynot avatar Jul 09 '24 15:07 fbouynot