Skip to content

Update get_acl_couner with command instead of getting value from redis#5616

Merged
ZhaohuiS merged 2 commits intosonic-net:masterfrom
ZhaohuiS:get_acl_counter
May 9, 2022
Merged

Update get_acl_couner with command instead of getting value from redis#5616
ZhaohuiS merged 2 commits intosonic-net:masterfrom
ZhaohuiS:get_acl_counter

Conversation

@ZhaohuiS
Copy link
Contributor

@ZhaohuiS ZhaohuiS commented May 7, 2022

Description of PR

Summary:
Fixes # (issue)
We can't get acl counters from redis COUNTERS_DB due to this RP sonic-net/sonic-swss#1943.
Parse the output of aclshow to get counters.

Signed-off-by: Zhaohui Sun zhaohuisun@microsoft.com

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911
  • 202012

Approach

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

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
@ZhaohuiS ZhaohuiS requested a review from a team as a code owner May 7, 2022 06:24
result = duthost.show_and_parse('aclshow')

if len(result) == 0:
return 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the "0" expected behavior? If not, could we some add debug/error print here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@StormLiangMS If there is no rule for acl, the output looks like below, result is empty.
We return 0 here. The log file includes all the output of shell command.

admin@str-dx010-acs-5:~$ aclshow -a
RULE NAME    TABLE NAME    PRIO    PACKETS COUNT    BYTES COUNT
-----------  ------------  ------  ---------------  -------------

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@StormLiangMS Rethinking about your comment, you are correct. get_acl_counter is called after setup acl rules, so if can't find any expected rule, fail the test case.

for rule in result:
if rule_name == rule['rule name']:
return int(rule['packets count'])
return 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's done in my second commit.

Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
@ZhaohuiS ZhaohuiS merged commit 06e0926 into sonic-net:master May 9, 2022
yaqiangz pushed a commit to yaqiangz/sonic-mgmt that referenced this pull request Nov 2, 2022
…value from redis

Update get_acl_couner with command instead of getting value from redis.

sonic-net#5616 was merged into internal, but currently, dualtor 202205 pipelines are running based on internal-202012 branch, so this fix should be cherry picked into internal-202012 branch to avoid test failure.

Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
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