Re: one more... iproute commands lockup whole system

2007-04-04 Thread jamal

On Wed, 2007-04-04 at 05:11 +0300, Denys wrote:
 I think this highly useful feature given by jamal, difficult to be avoided 
 from crash, if user not enough experienced in networking(like me). I guess 
 packet can be even not ipv4/ipv6 packet, maybe it can be cloned IPX or ARP, 
 so TTL field cannot be used. I checked maybe sk_buff have some fields, seems 
 also bad luck, if there can be something like internal counter for packet, 
 how much times it got redirected, it will help. 

Adding a field in the skb that keeps track of things would work well,
but would be a controvesial thing to do because it actually requires a
vector not just one field. There is a filed called cb[] but it cant be
used in this case because every time we redirect it could be trampled.

 But in my case of VLAN's it 
 is really my own mistake and difficult to avoid it. Only bad thing - machine 
 got completely locked up, and if it is remote system - it will not oops/or 
 reboot even. But i dont have any idea in mind how to avoid this, only than 
 big warning in DOC and internal iproute2 help :-)

Your case is easy to detect in user space because it is within the same
policy.
Would simple detection and rejection in tc/userspace be useful to add?
Note, this doesnt help the general problem though where you have nesting
as described in the document.

cheers,
jamal

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


Re: one more... iproute commands lockup whole system

2007-04-04 Thread Patrick McHardy
jamal wrote:
 On Wed, 2007-04-04 at 05:11 +0300, Denys wrote:
 
I think this highly useful feature given by jamal, difficult to be avoided 
from crash, if user not enough experienced in networking(like me). I guess 
packet can be even not ipv4/ipv6 packet, maybe it can be cloned IPX or ARP, 
so TTL field cannot be used.


We have a loop counter (RTTL) in tc_verd. For some reason it is reset
after ing_filter though.

 I checked maybe sk_buff have some fields, seems 
also bad luck, if there can be something like internal counter for packet, 
how much times it got redirected, it will help. 
 
 
 Adding a field in the skb that keeps track of things would work well,
 but would be a controvesial thing to do because it actually requires a
 vector not just one field. There is a filed called cb[] but it cant be
 used in this case because every time we redirect it could be trampled.


It would be interesting to find out what the problem is exactly.
The configuration itself looks harmless, so I'm guessing its
rather a deadlock than a loop.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: one more... iproute commands lockup whole system

2007-04-04 Thread Denys
Well, my case is my own mistake, i guess it is just misconfiguration, not 
actual bug. And also it is good push for me read doc's and think well before 
adding rules.
Maybe it can be in TODO, but it is not N1 priority i guess. There is more 
important things, what u want to do. Another thing, adding one more field in 
skb will add more overhead to whole kernel(i guess).

I have some interesting thing:

Rules:
tc qdisc del dev eth0.5 root
tc qdisc add dev eth0.5 handle 1: root htb
tc class add dev eth0.5 parent 1:0 classid 1:2 htb rate 128Kbit

tc qdisc add dev eth0.5 parent 1:2 handle 2: prio

tc filter add dev eth0.5 parent 1: protocol ip prio 10 u32 \
match ip src 195.69.208.253/32 flowid 1:2

tc filter add dev eth0.5 parent 2: protocol ip prio 10 u32 \
match ip src 195.69.208.253/32 flowid 2:1 \
action mirred egress redirect dev eth0.6

(it is not working, but just i tried few things)

At morning i wakeup and see in dmesg, also not sure if it's bug or result of 
misconfiguration:

