netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* iproute u32 filter - server hang
@ 2010-03-31  9:10 Paweł Staszewski
  2010-03-31  9:34 ` Patrick McHardy
  0 siblings, 1 reply; 6+ messages in thread
From: Paweł Staszewski @ 2010-03-31  9:10 UTC (permalink / raw)
  To: Linux Network Development list

I find some problem with iproute2 and u32 filters

To reproduce the problem (need to make one mistake in filter parent 
declaration 1:101):

tc qdisc add dev eth0 root handle 1: hfsc default 63
tc class add dev eth0 parent 1: classid 1:1 hfsc sc rate 100mbit ul rate 
100mbit
tc class add dev eth0 parent 1:1 classid 1:2 hfsc sc rate 1mbit ul rate 
1mbit
tc class add dev eth0 parent 1:1 classid 1:63 hfsc sc rate 99mbit ul 
rate 99mbit
tc class add dev eth0 parent 1:1 classid 1:101 hfsc sc rate 8kbit ul 
rate 1mbit
tc class add dev eth0 parent 1:101 classid 1:102 hfsc sc rate 8kbit ul 
rate 1mbit
tc filter add dev eth0 protocol ip parent 1: u32 match ip dst 
212.77.100.101 flowid 1:101
tc filter add dev eth0 protocol ip parent 1:101 u32 match ip protocol 1 
0xff flowid 1:101

ping 212.77.100.101
And after this server will stop responding to anything - without any 
error (hang).



With a little different rules:
tc qdisc add dev eth0 root handle 1: hfsc default 63
tc class add dev eth0 parent 1: classid 1:1 hfsc sc rate 100mbit ul rate 
100mbit
tc class add dev eth0 parent 1:1 classid 1:2 hfsc sc rate 1mbit ul rate 
1mbit
tc class add dev eth0 parent 1:1 classid 1:63 hfsc sc rate 99mbit ul 
rate 99mbit
tc class add dev eth0 parent 1:1 classid 1:101 hfsc sc rate 8kbit ul 
rate 1mbit
tc class add dev eth0 parent 1:101 classid 1:102 hfsc sc rate 8kbit ul 
rate 1mbit
tc filter add dev eth0 protocol ip parent 1: u32 match ip dst 
212.77.100.101 flowid 1:101
tc filter add dev eth0 protocol ip parent 1: u32 match ip protocol 1 
0xff flowid 1:101

ping 212.77.100.101
All is ok.

I check this with kernels 2.6.30.1 / 2.6.33 / 2.6.33.1
iproute tc utility version: iproute2-ss090324



Best Regards
Paweł Staszewski

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: iproute u32 filter - server hang
  2010-03-31  9:10 iproute u32 filter - server hang Paweł Staszewski
@ 2010-03-31  9:34 ` Patrick McHardy
  2010-03-31  9:42   ` Paweł Staszewski
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick McHardy @ 2010-03-31  9:34 UTC (permalink / raw)
  To: Paweł Staszewski; +Cc: Linux Network Development list

[-- Attachment #1: Type: text/plain, Size: 889 bytes --]

Paweł Staszewski wrote:
> I find some problem with iproute2 and u32 filters
> 
> To reproduce the problem (need to make one mistake in filter parent
> declaration 1:101):
> 
> ...
> tc filter add dev eth0 protocol ip parent 1:101 u32 match ip protocol 1
> 0xff flowid 1:101
> 
> ping 212.77.100.101
> And after this server will stop responding to anything - without any
> error (hang).

This is caused by hfsc_classify() looping endlessly since the filter
points to the originating class. hfsc_bind_tcf() is actually supposed
to prevent this, but it only prevents resolving the filter immediately
and we still run into the loop at runtime.

This patch (based on how CBQ handles this) should abort classification
and fall back to the default class. It would be better to simply catch
this at configuration time, but that looks a bit more involved. I'll try
to look into it this weekend.



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1295 bytes --]

diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index b38b39c..a435cf1 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1155,7 +1155,7 @@ static struct hfsc_class *
 hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 {
 	struct hfsc_sched *q = qdisc_priv(sch);
-	struct hfsc_class *cl;
+	struct hfsc_class *head, *cl;
 	struct tcf_result res;
 	struct tcf_proto *tcf;
 	int result;
@@ -1166,6 +1166,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 			return cl;
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
+	head = &q->root;
 	tcf = q->root.filter_list;
 	while (tcf && (result = tc_classify(skb, tcf, &res)) >= 0) {
 #ifdef CONFIG_NET_CLS_ACT
@@ -1180,6 +1181,8 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 		if ((cl = (struct hfsc_class *)res.class) == NULL) {
 			if ((cl = hfsc_find_class(res.classid, sch)) == NULL)
 				break; /* filter selected invalid classid */
+			if (cl->level >= head->level)
+				break; /* filter may only point downwards */
 		}
 
 		if (cl->level == 0)
@@ -1187,6 +1190,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 
 		/* apply inner filter chain */
 		tcf = cl->filter_list;
+		head = cl;
 	}
 
 	/* classification failed, try default class */

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: iproute u32 filter - server hang
  2010-03-31  9:34 ` Patrick McHardy
