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

add support for "sai_dbg_generate_dump" API #1423

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

aviramd
Copy link

@aviramd aviramd commented Sep 25, 2024

This update adds support for the sai_dbg_gen_dump API, enabling the generation of a debug dump file by the SAI when there is no SAI failure.

add support to the global API sai_dbg_generate_dump to the SaiInterface class.
add a new operation to the syncd to support SAI debug generate dump

HLD: sonic-net/SONiC#1846

@kcudnik
Copy link
Collaborator

kcudnik commented Oct 2, 2024

please make build success, after that i will review this

@kcudnik
Copy link
Collaborator

kcudnik commented Nov 5, 2024

please rebase to latest master

@aviramd
Copy link
Author

aviramd commented Nov 6, 2024

please rebase to latest master

Done

@aviramd
Copy link
Author

aviramd commented Nov 6, 2024

add HLD PR: sonic-net/SONiC#1846

@kcudnik
Copy link
Collaborator

kcudnik commented Nov 7, 2024

@aviramd can you fix whitespaces issue:

Checkig for white spaces ...
../proxylib/Sai.h:195:                  
../syncd/VendorSai.h:218:                    _In_ const char *dump_file_name) override;                    
../syncd/Syncd.cpp:412:    
../syncd/Syncd.cpp:434:        
../lib/ClientSai.cpp:1566:    
../lib/ClientSai.cpp:1568: 
../lib/Sai.cpp:788:    return SAI_STATUS_FAILURE; 
../lib/RedisRemoteSaiInterface.cpp:1923:    
../lib/RedisRemoteSaiInterface.cpp:1926: 
../lib/RedisRemoteSaiInterface.cpp:1932:        m_recorder->recordDbgGenDumpResponse(status);        
../lib/RedisRemoteSaiInterface.cpp:1936:    return SAI_STATUS_SUCCESS;  
../lib/Recorder.h:262:                    _In_ sai_status_t status);        
../lib/ClientServerSai.cpp:648:     
../lib/ClientServerSai.h:183:                    _In_ const char *dump_file_name) override;       
../lib/ServerSai.h:188:                    _In_ const char *dump_file_name) override;       
../lib/ServerSai.h:301:                    _In_ const swss::KeyOpFieldsValuesTuple &kco); 
../vslib/VirtualSwitchSaiInterface.h:196:                    _In_ const char *dump_file_name) override;               
../vslib/VirtualSwitchSaiInterface.h:197:                                    
../vslib/Sai.h:198:                    
../meta/SaiInterface.h:338:            
../meta/SaiInterface.h:341:                    
../meta/Meta.cpp:1427:    
ERROR: some files contain white spaces at the end of line, please fix
FAIL: checkwhitespace.sh

@kcudnik
Copy link
Collaborator

kcudnik commented Nov 7, 2024

You will also need to add some unit tests to satisfy code coverage

@aviramd aviramd force-pushed the SAI_DBG_GEN_DUMP_support branch from 0b1dee6 to c680f50 Compare November 24, 2024 14:32
@kcudnik
Copy link
Collaborator

kcudnik commented Nov 24, 2024

i hink you made some mistake, you only touch test file

@aviramd
Copy link
Author

aviramd commented Nov 24, 2024

i hink you made some mistake, you only touch test file

Yes you are right , i am working to fix it

This update introduces support for the sai_dbg_gen_dump API to generate
a debug dump file by the SAI
The general process is as follows:
The filename is written to configDB.
This action triggers the dbgGenDumpOrch to write the necessary data to ASIC DB.
The syncd process reads from ASIC DB and invokes the SAI.
SAI processes the request and returns a response.
syncd writes the response back to redis.
dbgGenDumpOrch retrieves the response and informs the caller by writing to configDB.
The caller then removes the data from configDB.
Signed-off-by: aviramd <[email protected]>
This reverts commit c680f50.
@kcudnik
Copy link
Collaborator

kcudnik commented Nov 24, 2024

you can try build this on your local machine to check whether it will compile first

@aviramd aviramd force-pushed the SAI_DBG_GEN_DUMP_support branch from b7bdc69 to a365865 Compare November 26, 2024 07:47
@aviramd
Copy link
Author

