bmcweb icon indicating copy to clipboard operation
bmcweb copied to clipboard

MessageId version format does not comply with the spec.

Open ifel opened this issue 1 year ago • 1 comments

Is this the right place to submit this?

  • [X] This is not a security vulnerability or a crashing bug
  • [X] This is not a question about how to use OpenBMC
  • [X] This is not a bug in an OpenBMC fork or a bug in code still under code review.
  • [X] This is not a request for a new feature.

Bug Description

Hello,

The "9.5.11.2 MessageId format" section of the spec (https://www.dmtf.org/sites/default/files/standards/documents/DSP0266_1.21.0.pdf) describes message id format as: "<MessageRegistryPrefix>.<MajorVersion>.<MinorVersion>.<MessageKey>"

However, the current implementation does not follow this format, and instead uses 3 component version like in this case: "MessageId": "Base.1.19.0.QueryParameterOutOfRange".

My understanding is, the version comes from the registry version (https://github.com/openbmc/bmcweb/blob/master/scripts/parse_registries.py#L247) as it is, without converting format.

Before actually starting to do anything on this front, I want to learn whether it was a deliberate decision, and what would be the blast radius of changing this to comply with the spec (i.e. removing the 3rd part of the version).

Version

ID=openbmc-fb-lf
NAME="Facebook OpenBMC"
VERSION="yosemite4-v2024.41.1"
VERSION_ID=yosemite4-v2024.41.1
VERSION_CODENAME="master"
PRETTY_NAME="Facebook OpenBMC yosemite4-v2024.41.1"
CPE_NAME="cpe:/o:openembedded:openbmc-fb-lf:yosemite4-v2024.41.1"
OPENBMC_TARGET_MACHINE="yosemite4"
EXTENDED_VERSION="yosemite4-v2024.41.1"

Additional Information

No response

ifel avatar Oct 24 '24 15:10 ifel

CC: @edtanous

ifel avatar Oct 28 '24 13:10 ifel

@jmbills Any thoughts here?

edtanous avatar Nov 06 '24 16:11 edtanous

We might have a mix of 2- and 3-component versions. All my OpenBMC MessageIds use only the 2-component version. If I remember correctly, we don't use the version for anything yet, so it may not have any impact to make the change. It might also help to have a pointer to an example where the code would change to get a better idea of the scope.

jmbills avatar Nov 06 '24 16:11 jmbills

We might have a mix of 2- and 3-component versions. All my OpenBMC MessageIds use only the 2-component version. If I remember correctly, we don't use the version for anything yet, so it may not have any impact to make the change. It might also help to have a pointer to an example where the code would change to get a better idea of the scope.

Right, but as the 3-component version breaks the specification, we need to get rid of them and use only 2-component ones. If you don't have objections to this, I will start working on replacing 3-component versions with 2-component ones.

I will appreciate any code pointers!

ifel avatar Nov 06 '24 18:11 ifel

I don't have objections to making this change. The code that I know of that searches the registry looks like it expects the 2-component version: https://github.com/openbmc/bmcweb/blob/master/redfish-core/src/registries.cpp#L34.

Where do you see the 3-component version that is incorrect? If it's only in the Redfish return data, it may be this code: https://github.com/openbmc/bmcweb/blob/master/redfish-core/include/registries.hpp#L90.

jmbills avatar Nov 06 '24 18:11 jmbills

https://gerrit.openbmc.org/c/openbmc/bmcweb/+/76180

Might be the change we need?

edtanous avatar Dec 02 '24 18:12 edtanous

Great! Thank you very much, it looks like it indeed!

ifel avatar Dec 18 '24 12:12 ifel