Subject: Re: [PATCH v2] pkt_sched: sch_drr: Fix drr_dequeue() loop

From: Patrick McHardy <[email-masked]>
Date: Mon, 24 Nov 2008 14:17:32 +0100

Jarek Poplawski wrote:
> On Mon, Nov 24, 2008 at 01:38:48PM +0100, Patrick McHardy wrote:
> ...
>> TBF with an inner DRR is fine. The other way around is broken
>> in the sense that the behaviour is undefined.
> 
> IMHO, this other way (e.g. a class with TBF per user), should work too.

The behaviour undefined, so what does "work too" mean in this context?

The main question is: what should be done with the class when it
throttles?

You suggest moving it to the end of the active list. Should its deficit
be recharged in that case? Possible no because it didn't send packets -
but then again it might have handed out *some* packets (less than the
deficit) before it started throttling. Both ways would introduce
unfairness.

What could be done without harming the algorithm is to treat throttled
classes as inactive until they become unthrottled again, meaning they
would be added to the end of the active list with a full deficit. But
we have no indication for specific classes, unthrottling simply triggers
another dequeue of the root, so the implementation would get quite
complicated, leaving alone the fact that each TBF would potentially
start its own watchdog, causing excessive wakeups.

And I don't see much use for this, what is the advantage over using
HTB or HFSC?

> BTW, since this "broken" config isn't very apparent, maybe you should
> add some warning?

Yes, but I don't want to add this to the ->dequeue() path. It belongs
in the ->init() path and we currently don't have enough information
in there.