Hi Shachar, On Wed, Jan 03, 2018 at 03:00:46PM +0000, Shachar Beiser wrote: <snip/> > > > --- 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 > > <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. 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 <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 ? Yes <snip/> Thanks, -- Nélio Laranjeiro 6WIND
