Re: Oops in filter add

[jamal <hadi@cyberus.ca>]

On Tue, 2007-20-03 at 08:29 +0100, Patrick McHardy wrote:
> jamal wrote:
> > On Tue, 2007-20-03 at 07:58 +0100, Patrick McHardy wrote:
> > 
> > Ok. It certainly used to matter in the old days.
> 
> 
> Actually it has never been used anywhere else but in ing_filter,
> it was introduced together with the TC actions.
> 

You are correct. I looked at old 2.4 and all i saw was:

----------
/* 
 revisit later: Use a private since lock dev->queue_lock is also
 used on the egress (might slow things for an iota)
*/
 
         if (dev->qdisc_ingress) {
                 spin_lock(&dev->queue_lock);
                 if ((q = dev->qdisc_ingress) != NULL)
                         fwres = q->enqueue(skb, q);
                 spin_unlock(&dev->queue_lock);
         }
------

So the resolution (as Dave points out) was wrong. In any case, restoring
queue_lock for now would slow things but will remove the race.

> 
> I'll try, but no promises, I'm a bit behind with various things myself.

I will ping you in a few days and if you havent done anything i will
take it up.
I am almost tempted to make the ingress filters to not have any
dependencies on egress whatsoever. It will create more locks but
will make the datapath faster. Actions can still be shared, but thats
lesser of an overhead.

cheers,
jamal