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

[Shachar Beiser]

> -----Original Message-----
> From: Nelio Laranjeiro [mailto:nelio.laranjeiro at 6wind.com]
> Sent: Tuesday, January 2, 2018 4:07 PM
> To: Shachar Beiser <shacharbe at mellanox.com>
> Cc: dev at dpdk.org; Adrien Mazarguil <adrien.mazarguil at 6wind.com>
> Subject: Re: [PATCH v4] net/mlx5: load libmlx5 and libibverbs in run-time
> 
> 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 at 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.

[S.B] The patch is based on this CONFIG_RTE_LIBRTE_MLX5_DLL , it was required by Adrian in the design phase to enable/disable this linkage mode.
          I will update the documentation .
> 
> > 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.
[S.B] I can revert the VERBS_H ,  MLX5DV_H  and not change it all . 
           There is no need to change in a different patch. 

> 
> > -
> >  # 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.
[S.B] OK.
> 
> >  /* 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.
[S.B] I guess you refer to all *.c files that includes the verbs.h .
           I will change them all ? 
> 
> <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