[code sync] Merge code from sonic-net/sonic-sairedis:202511 to 202603

syncd/FlexCounter.cpp

@@ -1844,7 +1844,7 @@ class PortPhyAttrContext : public AttrContext<sai_port_attr_t, PortPhyAttributeD
         }
     }
 
-    void initAttrData(
+    bool initAttrData(
         sai_object_id_t rid,
         sai_attribute_t *attr,
         PortPhyAttributeData* data)
@@ -1854,15 +1854,15 @@ class PortPhyAttrContext : public AttrContext<sai_port_attr_t, PortPhyAttributeD
         if (!attr || !data)
         {
             SWSS_LOG_ERROR("PORT_PHY_ATTR: Invalid input params : attr : %p, data : %p", attr, data);
-            return;
+            return false;
         }
 
         auto outer_it = m_portLaneCountMap.find(rid);
         if (outer_it == m_portLaneCountMap.end())
         {
           SWSS_LOG_ERROR("PORT_PHY_ATTR: Rid:0x%" PRIx64 " not found in m_portLaneCountMap, attr->id : %d",
                          rid, attr->id);
-          return;
+          return false;
         }
 
         const auto &attrLaneCountMap = outer_it->second;
@@ -1871,7 +1871,7 @@ class PortPhyAttrContext : public AttrContext<sai_port_attr_t, PortPhyAttributeD
         {
           SWSS_LOG_ERROR("PORT_PHY_ATTR: Attr Id(%d) not found in m_portLaneCountMap[Rid:0x%" PRIx64 "]",
                          attr->id, rid);
-          return;
+          return false;
         }
 
         auto portLaneCount = inner_it->second;
@@ -1883,23 +1883,23 @@ class PortPhyAttrContext : public AttrContext<sai_port_attr_t, PortPhyAttributeD
                 data->rxSignalDetectData.resize(portLaneCount);
                 attr->value.portlanelatchstatuslist.count = portLaneCount;
                 attr->value.portlanelatchstatuslist.list = data->rxSignalDetectData.data();
-                break;
+                return true;
 
             case SAI_PORT_ATTR_FEC_ALIGNMENT_LOCK:
                 data->fecAlignmentLockData.resize(portLaneCount);
                 attr->value.portlanelatchstatuslist.count = portLaneCount;
                 attr->value.portlanelatchstatuslist.list = data->fecAlignmentLockData.data();
-                break;
+                return true;
 
             case SAI_PORT_ATTR_RX_SNR:
                 data->rxSnrData.resize(portLaneCount);
                 attr->value.portsnrlist.count = portLaneCount;
                 attr->value.portsnrlist.list = data->rxSnrData.data();
-                break;
+                return true;
 
             default:
                 SWSS_LOG_ERROR("PORT_PHY_ATTR: initAttrData: Unsupported attr-id : %d", attr->id);
-                break;
+                return false;
         }
     }
 
@@ -1981,10 +1981,22 @@ class PortPhyAttrContext : public AttrContext<sai_port_attr_t, PortPhyAttributeD
             SWSS_LOG_DEBUG("Collecting %zu port attributes for VID 0x%" PRIx64 ", RID:0x%" PRIx64,
                            attrIds.size(), vid, rid);
 
+            bool attrDataInitialized = true;
             for (size_t i = 0; i < attrIds.size(); i++)
             {
                 attrs[i].id = attrIds[i];
-                initAttrData(rid, &attrs[i], &attrData);
+                if (!initAttrData(rid, &attrs[i], &attrData))
+                {
+                    SWSS_LOG_WARN("PORT_PHY_ATTR: Failed to initialize attribute %d for RID:0x%" PRIx64 ", skipping object",
+                                  attrIds[i], rid);
+                    attrDataInitialized = false;
+                    break;
+                }
+            }
+
+            if (!attrDataInitialized)
+            {
+                continue;
             }
 
             // Collect attributes from SAI

tests/aspell.en.pws

@@ -494,3 +494,4 @@ ipfix
 SNR
 tparam
 SIGSEGV
+CX

unittest/syncd/TestPortPhyAttr.cpp

@@ -265,3 +265,101 @@ TEST_F(TestPortPhyAttr, CollectDataAndValidateCountersDB)
 
     flexCounter->removeCounter(testPortOid);
 }
