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

Fail on ACL table creation #9406

Closed
bratashX opened this issue Nov 30, 2021 · 12 comments
Closed

Fail on ACL table creation #9406

bratashX opened this issue Nov 30, 2021 · 12 comments
Assignees
Labels
Triaged this issue has been triaged

Comments

@bratashX
Copy link
Contributor

bratashX commented Nov 30, 2021

Description

Creation ACL table failed because action_list for this table is empty.
Due to sonic-net/sonic-swss#1982, action list will be used from ACL_TABLE_TYPE objects.
Looks like in defaults ACL_TABLE_TYPE are absent action lists: https://github.com/Azure/sonic-swss/blob/bb0733aa67ffc4e430e70bcf2db2dc6316172b32/orchagent/aclorch.cpp#L2752-L2768.

Steps to reproduce the issue:

  1. Install latest SONiC master.
  2. config acl add table DATA_INGRESS_IPV4_TEST L3 -s ingress -p Ethernet8

Describe the results you received:

error appears on orchagent validation when create ACL table from CLI

'Nov 29 21:32:08.170178 igk-4-dut ERR swss#orchagent: :- validate: Action list for table DATA_INGRESS_IPV4_TEST is mandatory\n', 'Nov 29 21:32:08.170178 igk-4-dut ERR swss#orchagent: :- doAclTableTask: Failed to create ACL table DATA_INGRESS_IPV4_TEST, invalid configuration\n']

Describe the results you expected:

no errors in logs appears

Output of show version:

SONiC Software Version: SONiC.master.54827-dirty-20211127.211353
Distribution: Debian 11.1
Kernel: 5.10.0-8-2-amd64
Build commit: 6f8dbaca7
Build date: Sat Nov 27 21:22:18 UTC 2021
Built by: AzDevOps@sonic-build-workers-000XMH```
#### Output of `show techsupport`:


@prsunny
Copy link
Contributor

prsunny commented Dec 1, 2021

@stepanblyschak to take a look

@bingwang-ms
Copy link
Contributor

Ack. Will take a look shortly.

@bingwang-ms
Copy link
Contributor

bingwang-ms commented Dec 2, 2021

Hi, can I know on which platform you saw this error? I verified on a broadcom platform, but could't repro the issue

Dec  2 07:09:30.481601 str2-7260cx3-acs-9 NOTICE swss#orchagent: :- bindAclTable: Bind table DATA_INGRESS_IPV4_TEST to ports
Dec  2 07:09:30.483991 str2-7260cx3-acs-9 NOTICE swss#orchagent: :- bind: Successfully bound port oid: 1000000000024, group member oid:c000000001773
Dec  2 07:09:30.483991 str2-7260cx3-acs-9 NOTICE swss#orchagent: :- addAclTable: Created ACL table DATA_INGRESS_IPV4_TEST oid:7000000001772

The image I was running is

SONiC Software Version: SONiC.202106.54946-1ebe52847
Distribution: Debian 10.11
Kernel: 4.19.0-12-2-amd64
Build commit: 1ebe52847
Build date: Sun Nov 28 14:43:59 UTC 2021
Built by: AzDevOps@sonic-build-workers-000XPS

because I can't find a successful build of 6f8dbac.

@kperumalbfn
Copy link
Contributor

@bingwang-ms I don't think 202106 has these changes. Could you check in master?

@bingwang-ms
Copy link
Contributor

I'm able to repro this issue on a barefoot device.

redis-cli -n 6 hgetall "ACL_STAGE_CAPABILITY_TABLE|INGRESS"
1) "is_action_list_mandatory"
2) "true"
3) "action_list"
4) "DO_NOT_NAT_ACTION,PACKET_ACTION,MIRROR_INGRESS_ACTION"

The action_list is marked as mandatory for INGRESS stage, while the default action_list is empty.
@stepanblyschak Can you help take a look?

@stepanblyschak
Copy link
Collaborator

@bingwang-ms @bratashX Default ACL tables do not have action list, this is not changed comparing to previous releases. There is a new check added that if "is_action_list_mandatory" is true it is required to pass action list. It looks like vendor SAI returns "is_action_list_mandatory" true while it is not required as I assume in previous release the ACL table creation is working for you. It looks like vendor SAI issue.

@akokhan
Copy link
Contributor

akokhan commented Dec 7, 2021

@stepanblyschak , as you've mentioned, SONiC behavior has changed and it's not obvious (even HLD mentioned this). To keep previous behavior, shouldn't we warn but not fail? Thanks.

@stepanblyschak
Copy link
Collaborator

@akokhan The HLD mentions this (https://github.com/stepanblyschak/SONiC/blob/acl-table-type/doc/acl/ACL-Table-Type-HLD.md#state-db):

On table creation, a check is performed by the orchagent to check against the action list and action_list capability as well as is_action_list_mandatory flag.

AFAIK, capability checks are not warnings in SONiC.

I see few options:

  1. Change vendor SAI to return correct is_action_list_mandatory value (I would say it is a prefered one)
  2. Initialize default table types with all actions (This is actually a configuration change comparing to previous releases)
  3. Relax the condition and accept configuration (May affect user experience when configuring unsupported table on platforms which implement is_action_list_mandatory value correctly).

@akokhan
Copy link
Contributor

akokhan commented Dec 7, 2021

@stepanblyschak , thank you for the great summary. I believe the combination of 1 and 3 would the best option. IMO, we should not fail until it's really necessary. In this case it's probably ACL table create SAI call.

@bingwang-ms
Copy link
Contributor

Thanks @stepanblyschak and @akokhan for the great summary. I also prefer option 1. Relaxing the condition may be misleading for ACL table type for which action list is mandatory.

@zhangyanzhao zhangyanzhao added the Triaged this issue has been triaged label Dec 8, 2021
@bratashX
Copy link
Contributor Author

bratashX commented Dec 8, 2021

Issue resolved
@stepanblyschak thanks

@bratashX bratashX closed this as completed Dec 8, 2021
@selldinesh
Copy link
Contributor

Triaged

Hi
we are using barefoor dut with SONiC.202111.83809-dirty-20220324.191815 image.
When we run the acl/test_acl.py case , we are hitting the same issue, i.e in syslog we are getting failed to create aCL table, Can you please let us know what is the workaround to resolve this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Triaged this issue has been triaged
Projects
None yet
Development

No branches or pull requests

8 participants