Skip to content

Conversation

pandeytejas
Copy link

@pandeytejas pandeytejas commented Aug 28, 2025

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.

@pandeytejas pandeytejas force-pushed the master branch 5 times, most recently from 5d32716 to a87c2bf Compare August 28, 2025 16:49
@rlhui
Copy link
Collaborator

rlhui commented Sep 3, 2025

@pandeytejas , is this presented in SAI meeting?

@pandeytejas
Copy link
Author

pandeytejas commented Sep 4, 2025

No, this is not presented yet.

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

@pandeytejas
Copy link
Author

@rlhui , any updates on the above?

@tjchadaga
Copy link
Collaborator

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

@tjchadaga
Copy link
Collaborator

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

@pandeytejas
Copy link
Author

pandeytejas commented Sep 16, 2025

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

@pandeytejas pandeytejas force-pushed the master branch 2 times, most recently from 15c3a1e to 35eaef6 Compare September 16, 2025 20:57
@tjchadaga
Copy link
Collaborator

@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

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.

Signed-off-by: Tejas Pandey <[email protected]>
@JaiOCP
Copy link
Contributor

JaiOCP commented Sep 17, 2025 via email

@pandeytejas
Copy link
Author

@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.

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.

Signed-off-by: Tejas Pandey <[email protected]>
@pandeytejas
Copy link
Author

@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
Copy link
Contributor

JaiOCP commented Sep 22, 2025

@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
Copy link
Contributor

JaiOCP commented Sep 22, 2025

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."

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.

Signed-off-by: Tejas Pandey <[email protected]>
@pandeytejas
Copy link
Author

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.

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.

Signed-off-by: Tejas Pandey <[email protected]>
@tjchadaga tjchadaga added the reviewed PR is discussed in SAI Meeting label Sep 22, 2025
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.

4 participants