@ 2010-03-31  9:42   ` Paweł Staszewski
  2010-03-31  9:46     ` Patrick McHardy
  0 siblings, 1 reply; 6+ messages in thread
From: Paweł Staszewski @ 2010-03-31  9:42 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Linux Network Development list

W dniu 2010-03-31 11:34, Patrick McHardy pisze:
> Paweł Staszewski wrote:
>    
>> I find some problem with iproute2 and u32 filters
>>
>> To reproduce the problem (need to make one mistake in filter parent
>> declaration 1:101):
>>
>> ...
>> tc filter add dev eth0 protocol ip parent 1:101 u32 match ip protocol 1
>> 0xff flowid 1:101
>>
>> ping 212.77.100.101
>> And after this server will stop responding to anything - without any
>> error (hang).
>>      
> This is caused by hfsc_classify() looping endlessly since the filter
> points to the originating class. hfsc_bind_tcf() is actually supposed
> to prevent this, but it only prevents resolving the filter immediately
> and we still run into the loop at runtime.
>
> This patch (based on how CBQ handles this) should abort classification
> and fall back to the default class. It would be better to simply catch
> this at configuration time, but that looks a bit more involved. I'll try
> to look into it this weekend.
>
>
>    
I check this also with htb and the same problem like with hfsc.
This rules also hang my server.

tc qdisc del dev eth4 root
tc qdisc add dev eth4 root handle 1: htb default 63
tc class add dev eth4 parent 1: classid 1:1 htb rate 100mbit ceil 100mbit
tc class add dev eth4 parent 1:1 classid 1:2 htb rate 1mbit ceil 1mbit
tc class add dev eth4 parent 1:1 classid 1:63 htb rate 99mbit ceil 99mbit
tc class add dev eth4 parent 1:1 classid 1:101 htb rate 8kbit ceil 1mbit
tc class add dev eth4 parent 1:101 classid 1:102 htb rate 8kbit ceil 1mbit
tc filter add dev eth4 protocol ip parent 1: u32 match ip dst 
212.77.100.101 flowid 1:101
tc filter add dev eth4 protocol ip parent 1:101 u32 match ip protocol 1 
0xff flowid 1:101





^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: iproute u32 filter - server hang
  2010-03-31  9:42   ` Paweł Staszewski
@ 2010-03-31  9:46     ` Patrick McHardy
  2010-03-31  9:59       ` Paweł Staszewski
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick McHardy @ 2010-03-31  9:46 UTC (permalink / raw)
  To: Paweł Staszewski; +Cc: Linux Network Development list

Paweł Staszewski wrote:
> W dniu 2010-03-31 11:34, Patrick McHardy pisze:
>> Paweł Staszewski wrote:
>>   
>>> I find some problem with iproute2 and u32 filters
>>>
>>> To reproduce the problem (need to make one mistake in filter parent
>>> declaration 1:101):
>>>
>>> ...
>>> tc filter add dev eth0 protocol ip parent 1:101 u32 match ip protocol 1
>>> 0xff flowid 1:101
>>>
>>> ping 212.77.100.101
>>> And after this server will stop responding to anything - without any
>>> error (hang).
>>>      
>> This is caused by hfsc_classify() looping endlessly since the filter
>> points to the originating class. hfsc_bind_tcf() is actually supposed
>> to prevent this, but it only prevents resolving the filter immediately
>> and we still run into the loop at runtime.
>>
>> This patch (based on how CBQ handles this) should abort classification
>> and fall back to the default class. It would be better to simply catch
>> this at configuration time, but that looks a bit more involved. I'll try
>> to look into it this weekend.
>>
>>
>>    
> I check this also with htb and the same problem like with hfsc.
> This rules also hang my server.

Yes, HTB doesn't even catch loops when binding filters. As I said,
its a larger piece of work, for now please just try the patch I
sent.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: iproute u32 filter - server hang
  2010-03-31  9:46     ` Patrick McHardy
