Qdisc requeue should be void?

Qdisc requeue should be void?

[Stephen Hemminger]

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.
			
			

Re: Qdisc requeue should be void?

[Patrick McHardy]

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