From fb74c29a6e3ce18aaff324032b71146415403d0b Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 5 Aug 2021 13:45:46 +0300 Subject: [PATCH 1/7] [ACL] Flex Counters for ACL rules Signed-off-by: Stepan Blyschak --- doc/acl/ACL-Flex-Counters.md | 240 ++++++++++++++++++ .../img/acl-counters-acl-rule-add-flow.svg | 3 + .../img/acl-counters-acl-rule-remove-flow.svg | 3 + .../img/acl-counters-high-level-diagram.svg | 3 + 4 files changed, 249 insertions(+) create mode 100644 doc/acl/ACL-Flex-Counters.md create mode 100755 doc/acl/img/acl-counters-acl-rule-add-flow.svg create mode 100755 doc/acl/img/acl-counters-acl-rule-remove-flow.svg create mode 100755 doc/acl/img/acl-counters-high-level-diagram.svg diff --git a/doc/acl/ACL-Flex-Counters.md b/doc/acl/ACL-Flex-Counters.md new file mode 100644 index 00000000000..3b095b8d6e7 --- /dev/null +++ b/doc/acl/ACL-Flex-Counters.md @@ -0,0 +1,240 @@ + +# ACL Flex Counters Support # + + +## Table of Content +- Revision +- Scope +- Definitions/Abbreviations +- Overview +- Requirements +- Architecture Design +- High-Level Design +- SAI +- Orchagent +- Syncd +- COUNTERS DB +- Flows +- Create ACL rule +- Delete ACL rule +- Mirror flow enhancement +- SAI API +- Configuration and management + - CLI/YANG model Enhancements + - Config DB Enhancements +- Warmboot and Fastboot Design Impact +- Restrictions/Limitations +- Testing Requirements/Design + - Unit Test cases + - System Test cases +- Open/Action items + +### Revision + +| Rev | Date | Author | Change Description | +|:---:|:-----------:|:------------------:|-----------------------------------| +| 0.1 | | Stepan Blyshchak | Initial version | + +### Scope + +The scope of this document covers ACL rule counters support and enhancements in that area. +This document does not cover a reasonable option to create ACL rules without counters as +it may be that ACL rule counters consume additional ASIC resources that might or might not be +required for the end user. This feature is beyond the scope of this document. This design does +not change the existing user experience for ACL functionality. + +### Definitions/Abbreviations + +| Definitions/Abbreviation | Description | +|--------------------------|--------------------------------------------| +| ACL | Access Control List | +| API | Application Programmable Interface | +| Everflow | ERSPAN (Encapsulated Remote Switched Port Analysis) mirroring | +| FC | Flex Counter | +| VID | SAIRedis Virtual object identifier | +| RID | SAI Real object identifier | +| SAI | Switch Abstraction Interface | + +### Overview + +The current design of ACL rule counters implements polling at orchagent side at a constant hardcoded interval of 10 seconds. +While it is a simpler approach comparing to Flex Counters infrastructure it comes at a cost of scalability and performance issues. +Considering that orchagent is single threaded a pretty small amount of ACL rules in comparison to ASIC capabilities may take a while +to poll counters for lot more than 10 seconds and blocking orchagent from performing other tasks. This leads to a problem that orchagent +is always busy collecting counters for ACLs and slow responses to other tasks. + +Flex counters infrastructure on another hand already used for port, PG, queue, watermark counters solves this issue by delegating counter +polling to a separate thread in syncd and allowing to configure polling interval as well. + +### Requirements + +- Free up orchagent queue when lots of ACL rules are configured by delegating counters polling to Flex Counters thread in syncd. +- Support enabling and disabling polling based on user configuration and ```counterpoll``` CLI for it. +- Support changing polling interval in range [1-1000] sec and ```counterpoll``` CLI for it. + +### Architecture Design + +No SONiC architecture changes are required as an existing flex counter infrastructure is being used. + +

+Figure 1. ACL counters +

+ +### High-Level Design + +### SAI + +Unlike port, PG, queue, etc. ACL counters are separate SAI objects of type SAI_OBJECT_TYPE_ACL_COUNTER that are bound to an ACL rule object of type +SAI_OBJECT_TYPE_ACL_ENTRY that orchagent creates with two attributes that are being queried: + +| SAI Attribute | Description +|------------------------------|----------------------| +| SAI_ACL_COUNTER_ATTR_PACKETS | Get/set packet count | +| SAI_ACL_COUNTER_ATTR_BYTES | Get/set byte count | + +These objects as well as ACL rule are dynamic, thus at runtime they might be added or removed so the flex counter manager has to take it into consideration. + +### Orchagent + +A new type of FC is added to flex_counter/flex_counter_manager.h against its SAI object and a new FC group named "ACL" is added: + +Counter Type: +```c++ +CounterType::ACL_COUNTER +``` + +An ACL orchagent holds a new object of type FlexCounterManager and initialized with ```StatsMode::READ``` +and a default polling interval of 10 sec enabled by default: +```c++ +FlexCounterManager m_acl_fc_mgr; +``` + +Although these changes are enough to configure FC in syncd for polling at a certain interval its barely usable for CLI to consume +because of two reasons: +1. ACL counter is separate SAI object and needs to be mapped to ACL table name and ACL rule name. +2. ACL rule may be created and removed in the hardware depending on the rule type and state in the network. + One such example is a mirroring rule. A mirror rule can only exists if a mirror session is created/active. + Orchagent internally removes ACL mirror rule on session deactivation and creates it when the corresponding + session is activated. That means an ACL counter will be recreated as well and the counter will reset, however + it is not intended that ACL rule counter will reset upon session state change. A cache is required to hold an + old counter values and sum them with the newly created one. The solution to this problem is to not remove the + ACL rule counter but detach it from the ACL rule. + +### Syncd + +ACL FC group support in syncd/FlexCounter.cpp. + +### COUNTERS DB + +Counters table in COUNTERS DB: + +- "COUNTERS:oid:" + - key: SAI_ACL_COUNTER_ATTR_PACKETS + - value: Number of packets passed through this rule + - key: SAI_ACL_COUNTER_ATTR_BYTES + - value: Number of bytes passed through this rule + +``` +127.0.0.1:6379[2]> hgetall COUNTERS:oid:0x100000000037a + 1) "SAI_ACL_COUNTER_ATTR_PACKETS" + 2) "100" + 3) "SAI_ACL_COUNTER_ATTR_BYTES" + 4) "102400" +``` + +Mapping hash table in COUNTERS_DB: + +- "COUNTERS_ACL_COUNTER_RULE_MAP" + - key: ACL table name and ACL rule name separated COUNTERS DB separator (e.g: "L3_TABLE:RULE0") + - value: VID of the ACL counter + +E.g: + +``` +127.0.0.1:6379[2]> hgetall COUNTERS_ACL_COUNTER_RULE_MAP + 1) "DATA:RULE0" + 2) "oid:0x100000000037a" +``` + +### Flows + +### Create ACL rule + +

+Figure 2. Create ACL rule flow +

+ +### Delete ACL rule + +

+Figure 3. Delete ACL rule flow +

+ +### Mirror flow enhancement + +ACL counter should not be removed when mirror rule is removed on mirror session deactivation and upon mirror recreation attached back to the rule object. + +### SAI API + +No new SAI API is used. + +### Configuration and management +#### CLI/YANG model Enhancements +#### Config DB Enhancements + +Enable ACL counter polling: +``` +admin@sonic:~$ counterpoll acl enable +``` + +Disable ACL counter polling (NOTE: ACL counter objects are still configured in HW): +``` +admin@sonic:~$ counterpoll acl enable +``` + +Set ACL counter polling interval: +``` +admin@sonic:~$ counterpoll acl interval [INTERVAL IN MS] +``` + +Config DB schema with ACL key in FLEX COUNTER table: + +```json +{ +"FLEX_COUNTER_TABLE": { + "ACL": { + "FLEX_COUNTER_STATUS": "enable" + } + } +} +``` + +YANG model with ACL group: + +```yang + container ACL { + /* ACL_FLEX_COUNTER_GROUP */ + leaf FLEX_COUNTER_STATUS { + type flex_status; + } + } +``` + +### Warmboot and Fastboot Design Impact +N/A + +### Restrictions/Limitations +N/A + +### Testing Requirements/Design + +#### Unit Test cases + +1. Enhance test_flex_counters.py with ACL group +2. Enhance test_acl.py with check for ACL rule mapping and ACL counter OID inserted in FLEX COUNTER DB. + +#### System Test cases + +ACL/Everflow tests suite in sonic-mgmt covers the ACL counter functionality. + +### Open/Action items diff --git a/doc/acl/img/acl-counters-acl-rule-add-flow.svg b/doc/acl/img/acl-counters-acl-rule-add-flow.svg new file mode 100755 index 00000000000..24c3a94b833 --- /dev/null +++ b/doc/acl/img/acl-counters-acl-rule-add-flow.svg @@ -0,0 +1,3 @@ + + +
loop
loop
aclorch
aclorch
DATA|RULE0
DATA|RULE0
syncd
syncd
COUNTERS DB
COUNTERS DB
hset COUNTERS:<counter VID> attr value
hset COUNTERS:<counter VID> attr value
return
return
FLEX COUNTER DB
FLEX COUNTER DB
FC thread
FC thread
sai_acl_api->create_acl_entry
sai_acl_api->create_acl_entry
return
return
sai_acl_api->create_acl_counter
sai_acl_api->create_acl_counter
return
return
hset COUNTERS_ACL_COUNTER_RULE_MAP 
hset COUNTERS_ACL_COUNTER_RULE_MAP 
return
return
hset FLEX_COUNTER:<counter VID> attr IDs 
hset FLEX_COUNTER:<counter VID> attr IDs 
return
return
SAI
SAI
dispatch
dispatch
sai_acl_api->create_acl_entry
sai_acl_api->create_acl_entry
return
return
sai_acl_api->create_acl_counter
sai_acl_api->create_acl_counter
return
return
sai_acl_api->get_acl_counter_attribute
sai_acl_api->get_acl_counter_attribute
return
return
Set new rule to map
Set new rule to map
Viewer does not support full SVG 1.1
\ No newline at end of file diff --git a/doc/acl/img/acl-counters-acl-rule-remove-flow.svg b/doc/acl/img/acl-counters-acl-rule-remove-flow.svg new file mode 100755 index 00000000000..d71ae837832 --- /dev/null +++ b/doc/acl/img/acl-counters-acl-rule-remove-flow.svg @@ -0,0 +1,3 @@ + + +
aclorch
aclorch
DATA|RULE0
DATA|RULE0
syncd
syncd
COUNTERS DB
COUNTERS DB
FLEX COUNTER DB
FLEX COUNTER DB
FC thread
FC thread
SAI
SAI
sai_acl_api->remove_acl_counter
sai_acl_api->remove_acl_counter
return
return
sai_acl_api->remove_acl_entry
sai_acl_api->remove_acl_entry
return
return
sai_acl_api->remove_acl_counter
sai_acl_api->remove_acl_counter
return
return
sai_acl_api->remove_acl_entry
sai_acl_api->remove_acl_entry
return
return
Remove VID from rule mapping
Remove VID from rule mappi...
remove counter OID
remove counter OID
del FLEX_COUNTER:<counter VID>
del FLEX_COUNTER:<counter VID>
return
return
return
return
del COUNTERS_ACL_COUNTER_RULE_MAP 
del COUNTERS_ACL_COUNTER_RULE_MAP 
return
return
del COUNTERS:<counter VID>
del COUNTERS:<counter VID>
return
return
Viewer does not support full SVG 1.1
\ No newline at end of file diff --git a/doc/acl/img/acl-counters-high-level-diagram.svg b/doc/acl/img/acl-counters-high-level-diagram.svg new file mode 100755 index 00000000000..07cf42634af --- /dev/null +++ b/doc/acl/img/acl-counters-high-level-diagram.svg @@ -0,0 +1,3 @@ + + +
SONiC Switch
SONiC Switch
swss
swss
syncd
syncd
orchagent
orchagent
syncd
syncd
COUNTERS DB
COUNTERS DB
CONFIG DB
CONFIG DB
Flex Counters thread
Flex Counters th...
ACL orch
ACL orch
FLEX COUNTER DB
FLEX COUNTER DB
Actor
Actor
Viewer does not support full SVG 1.1
\ No newline at end of file From 70ba0f2c661aa34df8ba001f97229f2add2425ca Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak <38952541+stepanblyschak@users.noreply.github.com> Date: Tue, 17 Aug 2021 16:28:25 +0300 Subject: [PATCH 2/7] Update ACL-Flex-Counters.md --- doc/acl/ACL-Flex-Counters.md | 50 +++++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/doc/acl/ACL-Flex-Counters.md b/doc/acl/ACL-Flex-Counters.md index 3b095b8d6e7..27c5133a653 100644 --- a/doc/acl/ACL-Flex-Counters.md +++ b/doc/acl/ACL-Flex-Counters.md @@ -152,10 +152,33 @@ E.g: ``` 127.0.0.1:6379[2]> hgetall COUNTERS_ACL_COUNTER_RULE_MAP - 1) "DATA:RULE0" + 1) "L3_TABLE:RULE0" 2) "oid:0x100000000037a" ``` +### CLI + +*aclshow* utility is modified to read counters from COUNTERS DB using ACL_RULE table in CONFIG DB and COUNTERS_ACL_COUNTER_RULE_MAP in COUNTERS DB. +Utility reads all ACLs from ACL_RULE table, uses COUNTERS_ACL_COUNTER_RULE_MAP to get the VID of the ACL counter object and gets the counter values +from COUNTERS DB. If COUNTERS_ACL_COUNTER_RULE_MAP is missing an entry for the rule it either means that ACL rule was created without counter (which +is not supported right now, but might be possible in the future) or we hit a condition when orchagent hasn't yet created the rule or the entry in the +map. In both cases we will display N/A. + +``` +admin@sonic:~$ aclshow -a +RULE NAME TABLE NAME PRIO PACKETS COUNT BYTES COUNT +------------ ------------ ------ --------------- ------------- +RULE_1 DATAACL 9999 101 100 +RULE_2 DATAACL 9998 201 200 +RULE_3 DATAACL 9997 301 300 +RULE_4 DATAACL 9996 401 400 +RULE_7 DATAACL 9993 701 700 +RULE_9 DATAACL 9991 901 900 +RULE_10 DATAACL 9989 1001 1000 +DEFAULT_RULE DATAACL 1 2 1 +RULE_6 EVERFLOW 9994 601 600 +``` + ### Flows ### Create ACL rule @@ -164,12 +187,16 @@ E.g: Figure 2. Create ACL rule flow

+In case an *error* happened, we roll back, deleting objects with best effort and removing the ACL rule taks from m_toSync map. An error is printed in the syslog. + ### Delete ACL rule

Figure 3. Delete ACL rule flow

+In case an *error* happened, we proceed deleting objects with best effort and removing the ACL rule taks from m_toSync map. An error is printed in the syslog. + ### Mirror flow enhancement ACL counter should not be removed when mirror rule is removed on mirror session deactivation and upon mirror recreation attached back to the rule object. @@ -189,7 +216,7 @@ admin@sonic:~$ counterpoll acl enable Disable ACL counter polling (NOTE: ACL counter objects are still configured in HW): ``` -admin@sonic:~$ counterpoll acl enable +admin@sonic:~$ counterpoll acl disable ``` Set ACL counter polling interval: @@ -201,10 +228,11 @@ Config DB schema with ACL key in FLEX COUNTER table: ```json { -"FLEX_COUNTER_TABLE": { - "ACL": { - "FLEX_COUNTER_STATUS": "enable" - } + "FLEX_COUNTER_TABLE": { + "ACL": { + "FLEX_COUNTER_STATUS": "enable", + "POLL_INTERVAL": "10000" + } } } ``` @@ -219,6 +247,8 @@ YANG model with ACL group: } } ``` + +NOTE: YANG is currently not having POLL_INTERVAL field defined. ### Warmboot and Fastboot Design Impact N/A @@ -232,9 +262,17 @@ N/A 1. Enhance test_flex_counters.py with ACL group 2. Enhance test_acl.py with check for ACL rule mapping and ACL counter OID inserted in FLEX COUNTER DB. +3. Modify aclshow_test.py in sonic-utilities to work with new data source. #### System Test cases ACL/Everflow tests suite in sonic-mgmt covers the ACL counter functionality. ### Open/Action items + +- In case mirror session goes down, ACL rule is deleted but counters is not to save the counters values. This counter could be deleted from FLEX COUNTER DB to avoid polling it but it will trigger removal of the VID in the COUNTERS DB which we would like to keep. + - In the current desing it will work just like for PORT, QUEUE, PG counters for port in down state - meaning polling is happenning although there is no real point for it. + - It is more like an optimization for a bad scenario rather then a real optimization. A rule is created to be active and counter to be polled in the first place and there must be enough resources and free CPU time to do that. + - In case the ACL rule has more action, like mirror ingress on session s0 and mirror egress on session s1 (currently not supported but possible scenario) and if one sesssion goes down, say s0, the counter can't be removed anyway as there is still egress mirroring happening on s1. + - Considering previous items it does not make sense to remove the whole rule and a counter but just disable actions if the state in the network(session IP is reachable) requires. + - It is not clear how to remove mirror action from the rule in SAI as the SAI_ACL_ACTION_TYPE_MIRROR_INGRESS/SAI_ACL_ACTION_TYPE_MIRROR_EGRESS are not @allowempty. From 76028d17c75a3a4e908e40d64d270cc5035842cb Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak <38952541+stepanblyschak@users.noreply.github.com> Date: Tue, 17 Aug 2021 16:30:10 +0300 Subject: [PATCH 3/7] Update ACL-Flex-Counters.md --- doc/acl/ACL-Flex-Counters.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/acl/ACL-Flex-Counters.md b/doc/acl/ACL-Flex-Counters.md index 27c5133a653..6c011d599da 100644 --- a/doc/acl/ACL-Flex-Counters.md +++ b/doc/acl/ACL-Flex-Counters.md @@ -159,10 +159,10 @@ E.g: ### CLI *aclshow* utility is modified to read counters from COUNTERS DB using ACL_RULE table in CONFIG DB and COUNTERS_ACL_COUNTER_RULE_MAP in COUNTERS DB. -Utility reads all ACLs from ACL_RULE table, uses COUNTERS_ACL_COUNTER_RULE_MAP to get the VID of the ACL counter object and gets the counter values -from COUNTERS DB. If COUNTERS_ACL_COUNTER_RULE_MAP is missing an entry for the rule it either means that ACL rule was created without counter (which +Utility reads all ACLs from ACL_RULE table, uses the map to get the VID of the ACL counter object and gets the counter values +from COUNTERS DB. If the map is missing an entry for the rule it either means that ACL rule was created without counter (which is not supported right now, but might be possible in the future) or we hit a condition when orchagent hasn't yet created the rule or the entry in the -map. In both cases we will display N/A. +map. In both cases utility displays N/A. In case the COUNTERS DB is missing a VID that exists in the rule the utilitity displays N/A. That means that either polling is disabled for ACL group or syncd haven't yet put any value in COUNTERS DB. ``` admin@sonic:~$ aclshow -a From 959f26668b7d9c8956118b1f47f10b33417f023b Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Tue, 31 Aug 2021 12:02:58 +0300 Subject: [PATCH 4/7] update ACL flex counters HLD Signed-off-by: Stepan Blyschak --- doc/acl/ACL-Flex-Counters.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/acl/ACL-Flex-Counters.md b/doc/acl/ACL-Flex-Counters.md index 6c011d599da..910a70ae256 100644 --- a/doc/acl/ACL-Flex-Counters.md +++ b/doc/acl/ACL-Flex-Counters.md @@ -14,6 +14,7 @@ - Orchagent - Syncd - COUNTERS DB +- CLI - Flows - Create ACL rule - Delete ACL rule @@ -179,6 +180,14 @@ DEFAULT_RULE DATAACL 1 2 1 RULE_6 EVERFLOW 9994 601 600 ``` +*sonic-clear* utility is extended with the ACL group to clear ACL counters. Counters are dumped into a file under /home/admin and the next time *aclshow* is invoked +it shows the difference between dumped counters and actual from database. This approach makes each user have it's own view on the counters and aligned to other +counter groups. + +``` +admin@sonic:~$ sonic-clear acl +``` + ### Flows ### Create ACL rule From 08da3709f6a49db00b3ec1f9a8be81e9555afe98 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Tue, 31 Aug 2021 18:34:53 +0300 Subject: [PATCH 5/7] update ACL flex counters HLD Signed-off-by: Stepan Blyschak --- doc/acl/ACL-Flex-Counters.md | 28 +++++++++++++++---- .../img/acl-counters-acl-rule-add-flow.svg | 2 +- .../img/acl-counters-acl-rule-remove-flow.svg | 2 +- doc/acl/img/acl-fc-th-add.svg | 3 ++ doc/acl/img/acl-fc-th-removal.svg | 3 ++ 5 files changed, 30 insertions(+), 8 deletions(-) create mode 100755 doc/acl/img/acl-fc-th-add.svg create mode 100755 doc/acl/img/acl-fc-th-removal.svg diff --git a/doc/acl/ACL-Flex-Counters.md b/doc/acl/ACL-Flex-Counters.md index 910a70ae256..5b1529a628a 100644 --- a/doc/acl/ACL-Flex-Counters.md +++ b/doc/acl/ACL-Flex-Counters.md @@ -18,6 +18,8 @@ - Flows - Create ACL rule - Delete ACL rule +- ACL counter registration in FC +- ACL counter de-registration in FC - Mirror flow enhancement - SAI API - Configuration and management @@ -196,7 +198,7 @@ admin@sonic:~$ sonic-clear acl Figure 2. Create ACL rule flow

