Re: [dpdk-dev] [PATCH v4] net/mlx5: load libmlx5 and libibverbs in run-time

[Nelio Laranjeiro <nelio.laranjeiro@6wind.com>]

Hi Shachar,

Please see small comment bellow,

On Sun, Dec 31, 2017 at 07:52:51AM +0000, Shachar Beiser wrote:
> MLX5 PMD loads libraries: libibverbs and libmlx5.
> MLX5 PMD is not linked to external libraries.
> 
> Signed-off-by: Shachar Beiser <shacharbe@mellanox.com>
> ---
> v1: 
>     load external libraries in run-time
> v2:
>     * fix checkpatch warnings 
> v3:
>     * fix checkpatch warnings
> v4:
>     New  MACROs in order to reuse code  
> ---
>  config/common_base               |   1 +
>  drivers/net/mlx5/Makefile        |  22 ++-
>  drivers/net/mlx5/lib/mlx5_dll.c  | 294 +++++++++++++++++++++++++++++++++++++++
>  drivers/net/mlx5/lib/mlx5_dll.h  | 103 ++++++++++++++
>  drivers/net/mlx5/mlx5.c          |  17 ++-
>  drivers/net/mlx5/mlx5.h          |   4 +
>  drivers/net/mlx5/mlx5_flow.c     |   4 +
>  drivers/net/mlx5/mlx5_mac.c      |   4 +
>  drivers/net/mlx5/mlx5_mr.c       |   4 +
>  drivers/net/mlx5/mlx5_rss.c      |   4 +
>  drivers/net/mlx5/mlx5_rxmode.c   |   4 +
>  drivers/net/mlx5/mlx5_rxq.c      |   4 +
>  drivers/net/mlx5/mlx5_rxtx.c     |   4 +
>  drivers/net/mlx5/mlx5_rxtx.h     |   6 +-
>  drivers/net/mlx5/mlx5_rxtx_vec.c |   4 +
>  drivers/net/mlx5/mlx5_txq.c      |   4 +
>  mk/rte.app.mk                    |   8 +-
>  17 files changed, 479 insertions(+), 12 deletions(-)
>  create mode 100644 drivers/net/mlx5/lib/mlx5_dll.c
>  create mode 100644 drivers/net/mlx5/lib/mlx5_dll.h
> 
> diff --git a/config/common_base b/config/common_base
> index b8ee8f9..30c8fcf 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -236,6 +236,7 @@ CONFIG_RTE_LIBRTE_MLX4_TX_MP_CACHE=8
>  # Compile burst-oriented Mellanox ConnectX-4 & ConnectX-5 (MLX5) PMD
>  #
>  CONFIG_RTE_LIBRTE_MLX5_PMD=n
> +CONFIG_RTE_LIBRTE_MLX5_DLL=y
>  CONFIG_RTE_LIBRTE_MLX5_DEBUG=n
>  CONFIG_RTE_LIBRTE_MLX5_TX_MP_CACHE=8

Not sure a new configuration item is allowed.  If it is, the
documentation of such variable is missing.

> diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
> index a3984eb..24fa127 100644
> --- a/drivers/net/mlx5/Makefile
> +++ b/drivers/net/mlx5/Makefile
> @@ -53,18 +53,25 @@ SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_rss.c
>  SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_mr.c
>  SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_flow.c
>  SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_socket.c
> -
> +ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLL),y)
> +SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += lib/mlx5_dll.c
> +endif
>  # Basic CFLAGS.
>  CFLAGS += -O3
>  CFLAGS += -std=c11 -Wall -Wextra
>  CFLAGS += -g
>  CFLAGS += -I.
> +CFLAGS += -I$(SRCDIR)
>  CFLAGS += -D_BSD_SOURCE
>  CFLAGS += -D_DEFAULT_SOURCE
>  CFLAGS += -D_XOPEN_SOURCE=600
>  CFLAGS += $(WERROR_FLAGS)
>  CFLAGS += -Wno-strict-prototypes
> +ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLL),y)
> +LDLIBS += -ldl
> +else
>  LDLIBS += -libverbs -lmlx5
> +endif
>  LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
>  LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs
>  LDLIBS += -lrte_bus_pci
> @@ -105,26 +112,28 @@ endif
>  
>  mlx5_autoconf.h.new: FORCE
>  
> +VERBS_H := infiniband/verbs.h
> +MLX5DV_H := infiniband/mlx5dv.h
>  mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
>  	$Q $(RM) -f -- '$@'
>  	$Q sh -- '$<' '$@' \
>  		HAVE_IBV_DEVICE_VXLAN_SUPPORT \
> -		infiniband/verbs.h \
> +		$(VERBS_H) \
>  		enum IBV_DEVICE_VXLAN_SUPPORT \
>  		$(AUTOCONF_OUTPUT)
>  	$Q sh -- '$<' '$@' \
>  		HAVE_IBV_WQ_FLAG_RX_END_PADDING \
> -		infiniband/verbs.h \
> +		$(VERBS_H) \
>  		enum IBV_WQ_FLAG_RX_END_PADDING \
>  		$(AUTOCONF_OUTPUT)
>  	$Q sh -- '$<' '$@' \
>  		HAVE_IBV_MLX5_MOD_MPW \
> -		infiniband/mlx5dv.h \
> +		$(MLX5DV_H) \
>  		enum MLX5DV_CONTEXT_FLAGS_MPW_ALLOWED \
>  		$(AUTOCONF_OUTPUT)
>  	$Q sh -- '$<' '$@' \
>  		HAVE_IBV_MLX5_MOD_CQE_128B_COMP \
> -		infiniband/mlx5dv.h \
> +		$(MLX5DV_H) \
>  		enum MLX5DV_CONTEXT_FLAGS_CQE_128B_COMP \
>  		$(AUTOCONF_OUTPUT)
>  	$Q sh -- '$<' '$@' \
> @@ -144,10 +153,9 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
>  		$(AUTOCONF_OUTPUT)
>  	$Q sh -- '$<' '$@' \
>  		HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT \
> -		infiniband/verbs.h \
> +		$(VERBS_H)  \
>  		enum IBV_FLOW_SPEC_ACTION_COUNT \
>  		$(AUTOCONF_OUTPUT)

This modification should be inside its own patch, it is not directly
related to the this patch itself.

> -
>  # Create mlx5_autoconf.h or update it in case it differs from the new one.
>  
>  mlx5_autoconf.h: mlx5_autoconf.h.new
<snip/>
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index cd66fe1..eeef782 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -30,7 +30,8 @@
>   *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
> -
> +#define _GNU_SOURCE
> +#include <stdio.h>
>  #include <stddef.h>
>  #include <unistd.h>
>  #include <string.h>
> @@ -39,13 +40,17 @@
>  #include <stdlib.h>
>  #include <errno.h>
>  #include <net/if.h>
> -
> +#include <dlfcn.h>

The empty line should remain.

>  /* Verbs header. */
>  /* ISO C doesn't support unnamed structs/unions, disabling -pedantic. */
>  #ifdef PEDANTIC
>  #pragma GCC diagnostic ignored "-Wpedantic"
>  #endif
> +#ifdef RTE_LIBRTE_MLX5_DLL
> +#include "lib/mlx5_dll.h"
> +#else
>  #include <infiniband/verbs.h>
> +#endif

This could be done by the mlx5_dll.h file which could include the
correct header according to the configuration.

<snip/>
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 6a6a745..2dd2f6b 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -51,7 +51,9 @@ endif
>  
>  # Link only the libraries used in the application
>  LDFLAGS += --as-needed
> -
> +ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLL),y)
> +LDFLAGS += -ldl
> +endif
>  # default path for libs
>  _LDLIBS-y += -L$(RTE_SDK_BIN)/lib
>  
> @@ -142,7 +144,11 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_KNI)        += -lrte_pmd_kni
>  endif
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_LIO_PMD)        += -lrte_pmd_lio
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)       += -lrte_pmd_mlx4 -libverbs -lmlx4
> +ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLL),y)
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl
> +else
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -libverbs -lmlx5
> +endif
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_MRVL_PMD)       += -lrte_pmd_mrvl -L$(LIBMUSDK_PATH)/lib -lmusdk
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_NFP_PMD)        += -lrte_pmd_nfp
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL)       += -lrte_pmd_null
> -- 
> 1.8.3.1
> 

Thanks,

-- 
Nélio Laranjeiro
6WIND