All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
To: Shachar Beiser <shacharbe@mellanox.com>
Cc: dev@dpdk.org, Adrien Mazarguil <adrien.mazarguil@6wind.com>
Subject: Re: [PATCH v4] net/mlx5: load libmlx5 and libibverbs in run-time
Date: Tue, 2 Jan 2018 15:06:31 +0100	[thread overview]
Message-ID: <20180102140631.t3o3uhdovl7jlasy@laranjeiro-vm.dev.6wind.com> (raw)
In-Reply-To: <d17b90a4d765004f878e846b1bde1da6e954306f.1514706323.git.shacharbe@mellanox.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

  reply	other threads:[~2018-01-02 14:07 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-23 12:18 [PATCH v1 1/2] net/mlx5: load libmlx5 and libibverbs in run-time Shachar Beiser
2017-11-23 12:18 ` [PATCH v1 2/2] net/mlx4: load libmlx4 " Shachar Beiser
2017-11-23 13:13   ` [PATCH v2 1/2] net/mlx5: load libmlx5 " Shachar Beiser
2017-11-23 13:13   ` [PATCH v2 2/2] net/mlx4: load libmlx4 " Shachar Beiser
2017-11-23 15:24     ` [PATCH v3 1/2] net/mlx5: load libmlx5 " Shachar Beiser
2017-12-31  7:52       ` [PATCH v4] " Shachar Beiser
2018-01-02 14:06         ` Nelio Laranjeiro [this message]
2018-01-03 15:00           ` Shachar Beiser
2018-01-04  7:36             ` Nélio Laranjeiro
2018-01-04 17:30               ` Thomas Monjalon
2018-01-19 19:07               ` Marcelo Ricardo Leitner
2018-01-04 17:28         ` Thomas Monjalon
2018-01-19 18:48         ` Marcelo Ricardo Leitner
2017-11-23 15:24     ` [PATCH v3 2/2] net/mlx4: load libmlx4 " Shachar Beiser

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180102140631.t3o3uhdovl7jlasy@laranjeiro-vm.dev.6wind.com \
    --to=nelio.laranjeiro@6wind.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=shacharbe@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.