[orchagent][MirrorOrch] Add sampled port mirroring support to MirrorOrch by Janetxxx · Pull Request #4488 · sonic-net/sonic-swss

Janetxxx · 2026-04-16T07:22:44Z

What I did
Depends on: #4502

Adds sampled port mirroring support to MirrorOrch (orchagent), extending mirror session configuration to optionally create and attach a SAI SamplePacket object and to program ports using sampled-mirroring attributes instead of full-mirror attributes.

Extend MirrorEntry and CONFIG_DB parsing to support sample_rate and truncate_size.
Create/remove SAI SamplePacket objects when sample_rate > 0 and wire them into session activation/deactivation.
Update per-port mirroring programming to use SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE + SAI_PORT_ATTR_INGRESS_SAMPLE_MIRROR_SESSION for sampled mirroring.

Why I did it
Added sampled port mirroring with truncation support on ERSPAN sessions

How I verified it

Details if related

mssonicbld · 2026-04-16T07:22:52Z

/azp run

azure-pipelines · 2026-04-16T07:23:04Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull request overview

Adds sampled port mirroring support to MirrorOrch (orchagent), extending mirror session configuration to optionally create and attach a SAI SamplePacket object and to program ports using sampled-mirroring attributes instead of full-mirror attributes.

Changes:

Extend MirrorEntry and CONFIG_DB parsing to support sample_rate and truncate_size.
Create/remove SAI SamplePacket objects when sample_rate > 0 and wire them into session activation/deactivation.
Update per-port mirroring programming to use SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE + SAI_PORT_ATTR_INGRESS_SAMPLE_MIRROR_SESSION for sampled mirroring.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

File	Description
orchagent/mirrororch.h	Adds sampled-mirroring fields to `MirrorEntry` and declares helper APIs / updated port programming signature.
orchagent/mirrororch.cpp	Parses new CONFIG_DB fields, creates/removes SamplePacket, and programs ports for sampled mirroring.

mssonicbld · 2026-04-16T10:55:45Z

/azp run

azure-pipelines · 2026-04-16T10:55:55Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-04-16T11:23:12Z

/azp run

azure-pipelines · 2026-04-16T11:23:24Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-04-16T11:58:09Z

/azp run

- MirrorEntry: add sample_rate, truncate_size, samplepacketId fields - createEntry: parse sample_rate and truncate_size from CONFIG_DB - createEntry: validate sampled mirroring only supports RX direction - activateSession: create SamplePacket (TYPE=MIRROR_SESSION) when sample_rate > 0 - deactivateSession: remove SamplePacket before removing mirror session - setUnsetPortMirror: sampled path uses INGRESS_SAMPLEPACKET_ENABLE + INGRESS_SAMPLE_MIRROR_SESSION instead of INGRESS_MIRROR_SESSION - setUnsetPortMirror: rollback on partial failure - createSamplePacket/removeSamplePacket: new helper functions - setSessionState: include sample_rate and truncate_size Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Janet Cui <janet970527@gmail.com>

azure-pipelines · 2026-04-16T11:58:20Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-04-16T11:58:31Z

/azp run

azure-pipelines · 2026-04-16T11:58:43Z

Azure Pipelines successfully started running 1 pipeline(s).

banidoru

Resource leak: samplepacket not cleaned up if port mirror config fails after samplepacket creation succeeds in activateSession
Silent fallthrough: egress + sample_rate > 0 silently uses full-mirror path in setUnsetPortMirror — needs defensive guard
Undefined OID usage: createSamplePacket may use garbage samplepacketId after SAI failure if handleSaiCreateStatus returns task_success
Naming inconsistency: new fields use snake_case vs existing camelCase convention

banidoru

In setUnsetPortMirror, the sample_rate > 0 && ingress check is insufficient. If ingress is false, the function silently falls through to the non-sampled path. Add a guard to explicitly reject sampled egress mirroring.
In activateSession, a resource leak can occur. If createSamplePacket succeeds but configurePortMirrorSession fails, the created samplepacket is not cleaned up.
In createSamplePacket, if create_samplepacket fails, the code may log a success message and return true if handleSaiCreateStatus returns task_success, leading to the use of an uninitialized samplepacketId.
In deactivateSession, if removeSamplePacket fails, the error is logged but the function proceeds. This can lead to a resource leak. The function should return false on failure.

banidoru

