netdev - Re: [PATCH ipsec-next v2 1/2] net: refactor common skb header copy code for re-use

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <567fc2d7-63bf-4953-a4c0-e4aedfe6e917@redhat.com>
Date: Wed, 14 Aug 2024 11:46:56 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Christian Hopps <chopps@...pps.org>, devel@...ux-ipsec.org
Cc: Steffen Klassert <steffen.klassert@...unet.com>, netdev@...r.kernel.org,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Florian Westphal <fw@...len.de>, Sabrina Dubroca <sd@...asysnail.net>,
 Simon Horman <horms@...nel.org>, Antony Antony <antony@...nome.org>,
 Christian Hopps <chopps@...n.net>
Subject: Re: [PATCH ipsec-next v2 1/2] net: refactor common skb header copy
 code for re-use

On 8/9/24 10:34, Christian Hopps wrote:
> From: Christian Hopps <chopps@...n.net>
> 
> Factor out some common skb header copying code so that it can be re-used
> outside of skbuff.
> 
> Signed-off-by: Christian Hopps <chopps@...n.net>
> ---
>   include/linux/skbuff.h | 1 +
>   net/core/skbuff.c      | 8 +++++++-
>   2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 29c3ea5b6e93..8626f9a343db 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1374,6 +1374,7 @@ struct sk_buff *skb_morph(struct sk_buff *dst, struct sk_buff *src);
>   void skb_headers_offset_update(struct sk_buff *skb, int off);
>   int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask);
>   struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t priority);
> +void ___copy_skb_header(struct sk_buff *new, const struct sk_buff *old);
>   void skb_copy_header(struct sk_buff *new, const struct sk_buff *old);
>   struct sk_buff *skb_copy(const struct sk_buff *skb, gfp_t priority);
>   struct sk_buff *__pskb_copy_fclone(struct sk_buff *skb, int headroom,
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 83f8cd8aa2d1..da5a47d2c9ab 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -1515,7 +1515,7 @@ EXPORT_SYMBOL(napi_consume_skb);
>   	BUILD_BUG_ON(offsetof(struct sk_buff, field) !=		\
>   		     offsetof(struct sk_buff, headers.field));	\
>   
> -static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
> +void ___copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
>   {
>   	new->tstamp		= old->tstamp;
>   	/* We do not copy old->sk */
> @@ -1524,6 +1524,12 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
>   	skb_dst_copy(new, old);
>   	__skb_ext_copy(new, old);
>   	__nf_copy(new, old, false);
> +}
> +EXPORT_SYMBOL_GPL(___copy_skb_header);
> +
> +static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
> +{
> +	___copy_skb_header(new, old); >
>   	/* Note : this field could be in the headers group.
>   	 * It is not yet because we do not want to have a 16 bit hole

Could you please point where/how are you going to use this helper? 
factoring out this very core bits of skbuff copy looks quite bug prone - 
and exporting the helper could introduce additional unneeded function 
calls in the core code.

Thanks,

Paolo