Bug #4692
closed
CODELQ scheduler defaults to incorrect "target" and "interval" values.
100%
Description
If I setup CODELQ as my WAN's queue scheduler, when I run "pfctl -vsq | grep -i codel" the returned string is "altq on em0 codel( target 50 interval 5) bandwidth 300Mb tbrsize 36000". The target should be ~5ms, and the interval should be ~100ms. Seems like they got flip-flopped.
For reference see section 4.2 of https://tools.ietf.org/html/draft-nichols-tsvwg-codel-02
Files
Updated by Ben Cook over 9 years ago
Perhaps it is obvious, but it looks like the calls to "codel_alloc(100, 5, 0);" in one/all of the "altq_codel.diff" need to change to "codel_alloc(5, 100, 0);".
Updated by Ben Cook over 9 years ago
My apologies if this patch is incorrect or causes a fire. I figured I would try. It modifies pfsense-tools/patches/stable/10/altq_codel.diff to fix 4 uses of "codel_alloc" to the more optimal target/interval values.
Updated by Renato Botelho over 9 years ago
- Status changed from New to Feedback
- Assignee changed from Ermal Luçi to Renato Botelho
- Target version set to 2.2.4
- % Done changed from 0 to 100
Pull request has been merged. Thanks!
Updated by Ben Cook over 9 years ago
I think there is already a (newer) patch merged, but according to a few sources, the patch is not working.
https://github.com/pfsense/pfsense-tools/commit/3108a902bd816036a3abffd3ec669767140891a7
Revert it until someone can do a proper fix?
My assumption that this bug was so simple to fix that I could forgoe building pfSense to confirm the patch's functionality was plain stupid. No more code submissions from me until I can build pfSense reliably.
Updated by Renato Botelho over 9 years ago
Ben Cook wrote:
I think there is already a (newer) patch merged, but according to a few sources, the patch is not working.
https://github.com/pfsense/pfsense-tools/commit/3108a902bd816036a3abffd3ec669767140891a7
Revert it until someone can do a proper fix?
My assumption that this bug was so simple to fix that I could forgoe building pfSense to confirm the patch's functionality was plain stupid. No more code submissions from me until I can build pfSense reliably.
Which patch you say is not working? commit:3108a902bd or the one you submitted?
Did you try a recent 2.2.4 snapshot? Did you get same problem?
Updated by Ben Cook over 9 years ago
Renato Botelho wrote:
Ben Cook wrote:
I think there is already a (newer) patch merged, but according to a few sources, the patch is not working.
https://github.com/pfsense/pfsense-tools/commit/3108a902bd816036a3abffd3ec669767140891a7
Revert it until someone can do a proper fix?
My assumption that this bug was so simple to fix that I could forgoe building pfSense to confirm the patch's functionality was plain stupid. No more code submissions from me until I can build pfSense reliably.
Which patch you say is not working? commit:3108a902bd or the one you submitted?
Did you try a recent 2.2.4 snapshot? Did you get same problem?
commit:3108a902bd is mine from github, I think.
I will try out a snapshot.
Updated by Kieran Cawthray over 9 years ago
As far as I can see, the interval is correctly set to 100 on both the 20150721 and 20150719 nightly builds, the target can be set using 'queue limit'.
Updated by Kieran Cawthray over 9 years ago
Kieran Cawthray wrote:
As far as I can see, the interval is correctly set to 100 on both the 20150721 and 20150719 nightly builds, the target can be set using 'queue limit'.
I just checked 20150708 nightly and the interval is 100. I also tested 2.2.3 release and got an interval of 5, so it was fixed somewhere between 20150625 and 20150708, probably commit 3108a902bd816036a3abffd3ec669767140891a7 on 20150629?
Updated by Ben Cook over 9 years ago
Good to hear.
There are two different methods of employing codel.
1. Where codel is the one and only scheduling discipline for a single parent queue with no other sub-queues.
2. Where codel is a sub-discipline ("Codel Active Queue" check-box) under one of the primary schedulers (HFSC, CBQ, PRIQ, FAIRQ).
The pfctl command in my original post will show the values of codel in a method-1 situation, but does anyone know how to display codel's configuration values when codel is a sub-discipline of another scheduler like in method-2?
Updated by Renato Botelho over 9 years ago
- Assignee changed from Renato Botelho to Luiz Souza
Updated by Renato Botelho over 9 years ago
- Target version changed from 2.2.4 to 2.3
Codel patch is being reviewed
Updated by Luiz Souza about 9 years ago
The codel code (reviewed and) committed in FreeBSD is now backported to 2.3.