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

Some generated metadata items are not binary backward compatible #1905

Open
kcudnik opened this issue Sep 20, 2023 · 1 comment
Open

Some generated metadata items are not binary backward compatible #1905

kcudnik opened this issue Sep 20, 2023 · 1 comment
Assignees

Comments

@kcudnik
Copy link
Collaborator

kcudnik commented Sep 20, 2023

Those items:

  • struct sai_global_apis_t
  • enum sai_global_api_type_t
  • struct sai_switch_notifications_t
  • enum sai_switch_notification_type_t
  • struct sai_switch_pointers_t
  • enum sai_switch_pointer_type_t

are not binary backward compatible from sai version to version, because those are sorted when generated, but this mean if new global api will be added or new switch pointer/notification will be added, then it can end up in the middle of the struct/enum

This can cause potential issues, if we for example compile sonic syncd with one version of metadata headers, and then update only metadata library with other version, then shift in enums and pointers will cause unexpected behavior and lead to process crash

If we only update vendor libsai, we are fine since enums and structs are now backward binary compatible

Currently libsaimetadata is compiled with syncd together so this issue will not show up

For switch notification/pointer types, solution is pretty easy, just order enums/pointers in order they show up in SAI_SWITCH_ATTR_* and this will cause order to be backward compatible.

For global apis, the task is more complicated, since there is no easy way to figure out which api should be at the beginning and at the end. This would require to scrape git history each time, which is not optimal. Other solution would be to add extra TAG in doxyge comment for each global api (which could be enforced by sanity check) and this tag would be index number of each global api, and it would need to increase on each new api.

@kcudnik kcudnik self-assigned this Sep 25, 2023
kcudnik added a commit to kcudnik/SAI that referenced this issue Oct 10, 2023
kcudnik added a commit that referenced this issue Oct 14, 2023
@kcudnik
Copy link
Collaborator Author

kcudnik commented Nov 14, 2023

for sai_global_api_type_t also enum must be maintained, all places with GLOBAL_APIS needs to be revisited

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

1 participant