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] Implement bulk set support #1422

Merged
merged 11 commits into from
Dec 10, 2024

Conversation

stepanblyschak
Copy link
Contributor

@stepanblyschak stepanblyschak commented Sep 25, 2024

Added support for bulk set SAI API for port, PG and queue objects. This allows orchagent to configure those objects in bulk improving configuration time.

Signed-off-by: Stepan Blyschak <[email protected]>
@DanielaMurin
Copy link

Regarding the tests added in bulk_object.rec, I didn't explicitly see SAI_OBJECT_TYPE_TUNNEL, SAI_OBJECT_TYPE_NEXT_HOP, or SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER.
Are these object types included in the testing?

Thanks

syncd/Syncd.cpp Outdated Show resolved Hide resolved
@kcudnik
Copy link
Collaborator

kcudnik commented Oct 2, 2024

please satisfy code coverage

@stepanblyschak
Copy link
Contributor Author

stepanblyschak commented Nov 29, 2024

please satisfy code coverage

@kcudnik I added tests to tests/BCM56850/bulk_object.rec. I checked that the added code is covered. Does coverage report account for tests/BCM56850/bulk_object.rec?

@stepanblyschak
Copy link
Contributor Author

Regarding the tests added in bulk_object.rec, I didn't explicitly see SAI_OBJECT_TYPE_TUNNEL, SAI_OBJECT_TYPE_NEXT_HOP, or SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER. Are these object types included in the testing?

Thanks

Removed those, instead added PG, QUEUE objects and added test for them

@kcudnik
Copy link
Collaborator

kcudnik commented Nov 29, 2024

Regarding the tests added in bulk_object.rec, I didn't explicitly see SAI_OBJECT_TYPE_TUNNEL, SAI_OBJECT_TYPE_NEXT_HOP, or SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER. Are these object types included in the testing?
Thanks

Removed those, instead added PG, QUEUE objects and added test for them

It should, if not then you would need to add explicit unit tests

@stepanblyschak
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Stepan Blyschak <[email protected]>
kcudnik
kcudnik previously approved these changes Dec 3, 2024
@kcudnik
Copy link
Collaborator

kcudnik commented Dec 3, 2024

pleae fix build errors

Signed-off-by: Stepan Blyschak <[email protected]>
Signed-off-by: Stepan Blyschak <[email protected]>
@stepanblyschak
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stepanblyschak
Copy link
Contributor Author

Fails on:

Dockerfile:12
--------------------
  11 |     RUN dpkg --purge libswsscommon python3-swsscommon sonic-db-cli libsaimetadata libsairedis libsaivs syncd-vs swss sonic-eventd libdashapi
  12 | >>> RUN dpkg -i /debs/libdashapi_1.0.0_amd64.deb \
  13 | >>>             /debs/libswsscommon_1.0.0_amd64.deb \
  14 | >>>             /debs/python3-swsscommon_1.0.0_amd64.deb \
  15 | >>>             /debs/sonic-db-cli_1.0.0_amd64.deb \
  16 | >>>             /debs/libsaimetadata_1.0.0_amd64.deb \
  17 | >>>             /debs/libsairedis_1.0.0_amd64.deb \
  18 | >>>             /debs/libsaivs_1.0.0_amd64.deb \
  19 | >>>             /debs/syncd-vs_1.0.0_amd64.deb \
  20 | >>>             /debs/swss_1.0.0_amd64.deb
  21 |     
--------------------
ERROR: failed to solve: process "/bin/sh -c dpkg -i /debs/libdashapi_1.0.0_amd64.deb             /debs/libswsscommon_1.0.0_amd64.deb             /debs/python3-swsscommon_1.0.0_amd64.deb             /debs/sonic-db-cli_1.0.0_amd64.deb             /debs/libsaimetadata_1.0.0_amd64.deb             /debs/libsairedis_1.0.0_amd64.deb             /debs/libsaivs_1.0.0_amd64.deb             /debs/syncd-vs_1.0.0_amd64.deb             /debs/swss_1.0.0_amd64.deb" did not complete successfully: exit code: 1

##[error]Bash exited with code '1'.

I see same failure on other PRs.

@kcudnik
Copy link
Collaborator

kcudnik commented Dec 4, 2024

1.635 Configuration file '/etc/swss/config.d/netbouncer.json', does not exist on system.
1.

this must be some recent change to swss or swss common

@stepanblyschak
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stepanblyschak
Copy link
Contributor Author

@kcudnik Can we merge?

@kcudnik
Copy link
Collaborator

kcudnik commented Dec 10, 2024

pleae provide description why this change is made

@stepanblyschak
Copy link
Contributor Author

@kcudnik done

@kcudnik kcudnik merged commit 98bb52e into sonic-net:master Dec 10, 2024
15 checks passed
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.

4 participants