From 5e16aebb5c66de2ca42a02837ed38756ee5192ba Mon Sep 17 00:00:00 2001 From: xq9mend Date: Tue, 28 Apr 2026 15:43:08 +0000 Subject: [PATCH 1/7] Route FDB LEARN/AGED/MOVE events via ProducerStateTable Split FDB notification handling in NotificationProcessor: - FLUSH events continue via PUBLISH/NotificationProducer (existing path) - LEARN/AGED/MOVE events use ProducerStateTable (key-based dedup) ProducerStateTable provides MAC-keyed deduplication so rapid flap storms for the same MAC+BVID collapse to one entry instead of flooding the notification queue. Fallback to PUBLISH is retained when m_fdbEventStateProducer is null (ZeroMQ mode). Adds REDIS_TABLE_FDB_EVENT_STATE define in sairediscommon.h. Passes m_dbAsic to NotificationProcessor constructor from Syncd. Signed-off-by: xq9mend --- lib/sairediscommon.h | 2 + syncd/NotificationProcessor.cpp | 91 +++++++++++++++++++++++++++++---- syncd/NotificationProcessor.h | 6 ++- syncd/Syncd.cpp | 3 +- 4 files changed, 89 insertions(+), 13 deletions(-) diff --git a/lib/sairediscommon.h b/lib/sairediscommon.h index c2da2a08e5..2cd9a1c746 100644 --- a/lib/sairediscommon.h +++ b/lib/sairediscommon.h @@ -97,6 +97,8 @@ /** * @brief Table which will be used to send API response from syncd. */ +#define REDIS_TABLE_FDB_EVENT_STATE "FDB_EVENT_STATE" + #define REDIS_TABLE_GETRESPONSE "GETRESPONSE" // REDIS default database defines diff --git a/syncd/NotificationProcessor.cpp b/syncd/NotificationProcessor.cpp index 20eb3937cb..86ad03c344 100644 --- a/syncd/NotificationProcessor.cpp +++ b/syncd/NotificationProcessor.cpp @@ -10,6 +10,8 @@ #include "swss/logger.h" #include "swss/notificationproducer.h" +#include "swss/producerstatetable.h" + #include using namespace syncd; @@ -19,7 +21,8 @@ NotificationProcessor::NotificationProcessor( _In_ std::shared_ptr producer, _In_ std::shared_ptr client, _In_ std::function synchronizer, - _In_ std::function linkEventDampingApplier): + _In_ std::function linkEventDampingApplier, + _In_ std::shared_ptr dbAsic): m_synchronizer(synchronizer), m_linkEventDampingApplier(linkEventDampingApplier), m_client(client), @@ -30,6 +33,12 @@ NotificationProcessor::NotificationProcessor( m_runThread = false; m_notificationQueue = std::make_shared(); + + if (dbAsic) + { + m_fdbEventStateProducer = std::make_shared( + dbAsic.get(), REDIS_TABLE_FDB_EVENT_STATE); + } } NotificationProcessor::~NotificationProcessor() @@ -316,6 +325,14 @@ void NotificationProcessor::process_on_fdb_event( bool sendntf = true; + /* + * Separate FLUSH events from LEARN/AGED/MOVE events. + * FLUSH events are sent via PUBLISH (NotificationProducer) as before. + * LEARN/AGED/MOVE events use ProducerStateTable for key-based dedup. + */ + std::vector flushEvents; + std::vector nonFlushEvents; + for (uint32_t i = 0; i < count; i++) { sai_fdb_event_notification_data_t *fdb = &data[i]; @@ -336,24 +353,76 @@ void NotificationProcessor::process_on_fdb_event( m_translator->translateRidToVid(SAI_OBJECT_TYPE_FDB_ENTRY, fdb->fdb_entry.switch_id, fdb->attr_count, fdb->attr, true); - /* - * Currently because of brcm bug, we need to install fdb entries in - * asic view and currently this event don't have fdb type which is - * required on creation. - */ - redisPutFdbEntryToAsicView(fdb); + + if (fdb->event_type == SAI_FDB_EVENT_FLUSHED) + { + flushEvents.push_back(*fdb); + } + else + { + nonFlushEvents.push_back(*fdb); + } } - if (sendntf) + if (!sendntf) + { + SWSS_LOG_ERROR("FDB notification was not sent since it contain invalid OIDs, bug?"); + return; + } + + /* Send FLUSH events via PUBLISH (preserves existing behavior) */ + if (!flushEvents.empty()) { - std::string s = sai_serialize_fdb_event_ntf(count, data); + std::string s = sai_serialize_fdb_event_ntf( + (uint32_t)flushEvents.size(), flushEvents.data()); sendNotification(SAI_SWITCH_NOTIFICATION_NAME_FDB_EVENT, s); } - else + + /* Send LEARN/AGED/MOVE events via ProducerStateTable (key-based dedup) */ + if (m_fdbEventStateProducer && !nonFlushEvents.empty()) { - SWSS_LOG_ERROR("FDB notification was not sent since it contain invalid OIDs, bug?"); + for (auto &fdb : nonFlushEvents) + { + std::string key = sai_serialize_fdb_entry(fdb.fdb_entry); + + if (fdb.event_type == SAI_FDB_EVENT_AGED) + { + m_fdbEventStateProducer->del(key); + } + else + { + std::vector fvs; + + fvs.emplace_back("event_type", sai_serialize_fdb_event(fdb.event_type)); + + for (uint32_t j = 0; j < fdb.attr_count; j++) + { + if (fdb.attr[j].id == SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID) + { + fvs.emplace_back("bridge_port_id", + sai_serialize_object_id(fdb.attr[j].value.oid)); + } + else if (fdb.attr[j].id == SAI_FDB_ENTRY_ATTR_TYPE) + { + fvs.emplace_back("sai_fdb_type", + sai_serialize_enum(fdb.attr[j].value.s32, + &sai_metadata_enum_sai_fdb_entry_type_t)); + } + } + + m_fdbEventStateProducer->set(key, fvs); + } + } + } + else if (!nonFlushEvents.empty()) + { + /* Fallback: no ProducerStateTable (e.g., ZeroMQ mode), use PUBLISH */ + std::string s = sai_serialize_fdb_event_ntf( + (uint32_t)nonFlushEvents.size(), nonFlushEvents.data()); + + sendNotification(SAI_SWITCH_NOTIFICATION_NAME_FDB_EVENT, s); } } diff --git a/syncd/NotificationProcessor.h b/syncd/NotificationProcessor.h index 4d17748663..dea97954f8 100644 --- a/syncd/NotificationProcessor.h +++ b/syncd/NotificationProcessor.h @@ -7,6 +7,7 @@ #include "FlowDump.h" #include "swss/notificationproducer.h" +#include "swss/producerstatetable.h" #include #include @@ -23,7 +24,8 @@ namespace syncd _In_ std::shared_ptr producer, _In_ std::shared_ptr client, _In_ std::function synchronizer, - _In_ std::function linkEventDampingApplier = nullptr); + _In_ std::function linkEventDampingApplier = nullptr, + _In_ std::shared_ptr dbAsic = nullptr); virtual ~NotificationProcessor(); @@ -221,5 +223,7 @@ namespace syncd std::shared_ptr m_client; std::shared_ptr m_notifications; + + std::shared_ptr m_fdbEventStateProducer; }; } diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 64138af182..cfe1e0735b 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -187,7 +187,8 @@ Syncd::Syncd( m_notifications, m_client, std::bind(&Syncd::syncProcessNotification, this, _1), - std::bind(&Syncd::applyLinkEventDamping, this, _1, _2)); + std::bind(&Syncd::applyLinkEventDamping, this, _1, _2), + m_dbAsic); m_handler = std::make_shared(m_processor); m_sn.onFdbEvent = std::bind(&NotificationHandler::onFdbEvent, m_handler.get(), _1, _2); From 954923ba5d113bcbd6def9f52ce46d17fc21fbde Mon Sep 17 00:00:00 2001 From: xq9mend Date: Thu, 30 Apr 2026 05:51:31 +0000 Subject: [PATCH 2/7] Add unit tests for FDB ProducerStateTable routing in NotificationProcessor Cover the three new code paths added in process_on_fdb_event: - FdbLearnGoesToProducerStateTable: LEARN event writes to _FDB_EVENT_STATE:key - FdbAgedDeletesFromProducerStateTable: AGED event deletes the scratch key - FdbFlushDoesNotWriteToProducerStateTable: FLUSH event goes via PUBLISH Each test creates a NotificationProcessor with dbAsic to initialize m_fdbEventStateProducer, then verifies the Redis state after syncProcessNotification(). Signed-off-by: xq9mend --- syncd/NotificationProcessor.cpp | 7 +- unittest/syncd/TestNotificationProcessor.cpp | 171 +++++++++++++++++++ 2 files changed, 175 insertions(+), 3 deletions(-) diff --git a/syncd/NotificationProcessor.cpp b/syncd/NotificationProcessor.cpp index 86ad03c344..4a5b226554 100644 --- a/syncd/NotificationProcessor.cpp +++ b/syncd/NotificationProcessor.cpp @@ -328,7 +328,8 @@ void NotificationProcessor::process_on_fdb_event( /* * Separate FLUSH events from LEARN/AGED/MOVE events. * FLUSH events are sent via PUBLISH (NotificationProducer) as before. - * LEARN/AGED/MOVE events use ProducerStateTable for key-based dedup. + * LEARN/AGED/MOVE events use ProducerStateTable so rapid events for the + * same entry are merged before orchagent processes them. */ std::vector flushEvents; std::vector nonFlushEvents; @@ -380,7 +381,7 @@ void NotificationProcessor::process_on_fdb_event( sendNotification(SAI_SWITCH_NOTIFICATION_NAME_FDB_EVENT, s); } - /* Send LEARN/AGED/MOVE events via ProducerStateTable (key-based dedup) */ + /* Send LEARN/AGED/MOVE events via ProducerStateTable (merged by key) */ if (m_fdbEventStateProducer && !nonFlushEvents.empty()) { for (auto &fdb : nonFlushEvents) @@ -418,7 +419,7 @@ void NotificationProcessor::process_on_fdb_event( } else if (!nonFlushEvents.empty()) { - /* Fallback: no ProducerStateTable (e.g., ZeroMQ mode), use PUBLISH */ + /* Fallback: m_fdbEventStateProducer not initialized, use PUBLISH */ std::string s = sai_serialize_fdb_event_ntf( (uint32_t)nonFlushEvents.size(), nonFlushEvents.data()); diff --git a/unittest/syncd/TestNotificationProcessor.cpp b/unittest/syncd/TestNotificationProcessor.cpp index ea8f2f1d1d..ca57191f77 100644 --- a/unittest/syncd/TestNotificationProcessor.cpp +++ b/unittest/syncd/TestNotificationProcessor.cpp @@ -6,6 +6,9 @@ #include "lib/sairediscommon.h" #include "vslib/Sai.h" +#include "swss/table.h" +#include "swss/logger.h" + #include using namespace syncd; @@ -144,3 +147,171 @@ TEST(NotificationProcessor, NotificationProcessorTest) notificationProcessor->syncProcessNotification(flowBulkGetSessionEventItem); translator->eraseRidAndVid(0x123456789abcdef, 0x123456789abcdef); } + + +/* + * Helper: build a processor with a DB connection so m_fdbEventStateProducer + * is initialized. + */ +static std::shared_ptr makeProcessorWithDb( + std::shared_ptr dbAsic, + std::shared_ptr &outTranslator) +{ + SWSS_LOG_ENTER(); + auto sai = std::make_shared(); + auto client = std::make_shared(dbAsic); + auto producer = std::make_shared("ASIC_DB"); + + auto np = std::make_shared(producer, client, + [](const swss::KeyOpFieldsValuesTuple&){}, + dbAsic); + + auto switchConfigContainer = std::make_shared(); + auto ridxGen = std::make_shared(dbAsic, REDIS_KEY_VIDCOUNTER); + auto voim = std::make_shared(0, switchConfigContainer, ridxGen); + + outTranslator = std::make_shared(client, voim, sai); + np->m_translator = outTranslator; + + return np; +} + +/* + * ProducerStateTable writes field data to "_FDB_EVENT_STATE:{key}" in Redis. + * Use this helper to verify what was written. + */ +static std::string fdbStateKey(const std::string &fdbKey) +{ + // SWSS_LOG_ENTER omitted (simple helper) + return "_FDB_EVENT_STATE:" + fdbKey; +} + +/* + * Test: LEARN event is written to FDB_EVENT_STATE via ProducerStateTable. + */ +TEST(NotificationProcessor, FdbLearnGoesToProducerStateTable) +{ + auto dbAsic = std::make_shared("ASIC_DB", 0); + std::shared_ptr translator; + auto np = makeProcessorWithDb(dbAsic, translator); + + translator->insertRidAndVid(0x21000000000000, 0x210000000000); + translator->insertRidAndVid(0x1003a0000004c, 0x3a000000000c00); + translator->insertRidAndVid(0x2600000010, 0x26000000000010); + + std::string fdbKey = + "{\"bvid\":\"oid:0x26000000000010\"," + "\"mac\":\"00:00:00:00:00:10\"," + "\"switch_id\":\"oid:0x210000000000\"}"; + + dbAsic->del(fdbStateKey(fdbKey)); + + std::string learnData = + "[{\"fdb_entry\":\"{\\\"bvid\\\":\\\"oid:0x2600000010\\\"," + "\\\"mac\\\":\\\"00:00:00:00:00:10\\\"," + "\\\"switch_id\\\":\\\"oid:0x21000000000000\\\"}\"," + "\"fdb_event\":\"SAI_FDB_EVENT_LEARNED\"," + "\"list\":[" + "{\"id\":\"SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID\"," + "\"value\":\"oid:0x1003a0000004c\"}," + "{\"id\":\"SAI_FDB_ENTRY_ATTR_TYPE\"," + "\"value\":\"SAI_FDB_ENTRY_TYPE_DYNAMIC\"}" + "]}]"; + + std::vector empty; + swss::KeyOpFieldsValuesTuple item(SAI_SWITCH_NOTIFICATION_NAME_FDB_EVENT, learnData, empty); + np->syncProcessNotification(item); + + auto val = dbAsic->hget(fdbStateKey(fdbKey), "event_type"); + EXPECT_NE(val, nullptr) << "LEARN event not written to FDB_EVENT_STATE"; + if (val) + { + EXPECT_EQ(*val, "SAI_FDB_EVENT_LEARNED"); + } + + dbAsic->del(fdbStateKey(fdbKey)); + dbAsic->del("FDB_EVENT_STATE_KEY_SET"); + translator->eraseRidAndVid(0x21000000000000, 0x210000000000); + translator->eraseRidAndVid(0x1003a0000004c, 0x3a000000000c00); + translator->eraseRidAndVid(0x2600000010, 0x26000000000010); +} + +/* + * Test: AGED event issues DEL on FDB_EVENT_STATE (deletes the key). + */ +TEST(NotificationProcessor, FdbAgedDeletesFromProducerStateTable) +{ + auto dbAsic = std::make_shared("ASIC_DB", 0); + std::shared_ptr translator; + auto np = makeProcessorWithDb(dbAsic, translator); + + translator->insertRidAndVid(0x21000000000000, 0x210000000000); + translator->insertRidAndVid(0x2600000011, 0x26000000000011); + + std::string fdbKey = + "{\"bvid\":\"oid:0x26000000000011\"," + "\"mac\":\"00:00:00:00:00:11\"," + "\"switch_id\":\"oid:0x210000000000\"}"; + + /* Pre-populate so we can verify deletion */ + dbAsic->hset(fdbStateKey(fdbKey), "event_type", "SAI_FDB_EVENT_LEARNED"); + + std::string agedData = + "[{\"fdb_entry\":\"{\\\"bvid\\\":\\\"oid:0x2600000011\\\"," + "\\\"mac\\\":\\\"00:00:00:00:00:11\\\"," + "\\\"switch_id\\\":\\\"oid:0x21000000000000\\\"}\"," + "\"fdb_event\":\"SAI_FDB_EVENT_AGED\"," + "\"list\":[]}]"; + + std::vector empty; + swss::KeyOpFieldsValuesTuple item(SAI_SWITCH_NOTIFICATION_NAME_FDB_EVENT, agedData, empty); + np->syncProcessNotification(item); + + /* ProducerStateTable::del does not remove the key immediately in Redis; + * it publishes a DEL notification. The _G_ scratch key should be deleted. */ + auto val = dbAsic->hget(fdbStateKey(fdbKey), "event_type"); + EXPECT_EQ(val, nullptr) << "AGED event should delete entry from FDB_EVENT_STATE scratch"; + + dbAsic->del("FDB_EVENT_STATE_KEY_SET"); + translator->eraseRidAndVid(0x21000000000000, 0x210000000000); + translator->eraseRidAndVid(0x2600000011, 0x26000000000011); +} + +/* + * Test: FLUSH event goes via PUBLISH, not to FDB_EVENT_STATE. + */ +TEST(NotificationProcessor, FdbFlushDoesNotWriteToProducerStateTable) +{ + auto dbAsic = std::make_shared("ASIC_DB", 0); + std::shared_ptr translator; + auto np = makeProcessorWithDb(dbAsic, translator); + + translator->insertRidAndVid(0x21000000000000, 0x210000000000); + translator->insertRidAndVid(0x2600000012, 0x26000000000012); + + std::string fdbKey = + "{\"bvid\":\"oid:0x26000000000012\"," + "\"mac\":\"00:00:00:00:00:00\"," + "\"switch_id\":\"oid:0x210000000000\"}"; + + dbAsic->del(fdbStateKey(fdbKey)); + + std::string flushData = + "[{\"fdb_entry\":\"{\\\"bvid\\\":\\\"oid:0x2600000012\\\"," + "\\\"mac\\\":\\\"00:00:00:00:00:00\\\"," + "\\\"switch_id\\\":\\\"oid:0x21000000000000\\\"}\"," + "\"fdb_event\":\"SAI_FDB_EVENT_FLUSHED\"," + "\"list\":[]}]"; + + std::vector empty; + swss::KeyOpFieldsValuesTuple item(SAI_SWITCH_NOTIFICATION_NAME_FDB_EVENT, flushData, empty); + np->syncProcessNotification(item); + + /* FLUSH must NOT appear in the scratch table */ + auto val = dbAsic->hget(fdbStateKey(fdbKey), "event_type"); + EXPECT_EQ(val, nullptr) << "FLUSH event must not be written to FDB_EVENT_STATE"; + + dbAsic->del("FDB_EVENT_STATE_KEY_SET"); + translator->eraseRidAndVid(0x21000000000000, 0x210000000000); + translator->eraseRidAndVid(0x2600000012, 0x26000000000012); +} From 7ecf93fdeb69df1510b927312b69aabf0451946b Mon Sep 17 00:00:00 2001 From: xq9mend Date: Mon, 4 May 2026 05:42:41 +0000 Subject: [PATCH 3/7] Use flushdb() in tests to prevent ASIC_DB state leaking to subsequent tests Replace per-key del calls with dbAsic->flushdb() at the end of each new NotificationProcessor test. This ensures the entire ASIC_DB is clean after our tests run, preventing any Redis state pollution that could affect FlexCounter or other tests running in the same process. Signed-off-by: xq9mend --- unittest/syncd/TestNotificationProcessor.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/unittest/syncd/TestNotificationProcessor.cpp b/unittest/syncd/TestNotificationProcessor.cpp index ca57191f77..353bef82f6 100644 --- a/unittest/syncd/TestNotificationProcessor.cpp +++ b/unittest/syncd/TestNotificationProcessor.cpp @@ -164,7 +164,7 @@ static std::shared_ptr makeProcessorWithDb( auto np = std::make_shared(producer, client, [](const swss::KeyOpFieldsValuesTuple&){}, - dbAsic); + nullptr, dbAsic); auto switchConfigContainer = std::make_shared(); auto ridxGen = std::make_shared(dbAsic, REDIS_KEY_VIDCOUNTER); @@ -229,8 +229,7 @@ TEST(NotificationProcessor, FdbLearnGoesToProducerStateTable) EXPECT_EQ(*val, "SAI_FDB_EVENT_LEARNED"); } - dbAsic->del(fdbStateKey(fdbKey)); - dbAsic->del("FDB_EVENT_STATE_KEY_SET"); + dbAsic->flushdb(); translator->eraseRidAndVid(0x21000000000000, 0x210000000000); translator->eraseRidAndVid(0x1003a0000004c, 0x3a000000000c00); translator->eraseRidAndVid(0x2600000010, 0x26000000000010); @@ -272,7 +271,7 @@ TEST(NotificationProcessor, FdbAgedDeletesFromProducerStateTable) auto val = dbAsic->hget(fdbStateKey(fdbKey), "event_type"); EXPECT_EQ(val, nullptr) << "AGED event should delete entry from FDB_EVENT_STATE scratch"; - dbAsic->del("FDB_EVENT_STATE_KEY_SET"); + dbAsic->flushdb(); translator->eraseRidAndVid(0x21000000000000, 0x210000000000); translator->eraseRidAndVid(0x2600000011, 0x26000000000011); } @@ -311,7 +310,7 @@ TEST(NotificationProcessor, FdbFlushDoesNotWriteToProducerStateTable) auto val = dbAsic->hget(fdbStateKey(fdbKey), "event_type"); EXPECT_EQ(val, nullptr) << "FLUSH event must not be written to FDB_EVENT_STATE"; - dbAsic->del("FDB_EVENT_STATE_KEY_SET"); + dbAsic->flushdb(); translator->eraseRidAndVid(0x21000000000000, 0x210000000000); translator->eraseRidAndVid(0x2600000012, 0x26000000000012); } From 74d21ce14b7ded2bbccb986b0092ac5118fc20ca Mon Sep 17 00:00:00 2001 From: xq9mend Date: Thu, 6 Aug 2026 10:10:55 +0000 Subject: [PATCH 4/7] ci: re-trigger CI (vstest infra flake) Signed-off-by: xq9mend From 0d9d8bc3d163a919c9362dbc4039f38a49c910e5 Mon Sep 17 00:00:00 2001 From: xq9mend Date: Fri, 7 Aug 2026 10:30:11 +0000 Subject: [PATCH 5/7] ci: re-trigger CI (infra flake) Signed-off-by: xq9mend From 33ac8397f1b9610a76bf58b41576049bcd910244 Mon Sep 17 00:00:00 2001 From: xq9mend Date: Fri, 7 Aug 2026 20:30:03 +0000 Subject: [PATCH 6/7] ci: re-trigger CI (infra flake) Signed-off-by: xq9mend From db07e1a1cf231da8f4a3390a270454d73a6006fe Mon Sep 17 00:00:00 2001 From: xq9mend Date: Sun, 9 Aug 2026 14:49:46 +0000 Subject: [PATCH 7/7] ci: re-trigger CI (infra flake) Signed-off-by: xq9mend