to_uint<uint32_t> can throw on malformed config input, crashing orchagent instead of returning task_invalid_entry
deactivateSession clears ports before removing samplepacket — if samplepacket removal fails, state is half-torn-down with no recovery path
createSamplePacket logs success and returns true even when SAI create failed (if handleSaiCreateStatus returns task_success) — OID may be garbage
Resource leak: samplepacket not cleaned up when configurePortMirrorSession fails after successful createSamplePacket

banidoru · 2026-04-16T23:19:04Z

+            return false;
+        }
+    }
+


Half-deactivated state on samplepacket removal failure: At this point, configurePortMirrorSession(name, session, false) has already succeeded — ports are unconfigured. If removeSamplePacket fails and returns false, the mirror session and samplepacket SAI objects remain, but the ports no longer reference them. A subsequent retry of deactivateSession will attempt configurePortMirrorSession(false) again on already-cleared ports.

Consider restructuring so samplepacket removal failure still proceeds to remove the mirror session (log and continue), or skip port unconfiguration on retry if ports are already cleared.

Still open — this concern (half-deactivated state) is not addressed in the new commits.

Signed-off-by: Janet Cui <janet970527@gmail.com>

mssonicbld · 2026-04-23T10:18:29Z

/azp run

azure-pipelines · 2026-04-23T10:18:39Z

Azure Pipelines successfully started running 1 pipeline(s).

banidoru

Re-review (iteration 2) — new updateEntry commit reviewed.

updateEntry is delete+recreate: This works but is disruptive — it tears down the active mirror session (deactivates ports, removes SAI objects) and recreates from scratch, causing a traffic-mirroring gap. For mutable fields, consider updating in-place; for immutable fields, the delete+recreate approach is acceptable but should be documented.
Duplicate entry guard removed from createEntry: The old duplicate check (task_duplicated) was removed. Now createEntry no longer guards against duplicates — it relies on the caller (doTask) to check sessionExists() first. If any other caller invokes createEntry directly on an existing session, it will silently overwrite state. Consider keeping a defensive check or adding an assert.
updateEntry doesn't validate delete side-effects: If deleteEntry partially succeeds (e.g., deactivation fails mid-way but entry is removed from map), createEntry will proceed on potentially inconsistent SAI state.
All prior review concerns remain unaddressed — resource leaks, rollback gaps, STATE_DB persistence, LAG handling, naming, input validation, unit tests, etc. are still open.

banidoru

STATE_DB Persistence: and are still not persisted to STATE_DB, which is necessary for warm-restart scenarios.
Unit Tests: No new unit tests have been added to cover the new sampled mirroring logic, including success, failure, and rollback paths.
LAG Port Handling: The sampled mirroring implementation does not correctly handle LAG ports; it attempts to configure the LAG object itself rather than its members.
Resource Leaks: There are multiple paths where objects can be leaked upon partial failures during session activation or deactivation.
Error Handling: Several error paths have insufficient or incorrect error handling, such as silent fallthroughs and uncaught exceptions from string-to-integer conversions.

azure-pipelines · 2026-04-24T02:35:01Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-04-24T03:14:18Z

/azp run

azure-pipelines · 2026-04-24T03:14:27Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-04-24T03:20:39Z

/azp run

azure-pipelines · 2026-04-24T03:20:48Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-04-24T07:58:21Z

/azp run

azure-pipelines · 2026-04-24T07:58:32Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-04-24T09:07:56Z

/azp run

azure-pipelines · 2026-04-24T09:08:06Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-04-24T09:41:26Z

/azp run

azure-pipelines · 2026-04-24T09:41:37Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-04-24T09:41:40Z

/azp run

azure-pipelines · 2026-04-24T09:41:50Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Janet Cui <janet970527@gmail.com>

mssonicbld · 2026-04-24T10:11:04Z

/azp run

azure-pipelines · 2026-04-24T10:11:14Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-04-27T12:00:06Z

/azp run

azure-pipelines · 2026-04-27T12:00:30Z

Azure Pipelines successfully started running 1 pipeline(s).

Janetxxx · 2026-04-29T08:09:39Z

Closing this PR and merging contents into #4502

Copilot AI review requested due to automatic review settings April 16, 2026 07:22

Janetxxx requested a review from prsunny as a code owner April 16, 2026 07:22

Copilot started reviewing on behalf of Janetxxx April 16, 2026 07:23 View session

Janetxxx requested a review from r12f April 16, 2026 07:23

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Janetxxx removed request for prsunny and r12f April 16, 2026 10:43

