Re: [PATCH bpf-next v3] docs/bpf: Add documentation for BPF_MAP_TYPE_SK_STORAGE

[Donald Hunter <donald.hunter@gmail.com>]

Donald Hunter <donald.hunter@gmail.com> writes:

> David Vernet <void@manifault.com> writes:
>
>> On Wed, Dec 07, 2022 at 10:27:21AM +0000, Donald Hunter wrote:
>>> +
>>> +This snippet shows how to retrieve socket-local storage in a BPF program:
>>> +
>>> +.. code-block:: c
>>> +
>>> +    SEC("sockops")
>>> +    int _sockops(struct bpf_sock_ops *ctx)
>>> +    {
>>> +            struct my_storage *storage;
>>> +            struct bpf_sock *sk;
>>> +
>>> +            sk = ctx->sk;
>>> +            if (!sk)
>>> +                    return 1;
>>
>> Don't feel strongly about this one, but IMO it's nice for examples to
>> illustrate code that's as close to real and pristine as possible. To
>> that point, should this example perhaps be updated to return -ENOENT
>> here, and -ENOMEM below?
>
> Will do.
>

After digging into this a bit more I notice that the sockops programs in
tools/testing/selftests/bpf/progs mostly return 1 in all cases.

I'm assuming that sockops programs should return valid values for
some op types such as BPF_SOCK_OPS_TIMEOUT_INIT. Other than that I can't
find a definitive list. Do you know if valid return values are
enumerated anywhere, or do I need to dig some more?

>>> +
>>> +            storage = bpf_sk_storage_get(&socket_storage, sk, 0,
>>> +                                         BPF_LOCAL_STORAGE_GET_F_CREATE);
>>> +            if (!storage)
>>> +                    return 1;
>>> +
>>> +            /* Use 'storage' here */
>>
>> Let's return 0 at the end to make the example program technically
>> correct.
>
> Will do.
>
>>> +    }
>>
>> Thanks,
>> David