[ACL] Rollback the code of #1475 "Expand VLAN into VLAN members..."

[ezio-chen]

What I did

For the issue of #1753, it shall not expand the VLAN to VLAN member ports for all platform.
Also modify the unit test because the behavior is changed.

How I did it

Only expand VLAN to VLAN member for asic type is "mellanox"

How to verify it

Pass the unit test

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

admin@sonic:~$ show vlan br
+-----------+--------------+-----------+----------------+-----------------------+-------------+
|   VLAN ID | IP Address   | Ports     | Port Tagging   | DHCP Helper Address   | Proxy ARP   |
+===========+==============+===========+================+=======================+=============+
|       100 |              | Ethernet0 | tagged         |                       | disabled    |
+-----------+--------------+-----------+----------------+-----------------------+-------------+
admin@sonic:~$ sudo config acl add table L3_TABLE L3 -p Vlan100
admin@sonic:~$ show acl table
Name      Type    Binding    Description    Stage
--------  ------  ---------  -------------  -------
L3_TABLE  L3      Ethernet0  L3_TABLE       ingress
admin@sonic:~$

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

admin@sonic:~$ show vlan br
+-----------+--------------+-----------+----------------+-----------------------+-------------+
|   VLAN ID | IP Address   | Ports     | Port Tagging   | DHCP Helper Address   | Proxy ARP   |
+===========+==============+===========+================+=======================+=============+
|       100 |              | Ethernet0 | tagged         |                       | disabled    |
+-----------+--------------+-----------+----------------+-----------------------+-------------+
admin@sonic:~$ sudo config acl add table L3_TABLE L3 -p Vlan100
admin@sonic:~$ show acl table
Name      Type    Binding    Description    Stage
--------  ------  ---------  -------------  -------
L3_TABLE  L3      Vlan100    L3_TABLE       ingress
admin@sonic:~$

[prsunny]

Why remove the code? Not all platforms may support Vlan bind. So suggest adding an option in CLI to use vlan binding. If the option is not specified, let it expand as it is done currently. @qiluo-msft , @bingwang-ms , what do you think?

[bingwang-ms]

Why remove the code? Not all platforms may support Vlan bind. So suggest adding an option in CLI to use vlan binding. If the option is not specified, let it expand as it is done currently. @qiluo-msft , @bingwang-ms , what do you think?

Agree. The Vlan expland feature is required in some scenarios. Please consider add an option to disable this behavior.

[ezio-chen]

I still think this change is unreasonable, if a port join multiple VLAN, the ACL will affect to all VLANs on these ports.

For example, Ethernet0,Ethernet1 is the member of VLAN100 and VLAN200,
and you use the command to create the ACL
sudo config acl add table Vlan100_ACL L3 -p Vlan100
but the rules in Vlan100_ACL will affect all traffic in Vlan200.

Why user just use sudo config acl add table Vlan100_ACL L3 -p Ethernet0,Ethernet1 to create ACL table if they really want to bind the ACL on these ports ?

[qiluo-msft]

Thanks @ezio-chen for the contribution!
I agree current implementation is not perfect. We introduce this feature to temporarily enable some use case on Mellanox SN2700 platform, which does not support vlan acl binding.

I agree we should remove this feature in long term. To prevent sudden feature broken in short term, could you keep the behavior for Mellanox ASIC platforms and only change the behavior for all others?

[qiluo-msft]

As comment

[ganglyu]

I think we can keep this feature for Mellanox platform, and other platforms should not expand VLAN.

[ezio-chen]

OK, and for this modification, can you please give some suggestions?

Method 1: Distinguishing the HWSKU type internal, only expland VLAN on Mellanox.
Method 2: Add CLI options, e.g., config acl add table <table_name> <table_type> [ --expland-vlan ]
or other good ideas ?

[qiluo-msft]

Method 1 looks good to me.