Skip to content

[Evpn Warmreboot] Added Dependancy check logic in VrfMgr #1466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
prsunny merged 9 commits into from
Jan 8, 2021
Merged

[Evpn Warmreboot] Added Dependancy check logic in VrfMgr #1466

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3983fc1
[Evpn Warmreboot] Added Dependancy check logic in VrfMgr
nkelapur Oct 13, 2020
1c92c5f
[Evpn Warmreboot] Added Dependancy check logic in IntfMgr
nkelapur Oct 13, 2020
bceaae7
[Evpn Warmreboot] Added Dependancy check logic in IntfMgr
nkelapur Oct 13, 2020
6967f51
Merge branch 'evpn_warmboot' of https://github.com/nkelapur/sonic-sws…
nkelapur Oct 14, 2020
1d6e33e
[Evpn Warmreboot] fix for UT
nkelapur Oct 14, 2020
cdd5f7a
[Evpn Warmreboot] fix for UT
nkelapur Oct 14, 2020
a1845b7
[evpn_warmreboot] Fixed warmreboot UT
nkelapur Oct 15, 2020
1614c86
Marking Vrfmgrd and Intfmgrd to reconcile immediately after replay
nkelapur Dec 18, 2020
9d64a37
[evpn warmboot] Addressing Review comments
nkelapur Dec 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions cfgmgr/intfmgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ using namespace swss;

IntfMgr::IntfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const vector<string> &tableNames) :
Orch(cfgDb, tableNames),
m_cfgIntfTable(cfgDb, CFG_INTF_TABLE_NAME),
m_cfgVlanIntfTable(cfgDb, CFG_VLAN_INTF_TABLE_NAME),
m_cfgLagIntfTable(cfgDb, CFG_LAG_INTF_TABLE_NAME),
m_cfgLoopbackIntfTable(cfgDb, CFG_LOOPBACK_INTERFACE_TABLE_NAME),
m_statePortTable(stateDb, STATE_PORT_TABLE_NAME),
m_stateLagTable(stateDb, STATE_LAG_TABLE_NAME),
m_stateVlanTable(stateDb, STATE_VLAN_TABLE_NAME),
Expand All @@ -34,6 +38,12 @@ IntfMgr::IntfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
if (!WarmStart::isWarmStart())
{
flushLoopbackIntfs();
WarmStart::setWarmStartState("intfmgrd", WarmStart::WSDISABLED);
}
else
{
//Build the interface list to be replayed to Kernel
buildIntfReplayList();
}
}

Expand Down Expand Up @@ -172,6 +182,25 @@ int IntfMgr::getIntfIpCount(const string &alias)
return std::stoi(res);
}

void IntfMgr::buildIntfReplayList(void)
{
vector<string> intfList;

m_cfgIntfTable.getKeys(intfList);
std::copy( intfList.begin(), intfList.end(), std::inserter( m_pendingReplayIntfList, m_pendingReplayIntfList.end() ) );

m_cfgLoopbackIntfTable.getKeys(intfList);
std::copy( intfList.begin(), intfList.end(), std::inserter( m_pendingReplayIntfList, m_pendingReplayIntfList.end() ) );

m_cfgVlanIntfTable.getKeys(intfList);
std::copy( intfList.begin(), intfList.end(), std::inserter( m_pendingReplayIntfList, m_pendingReplayIntfList.end() ) );

m_cfgLagIntfTable.getKeys(intfList);
std::copy( intfList.begin(), intfList.end(), std::inserter( m_pendingReplayIntfList, m_pendingReplayIntfList.end() ) );

SWSS_LOG_INFO("Found %d Total Intfs to be replayed", (int)m_pendingReplayIntfList.size() );
}

bool IntfMgr::isIntfCreated(const string &alias)
{
vector<FieldValueTuple> temp;
Expand Down Expand Up @@ -626,6 +655,7 @@ bool IntfMgr::doIntfAddrTask(const vector<string>& keys,
void IntfMgr::doTask(Consumer &consumer)
{
SWSS_LOG_ENTER();
static bool replayNotDone = true;
Copy link
Copy Markdown
Collaborator

@prsunny prsunny Dec 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please rename to "replayDone" and set to false.

Copy link
Copy Markdown
Contributor Author

@nkelapur nkelapur Dec 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure .. will change it


auto it = consumer.m_toSync.begin();
while (it != consumer.m_toSync.end())
Expand All @@ -643,6 +673,11 @@ void IntfMgr::doTask(Consumer &consumer)
it++;
continue;
}
else
{
//Entry programmed, remove it from pending list if present
m_pendingReplayIntfList.erase(keys[0]);
}
}
else if (keys.size() == 2)
{
Expand All @@ -651,6 +686,11 @@ void IntfMgr::doTask(Consumer &consumer)
it++;
continue;
}
else
{
//Entry programmed, remove it from pending list if present
m_pendingReplayIntfList.erase(keys[0] + config_db_key_delimiter + keys[1] );
}
}
else
{
Expand All @@ -659,4 +699,13 @@ void IntfMgr::doTask(Consumer &consumer)

it = consumer.m_toSync.erase(it);
}

