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

[AclOrch] move ACL counters to flex counter infrastructure #1943

Merged
merged 12 commits into from
Nov 11, 2021

Conversation

stepanblyschak
Copy link
Contributor

@stepanblyschak stepanblyschak commented Oct 6, 2021

What I did

Added ACL Flex Counter group. Re-implemented ACL counters implemention to work with Flex Counter Infrastructure.

Why I did it

To improve orchagent performance when large amount of ACL rules are configured.

How I verified it

Together with depends PRs. Run ACL/Everflow test suite.

Details if related

DEPENDS ON: sonic-net/sonic-swss-common#533 sonic-net/sonic-sairedis#953
HLD: sonic-net/SONiC#857

Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 6, 2021

This pull request introduces 1 alert when merging 4eef944 into da49332 - view on LGTM.com

new alerts:

  • 1 for Unused import

Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
@liat-grozovik
Copy link
Collaborator

@bingwang-ms can you please help review?

@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

orchagent/aclorch.cpp Outdated Show resolved Hide resolved
orchagent/aclorch.cpp Outdated Show resolved Hide resolved
orchagent/aclorch.cpp Outdated Show resolved Hide resolved
@bingwang-ms
Copy link
Contributor

Reviewed and added several comments. Thanks

Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
bingwang-ms
bingwang-ms previously approved these changes Oct 19, 2021
@bingwang-ms
Copy link
Contributor

The change LGTM now. Thanks

@lgtm-com
Copy link

lgtm-com bot commented Nov 8, 2021

This pull request introduces 1 alert and fixes 1 when merging f422612 into e14a071 - view on LGTM.com

new alerts:

  • 1 for Syntax error

fixed alerts:

  • 1 for Inconsistent definition of copy constructor and assignment ('Rule of Two')

Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 8, 2021

This pull request introduces 1 alert and fixes 1 when merging f6d7881 into 70da9af - view on LGTM.com

new alerts:

  • 1 for Syntax error

fixed alerts:

  • 1 for Inconsistent definition of copy constructor and assignment ('Rule of Two')

Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2021

This pull request fixes 1 alert when merging 5d670fa into fe5b2a9 - view on LGTM.com

fixed alerts:

  • 1 for Inconsistent definition of copy constructor and assignment ('Rule of Two')

Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2021

This pull request fixes 1 alert when merging 8365820 into 05c7c05 - view on LGTM.com

fixed alerts:

  • 1 for Inconsistent definition of copy constructor and assignment ('Rule of Two')

@bingwang-ms bingwang-ms merged commit 5f8ebfa into sonic-net:master Nov 11, 2021
qiluo-msft pushed a commit to sonic-net/sonic-utilities that referenced this pull request Nov 15, 2021
#### What I did

Made a change for aclshow and counterpoll that adds support for ACL flex counters.

DEPENDS ON: sonic-net/sonic-swss-common#533 sonic-net/sonic-sairedis#953 sonic-net/sonic-swss#1943
HLD: sonic-net/SONiC#857

#### How I did it

Modified aclshow and counterpoll and UT.

#### How to verify it

Together with depends PRs. Run ACL/Everflow test suite.
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
What I did
Moved the "populate_mock" method to dump/helper.py i.e. common to all the tests
Moved the "add_to_ret_template" to Executor class i.e. common across all the modules.
How I did it
How to verify it
Unit Tests's:
ZhaohuiS added a commit to sonic-net/sonic-mgmt that referenced this pull request May 9, 2022
#5616)

What is the motivation for this PR?
We can't get acl counters from redis COUNTERS_DB due to this RP sonic-net/sonic-swss#1943.
test_acl_outer_vlan.py keeps failing because of the error Failed: Failed to retrieve acl counter for DATAACL_ingress_ipv6|rule_1.

How did you do it?
Parse the output of aclshow to get counters.

How did you verify/test it?
run acl/test_acl_outer_vlan.py

Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
malletvapid23 added a commit to malletvapid23/Sonic-Utility that referenced this pull request Aug 3, 2023
#### What I did

Made a change for aclshow and counterpoll that adds support for ACL flex counters.

DEPENDS ON: sonic-net/sonic-swss-common#533 sonic-net/sonic-sairedis#953 sonic-net/sonic-swss#1943
HLD: sonic-net/SONiC#857

#### How I did it

Modified aclshow and counterpoll and UT.

#### How to verify it

Together with depends PRs. Run ACL/Everflow test suite.
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.

3 participants