-
Notifications
You must be signed in to change notification settings - Fork 273
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
Add fabric port test to vslib. #737
Conversation
Signed-off-by: ngocdo <[email protected]>
Are there any queues IPGs or scheduler groups related with those ports ? |
Hmm which change in this commit your comment refers to? |
its not about this change, its about port properties, take a look inside source that for each port when it's created, another SG's, queues and IPGs are created for each port, but not in your case take a look at methods: in each of specific classes |
Hello Kamil, thanks for pointing these functions out -- very useful information. Fabric ports are different from regular front panel ports. They don't have properties like INGRESS_PRIORITY_GROUP, INGRESS_PRIORITY_GROUP_LIST, QOS_NUMBER_OF_SCHEDULER_GROUPS, QOS_SCHEDULER_GROUP_LIST, and etc...
QOS_NUMBER_OF_QUEUES is available for fabric ports, but currently not supported in network forwarding asics. I can't test on real hardware, and so queue counters for fabric ports are not collected yet.
Once Broadcom enables this feature, I will update swss, test on real hardware, as well as update vstest lib for this property. |
so if SAI_PORT_ATTR_QOS_NUMBER_OF_QUEUES will be supported, then also qos queues will also be supported later probably, so i think for now we can go with this approach |
retest this please |
Hello Kamil, do you have any advices for vs test failures? The first time I saw 4 failures, which looked weird to me (very unlikely to related to my changes). I retested, and now there were 299 failures. Those were due to another issue that seemed to system setup:
What should I do next? retest one more time? |
yea, this is some new error, not related to your changes |
Cool, so would you approve this PR to merge? |
Hello, I'm trying to follow up what the next step I should do to merge it up? |
retest vs please |
Thank you. It passed all. Definitely inside things got fixed. |
Could we merge it now? |
Now voq and fabric asics are enabled with fabric ports. Need to fix the tests to count for fabric ports. vstest infrastructure for fabric ports merged in sonic-net/sonic-sairedis#737 and 769. This infrastructure requires sonic-net/sonic-buildimage#6185 to be effective. However, this PR (6185) will have some swss tests failing. We need to disable those tests, let the PR merge, and reenable the tests.
Now voq and fabric asics are enabled with fabric ports. Need to fix the tests to count for fabric ports. vstest infrastructure for fabric ports merged in sonic-net/sonic-sairedis#737 and 769. This infrastructure requires sonic-net/sonic-buildimage#6185 to be effective. However, this PR (6185) will have some swss tests failing. We need to disable those tests, let the PR merge, and reenable the tests.
Now voq and fabric asics are enabled with fabric ports. Need to fix the tests to count for fabric ports. vstest infrastructure for fabric ports merged in sonic-net/sonic-sairedis#737 and 769. This infrastructure requires sonic-net/sonic-buildimage#6185 to be effective. However, this PR (6185) will have some swss tests failing. We need to disable those tests, let the PR merge, and reenable the tests.
Signed-off-by: ngocdo <[email protected]>
In this PR, I'm adding test for fabric ports.
Testing asic supported in this PR is VOQ because fabric ports are enabled on a VOQ asic so that it can connect to other VOQ asics or fabric asics.
This PR is to add support for testing changes in sonic-net/sonic-swss#1459.
Together with this PR is sonic-net/sonic-buildimage#6185 that defines lane mapping for fabric part in Force10-S6000.
Signed-off-by: ngocdo [email protected]