netdev - Re: [PATCH bpf-next 1/2] samples/bpf: fixup xdp_redirect tool to be able to support xdp multibuffer

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: <63d91da7-4040-a766-dcd7-bccbb4c02ef4@redhat.com>
Date: Tue, 30 May 2023 13:33:08 +0200
From: Jesper Dangaard Brouer <jbrouer@...hat.com>
To: Tariq Toukan <tariqt@...dia.com>, Alexei Starovoitov <ast@...nel.org>,
 John Fastabend <john.fastabend@...il.com>, Jakub Kicinski <kuba@...nel.org>
Cc: brouer@...hat.com, Daniel Borkmann <daniel@...earbox.net>,
 Jesper Dangaard Brouer <hawk@...nel.org>, bpf@...r.kernel.org,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 netdev@...r.kernel.org, Gal Pressman <gal@...dia.com>,
 Nimrod Oren <noren@...dia.com>
Subject: Re: [PATCH bpf-next 1/2] samples/bpf: fixup xdp_redirect tool to be
 able to support xdp multibuffer

On 29/05/2023 13.06, Tariq Toukan wrote:
> Expand the xdp multi-buffer support to xdp_redirect tool.
> Similar to what's done in commit
> 772251742262 ("samples/bpf: fixup some tools to be able to support xdp multibuffer")
> and its fix commit
> 7a698edf954c ("samples/bpf: Fix MAC address swapping in xdp2_kern").
> 

Have you tested if this cause a performance degradation?

(Also found possible bug below)

> Signed-off-by: Tariq Toukan <tariqt@...dia.com>
> Reviewed-by: Nimrod Oren <noren@...dia.com>
> ---
>   samples/bpf/xdp_redirect.bpf.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/samples/bpf/xdp_redirect.bpf.c b/samples/bpf/xdp_redirect.bpf.c
> index 7c02bacfe96b..620163eb7e19 100644
> --- a/samples/bpf/xdp_redirect.bpf.c
> +++ b/samples/bpf/xdp_redirect.bpf.c
> @@ -16,16 +16,21 @@
>   
>   const volatile int ifindex_out;
>   
> -SEC("xdp")
> +#define XDPBUFSIZE	64

Pktgen sample scripts will default send with 60 pkt length, because the
4 bytes FCS (end-frame checksum) is added by hardware.

Will this result in an error when bpf_xdp_load_bytes() tries to copy 64
bytes from a 60 bytes packet?

> +SEC("xdp.frags")
>   int xdp_redirect_prog(struct xdp_md *ctx)
>   {
> -	void *data_end = (void *)(long)ctx->data_end;
> -	void *data = (void *)(long)ctx->data;
> +	__u8 pkt[XDPBUFSIZE] = {};
> +	void *data_end = &pkt[XDPBUFSIZE-1];
> +	void *data = pkt;
>   	u32 key = bpf_get_smp_processor_id();
>   	struct ethhdr *eth = data;
>   	struct datarec *rec;
>   	u64 nh_off;
>   
> +	if (bpf_xdp_load_bytes(ctx, 0, pkt, sizeof(pkt)))
> +		return XDP_DROP;

E.g. sizeof(pkt) = 64 bytes here.

> +
>   	nh_off = sizeof(*eth);
>   	if (data + nh_off > data_end)
>   		return XDP_DROP;
> @@ -36,11 +41,14 @@ int xdp_redirect_prog(struct xdp_md *ctx)
>   	NO_TEAR_INC(rec->processed);
>   
>   	swap_src_dst_mac(data);
> +	if (bpf_xdp_store_bytes(ctx, 0, pkt, sizeof(pkt)))
> +		return XDP_DROP;
> +
>   	return bpf_redirect(ifindex_out, 0);
>   }
>   
>   /* Redirect require an XDP bpf_prog loaded on the TX device */
> -SEC("xdp")
> +SEC("xdp.frags")
>   int xdp_redirect_dummy_prog(struct xdp_md *ctx)
>   {
>   	return XDP_PASS;