Skip to content

Add wide label attribute for objects with a label attribute - #2327

Open
zechengh09 wants to merge 1 commit into
opencomputeproject:masterfrom
zechengh09:counter-label-extended
Open

Add wide label attribute for objects with a label attribute#2327
zechengh09 wants to merge 1 commit into
opencomputeproject:masterfrom
zechengh09:counter-label-extended

Conversation

@zechengh09

@zechengh09 zechengh09 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary:

This is similar to #1158, #1407, #1430, #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.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@zechengh09
zechengh09 marked this pull request as ready for review August 5, 2026 18:01
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Comment thread inc/saiacl.h Outdated
Comment thread inc/saicounter.h Outdated
@zechengh09 zechengh09 changed the title Add extended label attribute for counter and ACL counter Add wide label attribute for counter and ACL counter Aug 5, 2026
@zechengh09
zechengh09 force-pushed the counter-label-extended branch from 52ead54 to af73f3e Compare August 5, 2026 21:00
@zechengh09 zechengh09 changed the title Add wide label attribute for counter and ACL counter Add wide label attribute for objects with a label attribute Aug 6, 2026
@zechengh09
zechengh09 force-pushed the counter-label-extended branch from af73f3e to 16bac09 Compare August 6, 2026 15:53
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
zechengh09 force-pushed the counter-label-extended branch from 16bac09 to d446f26 Compare August 6, 2026 19:22

@JaiOCP JaiOCP left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks Good

Comment thread inc/sairouterinterface.h
*
* Replaces #SAI_ROUTER_INTERFACE_ATTR_LABEL, which is limited to 32 bytes.
* Exactly one of the two attributes may be set to a non-default
* value; setting both is invalid.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

from my perspective it could be valid, and more over they could be even different at the same time

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review - the wide label attribute is added to replace the current label attribute, which is marked as deprecated. It is kept for backward compatibility, but our goal is for both NOS and SDK starting using the new attribute in the new spec.

Therefore, only one attribute should be set at a given time. They could be different only if NOS starting to use the wide label with more than 32 characters.

@tjchadaga

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@tjchadaga tjchadaga added the reviewed PR is discussed in SAI Meeting label Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewed PR is discussed in SAI Meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants