From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro Subject: Re: [PATCH v4] net/mlx5: load libmlx5 and libibverbs in run-time Date: Thu, 4 Jan 2018 08:36:08 +0100 Message-ID: <20180104073608.bxyv6d5vzb7z4izz@shalom> References: <37757d5cfe32610ef1c7a17d72ea6bc1466023de.1511450393.git.shacharbe@mellanox.com> <20180102140631.t3o3uhdovl7jlasy@laranjeiro-vm.dev.6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: "dev@dpdk.org" , Adrien Mazarguil To: Shachar Beiser Return-path: Received: from mail-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48]) by dpdk.org (Postfix) with ESMTP id 29BE51B00B for ; Thu, 4 Jan 2018 08:35:07 +0100 (CET) Received: by mail-wm0-f48.google.com with SMTP id 64so1674083wme.3 for ; Wed, 03 Jan 2018 23:35:07 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Shachar, On Wed, Jan 03, 2018 at 03:00:46PM +0000, Shachar Beiser wrote: > > > --- 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 . Before updating the documentation you should speak with Thomas or Ferruh, not sure new items are allowed anymore. > > > 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. As you wish. > > > - > > > # Create mlx5_autoconf.h or update it in case it differs from the new one. > > > > > > mlx5_autoconf.h: mlx5_autoconf.h.new > > > > > 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 > > > #include > > > #include > > > #include > > > @@ -39,13 +40,17 @@ > > > #include > > > #include > > > #include > > > - > > > +#include > > > > The empty line should remain. > [S.B] OK. To be sure, the empty line should be between the system include and verbs ones. > > > > > /* 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 > > > +#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 ? Yes Thanks, -- Nélio Laranjeiro 6WIND