Skip to content

Toggle enablePerPortCounterDiscovery based on HWSKU config - #2002

Merged
lolyu merged 2 commits into
sonic-net:masterfrom
justin-wong-ce:genFix-confdb
Aug 6, 2026
Merged

Toggle enablePerPortCounterDiscovery based on HWSKU config#2002
lolyu merged 2 commits into
sonic-net:masterfrom
justin-wong-ce:genFix-confdb

Conversation

@justin-wong-ce

@justin-wong-ce justin-wong-ce commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Description of PR

Summary:
Apply the enablePerPortCounterDiscovery syncd arg when the file enable_per_port_counter_discovery exists and has the contents true in the machine's HWSKU config in sonic-buildimage/device/....

This is part 2 of 2 fix in addressing sonic-net/sonic-buildimage#28460. There are two fix methods under consideration, this PR outlines the method of deciding code path via the HWSKU config and needs to be used together with sonic-net/sonic-buildimage#28506.

This change also requires the changes in #2000, #2001, and sonic-net/sonic-buildimage#28506 to work.

Fixes sonic-net/sonic-buildimage#28460

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation update
  • Test improvement

Approach

What is the motivation for this PR?

In #1774 , a new counter support discovery method is introduced for being able to read counter support for platforms where different ports can have different capabilities.
The method worked well for some platforms, not so much for some others.

This change applies the new syncd arg enablePerPortCounterDiscovery (introduced in #2000) when enable_per_port_counter_discovery is populated as true in SYNCD_VARS, which is retrieved from DEVICE_METADATA in CONFIG_DB.

Work item tracking
  • Microsoft ADO (number only):

How did you do it?

Add logic in syncd_init_common.sh to apply the syncd arg enablePerPortCounterDiscovery when it is defined as so in SYNCD_VARS.

How did you verify/test it?

syncd unit tests pass on master and on 202605.
On physical hardware, an image is built with the changes cherry-picked to 202605 and tests are ran in 6 scenarios:
1):

  • ASIC check: use arg on broadcom
  • HWSKU config: not defined
  • Result: per-port counter discovery code path used

2):

  • ASIC check: use arg on broadcom
  • HWSKU config: enable_per_port_counter_discovery set to true
  • Result: per-port counter discovery code path used

3):

  • ASIC check: use arg on broadcom
  • HWSKU config: enable_per_port_counter_discovery set to false
  • Result: legacy code path used

4):

  • ASIC check: use arg on mellanox
  • HWSKU config: not defined
  • Result: legacy code path used

5):

  • ASIC check: use arg on mellanox
  • HWSKU config: enable_per_port_counter_discovery set to true
  • Result: per-port counter discovery code path used

6):

  • ASIC check: use arg on mellanox
  • HWSKU config: enable_per_port_counter_discovery set to false
  • Result: legacy code path used

In all 6 scenarios, syncd enters the expected code path.

Additionally, a subset of counters-focused sonic-mgmt tests are ran. There is no test result difference compared to tests done without this change.
Tests were ran on Arista-7260CX3-D108C8.

The list of sonic-mgmt tests ran:

test_pretest.py
dhcp_relay/test_dhcp_counter_stress.py
drop_packets/test_drop_counters.py
drop_packets/test_configurable_drop_counters.py
gnmi/test_gnmi_countersdb.py
snmp/test_snmp_queue_counters.py
test_posttest.py

Any platform specific information?

Broadcom

Documentation

Toggle the use of the per-port counter discovery arg based on whether
enable_per_port_counter_discovery option is enabled in the HWSKU
config.

Signed-off-by: Justin Wong <jvwong@arista.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

@azure-pipelines

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

dgsudharsan
dgsudharsan previously approved these changes Jul 21, 2026
@justin-wong-ce

justin-wong-ce commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Need to update the condition to override the ASIC type-based check from #2001 when enable_per_port_counter_discovery exists.

@justin-wong-ce

Copy link
Copy Markdown
Contributor Author

I will update after #2001 gets merged, updating the condition now just means a merge conflict to deal with when #2001 gets merged.

@justin-wong-ce
justin-wong-ce marked this pull request as ready for review July 30, 2026 05:03
@azure-pipelines

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

lolyu
lolyu previously approved these changes Aug 3, 2026

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

LGTM

@lolyu

lolyu commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Hi @justin-wong-ce, could you please help with the conflict?

@justin-wong-ce
justin-wong-ce dismissed stale reviews from lolyu and dgsudharsan via 2745b39 August 5, 2026 20:15
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

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

LGTM

@lolyu
lolyu merged commit a0a1dcb into sonic-net:master Aug 6, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants