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

Get initial speed from ASIC DB #1390

Merged
merged 3 commits into from
Aug 12, 2020

Conversation

samaity
Copy link
Contributor

@samaity samaity commented Aug 8, 2020

Signed-off-by: Sangita Maity sangitamaity0211@gmail.com
Co-authored-by: Vasant Patil vapatil@linkedin.com

What I did
Initialized the configured speed list by reading port speed from ASIC DB
sonic-buildimage #3910 PR has a dependency on it.
Otherwise test_speed test is getting failed and other few test cases are dependent on it.

23:21:21  test_speed.py::TestSpeedSet::test_SpeedAndBufferSet FAILED  

Why I did it

Since Speed is introduced in port_config.ini of VS platform, one of the buffer profiles is already created(speed 40G). Hence the test case is modified by reading port speed from ASIC DB. The list of speeds is [10G, 25G, 40G, 50G, 100G], the initial number of buffer profiles present is 4 (including the one created for 40G)
Test case walks through each speed and configures that speed on all the ports. Expects that a new buffer profile is created when a new speed is set on all ports. But note that since 40G profiles are already created, it expects no new buffer profile to be created when we set 40G on all ports.

Now, when we walk through the speed list, This is what happens

10G - yes a new profile is created
25G - yes, a new profile is created
40G - yes, a new profile is created. This is the problem!

Root cause: however, once the DPB feature gets in, the initial created buffer profile is NOT for 40G, instead it is for 100G.
Because we are setting the different speed for ports in platform.json

How I verified it
By running the VS test case

samaity@server09:~/REPO_FACTORY/GIT_REPO/dpb_test/sonic-buildimage/src/sonic-swss/tests$ sudo pytest -s -vv --dvsname=vs-sa  test_speed.py
 
 
samaity@server09:~/REPO_FACTORY/GIT_REPO/dpb_test/sonic-buildimage/src/sonic-swss/tests$ sudo pytest -s -vv --dvsname=vs-sa  test_speed.py
============================================================================================================ test session starts =============================================================================================================
platform linux2 -- Python 2.7.17, pytest-4.6.9, py-1.8.1, pluggy-0.13.1 -- /usr/bin/python
cachedir: .pytest_cache
rootdir: /home/samaity/REPO_FACTORY/GIT_REPO/dpb_test/sonic-buildimage/src/sonic-swss/tests
plugins: repeat-0.8.0
collected 1 item
 
test_speed.py::TestSpeedSet::test_SpeedAndBufferSet remove extra link dummy
PASSED
 

Signed-off-by: Sangita Maity sangitamaity0211@gmail.com
Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
@samaity
Copy link
Contributor Author

samaity commented Aug 10, 2020

retest vs please

2 similar comments
@samaity
Copy link
Contributor Author

samaity commented Aug 10, 2020

retest vs please

@zhenggen-xu
Copy link
Collaborator

retest vs please

tests/test_speed.py Outdated Show resolved Hide resolved
tests/test_speed.py Outdated Show resolved Hide resolved
tests/test_speed.py Outdated Show resolved Hide resolved
Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
@daall
Copy link
Contributor

daall commented Aug 12, 2020

retest vs please

@daall daall merged commit 4110acf into sonic-net:master Aug 12, 2020
@daall daall added the DVS ⭐ label Aug 12, 2020
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
Allow ecn unit tests to run without root privileges

**- How I did it**
Included the UTILITIES_UNIT_TESTING' env variable also as one of the conditions to determine if the command needs root privileges for execution

**- How to verify it**
Ran utilities test using the command "python3 setup.py test" and ecn_test.py passed. Prior to the fix, most of the testcases were failing with the error 'Root privileged required for this operation'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants