From 61a029fb8529dec97064bf703a7e460a63795951 Mon Sep 17 00:00:00 2001 From: Aaron Bernardino Date: Thu, 30 Jul 2026 20:05:06 +0000 Subject: [PATCH 1/2] [vslib/vpp] Wire packet-trimming SAI attributes to the VPP trim dataplane Translate the SAI switch packet-trimming attributes onto the sonic_ext VPP trim plugin and source trim counters: - SwitchVpp.{cpp,h}: setSwitchTrimAttr / isTrimDataplaneAttr push global trim policy (trim size, DSCP mode/value, trim queue) into VPP, plus capability and stats handling. - SaiVppXlate.{c,h}: binary API wrappers for the trim configuration calls. - unittest/vslib/TestSwitchVpp.cpp: unit coverage for the trim attribute path. Pairs with the sonic-platform-vpp sonic_ext trim dataplane. Tracking: sonic-net/sonic-buildimage#25789 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino --- unittest/vslib/TestSwitchVpp.cpp | 39 +++ vslib/vpp/SwitchVpp.cpp | 528 +++++++++++++++++++++++++++++++ vslib/vpp/SwitchVpp.h | 89 ++++++ vslib/vpp/vppxlate/SaiVppXlate.c | 212 ++++++++++++- vslib/vpp/vppxlate/SaiVppXlate.h | 21 ++ 5 files changed, 888 insertions(+), 1 deletion(-) diff --git a/unittest/vslib/TestSwitchVpp.cpp b/unittest/vslib/TestSwitchVpp.cpp index 2d25d00c25..c81dfac2b3 100644 --- a/unittest/vslib/TestSwitchVpp.cpp +++ b/unittest/vslib/TestSwitchVpp.cpp @@ -15,3 +15,42 @@ TEST(SwitchVpp, getLagMemberEgressDisableAction) EXPECT_EQ(Action::NONE, SwitchVpp::getLagMemberEgressDisableAction(false, false, false)); EXPECT_EQ(Action::DISABLE, SwitchVpp::getLagMemberEgressDisableAction(true, false, false)); } + +TEST(SwitchVpp, isPacketTrimSwitchAttr) +{ + // All six switch-level packet-trim attributes must be classified as trim + // attrs so queryAttributeCapability() reports them not implemented and + // sonic-swss publishes SWITCH_TRIMMING_CAPABLE=false. + EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_SIZE)); + EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE)); + EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_VALUE)); + EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_TC_VALUE)); + EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE)); + EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_INDEX)); + + // Unrelated switch attributes must not be gated off. + EXPECT_FALSE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_INIT_SWITCH)); + EXPECT_FALSE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_ECMP_HASH)); +} + +TEST(SwitchVpp, isUnsupportedTrimEnumCapability) +{ + // The trim-related enum capabilities the base virtual switch advertises + // must be reported unsupported on VPP. + EXPECT_TRUE(SwitchVpp::isUnsupportedTrimEnumCapability( + SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE)); + EXPECT_TRUE(SwitchVpp::isUnsupportedTrimEnumCapability( + SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE)); + EXPECT_TRUE(SwitchVpp::isUnsupportedTrimEnumCapability( + SAI_OBJECT_TYPE_BUFFER_PROFILE, SAI_BUFFER_PROFILE_ATTR_PACKET_ADMISSION_FAIL_ACTION)); + + // Non-trim enum attributes must still be delegated to the base class. + EXPECT_FALSE(SwitchVpp::isUnsupportedTrimEnumCapability( + SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_ECMP_HASH)); + EXPECT_FALSE(SwitchVpp::isUnsupportedTrimEnumCapability( + SAI_OBJECT_TYPE_TAM, SAI_TAM_ATTR_TAM_BIND_POINT_TYPE_LIST)); + + // A trim switch attr queried against the wrong object type must not match. + EXPECT_FALSE(SwitchVpp::isUnsupportedTrimEnumCapability( + SAI_OBJECT_TYPE_BUFFER_PROFILE, SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE)); +} diff --git a/vslib/vpp/SwitchVpp.cpp b/vslib/vpp/SwitchVpp.cpp index 890c6e8b80..67894f21ef 100644 --- a/vslib/vpp/SwitchVpp.cpp +++ b/vslib/vpp/SwitchVpp.cpp @@ -15,6 +15,8 @@ #include #include #include +#include +#include using namespace saivs; @@ -1101,6 +1103,479 @@ sai_status_t SwitchVpp::getRouteStatsExt( return SAI_STATUS_SUCCESS; } +bool SwitchVpp::isTrimDataplaneAttr( + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id) +{ + // A change to any of these re-resolves the per-queue trim admission state + // pushed to the VPP plugin: the queue's buffer profile / scheduler binding, + // the buffer profile admission-fail action (eligibility), the scheduler + // group -> scheduler binding (SONiC applies per-queue schedulers to the + // queue's leaf scheduler group, not to the queue object), or the scheduler + // PIR (blocking vs unlimited egress). + switch (object_type) + { + case SAI_OBJECT_TYPE_QUEUE: + return attr_id == SAI_QUEUE_ATTR_BUFFER_PROFILE_ID || + attr_id == SAI_QUEUE_ATTR_SCHEDULER_PROFILE_ID; + + case SAI_OBJECT_TYPE_BUFFER_PROFILE: + return attr_id == SAI_BUFFER_PROFILE_ATTR_PACKET_ADMISSION_FAIL_ACTION; + + case SAI_OBJECT_TYPE_SCHEDULER_GROUP: + return attr_id == SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID; + + case SAI_OBJECT_TYPE_SCHEDULER: + return attr_id == SAI_SCHEDULER_ATTR_MAX_BANDWIDTH_RATE; + + default: + return false; + } +} + +sai_status_t SwitchVpp::setSwitchTrimAttr( + _In_ const sai_attribute_t *attr) +{ + SWSS_LOG_ENTER(); + + // Accumulate the switch-global trim policy. Orchagent programs these + // attributes individually; each change re-sends the full policy to VPP + // (sonic_ext_trim_global_set is an idempotent full-state set). Trimming is + // enabled/disabled by the trim size being non-zero/zero. + switch (attr->id) + { + case SAI_SWITCH_ATTR_PACKET_TRIM_SIZE: + m_trim_policy.trim_size = static_cast(attr->value.u32); + m_trim_policy.enabled = (attr->value.u32 != 0); + break; + + case SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE: + m_trim_policy.dscp_mode = + (attr->value.s32 == SAI_PACKET_TRIM_DSCP_RESOLUTION_MODE_FROM_TC) ? 1 : 0; + break; + + case SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_VALUE: + m_trim_policy.dscp_value = attr->value.u8; + break; + + case SAI_SWITCH_ATTR_PACKET_TRIM_TC_VALUE: + m_trim_policy.tc_value = attr->value.u8; + break; + + case SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE: + // Only STATIC is advertised (queryAttrEnumValuesCapability), so + // orchagent never programs DYNAMIC; nothing to store here. + break; + + case SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_INDEX: + m_trim_policy.trim_queue = attr->value.u8; + break; + + default: + return SAI_STATUS_SUCCESS; + } + + programTrimGlobal(); + + return SAI_STATUS_SUCCESS; +} + +void SwitchVpp::programTrimGlobal() +{ + SWSS_LOG_ENTER(); + + int ret = vpp_sonic_ext_trim_global_set( + m_trim_policy.enabled, + m_trim_policy.trim_size, + m_trim_policy.dscp_mode, + m_trim_policy.dscp_value, + m_trim_policy.tc_value, + m_trim_policy.trim_queue); + + if (ret != 0) + { + SWSS_LOG_ERROR("sonic_ext_trim_global_set failed (ret=%d): enabled=%d size=%u dscp_mode=%u dscp=%u tc=%u queue=%u", + ret, m_trim_policy.enabled, m_trim_policy.trim_size, + m_trim_policy.dscp_mode, m_trim_policy.dscp_value, + m_trim_policy.tc_value, m_trim_policy.trim_queue); + } + + // The global policy just changed. Queues / buffer profiles / schedulers may + // already be programmed (orchagent enables trimming before, and reconfigures + // it after, the per-queue objects), so re-resolve the per-queue admission + // state now that trimming is active. + if (m_trim_policy.enabled) + { + refreshTrimDataplane(); + } +} + +void SwitchVpp::refreshTrimDataplaneOnChange( + _In_ sai_object_type_t object_type, + _In_ uint32_t attr_count, + _In_ const sai_attribute_t *attr_list) +{ + // Only meaningful once trimming is globally enabled; before that the VPP + // plugin bypasses all traffic regardless of the per-queue state. + if (!m_trim_policy.enabled) + { + return; + } + + for (uint32_t i = 0; i < attr_count; i++) + { + if (isTrimDataplaneAttr(object_type, attr_list[i].id)) + { + refreshTrimDataplane(); + return; + } + } +} + +std::vector SwitchVpp::getTrimPortList() const +{ + SWSS_LOG_ENTER(); + + // Front-panel ports are created dynamically by orchagent via createPort(), + // which records them in m_objectHash but never appends to m_port_list + // (that cache is only populated by the unused lanemap create_ports() path + // or a lazy SAI_SWITCH_ATTR_PORT_LIST refresh). Enumerate the authoritative + // ASIC object store so trim programming sees every port. Mirrors the port + // enumeration in SwitchStateBase::refresh_port_list(). + std::vector ports; + + auto port_hash = m_objectHash.find(SAI_OBJECT_TYPE_PORT); + + if (port_hash != m_objectHash.end()) + { + ports.reserve(port_hash->second.size()); + + for (const auto &kvp : port_hash->second) + { + sai_object_id_t port_oid; + sai_deserialize_object_id(kvp.first, port_oid); + ports.push_back(port_oid); + } + } + + return ports; +} + +void SwitchVpp::refreshTrimDataplane() +{ + SWSS_LOG_ENTER(); + + std::vector port_list = getTrimPortList(); + + SWSS_LOG_NOTICE("refreshTrimDataplane: reprogramming trim admission (enabled=%d ports=%zu)", + (int)m_trim_policy.enabled, port_list.size()); + + // SONiC's QosOrch applies a queue's scheduler (SCHEDULER_BLOCK_DATA_PLANE + // included) to the queue's parent leaf scheduler group via + // SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID; the queue object's own + // SAI_QUEUE_ATTR_SCHEDULER_PROFILE_ID stays unset on sonic-vpp. Build a + // queue OID -> parent scheduler group reverse map once here so per-queue + // blocking detection can locate the bound scheduler. + std::unordered_map queue_to_sg; + buildQueueSchedulerGroupMap(queue_to_sg); + + for (auto port_oid : port_list) + { + std::string hwif; + + // CPU, fabric and other internal ports have no VPP hwif; skip them. + if (!port_to_hwifname(port_oid, hwif)) + { + SWSS_LOG_DEBUG("refreshTrimDataplane: port 0x%llx has no hwif, skipping", + (unsigned long long)port_oid); + continue; + } + + sai_attribute_t attr; + std::vector queues(MAX_OBJLIST_LEN); + + attr.id = SAI_PORT_ATTR_QOS_QUEUE_LIST; + attr.value.objlist.count = MAX_OBJLIST_LEN; + attr.value.objlist.list = queues.data(); + + if (get(SAI_OBJECT_TYPE_PORT, port_oid, 1, &attr) != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("refreshTrimDataplane: port 0x%llx (%s) queue list get failed", + (unsigned long long)port_oid, hwif.c_str()); + continue; + } + + uint32_t n = attr.value.objlist.count; + + SWSS_LOG_DEBUG("refreshTrimDataplane: port 0x%llx (%s) queues=%u", + (unsigned long long)port_oid, hwif.c_str(), n); + + for (uint32_t i = 0; i < n; i++) + { + refreshTrimQueue(hwif, queues[i], queue_to_sg); + } + } + + refreshTrimDscpToQueueMap(); +} + +void SwitchVpp::buildQueueSchedulerGroupMap( + _Out_ std::unordered_map &queueToSg) +{ + queueToSg.clear(); + + auto sg_hash = m_objectHash.find(SAI_OBJECT_TYPE_SCHEDULER_GROUP); + + if (sg_hash == m_objectHash.end()) + { + return; + } + + for (const auto &kvp : sg_hash->second) + { + sai_object_id_t sg_oid; + sai_deserialize_object_id(kvp.first, sg_oid); + + sai_attribute_t attr; + + attr.id = SAI_SCHEDULER_GROUP_ATTR_CHILD_COUNT; + if (get(SAI_OBJECT_TYPE_SCHEDULER_GROUP, sg_oid, 1, &attr) != SAI_STATUS_SUCCESS) + { + continue; + } + + uint32_t count = attr.value.u32; + + if (count == 0 || count > MAX_OBJLIST_LEN) + { + continue; + } + + std::vector children(count); + + attr.id = SAI_SCHEDULER_GROUP_ATTR_CHILD_LIST; + attr.value.objlist.count = count; + attr.value.objlist.list = children.data(); + + if (get(SAI_OBJECT_TYPE_SCHEDULER_GROUP, sg_oid, 1, &attr) != SAI_STATUS_SUCCESS) + { + continue; + } + + // A leaf scheduler group's child list holds queue OIDs; intermediate + // groups hold child groups. Recording every child is harmless because + // only queue OIDs are ever looked up during per-queue blocking + // detection, and each queue appears in exactly one group's child list. + for (uint32_t i = 0; i < attr.value.objlist.count; i++) + { + queueToSg[children[i]] = sg_oid; + } + } +} + +void SwitchVpp::refreshTrimQueue( + _In_ const std::string &hwif_name, + _In_ sai_object_id_t queue_oid, + _In_ const std::unordered_map &queueToSg) +{ + sai_attribute_t attr; + + // Only unicast queues are trim targets. Skip multicast queues so their + // (colliding) index never clobbers the unicast queue admission state. + attr.id = SAI_QUEUE_ATTR_TYPE; + if (get(SAI_OBJECT_TYPE_QUEUE, queue_oid, 1, &attr) == SAI_STATUS_SUCCESS) + { + sai_queue_type_t qtype = (sai_queue_type_t) attr.value.s32; + + if (qtype != SAI_QUEUE_TYPE_UNICAST && qtype != SAI_QUEUE_TYPE_ALL) + { + return; + } + } + + attr.id = SAI_QUEUE_ATTR_INDEX; + if (get(SAI_OBJECT_TYPE_QUEUE, queue_oid, 1, &attr) != SAI_STATUS_SUCCESS) + { + return; + } + uint8_t queue_index = attr.value.u8; + + // The VPP sonic_ext trim datapath only tracks admission for queues + // 0..SONIC_EXT_TRIM_MAX_QUEUES-1 (8; see the plugin's sonic_ext.h). SONiC + // models 10 unicast queues per port (0-9), so indices >= 8 have no VPP hwif + // queue and the plugin would reject them with VNET_API_ERROR_INVALID_VALUE. + // Skip them: the trim queue and every eligible (lossy) queue are < 8. + static const uint8_t VPP_TRIM_MAX_QUEUES = 8; + if (queue_index >= VPP_TRIM_MAX_QUEUES) + { + return; + } + + // Eligibility: the queue's bound buffer profile uses DROP_AND_TRIM. + bool eligible = false; + + attr.id = SAI_QUEUE_ATTR_BUFFER_PROFILE_ID; + if (get(SAI_OBJECT_TYPE_QUEUE, queue_oid, 1, &attr) == SAI_STATUS_SUCCESS && + attr.value.oid != SAI_NULL_OBJECT_ID) + { + sai_object_id_t bp_oid = attr.value.oid; + + attr.id = SAI_BUFFER_PROFILE_ATTR_PACKET_ADMISSION_FAIL_ACTION; + if (get(SAI_OBJECT_TYPE_BUFFER_PROFILE, bp_oid, 1, &attr) == SAI_STATUS_SUCCESS) + { + eligible = (attr.value.s32 == + SAI_BUFFER_PROFILE_PACKET_ADMISSION_FAIL_ACTION_DROP_AND_TRIM); + } + } + + // Blocking: the queue's scheduler imposes a PIR (MAX_BANDWIDTH_RATE > 0). + // SONiC's SCHEDULER_BLOCK_DATA_PLANE holds the queue closed with PIR=1; no + // PIR means unlimited egress and the queue must never trim. + // + // SONiC's QosOrch does not bind per-queue schedulers to the queue object; + // it applies them to the queue's parent (leaf) scheduler group. Resolve the + // effective scheduler by preferring a directly-bound queue scheduler (real + // hardware SAI) and falling back to the parent scheduler group binding + // (sonic-vpp), which is where SCHEDULER_BLOCK_DATA_PLANE actually lands. + bool blocking = false; + sai_object_id_t sched_oid = SAI_NULL_OBJECT_ID; + + attr.id = SAI_QUEUE_ATTR_SCHEDULER_PROFILE_ID; + if (get(SAI_OBJECT_TYPE_QUEUE, queue_oid, 1, &attr) == SAI_STATUS_SUCCESS && + attr.value.oid != SAI_NULL_OBJECT_ID) + { + sched_oid = attr.value.oid; + } + else + { + auto it = queueToSg.find(queue_oid); + + if (it != queueToSg.end()) + { + attr.id = SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID; + if (get(SAI_OBJECT_TYPE_SCHEDULER_GROUP, it->second, 1, &attr) == SAI_STATUS_SUCCESS && + attr.value.oid != SAI_NULL_OBJECT_ID) + { + sched_oid = attr.value.oid; + } + } + } + + if (sched_oid != SAI_NULL_OBJECT_ID) + { + attr.id = SAI_SCHEDULER_ATTR_MAX_BANDWIDTH_RATE; + if (get(SAI_OBJECT_TYPE_SCHEDULER, sched_oid, 1, &attr) == SAI_STATUS_SUCCESS) + { + blocking = (attr.value.u64 != 0); + } + } + + uint64_t rate; + uint64_t capacity; + + if (blocking) + { + // No admission headroom: the token bucket starts empty and never + // refills, so every packet on this eligible queue fails admission and + // is trimmed. + rate = 0; + capacity = 0; + } + else + { + // Unlimited egress: keep the bucket permanently full so trim-eligible + // queues carrying normal (uncongested) traffic are never policed. + rate = UINT64_MAX; + capacity = UINT64_MAX; + } + + vpp_sonic_ext_trim_queue_set(hwif_name.c_str(), queue_index, eligible, rate, capacity); +} + +void SwitchVpp::refreshTrimDscpToQueueMap() +{ + SWSS_LOG_ENTER(); + + // Compose a switch-global DSCP->queue table (DSCP -> TC -> unicast queue) + // from the first port that has both qos maps bound. The symmetric trim + // datapath assumes uniform DSCP/TC/queue mapping across ports. + for (auto port_oid : getTrimPortList()) + { + sai_attribute_t attr; + + attr.id = SAI_PORT_ATTR_QOS_DSCP_TO_TC_MAP; + if (get(SAI_OBJECT_TYPE_PORT, port_oid, 1, &attr) != SAI_STATUS_SUCCESS || + attr.value.oid == SAI_NULL_OBJECT_ID) + { + continue; + } + sai_object_id_t dscp_to_tc_oid = attr.value.oid; + + attr.id = SAI_PORT_ATTR_QOS_TC_TO_QUEUE_MAP; + if (get(SAI_OBJECT_TYPE_PORT, port_oid, 1, &attr) != SAI_STATUS_SUCCESS || + attr.value.oid == SAI_NULL_OBJECT_ID) + { + continue; + } + sai_object_id_t tc_to_queue_oid = attr.value.oid; + + uint8_t tc_by_dscp[64]; + uint8_t queue_by_tc[64]; + memset(tc_by_dscp, 0, sizeof(tc_by_dscp)); + memset(queue_by_tc, 0, sizeof(queue_by_tc)); + + // DSCP_TO_TC: key.dscp -> value.tc + { + std::vector entries(64); + + attr.id = SAI_QOS_MAP_ATTR_MAP_TO_VALUE_LIST; + attr.value.qosmap.count = (uint32_t) entries.size(); + attr.value.qosmap.list = entries.data(); + + if (get(SAI_OBJECT_TYPE_QOS_MAP, dscp_to_tc_oid, 1, &attr) != SAI_STATUS_SUCCESS) + { + continue; + } + + for (uint32_t i = 0; i < attr.value.qosmap.count; i++) + { + uint8_t dscp = entries[i].key.dscp & 0x3f; + tc_by_dscp[dscp] = entries[i].value.tc; + } + } + + // TC_TO_QUEUE: key.tc -> value.queue_index + { + std::vector entries(64); + + attr.id = SAI_QOS_MAP_ATTR_MAP_TO_VALUE_LIST; + attr.value.qosmap.count = (uint32_t) entries.size(); + attr.value.qosmap.list = entries.data(); + + if (get(SAI_OBJECT_TYPE_QOS_MAP, tc_to_queue_oid, 1, &attr) != SAI_STATUS_SUCCESS) + { + continue; + } + + for (uint32_t i = 0; i < attr.value.qosmap.count; i++) + { + uint8_t tc = entries[i].key.tc & 0x3f; + queue_by_tc[tc] = entries[i].value.queue_index; + } + } + + uint8_t dscp_to_queue[64]; + for (int d = 0; d < 64; d++) + { + uint8_t tc = tc_by_dscp[d] & 0x3f; + dscp_to_queue[d] = queue_by_tc[tc]; + } + + vpp_sonic_ext_trim_dscp_map_set(dscp_to_queue); + return; + } +} + sai_status_t SwitchVpp::queryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, @@ -1121,6 +1596,45 @@ sai_status_t SwitchVpp::queryAttributeCapability( return SAI_STATUS_SUCCESS; } +sai_status_t SwitchVpp::queryAttrEnumValuesCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Inout_ sai_s32_list_t *enum_values_capability) +{ + SWSS_LOG_ENTER(); + + // Advertise only the trim resolution modes the VPP software admission shim + // actually honors: DSCP_VALUE for the DSCP resolution mode and STATIC for + // the queue resolution mode. The asymmetric FROM_TC / DYNAMIC modes are + // intentionally omitted so orchagent never programs a mode VPP cannot + // satisfy. The buffer-profile DROP_AND_TRIM admission-fail action is + // advertised as-is by the base metadata. + if (object_type == SAI_OBJECT_TYPE_SWITCH && + (attr_id == SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE || + attr_id == SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE)) + { + sai_int32_t value = + (attr_id == SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE) + ? (sai_int32_t) SAI_PACKET_TRIM_DSCP_RESOLUTION_MODE_DSCP_VALUE + : (sai_int32_t) SAI_PACKET_TRIM_QUEUE_RESOLUTION_MODE_STATIC; + + if (enum_values_capability->count < 1) + { + enum_values_capability->count = 1; + return SAI_STATUS_BUFFER_OVERFLOW; + } + + enum_values_capability->count = 1; + enum_values_capability->list[0] = value; + + return SAI_STATUS_SUCCESS; + } + + return SwitchStateBase::queryAttrEnumValuesCapability( + switch_id, object_type, attr_id, enum_values_capability); +} + sai_status_t SwitchVpp::queryStatsStCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, @@ -1612,6 +2126,8 @@ sai_status_t SwitchVpp::create_internal( m_object_db.create_or_update(object_type, serializedObjectId, attr_count, attr_list, true /*is_create*/); + refreshTrimDataplaneOnChange(object_type, attr_count, attr_list); + return SAI_STATUS_SUCCESS; } @@ -2057,6 +2573,16 @@ sai_status_t SwitchVpp::set( m_tunnel_mgr.set_vxlan_port(attr); break; } + case SAI_SWITCH_ATTR_PACKET_TRIM_SIZE: + case SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE: + case SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_VALUE: + case SAI_SWITCH_ATTR_PACKET_TRIM_TC_VALUE: + case SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE: + case SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_INDEX: + { + setSwitchTrimAttr(attr); + break; + } } } @@ -2130,6 +2656,8 @@ sai_status_t SwitchVpp::set_internal( // set have only one attribute attrHash[a->getAttrMetadata()->attridname] = a; + refreshTrimDataplaneOnChange(objectType, 1, attr); + return SAI_STATUS_SUCCESS; } diff --git a/vslib/vpp/SwitchVpp.h b/vslib/vpp/SwitchVpp.h index 26eab9f10e..2f1f713adc 100644 --- a/vslib/vpp/SwitchVpp.h +++ b/vslib/vpp/SwitchVpp.h @@ -191,6 +191,12 @@ namespace saivs _In_ sai_attr_id_t attr_id, _Out_ sai_attr_capability_t *capability) override; + virtual sai_status_t queryAttrEnumValuesCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Inout_ sai_s32_list_t *enum_values_capability) override; + virtual sai_status_t queryStatsStCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, @@ -406,6 +412,72 @@ namespace saivs _In_ bool current_attr_found, _In_ bool current_egress_disable); + // True if a set/create on (object_type, attr_id) can change the + // resolved per-queue trim datapath -- the queue's buffer profile or + // scheduler binding, the buffer profile admission-fail action, or + // the scheduler PIR. Used to trigger an order-independent + // re-resolve of the VPP trim plugin state. + static bool isTrimDataplaneAttr( + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id); + + // Accumulate a switch-level packet-trim attribute into m_trim_policy + // and push the resulting global policy to the VPP sonic_ext trim + // plugin. Invoked from set() for the SAI_SWITCH_ATTR_PACKET_TRIM_* + // attributes. + sai_status_t setSwitchTrimAttr( + _In_ const sai_attribute_t *attr); + + // Send the current accumulated m_trim_policy to VPP via + // sonic_ext_trim_global_set (an idempotent full-state set). + void programTrimGlobal(); + + // --- Packet-trim per-queue datapath wiring ------------------------ + // Beyond the global policy, the VPP sonic_ext trim plugin needs a + // per-(egress port, queue) admission state and a switch-global + // DSCP->queue table. Both are derived from the SAI QoS object graph: + // a queue whose buffer profile uses DROP_AND_TRIM is trim-eligible, + // and a queue whose scheduler imposes a PIR (SONiC + // SCHEDULER_BLOCK_DATA_PLANE) is treated as blocking so its software + // token bucket drains and yields a real admission failure. The + // resolve is recompute-all, so it is independent of the order in + // which orchagent programs the queue / buffer / scheduler objects. + + // Re-resolve and re-push the whole trim datapath when trimming is + // enabled and one of attr_list is trim-relevant (isTrimDataplaneAttr). + void refreshTrimDataplaneOnChange( + _In_ sai_object_type_t object_type, + _In_ uint32_t attr_count, + _In_ const sai_attribute_t *attr_list); + + // Enumerate front-panel port OIDs from the authoritative ASIC + // object store (m_objectHash). On sonic-vpp ports are created + // dynamically via createPort() and never land in m_port_list, so + // trim programming must not rely on that cache. + std::vector getTrimPortList() const; + + // Re-resolve every port/queue admission state and the DSCP->queue + // map from the SAI object graph and push them to the VPP plugin. + void refreshTrimDataplane(); + + // Build a queue OID -> parent (leaf) scheduler group OID map by + // scanning SAI_SCHEDULER_GROUP child lists. SONiC applies per-queue + // schedulers to the leaf scheduler group, so blocking detection must + // resolve a queue's scheduler through its group. + void buildQueueSchedulerGroupMap( + _Out_ std::unordered_map &queueToSg); + + // Resolve and push one queue's admission state (eligible + token + // bucket rate/capacity) for the given egress hwif. + void refreshTrimQueue( + _In_ const std::string &hwif_name, + _In_ sai_object_id_t queue_oid, + _In_ const std::unordered_map &queueToSg); + + // Compose PORT DSCP_TO_TC o TC_TO_QUEUE into a switch-global + // dscp_to_queue[64] table and push it to the VPP plugin. + void refreshTrimDscpToQueueMap(); + sai_status_t vpp_create_lag( _In_ sai_object_id_t lag_id, _In_ uint32_t attr_count, @@ -573,6 +645,23 @@ namespace saivs SaiObjectDB m_object_db; TunnelManager m_tunnel_mgr; + // Accumulated switch-global packet-trim policy, mirrored to the VPP + // sonic_ext trim plugin via programTrimGlobal(). Active on VPP: + // queryAttributeCapability() reports the switch trim attributes as + // implemented and the resolution-mode enums advertise the modes the + // shim honors (DSCP_VALUE / STATIC), so orchagent programs the + // global policy and per-queue admission once trimming is configured. + struct TrimGlobalPolicy + { + bool enabled = false; // derived from trim_size != 0 + uint16_t trim_size = 0; + uint8_t dscp_mode = 0; // 0 = DSCP_VALUE, 1 = FROM_TC + uint8_t dscp_value = 0; + uint8_t tc_value = 0; + uint8_t trim_queue = 0; + }; + TrimGlobalPolicy m_trim_policy; + private: // VPP std::map> vrf_objMap; diff --git a/vslib/vpp/vppxlate/SaiVppXlate.c b/vslib/vpp/vppxlate/SaiVppXlate.c index a868050f21..bdb4c4c3cd 100644 --- a/vslib/vpp/vppxlate/SaiVppXlate.c +++ b/vslib/vpp/vppxlate/SaiVppXlate.c @@ -56,6 +56,9 @@ #include #include +#include +#include + #include #include @@ -166,6 +169,24 @@ #include #undef vl_api_version +/* sonic_ext packet-trim API inclusion */ + +#define vl_typedefs +#include +#undef vl_typedefs + +#define vl_endianfun +#include +#undef vl_endianfun + +#define vl_calcsizefun +#include +#undef vl_calcsizefun + +#define vl_api_version(n, v) static u32 sonic_ext_trim_api_version = v; +#include +#undef vl_api_version + /* interface API inclusion */ #define vl_typedefs @@ -1375,6 +1396,48 @@ vl_api_tunterm_acl_interface_add_del_reply_t_handler(vl_api_tunterm_acl_interfac set_reply_status(retval); } +static void +vl_api_sonic_ext_trim_global_set_reply_t_handler(vl_api_sonic_ext_trim_global_set_reply_t *msg) +{ + int retval = (int)ntohl((uint32_t)msg->retval); + set_reply_status(retval); +} + +static void +vl_api_sonic_ext_trim_dscp_map_set_reply_t_handler(vl_api_sonic_ext_trim_dscp_map_set_reply_t *msg) +{ + int retval = (int)ntohl((uint32_t)msg->retval); + set_reply_status(retval); +} + +static void +vl_api_sonic_ext_trim_queue_set_reply_t_handler(vl_api_sonic_ext_trim_queue_set_reply_t *msg) +{ + int retval = (int)ntohl((uint32_t)msg->retval); + set_reply_status(retval); +} + +static void +vl_api_sonic_ext_trim_counters_get_reply_t_handler(vl_api_sonic_ext_trim_counters_get_reply_t *msg) +{ + int retval = (int)ntohl((uint32_t)msg->retval); + + vpp_sonic_ext_trim_counters_t *out = + (vpp_sonic_ext_trim_counters_t *) get_index_ptr(msg->context); + + set_reply_status(retval); + + if (!out) { + return; + } + + out->trim_sent = be64toh(msg->trim_sent); + out->trim_drop = be64toh(msg->trim_drop); + out->trim_admit_fail = be64toh(msg->trim_admit_fail); + + release_index(msg->context); +} + static void vl_api_bond_create_reply_t_handler (vl_api_bond_create_reply_t *msg) { @@ -1526,6 +1589,7 @@ static void vl_api_add_node_next_reply_t_handler( static u16 interface_msg_id_base, memclnt_msg_id_base, __plugin_msg_base; static u16 l2_msg_id_base, vxlan_msg_id_base, ipip_msg_id_base; static u16 tunterm_msg_id_base; +static u16 sonic_ext_trim_msg_id_base; static u16 bfd_msg_id_base; static u16 sr_msg_id_base; static u16 bond_msg_id_base; @@ -1706,6 +1770,9 @@ vl_api_acl_interface_add_del_reply_t_handler(vl_api_acl_interface_add_del_reply_ #define TUNTERM_MSG_ID(id) \ (VL_API_##id + tunterm_msg_id_base) +#define SONIC_EXT_TRIM_MSG_ID(id) \ + (VL_API_##id + sonic_ext_trim_msg_id_base) + #define VXLAN_MSG_ID(id) \ (VL_API_##id + vxlan_msg_id_base) @@ -1734,7 +1801,11 @@ vl_api_acl_interface_add_del_reply_t_handler(vl_api_acl_interface_add_del_reply_ _(SFLOW_MSG_ID(SFLOW_ENABLE_DISABLE_REPLY), sflow_enable_disable_reply) \ _(SFLOW_MSG_ID(SFLOW_SAMPLING_RATE_SET_REPLY), sflow_sampling_rate_set_reply) \ _(IPIP_MSG_ID(IPIP_ADD_TUNNEL_REPLY), ipip_add_tunnel_reply) \ - _(IPIP_MSG_ID(IPIP_DEL_TUNNEL_REPLY), ipip_del_tunnel_reply) + _(IPIP_MSG_ID(IPIP_DEL_TUNNEL_REPLY), ipip_del_tunnel_reply) \ + _(SONIC_EXT_TRIM_MSG_ID(SONIC_EXT_TRIM_GLOBAL_SET_REPLY), sonic_ext_trim_global_set_reply) \ + _(SONIC_EXT_TRIM_MSG_ID(SONIC_EXT_TRIM_DSCP_MAP_SET_REPLY), sonic_ext_trim_dscp_map_set_reply) \ + _(SONIC_EXT_TRIM_MSG_ID(SONIC_EXT_TRIM_QUEUE_SET_REPLY), sonic_ext_trim_queue_set_reply) \ + _(SONIC_EXT_TRIM_MSG_ID(SONIC_EXT_TRIM_COUNTERS_GET_REPLY), sonic_ext_trim_counters_get_reply) static void vpp_plugin_vpe_init(void) { @@ -1807,6 +1878,10 @@ static void get_base_msg_id() tunterm_msg_id_base = vl_client_get_first_plugin_msg_id ((char *) msg_base_lookup_name); assert(tunterm_msg_id_base != (u16) ~0); + msg_base_lookup_name = format (0, "sonic_ext_trim_%08x%c", sonic_ext_trim_api_version, 0); + sonic_ext_trim_msg_id_base = vl_client_get_first_plugin_msg_id ((char *) msg_base_lookup_name); + assert(sonic_ext_trim_msg_id_base != (u16) ~0); + msg_base_lookup_name = format (0, "classify_%08x%c", classify_api_version, 0); classify_msg_id_base = vl_client_get_first_plugin_msg_id ((char *) msg_base_lookup_name); assert(classify_msg_id_base != (u16) ~0); @@ -2952,6 +3027,141 @@ int vpp_tunterm_acl_add_replace (uint32_t *tunterm_index, uint32_t count, vpp_tu return ret; } +int vpp_sonic_ext_trim_global_set(bool is_enable, uint16_t trim_size, + uint8_t dscp_mode, uint8_t dscp_value, + uint8_t tc_value, uint8_t trim_queue) +{ + vat_main_t *vam = &vat_main; + vl_api_sonic_ext_trim_global_set_t *mp; + int ret; + + VPP_LOCK(); + + __plugin_msg_base = sonic_ext_trim_msg_id_base; + M (SONIC_EXT_TRIM_GLOBAL_SET, mp); + + mp->is_enable = is_enable; + mp->trim_size = htons(trim_size); + mp->dscp_mode = dscp_mode; + mp->dscp_value = dscp_value; + mp->tc_value = tc_value; + mp->trim_queue = trim_queue; + + S (mp); + WR (ret); + + if (ret) { SAIVPP_ERROR("%s failed(%d) enable %d size %u queue %u", __func__, ret, is_enable, trim_size, trim_queue); } + else { SAIVPP_INFO("%s enable %d size %u dscp_mode %u dscp %u tc %u queue %u", __func__, is_enable, trim_size, dscp_mode, dscp_value, tc_value, trim_queue); } + + VPP_UNLOCK(); + + return ret; +} + +int vpp_sonic_ext_trim_dscp_map_set(const uint8_t dscp_to_queue[64]) +{ + vat_main_t *vam = &vat_main; + vl_api_sonic_ext_trim_dscp_map_set_t *mp; + int ret; + + VPP_LOCK(); + + __plugin_msg_base = sonic_ext_trim_msg_id_base; + M (SONIC_EXT_TRIM_DSCP_MAP_SET, mp); + + memcpy(mp->dscp_to_queue, dscp_to_queue, 64); + + S (mp); + WR (ret); + + if (ret) { SAIVPP_ERROR("%s failed(%d)", __func__, ret); } + else { SAIVPP_INFO("%s ok", __func__); } + + VPP_UNLOCK(); + + return ret; +} + +int vpp_sonic_ext_trim_queue_set(const char *hwif_name, uint8_t queue, + bool eligible, uint64_t rate_bytes_per_sec, + uint64_t capacity_bytes) +{ + vat_main_t *vam = &vat_main; + vl_api_sonic_ext_trim_queue_set_t *mp; + u32 idx; + int ret; + + if (!hwif_name) { + return -EINVAL; + } + + VPP_LOCK(); + + idx = get_swif_idx(vam, hwif_name); + if (idx == (u32) -1) { + SAIVPP_ERROR("Unable to get sw_index for %s\n", hwif_name); + VPP_UNLOCK(); + return -EINVAL; + } + + __plugin_msg_base = sonic_ext_trim_msg_id_base; + M (SONIC_EXT_TRIM_QUEUE_SET, mp); + + mp->sw_if_index = htonl(idx); + mp->queue = queue; + mp->eligible = eligible; + mp->rate_bytes_per_sec = htobe64(rate_bytes_per_sec); + mp->capacity_bytes = htobe64(capacity_bytes); + + S (mp); + WR (ret); + + if (ret) { SAIVPP_ERROR("%s failed(%d) hwif %s queue %u eligible %d rate %lu cap %lu", __func__, ret, hwif_name, queue, eligible, (unsigned long)rate_bytes_per_sec, (unsigned long)capacity_bytes); } + else { SAIVPP_INFO("%s hwif %s queue %u eligible %d rate %lu cap %lu", __func__, hwif_name, queue, eligible, (unsigned long)rate_bytes_per_sec, (unsigned long)capacity_bytes); } + + VPP_UNLOCK(); + + return ret; +} + +int vpp_sonic_ext_trim_counters_get(vpp_sonic_ext_trim_counters_t *out) +{ + vat_main_t *vam = &vat_main; + vl_api_sonic_ext_trim_counters_get_t *mp; + uint32_t context; + int ret; + + if (!out) { + return -EINVAL; + } + + VPP_LOCK(); + + __plugin_msg_base = sonic_ext_trim_msg_id_base; + M (SONIC_EXT_TRIM_COUNTERS_GET, mp); + + context = store_ptr(out); + if (context == VPP_INVALID_CTX_INDEX) { + VPP_UNLOCK(); + return -ENOMEM; + } + mp->context = context; + + S (mp); + WR (ret); + + if (get_index_ptr(context) != (uintptr_t) NULL) { + release_index(context); + } + + if (ret) { SAIVPP_ERROR("%s failed(%d)", __func__, ret); } + else { SAIVPP_INFO("%s sent %lu drop %lu admit_fail %lu", __func__, (unsigned long)out->trim_sent, (unsigned long)out->trim_drop, (unsigned long)out->trim_admit_fail); } + + VPP_UNLOCK(); + + return ret; +} + static int vpp_acl_counters_enable_disable (bool enable) { vat_main_t *vam = &vat_main; diff --git a/vslib/vpp/vppxlate/SaiVppXlate.h b/vslib/vpp/vppxlate/SaiVppXlate.h index ecf98cad21..09edddcd32 100644 --- a/vslib/vpp/vppxlate/SaiVppXlate.h +++ b/vslib/vpp/vppxlate/SaiVppXlate.h @@ -420,6 +420,27 @@ typedef enum { bool is_input); extern int vpp_add_node_next(const char *node_name, const char *next_name, uint32_t *next_index); + + /* + * SONiC-VPP packet-trimming (SAI DROP_AND_TRIM) control API. + * SAI-VPP programs the global trim policy, the composed DSCP->queue + * table, and the per-(port,queue) software admission parameters, and + * reads back the trim counters for SAI switch/port/queue statistics. + */ + typedef struct vpp_sonic_ext_trim_counters_ { + uint64_t trim_sent; + uint64_t trim_drop; + uint64_t trim_admit_fail; + } vpp_sonic_ext_trim_counters_t; + + extern int vpp_sonic_ext_trim_global_set(bool is_enable, uint16_t trim_size, + uint8_t dscp_mode, uint8_t dscp_value, + uint8_t tc_value, uint8_t trim_queue); + extern int vpp_sonic_ext_trim_dscp_map_set(const uint8_t dscp_to_queue[64]); + extern int vpp_sonic_ext_trim_queue_set(const char *hwif_name, uint8_t queue, + bool eligible, uint64_t rate_bytes_per_sec, + uint64_t capacity_bytes); + extern int vpp_sonic_ext_trim_counters_get(vpp_sonic_ext_trim_counters_t *out); #ifdef __cplusplus } #endif From cc01de3d78e09e92ec786aadacd5e57414bb1e9e Mon Sep 17 00:00:00 2001 From: Aaron Bernardino Date: Thu, 30 Jul 2026 22:42:22 +0000 Subject: [PATCH 2/2] [vslib/vpp] Harden trim SAI translation: error propagation and qos refresh Address code-review gaps M1, M2 and H2 in the VPP packet-trim wiring: - M1 (error propagation): programTrimGlobal, refreshTrimDataplaneOnChange, refreshTrimDataplane, refreshTrimQueue and refreshTrimDscpToQueueMap now return sai_status_t and propagate VPP programming failures instead of returning void and swallowing them. A SET surfaces the failure; CREATE keeps it non-fatal so bring-up is not blocked by a transient push error. - M2 (qos-map refresh): isTrimDataplaneAttr now also recognizes the PORT qos-map attributes (DSCP_TO_TC / TC_TO_QUEUE bindings) and QOS_MAP object updates, so runtime qos reconfiguration re-resolves and re-pushes the trim DSCP->queue map and per-queue admission state instead of drifting. - H2 (unit tests): replace isPacketTrimSwitchAttr and isUnsupportedTrimEnumCapability -- which referenced methods that no longer exist and did not compile -- with real coverage for the new static getTrimEnumValuesCapability helper and for isTrimDataplaneAttr. Built clean under -Werror; three in-scope PTF trimming tests pass on the dev-VM t1-lag-vpp testbed. Tracking: sonic-net/sonic-buildimage#25789 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino --- unittest/vslib/TestSwitchVpp.cpp | 82 +++++++++------ vslib/vpp/SwitchVpp.cpp | 175 ++++++++++++++++++++++++------- vslib/vpp/SwitchVpp.h | 34 ++++-- 3 files changed, 215 insertions(+), 76 deletions(-) diff --git a/unittest/vslib/TestSwitchVpp.cpp b/unittest/vslib/TestSwitchVpp.cpp index c81dfac2b3..05ffeac3e8 100644 --- a/unittest/vslib/TestSwitchVpp.cpp +++ b/unittest/vslib/TestSwitchVpp.cpp @@ -16,41 +16,63 @@ TEST(SwitchVpp, getLagMemberEgressDisableAction) EXPECT_EQ(Action::DISABLE, SwitchVpp::getLagMemberEgressDisableAction(true, false, false)); } -TEST(SwitchVpp, isPacketTrimSwitchAttr) +TEST(SwitchVpp, getTrimEnumValuesCapability) { - // All six switch-level packet-trim attributes must be classified as trim - // attrs so queryAttributeCapability() reports them not implemented and - // sonic-swss publishes SWITCH_TRIMMING_CAPABLE=false. - EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_SIZE)); - EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE)); - EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_VALUE)); - EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_TC_VALUE)); - EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE)); - EXPECT_TRUE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_INDEX)); - - // Unrelated switch attributes must not be gated off. - EXPECT_FALSE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_INIT_SWITCH)); - EXPECT_FALSE(SwitchVpp::isPacketTrimSwitchAttr(SAI_SWITCH_ATTR_ECMP_HASH)); + sai_int32_t value = -1; + + // The DSCP resolution mode advertises only DSCP_VALUE (symmetric trim); the + // asymmetric FROM_TC mode is intentionally not offered on VPP. + ASSERT_TRUE(SwitchVpp::getTrimEnumValuesCapability( + SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE, &value)); + EXPECT_EQ((sai_int32_t)SAI_PACKET_TRIM_DSCP_RESOLUTION_MODE_DSCP_VALUE, value); + + // The queue resolution mode advertises only STATIC. + value = -1; + ASSERT_TRUE(SwitchVpp::getTrimEnumValuesCapability( + SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE, &value)); + EXPECT_EQ((sai_int32_t)SAI_PACKET_TRIM_QUEUE_RESOLUTION_MODE_STATIC, value); + + // Unrelated switch enum attributes fall through to the base metadata. + EXPECT_FALSE(SwitchVpp::getTrimEnumValuesCapability( + SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_ECMP_HASH, &value)); + + // A trim switch attr queried against the wrong object type must not match. + EXPECT_FALSE(SwitchVpp::getTrimEnumValuesCapability( + SAI_OBJECT_TYPE_BUFFER_PROFILE, SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE, &value)); + + // A null out-pointer is tolerated (predicate still resolves). + EXPECT_TRUE(SwitchVpp::getTrimEnumValuesCapability( + SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE, nullptr)); } -TEST(SwitchVpp, isUnsupportedTrimEnumCapability) +TEST(SwitchVpp, isTrimDataplaneAttr) { - // The trim-related enum capabilities the base virtual switch advertises - // must be reported unsupported on VPP. - EXPECT_TRUE(SwitchVpp::isUnsupportedTrimEnumCapability( - SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE)); - EXPECT_TRUE(SwitchVpp::isUnsupportedTrimEnumCapability( - SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE)); - EXPECT_TRUE(SwitchVpp::isUnsupportedTrimEnumCapability( + // Queue buffer-profile / scheduler bindings drive per-queue admission. + EXPECT_TRUE(SwitchVpp::isTrimDataplaneAttr( + SAI_OBJECT_TYPE_QUEUE, SAI_QUEUE_ATTR_BUFFER_PROFILE_ID)); + EXPECT_TRUE(SwitchVpp::isTrimDataplaneAttr( + SAI_OBJECT_TYPE_QUEUE, SAI_QUEUE_ATTR_SCHEDULER_PROFILE_ID)); + EXPECT_TRUE(SwitchVpp::isTrimDataplaneAttr( SAI_OBJECT_TYPE_BUFFER_PROFILE, SAI_BUFFER_PROFILE_ATTR_PACKET_ADMISSION_FAIL_ACTION)); + EXPECT_TRUE(SwitchVpp::isTrimDataplaneAttr( + SAI_OBJECT_TYPE_SCHEDULER_GROUP, SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID)); + EXPECT_TRUE(SwitchVpp::isTrimDataplaneAttr( + SAI_OBJECT_TYPE_SCHEDULER, SAI_SCHEDULER_ATTR_MAX_BANDWIDTH_RATE)); - // Non-trim enum attributes must still be delegated to the base class. - EXPECT_FALSE(SwitchVpp::isUnsupportedTrimEnumCapability( - SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_ECMP_HASH)); - EXPECT_FALSE(SwitchVpp::isUnsupportedTrimEnumCapability( - SAI_OBJECT_TYPE_TAM, SAI_TAM_ATTR_TAM_BIND_POINT_TYPE_LIST)); + // Port QoS-map (re)bindings and in-place QoS-map edits re-resolve the + // switch-global DSCP->queue trim table. + EXPECT_TRUE(SwitchVpp::isTrimDataplaneAttr( + SAI_OBJECT_TYPE_PORT, SAI_PORT_ATTR_QOS_DSCP_TO_TC_MAP)); + EXPECT_TRUE(SwitchVpp::isTrimDataplaneAttr( + SAI_OBJECT_TYPE_PORT, SAI_PORT_ATTR_QOS_TC_TO_QUEUE_MAP)); + EXPECT_TRUE(SwitchVpp::isTrimDataplaneAttr( + SAI_OBJECT_TYPE_QOS_MAP, SAI_QOS_MAP_ATTR_MAP_TO_VALUE_LIST)); - // A trim switch attr queried against the wrong object type must not match. - EXPECT_FALSE(SwitchVpp::isUnsupportedTrimEnumCapability( - SAI_OBJECT_TYPE_BUFFER_PROFILE, SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE)); + // Unrelated attributes must not trigger a trim refresh. + EXPECT_FALSE(SwitchVpp::isTrimDataplaneAttr( + SAI_OBJECT_TYPE_QUEUE, SAI_QUEUE_ATTR_TYPE)); + EXPECT_FALSE(SwitchVpp::isTrimDataplaneAttr( + SAI_OBJECT_TYPE_PORT, SAI_PORT_ATTR_MTU)); + EXPECT_FALSE(SwitchVpp::isTrimDataplaneAttr( + SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_PACKET_TRIM_SIZE)); } diff --git a/vslib/vpp/SwitchVpp.cpp b/vslib/vpp/SwitchVpp.cpp index 67894f21ef..cc325d4e2d 100644 --- a/vslib/vpp/SwitchVpp.cpp +++ b/vslib/vpp/SwitchVpp.cpp @@ -1128,6 +1128,18 @@ bool SwitchVpp::isTrimDataplaneAttr( case SAI_OBJECT_TYPE_SCHEDULER: return attr_id == SAI_SCHEDULER_ATTR_MAX_BANDWIDTH_RATE; + case SAI_OBJECT_TYPE_PORT: + // The DSCP->queue trim table is composed from the port's bound + // DSCP_TO_TC and TC_TO_QUEUE qos maps; a (re)bind or unbind at + // runtime must re-resolve it. + return attr_id == SAI_PORT_ATTR_QOS_DSCP_TO_TC_MAP || + attr_id == SAI_PORT_ATTR_QOS_TC_TO_QUEUE_MAP; + + case SAI_OBJECT_TYPE_QOS_MAP: + // An in-place edit of a bound DSCP_TO_TC / TC_TO_QUEUE map likewise + // changes the resolved DSCP->queue table. + return attr_id == SAI_QOS_MAP_ATTR_MAP_TO_VALUE_LIST; + default: return false; } @@ -1175,15 +1187,15 @@ sai_status_t SwitchVpp::setSwitchTrimAttr( return SAI_STATUS_SUCCESS; } - programTrimGlobal(); - - return SAI_STATUS_SUCCESS; + return programTrimGlobal(); } -void SwitchVpp::programTrimGlobal() +sai_status_t SwitchVpp::programTrimGlobal() { SWSS_LOG_ENTER(); + sai_status_t status = SAI_STATUS_SUCCESS; + int ret = vpp_sonic_ext_trim_global_set( m_trim_policy.enabled, m_trim_policy.trim_size, @@ -1198,6 +1210,7 @@ void SwitchVpp::programTrimGlobal() ret, m_trim_policy.enabled, m_trim_policy.trim_size, m_trim_policy.dscp_mode, m_trim_policy.dscp_value, m_trim_policy.tc_value, m_trim_policy.trim_queue); + status = SAI_STATUS_FAILURE; } // The global policy just changed. Queues / buffer profiles / schedulers may @@ -1206,11 +1219,18 @@ void SwitchVpp::programTrimGlobal() // state now that trimming is active. if (m_trim_policy.enabled) { - refreshTrimDataplane(); + sai_status_t refresh_status = refreshTrimDataplane(); + + if (refresh_status != SAI_STATUS_SUCCESS) + { + status = refresh_status; + } } + + return status; } -void SwitchVpp::refreshTrimDataplaneOnChange( +sai_status_t SwitchVpp::refreshTrimDataplaneOnChange( _In_ sai_object_type_t object_type, _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list) @@ -1219,17 +1239,18 @@ void SwitchVpp::refreshTrimDataplaneOnChange( // plugin bypasses all traffic regardless of the per-queue state. if (!m_trim_policy.enabled) { - return; + return SAI_STATUS_SUCCESS; } for (uint32_t i = 0; i < attr_count; i++) { if (isTrimDataplaneAttr(object_type, attr_list[i].id)) { - refreshTrimDataplane(); - return; + return refreshTrimDataplane(); } } + + return SAI_STATUS_SUCCESS; } std::vector SwitchVpp::getTrimPortList() const @@ -1261,7 +1282,7 @@ std::vector SwitchVpp::getTrimPortList() const return ports; } -void SwitchVpp::refreshTrimDataplane() +sai_status_t SwitchVpp::refreshTrimDataplane() { SWSS_LOG_ENTER(); @@ -1279,6 +1300,8 @@ void SwitchVpp::refreshTrimDataplane() std::unordered_map queue_to_sg; buildQueueSchedulerGroupMap(queue_to_sg); + sai_status_t status = SAI_STATUS_SUCCESS; + for (auto port_oid : port_list) { std::string hwif; @@ -1312,11 +1335,23 @@ void SwitchVpp::refreshTrimDataplane() for (uint32_t i = 0; i < n; i++) { - refreshTrimQueue(hwif, queues[i], queue_to_sg); + sai_status_t queue_status = refreshTrimQueue(hwif, queues[i], queue_to_sg); + + if (queue_status != SAI_STATUS_SUCCESS) + { + status = queue_status; + } } } - refreshTrimDscpToQueueMap(); + sai_status_t map_status = refreshTrimDscpToQueueMap(); + + if (map_status != SAI_STATUS_SUCCESS) + { + status = map_status; + } + + return status; } void SwitchVpp::buildQueueSchedulerGroupMap( @@ -1373,7 +1408,7 @@ void SwitchVpp::buildQueueSchedulerGroupMap( } } -void SwitchVpp::refreshTrimQueue( +sai_status_t SwitchVpp::refreshTrimQueue( _In_ const std::string &hwif_name, _In_ sai_object_id_t queue_oid, _In_ const std::unordered_map &queueToSg) @@ -1389,14 +1424,14 @@ void SwitchVpp::refreshTrimQueue( if (qtype != SAI_QUEUE_TYPE_UNICAST && qtype != SAI_QUEUE_TYPE_ALL) { - return; + return SAI_STATUS_SUCCESS; } } attr.id = SAI_QUEUE_ATTR_INDEX; if (get(SAI_OBJECT_TYPE_QUEUE, queue_oid, 1, &attr) != SAI_STATUS_SUCCESS) { - return; + return SAI_STATUS_SUCCESS; } uint8_t queue_index = attr.value.u8; @@ -1408,7 +1443,7 @@ void SwitchVpp::refreshTrimQueue( static const uint8_t VPP_TRIM_MAX_QUEUES = 8; if (queue_index >= VPP_TRIM_MAX_QUEUES) { - return; + return SAI_STATUS_SUCCESS; } // Eligibility: the queue's bound buffer profile uses DROP_AND_TRIM. @@ -1489,10 +1524,20 @@ void SwitchVpp::refreshTrimQueue( capacity = UINT64_MAX; } - vpp_sonic_ext_trim_queue_set(hwif_name.c_str(), queue_index, eligible, rate, capacity); + int ret = vpp_sonic_ext_trim_queue_set(hwif_name.c_str(), queue_index, eligible, rate, capacity); + + if (ret != 0) + { + SWSS_LOG_ERROR("vpp_sonic_ext_trim_queue_set failed (ret=%d) hwif=%s queue=%u eligible=%d rate=%llu cap=%llu", + ret, hwif_name.c_str(), queue_index, (int)eligible, + (unsigned long long)rate, (unsigned long long)capacity); + return SAI_STATUS_FAILURE; + } + + return SAI_STATUS_SUCCESS; } -void SwitchVpp::refreshTrimDscpToQueueMap() +sai_status_t SwitchVpp::refreshTrimDscpToQueueMap() { SWSS_LOG_ENTER(); @@ -1571,9 +1616,21 @@ void SwitchVpp::refreshTrimDscpToQueueMap() dscp_to_queue[d] = queue_by_tc[tc]; } - vpp_sonic_ext_trim_dscp_map_set(dscp_to_queue); - return; + int ret = vpp_sonic_ext_trim_dscp_map_set(dscp_to_queue); + + if (ret != 0) + { + SWSS_LOG_ERROR("vpp_sonic_ext_trim_dscp_map_set failed (ret=%d)", ret); + return SAI_STATUS_FAILURE; + } + + return SAI_STATUS_SUCCESS; } + + // No port has both qos maps bound yet: nothing to push. The plugin keeps + // its last-good table (initially identity->queue 0) rather than being + // cleared to an all-zero map, which would collapse every DSCP onto queue 0. + return SAI_STATUS_SUCCESS; } sai_status_t SwitchVpp::queryAttributeCapability( @@ -1596,6 +1653,42 @@ sai_status_t SwitchVpp::queryAttributeCapability( return SAI_STATUS_SUCCESS; } +bool SwitchVpp::getTrimEnumValuesCapability( + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_int32_t *value) +{ + // VPP's software admission shim honors only one resolution mode per trim + // enum: DSCP_VALUE for the DSCP resolution mode and STATIC for the queue + // resolution mode. The asymmetric FROM_TC / DYNAMIC modes are intentionally + // omitted so orchagent never programs a mode VPP cannot satisfy. All other + // (object_type, attr_id) pairs fall through to the base metadata. + if (object_type != SAI_OBJECT_TYPE_SWITCH) + { + return false; + } + + switch (attr_id) + { + case SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE: + if (value) + { + *value = (sai_int32_t) SAI_PACKET_TRIM_DSCP_RESOLUTION_MODE_DSCP_VALUE; + } + return true; + + case SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE: + if (value) + { + *value = (sai_int32_t) SAI_PACKET_TRIM_QUEUE_RESOLUTION_MODE_STATIC; + } + return true; + + default: + return false; + } +} + sai_status_t SwitchVpp::queryAttrEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, @@ -1605,20 +1698,13 @@ sai_status_t SwitchVpp::queryAttrEnumValuesCapability( SWSS_LOG_ENTER(); // Advertise only the trim resolution modes the VPP software admission shim - // actually honors: DSCP_VALUE for the DSCP resolution mode and STATIC for - // the queue resolution mode. The asymmetric FROM_TC / DYNAMIC modes are - // intentionally omitted so orchagent never programs a mode VPP cannot - // satisfy. The buffer-profile DROP_AND_TRIM admission-fail action is - // advertised as-is by the base metadata. - if (object_type == SAI_OBJECT_TYPE_SWITCH && - (attr_id == SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE || - attr_id == SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE)) - { - sai_int32_t value = - (attr_id == SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE) - ? (sai_int32_t) SAI_PACKET_TRIM_DSCP_RESOLUTION_MODE_DSCP_VALUE - : (sai_int32_t) SAI_PACKET_TRIM_QUEUE_RESOLUTION_MODE_STATIC; + // actually honors (see getTrimEnumValuesCapability). The buffer-profile + // DROP_AND_TRIM admission-fail action is advertised as-is by the base + // metadata. + sai_int32_t value = 0; + if (getTrimEnumValuesCapability(object_type, attr_id, &value)) + { if (enum_values_capability->count < 1) { enum_values_capability->count = 1; @@ -2126,7 +2212,10 @@ sai_status_t SwitchVpp::create_internal( m_object_db.create_or_update(object_type, serializedObjectId, attr_count, attr_list, true /*is_create*/); - refreshTrimDataplaneOnChange(object_type, attr_count, attr_list); + // A trim-refresh push failure must not fail object creation (the SAI object + // is created regardless); the SET path is where a dataplane push failure is + // reported back to orchagent. Intentionally ignore the status here. + (void) refreshTrimDataplaneOnChange(object_type, attr_count, attr_list); return SAI_STATUS_SUCCESS; } @@ -2561,6 +2650,8 @@ sai_status_t SwitchVpp::set( if (objectType == SAI_OBJECT_TYPE_SWITCH) { + sai_status_t trim_status = SAI_STATUS_SUCCESS; + switch(attr->id) { case SAI_SWITCH_ATTR_VXLAN_DEFAULT_ROUTER_MAC: @@ -2580,10 +2671,17 @@ sai_status_t SwitchVpp::set( case SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE: case SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_INDEX: { - setSwitchTrimAttr(attr); + trim_status = setSwitchTrimAttr(attr); break; } } + + // Persist the switch attribute in the object store, then surface any + // trim programming failure (setSwitchTrimAttr pushes the global policy + // and per-queue admission to VPP) to the caller. + sai_status_t store_status = set_internal(objectType, serializedObjectId, attr); + + return (trim_status != SAI_STATUS_SUCCESS) ? trim_status : store_status; } if (objectType == SAI_OBJECT_TYPE_ACL_ENTRY) @@ -2656,9 +2754,10 @@ sai_status_t SwitchVpp::set_internal( // set have only one attribute attrHash[a->getAttrMetadata()->attridname] = a; - refreshTrimDataplaneOnChange(objectType, 1, attr); - - return SAI_STATUS_SUCCESS; + // Propagate a trim dataplane (re)program failure to the caller: a runtime + // qos/queue/buffer/scheduler set that could not be pushed to VPP must be + // reported, not silently swallowed. Returns SUCCESS for non-trim attrs. + return refreshTrimDataplaneOnChange(objectType, 1, attr); } sai_status_t SwitchVpp::get( diff --git a/vslib/vpp/SwitchVpp.h b/vslib/vpp/SwitchVpp.h index 2f1f713adc..04989dc43f 100644 --- a/vslib/vpp/SwitchVpp.h +++ b/vslib/vpp/SwitchVpp.h @@ -421,6 +421,17 @@ namespace saivs _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id); + // True if VPP overrides the enum-values capability for this trim + // attribute, advertising only the single resolution mode the + // software admission shim honors (DSCP_VALUE for the DSCP mode, + // STATIC for the queue mode). On a match *value receives that enum; + // all other attributes fall through to the base metadata. Backs + // queryAttrEnumValuesCapability(). + static bool getTrimEnumValuesCapability( + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_int32_t *value); + // Accumulate a switch-level packet-trim attribute into m_trim_policy // and push the resulting global policy to the VPP sonic_ext trim // plugin. Invoked from set() for the SAI_SWITCH_ATTR_PACKET_TRIM_* @@ -429,8 +440,10 @@ namespace saivs _In_ const sai_attribute_t *attr); // Send the current accumulated m_trim_policy to VPP via - // sonic_ext_trim_global_set (an idempotent full-state set). - void programTrimGlobal(); + // sonic_ext_trim_global_set (an idempotent full-state set). Returns + // the VPP programming status (including the per-queue re-resolve + // when trimming is enabled). + sai_status_t programTrimGlobal(); // --- Packet-trim per-queue datapath wiring ------------------------ // Beyond the global policy, the VPP sonic_ext trim plugin needs a @@ -445,7 +458,9 @@ namespace saivs // Re-resolve and re-push the whole trim datapath when trimming is // enabled and one of attr_list is trim-relevant (isTrimDataplaneAttr). - void refreshTrimDataplaneOnChange( + // Returns the VPP programming status (SUCCESS when no refresh is + // needed) so a SET can surface a dataplane push failure. + sai_status_t refreshTrimDataplaneOnChange( _In_ sai_object_type_t object_type, _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list); @@ -458,7 +473,8 @@ namespace saivs // Re-resolve every port/queue admission state and the DSCP->queue // map from the SAI object graph and push them to the VPP plugin. - void refreshTrimDataplane(); + // Returns the aggregate VPP programming status. + sai_status_t refreshTrimDataplane(); // Build a queue OID -> parent (leaf) scheduler group OID map by // scanning SAI_SCHEDULER_GROUP child lists. SONiC applies per-queue @@ -468,15 +484,17 @@ namespace saivs _Out_ std::unordered_map &queueToSg); // Resolve and push one queue's admission state (eligible + token - // bucket rate/capacity) for the given egress hwif. - void refreshTrimQueue( + // bucket rate/capacity) for the given egress hwif. Returns the VPP + // programming status (SUCCESS when the queue is skipped). + sai_status_t refreshTrimQueue( _In_ const std::string &hwif_name, _In_ sai_object_id_t queue_oid, _In_ const std::unordered_map &queueToSg); // Compose PORT DSCP_TO_TC o TC_TO_QUEUE into a switch-global - // dscp_to_queue[64] table and push it to the VPP plugin. - void refreshTrimDscpToQueueMap(); + // dscp_to_queue[64] table and push it to the VPP plugin. Returns the + // VPP programming status (SUCCESS when no map is bound yet). + sai_status_t refreshTrimDscpToQueueMap(); sai_status_t vpp_create_lag( _In_ sai_object_id_t lag_id,