Skip to content

Syncd FDB notification batch-drop fix #2009

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
johanna-nexthop wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Syncd FDB notification batch-drop fix #2009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions syncd/NotificationProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -314,17 +315,16 @@ void NotificationProcessor::process_on_fdb_event(

SWSS_LOG_INFO("fdb event count: %u", count);

bool sendntf = true;
std::vector<sai_fdb_event_notification_data_t> 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;
}

Expand All @@ -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);
}
}

Expand Down
1 change: 1 addition & 0 deletions tests/aspell.en.pws
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ AttrHash
BCM
BFD
Bool
BVID
CHARDATA
COLDVIDS
CONST
Expand Down
21 changes: 21 additions & 0 deletions unittest/syncd/TestNotificationProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<swss::FieldValueTuple> 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);
Expand Down
Loading