Patchwork CBQ(but probably u32 filter bug), kernel "freeze", at least from 3.2.0

login
register
mail settings
Submitter Eric Dumazet
Date Aug. 28, 2012, 10:59 a.m.
Message ID <1346151568.3001.20.camel@edumazet-glaptop>
Download mbox | patch
Permalink /patch/180439/
State RFC
Delegated to: David Miller
Headers show

Comments

Eric Dumazet - Aug. 28, 2012, 10:59 a.m.
On Tue, 2012-08-28 at 07:50 +0300, Denys Fedoryshchenko wrote:
> Hi
> 
> Got information from friend, confirmed that it crashed at least two my 
> boxes also :)
> 3.0.5-rc1 is working fine, 3.4.1 , 3.2.0 from ubuntu  - crashing
> No watchdog fired, and didn't got yet significant debugging 
> information.
> 
> Very easy to reproduce:
> 1)run the script
> 2)ping 192.168.3.234
> 
> script:
> DEV_OUT=eth0
> ICMP="match ip protocol 1 0xff"
> U32="protocol ip u32"
> DST="match ip dst"
> tc qdisc add dev $DEV_OUT root handle 1: cbq avpkt 1000 bandwidth 
> 100mbit
> tc class add dev $DEV_OUT parent 1: classid 1:1 cbq rate 512kbit allot 
> 1500 prio 5 bounded isolated
> tc filter add dev $DEV_OUT parent 1:              prio 3 $U32 $ICMP 
> $DST 192.168.3.234 flowid 1:
> tc qdisc add dev $DEV_OUT parent 1:1 sfq perturb 10

Not sure what your friend expected from this buggy configuration.

It probably never worked at all.

CBQ needs at least one child class and one leaf class.

This scripts creates a loop inside CBQ, so cpu is probably looping in
cbq_enqueue() (or more exactly cbq_classify()), as instructed by the
sysadmin ;)

u32 (or sfq) seems ok.

Could you try the following patch ?

