Skip to content

Fix sai_thrift_read_port_counters for DNX#14743

Merged
arlakshm merged 1 commit intosonic-net:masterfrom
ysmanman:sai-thrift-count-fix
Oct 7, 2024
Merged

Fix sai_thrift_read_port_counters for DNX#14743
arlakshm merged 1 commit intosonic-net:masterfrom
ysmanman:sai-thrift-count-fix

Conversation

@ysmanman
Copy link
Contributor

Description of PR

SAI thrift call sai_thrift_read_port_counters() reads a collection of counters together, including SAI_PORT_STAT_IN_DROPPED_PKTS. Starting from BRCM SAI 11.x, SAI_PORT_STAT_IN_DROPPED_PKTS is not supported and retrieving the count may fail SAI call sonic-net/sonic-buildimage#19998. In order to retrieve other counters correctly, read SAI_PORT_STAT_IN_DROPPED_PKTS in a separate SAI call.

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

…from SAI 11.x.

Reading this counter may fail to retrieve other counters. To work around the issue,
read SAI_PORT_STAT_IN_DROPPED_PKTS count in a separate SAI call.
@ysmanman
Copy link
Contributor Author

Add @kenneth-arista @arlakshm @vmittal-msft @wenyiz2021 for viz.

@arlakshm
Copy link
Contributor

@mlok-nokia for viz..

# Read SAI_PORT_STAT_IN_DROPPED_PKTS now and insert the cnt at the correct
# index in the counter results.
if asic_type == 'broadcom':
in_drop_pkts_cnt_id = [SAI_PORT_STAT_IN_DROPPED_PKTS]
Copy link
Contributor

Choose a reason for hiding this comment

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

if BRCM SAI 11.x, SAI_PORT_STAT_IN_DROPPED_PKTS is not supported, will the call to sai_thrift_get_port_stats be successful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The counter is supported on Broadcom XGS. On Broadcom DNX, the SAI call will fail. But this does not fail test today since the sai thrift call does not check the return value. So on DNX, test will get count value 0.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we check for broadcom-dnx instead ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @vmittal-msft , today the SAI API sai_thrift_read_port_counters takes asic_type as input, which only tells if the asic if broadcom or not. If we need to restrict the change to DNX, we have to pass platform_type to the API, which requires a large scope of fix because the API is used in many places today.

Copy link
Contributor

@vmittal-msft vmittal-msft left a comment

Choose a reason for hiding this comment

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

Please see if we can ignore this for DNX only using available switch type or asic type.

@ysmanman
Copy link
Contributor Author

ysmanman commented Oct 7, 2024

Please see if we can ignore this for DNX only using available switch type or asic type.

Hi @vmittal-msft , today the SAI API sai_thrift_read_port_counters takes asic_type as input, which only tells if the asic if broadcom or not. If we need to restrict the change to DNX, we have to pass platform_type to the API, which requires a large scope of fix because the API is used in many places today.

@arlakshm arlakshm merged commit b781bad into sonic-net:master Oct 7, 2024
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Oct 7, 2024
…_PKTS starting from SAI 11.x. (sonic-net#14743)

SAI thrift call sai_thrift_read_port_counters() reads a collection of counters together, including SAI_PORT_STAT_IN_DROPPED_PKTS. Starting from BRCM SAI 11.x, SAI_PORT_STAT_IN_DROPPED_PKTS is not supported and retrieving the count may fail SAI call sonic-net/sonic-buildimage#19998. In order to retrieve other counters correctly, read SAI_PORT_STAT_IN_DROPPED_PKTS in a separate SAI call.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #14867

mssonicbld pushed a commit that referenced this pull request Oct 8, 2024
…_PKTS starting from SAI 11.x. (#14743)

SAI thrift call sai_thrift_read_port_counters() reads a collection of counters together, including SAI_PORT_STAT_IN_DROPPED_PKTS. Starting from BRCM SAI 11.x, SAI_PORT_STAT_IN_DROPPED_PKTS is not supported and retrieving the count may fail SAI call sonic-net/sonic-buildimage#19998. In order to retrieve other counters correctly, read SAI_PORT_STAT_IN_DROPPED_PKTS in a separate SAI call.
hdwhdw pushed a commit to hdwhdw/sonic-mgmt that referenced this pull request Oct 10, 2024
…_PKTS starting from SAI 11.x. (sonic-net#14743)

SAI thrift call sai_thrift_read_port_counters() reads a collection of counters together, including SAI_PORT_STAT_IN_DROPPED_PKTS. Starting from BRCM SAI 11.x, SAI_PORT_STAT_IN_DROPPED_PKTS is not supported and retrieving the count may fail SAI call sonic-net/sonic-buildimage#19998. In order to retrieve other counters correctly, read SAI_PORT_STAT_IN_DROPPED_PKTS in a separate SAI call.
vikshaw-Nokia pushed a commit to vikshaw-Nokia/sonic-mgmt that referenced this pull request Oct 23, 2024
…_PKTS starting from SAI 11.x. (sonic-net#14743)

SAI thrift call sai_thrift_read_port_counters() reads a collection of counters together, including SAI_PORT_STAT_IN_DROPPED_PKTS. Starting from BRCM SAI 11.x, SAI_PORT_STAT_IN_DROPPED_PKTS is not supported and retrieving the count may fail SAI call sonic-net/sonic-buildimage#19998. In order to retrieve other counters correctly, read SAI_PORT_STAT_IN_DROPPED_PKTS in a separate SAI call.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants