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

[qosorch] Handle 'config qos clear' for queue, scheduler and wred in orchagent #1083

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

amaneti
Copy link
Contributor

@amaneti amaneti commented Oct 4, 2019

What I did
(a) Handle 'config qos clear' for SCHEDULER in qosorch
(b) Handle 'config qos clear' for WRED in qosorch
Why I did it
SCHEDULER, WRED and QUEUE configurations are not cleanedup by using the command "config qos clear".
How I did it
With 'config qos clear', send SAI_NULL_OBJECT_ID to SAI for unbinding the scheduler and WRED from Queue
How I verified it
Test 1 Setup:
TgenPort1---Ethernet120--+DUT1+-- Ethernet0
TgenPort2---Ethernet121--+DUT1+
Test 1 Steps:
1a. Send line rate traffic from Ethernet120(Queue4) and Ethernet121(Queue3) towards Ethernet0 and Congestion occurs at the port Ethernet0 and packets dropped as per the default scheduler and queue config.

1b. Apply SCHEDULER config as below:
root@sonic:/home/admin# cat sched.json
{
"SCHEDULER": {
"scheduler.2": {
"type": "DWRR",
"weight": "20"
},
"scheduler.1": {
"type": "DWRR",
"weight": "50"
}
},
"QUEUE": {
"Ethernet0|3": {
"scheduler": "[SCHEDULER|scheduler.2]"
},
"Ethernet0|4": {
"scheduler": "[SCHEDULER|scheduler.1]"
}
}
}
root@sonic:/home/admin# config load sched.json
Load config from the file sched.json? [y/N]: y
Running command: /usr/local/bin/sonic-cfggen -j sched.json --write-to-db

1c. Verify that packets dropped as per the SCHEDULER and QUEUE config of sched.json
1d. Execute the command "config qos clear" to cleanup the config. Ensure that "show runningconfiguration all" doesn't show up any scheduler and queue config.
1e. Verify that packets are dropped as per the default config (as in Step 1a)

Test 2 Setup:
TgenPort1 -------------- Ethernet120 [DUT] Ethernet121 ------------ TgenPort2

Test 2 Steps:
2a. Send line rate traffic from Ethernet120(Queue3) towards Ethernet121. Check there are no WRED drops with default config.
2b. Apply the WRED config as below:
root@sonic:/home/admin# cat wred.json
{
"WRED_PROFILE": {
"AZURE_LOSSLESS" : {
"wred_green_enable" : "true",
"wred_yellow_enable" : "true",
"wred_red_enable" : "true",
"ecn" : "ecn_all",
"green_max_threshold" : "1000",
"green_min_threshold" : "100",
"yellow_max_threshold" : "2097152",
"yellow_min_threshold" : "1048576",
"red_max_threshold" : "2097152",
"red_min_threshold" : "1048576",
"green_drop_probability" : "100",
"yellow_drop_probability": "5",
"red_drop_probability" : "5"
}
},
"QUEUE": {
"Ethernet121|3": {
"wred_profile": "[WRED_PROFILE|AZURE_LOSSLESS]"
}
}
}
root@sonic:/home/admin# config load wred.json
Load config from the file wred.json? [y/N]: y
Running command: /usr/local/bin/sonic-cfggen -j wred.json --write-to-db

2c. Packet drops will be there due to WRED. Verify using 'show queue counters Ethernet121'
2d. Stop the traffic and execute "config qos clear"
2e. Ensure WRED_PROFILE configs are removed from "show runnningconfiguration all"
2f. Repeat the same traffic started in step 2b
2g. Check there are no WRED drops with default config.
Additional Information
Fixing the order of QOS_TABLE_NAMES with 'config qos clear' is done through sonic-net/sonic-utilities#694.

@msftclas
Copy link

msftclas commented Oct 4, 2019

CLA assistant check
All CLA requirements met.

@amaneti
Copy link
Contributor Author

amaneti commented Oct 11, 2019

retest this please

@@ -1253,6 +1253,20 @@ task_process_status QosOrch::handleQueueTable(Consumer& consumer)
SWSS_LOG_ERROR("Resolving scheduler reference failed");
return task_process_status::task_failed;
}
else
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably I miss something here, why we didn't hit this case:

            if (ref_resolve_status::success == resolve_result)
            {
                ....
                else if (op == DEL_COMMAND)
                {
                    // NOTE: The map is un-bound from the port. But the map itself still exists.
                    result = applySchedulerToQueueSchedulerGroup(port, queue_ind, SAI_NULL_OBJECT_ID);
                }
               ...

what is the value of "resolve_result" during you test?

@prsunny prsunny requested a review from neethajohn as a code owner September 2, 2022 23:17
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