Add SAI_ATTR_PORT_FW_REVISION to saiport

[pandeytejas]

This proposal outlines the addition of a new port attribute designed to provide a standardized method for querying the active SerDes firmware revision of a port.

Proposal -

Add label SAI_ATTR_PORT_FW_REVISION under sai_port_attr_h in saiport.h to store firmware related information. This attribute will store a string denoting the current SerDes firmware version running on the port.

[rlhui]

@pandeytejas , is this presented in SAI meeting?

[pandeytejas]

No, this is not presented yet.

Please let me know when is the earliest we can do that if that is necessary.

[pandeytejas]

@rlhui , any updates on the above?

[tjchadaga]

@pandeytejas - are you able to discuss this PR in this week's community meeting (on 9/18)?

[tjchadaga]

@pandeytejas - also, please resolve the DCO error by updating the signature

[pandeytejas]

@tjchadaga sure, I can discuss this PR on 9/18. Please send me an invite for the same.

[tjchadaga]

@pandeytejas - we have a couple of other PRs scheduled for 9/18. We can discuss this as well if time permits. Otherwise, I will add it to next week's agenda

[JaiOCP]

Tejas, We always provide an example workflow to be clear for the developers. Speculative attribute is not a good idea.On Sep 17, 2025, at 1:46 PM, Tejas Pandey ***@***.***> wrote: @pandeytejas commented on this pull request. In inc/saiport.h:

@@ -2687,6 +2687,26 @@ typedef enum _sai_port_attr_t

*/ SAI_PORT_ATTR_PAM4_EYE_VALUES, + /** + * @brief Get port firmware revision + * + * Standard attribute to collect port SerDes firmware rev or + * any other vendor specific firmware rev. + * + * @type sai_s8_list_t + * @flags READ_ONLY + */ + + SAI_PORT_ATTR_FW_REVISION, I don't have a specific example for this, we originally did this to be flexible so that vendors can use it as they like say some use only x.y whereas others use a single hex or even alphanumeric but we are open to changing it to multiple uint32, maybe we can discuss this in the wider forum? A usecase of this is to change the current firmware on a running port. We can have workflows that check the currently running firmware version by reading this attribute and then trigger and upgrade or downgrade if required. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: ***@***.***>

[pandeytejas]

@JaiOCP I think the last comment came out wrong - I do not have explicit example to support the string vs uint argument.

I can present a sample usecase and workflow for these attributes -

The usecase for this today is that based on the optic type and speed + modulation being used we have seen that certain serdes firmware would be more suitable over others which means we would have cases wherein we need to dynamically load the firmware. We currently do not have a way to query and cross check if the loaded firmware is what we expect and this attribute is to address that gap.

For a sample workflow -

1. The NOS will query for SAI_PORT_ATTR_FW_REVISION_SIZE (READ_ONLY) and SAI_PORT_ATTR_FW_REVISION (READ_ONLY)
2. The NOS allocates appropriate space for reading the fw revision based on the retrieved size. If it cannot - it will error out.
3. The NOS reads SAI_PORT_FW_REVISION
4. The NOS will read other relevant attributes and determine if SAI_PORT_ATTR_FW_REVISION is the expected one.

We do not have enums to support dynamically loading the firmware but that seems a good to have and could be added at a later stage. We will setup internal mechanism to address the mismatch issue such as triggering dynamic reload or alert pipelines whichever is more suitable for the vendor.

[pandeytejas]

@JaiOCP @tjchadaga based on the community sync, made following changes -

1. Removed Size Attribute
2. Made naming more specific to indicate that this is serdes firmware

Denoting a usecase as to how the values would be received by the NOS, we can use the following workflow to understand -

1. The NOS will query for SAI_PORT_ATTR_SERDES_FW_REVISION (READ_ONLY)
2. The NOS will receive a reply containing the count and start address to the memory containig the string denoting the serdes firmware revision (lets say for example the serdes fw revision here is Zr190). This will look like -

{
count: 6
list: 0x0b3625210
}

Now the memory address will contain Zr160\0 as the next 6 characters as indicated by the count field.
3. The NOS can allocate memory based on the count field and read the characters starting at the given address.

Please let me know if this is what you were looking for. Thanks for reviewing!

[JaiOCP]

@JaiOCP @tjchadaga based on the community sync, made following changes -

1. Removed Size Attribute
2. Made naming more specific to indicate that this is serdes firmware

Denoting a usecase as to how the values would be received by the NOS, we can use the following workflow to understand -

1. The NOS will query for SAI_PORT_ATTR_SERDES_FW_REVISION (READ_ONLY)
2. The NOS will receive a reply containing the count and start address to the memory containig the string denoting the serdes firmware revision (lets say for example the serdes fw revision here is Zr190). This will look like -

{
count: 6
list: 0x0b3625210
}

Now the memory address will contain Zr160\0 as the next 6 characters as indicated by the count field. 3. The NOS can allocate memory based on the count field and read the characters starting at the given address.

Please let me know if this is what you were looking for. Thanks for reviewing!

Yes, thats what I was looking. Thanks

[JaiOCP]

This proposal outlines the addition of a new port attribute designed to provide a standardized method for querying the active firmware revision of a port.

Proposal -

Add label SAI_ATTR_PORT_FW_REVISION under sai_port_attr_h in saiport.h to store firmware related information. This attribute will store a string denoting the current firmware version running on the port. This can denote either the SerDes firmware version or any other vendor specific firmware version that needs to be exported.

Please fix this statement now that attribute is specific to SERDES.

"This can denote either the SerDes firmware version

- [ ] or any other vendor specific firmware version

that needs to be exported."

[pandeytejas]

This proposal outlines the addition of a new port attribute designed to provide a standardized method for querying the active firmware revision of a port.
Proposal -
Add label SAI_ATTR_PORT_FW_REVISION under sai_port_attr_h in saiport.h to store firmware related information. This attribute will store a string denoting the current firmware version running on the port. This can denote either the SerDes firmware version or any other vendor specific firmware version that needs to be exported.

Please fix this statement now that attribute is specific to SERDES.

"This can denote either the SerDes firmware version

- [ ] or any other vendor specific firmware version

that needs to be exported."

Edited - commit message, original message and code comment. PTAL.

[pandeytejas]

Hi folks,

Is there something pending on this from my end? Can we get this merged into the mainline?

[tjchadaga]

/azp run

[azure-pipelines]

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

[pandeytejas]

@tjchadaga do I need to resolve these conflicts or would they be taken care of?

[tjchadaga]

@pandeytejas - please resolve branch conflicts and do a squash commit

[pandeytejas]

@tjchadaga - I resolved the merge conflict but I mistakenly squashed all commits, do I need to create a new commit now? is there any other way to resolve?

[pandeytejas]

@tjchadaga - fixed the issue, no conflicts now and a single commit. Please let me know if it can be merged now. Thank you!

[tjchadaga]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[tjchadaga]

@pandeytejas - could you please fix the errors reported by meta checker?

WARNING: not expected number of spaces after * (1,4,8) saiport.h 2726: *
WARNING: line ends in whitespace saiport.h 2726: *
WARNING: Header doesn't meet style requirements (most likely ident is not 4 or 8 spaces) saiport.h 2731: */

[pandeytejas]

Done, please re run the pipelines. Thanks!

[tjchadaga]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).