Fix HW Write Behavior for Multiswitch Agent HW Test Runs

Summary: To properly integrate zero SDK check with agent hw testing in multiswitch mode, I fetch the HwWriteBehavior from StateOperDelta in HwAgent and set the HW write behavior RAII object before applying initial state. Reviewed By: zechengh09 Differential Revision: D56329330 Privacy Context Container: L1125642 fbshipit-source-id: efe081f70d7301506fb24e20ab34f704754e96ed
codyohl · May 30, 2024 · 9117ee4 · 9117ee4
1 parent ed0d4c6
commit 9117ee4
Show file tree

Hide file tree

Showing 9 changed files with 36 additions and 20 deletions.
diff --git a/fboss/agent/SwSwitch.cpp b/fboss/agent/SwSwitch.cpp
@@ -1194,7 +1194,7 @@ void SwSwitch::init(
       this, std::move(tunMgr), hwSwitchInitFn, HwWriteBehavior::WRITE, flags);
 }
 
-void SwSwitch::init(SwitchFlags flags) {
+void SwSwitch::init(const HwWriteBehavior& hwWriteBehavior, SwitchFlags flags) {
   /* used for split Software Switch init */
   auto initialState = preInit(flags);
   initialState->publish();
@@ -1216,7 +1216,8 @@ void SwSwitch::init(SwitchFlags flags) {
   // deleting any cold boot state entries that hwswitch has learned from sdk
   if (bootType_ == BootType::WARM_BOOT) {
     try {
-      getHwSwitchHandler()->stateChanged(initialStateDelta, false);
+      getHwSwitchHandler()->stateChanged(
+          initialStateDelta, false, hwWriteBehavior);
     } catch (const std::exception& ex) {
       throw FbossError("Failed to sync initial state to HwSwitch: ", ex.what());
     }

diff --git a/fboss/agent/SwSwitch.h b/fboss/agent/SwSwitch.h
@@ -244,7 +244,9 @@ class SwSwitch : public HwSwitchCallback {
       const HwWriteBehavior& hwWriteBehavior,
       SwitchFlags flags = SwitchFlags::DEFAULT);
 
-  void init(SwitchFlags flags = SwitchFlags::DEFAULT);
+  void init(
+      const HwWriteBehavior& hwWriteBehavior,
+      SwitchFlags flags = SwitchFlags::DEFAULT);
 
   bool isFullyInitialized() const;
 

diff --git a/fboss/agent/mnpu/MultiSwitchHwSwitchHandler.cpp b/fboss/agent/mnpu/MultiSwitchHwSwitchHandler.cpp
@@ -94,7 +94,7 @@ MultiSwitchHwSwitchHandler::stateChanged(
     const fsdb::OperDelta& delta,
     bool transaction,
     const std::shared_ptr<SwitchState>& newState,
-    const HwWriteBehavior& /* hwWriteBehavior */) {
+    const HwWriteBehavior& hwWriteBehavior) {
   multiswitch::StateOperDelta stateDelta;
   {
     std::unique_lock<std::mutex> lk(stateUpdateMutex_);
@@ -126,7 +126,8 @@ MultiSwitchHwSwitchHandler::stateChanged(
         prevUpdateSwitchState_,
         delta,
         transaction,
-        currOperDeltaSeqNum_);
+        currOperDeltaSeqNum_,
+        hwWriteBehavior);
     ++currOperDeltaSeqNum_;
     nextOperDelta_ = &stateDelta;
   }
@@ -327,7 +328,8 @@ void MultiSwitchHwSwitchHandler::fillMultiswitchOperDelta(
     const std::shared_ptr<SwitchState>& state,
     const fsdb::OperDelta& delta,
     bool transaction,
-    int64_t lastSeqNum) {
+    int64_t lastSeqNum,
+    const HwWriteBehavior& hwWriteBehavior) {
   // Send full delta if this is first switchstate update.
   // Sequence number 0 indicates first update
   if (lastSeqNum == 0) {
@@ -340,6 +342,7 @@ void MultiSwitchHwSwitchHandler::fillMultiswitchOperDelta(
   }
   stateDelta.transaction() = transaction;
   stateDelta.seqNum() = lastSeqNum + 1;
+  stateDelta.hwWriteBehavior() = hwWriteBehavior;
 }
 
 void MultiSwitchHwSwitchHandler::operDeltaAckTimeout() {

diff --git a/fboss/agent/mnpu/MultiSwitchHwSwitchHandler.h b/fboss/agent/mnpu/MultiSwitchHwSwitchHandler.h
@@ -89,7 +89,8 @@ class MultiSwitchHwSwitchHandler : public HwSwitchHandler {
       const std::shared_ptr<SwitchState>& state,
       const fsdb::OperDelta& delta,
       bool transaction,
-      int64_t lastSeqNum);
+      int64_t lastSeqNum,
+      const HwWriteBehavior& hwWriteBehavior = HwWriteBehavior::WRITE);
   void operDeltaAckTimeout();
 
   SwSwitch* sw_;

diff --git a/fboss/agent/mnpu/OperDeltaSyncer.cpp b/fboss/agent/mnpu/OperDeltaSyncer.cpp
@@ -111,12 +111,17 @@ void OperDeltaSyncer::operSyncLoop() {
         stateOperDelta.operDelta()->changes()->size()) {
       if (*stateOperDelta.isFullState()) {
         XLOG(DBG2) << "Received full state oper delta from swswitch";
-        lastUpdateResult = processFullOperDelta(*stateOperDelta.operDelta());
+        lastUpdateResult = processFullOperDelta(
+            *stateOperDelta.operDelta(), *stateOperDelta.hwWriteBehavior());
       } else {
         auto oldState = hw_->getProgrammedState();
         lastUpdateResult = stateOperDelta.transaction().value()
-            ? hw_->stateChangedTransaction(*stateOperDelta.operDelta())
-            : hw_->stateChanged(*stateOperDelta.operDelta());
+            ? hw_->stateChangedTransaction(
+                  *stateOperDelta.operDelta(),
+                  HwWriteBehaviorRAII(*stateOperDelta.hwWriteBehavior()))
+            : hw_->stateChanged(
+                  *stateOperDelta.operDelta(),
+                  HwWriteBehaviorRAII(*stateOperDelta.hwWriteBehavior()));
         if (lastUpdateResult.changes()->empty()) {
           hw_->getPlatform()->stateChanged(
               StateDelta(oldState, hw_->getProgrammedState()));
@@ -137,12 +142,14 @@ void OperDeltaSyncer::operSyncLoop() {
 }
 
 fsdb::OperDelta OperDeltaSyncer::processFullOperDelta(
-    fsdb::OperDelta& operDelta) {
+    fsdb::OperDelta& operDelta,
+    const HwWriteBehavior& hwWriteBehavior) {
   // Enable deep comparison for full oper delta
   DeltaComparison::PolicyRAII policyGuard{DeltaComparison::Policy::DEEP};
   auto fullStateDelta = StateDelta(std::make_shared<SwitchState>(), operDelta);
   auto delta = StateDelta(hw_->getProgrammedState(), fullStateDelta.newState());
-  auto appliedState = hw_->stateChanged(delta);
+  auto appliedState =
+      hw_->stateChanged(delta, HwWriteBehaviorRAII(hwWriteBehavior));
   // return empty oper delta to indicate success. If update was not successful,
   // hwswitch would have crashed.
   CHECK(isStateDeltaEmpty(StateDelta(fullStateDelta.newState(), appliedState)));

diff --git a/fboss/agent/mnpu/OperDeltaSyncer.h b/fboss/agent/mnpu/OperDeltaSyncer.h
@@ -29,7 +29,9 @@ class OperDeltaSyncer {
  private:
   void initOperDeltaSync();
   void operSyncLoop();
-  fsdb::OperDelta processFullOperDelta(fsdb::OperDelta& operDelta);
+  fsdb::OperDelta processFullOperDelta(
+      fsdb::OperDelta& operDelta,
+      const HwWriteBehavior& hwWriteBehavior = HwWriteBehavior::WRITE);
 
   uint16_t serverPort_;
   SwitchID switchId_;

diff --git a/fboss/agent/mnpu/SplitSwAgentInitializer.cpp b/fboss/agent/mnpu/SplitSwAgentInitializer.cpp
@@ -18,9 +18,9 @@ namespace facebook::fboss {
 
 void SplitSwSwitchInitializer::initImpl(
     HwSwitchCallback* /* callback */,
-    const HwWriteBehavior& /* hwWriteBehavior */) {
+    const HwWriteBehavior& hwWriteBehavior) {
   // this blocks until at least one hardware switch is up
-  sw_->init(setupFlags());
+  sw_->init(hwWriteBehavior, setupFlags());
 }
 
 SplitSwAgentInitializer::SplitSwAgentInitializer() {

diff --git a/fboss/agent/test/SwitchHandlerTest.cpp b/fboss/agent/test/SwitchHandlerTest.cpp
@@ -184,7 +184,7 @@ TEST_F(SwSwitchHandlerTest, partialUpdateAndFullSync) {
   auto delta3 = StateDelta(stateV0, stateV2);
 
   getHwSwitchHandler()->connected(SwitchID(1));
-  sw_->init(SwitchFlags::DEFAULT);
+  sw_->init(HwWriteBehavior::WRITE, SwitchFlags::DEFAULT);
   sw_->initialConfigApplied(std::chrono::steady_clock::now());
   getHwSwitchHandler()->stateChanged(
       StateDelta(std::make_shared<SwitchState>(), sw_->getState()), false);
@@ -396,7 +396,7 @@ TEST_F(SwSwitchHandlerTest, reconnectingHwSwitch) {
 
   getHwSwitchHandler()->connected(SwitchID(1));
   getHwSwitchHandler()->connected(SwitchID(2));
-  sw_->init(SwitchFlags::DEFAULT);
+  sw_->init(HwWriteBehavior::WRITE, SwitchFlags::DEFAULT);
   sw_->initialConfigApplied(std::chrono::steady_clock::now());
   getHwSwitchHandler()->stateChanged(
       StateDelta(std::make_shared<SwitchState>(), sw_->getState()), false);
@@ -657,7 +657,7 @@ TEST_F(SwSwitchHandlerTest, initialSync) {
   folly::Baton<> client2Baton;
   auto sw = sw_.get();
   getHwSwitchHandler()->connected(SwitchID(1));
-  sw_->init(SwitchFlags::DEFAULT);
+  sw_->init(HwWriteBehavior::WRITE, SwitchFlags::DEFAULT);
   getHwSwitchHandler()->stateChanged(
       StateDelta(std::make_shared<SwitchState>(), sw->getState()), false);
   sw_->initialConfigApplied(std::chrono::steady_clock::now());
@@ -738,7 +738,7 @@ TEST_F(SwSwitchHandlerTest, initialSyncSwSwitchNotConfigured) {
   std::thread clientRequestThread1([&]() { clientThreadBody(1); });
   std::thread clientRequestThread2([&]() { clientThreadBody(2); });
   getHwSwitchHandler()->waitUntilHwSwitchConnected();
-  sw_->init(SwitchFlags::DEFAULT);
+  sw_->init(HwWriteBehavior::WRITE, SwitchFlags::DEFAULT);
   getHwSwitchHandler()->stateChanged(
       StateDelta(std::make_shared<SwitchState>(), sw->getState()), false);
   sw_->initialConfigApplied(std::chrono::steady_clock::now());

diff --git a/fboss/agent/test/TestUtils.cpp b/fboss/agent/test/TestUtils.cpp
@@ -708,7 +708,7 @@ std::unique_ptr<SwSwitch> setupMockSwitchWithoutHW(
       std::make_pair<std::string, std::string>("multi_switch", "true");
   thrift.defaultCommandLineArgs()->emplace(std::move(multiSwitch));
   swSwitch->setConfig(std::make_unique<AgentConfig>(thrift));
-  swSwitch->init(SwitchFlags::DEFAULT);
+  swSwitch->init(HwWriteBehavior::WRITE, SwitchFlags::DEFAULT);
   swSwitch->applyConfig("initial config", *config);
   swSwitch->initialConfigApplied(std::chrono::steady_clock::now());
   return swSwitch;