Project

General

Profile

Bug #4692

CODELQ scheduler defaults to incorrect "target" and "interval" values.

Added by Ben Cook over 4 years ago. Updated almost 4 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Traffic Shaper (ALTQ)
Target version:
Start date:
05/08/2015
Due date:
% Done:

100%

Estimated time:
0.00 h
Affected Version:
All
Affected Architecture:
All

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

codel_opts_value_fix.patch (1.36 KB) codel_opts_value_fix.patch Ben Cook, 05/08/2015 07:50 PM

History

#1 Updated by Ben Cook over 4 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);".

#2 Updated by Ben Cook over 4 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.

#3 Updated by Renato Botelho over 4 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!

#4 Updated by Ben Cook over 4 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.

#5 Updated by Renato Botelho over 4 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?

#6 Updated by Ben Cook over 4 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.

#7 Updated by Kieran Cawthray over 4 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'.

#8 Updated by Kieran Cawthray over 4 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?

#9 Updated by Ben Cook over 4 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?

#10 Updated by Renato Botelho over 4 years ago

  • Assignee changed from Renato Botelho to Luiz Souza

#11 Updated by Renato Botelho over 4 years ago

  • Target version changed from 2.2.4 to 2.3

Codel patch is being reviewed

#12 Updated by Luiz Souza about 4 years ago

The codel code (reviewed and) committed in FreeBSD is now backported to 2.3.

#13 Updated by Renato Botelho almost 4 years ago

  • Status changed from Feedback to Resolved

Fixed

Also available in: Atom PDF