[trim]: Add Packet Trimming Asym DSCP to OA by nazariig · Pull Request #3705 · sonic-net/sonic-swss

nazariig · 2025-06-12T14:33:11Z

Signed-off-by: Nazarii Hnydyn nazariig@nvidia.com

DEPENDS:

HLD: sonic-net/SONiC#1988

What I did

Implemented Packet Trimming Asymmetric DSCP feature

Why I did it

Implementation is done according to the Packet Trimming Asymmetric DSCP HLD

How I verified it

Run Packet Trimming VS UTs

Details if related

N/A

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

      .---.        .-----------
     /     \  __  /    ------
    / /     \(  )/    -----
   //////   ' \/ `   ---
  //// / // :    : ---
 // /   /  /`    '--
//          //..\\
       ====UU====UU====
           '//||\\`
             ''``

mssonicbld · 2025-06-12T14:33:19Z

/azp run

azure-pipelines · 2025-06-12T14:33:31Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-06-12T15:29:50Z

/azp run

azure-pipelines · 2025-06-12T15:30:00Z

Azure Pipelines successfully started running 1 pipeline(s).

kperumalbfn · 2025-06-30T22:27:23Z

@nazariig could you resolve the conflicts

mssonicbld · 2025-07-01T18:30:29Z

/azp run

azure-pipelines · 2025-07-01T18:30:39Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-01T18:32:02Z

/azp run

azure-pipelines · 2025-07-01T18:32:12Z

Azure Pipelines successfully started running 1 pipeline(s).

kperumalbfn · 2025-07-01T20:59:47Z

orchagent/buffer/buffercontainer.h

+    {
+        return ((pgRefCount > 0) || (iBufProfListRefCount > 0) || (eBufProfListRefCount)) ? true : false;
+    }
+


we could keep all the counts as private and have methods to set/get. Would be clean to set/get by one method instead of direct access

Currently not all egress buffer profiles are added to profile list. Could you please check?

https://github.com/sonic-net/sonic-buildimage/pull/22869/files

we could keep all the counts as private and have methods to set/get. Would be clean to set/get by one method instead of direct access

@kperumalbfn Getter/Setter is a way of doing business logic encapsulation. In this case it is redundant and won't provide any benefits. That was the reason for omitting the implementation

Currently not all egress buffer profiles are added to profile list. Could you please check?

https://github.com/sonic-net/sonic-buildimage/pull/22869/files

The current implementation relies on application db events, meaning all buffer profiles are processed in a single place - bufferorch. Please elaborate on how this relates to the current validation case ?

kperumalbfn · 2025-07-01T21:03:38Z

orchagent/buffer/bufferhelper.cpp

+{
+    auto &map = cfg.fieldValueMap;
+
+    const auto &cit = map.find(BUFFER_PORT_INGRESS_PROFILE_LIST_PROFILE_LIST);


Does bufferorch use ingress/egress buffer profile list for trim eligible validation?

@kperumalbfn validation is currently implemented for the next cases:

Buffer Profile is attached to either Ingress PG or Ingress/Egress Buffer Profile List and user tries to set trimming eligibility

Buffer Profile is configured as trimming eligible and user tries to attach it either to Ingress PG or Ingress/Egress Buffer Profile list

In both cases the error will be generated.
This guarantees that trimming eligible Buffer Profile can be used only with the Queue objects.

kperumalbfn · 2025-07-01T21:17:53Z

orchagent/buffer/buffercontainer.h

+
+    inline bool isTrimmingProhibited() const
+    {
+        return ((pgRefCount > 0) || (iBufProfListRefCount > 0) || (eBufProfListRefCount)) ? true : false;


Can we avoid this refcount to determine the trim eligibility?

if possible, buffer_profile-->buffer_pool(check ingress/egress)?

@stephenxs any other option?

Can we avoid this refcount to determine the trim eligibility?

@kperumalbfn currently the most reliable way. Checking counter value is a low cost operation comparing to collection iteration/search

if possible, buffer_profile-->buffer_pool(check ingress/egress)?

@kperumalbfn Buffer Profile to Buffer Pool relation check (resolveFieldRefArray) is implemented as part of:
task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tuple)
task_process_status BufferOrch::processIngressBufferProfileList(KeyOpFieldsValuesTuple &tuple)
task_process_status BufferOrch::processEgressBufferProfileList(KeyOpFieldsValuesTuple &tuple)

Trimming eligibility validation serves different purposes and takes place long after this check, so it is not relevant

Why not using the pools that the buffer profiles reference? That will be much easier to maintain.

kperumalbfn · 2025-07-01T21:41:36Z

orchagent/switchorch.cpp

+                    SWSS_LOG_ERROR("Failed to set switch trimming TC value in SAI");
+                    return false;
+                }
+


