Merge pull request #1303 from bingwang-ms/improve_show_acl_cmd

[HLD] Improve HLD for show acl enhancement
sonic-net · Mar 24, 2023 · 3a1def9 · 3a1def9
2 parents 668987f + 821db23
commit 3a1def9
Showing 1 changed file with 8 additions and 5 deletions.
diff --git a/doc/acl/ACL-enhancements-on-show-command.md b/doc/acl/ACL-enhancements-on-show-command.md
@@ -7,11 +7,12 @@
 | Rev |     Date    |       Author       | Change Description                |
 |:---:|:-----------:|:------------------:|-----------------------------------|
 | 0.1 |  2/6/2023           | Bing Wang   | Initial version                   |
+| 0.2 | 3/23/2023   | Bing Wang | Address review comments |
 
 ### Scope
 
 The scope of this document covers enhancements on show acl commands, including `show acl table` and `show acl rule`.
-Only dataplane ACL is covered in this design. The enhancement of control plane ACL will be covered in another document,
+Only dataplane ACL is covered in this design. The enhancement of control plane ACL will be covered in another document.
 
 ### Definitions/Abbreviations 
 
@@ -37,6 +38,8 @@ This design is to improve the show acl commands to add the status of ACL table o
 
 In current implementation, `orchagent` checks return value from SAI, and writes a log to syslog. 
 In the proposed design, we introduce a new table to `STATE_DB`, and `orchagent` will write the return status to the `STATE_DB` table. The user can check the status of ACL table or ACL rule creation with CLI `show acl table` or `show acl rule`.  The `show` command reads the status from `STATE_DB`, and reads configuration from `CONFIG_DB`, and finally combine the status and configuration in the output.
+
+The proposed change doesn't cover the internally added ACL table or ACL rule, such as the ACL table/rule added by PFC handler or Mux handler in dualtor setup. That is because the table/rule is not added by `CONFIG_DB` entries. Hence the show command can't find the corresponding configuration in `CONFIG_DB`. 
 <p align=center>
 <img src="img/acl-work-flow-with-state-db.png" alt="Figure 2. ACL work flow with STATE_DB">
 </p>
@@ -64,7 +67,7 @@ ACL rule status
 ```
 $ redis-cli -n 6 hgetall "ACL_RULE|DATAACL|RULE_1"   
 1) "status"
-2) "Active"
+2) "Inactive"
 ```
 #### Orchagent
 Add logic in `aclorch` to support writing return status into `STATE_DB`. When ACL table or ACL rule is being deleted, the corresponding entry in `STATE_DB` is also cleared.
@@ -87,9 +90,9 @@ DATAACL  L3      Ethernet0    DATAACL        ingress    Active
 show acl rule
 Table    Rule          Priority    Action    Match                Status
 -------  ------------  ----------  --------  -------------------  --------
-DATAACL  RULE_1        9999        DROP      DST_IP: 9.5.9.3/32   Active
+DATAACL  RULE_1        9999        DROP      DST_IP: 9.5.9.3/32   Inactive
                                              ETHER_TYPE: 2048
-DATAACL  RULE_2        9998        FORWARD   DST_IP: 10.2.1.2/32  Active
+DATAACL  RULE_2        9998        FORWARD   DST_IP: 10.2.1.2/32  Inactive
                                              ETHER_TYPE: 2048
                                              IP_PROTOCOL: 6
                                              L4_DST_PORT: 22
@@ -100,7 +103,7 @@ The existing test script for ACL in sonic-mgmt is parsing syslog to find a keywo
 		
 ### Warmboot and Fastboot Design Impact  
 
-No impact to Warmboot or Fastboot
+The new table in `STATE_DB` doesn't persist during warmboot or fastboot. So there is no impact to warmboot or fastboot.
 
 ### Restrictions/Limitations  
 N/A