Skip to content

[fdb] More strict checking of FDB entry count#1126

Merged
prsunny merged 2 commits intosonic-net:masterfrom
wangxin:fdb-strict-check
Oct 8, 2019
Merged

[fdb] More strict checking of FDB entry count#1126
prsunny merged 2 commits intosonic-net:masterfrom
wangxin:fdb-strict-check

Conversation

@wangxin
Copy link
Collaborator

@wangxin wangxin commented Sep 21, 2019

Description of PR

Summary:
Fixes # (issue)
In the FdbConfigReloadTest, we observed that the SDK
FDB count and number of MACs in "show mac" output
may be 0. And the test still passed because of "0 == 0".

This fix is to have more strict checking of the FDB entry
number. If any of the FDB count is 0, fail the test.

What's more, the reason of 0 FDB count is that config
reloading takes longer than before. The PTF script has
stopped running when FDB was cleared during config
reloading. The fix is to increase the PTF script running
time from 200 seconds to 300 seconds.

The third change is to replace raw config reloading
command with calling common_tasks/reload_config.yml

Type of change

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

Approach

How did you do it?

  • Increae PTF running time from 200s to 300s.
  • Replace executing config reload command with calling a task to do config reload.
  • Check the number of FDB entries learned after reboot. Fail the test if the number is 0.

How did you verify/test it?

Tested on Mellanox platform

Any platform specific information?

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

Documentation

In the FdbConfigReloadTest, we observed that the SDK
FDB count and number of MACs in "show mac" output
may be 0. And the test still passed because of "0 == 0".

This fix is to have more strict checking of the FDB entry
number. If any of the FDB count is 0, fail the test.

What's more, the reason of 0 FDB count is that config
reloading takes longer than before. The PTF script has
stopped running when FDB was cleared during config
reloading. The fix is to increase the PTF script running
time from 200 seconds to 300 seconds.

The third change is to replace raw config reloading
command with calling common_tasks/reload_config.yml

Signed-off-by: Xin Wang <xinw@mellanox.com>
when: sdk_fdb_count.stdout == show_mac_output.stdout

- fail: msg="No FDB is learned, something wrong with the switch"
when: sdk_fdb_count.stdout | int == 0 or show_mac_output.stdout | int == 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Just thinking, after fdb clear, the count would be 0 until it learns next right? I don't see a sleep in between. This I believe can break the test intermittently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, config reloading would clear FDB. And then new FDB entries would be learned if the PTF script is still running. Currently the PTF script runs 300 seconds. After the PTF script is started, the config reload command is executed immediately. There is no need to sleep in between. If config reloading takes more than 300s to complete, the test will fail because of 0 FDB learned. In this case, the config reloading takes too long and is a problem that need to be fixed.

@prsunny prsunny merged commit 64ed241 into sonic-net:master Oct 8, 2019
@wangxin wangxin deleted the fdb-strict-check branch January 10, 2020 07:53
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
Update sonic-sairedis submodule pointer to include the following:
* 1d3720e Added Flex Counters support for tunnel counters (sonic-net#886) ([sonic-net#1120](sonic-net/sonic-sairedis#1120))
* 6469456 [lgtm] Add uuid library (sonic-net#1119) ([sonic-net#1126](sonic-net/sonic-sairedis#1126))
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.

3 participants