Jamal Hadi Salim wrote:
On Sun, 2006-08-01 at 18:46 +0100, Patrick McHardy wrote:
[..]
Anyway, why do you want to keep this obvious broken behaviour?
I dont wanna keep it if it is broken Patrick;-> It seems to be a big
change at glance and normally i would like to run some serious tests
on something like this (this one you can actually test with a series of
mirred, pedit and gact while tcpdump is running - doesnt matter if
ingress or egress and then watch the hex dump of modified packets)
Its not a big change, just look at the patch, all it does is change
the argument to a single skb pointer instead of a double skb pointer
to a local variable. Every action did "struct sk_buff *skb = *pskb"
right at the beginning of their action function anyway.
Ok, so lets go over the steps you described to see if i will make sense
of it.
Someone calls:
-> tcf_action_exec(skb ...)
---> ret = a->ops->act(&skb, a, res)
------> one or more actions
You are saying the user (I assume either caller of tcf_action_exec() or
an action) will crash if the skb was replaced. So lets start with
actions:
I'm talking about users of tcf_action_exec.
The "enviromental" rules are:
1) if you steal any packet thou shalt clone it (as in queueing it etc).
2) If you munge any packet thou shalt call skb_expand in the case
someone else is referencing the skb. Then you "own" it. You must also
tell us if it is ok to munge the packet (TC_OK2MUNGE)
3) dropping packets you dont own is a nono. You simply return
TC_ACT_SHOT
The "enviromental" rules for callers of actions (qdiscs etc) are:
thou art responsible for freeing anything returned as being
TC_ACT_SHOT/STOLEN/QUEUED otherwise all is great.
This is all fine and doesn't need double skb pointers. Double pointers
are needed if an action wants to replace the skb in the callers context.
It can replace it in tcf_action_exec's context, but the caller of
tcf_action_exec will still see and use the old skb because
tcf_action_exec only gets a copy of its pointer. So using double
skb pointers is confusing at least, because it looks like actions could
replace skbs in the callers context, which they can't.
I hope that clears it up.
-
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
