Skip to content

Commit db9a1fa

Browse files
Ravi Vantipallimeta-codesync[bot]
authored andcommitted
Rollback routes in reverse order per fib delta
Summary: https://docs.google.com/document/d/1NAG_GtQ4dwZvPRFYKrt4WKqAu8VZe41Z2fFJ7RqYb2o/edit?tab=t.0#heading=h.xfucm5k7g5ql has details We want to process the fib Delta in reverse order to ensure correctness Reviewed By: zechengh09, jasmeetbagga Differential Revision: D86687218 fbshipit-source-id: 46ca1144bbd3377bd1e63eb578404c98756921a5
1 parent 1addc4d commit db9a1fa

File tree

2 files changed

+66
-48
lines changed

2 files changed

+66
-48
lines changed

fboss/agent/hw/sai/switch/SaiSwitch.cpp

Lines changed: 54 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -886,6 +886,52 @@ bool SaiSwitch::l2LearningModeChangeProhibited() const {
886886
return getSwitchRunState() >= l2LearningChangeProhibitedAfter;
887887
}
888888

889+
template <typename LockPolicyT, typename AddrT>
890+
void SaiSwitch::processRemovedRoutesDelta(
891+
const RouterID& routerID,
892+
const auto& routesDelta,
893+
const LockPolicyT& lockPolicy) {
894+
if (!rollbackInProgress_) {
895+
// Normal processing order
896+
processRemovedDelta(
897+
routesDelta,
898+
managerTable_->routeManager(),
899+
lockPolicy,
900+
&SaiRouteManager::removeRoute<AddrT>,
901+
routerID);
902+
} else {
903+
processRemovedRoutesDeltaInReverse<LockPolicyT, AddrT>(
904+
routerID, routesDelta, lockPolicy);
905+
}
906+
}
907+
908+
template <typename LockPolicyT, typename AddrT>
909+
void SaiSwitch::processChangedAndAddedRoutesDelta(
910+
const RouterID& routerID,
911+
const auto& routesDelta,
912+
const LockPolicyT& lockPolicy) {
913+
if (!rollbackInProgress_) {
914+
// Normal processing order: changed first, then added
915+
processChangedDelta(
916+
routesDelta,
917+
managerTable_->routeManager(),
918+
lockPolicy,
919+
&SaiRouteManager::changeRoute<AddrT>,
920+
routerID);
921+
processAddedDelta(
922+
routesDelta,
923+
managerTable_->routeManager(),
924+
lockPolicy,
925+
&SaiRouteManager::addRoute<AddrT>,
926+
routerID);
927+
} else {
928+
processChangedRoutesDeltaInReverse<LockPolicyT, AddrT>(
929+
routerID, routesDelta, lockPolicy);
930+
processAddedRoutesDeltaInReverse<LockPolicyT, AddrT>(
931+
routerID, routesDelta, lockPolicy);
932+
}
933+
}
934+
889935
std::shared_ptr<SwitchState> SaiSwitch::stateChangedImpl(
890936
const std::vector<StateDelta>& deltas) {
891937
FineGrainedLockPolicy lockPolicy(saiSwitchMutex_);
@@ -948,18 +994,10 @@ std::shared_ptr<SwitchState> SaiSwitch::stateChangedImplLocked(
948994
for (const auto& routeDelta : fibInfoDelta.getFibsMapDelta()) {
949995
auto routerID = routeDelta.getOld() ? routeDelta.getOld()->getID()
950996
: routeDelta.getNew()->getID();
951-
processRemovedDelta(
952-
routeDelta.getFibDelta<folly::IPAddressV4>(),
953-
managerTable_->routeManager(),
954-
lockPolicy,
955-
&SaiRouteManager::removeRoute<folly::IPAddressV4>,
956-
routerID);
957-
processRemovedDelta(
958-
routeDelta.getFibDelta<folly::IPAddressV6>(),
959-
managerTable_->routeManager(),
960-
lockPolicy,
961-
&SaiRouteManager::removeRoute<folly::IPAddressV6>,
962-
routerID);
997+
processRemovedRoutesDelta<LockPolicyT, folly::IPAddressV6>(
998+
routerID, routeDelta.getFibDelta<folly::IPAddressV6>(), lockPolicy);
999+
processRemovedRoutesDelta<LockPolicyT, folly::IPAddressV4>(
1000+
routerID, routeDelta.getFibDelta<folly::IPAddressV4>(), lockPolicy);
9631001
}
9641002
}
9651003

@@ -1288,46 +1326,14 @@ std::shared_ptr<SwitchState> SaiSwitch::stateChangedImplLocked(
12881326
&SaiFdbManager::addMac);
12891327
}
12901328

1291-
auto processV4RoutesChangedAndAddedDelta =
1292-
[this, &lockPolicy](RouterID rid, const auto& routesDelta) {
1293-
processChangedDelta(
1294-
routesDelta,
1295-
managerTable_->routeManager(),
1296-
lockPolicy,
1297-
&SaiRouteManager::changeRoute<folly::IPAddressV4>,
1298-
rid);
1299-
processAddedDelta(
1300-
routesDelta,
1301-
managerTable_->routeManager(),
1302-
lockPolicy,
1303-
&SaiRouteManager::addRoute<folly::IPAddressV4>,
1304-
rid);
1305-
};
1306-
1307-
auto processV6RoutesChangedAndAddedDelta =
1308-
[this, &lockPolicy](RouterID rid, const auto& routesDelta) {
1309-
processChangedDelta(
1310-
routesDelta,
1311-
managerTable_->routeManager(),
1312-
lockPolicy,
1313-
&SaiRouteManager::changeRoute<folly::IPAddressV6>,
1314-
rid);
1315-
processAddedDelta(
1316-
routesDelta,
1317-
managerTable_->routeManager(),
1318-
lockPolicy,
1319-
&SaiRouteManager::addRoute<folly::IPAddressV6>,
1320-
rid);
1321-
};
1322-
13231329
for (const auto& fibInfoDelta : delta.getFibsInfoDelta()) {
13241330
for (const auto& routeDelta : fibInfoDelta.getFibsMapDelta()) {
13251331
auto routerID = routeDelta.getOld() ? routeDelta.getOld()->getID()
13261332
: routeDelta.getNew()->getID();
1327-
processV4RoutesChangedAndAddedDelta(
1328-
routerID, routeDelta.getFibDelta<folly::IPAddressV4>());
1329-
processV6RoutesChangedAndAddedDelta(
1330-
routerID, routeDelta.getFibDelta<folly::IPAddressV6>());
1333+
processChangedAndAddedRoutesDelta<LockPolicyT, folly::IPAddressV6>(
1334+
routerID, routeDelta.getFibDelta<folly::IPAddressV6>(), lockPolicy);
1335+
processChangedAndAddedRoutesDelta<LockPolicyT, folly::IPAddressV4>(
1336+
routerID, routeDelta.getFibDelta<folly::IPAddressV4>(), lockPolicy);
13311337
}
13321338
}
13331339
{

fboss/agent/hw/sai/switch/SaiSwitch.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,18 @@ class SaiSwitch : public HwSwitch {
695695
std::optional<cfg::PfcWatchdogRecoveryAction> recoveryAction);
696696
void setPortOwnershipToAdapter();
697697

698+
template <typename LockPolicyT, typename AddrT>
699+
void processRemovedRoutesDelta(
700+
const RouterID& routerID,
701+
const auto& routesDelta,
702+
const LockPolicyT& lockPolicy);
703+
704+
template <typename LockPolicyT, typename AddrT>
705+
void processChangedAndAddedRoutesDelta(
706+
const RouterID& routerID,
707+
const auto& routesDelta,
708+
const LockPolicyT& lockPolicy);
709+
698710
bool processVlanUntaggedPackets() const;
699711

700712
/* reconstruction state apis */

0 commit comments

Comments
 (0)