Fail on ACL table creation

[bratashX]

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]

@stepanblyschak to take a look

[bingwang-ms]

Ack. Will take a look shortly.

[bingwang-ms]

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]

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

[bingwang-ms]

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]

@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]

@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]

@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]

@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]

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.