From 27a828e1ed06cfd92bb495bb15bc23be997e8f70 Mon Sep 17 00:00:00 2001 From: mssonicbld <79238446+mssonicbld@users.noreply.github.com> Date: Mon, 4 May 2026 13:13:22 +0800 Subject: [PATCH] [syncd]: Fall back to TCP connection when unix socket path is not defined (#1877) ## Description When FlexCounter connects to COUNTERS_DB, it previously always used a unix socket connection. If the target database has no unix socket path configured, this would fail (e.g. syncd is running on a DPU and the target DB is in the remote redis instance on the NPU, the remote redis instance is only reachable via TCP connection) ## Changes - At `FlexCounter` construction time, check `SonicDBConfig::getDbSock()` for the target DB. If the socket path is empty, set `m_isTcpConn = true`. - Pass `m_isTcpConn` to all 6 `DBConnector` instantiation sites across `FlexCounter` and its counter context classes (`PortPhyAttrContext`, `PortPhySerdesAttrContext`, `DashMeterCounterContext`). Signed-off-by: Sonic Build Admin --- syncd/FlexCounter.cpp | 41 +++++++---- syncd/FlexCounter.h | 2 + unittest/syncd/TestFlexCounter.cpp | 114 ++++++++++++++++++++++++++++- 3 files changed, 139 insertions(+), 18 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index c7f7700f..14e57487 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -14,6 +14,7 @@ #include "swss/redisapi.h" #include "swss/tokenize.h" +#include "swss/dbconnector.h" using namespace syncd; using namespace std; @@ -1762,9 +1763,11 @@ class PortPhyAttrContext : public AttrContext> m_portLaneCountMap; // Map: [VID][attr_id][lane_number] -> metadata std::map>> m_laneMetadata; @@ -2192,9 +2196,11 @@ class PortPhySerdesAttrContext : public AttrContext m_attrAliases; std::string m_dbCounters; + bool m_isTcpConn; std::map m_portSerdesIdToPortIdMap; std::map m_portSerdesIdToLaneCountMap; std::map> m_portSerdesTapsCountMap; @@ -2623,8 +2630,9 @@ class DashMeterCounterContext : public BaseCounterContext _In_ const std::string &name, _In_ const std::string &instance, _In_ sairedis::SaiInterface *vendor_sai, - _In_ std::string dbCounters): - BaseCounterContext(name, instance), m_dbCounters(dbCounters), m_vendorSai(vendor_sai) + _In_ std::string dbCounters, + _In_ bool isTcpConn): + BaseCounterContext(name, instance), m_dbCounters(dbCounters), m_isTcpConn(isTcpConn), m_vendorSai(vendor_sai) { SWSS_LOG_ENTER(); } @@ -2698,7 +2706,7 @@ class DashMeterCounterContext : public BaseCounterContext return; } // delete all meter bucket stats for this object from counters DB - swss::DBConnector db(m_dbCounters, 0); + swss::DBConnector db(m_dbCounters, 0, m_isTcpConn); swss::RedisPipeline pipeline(&db); swss::Table countersTable(&pipeline, COUNTERS_TABLE, true); for (const auto& object_key: it->second.object_keys) { @@ -2993,6 +3001,7 @@ class DashMeterCounterContext : public BaseCounterContext std::vector m_supportedMeterCounters; sai_object_type_t m_objectType = (sai_object_type_t) SAI_OBJECT_TYPE_METER_BUCKET_ENTRY; std::string m_dbCounters; + bool m_isTcpConn; sairedis::SaiInterface *m_vendorSai; sai_stats_mode_t m_groupStatsMode = SAI_STATS_MODE_READ; sai_object_id_t m_switchId = 0UL; @@ -3018,6 +3027,8 @@ FlexCounter::FlexCounter( m_enable = false; m_isDiscarded = false; + m_isTcpConn = swss::SonicDBConfig::getDbSock(dbCounters).empty(); + startFlexCounterThread(); } @@ -3089,7 +3100,7 @@ void FlexCounter::removeDataFromCountersDB( _In_ const std::string &ratePrefix) { SWSS_LOG_ENTER(); - swss::DBConnector db(m_dbCounters, 0); + swss::DBConnector db(m_dbCounters, 0, m_isTcpConn); swss::RedisPipeline pipeline(&db); swss::Table countersTable(&pipeline, COUNTERS_TABLE, false); @@ -3361,15 +3372,15 @@ std::shared_ptr FlexCounter::createCounterContext( } else if (context_name == COUNTER_TYPE_METER_BUCKET) { - return std::make_shared(context_name, instance, m_vendorSai.get(), m_dbCounters); + return std::make_shared(context_name, instance, m_vendorSai.get(), m_dbCounters, m_isTcpConn); } else if (context_name == ATTR_TYPE_PORT_PHY_ATTR) { - return std::make_shared(context_name, instance, SAI_OBJECT_TYPE_PORT, m_vendorSai.get(), m_statsMode, m_dbCounters); + return std::make_shared(context_name, instance, SAI_OBJECT_TYPE_PORT, m_vendorSai.get(), m_statsMode, m_dbCounters, m_isTcpConn); } else if (context_name == ATTR_TYPE_PORT_PHY_SERDES_ATTR) { - return std::make_shared(context_name, instance, SAI_OBJECT_TYPE_PORT_SERDES, m_vendorSai.get(), m_statsMode, m_dbCounters); + return std::make_shared(context_name, instance, SAI_OBJECT_TYPE_PORT_SERDES, m_vendorSai.get(), m_statsMode, m_dbCounters, m_isTcpConn); } else if (context_name == ATTR_TYPE_QUEUE) { @@ -3492,7 +3503,7 @@ void FlexCounter::flexCounterThreadRunFunction() { SWSS_LOG_ENTER(); - swss::DBConnector db(m_dbCounters, 0); + swss::DBConnector db(m_dbCounters, 0, m_isTcpConn); swss::RedisPipeline pipeline(&db); swss::Table countersTable(&pipeline, COUNTERS_TABLE, true); diff --git a/syncd/FlexCounter.h b/syncd/FlexCounter.h index 62e846e5..1d28cb23 100644 --- a/syncd/FlexCounter.h +++ b/syncd/FlexCounter.h @@ -216,6 +216,8 @@ namespace syncd std::string m_dbCounters; + bool m_isTcpConn; + bool m_isDiscarded; std::map> m_counterContext; diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index 54e8024c..afa6d77a 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -7,7 +7,9 @@ #include "NumberOidIndexGenerator.h" #include #include +#include #include +#include "swss/dbconnector.h" using namespace saimeta; using namespace sairedis; @@ -1550,11 +1552,15 @@ TEST(FlexCounter, bulkChunksize) // addCounterPlugin calls that set and then remove per-prefix // chunk sizes, the polling thread can poll with per-prefix // partitions that have fewer counters and different chunk - // sizes. Those polls should fall through to the per-counter - // switch below. + // sizes. After the merge-back, FlexCounter also re-probes + // bulk capability with single-object calls (object_count=1) + // that have all counters. Skip the assertion for both + // per-prefix polls and re-probe polls. if (unifiedBulkChunkSize > 0) { - if (object_count != unifiedBulkChunkSize && number_of_counters == allCounters.size()) + if (object_count != unifiedBulkChunkSize + && number_of_counters == allCounters.size() + && object_count > 1) { EXPECT_EQ(object_count, unifiedBulkChunkSize); } @@ -2309,3 +2315,105 @@ TEST(FlexCounter, noEniDashMeterCounter) counterVerifyFunc, false); } + +class FlexCounterTcpFallback : public ::testing::Test +{ +protected: + static constexpr const char *configPath = "/tmp/test_tcp_fallback_db_config.json"; + + void SetUp() override + { + const std::string configContent = R"({ + "INSTANCES": { + "redis": { + "hostname": "127.0.0.1", + "port": 6379, + "unix_socket_path": "" + } + }, + "DATABASES": { + "COUNTERS_DB": { + "id": 2, + "separator": ":", + "instance": "redis" + } + }, + "VERSION": "1.0" + })"; + + std::ofstream ofs(configPath); + ofs << configContent; + ofs.close(); + + swss::SonicDBConfig::reset(); + swss::SonicDBConfig::initialize(configPath); + } + + void TearDown() override + { + std::remove(configPath); + swss::SonicDBConfig::reset(); + swss::SonicDBConfig::initialize(); + } +}; + +TEST_F(FlexCounterTcpFallback, tcpFallbackWhenNoUnixSocket) +{ + EXPECT_TRUE(swss::SonicDBConfig::getDbSock("COUNTERS_DB").empty()); + + sai->mock_getStatsExt = [](sai_object_type_t, sai_object_id_t, uint32_t number_of_counters, const sai_stat_id_t *, sai_stats_mode_t, uint64_t *counters) { + for (uint32_t i = 0; i < number_of_counters; i++) + { + counters[i] = (i + 1) * 100; + } + return SAI_STATUS_SUCCESS; + }; + sai->mock_getStats = [](sai_object_type_t, sai_object_id_t, uint32_t number_of_counters, const sai_stat_id_t *, uint64_t *counters) { + for (uint32_t i = 0; i < number_of_counters; i++) + { + counters[i] = (i + 1) * 100; + } + return SAI_STATUS_SUCCESS; + }; + sai->mock_queryStatsCapability = [](sai_object_id_t, sai_object_type_t, sai_stat_capability_list_t *) { + return SAI_STATUS_FAILURE; + }; + sai->mock_bulkGetStats = [](sai_object_id_t, sai_object_type_t, uint32_t, const sai_object_key_t *, uint32_t, const sai_stat_id_t *, sai_stats_mode_t, sai_status_t *, uint64_t *) { + return SAI_STATUS_FAILURE; + }; + + // FlexCounter should detect empty socket path and use TCP + FlexCounter fc("test_tcp", sai, "COUNTERS_DB"); + + test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT); + + std::vector values; + values.emplace_back(POLL_INTERVAL_FIELD, "1000"); + values.emplace_back(FLEX_COUNTER_STATUS_FIELD, "enable"); + values.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ); + fc.addCounterPlugin(values); + + values.clear(); + values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_OCTETS,SAI_PORT_STAT_IF_IN_UCAST_PKTS"); + + auto object_ids = generateOids(1, SAI_OBJECT_TYPE_PORT); + fc.addCounter(object_ids[0], object_ids[0], values); + EXPECT_FALSE(fc.isEmpty()); + + // Use TCP to connect and verify counters were written + swss::DBConnector db("COUNTERS_DB", 0, true); + swss::RedisPipeline pipeline(&db); + swss::Table countersTable(&pipeline, COUNTERS_TABLE, false); + + waitForCounterKeys(countersTable, 1); + + std::string key = toOid(object_ids[0]); + waitForCounterValues(countersTable, key, + {"SAI_PORT_STAT_IF_IN_OCTETS", "SAI_PORT_STAT_IF_IN_UCAST_PKTS"}, + {"100", "200"}); + + fc.removeCounter(object_ids[0]); + EXPECT_TRUE(fc.isEmpty()); + + countersTable.del(key); +}