On Tue, Nov 7, 2023 at 10:55 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> On 11/6/23 10:29 PM, Yonghong Song wrote:
> > Martin reported that there is a libbpf complaining of non-zero-value tail
> > padding with LIBBPF_OPTS_RESET macro if struct bpf_netkit_opts is modified
> > to have a 4-byte tail padding. This only happens to clang compiler.
> > The commend line is: ./test_progs -t tc_netkit_multi_links
> > Martin and I did some investigation and found this indeed the case and
> > the following are the investigation details.
> >
> > Clang 18:
> > clang version 18.0.0
> > <I tried clang15/16/17 and they all have similar results>
> >
> > tools/lib/bpf/libbpf_common.h:
> > #define LIBBPF_OPTS_RESET(NAME, ...) \
> > do { \
> > memset(&NAME, 0, sizeof(NAME)); \
> > NAME = (typeof(NAME)) { \
> > .sz = sizeof(NAME), \
> > __VA_ARGS__ \
> > }; \
> > } while (0)
> >
> > #endif
> >
> > tools/lib/bpf/libbpf.h:
> > struct bpf_netkit_opts {
> > /* size of this struct, for forward/backward compatibility */
> > size_t sz;
> > __u32 flags;
> > __u32 relative_fd;
> > __u32 relative_id;
> > __u64 expected_revision;
> > size_t :0;
> > };
> > #define bpf_netkit_opts__last_field expected_revision
> > In the above struct bpf_netkit_opts, there is no tail padding.
> >
> > prog_tests/tc_netkit.c:
> > static void serial_test_tc_netkit_multi_links_target(int mode, int target)
> > {
> > ...
> > LIBBPF_OPTS(bpf_netkit_opts, optl);
> > ...
> > LIBBPF_OPTS_RESET(optl,
> > .flags = BPF_F_BEFORE,
> > .relative_fd = bpf_program__fd(skel->progs.tc1),
> > );
> > ...
> > }
> >
> > Let us make the following source change, note that we have a 4-byte
> > tailing padding now.
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 6cd9c501624f..0dd83910ae9a 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -803,13 +803,13 @@ bpf_program__attach_tcx(const struct bpf_program *prog, int ifindex,
> > struct bpf_netkit_opts {
> > /* size of this struct, for forward/backward compatibility */
> > size_t sz;
> > - __u32 flags;
> > __u32 relative_fd;
> > __u32 relative_id;
> > __u64 expected_revision;
> > + __u32 flags;
> > size_t :0;
> > };
> > -#define bpf_netkit_opts__last_field expected_revision
> > +#define bpf_netkit_opts__last_field flags
>
> The bpf_netkit_ops is in the bpf tree. If avoiding a hole in bpf_netkit_opts
> like above is preferred, probably the fix in this patch and the bpf_netkit_ops
> change should be in the same libbpf version?
Yes, they both will be in libbpf v1.3, which hopefully should be
released this week. Let's land the fix soon-ish so that it is synced
and goes into final v1.3.
>
> Ran the test in a loop. It resolved the issue.
>
> Tested-by: Martin KaFai Lau <martin.lau@xxxxxxxxxx>
>
