From: Jarek Poplawski <redacted>
Date: Fri, 25 Jul 2008 10:57:48 +0000

On 20-07-2008 01:35, Jussi Kivilinna wrote:

quoted

Signed-off-by: Jussi Kivilinna <redacted>

...

quoted

diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 1afe3ee..f1d2f8e 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c

@@ -370,7 +370,6 @@ static int
 cbq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 {
 	struct cbq_sched_data *q = qdisc_priv(sch);
-	int len = skb->len;
 	int uninitialized_var(ret);
 	struct cbq_class *cl = cbq_classify(skb, sch, &ret);
 

@@ -391,7 +390,7 @@ cbq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	if (ret == NET_XMIT_SUCCESS) {
 		sch->q.qlen++;
 		sch->bstats.packets++;
-		sch->bstats.bytes+=len;
+		sch->bstats.bytes += qdisc_pkt_len(skb);

Alas I didn't manage to read this earlier, but this type of changes
here and elsewhere in this patch looks wrong to me: we shouldn't
use skb pointer after ->enqueue().

It should be OK here, we have the root qdisc locked, so nobody
can remove it from the queue and if we got NET_XMIT_SUCCESS that
thing must not have been freed.

And anyways, we can't know the qdisc_pkt_len() until the enqueue
fucntion has been called.

Even TCP depends upon that NET_XMIT_* return value having some real
meaning, remember the HTB bug we tracked down last week? :-)