+
+/**
+ * Test that collectData() gracefully skips objects when initAttrData() fails.
+ *
+ * This simulates the Broadcom 7260CX3 scenario where SAI does not support
+ * SAI_PORT_ATTR_RX_SIGNAL_DETECT ΓÇö the lane count query returns NOT_SUPPORTED
+ * instead of BUFFER_OVERFLOW, so m_portLaneCountMap has no entry for the
+ * attribute, and initAttrData() returns false.
+ *
+ * Before the fix, collectData() would ignore the initAttrData() return value
+ * and call sai_get() anyway, which would fail and emit ERR syslog every 10s.
+ * After the fix, collectData() skips the object entirely.
+ *
+ * See: https://github.com/sonic-net/sonic-mgmt/issues/24023
+ */
+TEST_F(TestPortPhyAttr, CollectDataSkipsWhenInitAttrDataFails)
+{
+    // Use a different port OID to avoid picking up stale Redis data
+    // from the previous test (CollectDataAndValidateCountersDB).
+    sai_object_id_t failPortOid = 0x1000000000099;
+    sai_object_id_t failPortRid = 0x1000000000099;
+
+    int getCallCount = 0;
+
+    // Mock SAI: return NOT_SUPPORTED for the lane count query
+    // This means m_portLaneCountMap will NOT have an entry for this RID/attr,
+    // so initAttrData() will return false when collectData() calls it.
+    sai->mock_get = [&getCallCount](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) {
+            return SAI_STATUS_INVALID_PARAMETER;
+        }
+
+        // Track calls after addObject's lane count query phase
+        getCallCount++;
+
+        // Return NOT_SUPPORTED for the lane count query (attr_count == 1)
+        // This simulates Broadcom SAI on 7260CX3 not supporting RX_SIGNAL_DETECT
+        if (attr_count == 1) {
+            return SAI_STATUS_NOT_SUPPORTED;
+        }
+
+        // If collectData() reaches sai_get with multiple attrs, that's a bug
+        // The fix should prevent this from happening
+        return SAI_STATUS_FAILURE;
+    };
+
+    vector<swss::FieldValueTuple> portPhyAttrValues;
+    std::string attrIds = "SAI_PORT_ATTR_RX_SIGNAL_DETECT,SAI_PORT_ATTR_FEC_ALIGNMENT_LOCK,SAI_PORT_ATTR_RX_SNR";
+    portPhyAttrValues.emplace_back(PORT_PHY_ATTR_ID_LIST, attrIds);
+
+    test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT);
+
+    // addObject will call updatePortLaneCountMap which queries SAI for lane count.
+    // Since SAI returns NOT_SUPPORTED, m_portLaneCountMap will be empty.
+    flexCounter->addCounter(failPortOid, failPortRid, portPhyAttrValues);
+
+    int getCallsAfterAdd = getCallCount;
+
+    vector<swss::FieldValueTuple> 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); // 1.05 seconds - one poll cycle
+
+    // Verify that collectData did NOT make additional sai_get calls
+    // beyond what addObject's updatePortLaneCountMap already made.
+    // If initAttrData fails, collectData should skip the object entirely.
+    EXPECT_EQ(getCallCount, getCallsAfterAdd)
+        << "collectData should not call sai_get when initAttrData fails. "
+        << "Additional sai_get calls indicate the initAttrData return value was ignored.";
+
+    // Verify no data was written to COUNTERS_DB
+    swss::DBConnector db("COUNTERS_DB", 0);
+    swss::RedisPipeline pipeline(&db);
+    swss::Table countersTable(&pipeline, PORT_PHY_ATTR_TABLE, false);
+
+    std::string expectedKey = toOid(failPortOid);
+
+    std::string rxSignalDetectValue;
+    bool found = countersTable.hget(expectedKey, "phy_rx_signal_detect", rxSignalDetectValue);
+    EXPECT_FALSE(found) << "phy_rx_signal_detect should NOT be in COUNTERS_DB when initAttrData fails";
+
+    std::string fecAlignmentValue;
+    found = countersTable.hget(expectedKey, "pcs_fec_lane_alignment_lock", fecAlignmentValue);
+    EXPECT_FALSE(found) << "pcs_fec_lane_alignment_lock should NOT be in COUNTERS_DB when initAttrData fails";
+
+    std::string rxSnrValue;
+    found = countersTable.hget(expectedKey, "rx_snr", rxSnrValue);
+    EXPECT_FALSE(found) << "rx_snr should NOT be in COUNTERS_DB when initAttrData fails";
+
+    flexCounter->removeCounter(failPortOid);
+}