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 whil
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 h
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 th
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 mem
* 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 t
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,
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 befor
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 read_lock(qdi
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 str
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:
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 = tp->ops->ch
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
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 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
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
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 fi
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)
>
17 matches
Mail list logo
