[PATCH net-next] tc: fix tc actions in case of shared skb

[PATCH net-next] tc: fix tc actions in case of shared skb

[Alexei Starovoitov]

TC actions need to check for very unlikely event skb->users != 1,
otherwise subsequent pskb_may_pull/pskb_expand_head will crash.
When skb_shared() just drop the packet, since in the middle of actions
it's too late to call skb_share_check(), since classifiers/actions assume
the same skb pointer.

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
 net/sched/act_csum.c |    3 +++
 net/sched/act_nat.c  |    3 +++
 net/sched/act_vlan.c |    3 +++
 3 files changed, 9 insertions(+)

diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index b07c535ba8e7..ac150bdc24f4 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -510,6 +510,9 @@ static int tcf_csum(struct sk_buff *skb,
 	if (unlikely(action == TC_ACT_SHOT))
 		goto drop;
 
+	if (unlikely(skb_shared(skb)))
+		goto drop;
+
 	switch (tc_skb_protocol(skb)) {
 	case cpu_to_be16(ETH_P_IP):
 		if (!tcf_csum_ipv4(skb, update_flags))
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 5be0b3c1c5b0..8bb2657de635 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -114,6 +114,9 @@ static int tcf_nat(struct sk_buff *skb, const struct tc_action *a,
 	if (unlikely(action == TC_ACT_SHOT))
 		goto drop;
 
+	if (unlikely(skb_shared(skb)))
+		goto drop;
+
 	noff = skb_network_offset(skb);
 	if (!pskb_may_pull(skb, sizeof(*iph) + noff))
 		goto drop;
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 796785e0bf96..6365ae036c6e 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -33,6 +33,9 @@ static int tcf_vlan(struct sk_buff *skb, const struct tc_action *a,
 	bstats_update(&v->tcf_bstats, skb);
 	action = v->tcf_action;
 
+	if (unlikely(skb_shared(skb)))
+		goto drop;
+
 	switch (v->tcfv_action) {
 	case TCA_VLAN_ACT_POP:
 		err = skb_vlan_pop(skb);
-- 
1.7.9.5

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[David Miller]

From: Alexei Starovoitov <ast@plumgrid.com>
Date: Fri, 10 Jul 2015 17:10:11 -0700

> TC actions need to check for very unlikely event skb->users != 1,
> otherwise subsequent pskb_may_pull/pskb_expand_head will crash.
> When skb_shared() just drop the packet, since in the middle of actions
> it's too late to call skb_share_check(), since classifiers/actions assume
> the same skb pointer.
> 
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>

I think whatever creates this skb->users != 1 situation should be fixed,
they should clone the packet.

In fact, it would really help enormously if you could explain in detail
how this situation can actually arise.  Especially since I do not consider
it acceptable to drop the packet in this situation.

Thanks.

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[Alexei Starovoitov]

On 7/11/15 9:29 PM, David Miller wrote:
> From: Alexei Starovoitov <ast@plumgrid.com>
> Date: Fri, 10 Jul 2015 17:10:11 -0700
>
>> TC actions need to check for very unlikely event skb->users != 1,
>> otherwise subsequent pskb_may_pull/pskb_expand_head will crash.
>> When skb_shared() just drop the packet, since in the middle of actions
>> it's too late to call skb_share_check(), since classifiers/actions assume
>> the same skb pointer.
>>
>> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
>
> I think whatever creates this skb->users != 1 situation should be fixed,
> they should clone the packet.

In all normal cases skb->users == 1, but pktgen is using trick:
atomic_add(burst, &skb->users);
so when testing something like:
tc filter add dev $dev root pref 10 u32 match u32 0 0 flowid 1:2 \
   action vlan push id 2 action drop

it will crash:
[   31.999519] kernel BUG at ../net/core/skbuff.c:1130!
[   31.999519] invalid opcode: 0000 [#1] PREEMPT SMP
[   31.999519] Modules linked in: act_gact act_vlan cls_u32 sch_ingress 
veth pktgen
[   31.999519] CPU: 0 PID: 339 Comm: kpktgend_0 Not tainted 4.1.0+ #730
[   31.999519] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), [ 
   31.999519] Call Trace:
[   31.999519]  [<ffffffff8160eea7>] skb_vlan_push+0x1d7/0x200
[   31.999519]  [<ffffffffa0017108>] tcf_vlan+0x108/0x110 [act_vlan]
[   31.999519]  [<ffffffff81650d26>] tcf_action_exec+0x46/0x80
[   31.999519]  [<ffffffffa001f4fe>] u32_classify+0x30e/0x740 [cls_u32]
[   31.999519]  [<ffffffff810bcc6f>] ? __lock_acquire+0xbcf/0x1e80
[   31.999519]  [<ffffffff810bcc6f>] ? __lock_acquire+0xbcf/0x1e80
[   31.999519]  [<ffffffff8161f392>] ? __netif_receive_skb_core+0x1b2/0xce0
[   31.999519]  [<ffffffff8164c0c3>] tc_classify_compat+0xa3/0xb0
[   31.999519]  [<ffffffff8164ca03>] tc_classify+0x33/0x90
[   31.999519]  [<ffffffff8161f674>] __netif_receive_skb_core+0x494/0xce0
[   31.999519]  [<ffffffff8161f274>] ? __netif_receive_skb_core+0x94/0xce0
[   31.999519]  [<ffffffff810bf10d>] ? trace_hardirqs_on_caller+0xad/0x1d0
[   31.999519]  [<ffffffff8161fee1>] __netif_receive_skb+0x21/0x70
[   31.999519]  [<ffffffff81620b43>] netif_receive_skb_internal+0x23/0x1c0
[   31.999519]  [<ffffffff816219a9>] netif_receive_skb_sk+0x49/0x1e0
[   31.999519]  [<ffffffffa0006e8d>] pktgen_thread_worker+0x111d/0x1fa0 
[pktgen]

> In fact, it would really help enormously if you could explain in detail
> how this situation can actually arise.  Especially since I do not consider
> it acceptable to drop the packet in this situation.

It's not pretty to drop, but it's better than crash.
I don't think we can get rid of 'skb->users += burst' trick, since
that's where all performance comes from (for both TX and RX testing).

So the only cheap way I see to avoid crash is to do this
if (unlikely(skb_shared(skb)))
check in actions that call pskb_expand_head.

In all normal scenarios it won't be triggered and pktgen tests
won't be crashing.
Yes. pktgen numbers will be a bit meaningless, since act_vlan will be
dropping instead of adding vlan, so users cannot make any performance
conclusions, but still better than crash.

> the rules specified here:
> Documentation/networking/tc-actions-env-rules.txt
> insufficient?

Jamal,
that doc definitely needs updating. :)
It says:
"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."
that's incorrect. If somebody 'referencing' skb via skb->users > 1
it's too late to call pskb_expand_head. As you can see in the
crash trace above.

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[David Miller]

From: Alexei Starovoitov <ast@plumgrid.com>
Date: Mon, 13 Jul 2015 12:47:42 -0700

> In all normal cases skb->users == 1, but pktgen is using trick:
> atomic_add(burst, &skb->users);
> so when testing something like:

You can want pktgen rx (which is the only buggy case as far as I can
see, TX is fine) to run fast, but you must do so by abiding by the
appropriate SKB sharing rules.

You can't do an optimization in pktgen for RX processing that works
"some of the time".  We have shared SKB rules for a reason.

And I don't want to have to explain to someone in the future why that
drop check is there, and have to tell them "because pktgen is broken
and we decided to add a hack here rather than make pktgen send
properly formed SKBs into the RX path"

Ok?

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[Alexei Starovoitov]

On 7/13/15 1:04 PM, David Miller wrote:
> From: Alexei Starovoitov <ast@plumgrid.com>
> Date: Mon, 13 Jul 2015 12:47:42 -0700
>
>> In all normal cases skb->users == 1, but pktgen is using trick:
>> atomic_add(burst, &skb->users);
>> so when testing something like:
>
> You can want pktgen rx (which is the only buggy case as far as I can
> see, TX is fine) to run fast, but you must do so by abiding by the
> appropriate SKB sharing rules.
>
> You can't do an optimization in pktgen for RX processing that works
> "some of the time".  We have shared SKB rules for a reason.
>
> And I don't want to have to explain to someone in the future why that
> drop check is there, and have to tell them "because pktgen is broken
> and we decided to add a hack here rather than make pktgen send
> properly formed SKBs into the RX path"
>
> Ok?

in general all makes sense, but it is both RX and TX.
Without burst hack we cannot achieve line rate TX.
         atomic_add(burst, &pkt_dev->skb->users);
xmit_more:
         ret = netdev_start_xmit(pkt_dev->skb, odev, txq, --burst > 0);

in pktgen we check that driver can work with users > 1 via:
pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING

so real hw driver are mostly ready for users > 1, it's only
few tc actions struggle a bit.
We cannot check tc actions from pktgen, since they can be added
dynamically.
So I see three options:
1 get rid of burst hack for both RX and TX in pktgen (kills performance)
2 add unlikely(skb_shread) check to few tc actions
3 do nothing

I think 2 isn't that bad after all if properly documented with
"because pktgen is doing this hack for performance" ?

I'm fine with 3 too, since the whole pktgen business is for root
and for kernel hackers who suppose to know what they're doing.

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[Daniel Borkmann]

On 07/13/2015 10:17 PM, Alexei Starovoitov wrote:
...
> We cannot check tc actions from pktgen, since they can be added
> dynamically.
> So I see three options:
> 1 get rid of burst hack for both RX and TX in pktgen (kills performance)
> 2 add unlikely(skb_shread) check to few tc actions
> 3 do nothing
>
> I think 2 isn't that bad after all if properly documented with
> "because pktgen is doing this hack for performance" ?
>
> I'm fine with 3 too, since the whole pktgen business is for root
> and for kernel hackers who suppose to know what they're doing.

Hmm, one thing for option 3 could be that we add a modinfo tag
"experimental", so that on loading of pktgen module, we trigger
(like in case of staging) ...

   add_taint_module(mod, TAINT_CRAP, LOCKDEP_STILL_OK);

... and add a pr_warn() to the user, it may be more visible/clear
than the "Packet Generator (USE WITH CAUTION)" Kconfig title? ;)

It'd be a pity that we'd need the extra atomic read only for the
pktgen case. :/ With regards to option 2, you could hide that behind
a static inline helper wrapped in IS_ENABLED(CONFIG_NET_PKTGEN), but
that is a veeeery ugly workaround/hack as well (and distros might
even ship it nevertheless). I wouldn't be surprised if there are
other usage combinations with pktgen that would crash your box. :/

Best,
Daniel

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[Alexei Starovoitov]

On 7/13/15 1:55 PM, Daniel Borkmann wrote:
> On 07/13/2015 10:17 PM, Alexei Starovoitov wrote:
> ...
>> We cannot check tc actions from pktgen, since they can be added
>> dynamically.
>> So I see three options:
>> 1 get rid of burst hack for both RX and TX in pktgen (kills performance)
>> 2 add unlikely(skb_shread) check to few tc actions
>> 3 do nothing
...
> pktgen case. :/ With regards to option 2, you could hide that behind
> a static inline helper wrapped in IS_ENABLED(CONFIG_NET_PKTGEN), but
> that is a veeeery ugly workaround/hack as well (and distros might
> even ship it nevertheless).

naming such helper is a headache as well.
static inline bool is_pktgen_shared_skb(struct sk_buff *skb)
{
#if IS_ENABLED(CONFIG_NET_PKTGEN)
	/* pktgen uses skb->users += burst trick to reuse skb */
	return skb_shared(skb);
#else
	return false;
#endif
}
and in actions:
if (unlikely(is_pktgen_shared_skb(skb))) goto drop;

thoughts?

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[Daniel Borkmann]

On 07/14/2015 12:26 AM, Alexei Starovoitov wrote:
> On 7/13/15 1:55 PM, Daniel Borkmann wrote:
>> On 07/13/2015 10:17 PM, Alexei Starovoitov wrote:
>> ...
>>> We cannot check tc actions from pktgen, since they can be added
>>> dynamically.
>>> So I see three options:
>>> 1 get rid of burst hack for both RX and TX in pktgen (kills performance)
>>> 2 add unlikely(skb_shread) check to few tc actions
>>> 3 do nothing
> ...
>> pktgen case. :/ With regards to option 2, you could hide that behind
>> a static inline helper wrapped in IS_ENABLED(CONFIG_NET_PKTGEN), but
>> that is a veeeery ugly workaround/hack as well (and distros might
>> even ship it nevertheless).
>
> naming such helper is a headache as well.
> static inline bool is_pktgen_shared_skb(struct sk_buff *skb)
> {
> #if IS_ENABLED(CONFIG_NET_PKTGEN)
>      /* pktgen uses skb->users += burst trick to reuse skb */
>      return skb_shared(skb);
> #else
>      return false;
> #endif
> }
> and in actions:
> if (unlikely(is_pktgen_shared_skb(skb))) goto drop;
>
> thoughts?

As I mentioned above, so Fedora, for example, ships pktgen by default.
That means, we'd run into the above test for shared skb in every case,
meaning it won't help much and it's also a pretty nasty hack. ;)

One other thing that comes to mind, not sure if it's worth it though,
would be to split the skb->tc_verd's TC_NCLS itself into TC_NCLS/TC_NACT,
so that you can go into the classifier, but skip the action part.

Since in tcf_action_exec(), we already test for that, you might be able
to add this with no extra cost. pktgen would then need to tag its skb
with TC_NACT, so that you'll always return with TC_ACT_OK. And if you
really would want to test tc actions, then w/o pktgen bursting ...

Best,
Daniel

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[Jamal Hadi Salim]

On 07/14/15 06:29, Daniel Borkmann wrote:
> On 07/14/2015 12:26 AM, Alexei Starovoitov wrote:
>> On 7/13/15 1:55 PM, Daniel Borkmann wrote:
>>> On 07/13/2015 10:17 PM, Alexei Starovoitov wrote:
>>> ...
>>>> We cannot check tc actions from pktgen, since they can be added
>>>> dynamically.
>>>> So I see three options:
>>>> 1 get rid of burst hack for both RX and TX in pktgen (kills
>>>> performance)
>>>> 2 add unlikely(skb_shread) check to few tc actions
>>>> 3 do nothing
>> ...
>>> pktgen case. :/ With regards to option 2, you could hide that behind
>>> a static inline helper wrapped in IS_ENABLED(CONFIG_NET_PKTGEN), but
>>> that is a veeeery ugly workaround/hack as well (and distros might
>>> even ship it nevertheless).
>>
>> naming such helper is a headache as well.
>> static inline bool is_pktgen_shared_skb(struct sk_buff *skb)
>> {
>> #if IS_ENABLED(CONFIG_NET_PKTGEN)
>>      /* pktgen uses skb->users += burst trick to reuse skb */
>>      return skb_shared(skb);
>> #else
>>      return false;
>> #endif
>> }
>> and in actions:
>> if (unlikely(is_pktgen_shared_skb(skb))) goto drop;
>>
>> thoughts?
>
> As I mentioned above, so Fedora, for example, ships pktgen by default.
> That means, we'd run into the above test for shared skb in every case,
> meaning it won't help much and it's also a pretty nasty hack. ;)
>
> One other thing that comes to mind, not sure if it's worth it though,
> would be to split the skb->tc_verd's TC_NCLS itself into TC_NCLS/TC_NACT,
> so that you can go into the classifier, but skip the action part.
>
> Since in tcf_action_exec(), we already test for that, you might be able
> to add this with no extra cost. pktgen would then need to tag its skb
> with TC_NACT, so that you'll always return with TC_ACT_OK. And if you
> really would want to test tc actions, then w/o pktgen bursting ...
>

Would just a simple skb->mark not work? Drop if skb->mark = x
using skbedit. Or a brand new pktgen_burst_mode action that drops?

cheers,
jamal

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[Daniel Borkmann]

On 07/14/2015 01:57 PM, Jamal Hadi Salim wrote:
> On 07/14/15 06:29, Daniel Borkmann wrote:
>> On 07/14/2015 12:26 AM, Alexei Starovoitov wrote:
>>> On 7/13/15 1:55 PM, Daniel Borkmann wrote:
>>>> On 07/13/2015 10:17 PM, Alexei Starovoitov wrote:
>>>> ...
>>>>> We cannot check tc actions from pktgen, since they can be added
>>>>> dynamically.
>>>>> So I see three options:
>>>>> 1 get rid of burst hack for both RX and TX in pktgen (kills
>>>>> performance)
>>>>> 2 add unlikely(skb_shread) check to few tc actions
>>>>> 3 do nothing
>>> ...
>>>> pktgen case. :/ With regards to option 2, you could hide that behind
>>>> a static inline helper wrapped in IS_ENABLED(CONFIG_NET_PKTGEN), but
>>>> that is a veeeery ugly workaround/hack as well (and distros might
>>>> even ship it nevertheless).
>>>
>>> naming such helper is a headache as well.
>>> static inline bool is_pktgen_shared_skb(struct sk_buff *skb)
>>> {
>>> #if IS_ENABLED(CONFIG_NET_PKTGEN)
>>>      /* pktgen uses skb->users += burst trick to reuse skb */
>>>      return skb_shared(skb);
>>> #else
>>>      return false;
>>> #endif
>>> }
>>> and in actions:
>>> if (unlikely(is_pktgen_shared_skb(skb))) goto drop;
>>>
>>> thoughts?
>>
>> As I mentioned above, so Fedora, for example, ships pktgen by default.
>> That means, we'd run into the above test for shared skb in every case,
>> meaning it won't help much and it's also a pretty nasty hack. ;)
>>
>> One other thing that comes to mind, not sure if it's worth it though,
>> would be to split the skb->tc_verd's TC_NCLS itself into TC_NCLS/TC_NACT,
>> so that you can go into the classifier, but skip the action part.
>>
>> Since in tcf_action_exec(), we already test for that, you might be able
>> to add this with no extra cost. pktgen would then need to tag its skb
>> with TC_NACT, so that you'll always return with TC_ACT_OK. And if you
>> really would want to test tc actions, then w/o pktgen bursting ...
>
> Would just a simple skb->mark not work? Drop if skb->mark = x
> using skbedit. Or a brand new pktgen_burst_mode action that drops?

I think it's more about the fact that something could BUG() when used with
pktgen, otherwise you could just generally drop after classification.

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[Alexei Starovoitov]

On 7/14/15 3:29 AM, Daniel Borkmann wrote:
> One other thing that comes to mind, not sure if it's worth it though,
> would be to split the skb->tc_verd's TC_NCLS itself into TC_NCLS/TC_NACT,
> so that you can go into the classifier, but skip the action part.
>
> Since in tcf_action_exec(), we already test for that, you might be able
> to add this with no extra cost. pktgen would then need to tag its skb
> with TC_NACT, so that you'll always return with TC_ACT_OK. And if you
> really would want to test tc actions, then w/o pktgen bursting ...

imo it's even uglier. Majority of the actions are fine with shared skb,
so blank disable is no good at all.
The cost of 'unlikely(is_pktgen_shared_skb' is tiny, but fine,
we dug up the dirt enough.
I'm taking option 3 (do nothing) at this point.

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[David Miller]

From: Alexei Starovoitov <ast@plumgrid.com>
Date: Mon, 13 Jul 2015 15:26:46 -0700

> On 7/13/15 1:55 PM, Daniel Borkmann wrote:
>> On 07/13/2015 10:17 PM, Alexei Starovoitov wrote:
>> ...
>>> We cannot check tc actions from pktgen, since they can be added
>>> dynamically.
>>> So I see three options:
>>> 1 get rid of burst hack for both RX and TX in pktgen (kills
>>> performance)

