netdev - RE: [PATCH bpf-next v2 15/20] net, xdp: allow metadata

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]

Date: Mon, 03 Jul 2023 14:06:46 -0700
From: John Fastabend <john.fastabend@...il.com>
To: Larysa Zaremba <larysa.zaremba@...el.com>, 
 bpf@...r.kernel.org
Cc: Larysa Zaremba <larysa.zaremba@...el.com>, 
 ast@...nel.org, 
 daniel@...earbox.net, 
 andrii@...nel.org, 
 martin.lau@...ux.dev, 
 song@...nel.org, 
 yhs@...com, 
 john.fastabend@...il.com, 
 kpsingh@...nel.org, 
 sdf@...gle.com, 
 haoluo@...gle.com, 
 jolsa@...nel.org, 
 David Ahern <dsahern@...il.com>, 
 Jakub Kicinski <kuba@...nel.org>, 
 Willem de Bruijn <willemb@...gle.com>, 
 Jesper Dangaard Brouer <brouer@...hat.com>, 
 Anatoly Burakov <anatoly.burakov@...el.com>, 
 Alexander Lobakin <alexandr.lobakin@...el.com>, 
 Magnus Karlsson <magnus.karlsson@...il.com>, 
 Maryam Tahhan <mtahhan@...hat.com>, 
 xdp-hints@...-project.net, 
 netdev@...r.kernel.org, 
 Aleksander Lobakin <aleksander.lobakin@...el.com>
Subject: RE: [PATCH bpf-next v2 15/20] net, xdp: allow metadata > 32

Larysa Zaremba wrote:
> From: Aleksander Lobakin <aleksander.lobakin@...el.com>
> 
> When using XDP hints, metadata sometimes has to be much bigger
> than 32 bytes. Relax the restriction, allow metadata larger than 32 bytes
> and make __skb_metadata_differs() work with bigger lengths.
> 
> Now size of metadata is only limited by the fact it is stored as u8
> in skb_shared_info, so maximum possible value is 255. Other important
> conditions, such as having enough space for xdp_frame building, are already
> checked in bpf_xdp_adjust_meta().
> 
> The requirement of having its length aligned to 4 bytes is still
> valid.
> 
> Signed-off-by: Aleksander Lobakin <aleksander.lobakin@...el.com>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@...el.com>
> ---
>  include/linux/skbuff.h | 13 ++++++++-----
>  include/net/xdp.h      |  7 ++++++-
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 91ed66952580..cd49cdd71019 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -4209,10 +4209,13 @@ static inline bool __skb_metadata_differs(const struct sk_buff *skb_a,
>  {
>  	const void *a = skb_metadata_end(skb_a);
>  	const void *b = skb_metadata_end(skb_b);
> -	/* Using more efficient varaiant than plain call to memcmp(). */
> -#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64

Why are we removing the ifdef here? Its adding a runtime 'if' when its not
necessary. I would keep the ifdef and simply add the default case
in the switch.