netdev - Re: [PATCH bpf-next v4 5/8] libbpf: Add helper macro to clear opts structs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 11 Jul 2023 11:42:54 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: ast@...nel.org, andrii@...nel.org, martin.lau@...ux.dev,
 razor@...ckwall.org, sdf@...gle.com, john.fastabend@...il.com,
 kuba@...nel.org, dxu@...uu.xyz, joe@...ium.io, toke@...nel.org,
 davem@...emloft.net, bpf@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next v4 5/8] libbpf: Add helper macro to clear opts
 structs

On 7/11/23 6:02 AM, Andrii Nakryiko wrote:
> On Mon, Jul 10, 2023 at 1:12 PM Daniel Borkmann <daniel@...earbox.net> wrote:
>>
>> Add a small and generic LIBBPF_OPTS_CLEAR() helper macros which clears
>> an opts structure and reinitializes its .sz member to place the structure
>> size. I found this very useful when developing selftests, but it is also
>> generic enough as a macro next to the existing LIBBPF_OPTS() which hides
>> the .sz initialization, too.
>>
>> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
>> ---
>>   tools/lib/bpf/libbpf_common.h | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/tools/lib/bpf/libbpf_common.h b/tools/lib/bpf/libbpf_common.h
>> index 9a7937f339df..eb180023aa97 100644
>> --- a/tools/lib/bpf/libbpf_common.h
>> +++ b/tools/lib/bpf/libbpf_common.h
>> @@ -70,4 +70,15 @@
>>                  };                                                          \
>>          })
>>
>> +/* Helper macro to clear a libbpf options struct
>> + *
>> + * Small helper macro to reset all fields and to reinitialize the common
>> + * structure size member.
>> + */
>> +#define LIBBPF_OPTS_CLEAR(NAME)                                                    \
>> +       do {                                                                \
>> +               memset(&NAME, 0, sizeof(NAME));                             \
>> +               NAME.sz = sizeof(NAME);                                     \
>> +       } while (0)
>> +
> 
> This is fine, but I think you can go a half-step further and have
> something even more universal and useful. Something like this:
> 
> 
> #define LIBBPF_OPTS_RESET(NAME, ...)
>      do {
>          memset(&NAME, 0, sizeof(NAME));
>          NAME = (typeof(NAME)) {
>              .sz = sizeof(struct TYPE),
>              __VA_ARGS__
>          };
>       while (0);
> 
> I actually haven't tried if that typeof() trick works, but I hope it does :)

It does, I've used this in BPF code for Cilium, too. ;)

> Then your LIBBPF_OPTS_CLEAR() is just LIBBPF_OPTS_RESET(x). But you
> can also re-initialize:
> 
> LIBBPF_OPTS_RESET(x, .flags = 123, .prog_fd = 456);
> 
> It's more in line with LIBBPF_OPTS() itself in capabilities, except it
> works on existing variable.

Agree, changed into ...

/* Helper macro to clear and optionally reinitialize libbpf options struct
  *
  * Small helper macro to reset all fields and to reinitialize the common
  * structure size member. Values provided by users in struct initializer-
  * syntax as varargs can be provided as well to reinitialize options struct
  * specific members.
  */
#define LIBBPF_OPTS_RESET(NAME, ...)                                        \
         do {                                                                \
                 memset(&NAME, 0, sizeof(NAME));                             \
                 NAME = (typeof(NAME)) {                                     \
                         .sz = sizeof(NAME),                                 \
                         __VA_ARGS__                                         \
                 };                                                          \
         } while (0)

... and updated all the test cases.

Thanks,
Daniel