Thanks !



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - Sept. 6, 2012, 8:40 a.m.
On Tue, 2012-08-28 at 03:59 -0700, Eric Dumazet wrote:
> On Tue, 2012-08-28 at 07:50 +0300, Denys Fedoryshchenko wrote:
> > Hi
> > 
> > Got information from friend, confirmed that it crashed at least two my 
> > boxes also :)
> > 3.0.5-rc1 is working fine, 3.4.1 , 3.2.0 from ubuntu  - crashing
> > No watchdog fired, and didn't got yet significant debugging 
> > information.
> > 
> > Very easy to reproduce:
> > 1)run the script
> > 2)ping 192.168.3.234
> > 
> > script:
> > DEV_OUT=eth0
> > ICMP="match ip protocol 1 0xff"
> > U32="protocol ip u32"
> > DST="match ip dst"
> > tc qdisc add dev $DEV_OUT root handle 1: cbq avpkt 1000 bandwidth 
> > 100mbit
> > tc class add dev $DEV_OUT parent 1: classid 1:1 cbq rate 512kbit allot 
> > 1500 prio 5 bounded isolated
> > tc filter add dev $DEV_OUT parent 1:              prio 3 $U32 $ICMP 
> > $DST 192.168.3.234 flowid 1:
> > tc qdisc add dev $DEV_OUT parent 1:1 sfq perturb 10
> 
> Not sure what your friend expected from this buggy configuration.
> 
> It probably never worked at all.
> 
> CBQ needs at least one child class and one leaf class.
> 
> This scripts creates a loop inside CBQ, so cpu is probably looping in
> cbq_enqueue() (or more exactly cbq_classify()), as instructed by the
> sysadmin ;)
> 
> u32 (or sfq) seems ok.
> 
> Could you try the following patch ?
> 
> Thanks !
> 
> diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
> index 6aabd77..564b9fc 100644
> --- a/net/sched/sch_cbq.c
> +++ b/net/sched/sch_cbq.c
> @@ -250,10 +250,11 @@ cbq_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
>  			else if ((cl = defmap[res.classid & TC_PRIO_MAX]) == NULL)
>  				cl = defmap[TC_PRIO_BESTEFFORT];
>  
> -			if (cl == NULL || cl->level >= head->level)
> +			if (cl == NULL)
>  				goto fallback;
>  		}
> -
> +		if (cl->level >= head->level)
> +			goto fallback;
>  #ifdef CONFIG_NET_CLS_ACT
>  		switch (result) {
>  		case TC_ACT_QUEUED:
> 

Hi Denys

Any feedback on the suggested patch ?

Thanks !


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Denys Fedoryshchenko - Sept. 6, 2012, 1:47 p.m.
On 2012-09-06 11:40, Eric Dumazet wrote:
> On Tue, 2012-08-28 at 03:59 -0700, Eric Dumazet wrote:
>> On Tue, 2012-08-28 at 07:50 +0300, Denys Fedoryshchenko wrote:
>> > Hi
>> >
>> > Got information from friend, confirmed that it crashed at least 
>> two my
>> > boxes also :)
>> > 3.0.5-rc1 is working fine, 3.4.1 , 3.2.0 from ubuntu  - crashing
>> > No watchdog fired, and didn't got yet significant debugging
>> > information.
>> >
>> > Very easy to reproduce:
>> > 1)run the script
>> > 2)ping 192.168.3.234
>> >
>> > script:
>> > DEV_OUT=eth0
>> > ICMP="match ip protocol 1 0xff"
>> > U32="protocol ip u32"
>> > DST="match ip dst"
>> > tc qdisc add dev $DEV_OUT root handle 1: cbq avpkt 1000 bandwidth
>> > 100mbit
>> > tc class add dev $DEV_OUT parent 1: classid 1:1 cbq rate 512kbit 
>> allot
>> > 1500 prio 5 bounded isolated
>> > tc filter add dev $DEV_OUT parent 1:              prio 3 $U32 
>> $ICMP
>> > $DST 192.168.3.234 flowid 1:
>> > tc qdisc add dev $DEV_OUT parent 1:1 sfq perturb 10
>>
>> Not sure what your friend expected from this buggy configuration.
>>
>> It probably never worked at all.
>>
>> CBQ needs at least one child class and one leaf class.
>>
>> This scripts creates a loop inside CBQ, so cpu is probably looping 
>> in
>> cbq_enqueue() (or more exactly cbq_classify()), as instructed by the
>> sysadmin ;)
>>
>> u32 (or sfq) seems ok.
>>
>> Could you try the following patch ?
>>
>> Thanks !
>>
>> diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
>> index 6aabd77..564b9fc 100644
>> --- a/net/sched/sch_cbq.c
>> +++ b/net/sched/sch_cbq.c
>> @@ -250,10 +250,11 @@ cbq_classify(struct sk_buff *skb, struct Qdisc 
>> *sch, int *qerr)
>>  			else if ((cl = defmap[res.classid & TC_PRIO_MAX]) == NULL)
>>  				cl = defmap[TC_PRIO_BESTEFFORT];
>>
>> -			if (cl == NULL || cl->level >= head->level)
>> +			if (cl == NULL)
>>  				goto fallback;
>>  		}
>> -
>> +		if (cl->level >= head->level)
>> +			goto fallback;
>>  #ifdef CONFIG_NET_CLS_ACT
>>  		switch (result) {
>>  		case TC_ACT_QUEUED:
>>
>
> Hi Denys
>
> Any feedback on the suggested patch ?
>
> Thanks !
Dear Eric

Very sorry for delay, most of time in desert, without decent internet.
I will try to test today or tomorrow.

---
Denys Fedoryshchenko, Network Engineer, Virtual ISP S.A.L.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - Sept. 6, 2012, 1:56 p.m.
On Thu, 2012-09-06 at 16:47 +0300, Denys Fedoryshchenko wrote:

> Dear Eric
> 
> Very sorry for delay, most of time in desert, without decent internet.
> I will try to test today or tomorrow.

No problem, I reproduced the bug on my dev machine, but its always
better to have bug reporter adding its own 'Tested-by:' tag ;)

Thanks


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 6aabd77..564b9fc 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -250,10 +250,11 @@  cbq_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 			else if ((cl = defmap[res.classid & TC_PRIO_MAX]) == NULL)
 				cl = defmap[TC_PRIO_BESTEFFORT];
 
-			if (cl == NULL || cl->level >= head->level)
+			if (cl == NULL)
 				goto fallback;
 		}
-
+		if (cl->level >= head->level)
+			goto fallback;
 #ifdef CONFIG_NET_CLS_ACT
 		switch (result) {
 		case TC_ACT_QUEUED: