Use skb_act_clone
Subsystems (3)
PATCH 2/3 Use skb_act_clone
cheers,
jamal
[NET_CLS_ACT] Use skb_act_clone
clean skb_clone of any signs of CONFIG_NET_CLS_ACT and have mirred us skb_act_clone()
--- net/core/skbuff.c | 7 ------- net/sched/act_mirred.c | 2 +- 2 files changed, 1 insertions(+), 8 deletions(-)
net/core/skbuff.c +0 -7
@@ -415,13 +415,6 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb) n->hdr_len = skb->nohdr ?skb_headroom (skb) : skb->hdr_len; n->nohdr =0 ; n->destructor =NULL ; -#ifdef CONFIG_NET_CLS_ACT -/* FIXME What is this and why don't we do it in copy_skb_header? */ - n->tc_verd =SET_TC_VERD (n->tc_verd,0 ); - n->tc_verd =CLR_TC_OK2MUNGE (n->tc_verd); - n->tc_verd =CLR_TC_MUNGED (n->tc_verd); -C (iif); -#endif C (truesize);atomic_set (&n->users,1 );C (head);
net/sched/act_mirred.c +1 -1
@@ -166,7 +166,7 @@ bad_mirred:return TC_ACT_SHOT ; } - skb2 =skb_clone (skb,GFP_ATOMIC ); + skb2 =skb_act_clone (skb,GFP_ATOMIC );if (skb2 ==NULL ) goto bad_mirred; if (m->tcfm_eaction != TCA_EGRESS_MIRROR &&
Signature · 1 line
1.4.4.1.gaed4
jamal wrote:
[Can't quote for some reason ..]
I don't think this change is safe since we might loose the redirect counter and have endless redirect loops. ipip for example calls skb_realloc_headroom, which clones the skb when delta > 0, so it seems we really need to copy tc_verd in skb_clone(). Generally the assumption that drivers don't clone packets doesn't seem safe.
Quoted 3 lines · Patrick McHardy
jamal wrote:
[Can't quote for some reason ..]
Sorry, trying to be clever with git-format-patch but chickened out sending using git-am - so patch was just attached (Dave seems to be friendly to such an approach when he applies)
Quoted 6 lines
I don't think this change is safe since we might loose the redirect counter and have endless redirect loops. ipip for example calls
skb_realloc_headroom, which clones the skb when delta > 0, so it seems we really need to copytc_verdinskb_clone(). Generally the assumption that drivers don't clone packets doesn't seem safe.
Does the copying of tc_verd in __copy_skb_header() not cover that?
cheers,
jamal
Quoted 2 lines · jamal
Does the copying of
tc_verdin__copy_skb_header()not cover that?
Yep, I'm pretty sure it does.
In fact this patch completely resolves my concerns about tc_verd.
Thanks!
Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txtQuoted 6 lines · Herbert Xu
Quoted 1 line · jamal
Does the copying of
tc_verdin__copy_skb_header()not cover that?Yep, I'm pretty sure it does.
In fact this patch completely resolves my concerns about
tc_verd.
Indeed, I was looking at the wrong tree :)
Quoted 10 lines · Patrick McHardy
Quoted 6 lines · Herbert Xu
Quoted 1 line · jamal
Does the copying of
tc_verdin__copy_skb_header()not cover that?Yep, I'm pretty sure it does.
In fact this patch completely resolves my concerns about
tc_verd.Indeed, I was looking at the wrong tree :)
Ok, thanks guys.
cheers,
jamal