[fdb] Add FDB learning during config reload test by wangxin · Pull Request #887 · sonic-net/sonic-mgmt

wangxin · 2019-04-24T06:34:14Z

Description of PR

Summary:
Fixes # (issue)

This case is to verify that all the FDB entries learned by SDK are correctly reported to upper layer in the initialization process after config reload.

The added case in this PR is to cover the issue fixed by SAI commit: Mellanox/SAI-Implementation@fc5a8dd

Steps performed by the scripts:

Find out the list of ports in VLAN
Find out the list of ports in PortChannel
Start a PTF script in asynchronous. Keep it running and move on to next ansible task immediately. Details of the PTF script:
- The PTF script keeps sending packets with unique src MAC address to different vlan ports and lag ports in interleaved way. The unique src MAC addresses have same pre-defined prefix.
- Pause 0.1 second after sending 8 packets (4 sent to VLAN ports, 4 sent to LAG ports, interleaved)
- Repeat the above steps for 200 seconds.
Immediately execute “sudo config reload -y” after the PTF script is started.
After config reload is completed (need around 90 seconds), keep waiting until the PTF script is completed executing.
Check the number of learned FDB entries from SDK:
- docker exec syncd sx_api_fdb_dump.py | grep {{ dummy_mac_prefix }} | wc -l
Check the number of learned FDB entries from “show mac”:
- show mac | grep {{ dummy_mac_prefix }} | wc -l
Compare the number of learned FDB entries:
- If SDK entries < “show mac” entries, issue reproduced
- If SDK entries == “show mac” entries, verified OK

With the current tuning (send for 200 seconds, 8 packets in a batch, send to VLAN ports and LAG ports in interleaved pattern, 0.1s interval between batches), the issue could be reproduced 4 times in 6 tests.

Type of change

[] Bug fix
[] Testbed and Framework(new/improvement)
Test case(new/improvement)

Approach

How did you do it?

Extended the existing FDB testing scripts. Please refer to the description for detailed steps.

How did you verify/test it?

Verified on Mellanox platform.

Any platform specific information?

The command for checking FDB entries learned from SDK is Mellanox specific. It takes nearly 4 minutes to run this added test case. There is no point wasting time run the case and skip verification step for other vendors. Because of this, I set the whole new test case Mellanox specific for now.

If other vendors consider this test case useful, they need little effort to refactor this test case to make just the step for counting FDB entries learned in SDK vendor specific.

Supported testbed topology if it's a new test case?

['t0', 't0-64', 't0-116']

Documentation

https://github.com/Azure/SONiC/wiki/FDB-Scale-Test-Plan
fdb_test_ansible.log

This case is to ensure that all the FDB entries learned by SDK are correctly reported to upper layer in the initialization process after config reload. Signed-off-by: Xin Wang <xinw@mellanox.com>

Signed-off-by: Xin Wang <xinw@mellanox.com>

wangxin · 2019-04-24T09:58:54Z

@qiluo-msft Could you please help review this?

stepanblyschak · 2019-04-24T15:57:56Z

ansible/roles/test/tasks/fdb.yml

+    pause: seconds=10
+
+  - name: Count FDB entries from SDK
+    shell: "docker exec syncd sx_api_fdb_dump.py | grep {{ dummy_mac_prefix }} | wc -l"


Why there is a sx_api_fdb_dump.py call without platform check?
sx_api*.py scripts are Mellanox SDK example/dump scripts and do not exist in other vendors syncd container

UPD: Ok, I see there is a platform check at the bottom but it is better to think of generic approach

It takes around 4 minutes to run this case. There is no point taking time to run the test and skip the SDK checking step for other vendors for now.

qiluo-msft · 2019-04-24T19:18:04Z

ansible/roles/test/tasks/fdb.yml

+    become: yes
+
+  - name: Wait until the PTF script completed execution
+    shell: "ps -ef | grep /usr/bin/ptf | grep -v grep"


ps -ef | grep /usr/bin/ptf | grep -v grep [](start = 12, length = 41)

Could you try wait pid or tail --pid=$pid -f /dev/null ? #Closed

OK, updated and comitted.

qiluo-msft · 2019-04-24T19:19:46Z

ansible/roles/test/tasks/fdb.yml

+    pause: seconds=10
+
+  - name: Count FDB entries from SDK
+    shell: "docker exec syncd sx_api_fdb_dump.py | grep {{ dummy_mac_prefix }} | wc -l"


grep {{ dummy_mac_prefix }} [](start = 51, length = 27)

In stead of testing only dummy mac, how about testing all AGEABLE(dynamic) mac? #Closed

In this new test case, PTF script doesn't inject packets with src MAC set to MAC of PTF ports. So, all AGEABLE MAC are the dummy MAC addresses. #Closed

I understand that all AGEABLE MAC are the dummy MAC addresses. So the two option are the effectively the same.

I am wondering the test purpose. Should we test all AGEABLE(dynamic) mac, no matter whether the MAC is generated with dummy mac or not?

In reply to: 278421108 [](ancestors = 278421108)

Got your point. Updated the script to count all AGEABLE MAC.

qiluo-msft · 2019-04-24T19:21:12Z

ansible/roles/test/tasks/fdb.yml

+
+  always:
+    - name: clear FDB table
+      command: sonic-clear fdb all


If the test failed, should we keep the scene and not clear FDB table? #Closed

OK, updated and comitted.

qiluo-msft

commented

* Use "tail --pid=<pid> -f /dev/null" to wait for PTF script to complete * Clear FDB only when test case pass Signed-off-by: Xin Wang <xinw@mellanox.com>

praveen-li · 2019-04-25T17:53:49Z

Fo many PRs, Test results are not available for even single successful run. I think we should make it mandatory to paste test results at least once per PR. [A general comment]. Thanks.

qiluo-msft · 2019-04-25T22:02:29Z

ansible/roles/test/tasks/fdb.yml

+    become: yes
+
+  - name: Get PID of the PTF script
+    shell: "ps -ef | grep /usr/bin/ptf | grep -v grep | awk '{print $2}'"


ps -ef | grep [](start = 12, length = 13)

pgrep is just for this purpose #Closed

Right, my method is silly. Updated with - command: pgrep -f '/usr/bin/python /usr/bin/ptf'. Thanks!

Signed-off-by: Xin Wang <xinw@mellanox.com>

wangxin · 2019-04-26T11:12:35Z

@praveen-li Attached ansible test result of this script.

praveen-li · 2019-04-26T15:51:52Z

Thanks Xin. We will follow this for each PR. Sent from PC's Mobile

…

