Messages in this thread Patch in this message | | | Date | Wed, 3 Mar 2004 13:31:14 +0100 (MET) | From | Jesper Dangaard Brouer <> | Subject | [NET_SCHED] BUG in qdisc TBF (token bucket filter) |
| |
BUG in qdisc TBF (token bucket filter).
Problem in : Kernel 2.4.22 (and newer, tested till 2.4.25-rc2) Problem NOT in: kernel 2.4.21 (and older)
Problem: -------- After I add an tbf qdisc to an htb class, then the htb class disappear from the output-listing "tc -s class ls dev ethX".
tc-versions: I have tested with different version of the "tc" utility. And different versions of the htb patch.
"tc utility, iproute2-ss010824" (debian's iproute) "tc utility, iproute2-ss020116" (with htb3.6-020525)
I've not tested if the tfb qdisc and htb class still works, but only that the output-listing is wrong. (I'm running 2.4.21 on my router/firewall to be on the safe side.)
Howto reproduce: ---------------- #Removing previous 'root' handle/classification /sbin/tc qdisc del dev eth0 root # /sbin/tc qdisc add dev eth0 root handle 1: htb default 10 # /sbin/tc class add dev eth0 parent 1: classid 1:10 htb rate 500kbit
# output-listing of class'es tc -s class ls dev eth0 # # output: class htb 1:10 root prio 0 rate 500Kbit ceil 500Kbit burst 2239b cburst 2239b Sent 812 bytes 6 pkts (dropped 0, overlimits 0) lended: 6 borrowed: 0 giants: 0 tokens: 27239 ctokens: 27239
# the tbf line /sbin/tc qdisc add dev eth0 parent 1:10 handle 4210: tbf rate 500kbit \ latency 50ms burst 2239b
# output-listing of class'es tc -s class ls dev eth0 # # output: <NOTHING>
The output-listing of class'es returns, if I remove the tbf qdisc again.
Diff between file /net/sched/sch_tbf.c in kernel 2.4.21 and 2.4.22 is attached.
Hilsen Jesper Brouer
-- ------------------------------------------------------------------- System Administrator Dept. of Computer Science, University of Copenhagen E-mail: hawk@diku.dk, Direct Tel.: 353 21464 ------------------------------------------------------------------- --- linux-2.4.21/net/sched/sch_tbf.c Fri Dec 21 18:42:06 2001 +++ linux-2.4.22/net/sched/sch_tbf.c Mon Aug 25 13:44:44 2003 @@ -7,6 +7,8 @@ * 2 of the License, or (at your option) any later version. * * Authors: Alexey Kuznetsov, <kuznet@ms2.inr.ac.ru> + * Dmitry Torokhov <dtor@mail.ru> - allow attaching inner qdiscs - + * original idea by Martin Devera * */ @@ -123,62 +125,63 @@ long ptokens; /* Current number of P tokens */ psched_time_t t_c; /* Time check-point */ struct timer_list wd_timer; /* Watchdog timer */ + struct Qdisc *qdisc; /* Inner qdisc, default - bfifo queue */ }; #define L2T(q,L) ((q)->R_tab->data[(L)>>(q)->R_tab->rate.cell_log]) #define L2T_P(q,L) ((q)->P_tab->data[(L)>>(q)->P_tab->rate.cell_log]) -static int -tbf_enqueue(struct sk_buff *skb, struct Qdisc* sch) +static int tbf_enqueue(struct sk_buff *skb, struct Qdisc* sch) { struct tbf_sched_data *q = (struct tbf_sched_data *)sch->data; + int ret; - if (skb->len > q->max_size) - goto drop; - __skb_queue_tail(&sch->q, skb); - if ((sch->stats.backlog += skb->len) <= q->limit) { - sch->stats.bytes += skb->len; - sch->stats.packets++; - return 0; - } - - /* Drop action: undo the things that we just did, - * i.e. make tail drop - */ - - __skb_unlink(skb, &sch->q); - sch->stats.backlog -= skb->len; - -drop: - sch->stats.drops++; + if (skb->len > q->max_size || sch->stats.backlog + skb->len > q->limit) { + sch->stats.drops++; #ifdef CONFIG_NET_CLS_POLICE - if (sch->reshape_fail==NULL || sch->reshape_fail(skb, sch)) + if (sch->reshape_fail == NULL || sch->reshape_fail(skb, sch)) #endif - kfree_skb(skb); - return NET_XMIT_DROP; -} - -static int -tbf_requeue(struct sk_buff *skb, struct Qdisc* sch) -{ - __skb_queue_head(&sch->q, skb); + kfree_skb(skb); + + return NET_XMIT_DROP; + } + + if ((ret = q->qdisc->enqueue(skb, q->qdisc)) != 0) { + sch->stats.drops++; + return ret; + } + + sch->q.qlen++; sch->stats.backlog += skb->len; + sch->stats.bytes += skb->len; + sch->stats.packets++; return 0; } -static int -tbf_drop(struct Qdisc* sch) +static int tbf_requeue(struct sk_buff *skb, struct Qdisc* sch) { - struct sk_buff *skb; + struct tbf_sched_data *q = (struct tbf_sched_data *)sch->data; + int ret; + + if ((ret = q->qdisc->ops->requeue(skb, q->qdisc)) == 0) { + sch->q.qlen++; + sch->stats.backlog += skb->len; + } + + return ret; +} - skb = __skb_dequeue_tail(&sch->q); - if (skb) { - sch->stats.backlog -= skb->len; +static unsigned int tbf_drop(struct Qdisc* sch) +{ + struct tbf_sched_data *q = (struct tbf_sched_data *)sch->data; + unsigned int len; + + if ((len = q->qdisc->ops->drop(q->qdisc)) != 0) { + sch->q.qlen--; + sch->stats.backlog -= len; sch->stats.drops++; - kfree_skb(skb); - return 1; } - return 0; + return len; } static void tbf_watchdog(unsigned long arg) @@ -189,19 +192,19 @@ netif_schedule(sch->dev); } -static struct sk_buff * -tbf_dequeue(struct Qdisc* sch) +static struct sk_buff *tbf_dequeue(struct Qdisc* sch) { struct tbf_sched_data *q = (struct tbf_sched_data *)sch->data; struct sk_buff *skb; - skb = __skb_dequeue(&sch->q); + skb = q->qdisc->dequeue(q->qdisc); if (skb) { psched_time_t now; long toks; long ptoks = 0; - + unsigned int len = skb->len; + PSCHED_GET_TIME(now); toks = PSCHED_TDIFF_SAFE(now, q->t_c, q->buffer, 0); @@ -210,18 +213,19 @@ ptoks = toks + q->ptokens; if (ptoks > (long)q->mtu) ptoks = q->mtu; - ptoks -= L2T_P(q, skb->len); + ptoks -= L2T_P(q, len); } toks += q->tokens; if (toks > (long)q->buffer) toks = q->buffer; - toks -= L2T(q, skb->len); + toks -= L2T(q, len); if ((toks|ptoks) >= 0) { q->t_c = now; q->tokens = toks; q->ptokens = ptoks; - sch->stats.backlog -= skb->len; + sch->stats.backlog -= len; + sch->q.qlen--; sch->flags &= ~TCQ_F_THROTTLED; return skb; } @@ -245,20 +249,25 @@ This is the main idea of all FQ algorithms (cf. CSZ, HPFQ, HFSC) */ - __skb_queue_head(&sch->q, skb); - + + if (q->qdisc->ops->requeue(skb, q->qdisc) != NET_XMIT_SUCCESS) { + /* When requeue fails skb is dropped */ + sch->q.qlen--; + sch->stats.backlog -= len; + sch->stats.drops++; + } + sch->flags |= TCQ_F_THROTTLED; sch->stats.overlimits++; } return NULL; } - -static void -tbf_reset(struct Qdisc* sch) +static void tbf_reset(struct Qdisc* sch) { struct tbf_sched_data *q = (struct tbf_sched_data *)sch->data; + qdisc_reset(q->qdisc); skb_queue_purge(&sch->q); sch->stats.backlog = 0; PSCHED_GET_TIME(q->t_c); @@ -268,6 +277,31 @@ del_timer(&q->wd_timer); } +static struct Qdisc *tbf_create_dflt_qdisc(struct net_device *dev, u32 limit) +{ + struct Qdisc *q = qdisc_create_dflt(dev, &bfifo_qdisc_ops); + struct rtattr *rta; + int ret; + + if (q) { + rta = kmalloc(RTA_LENGTH(sizeof(struct tc_fifo_qopt)), GFP_KERNEL); + if (rta) { + rta->rta_type = RTM_NEWQDISC; + rta->rta_len = RTA_LENGTH(sizeof(struct tc_fifo_qopt)); + ((struct tc_fifo_qopt *)RTA_DATA(rta))->limit = limit; + + ret = q->ops->change(q, rta); + kfree(rta); + + if (ret == 0) + return q; + } + qdisc_destroy(q); + } + + return NULL; +} + static int tbf_change(struct Qdisc* sch, struct rtattr *opt) { int err = -EINVAL; @@ -276,6 +310,7 @@ struct tc_tbf_qopt *qopt; struct qdisc_rate_table *rtab = NULL; struct qdisc_rate_table *ptab = NULL; + struct Qdisc *child = NULL; int max_size,n; if (rtattr_parse(tb, TCA_TBF_PTAB, RTA_DATA(opt), RTA_PAYLOAD(opt)) || @@ -308,8 +343,14 @@ } if (max_size < 0) goto done; + + if (q->qdisc == &noop_qdisc) { + if ((child = tbf_create_dflt_qdisc(sch->dev, qopt->limit)) == NULL) + goto done; + } sch_tree_lock(sch); + if (child) q->qdisc = child; q->limit = qopt->limit; q->mtu = qopt->mtu; q->max_size = max_size; @@ -342,6 +383,8 @@ init_timer(&q->wd_timer); q->wd_timer.function = tbf_watchdog; q->wd_timer.data = (unsigned long)sch; + + q->qdisc = &noop_qdisc; if ((err = tbf_change(sch, opt)) != 0) { MOD_DEC_USE_COUNT; @@ -359,6 +402,9 @@ qdisc_put_rtab(q->P_tab); if (q->R_tab) qdisc_put_rtab(q->R_tab); + + qdisc_destroy(q->qdisc); + q->qdisc = &noop_qdisc; MOD_DEC_USE_COUNT; } @@ -391,10 +437,93 @@ return -1; } +static int tbf_dump_class(struct Qdisc *sch, unsigned long cl, + struct sk_buff *skb, struct tcmsg *tcm) +{ + struct tbf_sched_data *q = (struct tbf_sched_data*)sch->data; + + if (cl != 1) /* only one class */ + return -ENOENT; + + tcm->tcm_parent = TC_H_ROOT; + tcm->tcm_handle = 1; + tcm->tcm_info = q->qdisc->handle; + + return 0; +} + +static int tbf_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new, + struct Qdisc **old) +{ + struct tbf_sched_data *q = (struct tbf_sched_data *)sch->data; + + if (new == NULL) + new = &noop_qdisc; + + sch_tree_lock(sch); + *old = xchg(&q->qdisc, new); + qdisc_reset(*old); + sch_tree_unlock(sch); + + return 0; +} + +static struct Qdisc *tbf_leaf(struct Qdisc *sch, unsigned long arg) +{ + struct tbf_sched_data *q = (struct tbf_sched_data *)sch->data; + return q->qdisc; +} + +static unsigned long tbf_get(struct Qdisc *sch, u32 classid) +{ + return 1; +} + +static void tbf_put(struct Qdisc *sch, unsigned long arg) +{ +} + +static int tbf_change_class(struct Qdisc *sch, u32 classid, u32 parentid, + struct rtattr **tca, unsigned long *arg) +{ + return -ENOSYS; +} + +static int tbf_delete(struct Qdisc *sch, unsigned long arg) +{ + return -ENOSYS; +} + +static void tbf_walk(struct Qdisc *sch, struct qdisc_walker *walker) +{ + struct tbf_sched_data *q = (struct tbf_sched_data *)sch->data; + + if (!walker->stop) { + if (walker->count >= walker->skip) + if (walker->fn(sch, (unsigned long)q, walker) < 0) { + walker->stop = 1; + return; + } + walker->count++; + } +} + +static struct Qdisc_class_ops tbf_class_ops = +{ + .graft = tbf_graft, + .leaf = tbf_leaf, + .get = tbf_get, + .put = tbf_put, + .change = tbf_change_class, + .delete = tbf_delete, + .walk = tbf_walk, + .dump = tbf_dump_class, +}; + struct Qdisc_ops tbf_qdisc_ops = { NULL, - NULL, + &tbf_class_ops, "tbf", sizeof(struct tbf_sched_data),
| |