Add label attribute for ACL entry - #2322
Merged
Merged
Conversation
Summary: This is similar to opencomputeproject#1158, opencomputeproject#1407, opencomputeproject#1430. Those PRs added label attribute for LAG/virtual router, counter, ACL Counter. This PR adds similar label attribute for ACL entries. Adding a label attribute that can be used to uniquely identify ACL entry object during warmboot. This attribute is considered as user data attached to the object. Signed-off-by: Shrikrishna (Shri) Khare <skhare@meta.com> Signed-off-by: Jasmeet Bagga <jasmeetbagga@meta.com> Signed-off-by: Ron He <zecheng@meta.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
zechengh09
marked this pull request as ready for review
July 24, 2026 23:26
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
kcudnik
approved these changes
Jul 25, 2026
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
JaiOCP
reviewed
Jul 27, 2026
| * @default empty | ||
| */ | ||
| SAI_ACL_ENTRY_ATTR_LABEL, | ||
|
|
Contributor
There was a problem hiding this comment.
Why the data type is specified as s8_list_t instead of char?
Contributor
There was a problem hiding this comment.
The SAI spec limits char datatype to 32-bytes. In practice, we have seen cases where the label length may exceed this limit. For example, we may want to capture the use case for the ACL entry in the label name itself, and that could exceed 32-bytes.
sai_s8_list_t keeps it generic, so NOS can choose the desired label length without being constrained by the 32-byte limits.
Collaborator
There was a problem hiding this comment.
32 bytes for label is i think long enough, so far almost all labels are chars, and this should be a label not poem
JaiOCP
approved these changes
Jul 30, 2026
tjchadaga
approved these changes
Aug 3, 2026
zechengh09
pushed a commit
to zechengh09/SAI
that referenced
this pull request
Aug 5, 2026
Summary: This is similar to opencomputeproject#1158, opencomputeproject#1407, opencomputeproject#1430, opencomputeproject#2322. SAI_COUNTER_ATTR_LABEL and SAI_ACL_COUNTER_ATTR_LABEL are of type char, which maps to sai_attribute_value_t::chardata[32]. That is 32 bytes including the NUL terminator, so 31 usable characters. That can be too small for real counter identifiers. Add SAI_COUNTER_ATTR_LABEL_WIDE and SAI_ACL_COUNTER_ATTR_LABEL_WIDE of type sai_s8_list_t. A list keeps the label length open, so the NOS can choose the length it needs instead of being constrained by an arbitrary limit in the SAI spec. Both are appended at the end of their attribute enum so that no existing attribute is renumbered. The existing char attributes are marked deprecated. Exactly one of the two attributes may be set to a non-default value; setting both is invalid. Signed-off-by: Shrikrishna (Shri) Khare <skhare@meta.com> Signed-off-by: Jasmeet Bagga <jasmeetbagga@meta.com> Signed-off-by: Ron He <zecheng@meta.com>
zechengh09
pushed a commit
to zechengh09/SAI
that referenced
this pull request
Aug 6, 2026
Summary: This is similar to opencomputeproject#1158, opencomputeproject#1407, opencomputeproject#1430, opencomputeproject#2322. SAI label attributes are of type char, which maps to sai_attribute_value_t::chardata[32]. That is 32 bytes including the NUL terminator, so 31 usable characters. That can be too small for real identifiers. Add a LABEL_WIDE attribute of type sai_s8_list_t alongside each existing char label attribute. A list keeps the label length open, so the NOS can choose the length it needs instead of being constrained by an arbitrary limit in the SAI spec. Each new attribute is appended at the end of its attribute enum so that no existing attribute is renumbered. Objects covered: counter, ACL counter, LAG, mirror session, prefix compression table, policer, scheduler, next hop group, virtual router, UDF group and router interface. The existing char attributes are marked deprecated. Exactly one of the two attributes may be set to a non-default value; setting both is invalid. Signed-off-by: Shrikrishna (Shri) Khare <skhare@meta.com> Signed-off-by: Jasmeet Bagga <jasmeetbagga@meta.com> Signed-off-by: Ron He <zecheng@meta.com>
zechengh09
pushed a commit
to zechengh09/SAI
that referenced
this pull request
Aug 6, 2026
Summary: This is similar to opencomputeproject#1158, opencomputeproject#1407, opencomputeproject#1430, opencomputeproject#2322. SAI label attributes are of type char, which maps to sai_attribute_value_t::chardata[32]. That is 32 bytes including the NUL terminator, so 31 usable characters. That can be too small for real identifiers. Add a LABEL_WIDE attribute of type sai_s8_list_t alongside each existing char label attribute. A list keeps the label length open, so the NOS can choose the length it needs instead of being constrained by an arbitrary limit in the SAI spec. Each new attribute is appended at the end of its attribute enum so that no existing attribute is renumbered. Objects covered: counter, ACL counter, LAG, mirror session, prefix compression table, policer, scheduler, next hop group, virtual router, UDF group and router interface. The existing char attributes are marked deprecated. Exactly one of the two attributes may be set to a non-default value; setting both is invalid. Signed-off-by: Shrikrishna (Shri) Khare <skhare@meta.com> Signed-off-by: Jasmeet Bagga <jasmeetbagga@meta.com> Signed-off-by: Ron He <zecheng@meta.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
This is similar to #1158, #1407, #1430.
Those PRs added label attribute for LAG/virtual router, counter, ACL Counter. This PR adds similar label attribute for ACL entries.
Adding a label attribute that can be used to uniquely identify ACL entry object during warmboot. This attribute is considered as user data attached to the object.