[mirrororch][VS test]: Add VS tests for sampled ERSPAN port mirroring by Janetxxx · Pull Request #4529 · sonic-net/sonic-swss

Janetxxx · 2026-04-28T09:05:41Z

What I did
Added virtual switch (VS) integration tests for sampled ERSPAN port mirroring with truncation support.

New test file: tests/test_mirror_sampled_erspan.py with 13 test cases
Extended tests/dvslib/dvs_mirror.py with create_erspan_session_sampled() helper

Test coverage:

Create/Delete: SAMPLEPACKET object lifecycle, port attribute binding and cleanup, truncation attributes
Update: in-place sample_rate update, path transition (full mirror ↔ sampled mirror)
Invalid input rejection: sample_rate with direction=TX or BOTH, truncate_size without sample_rate
Backward compatibility: no SAMPLEPACKET created when sample_rate absent
State and capability: STATE_DB field verification, switch capability entries, multiple source port binding

Why I did it
To validate the key software logic paths of the sampled mirroring pipeline from CONFIG_DB → MirrorOrch → ASIC_DB, including create/delete lifecycle, in-place update, path transitions, invalid input rejection, and backward compatibility.

How I verified it
Tests are validated against the virtual switch (vssyncd). Each test verifies the CONFIG_DB → MirrorOrch → ASIC_DB pipeline via ASIC_DB and STATE_DB assertions.

Details if related
Depends on: #4502.

Note: The switchorch/mirrororch changes in the diff are from #4502 and will disappear after it is merged and this PR is rebased.

mssonicbld · 2026-04-28T09:05:50Z

/azp run

azure-pipelines · 2026-04-28T09:06:01Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull request overview

Adds sampled ERSPAN port-mirroring coverage and wiring, including truncation support, to validate end-to-end behavior from CONFIG_DB through ASIC_DB and STATE_DB.

Changes:

Added a new VS integration test suite for sampled ERSPAN port mirroring (including truncation and backward compatibility).
Extended DVSLib mirror helpers with a create_erspan_session_sampled() convenience method.
Updated MirrorOrch to parse sample_rate / truncate_size, manage SAI SamplePacket objects, and introduce an update path for existing sessions.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
tests/test_mirror_sampled_erspan.py	New VS tests validating SamplePacket creation, port binding, truncation, and STATE_DB entries.
tests/mock_tests/mirrororch_ut.cpp	Adds unit tests targeting sampled/full mirror port programming paths and validation cases.
tests/dvslib/dvs_mirror.py	Adds a helper to create sampled ERSPAN mirror sessions via CONFIG_DB.
orchagent/mirrororch.h	Extends `MirrorEntry` with sampled-mirroring fields and updates MirrorOrch APIs/helpers.
orchagent/mirrororch.cpp	Implements CONFIG parsing/validation for sampled mirroring, SamplePacket lifecycle, port programming, and update handling.

Copilot · 2026-04-28T09:15:19Z

+            sai_attribute_t attr;
+            attr.id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_SIZE;
+            attr.value.u32 = new_truncate_size;
+            sai_status_t status = sai_samplepacket_api->set_samplepacket_attribute(
+                session.samplepacketId, &attr);
+            if (status != SAI_STATUS_SUCCESS)
+            {
+                // Rollback sample_rate if it was changed
+                if (sample_rate_changed)
+                {
+                    sai_attribute_t rollback_attr;
+                    rollback_attr.id = SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE;
+                    rollback_attr.value.u32 = session.sample_rate;
+                    sai_samplepacket_api->set_samplepacket_attribute(
+                        session.samplepacketId, &rollback_attr);
+                }
+                SWSS_LOG_ERROR("Failed to update truncate_size for session %s, status %d", key.c_str(), status);
+                return task_process_status::task_failed;
+            }


When updating truncation in-place, only SAI_SAMPLEPACKET_ATTR_TRUNCATE_SIZE is set. If the intent is to enable/disable truncation dynamically, this likely also needs to update SAI_SAMPLEPACKET_ATTR_TRUNCATE_ENABLE (e.g., set false when new_truncate_size == 0, set true before setting size when > 0) and re-check truncation capability; otherwise truncation may remain enabled with a stale size or the SAI call may be rejected.

Suggested change

sai_attribute_t attr;

attr.id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_SIZE;

attr.value.u32 = new_truncate_size;

sai_status_t status = sai_samplepacket_api->set_samplepacket_attribute(

session.samplepacketId, &attr);

if (status != SAI_STATUS_SUCCESS)

{

// Rollback sample_rate if it was changed

if (sample_rate_changed)

{

sai_attribute_t rollback_attr;

rollback_attr.id = SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE;

rollback_attr.value.u32 = session.sample_rate;

sai_samplepacket_api->set_samplepacket_attribute(

session.samplepacketId, &rollback_attr);

}

SWSS_LOG_ERROR("Failed to update truncate_size for session %s, status %d", key.c_str(), status);

return task_process_status::task_failed;

}

const bool current_truncate_enable = session.truncate_size != 0;

const bool new_truncate_enable = new_truncate_size != 0;

bool truncate_enable_updated = false;

bool truncate_size_updated = false;

sai_status_t status = SAI_STATUS_SUCCESS;

if (new_truncate_enable && !current_truncate_enable)

{

sai_attribute_t attr;

attr.id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_ENABLE;

attr.value.booldata = true;

status = sai_samplepacket_api->set_samplepacket_attribute(

session.samplepacketId, &attr);

if (status != SAI_STATUS_SUCCESS)

{

if (sample_rate_changed)

{

sai_attribute_t rollback_attr;

rollback_attr.id = SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE;

rollback_attr.value.u32 = session.sample_rate;

sai_samplepacket_api->set_samplepacket_attribute(

session.samplepacketId, &rollback_attr);

}

SWSS_LOG_ERROR("Failed to enable truncation for session %s, status %d", key.c_str(), status);

return task_process_status::task_failed;

}

truncate_enable_updated = true;

}

if (new_truncate_enable)

{

sai_attribute_t attr;

attr.id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_SIZE;

attr.value.u32 = new_truncate_size;

status = sai_samplepacket_api->set_samplepacket_attribute(

session.samplepacketId, &attr);

if (status != SAI_STATUS_SUCCESS)

{

if (truncate_enable_updated)

{

sai_attribute_t rollback_attr;

rollback_attr.id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_ENABLE;

rollback_attr.value.booldata = current_truncate_enable;

sai_samplepacket_api->set_samplepacket_attribute(

session.samplepacketId, &rollback_attr);

}

if (sample_rate_changed)

{

sai_attribute_t rollback_attr;

rollback_attr.id = SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE;

rollback_attr.value.u32 = session.sample_rate;

sai_samplepacket_api->set_samplepacket_attribute(

session.samplepacketId, &rollback_attr);

}

SWSS_LOG_ERROR("Failed to update truncate_size for session %s, status %d", key.c_str(), status);

return task_process_status::task_failed;

}

truncate_size_updated = true;

}

if (!new_truncate_enable && current_truncate_enable)

{

sai_attribute_t attr;

attr.id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_ENABLE;

attr.value.booldata = false;

status = sai_samplepacket_api->set_samplepacket_attribute(

session.samplepacketId, &attr);

if (status != SAI_STATUS_SUCCESS)

{

if (truncate_size_updated)

{

sai_attribute_t rollback_attr;

rollback_attr.id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_SIZE;

rollback_attr.value.u32 = session.truncate_size;

sai_samplepacket_api->set_samplepacket_attribute(

session.samplepacketId, &rollback_attr);

}

if (truncate_enable_updated)

{

sai_attribute_t rollback_attr;

rollback_attr.id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_ENABLE;

rollback_attr.value.booldata = current_truncate_enable;

sai_samplepacket_api->set_samplepacket_attribute(

session.samplepacketId, &rollback_attr);

}

if (sample_rate_changed)

{

sai_attribute_t rollback_attr;

rollback_attr.id = SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE;

rollback_attr.value.u32 = session.sample_rate;

sai_samplepacket_api->set_samplepacket_attribute(

session.samplepacketId, &rollback_attr);

}

SWSS_LOG_ERROR("Failed to disable truncation for session %s, status %d", key.c_str(), status);

return task_process_status::task_failed;

}

truncate_enable_updated = true;

}

Copilot · 2026-04-28T09:15:20Z

+                                    sai_object_id_t samplepacketId = SAI_NULL_OBJECT_ID,
+                                    uint32_t sample_rate = 0);


setUnsetPortMirror() now has defaulted samplepacketId/sample_rate parameters. This makes existing call sites that weren't updated (e.g., LAG member update paths) silently fall back to the full-mirror attribute programming even when the session is sampled, which can misprogram ports. Consider removing the defaults (forcing call sites to pass the session context) or auditing/updating every call site to pass session.samplepacketId and session.sample_rate when applicable.

Suggested change

sai_object_id_t samplepacketId = SAI_NULL_OBJECT_ID,

uint32_t sample_rate = 0);

sai_object_id_t samplepacketId,

uint32_t sample_rate);

Copilot · 2026-04-28T09:15:20Z

+        cap_entry = dvs.state_db.wait_for_entry("SWITCH_CAPABILITY", "switch")
+        assert "PORT_INGRESS_SAMPLE_MIRROR_CAPABLE" in cap_entry
+        assert "PORT_EGRESS_SAMPLE_MIRROR_CAPABLE" in cap_entry
+        assert "SAMPLEPACKET_TRUNCATION_CAPABLE" in cap_entry


