Skip to content

Commit

Permalink
Fix state machine handler race conditions introduced by strand::wrap (
Browse files Browse the repository at this point in the history
#257)

Approach
What is the motivation for this PR?
As the subject.

Work item tracking
Microsoft ADO (number only): 28585413
How did you do it?
Post the handler directly to handler instead of calling it with strand::wrap

How did you verify/test it?
UT

Any platform specific information?
Documentation
  • Loading branch information
lolyu authored and mssonicbld committed Aug 24, 2024
1 parent f7ef0fa commit 2725e7c
Show file tree
Hide file tree
Showing 10 changed files with 635 additions and 56 deletions.
60 changes: 24 additions & 36 deletions src/MuxPort.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,11 @@ void MuxPort::handleBladeIpv4AddressUpdate(boost::asio::ip::address address)
{
MUXLOGDEBUG(boost::format("port: %s") % mMuxPortConfig.getPortName());

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handleSwssBladeIpv4AddressUpdate,
mLinkManagerStateMachinePtr.get(),
address
)));
));
}

//
Expand All @@ -113,12 +112,11 @@ void MuxPort::handleSoCIpv4AddressUpdate(boost::asio::ip::address address)
{
MUXLOGDEBUG(boost::format("port: %s") % mMuxPortConfig.getPortName());

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handleSwssSoCIpv4AddressUpdate,
mLinkManagerStateMachinePtr.get(),
address
)));
));
}

//
Expand All @@ -135,12 +133,11 @@ void MuxPort::handleLinkState(const std::string &linkState)
label = link_state::LinkState::Label::Up;
}

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handleSwssLinkStateNotification,
mLinkManagerStateMachinePtr.get(),
label
)));
));
}

//
Expand All @@ -157,12 +154,11 @@ void MuxPort::handlePeerLinkState(const std::string &linkState)
label = link_state::LinkState::Label::Down;
}

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handlePeerLinkStateNotification,
mLinkManagerStateMachinePtr.get(),
label
)));
));
}

//
Expand All @@ -174,12 +170,11 @@ void MuxPort::handleGetServerMacAddress(const std::array<uint8_t, ETHER_ADDR_LEN
{
MUXLOGDEBUG(mMuxPortConfig.getPortName());

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handleGetServerMacAddressNotification,
mLinkManagerStateMachinePtr.get(),
address
)));
));
}

//
Expand All @@ -191,11 +186,10 @@ void MuxPort::handleUseWellKnownMacAddress()
{
MUXLOGDEBUG(mMuxPortConfig.getPortName());

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handleUseWellKnownMacAddressNotification,
mLinkManagerStateMachinePtr.get()
)));
));
}

//
Expand All @@ -207,11 +201,10 @@ void MuxPort::handleSrcMacAddressUpdate()
{
MUXLOGDEBUG(mMuxPortConfig.getPortName());

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handleSrcMacConfigNotification,
mLinkManagerStateMachinePtr.get()
)));
));
}

//
Expand All @@ -232,12 +225,11 @@ void MuxPort::handleGetMuxState(const std::string &muxState)
label = mux_state::MuxState::Label::Error;
}

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handleGetMuxStateNotification,
mLinkManagerStateMachinePtr.get(),
label
)));
));
}

//
Expand Down Expand Up @@ -289,12 +281,11 @@ void MuxPort::handleMuxState(const std::string &muxState)
label = mux_state::MuxState::Label::Error;
}

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handleMuxStateNotification,
mLinkManagerStateMachinePtr.get(),
label
)));
));
}

//
Expand All @@ -321,12 +312,11 @@ void MuxPort::handleMuxConfig(const std::string &config)
mode = common::MuxPortConfig::Detached;
}

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handleMuxConfigNotification,
mLinkManagerStateMachinePtr.get(),
mode
)));
));
}

//
Expand All @@ -347,12 +337,11 @@ void MuxPort::handlePeerMuxState(const std::string &peerMuxState)
label = mux_state::MuxState::Label::Error;
}

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handlePeerMuxStateNotification,
mLinkManagerStateMachinePtr.get(),
label
)));
));
}

//
Expand Down Expand Up @@ -441,12 +430,11 @@ void MuxPort::handleTsaEnable(bool enable)
mode = common::MuxPortConfig::Standby;
}

boost::asio::io_service &ioService = mStrand.context();
ioService.post(mStrand.wrap(boost::bind(
boost::asio::post(mStrand, boost::bind(
&link_manager::LinkManagerStateMachineBase::handleMuxConfigNotification,
mLinkManagerStateMachinePtr.get(),
mode
)));
));
}

} /* namespace mux */
9 changes: 9 additions & 0 deletions src/MuxPort.h
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,15 @@ class MuxPort: public std::enable_shared_from_this<MuxPort>
*/
void setComponentInitState(uint8_t component) {mLinkManagerStateMachinePtr->setComponentInitState(component);};

