Re: Resubmit: [PATCH] net: fix PRIO qdisc bands init

2006-01-17 Thread David S. Miller
From: Amnon Aaronsohn [EMAIL PROTECTED] Date: Sat, 14 Jan 2006 16:24:59 +0200 (IST) Currently when PRIO is configured to use N bands, it lets the packets be directed to any of the bands 0..N-1. However, PRIO attaches a fifo qdisc only to the bands that appear in the priomap; the rest of the N

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-15 Thread jamal
On Fri, 2006-13-01 at 18:16 +0100, Thomas Graf wrote: * Patrick McHardy [EMAIL PROTECTED] 2006-01-12 06:25 In the last case in prio_classify (band q-bands) the band is returned directly. This could come from attached filters or from skb-priority. Its still not a fix since all bands

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-15 Thread Patrick McHardy
jamal wrote: On Fri, 2006-13-01 at 18:16 +0100, Thomas Graf wrote: * Patrick McHardy [EMAIL PROTECTED] 2006-01-12 06:25 In the last case in prio_classify (band q-bands) the band is returned directly. This could come from attached filters or from skb-priority. Its still not a fix since all

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-15 Thread jamal
On Mon, 2006-16-01 at 01:21 +0100, Patrick McHardy wrote: jamal wrote: The policy comes from user space. The kernel is supposed to be dumb, it just implements the mechanisms. If you tell the kernel here are 4 queues but i only want you to send packets to the first 3 and then somehow a

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-15 Thread Patrick McHardy
jamal wrote: On Mon, 2006-16-01 at 01:21 +0100, Patrick McHardy wrote: Well, part of the mechanism is manuall classification without the priomap using filters or skb-priority. So I disagree with this statement. So lets agree to disagree then. If i create a policy which specifies what

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-15 Thread jamal
On Mon, 2006-16-01 at 05:24 +0100, Patrick McHardy wrote: jamal wrote: On Mon, 2006-16-01 at 01:21 +0100, Patrick McHardy wrote: Well, part of the mechanism is manuall classification without the priomap using filters or skb-priority. So I disagree with this statement. So lets

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-15 Thread Patrick McHardy
jamal wrote: On Mon, 2006-16-01 at 05:24 +0100, Patrick McHardy wrote: If I say I want n bands, I don't want half of them initialized and the other half not. That's just confusing. Thats where we disagree - The kernel should not be making such decisions. Corrolary: If i wanted to have the

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-15 Thread jamal
On Mon, 2006-16-01 at 05:46 +0100, Patrick McHardy wrote: So as you can see it is perfectly fine to classify to any band without even using priomap. I dont think there is any arguement about that (eventually you have to fall to priomap though). I could also specify with filters or eventually

Re: Resubmit: [PATCH] net: fix PRIO qdisc bands init

2006-01-14 Thread jamal
On Sat, 2006-14-01 at 16:24 +0200, Amnon Aaronsohn wrote: Hi, Here's a clearer description of the patch. Currently when PRIO is configured to use N bands, it lets the packets be directed to any of the bands 0..N-1. However, PRIO attaches a fifo qdisc only to the bands that appear in the

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-13 Thread Thomas Graf
* Patrick McHardy [EMAIL PROTECTED] 2006-01-12 06:25 David S. Miller wrote: From: jamal [EMAIL PROTECTED] Date: Tue, 10 Jan 2006 23:02:45 -0500 So it is an optimization - not a bug then, correct? It is only an optimization in as much as it avoids duplicate work during initialization.

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-11 Thread Amnon Aaronsohn
So it is an optimization - not a bug then, correct? It's a bug fix that happens to be an optimization :) No matter what you set skb-priority to, that gets translated by prio2band[] which should only point to actually initialized queues. That's not correct. If prio_classify() sees that

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-11 Thread jamal
On Wed, 2006-11-01 at 01:54 -0800, Amnon Aaronsohn wrote: So it is an optimization - not a bug then, correct? It's a bug fix that happens to be an optimization :) No matter what you set skb-priority to, that gets translated by prio2band[] which should only point to actually

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-11 Thread Amnon Aaronsohn
--- jamal [EMAIL PROTECTED] wrote: You will always point to correctly initialized queues with any value of skb-priority. Ok, to put it concretely: Suppose we have prio configured thus: tc qdisc add dev eth0 root handle 1: prio bands 4 This qdisc has 4 bands but it uses the default priomap

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-11 Thread Patrick McHardy
David S. Miller wrote: From: jamal [EMAIL PROTECTED] Date: Tue, 10 Jan 2006 23:02:45 -0500 So it is an optimization - not a bug then, correct? It is only an optimization in as much as it avoids duplicate work during initialization. Like you I don't see how this patch fixes anything. No

[PATCH] net: fix PRIO qdisc bands init

2006-01-10 Thread Amnon Aaronsohn
The PRIO queuing discipline currently initializes only the bands that appear in the priomap. It should instead initialize all the configured bands. Signed-off-by: Amnon Aaronsohn [EMAIL PROTECTED] --- --- linux-2.6.14/net/sched/sch_prio.c 2005-10-28 02:02:08.0 +0200 +++

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-10 Thread jamal
lkml trimmed off. On Tue, 2006-10-01 at 12:30 +0200, Amnon Aaronsohn wrote: The PRIO queuing discipline currently initializes only the bands that appear in the priomap. It should instead initialize all the configured bands. Did you actually run into some issues that prompted you to produce

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-10 Thread Amnon Aaronsohn
--- jamal [EMAIL PROTECTED] wrote: Did you actually run into some issues that prompted you to produce this patch? Why would you want to have something not in the priomap to do any queueing of packets? I wrote this patch because the qdisc wasn't running correctly when I used priorities that

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-10 Thread jamal
On Tue, 2006-10-01 at 07:18 -0800, Amnon Aaronsohn wrote: [..] Anyway, even when all the bands appear in the priomap, it makes more sense to go over each band only once as the patch does, instead of multiple times as the current implementation. So it is an optimization - not a bug then,

Re: [PATCH] net: fix PRIO qdisc bands init

2006-01-10 Thread David S. Miller
From: jamal [EMAIL PROTECTED] Date: Tue, 10 Jan 2006 23:02:45 -0500 So it is an optimization - not a bug then, correct? It is only an optimization in as much as it avoids duplicate work during initialization. Like you I don't see how this patch fixes anything. No matter what you set