[46632.941527] KERNEL: assertion (!cl-level  cl-un.leaf.q  cl-
un.leaf.q-q.qlen) failed at net/sched/sch_htb.c (585)
[46633.270732] KERNEL: assertion (!cl-level  cl-un.leaf.q  cl-
un.leaf.q-q.qlen) failed at net/sched/sch_htb.c (585)
[46633.379446] KERNEL: assertion (!cl-level  cl-un.leaf.q  cl-
un.leaf.q-q.qlen) failed at net/sched/sch_htb.c (585)
[46633.450751] KERNEL: assertion (!cl-level  cl-un.leaf.q  cl-
un.leaf.q-q.qlen) failed at net/sched/sch_htb.c (585)
[46633.570702] KERNEL: assertion (!cl-level  cl-un.leaf.q  cl-
un.leaf.q-q.qlen) failed at net/sched/sch_htb.c (585)



On Wed, 04 Apr 2007 06:55:14 -0400, jamal wrote
 On Wed, 2007-04-04 at 05:11 +0300, Denys wrote:
  I think this highly useful feature given by jamal, difficult to be 
avoided 
  from crash, if user not enough experienced in networking(like me). I 
guess 
  packet can be even not ipv4/ipv6 packet, maybe it can be cloned IPX or 
ARP, 
  so TTL field cannot be used. I checked maybe sk_buff have some fields, 
seems 
  also bad luck, if there can be something like internal counter for 
packet, 
  how much times it got redirected, it will help.
 
 Adding a field in the skb that keeps track of things would work well,
 but would be a controvesial thing to do because it actually requires 
 a vector not just one field. There is a filed called cb[] but it 
 cant be used in this case because every time we redirect it could be 
 trampled.
 
  But in my case of VLAN's it 
  is really my own mistake and difficult to avoid it. Only bad thing - 
machine 
  got completely locked up, and if it is remote system - it will not oops/
or 
  reboot even. But i dont have any idea in mind how to avoid this, only 
than 
  big warning in DOC and internal iproute2 help :-)
 
 Your case is easy to detect in user space because it is within the same
 policy.
 Would simple detection and rejection in tc/userspace be useful to 
 add? Note, this doesnt help the general problem though where you 
 have nesting as described in the document.
 
 cheers,
 jamal
 
 -
 To unsubscribe from this list: send the line unsubscribe netdev in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
Denys Fedoryshchenko
Technical Manager
Virtual ISP S.A.L.

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


Re: one more... iproute commands lockup whole system

2007-04-04 Thread jamal
On Wed, 2007-04-04 at 15:36 +0200, Patrick McHardy wrote:

 
 We have a loop counter (RTTL) in tc_verd. For some reason it is reset
 after ing_filter though.
 

Essentially it is valuable just to avoid a lot of stacking (separate
issue) and not to avoid the locking issue he is seeing.


 It would be interesting to find out what the problem is exactly.
 The configuration itself looks harmless, so I'm guessing its
 rather a deadlock than a loop.

We know it is a deadlock. 
If you redirect the first time queue lock for eth0 will be held, before
it is released if you do another redirect, it will again be heading
towards eth0 and it will deadlock on grabbing the queue lock.

cheers,
jamal

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


Re: one more... iproute commands lockup whole system

2007-04-04 Thread Patrick McHardy
jamal wrote:
 On Wed, 2007-04-04 at 15:36 +0200, Patrick McHardy wrote:
 
It would be interesting to find out what the problem is exactly.
The configuration itself looks harmless, so I'm guessing its
rather a deadlock than a loop.
 
 
 We know it is a deadlock. 
 If you redirect the first time queue lock for eth0 will be held, before
 it is released if you do another redirect, it will again be heading
 towards eth0 and it will deadlock on grabbing the queue lock.


He only used a single redirect to eth0.5, but its probably due to the
fact that the VLAN hard_start_xmit function transmits on eth0 again.
How about adding something like ifb's ri_tasklet to act_mirred?

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


Re: HTB/act_mirred problem [was: one more... iproute commands lockup whole system]

2007-04-04 Thread jamal
On Wed, 2007-04-04 at 16:13 +0200, Patrick McHardy wrote:

 This seems to be due to be caused by act_mirred returning TC_ACT_STOLEN,
 which is translated to NET_XMIT_SUCCESS within prio, causing HTB to
 increase the q.qlen counter and activating the class despite no packet
 beeing queued.
 
 Jamal, we can't return NET_XMIT_SUCCESS unless we've really queued the
 packet. I can't remeber the reason why this is done, could you remind
 me?

