> On Feb 27, 2023, at 11:37 AM, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Thu, Feb 23, 2023 at 2:05 PM Aditi Ghag <aditi.ghag@xxxxxxxxxxxxx> wrote:
>>
>> The test cases for TCP and UDP iterators mirror the intended usages of the
>> helper.
>>
>> The destroy helpers set `ECONNABORTED` error code that we can validate in the
>> test code with client sockets. But UDP sockets have an overriding error code
>> from the disconnect called during abort, so the error code the validation is
>> only done for TCP sockets.
>>
>> The `struct sock` is redefined as vmlinux.h forward declares the struct, and the
>> loader fails to load the program as it finds the BTF FWD type for the struct
>> incompatible with the BTF STRUCT type.
>>
>> Here are the snippets of the verifier error, and corresponding BTF output:
>>
>> ```
>> verifier error: extern (func ksym) ...: func_proto ... incompatible with kernel
>>
>> BTF for selftest prog binary:
>>
>> [104] FWD 'sock' fwd_kind=struct
>> [70] PTR '(anon)' type_id=104
>> [84] FUNC_PROTO '(anon)' ret_type_id=2 vlen=1
>> '(anon)' type_id=70
>> [85] FUNC 'bpf_sock_destroy' type_id=84 linkage=extern
>> --
>> [96] DATASEC '.ksyms' size=0 vlen=1
>> type_id=85 offset=0 size=0 (FUNC 'bpf_sock_destroy')
>>
>> BTF for selftest vmlinux:
>>
>> [74923] FUNC 'bpf_sock_destroy' type_id=48965 linkage=static
>> [48965] FUNC_PROTO '(anon)' ret_type_id=9 vlen=1
>> 'sk' type_id=1340
>> [1340] PTR '(anon)' type_id=2363
>> [2363] STRUCT 'sock' size=1280 vlen=93
>> ```
>>
>> Signed-off-by: Aditi Ghag <aditi.ghag@xxxxxxxxxxxxx>
>> ---
>> .../selftests/bpf/prog_tests/sock_destroy.c | 125 ++++++++++++++++++
>> .../selftests/bpf/progs/sock_destroy_prog.c | 110 +++++++++++++++
>> 2 files changed, 235 insertions(+)
>> create mode 100644 tools/testing/selftests/bpf/prog_tests/sock_destroy.c
>> create mode 100644 tools/testing/selftests/bpf/progs/sock_destroy_prog.c
>>
>
> [...]
>
>> + n = send(clien, "t", 1, 0);
>> + if (CHECK(n < 0, "client_send", "client failed to send on socket\n"))
>> + goto cleanup;
>> +
>> + start_iter_sockets(skel->progs.iter_tcp6);
>> +
>> + n = send(clien, "t", 1, 0);
>> + if (CHECK(n > 0, "client_send after destroy", "succeeded on destroyed socket\n"))
>> + goto cleanup;
>> + CHECK(errno != ECONNABORTED, "client_send", "unexpected error code on destroyed socket\n");
>> +
>
> please don't use CHECK() macros, prefere ASSERT_xxx() ones
Yes, this'll be handled in the next revision. Thanks!
>
>> +
>> +cleanup:
>> + close(clien);
>> +cleanup_serv:
>> + close(serv);
>> +}
>> +
>> +
>> +void test_udp(struct sock_destroy_prog *skel)
>
> are these meant to be subtests? If yes, model them as such?
I wasn't aware of subtests markers. I'll look into the other tests for reference.
>
> and in either case, make these funcs static
Will do!
>
>> +{
>> + int serv = -1, clien = -1, n = 0;
>> +
>> + serv = start_server(AF_INET6, SOCK_DGRAM, NULL, 6161, 0);
>> + if (CHECK(serv < 0, "start_server", "failed to start server\n"))
>> + goto cleanup_serv;
>> +
>
> [...]
