Re: [PATCH bpf-next v1 3/3] selftests/bpf: tests for using dynptrs to parse skb and xdp buffers

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Joanne Koong <joannelkoong@gmail.com>
Cc: bpf <bpf@vger.kernel.org>, Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>
Subject: Re: [PATCH bpf-next v1 3/3] selftests/bpf: tests for using dynptrs to parse skb and xdp buffers
Date: Mon, 1 Aug 2022 12:12:26 -0700	[thread overview]
Message-ID: <CAADnVQJiA+Ari7_MmBLgNSDPoCY_wmQTdE9oqCX1DGqo6nVXxw@mail.gmail.com> (raw)
In-Reply-To: <20220726184706.954822-4-joannelkoong@gmail.com>

On Tue, Jul 26, 2022 at 11:48 AM Joanne Koong <joannelkoong@gmail.com> wrote:
>
>
> -static __always_inline int handle_ipv4(struct xdp_md *xdp)
> +static __always_inline int handle_ipv4(struct xdp_md *xdp, struct bpf_dynptr *xdp_ptr)
>  {
> -       void *data_end = (void *)(long)xdp->data_end;
> -       void *data = (void *)(long)xdp->data;
> +       struct bpf_dynptr new_xdp_ptr;
>         struct iptnl_info *tnl;
>         struct ethhdr *new_eth;
>         struct ethhdr *old_eth;
> -       struct iphdr *iph = data + sizeof(struct ethhdr);
> +       struct iphdr *iph;
>         __u16 *next_iph;
>         __u16 payload_len;
>         struct vip vip = {};
> @@ -90,10 +90,12 @@ static __always_inline int handle_ipv4(struct xdp_md *xdp)
>         __u32 csum = 0;
>         int i;
>
> -       if (iph + 1 > data_end)
> +       iph = bpf_dynptr_data(xdp_ptr, ethhdr_sz,
> +                             iphdr_sz + (tcphdr_sz > udphdr_sz ? tcphdr_sz : udphdr_sz));
> +       if (!iph)
>                 return XDP_DROP;

dynptr based xdp/skb access looks neat.
Maybe in addition to bpf_dynptr_data() we can add helper(s)
that return skb/xdp_md from dynptr?
This way the code will be passing dynptr only and there will
be no need to pass around 'struct xdp_md *xdp' (like this function).

Separately please keep the existing tests instead of converting them.
Either ifdef data/data_end vs dynptr style or copy paste
the whole test into a new .c file. Whichever is cleaner.