linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] libbpf: Fix build with latest gcc/binutils with LTO
@ 2021-08-27  7:25 Michal Suchanek
  2021-08-30 21:35 ` Andrii Nakryiko
  0 siblings, 1 reply; 4+ messages in thread
From: Michal Suchanek @ 2021-08-27  7:25 UTC (permalink / raw)
  To: bpf
  Cc: Patrick McCarty, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Björn Töpel, Magnus Karlsson,
	Jonathan Lemon, netdev, linux-kernel, Michal Suchanek

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
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1863059
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1188749
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
Make the change conditional on GCC version
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)
 /* 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] libbpf: Fix build with latest gcc/binutils with LTO
  2021-08-27  7:25 [PATCH] libbpf: Fix build with latest gcc/binutils with LTO Michal Suchanek
@ 2021-08-30 21:35 ` Andrii Nakryiko
  0 siblings, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2021-08-30 21:35 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: bpf, Patrick McCarty, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Björn Töpel, Magnus Karlsson,
	Jonathan Lemon, Networking, open list

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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] libbpf: Fix build with latest gcc/binutils with LTO
  2021-08-27  7:28 Michal Suchanek
@ 2021-08-27 10:35 ` Michal Suchánek
  0 siblings, 0 replies; 4+ messages in thread
From: Michal Suchánek @ 2021-08-27 10:35 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Björn Töpel, Magnus Karlsson, Jonathan Lemon,
	netdev, linux-kernel, Patrick McCarty

Hello,

this is a duplicate of

https://patchwork.kernel.org/project/netdevbpf/patch/20210827072539.3399-1-msuchanek@suse.de/

Sorry aboit the noise.

On Fri, Aug 27, 2021 at 09:28:55AM +0200, Michal Suchanek 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
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1863059
> Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1188749
> Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
> Make the change conditional on GCC version
> 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)
>  /* 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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH] libbpf: Fix build with latest gcc/binutils with LTO
@ 2021-08-27  7:28 Michal Suchanek
  2021-08-27 10:35 ` Michal Suchánek
  0 siblings, 1 reply; 4+ messages in thread
From: Michal Suchanek @ 2021-08-27  7:28 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Björn Töpel, Magnus Karlsson, Jonathan Lemon,
	netdev, linux-kernel, Patrick McCarty, Michal Suchanek

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
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1863059
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1188749
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
Make the change conditional on GCC version
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)
 /* 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-30 21:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27  7:25 [PATCH] libbpf: Fix build with latest gcc/binutils with LTO Michal Suchanek
2021-08-30 21:35 ` Andrii Nakryiko
2021-08-27  7:28 Michal Suchanek
2021-08-27 10:35 ` Michal Suchánek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).