aviramd commented Nov 26, 2024

I have some git issues on my local machine
I am working to solve it , please ignore the last commit

@aviramd aviramd force-pushed the SAI_DBG_GEN_DUMP_support branch from a365865 to 75319be Compare November 26, 2024 09:03
@aviramd
Copy link
Author

aviramd commented Nov 26, 2024

Is it possible to execute the sairedis unit test locally? if yes how?

@kcudnik
Copy link
Collaborator

kcudnik commented Nov 27, 2024

Is it possible to execute the sairedis unit test locally? if yes how?

Yes, type make check in local console

@aviramd aviramd marked this pull request as draft November 28, 2024 14:00
@aviramd aviramd force-pushed the SAI_DBG_GEN_DUMP_support branch from 413ea6b to 29ea2bb Compare November 28, 2024 14:38
@aviramd aviramd force-pushed the SAI_DBG_GEN_DUMP_support branch from 6c22b65 to 4295cc6 Compare December 2, 2024 10:43
@aviramd aviramd force-pushed the SAI_DBG_GEN_DUMP_support branch 17 times, most recently from 08d8937 to df0f4de Compare December 4, 2024 09:17
@kcudnik
Copy link
Collaborator

kcudnik commented Dec 4, 2024

to avoid so many push commits please test everything locally
type "make" and "make check" to run unittests locally

@aviramd aviramd force-pushed the SAI_DBG_GEN_DUMP_support branch from df0f4de to e51ff47 Compare December 4, 2024 09:51
@aviramd
Copy link
Author

aviramd commented Dec 4, 2024

to avoid so many push commits please test everything locally type "make" and "make check" to run unittests locally

Is this the correct procedure for testing?

  1. Build the target.
  2. Run sonic-slave-bookworm-user.
  3. Inside the slave container, navigate to sonic/src/sonic-sairedis/tests.
  4. Run "make check".
    Should I install any packages after entering the slave Docker container?

Also, after building the target, a cleanup occurs, and the make file (which is auto-generated during the build) gets deleted. How can I prevent this from happening?

@aviramd aviramd force-pushed the SAI_DBG_GEN_DUMP_support branch from e51ff47 to 747da04 Compare December 4, 2024 12:50
@kcudnik
Copy link
Collaborator

kcudnik commented Dec 4, 2024

no, you don't need to build entire docker
you can locally ./configure --with-sai=vs locally
on your machine

@aviramd
Copy link
Author

aviramd commented Dec 11, 2024

no, you don't need to build entire docker you can locally ./configure --with-sai=vs locally on your machine

Thank you for your support! I tried running the command locally, but I encountered the following error when execute "make":

"libtool: compile: g++ ... -c AttrKeyMap.cpp -fPIC -DPIC -o .libs/libsaimeta_la-AttrKeyMap.o
In file included from AttrKeyMap.cpp:3:
sai_serialize.h:18:10: fatal error: swss/logger.h: No such file or directory"
Any advice on how to resolve this would be appreciated!

@kcudnik
Copy link
Collaborator

kcudnik commented Dec 12, 2024

Yes you need to install swss-common library also from source

@aviramd
Copy link
Author

aviramd commented Dec 12, 2024

no, you don't need to build entire docker
you can locally ./configure --with-sai=vs locally
on your machine

Thank you very much, I can run the tests now with 'make check'.
Is it also possible to test the code coverage locally? I ran the configure command with the --enable-code-coverage=yes option.
if yes, where can I see the results

@kcudnik
Copy link
Collaborator

kcudnik commented Dec 12, 2024

no, you don't need to build entire docker
you can locally ./configure --with-sai=vs locally
on your machine

Thank you very much, I can run the tests now with 'make check'. Is it also possible to test the code coverage locally? I ran the configure command with the --enable-code-coverage=yes option. if yes, where can I see the results

results are done in *gcno *gcna files, but they need to be processed, in azure pipeline gcovr results are colleted and then putt tohether, in pipeline output is done in json, and then module is preenting them in nice form on github, locally you can generate nice html, take a look gcovr help:
https://gcovr.com/en/stable/output/html.html

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

Successfully merging this pull request may close these issues.

2 participants