Skip to content

Fix sai_thrift_read_port_counters for DNX #14743

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
arlakshm merged 1 commit into from
Oct 7, 2024
Merged

Fix sai_thrift_read_port_counters for DNX #14743

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion tests/saitests/py3/switch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,14 @@ def sai_thrift_read_port_counters(client, asic_type, port):
port_cnt_ids.append(SAI_PORT_STAT_PFC_7_TX_PKTS)
port_cnt_ids.append(SAI_PORT_STAT_IF_OUT_OCTETS)
port_cnt_ids.append(SAI_PORT_STAT_IF_OUT_UCAST_PKTS)
port_cnt_ids.append(SAI_PORT_STAT_IN_DROPPED_PKTS)

# broadcom-dnx does not support SAI_PORT_STAT_IN_DROPPED_PKTS. Reading this counter may fail
# to retrieve other counters (https://github.com/sonic-net/sonic-buildimage/issues/19998).
# Since we cannot tell if the ASIC is DNX or or not from provided asic_type, read the counter
# separately in another SAI call for all broadcom ASIC.
if asic_type != 'broadcom':
port_cnt_ids.append(SAI_PORT_STAT_IN_DROPPED_PKTS)

port_cnt_ids.append(SAI_PORT_STAT_OUT_DROPPED_PKTS)
port_cnt_ids.append(SAI_PORT_STAT_IF_IN_UCAST_PKTS)
port_cnt_ids.append(SAI_PORT_STAT_IF_IN_NON_UCAST_PKTS)
Expand All @@ -819,6 +826,14 @@ def sai_thrift_read_port_counters(client, asic_type, port):
if asic_type == 'mellanox':
counters_results.append(0)

# 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

@ansrajpu-git ansrajpu-git Sep 26, 2024

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

@ysmanman ysmanman Sep 26, 2024

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

@vmittal-msft vmittal-msft Sep 27, 2024

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

@ysmanman ysmanman Oct 7, 2024

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.

in_drop_pkts_cnt_result = client.sai_thrift_get_port_stats(
port, in_drop_pkts_cnt_id, 1)
counters_results.insert(12, in_drop_pkts_cnt_result[0])

queue_list = []
port_attr_list = client.sai_thrift_get_port_attribute(port)
attr_list = port_attr_list.attr_list
Expand Down