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

vlanmgr changes related to EVPN VxLan warmboot #1460

Merged
merged 9 commits into from
Dec 27, 2020
Merged

vlanmgr changes related to EVPN VxLan warmboot #1460

merged 9 commits into from
Dec 27, 2020

Conversation

anilkpan
Copy link
Contributor

@anilkpan anilkpan commented Oct 9, 2020

This PR is for vlanmgr changes related to EVPN VxLan warmboot.

Please refer to the HLD of EVPN VxLan for details.
sonic-net/SONiC#437

@prsunny
Copy link
Collaborator

prsunny commented Oct 21, 2020

@heidinet2007 to review

@lguohan lguohan added the evpn label Nov 4, 2020
@@ -282,6 +302,7 @@ void VlanMgr::doVlanTask(Consumer &consumer)
if (isVlanStateOk(key) && m_vlans.find(key) == m_vlans.end())
{
m_vlans.insert(key);
m_vlanReplay.erase(kfvKey(t));
Copy link
Contributor

@qiluo-msft qiluo-msft Nov 7, 2020

Choose a reason for hiding this comment

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

kfvKey(t) [](start = 35, length = 9)

erase it directly? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@qiluo-msft sorry, I didn't understand, here I am erasing the key from the set m_vlanReplay, what do you mean by erase 'it' directly?

@@ -292,6 +313,7 @@ void VlanMgr::doVlanTask(Consumer &consumer)
{
addHostVlan(vlan_id);
}
m_vlanReplay.erase(kfvKey(t));
Copy link
Contributor

@qiluo-msft qiluo-msft Nov 7, 2020

Choose a reason for hiding this comment

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

kfvKey(t) [](start = 31, length = 9)

The same #Closed

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

It's look good to me. Not the domain expert, please check with others for review comments.

@anilkpan
Copy link
Contributor Author

retest this please

@anilkpan
Copy link
Contributor Author

retest this please

WarmStart::isWarmStart())
{
replayDone = true;
WarmStart::setWarmStartState("vlanmgrd", WarmStart::RECONCILED);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this change from REPLAYED?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prsunny test_VlanMgrdWarmRestart was failing as it expected the state to be RECONCILED. I understand now that as per HLD it is supposed to be REPLAYED. I will discuss with team members and check on how to handle this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prsunny I will change the existing pytest script to match the EVPN HLD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prsunny I will change the existing pytest to expect REPLAYED instead of RECONCILED to match the EVPN HLD

@@ -437,7 +437,8 @@ def test_VlanMgrdWarmRestart(self, dvs, testlog):
(status, fvs) = tbl.get("Vlan20:11.0.0.11")
assert status == True

swss_app_check_RestoreCount_single(state_db, restore_count, "vlanmgrd")
swss_check_RestoreCount(dvs, state_db, restore_count)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@qiluo-msft , could you review this?

Copy link
Contributor

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are more dependencies on the Reconciled state in the vs tests, I will check and fix.

@srj102
Copy link
Contributor

srj102 commented Dec 9, 2020

retest this please

@prsunny prsunny merged commit 6eb36d9 into sonic-net:master Dec 27, 2020
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
…eboot (sonic-net#1460)

Check if any warm restart flag is set when issuing a warm-reboot. This check avoids starting a warm reboot while another warm restart is in progress. In the scenario where a warm reboot is issued with another warm restart in progress, the warm restart flag may be reset and part of the components have a risk of doing cold reboot.
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
* mclagsyncd enhancements as per HLD at sonic-net/SONiC#596

* addressed LGTM alert

* UT Fix unique IP configuration

* modified ip address validate function for mclag config verication

* Add soft-reboot reboot type (sonic-net#1453)

What I did
Add a new reboot named as soft-reboot which can be performed by "kexec -e"

How I did it
Replace the platform reboot with "kexec -e" for the cold reboot case.

How to verify it
Verified the reboot on DUT and check the reboot-cause

* [warm-reboot] Check if warm restart flag is set when issuing a warm-reboot (sonic-net#1460)

Check if any warm restart flag is set when issuing a warm-reboot. This check avoids starting a warm reboot while another warm restart is in progress. In the scenario where a warm reboot is issued with another warm restart in progress, the warm restart flag may be reset and part of the components have a risk of doing cold reboot.

* Added mclag config commands

* removed unwanted imports

* added mclag tests

* fixed build issue

* corrected mclag test

* corrected mclag test

* corrected mclag test case

* updated testcase for mclag

* updated mclag config

* updated mclag test cases

* updated mclag test case

* updated mclag test cases

* fixed alert

* updated mclag test cases

* updated mclag test cases

* updated mclag config

* modified mclag test cases

* updated mclag test case

* updated mclag test case

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test case

* updated mclag test case

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test case

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test cases

* updated mclag test case

* updated mclag test cases

* updated mclag test case

* updated mclag config to use swsscommon instead of swssdk

* updated mclag config to use swsscommon

* updated mclag config script file

* fixed mclag test cases to verify config db

* updated mclag test case with config db verify function

* fixed build issue

* updated test case

* updated mclag test case

* addressed review comments

Co-authored-by: Tapash Das <tapash.das@broadcom.com>
Co-authored-by: Tapash Das <48195098+tapashdas@users.noreply.github.com>
Co-authored-by: Sujin Kang <sujkang@microsoft.com>
Co-authored-by: Shi Su <67605788+shi-su@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants