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

[syncd]: Enable port bulk API #1197

Merged
merged 4 commits into from
Feb 7, 2023
Merged

Conversation

nazariig
Copy link
Collaborator

@nazariig nazariig commented Jan 23, 2023

Signed-off-by: Nazarii Hnydyn nazariig@nvidia.com

HLD: sonic-net/SONiC#1084

Summary:

  1. Port bulk API support for syncd
  2. New test infrastructure for syncd
  3. New test infrastructure for VendorSai
  4. GCOVR parser hotfix: rollback to stable v5.0

@nazariig
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nazariig nazariig marked this pull request as ready for review January 27, 2023 13:27
@nazariig
Copy link
Collaborator Author

nazariig commented Jan 27, 2023

PR fails due to GCOVR errors:

+ gcovr --version
gcovr 5.2

Copyright (c) 2013-2022 the gcovr authors
Copyright (c) 2013 Sandia Corporation.
Under the terms of Contract DE-AC04-94AL85000 with Sandia Corporation,
the U.S. Government retains certain rights in this software.
+ find SAI/meta -name '*.gc*'
+ xargs rm -vf
removed 'SAI/meta/libsaimetadata_la-saimetadata.gcno'
removed 'SAI/meta/libsaimetadata_la-saimetadatautils.gcno'
removed 'SAI/meta/.libs/libsaimetadata_la-saimetadata.gcno'
removed 'SAI/meta/.libs/libsaimetadata_la-saimetadatautils.gcno'
removed 'SAI/meta/.libs/libsaimetadata_la-saimetadatautils.gcda'
removed 'SAI/meta/.libs/libsaimetadata_la-saimetadata.gcda'
removed 'SAI/meta/.libs/libsaimetadata_la-saiserialize.gcno'
removed 'SAI/meta/.libs/libsaimetadata_la-saiserialize.gcda'
removed 'SAI/meta/libsaimetadata_la-saiserialize.gcno'
+ gcovr -r ./ -e '.*/SAI/.*' -e .+/json.hpp -e swss/.+ -e '.*/.libs/.*' -e '.*/debian/.*' --exclude-unreachable-branches --exclude-throw-branches -x --xml-pretty -o coverage.xml
(WARNING) Unrecognized GCOV output for /__w/1/s/meta/SaiSerialize.cpp
	  function sai_serialize_object_id[abi:cxx11](unsigned long) called 7486270 returned NAN % blocks executed 62%
	This is indicative of a gcov output parse error.
	Please report this to the gcovr developers
	at <[https://github.com/gcovr/gcovr/issues>.](https://github.com/gcovr/gcovr/issues%3E.)
(WARNING) Exception during parsing:
	UnknownLineType: function sai_serialize_object_id[abi:cxx11](unsigned long) called 7486270 returned NAN % blocks executed 62%
(ERROR) Exiting because of parse errors.
	You can run gcovr with --gcov-ignore-parse-errors
	to continue anyway.
(ERROR) Trouble processing '/__w/1/s/meta/.libs/libsaimeta_la-SaiSerialize.gcda' with working directory '/__w/1/s/meta'.
Stdout of gcov was >>File 'SaiSerialize.cpp'
Lines executed:94.24% of 2100
Branches executed:93.37% of 3589
Taken at least once:53.39% of 3589
Calls executed:73.83% of 3076
Creating 'SaiSerialize.cpp##74fbe304ca213391ad9d6de89ec6b791.gcov'

This is a known bug: gcovr/gcovr#708
According to the GCOVR maintainers, it will be fixed as part of the upcoming release.

Committing a rollback to GCOV v5.0 as a W/A.

@nazariig
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nazariig
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nazariig
Copy link
Collaborator Author

@kcudnik / @saiarcot895 would you please have a look?

@nazariig
Copy link
Collaborator Author

@kcudnik / @saiarcot895 just a kind reminder

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
@nazariig
Copy link
Collaborator Author

nazariig commented Feb 2, 2023

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nazariig
Copy link
Collaborator Author

nazariig commented Feb 6, 2023

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nazariig
Copy link
Collaborator Author

nazariig commented Feb 6, 2023

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nazariig
Copy link
Collaborator Author

nazariig commented Feb 7, 2023

@saiarcot895 can you please help to merge this PR?

@saiarcot895 saiarcot895 merged commit a2c37b8 into sonic-net:master Feb 7, 2023
saiarcot895 added a commit to saiarcot895/sonic-swss-common that referenced this pull request Feb 8, 2023
The path to the sairedis test binary (for `setcap`) has changed after
sonic-net/sonic-sairedis#1197. Update the path here.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
liuh-80 pushed a commit to sonic-net/sonic-swss-common that referenced this pull request Feb 20, 2023
The path to the sairedis test binary (for `setcap`) has changed after
sonic-net/sonic-sairedis#1197. Update the path here.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
StormLiangMS pushed a commit that referenced this pull request Mar 7, 2023
* [syncd]: Enable port bulk API.

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>

* [syncd]: Add UT infra.

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>

* [gcovr]: Temporarily disable parse errors.

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>

* [gcovr]: Rollback to gcovr v5.0.

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>

---------

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
public:
virtual void SetUp() override
{
m_vsai = std::make_shared<VendorSai>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is not required in setup, this init and teardown should be in the test itself

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kcudnik having the same instruction in each test will lead to code duplication. The whole idea of using this fixture is to minimize amount of the identical code and let the user to focus on API testing. Also, here we want to force a cleanup when the test fails unexpectedly. Please elaborate on why it is a wrong approach?

liushilongbuaa pushed a commit to liushilongbuaa/sonic-swss-common that referenced this pull request Mar 10, 2023
The path to the sairedis test binary (for `setcap`) has changed after
sonic-net/sonic-sairedis#1197. Update the path here.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants