| Message ID | 1436573411-5021-1-git-send-email-ast@plumgrid.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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. -- 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
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 -- 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
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. -- 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
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? -- 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
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. -- 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
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 -- 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
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? -- 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
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 -- 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
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 -- 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
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. -- 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
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. -- 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
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. -- 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
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. -- 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
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 -- 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
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? -- 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
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);
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(+)