Skip to content

Add new config to enable per-port counter discovery for syncd - #28506

Open
justin-wong-ce wants to merge 6 commits into
sonic-net:masterfrom
justin-wong-ce:genFix-confdb
Open

Add new config to enable per-port counter discovery for syncd#28506
justin-wong-ce wants to merge 6 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

Adding a new HWSKU config enable_per_port_counter_discovery to be placed in sonic-buildimage/device/... to toggle the per-port counter discovery feature when checking for counters to poll during syncd's initialization.

This config will be stored into CONFIG_DB and be used to decide what arguments to use to invoke syncd with via syncd_init_common.sh.

An example is added in this change for device/arista/x86_64-arista_7060x6_64pe_b/Arista-7060X6-64PE-B-C512S2.

This is part 2 of 2 fix in addressing #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-sairedis#2002.

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

Why I did it

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 sonic-net/sonic-sairedis#2000) when enable_per_port_counter_discovery is populated in the HWSKU's config. This will be read from sonic-buildimage/device... and then set into DEVICE_METADATA in CONFIG_DB.

Work item tracking
  • Microsoft ADO (number only):

How I did it

  • Added logic to image to read the new config file from the HWSKU config folder.
  • Added logic to populate the new config as part of variables to pass to syncd for initialization.
  • Added enable_per_port_counter_discovery as a valid CONFIG_DB entry in the device_metadata yang file.
  • Added example config for device/arista/x86_64-arista_7060x6_64pe_b/Arista-7060X6-64PE-B-C512S2

How to verify it

(Please see the test result section below)

Which release branch to backport (provide reason below if selected)

  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605
  • 202608

Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO):
Failure type:

Tested branch

  • master
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605
  • 202608
  • N/A

Test result

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.

Description for the changelog

Add enable_per_port_counter_discovery config definitions and propogation logic.

Link to config_db schema for YANG module changes

https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md#device-metadata

A picture of a cute animal (not mandatory but encouraged)

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

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

@mssonicbld

Copy link
Copy Markdown
Collaborator

This PR has backport request for branch(es): 202605.
Added label(s) for branch(es) 202605.

---Powered by SONiC BuildBot

@mssonicbld mssonicbld added the Tested for 202605 branch Tested for 202605 branch label Jul 21, 2026
@mssonicbld

Copy link
Copy Markdown
Collaborator

The Tested branch section has been ticked and Test result is provided for branch(es): 202605. Added label(s): Tested for 202605 Branch.

---Powered by SONiC BuildBot

@justin-wong-ce

Copy link
Copy Markdown
Contributor Author

/azpw retry

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) jobs...

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) stages in build 1170168:

✅Stage Test:

  • Job impacted-area-kvmtest-t1-lag-vpp by Elastictest: retried.

@dgsudharsan dgsudharsan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you please add UT for lang model changes?

@justin-wong-ce

Copy link
Copy Markdown
Contributor Author

Can you please add UT for lang model changes?

Ack, will be working on it

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

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

@justin-wong-ce

Copy link
Copy Markdown
Contributor Author

Can you please add UT for lang model changes?

Added

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

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

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

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

@justin-wong-ce

Copy link
Copy Markdown
Contributor Author

Need to add logic such that SYNCD_VARS will be able to reflect true, false, and undefined state, rather than the current true, false (where it defaults to false).

We need this logic to be able to know when to use the ASIC type based check from sonic-net/sonic-sairedis#2001 and when to use the HWSKU config.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

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

@justin-wong-ce
justin-wong-ce marked this pull request as ready for review July 24, 2026 20:33
@azure-pipelines

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

@justin-wong-ce

Copy link
Copy Markdown
Contributor Author

/azpw retry

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) jobs...

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) stages in build 1173234:

✅Stage Test:

  • Job impacted-area-kvmtest-multi-asic-t1 by Elastictest: retried.
  • Job impacted-area-kvmtest-t2 by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag-vpp by Elastictest: retried.
  • Job impacted-area-kvmtest-t0-sonic by Elastictest: retried.
  • Job impacted-area-kvmtest-t0-2vlans by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag by Elastictest: retried.
  • Job impacted-area-kvmtest-dpu by Elastictest: retried.
  • Job impacted-area-kvmtest-dualtor by Elastictest: retried.
  • Job impacted-area-kvmtest-t0 by Elastictest: retried.

