On Wed, 2006-11-01 at 05:54 -0800, Amnon Aaronsohn wrote: > --- 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 > which mentions only bands 0-2.
Yes, this could happen - we need to fix tc. Send a patch such that when the bands are greater than the default then priomap also needs to be changed. > When prio_init() is > called it sets all bands to noop_qdisc. > Then it calls prio_tune(), which goes over the > prio2band[] array, and initializes the bands that > appear in the array. Since the array doesn't > contain the number 3, band 3 isn't initialized -- it's > still set to noop_qdisc, while the other 3 bands use > the default qdisc. > > So, using CLASSIFY iptables target set to class 1:4 > will make skb->priority point to band 3, which is > incorrectly initialized, even though we configured > prio to have bands 0-3. That's the bug. > tc is where the bug is for sure because it provides bad policy. The consequences of what you have described which is bad configuration implies that that packet ends up in a blackhole i.e the packet gets dropped. It is not exactly a bug; the system will not oops; you get what you programmed it to do. I would agree though that if you can test with the scenarios i described earlier to be safe, this is an ok optimization (not a bug fix ;->) > Even if you consider such a use of the priority > illegitimate, the loop > in prio_tune() still seems a little strange. Why does > it need to go over prio2band and access each band > multiple times instead of simply going linearly over > the requested number of bands? > So send a patch for tc then do the tests i described earlier and if all goes well your patch should be fine. cheers, jamal - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
