Thread (87 messages) 87 messages, 6 authors, 2006-07-09

Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device

From: Jamal Hadi Salim <hidden>
Date: 2006-07-08 14:14:39

On Sat, 2006-08-07 at 12:54 +0200, Thomas Graf wrote:
* jamal [off-list ref] 2006-07-01 09:35
It's pretty clear actually, given eth0->ifb0->ifb1 it would look
like:

dev_queue_xmit(eth0)
  tcf_mirred -> ifb0
dev_queue_xmit(ifb0)
  tcf_mirred -> ifb1
dev_queue_xmit(ifb1)
  ifb_xmit(ifb1)
  <QUEUE> /* Here we queue the packet for the first time and
             release the stack of tx locks acquired on the
	     way. TC_FROM was never reset up here so this can't
	     possibly prevent any tx deadlocks. However, the
	     !from check is effective later on ... */
  ri_tasklet(ifb1)
dev_queue_xmit(ifb0)
  ifb_xmit(ifb0) /* classification disabled */
    /* Drop due to !from with input_dev = ifb1 which is
       good as it prevents to loop the packet back to
       ifb1 again which I refered to earlier */

Is this how it was intented?
yes, indeed. This is one class pathalogical case that was/will be caught
by testing; hence the speacial casing. IOW, this will have a "break"
because of the queue. 
There are other such as loop to self (nasty for say egress eth0->eth0),
which i proposed that Herberts qdisc_is_running patch may resolve (I
need to test).
I tried to stay out of A->B->A for now since that's currently
broken due to mirred unless the deadlock is intentional, f.e.
when setting up eth0->ifb0->eth0 like this:

ip link set ifb0 up
tc qdisc add dev ifb0 parent root handle 1: prio
tc qdisc add dev eth0 parent root handle 1: prio
tc filter add dev eth0 parent 1: protocol ip prio 10 u32
          match ip protocol 1 0xff flowid 1:1
	  action mirred egress redirect dev ifb0
tc filter add dev ifb0 parent 1: protocol ip prio 10 u32
          match ip protocol 1 0xff flowid 1:1
	  action mirred egress redirect dev eth0
I dont know if i tested for the above specific setup. I have to look at
my testcases when i get the opportunity.
The problem is:
There is no easy way to detect such a deadlock. I think it reduces to
the same case as eth0->eth0, i.e:

tc qdisc add dev eth0 root handle 1: prio
tc filter add dev eth0 parent 1: protocol ip prio 10 u32 \
match u32 0 0 flowid 1:1 \
action mirred egress redirect dev eth0


I have a dated  patch to mirred (may not apply cleanly) that i believe
will fix this specific one. Try to see if it also fixes this case you
have.
I do not want to submit this patch because i have a feeling there is a
lot more sophisticated way to do this. I would like to test Herberts
changes first. If you have time maybe you can try Daves 2.6.18 tree.

This is assuming that no tx deadlock happens of course. I
did try this out just to make sure, the machine just hung.
I can't see any code trying to prevent this but this is
another discussion as ifb is not involved in this, I think
it's purely a problem of mirred.
Its the second pathological case tx deadlock essentially and mirred
could help - it is not the mirred lock really if the attached patch
fixes it, but i could be wrong.

cheers,
jamal

PS:- My responses will have some latency due to accessibility
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help