Re: [PATCH v2 bpf-next 4/6] selftests/bpf: Add IPv4 and IPv6 sockaddr test cases

[Martin KaFai Lau <martin.lau@linux.dev>]

On 4/18/24 9:37 AM, Jordan Rife wrote:
>> The test_sock_addr.{c,sh} can be retired as long as all its tests are migrated
>> to sock_addr.c
> 
> test_sock_addr.c has a few more test dimensions than
> prog_tests/sock_addr.c currently does, so it covers a few more
> scenarios.
> 
> struct sock_addr_test {
>      const char *descr;
>      /* BPF prog properties */
>      load_fn loadfn;
>      enum bpf_attach_type expected_attach_type;
>      enum bpf_attach_type attach_type;
>      /* Socket properties */
>      int domain;
>      int type;
>      /* IP:port pairs for BPF prog to override */
>      const char *requested_ip;
>      unsigned short requested_port;
>      const char *expected_ip;
>      unsigned short expected_port;
>      const char *expected_src_ip;
>      /* Expected test result */
>      enum {
>          LOAD_REJECT,
>          ATTACH_REJECT,
>          ATTACH_OKAY,
>          SYSCALL_EPERM,
>          SYSCALL_ENOTSUPP,
>          SUCCESS,
>      } expected_result;
> };
> 
> We focus on the "happy path" scenarios currently in
> prog_tests/sock_addr.c while test_sock_addr.c has test cases that
> cover a range of scenarios where loading or attaching a BPF program
> should fail. There are also a few asm tests that use program loader
> functions like sendmsg4_rw_asm_prog_load which specifies a series of
> BPF instructions directly rather than loading one of the skeletons.
> Adding in these test dimensions and migrating the test cases is a
> slightly bigger lift for this patch series. Do we want to try to
> migrate all of these to prog_tests/sock_addr.c in order to fully
> retire it?

I don't want to keep this set hostage too much until everything is migrated from 
test_sock_addr.c. As long as for the tests you find useful in test_sock_addr.c 
in this patch set and moved them to prog_tests/sock_addr.c, it is heading in the 
right direction. For the moved test, please remove them from test_sock_addr.c so 
that it is clear what else needs to be done.

[ Side note for future migration attempt, at least for the LOAD_REJECT one, it 
probably makes sense to write it like progs/verifier_*.c ]