IIRC, It had to do with not confusing TCP to try and retransmit. I can
go back and look at my notes to be certain. At one point i posted those
notes, it maybe time to add them to the kernel code or doc somewhere.

cheers,
jamal

PS:- i may a little slow in responding for the next few hours.

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


Re: one more... iproute commands lockup whole system

2007-04-04 Thread Patrick McHardy
jamal wrote:
 On Wed, 2007-04-04 at 16:07 +0200, Patrick McHardy wrote:
 
How about adding something like ifb's ri_tasklet to act_mirred?
 
 
 You mean having a tasklet or the constraint checks?


Have a tasklet to avoid the deadlocks.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: HTB/act_mirred problem [was: one more... iproute commands lockup whole system]

2007-04-04 Thread Patrick McHardy
jamal wrote:
 On Wed, 2007-04-04 at 16:13 +0200, Patrick McHardy wrote:
 
This seems to be due to be caused by act_mirred returning TC_ACT_STOLEN,
which is translated to NET_XMIT_SUCCESS within prio, causing HTB to
increase the q.qlen counter and activating the class despite no packet
beeing queued.

Jamal, we can't return NET_XMIT_SUCCESS unless we've really queued the
packet. I can't remeber the reason why this is done, could you remind
me?
 
 
 IIRC, It had to do with not confusing TCP to try and retransmit. 


No, that was the default return code which applies to TC_ACT_SHOT
and unclassified packets (29f1df6cc1c3ee3530939f0e38d80a9b50645ba5).
Returning NET_XMIT_SUCCESS for TC_ACT_STOLEN/TC_ACT_QUEUED has always
been done.

Anyway, we can't return NET_XMIT_SUCCESS, so how about just returning
NET_XMIT_BYPASS in all cases where the packet was stolen/dropped/...
by TC actions?

 I can
 go back and look at my notes to be certain. At one point i posted those
 notes, it maybe time to add them to the kernel code or doc somewhere.


In case they're still up to date, adding them somewhere under
Documentation/ sounds good.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: one more... iproute commands lockup whole system

2007-04-04 Thread Denys
Yes, it is real setup by the way, i just copypaste script here. I tried to 
remove some commands to leave only minimum to reproduce the problem.
Right now i dont have another ethernet card on that system, i will add it 
today, when i come back home and i will test in different situations. Also it 
is interesting, how system will act when i will redirect from eth0 to ppp0, 
tunnels, device with hardware offloading to device without hardware 
offloading and etc.

On Wed, 04 Apr 2007 10:10:10 -0400, jamal wrote
 On Wed, 2007-04-04 at 15:56 +0300, Denys wrote:
 
  Rules:
  tc qdisc del dev eth0.5 root
  tc qdisc add dev eth0.5 handle 1: root htb
  tc class add dev eth0.5 parent 1:0 classid 1:2 htb rate 128Kbit
  
  tc qdisc add dev eth0.5 parent 1:2 handle 2: prio
  
  tc filter add dev eth0.5 parent 1: protocol ip prio 10 u32 \
  match ip src 195.69.208.253/32 flowid 1:2
  
  tc filter add dev eth0.5 parent 2: protocol ip prio 10 u32 \
  match ip src 195.69.208.253/32 flowid 2:1 \
  action mirred egress redirect dev eth0.6
 
  (it is not working, but just i tried few things)
 
 
 This will still mean the physical device is eventually going to be 
 eth0. You are hanging, correct?
 If you redirected to another vlan ontop of a different physical
 ethernet, and the problem persists then it will be worrisome.
 Actually i am begining to doubt myself. Maybe the vlan device is acting
 strangely. I will try to test your simple setup towards the end of 
 day and see what happens.
 
 cheers,
 jamal
 
 -
 To unsubscribe from this list: send the line unsubscribe netdev in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
