Skip to content
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

Configuration to control ACL set/entry counter allocation #1122

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nokia1adam
Copy link
Contributor

This code is a Contribution to the OpenConfig Public project (“Work”) made under the Google Software Grant and Corporate Contributor License Agreement (“CLA”) and governed by the Apache License 2.0. No other rights or licenses in or to any of Nokia’s intellectual property are granted for any other purpose. This code is provided on an “as is” basis without any warranties of any kind.

Change Scope

At present, the allocation of system resources needed to provide counters for packets matching ACL entries cannot be controlled through configuration. This PR aims to fill this gap by introducing the following new paths:
acl/acl-sets/acl-set/config/counter
acl/acl-sets/acl-set/state/counter
acl/acl-sets/acl-set/acl-entries/acl-entry/config/counter
acl/acl-sets/acl-set/acl-entries/acl-entry/state/counter

The permitted values are: NONE, INTERFACE_ONLY, AGGREGATE_ONLY and INTERFACE_AGGREGATE. These are the same values as supported by the existing ACL_COUNTER_CAPABILITY except now NONE has been added as well, in order to allow the operator to disable stats collection for specific ACL sets or ACL entries in order to conserve limited resources.

The description for /acl/state/counter-capability has been amended to recommend that no value should be returned in state if the support of counters is not uniformly configured for all ACL sets and all ACL entries.

Implementations

Many vendors support some form of configuration control for ACL counters.
Arista: https://www.arista.com/en/um-eos/eos-acls-and-route-maps
Nokia: https://documentation.nokia.com/srlinux/24-3/books/acl-policy-based-routing/access-control-lists.html

@nokia1adam nokia1adam requested a review from a team as a code owner June 3, 2024 14:24
@dplore
Copy link
Member

dplore commented Jun 6, 2024

/gcbrun

@OpenConfigBot
Copy link

No major YANG version changes in commit 849eca5

@dplore
Copy link
Member

dplore commented Jun 6, 2024

OC does not have a way to enable/disable ACL counters so this sounds like a good gap to close.

EOS support looks like counters per-entry which enables/disables per acl-set and are reported as AGGREGATE_ONLY if enabled. Enable is at the acl-set level.

My interpretation of the SRLinux doc is it supports enable/disable of statistics-per-entry at an ACL set level, but can report statistics at a per interface level. Does this map to INTERFACE_ONLY? Is that correct @nokia1adam

@dplore dplore self-assigned this Jun 6, 2024
@nokia1adam
Copy link
Contributor Author

@dplore, the SRLinux implementation has two different controls: one to control whether the ACL is shared by multiple interfaces or not and another to control whether stats are enabled or disabled for all entries of the acl-set. So these two controls together will determine whether a particular acl-set's entries have AGGREGATE or INTERFACE capability.

Copy link
Member

@dplore dplore left a comment

Choose a reason for hiding this comment

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

Please also propose some description of what should happen if the counter leaf is not configured. (I would guess the default value is implementation defined)

@@ -527,6 +550,15 @@ module openconfig-acl {
"Description, or comment, for the ACL set";
}

leaf counter {
Copy link
Member

Choose a reason for hiding this comment

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

this leaf is not needed, as the grouping "access-list-entries-config" is used in both config and state containers.

@dplore
Copy link
Member

dplore commented Oct 30, 2024

@nokia1adam let me know if you'd like to continue this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Waiting for author
Development

Successfully merging this pull request may close these issues.

3 participants