Skip to content

Bulk get/set for switches #2174

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shri-khare
Copy link
Contributor

@shri-khare shri-khare commented Jun 2, 2025

Bulk get/set for switches

@shri-khare shri-khare force-pushed the bulk-get-set-switch branch 2 times, most recently from 95af4c8 to 0ea3df1 Compare June 2, 2025 00:26
@shri-khare shri-khare marked this pull request as ready for review June 2, 2025 21:12
Copy link
Contributor

@JaiOCP JaiOCP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good

@kcudnik
Copy link
Collaborator

kcudnik commented Jun 3, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kcudnik
Copy link
Collaborator

kcudnik commented Jun 3, 2025

Processing saiwred_8h.xml
ERROR: invalid object type SAI_OBJECT_TYPE_SWITCHE extracted from bulk definition: sai_bulk_object_set_attribute_fn       set_switches_attribute
ERROR: invalid object type SAI_OBJECT_TYPE_SWITCHE extracted from bulk definition: sai_bulk_object_get_attribute_fn       get_switches_attribute

please fix build error, take a look what other bulk functions are declared, optionally since object type is extracted from name like "routes" -> "route" then from "switches" -> "switche" and it should be "switch", parser needs to be fixed in this place to correct extract object name, please fix parse.pl somewhere about here: https://github.com/opencomputeproject/SAI/blob/master/meta/parse.pl#L4453, you can add explicit check if its SWITCHE then replace that to SWITCH

@shri-khare shri-khare force-pushed the bulk-get-set-switch branch from 0ea3df1 to 8f262b4 Compare June 4, 2025 19:43
@shri-khare
Copy link
Contributor Author

Thanks @kcudnik.

I am unable to locally run make in the meta to reproduce this error, which makes it harder to iterate while fixing. In the past, I could get around this issue by compiling and using old doxygen 1.8.13, but this time around, that did not work either.

Is this workflow documented anywhere? Here the error I hit:

[/local/github/SAI.git/meta: (bulk-get-set-switch)]$ doxygen -V
1.11.0
with sqlite3 3.42.0.

