I agree with you, I'll work on squashing the two patches together during integration.
Kindest regards, Raslan Darawsheh > -----Original Message----- > From: Matan Azrad <ma...@mellanox.com> > Sent: Thursday, January 30, 2020 10:10 AM > To: Matan Azrad <ma...@mellanox.com>; dev@dpdk.org; Slava Ovsiienko > <viachesl...@mellanox.com> > Cc: Raslan Darawsheh <rasl...@mellanox.com> > Subject: RE: [dpdk-dev] [PATCH v4 03/25] common/mlx5: share the mlx5 glue > reference > > Self-suggestion. > It makes sense to squash this patch to the previous patch since the glue > started to move in the previous patch. > > Raslan, can we do it in integration or we need to send all the series again > for > it? > > From: Matan Azrad > > A new Mellanox vdpa PMD will be added to support vdpa operations by > > Mellanox adapters. > > > > Both, the mlx5 PMD and the vdpa mlx5 PMD should initialize the glue. > > > > The glue initialization should be only one per process, so all the > > mlx5 PMDs using the glue should share the same glue object. > > > > Move the glue initialization to be in common/mlx5 library to be > > initialized by its constructor only once. > > > > Signed-off-by: Matan Azrad <ma...@mellanox.com> > > Acked-by: Viacheslav Ovsiienko <viachesl...@mellanox.com> > > --- > > drivers/common/mlx5/mlx5_common.c | 173 > > +++++++++++++++++++++++++++++++++++++- > > drivers/net/mlx5/Makefile | 9 -- > > drivers/net/mlx5/meson.build | 4 - > > drivers/net/mlx5/mlx5.c | 172 > > +------------------------------------ > > 4 files changed, 173 insertions(+), 185 deletions(-) > > > > diff --git a/drivers/common/mlx5/mlx5_common.c > > b/drivers/common/mlx5/mlx5_common.c > > index 14ebd30..9c88a63 100644 > > --- a/drivers/common/mlx5/mlx5_common.c > > +++ b/drivers/common/mlx5/mlx5_common.c > > @@ -2,16 +2,185 @@ > > * Copyright 2019 Mellanox Technologies, Ltd > > */ > > > > +#include <dlfcn.h> > > +#include <unistd.h> > > +#include <string.h> > > + > > +#include <rte_errno.h> > > + > > #include "mlx5_common.h" > > +#include "mlx5_common_utils.h" > > +#include "mlx5_glue.h" > > > > > > int mlx5_common_logtype; > > > > > > -RTE_INIT(rte_mlx5_common_pmd_init) > > +#ifdef RTE_IBVERBS_LINK_DLOPEN > > + > > +/** > > + * Suffix RTE_EAL_PMD_PATH with "-glue". > > + * > > + * This function performs a sanity check on RTE_EAL_PMD_PATH before > > + * suffixing its last component. > > + * > > + * @param buf[out] > > + * Output buffer, should be large enough otherwise NULL is returned. > > + * @param size > > + * Size of @p out. > > + * > > + * @return > > + * Pointer to @p buf or @p NULL in case suffix cannot be appended. > > + */ > > +static char * > > +mlx5_glue_path(char *buf, size_t size) { > > + static const char *const bad[] = { "/", ".", "..", NULL }; > > + const char *path = RTE_EAL_PMD_PATH; > > + size_t len = strlen(path); > > + size_t off; > > + int i; > > + > > + while (len && path[len - 1] == '/') > > + --len; > > + for (off = len; off && path[off - 1] != '/'; --off) > > + ; > > + for (i = 0; bad[i]; ++i) > > + if (!strncmp(path + off, bad[i], (int)(len - off))) > > + goto error; > > + i = snprintf(buf, size, "%.*s-glue", (int)len, path); > > + if (i == -1 || (size_t)i >= size) > > + goto error; > > + return buf; > > +error: > > + RTE_LOG(ERR, PMD, "unable to append \"-glue\" to last component > > of" > > + " RTE_EAL_PMD_PATH (\"" RTE_EAL_PMD_PATH "\"), > > please" > > + " re-configure DPDK"); > > + return NULL; > > +} > > +#endif > > + > > +/** > > + * Initialization routine for run-time dependency on rdma-core. > > + */ > > +RTE_INIT_PRIO(mlx5_glue_init, CLASS) > > { > > - /* Initialize driver log type. */ > > + 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[] = { > > + /* > > + * A basic security check is necessary before trusting > > + * MLX5_GLUE_PATH, which may override > > RTE_EAL_PMD_PATH. > > + */ > > + (geteuid() == getuid() && getegid() == getgid() ? > > + getenv("MLX5_GLUE_PATH") : NULL), > > + /* > > + * When RTE_EAL_PMD_PATH is set, use its glue-suffixed > > + * variant, otherwise let dlopen() look up libraries on its > > + * own. > > + */ > > + (*RTE_EAL_PMD_PATH ? > > + mlx5_glue_path(glue_path, sizeof(glue_path)) : ""), > > + }; > > + unsigned int i = 0; > > + void **sym; > > + const char *dlmsg; > > + > > + while (!handle && i != RTE_DIM(path)) { > > + const char *end; > > + size_t len; > > + int ret; > > + > > + if (!path[i]) { > > + ++i; > > + continue; > > + } > > + end = strpbrk(path[i], ":;"); > > + if (!end) > > + end = path[i] + strlen(path[i]); > > + len = end - path[i]; > > + ret = 0; > > + do { > > + char name[ret + 1]; > > + > > + ret = snprintf(name, sizeof(name), "%.*s%s" > > MLX5_GLUE, > > + (int)len, path[i], > > + (!len || *(end - 1) == '/') ? "" : "/"); > > + if (ret == -1) > > + break; > > + if (sizeof(name) != (size_t)ret + 1) > > + continue; > > + DRV_LOG(DEBUG, "Looking for rdma-core glue as " > > + "\"%s\"", name); > > + handle = dlopen(name, RTLD_LAZY); > > + break; > > + } while (1); > > + path[i] = end + 1; > > + if (!*end) > > + ++i; > > + } > > + if (!handle) { > > + rte_errno = EINVAL; > > + dlmsg = dlerror(); > > + if (dlmsg) > > + DRV_LOG(WARNING, "Cannot load glue library: %s", > > dlmsg); > > + goto glue_error; > > + } > > + sym = dlsym(handle, "mlx5_glue"); > > + if (!sym || !*sym) { > > + rte_errno = EINVAL; > > + dlmsg = dlerror(); > > + if (dlmsg) > > + DRV_LOG(ERR, "Cannot resolve glue symbol: %s", > > dlmsg); > > + goto glue_error; > > + } > > + mlx5_glue = *sym; > > +#endif /* RTE_IBVERBS_LINK_DLOPEN */ > > +#ifndef NDEBUG > > + /* Glue structure must not contain any NULL pointers. */ > > + { > > + unsigned int i; > > + > > + for (i = 0; i != sizeof(*mlx5_glue) / sizeof(void *); ++i) > > + assert(((const void *const *)mlx5_glue)[i]); > > + } > > +#endif > > + if (strcmp(mlx5_glue->version, MLX5_GLUE_VERSION)) { > > + rte_errno = EINVAL; > > + DRV_LOG(ERR, "rdma-core glue \"%s\" mismatch: \"%s\" is " > > + "required", mlx5_glue->version, > > MLX5_GLUE_VERSION); > > + goto glue_error; > > + } > > + 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)"); > > + mlx5_glue = NULL; > > + return; > > } > > diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile > > index > > a9558ca..dc6b3c8 100644 > > --- a/drivers/net/mlx5/Makefile > > +++ b/drivers/net/mlx5/Makefile > > @@ -6,15 +6,6 @@ include $(RTE_SDK)/mk/rte.vars.mk > > > > # Library name. > > LIB = librte_pmd_mlx5.a > > -LIB_GLUE = $(LIB_GLUE_BASE).$(LIB_GLUE_VERSION) > > -LIB_GLUE_BASE = librte_pmd_mlx5_glue.so -LIB_GLUE_VERSION = > 20.02.0 > > - > > -ifeq ($(CONFIG_RTE_IBVERBS_LINK_DLOPEN),y) > > -CFLAGS += -DMLX5_GLUE='"$(LIB_GLUE)"' > > -CFLAGS += -DMLX5_GLUE_VERSION='"$(LIB_GLUE_VERSION)"' > > -LDLIBS += -ldl > > -endif > > > > # Sources. > > SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5.c diff --git > > a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build index > > f6d0db9..e10ef3a 100644 > > --- a/drivers/net/mlx5/meson.build > > +++ b/drivers/net/mlx5/meson.build > > @@ -8,10 +8,6 @@ if not is_linux > > subdir_done() > > endif > > > > -LIB_GLUE_BASE = 'librte_pmd_mlx5_glue.so' > > -LIB_GLUE_VERSION = '20.02.0' > > -LIB_GLUE = LIB_GLUE_BASE + '.' + LIB_GLUE_VERSION > > - > > allow_experimental_apis = true > > deps += ['hash', 'common_mlx5'] > > sources = files( > > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index > > 7cf357d..8fbe826 100644 > > --- a/drivers/net/mlx5/mlx5.c > > +++ b/drivers/net/mlx5/mlx5.c > > @@ -7,7 +7,6 @@ > > #include <unistd.h> > > #include <string.h> > > #include <assert.h> > > -#include <dlfcn.h> > > #include <stdint.h> > > #include <stdlib.h> > > #include <errno.h> > > @@ -3505,138 +3504,6 @@ struct mlx5_flow_id_pool * > > RTE_PCI_DRV_PROBE_AGAIN, > > }; > > > > -#ifdef RTE_IBVERBS_LINK_DLOPEN > > - > > -/** > > - * Suffix RTE_EAL_PMD_PATH with "-glue". > > - * > > - * This function performs a sanity check on RTE_EAL_PMD_PATH before > > - * suffixing its last component. > > - * > > - * @param buf[out] > > - * Output buffer, should be large enough otherwise NULL is returned. > > - * @param size > > - * Size of @p out. > > - * > > - * @return > > - * Pointer to @p buf or @p NULL in case suffix cannot be appended. > > - */ > > -static char * > > -mlx5_glue_path(char *buf, size_t size) -{ > > - static const char *const bad[] = { "/", ".", "..", NULL }; > > - const char *path = RTE_EAL_PMD_PATH; > > - size_t len = strlen(path); > > - size_t off; > > - int i; > > - > > - while (len && path[len - 1] == '/') > > - --len; > > - for (off = len; off && path[off - 1] != '/'; --off) > > - ; > > - for (i = 0; bad[i]; ++i) > > - if (!strncmp(path + off, bad[i], (int)(len - off))) > > - goto error; > > - i = snprintf(buf, size, "%.*s-glue", (int)len, path); > > - if (i == -1 || (size_t)i >= size) > > - goto error; > > - return buf; > > -error: > > - DRV_LOG(ERR, > > - "unable to append \"-glue\" to last component of" > > - " RTE_EAL_PMD_PATH (\"" RTE_EAL_PMD_PATH "\")," > > - " please re-configure DPDK"); > > - return NULL; > > -} > > - > > -/** > > - * Initialization routine for run-time dependency on rdma-core. > > - */ > > -static int > > -mlx5_glue_init(void) > > -{ > > - char glue_path[sizeof(RTE_EAL_PMD_PATH) - 1 + sizeof("-glue")]; > > - const char *path[] = { > > - /* > > - * A basic security check is necessary before trusting > > - * MLX5_GLUE_PATH, which may override > > RTE_EAL_PMD_PATH. > > - */ > > - (geteuid() == getuid() && getegid() == getgid() ? > > - getenv("MLX5_GLUE_PATH") : NULL), > > - /* > > - * When RTE_EAL_PMD_PATH is set, use its glue-suffixed > > - * variant, otherwise let dlopen() look up libraries on its > > - * own. > > - */ > > - (*RTE_EAL_PMD_PATH ? > > - mlx5_glue_path(glue_path, sizeof(glue_path)) : ""), > > - }; > > - unsigned int i = 0; > > - void *handle = NULL; > > - void **sym; > > - const char *dlmsg; > > - > > - while (!handle && i != RTE_DIM(path)) { > > - const char *end; > > - size_t len; > > - int ret; > > - > > - if (!path[i]) { > > - ++i; > > - continue; > > - } > > - end = strpbrk(path[i], ":;"); > > - if (!end) > > - end = path[i] + strlen(path[i]); > > - len = end - path[i]; > > - ret = 0; > > - do { > > - char name[ret + 1]; > > - > > - ret = snprintf(name, sizeof(name), "%.*s%s" > > MLX5_GLUE, > > - (int)len, path[i], > > - (!len || *(end - 1) == '/') ? "" : "/"); > > - if (ret == -1) > > - break; > > - if (sizeof(name) != (size_t)ret + 1) > > - continue; > > - DRV_LOG(DEBUG, "looking for rdma-core glue as > > \"%s\"", > > - name); > > - handle = dlopen(name, RTLD_LAZY); > > - break; > > - } while (1); > > - path[i] = end + 1; > > - if (!*end) > > - ++i; > > - } > > - if (!handle) { > > - rte_errno = EINVAL; > > - dlmsg = dlerror(); > > - if (dlmsg) > > - DRV_LOG(WARNING, "cannot load glue library: %s", > > dlmsg); > > - goto glue_error; > > - } > > - sym = dlsym(handle, "mlx5_glue"); > > - if (!sym || !*sym) { > > - rte_errno = EINVAL; > > - dlmsg = dlerror(); > > - if (dlmsg) > > - DRV_LOG(ERR, "cannot resolve glue symbol: %s", > > dlmsg); > > - goto glue_error; > > - } > > - mlx5_glue = *sym; > > - return 0; > > -glue_error: > > - if (handle) > > - dlclose(handle); > > - DRV_LOG(WARNING, > > - "cannot initialize PMD due to missing run-time dependency > > on" > > - " rdma-core libraries (libibverbs, libmlx5)"); > > - return -rte_errno; > > -} > > - > > -#endif > > - > > /** > > * Driver initialization routine. > > */ > > @@ -3651,43 +3518,8 @@ struct mlx5_flow_id_pool * > > mlx5_set_ptype_table(); > > mlx5_set_cksum_table(); > > mlx5_set_swp_types_table(); > > - /* > > - * 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 RTE_IBVERBS_LINK_DLOPEN > > - if (mlx5_glue_init()) > > - return; > > - assert(mlx5_glue); > > -#endif > > -#ifndef NDEBUG > > - /* Glue structure must not contain any NULL pointers. */ > > - { > > - unsigned int i; > > - > > - for (i = 0; i != sizeof(*mlx5_glue) / sizeof(void *); ++i) > > - assert(((const void *const *)mlx5_glue)[i]); > > - } > > -#endif > > - if (strcmp(mlx5_glue->version, MLX5_GLUE_VERSION)) { > > - DRV_LOG(ERR, > > - "rdma-core glue \"%s\" mismatch: \"%s\" is > > required", > > - mlx5_glue->version, MLX5_GLUE_VERSION); > > - return; > > - } > > - mlx5_glue->fork_init(); > > - rte_pci_register(&mlx5_driver); > > + if (mlx5_glue) > > + rte_pci_register(&mlx5_driver); > > } > > > > RTE_PMD_EXPORT_NAME(net_mlx5, __COUNTER__); > > -- > > 1.8.3.1