@justin-wong-ce

Copy link
Copy Markdown
Contributor Author

/azpw retry

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) jobs...

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) stages in build 1173234:

✅Stage Test:

  • Job impacted-area-kvmtest-dualtor by Elastictest: retried.
  • Job impacted-area-kvmtest-t2 by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag-vpp by Elastictest: retried.
  • Job impacted-area-kvmtest-dpu by Elastictest: retried.
  • Job impacted-area-kvmtest-t0-sonic by Elastictest: retried.
  • Job impacted-area-kvmtest-multi-asic-t1 by Elastictest: retried.
  • Job impacted-area-kvmtest-t0 by Elastictest: retried.
  • Job impacted-area-kvmtest-t0-2vlans by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag by Elastictest: retried.

@justin-wong-ce

justin-wong-ce commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Will address failing CI when sonic-net/sonic-sairedis#2001 is merged

@justin-wong-ce

Copy link
Copy Markdown
Contributor Author

/azpw retry

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) jobs...

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) stages in build 1173234:

✅Stage Test:

  • Job impacted-area-kvmtest-multi-asic-t1 by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag-vpp by Elastictest: retried.
  • Job impacted-area-kvmtest-t2 by Elastictest: retried.
  • Job impacted-area-kvmtest-dualtor by Elastictest: retried.
  • Job impacted-area-kvmtest-t0-sonic by Elastictest: retried.
  • Job impacted-area-kvmtest-t0 by Elastictest: retried.
  • Job impacted-area-kvmtest-dpu by Elastictest: retried.
  • Job impacted-area-kvmtest-t0-2vlans by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag by Elastictest: retried.

Adding a new HWSKU config to toggle per-port counter discovery when
checking for counter to polls in syncd initialization.

This config will be stored into CONFIG_DB and and used to decide what
arguments to use to invoke syncd via syncd_init_common.sh

Signed-off-by: Justin Wong <jvwong@arista.com>
Signed-off-by: Justin Wong <jvwong@arista.com>
Add the new entry enable_per_port_counter_discovery to be part of yang's
device_metadata unit test.

Signed-off-by: Justin Wong <jvwong@arista.com>
Cast the new enable_per_port_counter_discovery config file to all Arista
HWSKUs as they all need the per-port counter discovery logic for
counters to fully work.

Signed-off-by: Justin Wong <jvwong@arista.com>
This reverts commit 9430c38.
Broadcom ASICs no longer need a config file to use the per-port
discovery code path.

Retaining config in Arista-7060X6-64PE-B-C512S2 as an example for what
the config should look like.

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

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

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

@justin-wong-ce

Copy link
Copy Markdown
Contributor Author

/azpw retry

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) jobs...

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) stages in build 1185058:

✅Stage Test:

  • Job impacted-area-kvmtest-multi-asic-t1 by Elastictest: retried.
  • Job impacted-area-kvmtest-t2 by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag-vpp by Elastictest: retried.
  • Job impacted-area-kvmtest-t0-sonic by Elastictest: retried.
  • Job impacted-area-kvmtest-t0-2vlans by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag by Elastictest: retried.
  • Job impacted-area-kvmtest-dpu by Elastictest: retried.
  • Job impacted-area-kvmtest-dualtor by Elastictest: retried.
  • Job impacted-area-kvmtest-t0 by Elastictest: retried.

{%if DEVICE_METADATA.localhost.supporting_bulk_counter_groups is defined and DEVICE_METADATA.localhost.supporting_bulk_counter_groups != '' -%}
"supporting_bulk_counter_groups": "{{ DEVICE_METADATA.localhost.supporting_bulk_counter_groups|join(',') }}",
{% endif -%}
"enable_per_port_counter_discovery": {% if DEVICE_METADATA.localhost.enable_per_port_counter_discovery|string|lower == "true" %}"true"{% else %}"false"{% endif %},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Missing if statement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants