Revert D79321705

Summary:
This diff reverts D79321705
Depends on D91360194
Failing DSF scaling tests

Depends on D79321705

Differential Revision: D91360232

fbshipit-source-id: d7d318123a0cfda6f8b4e1762d99896e05cdef0b

Author: generatedunixname89002005232357

fboss/agent/BUCK

@@ -1317,7 +1317,6 @@ cpp_library(
         "//fboss/agent/rib:standalone_rib",
         "//fboss/agent/state:nodebase",
         "//fboss/agent/state:state",
-        "//fboss/agent/state:state_utils",
         "//fboss/lib:common_file_utils",
         "//fboss/lib:hw_write_behavior",
         "//folly:file_util",

fboss/agent/HwSwitch.cpp

@@ -18,7 +18,6 @@
 #include "fboss/agent/hw/switch_asics/HwAsic.h"
 #include "fboss/agent/rib/ForwardingInformationBaseUpdater.h"
 #include "fboss/agent/state/StateDelta.h"
-#include "fboss/agent/state/StateUtils.h"
 #include "fboss/agent/state/SwitchState.h"
 #include "fboss/agent/state/TransceiverMap.h"
 #include "fboss/lib/HwWriteBehavior.h"
@@ -290,10 +289,6 @@ void HwSwitch::preRollback(const StateDelta& /*delta*/) noexcept {
       << "Transactions is supported but rollback is implemented on this switch";
 }
 
-void HwSwitch::rollbackPartialRoutes(const StateDelta& /* delta */) noexcept {
-  XLOG(FATAL) << "rollbackPartialRoutes  is not implemented on this switch";
-}
-
 void HwSwitch::rollback(const std::vector<StateDelta>& /* deltas */) noexcept {
   XLOG(FATAL)
       << "Transactions is supported but rollback is implemented on this switch";
@@ -314,16 +309,6 @@ void HwSwitch::setProgrammedState(const std::shared_ptr<SwitchState>& state) {
   (*programmedState)->publish();
 }
 
-std::shared_ptr<SwitchState> HwSwitch::getIntermediateState() const {
-  auto intermediateState = intermediateState_.rlock();
-  return *intermediateState;
-}
-
-void HwSwitch::setIntermediateState(const std::shared_ptr<SwitchState>& state) {
-  auto intermediateState = intermediateState_.wlock();
-  *intermediateState = state;
-}
-
 fsdb::OperDelta HwSwitch::stateChanged(
     const std::vector<fsdb::OperDelta>& deltas,
     const HwWriteBehaviorRAII& /*behavior*/) {
@@ -361,38 +346,11 @@ fsdb::OperDelta HwSwitch::stateChangedTransaction(
   } catch (const FbossError& e) {
     XLOG(WARNING) << " Transaction failed with error : " << *e.message()
                   << " attempting rollback";
-
-    // If deltas were of size 10 and delta from 1-5 succeeded and 6th failed,
-    // HwSwitch state would still be goodKnownState but SaiSwitch would have
-    // the routes from 1-5 and the partial applied state from the 6th delta
-    // SaiRouteManager would read from HW to reconstruct FIB and then reverse
-    // apply the deltas to get back to known good state
-    //
-    // To understand next few lines of code
-    // Step 1: If 1-5 succeeded and 6 failed, then 5 is intermediateState and
-    //         5' is HW state. This step resets HW state from 5' -> 5.
-    XLOG(DBG2) << "Partially rollbacking HW routes";
-    auto hwState = this->constructSwitchStateWithFib();
-    auto intermediateState = getIntermediateState();
-    this->rollbackPartialRoutes(StateDelta(hwState, intermediateState));
-
-    // Step 2: 5 is the current HW state. We don't care about 6-10 anymore
-    //         We will generate a reverse state delta which would look like
-    //         [(5, 4), (4, 3), (3, 2), (2, 1)] from original vector
-    auto reversedDeltas = utility::computeReversedDeltas(
-        deltas, getProgrammedState(), getIntermediateState());
-
-    // Steps 3: finally insert the empty to intermediate state delta
-    // empty -> intermediate goes first, basically to reclaim objects after
-    // SaiStore re-init
-    // Refer to SaiSwitch::rollback for more context about using empty
-    // Final vector applied to HW would be
-    // [(empty, 5), (5, 4), (4, 3), (3, 2), (2, 1)]
-    reversedDeltas.emplace(
-        reversedDeltas.begin(),
-        std::make_shared<SwitchState>(),
-        intermediateState);
-    this->rollback(reversedDeltas);
+    auto delta = StateDelta(getProgrammedState(), deltas.front());
+    this->preRollback(delta);
+    std::vector<StateDelta> goodStateDeltas;
+    goodStateDeltas.emplace_back(getProgrammedState(), deltas.front());
+    this->rollback(goodStateDeltas);
     setProgrammedState(goodKnownState);
     return deltas.front();
   }

fboss/agent/HwSwitch.h

@@ -171,7 +171,6 @@ class HwSwitch {
       const HwWriteBehaviorRAII& behavior =
           HwWriteBehaviorRAII(HwWriteBehavior::WRITE));
   virtual void preRollback(const StateDelta& delta) noexcept;
-  virtual void rollbackPartialRoutes(const StateDelta& delta) noexcept;
   virtual void rollback(const std::vector<StateDelta>& deltas) noexcept;
   virtual std::shared_ptr<SwitchState> constructSwitchStateWithFib() noexcept;
 
@@ -400,7 +399,6 @@ class HwSwitch {
       folly::MacAddress mac) const = 0;
 
   std::shared_ptr<SwitchState> getProgrammedState() const;
-  std::shared_ptr<SwitchState> getIntermediateState() const;
   fsdb::OperDelta stateChanged(
       const std::vector<fsdb::OperDelta>& deltas,
       const HwWriteBehaviorRAII& behavior =
@@ -438,7 +436,6 @@ class HwSwitch {
 
  protected:
   void setProgrammedState(const std::shared_ptr<SwitchState>& state);
-  void setIntermediateState(const std::shared_ptr<SwitchState>& state);
 
  private:
   HwInitResult initLightImpl(Callback* callback, bool failHwCallsOnWarmboot);
@@ -479,7 +476,6 @@ class HwSwitch {
   std::optional<int64_t> switchId_;
 
   folly::Synchronized<std::shared_ptr<SwitchState>> programmedState_;
-  folly::Synchronized<std::shared_ptr<SwitchState>> intermediateState_;
 
   // Collecting phy Info is currently inefficient on some platforms. Instead of
   // collecting them every second, tune down the frequency to only collect once

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

@@ -722,56 +722,9 @@ void SaiSwitch::preRollback(const StateDelta& delta) noexcept {
   }
 }
 
-void SaiSwitch::rollbackPartialRoutes(const StateDelta& delta) noexcept {
-  // This API called before the actual rollback
-
-  // During state change, assuming there are state delta vector is of size 10
-  // If deltas were of size 10 and delta from 0-5 succeeded and 6th failed,
-  // HwSwitch state would still be goodKnownState but SaiSwitch would have
-  // the routes from 0-5 and the partial applied state from the 6th delta
-  // This API will be invoked to clear partially applied delta
-  try {
-    CoarseGrainedLockPolicy lockPolicy(saiSwitchMutex_);
-
-    for (const auto& fibInfoDelta : delta.getFibsInfoDelta()) {
-      for (const auto& routeDelta : fibInfoDelta.getFibsMapDelta()) {
-        auto routerID = routeDelta.getOld() ? routeDelta.getOld()->getID()
-                                            : routeDelta.getNew()->getID();
-        // v6 routes first since they go after v4 during normal operation
-        processRemovedRoutesDeltaInReverse<
-            CoarseGrainedLockPolicy,
-            folly::IPAddressV6>(
-            routerID, routeDelta.getFibDelta<folly::IPAddressV6>(), lockPolicy);
-        processChangedRoutesDeltaInReverse<
-            CoarseGrainedLockPolicy,
-            folly::IPAddressV6>(
-            routerID, routeDelta.getFibDelta<folly::IPAddressV6>(), lockPolicy);
-        processAddedRoutesDeltaInReverse<
-            CoarseGrainedLockPolicy,
-            folly::IPAddressV6>(
-            routerID, routeDelta.getFibDelta<folly::IPAddressV6>(), lockPolicy);
-
-        processRemovedRoutesDeltaInReverse<
-            CoarseGrainedLockPolicy,
-            folly::IPAddressV4>(
-            routerID, routeDelta.getFibDelta<folly::IPAddressV4>(), lockPolicy);
-        processChangedRoutesDeltaInReverse<
-            CoarseGrainedLockPolicy,
-            folly::IPAddressV4>(
-            routerID, routeDelta.getFibDelta<folly::IPAddressV4>(), lockPolicy);
-        processAddedRoutesDeltaInReverse<
-            CoarseGrainedLockPolicy,
-            folly::IPAddressV4>(
-            routerID, routeDelta.getFibDelta<folly::IPAddressV4>(), lockPolicy);
-      }
-    }
-  } catch (const std::exception& ex) {
-    // Partial route Rollback failed. Fail hard.
-    XLOG(FATAL) << " Partial route rollback failed with : " << ex.what();
-  }
-}
-
 void SaiSwitch::rollback(const std::vector<StateDelta>& deltas) noexcept {
+  CHECK_EQ(deltas.size(), 1);
+  const auto& knownGoodState = deltas.front().oldState();
   auto curBootType = getBootType();
   // Attempt rollback
   // Detailed design is in the sai_switch_transactions wiki, but at a high
@@ -825,9 +778,9 @@ void SaiSwitch::rollback(const std::vector<StateDelta>& deltas) noexcept {
         HwWriteBehavior::FAIL,
         &hwSwitchJson[kAdapterKeys],
         &hwSwitchJson[kAdapterKey2AdapterHostKey]);
-    for (const auto& delta : deltas) {
-      stateChangedImplLocked(delta, lockPolicy);
-    }
+    stateChangedImplLocked(
+        StateDelta(std::make_shared<SwitchState>(), knownGoodState),
+        lockPolicy);
     saiStore_->printWarmbootHandles();
     saiStore_->removeUnexpectedUnclaimedWarmbootHandles();
     bootType_ = curBootType;
@@ -894,7 +847,6 @@ std::shared_ptr<SwitchState> SaiSwitch::stateChangedImpl(
   if (deltas.size() == 0) {
     return getProgrammedState();
   }
-  setIntermediateState(getProgrammedState());
   int count = 0;
   std::shared_ptr<SwitchState> appliedState{nullptr};
   for (const auto& delta : deltas) {
@@ -906,8 +858,6 @@ std::shared_ptr<SwitchState> SaiSwitch::stateChangedImpl(
                  << deltas.size() << " deltas";
       return appliedState;
     }
-    // Save the current delta that applied cleanly
-    setIntermediateState(appliedState);
     count++;
   }
   return appliedState;
@@ -4999,8 +4949,9 @@ phy::FecMode SaiSwitch::getPortFECMode(PortID portId) const {
 }
 
 void SaiSwitch::rollbackInTest(const StateDelta& delta) {
+  preRollback(delta);
   std::vector<StateDelta> deltas;
-  deltas.emplace_back(std::make_shared<SwitchState>(), delta.oldState());
+  deltas.emplace_back(delta.oldState(), delta.newState());
   rollback(deltas);
   setProgrammedState(delta.oldState());
 }

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

@@ -304,7 +304,6 @@ class SaiSwitch : public HwSwitch {
       const StateDelta& delta,
       const LockPolicyT& lk);
   void preRollback(const StateDelta& delta) noexcept override;
-  void rollbackPartialRoutes(const StateDelta& delta) noexcept override;
   void rollback(const std::vector<StateDelta>& deltas) noexcept override;
   std::string listObjectsLocked(
       const std::vector<sai_object_type_t>& objects,