nit: empty new lines, pls check

@kperumalbfn new line is added after if statement. No extra spaces are observed. Please elaborate on the issue

kperumalbfn · 2025-07-01T21:42:46Z

orchagent/switchorch.cpp

+            }
+            else
+            {
+                SWSS_LOG_WARN("Skip setting switch trimming TC value for symmetric DSCP mode");


return from here?

@kperumalbfn the implementation is by design. If the user tries to set TC value while operating in symmetric DSCP mode, SWSS will save the new value to cache and print the warning message. Later on if the mode will be switched to asymmetric, the cached value will be set to SAI.

The warning message is printed only once - in case if a new (differs from the value stored in the cache) TC value is configured.
That helps to maintain the minimal informative level while avoiding the log pollution (in case user/controller sets the same value for multiple times).

mssonicbld · 2025-07-14T16:38:26Z

/azp run

azure-pipelines · 2025-07-14T16:38:36Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>

mssonicbld · 2025-07-14T18:30:33Z

/azp run

azure-pipelines · 2025-07-14T18:30:44Z

Azure Pipelines successfully started running 1 pipeline(s).

prsunny

approving for merge. Code reviewed by SMEs

r12f · 2025-07-15T06:47:58Z

hi @nazariig , do you mind to help create a manual cherry pick to 202412 for this PR?

dgsudharsan · 2025-07-15T15:07:34Z

@r12f Here is the PR Azure/sonic-swss.msft#105

mssonicbld · 2025-07-15T17:14:17Z

Cherry-pick PR to 202505: #3765

DEPENDS: [trim]: Add Packet Trimming Asym DSCP to VS lib sonic-sairedis#1610 [trim]: Add Packet Trimming to OA sonic-net#3594 HLD: sonic-net/SONiC#1988 What I did Implemented Packet Trimming Asymmetric DSCP feature Why I did it Implementation is done according to the Packet Trimming Asymmetric DSCP HLD

mssonicbld · 2025-08-03T01:39:21Z

Cherry-pick PR to msft-202506: Azure/sonic-swss.msft#120

DEPENDS: [trim]: Add Packet Trimming Asym DSCP to VS lib sonic-sairedis#1610 [trim]: Add Packet Trimming to OA sonic-net#3594 HLD: sonic-net/SONiC#1988 What I did Implemented Packet Trimming Asymmetric DSCP feature Why I did it Implementation is done according to the Packet Trimming Asymmetric DSCP HLD

DEPENDS: [trim]: Add Packet Trimming Asym DSCP to VS lib sonic-sairedis#1610 [trim]: Add Packet Trimming to OA sonic-net#3594 HLD: sonic-net/SONiC#1988 What I did Implemented Packet Trimming Asymmetric DSCP feature Why I did it Implementation is done according to the Packet Trimming Asymmetric DSCP HLD Signed-off-by: Lawrence Lee <lawlee@microsoft.com>

DEPENDS: [trim]: Add Packet Trimming Asym DSCP to VS lib sonic-sairedis#1610 [trim]: Add Packet Trimming to OA sonic-net#3594 HLD: sonic-net/SONiC#1988 What I did Implemented Packet Trimming Asymmetric DSCP feature Why I did it Implementation is done according to the Packet Trimming Asymmetric DSCP HLD Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>

nazariig requested review from kperumalbfn and r12f June 12, 2025 14:33

nazariig requested review from dgsudharsan, keboliu, liat-grozovik, moshemos and volodymyrsamotiy June 12, 2025 14:33

nazariig added the Request for msft-202412 Branch label Jun 12, 2025

mssonicbld added the Cherry Pick Conflict_msft-202412 label Jun 12, 2025

nazariig mentioned this pull request Jun 12, 2025

Packet Trimming: Asymmetric DSCP HLD sonic-net/SONiC#1988

Merged

nazariig marked this pull request as ready for review June 12, 2025 16:03

nazariig requested review from abdosi, bingwang-ms, judyjoseph and prsunny as code owners June 12, 2025 16:03

zhangyanzhao requested a review from developfast June 27, 2025 04:08

nazariig mentioned this pull request Jul 1, 2025

switch-trimming global config capability check #3730

Open

nazariig force-pushed the master-trim-asym branch from 6bdedb1 to 2709cc6 Compare July 1, 2025 18:30

nazariig force-pushed the master-trim-asym branch from 2709cc6 to 10211ef Compare July 1, 2025 18:31

kperumalbfn reviewed Jul 1, 2025

View reviewed changes

[trim]: Add Packet Trimming Asym DSCP to OA

837fb98

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>

nazariig force-pushed the master-trim-asym branch from a169dde to 837fb98 Compare July 14, 2025 18:29

prsunny approved these changes Jul 14, 2025

View reviewed changes

prsunny merged commit ad80fa5 into sonic-net:master Jul 14, 2025
15 checks passed

prsunny added the Approved for 202505 Branch label Jul 15, 2025

mssonicbld mentioned this pull request Jul 15, 2025

[action] [PR:3705] [trim]: Add Packet Trimming Asym DSCP to OA #3765

Merged

mssonicbld added the Created PR to 202505 Branch label Jul 15, 2025

prabhataravind added the Request for msft-202506 Branch label Jul 15, 2025

mssonicbld added Included in 202505 Branch and removed Created PR to 202505 Branch labels Jul 15, 2025

r12f added Approved for msft-202412 Branch Included in msft-202412 Branch and removed Cherry Pick Conflict_msft-202412 labels Jul 16, 2025

mssonicbld added the Cherry Pick Conflict_msft-202412 label Jul 16, 2025

prabhataravind added the Approved for msft-202506 Branch label Aug 3, 2025

mssonicbld mentioned this pull request Aug 3, 2025

[action] [PR:3705] [trim]: Add Packet Trimming Asym DSCP to OA Azure/sonic-swss.msft#120

Merged

mssonicbld added the Created PR to msft-202506 Branch label Aug 3, 2025

Conversation

nazariig commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

mssonicbld commented Jun 12, 2025

Uh oh!

azure-pipelines bot commented Jun 12, 2025

Uh oh!

mssonicbld commented Jun 12, 2025

Uh oh!

azure-pipelines bot commented Jun 12, 2025

Uh oh!

kperumalbfn commented Jun 30, 2025

Uh oh!

mssonicbld commented Jul 1, 2025

Uh oh!

azure-pipelines bot commented Jul 1, 2025

Uh oh!

mssonicbld commented Jul 1, 2025

Uh oh!

azure-pipelines bot commented Jul 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Jul 14, 2025

Uh oh!

azure-pipelines bot commented Jul 14, 2025

Uh oh!

mssonicbld commented Jul 14, 2025

Uh oh!

azure-pipelines bot commented Jul 14, 2025

Uh oh!

prsunny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

r12f commented Jul 15, 2025

Uh oh!

dgsudharsan commented Jul 15, 2025

Uh oh!

mssonicbld commented Jul 15, 2025

Uh oh!

mssonicbld commented Aug 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

nazariig commented Jun 12, 2025 •

edited

Loading