Re: [PATCH 8/9]: sch_hfsc: Use ->requeue queue instead of ops.

[Patrick McHardy <kaber@trash.net>]

David Miller wrote:
> sch_hfsc: Use ->requeue queue instead of ops.
> 
> In fact this "peek head SKB for len" sequence could be
> optimized even further.
> 
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>  net/sched/sch_hfsc.c |    7 +------
>  1 files changed, 1 insertions(+), 6 deletions(-)
> 
> diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
> index ea46760..bed4fd2 100644
> --- a/net/sched/sch_hfsc.c
> +++ b/net/sched/sch_hfsc.c
> @@ -896,12 +896,7 @@ qdisc_peek_len(struct Qdisc *sch)
>  		return 0;
>  	}
>  	len = qdisc_pkt_len(skb);
> -	if (unlikely(sch->ops->requeue(skb, sch) != NET_XMIT_SUCCESS)) {
> -		if (net_ratelimit())
> -			printk("qdisc_peek_len: failed to requeue\n");
> -		qdisc_tree_decrease_qlen(sch, 1);
> -		return 0;
> -	}
> +	__skb_queue_head(&sch->requeue, skb);

Sorry for the late response, I didn't follow netdev very closely the
past days. This change does not work properly I'm afraid, it breaks
priorization on two levels:

- when a new packet is enqueued to a higher prioritized class within
HFSC (or a different qdisc), the class peeked at will still be served
on dequeue regardless, increasing worst case latency by a full packet
transmission time.

- when a packet is enqueued to the same class and inner qdisc after
peeking, the inner qdisc can decide to prioritize that packet and
hand it out on the next ->dequeue, but the upper qdisc will serve
the packet peeked at regardless. This results in the same effect,
the worst case latency increases by a full packet transmission time.

I think we really need either ->requeue or a real ->peek operation.