On Tue, 2007-20-03 at 11:58 +0100, Patrick McHardy wrote:
jamal wrote:
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.
Yes. I think thats what we should do for 2.6.21, since fixing
this while
On Tue, 2007-20-03 at 07:58 +0100, Patrick McHardy wrote:
jamal wrote:
The main idea is to avoid one BigLock for both ingress and egress;
Which was/is still useful in the compat mode where netfilter is used
instead.
In that case is isn't even used.
Ok. It certainly used to matter
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 would need to make qdisc_lock_tree() aware of the
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
Thanks for all your replies!
One thing I did notice in examining tc_ctl_tfilter was that there is
something like:
qdisc_lock_tree(dev);
tp-next = *back;
*back = tp;
qdisc_unlock_tree(dev);
And then proceed to the data structure down below with:
err =
jamal wrote:
On Tue, 2007-20-03 at 08:29 +0100, Patrick McHardy wrote:
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
Chris Madden wrote:
Thanks for all your replies!
One thing I did notice in examining tc_ctl_tfilter was that there is
something like:
qdisc_lock_tree(dev);
tp-next = *back;
*back = tp;
qdisc_unlock_tree(dev);
And then proceed to the data structure down
jamal wrote:
On Mon, 2007-19-03 at 19:22 -0700, David Miller wrote:
Can you just replace the above with dev-queue_lock and see if
that makes your problem go away? THanks.
It should;
i will stare at the code later and see if i can send a better patch,
maybe a
Chris Madden wrote:
Ok, I replaced ingress_lock with queue_lock in ing_filter and it died in
the same place. Trace below (looks to be substantively the same)...
If I am reading tc_ctl_tfilter correctly, we are adding our new
tcf_proto to the end of the list, and it is getting used before
Patrick McHardy wrote:
Chris Madden wrote:
Ok, I replaced ingress_lock with queue_lock in ing_filter and it died in
the same place. Trace below (looks to be substantively the same)...
If I am reading tc_ctl_tfilter correctly, we are adding our new
tcf_proto to the end of the list, and it is
* Patrick McHardy [EMAIL PROTECTED] 2007-03-20 15:57
Actually its only cls_basic thats broken, in case of route and fw
its intentional for backwards-compatibility.
Absolutely right, thanks Patrick.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to
Thomas Graf wrote:
* Patrick McHardy [EMAIL PROTECTED] 2007-03-20 15:57
Actually its only cls_basic thats broken, in case of route and fw
its intentional for backwards-compatibility.
Absolutely right, thanks Patrick.
There was a small bug in my patch (broken return value on memory
Patrick McHardy wrote:
There was a small bug in my patch (broken return value on memory
allocation failure, not relevant for testing though). I'll push
the fixed patch to Dave once Chris confirms that it fixes the
problem he's seeing.
Looks like that may have done it. I ramped up the
Chris Madden wrote:
Patrick McHardy wrote:
There was a small bug in my patch (broken return value on memory
allocation failure, not relevant for testing though). I'll push
the fixed patch to Dave once Chris confirms that it fixes the
problem he's seeing.
Looks like that may have done it.
From: Chris Madden [EMAIL PROTECTED]
Date: Mon, 19 Mar 2007 16:10:29 -0400
I did some digging, and it appears the filter add isn't mutexed right.
Inside net/core/dev.c, ing_filter, I see:
spin_lock(dev-ingress_lock);
if ((q = dev-qdisc_ingress) != NULL)
result
On Mon, 2007-19-03 at 19:22 -0700, David Miller wrote:
I think this should use dev-queue_lock.
It would slow down things if he is doing both ingress and egress
traffic as well as control changes.
It looks like the idea might have been to allow more parallelized
running of ingress
jamal wrote:
On Mon, 2007-19-03 at 19:22 -0700, David Miller wrote:
It looks like the idea might have been to allow more parallelized
running of ingress filters, but this is done wrong and leads to
the crashes you are seeing.
The main idea is to avoid one BigLock for both ingress and
17 matches
Mail list logo
