After reboot, ACL bound to VLAN interface does not work

[ArthiGovindaraj]

when ACL is bound to VLAN interface and then user saves config and gives reboot. ACL will not work.

What I did ISSUE: when ACL is bound to VLAN interface and then user saves config and gives reboot. ACL will not work. RCA: After reboot, ACL is configured first and then VLAN is created. Due to this ordering issue, ACL table is created without being bound to the VLAN interface. FIX: When the VLAN interface is created, notification of port change is sent to ACLOrch Class. ACLOrch handles the notification and binds the ACL table to the VLAN interface post creation. Similarly, ACL needs to be removed from the VLAN before deleting the VLAN interface. Otherwise, VLAN deletion will fail due to reference count error.

Why I did it If Issue is not fixed, after reboot ACL bound to VLAN interface will not work. How I verified it Create ACL table with ACL rule to drop matching traffic Bind ACL table to Vlan interface <Vlan 25> Create VLAN 25 and bind members to VLAN after this. ACL counters bound to the VLAN interface should increment and packet should get dropped. root@sonic:~# aclshow -a RULE NAME TABLE NAME PRIO PACKETS COUNT BYTES COUNT acl_rule_001 acl_table_001 1 51560 5089600

Remove the ACL table Delete the VLAN interface. On deleting VLAN interface with ACL bound to it, error is seen in syslog and VLAN deletion fails. Details if related

[prsunny]

Tests are missing for this PR. Can you please add some?

[bingwang-ms]

Hi @ArthiGovindaraj , if I recall correctly, VLAN is expanded into physical ports when binding a VLAN to an ACL table. So can you please help me understand how does this issue happen?

[bingwang-ms]

The code is https://github.com/sonic-net/sonic-utilities/blob/7670609a47e1419e5f3154866304f7578a02cd96/config/main.py#L5791

[ArthiGovindaraj]

Hi @bingwang-ms ,

Yes, you are right. Using CLICK command we cannot bind ACL to vlan interface. But, we could use config load command to achieve the same "config load -y ing_acl.json".

I have given the steps in the attached file: vlan_testcase_steps.txt

Let me know if any further details are needed.

[bingwang-ms]

Hi @ArthiGovindaraj Thanks for sharing the steps. I'm not sure if orchagent can handle the case when ports == Vlan25, because the expected input is physical port name or LAG name. Is that verified? And another concern about this change is it may break the Yang-model validation if Vlan25 is written into config_db. The yang-model for ACL ports is https://github.com/sonic-net/sonic-buildimage/blob/732f42e1a3c004f5539eeb2ed14143a577e02bbb/src/sonic-yang-models/yang-templates/sonic-acl.yang.j2#L360 So it doesn't seem to be a valid scenario to me.

[ArthiGovindaraj]

I'm not sure if orchagent can handle the case when ports == Vlan25, because the expected input is physical port name or LAG name. Is that verified? [Arthi]: Yes, this is verified. BIND_POINT_TYPE in orchangent table creation purpose is for including the qualifier like INPORTS/LAG in the ACL table. In case of VLAN, since already SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID is part of all the tables it will work irrespective of whether we specify the BIND_TYPE as VLAN or not. We have verified with/without BIND_POINT_TYPE_VLAN and both cases work. And another concern about this change is it may break the Yang-model validation if Vlan25 is written into config_db. The yang-model for ACL ports is https://github.com/sonic-net/sonic-buildimage/blob/732f42e1a3c004f5539eeb2ed14143a577e02bbb/src/sonic-yang-models/yang-templates/sonic-acl.yang.j2#L360 [Arthi] I agree on the concern w.r.t yang model. Can we add support for VLAN interface for ACL in the sonic yang model ?

For further acceptance of these changes,

• I could add orchagent change to support BIND_POINT_TYPE_VLAN for L3/L3V6 table
• sonic yang model support for VLAN interface to this pull request. From CLICK since VLAN being expected to be expanded as ports is the expected behavior, we are not modifying it. Instead we could also provide provision for the user to support VLAN from CONFIG_DB.

Let me know your inputs.