________________________________ From: Xin Wang <notifications@github.com> Sent: Friday, April 26, 2019 4:12:39 AM To: Azure/sonic-mgmt Cc: Praveen Chaudhary; Mention Subject: Re: [Azure/sonic-mgmt] [fdb] Add FDB learning during config reload test (#887) @praveen-li<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpraveen-li&data=02%7C01%7Cpchaudhary%40linkedin.com%7Cfd3dfee603ce44b7da2008d6ca381c84%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636918739676592755&sdata=G%2FpFzH1XQWtkfbqzJiWZUaGqVOJukM11xS8bQlymIWA%3D&reserved=0> Attached ansible test result of this script. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzure%2Fsonic-mgmt%2Fpull%2F887%23issuecomment-487020899&data=02%7C01%7Cpchaudhary%40linkedin.com%7Cfd3dfee603ce44b7da2008d6ca381c84%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636918739676602728&sdata=t9B6szOIhShNSGeXRfttagLGTdQGBk7BAB%2BtMBz45KY%3D&reserved=0>, or mute the thread<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAJHZHPKKA6S32LPOFNKQIT3PSLPSPANCNFSM4HIAYRLQ&data=02%7C01%7Cpchaudhary%40linkedin.com%7Cfd3dfee603ce44b7da2008d6ca381c84%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636918739676602728&sdata=3Kp%2FNJAgrh9%2F1MeCdRXUNsHEp2YUvt9RjZnW9xVTaMY%3D&reserved=0>.

qiluo-msft · 2019-04-26T17:07:12Z

@stepanblyschak any more concern? I notice you 'requested changes' on this PR.

[Vxlan] : adding show vnet/vxlan cmds (sonic-net#880) [show][bgp] Use only 'show ip bgp' as the base and use bgp_frr_v4 file for FRR routing stack (sonic-net#884) [fast reboot] set a fast-reboot DB flag (sonic-net#887) [show] Add 'ip/ipv6 bgp network' commands (sonic-net#888)

Xin Wang added 2 commits April 24, 2019 14:22

[fdb] Add FDB learning during config reload test

e7adbb1

This case is to ensure that all the FDB entries learned by SDK are correctly reported to upper layer in the initialization process after config reload. Signed-off-by: Xin Wang <xinw@mellanox.com>

[fdb] Make the fdb testing upon config reload Mellanox specific

22b9006

Signed-off-by: Xin Wang <xinw@mellanox.com>

liat-grozovik requested a review from qiluo-msft April 24, 2019 14:57

liat-grozovik approved these changes Apr 24, 2019

View reviewed changes

stepanblyschak requested changes Apr 24, 2019

View reviewed changes

qiluo-msft reviewed Apr 24, 2019

View reviewed changes

[fdb] Update according to review comments

9492f5a

* Use "tail --pid=<pid> -f /dev/null" to wait for PTF script to complete * Clear FDB only when test case pass Signed-off-by: Xin Wang <xinw@mellanox.com>

qiluo-msft reviewed Apr 25, 2019

View reviewed changes

Xin Wang added 2 commits April 26, 2019 18:48

[fdb] Use pgrep to find out pid of the PTF script

7034e6b

Signed-off-by: Xin Wang <xinw@mellanox.com>

[fdb] Count all AGEABLE(dynamic) MAC addresses, not just dummy MAC

9628d07

Signed-off-by: Xin Wang <xinw@mellanox.com>

qiluo-msft approved these changes Apr 26, 2019

View reviewed changes

stepanblyschak approved these changes Apr 30, 2019

View reviewed changes

liat-grozovik merged commit c5e6a93 into sonic-net:master May 1, 2019

wangxin deleted the fdb-issue-pr branch May 24, 2019 03:32

Conversation

wangxin commented Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Approach

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

wangxin commented Apr 24, 2019

Uh oh!

stepanblyschak Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wangxin Apr 25, 2019

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wangxin Apr 25, 2019

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wangxin Apr 25, 2019 • edited by qiluo-msft Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Apr 25, 2019

Choose a reason for hiding this comment

Uh oh!

wangxin Apr 26, 2019

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wangxin Apr 25, 2019

Choose a reason for hiding this comment

Uh oh!

qiluo-msft left a comment

Choose a reason for hiding this comment

Uh oh!

praveen-li commented Apr 25, 2019

Uh oh!

qiluo-msft Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wangxin Apr 26, 2019

Choose a reason for hiding this comment

Uh oh!

wangxin commented Apr 26, 2019

Uh oh!

praveen-li commented Apr 26, 2019 via email

Uh oh!

qiluo-msft commented Apr 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wangxin commented Apr 24, 2019 •

edited

Loading

stepanblyschak Apr 24, 2019 •

edited

Loading

qiluo-msft Apr 24, 2019 •

edited

Loading

qiluo-msft Apr 24, 2019 •

edited

Loading

wangxin Apr 25, 2019 •

edited by qiluo-msft

Loading

qiluo-msft Apr 24, 2019 •

edited

Loading

qiluo-msft Apr 25, 2019 •

edited

Loading