-In case an *error* happened, we roll back, deleting objects with best effort and removing the ACL rule taks from m_toSync map. An error is printed in the syslog. +In case an *error* happened, we roll back, deleting objects with best effort and removing the ACL rule tasks from m_toSync map. An error is printed in the syslog. ### Delete ACL rule @@ -204,7 +206,19 @@ In case an *error* happened, we roll back, deleting objects with best effort and Figure 3. Delete ACL rule flow

-In case an *error* happened, we proceed deleting objects with best effort and removing the ACL rule taks from m_toSync map. An error is printed in the syslog. +In case an *error* happened, we roll back, deleting objects with best effort. An error is printed in the syslog. + +### ACL counter registration in FC + +

+Figure 3. ACL counter registration in FC +

+ +### ACL counter de-registration in FC + +

+Figure 4. ACL counter de-registration in FC +

### Mirror flow enhancement @@ -240,9 +254,9 @@ Config DB schema with ACL key in FLEX COUNTER table: "FLEX_COUNTER_TABLE": { "ACL": { "FLEX_COUNTER_STATUS": "enable", - "POLL_INTERVAL": "10000" + "POLL_INTERVAL": "10000" } - } + } } ``` @@ -271,11 +285,13 @@ N/A 1. Enhance test_flex_counters.py with ACL group 2. Enhance test_acl.py with check for ACL rule mapping and ACL counter OID inserted in FLEX COUNTER DB. -3. Modify aclshow_test.py in sonic-utilities to work with new data source. +3. Enhance test_mirror.py with the check for ACL rule mapping and ACL counter OID inserted in FLEX COUNTER DB. Cover the flow of deactivation/activating mirror session. +4. Modify aclshow_test.py in sonic-utilities to work with new data source. #### System Test cases -ACL/Everflow tests suite in sonic-mgmt covers the ACL counter functionality. +- ACL/Everflow tests suite in sonic-mgmt covers the ACL counter functionality. +- Warm/Fast reboot tests to check the changes do not break these features. ### Open/Action items diff --git a/doc/acl/img/acl-counters-acl-rule-add-flow.svg b/doc/acl/img/acl-counters-acl-rule-add-flow.svg index 24c3a94b833..77b8b637282 100755 --- a/doc/acl/img/acl-counters-acl-rule-add-flow.svg +++ b/doc/acl/img/acl-counters-acl-rule-add-flow.svg @@ -1,3 +1,3 @@ -
loop
loop
aclorch
aclorch
DATA|RULE0
DATA|RULE0
syncd
syncd
COUNTERS DB
COUNTERS DB
hset COUNTERS:<counter VID> attr value
hset COUNTERS:<counter VID> attr value
return
return
FLEX COUNTER DB
FLEX COUNTER DB
FC thread
FC thread
sai_acl_api->create_acl_entry
sai_acl_api->create_acl_entry
return
return
sai_acl_api->create_acl_counter
sai_acl_api->create_acl_counter
return
return
hset COUNTERS_ACL_COUNTER_RULE_MAP 
hset COUNTERS_ACL_COUNTER_RULE_MAP 
return
return
hset FLEX_COUNTER:<counter VID> attr IDs 
hset FLEX_COUNTER:<counter VID> attr IDs 
return
return
SAI
SAI
dispatch
dispatch
sai_acl_api->create_acl_entry
sai_acl_api->create_acl_entry
return
return
sai_acl_api->create_acl_counter
sai_acl_api->create_acl_counter
return
return
sai_acl_api->get_acl_counter_attribute
sai_acl_api->get_acl_counter_attribute
return
return
Set new rule to map
Set new rule to map
Viewer does not support full SVG 1.1
\ No newline at end of file +
aclorch
aclorch
DATA|RULE0
DATA|RULE0
syncd
syncd
COUNTERS DB
COUNTERS DB
FLEX COUNTER DB
FLEX COUNTER DB
sai_acl_api->create_acl_counter
sai_acl_api->create_acl_counter
return
return
sai_acl_api->create_acl_entry
sai_acl_api->create_acl_entry
return
return
hset COUNTERS_ACL_COUNTER_RULE_MAP 
hset COUNTERS_ACL_COUNTER_RULE_MAP 
return
return
hset FLEX_COUNTER:<counter VID> attr IDs 
hset FLEX_COUNTER:<counter VID> attr IDs 
return
return
SAI
SAI
sai_acl_api->create_acl_counter
sai_acl_api->create_acl_counter
return
return
sai_acl_api->create_acl_entry
sai_acl_api->create_acl_entry
return
return
Set new rule to map
Set new rule to map
Viewer does not support full SVG 1.1
\ No newline at end of file diff --git a/doc/acl/img/acl-counters-acl-rule-remove-flow.svg b/doc/acl/img/acl-counters-acl-rule-remove-flow.svg index d71ae837832..760888ed2cc 100755 --- a/doc/acl/img/acl-counters-acl-rule-remove-flow.svg +++ b/doc/acl/img/acl-counters-acl-rule-remove-flow.svg @@ -1,3 +1,3 @@ -
aclorch
aclorch
DATA|RULE0
DATA|RULE0
syncd
syncd
COUNTERS DB
COUNTERS DB
FLEX COUNTER DB
FLEX COUNTER DB
FC thread
FC thread
SAI
SAI
sai_acl_api->remove_acl_counter
sai_acl_api->remove_acl_counter
return
return
sai_acl_api->remove_acl_entry
sai_acl_api->remove_acl_entry
return
return
sai_acl_api->remove_acl_counter
sai_acl_api->remove_acl_counter
return
return
sai_acl_api->remove_acl_entry
sai_acl_api->remove_acl_entry
return
return
Remove VID from rule mapping
Remove VID from rule mappi...
remove counter OID
remove counter OID
del FLEX_COUNTER:<counter VID>
del FLEX_COUNTER:<counter VID>
return
return
return
return
del COUNTERS_ACL_COUNTER_RULE_MAP 
del COUNTERS_ACL_COUNTER_RULE_MAP 
return
return
del COUNTERS:<counter VID>
del COUNTERS:<counter VID>
return
return
Viewer does not support full SVG 1.1
\ No newline at end of file +
aclorch
aclorch
DATA|RULE0
DATA|RULE0
syncd
syncd
COUNTERS DB
COUNTERS DB
FLEX COUNTER DB
FLEX COUNTER DB
SAI
SAI
Remove VID from rule mapping
Remove VID from rule mappi...
del FLEX_COUNTER:<counter VID>
del FLEX_COUNTER:<counter VID>
return
return
del COUNTERS_ACL_COUNTER_RULE_MAP 
del COUNTERS_ACL_COUNTER_RULE_MAP 
return
return
sai_acl_api->remove_acl_entry
sai_acl_api->remove_acl_entry
return
return
sai_acl_api->remove_acl_counter
sai_acl_api->remove_acl_counter
return
return
sai_acl_api->remove_acl_entry
sai_acl_api->remove_acl_entry
return
return
sai_acl_api->remove_acl_counter
sai_acl_api->remove_acl_counter
return
return
Viewer does not support full SVG 1.1
\ No newline at end of file diff --git a/doc/acl/img/acl-fc-th-add.svg b/doc/acl/img/acl-fc-th-add.svg new file mode 100755 index 00000000000..94c564a68a1 --- /dev/null +++ b/doc/acl/img/acl-fc-th-add.svg @@ -0,0 +1,3 @@ + + +
loop
loop
aclorch
aclorch
DATA|RULE0
DATA|RULE0
COUNTERS DB
COUNTERS DB
hset COUNTERS:<counter VID> attr value
hset COUNTERS:<counter VID> attr value
return
return
FLEX COUNTER DB
FLEX COUNTER DB
FC thread
FC thread
hset FLEX_COUNTER:<counter VID> attr IDs 
hset FLEX_COUNTER:<counter VID> attr IDs 
return
return
SAI
SAI
dispatch
dispatch
sai_acl_api->get_acl_counter_attribute
sai_acl_api->get_acl_counter_attribute
return
return
Viewer does not support full SVG 1.1
\ No newline at end of file diff --git a/doc/acl/img/acl-fc-th-removal.svg b/doc/acl/img/acl-fc-th-removal.svg new file mode 100755 index 00000000000..d1d1489d236 --- /dev/null +++ b/doc/acl/img/acl-fc-th-removal.svg @@ -0,0 +1,3 @@ + + +
aclorch
aclorch
COUNTERS DB
COUNTERS DB
FLEX COUNTER DB
FLEX COUNTER DB
FC thread
FC thread
remove counter OID
remove counter OID
del FLEX_COUNTER:<counter VID>
del FLEX_COUNTER:<counter VID>
return
return
return
return
del COUNTERS:<counter VID>
del COUNTERS:<counter VID>
return
return
Viewer does not support full SVG 1.1
\ No newline at end of file From 04abc04eee6b3f703bbda58c0df0fb01d0e157a0 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Mon, 6 Sep 2021 12:49:50 +0300 Subject: [PATCH 6/7] update ACL flex counter HLD Signed-off-by: Stepan Blyschak --- doc/acl/ACL-Flex-Counters.md | 13 ++++++++----- doc/acl/img/acl-mirror-rule-flow.svg | 3 +++ 2 files changed, 11 insertions(+), 5 deletions(-) create mode 100755 doc/acl/img/acl-mirror-rule-flow.svg diff --git a/doc/acl/ACL-Flex-Counters.md b/doc/acl/ACL-Flex-Counters.md index 5b1529a628a..7ef1d5d8fc5 100644 --- a/doc/acl/ACL-Flex-Counters.md +++ b/doc/acl/ACL-Flex-Counters.md @@ -224,6 +224,10 @@ In case an *error* happened, we roll back, deleting objects with best effort. An ACL counter should not be removed when mirror rule is removed on mirror session deactivation and upon mirror recreation attached back to the rule object. +

