netdev - Re: [PATCH bpf-next] samples/bpf: xdp1 and xdp2 reduce XDPBUFSIZE to 60

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: <948629b6-8607-9797-8897-7d3e0535fa7b@gmail.com>
Date: Wed, 31 May 2023 10:23:41 +0300
From: Tariq Toukan <ttoukan.linux@...il.com>
To: Jesper Dangaard Brouer <brouer@...hat.com>,
 Tariq Toukan <ttoukan.linux@...il.com>,
 Daniel Borkmann <borkmann@...earbox.net>, Alexei Starovoitov
 <ast@...nel.org>, Andrii Nakryiko <andrii.nakryiko@...il.com>,
 bpf@...r.kernel.org
Cc: gal@...dia.com, lorenzo@...nel.org, netdev@...r.kernel.org,
 andrew.gospodarek@...adcom.com, Tariq Toukan <tariqt@...dia.com>
Subject: Re: [PATCH bpf-next] samples/bpf: xdp1 and xdp2 reduce XDPBUFSIZE to
 60



On 30/05/2023 17:30, Jesper Dangaard Brouer wrote:
> Default samples/pktgen scripts send 60 byte packets as hardware
> adds 4-bytes FCS checksum, which fulfils minimum Ethernet 64 bytes
> frame size.
> 
> XDP layer will not necessary have access to the 4-bytes FCS checksum.
> 
> This leads to bpf_xdp_load_bytes() failing as it tries to copy
> 64-bytes from an XDP packet that only have 60-bytes available.
> 
> Fixes: 772251742262 ("samples/bpf: fixup some tools to be able to support xdp multibuffer")
> Signed-off-by: Jesper Dangaard Brouer <brouer@...hat.com>
> ---
>   samples/bpf/xdp1_kern.c |    2 +-
>   samples/bpf/xdp2_kern.c |    2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/samples/bpf/xdp1_kern.c b/samples/bpf/xdp1_kern.c
> index 0a5c704badd0..d91f27cbcfa9 100644
> --- a/samples/bpf/xdp1_kern.c
> +++ b/samples/bpf/xdp1_kern.c
> @@ -39,7 +39,7 @@ static int parse_ipv6(void *data, u64 nh_off, void *data_end)
>   	return ip6h->nexthdr;
>   }
>   
> -#define XDPBUFSIZE	64
> +#define XDPBUFSIZE	60

Perf with the presence of load/store copies is far from being optimal..
Still, do we care if memcpy of 60 bytes performs worse than 64 (full 
cacheline)?
Maybe not really in this case, looking forward for the replacement of 
memcpy with the proper dyncptr API.

Other than that:
Reviewed-by: Tariq Toukan <tariqt@...dia.com>


>   SEC("xdp.frags")
>   int xdp_prog1(struct xdp_md *ctx)
>   {
> diff --git a/samples/bpf/xdp2_kern.c b/samples/bpf/xdp2_kern.c
> index 67804ecf7ce3..8bca674451ed 100644
> --- a/samples/bpf/xdp2_kern.c
> +++ b/samples/bpf/xdp2_kern.c
> @@ -55,7 +55,7 @@ static int parse_ipv6(void *data, u64 nh_off, void *data_end)
>   	return ip6h->nexthdr;
>   }
>   
> -#define XDPBUFSIZE	64
> +#define XDPBUFSIZE	60
>   SEC("xdp.frags")
>   int xdp_prog1(struct xdp_md *ctx)
>   {
> 
>