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

[pytest/hash] Enhance hash test #1494

Closed
wants to merge 6 commits into from
Closed

Conversation

William-zx
Copy link
Contributor

@William-zx William-zx commented Mar 27, 2020

Description of PR

Summary:
Made the following change:

  1. Rewrite hash test #1506
  2. Complete hash key ingress-port test #1509
  3. Generate available send packet ports instead of hard coding src_ports in hash_test.py. #1511
  4. Add some hash keys: 'src-mac', 'dst-mac', 'ip-proto', 'vlan-id'. #1512

Type of change

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

Approach

How did you do it?

How did you verify/test it?

pytest --pdb -s -vvv --capture=no --disable_loganalyzer --inventory veos --host-pattern all -user admin --testbed vms-t1 --testbed_file testbed.csv --duration=0 --show-capture=stdout fib/test_fib.py::Test_Hash
============================================================== test session starts ==============================================================
platform linux2 -- Python 2.7.12, pytest-4.6.3, py-1.8.0, pluggy-0.12.0 -- /usr/bin/python
cachedir: .pytest_cache
ansible: 2.0.0.2
rootdir: /var/root/sonic-mgmt/tests, inifile: pytest.ini
plugins: ansible-2.0.2
collected 2 items

fib/test_fib.py::Test_Hash::test_hash_ipv4 PASSED
fib/test_fib.py::Test_Hash::test_hash_ipv6 PASSED

============================================================ slowest test durations =============================================================
670.56s call fib/test_fib.py::Test_Hash::test_hash_ipv6
634.48s call fib/test_fib.py::Test_Hash::test_hash_ipv4
11.01s setup fib/test_fib.py::Test_Hash::test_hash_ipv4
2.20s teardown fib/test_fib.py::Test_Hash::test_hash_ipv6
0.60s teardown fib/test_fib.py::Test_Hash::test_hash_ipv4
0.42s setup fib/test_fib.py::Test_Hash::test_hash_ipv6
========================================================== 2 passed in 1319.29 seconds ==========================================================

Any platform specific information?

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

Documentation


# The sample is too little for hash_key vlan-id and ingress-port, check it loose
check_balancing_mode = 'loose' if hash_key in ['vlan-id', 'ingress-port'] else 'strict'
self.check_balancing(next_hop.get_next_hop(), hit_count_map, check_balancing_mode)
Copy link
Contributor

Choose a reason for hiding this comment

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

i am not sure why we introduce loose mode, it seems defeating the purpose of the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Load balance is established in the case of a large number of samples. When the sample size is too small, the result may be hashed to the same destination, and the result is load balance not very well. In our hash test, the number of vlan-id and ingress-port was small, so i add them to the loose mode.

@lguohan
Copy link
Contributor

lguohan commented Mar 27, 2020

@William-zx , there are many enhancement in the pr. it is very difficult to review all enhancement in one pr. can you break your enhancement in three PRs? We'll review them one by one.

Copy link
Contributor

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

@William-zx , there are many enhancement in the pr. it is very difficult to review all enhancement in one pr. can you break your enhancement in three PRs? We'll review them one by one.

@William-zx
Copy link
Contributor Author

@William-zx , there are many enhancement in the pr. it is very difficult to review all enhancement in one pr. can you break your enhancement in three PRs? We'll review them one by one.

Break the enhancement in four PRs:

  1. Rewrite hash test #1506
  2. Complete hash key ingress-port test #1509
  3. Generate available send packet ports instead of hard coding src_ports in hash_test.py. #1511
  4. Add some hash keys: 'src-mac', 'dst-mac', 'ip-proto', 'vlan-id'. #1512

1. Iterate once for every port when hash key is ingress port.
2. In loose mode, do not need to enter the check_balancing function.
3. Delete loose mode in check_balancing function.
@William-zx
Copy link
Contributor Author

four sub PRs have been merged into the master. close this PR

@William-zx William-zx closed this Apr 23, 2020
@William-zx William-zx deleted the hash branch April 23, 2020 03:27
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.

2 participants