From e4b680b2188be5ccbbb322702e8c671244925b1b Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Thu, 18 Jun 2026 01:05:21 +0000 Subject: [PATCH 01/11] Address racey SAI PHY Serdes object causing bad syncd setup Mitigate a race condition in Port PHY Serdes initialization (syncd) where the SAI is still busy with the object from its creation when syncd's intialization attempts to read attributes from the object for syncd's own setup. Do this by allowing 3 retries with 10ms wait intervals in between tries. The retry will only trigger when the failure reason is SAI_STATUS_OBJECT_IN_USE. Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 90 ++++++++++++++++++++++++++++++++----------- 1 file changed, 68 insertions(+), 22 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 948c0b2b40..abe251b1af 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include "FlexCounter.h" #include "VidManager.h" @@ -2229,16 +2230,30 @@ class PortPhySerdesAttrContext : public AttrContextget(Base::m_objectType, port_serdes_rid, 1, &attr); - if (status == SAI_STATUS_SUCCESS) + for (uint32_t tries = 0; tries < 3; tries++) { - port_rid = attr.value.oid; - return true; - } + sai_status_t status = Base::m_vendorSai->get(Base::m_objectType, port_serdes_rid, 1, &attr); - SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get port RID for port serdes RID:0x%" PRIx64 ", status:%d", - port_serdes_rid, status); + if (status == SAI_STATUS_SUCCESS) + { + port_rid = attr.value.oid; + return true; + } + else if (status == SAI_STATUS_OBJECT_IN_USE and tries < 2) + { + // SAI object is busy - retry in 10ms + SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting port RID for port serdes RID:0x%" PRIx64 "...", + port_serdes_rid); + std::this_thread::sleep_for(chrono::milliseconds(10)); + } + else + { + SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get port RID for port serdes RID:0x%" PRIx64 ", status:%d", + port_serdes_rid, status); + break; + } + } return false; } @@ -2311,13 +2326,28 @@ class PortPhySerdesAttrContext : public AttrContextget(SAI_OBJECT_TYPE_PORT, port_rid, 1, &attr); - if (status != SAI_STATUS_BUFFER_OVERFLOW) + for (uint32_t tries = 0; tries < 3; tries++) { - SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get hardware lane count for port RID:0x%" PRIx64 ", status:%d", - port_rid, status); - return; + sai_status_t status = Base::m_vendorSai->get(SAI_OBJECT_TYPE_PORT, port_rid, 1, &attr); + + // The SAI status expected is SAI_STATUS_BUFFER_OVERFLOW since we pass in a nullptr + // This is the agreed method with Broadcom for retriving the actual lane count + if (status == SAI_STATUS_BUFFER_OVERFLOW) + break; + else if (status == SAI_STATUS_OBJECT_IN_USE && tries < 2) + { + // SAI object is busy - retry in 10ms + SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting hardware lane count for port RID:0x%" PRIx64 "...", + port_rid); + std::this_thread::sleep_for(chrono::milliseconds(10)); + } + else + { + SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get hardware lane count for port RID:0x%" PRIx64 ", status:%d", + port_rid, status); + return; + } } laneCount = attr.value.u32list.count; @@ -2345,18 +2375,34 @@ class PortPhySerdesAttrContext : public AttrContextget( - Base::m_objectType, - port_serdes_rid, - 1, - &attr); - - if (status != SAI_STATUS_SUCCESS) + bool failed = false; + for (uint32_t tries = 0; tries < 3; tries++) { - SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get port serdes count attr %s for port_serdes RID:0x%" PRIx64 ", status:%d", - sai_serialize_port_serdes_attr(attrId).c_str(), port_serdes_rid, status); - continue; + sai_status_t status = Base::m_vendorSai->get( + Base::m_objectType, + port_serdes_rid, + 1, + &attr); + + if (status == SAI_STATUS_SUCCESS) + break; + else if (status == SAI_STATUS_OBJECT_IN_USE && tries < 2) + { + // SAI object is busy - retry in 10ms + SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting port serdes count attr %s for port_serdes RID:0x%" PRIx64 "...", + sai_serialize_port_serdes_attr(attrId).c_str(), port_serdes_rid); + std::this_thread::sleep_for(chrono::milliseconds(10)); + } + else + { + SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get port serdes count attr %s for port_serdes RID:0x%" PRIx64 ", status:%d", + sai_serialize_port_serdes_attr(attrId).c_str(), port_serdes_rid, status); + failed = true; + continue; + } } + if (failed) + continue; count = attr.value.u32; From d67380df7236ea99e89ddb9bf1c418cb76c65e67 Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Thu, 18 Jun 2026 02:00:31 +0000 Subject: [PATCH 02/11] retriving -> retrieving Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index abe251b1af..e7a7a88a30 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -2332,7 +2332,7 @@ class PortPhySerdesAttrContext : public AttrContextget(SAI_OBJECT_TYPE_PORT, port_rid, 1, &attr); // The SAI status expected is SAI_STATUS_BUFFER_OVERFLOW since we pass in a nullptr - // This is the agreed method with Broadcom for retriving the actual lane count + // This is the agreed method with Broadcom for retrieving the actual lane count if (status == SAI_STATUS_BUFFER_OVERFLOW) break; else if (status == SAI_STATUS_OBJECT_IN_USE && tries < 2) From 0d31fcf8ecbba2e1aa017224154496a1dbc38c7e Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Thu, 18 Jun 2026 18:28:07 +0000 Subject: [PATCH 03/11] Change to more deterministic handling of SAI_STATUS_OBJECT_IN_USE Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index e7a7a88a30..9c3e53a1d6 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -2327,28 +2327,29 @@ class PortPhySerdesAttrContext : public AttrContextget(SAI_OBJECT_TYPE_PORT, port_rid, 1, &attr); - - // The SAI status expected is SAI_STATUS_BUFFER_OVERFLOW since we pass in a nullptr - // This is the agreed method with Broadcom for retrieving the actual lane count - if (status == SAI_STATUS_BUFFER_OVERFLOW) - break; - else if (status == SAI_STATUS_OBJECT_IN_USE && tries < 2) - { - // SAI object is busy - retry in 10ms - SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting hardware lane count for port RID:0x%" PRIx64 "...", - port_rid); - std::this_thread::sleep_for(chrono::milliseconds(10)); - } - else + if (status != SAI_STATUS_OBJECT_IN_USE) { - SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get hardware lane count for port RID:0x%" PRIx64 ", status:%d", - port_rid, status); - return; + break; } + + // SAI object is busy - retry in 10ms + SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting hardware lane count for port RID:0x%" PRIx64 "...", + port_rid); + std::this_thread::sleep_for(chrono::milliseconds(10)); } + // The SAI status expected is SAI_STATUS_BUFFER_OVERFLOW since we pass in a nullptr + // This is the agreed method with Broadcom for retrieving the actual lane count + if (status != SAI_STATUS_BUFFER_OVERFLOW) + { + SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get hardware lane count for port RID:0x%" PRIx64 ", status:%d", + port_rid, status); + return; + } + laneCount = attr.value.u32list.count; From 117aa7226b8b552d9dc02305716a70cfb3616868 Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Thu, 18 Jun 2026 18:36:27 +0000 Subject: [PATCH 04/11] Cast revised handling to other code path Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 9c3e53a1d6..02605ef394 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -2376,8 +2376,8 @@ class PortPhySerdesAttrContext : public AttrContextget( Base::m_objectType, @@ -2385,25 +2385,22 @@ class PortPhySerdesAttrContext : public AttrContext Date: Thu, 18 Jun 2026 18:42:29 +0000 Subject: [PATCH 05/11] Further casts Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 02605ef394..ed1ba15317 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -2231,29 +2231,28 @@ class PortPhySerdesAttrContext : public AttrContextget(Base::m_objectType, port_serdes_rid, 1, &attr); - - if (status == SAI_STATUS_SUCCESS) - { - port_rid = attr.value.oid; - return true; - } - else if (status == SAI_STATUS_OBJECT_IN_USE and tries < 2) - { - // SAI object is busy - retry in 10ms - SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting port RID for port serdes RID:0x%" PRIx64 "...", - port_serdes_rid); - std::this_thread::sleep_for(chrono::milliseconds(10)); - } - else + status = Base::m_vendorSai->get(Base::m_objectType, port_serdes_rid, 1, &attr); + if (status != SAI_STATUS_OBJECT_IN_USE) { - SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get port RID for port serdes RID:0x%" PRIx64 ", status:%d", - port_serdes_rid, status); break; } + // SAI object is busy - retry in 10ms + SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting port RID for port serdes RID:0x%" PRIx64 "...", + port_serdes_rid); + std::this_thread::sleep_for(chrono::milliseconds(10)); + } + + if (status == SAI_STATUS_SUCCESS) + { + port_rid = attr.value.oid; + return true; } + + SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get port RID for port serdes RID:0x%" PRIx64 ", status:%d", + port_serdes_rid, status); return false; } @@ -2330,7 +2329,7 @@ class PortPhySerdesAttrContext : public AttrContextget(SAI_OBJECT_TYPE_PORT, port_rid, 1, &attr); + status = Base::m_vendorSai->get(SAI_OBJECT_TYPE_PORT, port_rid, 1, &attr); if (status != SAI_STATUS_OBJECT_IN_USE) { break; @@ -2341,6 +2340,7 @@ class PortPhySerdesAttrContext : public AttrContextget( + status = Base::m_vendorSai->get( Base::m_objectType, port_serdes_rid, 1, @@ -2389,6 +2389,7 @@ class PortPhySerdesAttrContext : public AttrContext Date: Thu, 18 Jun 2026 19:03:43 +0000 Subject: [PATCH 06/11] Fix whiteline and typo Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index ed1ba15317..e577a94c47 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -2231,7 +2231,7 @@ class PortPhySerdesAttrContext : public AttrContextget(Base::m_objectType, port_serdes_rid, 1, &attr); @@ -2350,7 +2350,6 @@ class PortPhySerdesAttrContext : public AttrContext Date: Sat, 4 Jul 2026 01:25:53 +0000 Subject: [PATCH 07/11] Switch back to retries but with 5 retries Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 106 +++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 52 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index e577a94c47..9b99f5fbd2 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -2231,28 +2231,29 @@ class PortPhySerdesAttrContext : public AttrContextget(Base::m_objectType, port_serdes_rid, 1, &attr); - if (status != SAI_STATUS_OBJECT_IN_USE) + sai_status_t status = Base::m_vendorSai->get(Base::m_objectType, port_serdes_rid, 1, &attr); + + if (status == SAI_STATUS_SUCCESS) + { + port_rid = attr.value.oid; + return true; + } + else if (status == SAI_STATUS_OBJECT_IN_USE and tries < 2) { + // SAI object is busy - retry in 10ms + SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting port RID for port serdes RID:0x%" PRIx64 "...", + port_serdes_rid); + std::this_thread::sleep_for(chrono::milliseconds(10)); + } + else + { + SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get port RID for port serdes RID:0x%" PRIx64 ", status:%d", + port_serdes_rid, status); break; } - // SAI object is busy - retry in 10ms - SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting port RID for port serdes RID:0x%" PRIx64 "...", - port_serdes_rid); - std::this_thread::sleep_for(chrono::milliseconds(10)); } - - if (status == SAI_STATUS_SUCCESS) - { - port_rid = attr.value.oid; - return true; - } - - SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get port RID for port serdes RID:0x%" PRIx64 ", status:%d", - port_serdes_rid, status); return false; } @@ -2326,28 +2327,27 @@ class PortPhySerdesAttrContext : public AttrContextget(SAI_OBJECT_TYPE_PORT, port_rid, 1, &attr); - if (status != SAI_STATUS_OBJECT_IN_USE) - { + sai_status_t status = Base::m_vendorSai->get(SAI_OBJECT_TYPE_PORT, port_rid, 1, &attr); + + // The SAI status expected is SAI_STATUS_BUFFER_OVERFLOW since we pass in a nullptr + // This is the agreed method with Broadcom for retrieving the actual lane count + if (status == SAI_STATUS_BUFFER_OVERFLOW) break; + else if (status == SAI_STATUS_OBJECT_IN_USE && tries < 2) + { + // SAI object is busy - retry in 10ms + SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting hardware lane count for port RID:0x%" PRIx64 "...", + port_rid); + std::this_thread::sleep_for(chrono::milliseconds(10)); + } + else + { + SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get hardware lane count for port RID:0x%" PRIx64 ", status:%d", + port_rid, status); + return; } - - // SAI object is busy - retry in 10ms - SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting hardware lane count for port RID:0x%" PRIx64 "...", - port_rid); - std::this_thread::sleep_for(chrono::milliseconds(10)); - } - - // The SAI status expected is SAI_STATUS_BUFFER_OVERFLOW since we pass in a nullptr - // This is the agreed method with Broadcom for retrieving the actual lane count - if (status != SAI_STATUS_BUFFER_OVERFLOW) - { - SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get hardware lane count for port RID:0x%" PRIx64 ", status:%d", - port_rid, status); - return; } laneCount = attr.value.u32list.count; @@ -2375,32 +2375,34 @@ class PortPhySerdesAttrContext : public AttrContextget( + sai_status_t status = Base::m_vendorSai->get( Base::m_objectType, port_serdes_rid, 1, &attr); - if (status != SAI_STATUS_OBJECT_IN_USE) - { + if (status == SAI_STATUS_SUCCESS) break; + else if (status == SAI_STATUS_OBJECT_IN_USE && tries < 2) + { + // SAI object is busy - retry in 10ms + SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting port serdes count attr %s for port_serdes RID:0x%" PRIx64 "...", + sai_serialize_port_serdes_attr(attrId).c_str(), port_serdes_rid); + std::this_thread::sleep_for(chrono::milliseconds(10)); + } + else + { + SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get port serdes count attr %s for port_serdes RID:0x%" PRIx64 ", status:%d", + sai_serialize_port_serdes_attr(attrId).c_str(), port_serdes_rid, status); + failed = true; + continue; } - - // SAI object is busy - retry in 10ms - SWSS_LOG_WARN("PORT_PHY_SERDES_ATTR: SAI object in use, retry getting port serdes count attr %s for port_serdes RID:0x%" PRIx64 "...", - sai_serialize_port_serdes_attr(attrId).c_str(), port_serdes_rid); - std::this_thread::sleep_for(chrono::milliseconds(10)); } - - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("PORT_PHY_SERDES_ATTR: Failed to get port serdes count attr %s for port_serdes RID:0x%" PRIx64 ", status:%d", - sai_serialize_port_serdes_attr(attrId).c_str(), port_serdes_rid, status); + if (failed) continue; - } count = attr.value.u32; From faf8c21a9afc13c687833d7524a345bf1c14dc06 Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Tue, 7 Jul 2026 20:52:08 +0000 Subject: [PATCH 08/11] bump tries count, make code more easy to understand Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 9b99f5fbd2..d047ccbe06 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -2231,7 +2231,7 @@ class PortPhySerdesAttrContext : public AttrContextget(Base::m_objectType, port_serdes_rid, 1, &attr); @@ -2240,7 +2240,7 @@ class PortPhySerdesAttrContext : public AttrContextget(SAI_OBJECT_TYPE_PORT, port_rid, 1, &attr); @@ -2335,7 +2335,7 @@ class PortPhySerdesAttrContext : public AttrContextget( Base::m_objectType, @@ -2386,7 +2386,7 @@ class PortPhySerdesAttrContext : public AttrContext Date: Tue, 14 Jul 2026 00:13:30 +0000 Subject: [PATCH 09/11] Add unit test for retry logic Signed-off-by: Justin Wong --- unittest/syncd/TestPortPhySerdesAttr.cpp | 328 +++++++++++++++++++++++ 1 file changed, 328 insertions(+) diff --git a/unittest/syncd/TestPortPhySerdesAttr.cpp b/unittest/syncd/TestPortPhySerdesAttr.cpp index 47aa150f9f..f37f0bd204 100644 --- a/unittest/syncd/TestPortPhySerdesAttr.cpp +++ b/unittest/syncd/TestPortPhySerdesAttr.cpp @@ -356,3 +356,331 @@ TEST_F(TestPortPhySerdesAttr, CollectDataAndValidateCountersDB) flexCounter->removeCounter(testPortSerdesOid); } +TEST_F(TestPortPhySerdesAttr, RetryOnObjectInUseThenSucceed) +{ + int portIdCalls = 0; + int hwLaneListCalls = 0; + int txFirCountCalls = 0; + const int FAIL_COUNT = 2; + + sai->mock_get = [&](sai_object_type_t object_type, + sai_object_id_t object_id, + uint32_t attr_count, + sai_attribute_t *attr_list) -> sai_status_t + { + if (object_type == SAI_OBJECT_TYPE_PORT_SERDES) { + for (uint32_t i = 0; i < attr_count; i++) { + switch (attr_list[i].id) { + case SAI_PORT_SERDES_ATTR_PORT_ID: + portIdCalls++; + if (portIdCalls <= FAIL_COUNT) + return SAI_STATUS_OBJECT_IN_USE; + attr_list[i].value.oid = 0x1000000000001; + break; + + case SAI_PORT_SERDES_ATTR_TX_FIR_COUNT: + txFirCountCalls++; + if (txFirCountCalls <= FAIL_COUNT) + return SAI_STATUS_OBJECT_IN_USE; + attr_list[i].value.u32 = TEST_TAP_COUNT; + break; + + case SAI_PORT_SERDES_ATTR_RX_VGA: + if (attr_list[i].value.u32list.list == nullptr) { + attr_list[i].value.u32list.count = TEST_LANE_COUNT; + return SAI_STATUS_BUFFER_OVERFLOW; + } else { + uint32_t count = attr_list[i].value.u32list.count; + for (uint32_t lane = 0; lane < count && lane < TEST_LANE_COUNT; lane++) { + attr_list[i].value.u32list.list[lane] = 177 + lane; + } + attr_list[i].value.u32list.count = std::min(count, TEST_LANE_COUNT); + } + break; + + case SAI_PORT_SERDES_ATTR_TX_FIR_TAPS_LIST: + if (attr_list[i].value.portserdestaps.list == nullptr) { + attr_list[i].value.portserdestaps.count = TEST_TAP_COUNT; + return SAI_STATUS_BUFFER_OVERFLOW; + } else { + uint32_t tap_count = attr_list[i].value.portserdestaps.count; + for (uint32_t tap_idx = 0; tap_idx < tap_count && tap_idx < TEST_TAP_COUNT; tap_idx++) { + if (attr_list[i].value.portserdestaps.list[tap_idx].list == nullptr) { + attr_list[i].value.portserdestaps.list[tap_idx].count = TEST_LANE_COUNT; + } else { + uint32_t lane_count = attr_list[i].value.portserdestaps.list[tap_idx].count; + for (uint32_t lane = 0; lane < lane_count && lane < TEST_LANE_COUNT; lane++) { + int32_t base_value = -23 + (tap_idx * 10); + attr_list[i].value.portserdestaps.list[tap_idx].list[lane] = base_value + lane; + } + attr_list[i].value.portserdestaps.list[tap_idx].count = std::min(lane_count, TEST_LANE_COUNT); + } + } + attr_list[i].value.portserdestaps.count = std::min(tap_count, TEST_TAP_COUNT); + } + break; + + default: + return SAI_STATUS_NOT_SUPPORTED; + } + } + return SAI_STATUS_SUCCESS; + } else if (object_type == SAI_OBJECT_TYPE_PORT) { + for (uint32_t i = 0; i < attr_count; i++) { + if (attr_list[i].id == SAI_PORT_ATTR_HW_LANE_LIST) { + hwLaneListCalls++; + if (hwLaneListCalls <= FAIL_COUNT) + return SAI_STATUS_OBJECT_IN_USE; + if (attr_list[i].value.u32list.list == nullptr) { + attr_list[i].value.u32list.count = TEST_LANE_COUNT; + return SAI_STATUS_BUFFER_OVERFLOW; + } else { + uint32_t count = attr_list[i].value.u32list.count; + for (uint32_t lane = 0; lane < count && lane < TEST_LANE_COUNT; lane++) { + attr_list[i].value.u32list.list[lane] = lane; + } + attr_list[i].value.u32list.count = std::min(count, TEST_LANE_COUNT); + return SAI_STATUS_SUCCESS; + } + } + } + } + return SAI_STATUS_INVALID_PARAMETER; + }; + + swss::DBConnector db("COUNTERS_DB", 0); + swss::Table portSerdesIdToPortIdTable(&db, "COUNTERS_PORT_SERDES_ID_TO_PORT_ID_MAP"); + portSerdesIdToPortIdTable.hset("", toOid(testPortSerdesOid), toOid(testPortOid)); + + test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT_SERDES); + + vector portSerdesAttrValues; + portSerdesAttrValues.emplace_back(PORT_PHY_SERDES_ATTR_ID_LIST, + "SAI_PORT_SERDES_ATTR_RX_VGA,SAI_PORT_SERDES_ATTR_TX_FIR_TAPS_LIST"); + + flexCounter->addCounter(testPortSerdesOid, testPortSerdesRid, portSerdesAttrValues); + + EXPECT_EQ(portIdCalls, FAIL_COUNT + 1); + EXPECT_EQ(hwLaneListCalls, FAIL_COUNT + 1); + EXPECT_EQ(txFirCountCalls, FAIL_COUNT + 1); + + vector pluginValues; + pluginValues.emplace_back(POLL_INTERVAL_FIELD, "1000"); + pluginValues.emplace_back(FLEX_COUNTER_STATUS_FIELD, "enable"); + pluginValues.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ); + flexCounter->addCounterPlugin(pluginValues); + + usleep(1000 * 1050); + + swss::RedisPipeline pipeline(&db); + swss::Table portPhyAttrTable(&pipeline, PORT_PHY_ATTR_TABLE, false); + + std::string expectedKey = toOid(testPortOid); + std::string rxVgaValue; + bool found = portPhyAttrTable.hget(expectedKey, "rx_vga", rxVgaValue); + EXPECT_TRUE(found) << "rx_vga not found after retry - data collection should succeed"; + + flexCounter->removeCounter(testPortSerdesOid); +} + +TEST_F(TestPortPhySerdesAttr, RetryExhaustedGetPortRid) +{ + int portIdCalls = 0; + + sai->mock_get = [&](sai_object_type_t object_type, + sai_object_id_t object_id, + uint32_t attr_count, + sai_attribute_t *attr_list) -> sai_status_t + { + if (object_type == SAI_OBJECT_TYPE_PORT_SERDES) { + for (uint32_t i = 0; i < attr_count; i++) { + if (attr_list[i].id == SAI_PORT_SERDES_ATTR_PORT_ID) { + portIdCalls++; + return SAI_STATUS_OBJECT_IN_USE; + } + + if (attr_list[i].id == SAI_PORT_SERDES_ATTR_TX_FIR_COUNT) { + attr_list[i].value.u32 = TEST_TAP_COUNT; + break; + } + } + return SAI_STATUS_SUCCESS; + } + return SAI_STATUS_SUCCESS; + }; + + swss::DBConnector db("COUNTERS_DB", 0); + swss::Table portSerdesIdToPortIdTable(&db, "COUNTERS_PORT_SERDES_ID_TO_PORT_ID_MAP"); + portSerdesIdToPortIdTable.hset("", toOid(testPortSerdesOid), toOid(testPortOid)); + + test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT_SERDES); + + vector portSerdesAttrValues; + portSerdesAttrValues.emplace_back(PORT_PHY_SERDES_ATTR_ID_LIST, "SAI_PORT_SERDES_ATTR_RX_VGA"); + + flexCounter->addCounter(testPortSerdesOid, testPortSerdesRid, portSerdesAttrValues); + + EXPECT_EQ(portIdCalls, 5) << "All 5 retry attempts should have been made"; + + vector pluginValues; + pluginValues.emplace_back(POLL_INTERVAL_FIELD, "1000"); + pluginValues.emplace_back(FLEX_COUNTER_STATUS_FIELD, "enable"); + pluginValues.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ); + flexCounter->addCounterPlugin(pluginValues); + + swss::RedisPipeline pipeline(&db); + swss::Table portPhyAttrTable(&pipeline, PORT_PHY_ATTR_TABLE, false); + + std::string expectedKey = toOid(testPortOid); + portPhyAttrTable.del(expectedKey); + + usleep(1000 * 1050); + + std::string rxVgaValue; + bool found = portPhyAttrTable.hget(expectedKey, "rx_vga", rxVgaValue); + EXPECT_FALSE(found) << "No data should appear when port RID lookup exhausted retries"; + + flexCounter->removeCounter(testPortSerdesOid); +} + +TEST_F(TestPortPhySerdesAttr, RetryExhaustedLaneCount) +{ + int hwLaneListCalls = 0; + + sai->mock_get = [&](sai_object_type_t object_type, + sai_object_id_t object_id, + uint32_t attr_count, + sai_attribute_t *attr_list) -> sai_status_t + { + if (object_type == SAI_OBJECT_TYPE_PORT_SERDES) { + for (uint32_t i = 0; i < attr_count; i++) { + if (attr_list[i].id == SAI_PORT_SERDES_ATTR_PORT_ID) { + attr_list[i].value.oid = 0x1000000000001; + break; + } + if (attr_list[i].id == SAI_PORT_SERDES_ATTR_TX_FIR_COUNT) { + attr_list[i].value.u32 = TEST_TAP_COUNT; + break; + } + } + return SAI_STATUS_SUCCESS; + } else if (object_type == SAI_OBJECT_TYPE_PORT) { + for (uint32_t i = 0; i < attr_count; i++) { + if (attr_list[i].id == SAI_PORT_ATTR_HW_LANE_LIST) { + hwLaneListCalls++; + return SAI_STATUS_OBJECT_IN_USE; + } + } + } + return SAI_STATUS_SUCCESS; + }; + + swss::DBConnector db("COUNTERS_DB", 0); + swss::Table portSerdesIdToPortIdTable(&db, "COUNTERS_PORT_SERDES_ID_TO_PORT_ID_MAP"); + portSerdesIdToPortIdTable.hset("", toOid(testPortSerdesOid), toOid(testPortOid)); + + test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT_SERDES); + + vector portSerdesAttrValues; + portSerdesAttrValues.emplace_back(PORT_PHY_SERDES_ATTR_ID_LIST, "SAI_PORT_SERDES_ATTR_RX_VGA"); + + flexCounter->addCounter(testPortSerdesOid, testPortSerdesRid, portSerdesAttrValues); + + EXPECT_EQ(hwLaneListCalls, 5) << "All 5 retry attempts should have been made"; + + vector pluginValues; + pluginValues.emplace_back(POLL_INTERVAL_FIELD, "1000"); + pluginValues.emplace_back(FLEX_COUNTER_STATUS_FIELD, "enable"); + pluginValues.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ); + flexCounter->addCounterPlugin(pluginValues); + + swss::RedisPipeline pipeline(&db); + swss::Table portPhyAttrTable(&pipeline, PORT_PHY_ATTR_TABLE, false); + + std::string expectedKey = toOid(testPortOid); + portPhyAttrTable.del(expectedKey); + + usleep(1000 * 1050); + + std::string rxVgaValue; + bool found = portPhyAttrTable.hget(expectedKey, "rx_vga", rxVgaValue); + EXPECT_FALSE(found) << "No data should appear when lane count lookup exhausted retries"; + + flexCounter->removeCounter(testPortSerdesOid); +} + +TEST_F(TestPortPhySerdesAttr, RetryExhaustedTapsCount) +{ + int txFirCountCalls = 0; + + sai->mock_get = [&](sai_object_type_t object_type, + sai_object_id_t object_id, + uint32_t attr_count, + sai_attribute_t *attr_list) -> sai_status_t + { + if (object_type == SAI_OBJECT_TYPE_PORT_SERDES) { + for (uint32_t i = 0; i < attr_count; i++) { + if (attr_list[i].id == SAI_PORT_SERDES_ATTR_PORT_ID) { + attr_list[i].value.oid = 0x1000000000001; + break; + } + if (attr_list[i].id == SAI_PORT_SERDES_ATTR_TX_FIR_COUNT) { + txFirCountCalls++; + return SAI_STATUS_OBJECT_IN_USE; + } + } + return SAI_STATUS_SUCCESS; + } else if (object_type == SAI_OBJECT_TYPE_PORT) { + for (uint32_t i = 0; i < attr_count; i++) { + if (attr_list[i].id == SAI_PORT_ATTR_HW_LANE_LIST) { + if (attr_list[i].value.u32list.list == nullptr) { + attr_list[i].value.u32list.count = TEST_LANE_COUNT; + return SAI_STATUS_BUFFER_OVERFLOW; + } else { + uint32_t count = attr_list[i].value.u32list.count; + for (uint32_t lane = 0; lane < count && lane < TEST_LANE_COUNT; lane++) { + attr_list[i].value.u32list.list[lane] = lane; + } + attr_list[i].value.u32list.count = std::min(count, TEST_LANE_COUNT); + return SAI_STATUS_SUCCESS; + } + } + } + } + return SAI_STATUS_SUCCESS; + }; + + swss::DBConnector db("COUNTERS_DB", 0); + swss::Table portSerdesIdToPortIdTable(&db, "COUNTERS_PORT_SERDES_ID_TO_PORT_ID_MAP"); + portSerdesIdToPortIdTable.hset("", toOid(testPortSerdesOid), toOid(testPortOid)); + + test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT_SERDES); + + vector portSerdesAttrValues; + portSerdesAttrValues.emplace_back(PORT_PHY_SERDES_ATTR_ID_LIST, "SAI_PORT_SERDES_ATTR_RX_VGA"); + + flexCounter->addCounter(testPortSerdesOid, testPortSerdesRid, portSerdesAttrValues); + + EXPECT_EQ(txFirCountCalls, 5) << "All 5 retry attempts should have been made"; + + vector pluginValues; + pluginValues.emplace_back(POLL_INTERVAL_FIELD, "1000"); + pluginValues.emplace_back(FLEX_COUNTER_STATUS_FIELD, "enable"); + pluginValues.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ); + flexCounter->addCounterPlugin(pluginValues); + + swss::RedisPipeline pipeline(&db); + swss::Table portPhyAttrTable(&pipeline, PORT_PHY_ATTR_TABLE, false); + + std::string expectedKey = toOid(testPortOid); + portPhyAttrTable.del(expectedKey); + + usleep(1000 * 1050); + + std::string rxVgaValue; + bool found = portPhyAttrTable.hget(expectedKey, "rx_vga", rxVgaValue); + EXPECT_FALSE(found) << "No data should appear when taps count lookup exhausted retries"; + + flexCounter->removeCounter(testPortSerdesOid); +} + From a33c0e803348a6958042a3958a0af1271a6910cc Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Tue, 14 Jul 2026 01:06:02 +0000 Subject: [PATCH 10/11] added comments for new test cases Signed-off-by: Justin Wong --- unittest/syncd/TestPortPhySerdesAttr.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/unittest/syncd/TestPortPhySerdesAttr.cpp b/unittest/syncd/TestPortPhySerdesAttr.cpp index f37f0bd204..f14a81604d 100644 --- a/unittest/syncd/TestPortPhySerdesAttr.cpp +++ b/unittest/syncd/TestPortPhySerdesAttr.cpp @@ -356,6 +356,13 @@ TEST_F(TestPortPhySerdesAttr, CollectDataAndValidateCountersDB) flexCounter->removeCounter(testPortSerdesOid); } +/** + * Verify transient SAI_STATUS_OBJECT_IN_USE errors are mitigated with retries + * and results in a good syncd state such that RX_VGA entries appears in + * PORT_PHY_ATTR_TABLE after retries. + * The test logic bounds the number of retries to 5, hence testing with 2 fails + + * 1 successful final try in this test case. + */ TEST_F(TestPortPhySerdesAttr, RetryOnObjectInUseThenSucceed) { int portIdCalls = 0; @@ -483,6 +490,11 @@ TEST_F(TestPortPhySerdesAttr, RetryOnObjectInUseThenSucceed) flexCounter->removeCounter(testPortSerdesOid); } +/** + * Verify the failure is acknowledged after failing all retry attempts. + * syncd should end in a bad state where no data appears in PORT_PHY_ATTR_TABLE + * since the port RID mapping was never established. + */ TEST_F(TestPortPhySerdesAttr, RetryExhaustedGetPortRid) { int portIdCalls = 0; @@ -543,6 +555,11 @@ TEST_F(TestPortPhySerdesAttr, RetryExhaustedGetPortRid) flexCounter->removeCounter(testPortSerdesOid); } +/** + * Verify the failure is acknowledged after failing all retry attempts. + * syncd should end in a bad state where no data appears in PORT_PHY_ATTR_TABLE + * since the lane count needed was never determined. + */ TEST_F(TestPortPhySerdesAttr, RetryExhaustedLaneCount) { int hwLaneListCalls = 0; @@ -609,6 +626,12 @@ TEST_F(TestPortPhySerdesAttr, RetryExhaustedLaneCount) flexCounter->removeCounter(testPortSerdesOid); } +/** + * Verify the failure is acknowledged after failing all retry attempts. + * syncd should end in a bad state where no data appears in PORT_PHY_ATTR_TABLE + * since the tap count needed for TX_FIR_TAPS_LIST initialization was never + * determined. + */ TEST_F(TestPortPhySerdesAttr, RetryExhaustedTapsCount) { int txFirCountCalls = 0; From 8aa4ae924968b9b2dda1e1359c7e59010808487e Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Tue, 14 Jul 2026 16:45:32 +0000 Subject: [PATCH 11/11] fix typo Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index d047ccbe06..52314c5d11 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -2398,7 +2398,7 @@ class PortPhySerdesAttrContext : public AttrContext