diff --git a/syncd/NotificationProcessor.cpp b/syncd/NotificationProcessor.cpp index 20eb3937cb..d6f8b41315 100644 --- a/syncd/NotificationProcessor.cpp +++ b/syncd/NotificationProcessor.cpp @@ -280,7 +280,8 @@ bool NotificationProcessor::check_fdb_event_notification_data( if (!m_translator->checkRidExists(attr.value.oid, true)) { - SWSS_LOG_WARN("RID 0x%" PRIx64 " on %s is not present on local ASIC DB", attr.value.oid, meta->attridname); + SWSS_LOG_WARN("RID 0x%" PRIx64 " on %s is not present on local ASIC DB: %s", attr.value.oid, meta->attridname, + sai_serialize_fdb_entry(data.fdb_entry).c_str()); result = false; } @@ -314,17 +315,16 @@ void NotificationProcessor::process_on_fdb_event( SWSS_LOG_INFO("fdb event count: %u", count); - bool sendntf = true; + std::vector validEntries; for (uint32_t i = 0; i < count; i++) { sai_fdb_event_notification_data_t *fdb = &data[i]; - sendntf &= check_fdb_event_notification_data(*fdb); - - if (!sendntf) + if (!check_fdb_event_notification_data(*fdb)) { - SWSS_LOG_ERROR("invalid OIDs in fdb notifications, NOT translating and NOT storing in ASIC DB"); + SWSS_LOG_ERROR("invalid OIDs in fdb notification entry %u, NOT translating and NOT storing in ASIC DB: %s", + i, sai_serialize_fdb_event_ntf(1, fdb).c_str()); continue; } @@ -343,17 +343,21 @@ void NotificationProcessor::process_on_fdb_event( */ redisPutFdbEntryToAsicView(fdb); + + validEntries.push_back(*fdb); } - if (sendntf) + if (!validEntries.empty()) { - std::string s = sai_serialize_fdb_event_ntf(count, data); + std::string s = sai_serialize_fdb_event_ntf((uint32_t)validEntries.size(), validEntries.data()); sendNotification(SAI_SWITCH_NOTIFICATION_NAME_FDB_EVENT, s); } - else + + if (validEntries.size() != count) { - SWSS_LOG_ERROR("FDB notification was not sent since it contain invalid OIDs, bug?"); + SWSS_LOG_ERROR("FDB notification contained %u invalid entries out of %u total; only valid entries were stored in ASIC DB and forwarded", + count - (uint32_t)validEntries.size(), count); } } diff --git a/tests/aspell.en.pws b/tests/aspell.en.pws index 5eb0162a8d..f2f6598c69 100644 --- a/tests/aspell.en.pws +++ b/tests/aspell.en.pws @@ -17,6 +17,7 @@ AttrHash BCM BFD Bool +BVID CHARDATA COLDVIDS CONST diff --git a/unittest/syncd/TestNotificationProcessor.cpp b/unittest/syncd/TestNotificationProcessor.cpp index ea8f2f1d1d..9c40f86219 100644 --- a/unittest/syncd/TestNotificationProcessor.cpp +++ b/unittest/syncd/TestNotificationProcessor.cpp @@ -77,6 +77,27 @@ TEST(NotificationProcessor, NotificationProcessorTest) EXPECT_EQ(*bridgeport, "oid:0x3a000000000a99"); EXPECT_EQ(ip, nullptr); + // Verify one invalid FDB event does not block later valid entries in the same batch. + translator->insertRidAndVid(0x21000000000000,0x210000000000); + translator->insertRidAndVid(0x2600000003,0x26000000000003); + translator->insertRidAndVid(0x1003a0000004c,0x3a000000000a9c); + // Leave the bad entry's BVID and bridge-port RIDs unregistered on purpose. + + static std::string fdb_batch_data = "[{\"fdb_entry\":\"{\\\"bvid\\\":\\\"oid:0x2600000002\\\",\\\"mac\\\":\\\"00:00:00:00:00:02\\\",\\\"switch_id\\\":\\\"oid:0x21000000000000\\\"}\",\"fdb_event\":\"SAI_FDB_EVENT_LEARNED\",\"list\":[{\"id\":\"SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID\",\"value\":\"oid:0x1003a0000004b\"}]},{\"fdb_entry\":\"{\\\"bvid\\\":\\\"oid:0x2600000003\\\",\\\"mac\\\":\\\"00:00:00:00:00:03\\\",\\\"switch_id\\\":\\\"oid:0x21000000000000\\\"}\",\"fdb_event\":\"SAI_FDB_EVENT_LEARNED\",\"list\":[{\"id\":\"SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID\",\"value\":\"oid:0x1003a0000004c\"}]}]"; + std::vector fdb_batch_entry; + swss::KeyOpFieldsValuesTuple batchItem(SAI_SWITCH_NOTIFICATION_NAME_FDB_EVENT, fdb_batch_data, fdb_batch_entry); + + notificationProcessor->syncProcessNotification(batchItem); + + translator->eraseRidAndVid(0x21000000000000,0x210000000000); + translator->eraseRidAndVid(0x2600000003,0x26000000000003); + translator->eraseRidAndVid(0x1003a0000004c,0x3a000000000a9c); + + std::string goodKey = "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY:{\"bvid\":\"oid:0x26000000000003\",\"mac\":\"00:00:00:00:00:03\",\"switch_id\":\"oid:0x210000000000\"}"; + auto goodBridgePort = dbAsic->hget(goodKey, "SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID"); + ASSERT_NE(goodBridgePort, nullptr); + EXPECT_EQ(*goodBridgePort, "oid:0x3a000000000a9c"); + //Test ICMP_ECHO_SESSION_STATE_CHANGE Notification translator->insertRidAndVid(0x21000000000000,0x210000000000); translator->insertRidAndVid(0x100000000003a,0x100000000003a);