[/local/github/SAI.git/meta: (bulk-get-set-switch)]$ make
GNU Make 4.3
gcc (GCC) 11.5.0 20240719 (Red Hat 11.5.0-5)
This is perl 5, version 32, subversion 1 (v5.32.1) built for x86_64-linux-thread-multi
@(#) International Ispell Version 3.1.20 (but really Aspell 0.60.8)
dot - graphviz version 2.44.0 (0)
doxygen: 1.11.0
GNU nm (GNU Binutils) 2.37
doxygen Doxyfile 2>&1 | perl -npe '$e=1 if /warning/i; END{exit $e}'
warning: Tag 'HTML_TIMESTAMP' at line 1133 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'FORMULA_TRANSPARENT' at line 1413 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'DOT_FONTNAME' at line 2079 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'DOT_FONTSIZE' at line 2086 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'DOT_TRANSPARENT' at line 2280 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
make: *** [Makefile:118: xml] Error 1

Also, pls note, that I have revised the PR with additional code changes.
It should continue to fail with the error though given the code still uses "switches".

@shri-khare
Copy link
Contributor Author

shri-khare commented Jun 4, 2025

Thanks @kcudnik.

I am unable to locally run make in the meta to reproduce this error, which makes it harder to iterate while fixing. In the past, I could get around this issue by compiling and using old doxygen 1.8.13, but this time around, that did not work either.

Is this workflow documented anywhere? Here the error I hit:

[/local/github/SAI.git/meta: (bulk-get-set-switch)]$ doxygen -V
1.11.0
with sqlite3 3.42.0.

[/local/github/SAI.git/meta: (bulk-get-set-switch)]$ make
GNU Make 4.3
gcc (GCC) 11.5.0 20240719 (Red Hat 11.5.0-5)
This is perl 5, version 32, subversion 1 (v5.32.1) built for x86_64-linux-thread-multi
@(#) International Ispell Version 3.1.20 (but really Aspell 0.60.8)
dot - graphviz version 2.44.0 (0)
doxygen: 1.11.0
GNU nm (GNU Binutils) 2.37
doxygen Doxyfile 2>&1 | perl -npe '$e=1 if /warning/i; END{exit $e}'
warning: Tag 'HTML_TIMESTAMP' at line 1133 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'FORMULA_TRANSPARENT' at line 1413 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'DOT_FONTNAME' at line 2079 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'DOT_FONTSIZE' at line 2086 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'DOT_TRANSPARENT' at line 2280 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
make: *** [Makefile:118: xml] Error 1

@shri-khare shri-khare force-pushed the bulk-get-set-switch branch 2 times, most recently from 4c1dfb7 to dadd821 Compare June 5, 2025 01:55
@shri-khare
Copy link
Contributor Author

/azp run

Copy link

Commenter does not have sufficient privileges for PR 2174 in repo opencomputeproject/SAI

@tjchadaga
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@shri-khare
Copy link
Contributor Author

/azpw run

@shri-khare shri-khare force-pushed the bulk-get-set-switch branch from dadd821 to 33722d4 Compare June 5, 2025 15:53
Signed-off-by: Shrikrishna (Shri) Khare <[email protected]>
@shri-khare shri-khare force-pushed the bulk-get-set-switch branch from 33722d4 to ae5963d Compare June 5, 2025 15:53
@shri-khare
Copy link
Contributor Author

/azpw run

@tjchadaga
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tjchadaga tjchadaga added the reviewed PR is discussed in SAI Meeting label Jun 6, 2025
@kcudnik
Copy link
Collaborator

kcudnik commented Jun 9, 2025

can you update your local doxygen ? seems like your is too old and dont support some tags

@kcudnik
Copy link
Collaborator

kcudnik commented Jun 9, 2025

In file included from ../inc/sai.h:64,
                 from saisanitycheck.c:28:
../inc/saiswitch.h:4067:1: error: no semicolon at end of struct or union [-Werror]
 4067 | } sai_switch_api_t;
      | ^
cc1: all warnings being treated as errors
make: *** [Makefile:142: saisanitycheck.o] Error 1

@shri-khare
Copy link
Contributor Author

@kcudnik

I built and used the latest doxygen, but local make in meta continues to fail.

$ doxygen --version
1.15.0 (babbc98591ee150adb0e57c5a7f38c4ad95c5624)
$ cd /github/sai.git/meta
$ make
GNU Make 4.3
gcc (GCC) 11.5.0 20240719 (Red Hat 11.5.0-5)
This is perl 5, version 32, subversion 1 (v5.32.1) built for x86_64-linux-thread-multi
@(#) International Ispell Version 3.1.20 (but really Aspell 0.60.8)
dot - graphviz version 2.44.0 (0)
doxygen: 1.15.0 (babbc98591ee150adb0e57c5a7f38c4ad95c5624)
GNU nm (GNU Binutils) 2.37
doxygen Doxyfile 2>&1 | perl -npe '$e=1 if /warning/i; END{exit $e}'
warning: Tag 'HTML_TIMESTAMP' at line 1133 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'FORMULA_TRANSPARENT' at line 1413 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'DOT_FONTNAME' at line 2079 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'DOT_FONTSIZE' at line 2086 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
warning: Tag 'DOT_TRANSPARENT' at line 2280 of file 'Doxyfile' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
make: *** [Makefile:118: xml] Error 1

@kcudnik
Copy link
Collaborator

kcudnik commented Jun 12, 2025

then new support needs to be added for that version, since some of the fields in config are obsolete and causing warnings

@kcudnik
Copy link
Collaborator

kcudnik commented Jun 12, 2025

as a quick fix you can comment those configs in Doxygen and Doxygen.compat file which are causing errors

@tjchadaga
Copy link
Collaborator

@shri-khare - were you able to try the workaround that Kamil suggested? Please let us know if this is still an issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reviewed PR is discussed in SAI Meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants