[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

two bug fixed for 2.4.0-prerelease



Hi Dave,

I've attached a patch that fixes two problems in traffic control. The
first one fixes an historical accident, while the second one corrects
a potential oops. In detail:

 - sch_dsmark limited range of default_index values to [1,indices-1],
   which is inconsistent with class range [0,0xffff] and with intended
   semantics of tc_index (reported by Rui Prior, fixed by me)
 - sch_gred may have tripped over NULL pointer if receiving traffic
   before complete initialization (reported by Rui Prior, fixed by me,
   fix approved by Jamal)

Compatibility note: the sch_dsmark change is fully backwards-compatible
with current correct uses. Nevertheless, it may be desirable to change
the behaviour of iproute2/tc for "default_index 0". But this is a
user-space issue.

The patch is for 2.4.0-prerelease plus prerelease-diff of January 1st.
I hope it's still possible to get those fixes into 2.4.

Cheers, Werner

---------------------------------- cut here -----------------------------------

--- linux.orig/net/sched/sch_dsmark.c	Tue Feb 22 01:32:27 2000
+++ linux/net/sched/sch_dsmark.c	Thu Jan  4 07:59:52 2001
@@ -39,16 +39,22 @@
  *   x:y y>0	 y+1		use entry [y]
  *   ...	 ...		...
  * x:indices-1	indices		use entry [indices-1]
+ *   ...	 ...		...
+ *   x:y	 y+1		use entry [y & (indices-1)]
+ *   ...	 ...		...
+ * 0xffff	0x10000		use entry [indices-1]
  */
 
 
+#define NO_DEFAULT_INDEX	(1 << 16)
+
 struct dsmark_qdisc_data {
 	struct Qdisc		*q;
 	struct tcf_proto	*filter_list;
 	__u8			*mask;	/* "owns" the array */
 	__u8			*value;
 	__u16			indices;
-	__u16			default_index;
+	__u32			default_index;	/* index range is 0...0xffff */
 	int			set_tc_index;
 };
 
@@ -217,7 +223,7 @@
 			case TC_POLICE_UNSPEC:
 				/* fall through */
 			default:
-				if (p->default_index)
+				if (p->default_index != NO_DEFAULT_INDEX)
 					skb->tc_index = p->default_index;
 				break;
 		};
@@ -325,14 +331,12 @@
 		if (tmp & 1)
 			return -EINVAL;
 	}
-	p->default_index = 0;
+	p->default_index = NO_DEFAULT_INDEX;
 	if (tb[TCA_DSMARK_DEFAULT_INDEX-1]) {
 		if (RTA_PAYLOAD(tb[TCA_DSMARK_DEFAULT_INDEX-1]) < sizeof(__u16))
 			return -EINVAL;
 		p->default_index =
 		    *(__u16 *) RTA_DATA(tb[TCA_DSMARK_DEFAULT_INDEX-1]);
-		if (!p->default_index || p->default_index >= p->indices)
-			return -EINVAL;
 	}
 	p->set_tc_index = !!tb[TCA_DSMARK_SET_TC_INDEX-1];
 	p->mask = kmalloc(p->indices*2,GFP_KERNEL);
@@ -411,9 +415,11 @@
 	rta = (struct rtattr *) b;
 	RTA_PUT(skb,TCA_OPTIONS,0,NULL);
 	RTA_PUT(skb,TCA_DSMARK_INDICES,sizeof(__u16),&p->indices);
-	if (p->default_index)
-		RTA_PUT(skb,TCA_DSMARK_DEFAULT_INDEX, sizeof(__u16),
-			&p->default_index);
+	if (p->default_index != NO_DEFAULT_INDEX) {
+		__u16 tmp = p->default_index;
+
+		RTA_PUT(skb,TCA_DSMARK_DEFAULT_INDEX, sizeof(__u16), &tmp);
+	}
 	if (p->set_tc_index)
 		RTA_PUT(skb, TCA_DSMARK_SET_TC_INDEX, 0, NULL);
 	rta->rta_len = skb->tail-b;
--- linux.orig/net/sched/sch_gred.c	Sat Aug 12 21:09:55 2000
+++ linux/net/sched/sch_gred.c	Thu Jan  4 07:22:32 2001
@@ -110,12 +110,9 @@
 	unsigned long	qave=0;	
 	int i=0;
 
-	if (!t->initd) {
-		DPRINTK("NO GRED Queues setup yet! Enqueued anyway\n");
-		if (q->backlog <= q->limit) {
-			__skb_queue_tail(&sch->q, skb);
-			return NET_XMIT_DROP; /* @@@@ */
-		}
+	if (!t->initd && skb_queue_len(&sch->q) <= sch->dev->tx_queue_len) {
+		D2PRINTK("NO GRED Queues setup yet! Enqueued anyway\n");
+		goto do_enqueue;
 	}
 
 
@@ -179,11 +176,12 @@
 		q->qcount = -1;
 enqueue:
 		if (q->backlog <= q->limit) {
+			q->backlog += skb->len;
+do_enqueue:
 			__skb_queue_tail(&sch->q, skb);
 			sch->stats.backlog += skb->len;
 			sch->stats.bytes += skb->len;
 			sch->stats.packets++;
-			q->backlog += skb->len;
 			return 0;
 		} else {
 			q->pdrop++;

-- 
  _________________________________________________________________________
 / Werner Almesberger, ICA, EPFL, CH           Werner.Almesberger@epfl.ch /
/_IN_N_032__Tel_+41_21_693_6621__Fax_+41_21_693_6610_____________________/