Add ALTQ(9) CoDel algorithm support

[loos]

Summary

Reworked patch to add ALTQ(9) support for CoDel algorithm.

CoDel is a parameterless queue discipline that handles variable bandwidth and RTT.

It can be used as the single queue discipline on an interface or as a sub discipline of existing queue disciplines.

First submitted by eri here:
https://lists.freebsd.org/pipermail/freebsd-net/2013-June/035731.html

This patch address all the issues raised in this thread.

Obtained from: pfsense

[gnn]

Please fix up the spl issue. Other than that this looks good to commit.

sys/net/altq/altq_codel.c

87

Big lock? We don't have spl routines anymore. These need proper locks or just remove the spl code if no new lock is necessary.

[rpaulo]

Probably should be reviewed by one more person...

sbin/pfctl/parse.y

303

node_codel_opts?

1898–1904

Wrong indentation?

sys/net/altq/altq_codel.c

87

These are no-ops and perhaps should be replaced by proper locking.

359

Can you quote a book instead of wikipedia...? Newton's method can be found in any numerical analysis book.

[eri]

Can you please put my copyright in the codel files since it was forgotten addition when i did the port?

[loos]

Changes in this version:

• Removed the spl() locking, it was a noop and it doesn't seems to require additional locking (besides the existing locking in altq).

• Removed the quote to wikipedia, the Newton method is well known and doesn't need a wikipedia reference.

• Added the Ermal's copyright.

• Added a 'target' keyword to avoid abusing from 'qlimit' in CoDel setup.

[loos]

In D3272#66304, @eri wrote:

Can you please put my copyright in the codel files since it was forgotten addition when i did the port?

Sure Ermal, already done in this version.

Let me know if I need to change something (I have no idea about what year(s) I should add there).

[loos]

sbin/pfctl/parse.y

303

We don't have or use node_code_opts here. In this case, this is correct.

1898–1904

Yeah, it's weird but this also follows the indentation used in the rest of file (i.e. matches the other cases).

[hiren]

Very nice work! Thanks a lot.

Can you share any testing details if possible? How was this tested or how can one help test this?

[glebius]

sys/net/altq/altq_codel.c

105

As usually in ALTQ they use M_NOWAIT in ioctl() context. Why? Why don't make the configuration codepath non faulty?

[eri]

This can go in.
I am working on restructuring all this to be a bit more modern and not so...hackish.

The testing done on this code has been through many installations of pfSense and no issues have been reported so far that i am aware of.

[eri]

One thing not mentioned in this review is that the codel queue type can be used on any other scheduler like PRIQ, CBQ, HFSC, FAIRQ.
It is quite flexible at that.

It surely needs a man page but even the other schedulers do not have it probably can take the others from NetBSD?

[loos]

sys/net/altq/altq_codel.c

105

In this case altq_add() is called with PF_RULES_WLOCK() held. Sleeping here is not safe.

[loos]

In D3272#67425, @hiren wrote:

Very nice work! Thanks a lot.

Can you share any testing details if possible? How was this tested or how can one help test this?

I'm also looking for a controlled way to validate this change. I'll try to push some different loads on my test environment and check if I can simulate a good test case.

Thanks!

[bcr]

OK from manpages, but don't forget to bump the document date (.Dd).

[adrian]

yes please!

[kp]

In D3272#247495, @adrian wrote:

yes please!

Note that this was committed back in 2015:
https://svnweb.freebsd.org/base?view=revision&revision=287009

I'm not sure why phabricator didn't pick it up.