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

[show] invalid acl table output with single port #1498

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

d-dashkov
Copy link
Contributor

- What I did
Fixed the bug "Observing the binding field is not proper while executing "show acl table" command"
fixes sonic-net/SONiC#305

- How I did it
Added changes to the function show_table in acl_lader.
Added the checking of a value type for the ports. If the value is a string then we print it as is in other cases we split and sort the values.

- How to verify it
Follow the description in the bug.

1.Create an acl :

"ACL_TABLE": {
	"1": {
		"stage": "INGRESS",
		"type": "MIRROR",
		"policy_desc": "Mirror Session",
		"ports": "Ethernet16"
	}
}
  1. $ config reload -y
  2. $ show acl table

- Previous command output (if the output of a command-line utility has changed)

Name    Type    Binding    Description     Stage
------  ------  ---------  --------------  -------
1       MIRROR  1          Mirror Session  ingress
                6
                E
                e
                e
                h
                n
                r
                t
                t

- New command output (if the output of a command-line utility has changed)

  Name  Type    Binding     Description     Stage
------  ------  ----------  --------------  -------
     1  MIRROR  Ethernet16  Mirror Session  ingress

Signed-off-by: d-dashkov <Dmytro_Dashkov@Jabil.com>
@lguohan lguohan requested a review from daall March 17, 2021 15:19
Copy link
Contributor

@daall daall left a comment

Choose a reason for hiding this comment

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

See question. Also, please add a new unit test case(s) to tests/acl_loader_test.py.

@@ -706,7 +706,10 @@ def show_table(self, table_name):
if not val["ports"]:
data.append([key, val["type"], "", val["policy_desc"], stage])
else:
ports = natsorted(val["ports"])
if isinstance(val['ports'], str):
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried the case where "ports" is a comma-separated string like "Ethernet0,Ethernet1,..."? Does this come back as a string or a list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@daall
I`ve tried cases with different acl command and they all work fine:

#sudo config acl add table test_1 L3 -p Ethernet0,Ethernet4 -s ingress
"test_1": {
	"policy_desc": "Mirror Session",
	"ports": [
		"Ethernet0",
		"Ethernet4"
	],
	"stage": "INGRESS",
	"type": "MIRROR"
}

#sudo config acl add table test_2 L3 -p "Ethernet0,Ethernet4" -s ingress
"test_2": {
	"policy_desc": "Mirror Session",
	"ports": [
		"Ethernet0",
		"Ethernet4"
	],
	"stage": "INGRESS",
	"type": "MIRROR"
}

#sudo config acl add table test_single L3 -p Ethernet0 -s ingress
"test_single": {
	"policy_desc": "Mirror Session",
	"ports": [
		"Ethernet0"
	],
	"stage": "INGRESS",
	"type": "MIRROR"
}

This bug happens only when I manually write the wrong value in config db.json, as this stated in issue(instead of list, write single str port)

"1": {
	"stage": "INGRESS",
	"type": "MIRROR",
	"policy_desc": "Mirror Session",
	"ports": "Ethernet16" <= incorrect value
}

stepanblyschak pushed a commit to stepanblyschak/sonic-utilities that referenced this pull request Apr 28, 2022
7f50b9815e14d90c02d9dce63fd08d90e25cee3f (HEAD -> 201911, origin/201911) handled update() function of fdb orchagent for FDB FLUSH event (sonic-net#1534)
17adc13b6ca21846fe27c94d6a16f9909c712d77 Add a check for warm-restart, and do a clear only when warm-restart is enable. (sonic-net#1498)
d097260a5aa7bd611babd5062e220056374e23d8 Fixed compilation failure with debug option (sonic-net#1518)

Signed-off-by: Abhishek Dosi <abdosi@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.

Observing the binding field is not proper while executing "show acl table" command
2 participants