net_sched: accurate bytes/packets stats/rates

In commit 44b8288308ac9d (net_sched: pfifo_head_drop problem), we fixed
a problem with pfifo_head drops that incorrectly decreased
sch->bstats.bytes and sch->bstats.packets

Several qdiscs (CHOKe, SFQ, pfifo_head, ...) are able to drop a
previously enqueued packet, and bstats cannot be changed, so
bstats/rates are not accurate (over estimated)

This patch changes the qdisc_bstats updates to be done at dequeue() time
instead of enqueue() time. bstats counters no longer account for dropped
frames, and rates are more correct, since enqueue() bursts dont have
effect on dequeue() rate.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index e9eee99d8b1f4668aef09526341d0e032a6f120b..160a407c19632a2220fa6fc5efc120086fd076ee 100644 (file)
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -445,7 +445,6 @@ static inline int __qdisc_enqueue_tail(struct sk_buff *skb, struct Qdisc *sch,
 {
        __skb_queue_tail(list, skb);
        sch->qstats.backlog += qdisc_pkt_len(skb);
-       qdisc_bstats_update(sch, skb);
 
        return NET_XMIT_SUCCESS;
 }
@@ -460,8 +459,10 @@ static inline struct sk_buff *__qdisc_dequeue_head(struct Qdisc *sch,
 {
        struct sk_buff *skb = __skb_dequeue(list);
 
-       if (likely(skb != NULL))
+       if (likely(skb != NULL)) {
                sch->qstats.backlog -= qdisc_pkt_len(skb);
+               qdisc_bstats_update(sch, skb);
+       }
 
        return skb;
 }
@@ -474,10 +475,11 @@ static inline struct sk_buff *qdisc_dequeue_head(struct Qdisc *sch)
 static inline unsigned int __qdisc_queue_drop_head(struct Qdisc *sch,
                                              struct sk_buff_head *list)
 {
-       struct sk_buff *skb = __qdisc_dequeue_head(sch, list);
+       struct sk_buff *skb = __skb_dequeue(list);
 
        if (likely(skb != NULL)) {
                unsigned int len = qdisc_pkt_len(skb);
+               sch->qstats.backlog -= len;
                kfree_skb(skb);
                return len;
        }

diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index c80d1c210c5d621454acf3388a63440c3fbfe740..5f63ec58942c10ac5088c6e6fca6e40463235a0b 100644 (file)
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -390,7 +390,6 @@ cbq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
        ret = qdisc_enqueue(skb, cl->q);
        if (ret == NET_XMIT_SUCCESS) {
                sch->q.qlen++;
-               qdisc_bstats_update(sch, skb);
                cbq_mark_toplevel(q, cl);
                if (!cl->next_alive)
                        cbq_activate_class(cl);
@@ -649,7 +648,6 @@ static int cbq_reshape_fail(struct sk_buff *skb, struct Qdisc *child)
                ret = qdisc_enqueue(skb, cl->q);
                if (ret == NET_XMIT_SUCCESS) {
                        sch->q.qlen++;
-                       qdisc_bstats_update(sch, skb);
                        if (!cl->next_alive)
                                cbq_activate_class(cl);
                        return 0;
@@ -971,6 +969,7 @@ cbq_dequeue(struct Qdisc *sch)
 
                skb = cbq_dequeue_1(sch);
                if (skb) {
+                       qdisc_bstats_update(sch, skb);
                        sch->q.qlen--;
                        sch->flags &= ~TCQ_F_THROTTLED;
                        return skb;

diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index de55e642eafc2924bd2b6220aaac1d38e879954e..6b7fe4a84f138b903928a8c5f5f90f8fe3b855dc 100644 (file)
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -376,7 +376,6 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch)
        }
 
        bstats_update(&cl->bstats, skb);
-       qdisc_bstats_update(sch, skb);
 
        sch->q.qlen++;
        return err;
@@ -403,6 +402,7 @@ static struct sk_buff *drr_dequeue(struct Qdisc *sch)
                        skb = qdisc_dequeue_peeked(cl->qdisc);
                        if (cl->qdisc->q.qlen == 0)
                                list_del(&cl->alist);
+                       qdisc_bstats_update(sch, skb);
                        sch->q.qlen--;
                        return skb;
                }

diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index 60f4bdd4408e25888b4575bd9ab7b75441cab8c6..0f7bf3fdfea5a63683d86887e4d9fad99ff40a82 100644 (file)
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -260,7 +260,6 @@ static int dsmark_enqueue(struct sk_buff *skb, struct Qdisc *sch)
                return err;
        }
 
-       qdisc_bstats_update(sch, skb);
        sch->q.qlen++;
 
        return NET_XMIT_SUCCESS;
@@ -283,6 +282,7 @@ static struct sk_buff *dsmark_dequeue(struct Qdisc *sch)
        if (skb == NULL)
                return NULL;
 
+       qdisc_bstats_update(sch, skb);
        sch->q.qlen--;
 
        index = skb->tc_index & (p->indices - 1);

diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c
index aa4d6337e43cab4f3a094d6b2028be50afdcada5..d468b479aa937f410a665045eb4018bfc08de255 100644 (file)
--- a/net/sched/sch_fifo.c
+++ b/net/sched/sch_fifo.c
@@ -46,17 +46,14 @@ static int pfifo_enqueue(struct sk_buff *skb, struct Qdisc* sch)
 
 static int pfifo_tail_enqueue(struct sk_buff *skb, struct Qdisc* sch)
 {
-       struct sk_buff *skb_head;
        struct fifo_sched_data *q = qdisc_priv(sch);
 
        if (likely(skb_queue_len(&sch->q) < q->limit))
                return qdisc_enqueue_tail(skb, sch);
 
        /* queue full, remove one skb to fulfill the limit */
-       skb_head = qdisc_dequeue_head(sch);
+       __qdisc_queue_drop_head(sch, &sch->q);
        sch->qstats.drops++;
-       kfree_skb(skb_head);
-
        qdisc_enqueue_tail(skb, sch);
 
        return NET_XMIT_CN;

diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 2e45791d4f6cf7970d590fde138981254b359403..14a799de1c3535ed99c9af448080265fa406fe88 100644 (file)
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1600,7 +1600,6 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch)
                set_active(cl, qdisc_pkt_len(skb));
 
        bstats_update(&cl->bstats, skb);
-       qdisc_bstats_update(sch, skb);
        sch->q.qlen++;
 
        return NET_XMIT_SUCCESS;
@@ -1666,6 +1665,7 @@ hfsc_dequeue(struct Qdisc *sch)
        }
 
        sch->flags &= ~TCQ_F_THROTTLED;
+       qdisc_bstats_update(sch, skb);
        sch->q.qlen--;
 
        return skb;

diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 984c1b0c68369dd24b4837fd681126abc229ce92..fc12fe6f559796d11aaa8b4e769b26a6a28643e9 100644 (file)
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -574,7 +574,6 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch)
        }
 
        sch->q.qlen++;
-       qdisc_bstats_update(sch, skb);
        return NET_XMIT_SUCCESS;
 }
 
@@ -842,7 +841,7 @@ next:
 
 static struct sk_buff *htb_dequeue(struct Qdisc *sch)
 {
-       struct sk_buff *skb = NULL;
+       struct sk_buff *skb;
        struct htb_sched *q = qdisc_priv(sch);
        int level;
        psched_time_t next_event;
@@ -851,6 +850,8 @@ static struct sk_buff *htb_dequeue(struct Qdisc *sch)
        /* try to dequeue direct packets as high prio (!) to minimize cpu work */
        skb = __skb_dequeue(&q->direct_queue);
        if (skb != NULL) {
+ok:
+               qdisc_bstats_update(sch, skb);
                sch->flags &= ~TCQ_F_THROTTLED;
                sch->q.qlen--;
                return skb;
@@ -884,11 +885,8 @@ static struct sk_buff *htb_dequeue(struct Qdisc *sch)
                        int prio = ffz(m);
                        m |= 1 << prio;
                        skb = htb_dequeue_tree(q, prio, level);
-                       if (likely(skb != NULL)) {
-                               sch->q.qlen--;
-                               sch->flags &= ~TCQ_F_THROTTLED;
-                               goto fin;
-                       }
+                       if (likely(skb != NULL))
+                               goto ok;
                }
        }
        sch->qstats.overlimits++;

diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
index 21f13da24763977eaea671c19a58357018f72899..436a2e75b322db524764a2483cd385e905aa4534 100644 (file)
--- a/net/sched/sch_multiq.c
+++ b/net/sched/sch_multiq.c
@@ -83,7 +83,6 @@ multiq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 
        ret = qdisc_enqueue(skb, qdisc);
        if (ret == NET_XMIT_SUCCESS) {
-               qdisc_bstats_update(sch, skb);
                sch->q.qlen++;
                return NET_XMIT_SUCCESS;
        }
@@ -112,6 +111,7 @@ static struct sk_buff *multiq_dequeue(struct Qdisc *sch)
                        qdisc = q->queues[q->curband];
                        skb = qdisc->dequeue(qdisc);
                        if (skb) {
+                               qdisc_bstats_update(sch, skb);
                                sch->q.qlen--;
                                return skb;
                        }

diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 1c4bce86347977ec93b1fa5a0fdcceaf34cfe257..6a3006b38dc55e65c4629a967a326eed684d7fd1 100644 (file)
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -240,7 +240,6 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 
        if (likely(ret == NET_XMIT_SUCCESS)) {
                sch->q.qlen++;
-               qdisc_bstats_update(sch, skb);
        } else if (net_xmit_drop_count(ret)) {
                sch->qstats.drops++;
        }
