Subject: Re: [PATCH REPOST 1/2] NET: Accurate packet scheduling for ATM/ADSL (kernel)

From: Patrick McHardy <[email-masked]>
Date: Thu, 19 Oct 2006 16:38:27 +0200

jamal wrote:
> ACKed-by: Jamal Hadi Salim
> 
> When Patrick has his patch ready after this goes in we can revisit.

NACK.

I still think this patch shouldn't go in. There's no point in doing the
same thing twice, and I haven't heard a compelling argument why it has
to be done in a way that only helps qdiscs using rtabs while ignoring
statistics and estimators (I even provided a patch to show how to do
it without these limitations).

Besides that:

+static inline u32 qdisc_l2t(struct qdisc_rate_table* rtab, int pktlen)
+{
+	int slot = pktlen + rtab->rate.cell_align;
+	if (slot < 0)
+	  	slot = 0;

Why would it go negative? A negative cell_align doesn't make sense I
guess.

+	slot >>= rtab->rate.cell_log;
+	if (slot > 255)
+		return rtab->data[255] + 1;

Whats the point of this? Is it just to keep htb giant statistics
working?