Skip to content
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.

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

[intfmgrd] reach reconciled state at start when there are no interfaces configuration to process #1695

Merged
merged 4 commits into from
Apr 7, 2021

Conversation

volodymyrsamotiy
Copy link
Collaborator

Signed-off-by: Stepan Blyschak [email protected]

What I did

If there is no pending configuration intfmgrd should reach reconciled state immediately. Same does for example vlanmgrd.

Why I did it

fdbsyncd waits till intfmgrd reaches reconciled state otherwise exits with exception about timeout. As a result sanity checkers complain about that.

How I verified it

I tested it on the switch by running warm-reboot and observed no fdbsyncd failure.
Also I wrote a simple test for intfmgrd warm restart when there are no interfaces in config_db.

Details if related

@prsunny
Copy link
Collaborator

prsunny commented Apr 6, 2021

@srj102 , @tapashdas , @nkelapur to review

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() );
}

void IntfMgr::setWarmReplayDoneState()
{
replayDone = true;
Copy link
Contributor

@nkelapur nkelapur Apr 6, 2021

Choose a reason for hiding this comment

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

please follow the class member naming convention now that "replayDone" is a member variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, fixed

Copy link
Contributor

@nkelapur nkelapur left a comment

Choose a reason for hiding this comment

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

Overall changes look good

Signed-off-by: Stepan Blyschak <[email protected]>
@prsunny
Copy link
Collaborator

prsunny commented Apr 7, 2021

@stepanblyschak , @nkelapur , this is required for 202012 branch, correct?

@nkelapur
Copy link
Contributor

nkelapur commented Apr 7, 2021

@stepanblyschak , @nkelapur , this is required for 202012 branch, correct?

Yes, this should be required for passing the sanity tests as mentioned by Stepan. However as I understand, in practical usage there will not be a scenario where there are no interfaces in the system, and hence the reported issue should not occur.

@prsunny prsunny merged commit 5c63670 into sonic-net:master Apr 7, 2021
@stepanblyschak
Copy link
Contributor

@nkelapur @prsunny This is required for 202012.
There can be a L2 config only deployment where there are no L3 interfaces in INTERFACE table, so it might be a production issue, not just a sanity check issue.

@dprital
Copy link
Collaborator

dprital commented Apr 7, 2021

@daall - Can you please merge to 202012 ?

@yxieca
Copy link
Contributor

yxieca commented Apr 8, 2021

@volodymyrsamotiy please create PR for 202012 branch as this change cannot be cherry-picked cleanly.

raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
…es configuration to process (sonic-net#1695)

* fix intfmgrd reconciliation when there are no interfaces in the system

Signed-off-by: Stepan Blyschak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants