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

MessageId version format does not comply with the spec. #293

Open
4 tasks done
ifel opened this issue Oct 24, 2024 · 6 comments
Open
4 tasks done

MessageId version format does not comply with the spec. #293

ifel opened this issue Oct 24, 2024 · 6 comments

Comments

@ifel
Copy link
Contributor

ifel commented Oct 24, 2024

Is this the right place to submit this?

  • This is not a security vulnerability or a crashing bug
  • This is not a question about how to use OpenBMC
  • This is not a bug in an OpenBMC fork or a bug in code still under code review.
  • 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:
"..."

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
Copy link
Contributor Author

ifel commented Oct 28, 2024

CC: @edtanous

@edtanous
Copy link
Contributor

edtanous commented Nov 6, 2024

@jmbills Any thoughts here?

@jmbills
Copy link
Contributor

jmbills commented Nov 6, 2024

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.

@ifel
Copy link
Contributor Author

ifel commented Nov 6, 2024

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!

@jmbills
Copy link
Contributor

jmbills commented Nov 6, 2024

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.

@edtanous
Copy link
Contributor

edtanous commented Dec 2, 2024

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

Might be the change we need?

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

No branches or pull requests

3 participants