Re: [PATCH] act_mirred: don't clone skb when skb isn't shared

On Sat, Jun 5, 2010 at 8:53 PM, jamal [off-list ref] wrote:

On Fri, 2010-06-04 at 21:43 +0800, Changli Gao wrote:

quoted

don't clone skb when skb isn't shared

When the tcf_action is TC_ACT_STOLEN, and the skb isn't shared, we don't need
to clone a new skb. As the skb will be freed after this function returns, we
can use it freely once we get a reference to it.

It looks like a good optimization - but i am not a big fan of one-offs
[because usability goes down and I am forced to explain it longer in the
rules (refer to: Documentation/networking/tc-actions-env-rules.txt)]

Thanks. BTW: act_nat.c doesn't obey the following rule, and you plan
to remove TC_MUNGED and TC_OK2MUNGE?

2) If you munge any packet thou shalt call pskb_expand_head in the case
someone else is referencing the skb. After that you "own" the skb.
You must also tell us if it is ok to munge the packet (TC_OK2MUNGE),
this way any action downstream can stomp on the packet.

How about you update skb_act_clone to take take the action code as well
and do the check the if stolen/queued it does a skb_get otherwise it
calls skb_clone?

Good idea. Thanks.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)