+Figure 5. ACL mirror flow enhancement +

+ ### SAI API No new SAI API is used. @@ -274,7 +278,8 @@ YANG model with ACL group: NOTE: YANG is currently not having POLL_INTERVAL field defined. ### Warmboot and Fastboot Design Impact -N/A + +Counter polling is delayed at system startup. ### Restrictions/Limitations N/A @@ -296,8 +301,6 @@ N/A ### Open/Action items - In case mirror session goes down, ACL rule is deleted but counters is not to save the counters values. This counter could be deleted from FLEX COUNTER DB to avoid polling it but it will trigger removal of the VID in the COUNTERS DB which we would like to keep. - - In the current desing it will work just like for PORT, QUEUE, PG counters for port in down state - meaning polling is happenning although there is no real point for it. + - In the current desing it will work just like for PORT, QUEUE, PG counters for port in down state - meaning polling is happening although there is no real point for it. - It is more like an optimization for a bad scenario rather then a real optimization. A rule is created to be active and counter to be polled in the first place and there must be enough resources and free CPU time to do that. - - In case the ACL rule has more action, like mirror ingress on session s0 and mirror egress on session s1 (currently not supported but possible scenario) and if one sesssion goes down, say s0, the counter can't be removed anyway as there is still egress mirroring happening on s1. - - Considering previous items it does not make sense to remove the whole rule and a counter but just disable actions if the state in the network(session IP is reachable) requires. - - It is not clear how to remove mirror action from the rule in SAI as the SAI_ACL_ACTION_TYPE_MIRROR_INGRESS/SAI_ACL_ACTION_TYPE_MIRROR_EGRESS are not @allowempty. + - Current FC infrastructure does not allow to enable/disable polling on per-counter OID basis. diff --git a/doc/acl/img/acl-mirror-rule-flow.svg b/doc/acl/img/acl-mirror-rule-flow.svg new file mode 100755 index 00000000000..0527852718d --- /dev/null +++ b/doc/acl/img/acl-mirror-rule-flow.svg @@ -0,0 +1,3 @@ + + +
MirrorOrch
MirrorOrch
AclOrch
AclOrch
SAI
SAI
FLEX COUNTER DB
FLEX COUNTER DB
AclRule
AclRule
session state change
session state change
AclRule::update()
AclRule::update()
sai_acl_api->remove_acl_entry()/
sai_acl_api->create_acl_entry()
sai_acl_ap...
Based on session state
remove or re-create ACL entry
Based on session state...
ACL counter is not removed from the HW.
FC thread still polls the counter.
ACL counter is not removed from the HW....
Viewer does not support full SVG 1.1
\ No newline at end of file From 7add0f7e2a01593804df4b657b11085a0fa9325b Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Mon, 6 Sep 2021 13:11:37 +0300 Subject: [PATCH 7/7] update ACL flex counter HLD Signed-off-by: Stepan Blyschak --- doc/acl/ACL-Flex-Counters.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/acl/ACL-Flex-Counters.md b/doc/acl/ACL-Flex-Counters.md index 7ef1d5d8fc5..76d16072571 100644 --- a/doc/acl/ACL-Flex-Counters.md +++ b/doc/acl/ACL-Flex-Counters.md @@ -300,7 +300,8 @@ N/A ### Open/Action items +- Can a default FC ACL counter state be disabled in init_cfg.json? Only enabled when generating configuration from minigraph.xml or when performing S2S upgrade in DB migrator? - In case mirror session goes down, ACL rule is deleted but counters is not to save the counters values. This counter could be deleted from FLEX COUNTER DB to avoid polling it but it will trigger removal of the VID in the COUNTERS DB which we would like to keep. - In the current desing it will work just like for PORT, QUEUE, PG counters for port in down state - meaning polling is happening although there is no real point for it. - It is more like an optimization for a bad scenario rather then a real optimization. A rule is created to be active and counter to be polled in the first place and there must be enough resources and free CPU time to do that. - - Current FC infrastructure does not allow to enable/disable polling on per-counter OID basis. + - Current FC infrastructure does not allow to enable/disable polling on per-counter OID basis but keeping the counter value in COUNTERS DB.