From 8cfd52d849fd406cecc3d189c99bfc3ae7618de0 Mon Sep 17 00:00:00 2001 From: Janet Cui Date: Thu, 16 Apr 2026 07:18:45 +0000 Subject: [PATCH 1/5] [orchagent] Add sampled port mirroring support to MirrorOrch - 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 --- orchagent/mirrororch.cpp | 197 ++++++++++++++++++++++++++++++++++++++- orchagent/mirrororch.h | 13 ++- 2 files changed, 206 insertions(+), 4 deletions(-) diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp index 35a58cb887b..920aa3d62a5 100644 --- a/orchagent/mirrororch.cpp +++ b/orchagent/mirrororch.cpp @@ -31,6 +31,8 @@ #define MIRROR_SESSION_DST_PORT "dst_port" #define MIRROR_SESSION_DIRECTION "direction" #define MIRROR_SESSION_TYPE "type" +#define MIRROR_SESSION_SAMPLE_RATE "sample_rate" +#define MIRROR_SESSION_TRUNCATE_SIZE "truncate_size" #define MIRROR_SESSION_DEFAULT_VLAN_PRI 0 #define MIRROR_SESSION_DEFAULT_VLAN_CFI 0 @@ -47,6 +49,7 @@ extern sai_switch_api_t *sai_switch_api; extern sai_mirror_api_t *sai_mirror_api; extern sai_port_api_t *sai_port_api; +extern sai_samplepacket_api_t *sai_samplepacket_api; extern sai_object_id_t gSwitchId; extern PortsOrch* gPortsOrch; @@ -59,6 +62,9 @@ MirrorEntry::MirrorEntry(const string& platform) : dscp(8), ttl(255), queue(0), + sample_rate(0), + truncate_size(0), + samplepacketId(SAI_NULL_OBJECT_ID), sessionId(0), refCount(0) { @@ -473,6 +479,14 @@ task_process_status MirrorOrch::createEntry(const string& key, const vector(fvValue(i)); + } + else if (fvField(i) == MIRROR_SESSION_TRUNCATE_SIZE) + { + entry.truncate_size = to_uint(fvValue(i)); + } else { SWSS_LOG_ERROR("Failed to parse session %s configuration. Unknown attribute %s", key.c_str(), fvField(i).c_str()); @@ -497,6 +511,22 @@ task_process_status MirrorOrch::createEntry(const string& key, const vector 0 && entry.sample_rate == 0) + { + SWSS_LOG_ERROR("Truncate size requires sampled mirroring to be enabled for session %s", + key.c_str()); + return task_process_status::task_invalid_entry; + } + + // Sampled mirroring requires RX direction + if (entry.sample_rate > 0 && entry.direction != MIRROR_RX_DIRECTION) + { + SWSS_LOG_ERROR("Sampled mirroring requires RX direction for session %s", + key.c_str()); + return task_process_status::task_invalid_entry; + } + if (!isHwResourcesAvailable()) { SWSS_LOG_ERROR("Failed to create session %s: HW resources are not available", key.c_str()); @@ -811,7 +841,9 @@ bool MirrorOrch::updateSession(const string& name, MirrorEntry& session) bool MirrorOrch::setUnsetPortMirror(Port port, bool ingress, bool set, - sai_object_id_t sessionId) + sai_object_id_t sessionId, + sai_object_id_t samplepacketId, + uint32_t sample_rate) { // Check if the mirror direction is supported by the ASIC if (ingress && !m_switchOrch->isPortIngressMirrorSupported()) @@ -826,6 +858,70 @@ bool MirrorOrch::setUnsetPortMirror(Port port, } sai_status_t status; + + if (sample_rate > 0 && ingress) + { + // 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; + + sai_attribute_t mirror_attr; + mirror_attr.id = SAI_PORT_ATTR_INGRESS_SAMPLE_MIRROR_SESSION; + if (set) + { + mirror_attr.value.objlist.count = 1; + mirror_attr.value.objlist.list = &sessionId; + } + else + { + mirror_attr.value.objlist.count = 0; + } + + // Set/clear in correct order + if (set) + { + // Set: SAMPLEPACKET_ENABLE first, then SAMPLE_MIRROR_SESSION + status = sai_port_api->set_port_attribute(port.m_port_id, &sp_attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set SAMPLEPACKET_ENABLE on port %s, status %d", + port.m_alias.c_str(), status); + return false; + } + status = sai_port_api->set_port_attribute(port.m_port_id, &mirror_attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set SAMPLE_MIRROR_SESSION on port %s, status %d", + port.m_alias.c_str(), status); + // Rollback: clear SAMPLEPACKET_ENABLE + sp_attr.value.oid = SAI_NULL_OBJECT_ID; + sai_port_api->set_port_attribute(port.m_port_id, &sp_attr); + return false; + } + } + else + { + // Clear: SAMPLE_MIRROR_SESSION first, then SAMPLEPACKET_ENABLE (reverse order) + status = sai_port_api->set_port_attribute(port.m_port_id, &mirror_attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to clear SAMPLE_MIRROR_SESSION on port %s, status %d", + port.m_alias.c_str(), status); + return false; + } + status = sai_port_api->set_port_attribute(port.m_port_id, &sp_attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to clear SAMPLEPACKET_ENABLE on port %s, status %d", + port.m_alias.c_str(), status); + return false; + } + } + return true; + } + + // Full mirror path (existing behavior) sai_attribute_t port_attr; port_attr.id = ingress ? SAI_PORT_ATTR_INGRESS_MIRROR_SESSION: SAI_PORT_ATTR_EGRESS_MIRROR_SESSION; @@ -896,7 +992,7 @@ bool MirrorOrch::configurePortMirrorSession(const string& name, MirrorEntry& ses } if (session.direction == MIRROR_RX_DIRECTION || session.direction == MIRROR_BOTH_DIRECTION) { - if (!setUnsetPortMirror(port, true, set, session.sessionId)) + if (!setUnsetPortMirror(port, true, set, session.sessionId, session.samplepacketId, session.sample_rate)) { SWSS_LOG_ERROR("Failed to configure mirror session %s port %s", name.c_str(), port.m_alias.c_str()); @@ -905,7 +1001,7 @@ bool MirrorOrch::configurePortMirrorSession(const string& name, MirrorEntry& ses } if (session.direction == MIRROR_TX_DIRECTION || session.direction == MIRROR_BOTH_DIRECTION) { - if (!setUnsetPortMirror(port, false, set, session.sessionId)) + if (!setUnsetPortMirror(port, false, set, session.sessionId, session.samplepacketId, session.sample_rate)) { SWSS_LOG_ERROR("Failed to configure mirror session %s port %s", name.c_str(), port.m_alias.c_str()); @@ -1079,6 +1175,18 @@ bool MirrorOrch::activateSession(const string& name, MirrorEntry& session) session.status = true; + // Create SamplePacket if sample_rate > 0 + if (session.sample_rate > 0) + { + if (!createSamplePacket(name, session)) + { + SWSS_LOG_ERROR("Failed to create samplepacket, removing mirror session %s", name.c_str()); + sai_mirror_api->remove_mirror_session(session.sessionId); + session.status = false; + return false; + } + } + if (!session.src_port.empty() && !session.direction.empty()) { status = configurePortMirrorSession(name, session, true); @@ -1120,6 +1228,16 @@ bool MirrorOrch::deactivateSession(const string& name, MirrorEntry& session) } } + // Remove SamplePacket if it exists + if (session.samplepacketId != SAI_NULL_OBJECT_ID) + { + if (!removeSamplePacket(name, session)) + { + SWSS_LOG_ERROR("Failed to remove samplepacket for session %s", name.c_str()); + return false; + } + } + status = sai_mirror_api->remove_mirror_session(session.sessionId); if (status != SAI_STATUS_SUCCESS) @@ -1178,6 +1296,79 @@ bool MirrorOrch::updateSessionDstMac(const string& name, MirrorEntry& session) return true; } + +bool MirrorOrch::createSamplePacket(const string& name, MirrorEntry& session) +{ + SWSS_LOG_ENTER(); + + sai_attribute_t attrs[4]; + uint32_t attr_count = 0; + + attrs[attr_count].id = SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE; + attrs[attr_count].value.u32 = session.sample_rate; + attr_count++; + + attrs[attr_count].id = SAI_SAMPLEPACKET_ATTR_TYPE; + attrs[attr_count].value.s32 = SAI_SAMPLEPACKET_TYPE_MIRROR_SESSION; + attr_count++; + + if (session.truncate_size > 0) + { + attrs[attr_count].id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_ENABLE; + attrs[attr_count].value.booldata = true; + attr_count++; + + attrs[attr_count].id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_SIZE; + attrs[attr_count].value.u32 = session.truncate_size; + attr_count++; + } + + sai_status_t status = sai_samplepacket_api->create_samplepacket( + &session.samplepacketId, gSwitchId, attr_count, attrs); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create samplepacket for session %s, status %d", + name.c_str(), status); + task_process_status handle_status = handleSaiCreateStatus(SAI_API_SAMPLEPACKET, status); + if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } + + SWSS_LOG_NOTICE("Created samplepacket for session %s, rate %u, truncate %u", + name.c_str(), session.sample_rate, session.truncate_size); + return true; +} + +bool MirrorOrch::removeSamplePacket(const string& name, MirrorEntry& session) +{ + SWSS_LOG_ENTER(); + + if (session.samplepacketId == SAI_NULL_OBJECT_ID) + { + return true; + } + + sai_status_t status = sai_samplepacket_api->remove_samplepacket(session.samplepacketId); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to remove samplepacket for session %s, status %d", + name.c_str(), status); + task_process_status handle_status = handleSaiRemoveStatus(SAI_API_SAMPLEPACKET, status); + if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } + + session.samplepacketId = SAI_NULL_OBJECT_ID; + SWSS_LOG_NOTICE("Removed samplepacket for session %s", name.c_str()); + return true; +} + bool MirrorOrch::updateSessionDstPort(const string& name, MirrorEntry& session) { SWSS_LOG_ENTER(); diff --git a/orchagent/mirrororch.h b/orchagent/mirrororch.h index 15bdc789629..d11a1b37ed0 100644 --- a/orchagent/mirrororch.h +++ b/orchagent/mirrororch.h @@ -57,6 +57,11 @@ struct MirrorEntry sai_object_id_t portId; } neighborInfo; + // Sampled mirroring fields + uint32_t sample_rate; // 0 = full mirror (default) + uint32_t truncate_size; // 0 = no truncation (default) + sai_object_id_t samplepacketId; // SAI_NULL_OBJECT_ID if not sampled + sai_object_id_t sessionId; int64_t refCount; @@ -136,9 +141,15 @@ class MirrorOrch : public Orch, public Observer, public Subject bool validateSrcPortList(const string& srcPort); bool validateDstPort(const string& dstPort); bool setUnsetPortMirror(Port port, bool ingress, bool set, - sai_object_id_t sessionId); + sai_object_id_t sessionId, + sai_object_id_t samplepacketId = SAI_NULL_OBJECT_ID, + uint32_t sample_rate = 0); bool configurePortMirrorSession(const string&, MirrorEntry&, bool enable); + // Sampled mirroring helpers + bool createSamplePacket(const string& name, MirrorEntry& session); + bool removeSamplePacket(const string& name, MirrorEntry& session); + void doTask(Consumer& consumer); }; From 7d1e2ab4cd6d7a09ef06161c2dc19b30d995c557 Mon Sep 17 00:00:00 2001 From: Janet Cui Date: Thu, 23 Apr 2026 10:18:04 +0000 Subject: [PATCH 2/5] Add updateEntry implementationAdd updateEntry implementation Signed-off-by: Janet Cui --- orchagent/mirrororch.cpp | 32 +++++++++++++++++++++++++------- orchagent/mirrororch.h | 1 + 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp index 920aa3d62a5..898c79c83c6 100644 --- a/orchagent/mirrororch.cpp +++ b/orchagent/mirrororch.cpp @@ -391,12 +391,7 @@ task_process_status MirrorOrch::createEntry(const string& key, const vector& data) +{ + SWSS_LOG_ENTER(); + + SWSS_LOG_NOTICE("Updating mirror session %s", key.c_str()); + + auto task_status = deleteEntry(key); + if (task_status != task_process_status::task_success) + { + SWSS_LOG_ERROR("Failed to delete existing mirror session %s during update", key.c_str()); + return task_status; + } + + return createEntry(key, data); +} + task_process_status MirrorOrch::deleteEntry(const string& name) { SWSS_LOG_ENTER(); @@ -1775,7 +1786,14 @@ void MirrorOrch::doTask(Consumer& consumer) if (op == SET_COMMAND) { - task_status = createEntry(key, kfvFieldsValues(t)); + if (sessionExists(key)) + { + task_status = updateEntry(key, kfvFieldsValues(t)); + } + else + { + task_status = createEntry(key, kfvFieldsValues(t)); + } } else if (op == DEL_COMMAND) { diff --git a/orchagent/mirrororch.h b/orchagent/mirrororch.h index d11a1b37ed0..2e62eec3a6d 100644 --- a/orchagent/mirrororch.h +++ b/orchagent/mirrororch.h @@ -113,6 +113,7 @@ class MirrorOrch : public Orch, public Observer, public Subject bool isHwResourcesAvailable(); task_process_status createEntry(const string&, const vector&); + task_process_status updateEntry(const string&, const vector&); task_process_status deleteEntry(const string&); bool activateSession(const string&, MirrorEntry&); From 60583eb53b05d62ebc916249fc1af2e28aa729ff Mon Sep 17 00:00:00 2001 From: Janet Cui Date: Thu, 23 Apr 2026 11:14:06 +0000 Subject: [PATCH 3/5] Implement hybrid session update: in-place for mutable fields, delete+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 --- orchagent/mirrororch.cpp | 119 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 114 insertions(+), 5 deletions(-) diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp index 898c79c83c6..0749b9597c4 100644 --- a/orchagent/mirrororch.cpp +++ b/orchagent/mirrororch.cpp @@ -553,14 +553,123 @@ task_process_status MirrorOrch::updateEntry(const string& key, const vectorsecond; + + // Determine if any immutable fields have changed + bool immutable_changed = false; + uint32_t new_sample_rate = session.sample_rate; + uint32_t new_truncate_size = session.truncate_size; + + for (auto& fv : data) + { + const auto& field = fvField(fv); + const auto& value = fvValue(fv); + + if (field == MIRROR_SESSION_SAMPLE_RATE) + { + new_sample_rate = to_uint(value); + } + else if (field == MIRROR_SESSION_TRUNCATE_SIZE) + { + new_truncate_size = to_uint(value); + } + else if (field == MIRROR_SESSION_SRC_IP || + field == MIRROR_SESSION_DST_IP || + field == MIRROR_SESSION_GRE_TYPE || + field == MIRROR_SESSION_DSCP || + field == MIRROR_SESSION_TTL || + field == MIRROR_SESSION_QUEUE || + field == MIRROR_SESSION_SRC_PORT || + field == MIRROR_SESSION_DIRECTION || + field == MIRROR_SESSION_POLICER) + { + immutable_changed = true; + } + } + + // If any immutable fields changed, must delete and recreate + if (immutable_changed) + { + SWSS_LOG_NOTICE("Immutable fields changed for session %s, performing delete+recreate", key.c_str()); + auto task_status = deleteEntry(key); + if (task_status != task_process_status::task_success) + { + SWSS_LOG_ERROR("Failed to delete mirror session %s during update", key.c_str()); + return task_status; + } + return createEntry(key, data); + } + + // Only mutable fields (sample_rate, truncate_size) changed - update in-place + bool sample_rate_changed = (new_sample_rate != session.sample_rate); + bool truncate_size_changed = (new_truncate_size != session.truncate_size); + + if (!sample_rate_changed && !truncate_size_changed) { - SWSS_LOG_ERROR("Failed to delete existing mirror session %s during update", key.c_str()); - return task_status; + SWSS_LOG_NOTICE("No field changes detected for session %s", key.c_str()); + return task_process_status::task_success; } - return createEntry(key, data); + // If samplepacket exists, update its attributes in-place + if (session.samplepacketId != SAI_NULL_OBJECT_ID) + { + if (sample_rate_changed) + { + sai_attribute_t attr; + attr.id = SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE; + attr.value.u32 = new_sample_rate; + sai_status_t status = sai_samplepacket_api->set_samplepacket_attribute( + session.samplepacketId, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to update sample_rate for session %s, status %d", key.c_str(), status); + return task_process_status::task_failed; + } + session.sample_rate = new_sample_rate; + SWSS_LOG_NOTICE("Updated sample_rate to %u for session %s", new_sample_rate, key.c_str()); + } + + if (truncate_size_changed) + { + 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) + { + SWSS_LOG_ERROR("Failed to update truncate_size for session %s, status %d", key.c_str(), status); + return task_process_status::task_failed; + } + session.truncate_size = new_truncate_size; + SWSS_LOG_NOTICE("Updated truncate_size to %u for session %s", new_truncate_size, key.c_str()); + } + } + else + { + // No samplepacket exists (full mirror path) but sample_rate/truncate_size changed + // Need to transition from full mirror to sampled path - delete+recreate + SWSS_LOG_NOTICE("Transitioning session %s from full mirror to sampled path, performing delete+recreate", key.c_str()); + auto task_status = deleteEntry(key); + if (task_status != task_process_status::task_success) + { + SWSS_LOG_ERROR("Failed to delete mirror session %s during path transition", key.c_str()); + return task_status; + } + return createEntry(key, data); + } + + // Update STATE_DB with new values + setSessionState(key, session); + + return task_process_status::task_success; } task_process_status MirrorOrch::deleteEntry(const string& name) From 8f85b07d2ad677f2a285947fbce241f10daed52d Mon Sep 17 00:00:00 2001 From: Janet Cui Date: Fri, 24 Apr 2026 01:14:31 +0000 Subject: [PATCH 4/5] Write sample_rate and truncate_size to STATE_DB in setSessionState Signed-off-by: Janet Cui --- orchagent/mirrororch.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp index b7ecab76c8f..cd45ea493e1 100644 --- a/orchagent/mirrororch.cpp +++ b/orchagent/mirrororch.cpp @@ -820,6 +820,22 @@ void MirrorOrch::setSessionState(const string& name, const MirrorEntry& session, fvVector.emplace_back(MIRROR_SESSION_NEXT_HOP_IP, value); } + if (attr.empty() || attr == MIRROR_SESSION_SAMPLE_RATE) + { + if (session.sample_rate > 0) + { + fvVector.emplace_back(MIRROR_SESSION_SAMPLE_RATE, to_string(session.sample_rate)); + } + } + + if (attr.empty() || attr == MIRROR_SESSION_TRUNCATE_SIZE) + { + if (session.truncate_size > 0) + { + fvVector.emplace_back(MIRROR_SESSION_TRUNCATE_SIZE, to_string(session.truncate_size)); + } + } + m_mirrorTable.set(name, fvVector); } From da470322116366ec5be2745e9d7597834b484a58 Mon Sep 17 00:00:00 2001 From: Janet Cui Date: Fri, 24 Apr 2026 02:34:37 +0000 Subject: [PATCH 5/5] Add capability check with fallback for sampled mirroring and truncation Signed-off-by: Janet Cui --- orchagent/mirrororch.cpp | 76 ++++++++++++++---- tests/mock_tests/mirrororch_ut.cpp | 119 +++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 13 deletions(-) diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp index cd45ea493e1..48be3b308c1 100644 --- a/orchagent/mirrororch.cpp +++ b/orchagent/mirrororch.cpp @@ -388,6 +388,13 @@ task_process_status MirrorOrch::createEntry(const string& key, const vector(fvValue(i)); + try { + entry.sample_rate = to_uint(fvValue(i)); + } catch (const exception& e) { + SWSS_LOG_ERROR("Invalid sample_rate for session %s: %s", key.c_str(), e.what()); + return task_process_status::task_invalid_entry; + } } else if (fvField(i) == MIRROR_SESSION_TRUNCATE_SIZE) { - entry.truncate_size = to_uint(fvValue(i)); + try { + entry.truncate_size = to_uint(fvValue(i)); + } catch (const exception& e) { + SWSS_LOG_ERROR("Invalid truncate_size for session %s: %s", key.c_str(), e.what()); + return task_process_status::task_invalid_entry; + } } else { @@ -1031,8 +1048,15 @@ bool MirrorOrch::setUnsetPortMirror(Port port, sai_status_t status; - if (sample_rate > 0 && ingress) + if (sample_rate > 0) { + if (!ingress) + { + SWSS_LOG_ERROR("Sampled mirroring on egress is not supported for port %s", + port.m_alias.c_str()); + return false; + } + // Sampled mirroring path: use SAMPLEPACKET_ENABLE + SAMPLE_MIRROR_SESSION sai_attribute_t sp_attr; sp_attr.id = SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE; @@ -1350,7 +1374,14 @@ bool MirrorOrch::activateSession(const string& name, MirrorEntry& session) // Create SamplePacket if sample_rate > 0 if (session.sample_rate > 0) { - if (!createSamplePacket(name, session)) + if (!m_switchOrch->isPortIngressSampleMirrorSupported()) + { + SWSS_LOG_WARN("Sampled mirroring not supported on this platform, " + "falling back to full mirror for session %s", name.c_str()); + session.sample_rate = 0; + session.truncate_size = 0; + } + else if (!createSamplePacket(name, session)) { SWSS_LOG_ERROR("Failed to create samplepacket, removing mirror session %s", name.c_str()); sai_mirror_api->remove_mirror_session(session.sessionId); @@ -1365,6 +1396,12 @@ bool MirrorOrch::activateSession(const string& name, MirrorEntry& session) if (status == false) { SWSS_LOG_ERROR("Failed to activate port mirror session %s", name.c_str()); + // Clean up samplepacket if it was created + if (session.samplepacketId != SAI_NULL_OBJECT_ID) + { + removeSamplePacket(name, session); + } + sai_mirror_api->remove_mirror_session(session.sessionId); session.status = false; return false; } @@ -1486,13 +1523,22 @@ bool MirrorOrch::createSamplePacket(const string& name, MirrorEntry& session) if (session.truncate_size > 0) { - attrs[attr_count].id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_ENABLE; - attrs[attr_count].value.booldata = true; - attr_count++; + if (!m_switchOrch->isSamplepacketTruncationSupported()) + { + SWSS_LOG_WARN("Truncation not supported on this platform, " + "skipping truncation for session %s", name.c_str()); + session.truncate_size = 0; + } + else + { + attrs[attr_count].id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_ENABLE; + attrs[attr_count].value.booldata = true; + attr_count++; - attrs[attr_count].id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_SIZE; - attrs[attr_count].value.u32 = session.truncate_size; - attr_count++; + attrs[attr_count].id = SAI_SAMPLEPACKET_ATTR_TRUNCATE_SIZE; + attrs[attr_count].value.u32 = session.truncate_size; + attr_count++; + } } sai_status_t status = sai_samplepacket_api->create_samplepacket( @@ -1502,16 +1548,20 @@ bool MirrorOrch::createSamplePacket(const string& name, MirrorEntry& session) { SWSS_LOG_ERROR("Failed to create samplepacket for session %s, status %d", name.c_str(), status); + session.samplepacketId = SAI_NULL_OBJECT_ID; task_process_status handle_status = handleSaiCreateStatus(SAI_API_SAMPLEPACKET, status); if (handle_status != task_success) { return parseHandleSaiStatusFailure(handle_status); } } + else + { + SWSS_LOG_NOTICE("Created samplepacket for session %s, rate %u, truncate %u", + name.c_str(), session.sample_rate, session.truncate_size); + } - SWSS_LOG_NOTICE("Created samplepacket for session %s, rate %u, truncate %u", - name.c_str(), session.sample_rate, session.truncate_size); - return true; + return (status == SAI_STATUS_SUCCESS); } bool MirrorOrch::removeSamplePacket(const string& name, MirrorEntry& session) diff --git a/tests/mock_tests/mirrororch_ut.cpp b/tests/mock_tests/mirrororch_ut.cpp index 7e3f9c94a40..6a0cfcf85cc 100644 --- a/tests/mock_tests/mirrororch_ut.cpp +++ b/tests/mock_tests/mirrororch_ut.cpp @@ -53,5 +53,124 @@ namespace mirrororch_test auto ret = gMirrorOrch->setUnsetPortMirror(dummyPort, /*ingress*/ false, /*set*/ true, /*sessionId*/ SAI_NULL_OBJECT_ID); ASSERT_FALSE(ret); } + + TEST_F(MirrorOrchTest, SampledPathUsesCorrectPortAttributes) + { + // Verify setUnsetPortMirror enters sampled path when sample_rate > 0 + ASSERT_NE(gSwitchOrch, nullptr); + ASSERT_NE(gMirrorOrch, nullptr); + + gSwitchOrch->m_portIngressMirrorSupported = true; + gSwitchOrch->m_portIngressSampleMirrorSupported = true; + + Port dummyPort; + dummyPort.m_port_id = SAI_NULL_OBJECT_ID; + + // Sampled path: sample_rate > 0 with samplepacketId + // SAI mock may fail but function should not crash + gMirrorOrch->setUnsetPortMirror(dummyPort, /*ingress*/ true, /*set*/ true, + /*sessionId*/ SAI_NULL_OBJECT_ID, + /*samplepacketId*/ (sai_object_id_t)0x1234, + /*sample_rate*/ 50000); + } + + TEST_F(MirrorOrchTest, FullMirrorPathWhenSampleRateZero) + { + // Verify setUnsetPortMirror uses full mirror path when sample_rate == 0 + ASSERT_NE(gSwitchOrch, nullptr); + ASSERT_NE(gMirrorOrch, nullptr); + + gSwitchOrch->m_portIngressMirrorSupported = true; + + Port dummyPort; + dummyPort.m_port_id = SAI_NULL_OBJECT_ID; + + // Full mirror path: sample_rate == 0 + gMirrorOrch->setUnsetPortMirror(dummyPort, /*ingress*/ true, /*set*/ true, + /*sessionId*/ SAI_NULL_OBJECT_ID, + /*samplepacketId*/ SAI_NULL_OBJECT_ID, + /*sample_rate*/ 0); + } + + TEST_F(MirrorOrchTest, FallbackToFullMirrorWhenSampledUnsupported) + { + // Verify activateSession falls back to full mirror when sampled not supported + ASSERT_NE(gSwitchOrch, nullptr); + ASSERT_NE(gMirrorOrch, nullptr); + + gSwitchOrch->m_portIngressMirrorSupported = true; + gSwitchOrch->m_portIngressSampleMirrorSupported = false; + + MirrorEntry entry(""); + entry.sample_rate = 50000; + entry.truncate_size = 128; + + // Simulate: activateSession should detect unsupported and clear sample_rate + // We test the capability check logic directly + if (!gSwitchOrch->isPortIngressSampleMirrorSupported()) + { + entry.sample_rate = 0; + entry.truncate_size = 0; + } + ASSERT_EQ(entry.sample_rate, (uint32_t)0); + ASSERT_EQ(entry.truncate_size, (uint32_t)0); + } + + TEST_F(MirrorOrchTest, SkipTruncationWhenUnsupported) + { + // Verify createSamplePacket skips truncation when not supported + ASSERT_NE(gSwitchOrch, nullptr); + ASSERT_NE(gMirrorOrch, nullptr); + + gSwitchOrch->m_portIngressSampleMirrorSupported = true; + gSwitchOrch->m_samplepacketTruncationSupported = false; + + MirrorEntry entry(""); + entry.sample_rate = 50000; + entry.truncate_size = 128; + + // createSamplePacket should detect truncation unsupported and clear truncate_size + gMirrorOrch->createSamplePacket("test_session", entry); + ASSERT_EQ(entry.truncate_size, (uint32_t)0); + } + + TEST_F(MirrorOrchTest, RemoveSamplePacketHandlesNullOid) + { + // Verify removeSamplePacket handles NULL samplepacketId gracefully + ASSERT_NE(gMirrorOrch, nullptr); + + MirrorEntry entry(""); + entry.samplepacketId = SAI_NULL_OBJECT_ID; + + bool ret = gMirrorOrch->removeSamplePacket("test_session", entry); + ASSERT_TRUE(ret); + ASSERT_EQ(entry.samplepacketId, SAI_NULL_OBJECT_ID); + } + + TEST_F(MirrorOrchTest, MirrorEntryDefaultValues) + { + // Verify MirrorEntry initializes new fields correctly + MirrorEntry entry(""); + ASSERT_EQ(entry.sample_rate, (uint32_t)0); + ASSERT_EQ(entry.truncate_size, (uint32_t)0); + ASSERT_EQ(entry.samplepacketId, SAI_NULL_OBJECT_ID); + } + + TEST_F(MirrorOrchTest, ValidationRejectsTruncateWithoutSampleRate) + { + // Verify createEntry rejects truncate_size > 0 when sample_rate == 0 + ASSERT_NE(gMirrorOrch, nullptr); + + vector data; + data.emplace_back("type", "ERSPAN"); + data.emplace_back("src_ip", "10.0.0.1"); + data.emplace_back("dst_ip", "10.0.0.2"); + data.emplace_back("dscp", "8"); + data.emplace_back("ttl", "64"); + data.emplace_back("truncate_size", "128"); + + auto status = gMirrorOrch->createEntry("invalid_session", data); + ASSERT_EQ(status, task_process_status::task_invalid_entry); + } }