if (replayNotDone && WarmStart::isWarmStart() && m_pendingReplayIntfList.empty() )
{
replayNotDone = false;
WarmStart::setWarmStartState("intfmgrd", WarmStart::REPLAYED);
// There is no operation to be performed for intfmgr reconcillation
// Hence mark it reconciled right away
WarmStart::setWarmStartState("intfmgrd", WarmStart::RECONCILED);
}
}
3 changes: 3 additions & 0 deletions cfgmgr/intfmgr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ class IntfMgr : public Orch

private:
ProducerStateTable m_appIntfTableProducer;
Table m_cfgIntfTable, m_cfgVlanIntfTable, m_cfgLagIntfTable, m_cfgLoopbackIntfTable;
Table m_statePortTable, m_stateLagTable, m_stateVlanTable, m_stateVrfTable, m_stateIntfTable;

std::set<std::string> m_subIntfList;
std::set<std::string> m_loopbackIntfList;
std::set<std::string> m_pendingReplayIntfList;

void setIntfIp(const std::string &alias, const std::string &opCmd, const IpPrefix &ipPrefix);
void setIntfVrf(const std::string &alias, const std::string &vrfName);
Expand All @@ -36,6 +38,7 @@ class IntfMgr : public Orch
bool isIntfCreated(const std::string &alias);
bool isIntfChangeVrf(const std::string &alias, const std::string &vrfName);
int getIntfIpCount(const std::string &alias);
void buildIntfReplayList(void);

void addLoopbackIntf(const std::string &alias);
void delLoopbackIntf(const std::string &alias);
Expand Down
5 changes: 5 additions & 0 deletions cfgmgr/vrfmgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ VrfMgr::VrfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, con
<< IP_CMD << " -6 rule add pref " << TABLE_LOCAL_PREF << " table local && " << IP_CMD << " -6 rule del pref 0";
EXEC_WITH_ERROR_THROW(cmd.str(), res);
}

if (!WarmStart::isWarmStart())
{
WarmStart::setWarmStartState("vrfmgrd", WarmStart::WSDISABLED);
}
}

uint32_t VrfMgr::getFreeTable(void)
Expand Down
12 changes: 12 additions & 0 deletions cfgmgr/vrfmgrd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ mutex gDbMutex;
int main(int argc, char **argv)
{
Logger::linkToDbNative("vrfmgrd");
bool isWarmStart = false;
SWSS_LOG_ENTER();

SWSS_LOG_NOTICE("--- Starting vrfmgrd ---");
Expand All @@ -55,6 +56,8 @@ int main(int argc, char **argv)

VrfMgr vrfmgr(&cfgDb, &appDb, &stateDb, cfg_vrf_tables);

isWarmStart = WarmStart::isWarmStart();

// TODO: add tables in stateDB which interface depends on to monitor list
std::vector<Orch *> cfgOrchList = {&vrfmgr};

Expand All @@ -68,6 +71,7 @@ int main(int argc, char **argv)
while (true)
{
Selectable *sel;
static bool firstReadTimeout = true;
int ret;

ret = s.select(&sel, SELECT_TIMEOUT);
Expand All @@ -79,6 +83,14 @@ int main(int argc, char **argv)
if (ret == Select::TIMEOUT)
{
vrfmgr.doTask();
if (isWarmStart && firstReadTimeout)
{
firstReadTimeout = false;
WarmStart::setWarmStartState("vrfmgrd", WarmStart::REPLAYED);
// There is no operation to be performed for vrfmgrd reconcillation
// Hence mark it reconciled right away
WarmStart::setWarmStartState("vrfmgrd", WarmStart::RECONCILED);
}
continue;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/test_warm_reboot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def swss_check_RestoreCount(dvs, state_db, restore_count):
if fv[0] == "restore_count":
assert int(fv[1]) == restore_count[key] + 1
elif fv[0] == "state":
assert fv[1] == "reconciled"
assert fv[1] == "reconciled" or fv[1] == "disabled"
Copy link
Copy Markdown
Collaborator

@prsunny prsunny Dec 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to check for "disabled"? If not, please remove

Copy link
Copy Markdown
Contributor Author

@nkelapur nkelapur Dec 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we still need "disabled" to handle the cold restart scenario, specially for docker restart


def check_port_oper_status(appl_db, port_name, state):
portTbl = swsscommon.Table(appl_db, swsscommon.APP_PORT_TABLE_NAME)
Expand Down Expand Up @@ -76,7 +76,7 @@ def swss_app_check_RestoreCount_single(state_db, restore_count, name):
if fv[0] == "restore_count":
assert int(fv[1]) == restore_count[key] + 1
elif fv[0] == "state":
assert fv[1] == "reconciled"
assert fv[1] == "reconciled" or fv[1] == "disabled"

def swss_app_check_warmstart_state(state_db, name, state):
warmtbl = swsscommon.Table(state_db, swsscommon.STATE_WARM_RESTART_TABLE_NAME)
Expand Down