Most existing VS pytest modules (including other mirror tests) end with a test_nonflaky_dummy() workaround to avoid Flaky invoking module teardown before retrying the final test (see e.g. tests/test_mirror_port_erspan.py:585-588). This new module doesn’t include that sentinel test, which may reintroduce the known flake behavior when the last test fails. Consider adding the dummy test at the end of this file as well.

Suggested change

assert "SAMPLEPACKET_TRUNCATION_CAPABLE" in cap_entry

assert "SAMPLEPACKET_TRUNCATION_CAPABLE" in cap_entry

def test_nonflaky_dummy(self):

pass

Copilot · 2026-04-28T09:15:21Z

+        // No samplepacket exists (full mirror path) but sample_rate/truncate_size changed
+        // Need to transition from full mirror to sampled path - delete+recreate


In the session.samplepacketId == SAI_NULL_OBJECT_ID branch, any change to truncate_size on a full-mirror session will trigger a delete+recreate even though truncate_size > 0 is invalid unless sample_rate > 0. This can unnecessarily delete a working session; consider rejecting truncate-only updates on full-mirror sessions (task_invalid_entry) and only doing delete+recreate when transitioning to sampled mode with a full, validated config payload.

Suggested change

// No samplepacket exists (full mirror path) but sample_rate/truncate_size changed

// Need to transition from full mirror to sampled path - delete+recreate

// No samplepacket exists (full mirror path). Only transition through

// delete+recreate when the requested configuration actually enables

// sampled mode. Reject truncate-only updates on a full-mirror session

// to avoid deleting a working session for an invalid configuration.

if (new_sample_rate == 0)

{

SWSS_LOG_ERROR("Invalid update for full mirror session %s: truncate_size requires sample_rate > 0", key.c_str());

return task_process_status::task_invalid_entry;

}

Copilot · 2026-04-28T09:15:21Z

+        if (session.truncate_size > 0)
+        {
+            fvVector.emplace_back(MIRROR_SESSION_TRUNCATE_SIZE, to_string(session.truncate_size));
+        }


STATE_DB updates for truncate_size are only written when the value is > 0. If truncation is later disabled (truncate_size updated to 0), this function will omit the field and Table::set() won't remove the old hash field, leaving stale truncate_size in STATE_DB. Consider explicitly writing truncate_size=0 (or deleting the field) when truncation is disabled so STATE_DB reflects the current config.

Suggested change

if (session.truncate_size > 0)

{

fvVector.emplace_back(MIRROR_SESSION_TRUNCATE_SIZE, to_string(session.truncate_size));

}

fvVector.emplace_back(MIRROR_SESSION_TRUNCATE_SIZE, to_string(session.truncate_size));

Copilot · 2026-04-28T09:15:21Z

+        // Sampled mirroring path: use SAMPLEPACKET_ENABLE + SAMPLE_MIRROR_SESSION
+        sai_attribute_t sp_attr;
+        sp_attr.id = SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE;
+        sp_attr.value.oid = set ? samplepacketId : SAI_NULL_OBJECT_ID;
+


The sampled-mirroring path configures only port.m_port_id directly. For LAG source ports, the full-mirror path programs each member port, but the sampled path doesn't handle port.m_type == Port::LAG, so sampled mirroring on LAGs (or dynamic LAG member updates) may not be programmed correctly. Consider mirroring the existing LAG-member handling from the full-mirror branch for the sampled attributes too.

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

- 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>

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

…+recreate for immutable - Update sample_rate and truncate_size in-place via SAI set_samplepacket_attribute - Fall back to delete+recreate when immutable fields change (src_ip, dst_ip, etc.) - Handle transition from full mirror to sampled path via delete+recreate Signed-off-by: Janet Cui <janet970527@gmail.com>

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

- Add test_mirror_sampled_erspan.py with 5 VS test cases - Test SAMPLEPACKET creation/deletion in ASIC_DB - Test truncation attributes on SAMPLEPACKET - Test backward compatibility (no SAMPLEPACKET without sample_rate) - Test STATE_DB session state and capability entries - Extend dvs_mirror.py with create_erspan_session_sampled helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Janet Cui <janet970527@gmail.com>

mssonicbld · 2026-05-05T02:15:49Z

/azp run

azure-pipelines · 2026-05-05T02:15:59Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-05-05T02:31:31Z

/azp run

azure-pipelines · 2026-05-05T02:31:43Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-05-05T02:58:30Z

/azp run

azure-pipelines · 2026-05-05T02:58:40Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-05-05T04:45:28Z

/azp run

azure-pipelines · 2026-05-05T04:45:39Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-05-05T05:03:54Z

/azp run

azure-pipelines · 2026-05-05T05:04:04Z

