Policy Based Metering (PBM) - HLD #4
Conversation
shaygol
requested review from
a team,
amazor,
marvellgit,
matiAlfaro,
VladimirKuk,
shiraez,
aviramd and
DanielaMurin
and removed request for
a team
doc/policy_based_metering/PBM-HLD.md
Outdated
| ; value annotations | ||
| port_name = 1*64VCHAR ; name of the port, must be unique | ||
| max_ports = 1*5DIGIT ; number of ports supported on the chip | ||
| * ext_actions_list = ["POLICER", "ACTION1", "ACTION2", ...] ; additional action list - can be extended in the future with new actions |
There was a problem hiding this comment.
Currently only POLICER, right? I'd remove the not implemented ones (ACTION*)
Where is the mapping of the name POLICER to the policer table
empty is default? it always happens? for backward compatibility
There was a problem hiding this comment.
Empty is the default - this list defines the actions that the ACL rules can be used
doc/policy_based_metering/PBM-HLD.md
Outdated
| ### Restrictions/Limitations | ||
|
|
||
| - Policers must be supported | ||
| - Policer cannot be deleted when referenced by ACL rule. |
There was a problem hiding this comment.
missing Yang model, in it you can implemented this
doc/policy_based_metering/PBM-HLD.md
Outdated
| --- | ||
| ### Open/Action Items | ||
| - Are there specific ASIC platforms with limitations for this feature? | ||
| - ACL-Loader utility uses the an external python library (openconfig_acl) that rely on pre-defined, structured models. In order to support the new 'policer_action' per rule the openconfig_acl schema need to be changed. |
There was a problem hiding this comment.
why is this open? effort? what is the the side effect?
There was a problem hiding this comment.
Theoretically, there is an option to be a contributor for openconfig_acl (.py) (https://openconfig.net/about/contribute/) - but...
High-level design for the PBM feature - the ability to attach policers to ACL entries
- update HLD info - update the image with the DB names
|
Please see this for reference (HLD to config custom ACL table type): HLD |
| ; value annotations | ||
| port_name = 1*64VCHAR ; name of the port, must be unique | ||
| max_ports = 1*5DIGIT ; number of ports supported on the chip | ||
| * ext_actions_list = ["POLICER"] ; additional action list - can be extended in the future |
There was a problem hiding this comment.
Please see this for reference (HLD to config custom ACL table type): HLD
Because there already exists an existing mechanism for custom ACLs, not sure you need this new configurable field. Instead add a new type "POLICER".
If the user wants to do special cases, they can create custom ACL table types and use your implemented POLICER rule.
|
|
||
| * packet_action = "forward"/"drop"/"redirect" ; option for actions when the rule are matched. | ||
| * packet_action_mirror = True/false ; it hould be a sub set of the table 'ext_actions_list' felid | ||
| * packet_action_do_not_nat = True/false |
There was a problem hiding this comment.
I am confused, are there changes to the possible values in:
- packet_action
- packet_action_mirror
- packet_action_do_not_nat
?
| ; could be platform dependent | ||
|
|
||
| * packet_action = "forward"/"drop"/"redirect" ; option for actions when the rule are matched. | ||
| * packet_action_mirror = True/false ; it hould be a sub set of the table 'ext_actions_list' felid |
There was a problem hiding this comment.
Also, is "ext_actions_list" a new field in the ACL_RULES_TABLE too?
If we get rid of this field in the table, i assume we get rid of it here too, correct?
| config policer update <name> --rate <rate> --burst <burst> | ||
|
|
||
| # Add/Remove/Update ACL tables --> new field 'ext_actions_list' | ||
| config acl add table [OPTIONS] <table_name> <table_type> [--ext_actions_list <actions_list>] |
There was a problem hiding this comment.
Can you create a custom ACL using the CLI?
| No SONiC architecture changes are required as the existing infrastructure is being used. | ||
| ##### *Image 1: ACL Configuration Flow Overview* | ||
|
|
||
|  |
There was a problem hiding this comment.
I'm not sure I understand this picture. What are you trying to show? Either use a different picture, or give a short explanation
High-level design for the PBM feature - the ability to attach policers to ACL entries