@@ -289,6 +288,7 @@ static struct sk_buff *netem_dequeue(struct Qdisc *sch)
                                skb->tstamp.tv64 = 0;
 #endif
                        pr_debug("netem_dequeue: return skb=%p\n", skb);
+                       qdisc_bstats_update(sch, skb);
                        sch->q.qlen--;
                        return skb;
                }
@@ -476,7 +476,6 @@ static int tfifo_enqueue(struct sk_buff *nskb, struct Qdisc *sch)
                __skb_queue_after(list, skb, nskb);
 
                sch->qstats.backlog += qdisc_pkt_len(nskb);
-               qdisc_bstats_update(sch, nskb);
 
                return NET_XMIT_SUCCESS;
        }

diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index 966158d49dd105ce882ece6a6ccd6f072f5676ca..fbd710d619bf372535d21ef38c750c7e6e6c9cf0 100644 (file)
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -84,7 +84,6 @@ prio_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 
        ret = qdisc_enqueue(skb, qdisc);
        if (ret == NET_XMIT_SUCCESS) {
-               qdisc_bstats_update(sch, skb);
                sch->q.qlen++;
                return NET_XMIT_SUCCESS;
        }
@@ -116,6 +115,7 @@ static struct sk_buff *prio_dequeue(struct Qdisc* sch)
                struct Qdisc *qdisc = q->queues[prio];
                struct sk_buff *skb = qdisc->dequeue(qdisc);
                if (skb) {
+                       qdisc_bstats_update(sch, skb);
                        sch->q.qlen--;
                        return skb;
                }

diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index a6009c5a2c97dbe446b162923fb997db00f00ee6..9f98dbd32d4c8176d6c940dd098f71fa0c00cb4d 100644 (file)
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -94,7 +94,6 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc* sch)
 
        ret = qdisc_enqueue(skb, child);
        if (likely(ret == NET_XMIT_SUCCESS)) {
-               qdisc_bstats_update(sch, skb);
                sch->q.qlen++;
        } else if (net_xmit_drop_count(ret)) {
                q->stats.pdrop++;
@@ -114,11 +113,13 @@ static struct sk_buff * red_dequeue(struct Qdisc* sch)
        struct Qdisc *child = q->qdisc;
 
        skb = child->dequeue(child);
-       if (skb)
+       if (skb) {
+               qdisc_bstats_update(sch, skb);
                sch->q.qlen--;
-       else if (!red_is_idling(&q->parms))
-               red_start_of_idle_period(&q->parms);
-
+       } else {
+               if (!red_is_idling(&q->parms))
+                       red_start_of_idle_period(&q->parms);
+       }
        return skb;
 }
 

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 239ec53a634dace6d29a3294c2a98e4c401539b8..edea8cefec6c9502e0a841e4bb4bd3ef11003495 100644 (file)
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -402,10 +402,8 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
                q->tail = slot;
                slot->allot = q->scaled_quantum;
        }
-       if (++sch->q.qlen <= q->limit) {
-               qdisc_bstats_update(sch, skb);
+       if (++sch->q.qlen <= q->limit)
                return NET_XMIT_SUCCESS;
-       }
 
        sfq_drop(sch);
        return NET_XMIT_CN;
@@ -445,6 +443,7 @@ next_slot:
        }
        skb = slot_dequeue_head(slot);
        sfq_dec(q, a);
+       qdisc_bstats_update(sch, skb);
        sch->q.qlen--;
        sch->qstats.backlog -= qdisc_pkt_len(skb);
 

diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 77565e721811f5cc5789a24987be3b0c04a6b371..e93165820c3f02ec4edc072b26b04dcdd4645ce0 100644 (file)
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -134,7 +134,6 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc* sch)
        }
 
        sch->q.qlen++;
-       qdisc_bstats_update(sch, skb);
        return NET_XMIT_SUCCESS;
 }
 
@@ -187,6 +186,7 @@ static struct sk_buff *tbf_dequeue(struct Qdisc* sch)
                        q->ptokens = ptoks;
                        sch->q.qlen--;
                        sch->flags &= ~TCQ_F_THROTTLED;
+                       qdisc_bstats_update(sch, skb);
                        return skb;
                }
 

diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 84ce48eadff4952e2552306fae3197858af86c59..d84e7329660fb5f21e5e87d11fae435e27907a8b 100644 (file)
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -87,7 +87,6 @@ teql_enqueue(struct sk_buff *skb, struct Qdisc* sch)
 
        if (q->q.qlen < dev->tx_queue_len) {
                __skb_queue_tail(&q->q, skb);
-               qdisc_bstats_update(sch, skb);
                return NET_XMIT_SUCCESS;
        }
 
@@ -111,6 +110,8 @@ teql_dequeue(struct Qdisc* sch)
                        dat->m->slaves = sch;
                        netif_wake_queue(m);
                }
+       } else {
+               qdisc_bstats_update(sch, skb);
        }
        sch->q.qlen = dat->q.qlen + dat_queue->qdisc->q.qlen;
        return skb;