[dpdk-dev] [PATCH 0/3] mlx: fix build with -fno-common (gcc 10)

[dpdk-dev] [PATCH 0/3] mlx: fix build with -fno-common (gcc 10)

[Thomas Monjalon]

In GCC 10, -fno-common will be the default.
There are 2 ways of solving issues:
  - add -fcommon
  - stop allocating variables in .h files

In this patchset, the variables are declared extern,
because it is cleaner anyway.


Thomas Monjalon (3):
  common/mlx5: split glue initialization
  common/mlx5: fix build with -fno-common
  net/mlx4: fix build with -fno-common

 drivers/common/mlx5/mlx5_common.c | 80 +++++++++++++++++++------------
 drivers/common/mlx5/mlx5_glue.h   |  2 +-
 drivers/net/mlx4/mlx4.c           |  4 ++
 drivers/net/mlx4/mlx4_glue.h      |  2 +-
 drivers/net/mlx4/mlx4_rxtx.h      |  2 +-
 5 files changed, 57 insertions(+), 33 deletions(-)

-- 
2.25.0

[dpdk-dev] [PATCH 1/3] common/mlx5: split glue initialization

[Thomas Monjalon]

The function mlx5_glue_init was doing three things:
	- initialize logs
	- load glue library if in dlopen mode
	- initialize glue layer
They are split in three functions for clarity.

The config option RTE_IBVERBS_LINK_DLOPEN is not used anymore
outside of make and meson files. It is replaced with MLX5_GLUE,
which is defined in the same condition and is already used with dlopen.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/common/mlx5/mlx5_common.c | 77 +++++++++++++++++++------------
 1 file changed, 47 insertions(+), 30 deletions(-)

diff --git a/drivers/common/mlx5/mlx5_common.c b/drivers/common/mlx5/mlx5_common.c
index 610fb480b5..f96506c27a 100644
--- a/drivers/common/mlx5/mlx5_common.c
+++ b/drivers/common/mlx5/mlx5_common.c
@@ -160,7 +160,7 @@ mlx5_translate_port_name(const char *port_name_in,
 	return;
 }
 
-#ifdef RTE_IBVERBS_LINK_DLOPEN
+#ifdef MLX5_GLUE
 
 /**
  * Suffix RTE_EAL_PMD_PATH with "-glue".
@@ -202,36 +202,12 @@ mlx5_glue_path(char *buf, size_t size)
 		" re-configure DPDK");
 	return NULL;
 }
-#endif
 
-/**
- * Initialization routine for run-time dependency on rdma-core.
- */
-RTE_INIT_PRIO(mlx5_glue_init, CLASS)
+static int
+mlx5_glue_dlopen(void)
 {
 	void *handle = NULL;
 
-	/* Initialize common log type. */
-	mlx5_common_logtype = rte_log_register("pmd.common.mlx5");
-	if (mlx5_common_logtype >= 0)
-		rte_log_set_level(mlx5_common_logtype, RTE_LOG_NOTICE);
-	/*
-	 * RDMAV_HUGEPAGES_SAFE tells ibv_fork_init() we intend to use
-	 * huge pages. Calling ibv_fork_init() during init allows
-	 * applications to use fork() safely for purposes other than
-	 * using this PMD, which is not supported in forked processes.
-	 */
-	setenv("RDMAV_HUGEPAGES_SAFE", "1", 1);
-	/* Match the size of Rx completion entry to the size of a cacheline. */
-	if (RTE_CACHE_LINE_SIZE == 128)
-		setenv("MLX5_CQE_SIZE", "128", 0);
-	/*
-	 * MLX5_DEVICE_FATAL_CLEANUP tells ibv_destroy functions to
-	 * cleanup all the Verbs resources even when the device was removed.
-	 */
-	setenv("MLX5_DEVICE_FATAL_CLEANUP", "1", 1);
-	/* The glue initialization was done earlier by mlx5 common library. */
-#ifdef RTE_IBVERBS_LINK_DLOPEN
 	char glue_path[sizeof(RTE_EAL_PMD_PATH) - 1 + sizeof("-glue")];
 	const char *path[] = {
 		/*
@@ -301,7 +277,49 @@ RTE_INIT_PRIO(mlx5_glue_init, CLASS)
 		goto glue_error;
 	}
 	mlx5_glue = *sym;
-#endif /* RTE_IBVERBS_LINK_DLOPEN */
+	return 0;
+
+glue_error:
+	if (handle)
+		dlclose(handle);
+	return -1;
+}
+
+#endif
+
+RTE_INIT_PRIO(mlx5_log_init, LOG)
+{
+	mlx5_common_logtype = rte_log_register("pmd.common.mlx5");
+	if (mlx5_common_logtype >= 0)
+		rte_log_set_level(mlx5_common_logtype, RTE_LOG_NOTICE);
+}
+
+/**
+ * Initialization routine for run-time dependency on rdma-core.
+ */
+RTE_INIT_PRIO(mlx5_glue_init, CLASS)
+{
+	/*
+	 * RDMAV_HUGEPAGES_SAFE tells ibv_fork_init() we intend to use
+	 * huge pages. Calling ibv_fork_init() during init allows
+	 * applications to use fork() safely for purposes other than
+	 * using this PMD, which is not supported in forked processes.
+	 */
+	setenv("RDMAV_HUGEPAGES_SAFE", "1", 1);
+	/* Match the size of Rx completion entry to the size of a cacheline. */
+	if (RTE_CACHE_LINE_SIZE == 128)
+		setenv("MLX5_CQE_SIZE", "128", 0);
+	/*
+	 * MLX5_DEVICE_FATAL_CLEANUP tells ibv_destroy functions to
+	 * cleanup all the Verbs resources even when the device was removed.
+	 */
+	setenv("MLX5_DEVICE_FATAL_CLEANUP", "1", 1);
+
+#ifdef MLX5_GLUE
+	if (mlx5_glue_dlopen() != 0)
+		goto glue_error;
+#endif
+
 #ifdef RTE_LIBRTE_MLX5_DEBUG
 	/* Glue structure must not contain any NULL pointers. */
 	{
@@ -319,9 +337,8 @@ RTE_INIT_PRIO(mlx5_glue_init, CLASS)
 	}
 	mlx5_glue->fork_init();
 	return;
+
 glue_error:
-	if (handle)
-		dlclose(handle);
 	DRV_LOG(WARNING, "Cannot initialize MLX5 common due to missing"
 		" run-time dependency on rdma-core libraries (libibverbs,"
 		" libmlx5)");
-- 
2.25.0

[dpdk-dev] [PATCH 2/3] common/mlx5: fix build with -fno-common

[Thomas Monjalon]

The variable storages of the same name are merged together
if compiled with -fcommon. This is the default.
This default behaviour allows to declare a variable in a header file
and share the variable in every .o binaries thanks to merge at link-time.

In the case of dlopen linking of the glue library, the pointer mlx5_glue
is referencing the glue functions struct and is set after calling dlopen.

If compiling with -fno-common (default in GCC 10), the variable
must be declared as extern to avoid multiple re-definitions.
In case the glue layer is split in glue library, the variable mlx5_glue
needs to have its own storage for the rest of the PMD.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/common/mlx5/mlx5_common.c | 3 +++
 drivers/common/mlx5/mlx5_glue.h   | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/common/mlx5/mlx5_common.c b/drivers/common/mlx5/mlx5_common.c
index f96506c27a..6110cf49de 100644
--- a/drivers/common/mlx5/mlx5_common.c
+++ b/drivers/common/mlx5/mlx5_common.c
@@ -16,6 +16,9 @@
 
 int mlx5_common_logtype;
 
+#ifdef MLX5_GLUE
+const struct mlx5_glue *mlx5_glue = NULL;
+#endif
 
 /**
  * Get PCI information by sysfs device path.
diff --git a/drivers/common/mlx5/mlx5_glue.h b/drivers/common/mlx5/mlx5_glue.h
index 6238b43946..29678623e4 100644
--- a/drivers/common/mlx5/mlx5_glue.h
+++ b/drivers/common/mlx5/mlx5_glue.h
@@ -300,6 +300,6 @@ struct mlx5_glue {
 			 size_t event_resp_len);
 };
 
-const struct mlx5_glue *mlx5_glue;
+extern const struct mlx5_glue *mlx5_glue;
 
 #endif /* MLX5_GLUE_H_ */
-- 
2.25.0

[dpdk-dev] [PATCH 3/3] net/mlx4: fix build with -fno-common

[Thomas Monjalon]

The variable storages of the same name are merged together
if compiled with -fcommon. This is the default.
This default behaviour allows to declare a variable in a header file
and share the variable in every .o binaries thanks to merge at link-time.

In the case of dlopen linking of the glue library, the pointer mlx4_glue
is referencing the glue functions struct and is set after calling dlopen.

If compiling with -fno-common (default in GCC 10), the variables
must be declared as extern to avoid multiple re-definitions.
In case the glue layer is split in glue library, the variable mlx4_glue
needs to have its own storage for the rest of the PMD.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/net/mlx4/mlx4.c      | 4 ++++
 drivers/net/mlx4/mlx4_glue.h | 2 +-
 drivers/net/mlx4/mlx4_rxtx.h | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index a1dd658176..49e721df5f 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -47,6 +47,10 @@
 #include "mlx4_rxtx.h"
 #include "mlx4_utils.h"
 
+#ifdef MLX4_GLUE
+const struct mlx4_glue *mlx4_glue = NULL;
+#endif
+
 static const char *MZ_MLX4_PMD_SHARED_DATA = "mlx4_pmd_shared_data";
 
 /* Shared memory between primary and secondary processes. */
diff --git a/drivers/net/mlx4/mlx4_glue.h b/drivers/net/mlx4/mlx4_glue.h
index 668ca86700..5d9e985495 100644
--- a/drivers/net/mlx4/mlx4_glue.h
+++ b/drivers/net/mlx4/mlx4_glue.h
@@ -84,6 +84,6 @@ struct mlx4_glue {
 				   void *attr);
 };
 
-const struct mlx4_glue *mlx4_glue;
+extern const struct mlx4_glue *mlx4_glue;
 
 #endif /* MLX4_GLUE_H_ */
diff --git a/drivers/net/mlx4/mlx4_rxtx.h b/drivers/net/mlx4/mlx4_rxtx.h
index 8baf33fa94..9de6c59411 100644
--- a/drivers/net/mlx4/mlx4_rxtx.h
+++ b/drivers/net/mlx4/mlx4_rxtx.h
@@ -124,7 +124,7 @@ struct txq {
 
 /* mlx4_rxq.c */
 
-uint8_t mlx4_rss_hash_key_default[MLX4_RSS_HASH_KEY_SIZE];
+extern uint8_t mlx4_rss_hash_key_default[MLX4_RSS_HASH_KEY_SIZE];
 int mlx4_rss_init(struct mlx4_priv *priv);
 void mlx4_rss_deinit(struct mlx4_priv *priv);
 struct mlx4_rss *mlx4_rss_get(struct mlx4_priv *priv, uint64_t fields,
-- 
2.25.0

Re: [dpdk-dev] [PATCH 0/3] mlx: fix build with -fno-common (gcc 10)

[Matan Azrad]

 From:  Thomas Monjalon
> In GCC 10, -fno-common will be the default.
> There are 2 ways of solving issues:
>   - add -fcommon
>   - stop allocating variables in .h files
> 
> In this patchset, the variables are declared extern, because it is cleaner
> anyway.
> 
> 

Series-acked-by: Matan Azrad <matan@mellanox.com>
> Thomas Monjalon (3):
>   common/mlx5: split glue initialization
>   common/mlx5: fix build with -fno-common
>   net/mlx4: fix build with -fno-common
> 
>  drivers/common/mlx5/mlx5_common.c | 80 +++++++++++++++++++--------
> ----
>  drivers/common/mlx5/mlx5_glue.h   |  2 +-
>  drivers/net/mlx4/mlx4.c           |  4 ++
>  drivers/net/mlx4/mlx4_glue.h      |  2 +-
>  drivers/net/mlx4/mlx4_rxtx.h      |  2 +-
>  5 files changed, 57 insertions(+), 33 deletions(-)
> 
> --
> 2.25.0

Re: [dpdk-dev] [PATCH 0/3] mlx: fix build with -fno-common (gcc 10)

[Thomas Monjalon]

17/03/2020 09:34, Matan Azrad:
>  From:  Thomas Monjalon
> > In GCC 10, -fno-common will be the default.
> > There are 2 ways of solving issues:
> >   - add -fcommon
> >   - stop allocating variables in .h files
> > 
> > In this patchset, the variables are declared extern, because it is cleaner
> > anyway.
> 
> Series-acked-by: Matan Azrad <matan@mellanox.com>
> 
> > Thomas Monjalon (3):
> >   common/mlx5: split glue initialization
> >   common/mlx5: fix build with -fno-common
> >   net/mlx4: fix build with -fno-common

It seems these patches are also fixing usage of glue dlopen
with PMD as shared library.
We should add Cc: stable@dpdk.org and identify a root cause.

[dpdk-dev] [PATCH v2 0/3] mlx: fix build with -fno-common (gcc 10)

[Thomas Monjalon]

In GCC 10, -fno-common will be the default.
There are 2 ways of solving issues:
  - add -fcommon
  - stop allocating variables in .h files

In this patchset, the variables are declared extern,
because it is cleaner anyway.


v2: do not initialize global variables and Cc stable


Thomas Monjalon (3):
  common/mlx5: split glue initialization
  common/mlx5: fix build with -fno-common
  net/mlx4: fix build with -fno-common

 drivers/common/mlx5/mlx5_common.c | 78 +++++++++++++++++++------------
 drivers/common/mlx5/mlx5_glue.h   |  2 +-
 drivers/net/mlx4/mlx4.c           |  4 ++
 drivers/net/mlx4/mlx4_glue.h      |  2 +-
 drivers/net/mlx4/mlx4_rxtx.h      |  2 +-
 5 files changed, 55 insertions(+), 33 deletions(-)

-- 
2.26.0

[dpdk-dev] [PATCH v2 1/3] common/mlx5: split glue initialization

[Thomas Monjalon]

The function mlx5_glue_init was doing three things:
	- initialize logs
	- load glue library if in dlopen mode
	- initialize glue layer
They are split in three functions for clarity.

The config option RTE_IBVERBS_LINK_DLOPEN is not used anymore
outside of make and meson files. It is replaced with MLX5_GLUE,
which is defined in the same condition and is already used with dlopen.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/common/mlx5/mlx5_common.c | 75 ++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 30 deletions(-)

diff --git a/drivers/common/mlx5/mlx5_common.c b/drivers/common/mlx5/mlx5_common.c
index 046339c46f..9a30e6e0aa 100644
--- a/drivers/common/mlx5/mlx5_common.c
+++ b/drivers/common/mlx5/mlx5_common.c
@@ -162,7 +162,7 @@ mlx5_translate_port_name(const char *port_name_in,
 	return;
 }
 
-#ifdef RTE_IBVERBS_LINK_DLOPEN
+#ifdef MLX5_GLUE
 
 /**
  * Suffix RTE_EAL_PMD_PATH with "-glue".
@@ -204,32 +204,10 @@ mlx5_glue_path(char *buf, size_t size)
 		" re-configure DPDK");
 	return NULL;
 }
-#endif
 
-/**
- * Initialization routine for run-time dependency on rdma-core.
- */
-RTE_INIT_PRIO(mlx5_glue_init, CLASS)
+static int
+mlx5_glue_dlopen(void)
 {
-	mlx5_common_logtype = rte_log_register_type_and_pick_level("pmd.common.mlx5", RTE_LOG_NOTICE);
-
-	/*
-	 * RDMAV_HUGEPAGES_SAFE tells ibv_fork_init() we intend to use
-	 * huge pages. Calling ibv_fork_init() during init allows
-	 * applications to use fork() safely for purposes other than
-	 * using this PMD, which is not supported in forked processes.
-	 */
-	setenv("RDMAV_HUGEPAGES_SAFE", "1", 1);
-	/* Match the size of Rx completion entry to the size of a cacheline. */
-	if (RTE_CACHE_LINE_SIZE == 128)
-		setenv("MLX5_CQE_SIZE", "128", 0);
-	/*
-	 * MLX5_DEVICE_FATAL_CLEANUP tells ibv_destroy functions to
-	 * cleanup all the Verbs resources even when the device was removed.
-	 */
-	setenv("MLX5_DEVICE_FATAL_CLEANUP", "1", 1);
-	/* The glue initialization was done earlier by mlx5 common library. */
-#ifdef RTE_IBVERBS_LINK_DLOPEN
 	char glue_path[sizeof(RTE_EAL_PMD_PATH) - 1 + sizeof("-glue")];
 	void *handle = NULL;
 
@@ -301,7 +279,47 @@ RTE_INIT_PRIO(mlx5_glue_init, CLASS)
 		goto glue_error;
 	}
 	mlx5_glue = *sym;
-#endif /* RTE_IBVERBS_LINK_DLOPEN */
+	return 0;
+
+glue_error:
+	if (handle)
+		dlclose(handle);
+	return -1;
+}
+
+#endif
+
+RTE_INIT_PRIO(mlx5_log_init, LOG)
+{
+	mlx5_common_logtype = rte_log_register_type_and_pick_level("pmd.common.mlx5", RTE_LOG_NOTICE);
+}
+
+/**
+ * Initialization routine for run-time dependency on rdma-core.
+ */
+RTE_INIT_PRIO(mlx5_glue_init, CLASS)
+{
+	/*
+	 * RDMAV_HUGEPAGES_SAFE tells ibv_fork_init() we intend to use
+	 * huge pages. Calling ibv_fork_init() during init allows
+	 * applications to use fork() safely for purposes other than
+	 * using this PMD, which is not supported in forked processes.
+	 */
+	setenv("RDMAV_HUGEPAGES_SAFE", "1", 1);
+	/* Match the size of Rx completion entry to the size of a cacheline. */
+	if (RTE_CACHE_LINE_SIZE == 128)
+		setenv("MLX5_CQE_SIZE", "128", 0);
+	/*
+	 * MLX5_DEVICE_FATAL_CLEANUP tells ibv_destroy functions to
+	 * cleanup all the Verbs resources even when the device was removed.
+	 */
+	setenv("MLX5_DEVICE_FATAL_CLEANUP", "1", 1);
+
+#ifdef MLX5_GLUE
+	if (mlx5_glue_dlopen() != 0)
+		goto glue_error;
+#endif
+
 #ifdef RTE_LIBRTE_MLX5_DEBUG
 	/* Glue structure must not contain any NULL pointers. */
 	{
@@ -319,11 +337,8 @@ RTE_INIT_PRIO(mlx5_glue_init, CLASS)
 	}
 	mlx5_glue->fork_init();
 	return;
+
 glue_error:
-#ifdef RTE_IBVERBS_LINK_DLOPEN
-	if (handle)
-		dlclose(handle);
-#endif
 	DRV_LOG(WARNING, "Cannot initialize MLX5 common due to missing"
 		" run-time dependency on rdma-core libraries (libibverbs,"
 		" libmlx5)");
-- 
2.26.0

[dpdk-dev] [PATCH v2 2/3] common/mlx5: fix build with -fno-common

[Thomas Monjalon]

The variable storages of the same name are merged together
if compiled with -fcommon. This is the default.
This default behaviour allows to declare a variable in a header file
and share the variable in every .o binaries thanks to merge at link-time.

In the case of dlopen linking of the glue library, the pointer mlx5_glue
is referencing the glue functions struct and is set after calling dlopen.

If compiling with -fno-common (default in GCC 10), the variable
must be declared as extern to avoid multiple re-definitions.
In case the glue layer is split in glue library, the variable mlx5_glue
needs to have its own storage for the rest of the PMD.

Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/common/mlx5/mlx5_common.c | 3 +++
 drivers/common/mlx5/mlx5_glue.h   | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/common/mlx5/mlx5_common.c b/drivers/common/mlx5/mlx5_common.c
index 9a30e6e0aa..98a82c262f 100644
--- a/drivers/common/mlx5/mlx5_common.c
+++ b/drivers/common/mlx5/mlx5_common.c
@@ -18,6 +18,9 @@
 
 int mlx5_common_logtype;
 
+#ifdef MLX5_GLUE
+const struct mlx5_glue *mlx5_glue;
+#endif
 
 /**
  * Get PCI information by sysfs device path.
diff --git a/drivers/common/mlx5/mlx5_glue.h b/drivers/common/mlx5/mlx5_glue.h
index 6238b43946..29678623e4 100644
--- a/drivers/common/mlx5/mlx5_glue.h
+++ b/drivers/common/mlx5/mlx5_glue.h
@@ -300,6 +300,6 @@ struct mlx5_glue {
 			 size_t event_resp_len);
 };
 
-const struct mlx5_glue *mlx5_glue;
+extern const struct mlx5_glue *mlx5_glue;
 
 #endif /* MLX5_GLUE_H_ */
-- 
2.26.0

[dpdk-dev] [PATCH v2 3/3] net/mlx4: fix build with -fno-common

[Thomas Monjalon]

The variable storages of the same name are merged together
if compiled with -fcommon. This is the default.
This default behaviour allows to declare a variable in a header file
and share the variable in every .o binaries thanks to merge at link-time.

In the case of dlopen linking of the glue library, the pointer mlx4_glue
is referencing the glue functions struct and is set after calling dlopen.

If compiling with -fno-common (default in GCC 10), the variables
must be declared as extern to avoid multiple re-definitions.
In case the glue layer is split in glue library, the variable mlx4_glue
needs to have its own storage for the rest of the PMD.

Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx4/mlx4.c      | 4 ++++
 drivers/net/mlx4/mlx4_glue.h | 2 +-
 drivers/net/mlx4/mlx4_rxtx.h | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 339a4d3ab6..ee15f28dff 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -49,6 +49,10 @@
 #include "mlx4_rxtx.h"
 #include "mlx4_utils.h"
 
+#ifdef MLX4_GLUE
+const struct mlx4_glue *mlx4_glue;
+#endif
+
 static const char *MZ_MLX4_PMD_SHARED_DATA = "mlx4_pmd_shared_data";
 
 /* Shared memory between primary and secondary processes. */
diff --git a/drivers/net/mlx4/mlx4_glue.h b/drivers/net/mlx4/mlx4_glue.h
index 668ca86700..5d9e985495 100644
--- a/drivers/net/mlx4/mlx4_glue.h
+++ b/drivers/net/mlx4/mlx4_glue.h
@@ -84,6 +84,6 @@ struct mlx4_glue {
 				   void *attr);
 };
 
-const struct mlx4_glue *mlx4_glue;
+extern const struct mlx4_glue *mlx4_glue;
 
 #endif /* MLX4_GLUE_H_ */
diff --git a/drivers/net/mlx4/mlx4_rxtx.h b/drivers/net/mlx4/mlx4_rxtx.h
index 8baf33fa94..9de6c59411 100644
--- a/drivers/net/mlx4/mlx4_rxtx.h
+++ b/drivers/net/mlx4/mlx4_rxtx.h
@@ -124,7 +124,7 @@ struct txq {
 
 /* mlx4_rxq.c */
 
-uint8_t mlx4_rss_hash_key_default[MLX4_RSS_HASH_KEY_SIZE];
+extern uint8_t mlx4_rss_hash_key_default[MLX4_RSS_HASH_KEY_SIZE];
 int mlx4_rss_init(struct mlx4_priv *priv);
 void mlx4_rss_deinit(struct mlx4_priv *priv);
 struct mlx4_rss *mlx4_rss_get(struct mlx4_priv *priv, uint64_t fields,
-- 
2.26.0

[dpdk-dev] [PATCH v3 0/3] mlx: fix build with -fno-common (gcc 10)

[Thomas Monjalon]

In GCC 10, -fno-common will be the default.
There are 2 ways of solving issues:
  - add -fcommon
  - stop allocating variables in .h files

In this patchset, the variables are declared extern,
because it is cleaner anyway.


v2: do not initialize global variables and Cc stable
v3: fix rebase


Thomas Monjalon (3):
  common/mlx5: split glue initialization
  common/mlx5: fix build with -fno-common
  net/mlx4: fix build with -fno-common

 drivers/common/mlx5/mlx5_common.c | 82 +++++++++++++++++++------------
 drivers/common/mlx5/mlx5_glue.h   |  2 +-
 drivers/net/mlx4/mlx4.c           |  4 ++
 drivers/net/mlx4/mlx4_glue.h      |  2 +-
 drivers/net/mlx4/mlx4_rxtx.h      |  2 +-
 5 files changed, 57 insertions(+), 35 deletions(-)

-- 
2.26.0

[dpdk-dev] [PATCH v3 1/3] common/mlx5: split glue initialization

[Thomas Monjalon]

The function mlx5_glue_init was doing three things:
	- initialize logs
	- load glue library if in dlopen mode
	- initialize glue layer
They are split in three functions for clarity.

The config option RTE_IBVERBS_LINK_DLOPEN is not used anymore
outside of make and meson files. It is replaced with MLX5_GLUE,
which is defined in the same condition and is already used with dlopen.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/common/mlx5/mlx5_common.c | 79 ++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 32 deletions(-)

diff --git a/drivers/common/mlx5/mlx5_common.c b/drivers/common/mlx5/mlx5_common.c
index 9ea56f2ef2..d8c01a5d14 100644
--- a/drivers/common/mlx5/mlx5_common.c
+++ b/drivers/common/mlx5/mlx5_common.c
@@ -162,7 +162,7 @@ mlx5_translate_port_name(const char *port_name_in,
 	return;
 }
 
-#ifdef RTE_IBVERBS_LINK_DLOPEN
+#ifdef MLX5_GLUE
 
 /**
  * Suffix RTE_EAL_PMD_PATH with "-glue".
@@ -204,34 +204,10 @@ mlx5_glue_path(char *buf, size_t size)
 		" re-configure DPDK");
 	return NULL;
 }
-#endif
 
-/**
- * Initialization routine for run-time dependency on rdma-core.
- */
-RTE_INIT_PRIO(mlx5_glue_init, CLASS)
+static int
+mlx5_glue_dlopen(void)
 {
-	/* Initialize common log type. */
-	mlx5_common_logtype = rte_log_register("pmd.common.mlx5");
-	if (mlx5_common_logtype >= 0)
-		rte_log_set_level(mlx5_common_logtype, RTE_LOG_NOTICE);
-	/*
-	 * RDMAV_HUGEPAGES_SAFE tells ibv_fork_init() we intend to use
-	 * huge pages. Calling ibv_fork_init() during init allows
-	 * applications to use fork() safely for purposes other than
-	 * using this PMD, which is not supported in forked processes.
-	 */
-	setenv("RDMAV_HUGEPAGES_SAFE", "1", 1);
-	/* Match the size of Rx completion entry to the size of a cacheline. */
-	if (RTE_CACHE_LINE_SIZE == 128)
-		setenv("MLX5_CQE_SIZE", "128", 0);
-	/*
-	 * MLX5_DEVICE_FATAL_CLEANUP tells ibv_destroy functions to
-	 * cleanup all the Verbs resources even when the device was removed.
-	 */
-	setenv("MLX5_DEVICE_FATAL_CLEANUP", "1", 1);
-	/* The glue initialization was done earlier by mlx5 common library. */
-#ifdef RTE_IBVERBS_LINK_DLOPEN
 	char glue_path[sizeof(RTE_EAL_PMD_PATH) - 1 + sizeof("-glue")];
 	void *handle = NULL;
 
@@ -303,7 +279,49 @@ RTE_INIT_PRIO(mlx5_glue_init, CLASS)
 		goto glue_error;
 	}
 	mlx5_glue = *sym;
-#endif /* RTE_IBVERBS_LINK_DLOPEN */
+	return 0;
+
+glue_error:
+	if (handle)
+		dlclose(handle);
+	return -1;
+}
+
+#endif
+
+RTE_INIT_PRIO(mlx5_log_init, LOG)
+{
+	mlx5_common_logtype = rte_log_register("pmd.common.mlx5");
+	if (mlx5_common_logtype >= 0)
+		rte_log_set_level(mlx5_common_logtype, RTE_LOG_NOTICE);
+}
+
+/**
+ * Initialization routine for run-time dependency on rdma-core.
+ */
+RTE_INIT_PRIO(mlx5_glue_init, CLASS)
+{
+	/*
+	 * RDMAV_HUGEPAGES_SAFE tells ibv_fork_init() we intend to use
+	 * huge pages. Calling ibv_fork_init() during init allows
+	 * applications to use fork() safely for purposes other than
+	 * using this PMD, which is not supported in forked processes.
+	 */
+	setenv("RDMAV_HUGEPAGES_SAFE", "1", 1);
+	/* Match the size of Rx completion entry to the size of a cacheline. */
+	if (RTE_CACHE_LINE_SIZE == 128)
+		setenv("MLX5_CQE_SIZE", "128", 0);
+	/*
+	 * MLX5_DEVICE_FATAL_CLEANUP tells ibv_destroy functions to
+	 * cleanup all the Verbs resources even when the device was removed.
+	 */
+	setenv("MLX5_DEVICE_FATAL_CLEANUP", "1", 1);
+
+#ifdef MLX5_GLUE
+	if (mlx5_glue_dlopen() != 0)
+		goto glue_error;
+#endif
+
 #ifdef RTE_LIBRTE_MLX5_DEBUG
 	/* Glue structure must not contain any NULL pointers. */
 	{
@@ -321,11 +339,8 @@ RTE_INIT_PRIO(mlx5_glue_init, CLASS)
 	}
 	mlx5_glue->fork_init();
 	return;
+
 glue_error:
-#ifdef RTE_IBVERBS_LINK_DLOPEN
-	if (handle)
-		dlclose(handle);
-#endif
 	DRV_LOG(WARNING, "Cannot initialize MLX5 common due to missing"
 		" run-time dependency on rdma-core libraries (libibverbs,"
 		" libmlx5)");
-- 
2.26.0

[dpdk-dev] [PATCH v3 2/3] common/mlx5: fix build with -fno-common

[Thomas Monjalon]

The variable storages of the same name are merged together
if compiled with -fcommon. This is the default.
This default behaviour allows to declare a variable in a header file
and share the variable in every .o binaries thanks to merge at link-time.

In the case of dlopen linking of the glue library, the pointer mlx5_glue
is referencing the glue functions struct and is set after calling dlopen.

If compiling with -fno-common (default in GCC 10), the variable
must be declared as extern to avoid multiple re-definitions.
In case the glue layer is split in glue library, the variable mlx5_glue
needs to have its own storage for the rest of the PMD.

Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/common/mlx5/mlx5_common.c | 3 +++
 drivers/common/mlx5/mlx5_glue.h   | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/common/mlx5/mlx5_common.c b/drivers/common/mlx5/mlx5_common.c
index d8c01a5d14..42610459f7 100644
--- a/drivers/common/mlx5/mlx5_common.c
+++ b/drivers/common/mlx5/mlx5_common.c
@@ -18,6 +18,9 @@
 
 int mlx5_common_logtype;
 
+#ifdef MLX5_GLUE
+const struct mlx5_glue *mlx5_glue;
+#endif
 
 /**
  * Get PCI information by sysfs device path.
diff --git a/drivers/common/mlx5/mlx5_glue.h b/drivers/common/mlx5/mlx5_glue.h
index 6238b43946..29678623e4 100644
--- a/drivers/common/mlx5/mlx5_glue.h
+++ b/drivers/common/mlx5/mlx5_glue.h
@@ -300,6 +300,6 @@ struct mlx5_glue {
 			 size_t event_resp_len);
 };
 
-const struct mlx5_glue *mlx5_glue;
+extern const struct mlx5_glue *mlx5_glue;
 
 #endif /* MLX5_GLUE_H_ */
-- 
2.26.0

[dpdk-dev] [PATCH v3 3/3] net/mlx4: fix build with -fno-common

[Thomas Monjalon]

The variable storages of the same name are merged together
if compiled with -fcommon. This is the default.
This default behaviour allows to declare a variable in a header file
and share the variable in every .o binaries thanks to merge at link-time.

In the case of dlopen linking of the glue library, the pointer mlx4_glue
is referencing the glue functions struct and is set after calling dlopen.

If compiling with -fno-common (default in GCC 10), the variables
must be declared as extern to avoid multiple re-definitions.
In case the glue layer is split in glue library, the variable mlx4_glue
needs to have its own storage for the rest of the PMD.

Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx4/mlx4.c      | 4 ++++
 drivers/net/mlx4/mlx4_glue.h | 2 +-
 drivers/net/mlx4/mlx4_rxtx.h | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 8e298788af..5d7202720b 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -49,6 +49,10 @@
 #include "mlx4_rxtx.h"
 #include "mlx4_utils.h"
 
+#ifdef MLX4_GLUE
+const struct mlx4_glue *mlx4_glue;
+#endif
+
 static const char *MZ_MLX4_PMD_SHARED_DATA = "mlx4_pmd_shared_data";
 
 /* Shared memory between primary and secondary processes. */
diff --git a/drivers/net/mlx4/mlx4_glue.h b/drivers/net/mlx4/mlx4_glue.h
index 668ca86700..5d9e985495 100644
--- a/drivers/net/mlx4/mlx4_glue.h
+++ b/drivers/net/mlx4/mlx4_glue.h
@@ -84,6 +84,6 @@ struct mlx4_glue {
 				   void *attr);
 };
 
-const struct mlx4_glue *mlx4_glue;
+extern const struct mlx4_glue *mlx4_glue;
 
 #endif /* MLX4_GLUE_H_ */
diff --git a/drivers/net/mlx4/mlx4_rxtx.h b/drivers/net/mlx4/mlx4_rxtx.h
index 8baf33fa94..9de6c59411 100644
--- a/drivers/net/mlx4/mlx4_rxtx.h
+++ b/drivers/net/mlx4/mlx4_rxtx.h
@@ -124,7 +124,7 @@ struct txq {
 
 /* mlx4_rxq.c */
 
-uint8_t mlx4_rss_hash_key_default[MLX4_RSS_HASH_KEY_SIZE];
+extern uint8_t mlx4_rss_hash_key_default[MLX4_RSS_HASH_KEY_SIZE];
 int mlx4_rss_init(struct mlx4_priv *priv);
 void mlx4_rss_deinit(struct mlx4_priv *priv);
 struct mlx4_rss *mlx4_rss_get(struct mlx4_priv *priv, uint64_t fields,
-- 
2.26.0

Re: [dpdk-dev] [PATCH v3 0/3] mlx: fix build with -fno-common (gcc 10)

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Thomas Monjalon
> Sent: Wednesday, April 8, 2020 3:09 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v3 0/3] mlx: fix build with -fno-common (gcc 10)
> 
> In GCC 10, -fno-common will be the default.
> There are 2 ways of solving issues:
>   - add -fcommon
>   - stop allocating variables in .h files
> 
> In this patchset, the variables are declared extern,
> because it is cleaner anyway.
> 
> 
> v2: do not initialize global variables and Cc stable
> v3: fix rebase
> 
> 
> Thomas Monjalon (3):
>   common/mlx5: split glue initialization
>   common/mlx5: fix build with -fno-common
>   net/mlx4: fix build with -fno-common
> 
>  drivers/common/mlx5/mlx5_common.c | 82 +++++++++++++++++++--------
> ----
>  drivers/common/mlx5/mlx5_glue.h   |  2 +-
>  drivers/net/mlx4/mlx4.c           |  4 ++
>  drivers/net/mlx4/mlx4_glue.h      |  2 +-
>  drivers/net/mlx4/mlx4_rxtx.h      |  2 +-
>  5 files changed, 57 insertions(+), 35 deletions(-)
> 
> --
> 2.26.0

Series applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh