Re: Qdisc requeue should be void?

[Patrick McHardy <kaber@trash.net>]

Stephen Hemminger wrote:
> There is an design problem with the qdisc interface that causes qlen related bugs
> in netem, tbf, and other qdisc's that peek at the top of the queue. The problem is
> that requeue needs to be called from the dequeue function but requeue can fail.
> If requeue fails, then the calling qdisc can not properly handle the error.  If it
> returns NULL, then the parent's expectation about qlen gets messed up.
> 
> Example:
> 
> 	prio (qlen = 1)
> 		skb = netem dequeue 
> 			skb = htb dequeue 
> 			... decides not to send this skb now
> 			htp requeue(skb) fails
> 				?? what now 
> 				--netem.qlen // := 0
> 				return NULL
> 		 skb is NULL
> 
> at this point prio qlen is 1 but underlying queue's are empty.
> 
> My proposal is to require requeue to always succeed and change it to be
> void instead of returning int.

Perhaps we should add a ->peek() operation which guarantees that the
next dequeued packet is the one peeked at. This would also help with
a second problem resulting from requeueing in at least TBF and HFSC.
TBF looks at a packet and if it can't be sent immediately calculates
the delay from the packet's length. HFSC does the same to calculate
the deadline for a class. Both assume the next packet dequeued from
the underlying qdisc is the one requeued, which is only true with
non-reordering qdiscs. Adding a peek-operation increases the worst-case
delay by one maximum sized packet transmission time, but otherwise
these qdiscs can't make proper use of reordering qdiscs like SFQ
at all.

Regards
Patrick