| Message ID | 20210827072539.3399-1-msuchanek@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | libbpf: Fix build with latest gcc/binutils with LTO | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Fri, Aug 27, 2021 at 12:25 AM Michal Suchanek <msuchanek@suse.de> wrote: > > From: Patrick McCarty <patrick.mccarty@intel.com> > > After updating to binutils 2.35, the build began to fail with an > assembler error. A bug was opened on the Red Hat Bugzilla a few days > later for the same issue. > > Work around the problem by using the new `symver` attribute (introduced > in GCC 10) as needed, instead of the `COMPAT_VERSION` and > `DEFAULT_VERSION` macros, which expand to assembler directives. > > Fixes: https://github.com/libbpf/libbpf/issues/338 This is not a proper tag. We used the following form before: [0] Closes: https://github.com/libbpf/libbpf/issues/280 So it's a reference. And then mention in commit message that this was initiated by the issue on Github ([0]), or something along those lines. > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1863059 > Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1188749 These are also not proper Fixes: tags for kernel. It's fine if you mention that this change fixes those bugs, but maybe use the reference ([1], [2]) style for that? > Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com> > Make the change conditional on GCC version This is not a tag, maybe remove this or make it part of the commit message properly? > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > --- > tools/lib/bpf/libbpf_internal.h | 23 +++++++++++++++++------ > tools/lib/bpf/xsk.c | 4 ++-- > 2 files changed, 19 insertions(+), 8 deletions(-) > > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h > index 016ca7cb4f8a..af0f3fb102c0 100644 > --- a/tools/lib/bpf/libbpf_internal.h > +++ b/tools/lib/bpf/libbpf_internal.h > @@ -86,20 +86,31 @@ > (offsetof(TYPE, FIELD) + sizeof(((TYPE *)0)->FIELD)) > #endif > > +#ifdef __GNUC__ > +# if __GNUC__ >= 10 > +# define DEFAULT_VERSION(internal_name, api_name, version) \ > +__attribute__((__symver__(#api_name "@@" #version))) > +# define COMPAT_VERSION(internal_name, api_name, version) \ > +__attribute__((__symver__(#api_name "@" #version))) > +# endif > +#endif > + > +#if !defined(COMPAT_VERSION) || !defined(DEFAULT_VERSION) This seems wrong. If __GNUC__ && __GNUC__ >= 10 we'll define DEFAULT_VERSION and COMPAT_VERSION as if we are linking in shared library mode. This will be wrong on new GCC *and* static linking mode. I think the above declarations should be inside #ifdef SHARED section. Also, can you please write it out as #if defined(__GNUC__) && __GNUC__ >= 10, instead of doubly nested #if/#ifdef condition? > /* Symbol versioning is different between static and shared library. > * Properly versioned symbols are needed for shared library, but > * only the symbol of the new version is needed for static library. > */ > -#ifdef SHARED > -# define COMPAT_VERSION(internal_name, api_name, version) \ > +# ifdef SHARED > +# define COMPAT_VERSION(internal_name, api_name, version) \ > asm(".symver " #internal_name "," #api_name "@" #version); > -# define DEFAULT_VERSION(internal_name, api_name, version) \ > +# define DEFAULT_VERSION(internal_name, api_name, version) \ > asm(".symver " #internal_name "," #api_name "@@" #version); > -#else > -# define COMPAT_VERSION(internal_name, api_name, version) > -# define DEFAULT_VERSION(internal_name, api_name, version) \ > +# else > +# define COMPAT_VERSION(internal_name, api_name, version) > +# define DEFAULT_VERSION(internal_name, api_name, version) \ > extern typeof(internal_name) api_name \ > __attribute__((alias(#internal_name))); > +# endif > #endif > > extern void libbpf_print(enum libbpf_print_level level, > diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c > index e9b619aa0cdf..a2111696ba91 100644 > --- a/tools/lib/bpf/xsk.c > +++ b/tools/lib/bpf/xsk.c > @@ -281,6 +281,7 @@ static int xsk_create_umem_rings(struct xsk_umem *umem, int fd, > return err; > } > > +DEFAULT_VERSION(xsk_umem__create_v0_0_4, xsk_umem__create, LIBBPF_0.0.4) > int xsk_umem__create_v0_0_4(struct xsk_umem **umem_ptr, void *umem_area, > __u64 size, struct xsk_ring_prod *fill, > struct xsk_ring_cons *comp, > @@ -345,6 +346,7 @@ struct xsk_umem_config_v1 { > __u32 frame_headroom; > }; > > +COMPAT_VERSION(xsk_umem__create_v0_0_2, xsk_umem__create, LIBBPF_0.0.2) > int xsk_umem__create_v0_0_2(struct xsk_umem **umem_ptr, void *umem_area, > __u64 size, struct xsk_ring_prod *fill, > struct xsk_ring_cons *comp, > @@ -358,8 +360,6 @@ int xsk_umem__create_v0_0_2(struct xsk_umem **umem_ptr, void *umem_area, > return xsk_umem__create_v0_0_4(umem_ptr, umem_area, size, fill, comp, > &config); > } > -COMPAT_VERSION(xsk_umem__create_v0_0_2, xsk_umem__create, LIBBPF_0.0.2) > -DEFAULT_VERSION(xsk_umem__create_v0_0_4, xsk_umem__create, LIBBPF_0.0.4) > > static enum xsk_prog get_xsk_prog(void) > { > -- > 2.31.1 >
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h index 016ca7cb4f8a..af0f3fb102c0 100644 --- a/tools/lib/bpf/libbpf_internal.h +++ b/tools/lib/bpf/libbpf_internal.h @@ -86,20 +86,31 @@ (offsetof(TYPE, FIELD) + sizeof(((TYPE *)0)->FIELD)) #endif +#ifdef __GNUC__ +# if __GNUC__ >= 10 +# define DEFAULT_VERSION(internal_name, api_name, version) \ +__attribute__((__symver__(#api_name "@@" #version))) +# define COMPAT_VERSION(internal_name, api_name, version) \ +__attribute__((__symver__(#api_name "@" #version))) +# endif +#endif + +#if !defined(COMPAT_VERSION) || !defined(DEFAULT_VERSION) /* Symbol versioning is different between static and shared library. * Properly versioned symbols are needed for shared library, but * only the symbol of the new version is needed for static library. */ -#ifdef SHARED -# define COMPAT_VERSION(internal_name, api_name, version) \ +# ifdef SHARED +# define COMPAT_VERSION(internal_name, api_name, version) \ asm(".symver " #internal_name "," #api_name "@" #version); -# define DEFAULT_VERSION(internal_name, api_name, version) \ +# define DEFAULT_VERSION(internal_name, api_name, version) \ asm(".symver " #internal_name "," #api_name "@@" #version); -#else -# define COMPAT_VERSION(internal_name, api_name, version) -# define DEFAULT_VERSION(internal_name, api_name, version) \ +# else +# define COMPAT_VERSION(internal_name, api_name, version) +# define DEFAULT_VERSION(internal_name, api_name, version) \ extern typeof(internal_name) api_name \ __attribute__((alias(#internal_name))); +# endif #endif extern void libbpf_print(enum libbpf_print_level level, diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c index e9b619aa0cdf..a2111696ba91 100644 --- a/tools/lib/bpf/xsk.c +++ b/tools/lib/bpf/xsk.c @@ -281,6 +281,7 @@ static int xsk_create_umem_rings(struct xsk_umem *umem, int fd, return err; } +DEFAULT_VERSION(xsk_umem__create_v0_0_4, xsk_umem__create, LIBBPF_0.0.4) int xsk_umem__create_v0_0_4(struct xsk_umem **umem_ptr, void *umem_area, __u64 size, struct xsk_ring_prod *fill, struct xsk_ring_cons *comp, @@ -345,6 +346,7 @@ struct xsk_umem_config_v1 { __u32 frame_headroom; }; +COMPAT_VERSION(xsk_umem__create_v0_0_2, xsk_umem__create, LIBBPF_0.0.2) int xsk_umem__create_v0_0_2(struct xsk_umem **umem_ptr, void *umem_area, __u64 size, struct xsk_ring_prod *fill, struct xsk_ring_cons *comp, @@ -358,8 +360,6 @@ int xsk_umem__create_v0_0_2(struct xsk_umem **umem_ptr, void *umem_area, return xsk_umem__create_v0_0_4(umem_ptr, umem_area, size, fill, comp, &config); } -COMPAT_VERSION(xsk_umem__create_v0_0_2, xsk_umem__create, LIBBPF_0.0.2) -DEFAULT_VERSION(xsk_umem__create_v0_0_4, xsk_umem__create, LIBBPF_0.0.4) static enum xsk_prog get_xsk_prog(void) {