[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1174382739.4864.11.camel@localhost>
Date: Tue, 20 Mar 2007 05:25:39 -0400
From: jamal <hadi@...erus.ca>
To: Patrick McHardy <kaber@...sh.net>
Cc: David Miller <davem@...emloft.net>, chris@...lexsecurity.com,
netdev@...r.kernel.org, tgraf@...g.ch
Subject: Re: Oops in filter add
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
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists