[prev in list] [next in list] [prev in thread] [next in thread] 

List:       lartc
Subject:    [LARTC] HTB bug fix
From:       devik <devik () cdi ! cz>
Date:       2002-09-28 14:17:08
[Download RAW message or body]

Hello Thomasz (and LARTC),

regarding the crash-suite you send me. I located
the bug and attached patch should fix it.
Everyone who experienced crashes after "tc class change"
ahould apply and let me know.
If it will fix the issue I'll push it into 2.4.20 before
stable one.

Detail:
"tc class change" applied to inner class overwrites internal
union members because of missing test for leaf node.

best regards,
devik

PS: thanks, Thomasz

["thomas_htb_fix.diff" (TEXT/PLAIN)]

--- net/sched/sch_htb.c	Thu Sep 26 11:41:08 2002
+++ net/sched/sch_htb.c	Sat Sep 28 15:53:59 2002
@@ -15,9 +15,11 @@
  *			helped a lot to locate nasty class stall bug
  *		Andi Kleen, Jamal Hadi, Bert Hubert
  *			code review and helpful comments on shaping
+ *		Tomasz Wrona, <tw@eter.tym.pl>
+ *			created test case so that I was able to fix nasty bug
  *		and many others. thanks.
  *
- * $Id: sch_htb.c,v 1.13 2002/05/25 09:04:50 devik Exp $
+ * $Id: sch_htb.c,v 1.14 2002/09/28 12:55:22 devik Exp devik $
  */
 #include <linux/config.h>
 #include <linux/module.h>
@@ -69,7 +71,7 @@
 #define HTB_HYSTERESIS 1/* whether to use mode hysteresis for speedup */
 #define HTB_QLOCK(S) spin_lock_bh(&(S)->dev->queue_lock)
 #define HTB_QUNLOCK(S) spin_unlock_bh(&(S)->dev->queue_lock)
-#define HTB_VER 0x30006	/* major must be matched with number suplied by TC as \
version */ +#define HTB_VER 0x30007	/* major must be matched with number suplied by \
TC as version */  
 #if HTB_VER >> 16 != TC_HTB_PROTOVER
 #error "Mismatched sch_htb.c and pkt_sch.h"
@@ -1174,6 +1176,8 @@
 	skb_queue_head_init(&q->direct_queue);
 
 	q->direct_qlen = sch->dev->tx_queue_len;
+	if (q->direct_qlen < 2) /* some devices have zero tx_queue_len */
+		q->direct_qlen = 2;
 	q->timer.function = htb_timer;
 	q->timer.data = (unsigned long)sch;
 
@@ -1434,12 +1438,16 @@
 	parent = parentid == TC_H_ROOT ? NULL : htb_find (parentid,sch);
 
 	hopt = RTA_DATA(tb[TCA_HTB_PARMS-1]);
-	HTB_DBG(0,1,"htb_chg cl=%p, clid=%X, opt/prio=%d, rate=%u, buff=%d, quant=%d\n", \
cl,cl?cl->classid:0,(int)hopt->prio,hopt->rate.rate,hopt->buffer,hopt->quantum); \
+	HTB_DBG(0,1,"htb_chg cl=%p(%X), clid=%X, parid=%X, opt/prio=%d, rate=%u, buff=%d, \
quant=%d\n", cl,cl?cl->classid:0,classid,parentid,(int)hopt->prio,hopt->rate.rate,hopt->buffer,hopt->quantum);
  rtab = qdisc_get_rtab(&hopt->rate, tb[TCA_HTB_RTAB-1]);
 	ctab = qdisc_get_rtab(&hopt->ceil, tb[TCA_HTB_CTAB-1]);
 	if (!rtab || !ctab) goto failure;
 
 	if (!cl) { /* new class */
+		/* check for valid classid */
+		if (!classid || TC_H_MAJ(classid^sch->handle) || htb_find(classid,sch))
+			goto failure;
+
 		/* check maximal depth */
 		if (parent && parent->parent && parent->parent->level < 2) {
 			printk(KERN_ERR "htb: tree is too deep\n");
@@ -1501,19 +1509,23 @@
 #endif
 	} else sch_tree_lock(sch);
 
-	cl->un.leaf.quantum = rtab->rate.rate / q->rate2quantum;
-	if (!hopt->quantum && cl->un.leaf.quantum < 1000) {
-		printk(KERN_WARNING "HTB: quantum of class %X is small. Consider r2q change.", \
                cl->classid);
-		cl->un.leaf.quantum = 1000;
-	}
-	if (!hopt->quantum && cl->un.leaf.quantum > 200000) {
-		printk(KERN_WARNING "HTB: quantum of class %X is big. Consider r2q change.", \
                cl->classid);
-		cl->un.leaf.quantum = 200000;
-	}
-	if (hopt->quantum)
-		cl->un.leaf.quantum = hopt->quantum;
-	if ((cl->un.leaf.prio = hopt->prio) >= TC_HTB_NUMPRIO)
-		cl->un.leaf.prio = TC_HTB_NUMPRIO - 1;
+	/* it used to be a nasty bug here, we have to check that node
+           is really leaf before changing cl->un.leaf ! */
+	if (!cl->level) {
+		cl->un.leaf.quantum = rtab->rate.rate / q->rate2quantum;
+		if (!hopt->quantum && cl->un.leaf.quantum < 1000) {
+			printk(KERN_WARNING "HTB: quantum of class %X is small. Consider r2q change.", \
cl->classid); +			cl->un.leaf.quantum = 1000;
+		}
+		if (!hopt->quantum && cl->un.leaf.quantum > 200000) {
+			printk(KERN_WARNING "HTB: quantum of class %X is big. Consider r2q change.", \
cl->classid); +			cl->un.leaf.quantum = 200000;
+		}
+		if (hopt->quantum)
+			cl->un.leaf.quantum = hopt->quantum;
+		if ((cl->un.leaf.prio = hopt->prio) >= TC_HTB_NUMPRIO)
+			cl->un.leaf.prio = TC_HTB_NUMPRIO - 1;
+	}
 
 	cl->buffer = hopt->buffer;
 	cl->cbuffer = hopt->cbuffer;


_______________________________________________
LARTC mailing list / LARTC@mailman.ds9a.nl
http://mailman.ds9a.nl/mailman/listinfo/lartc HOWTO: http://lartc.org/

[prev in list] [next in list] [prev in thread] [next in thread]