Denys Fedoryshchenko
Technical Manager
Virtual ISP S.A.L.

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


Re: HTB/act_mirred problem [was: one more... iproute commands lockup whole system]

2007-04-04 Thread jamal
On Wed, 2007-04-04 at 16:50 +0200, Patrick McHardy wrote:

 Anyway, we can't return NET_XMIT_SUCCESS, so how about just returning
 NET_XMIT_BYPASS in all cases where the packet was stolen/dropped/...
 by TC actions?

Ok, I think i get you now. Yes, that would work, but:
for the dropped case, you need to record into the dropped stats (unlike
the the other two).

  I can
  go back and look at my notes to be certain. At one point i posted those
  notes, it maybe time to add them to the kernel code or doc somewhere.
 
 
 In case they're still up to date, adding them somewhere under
 Documentation/ sounds good.

I think it is still uotodate, if not i will make sure it is before i
submit it.

cheers,
jamal

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


Re: one more... iproute commands lockup whole system

2007-04-04 Thread jamal
On Wed, 2007-04-04 at 16:39 +0200, Patrick McHardy wrote:

 Have a tasklet to avoid the deadlocks.

sounds like a good idea from the outset. The principle of punishing only
users of mirred is the right one. It will also provide a clean way to
drop all redirect to self.

The dilema is that the cost may end up being in complexity and
performance. Let me throw a few wrenches your way:
- Mirroring doesnt have this problem, only redirecting does. And the
main problem is on egress. So you may have to do the enqueueing only for
those cases, or we may just have to separate redirects from mirroring as
two separate actions. 
- We need to be able to support literally hundreds of such actions, so
if you do a single tasklet per action you will have quiet a few running
- i dont know what the impact is. A solution might be to do a single
tasklet with the mirred table; you will need many queues.
-There may be others like lock contention etc on trying to reinject
which will affect perfomance.
 
I am not against the idea Patrick, I am just worried about performance
and i feel that the majority of people who redirect will, once bitten
(as was Denys), eventually resort to doing the right config and would
rather have good perfomance. 

I could be selfish and wrong by looking at my needs for this action but
i prefer performance and from that perspective i look at this from
someone who didnt read the instructions on a gun and shot at their toe.
You could add a sensor that recognises a toe is being aimed at and pop
up an LCD with a question are you sure you want to shoot at _a
toe_? ;- but that adds to the cost.
I am exagerating - but i hope you get my point.  

cheers,
jamal

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


one more... iproute commands lockup whole system

2007-04-03 Thread Denys
I'm not sure it is mistake or error, but i feel it is dangerous, cause 
commands locking up the system, no kernel panic, no oops, so only watchdog 
can save poor server (and not sure this even)

Commands to lockup system (just i am giving my example, i didnt sort out what 
exactly locked up system, i guess redirecting to eth0.5, which is not 
intended for that):

vconfig add eth0 5
ifconfig eth0.5 192.168.1.2 netmask 255.255.255.128

tc qdisc add dev eth0 ingress
tc filter add dev eth0 parent : protocol ip prio 6 u32 \
match ip src 195.69.208.252/32 flowid 1:16 \
action police rate 64kbit burst 90k pipe \
action mirred egress mirror dev eth0.5

--
Denys Fedoryshchenko
Technical Manager
Virtual ISP S.A.L.

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


Re: one more... iproute commands lockup whole system

2007-04-03 Thread jamal


On Wed, 2007-04-04 at 03:03 +0300, Denys wrote:
 I'm not sure it is mistake or error, but i feel it is dangerous, cause 
 commands locking up the system, no kernel panic, no oops, so only watchdog 
 can save poor server (and not sure this even)
 
 Commands to lockup system (just i am giving my example, i didnt sort out what 
 exactly locked up system, i guess redirecting to eth0.5, which is not 
 intended for that):
 

read:
doc/actions/mirred-usage

cheers,
jamal


 vconfig add eth0 5
 ifconfig eth0.5 192.168.1.2 netmask 255.255.255.128
 
 tc qdisc add dev eth0 ingress
 tc filter add dev eth0 parent : protocol ip prio 6 u32 \
 match ip src 195.69.208.252/32 flowid 1:16 \
 action police rate 64kbit burst 90k pipe \
 action mirred egress mirror dev eth0.5
 
 --
 Denys Fedoryshchenko
 Technical Manager
 Virtual ISP S.A.L.
 
 -
 To unsubscribe from this list: send the line unsubscribe netdev in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

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


Re: one more... iproute commands lockup whole system

2007-04-03 Thread Patrick McHardy
jamal wrote:
 On Wed, 2007-04-04 at 03:03 +0300, Denys wrote:
 
I'm not sure it is mistake or error, but i feel it is dangerous, cause 
commands locking up the system, no kernel panic, no oops, so only watchdog 
can save poor server (and not sure this even)

Commands to lockup system (just i am giving my example, i didnt sort out what 
exactly locked up system, i guess redirecting to eth0.5, which is not 
intended for that):
 
 
 read:
 doc/actions/mirred-usage


Are you refering to What NOT to do if you dont want your machine to
crash:? I think we should make sure users can't even accidentally
crash their box, so this should at least be caught at runtime.
I thought the TTL stuff was intended to avoid this ..

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


Re: one more... iproute commands lockup whole system

2007-04-03 Thread jamal
On Wed, 2007-04-04 at 03:39 +0200, Patrick McHardy wrote:

 Are you refering to What NOT to do if you dont want your machine to
 crash:? 

yes.

 I think we should make sure users can't even accidentally
 crash their box, so this should at least be caught at runtime.

It is hard to do without penalizing the common use. Hence
the documentation. 
Sometimes - that is the most sane thing to do. 

I thought the TTL stuff was intended to avoid this ..

The problem is recursive dev queue locking of the same device.  
TTL is not very helpful there.

I dont want to rehash all those discussions we already had; so if you
have a clever way to fix this (without affecting performance of a sane
user), please send a patch and lets discuss.

cheers,
jamal

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


Re: one more... iproute commands lockup whole system

2007-04-03 Thread Denys
I think this highly useful feature given by jamal, difficult to be avoided 
from crash, if user not enough experienced in networking(like me). I guess 
packet can be even not ipv4/ipv6 packet, maybe it can be cloned IPX or ARP, 
so TTL field cannot be used. I checked maybe sk_buff have some fields, seems 
also bad luck, if there can be something like internal counter for packet, 
how much times it got redirected, it will help. But in my case of VLAN's it 
is really my own mistake and difficult to avoid it. Only bad thing - machine 
got completely locked up, and if it is remote system - it will not oops/or 
reboot even. But i dont have any idea in mind how to avoid this, only than 
big warning in DOC and internal iproute2 help :-)

On Wed, 04 Apr 2007 03:39:12 +0200, Patrick McHardy wrote
 jamal wrote:
  On Wed, 2007-04-04 at 03:03 +0300, Denys wrote:
  
 I'm not sure it is mistake or error, but i feel it is dangerous, cause 
 commands locking up the system, no kernel panic, no oops, so only 
watchdog 
 can save poor server (and not sure this even)
 
 Commands to lockup system (just i am giving my example, i didnt sort out 
what 
 exactly locked up system, i guess redirecting to eth0.5, which is not 
 intended for that):
  
  
  read:
  doc/actions/mirred-usage
 
 Are you refering to What NOT to do if you dont want your machine to
 crash:? I think we should make sure users can't even accidentally
 crash their box, so this should at least be caught at runtime.
 I thought the TTL stuff was intended to avoid this ..
 
 -
 To unsubscribe from this list: send the line unsubscribe netdev in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
Denys Fedoryshchenko
Technical Manager
Virtual ISP S.A.L.

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