Re: [PATCH v7 bpf-next 4/4] selftests/bpf: Add geneve with bpf_skb_set_tunnel_opt_dynptr test-case to test_progs

[Shmulik Ladkani <shmulik@metanetworks.com>]

On Mon, 19 Sep 2022 19:58:20 -0700 Yonghong Song <yhs@fb.com> wrote:

> > +	/* set empty geneve options (of runtime length) using a dynptr */
> > +	__builtin_memset(opts, 0x0, sizeof(*opts));
> > +	if (*local_ip % 2)
> > +		bpf_dynptr_from_mem(opts, GENEVE_OPTS_LEN1, 0, &dptr);
> > +	else
> > +		bpf_dynptr_from_mem(opts, GENEVE_OPTS_LEN0, 0, &dptr);
> > +	ret = bpf_skb_set_tunnel_opt_dynptr(skb, &dptr);  
> 
> I think the above example is not good. since it can write as
> 	if (*local_ip % 2)
> 		ret = bpf_skb_set_tunnel_opt(skb, opts, GENEVE_OPTS_LEN1);
> 	else
> 		ret = bpf_skb_set_tunnel_opt(skb, opts,	GENEVE_OPTS_LEN0);
> 
> In the commit message of Patch 2, we have
> 
> ===
> For example, we have an ebpf program that gets geneve options on
> incoming packets, stores them into a map (using a key representing
> the incoming flow), and later needs to assign *same* options to
> reply packets (belonging to same flow).
> ===
> 
> It would be great if you can create a test case for the above
> use case.

Yes, but please note dynptr trim/advance API is still WIP:

https://lore.kernel.org/bpf/CAJnrk1a53F=LLaU+gdmXGcZBBeUR-anALT3iO6pyHKiZpD0cNw@mail.gmail.com/

However, once we settled on the API for setting variable length tunnel
options from a *dynptr* (and not from raw buffer+len), we can just
exercise 'bpf_skb_set_tunnel_opt_dynptr' regardless the original
usecase (i.e. we can assume dynptrs can be properly mangled).

In any case, I can later amend the test once all dynptr convenience
helpers are accepted.

Best,
Shmulik