From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 9063F1B2E6 for ; Fri, 19 Jan 2018 20:07:25 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9B32082A; Fri, 19 Jan 2018 19:07:24 +0000 (UTC) Received: from localhost.localdomain (ovpn-116-5.gru2.redhat.com [10.97.116.5]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7F4B060BE7; Fri, 19 Jan 2018 19:07:23 +0000 (UTC) Date: Fri, 19 Jan 2018 17:07:22 -0200 From: Marcelo Ricardo Leitner To: =?iso-8859-1?Q?N=E9lio?= Laranjeiro Cc: Shachar Beiser , "dev@dpdk.org" , Adrien Mazarguil Message-ID: <20180119190722.GB6615@localhost.localdomain> References: <37757d5cfe32610ef1c7a17d72ea6bc1466023de.1511450393.git.shacharbe@mellanox.com> <20180102140631.t3o3uhdovl7jlasy@laranjeiro-vm.dev.6wind.com> <20180104073608.bxyv6d5vzb7z4izz@shalom> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180104073608.bxyv6d5vzb7z4izz@shalom> User-Agent: Mutt/1.9.1 (2017-09-22) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Fri, 19 Jan 2018 19:07:24 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v4] net/mlx5: load libmlx5 and libibverbs in run-time X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Jan 2018 19:07:26 -0000 Hello, On Thu, Jan 04, 2018 at 08:36:08AM +0100, Nélio Laranjeiro wrote: > 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 Actually, why this change? It shouldn't be necessary. As the patch is defining functions with the very same prototype, it shouldn't matter if their declarations are coming from one header or another. Marcelo