From b128dc4510b8a8e48580ecd08900af66acb98da6 Mon Sep 17 00:00:00 2001 From: Vishrant Vasavada Date: Tue, 18 Aug 2026 18:57:44 -0700 Subject: [PATCH] Dynamic FDB entries should be skipped during warm boot if aged out in h/w Signed-off-by: Vishrant Vasavada --- fboss/agent/hw/sai/api/FdbApi.h | 6 +++ fboss/agent/hw/sai/api/Traits.h | 8 ++++ fboss/agent/hw/sai/store/SaiObject.h | 3 +- fboss/agent/hw/sai/store/SaiStore.h | 42 +++++++++++++++++-- .../agent/hw/sai/store/tests/FdbStoreTest.cpp | 27 ++++++++++++ fboss/agent/hw/sai/switch/SaiFdbManager.cpp | 6 +-- 6 files changed, 84 insertions(+), 8 deletions(-) diff --git a/fboss/agent/hw/sai/api/FdbApi.h b/fboss/agent/hw/sai/api/FdbApi.h index 44f681fea8fb9..1861ac0aeb681 100644 --- a/fboss/agent/hw/sai/api/FdbApi.h +++ b/fboss/agent/hw/sai/api/FdbApi.h @@ -102,6 +102,12 @@ SAI_ATTRIBUTE_NAME(Fdb, Metadata) template <> struct IsSaiEntryStruct : public std::true_type {}; +// Dynamic FDB entries age out in hardware on their own, so an entry FBOSS +// tracks - in the store, or in a warm boot state written moments earlier - may +// already be gone from HW. +template <> +struct SaiObjectMayBeMissingInHw : public std::true_type {}; + class FdbApi : public SaiApi { public: static constexpr sai_api_t ApiType = SAI_API_FDB; diff --git a/fboss/agent/hw/sai/api/Traits.h b/fboss/agent/hw/sai/api/Traits.h index 1b3028bfb8e6d..ef24c50f3f54d 100644 --- a/fboss/agent/hw/sai/api/Traits.h +++ b/fboss/agent/hw/sai/api/Traits.h @@ -488,6 +488,14 @@ concept SaiAttributeTuple = template struct IsSaiObjectOwnedByAdapter : public std::false_type {}; +/* + * Hardware can delete objects of some types on its own, without the agent + * asking. For those, an object FBOSS still tracks may already be gone from + * hardware, so ITEM_NOT_FOUND is an expected outcome rather than a fatal one. + */ +template +struct SaiObjectMayBeMissingInHw : public std::false_type {}; + template struct SaiObjectHasStats : public std::false_type {}; diff --git a/fboss/agent/hw/sai/store/SaiObject.h b/fboss/agent/hw/sai/store/SaiObject.h index b63edba46631d..f8828a66d9900 100644 --- a/fboss/agent/hw/sai/store/SaiObject.h +++ b/fboss/agent/hw/sai/store/SaiObject.h @@ -536,7 +536,8 @@ class SaiObject { bool ownedByAdapter_{IsSaiObjectOwnedByAdapter::value}; // For some object types we can ignore missing in HW errors // on when deleting. - bool ignoreMissingInHwOnDelete_{false}; + bool ignoreMissingInHwOnDelete_{ + SaiObjectMayBeMissingInHw::value}; // For some object types we want to skip remove call when deleting bool skipRemove_{false}; typename SaiObjectTraits::AdapterKey adapterKey_; diff --git a/fboss/agent/hw/sai/store/SaiStore.h b/fboss/agent/hw/sai/store/SaiStore.h index 1b418f926c598..db347f001b71d 100644 --- a/fboss/agent/hw/sai/store/SaiStore.h +++ b/fboss/agent/hw/sai/store/SaiStore.h @@ -184,10 +184,13 @@ class SaiObjectStore { keys.end()); } for (const auto& k : keys) { - ObjectType obj = getObject(k, adapterKeys2AdapterHostKey); - auto adapterHostKey = obj.adapterHostKey(); - XLOGF(DBG5, "SaiStore reloaded {}", obj); - auto ins = objects_.refOrInsert(adapterHostKey, std::move(obj)); + auto obj = getObjectIfInHw(k, adapterKeys2AdapterHostKey); + if (!obj) { + continue; + } + auto adapterHostKey = obj->adapterHostKey(); + XLOGF(DBG5, "SaiStore reloaded {}", *obj); + auto ins = objects_.refOrInsert(adapterHostKey, std::move(*obj)); if (!ins.second) { XLOG(FATAL) << "[" << saiObjectTypeToString(SaiObjectTraits::ObjectType) << "]" << " Unexpected duplicate adapterHostKey"; @@ -495,6 +498,37 @@ class SaiObjectStore { }); } + /* + * Warm boot reloads objects from adapter keys saved at graceful exit. For + * object types hardware can delete on its own, that snapshot is only a best + * effort: e.g. a dynamic FDB entry can age out after the keys are written, or + * after an age event the agent never got to process. Skip such keys instead + * of letting the missing object abort warm boot - switch state replays + * whatever is still needed, and removeUnclaimedDynanicEntries() cleans up the + * rest. + */ + std::optional getObjectIfInHw( + const typename SaiObjectTraits::AdapterKey& key, + const folly::dynamic* adapterKeys2AdapterHostKey) { + if constexpr (SaiObjectMayBeMissingInHw::value) { + try { + return getObject(key, adapterKeys2AdapterHostKey); + } catch (const SaiApiError& e) { + if (e.getSaiStatus() != SAI_STATUS_ITEM_NOT_FOUND) { + throw; + } + XLOGF( + WARN, + "[{}] skipping {} on reload, no longer present in hardware", + objectTypeName().str(), + key); + return std::nullopt; + } + } else { + return getObject(key, adapterKeys2AdapterHostKey); + } + } + ObjectType getObject( typename ObjectTraits::AdapterKey key, const folly::dynamic* adapterKey2AdapterHostKey) { diff --git a/fboss/agent/hw/sai/store/tests/FdbStoreTest.cpp b/fboss/agent/hw/sai/store/tests/FdbStoreTest.cpp index 9e5f7bd2b508f..4232efe79375c 100644 --- a/fboss/agent/hw/sai/store/tests/FdbStoreTest.cpp +++ b/fboss/agent/hw/sai/store/tests/FdbStoreTest.cpp @@ -80,6 +80,33 @@ TEST_F(SaiStoreTest, fdbSetBridgePort) { EXPECT_EQ(GET_OPT_ATTR(Fdb, Metadata, obj->attributes()), 23); } +// Hardware ages dynamic FDB entries out on its own, so an entry saved in warm +// boot state can be gone by the time the store reloads it. Reload must skip it +// rather than let ITEM_NOT_FOUND abort warm boot. +TEST_F(SaiStoreTest, fdbAgedOutBeforeReload) { + auto& fdbApi = saiApiTable->fdbApi(); + SaiFdbTraits::FdbEntry present(0, 10, folly::MacAddress{"42:42:42:42:42:42"}); + SaiFdbTraits::FdbEntry aged(0, 10, folly::MacAddress{"42:42:42:42:42:43"}); + fdbApi.create(present, {SAI_FDB_ENTRY_TYPE_STATIC, 42, 24}); + fdbApi.create(aged, {SAI_FDB_ENTRY_TYPE_DYNAMIC, 42, 24}); + + // Warm boot state is written with both entries in it + folly::dynamic adapterKeys; + { + SaiStore preWarmBoot(0); + preWarmBoot.reload(); + adapterKeys = preWarmBoot.adapterKeysFollyDynamic(); + } + // ... and HW ages the dynamic one out while the agent is down + fdbApi.remove(aged); + + SaiStore postWarmBoot(0); + postWarmBoot.reload(&adapterKeys); + auto& store = postWarmBoot.get(); + EXPECT_NE(store.get(present), nullptr); + EXPECT_EQ(store.get(aged), nullptr); +} + TEST_F(SaiStoreTest, fdbSerDeser) { auto& fdbApi = saiApiTable->fdbApi(); folly::MacAddress mac{"42:42:42:42:42:42"}; diff --git a/fboss/agent/hw/sai/switch/SaiFdbManager.cpp b/fboss/agent/hw/sai/switch/SaiFdbManager.cpp index 0b2f1d8343dee..7279d18369382 100644 --- a/fboss/agent/hw/sai/switch/SaiFdbManager.cpp +++ b/fboss/agent/hw/sai/switch/SaiFdbManager.cpp @@ -52,14 +52,14 @@ void ManagedFdbEntry::createObject(PublisherObjects objects) { SaiFdbTraits::CreateAttributes attributes{type_, bridgePortId, metadata_}; auto fdbEntry = manager_->createSaiObject(entry, attributes, intfIDAndMac_); - // For FDB entry, on delete, Ignore error if entry was already removed from - // HW. One scenario where this can occur is the following + // Missing in HW is tolerated for FDB entries on delete and on warm boot + // reload - see SaiObjectMayBeMissingInHw. One scenario where + // this can occur is the following // - We learn a MAC and install it in HW // - Later we get a state update to transform this into a STATIC FDB entry // - Meanwhile the dynamic MAC ages out and gets deleted // - While processing the state delta for changed MAC entry, we try to // delete the dynamic entry before adding static entry - fdbEntry->setIgnoreMissingInHwOnDelete(true); // If pending_L2_entry is not supported, dynamic l2 entry is already deleted // from HW table upon receiving the l2 age event. Therefore, no need to make // the remove call down to SDK layer.