Skip to content

Custom ACL Based Metering #1889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

shaygol
Copy link

@shaygol shaygol commented Jan 8, 2025

Describes the Custom ACL Based Metering (CABM) feature design in SONiC.

Repository PR Title State Context
sonic-swss [AclOrch] Custom ACL Based Metering PR Status PR Checks
sonic-buildimage [sonic-buildimage] Custom ACL Based Metering PR Status PR Checks
sonic-utilities [sonic-utilities] Custom ACL Based Metering PR Status PR Checks

Describes the Custom ACL Based Metering (CABM) feature design in SONiC.
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

linux-foundation-easycla bot commented Jan 8, 2025

CLA Missing ID

  • ✅login: shaygol / (0f08d82)
  • ❌ The commit (1a0f703, 767a9a0). This user is authorized, but they must confirm their affiliation with their company. Start the authorization process by clicking here, click "Corporate", select the appropriate company from the list, then confirm your affiliation on the page that appears. For further assistance with EasyCLA, please submit a support request ticket.

Copy link

No pipelines are associated with this pull request.

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

No pipelines are associated with this pull request.

@zhangyanzhao
Copy link
Collaborator


#### **Configuration Flow**

![alt text](Config_Flow.jpg)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are validating the ACL action policer capability based on information from STATE_DB, please use it in the diagram.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ACL-Orch saves the information in both STATE_DB and local DB. So in my case, I can just check the local DB.

##### ACL Table Type Table
When a new ACL table is created, SAI needs to receive a list of supported actions which the rules belonging to this table are allowed to use.
To support the new policer action, the custom table types table schema will be extended with a policer action attribute - **"POLICER_ACTION"** for the actions attribute field.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please highlight the changes done on top of the existing tables for this feature.

...
+ /* prevent deletion of policer that referenced by ACL rule.
+ Note that new policer won't be referenced by any ACL rules initially */
+ must "not(../acl:sonic-acl/acl:ACL_RULE/acl:ACL_RULE_LIST[acl:policer_action=current()/name])" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may not be required, please double check.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, thanks. I'll update the HLD accordingly.

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

No pipelines are associated with this pull request.

@shaygol
Copy link
Author

shaygol commented Jan 28, 2025

@venkatmahalingam @bingwang-ms Can you please review?

config acl add table [OPTIONS] <table_name> <table_type>

# Config new ACL rules --> new optional field 'policer_name'
config acl update full [OPTIONS] [--policer_name <policer_name>] <FILE_NAME>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not consider placing POLICER_ACTION in the JSON file for configuration as well?


# Config new ACL rules --> new optional field 'policer_name'
config acl update full [OPTIONS] [--policer_name <policer_name>] <FILE_NAME>
config acl update incremental [OPTIONS] [--policer_name <policer_name>] <FILE_NAME>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the --policer option is used, can you explain how acl-loader converts the actions configured in the JSON, and whether it is consistent with the mirror processing logic?


---
### Requirements
#### Functional Requirements

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you could explain how to handle updates to the policer—whether it's better to create a new policer and re-associate it with the ACL rule, or to directly update the attributes of the existing policer?

@anilpannala anilpannala moved this from 🏗 In Progress to MovedToBacklog in SONiC 202505 Release May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: MovedToBacklog
Development

Successfully merging this pull request may close these issues.

6 participants