Re: [PATCH] sch_tbf.c sporadically returning EINVAL

[Werner Almesberger <Werner.Almesberger@epfl.ch>]

kuznet@ms2.inr.ac.ru wrote:
> It is not required. mtu can have any value, provided filter
> does not classify larger packets to this qdisc.

Hmm, so do you suggest to change the check in the kernel such that it is
no longer based on psched_mtu ? If yes, what would you think of the patch
below ?

> Actually, tc must calculate max_size. I hate putting validity
> checks to kernel. Unfortunately, I forgot "max_size" in TBF parameters. Alas.

3x agreed ;-)

> So, if it is wrong, nothing happens but deadlocking tbf.
> Well, it is not much worse than printing an error message. 8)
> It also shows that something is configured wrongly. 8)

Hehe, I can think of a nice adaptation of Perl's motto "There's more
than one way to do it." for traffic control: "There's more than one way
to shoot yourself in the foot." ;-)

Well, we can catch most likely mistakes in user space and issue some
polite warning. Nevertheless, I must say that I don't really like TBF
deadlocks. If it just discards offending packets, we'll at least get
some other traffic through. My patch should take this into account.
(I hope - by how tired I am right now, I'm actually suprised that it
even compiles ;-)

Cheers, Werner

------------------------------------ patch ------------------------------------

--- linux/net/sched/sch_tbf.c.orig	Sun Feb 11 13:30:04 2001
+++ linux/net/sched/sch_tbf.c	Wed Feb 14 23:25:58 2001
@@ -276,7 +276,7 @@
 	struct tc_tbf_qopt *qopt;
 	struct qdisc_rate_table *rtab = NULL;
 	struct qdisc_rate_table *ptab = NULL;
-	int max_size;
+	int max_size,n;
 
 	if (rtattr_parse(tb, TCA_TBF_PTAB, RTA_DATA(opt), RTA_PAYLOAD(opt)) ||
 	    tb[TCA_TBF_PARMS-1] == NULL ||
@@ -295,16 +295,17 @@
 			goto done;
 	}
 
-	max_size = psched_mtu(sch->dev);
+	for (n = 0; n < 256; n++)
+		if (rtab->data[n] > qopt->buffer) break;
+	max_size = (n << qopt->rate.cell_log)-1;
 	if (ptab) {
-		int n = max_size>>qopt->peakrate.cell_log;
-		while (n>0 && ptab->data[n-1] > qopt->mtu) {
-			max_size -= (1<<qopt->peakrate.cell_log);
-			n--;
-		}
+		int size;
+
+		for (n = 0; n < 256; n++)
+			if (ptab->data[n] > qopt->mtu) break;
+		size = (n << qopt->peakrate.cell_log)-1;
+		if (size < max_size) max_size = size;
 	}
-	if (rtab->data[max_size>>qopt->rate.cell_log] > qopt->buffer)
-		goto done;
 
 	sch_tree_lock(sch);
 	q->limit = qopt->limit;

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