Janetxxx force-pushed the dev/jc/sampled-port-mirror branch from 01e7dc0 to bc52765 Compare April 16, 2026 10:54

Janetxxx force-pushed the dev/jc/sampled-port-mirror branch from bc52765 to ccbb144 Compare April 16, 2026 11:23

Janetxxx force-pushed the dev/jc/sampled-port-mirror branch from ccbb144 to 987a4e4 Compare April 16, 2026 11:58

Janetxxx force-pushed the dev/jc/sampled-port-mirror branch from 987a4e4 to 8cfd52d Compare April 16, 2026 11:58

banidoru reviewed Apr 16, 2026

View reviewed changes

Comment thread orchagent/mirrororch.cpp

Comment thread orchagent/mirrororch.cpp

Comment thread orchagent/mirrororch.cpp Outdated

Comment thread orchagent/mirrororch.h

banidoru suggested changes Apr 16, 2026

View reviewed changes

Comment thread orchagent/mirrororch.cpp

Comment thread orchagent/mirrororch.cpp

Comment thread orchagent/mirrororch.cpp Outdated

Comment thread orchagent/mirrororch.cpp

banidoru reviewed Apr 16, 2026

View reviewed changes

Add updateEntry implementationAdd updateEntry implementation

7d1e2ab

Signed-off-by: Janet Cui <janet970527@gmail.com>

banidoru reviewed Apr 23, 2026

View reviewed changes

Comment thread orchagent/mirrororch.cpp

Comment thread orchagent/mirrororch.cpp Outdated

Comment thread orchagent/mirrororch.cpp

banidoru suggested changes Apr 23, 2026

View reviewed changes

Janetxxx force-pushed the dev/jc/sampled-port-mirror branch from c979e04 to aa86f38 Compare April 24, 2026 03:12

Janetxxx force-pushed the dev/jc/sampled-port-mirror branch from aa86f38 to 7478d5c Compare April 24, 2026 03:20

Janetxxx force-pushed the dev/jc/sampled-port-mirror branch from 7478d5c to d479e35 Compare April 24, 2026 07:58

Janetxxx force-pushed the dev/jc/sampled-port-mirror branch from d479e35 to 6603185 Compare April 24, 2026 09:07

Janetxxx force-pushed the dev/jc/sampled-port-mirror branch from 6603185 to 1f9eb70 Compare April 24, 2026 09:41

Janetxxx force-pushed the dev/jc/sampled-port-mirror branch from 1f9eb70 to 2935ddc Compare April 24, 2026 09:41

Add capability check with fallback for sampled mirroring and truncation

da47032

Signed-off-by: Janet Cui <janet970527@gmail.com>

Janetxxx force-pushed the dev/jc/sampled-port-mirror branch from 2935ddc to da47032 Compare April 24, 2026 10:10

Merge branch 'master' into dev/jc/sampled-port-mirror

079c5c4

Janetxxx mentioned this pull request Apr 28, 2026

[mirrororch][VS test]: Add VS tests for sampled ERSPAN port mirroring #4529

Closed

Janetxxx changed the title ~~[orchagent] Add sampled port mirroring support to MirrorOrch~~ [orchagent][MirrorOrch] Add sampled port mirroring support to MirrorOrch Apr 29, 2026

Janetxxx closed this Apr 29, 2026

Uh oh!

Conversation

Janetxxx commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Apr 16, 2026

Uh oh!

azure-pipelines Bot commented Apr 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Apr 16, 2026

Uh oh!

azure-pipelines Bot commented Apr 16, 2026

Uh oh!

mssonicbld commented Apr 16, 2026

Uh oh!

azure-pipelines Bot commented Apr 16, 2026

Uh oh!

mssonicbld commented Apr 16, 2026

Uh oh!

azure-pipelines Bot commented Apr 16, 2026

Uh oh!

mssonicbld commented Apr 16, 2026

Uh oh!

azure-pipelines Bot commented Apr 16, 2026

Uh oh!

banidoru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

banidoru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

banidoru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

banidoru Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

banidoru Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Apr 23, 2026

Uh oh!

azure-pipelines Bot commented Apr 23, 2026

Uh oh!

banidoru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

banidoru left a comment

Choose a reason for hiding this comment

Uh oh!

azure-pipelines Bot commented Apr 24, 2026

Uh oh!

mssonicbld commented Apr 24, 2026

Uh oh!

Janetxxx commented Apr 16, 2026 •

edited

Loading

Janetxxx commented Apr 29, 2026 •

edited

Loading