#1 is a serious consideration if you don't come up with better ideas,
since an optimization is for nothing if it knowingly breaks things.

>>> 2 add unlikely(skb_shread) check to few tc actions
>>> 3 do nothing
> ...
>> pktgen case. :/ With regards to option 2, you could hide that behind
>> a static inline helper wrapped in IS_ENABLED(CONFIG_NET_PKTGEN), but
>> that is a veeeery ugly workaround/hack as well (and distros might
>> even ship it nevertheless).
> 
> naming such helper is a headache as well.
> static inline bool is_pktgen_shared_skb(struct sk_buff *skb)
> {
> #if IS_ENABLED(CONFIG_NET_PKTGEN)

As mentioned by others, this ifdef accomplishes nothing as indeed
every distribution turns pktgen on so %99.9999 of all users will not
benefit from this optimized check.

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[Alexei Starovoitov]

On 7/14/15 3:34 PM, David Miller wrote:
>>>> 1 get rid of burst hack for both RX and TX in pktgen (kills
>>>> >>>performance)
> #1 is a serious consideration if you don't come up with better ideas,
> since an optimization is for nothing if it knowingly breaks things.

I've dug up the pktgen source from 2002 and see:
   atomic_inc(&skb->users);
   odev->hard_start_xmit(skb, odev);
so it did this trick forever.
Looks like it's a fundamental way how pktgen was working
and working still. Even when new 'burst' feature is not used,
pktgen still increments skb->users to hold skb.
At present I don't have good ideas how to redesign pktgen
and since apparently no one noticed this tc_action+pktgen
breakage for years, it's probably ok to leave everything as-is until
better ideas come. I'm not giving up yet. Just ran out of ideas.

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[John Fastabend]

On 15-07-14 04:08 PM, Alexei Starovoitov wrote:
> On 7/14/15 3:34 PM, David Miller wrote:
>>>>> 1 get rid of burst hack for both RX and TX in pktgen (kills
>>>>> >>>performance)
>> #1 is a serious consideration if you don't come up with better ideas,
>> since an optimization is for nothing if it knowingly breaks things.
> 
> I've dug up the pktgen source from 2002 and see:
>   atomic_inc(&skb->users);
>   odev->hard_start_xmit(skb, odev);
> so it did this trick forever.
> Looks like it's a fundamental way how pktgen was working
> and working still. Even when new 'burst' feature is not used,
> pktgen still increments skb->users to hold skb.
> At present I don't have good ideas how to redesign pktgen
> and since apparently no one noticed this tc_action+pktgen
> breakage for years, it's probably ok to leave everything as-is until
> better ideas come. I'm not giving up yet. Just ran out of ideas.
> 

Right and we hit this issue when pktgen is run over any stacked device
with clone_skb set. I've always put it in the don't do this category but
a fix would be nice.

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

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[Alexei Starovoitov]

On 7/14/15 5:58 PM, John Fastabend wrote:
> Right and we hit this issue when pktgen is run over any stacked device
> with clone_skb set. I've always put it in the don't do this category but
> a fix would be nice.

hmm, are you sure that commit
52d6c8c6ca12 ("net: pktgen: disable xmit_clone on virtual devices")
didn't fix it already?

Re: [PATCH net-next] tc: fix tc actions in case of shared skb

[Jamal Hadi Salim]

On 07/10/15 20:10, Alexei Starovoitov wrote:
> TC actions need to check for very unlikely event skb->users != 1,
> otherwise subsequent pskb_may_pull/pskb_expand_head will crash.
> When skb_shared() just drop the packet, since in the middle of actions
> it's too late to call skb_share_check(), since classifiers/actions assume
> the same skb pointer.
>

Alexei,
To add to what Dave said - are the rules specified here:
Documentation/networking/tc-actions-env-rules.txt
insufficient?

cheers,
jamal