Azure Pipelines successfully started running 1 pipeline(s).

… sampled ERSPAN mirroring Signed-off-by: Janet Cui <janet970527@gmail.com>

mssonicbld · 2026-05-05T06:19:16Z

/azp run

azure-pipelines · 2026-05-05T06:19:27Z

Azure Pipelines successfully started running 1 pipeline(s).

Janetxxx · 2026-05-06T05:02:16Z

Closing this PR — VS tests have been merged into #4502 to consolidate code and tests in a single PR and meet diff
coverage requirements.

Copilot AI review requested due to automatic review settings April 28, 2026 09:05

Janetxxx requested a review from prsunny as a code owner April 28, 2026 09:05

github-advanced-security AI found potential problems Apr 28, 2026

View reviewed changes

Comment thread tests/test_mirror_sampled_erspan.py Fixed

Comment thread tests/test_mirror_sampled_erspan.py Fixed

Comment thread tests/test_mirror_sampled_erspan.py Fixed

Copilot AI reviewed Apr 28, 2026

View reviewed changes

Janetxxx and others added 8 commits April 29, 2026 07:44

add capability check for sampled port mirror

aac71c0

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

extend mock tests

44ced6b

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

Add updateEntry implementationAdd updateEntry implementation

9e48163

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

Write sample_rate and truncate_size to STATE_DB in setSessionState

cd8ec73

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

Add capability check with fallback for sampled mirroring and truncation

427887c

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

Janetxxx force-pushed the dev/jc/sampled-mirror-vs-test branch from b5d2f90 to 86310ad Compare May 5, 2026 02:09

github-advanced-security AI found potential problems May 5, 2026

View reviewed changes

Comment thread tests/test_mirror_sampled_erspan.py Fixed

Comment thread tests/test_mirror_sampled_erspan.py Fixed

Comment thread tests/test_mirror_sampled_erspan.py Fixed

Janetxxx force-pushed the dev/jc/sampled-mirror-vs-test branch from 6a2ba68 to 3143ffa Compare May 5, 2026 02:58

github-advanced-security AI found potential problems May 5, 2026

View reviewed changes

Comment thread tests/test_mirror_sampled_erspan.py Fixed

Janetxxx force-pushed the dev/jc/sampled-mirror-vs-test branch from 3143ffa to d1296fa Compare May 5, 2026 04:45

Janetxxx force-pushed the dev/jc/sampled-mirror-vs-test branch from d1296fa to 9d350a8 Compare May 5, 2026 05:03

[VS test] Add update, invalid entry, cleanup and multi-port tests for…

14e1bbb

… sampled ERSPAN mirroring Signed-off-by: Janet Cui <janet970527@gmail.com>

Janetxxx force-pushed the dev/jc/sampled-mirror-vs-test branch from 9d350a8 to 14e1bbb Compare May 5, 2026 05:55

Janetxxx closed this May 6, 2026

		sai_object_id_t samplepacketId = SAI_NULL_OBJECT_ID,
		uint32_t sample_rate = 0);

		// No samplepacket exists (full mirror path) but sample_rate/truncate_size changed
		// Need to transition from full mirror to sampled path - delete+recreate

-        // No samplepacket exists (full mirror path) but sample_rate/truncate_size changed
-        // Need to transition from full mirror to sampled path - delete+recreate
+        // No samplepacket exists (full mirror path). Only transition through
+        // delete+recreate when the requested configuration actually enables
+        // sampled mode. Reject truncate-only updates on a full-mirror session
+        // to avoid deleting a working session for an invalid configuration.
+        if (new_sample_rate == 0)
+        {
+            SWSS_LOG_ERROR("Invalid update for full mirror session %s: truncate_size requires sample_rate > 0", key.c_str());
+            return task_process_status::task_invalid_entry;
+        }

Uh oh!

Conversation

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

Uh oh!

Uh oh!

mssonicbld commented Apr 28, 2026

Uh oh!

azure-pipelines Bot commented Apr 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented May 5, 2026

Uh oh!

azure-pipelines Bot commented May 5, 2026

Uh oh!

mssonicbld commented May 5, 2026

Uh oh!

azure-pipelines Bot commented May 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented May 5, 2026

Uh oh!

azure-pipelines Bot commented May 5, 2026

Uh oh!

Uh oh!

mssonicbld commented May 5, 2026

Uh oh!

azure-pipelines Bot commented May 5, 2026

Uh oh!

mssonicbld commented May 5, 2026

Uh oh!

azure-pipelines Bot commented May 5, 2026

Uh oh!

mssonicbld commented May 5, 2026

Uh oh!

azure-pipelines Bot commented May 5, 2026

Uh oh!

Janetxxx commented May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Janetxxx commented Apr 28, 2026 •

edited

Loading