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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 18 Aug 2008 16:11:57 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	David Miller <davem@...emloft.net>
CC:	netdev@...r.kernel.org, jarkao2@...il.com
Subject: Re: [PATCH 8/9]: sch_hfsc: Use ->requeue queue instead of ops.

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@...emloft.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.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html