@ 2010-03-31  9:59       ` Paweł Staszewski
  2010-03-31 10:01         ` Patrick McHardy
  0 siblings, 1 reply; 6+ messages in thread
From: Paweł Staszewski @ 2010-03-31  9:59 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Linux Network Development list

W dniu 2010-03-31 11:46, Patrick McHardy pisze:
> Paweł Staszewski wrote:
>    
>> W dniu 2010-03-31 11:34, Patrick McHardy pisze:
>>      
>>> Paweł Staszewski wrote:
>>>
>>>        
>>>> I find some problem with iproute2 and u32 filters
>>>>
>>>> To reproduce the problem (need to make one mistake in filter parent
>>>> declaration 1:101):
>>>>
>>>> ...
>>>> tc filter add dev eth0 protocol ip parent 1:101 u32 match ip protocol 1
>>>> 0xff flowid 1:101
>>>>
>>>> ping 212.77.100.101
>>>> And after this server will stop responding to anything - without any
>>>> error (hang).
>>>>
>>>>          
>>> This is caused by hfsc_classify() looping endlessly since the filter
>>> points to the originating class. hfsc_bind_tcf() is actually supposed
>>> to prevent this, but it only prevents resolving the filter immediately
>>> and we still run into the loop at runtime.
>>>
>>> This patch (based on how CBQ handles this) should abort classification
>>> and fall back to the default class. It would be better to simply catch
>>> this at configuration time, but that looks a bit more involved. I'll try
>>> to look into it this weekend.
>>>
>>>
>>>
>>>        
>> I check this also with htb and the same problem like with hfsc.
>> This rules also hang my server.
>>      
> Yes, HTB doesn't even catch loops when binding filters. As I said,
> its a larger piece of work, for now please just try the patch I
> sent.
>    

Yes.
Your patch fix this problem.


Thanks
Paweł


> --
> 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
>
>
>    

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: iproute u32 filter - server hang
  2010-03-31  9:59       ` Paweł Staszewski
@ 2010-03-31 10:01         ` Patrick McHardy
  0 siblings, 0 replies; 6+ messages in thread
From: Patrick McHardy @ 2010-03-31 10:01 UTC (permalink / raw)
  To: Paweł Staszewski; +Cc: Linux Network Development list

Paweł Staszewski wrote:
> W dniu 2010-03-31 11:46, Patrick McHardy pisze:
>>>>> tc filter add dev eth0 protocol ip parent 1:101 u32 match ip
>>>>> protocol 1
>>>>> 0xff flowid 1:101
>>>>>
>>>>> ping 212.77.100.101
>>>>> And after this server will stop responding to anything - without any
>>>>> error (hang).
>>>>>
>>>>>          
>>>> This is caused by hfsc_classify() looping endlessly since the filter
>>>> points to the originating class. hfsc_bind_tcf() is actually supposed
>>>> to prevent this, but it only prevents resolving the filter immediately
>>>> and we still run into the loop at runtime.
>>>>
>>>> This patch (based on how CBQ handles this) should abort classification
>>>> and fall back to the default class. It would be better to simply catch
>>>> this at configuration time, but that looks a bit more involved. I'll
>>>> try
>>>> to look into it this weekend.
>>>>
>>>>
>>>>
>>>>        
>>> I check this also with htb and the same problem like with hfsc.
>>> This rules also hang my server.
>>>      
>> Yes, HTB doesn't even catch loops when binding filters. As I said,
>> its a larger piece of work, for now please just try the patch I
>> sent.
>>    
> 
> Yes.
> Your patch fix this problem.

Thanks for testing, I'll let you know once I have a complete
patch for this problem.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-03-31 10:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-31  9:10 iproute u32 filter - server hang Paweł Staszewski
2010-03-31  9:34 ` Patrick McHardy
2010-03-31  9:42   ` Paweł Staszewski
2010-03-31  9:46     ` Patrick McHardy
2010-03-31  9:59       ` Paweł Staszewski
2010-03-31 10:01         ` Patrick McHardy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).