/**
*@method resetLinkManagerStateMachinePtr
*
*@brief reset the LinkManagerStateMachinePtr (used during unit test)
*
*@return none
*/
void resetLinkManagerStateMachinePtr(link_manager::LinkManagerStateMachineBase *stateMachinePtr) { mLinkManagerStateMachinePtr.reset(stateMachinePtr); };

private:
std::shared_ptr<mux::DbInterface> mDbInterfacePtr = nullptr;
common::MuxPortConfig mMuxPortConfig;
Expand Down
121 changes: 121 additions & 0 deletions test/FakeLinkManagerStateMachine.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Copyright 2021 (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <common/BoostAsioBehavior.h>
#include <boost/asio.hpp>
#include <boost/bind/bind.hpp>

#include "FakeLinkManagerStateMachine.h"
#include "common/MuxLogger.h"

namespace test
{

FakeLinkManagerStateMachine::FakeLinkManagerStateMachine(
mux::MuxPort *muxPortPtr,
boost::asio::io_service::strand &strand,
common::MuxPortConfig &muxPortConfig
) :
link_manager::LinkManagerStateMachineBase(
muxPortPtr,
strand,
muxPortConfig,
{link_prober::LinkProberState::Label::Wait,
mux_state::MuxState::Label::Wait,
link_state::LinkState::Label::Down}
)
{
assert(muxPortPtr != nullptr);
}

void FakeLinkManagerStateMachine::handleSwssBladeIpv4AddressUpdate(boost::asio::ip::address address)
{
MUXLOGDEBUG(getMuxPortConfig().getPortName());
mLastSwssBladeIpv4Address = address;
++mSwssBladeIpv4AddressUpdateHandlerCalled;
}

void FakeLinkManagerStateMachine::handleSwssSoCIpv4AddressUpdate(boost::asio::ip::address address)
{
MUXLOGDEBUG(getMuxPortConfig().getPortName());
mLastSwssSoCIpv4Address = address;
++mSwssSoCIpv4AddressUpdateHandlerCalled;
}

void FakeLinkManagerStateMachine::handleGetServerMacAddressNotification(std::array<uint8_t, ETHER_ADDR_LEN> address)
{
MUXLOGDEBUG(getMuxPortConfig().getPortName());
mLastServerMacAddress = address;
++mGetServerMacAddressNotificationHandlerCalled;
}

void FakeLinkManagerStateMachine::handleGetMuxStateNotification(mux_state::MuxState::Label label)
{
MUXLOGDEBUG(getMuxPortConfig().getPortName());
mLastGetMuxState = label;
++mGetMuxStateNotificationHandlerCalled;
}

void FakeLinkManagerStateMachine::handleProbeMuxStateNotification(mux_state::MuxState::Label label)
{
MUXLOGDEBUG(getMuxPortConfig().getPortName());
mLastProbeMuxState = label;
++mProbeMuxStateNotificationHandlerCalled;
}

void FakeLinkManagerStateMachine::handleMuxStateNotification(mux_state::MuxState::Label label)
{
MUXLOGDEBUG(getMuxPortConfig().getPortName());
mLastMuxStateNotification = label;
++mMuxStateNotificationHandlerCalled;
}

void FakeLinkManagerStateMachine::handleSwssLinkStateNotification(const link_state::LinkState::Label label)
{
MUXLOGDEBUG(getMuxPortConfig().getPortName());
mLastSwssLinkState = label;
++mSwssLinkStateNotificationHandlerCalled;
}

void FakeLinkManagerStateMachine::handlePeerLinkStateNotification(const link_state::LinkState::Label label)
{
MUXLOGDEBUG(getMuxPortConfig().getPortName());
mLastPeerLinkState = label;
++mPeerLinkStateNotificationHandlerCalled;
}

void FakeLinkManagerStateMachine::handlePeerMuxStateNotification(mux_state::MuxState::Label label)
{
MUXLOGDEBUG(getMuxPortConfig().getPortName());
mLastPeerMuxState = label;
++mPeerMuxStateNotificationHandlerCalled;
}

void FakeLinkManagerStateMachine::handleMuxConfigNotification(const common::MuxPortConfig::Mode mode)
{
MUXLOGDEBUG(getMuxPortConfig().getPortName());
mLastMuxConfig = mode;
++mMuxConfigNotificationHandlerCalled;
}

void FakeLinkManagerStateMachine::handleDefaultRouteStateNotification(const DefaultRoute routeState)
{
MUXLOGDEBUG(getMuxPortConfig().getPortName());
mLastDefaultRouteState = routeState;
++mDefaultRouteStateNotificationHandlerCalled;
}

} /* namespace test */
Loading

0 comments on commit 2725e7c

Please sign in to comment.