[dpdk-dev] [PATCH v9 0/7] export PMD infos
This is a respin of the series from Neil. It was planned to be integrated in 16.07-rc1 but the discovered issues make a new revision needed. There are still few things which could be improved but it is not mandatory to fix them for an integration in 16.07-rc2: - fix make clean after pmdinfogen - build installable pmdinfogen for target - convert pmdinfo.py to Python 3 - document dependency pyelftools Changes done in this v9: - fix build dependency of drivers on pmdinfogen - fix build of mlx4, mlx5, aesni - fix new drivers bnxt, thunderx, kasumi - fix MAINTAINERS file - fix coding style in pmdinfogen - add compiler checks for pmdinfogen - remove useless functions in pmdinfogen - fail build if pmdinfogen fails (set -e) - fix verbose pmdinfogen run - build pmdinfogen in buildtools directory (was app) - install pmdinfogen in sdk package (was runtime) - fix CamelCase in pmdinfo.py - prefix executables with dpdk- - rename PMD_REGISTER_DRIVER -> RTE_REGISTER_DRIVER - separate commit for hostapp.mk refresh - remove useless hostlib.mk - spread doc in appropriate patches Please review carefully. Older changes: v8) * Modified symlink for pmdinfo to use dpdk_ * Added dpdk_pmdinfogen for pmdinfogen binary v7) * Fixed up copyright * Switched buildtool makefile to use hostapp.mk * Modified arch check to use RTE_ARCH_64 * Modifed hostapp.mk to drop output in build/app * Additional testing on ppc64 to ensure big endian works * Modified TO_NATIVE macro to use rte_byteorder inlines based on endianess of target ELF file * Ran checkpatch on commits * Fixed some typos in doc v6) * Added some programming guide documentation * Reformatted python script with pep8 v5) * Added a dpdk- prefix to pmdinfo symlink * Renamed build announcement to PMDINFOGEN/BUILD * Removed erroneous true statement from makefile * Removed duplicate rte.hostapp.mk makefile * Fixed some whitespace * Whitespace fixups * Fixed makefile if; then style * Renamed module string C file * Removed duplicate rte_pci_id definition * Clarified macro names * Removed PMD type attribute * Fixed tools usage for 32 bit arches * Removed some unused code * Added a few comments v4) * Modified the operation of the -p option. As much as I don't like implying that autoloaded pmds are guaranteed to be there at run time, I'm having a hard time seeing how we can avoid specifying the application file to scan for the autoload directory. Without it we can't determine which library the user means in a multiversion installation * Cleaned up the help text * Added a rule for an install target for pmdinfo * Guarded against some tracebacks in pmdinfo * Use DT_NEEDED entries to get versioned libraries in -p mode * Fixed traceback that occurs on lack of input arguments * Fixed some erroneous macro usage in drivers that aren't in the default build v3) * Made table mode printing the default mode * Added a default path to the pci.ids file * Modifed pmdinfo to use python rather than python3 * Cleaned up some changelog entries * Added an export for RTE_EAL_PMD_PATH * Added a plugin option to pmdinfo to scan for autoloaded DSO's v2) * Made the export macros a bit easier to expand * Added a macro to optionally export command line strings * Renamed utilties to be more appropriate (pmdinfo -> pmdinfogen, pmd_hw_support.py -> pmdinfo.py) * Added search capabilities to pmdinfo.py so that we search for libraries linked using DT_NEEDINFO entries. We search DT_RUNPATH, LD_LIBRARY_PATH, /usr/lib and /lib * Added serch abilities if the specified binary to pmdinfo isn't found, we search LD_LIBRARY_PATH, /usr/lib and /lib * Added an option to pmdinfo.py to pretty-print hardware support info using the pci.ids database Original summary So heres attempt number 2 at a method for exporting PMD hardware support information. As we discussed previously, the consensus seems to be that pmd information should be: 1) Able to be interrogated on any ELF binary (application binary or individual DSO) 2) Equally functional on statically linked applications or on DSO's 3) Resilient to symbol stripping 4) Script friendly 5) Show kernel dependencies 6) List driver options 7) Show driver name 8) Offer human readable output 9) Show DPDK version 10) Show driver version 11) Allow for expansion 12) Not place additional build environment dependencies on an application I added item 12 myself, because I don't think its reasonable to require applications to use a specific linker script to get hardware support information (which precludes the use of a special .modinfo section like the kernel uses) However, we still can use some tricks from the kernel to make this work. In this approach, what I've done is the following: A) Modified the driver registration macro to also define a variable: this_pmd_name= "name" Based on the unique name string pointed to by the above variable, we
[dpdk-dev] [PATCH v9 1/7] drivers: export infos as string symbols
From: Neil Horman Modify the PMD_REGISTER_DRIVER macro, adding a name argument to it. The addition of a name argument creates a token that can be used for subsequent macros in the creation of unique symbol names to export additional bits of information for use by the dpdk-pmdinfogen tool. For example: RTE_REGISTER_DRIVER(ena, ena_driver); registers the ena_driver struct as PMD_REGISTER_DRIVER did, and creates a symbol const char rte_pmd_name0[] __attribute__((used)) = "ena"; which dpdk-pmdinfogen can search for and extract. The subsequent macro RTE_EXPORT_PCI_TABLE(ena, ena_pci_id_map); creates a symbol const char ena_pci_table_export[] __attribute__((used)) = "ena_pci_id_map"; Which allows dpdk-pmdinfogen to find the pci table of this driver. Using this pattern, we can export arbitrary bits of information. dpdk-pmdinfo uses this information to extract hardware support from an object file and create a json string to make hardware support info discoverable later. Signed-off-by: Neil Horman Acked-by: Panu Matilainen Acked-by: Remy Horton Signed-off-by: Thomas Monjalon --- doc/guides/prog_guide/dev_kit_build_system.rst | 19 drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 4 +++- drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 4 +++- drivers/crypto/kasumi/rte_kasumi_pmd.c | 4 +++- drivers/crypto/null/null_crypto_pmd.c | 4 +++- drivers/crypto/qat/rte_qat_cryptodev.c | 3 ++- drivers/crypto/snow3g/rte_snow3g_pmd.c | 4 +++- drivers/net/af_packet/rte_eth_af_packet.c | 4 +++- drivers/net/bnx2x/bnx2x_ethdev.c | 6 -- drivers/net/bnxt/bnxt_ethdev.c | 3 ++- drivers/net/bonding/rte_eth_bond_pmd.c | 5 - drivers/net/cxgbe/cxgbe_ethdev.c | 3 ++- drivers/net/e1000/em_ethdev.c | 3 ++- drivers/net/e1000/igb_ethdev.c | 6 -- drivers/net/ena/ena_ethdev.c | 3 ++- drivers/net/enic/enic_ethdev.c | 3 ++- drivers/net/fm10k/fm10k_ethdev.c | 3 ++- drivers/net/i40e/i40e_ethdev.c | 3 ++- drivers/net/i40e/i40e_ethdev_vf.c | 3 ++- drivers/net/ixgbe/ixgbe_ethdev.c | 6 -- drivers/net/mlx4/mlx4.c| 3 ++- drivers/net/mlx5/mlx5.c| 3 ++- drivers/net/mpipe/mpipe_tilegx.c | 4 ++-- drivers/net/nfp/nfp_net.c | 3 ++- drivers/net/null/rte_eth_null.c| 3 ++- drivers/net/pcap/rte_eth_pcap.c| 4 +++- drivers/net/qede/qede_ethdev.c | 6 -- drivers/net/ring/rte_eth_ring.c| 3 ++- drivers/net/szedata2/rte_eth_szedata2.c| 3 ++- drivers/net/thunderx/nicvf_ethdev.c| 3 ++- drivers/net/vhost/rte_eth_vhost.c | 3 ++- drivers/net/virtio/virtio_ethdev.c | 3 ++- drivers/net/virtio/virtio_user_ethdev.c| 2 +- drivers/net/vmxnet3/vmxnet3_ethdev.c | 3 ++- drivers/net/xenvirt/rte_eth_xenvirt.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 2 +- lib/librte_eal/common/include/rte_dev.h| 30 -- 37 files changed, 127 insertions(+), 46 deletions(-) diff --git a/doc/guides/prog_guide/dev_kit_build_system.rst b/doc/guides/prog_guide/dev_kit_build_system.rst index 3e89eae..f3a4bff 100644 --- a/doc/guides/prog_guide/dev_kit_build_system.rst +++ b/doc/guides/prog_guide/dev_kit_build_system.rst @@ -304,6 +304,25 @@ Misc * rte.subdir.mk: Build several directories in the development kit framework. +.. _Internally_Generated_Build_Tools: + +Internally Generated Build Tools + + +These well known symbol names are defined by various macros and used to export +important information about hardware support and usage for PMD files. For +instance the macro: + +.. code-block:: c + + RTE_REGISTER_DRIVER(drv, name) + +Creates the following symbol: + +.. code-block:: c + + static char rte_pmd_name0[] __attribute__((used)) = ""; + .. _Useful_Variables_Provided_by_the_Build_System: Useful Variables Provided by the Build System diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c index 2987ef6..3e13eae 100644 --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c @@ -521,4 +521,6 @@ static struct rte_driver aesni_gcm_pmd_drv = { .uninit = aesni_gcm_uninit }; -PMD_REGISTER_DRIVER(aesni_gcm_pmd_drv); +RTE_REGISTER_DRIVER(aesni_gcm, aesni_gcm_pmd_drv); +RTE_EXPORT_PARAM_STRING(aesni_gcm, "max_nb_queue_pairs= " +"max_nb_sessions= socket_id="); diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c index 6554fc4..2afc2bb 100644 --- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c +++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c @@
[dpdk-dev] [PATCH v9 2/7] mk: remove recipe for tool library
It is difficult to imagine why it could be needed to build a library for the build environment. If building a tool, rte.hostapp.mk should be sufficient. Signed-off-by: Thomas Monjalon --- doc/guides/prog_guide/dev_kit_build_system.rst | 2 - mk/rte.hostlib.mk | 116 - mk/rte.sdkbuild.mk | 2 +- mk/rte.sdkconfig.mk| 2 +- 4 files changed, 2 insertions(+), 120 deletions(-) delete mode 100644 mk/rte.hostlib.mk diff --git a/doc/guides/prog_guide/dev_kit_build_system.rst b/doc/guides/prog_guide/dev_kit_build_system.rst index f3a4bff..9b0de83 100644 --- a/doc/guides/prog_guide/dev_kit_build_system.rst +++ b/doc/guides/prog_guide/dev_kit_build_system.rst @@ -275,8 +275,6 @@ Generate a .a library. * rte.extlib.mk: external library -* rte.hostlib.mk: host library in the development kit framework - Install ^^^ diff --git a/mk/rte.hostlib.mk b/mk/rte.hostlib.mk deleted file mode 100644 index fe24049..000 --- a/mk/rte.hostlib.mk +++ /dev/null @@ -1,116 +0,0 @@ -# BSD LICENSE -# -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. -# All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions -# are met: -# -# * Redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer. -# * Redistributions in binary form must reproduce the above copyright -# notice, this list of conditions and the following disclaimer in -# the documentation and/or other materials provided with the -# distribution. -# * Neither the name of Intel Corporation nor the names of its -# contributors may be used to endorse or promote products derived -# from this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -# tell rte.compile-pre.mk to use HOSTCC instead of CC -USE_HOST := 1 -include $(RTE_SDK)/mk/internal/rte.compile-pre.mk -include $(RTE_SDK)/mk/internal/rte.install-pre.mk -include $(RTE_SDK)/mk/internal/rte.clean-pre.mk -include $(RTE_SDK)/mk/internal/rte.build-pre.mk -include $(RTE_SDK)/mk/internal/rte.depdirs-pre.mk - -# VPATH contains at least SRCDIR -VPATH += $(SRCDIR) - -_BUILD = $(HOSTLIB) -_INSTALL = $(INSTALL-FILES-y) $(SYMLINK-FILES-y) $(RTE_OUTPUT)/hostlib/$(HOSTLIB) -_CLEAN = doclean - -.PHONY: all -all: install - -.PHONY: install -install: build _postinstall - -_postinstall: build - -.PHONY: build -build: _postbuild - -exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1 - -O_TO_A = $(AR) crus $(HOSTLIB) $(OBJS-y) -O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight -O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)"," HOSTAR $(@)") -O_TO_A_CMD = "cmd_$@ = $(O_TO_A_STR)" -O_TO_A_DO = @set -e; \ - echo $(O_TO_A_DISP); \ - $(O_TO_A) && \ - echo $(O_TO_A_CMD) > $(call exe2cmd,$(@)) - --include .$(HOSTLIB).cmd - -# -# Archive objects in .a file if needed -# -$(HOSTLIB): $(OBJS-y) FORCE - @[ -d $(dir $@) ] || mkdir -p $(dir $@) - $(if $(D),\ - @echo -n "$@ -> $< " ; \ - echo -n "file_missing=$(call boolean,$(file_missing)) " ; \ - echo -n "cmdline_changed=$(call boolean,$(call cmdline_changed,$(O_TO_A_STR))) " ; \ - echo -n "depfile_missing=$(call boolean,$(depfile_missing)) " ; \ - echo "depfile_newer=$(call boolean,$(depfile_newer)) ") - $(if $(or \ - $(file_missing),\ - $(call cmdline_changed,$(O_TO_A_STR)),\ - $(depfile_missing),\ - $(depfile_newer)),\ - $(O_TO_A_DO)) - -# -# install lib in $(RTE_OUTPUT)/hostlib -# -$(RTE_OUTPUT)/hostlib/$(HOSTLIB): $(HOSTLIB) - @echo " INSTALL-HOSTLIB $(HOSTLIB)" - @[ -d $(RTE_OUTPUT)/hostlib ] || mkdir -p $(RTE_OUTPUT)/hostlib - $(Q)cp -f $(HOSTLIB) $(RTE_OUTPUT)/hostlib - -# -# Clean all generated files -# -.PHONY: clean -clean: _postclean - -.PHONY: doclean -doclean: - $(Q)rm -rf $(HOSTLIB) $(OBJS-all) $(DEPS-all)
[dpdk-dev] [PATCH v9 3/7] mk: refresh recipe for any host application
Make the recipe simpler to use and build in configurable directory. HOSTAPP_DIR must be set before including rte.hostapp.mk. Remove LDLIBS_FILES as libraries should not be used in an hostapp. Remove the "INSTALL-HOSTAPP" and build directly in the right directory. Signed-off-by: Thomas Monjalon --- doc/guides/freebsd_gsg/build_dpdk.rst | 2 +- doc/guides/linux_gsg/build_dpdk.rst| 2 +- doc/guides/prog_guide/dev_kit_build_system.rst | 2 +- mk/rte.hostapp.mk | 23 +-- mk/rte.sdkbuild.mk | 3 +-- mk/rte.sdkconfig.mk| 3 +-- 6 files changed, 10 insertions(+), 25 deletions(-) diff --git a/doc/guides/freebsd_gsg/build_dpdk.rst b/doc/guides/freebsd_gsg/build_dpdk.rst index 1d92c08..93c4366 100644 --- a/doc/guides/freebsd_gsg/build_dpdk.rst +++ b/doc/guides/freebsd_gsg/build_dpdk.rst @@ -183,7 +183,7 @@ contains the kernel modules to install: ls x86_64-native-bsdapp-gcc -app build hostapp include kmod lib Makefile +app build include kmod lib Makefile .. _loading_contigmem: diff --git a/doc/guides/linux_gsg/build_dpdk.rst b/doc/guides/linux_gsg/build_dpdk.rst index 198c0b6..fb2c481 100644 --- a/doc/guides/linux_gsg/build_dpdk.rst +++ b/doc/guides/linux_gsg/build_dpdk.rst @@ -152,7 +152,7 @@ A kmod directory is also present that contains kernel modules which may be load ls x86_64-native-linuxapp-gcc -app build hostapp include kmod lib Makefile +app build include kmod lib Makefile Loading Modules to Enable Userspace IO for DPDK --- diff --git a/doc/guides/prog_guide/dev_kit_build_system.rst b/doc/guides/prog_guide/dev_kit_build_system.rst index 9b0de83..dedd18a 100644 --- a/doc/guides/prog_guide/dev_kit_build_system.rst +++ b/doc/guides/prog_guide/dev_kit_build_system.rst @@ -264,7 +264,7 @@ These Makefiles generate a binary application. * rte.extapp.mk: External application -* rte.hostapp.mk: Host application in the development kit framework +* rte.hostapp.mk: Prerequisite tool to build DPDK Library ^^^ diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk index c44d0f8..05bbd26 100644 --- a/mk/rte.hostapp.mk +++ b/mk/rte.hostapp.mk @@ -40,8 +40,8 @@ include $(RTE_SDK)/mk/internal/rte.depdirs-pre.mk # VPATH contains at least SRCDIR VPATH += $(SRCDIR) -_BUILD = $(HOSTAPP) -_INSTALL = $(INSTALL-FILES-y) $(SYMLINK-FILES-y) $(RTE_OUTPUT)/hostapp/$(HOSTAPP) +_BUILD = $(RTE_OUTPUT)/$(HOSTAPP_DIR)/$(HOSTAPP) +_INSTALL = $(INSTALL-FILES-y) $(SYMLINK-FILES-y) $(RTE_OUTPUT)/$(HOSTAPP_DIR)/$(HOSTAPP) _CLEAN = doclean .PHONY: all @@ -60,7 +60,7 @@ exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1 O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \ $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS) O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight -O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," HOSTLD $(@)") +O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," HOSTLD $(@F)") O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)" O_TO_EXE_DO = @set -e; \ echo $(O_TO_EXE_DISP); \ @@ -69,15 +69,10 @@ O_TO_EXE_DO = @set -e; \ -include .$(HOSTAPP).cmd -# list of .a files that are linked to this application -LDLIBS_FILES := $(wildcard \ - $(addprefix $(RTE_OUTPUT)/lib/, \ - $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS) - # # Compile executable file if needed # -$(HOSTAPP): $(OBJS-y) $(LDLIBS_FILES) FORCE +$(RTE_OUTPUT)/$(HOSTAPP_DIR)/$(HOSTAPP): $(OBJS-y) FORCE @[ -d $(dir $@) ] || mkdir -p $(dir $@) $(if $(D),\ @echo -n "$@ -> $< " ; \ @@ -93,14 +88,6 @@ $(HOSTAPP): $(OBJS-y) $(LDLIBS_FILES) FORCE $(O_TO_EXE_DO)) # -# install app in $(RTE_OUTPUT)/hostapp -# -$(RTE_OUTPUT)/hostapp/$(HOSTAPP): $(HOSTAPP) - @echo " INSTALL-HOSTAPP $(HOSTAPP)" - @[ -d $(RTE_OUTPUT)/hostapp ] || mkdir -p $(RTE_OUTPUT)/hostapp - $(Q)cp -f $(HOSTAPP) $(RTE_OUTPUT)/hostapp - -# # Clean all generated files # .PHONY: clean @@ -109,7 +96,7 @@ clean: _postclean .PHONY: doclean doclean: - $(Q)rm -rf $(HOSTAPP) $(OBJS-all) $(DEPS-all) $(DEPSTMP-all) \ + $(Q)rm -rf $(OBJS-all) $(DEPS-all) $(DEPSTMP-all) \ $(CMDS-all) $(INSTALL-FILES-all) .$(HOSTAPP).cmd diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk index 6dbdb5d..f1a163a 100644 --- a/mk/rte.sdkbuild.mk +++ b/mk/rte.sdkbuild.mk @@ -63,8 +63,7 @@ build: $(ROOTDIRS-y) .PHONY: clean clean: $(CLEANDIRS) @rm -rf $(RTE_OUTPUT)/include $(RTE_OUTPUT)/app \ - $(RTE_OUTPUT)/hostapp $(RTE_OUTPUT)/lib \ - $(RTE_OUTPUT)/kmod + $(RTE_OUTPUT)/lib $(RTE_OUTPUT)/kmod @[ -d $(RTE_OUTPUT)/include ] || mkdir -p $(RTE_OUTPUT)/include @$(RTE_SDK)/scripts/gen-config-h.sh $(RTE_OUTPUT)/.config \ > $(RTE_OUTPUT)/include/rte_config.h diff --git a/mk/rte.sdkconfig.mk b
[dpdk-dev] [PATCH v9 4/7] pmdinfogen: parse driver to generate code to export
From: Neil Horman dpdk-pmdinfogen is a tool used to parse object files and build JSON strings for use in later determining hardware support in a DSO or application binary. dpdk-pmdinfogen looks for the non-exported symbol names rte_pmd_name (where n is a integer counter) and _pci_table_export. It records the name of each of these tuples, using the later to find the symbolic name of the PCI table for physical devices that the object supports. With this information, it outputs a C file with a single line of the form: static char *_pmd_info[] __attribute__((used)) = " \ PMD_INFO_STRING="; Where is the arbitrary name of the PMD, and is the JSON encoded string that hold relevant PMD information, including the PMD name, type and optional array of PCI device/vendor IDs that the driver supports. This C file is suitable for compiling to object code, then relocatably linking into the parent file from which the C was generated. This creates an entry in the string table of the object that can inform a later tool about hardware support. Note 1: When installed as part of a SDK package, dpdk-pmdinfogen should be built for the SDK target. It is not handled currently. Note 2: Some generated files are not cleaned by "make clean". Signed-off-by: Neil Horman Acked-by: Panu Matilainen Acked-by: Remy Horton Signed-off-by: Thomas Monjalon --- GNUmakefile| 2 +- MAINTAINERS| 4 + GNUmakefile => buildtools/Makefile | 17 +- GNUmakefile => buildtools/pmdinfogen/Makefile | 21 +- buildtools/pmdinfogen/pmdinfogen.c | 451 + buildtools/pmdinfogen/pmdinfogen.h | 99 ++ doc/guides/prog_guide/dev_kit_build_system.rst | 15 +- mk/rte.sdkbuild.mk | 2 +- mk/rte.sdkinstall.mk | 3 + 9 files changed, 587 insertions(+), 27 deletions(-) copy GNUmakefile => buildtools/Makefile (87%) copy GNUmakefile => buildtools/pmdinfogen/Makefile (84%) create mode 100644 buildtools/pmdinfogen/pmdinfogen.c create mode 100644 buildtools/pmdinfogen/pmdinfogen.h diff --git a/GNUmakefile b/GNUmakefile index b59e4b6..00fe0db 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -40,6 +40,6 @@ export RTE_SDK # directory list # -ROOTDIRS-y := lib drivers app +ROOTDIRS-y := buildtools lib drivers app include $(RTE_SDK)/mk/rte.sdkroot.mk diff --git a/MAINTAINERS b/MAINTAINERS index a59191e..1a8a3b7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -68,6 +68,10 @@ F: lib/librte_compat/ F: doc/guides/rel_notes/deprecation.rst F: scripts/validate-abi.sh +Driver information +M: Neil Horman +F: buildtools/pmdinfogen/ + Environment Abstraction Layer - diff --git a/GNUmakefile b/buildtools/Makefile similarity index 87% copy from GNUmakefile copy to buildtools/Makefile index b59e4b6..35a42ff 100644 --- a/GNUmakefile +++ b/buildtools/Makefile @@ -1,6 +1,6 @@ # BSD LICENSE # -# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. +# Copyright(c) 2016 Neil Horman. All rights reserved. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -29,17 +29,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. -# -# Head Makefile for compiling rte SDK -# - -RTE_SDK := $(CURDIR) -export RTE_SDK - -# -# directory list -# +include $(RTE_SDK)/mk/rte.vars.mk -ROOTDIRS-y := lib drivers app +DIRS-y += pmdinfogen -include $(RTE_SDK)/mk/rte.sdkroot.mk +include $(RTE_SDK)/mk/rte.subdir.mk diff --git a/GNUmakefile b/buildtools/pmdinfogen/Makefile similarity index 84% copy from GNUmakefile copy to buildtools/pmdinfogen/Makefile index b59e4b6..327927e 100644 --- a/GNUmakefile +++ b/buildtools/pmdinfogen/Makefile @@ -1,6 +1,6 @@ # BSD LICENSE # -# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. +# Copyright(c) 2016 Neil Horman. All rights reserved. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -29,17 +29,16 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -# -# Head Makefile for compiling rte SDK -# +include $(RTE_SDK)/mk/rte.vars.mk -RTE_SDK := $(CURDIR) -export RTE_SDK +HOSTAPP_DIR = buildtools +HOSTAPP = dpdk-pmdinfogen -# -# directory list -# +SRCS-y += pmdinfogen.c + +HOST_CFLAGS += $(WERROR_FLAGS) -g +HOST_CFLAGS += -I$(RTE_OUTPUT)/include -ROOTDIRS-y := lib drivers app +DEPDIRS-y += lib/librte_eal -include $(RTE_SDK)/mk/rte.sdkroot.mk +include $(RTE_SDK)/mk/rte.hostapp.mk diff --git a/buildtools/pmdinfogen/pmdinfogen.c b/buildtools/pmdinfogen/pmdinfogen.c new file mode 100644 index 000..101bce1 --- /dev/null +++ b/buildtools/pmdinfogen/pmdinfogen.c @@ -0,0 +1,451 @@ +/* Pos
[dpdk-dev] [PATCH v9 5/7] mk: link infos generated by pmdinfogen
Generate informations to export from objects which register a driver. The C code generated by dpdk-pmdinfogen is compiled and linked into the original object file. This effectively just adds the JSON string into the string table of the object that defines the PMD to the outside world. Signed-off-by: Neil Horman Acked-by: Panu Matilainen Acked-by: Remy Horton Signed-off-by: Thomas Monjalon --- mk/internal/rte.compile-pre.mk | 12 mk/rte.sdkbuild.mk | 1 + 2 files changed, 13 insertions(+) diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk index b9bff4a..3967390 100644 --- a/mk/internal/rte.compile-pre.mk +++ b/mk/internal/rte.compile-pre.mk @@ -84,10 +84,22 @@ C_TO_O = $(CC) -Wp,-MD,$(call obj2dep,$(@)).tmp $(CFLAGS) \ C_TO_O_STR = $(subst ','\'',$(C_TO_O)) #'# fix syntax highlight C_TO_O_DISP = $(if $(V),"$(C_TO_O_STR)"," CC $(@)") endif +PMDINFO_GEN = $(RTE_SDK_BIN)/buildtools/dpdk-pmdinfogen $@ $@.pmd.c +PMDINFO_CC = $(CC) $(CFLAGS) -c -o $@.pmd.o $@.pmd.c +PMDINFO_LD = $(CROSS)ld $(LDFLAGS) -r -o $@.o $@.pmd.o $@ +PMDINFO_TO_O = if grep -q 'RTE_REGISTER_DRIVER(.*)' $<; then \ + echo "$(if $V,$(PMDINFO_GEN), PMDINFO $@.pmd.c)" && \ + $(PMDINFO_GEN) && \ + echo "$(if $V,$(PMDINFO_CC), CC $@.pmd.o)" && \ + $(PMDINFO_CC) && \ + echo "$(if $V,$(PMDINFO_LD), LD $@)" && \ + $(PMDINFO_LD) && \ + mv -f $@.o $@; fi C_TO_O_CMD = 'cmd_$@ = $(C_TO_O_STR)' C_TO_O_DO = @set -e; \ echo $(C_TO_O_DISP); \ $(C_TO_O) && \ + $(PMDINFO_TO_O) && \ echo $(C_TO_O_CMD) > $(call obj2cmd,$(@)) && \ sed 's,'$@':,dep_'$@' =,' $(call obj2dep,$(@)).tmp > $(call obj2dep,$(@)) && \ rm -f $(call obj2dep,$(@)).tmp diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk index 5edbf50..b6b66bf 100644 --- a/mk/rte.sdkbuild.mk +++ b/mk/rte.sdkbuild.mk @@ -49,6 +49,7 @@ $(1): $(sort $(LOCAL_DEPDIRS-$(1))) endef $(foreach d,$(ROOTDIRS-y),$(eval $(call depdirs_rule,$(d +drivers: | buildtools # # build and clean targets -- 2.7.0
[dpdk-dev] [PATCH v9 6/7] eal: export default plugin path to external tools
From: Neil Horman Export a symbol containing the string: DPDK_PLUGIN_PATH="$(CONFIG_RTE_EAL_PMD_PATH)" Where the latter half of the string is set at build time to a location from which autoloaded DSO's will be found. This string is used by dpdk-pmdinfo in 'plugin' mode, whereby a user can specify a DPDK installation directory (or static binary), and scan the associated path (if found) for pmd DSO's and report on their hardware support. Signed-off-by: Neil Horman Acked-by: Panu Matilainen Acked-by: Remy Horton --- lib/librte_eal/common/eal_common_options.c | 4 1 file changed, 4 insertions(+) diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c index 3efc90f..7e9f7b8 100644 --- a/lib/librte_eal/common/eal_common_options.c +++ b/lib/librte_eal/common/eal_common_options.c @@ -115,6 +115,10 @@ TAILQ_HEAD_INITIALIZER(solib_list); /* Default path of external loadable drivers */ static const char *default_solib_dir = RTE_EAL_PMD_PATH; +/* Stringified version of default solib path */ +static const char dpdk_solib_path[] __attribute__((used)) = +"DPDK_PLUGIN_PATH=" RTE_EAL_PMD_PATH; + static int master_lcore_parsed; static int mem_parsed; -- 2.7.0
[dpdk-dev] [PATCH v9 7/7] tools: query binaries for support information
From: Neil Horman This tool searches for the primer string PMD_INFO_STRING= in any ELF binary, and, if found parses the remainder of the string as a json encoded string, outputting the results in either a human readable or raw, script parseable format. Note that, in the case of dynamically linked applications, pmdinfo.py will scan for implicitly linked PMDs by searching the specified binaries .dynamic section for DT_NEEDED entries that contain the substring librte_pmd. The DT_RUNPATH, LD_LIBRARY_PATH, /usr/lib and /lib are searched for these libraries, in that order. If a file is specified with no path, it is assumed to be a PMD DSO, and the LD_LIBRARY_PATH, /usr/lib[64]/ and /lib[64] is searched for it. Currently the tool can output data in 3 formats: a) raw, suitable for scripting, where the raw JSON strings are dumped out b) table format (default) where hex pci ids are dumped in a table format c) pretty, where a user supplied pci.ids file is used to print out vendor and device strings There is a dependency on pyelftools. The script is not yet compatible with Python 3. Signed-off-by: Neil Horman Acked-by: Panu Matilainen Acked-by: Remy Horton --- MAINTAINERS| 1 + lib/librte_eal/common/eal_common_options.c | 2 +- mk/rte.sdkinstall.mk | 2 + tools/dpdk-pmdinfo.py | 628 + 4 files changed, 632 insertions(+), 1 deletion(-) create mode 100755 tools/dpdk-pmdinfo.py diff --git a/MAINTAINERS b/MAINTAINERS index 1a8a3b7..1e972f0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -71,6 +71,7 @@ F: scripts/validate-abi.sh Driver information M: Neil Horman F: buildtools/pmdinfogen/ +F: tools/dpdk-pmdinfo.py Environment Abstraction Layer diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c index 7e9f7b8..b562c8a 100644 --- a/lib/librte_eal/common/eal_common_options.c +++ b/lib/librte_eal/common/eal_common_options.c @@ -115,7 +115,7 @@ TAILQ_HEAD_INITIALIZER(solib_list); /* Default path of external loadable drivers */ static const char *default_solib_dir = RTE_EAL_PMD_PATH; -/* Stringified version of default solib path */ +/* Stringified version of default solib path used by dpdk-pmdinfo.py */ static const char dpdk_solib_path[] __attribute__((used)) = "DPDK_PLUGIN_PATH=" RTE_EAL_PMD_PATH; diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk index 2b92157..76be308 100644 --- a/mk/rte.sdkinstall.mk +++ b/mk/rte.sdkinstall.mk @@ -126,6 +126,8 @@ install-runtime: $(Q)$(call rte_mkdir, $(DESTDIR)$(sbindir)) $(Q)$(call rte_symlink,$(DESTDIR)$(datadir)/tools/dpdk_nic_bind.py, \ $(DESTDIR)$(sbindir)/dpdk_nic_bind) + $(Q)$(call rte_symlink,$(DESTDIR)$(datadir)/tools/dpdk-pmdinfo.py, \ + $(DESTDIR)$(bindir)/dpdk-pmdinfo) install-kmod: ifneq ($(wildcard $O/kmod/*),) diff --git a/tools/dpdk-pmdinfo.py b/tools/dpdk-pmdinfo.py new file mode 100755 index 000..b8a9be2 --- /dev/null +++ b/tools/dpdk-pmdinfo.py @@ -0,0 +1,628 @@ +#!/usr/bin/python +# - +# +# Utility to dump PMD_INFO_STRING support from an object file +# +# - +import os +import sys +from optparse import OptionParser +import string +import json + +# For running from development directory. It should take precedence over the +# installed pyelftools. +sys.path.insert(0, '.') + + +from elftools import __version__ +from elftools.common.exceptions import ELFError +from elftools.common.py3compat import ( +ifilter, byte2int, bytes2str, itervalues, str2bytes) +from elftools.elf.elffile import ELFFile +from elftools.elf.dynamic import DynamicSection, DynamicSegment +from elftools.elf.enums import ENUM_D_TAG +from elftools.elf.segments import InterpSegment +from elftools.elf.sections import SymbolTableSection +from elftools.elf.gnuversions import ( +GNUVerSymSection, GNUVerDefSection, +GNUVerNeedSection, +) +from elftools.elf.relocation import RelocationSection +from elftools.elf.descriptions import ( +describe_ei_class, describe_ei_data, describe_ei_version, +describe_ei_osabi, describe_e_type, describe_e_machine, +describe_e_version_numeric, describe_p_type, describe_p_flags, +describe_sh_type, describe_sh_flags, +describe_symbol_type, describe_symbol_bind, describe_symbol_visibility, +describe_symbol_shndx, describe_reloc_type, describe_dyn_tag, +describe_ver_flags, +) +from elftools.elf.constants import E_FLAGS +from elftools.dwarf.dwarfinfo import DWARFInfo +from elftools.dwarf.descriptions import ( +describe_reg_name, describe_attr_value, set_global_machine_arch, +describe_CFI_instructions, describe_CFI_register_rule, +describe_CFI_CFA_rule, +) +from elftools.dwarf.constants import ( +DW
[dpdk-dev] [dpdk-announce] release candidate 16.07-rc1
A new DPDK release candidate is ready for testing: http://dpdk.org/browse/dpdk/tag/?id=v16.07-rc1 It is the first release candidate for DPDK 16.07. This version must be released before the end of July. Therefore we have 3 weeks to make the validation and fixes. The current release notes show most of the main changes: http://dpdk.org/browse/dpdk/tree/doc/guides/rel_notes/release_16_07.rst There were a lot of changes in mempool, thus it is important to check the memory usage and the performances. The last big changes which could be accepted in the RC2 are: - pmdinfo - rte_pci_dev_ids.h removal - possible more EAL cleanup If you are not involved in the last minute changes, you can help by - testing and reporting bugs - reviewing pending patches - reviewing and testing at the same time :) Please start now discussing the changes you plan to do for the next release cycle (16.11). Thank you everyone
[dpdk-dev] [PATCH] vhost: reset queue state in destroy_device
On Fri, Jul 01, 2016 at 09:31:12AM -0700, Rich Lane wrote: > Fixes a bug where rte_eth_vhost_get_queue_event would not return enabled > queues > after a guest application restart. > > Fixes: ee584e9710b9 ("vhost: add driver on top of the library") > Signed-off-by: Rich Lane Acked-by: Yuanhan Liu Just some minor comments about the title (nothing big deal; it's just some DPDK preferences): - a bug fix patch need be titled as "fix ..." - it's better to hide specific function in the title. If you run "scripts/check-git-log.sh", you will find a warning. - the prefix should be "net/vhost". So, I'd like to reword the title a bit, to something like: "net/vhost: fix queue state not reset on destroy" If have no objection, I could fix it while apply. --yliu
[dpdk-dev] [PATCH v3] doc: virtio PMD Rx/Tx callbacks
On Thu, Jun 30, 2016 at 11:28:01PM -0400, Zhihong Wang wrote: > This patch explains current virtio PMD Rx/Tx callbacks, to help understand > what's the difference, and how to enable the right ones. > > Signed-off-by: Zhihong Wang > Acked-by: John McNamara Applied to dpdk-next-virtio. > -- > Changes in v3: However, this is still wrong. It must start with 3 dashes, ---. $ man git am The patch is expected to be inline, directly following the message. Any line that is of the form: ? three-dashes and end-of-line, or ? a line that begins with "diff -", or ? a line that begins with "Index: " is taken as the beginning of a patch, and the commit log message is terminated before the first occurrence of such a line. This is just a note; I have fixed it while apply. --yliu
[dpdk-dev] [PATCH] examples/vhost: fix failure without hints
On Fri, Jul 01, 2016 at 08:50:31AM +, Jianfeng Tan wrote: > When the specified cores and memory lie on different numa socket with > physical NIC, vhost fails to set up rx queue, and exits without any > hints. This could leads to confusion of users. > > This patch fixes it by adding some error messages when calling ether > APIs returns errors. > > Fixes: 4796ad63ba1f ("examples/vhost: import userspace vhost application") I will not call it a "fix", instead, it's an "enhancement" to me. > Reported-by: Yulong Pei And since it's not a bug, it might be "Suggested-by: ...". --yliu
[dpdk-dev] [PATCH v3] doc: virtio PMD Rx/Tx callbacks
> -Original Message- > From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com] > Sent: Monday, July 4, 2016 10:57 AM > To: Wang, Zhihong > Cc: dev at dpdk.org; Richardson, Bruce ; > Mcnamara, > John > Subject: Re: [PATCH v3] doc: virtio PMD Rx/Tx callbacks > > On Thu, Jun 30, 2016 at 11:28:01PM -0400, Zhihong Wang wrote: > > This patch explains current virtio PMD Rx/Tx callbacks, to help understand > > what's the difference, and how to enable the right ones. > > > > Signed-off-by: Zhihong Wang > > Acked-by: John McNamara > > Applied to dpdk-next-virtio. > > > -- > > Changes in v3: > > However, this is still wrong. It must start with 3 dashes, ---. > > $ man git am > > The patch is expected to be inline, directly following the > message. Any line that is of the form: > > * three-dashes and end-of-line, or > > * a line that begins with "diff -", or > > * a line that begins with "Index: " > > is taken as the beginning of a patch, and the commit log message > is terminated before the first occurrence of such a line. > > > This is just a note; I have fixed it while apply. Noted. Thanks! > > --yliu
[dpdk-dev] [PATCH v3] i40e: fix VLAN filter in promiscuous mode
> -Original Message- > From: Wu, Jingjing > Sent: Thursday, June 30, 2016 9:26 AM > To: Zhang, Helin > Cc: dev at dpdk.org; Wu, Jingjing; Yigit, Ferruh > Subject: [PATCH v3] i40e: fix VLAN filter in promiscuous mode > > For VLAN filtering VLAN table should be enabled. > But VLAN table is disabled by default until a rule added. In promiscuous mode > no rule is added to enable the VLAN table. > > So this patch clears promiscuous VLAN flag on VSI, and adds a rule to enable > VLAN table to fix VLAN filtering in promiscuous mode. > > Fixes: 4861cde46116 ("i40e: new poll mode driver") > Signed-off-by: Jingjing Wu Acked-by: Helin Zhang
[dpdk-dev] [PATCH v5 1/2] ethdev: remove get_reg_length callback
From: Zyta Szpak Removes hard-coded assumption that device registers are always 32 bits wide. The rte_eth_dev_get_reg_length and rte_eth_dev_get_reg_info callbacks did not provide register size to the app in any way while is needed to allocate correct number of bytes before retrieving registers using rte_eth_dev_get_reg. This commit changes rte_eth_dev_get_reg_info so that it can be used to retrieve both the number of registers and their width, and removes the now-redundant rte_eth_dev_get_reg_length. Signed-off-by: Zyta Szpak --- drivers/net/cxgbe/cxgbe_ethdev.c | 11 +-- drivers/net/e1000/igb_ethdev.c | 14 -- drivers/net/i40e/i40e_ethdev.c | 15 ++- drivers/net/ixgbe/ixgbe_ethdev.c | 14 -- drivers/net/thunderx/nicvf_ethdev.c| 14 +- drivers/net/thunderx/nicvf_ethdev.h| 1 + lib/librte_ether/rte_dev_info.h| 1 + lib/librte_ether/rte_ethdev.c | 12 lib/librte_ether/rte_ethdev.h | 25 + lib/librte_ether/rte_ether_version.map | 1 - 10 files changed, 51 insertions(+), 57 deletions(-) diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c index 6c130ed..90098f9 100644 --- a/drivers/net/cxgbe/cxgbe_ethdev.c +++ b/drivers/net/cxgbe/cxgbe_ethdev.c @@ -934,7 +934,15 @@ static int cxgbe_get_regs(struct rte_eth_dev *eth_dev, struct port_info *pi = (struct port_info *)(eth_dev->data->dev_private); struct adapter *adapter = pi->adapter; - regs->length = cxgbe_get_regs_len(eth_dev); + if (regs->data == NULL) { + regs->length = cxgbe_get_regs_len(eth_dev); + regs->width = sizeof(uint32_t); + regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) | + (CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) | + (1 << 16); + return 0; + } + regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) | (CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) | (1 << 16); @@ -971,7 +979,6 @@ static const struct eth_dev_ops cxgbe_eth_dev_ops = { .get_eeprom_length = cxgbe_get_eeprom_length, .get_eeprom = cxgbe_get_eeprom, .set_eeprom = cxgbe_set_eeprom, - .get_reg_length = cxgbe_get_regs_len, .get_reg= cxgbe_get_regs, }; diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index dea50f3..594655d 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -386,7 +386,6 @@ static const struct eth_dev_ops eth_igb_ops = { .timesync_disable = igb_timesync_disable, .timesync_read_rx_timestamp = igb_timesync_read_rx_timestamp, .timesync_read_tx_timestamp = igb_timesync_read_tx_timestamp, - .get_reg_length = eth_igb_get_reg_length, .get_reg = eth_igb_get_regs, .get_eeprom_length= eth_igb_get_eeprom_length, .get_eeprom = eth_igb_get_eeprom, @@ -426,7 +425,6 @@ static const struct eth_dev_ops igbvf_eth_dev_ops = { .rxq_info_get = igb_rxq_info_get, .txq_info_get = igb_txq_info_get, .mac_addr_set = igbvf_default_mac_addr_set, - .get_reg_length = igbvf_get_reg_length, .get_reg = igbvf_get_regs, }; @@ -4947,6 +4945,12 @@ eth_igb_get_regs(struct rte_eth_dev *dev, int count = 0; const struct reg_info *reg_group; + if (data == NULL) { + regs->length = eth_igb_get_reg_length(dev); + regs->width = sizeof(uint32_t); + return 0; + } + /* Support only full register dump */ if ((regs->length == 0) || (regs->length == (uint32_t)eth_igb_get_reg_length(dev))) { @@ -4971,6 +4975,12 @@ igbvf_get_regs(struct rte_eth_dev *dev, int count = 0; const struct reg_info *reg_group; + if (data == NULL) { + regs->length = igbvf_get_reg_length(dev); + regs->width = sizeof(uint32_t); + return 0; + } + /* Support only full register dump */ if ((regs->length == 0) || (regs->length == (uint32_t)igbvf_get_reg_length(dev))) { diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index c07da1b..8e3e27d 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -440,8 +440,6 @@ static int i40e_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, static int i40e_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id); -static int i40e_get_reg_length(struct rte_eth_dev *dev); - static int i40e_get_regs(struct rte_eth_dev *dev, struct rte_dev_reg_info *regs); @@ -524,7 +522,6
[dpdk-dev] [PATCH v5 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
From: Zyta Szpak Version 4 of fixing the fixed register width assumption. The app was allocating too little space for 64-bit registers which resulted in memory corruption. This commit resolves this by getting the number of registers and size of register by rte_eth_dev_get_reg_info function called first time with data=NULL. Signed-off-by: Zyta Szpak --- examples/ethtool/lib/rte_ethtool.c | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c index 54391f2..a1f91d4 100644 --- a/examples/ethtool/lib/rte_ethtool.c +++ b/examples/ethtool/lib/rte_ethtool.c @@ -46,6 +46,7 @@ int rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo) { struct rte_eth_dev_info dev_info; + struct rte_dev_reg_info reg_info; int n; if (drvinfo == NULL) @@ -65,7 +66,9 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo) dev_info.pci_dev->addr.domain, dev_info.pci_dev->addr.bus, dev_info.pci_dev->addr.devid, dev_info.pci_dev->addr.function); - n = rte_eth_dev_get_reg_length(port_id); + memset(®_info, 0, sizeof(reg_info)); + rte_eth_dev_get_reg_info(port_id, ®_info); + n = reg_info.length; if (n > 0) drvinfo->regdump_len = n; else @@ -86,12 +89,16 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo) int rte_ethtool_get_regs_len(uint8_t port_id) { - int count_regs; + struct rte_dev_reg_info reg_info; + int ret; + + memset(®_info, 0, sizeof(reg_info)); + + ret = rte_eth_dev_get_reg_info(port_id, ®_info); + if (ret) + return ret; - count_regs = rte_eth_dev_get_reg_length(port_id); - if (count_regs > 0) - return count_regs * sizeof(uint32_t); - return count_regs; + return reg_info.length * reg_info.width; } int -- 2.7.4
[dpdk-dev] [PATCH] net/thunderx: fix start/stop with different queue size
From: Kamil Rytarowski Allocate maximum supported hardware ring hardware descriptors memory on the first rte_eth_dma_zone_reserve call in-order to get sufficient hardware ring buffer space on subsequent queue setup request with different queue size. Fixes: aa0d976e501d ("net/thunderx: add Rx queue setup and release") Fixes: 3f3c6f9724a8 ("net/thunderx: add Tx queue setup and release") Fixes: 7413feee662d ("net/thunderx: add device start/stop and close") Signed-off-by: Kamil Rytarowski Signed-off-by: Zyta Szpak Signed-off-by: Jerin Jacob --- drivers/net/thunderx/base/nicvf_hw_defs.h | 3 +++ drivers/net/thunderx/nicvf_ethdev.c | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/net/thunderx/base/nicvf_hw_defs.h b/drivers/net/thunderx/base/nicvf_hw_defs.h index 88ecd17..2f2b225 100644 --- a/drivers/net/thunderx/base/nicvf_hw_defs.h +++ b/drivers/net/thunderx/base/nicvf_hw_defs.h @@ -164,6 +164,7 @@ #define RBDR_QUEUE_SZ_128K (128 * 1024) #define RBDR_QUEUE_SZ_256K (256 * 1024) #define RBDR_QUEUE_SZ_512K (512 * 1024) +#define RBDR_QUEUE_SZ_MAX RBDR_QUEUE_SZ_512K #define RBDR_SIZE_SHIFT (13) /* 8k */ @@ -174,6 +175,7 @@ #define SND_QUEUE_SZ_16K(16 * 1024) #define SND_QUEUE_SZ_32K(32 * 1024) #define SND_QUEUE_SZ_64K(64 * 1024) +#define SND_QUEUE_SZ_MAXSND_QUEUE_SZ_64K #define SND_QSIZE_SHIFT (10) /* 1k */ @@ -184,6 +186,7 @@ #define CMP_QUEUE_SZ_16K(16 * 1024) #define CMP_QUEUE_SZ_32K(32 * 1024) #define CMP_QUEUE_SZ_64K(64 * 1024) +#define CMP_QUEUE_SZ_MAXCMP_QUEUE_SZ_64K #define CMP_QSIZE_SHIFT (10) /* 1k */ diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c index 48ed381..f42de4b 100644 --- a/drivers/net/thunderx/nicvf_ethdev.c +++ b/drivers/net/thunderx/nicvf_ethdev.c @@ -495,7 +495,7 @@ nicvf_qset_cq_alloc(struct nicvf *nic, struct nicvf_rxq *rxq, uint16_t qidx, uint32_t desc_cnt) { const struct rte_memzone *rz; - uint32_t ring_size = desc_cnt * sizeof(union cq_entry_t); + uint32_t ring_size = CMP_QUEUE_SZ_MAX * sizeof(union cq_entry_t); rz = rte_eth_dma_zone_reserve(nic->eth_dev, "cq_ring", qidx, ring_size, NICVF_CQ_BASE_ALIGN_BYTES, nic->node); @@ -518,7 +518,7 @@ nicvf_qset_sq_alloc(struct nicvf *nic, struct nicvf_txq *sq, uint16_t qidx, uint32_t desc_cnt) { const struct rte_memzone *rz; - uint32_t ring_size = desc_cnt * sizeof(union sq_entry_t); + uint32_t ring_size = SND_QUEUE_SZ_MAX * sizeof(union sq_entry_t); rz = rte_eth_dma_zone_reserve(nic->eth_dev, "sq", qidx, ring_size, NICVF_SQ_BASE_ALIGN_BYTES, nic->node); @@ -551,7 +551,7 @@ nicvf_qset_rbdr_alloc(struct nicvf *nic, uint32_t desc_cnt, uint32_t buffsz) return -ENOMEM; } - ring_size = sizeof(struct rbdr_entry_t) * desc_cnt; + ring_size = sizeof(struct rbdr_entry_t) * RBDR_QUEUE_SZ_MAX; rz = rte_eth_dma_zone_reserve(nic->eth_dev, "rbdr", 0, ring_size, NICVF_RBDR_BASE_ALIGN_BYTES, nic->node); if (rz == NULL) { -- 2.5.5
[dpdk-dev] [PATCH v2 1/3] virtio: conditional compilation cleanup
On Fri, Jul 01, 2016 at 04:46:36PM +0530, Jerin Jacob wrote: > @@ -494,9 +486,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > { > uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX; > > -#ifdef RTE_MACHINE_CPUFLAG_SSSE3 > - struct virtio_hw *hw = dev->data->dev_private; > -#endif > struct virtnet_tx *txvq; > struct virtqueue *vq; > uint16_t tx_free_thresh; > @@ -511,13 +500,14 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > } > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > + struct virtio_hw *hw = dev->data->dev_private; I'd suggest to move above declaration to ... > /* Use simple rx/tx func if single segment and no offloads */ > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS && >!vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { here: we should try to avoid declaring vars in the middle of a code block. --yliu
[dpdk-dev] [PATCH v2 2/3] virtio: move SSE based Rx implementation to separate file
On Fri, Jul 01, 2016 at 04:46:37PM +0530, Jerin Jacob wrote: > * Introduced cpuflag based run-time detection to > select the SSE based simple Rx handler > * Split out SSE instruction based virtio simple Rx > implementation to a separate file As your commit log says, it does two things, therefore, I'd suggest you to do it in two patches, with each just does one thing as you mentioned. > +static void > +virtio_update_rxtx_handler(struct rte_eth_dev *dev, > +const struct rte_eth_txconf *tx_conf) > +{ > + uint8_t use_simple_rxtx = 0; > + struct virtio_hw *hw = dev->data->dev_private; > + > +#if defined RTE_ARCH_X86 > + if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE3)) > + use_simple_rxtx = 1; > +#endif > + /* Use simple rx/tx func if single segment and no offloads */ > + if (use_simple_rxtx && > +(tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS && > + !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { The alignment here is not consistent, something like following is what I'd suggest: if (use_simple_rxtx && (tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS && !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { --yliu
[dpdk-dev] [PATCH] net/fm10k: fix Rx descriptor read timing
We find that when traffic is light, a few amount of packets will be wrongly parsed (e.g. packet type), however this issue will not happen when traffic is heavy. The root cause is some fields in fm10k_rx_desc are read at wrong timing. When the input speed is slower than software's capability, fm10k scalar Rx function accesses descriptors closely to HW, so there's potential sequence: some fields like pkt_info in fm10k_rx_desc are read before HW writeback but some fields like DD bit are read after HW writeback, this will lead to the later packet parsing function using incorrect value. This patch fixes this issue by reading and parsing Rx descriptor after DD bit is set. Fixes: 4b61d3bfa941 ("fm10k: add receive and tranmit") Fixes: c82dd0a7bfa5 ("fm10k: add scatter receive") Signed-off-by: Wang Xiao W --- drivers/net/fm10k/fm10k_rxtx.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c index dd92a91..5b2d04b 100644 --- a/drivers/net/fm10k/fm10k_rxtx.c +++ b/drivers/net/fm10k/fm10k_rxtx.c @@ -114,10 +114,10 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, nb_pkts = RTE_MIN(nb_pkts, q->alloc_thresh); for (count = 0; count < nb_pkts; ++count) { + if (!(q->hw_ring[next_dd].d.staterr & FM10K_RXD_STATUS_DD)) + break; mbuf = q->sw_ring[next_dd]; desc = q->hw_ring[next_dd]; - if (!(desc.d.staterr & FM10K_RXD_STATUS_DD)) - break; #ifdef RTE_LIBRTE_FM10K_DEBUG_RX dump_rxd(&desc); #endif @@ -228,10 +228,10 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, nb_seg = RTE_MIN(nb_pkts, q->alloc_thresh); for (count = 0; count < nb_seg; count++) { + if (!(q->hw_ring[next_dd].d.staterr & FM10K_RXD_STATUS_DD)) + break; mbuf = q->sw_ring[next_dd]; desc = q->hw_ring[next_dd]; - if (!(desc.d.staterr & FM10K_RXD_STATUS_DD)) - break; #ifdef RTE_LIBRTE_FM10K_DEBUG_RX dump_rxd(&desc); #endif -- 1.9.3
[dpdk-dev] [PATCH v2 3/3] virtio: add neon support
On Fri, Jul 01, 2016 at 04:46:38PM +0530, Jerin Jacob wrote: > Added neon based Rx vector implementation. > Selection of the new handler based neon availability at runtime. > Updated the release notes and MAINTAINERS file. > > Signed-off-by: Jerin Jacob > --- > MAINTAINERS | 1 + > doc/guides/rel_notes/release_16_07.rst | 2 + > drivers/net/virtio/Makefile | 2 + > drivers/net/virtio/virtio_rxtx.c | 3 + > drivers/net/virtio/virtio_rxtx_simple_neon.c | 235 > +++ > 5 files changed, 243 insertions(+) > create mode 100644 drivers/net/virtio/virtio_rxtx_simple_neon.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index a59191e..ab04cee 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -143,6 +143,7 @@ F: lib/librte_acl/acl_run_neon.* > F: lib/librte_lpm/rte_lpm_neon.h > F: lib/librte_hash/rte*_arm64.h > F: drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c > +F: drivers/net/virtio/virtio_rxtx_simple_neon.c > > EZchip TILE-Gx > M: Zhigang Lu > diff --git a/doc/guides/rel_notes/release_16_07.rst > b/doc/guides/rel_notes/release_16_07.rst > index 9e2a817..3a5add5 100644 > --- a/doc/guides/rel_notes/release_16_07.rst > +++ b/doc/guides/rel_notes/release_16_07.rst This series basically looks good to me, but I don't think we can make it for v16.07: you missed v1 deadline; it's also too late: rc1 was already out. --yliu > @@ -174,6 +174,8 @@ New Features >section of the "Network Interface Controller Drivers" document. > > > +* **Virtio NEON support for ARM.** > + > Resolved Issues > --- >
[dpdk-dev] [PATCH 1/3] net/mlx: fix setting of interface flags
According to the documentation, the function priv_set_flags(priv, keep, flags) should not modify the flags in "keep" mask. So 'flags' argument should be masked with '~keep' before ORing it with the previous flags value. This avoids to mess the kernel interface flags when calling priv_set_flags(priv, ~IFF_UP, ~IFF_UP) in priv_set_link(): $ ip link 26: eth0: BROADCAST,MULTICAST,NOARP,ALLMULTI,PROMISC,DEBUG,\ DYNAMIC,AUTOMEDIA,PORTSEL,NOTRAILERS Fixes: 7fae69eeff13 ("mlx4: new poll mode driver") Fixes: 771fa900b73a ("mlx5: introduce new driver for Mellanox ConnectX-4 adapters") Reported-by: Fengtian Guo Signed-off-by: Olivier Matz --- drivers/net/mlx4/mlx4.c| 2 +- drivers/net/mlx5/mlx5_ethdev.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c index f8ed42b..1dd14f4 100644 --- a/drivers/net/mlx4/mlx4.c +++ b/drivers/net/mlx4/mlx4.c @@ -689,7 +689,7 @@ priv_set_flags(struct priv *priv, unsigned int keep, unsigned int flags) if (priv_get_sysfs_ulong(priv, "flags", &tmp) == -1) return -1; tmp &= keep; - tmp |= flags; + tmp |= (flags & (~keep)); return priv_set_sysfs_ulong(priv, "flags", tmp); } diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c index 0e7ed01..6fef902 100644 --- a/drivers/net/mlx5/mlx5_ethdev.c +++ b/drivers/net/mlx5/mlx5_ethdev.c @@ -461,7 +461,7 @@ priv_set_flags(struct priv *priv, unsigned int keep, unsigned int flags) if (priv_get_sysfs_ulong(priv, "flags", &tmp) == -1) return -1; tmp &= keep; - tmp |= flags; + tmp |= (flags & (~keep)); return priv_set_sysfs_ulong(priv, "flags", tmp); } -- 2.8.1
[dpdk-dev] [PATCH 2/3] net/mlx5: fix api comment of link set function
Fixes: 62072098b54e ("mlx5: support setting link up or down") Signed-off-by: Olivier Matz --- drivers/net/mlx5/mlx5_ethdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c index 6fef902..130e15d 100644 --- a/drivers/net/mlx5/mlx5_ethdev.c +++ b/drivers/net/mlx5/mlx5_ethdev.c @@ -1150,7 +1150,7 @@ priv_dev_interrupt_handler_install(struct priv *priv, struct rte_eth_dev *dev) /** * Change the link state (UP / DOWN). * - * @param dev + * @param priv * Pointer to Ethernet device structure. * @param up * Nonzero for link up, otherwise link down. -- 2.8.1
[dpdk-dev] [PATCH 0/3] net/mlx: fix link state modification
This series adds a missing device operation in mlx4 to set the link state. It also fixes the function to set the flags on the kernel interface in both mlx4 and mlx5. Guo Fengtian (1): net/mlx4: add link up/down callback function Olivier Matz (2): net/mlx: fix setting of interface flags net/mlx5: fix api comment of link set function drivers/net/mlx4/mlx4.c| 88 +- drivers/net/mlx5/mlx5_ethdev.c | 4 +- 2 files changed, 89 insertions(+), 3 deletions(-) -- 2.8.1
[dpdk-dev] [PATCH 3/3] net/mlx4: add link up/down callback function
From: Guo Fengtian Implement dev_set_link_up and dev_set_link_down device operations. Code is inspired from mlx5. Signed-off-by: Guo Fengtian --- drivers/net/mlx4/mlx4.c | 86 + 1 file changed, 86 insertions(+) diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c index 1dd14f4..5dd77c2 100644 --- a/drivers/net/mlx4/mlx4.c +++ b/drivers/net/mlx4/mlx4.c @@ -4328,6 +4328,90 @@ mlx4_dev_close(struct rte_eth_dev *dev) } /** + * Change the link state (UP / DOWN). + * + * @param priv + * Pointer to Ethernet device private data. + * @param up + * Nonzero for link up, otherwise link down. + * + * @return + * 0 on success, errno value on failure. + */ +static int +priv_set_link(struct priv *priv, int up) +{ + struct rte_eth_dev *dev = priv->dev; + int err; + unsigned int i; + + if (up) { + err = priv_set_flags(priv, ~IFF_UP, IFF_UP); + if (err) + return err; + for (i = 0; i < priv->rxqs_n; i++) + if ((*priv->rxqs)[i]->sp) + break; + /* Check if an sp queue exists. +* Note: Some old frames might be received. +*/ + if (i == priv->rxqs_n) + dev->rx_pkt_burst = mlx4_rx_burst; + else + dev->rx_pkt_burst = mlx4_rx_burst_sp; + dev->tx_pkt_burst = mlx4_tx_burst; + } else { + err = priv_set_flags(priv, ~IFF_UP, ~IFF_UP); + if (err) + return err; + dev->rx_pkt_burst = removed_rx_burst; + dev->tx_pkt_burst = removed_tx_burst; + } + return 0; +} + +/** + * DPDK callback to bring the link DOWN. + * + * @param dev + * Pointer to Ethernet device structure. + * + * @return + * 0 on success, errno value on failure. + */ +static int +mlx4_set_link_down(struct rte_eth_dev *dev) +{ + struct priv *priv = dev->data->dev_private; + int err; + + priv_lock(priv); + err = priv_set_link(priv, 0); + priv_unlock(priv); + return err; +} + +/** + * DPDK callback to bring the link UP. + * + * @param dev + * Pointer to Ethernet device structure. + * + * @return + * 0 on success, errno value on failure. + */ +static int +mlx4_set_link_up(struct rte_eth_dev *dev) +{ + struct priv *priv = dev->data->dev_private; + int err; + + priv_lock(priv); + err = priv_set_link(priv, 1); + priv_unlock(priv); + return err; +} +/** * DPDK callback to get information about the device. * * @param dev @@ -5134,6 +5218,8 @@ static const struct eth_dev_ops mlx4_dev_ops = { .dev_configure = mlx4_dev_configure, .dev_start = mlx4_dev_start, .dev_stop = mlx4_dev_stop, + .dev_set_link_down = mlx4_set_link_down, + .dev_set_link_up = mlx4_set_link_up, .dev_close = mlx4_dev_close, .promiscuous_enable = mlx4_promiscuous_enable, .promiscuous_disable = mlx4_promiscuous_disable, -- 2.8.1
[dpdk-dev] [PATCH v2 1/3] virtio: conditional compilation cleanup
On Mon, Jul 04, 2016 at 03:36:48PM +0800, Yuanhan Liu wrote: > On Fri, Jul 01, 2016 at 04:46:36PM +0530, Jerin Jacob wrote: > > @@ -494,9 +486,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > > { > > uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX; > > > > -#ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > - struct virtio_hw *hw = dev->data->dev_private; > > -#endif > > struct virtnet_tx *txvq; > > struct virtqueue *vq; > > uint16_t tx_free_thresh; > > @@ -511,13 +500,14 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > > } > > > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > + struct virtio_hw *hw = dev->data->dev_private; > > I'd suggest to move above declaration to ... > > > /* Use simple rx/tx func if single segment and no offloads */ > > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS && > > !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > > here: we should try to avoid declaring vars in the middle of a code block. Next patch in this series, moving all rxtx handler selection code to separate function(virtio_update_rxtx_handler()) where declaration comes as first line in the function.i.e the comment is taken care of in the series. > > --yliu
[dpdk-dev] [PATCH v2 2/3] virtio: move SSE based Rx implementation to separate file
On Mon, Jul 04, 2016 at 03:42:47PM +0800, Yuanhan Liu wrote: > On Fri, Jul 01, 2016 at 04:46:37PM +0530, Jerin Jacob wrote: > > * Introduced cpuflag based run-time detection to > > select the SSE based simple Rx handler > > * Split out SSE instruction based virtio simple Rx > > implementation to a separate file > > As your commit log says, it does two things, therefore, I'd suggest you > to do it in two patches, with each just does one thing as you mentioned. OK. Will fix it in next revision. > > > +static void > > +virtio_update_rxtx_handler(struct rte_eth_dev *dev, > > + const struct rte_eth_txconf *tx_conf) > > +{ > > + uint8_t use_simple_rxtx = 0; > > + struct virtio_hw *hw = dev->data->dev_private; > > + > > +#if defined RTE_ARCH_X86 > > + if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE3)) > > + use_simple_rxtx = 1; > > +#endif > > + /* Use simple rx/tx func if single segment and no offloads */ > > + if (use_simple_rxtx && > > + (tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS && > > + !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > > The alignment here is not consistent, something like following is what > I'd suggest: > > if (use_simple_rxtx && > (tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS && > !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { OK. Will fix it in next revision. > > --yliu
[dpdk-dev] [PATCH v2 1/3] virtio: conditional compilation cleanup
On Mon, Jul 04, 2016 at 02:06:27PM +0530, Jerin Jacob wrote: > On Mon, Jul 04, 2016 at 03:36:48PM +0800, Yuanhan Liu wrote: > > On Fri, Jul 01, 2016 at 04:46:36PM +0530, Jerin Jacob wrote: > > > @@ -494,9 +486,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > > > { > > > uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX; > > > > > > -#ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > - struct virtio_hw *hw = dev->data->dev_private; > > > -#endif > > > struct virtnet_tx *txvq; > > > struct virtqueue *vq; > > > uint16_t tx_free_thresh; > > > @@ -511,13 +500,14 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > > > } > > > > > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > + struct virtio_hw *hw = dev->data->dev_private; > > > > I'd suggest to move above declaration to ... > > > > > /* Use simple rx/tx func if single segment and no offloads */ > > > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS && > > >!vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > > > > here: we should try to avoid declaring vars in the middle of a code block. > > Next patch in this series, moving all rxtx handler selection code to > separate function(virtio_update_rxtx_handler()) where declaration comes > as first line in the function.i.e the comment is taken care of in the > series. Yes, I saw that. But in principle, each patch is atomic: it's not a good idea/practice to introduce issues in path A and then fix it in path B. --yliu
[dpdk-dev] [PATCH v2 3/3] virtio: add neon support
On Mon, Jul 04, 2016 at 03:53:22PM +0800, Yuanhan Liu wrote: > On Fri, Jul 01, 2016 at 04:46:38PM +0530, Jerin Jacob wrote: > > Added neon based Rx vector implementation. > > Selection of the new handler based neon availability at runtime. > > Updated the release notes and MAINTAINERS file. > > > > Signed-off-by: Jerin Jacob > > --- > > MAINTAINERS | 1 + > > doc/guides/rel_notes/release_16_07.rst | 2 + > > drivers/net/virtio/Makefile | 2 + > > drivers/net/virtio/virtio_rxtx.c | 3 + > > drivers/net/virtio/virtio_rxtx_simple_neon.c | 235 > > +++ > > 5 files changed, 243 insertions(+) > > create mode 100644 drivers/net/virtio/virtio_rxtx_simple_neon.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index a59191e..ab04cee 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -143,6 +143,7 @@ F: lib/librte_acl/acl_run_neon.* > > F: lib/librte_lpm/rte_lpm_neon.h > > F: lib/librte_hash/rte*_arm64.h > > F: drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c > > +F: drivers/net/virtio/virtio_rxtx_simple_neon.c > > > > EZchip TILE-Gx > > M: Zhigang Lu > > diff --git a/doc/guides/rel_notes/release_16_07.rst > > b/doc/guides/rel_notes/release_16_07.rst > > index 9e2a817..3a5add5 100644 > > --- a/doc/guides/rel_notes/release_16_07.rst > > +++ b/doc/guides/rel_notes/release_16_07.rst > > This series basically looks good to me, but I don't think we can make it > for v16.07: you missed v1 deadline; it's also too late: rc1 was already out. OK. But I thought, Thomas hasn't pulled the changes from dpdk-next-virtio. Even if didn't make it for v16.07, I would suggest you to consider taking the changes to dpdk-next-virtio as this change involves file restructuring (Will have issue with re-basing in future) without having any functional impact. > > --yliu > > > @@ -174,6 +174,8 @@ New Features > >section of the "Network Interface Controller Drivers" document. > > > > > > +* **Virtio NEON support for ARM.** > > + > > Resolved Issues > > --- > >
[dpdk-dev] [PATCH] examples/vm_power_manager: remove dependency on internal header file
Macro CHANNEL_CMDS_MAX_CPUS stand for the maximum number of cores controlled by virtual channels. This macro only be used in the example, so remove it from library to example header file. Signed-off-by: Marvin Liu diff --git a/examples/vm_power_manager/channel_manager.h b/examples/vm_power_manager/channel_manager.h index 67e26ec..47c3b9c 100644 --- a/examples/vm_power_manager/channel_manager.h +++ b/examples/vm_power_manager/channel_manager.h @@ -41,7 +41,13 @@ extern "C" { #include #include #include -#include "channel_commands.h" + +/* Maximum number of CPUs */ +#define CHANNEL_CMDS_MAX_CPUS64 +#if CHANNEL_CMDS_MAX_CPUS > 64 +#error Maximum number of cores is 64, overflow is guaranteed to \ +cause problems with VM Power Management +#endif /* Maximum name length including '\0' terminator */ #define CHANNEL_MGR_MAX_NAME_LEN64 diff --git a/lib/librte_power/channel_commands.h b/lib/librte_power/channel_commands.h index de6d926..383897b 100644 --- a/lib/librte_power/channel_commands.h +++ b/lib/librte_power/channel_commands.h @@ -40,13 +40,6 @@ extern "C" { #include -/* Maximum number of CPUs */ -#define CHANNEL_CMDS_MAX_CPUS64 -#if CHANNEL_CMDS_MAX_CPUS > 64 -#error Maximum number of cores is 64, overflow is guaranteed to \ - cause problems with VM Power Management -#endif - /* Maximum number of channels per VM */ #define CHANNEL_CMDS_MAX_VM_CHANNELS 64 -- 1.9.3
[dpdk-dev] [PATCH v3] hash: new function to retrieve a key given its position
The function rte_hash_get_key_with_position is added in this patch. As the position returned when adding a key is frequently used as an offset into an array of user data, this function performs the operation of retrieving a key given this offset. A possible use case would be to delete a key from the hash table when its entry in the array of data has certain value. For instance, the key could be a flow 5-tuple, and the value stored in the array a time stamp. Signed-off-by: Juan Antonio Montesinos Signed-off-by: Yari Adan Petralanda --- app/test/test_hash.c | 42 lib/librte_hash/rte_cuckoo_hash.c| 20 + lib/librte_hash/rte_hash.h | 18 lib/librte_hash/rte_hash_version.map | 7 ++ 4 files changed, 87 insertions(+) diff --git a/app/test/test_hash.c b/app/test/test_hash.c index 7e41725..29abcd9 100644 --- a/app/test/test_hash.c +++ b/app/test/test_hash.c @@ -421,6 +421,46 @@ static int test_add_update_delete(void) } /* + * Sequence of operations for retrieving a key with its position + * + * - create table + * - add key + * - get the key with its position: hit + * - delete key + * - try to get the deleted key: miss + * + */ +static int test_hash_get_key_with_position(void) +{ + struct rte_hash *handle = NULL; + int pos, expectedPos, result; + void *key; + + ut_params.name = "hash_get_key_w_pos"; + handle = rte_hash_create(&ut_params); + RETURN_IF_ERROR(handle == NULL, "hash creation failed"); + + pos = rte_hash_add_key(handle, &keys[0]); + print_key_info("Add", &keys[0], pos); + RETURN_IF_ERROR(pos < 0, "failed to add key (pos0=%d)", pos); + expectedPos = pos; + + result = rte_hash_get_key_with_position(handle, pos, &key); + RETURN_IF_ERROR(result != 0, "error retrieving a key"); + + pos = rte_hash_del_key(handle, &keys[0]); + print_key_info("Del", &keys[0], pos); + RETURN_IF_ERROR(pos != expectedPos, + "failed to delete key (pos0=%d)", pos); + + result = rte_hash_get_key_with_position(handle, pos, &key); + RETURN_IF_ERROR(result != -ENOENT, "non valid key retrieved"); + + rte_hash_free(handle); + return 0; +} + +/* * Sequence of operations for find existing hash table * * - create table @@ -1442,6 +1482,8 @@ test_hash(void) return -1; if (test_hash_add_delete_jhash_3word() < 0) return -1; + if (test_hash_get_key_with_position() < 0) + return -1; if (test_hash_find_existing() < 0) return -1; if (test_add_update_delete() < 0) diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c index 7b7d1f8..a3c9091 100644 --- a/lib/librte_hash/rte_cuckoo_hash.c +++ b/lib/librte_hash/rte_cuckoo_hash.c @@ -973,6 +973,26 @@ rte_hash_del_key(const struct rte_hash *h, const void *key) return __rte_hash_del_key_with_hash(h, key, rte_hash_hash(h, key)); } +int +rte_hash_get_key_with_position(const struct rte_hash *h, const int32_t position, + void **key) +{ + RETURN_IF_TRUE(((h == NULL) || (key == NULL)), -EINVAL); + + struct rte_hash_key *k, *keys = h->key_store; + k = (struct rte_hash_key *) ((char *) keys + (position + 1) * +h->key_entry_size); + *key = k->key; + + if (position != + __rte_hash_lookup_with_hash(h, *key, rte_hash_hash(h, *key), + NULL)) { + return -ENOENT; + } + + return 0; +} + /* Lookup bulk stage 0: Prefetch input key */ static inline void lookup_stage0(unsigned *idx, uint64_t *lookup_mask, diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index 724315a..78af365 100644 --- a/lib/librte_hash/rte_hash.h +++ b/lib/librte_hash/rte_hash.h @@ -268,6 +268,24 @@ rte_hash_del_key(const struct rte_hash *h, const void *key); int32_t rte_hash_del_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t sig); +/** + * Find a key in the hash table given the position. + * This operation is multi-thread safe. + * + * @param h + * Hash table to get the key from. + * @param position + * Position returned when the key was inserted. + * @param key + * Output containing a pointer to the key + * @return + * - 0 if retrieved successfully + * - EINVAL if the parameters are invalid. + * - ENOENT if no valid key is found in the given position. + */ +int +rte_hash_get_key_with_position(const struct rte_hash *h, const int32_t position, + void **key); /** * Find a key-value pair in the hash table. diff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map index 4f25436..4237995 100644 --- a/lib/librte_hash/rte_hash_version.map +++ b/lib/librte_hash/rte_hash_version.map @@ -3
[dpdk-dev] [PATCH v2 3/3] virtio: add neon support
On Mon, Jul 04, 2016 at 02:25:55PM +0530, Jerin Jacob wrote: > On Mon, Jul 04, 2016 at 03:53:22PM +0800, Yuanhan Liu wrote: > > On Fri, Jul 01, 2016 at 04:46:38PM +0530, Jerin Jacob wrote: > > > Added neon based Rx vector implementation. > > > Selection of the new handler based neon availability at runtime. > > > Updated the release notes and MAINTAINERS file. > > > > > > Signed-off-by: Jerin Jacob > > > --- > > > MAINTAINERS | 1 + > > > doc/guides/rel_notes/release_16_07.rst | 2 + > > > drivers/net/virtio/Makefile | 2 + > > > drivers/net/virtio/virtio_rxtx.c | 3 + > > > drivers/net/virtio/virtio_rxtx_simple_neon.c | 235 > > > +++ > > > 5 files changed, 243 insertions(+) > > > create mode 100644 drivers/net/virtio/virtio_rxtx_simple_neon.c > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index a59191e..ab04cee 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -143,6 +143,7 @@ F: lib/librte_acl/acl_run_neon.* > > > F: lib/librte_lpm/rte_lpm_neon.h > > > F: lib/librte_hash/rte*_arm64.h > > > F: drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c > > > +F: drivers/net/virtio/virtio_rxtx_simple_neon.c > > > > > > EZchip TILE-Gx > > > M: Zhigang Lu > > > diff --git a/doc/guides/rel_notes/release_16_07.rst > > > b/doc/guides/rel_notes/release_16_07.rst > > > index 9e2a817..3a5add5 100644 > > > --- a/doc/guides/rel_notes/release_16_07.rst > > > +++ b/doc/guides/rel_notes/release_16_07.rst > > > > This series basically looks good to me, but I don't think we can make it > > for v16.07: you missed v1 deadline; it's also too late: rc1 was already out. > > OK. But I thought, Thomas hasn't pulled the changes from dpdk-next-virtio. > > Even if didn't make it for v16.07, I would suggest you to consider taking > the changes to dpdk-next-virtio as this change involves file restructuring > (Will have issue with re-basing in future) without having any functional > impact. Yes, that's my plan. I will do the merge ASAP, when - v16.07 is out. - your patches are ready. --yliu
[dpdk-dev] Redirect all packets to a specific VM pool
Hello, I have a setup with SR-IOV where I want to forward all the packets to a specific VM pool. I found that in some Intel NICs it is possible to set a field called default pool. (Flag DEF_PL within the PFVTCTL register). In order to setup this using DPDK, I used the default_pool field in the rte_eth_vmdq_rx_conf structure, something like this: .vmdq_rx_conf = { .nb_queue_pools = ETH_8_POOLS, .enable_default_pool = 1, .default_pool = 5, .nb_pool_maps = 1, .pool_map = {{0, 0},}, }, However it appears not to be working, all the packets are being forwarded to the host pool. Am I missing something? Is there a better approach to reach my goal? Thank you very much for your help. Mauricio V.
[dpdk-dev] [PATCH v2 1/3] virtio: conditional compilation cleanup
On Mon, Jul 04, 2016 at 04:42:32PM +0800, Yuanhan Liu wrote: > On Mon, Jul 04, 2016 at 02:06:27PM +0530, Jerin Jacob wrote: > > On Mon, Jul 04, 2016 at 03:36:48PM +0800, Yuanhan Liu wrote: > > > On Fri, Jul 01, 2016 at 04:46:36PM +0530, Jerin Jacob wrote: > > > > @@ -494,9 +486,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > > > > { > > > > uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX; > > > > > > > > -#ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > > - struct virtio_hw *hw = dev->data->dev_private; > > > > -#endif > > > > struct virtnet_tx *txvq; > > > > struct virtqueue *vq; > > > > uint16_t tx_free_thresh; > > > > @@ -511,13 +500,14 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > > > > } > > > > > > > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > > + struct virtio_hw *hw = dev->data->dev_private; > > > > > > I'd suggest to move above declaration to ... > > > > > > > /* Use simple rx/tx func if single segment and no offloads */ > > > > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == > > > > VIRTIO_SIMPLE_FLAGS && > > > > !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > > > > > > here: we should try to avoid declaring vars in the middle of a code block. > > > > Next patch in this series, moving all rxtx handler selection code to > > separate function(virtio_update_rxtx_handler()) where declaration comes > > as first line in the function.i.e the comment is taken care of in the > > series. > > Yes, I saw that. But in principle, each patch is atomic: it's not a > good idea/practice to introduce issues in path A and then fix it in > path B. In my view it was not an issue as I was removing all possible conditional compilation flag. If I were to move the declaration to top then another conditional compilation RTE_MACHINE_CPUFLAG_SSSE3 flag I need to add around declaring the variable. Hope this justifies the reason. If you are not convinced then let me know, if will add the change in next revision. Jerin > > --yliu
[dpdk-dev] [PATCH v3] hash: new function to retrieve a key given its position
Hi Yari, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Yari Adan > Petralanda > Sent: Monday, July 04, 2016 10:00 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3] hash: new function to retrieve a key given its > position > > The function rte_hash_get_key_with_position is added in this patch. > As the position returned when adding a key is frequently used as an > offset into an array of user data, this function performs the operation > of retrieving a key given this offset. > > A possible use case would be to delete a key from the hash table when > its entry in the array of data has certain value. For instance, the key > could be a flow 5-tuple, and the value stored in the array a time > stamp. > > Signed-off-by: Juan Antonio Montesinos > > Signed-off-by: Yari Adan Petralanda Could you mention what you changed in this v3? We always add the changes that we have made between versions, so reviewers know what you have changed. Thanks, Pablo
[dpdk-dev] [PATCH v2 01/16] vfio: fix include of eal_private.h to be local
> -Original Message- > From: Jan Viktorin [mailto:viktorin at rehivetech.com] > Sent: Monday, June 13, 2016 2:02 PM > To: dev at dpdk.org > Cc: Jan Viktorin ; Burakov, Anatoly > ; David Marchand > ; Wiles, Keith ; > Santosh Shukla ; Stephen Hemminger > ; Shreyansh Jain > > Subject: [PATCH v2 01/16] vfio: fix include of eal_private.h to be local > > Signed-off-by: Jan Viktorin > --- > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > index 10266f8..257162b 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > @@ -43,11 +43,11 @@ > #include > #include > #include > -#include > > #include "eal_filesystem.h" > #include "eal_pci_init.h" > #include "eal_vfio.h" > +#include "eal_private.h" > > /** > * @file > -- > 2.8.0 There's no patch cover letter so I'll reply to the first patch. I've done some cursory testing with a NIC, nothing seems to be broken and the code looks OK to me. So, once this patchset is rebased on latest master (patches 9, 14 and 15 no longer apply cleanly), ack from me. Thanks, Anatoly
[dpdk-dev] [PATCH v5 1/2] ethdev: remove get_reg_length callback
> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c > @@ -934,7 +934,15 @@ static int cxgbe_get_regs(struct rte_eth_dev *eth_dev, > struct port_info *pi = (struct port_info *)(eth_dev->data->dev_private); > struct adapter *adapter = pi->adapter; > > - regs->length = cxgbe_get_regs_len(eth_dev); > + if (regs->data == NULL) { > + regs->length = cxgbe_get_regs_len(eth_dev); > + regs->width = sizeof(uint32_t); > + regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) | > + (CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) | > + (1 << 16); > + return 0; > + } > + > regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) | > (CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) | > (1 << 16); Code duplication.. Rest looks ok and passed a quick compile test. Might need to keep an eye out for other driver changes.
[dpdk-dev] [PATCH v5 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
On 04/07/2016 07:51, Zyta Szpak wrote: > From: Zyta Szpak > > Version 4 of fixing the fixed register width assumption. > The app was allocating too little space for 64-bit registers > which resulted in memory corruption. This commit resolves > this by getting the number of registers and size of register > by rte_eth_dev_get_reg_info function called first time > with data=NULL. Comments regarding commit message on v4 1/2 also apply here.. It is also not v4.. :) > Signed-off-by: Zyta Szpak > --- > examples/ethtool/lib/rte_ethtool.c | 19 +-- > 1 file changed, 13 insertions(+), 6 deletions(-)
[dpdk-dev] [PATCH v2 01/16] vfio: fix include of eal_private.h to be local
On Mon, 4 Jul 2016 10:22:08 + "Burakov, Anatoly" wrote: > > -Original Message- > > From: Jan Viktorin [mailto:viktorin at rehivetech.com] > > Sent: Monday, June 13, 2016 2:02 PM > > To: dev at dpdk.org > > Cc: Jan Viktorin ; Burakov, Anatoly > > ; David Marchand > > ; Wiles, Keith ; > > Santosh Shukla ; Stephen Hemminger > > ; Shreyansh Jain > > > > Subject: [PATCH v2 01/16] vfio: fix include of eal_private.h to be local > > > > Signed-off-by: Jan Viktorin > > --- > > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > > b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > > index 10266f8..257162b 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > > @@ -43,11 +43,11 @@ > > #include > > #include > > #include > > -#include > > > > #include "eal_filesystem.h" > > #include "eal_pci_init.h" > > #include "eal_vfio.h" > > +#include "eal_private.h" > > > > /** > > * @file > > -- > > 2.8.0 > Hello Anatoly, > There's no patch cover letter so I'll reply to the first patch. yes, there is (and you are in CC), may be lost in the traffic... http://dpdk.org/ml/archives/dev/2016-June/041085.html ID: 1465822926-23742-1-git-send-email-viktorin at rehivetech.com > I've done some cursory testing with a NIC, nothing seems to be broken and the > code looks OK to me. So, once this patchset is rebased on latest master > (patches 9, 14 and 15 no longer apply cleanly), ack from me. Thank you very much! I am going to do the rebase. Jan > > Thanks, > Anatoly -- Jan Viktorin E-mail: Viktorin at RehiveTech.com System Architect Web:www.RehiveTech.com RehiveTech Brno, Czech Republic
[dpdk-dev] [PATCH v2 1/3] virtio: conditional compilation cleanup
On Mon, Jul 04, 2016 at 02:37:55PM +0530, Jerin Jacob wrote: > On Mon, Jul 04, 2016 at 04:42:32PM +0800, Yuanhan Liu wrote: > > On Mon, Jul 04, 2016 at 02:06:27PM +0530, Jerin Jacob wrote: > > > On Mon, Jul 04, 2016 at 03:36:48PM +0800, Yuanhan Liu wrote: > > > > On Fri, Jul 01, 2016 at 04:46:36PM +0530, Jerin Jacob wrote: > > > > > @@ -494,9 +486,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > > > > > { > > > > > uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX; > > > > > > > > > > -#ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > > > - struct virtio_hw *hw = dev->data->dev_private; > > > > > -#endif > > > > > struct virtnet_tx *txvq; > > > > > struct virtqueue *vq; > > > > > uint16_t tx_free_thresh; > > > > > @@ -511,13 +500,14 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev > > > > > *dev, > > > > > } > > > > > > > > > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > > > + struct virtio_hw *hw = dev->data->dev_private; > > > > > > > > I'd suggest to move above declaration to ... > > > > > > > > > /* Use simple rx/tx func if single segment and no offloads */ > > > > > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == > > > > > VIRTIO_SIMPLE_FLAGS && > > > > >!vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > > > > > > > > here: we should try to avoid declaring vars in the middle of a code > > > > block. > > > > > > Next patch in this series, moving all rxtx handler selection code to > > > separate function(virtio_update_rxtx_handler()) where declaration comes > > > as first line in the function.i.e the comment is taken care of in the > > > series. > > > > Yes, I saw that. But in principle, each patch is atomic: it's not a > > good idea/practice to introduce issues in path A and then fix it in > > path B. > > In my view it was not an issue as I was removing all possible > conditional compilation flag. If I were to move the declaration to top > then another conditional compilation RTE_MACHINE_CPUFLAG_SSSE3 > flag I need to add around declaring the variable. Nope, I was suggesting to move it inside the "if" block. So, this is actually consistent with what you are trying to do. Besides, it removes an declaration in the middle. --yliu > Hope this justifies the reason. If you are not convinced then let me know, > if will add the change in next revision. > > Jerin > > > > > --yliu
[dpdk-dev] [PATCH v5 1/2] ethdev: remove get_reg_length callback
Very good point.. this one and the other :) THere comes fixed version. 2016-07-04 12:38 GMT+02:00 Remy Horton : > > +++ b/drivers/net/cxgbe/cxgbe_ethdev.c >> @@ -934,7 +934,15 @@ static int cxgbe_get_regs(struct rte_eth_dev >> *eth_dev, >> struct port_info *pi = (struct port_info >> *)(eth_dev->data->dev_private); >> struct adapter *adapter = pi->adapter; >> >> - regs->length = cxgbe_get_regs_len(eth_dev); >> + if (regs->data == NULL) { >> + regs->length = cxgbe_get_regs_len(eth_dev); >> + regs->width = sizeof(uint32_t); >> + regs->version = >> CHELSIO_CHIP_VERSION(adapter->params.chip) | >> + (CHELSIO_CHIP_RELEASE(adapter->params.chip) << >> 10) | >> + (1 << 16); >> + return 0; >> + } >> + >> regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) | >> (CHELSIO_CHIP_RELEASE(adapter->params.chip) << >> 10) | >> (1 << 16); >> > > Code duplication.. > > Rest looks ok and passed a quick compile test. Might need to keep an eye > out for other driver changes. > -- Zyta Szpak Software Development Engineer Semihalf sp. z o.o. ul. Krakusa 11 30-535 Krak?w
[dpdk-dev] [PATCH v6 1/2] ethdev: remove get_reg_length callback
From: Zyta Szpak Removes hard-coded assumption that device registers are always 32 bits wide. The rte_eth_dev_get_reg_length and rte_eth_dev_get_reg_info callbacks did not provide register size to the app in any way while is needed to allocate correct number of bytes before retrieving registers using rte_eth_dev_get_reg. This commit changes rte_eth_dev_get_reg_info so that it can be used to retrieve both the number of registers and their width, and removes the now-redundant rte_eth_dev_get_reg_length. Signed-off-by: Zyta Szpak --- drivers/net/cxgbe/cxgbe_ethdev.c | 14 ++ drivers/net/e1000/igb_ethdev.c | 14 -- drivers/net/i40e/i40e_ethdev.c | 15 ++- drivers/net/ixgbe/ixgbe_ethdev.c | 14 -- drivers/net/thunderx/nicvf_ethdev.c| 14 +- drivers/net/thunderx/nicvf_ethdev.h| 1 + lib/librte_ether/rte_dev_info.h| 1 + lib/librte_ether/rte_ethdev.c | 12 lib/librte_ether/rte_ethdev.h | 25 + lib/librte_ether/rte_ether_version.map | 1 - 10 files changed, 52 insertions(+), 59 deletions(-) diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c index 6c130ed..f822d8f 100644 --- a/drivers/net/cxgbe/cxgbe_ethdev.c +++ b/drivers/net/cxgbe/cxgbe_ethdev.c @@ -934,10 +934,17 @@ static int cxgbe_get_regs(struct rte_eth_dev *eth_dev, struct port_info *pi = (struct port_info *)(eth_dev->data->dev_private); struct adapter *adapter = pi->adapter; - regs->length = cxgbe_get_regs_len(eth_dev); regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) | - (CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) | - (1 << 16); + (CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) | + (1 << 16); + + if (regs->data == NULL) { + regs->length = cxgbe_get_regs_len(eth_dev); + regs->width = sizeof(uint32_t); + + return 0; + } + t4_get_regs(adapter, regs->data, (regs->length * sizeof(uint32_t))); return 0; @@ -971,7 +978,6 @@ static const struct eth_dev_ops cxgbe_eth_dev_ops = { .get_eeprom_length = cxgbe_get_eeprom_length, .get_eeprom = cxgbe_get_eeprom, .set_eeprom = cxgbe_set_eeprom, - .get_reg_length = cxgbe_get_regs_len, .get_reg= cxgbe_get_regs, }; diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index dea50f3..594655d 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -386,7 +386,6 @@ static const struct eth_dev_ops eth_igb_ops = { .timesync_disable = igb_timesync_disable, .timesync_read_rx_timestamp = igb_timesync_read_rx_timestamp, .timesync_read_tx_timestamp = igb_timesync_read_tx_timestamp, - .get_reg_length = eth_igb_get_reg_length, .get_reg = eth_igb_get_regs, .get_eeprom_length= eth_igb_get_eeprom_length, .get_eeprom = eth_igb_get_eeprom, @@ -426,7 +425,6 @@ static const struct eth_dev_ops igbvf_eth_dev_ops = { .rxq_info_get = igb_rxq_info_get, .txq_info_get = igb_txq_info_get, .mac_addr_set = igbvf_default_mac_addr_set, - .get_reg_length = igbvf_get_reg_length, .get_reg = igbvf_get_regs, }; @@ -4947,6 +4945,12 @@ eth_igb_get_regs(struct rte_eth_dev *dev, int count = 0; const struct reg_info *reg_group; + if (data == NULL) { + regs->length = eth_igb_get_reg_length(dev); + regs->width = sizeof(uint32_t); + return 0; + } + /* Support only full register dump */ if ((regs->length == 0) || (regs->length == (uint32_t)eth_igb_get_reg_length(dev))) { @@ -4971,6 +4975,12 @@ igbvf_get_regs(struct rte_eth_dev *dev, int count = 0; const struct reg_info *reg_group; + if (data == NULL) { + regs->length = igbvf_get_reg_length(dev); + regs->width = sizeof(uint32_t); + return 0; + } + /* Support only full register dump */ if ((regs->length == 0) || (regs->length == (uint32_t)igbvf_get_reg_length(dev))) { diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index c07da1b..8e3e27d 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -440,8 +440,6 @@ static int i40e_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, static int i40e_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id); -static int i40e_get_reg_length(struct rte_eth_dev *dev); - static int i40e_get_regs(struct rte_eth_dev *dev, struct rte_dev_reg_info *regs); @@ -5
[dpdk-dev] [PATCH v6 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
From: Zyta Szpak This change deals with hard-coded register width. The app was allocating too little space for 64-bit registers which resulted in memory corruption. This commit resolves this by getting the number of registers and size of register by rte_eth_dev_get_reg_info function called first time with data=NULL. Signed-off-by: Zyta Szpak --- examples/ethtool/lib/rte_ethtool.c | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c index 54391f2..a1f91d4 100644 --- a/examples/ethtool/lib/rte_ethtool.c +++ b/examples/ethtool/lib/rte_ethtool.c @@ -46,6 +46,7 @@ int rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo) { struct rte_eth_dev_info dev_info; + struct rte_dev_reg_info reg_info; int n; if (drvinfo == NULL) @@ -65,7 +66,9 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo) dev_info.pci_dev->addr.domain, dev_info.pci_dev->addr.bus, dev_info.pci_dev->addr.devid, dev_info.pci_dev->addr.function); - n = rte_eth_dev_get_reg_length(port_id); + memset(®_info, 0, sizeof(reg_info)); + rte_eth_dev_get_reg_info(port_id, ®_info); + n = reg_info.length; if (n > 0) drvinfo->regdump_len = n; else @@ -86,12 +89,16 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo) int rte_ethtool_get_regs_len(uint8_t port_id) { - int count_regs; + struct rte_dev_reg_info reg_info; + int ret; + + memset(®_info, 0, sizeof(reg_info)); + + ret = rte_eth_dev_get_reg_info(port_id, ®_info); + if (ret) + return ret; - count_regs = rte_eth_dev_get_reg_length(port_id); - if (count_regs > 0) - return count_regs * sizeof(uint32_t); - return count_regs; + return reg_info.length * reg_info.width; } int -- 2.7.4
[dpdk-dev] [PATCH v2 1/3] virtio: conditional compilation cleanup
On Mon, Jul 04, 2016 at 07:02:25PM +0800, Yuanhan Liu wrote: > On Mon, Jul 04, 2016 at 02:37:55PM +0530, Jerin Jacob wrote: > > On Mon, Jul 04, 2016 at 04:42:32PM +0800, Yuanhan Liu wrote: > > > On Mon, Jul 04, 2016 at 02:06:27PM +0530, Jerin Jacob wrote: > > > > On Mon, Jul 04, 2016 at 03:36:48PM +0800, Yuanhan Liu wrote: > > > > > On Fri, Jul 01, 2016 at 04:46:36PM +0530, Jerin Jacob wrote: > > > > > > @@ -494,9 +486,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev > > > > > > *dev, > > > > > > { > > > > > > uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX; > > > > > > > > > > > > -#ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > > > > - struct virtio_hw *hw = dev->data->dev_private; > > > > > > -#endif > > > > > > struct virtnet_tx *txvq; > > > > > > struct virtqueue *vq; > > > > > > uint16_t tx_free_thresh; > > > > > > @@ -511,13 +500,14 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev > > > > > > *dev, > > > > > > } > > > > > > > > > > > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > > > > + struct virtio_hw *hw = dev->data->dev_private; > > > > > > > > > > I'd suggest to move above declaration to ... > > > > > > > > > > > /* Use simple rx/tx func if single segment and no offloads */ > > > > > > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == > > > > > > VIRTIO_SIMPLE_FLAGS && > > > > > > !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > > > > > > > > > > here: we should try to avoid declaring vars in the middle of a code > > > > > block. > > > > > > > > Next patch in this series, moving all rxtx handler selection code to > > > > separate function(virtio_update_rxtx_handler()) where declaration comes > > > > as first line in the function.i.e the comment is taken care of in the > > > > series. > > > > > > Yes, I saw that. But in principle, each patch is atomic: it's not a > > > good idea/practice to introduce issues in path A and then fix it in > > > path B. > > > > In my view it was not an issue as I was removing all possible > > conditional compilation flag. If I were to move the declaration to top > > then another conditional compilation RTE_MACHINE_CPUFLAG_SSSE3 > > flag I need to add around declaring the variable. > > Nope, I was suggesting to move it inside the "if" block. So, this > is actually consistent with what you are trying to do. Besides, it > removes an declaration in the middle. Just to get the clarity on "moving inside the 'if' block" Are you suggesting to have like below? #ifdef RTE_MACHINE_CPUFLAG_SSSE3 + struct virtio_hw *hw; /* Use simple rx/tx func if single segment and no offloads */ if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS && !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { PMD_INIT_LOG(INFO, "Using simple rx/tx path"); dev->tx_pkt_burst = virtio_xmit_pkts_simple; dev->rx_pkt_burst = virtio_recv_pkts_vec; - use_simple_rxtx = 1; + hw = dev->data->dev_private; + hw->use_simple_rxtx = 1; } #endif Instead of following scheme in existing patch, #ifdef RTE_MACHINE_CPUFLAG_SSSE3 + struct virtio_hw *hw = dev->data->dev_private; /* Use simple rx/tx func if single segment and no offloads */ if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS && !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { PMD_INIT_LOG(INFO, "Using simple rx/tx path"); dev->tx_pkt_burst = virtio_xmit_pkts_simple; dev->rx_pkt_burst = virtio_recv_pkts_vec; - use_simple_rxtx = 1; + hw->use_simple_rxtx = 1; } #endif The former case will have issue as "hw" been used in "if" with vtpci_with_feature. OR if you meant just floating "struct virtio_hw *hw" without RTE_MACHINE_CPUFLAG_SSSE3 then it comes error on non x86 as unused "hw" variable. If you meant something else then let me know? > > --yliu > > > Hope this justifies the reason. If you are not convinced then let me know, > > if will add the change in next revision. > > > > Jerin > > > > > > > > --yliu
[dpdk-dev] [PATCH v2 1/3] virtio: conditional compilation cleanup
On Mon, Jul 04, 2016 at 05:45:57PM +0530, Jerin Jacob wrote: > On Mon, Jul 04, 2016 at 07:02:25PM +0800, Yuanhan Liu wrote: > > On Mon, Jul 04, 2016 at 02:37:55PM +0530, Jerin Jacob wrote: > > > On Mon, Jul 04, 2016 at 04:42:32PM +0800, Yuanhan Liu wrote: > > > > On Mon, Jul 04, 2016 at 02:06:27PM +0530, Jerin Jacob wrote: > > > > > On Mon, Jul 04, 2016 at 03:36:48PM +0800, Yuanhan Liu wrote: > > > > > > On Fri, Jul 01, 2016 at 04:46:36PM +0530, Jerin Jacob wrote: > > > > > > > @@ -494,9 +486,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev > > > > > > > *dev, > > > > > > > { > > > > > > > uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX; > > > > > > > > > > > > > > -#ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > > > > > - struct virtio_hw *hw = dev->data->dev_private; > > > > > > > -#endif > > > > > > > struct virtnet_tx *txvq; > > > > > > > struct virtqueue *vq; > > > > > > > uint16_t tx_free_thresh; > > > > > > > @@ -511,13 +500,14 @@ virtio_dev_tx_queue_setup(struct > > > > > > > rte_eth_dev *dev, > > > > > > > } > > > > > > > > > > > > > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > > > > > + struct virtio_hw *hw = dev->data->dev_private; > > > > > > > > > > > > I'd suggest to move above declaration to ... > > > > > > > > > > > > > /* Use simple rx/tx func if single segment and no offloads */ > > > > > > > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == > > > > > > > VIRTIO_SIMPLE_FLAGS && > > > > > > >!vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > > > > > > > > > > > > here: we should try to avoid declaring vars in the middle of a code > > > > > > block. > > > > > > > > > > Next patch in this series, moving all rxtx handler selection code to > > > > > separate function(virtio_update_rxtx_handler()) where declaration > > > > > comes > > > > > as first line in the function.i.e the comment is taken care of in the > > > > > series. > > > > > > > > Yes, I saw that. But in principle, each patch is atomic: it's not a > > > > good idea/practice to introduce issues in path A and then fix it in > > > > path B. > > > > > > In my view it was not an issue as I was removing all possible > > > conditional compilation flag. If I were to move the declaration to top > > > then another conditional compilation RTE_MACHINE_CPUFLAG_SSSE3 > > > flag I need to add around declaring the variable. > > > > Nope, I was suggesting to move it inside the "if" block. So, this > > is actually consistent with what you are trying to do. Besides, it > > removes an declaration in the middle. > > Just to get the clarity on "moving inside the 'if' block" > > Are you suggesting to have like below? > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > + struct virtio_hw *hw; > /* Use simple rx/tx func if single segment and no offloads */ > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == > VIRTIO_SIMPLE_FLAGS && > !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > PMD_INIT_LOG(INFO, "Using simple rx/tx path"); > dev->tx_pkt_burst = virtio_xmit_pkts_simple; > dev->rx_pkt_burst = virtio_recv_pkts_vec; > - use_simple_rxtx = 1; > + hw = dev->data->dev_private; > + hw->use_simple_rxtx = 1; > } > #endif > > > Instead of following scheme in existing patch, > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > + struct virtio_hw *hw = dev->data->dev_private; > /* Use simple rx/tx func if single segment and no offloads */ > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == > VIRTIO_SIMPLE_FLAGS && > !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > PMD_INIT_LOG(INFO, "Using simple rx/tx path"); > dev->tx_pkt_burst = virtio_xmit_pkts_simple; > dev->rx_pkt_burst = virtio_recv_pkts_vec; > - use_simple_rxtx = 1; > + hw->use_simple_rxtx = 1; > } > #endif > > > The former case will have issue as "hw" been used in "if" with > vtpci_with_feature. Oh, my bad. I overlooked it. Sorry for that! > OR > > if you meant just floating "struct virtio_hw *hw" without > RTE_MACHINE_CPUFLAG_SSSE3 > then it comes error on non x86 as unused "hw" variable. > > If you meant something else then let me know? I then prefer to keep the "#ifdef .. #endif" on top then. It will stop us from offending a minor rule, while you can remove the ugly "#ifdef" block in the next patch. Works to you? --yliu
[dpdk-dev] Low packet generation rate of 526kpps using pktgen-dpdk from inside VM
Hi, With pktgen-dpdk inside VM, it is observed that packet TX rate (526Kpps with 64byte size) is very low. How can higher packet rates be achieved ? On enabling logs of virtio pmd in DPDK, getting following logs in syslogs: PMD: virtio_xmit_pkts() tx: No free tx descriptors to transmit Two PC setup having configuration as follow: -> Xeon Processor : Intel(R) Xeon(R) CPU E5-2603 v3 @ 1.60GHz having 6 cores. -> RAM : 16GB ->One PC is generating traffic other is receiving the packets using pktgen tool. The two PC are connected directly with 10G Ethernet interface. Generation side PC - Host -> 2 cores dedicated to OVS-DPDK -> 1 core is free for Linux kernel -> OS: Ubuntu 14.04 -> Linux kernel : 3.13.0-85-generic -> DPDK version : 2.2.0 -> OVS version : 2.5.0 -> VFIO driver for port binding VM - Total 3 cores -> 2 cores for pktgen -> 1 core for Linux kernel -> OS: Ubuntu 15.04 -> Linux kernel : 3.19.8 -> DPDK version : 16.04 -> Pktgen version : 3.0.00 -> UIO driver for port binding DPDK, OVS and pktgen are build using - x86_64-ivshmen-linuxapp-gcc option. VM launched through following Qemu command : * qemu-system-x86_64 --enable-kvm -smp 3 -cpu host -m 2048 -chardev socket,id=charnet0,path=/var/run/openvswitch/vhost-user1 -netdev type=vhost-user,id=hostnet0,chardev=charnet0,vhostforce=on -device virtio-net-pci,netdev=hostnet0,mac=16:fa:13:26:31:1b -object memory-backend-file,id=mem,size=2048M,mem-path=/dev/hugepages,share=on -numa node,memdev=mem -mem-prealloc -hda /home/aricent/ubuntu_15_04.img Pktgen is launched using following command: Pktgen -c -0x3 -n 4 -m 512 -b 000:00:04.0 - p 0x3 -t -m "[1:1].0" OVS command used: * /usr/sbin/ovs-vswitchd --dpdk -c 0xc -n 4 -m 2048 -- unix:/var/run/openvswitch/db.sock -vconsole:emer -vsyslog:err -vfile:info --mlockall --no-chdir --log-file=/var/log/openvswitch/ovs-vswitchd.log --pidfile=/var/run/openvswitch/ovs-vswitchd.pid --detach -monitor Thanks & Regards Abhishek "DISCLAIMER: This message is proprietary to Aricent and is intended solely for the use of the individual to whom it is addressed. It may contain privileged or confidential information and should not be circulated or used for any purpose other than for what it is intended. If you have received this message in error, please notify the originator immediately. If you are not the intended recipient, you are notified that you are strictly prohibited from using, copying, altering, or disclosing the contents of this message. Aricent accepts no responsibility for loss or damage arising from the use of the information transmitted by this email including damage from virus."
[dpdk-dev] [PATCH v9 0/7] export PMD infos
On Mon, Jul 04, 2016 at 03:13:58AM +0200, Thomas Monjalon wrote: > This is a respin of the series from Neil. > It was planned to be integrated in 16.07-rc1 but the discovered issues > make a new revision needed. > There are still few things which could be improved but it is not mandatory > to fix them for an integration in 16.07-rc2: > - fix make clean after pmdinfogen Clean works just fine, whats wrong with it? > - build installable pmdinfogen for target Again, this seems to be working just fine for me, what do you see as wrong with it? > - convert pmdinfo.py to Python 3 This was discussed weeks ago earlier in this thread, since there is no requirement for python3 we decided not to use it. Why renege on that now? > - document dependency pyelftools In what capacity beyond simply noting the fact that it imports that package? > > Changes done in this v9: > - fix build dependency of drivers on pmdinfogen In what way is it broken? The drivers directory is dependent on building pmdinfogen, what more do you want? > - fix build of mlx4, mlx5, aesni They weren't broken as of V8, not sure what you're getting at here > - fix new drivers bnxt, thunderx, kasumi > - fix MAINTAINERS file > - fix coding style in pmdinfogen > - add compiler checks for pmdinfogen > - remove useless functions in pmdinfogen > - fail build if pmdinfogen fails (set -e) > - fix verbose pmdinfogen run > - build pmdinfogen in buildtools directory (was app) Are you kidding me!? We just went 10 rounds over where to build this stupid application, and after I finally gave in to your demands, you change it one last time and call it a fix? Thats a real slap in the face, thanks for that. > - install pmdinfogen in sdk package (was runtime) > - fix CamelCase in pmdinfo.py > - prefix executables with dpdk- Again, seriously? Are you just trying to assert some sort of not invented here mentality? We had this conversation over and over again weeks ago, but you couldn't possibly chime in then? And you have to slip it in now, because this is the way you want it, as though I did it wrong previously? > - rename PMD_REGISTER_DRIVER -> RTE_REGISTER_DRIVER Annnd...one more, of course. There simply no way you could leave this alone, and modify subsequent pending patches to match the existing macro format, is there? > - separate commit for hostapp.mk refresh > - remove useless hostlib.mk What does this have to do with 'fixes' for this series? > - spread doc in appropriate patches > Again, why? Whats wrong with adding documentation in its own patch? To modify my work to fit your sensibilities and call it a fix is really quite an insult. Honestly, at this point, I'm done. Do what you want to it, I don't plan on making any more changes. > Please review carefully. > > Older changes: > > v8) > * Modified symlink for pmdinfo to use dpdk_ > * Added dpdk_pmdinfogen for pmdinfogen binary > > v7) > * Fixed up copyright > * Switched buildtool makefile to use hostapp.mk > * Modified arch check to use RTE_ARCH_64 > * Modifed hostapp.mk to drop output in build/app > * Additional testing on ppc64 to ensure big endian works > * Modified TO_NATIVE macro to use rte_byteorder inlines >based on endianess of target ELF file > * Ran checkpatch on commits > * Fixed some typos in doc > > v6) > * Added some programming guide documentation > * Reformatted python script with pep8 > > v5) > * Added a dpdk- prefix to pmdinfo symlink > * Renamed build announcement to PMDINFOGEN/BUILD > * Removed erroneous true statement from makefile > * Removed duplicate rte.hostapp.mk makefile > * Fixed some whitespace > * Whitespace fixups > * Fixed makefile if; then style > * Renamed module string C file > * Removed duplicate rte_pci_id definition > * Clarified macro names > * Removed PMD type attribute > * Fixed tools usage for 32 bit arches > * Removed some unused code > * Added a few comments > > v4) > * Modified the operation of the -p option. As much as I don't like implying > that autoloaded pmds are guaranteed to be there at run time, I'm having a hard > time seeing how we can avoid specifying the application file to scan for the > autoload directory. Without it we can't determine which library the user > means > in a multiversion installation > * Cleaned up the help text > * Added a rule for an install target for pmdinfo > * Guarded against some tracebacks in pmdinfo > * Use DT_NEEDED entries to get versioned libraries in -p mode > * Fixed traceback that occurs on lack of input arguments > * Fixed some erroneous macro usage in drivers that aren't in the default > build > > v3) > * Made table mode printing the default mode > * Added a default path to the pci.ids file > * Modifed pmdinfo to use python rather than python3 > * Cleaned up some changelog entries > * Added an export for RTE_EAL_PMD_PATH > * Added a plugin option to pmdinfo to scan for autoloaded DSO's > > v2) > * Made the
[dpdk-dev] [PATCH] mbuf: make rearm_data address naturally aligned
On Mon, May 23, 2016 at 01:19:46PM +0200, Olivier Matz wrote: > Hi, > > On 05/19/2016 05:50 PM, Thomas Monjalon wrote: > > 2016-05-19 19:05, Jerin Jacob: > >> On Thu, May 19, 2016 at 12:18:57PM +, Ananyev, Konstantin wrote: > On Thu, May 19, 2016 at 12:20:16AM +0530, Jerin Jacob wrote: > > On Wed, May 18, 2016 at 05:43:00PM +0100, Bruce Richardson wrote: > >> On Wed, May 18, 2016 at 07:27:43PM +0530, Jerin Jacob wrote: > >>> I wonder does anyone really use mbuf port field? > >>> My though was - could we to drop it completely? > >>> Actually, after discussing it with Bruce offline, an interesting idea > >>> came out: > >>> if we'll drop port and make mbuf_prefree() to reset nb_segs=1, then > >>> we can reduce RX rearm_data to 4B. So with that layout: > >>> > >>> struct rte_mbuf { > >>> > >>> MARKER cacheline0; > >>> > >>> void *buf_addr; > >>> phys_addr_t buf_physaddr; > >>> uint16_t buf_len; > >>> uint8_t nb_segs; > >>> uint8_t reserved_1byte; /* former port */ > >>> > >>> MARKER32 rearm_data; > >>> uint16_t data_off; > >>>uint16_t refcnt; > >>> > >>> uint64_t ol_flags; > >>> ... > >>> > >>> We can keep buf_len at its place and avoid 2B gap, while making rearm_data > >>> 4B long and 4B aligned. > >> > >> Couple of comments, > >> - IMO, It is good if nb_segs can move under rearm_data, as some > >> drivers(not in ixgbe may be) can write nb_segs in one shot also > >> in segmented rx handler case > >> - I think, it makes sense to keep port in mbuf so that application > >> can make use of it(Not sure what real application developers think of > >> this) > > > > I agree we could try to remove the port id from mbuf. > > These mbuf data are a common base to pass infos between drivers and apps. > > If you need to store some data which are read and write from the app only, > > you can have use the private zone (see rte_pktmbuf_priv_size). > > At the first read, I was in favor of keeping the port_id in the > mbuf. But after checking the examples and applications, I'm not > opposed to remove it. Indeed, this information could go in an > application-specific part or it could be an additional function > parameter in the application processing function. > > The same question could be raised for nb_seg: it seems this info > is not used a lot, and having a 8 bits value here also prevents from > having long chains (ex: for socket buffer in a tcp stack). > > Just an idea thrown in the air: if these 2 fields are removed, it > brings some room for the m->next field to go in the first cache line. > This was mentioned several times (at least [1]). > > [1] http://dpdk.org/ml/archives/dev/2015-June/019182.html Can we come to some consensus on this for this release. The original problem was mbuf->rearm_data not being natually aligned and the assosiated performacnce issues with ARM architecture for non naturally aligned access. I believe that is fixing in this patch without any performance degradation on IA. I believe that is a good progress. Can we make further mbuff improvements as a additional problem to solve. Thoughts ? Jerin
[dpdk-dev] [PATCH v2 1/3] virtio: conditional compilation cleanup
On Mon, Jul 04, 2016 at 08:26:30PM +0800, Yuanhan Liu wrote: > On Mon, Jul 04, 2016 at 05:45:57PM +0530, Jerin Jacob wrote: > > On Mon, Jul 04, 2016 at 07:02:25PM +0800, Yuanhan Liu wrote: > > > On Mon, Jul 04, 2016 at 02:37:55PM +0530, Jerin Jacob wrote: > > > > On Mon, Jul 04, 2016 at 04:42:32PM +0800, Yuanhan Liu wrote: > > > > > On Mon, Jul 04, 2016 at 02:06:27PM +0530, Jerin Jacob wrote: > > > > > > On Mon, Jul 04, 2016 at 03:36:48PM +0800, Yuanhan Liu wrote: > > > > > > > On Fri, Jul 01, 2016 at 04:46:36PM +0530, Jerin Jacob wrote: > > > > > > > > @@ -494,9 +486,6 @@ virtio_dev_tx_queue_setup(struct > > > > > > > > rte_eth_dev *dev, > > > > > > > > { > > > > > > > > uint8_t vtpci_queue_idx = 2 * queue_idx + > > > > > > > > VTNET_SQ_TQ_QUEUE_IDX; > > > > > > > > > > > > > > > > -#ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > > > > > > - struct virtio_hw *hw = dev->data->dev_private; > > > > > > > > -#endif > > > > > > > > struct virtnet_tx *txvq; > > > > > > > > struct virtqueue *vq; > > > > > > > > uint16_t tx_free_thresh; > > > > > > > > @@ -511,13 +500,14 @@ virtio_dev_tx_queue_setup(struct > > > > > > > > rte_eth_dev *dev, > > > > > > > > } > > > > > > > > > > > > > > > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > > > > > > > + struct virtio_hw *hw = dev->data->dev_private; > > > > > > > > > > > > > > I'd suggest to move above declaration to ... > > > > > > > > > > > > > > > /* Use simple rx/tx func if single segment and no > > > > > > > > offloads */ > > > > > > > > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == > > > > > > > > VIRTIO_SIMPLE_FLAGS && > > > > > > > > !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > > > > > > > > > > > > > > here: we should try to avoid declaring vars in the middle of a > > > > > > > code block. > > > > > > > > > > > > Next patch in this series, moving all rxtx handler selection code to > > > > > > separate function(virtio_update_rxtx_handler()) where declaration > > > > > > comes > > > > > > as first line in the function.i.e the comment is taken care of in > > > > > > the > > > > > > series. > > > > > > > > > > Yes, I saw that. But in principle, each patch is atomic: it's not a > > > > > good idea/practice to introduce issues in path A and then fix it in > > > > > path B. > > > > > > > > In my view it was not an issue as I was removing all possible > > > > conditional compilation flag. If I were to move the declaration to top > > > > then another conditional compilation RTE_MACHINE_CPUFLAG_SSSE3 > > > > flag I need to add around declaring the variable. > > > > > > Nope, I was suggesting to move it inside the "if" block. So, this > > > is actually consistent with what you are trying to do. Besides, it > > > removes an declaration in the middle. > > > > Just to get the clarity on "moving inside the 'if' block" > > > > Are you suggesting to have like below? > > > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > + struct virtio_hw *hw; > > /* Use simple rx/tx func if single segment and no offloads */ > > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == > > VIRTIO_SIMPLE_FLAGS && > > !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > > PMD_INIT_LOG(INFO, "Using simple rx/tx path"); > > dev->tx_pkt_burst = virtio_xmit_pkts_simple; > > dev->rx_pkt_burst = virtio_recv_pkts_vec; > > - use_simple_rxtx = 1; > > + hw = dev->data->dev_private; > > + hw->use_simple_rxtx = 1; > > } > > #endif > > > > > > Instead of following scheme in existing patch, > > > > #ifdef RTE_MACHINE_CPUFLAG_SSSE3 > > + struct virtio_hw *hw = dev->data->dev_private; > > /* Use simple rx/tx func if single segment and no offloads */ > > if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == > > VIRTIO_SIMPLE_FLAGS && > > !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { > > PMD_INIT_LOG(INFO, "Using simple rx/tx path"); > > dev->tx_pkt_burst = virtio_xmit_pkts_simple; > > dev->rx_pkt_burst = virtio_recv_pkts_vec; > > - use_simple_rxtx = 1; > > + hw->use_simple_rxtx = 1; > > } > > #endif > > > > > > The former case will have issue as "hw" been used in "if" with > > vtpci_with_feature. > > Oh, my bad. I overlooked it. Sorry for that! > > > OR > > > > if you meant just floating "struct virtio_hw *hw" without > > RTE_MACHINE_CPUFLAG_SSSE3 > > then it comes error on non x86 as unused "hw" variable. > > > > If you meant something else then let me know? > > I then prefer to keep the "#ifdef .. #endif" on top then. It will stop > us from offending a minor rule, while you can remove the ugly "#ifdef" > block in the next patch. > > Works to you? OK. As you wish :-) > > --yliu
[dpdk-dev] [PATCH v2 1/3] virtio: conditional compilation cleanup
On Mon, Jul 04, 2016 at 06:20:42PM +0530, Jerin Jacob wrote: > > > The former case will have issue as "hw" been used in "if" with > > > vtpci_with_feature. > > > > Oh, my bad. I overlooked it. Sorry for that! > > > > > OR > > > > > > if you meant just floating "struct virtio_hw *hw" without > > > RTE_MACHINE_CPUFLAG_SSSE3 > > > then it comes error on non x86 as unused "hw" variable. > > > > > > If you meant something else then let me know? > > > > I then prefer to keep the "#ifdef .. #endif" on top then. It will stop > > us from offending a minor rule, while you can remove the ugly "#ifdef" > > block in the next patch. > > > > Works to you? > > OK. As you wish :-) Thank you! --yliu
[dpdk-dev] [PATCH] mbuf: make rearm_data address naturally aligned
Hi Jerin, On 07/04/2016 02:45 PM, Jerin Jacob wrote: > On Mon, May 23, 2016 at 01:19:46PM +0200, Olivier Matz wrote: >> Hi, >> >> On 05/19/2016 05:50 PM, Thomas Monjalon wrote: >>> 2016-05-19 19:05, Jerin Jacob: On Thu, May 19, 2016 at 12:18:57PM +, Ananyev, Konstantin wrote: >> On Thu, May 19, 2016 at 12:20:16AM +0530, Jerin Jacob wrote: >>> On Wed, May 18, 2016 at 05:43:00PM +0100, Bruce Richardson wrote: On Wed, May 18, 2016 at 07:27:43PM +0530, Jerin Jacob wrote: > I wonder does anyone really use mbuf port field? > My though was - could we to drop it completely? > Actually, after discussing it with Bruce offline, an interesting idea > came out: > if we'll drop port and make mbuf_prefree() to reset nb_segs=1, then > we can reduce RX rearm_data to 4B. So with that layout: > > struct rte_mbuf { > > MARKER cacheline0; > > void *buf_addr; > phys_addr_t buf_physaddr; > uint16_t buf_len; > uint8_t nb_segs; > uint8_t reserved_1byte; /* former port */ > > MARKER32 rearm_data; > uint16_t data_off; >uint16_t refcnt; > > uint64_t ol_flags; > ... > > We can keep buf_len at its place and avoid 2B gap, while making rearm_data > 4B long and 4B aligned. Couple of comments, - IMO, It is good if nb_segs can move under rearm_data, as some drivers(not in ixgbe may be) can write nb_segs in one shot also in segmented rx handler case - I think, it makes sense to keep port in mbuf so that application can make use of it(Not sure what real application developers think of this) >>> >>> I agree we could try to remove the port id from mbuf. >>> These mbuf data are a common base to pass infos between drivers and apps. >>> If you need to store some data which are read and write from the app only, >>> you can have use the private zone (see rte_pktmbuf_priv_size). >> >> At the first read, I was in favor of keeping the port_id in the >> mbuf. But after checking the examples and applications, I'm not >> opposed to remove it. Indeed, this information could go in an >> application-specific part or it could be an additional function >> parameter in the application processing function. >> >> The same question could be raised for nb_seg: it seems this info >> is not used a lot, and having a 8 bits value here also prevents from >> having long chains (ex: for socket buffer in a tcp stack). >> >> Just an idea thrown in the air: if these 2 fields are removed, it >> brings some room for the m->next field to go in the first cache line. >> This was mentioned several times (at least [1]). >> >> [1] http://dpdk.org/ml/archives/dev/2015-June/019182.html > > > Can we come to some consensus on this for this release. The original problem > was > mbuf->rearm_data not being natually aligned and the assosiated performacnce > issues with ARM architecture for non naturally aligned access. > I believe that is fixing in this patch without any performance degradation on > IA. > I believe that is a good progress. Can we make further mbuff improvements as > a additional problem to solve. > > Thoughts ? Changing the mbuf topology is something that should happen as rarely as possible, so I think we should group all mbuf modifications in one version. Your issue (mbuf->rearm alignment), the removing of uneeded fields (port id, maybe nb_segs), and possibly other things should be addressed for next version (16.11). I'll send a deprecation notice before the 16.07 is out if there is no opposition. Regards, Olivier
[dpdk-dev] [PATCH v1 28/28] ether: support SoC device/driver
On Wed, 29 Jun 2016 15:12:07 +0530 Shreyansh jain wrote: > Hi Jan, > > On Friday 06 May 2016 07:18 PM, Jan Viktorin wrote: > > Signed-off-by: Jan Viktorin > > --- > > lib/librte_ether/rte_ethdev.c | 127 > > +- > > lib/librte_ether/rte_ethdev.h | 31 +++ > > 2 files changed, 157 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > > index 4af2e5f..9259c2c 100644 > > --- a/lib/librte_ether/rte_ethdev.c > > +++ b/lib/librte_ether/rte_ethdev.c > > @@ -320,6 +320,99 @@ rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev) > > } > > > [...] > > +int > > rte_eth_dev_is_valid_port(uint8_t port_id) > > { > > if (port_id >= RTE_MAX_ETHPORTS || > > @@ -1431,7 +1524,7 @@ rte_eth_dev_info_get(uint8_t port_id, struct > > rte_eth_dev_info *dev_info) > > > > RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get); > > (*dev->dev_ops->dev_infos_get)(dev, dev_info); > > - dev_info->pci_dev = dev->pci_dev; > > + dev_info->soc_dev = dev->soc_dev; > > I think both the members, pci_dev and soc_dev, should be updated by this call. > Is there some specific reason why soc_dev is the only one which is getting > updated? Yes, looks like a mistake. Thanks! And sorry for delayed reply. Jan > > > dev_info->driver_name = dev->data->drv_name; > > } > > > [...] > > - > Shreyansh > -- Jan Viktorin E-mail: Viktorin at RehiveTech.com System Architect Web:www.RehiveTech.com RehiveTech Brno, Czech Republic
[dpdk-dev] [PATCH v9 0/7] export PMD infos
Hi Neil, I don't really understand why you don't accept I contribute to this patchset. More details below. 2016-07-04 08:34, Neil Horman: > On Mon, Jul 04, 2016 at 03:13:58AM +0200, Thomas Monjalon wrote: > > This is a respin of the series from Neil. > > It was planned to be integrated in 16.07-rc1 but the discovered issues > > make a new revision needed. > > There are still few things which could be improved but it is not mandatory > > to fix them for an integration in 16.07-rc2: > > - fix make clean after pmdinfogen > Clean works just fine, whats wrong with it? "make clean" do not remove the .pmd.* files. But it is not a blocking issue. > > - build installable pmdinfogen for target > Again, this seems to be working just fine for me, what do you see as wrong > with > it? If we install the SDK as a binary package for another arch, we cannot use pmdinfogen as it is not cross-compiled (on purpose). It was just a thought for possible future enhancement and is not blocking. > > - convert pmdinfo.py to Python 3 > This was discussed weeks ago earlier in this thread, since there is no > requirement for python3 we decided not to use it. Why renege on that now? Yes we can take the script as is. It is just a note for future enhancement. Not blocking. > > - document dependency pyelftools > In what capacity beyond simply noting the fact that it imports that package? I don't know where we could document this dependency. Just a note, not blocking. > > Changes done in this v9: > > - fix build dependency of drivers on pmdinfogen > In what way is it broken? The drivers directory is dependent on building > pmdinfogen, what more do you want? The drivers directory Makefile is a rte.subdir.mk. It does not handle DEPDIRS. I've fix it in mk/rte.sdkbuild.mk drivers: | buildtools > > - fix build of mlx4, mlx5, aesni > They weren't broken as of V8, not sure what you're getting at here Yes they were because of typos. I guess you were not compiling them. > > - fix new drivers bnxt, thunderx, kasumi > > - fix MAINTAINERS file > > - fix coding style in pmdinfogen > > - add compiler checks for pmdinfogen > > - remove useless functions in pmdinfogen > > - fail build if pmdinfogen fails (set -e) > > - fix verbose pmdinfogen run > > - build pmdinfogen in buildtools directory (was app) > Are you kidding me!? We just went 10 rounds over where to build this stupid > application, and after I finally gave in to your demands, you change it one > last time and call it a fix? Thats a real slap in the face, thanks for that. We were discussing reusing hostapp.mk. And I was OK to use app/ as build directory. After checking sdkinstall.mk, I changed my mind to build in buildtools/. I do not call it a fix, just a change. Where do you see it is a fix? No slap. > > - install pmdinfogen in sdk package (was runtime) > > - fix CamelCase in pmdinfo.py > > - prefix executables with dpdk- > Again, seriously? Are you just trying to assert some sort of not invented > here > mentality? We had this conversation over and over again weeks ago, but you > couldn't possibly chime in then? And you have to slip it in now, because this > is the way you want it, as though I did it wrong previously? You did not do it wrong. After more thoughts, I really think we must be careful to have a consistent namespace with tools we export publically. I'm thinking to re-use the dpdk- prefix for other tools. It's open to discussion as everything. > > - rename PMD_REGISTER_DRIVER -> RTE_REGISTER_DRIVER > Annnd...one more, of course. There simply no way you could leave this alone, > and modify subsequent pending patches to match the existing macro format, is > there? I'm just trying to have a consistent namespace: RTE_ in code, dpdk- for executables. > > - separate commit for hostapp.mk refresh > > - remove useless hostlib.mk > What does this have to do with 'fixes' for this series? It is not a fix, just a patch that I have added in the series. > > - spread doc in appropriate patches > > > Again, why? Whats wrong with adding documentation in its own patch? To > modify > my work to fit your sensibilities and call it a fix is really quite an insult. It is neither a fix nor an insult. Having the doc and the code in the same patch helps to understand the patch. > Honestly, at this point, I'm done. Do what you want to it, I don't plan on > making any more changes. Please do not take it personnaly. I'm just trying to work on this interesting feature. Ideally you would have say "thank you for the work". At least it would be interesting to have your review.
[dpdk-dev] [PATCH] bnx2x: set random MAC address if one isn't assigned
On Thu, Jun 30, 2016 at 07:50:55PM +, Rasesh Mody wrote: > > From: Bruce Richardson [mailto:bruce.richardson at intel.com] > > Sent: Wednesday, June 29, 2016 3:56 AM > > > > On Tue, Jun 21, 2016 at 08:18:46AM -0400, Chas Williams wrote: > > > If the PF hasn't assigned an address, assign one randomly. While > > > here, convert to use DPDK's ether address utility routines. > > > > > > Fixes: 540a211084a7 ("bnx2x: driver core") > > > > > > Signed-off-by: Chas Williams <3chas3 at gmail.com> > > > > Maintainers, any comment or ack on this patch? > > > > /Bruce > > Acked-by: Rasesh Mody > > Thanks! > Rasesh Applied to dpdk-next-net/rel_16_07 /Bruce
[dpdk-dev] [PATCH v6 1/2] ethdev: remove get_reg_length callback
On 04/07/2016 12:36, Zyta Szpak wrote: > From: Zyta Szpak > > Removes hard-coded assumption that device registers > are always 32 bits wide. The rte_eth_dev_get_reg_length > and rte_eth_dev_get_reg_info callbacks did not > provide register size to the app in any way while is > needed to allocate correct number of bytes before > retrieving registers using rte_eth_dev_get_reg. > > This commit changes rte_eth_dev_get_reg_info so that > it can be used to retrieve both the number of registers > and their width, and removes the now-redundant > rte_eth_dev_get_reg_length. > > Signed-off-by: Zyta Szpak Acked-by: Remy Horton
[dpdk-dev] [PATCH v6 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
On 04/07/2016 12:36, Zyta Szpak wrote: > From: Zyta Szpak > > This change deals with hard-coded register width. > The app was allocating too little space for 64-bit registers > which resulted in memory corruption. This commit resolves > this by getting the number of registers and size of register > by rte_eth_dev_get_reg_info function called first time > with data=NULL. > > Signed-off-by: Zyta Szpak Acked-by: Remy Horton
[dpdk-dev] [PATCH v1 02/28] eal: extract function eal_parse_sysfs_valuef
Hello Shreyansh, On Thu, 16 Jun 2016 11:47:29 + Shreyansh Jain wrote: > Sorry, didn't notice this email earlier... > Comments inline > > > -Original Message- > > From: Jan Viktorin [mailto:viktorin at rehivetech.com] > > Sent: Wednesday, June 15, 2016 3:26 PM > > To: Shreyansh Jain > > Cc: dev at dpdk.org; David Marchand ; Thomas > > Monjalon > > ; Bruce Richardson > intel.com>; > > Declan Doherty ; jianbo.liu at linaro.org; > > jerin.jacob at caviumnetworks.com; Keith Wiles ; > > Stephen > > Hemminger > > Subject: Re: [dpdk-dev] [PATCH v1 02/28] eal: extract function > > eal_parse_sysfs_valuef > > > > On Tue, 14 Jun 2016 04:30:57 + > > Shreyansh Jain wrote: > > > > > Hi Jan, > > > > > [...] > > > > > > > > > > > > I almost skipped the '..f' in the name and wondered how two functions > > > > > > > > > having same name exist :D > > > > > > > > I agree that a better name would be nice here. This convention was > > > > based > > on > > > > the libc naming > > > > (fopen, fclose) but the "f" letter could not be at the beginning. > > > > > > > > What about one of those? > > > > > > > > * eal_parse_sysfs_fd_value > > > > * eal_parse_sysfs_file_value > > > > > > I don't have any better idea than above. > > > > > > Though, I still feel that 'eal_parse_sysfs_value -> > > eal_parse_sysfs_file_value' would be slightly asymmetrical - but again, this > > is highly subjective argument. > > > > I don't see any asymmetry here. The functions equal, just the new one > > accepts > > a file pointer instead of a path > > and we don't have function name overloading in C. > > Asymmetrical because cascading function names maybe additive for easy > reading/recall. > > 'eal_parse_sysfs_value ==> eal_parse_sysfs_value_ ==> > eal_parse_sysfs_value__' > > Obviously, this is not a rule - it just makes reading and recalling of > cascade easier. > As for: > > eal_parse_sysfs_value => eal_parse_sysfs_file_value > > inserts an identifier between a name, making it (slightly) difficult to > correlate. > > Again, as I mentioned earlier, this is subjective argument and matter of > (personal!) choice. > > > > > > > > > Or, eal_parse_sysfs_value -> eal_parse_sysfs_value_read() may be... > > > > I think, I'll go with eal_parse_sysfs_file_value for v2. Ideally, it should > > be > > eal_parse_sysfs_path_value and eal_parse_sysfs_file_value. Thus, this looks > > like > > a good way. > > > > > > > > But, eal_parse_sysfs_file_value is still preferred than > > eal_parse_sysfs_fd_value, for me. > > > > Agree. > > > [...] I've finally returned to your idea to name it eal_parse_sysfs_value_read. Thanks. Jan > > - > Shreyansh -- Jan Viktorin E-mail: Viktorin at RehiveTech.com System Architect Web:www.RehiveTech.com RehiveTech Brno, Czech Republic
[dpdk-dev] [PATCH] bnx2x: Call bnx2x_init_rte() later
On Thu, Jun 30, 2016 at 08:01:24PM +, Rasesh Mody wrote: > > From: Bruce Richardson [mailto:bruce.richardson at intel.com] > > Sent: Wednesday, June 29, 2016 3:59 AM > > > > On Tue, Jun 28, 2016 at 12:14:50PM -0400, Chas Williams wrote: > > > We need sc->igu_sb_cnt determined before calculating the number of > > > queues we can support. > > > > > > Fixes: a787538ee754 ("bnx2x: fix MSIX vector and VF resource counts") > > > > > > Signed-off-by: Chas Williams <3chas3 at gmail.com> > > > --- > > > > Hi Chas, > > > > we generally try to avoid making the commit titles too technical, so we > > avoid > > using function or variable names in the titles. Since this is a bug fix the > > best > > form of title to be used is "fix ", e.g. > > "fix incorrect number of supported queues". > > > > Maintainers, any comments or ack for this patch. > > > > /Bruce > > Acked-by: Rasesh Mody > > Thanks! > Rasesh Applied to dpdk_next_net/rel_16_07 with title "net/bnx2x: fix incorrect number of supported queues". /Bruce
[dpdk-dev] [PATCH v3] i40e: fix VLAN filter in promiscuous mode
On Mon, Jul 04, 2016 at 06:33:30AM +, Zhang, Helin wrote: > > > > -Original Message- > > From: Wu, Jingjing > > Sent: Thursday, June 30, 2016 9:26 AM > > To: Zhang, Helin > > Cc: dev at dpdk.org; Wu, Jingjing; Yigit, Ferruh > > Subject: [PATCH v3] i40e: fix VLAN filter in promiscuous mode > > > > For VLAN filtering VLAN table should be enabled. > > But VLAN table is disabled by default until a rule added. In promiscuous > > mode > > no rule is added to enable the VLAN table. > > > > So this patch clears promiscuous VLAN flag on VSI, and adds a rule to enable > > VLAN table to fix VLAN filtering in promiscuous mode. > > > > Fixes: 4861cde46116 ("i40e: new poll mode driver") > > Signed-off-by: Jingjing Wu > Acked-by: Helin Zhang Applied to dpdk-next-net/rel_16_07 /Bruce
[dpdk-dev] [PATCH v3] hash: new function to retrieve a key given its position
Sorry, didn't mentioned the changes introduced in v3: *) solves some tabs for whitespaces issues *) references the right DPDK version *) reduces the size of the rte_hash_create() argument in the unit tests Thanks!! On 07/04/2016 10:59 AM, Yari Adan Petralanda wrote: > The function rte_hash_get_key_with_position is added in this patch. > As the position returned when adding a key is frequently used as an > offset into an array of user data, this function performs the operation > of retrieving a key given this offset. > > A possible use case would be to delete a key from the hash table when > its entry in the array of data has certain value. For instance, the key > could be a flow 5-tuple, and the value stored in the array a time > stamp. > > Signed-off-by: Juan Antonio Montesinos ericsson.com> > Signed-off-by: Yari Adan Petralanda > --- > app/test/test_hash.c | 42 > > lib/librte_hash/rte_cuckoo_hash.c| 20 + > lib/librte_hash/rte_hash.h | 18 > lib/librte_hash/rte_hash_version.map | 7 ++ > 4 files changed, 87 insertions(+) > > diff --git a/app/test/test_hash.c b/app/test/test_hash.c > index 7e41725..29abcd9 100644 > --- a/app/test/test_hash.c > +++ b/app/test/test_hash.c > @@ -421,6 +421,46 @@ static int test_add_update_delete(void) > } > > /* > + * Sequence of operations for retrieving a key with its position > + * > + * - create table > + * - add key > + * - get the key with its position: hit > + * - delete key > + * - try to get the deleted key: miss > + * > + */ > +static int test_hash_get_key_with_position(void) > +{ > + struct rte_hash *handle = NULL; > + int pos, expectedPos, result; > + void *key; > + > + ut_params.name = "hash_get_key_w_pos"; > + handle = rte_hash_create(&ut_params); > + RETURN_IF_ERROR(handle == NULL, "hash creation failed"); > + > + pos = rte_hash_add_key(handle, &keys[0]); > + print_key_info("Add", &keys[0], pos); > + RETURN_IF_ERROR(pos < 0, "failed to add key (pos0=%d)", pos); > + expectedPos = pos; > + > + result = rte_hash_get_key_with_position(handle, pos, &key); > + RETURN_IF_ERROR(result != 0, "error retrieving a key"); > + > + pos = rte_hash_del_key(handle, &keys[0]); > + print_key_info("Del", &keys[0], pos); > + RETURN_IF_ERROR(pos != expectedPos, > + "failed to delete key (pos0=%d)", pos); > + > + result = rte_hash_get_key_with_position(handle, pos, &key); > + RETURN_IF_ERROR(result != -ENOENT, "non valid key retrieved"); > + > + rte_hash_free(handle); > + return 0; > +} > + > +/* > * Sequence of operations for find existing hash table > * > * - create table > @@ -1442,6 +1482,8 @@ test_hash(void) > return -1; > if (test_hash_add_delete_jhash_3word() < 0) > return -1; > + if (test_hash_get_key_with_position() < 0) > + return -1; > if (test_hash_find_existing() < 0) > return -1; > if (test_add_update_delete() < 0) > diff --git a/lib/librte_hash/rte_cuckoo_hash.c > b/lib/librte_hash/rte_cuckoo_hash.c > index 7b7d1f8..a3c9091 100644 > --- a/lib/librte_hash/rte_cuckoo_hash.c > +++ b/lib/librte_hash/rte_cuckoo_hash.c > @@ -973,6 +973,26 @@ rte_hash_del_key(const struct rte_hash *h, const void > *key) > return __rte_hash_del_key_with_hash(h, key, rte_hash_hash(h, key)); > } > > +int > +rte_hash_get_key_with_position(const struct rte_hash *h, const int32_t > position, > +void **key) > +{ > + RETURN_IF_TRUE(((h == NULL) || (key == NULL)), -EINVAL); > + > + struct rte_hash_key *k, *keys = h->key_store; > + k = (struct rte_hash_key *) ((char *) keys + (position + 1) * > + h->key_entry_size); > + *key = k->key; > + > + if (position != > + __rte_hash_lookup_with_hash(h, *key, rte_hash_hash(h, *key), > + NULL)) { > + return -ENOENT; > + } > + > + return 0; > +} > + > /* Lookup bulk stage 0: Prefetch input key */ > static inline void > lookup_stage0(unsigned *idx, uint64_t *lookup_mask, > diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h > index 724315a..78af365 100644 > --- a/lib/librte_hash/rte_hash.h > +++ b/lib/librte_hash/rte_hash.h > @@ -268,6 +268,24 @@ rte_hash_del_key(const struct rte_hash *h, const void > *key); > int32_t > rte_hash_del_key_with_hash(const struct rte_hash *h, const void *key, > hash_sig_t sig); > > +/** > + * Find a key in the hash table given the position. > + * This operation is multi-thread safe. > + * > + * @param h > + * Hash table to get the key from. > + * @param position > + * Position returned when the key was inserted. > + * @param key > + * Output containing a pointer to the key > + * @return > + * - 0 if retrieved successfully > + * - EINVAL if the param
[dpdk-dev] [PATCH 0/4] i40e: fix coverity defects
On Thu, Jun 30, 2016 at 03:34:16PM +0800, Beilei Xing wrote: > Fix some open coverity defects. > > Beilei Xing (4): > i40e: fix wrong operator > i40e: fix dereference before null check > i40e: fix out-of-bounds access > examples/tep_term: fix out-of-bounds access > > drivers/net/i40e/i40e_ethdev.c | 35 > -- > drivers/net/i40e/i40e_ethdev_vf.c | 7 +-- > examples/tep_termination/vxlan_setup.c | 2 +- > 3 files changed, 27 insertions(+), 17 deletions(-) > > -- Hi Beilei, thanks for the patches. Since these are coverity fixes, can you please do a V2 with the coverity id's being fixed called out in the commit message. The format to use is: Coverity issue: x y just above the fixes line in the patch. [Use "git log" to see examples in other commits]. Also, for convenience sake for myself and Thomas, can you include just the three i40e patches in one set - since those go to next-net tree - and submit the examples fix as a separate standalone patch to go in the main tree. Thanks, /Bruce
[dpdk-dev] [PATCH v1 28/28] ether: support SoC device/driver
On Monday 04 July 2016 06:34 PM, Jan Viktorin wrote: > On Wed, 29 Jun 2016 15:12:07 +0530 > Shreyansh jain wrote: > >> Hi Jan, >> >> On Friday 06 May 2016 07:18 PM, Jan Viktorin wrote: >>> Signed-off-by: Jan Viktorin >>> --- >>> lib/librte_ether/rte_ethdev.c | 127 >>> +- >>> lib/librte_ether/rte_ethdev.h | 31 +++ >>> 2 files changed, 157 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c >>> index 4af2e5f..9259c2c 100644 >>> --- a/lib/librte_ether/rte_ethdev.c >>> +++ b/lib/librte_ether/rte_ethdev.c >>> @@ -320,6 +320,99 @@ rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev) >>> } >>> >> [...] >>> +int >>> rte_eth_dev_is_valid_port(uint8_t port_id) >>> { >>> if (port_id >= RTE_MAX_ETHPORTS || >>> @@ -1431,7 +1524,7 @@ rte_eth_dev_info_get(uint8_t port_id, struct >>> rte_eth_dev_info *dev_info) >>> >>> RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get); >>> (*dev->dev_ops->dev_infos_get)(dev, dev_info); >>> - dev_info->pci_dev = dev->pci_dev; >>> + dev_info->soc_dev = dev->soc_dev; >> >> I think both the members, pci_dev and soc_dev, should be updated by this >> call. >> Is there some specific reason why soc_dev is the only one which is getting >> updated? > > Yes, looks like a mistake. Thanks! And sorry for delayed reply. No problems - thanks for confirmation. I have gone through almost complete series and as and when you rebase it, it would have my ACK. rte_driver patchset which I sent last are broken - I will publish an updated version very soon. > > Jan > >> >>> dev_info->driver_name = dev->data->drv_name; >>> } >>> >> [...] >> >> - >> Shreyansh >> > > > - Shreyansh
[dpdk-dev] Low packet generation rate of 526kpps using pktgen-dpdk from inside VM
-Original Message- From: dev on behalf of Abhishek Mahajan Date: Monday, July 4, 2016 at 7:27 AM To: "dev at dpdk.org" Cc: "Addepalli, Srinivasa R" , "Shivastava, RakeshX" Subject: [dpdk-dev] Low packet generation rate of 526kpps using pktgen-dpdk from inside VM Hi, With pktgen-dpdk inside VM, it is observed that packet TX rate (526Kpps with 64byte size) is very low. How can higher packet rates be achieved ? On enabling logs of virtio pmd in DPDK, getting following logs in syslogs: PMD: virtio_xmit_pkts() tx: No free tx descriptors to transmit Two PC setup having configuration as follow: -> Xeon Processor : Intel(R) Xeon(R) CPU E5-2603 v3 @ 1.60GHz having 6 cores. -> RAM : 16GB ->One PC is generating traffic other is receiving the packets using pktgen tool. The two PC are connected directly with 10G Ethernet interface. Generation side PC - Host -> 2 cores dedicated to OVS-DPDK -> 1 core is free for Linux kernel -> OS: Ubuntu 14.04 -> Linux kernel : 3.13.0-85-generic -> DPDK version : 2.2.0 -> OVS version : 2.5.0 -> VFIO driver for port binding VM - Total 3 cores -> 2 cores for pktgen -> 1 core for Linux kernel -> OS: Ubuntu 15.04 -> Linux kernel : 3.19.8 -> DPDK version : 16.04 -> Pktgen version : 3.0.00 -> UIO driver for port binding DPDK, OVS and pktgen are build using - x86_64-ivshmen-linuxapp-gcc option. VM launched through following Qemu command : * qemu-system-x86_64 --enable-kvm -smp 3 -cpu host -m 2048 -chardev socket,id=charnet0,path=/var/run/openvswitch/vhost-user1 -netdev type=vhost-user,id=hostnet0,chardev=charnet0,vhostforce=on -device virtio-net-pci,netdev=hostnet0,mac=16:fa:13:26:31:1b -object memory-backend-file,id=mem,size=2048M,mem-path=/dev/hugepages,share=on -numa node,memdev=mem -mem-prealloc -hda /home/aricent/ubuntu_15_04.img Pktgen is launched using following command: Pktgen -c -0x3 -n 4 -m 512 -b 000:00:04.0 - p 0x3 -t -m "[1:1].0" KW: I do not think this is Pktgen problem, but please try pktgen ?c 0x6 ?n 4 ?m 512 ?b :00:04.0 -- -T ?m?[2:2].0? Core 0 needs to be left alone for Linux and core 1 is used for Pktgen display and core 2 is used for Rx/Tx on port 0. OVS command used: * /usr/sbin/ovs-vswitchd --dpdk -c 0xc -n 4 -m 2048 -- unix:/var/run/openvswitch/db.sock -vconsole:emer -vsyslog:err -vfile:info --mlockall --no-chdir --log-file=/var/log/openvswitch/ovs-vswitchd.log --pidfile=/var/run/openvswitch/ovs-vswitchd.pid --detach -monitor Thanks & Regards Abhishek "DISCLAIMER: This message is proprietary to Aricent and is intended solely for the use of the individual to whom it is addressed. It may contain privileged or confidential information and should not be circulated or used for any purpose other than for what it is intended. If you have received this message in error, please notify the originator immediately. If you are not the intended recipient, you are notified that you are strictly prohibited from using, copying, altering, or disclosing the contents of this message. Aricent accepts no responsibility for loss or damage arising from the use of the information transmitted by this email including damage from virus."
[dpdk-dev] [PATCH v1 28/28] ether: support SoC device/driver
On Mon, 4 Jul 2016 19:57:18 +0530 Shreyansh jain wrote: [...] > >>> @@ -1431,7 +1524,7 @@ rte_eth_dev_info_get(uint8_t port_id, struct > >>> rte_eth_dev_info *dev_info) > >>> > >>> RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get); > >>> (*dev->dev_ops->dev_infos_get)(dev, dev_info); > >>> - dev_info->pci_dev = dev->pci_dev; > >>> + dev_info->soc_dev = dev->soc_dev; > >> > >> I think both the members, pci_dev and soc_dev, should be updated by this > >> call. > >> Is there some specific reason why soc_dev is the only one which is getting > >> updated? > > > > Yes, looks like a mistake. Thanks! And sorry for delayed reply. > > No problems - thanks for confirmation. > I have gone through almost complete series and as and when you rebase it, it > would have my ACK. OK, thanks. That's what I am playing with right now. I've rebased on v3 of this patch. There will be some more tests in my v2. > rte_driver patchset which I sent last are broken - I will publish an updated > version very soon. I am surprised that you've changed the args to RTE_EAL_PCI_REGISTER... Are you sure about this step? I wrote that I'll change it myself for v2 for SoC to accept name and pointer as it was originally for PCI... Jan
[dpdk-dev] [PATCH] doc: announce API change for virtual device initialization
In order to create a virtual device, user needs to call rte_eal_vdev_init generally, but this function returns 0 on success or negative number if error. Instead, something more useful would be to return the port or device id of the device created, so the user can call rte_eal_vdev_init function and use the device straight away, using the id returned by the function. Signed-off-by: Pablo de Lara --- doc/guides/rel_notes/deprecation.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index f502f86..60cc7c7 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -41,3 +41,8 @@ Deprecation Notices * The mempool functions for single/multi producer/consumer are deprecated and will be removed in 16.11. It is replaced by rte_mempool_generic_get/put functions. + +* The rte_eal_vdev_init function will be changed in 16.11 to return + the port/device id of the device created, instead of 0, when it has been + initialized successfully, so user can use the returned value straight away + to call all the device functions that require that parameter. -- 2.5.0
[dpdk-dev] [PATCH v2 01/16] vfio: fix include of eal_private.h to be local
On Mon, 4 Jul 2016 10:22:08 + "Burakov, Anatoly" wrote: [...] > There's no patch cover letter so I'll reply to the first patch. I've done > some cursory testing with a NIC, nothing seems to be broken and the code > looks OK to me. So, once this patchset is rebased on latest master (patches > 9, 14 and 15 no longer apply cleanly), ack from me. I am going to post v3. Should I include the ACKs on all patches or will you post them yourself? Jan > > Thanks, > Anatoly
[dpdk-dev] [PATCH v2 01/16] vfio: fix include of eal_private.h to be local
> -Original Message- > From: Jan Viktorin [mailto:viktorin at rehivetech.com] > Sent: Monday, July 4, 2016 4:01 PM > To: Burakov, Anatoly > Cc: dev at dpdk.org; David Marchand ; Wiles, > Keith ; Santosh Shukla ; > Stephen Hemminger ; Shreyansh Jain > > Subject: Re: [PATCH v2 01/16] vfio: fix include of eal_private.h to be local > > On Mon, 4 Jul 2016 10:22:08 + > "Burakov, Anatoly" wrote: > > [...] > > > There's no patch cover letter so I'll reply to the first patch. I've done > > some > cursory testing with a NIC, nothing seems to be broken and the code looks > OK to me. So, once this patchset is rebased on latest master (patches 9, 14 > and 15 no longer apply cleanly), ack from me. > > I am going to post v3. Should I include the ACKs on all patches or will you > post > them yourself? > I'll test the patchset again and ack the patchset. Thanks!
[dpdk-dev] [PATCH v3 00/16] Make VFIO support less dependent on PCI
Hello, I've rebased the v2 of this patch set on top of the current master. It builds well for my setup (both VFIO enabled and disabled). Regards Jan v3: * 0012: Acked-by: John McNamara Jan Viktorin (16): vfio: fix include of eal_private.h to be local vfio: move VFIO-specific stuff to eal_vfio.h vfio: move common vfio constants to eal_vfio.h vfio: move vfio_iommu_type and dma_map functions to eal_vfio vfio: generalize pci_vfio_set_iommu_type vfio: generalize pci_vfio_has_supported_extensions vfio: move vfio-specific SOCKET_* constants vfio: generalize pci_vfio_get_container_fd vfio: generalize pci_vfio_get_group_no vfio: extract setup logic out of pci_vfio_map_resource vfio: move global vfio_cfg to eal_vfio.c vfio: fix typo in doc for vfio_setup_device vfio: make vfio_*_dma_map and iommu_types private vfio: rename and generalize eal_pci_vfio_mp_sync vfio: initialize vfio out of the PCI subsystem vfio: change VFIO init to be extendable lib/librte_eal/linuxapp/eal/Makefile | 5 +- lib/librte_eal/linuxapp/eal/eal.c | 33 ++ lib/librte_eal/linuxapp/eal/eal_pci.c | 17 +- lib/librte_eal/linuxapp/eal/eal_pci_init.h | 41 -- lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 517 +-- lib/librte_eal/linuxapp/eal/eal_vfio.c | 547 + lib/librte_eal/linuxapp/eal/eal_vfio.h | 94 .../{eal_pci_vfio_mp_sync.c => eal_vfio_mp_sync.c} | 12 +- 8 files changed, 691 insertions(+), 575 deletions(-) create mode 100644 lib/librte_eal/linuxapp/eal/eal_vfio.c rename lib/librte_eal/linuxapp/eal/{eal_pci_vfio_mp_sync.c => eal_vfio_mp_sync.c} (97%) -- 2.8.0
[dpdk-dev] [PATCH v3 01/16] vfio: fix include of eal_private.h to be local
Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index f91b924..8b7d53f 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -43,11 +43,11 @@ #include #include #include -#include #include "eal_filesystem.h" #include "eal_pci_init.h" #include "eal_vfio.h" +#include "eal_private.h" /** * @file -- 2.8.0
[dpdk-dev] [PATCH v3 02/16] vfio: move VFIO-specific stuff to eal_vfio.h
The common VFIO definitions should be separated from the PCI-specific parts. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal_pci_init.h | 28 lib/librte_eal/linuxapp/eal/eal_vfio.h | 28 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h index f72a254..75b8ed3 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h +++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h @@ -74,8 +74,6 @@ int pci_uio_ioport_unmap(struct rte_pci_ioport *p); #ifdef VFIO_PRESENT -#define VFIO_MAX_GROUPS 64 - int pci_vfio_enable(void); int pci_vfio_is_enabled(void); int pci_vfio_mp_sync_setup(void); @@ -99,15 +97,6 @@ int pci_vfio_map_resource(struct rte_pci_device *dev); int pci_vfio_get_group_fd(int iommu_group_fd); int pci_vfio_get_container_fd(void); -/* - * Function prototypes for VFIO multiprocess sync functions - */ -int vfio_mp_sync_send_request(int socket, int req); -int vfio_mp_sync_receive_request(int socket); -int vfio_mp_sync_send_fd(int socket, int fd); -int vfio_mp_sync_receive_fd(int socket); -int vfio_mp_sync_connect_to_primary(void); - /* socket comm protocol definitions */ #define SOCKET_REQ_CONTAINER 0x100 #define SOCKET_REQ_GROUP 0x200 @@ -115,23 +104,6 @@ int vfio_mp_sync_connect_to_primary(void); #define SOCKET_NO_FD 0x1 #define SOCKET_ERR 0xFF -/* - * we don't need to store device fd's anywhere since they can be obtained from - * the group fd via an ioctl() call. - */ -struct vfio_group { - int group_no; - int fd; -}; - -struct vfio_config { - int vfio_enabled; - int vfio_container_fd; - int vfio_container_has_dma; - int vfio_group_idx; - struct vfio_group vfio_groups[VFIO_MAX_GROUPS]; -}; - #endif #endif /* EAL_PCI_INIT_H_ */ diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h index f483bf4..cedbeb0 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h @@ -60,6 +60,34 @@ #define RTE_VFIO_NOIOMMU VFIO_NOIOMMU_IOMMU #endif +#define VFIO_MAX_GROUPS 64 + +/* + * Function prototypes for VFIO multiprocess sync functions + */ +int vfio_mp_sync_send_request(int socket, int req); +int vfio_mp_sync_receive_request(int socket); +int vfio_mp_sync_send_fd(int socket, int fd); +int vfio_mp_sync_receive_fd(int socket); +int vfio_mp_sync_connect_to_primary(void); + +/* + * we don't need to store device fd's anywhere since they can be obtained from + * the group fd via an ioctl() call. + */ +struct vfio_group { + int group_no; + int fd; +}; + +struct vfio_config { + int vfio_enabled; + int vfio_container_fd; + int vfio_container_has_dma; + int vfio_group_idx; + struct vfio_group vfio_groups[VFIO_MAX_GROUPS]; +}; + #define VFIO_PRESENT #endif /* kernel version */ #endif /* RTE_EAL_VFIO */ -- 2.8.0
[dpdk-dev] [PATCH v3 03/16] vfio: move common vfio constants to eal_vfio.h
Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 7 --- lib/librte_eal/linuxapp/eal/eal_vfio.h | 7 +++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index 8b7d53f..c8f9ec1 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -69,13 +69,6 @@ static struct rte_tailq_elem rte_vfio_tailq = { }; EAL_REGISTER_TAILQ(rte_vfio_tailq) -#define VFIO_DIR "/dev/vfio" -#define VFIO_CONTAINER_PATH "/dev/vfio/vfio" -#define VFIO_GROUP_FMT "/dev/vfio/%u" -#define VFIO_NOIOMMU_GROUP_FMT "/dev/vfio/noiommu-%u" -#define VFIO_GET_REGION_ADDR(x) ((uint64_t) x << 40ULL) -#define VFIO_GET_REGION_IDX(x) (x >> 40) - /* per-process VFIO config */ static struct vfio_config vfio_cfg; diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h index cedbeb0..bcf6860 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h @@ -88,6 +88,13 @@ struct vfio_config { struct vfio_group vfio_groups[VFIO_MAX_GROUPS]; }; +#define VFIO_DIR "/dev/vfio" +#define VFIO_CONTAINER_PATH "/dev/vfio/vfio" +#define VFIO_GROUP_FMT "/dev/vfio/%u" +#define VFIO_NOIOMMU_GROUP_FMT "/dev/vfio/noiommu-%u" +#define VFIO_GET_REGION_ADDR(x) ((uint64_t) x << 40ULL) +#define VFIO_GET_REGION_IDX(x) (x >> 40) + #define VFIO_PRESENT #endif /* kernel version */ #endif /* RTE_EAL_VFIO */ -- 2.8.0
[dpdk-dev] [PATCH v3 04/16] vfio: move vfio_iommu_type and dma_map functions to eal_vfio
We make the iommu_types public temporarily here until the depending stuff is refactored. The iommu_types and dma_map functions will be changed to be private inside the eal_vfio module later. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/Makefile | 1 + lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 62 --- lib/librte_eal/linuxapp/eal/eal_vfio.c | 79 ++ lib/librte_eal/linuxapp/eal/eal_vfio.h | 23 + 4 files changed, 103 insertions(+), 62 deletions(-) create mode 100644 lib/librte_eal/linuxapp/eal/eal_vfio.c diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile index 30b30f3..517554f 100644 --- a/lib/librte_eal/linuxapp/eal/Makefile +++ b/lib/librte_eal/linuxapp/eal/Makefile @@ -66,6 +66,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_xen_memory.c endif SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_thread.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_log.c +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_vfio.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_pci.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_pci_uio.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_pci_vfio.c diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index c8f9ec1..c52ea37 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -72,68 +72,6 @@ EAL_REGISTER_TAILQ(rte_vfio_tailq) /* per-process VFIO config */ static struct vfio_config vfio_cfg; -/* DMA mapping function prototype. - * Takes VFIO container fd as a parameter. - * Returns 0 on success, -1 on error. - * */ -typedef int (*vfio_dma_func_t)(int); - -struct vfio_iommu_type { - int type_id; - const char *name; - vfio_dma_func_t dma_map_func; -}; - -static int vfio_type1_dma_map(int); -static int vfio_noiommu_dma_map(int); - -/* IOMMU types we support */ -static const struct vfio_iommu_type iommu_types[] = { - /* x86 IOMMU, otherwise known as type 1 */ - { RTE_VFIO_TYPE1, "Type 1", &vfio_type1_dma_map}, - /* IOMMU-less mode */ - { RTE_VFIO_NOIOMMU, "No-IOMMU", &vfio_noiommu_dma_map}, -}; - -int -vfio_type1_dma_map(int vfio_container_fd) -{ - const struct rte_memseg *ms = rte_eal_get_physmem_layout(); - int i, ret; - - /* map all DPDK segments for DMA. use 1:1 PA to IOVA mapping */ - for (i = 0; i < RTE_MAX_MEMSEG; i++) { - struct vfio_iommu_type1_dma_map dma_map; - - if (ms[i].addr == NULL) - break; - - memset(&dma_map, 0, sizeof(dma_map)); - dma_map.argsz = sizeof(struct vfio_iommu_type1_dma_map); - dma_map.vaddr = ms[i].addr_64; - dma_map.size = ms[i].len; - dma_map.iova = ms[i].phys_addr; - dma_map.flags = VFIO_DMA_MAP_FLAG_READ | VFIO_DMA_MAP_FLAG_WRITE; - - ret = ioctl(vfio_container_fd, VFIO_IOMMU_MAP_DMA, &dma_map); - - if (ret) { - RTE_LOG(ERR, EAL, " cannot set up DMA remapping, " - "error %i (%s)\n", errno, strerror(errno)); - return -1; - } - } - - return 0; -} - -int -vfio_noiommu_dma_map(int __rte_unused vfio_container_fd) -{ - /* No-IOMMU mode does not need DMA mapping */ - return 0; -} - int pci_vfio_read_config(const struct rte_intr_handle *intr_handle, void *buf, size_t len, off_t offs) diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c new file mode 100644 index 000..d3ffebe --- /dev/null +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c @@ -0,0 +1,79 @@ +/*- + * BSD LICENSE + * + * Copyright(c) 2010-2014 Intel Corporation. All rights reserved. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OW
[dpdk-dev] [PATCH v3 05/16] vfio: generalize pci_vfio_set_iommu_type
The pci_vfio_set_iommu_type is not PCI-specific and it is a private function of the eal_pci_vfio.c. We just rename the function and make it available even for non-PCI devices. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 25 + lib/librte_eal/linuxapp/eal/eal_vfio.c | 22 ++ lib/librte_eal/linuxapp/eal/eal_vfio.h | 4 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index c52ea37..5fdf23f 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -203,29 +203,6 @@ pci_vfio_set_bus_master(int dev_fd) return 0; } -/* pick IOMMU type. returns a pointer to vfio_iommu_type or NULL for error */ -static const struct vfio_iommu_type * -pci_vfio_set_iommu_type(int vfio_container_fd) { - unsigned idx; - for (idx = 0; idx < RTE_DIM(iommu_types); idx++) { - const struct vfio_iommu_type *t = &iommu_types[idx]; - - int ret = ioctl(vfio_container_fd, VFIO_SET_IOMMU, - t->type_id); - if (!ret) { - RTE_LOG(NOTICE, EAL, " using IOMMU type %d (%s)\n", - t->type_id, t->name); - return t; - } - /* not an error, there may be more supported IOMMU types */ - RTE_LOG(DEBUG, EAL, " set IOMMU type %d (%s) failed, " - "error %i (%s)\n", t->type_id, t->name, errno, - strerror(errno)); - } - /* if we didn't find a suitable IOMMU type, fail */ - return NULL; -} - /* check if we have any supported extensions */ static int pci_vfio_has_supported_extensions(int vfio_container_fd) { @@ -689,7 +666,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev) vfio_cfg.vfio_container_has_dma == 0) { /* select an IOMMU type which we will be using */ const struct vfio_iommu_type *t = - pci_vfio_set_iommu_type(vfio_cfg.vfio_container_fd); + vfio_set_iommu_type(vfio_cfg.vfio_container_fd); if (!t) { RTE_LOG(ERR, EAL, " %s failed to select IOMMU type\n", pci_addr); return -1; diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c index d3ffebe..ff85283 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c @@ -39,6 +39,28 @@ #include "eal_vfio.h" +const struct vfio_iommu_type * +vfio_set_iommu_type(int vfio_container_fd) { + unsigned idx; + for (idx = 0; idx < RTE_DIM(iommu_types); idx++) { + const struct vfio_iommu_type *t = &iommu_types[idx]; + + int ret = ioctl(vfio_container_fd, VFIO_SET_IOMMU, + t->type_id); + if (!ret) { + RTE_LOG(NOTICE, EAL, " using IOMMU type %d (%s)\n", + t->type_id, t->name); + return t; + } + /* not an error, there may be more supported IOMMU types */ + RTE_LOG(DEBUG, EAL, " set IOMMU type %d (%s) failed, " + "error %i (%s)\n", t->type_id, t->name, errno, + strerror(errno)); + } + /* if we didn't find a suitable IOMMU type, fail */ + return NULL; +} + int vfio_type1_dma_map(int vfio_container_fd) { diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h index c62f269..afbb98a 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h @@ -107,6 +107,10 @@ struct vfio_iommu_type { vfio_dma_func_t dma_map_func; }; +/* pick IOMMU type. returns a pointer to vfio_iommu_type or NULL for error */ +const struct vfio_iommu_type * +vfio_set_iommu_type(int vfio_container_fd); + int vfio_type1_dma_map(int); int vfio_noiommu_dma_map(int); -- 2.8.0
[dpdk-dev] [PATCH v3 06/16] vfio: generalize pci_vfio_has_supported_extensions
The pci_vfio_has_supported_extensions is not PCI-specific and it is a private function of the eal_pci_vfio.c. We just rename the function and make it available even for non-PCI devices. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 36 +- lib/librte_eal/linuxapp/eal/eal_vfio.c | 33 +++ lib/librte_eal/linuxapp/eal/eal_vfio.h | 4 3 files changed, 38 insertions(+), 35 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index 5fdf23f..f93db4a 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -203,40 +203,6 @@ pci_vfio_set_bus_master(int dev_fd) return 0; } -/* check if we have any supported extensions */ -static int -pci_vfio_has_supported_extensions(int vfio_container_fd) { - int ret; - unsigned idx, n_extensions = 0; - for (idx = 0; idx < RTE_DIM(iommu_types); idx++) { - const struct vfio_iommu_type *t = &iommu_types[idx]; - - ret = ioctl(vfio_container_fd, VFIO_CHECK_EXTENSION, - t->type_id); - if (ret < 0) { - RTE_LOG(ERR, EAL, " could not get IOMMU type, " - "error %i (%s)\n", errno, - strerror(errno)); - close(vfio_container_fd); - return -1; - } else if (ret == 1) { - /* we found a supported extension */ - n_extensions++; - } - RTE_LOG(DEBUG, EAL, " IOMMU type %d (%s) is %s\n", - t->type_id, t->name, - ret ? "supported" : "not supported"); - } - - /* if we didn't find any supported IOMMU types, fail */ - if (!n_extensions) { - close(vfio_container_fd); - return -1; - } - - return 0; -} - /* set up interrupt support (but not enable interrupts) */ static int pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd) @@ -360,7 +326,7 @@ pci_vfio_get_container_fd(void) return -1; } - ret = pci_vfio_has_supported_extensions(vfio_container_fd); + ret = vfio_has_supported_extensions(vfio_container_fd); if (ret) { RTE_LOG(ERR, EAL, " no supported IOMMU " "extensions found!\n"); diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c index ff85283..6a95d2a 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c @@ -62,6 +62,39 @@ vfio_set_iommu_type(int vfio_container_fd) { } int +vfio_has_supported_extensions(int vfio_container_fd) { + int ret; + unsigned idx, n_extensions = 0; + for (idx = 0; idx < RTE_DIM(iommu_types); idx++) { + const struct vfio_iommu_type *t = &iommu_types[idx]; + + ret = ioctl(vfio_container_fd, VFIO_CHECK_EXTENSION, + t->type_id); + if (ret < 0) { + RTE_LOG(ERR, EAL, " could not get IOMMU type, " + "error %i (%s)\n", errno, + strerror(errno)); + close(vfio_container_fd); + return -1; + } else if (ret == 1) { + /* we found a supported extension */ + n_extensions++; + } + RTE_LOG(DEBUG, EAL, " IOMMU type %d (%s) is %s\n", + t->type_id, t->name, + ret ? "supported" : "not supported"); + } + + /* if we didn't find any supported IOMMU types, fail */ + if (!n_extensions) { + close(vfio_container_fd); + return -1; + } + + return 0; +} + +int vfio_type1_dma_map(int vfio_container_fd) { const struct rte_memseg *ms = rte_eal_get_physmem_layout(); diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h index afbb98a..8cb0d1d 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h @@ -111,6 +111,10 @@ struct vfio_iommu_type { const struct vfio_iommu_type * vfio_set_iommu_type(int vfio_container_fd); +/* check if we have any supported extensions */ +int +vfio_has_supported_extensions(int vfio_container_fd); + int vfio_type1_dma_map(int); int vfio_noiommu_dma_map(int); -- 2.8.0
[dpdk-dev] [PATCH v3 07/16] vfio: move vfio-specific SOCKET_* constants
The constants are not PCI-specific. Move them into the eal_vfio.h. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal_pci_init.h | 7 --- lib/librte_eal/linuxapp/eal/eal_vfio.h | 6 ++ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h index 75b8ed3..883787f 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h +++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h @@ -97,13 +97,6 @@ int pci_vfio_map_resource(struct rte_pci_device *dev); int pci_vfio_get_group_fd(int iommu_group_fd); int pci_vfio_get_container_fd(void); -/* socket comm protocol definitions */ -#define SOCKET_REQ_CONTAINER 0x100 -#define SOCKET_REQ_GROUP 0x200 -#define SOCKET_OK 0x0 -#define SOCKET_NO_FD 0x1 -#define SOCKET_ERR 0xFF - #endif #endif /* EAL_PCI_INIT_H_ */ diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h index 8cb0d1d..121df0a 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h @@ -115,6 +115,12 @@ vfio_set_iommu_type(int vfio_container_fd); int vfio_has_supported_extensions(int vfio_container_fd); +#define SOCKET_REQ_CONTAINER 0x100 +#define SOCKET_REQ_GROUP 0x200 +#define SOCKET_OK 0x0 +#define SOCKET_NO_FD 0x1 +#define SOCKET_ERR 0xFF + int vfio_type1_dma_map(int); int vfio_noiommu_dma_map(int); -- 2.8.0
[dpdk-dev] [PATCH v3 08/16] vfio: generalize pci_vfio_get_container_fd
The pci_vfio_get_container_fd is not PCI-specific. Move the implementation to the eal_vfio.c as vfio_get_container_fd. No other code seems to call this function. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal_pci_init.h | 1 - lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 67 +--- lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c | 2 +- lib/librte_eal/linuxapp/eal/eal_vfio.c | 72 ++ lib/librte_eal/linuxapp/eal/eal_vfio.h | 4 ++ 5 files changed, 78 insertions(+), 68 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h index 883787f..d34212b 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h +++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h @@ -95,7 +95,6 @@ int pci_vfio_ioport_unmap(struct rte_pci_ioport *p); /* map VFIO resource prototype */ int pci_vfio_map_resource(struct rte_pci_device *dev); int pci_vfio_get_group_fd(int iommu_group_fd); -int pci_vfio_get_container_fd(void); #endif diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index f93db4a..528479a 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -299,71 +299,6 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd) return -1; } -/* open container fd or get an existing one */ -int -pci_vfio_get_container_fd(void) -{ - int ret, vfio_container_fd; - - /* if we're in a primary process, try to open the container */ - if (internal_config.process_type == RTE_PROC_PRIMARY) { - vfio_container_fd = open(VFIO_CONTAINER_PATH, O_RDWR); - if (vfio_container_fd < 0) { - RTE_LOG(ERR, EAL, " cannot open VFIO container, " - "error %i (%s)\n", errno, strerror(errno)); - return -1; - } - - /* check VFIO API version */ - ret = ioctl(vfio_container_fd, VFIO_GET_API_VERSION); - if (ret != VFIO_API_VERSION) { - if (ret < 0) - RTE_LOG(ERR, EAL, " could not get VFIO API version, " - "error %i (%s)\n", errno, strerror(errno)); - else - RTE_LOG(ERR, EAL, " unsupported VFIO API version!\n"); - close(vfio_container_fd); - return -1; - } - - ret = vfio_has_supported_extensions(vfio_container_fd); - if (ret) { - RTE_LOG(ERR, EAL, " no supported IOMMU " - "extensions found!\n"); - return -1; - } - - return vfio_container_fd; - } else { - /* -* if we're in a secondary process, request container fd from the -* primary process via our socket -*/ - int socket_fd; - - socket_fd = vfio_mp_sync_connect_to_primary(); - if (socket_fd < 0) { - RTE_LOG(ERR, EAL, " cannot connect to primary process!\n"); - return -1; - } - if (vfio_mp_sync_send_request(socket_fd, SOCKET_REQ_CONTAINER) < 0) { - RTE_LOG(ERR, EAL, " cannot request container fd!\n"); - close(socket_fd); - return -1; - } - vfio_container_fd = vfio_mp_sync_receive_fd(socket_fd); - if (vfio_container_fd < 0) { - RTE_LOG(ERR, EAL, " cannot get container fd!\n"); - close(socket_fd); - return -1; - } - close(socket_fd); - return vfio_container_fd; - } - - return -1; -} - /* open group fd or get an existing one */ int pci_vfio_get_group_fd(int iommu_group_no) @@ -950,7 +885,7 @@ pci_vfio_enable(void) return 0; } - vfio_cfg.vfio_container_fd = pci_vfio_get_container_fd(); + vfio_cfg.vfio_container_fd = vfio_get_container_fd(); /* check if we have VFIO driver enabled */ if (vfio_cfg.vfio_container_fd != -1) { diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c index d54ded8..a759f02 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c @@ -296,7 +296,7 @@ pci_vfio_mp_sync_thread(void __rte_unused * arg) switch (ret) { case SOCKET_REQ_CONTAINER: - fd = pci_vfio_get_container_fd(); + fd = vfio_get_container_fd();
[dpdk-dev] [PATCH v3 09/16] vfio: generalize pci_vfio_get_group_no
Generalize the pci_vfio_get_group_no to not be PCI-specific. Move the general implementation to the eal_vfio.c as vfio_get_group_no and leave the original pci_vfio_get_group_no being a wrapper around this to preserve compilation issues. The pci_vfio_get_group_no function will be removed later. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 38 +- lib/librte_eal/linuxapp/eal/eal_vfio.c | 43 ++ lib/librte_eal/linuxapp/eal/eal_vfio.h | 7 + 3 files changed, 51 insertions(+), 37 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index 528479a..c2ff465 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -401,43 +401,7 @@ pci_vfio_get_group_fd(int iommu_group_no) static int pci_vfio_get_group_no(const char *pci_addr, int *iommu_group_no) { - char linkname[PATH_MAX]; - char filename[PATH_MAX]; - char *tok[16], *group_tok, *end; - int ret; - - memset(linkname, 0, sizeof(linkname)); - memset(filename, 0, sizeof(filename)); - - /* try to find out IOMMU group for this device */ - snprintf(linkname, sizeof(linkname), -"%s/%s/iommu_group", pci_get_sysfs_path(), pci_addr); - - ret = readlink(linkname, filename, sizeof(filename)); - - /* if the link doesn't exist, no VFIO for us */ - if (ret < 0) - return 0; - - ret = rte_strsplit(filename, sizeof(filename), - tok, RTE_DIM(tok), '/'); - - if (ret <= 0) { - RTE_LOG(ERR, EAL, " %s cannot get IOMMU group\n", pci_addr); - return -1; - } - - /* IOMMU group is always the last token */ - errno = 0; - group_tok = tok[ret - 1]; - end = group_tok; - *iommu_group_no = strtol(group_tok, &end, 10); - if ((end != group_tok && *end != '\0') || errno != 0) { - RTE_LOG(ERR, EAL, " %s error parsing IOMMU number!\n", pci_addr); - return -1; - } - - return 1; + return vfio_get_group_no(pci_get_sysfs_path(), pci_addr, iommu_group_no); } static void diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c index 5660ddd..2bf810e 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c @@ -165,6 +165,49 @@ vfio_get_container_fd(void) } int +vfio_get_group_no(const char *sysfs_base, + const char *dev_addr, int *iommu_group_no) +{ + char linkname[PATH_MAX]; + char filename[PATH_MAX]; + char *tok[16], *group_tok, *end; + int ret; + + memset(linkname, 0, sizeof(linkname)); + memset(filename, 0, sizeof(filename)); + + /* try to find out IOMMU group for this device */ + snprintf(linkname, sizeof(linkname), +"%s/%s/iommu_group", sysfs_base, dev_addr); + + ret = readlink(linkname, filename, sizeof(filename)); + + /* if the link doesn't exist, no VFIO for us */ + if (ret < 0) + return 0; + + ret = rte_strsplit(filename, sizeof(filename), + tok, RTE_DIM(tok), '/'); + + if (ret <= 0) { + RTE_LOG(ERR, EAL, " %s cannot get IOMMU group\n", dev_addr); + return -1; + } + + /* IOMMU group is always the last token */ + errno = 0; + group_tok = tok[ret - 1]; + end = group_tok; + *iommu_group_no = strtol(group_tok, &end, 10); + if ((end != group_tok && *end != '\0') || errno != 0) { + RTE_LOG(ERR, EAL, " %s error parsing IOMMU number!\n", dev_addr); + return -1; + } + + return 1; +} + +int vfio_type1_dma_map(int vfio_container_fd) { const struct rte_memseg *ms = rte_eal_get_physmem_layout(); diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h index 0fb05a4..619cd6b 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h @@ -119,6 +119,13 @@ vfio_has_supported_extensions(int vfio_container_fd); int vfio_get_container_fd(void); +/* parse IOMMU group number for a device + * returns 1 on success, -1 for errors, 0 for non-existent group + */ +int +vfio_get_group_no(const char *sysfs_base, + const char *dev_addr, int *iommu_group_no); + #define SOCKET_REQ_CONTAINER 0x100 #define SOCKET_REQ_GROUP 0x200 #define SOCKET_OK 0x0 -- 2.8.0
[dpdk-dev] [PATCH v3 10/16] vfio: extract setup logic out of pci_vfio_map_resource
The setup logic access the global vfio_cfg variable that will be moved in the following commits. We need to separate all accesses to this variable to a general code. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 85 +- 1 file changed, 49 insertions(+), 36 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index c2ff465..2792c23 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -411,37 +411,22 @@ clear_current_group(void) vfio_cfg.vfio_groups[vfio_cfg.vfio_group_idx].fd = -1; } - -/* - * map the PCI resources of a PCI device in virtual memory (VFIO version). - * primary and secondary processes follow almost exactly the same path +/** + * Setup vfio_cfg for the device indentified by its address. It discovers + * the configured I/O MMU groups or sets a new one for the device. If a new + * groups is assigned, the DMA mapping is performed. + * Returns 0 on success, a negative value on failure and a positive value in + * case the given device cannot be managed this way. */ -int -pci_vfio_map_resource(struct rte_pci_device *dev) +static int pci_vfio_setup_device(const char *pci_addr, int *vfio_dev_fd, + struct vfio_device_info *device_info) { struct vfio_group_status group_status = { .argsz = sizeof(group_status) }; - struct vfio_device_info device_info = { .argsz = sizeof(device_info) }; - int vfio_group_fd, vfio_dev_fd; + int vfio_group_fd; int iommu_group_no; - char pci_addr[PATH_MAX] = {0}; - struct rte_pci_addr *loc = &dev->addr; - int i, ret, msix_bar; - struct mapped_pci_resource *vfio_res = NULL; - struct mapped_pci_res_list *vfio_res_list = RTE_TAILQ_CAST(rte_vfio_tailq.head, mapped_pci_res_list); - - struct pci_map *maps; - uint32_t msix_table_offset = 0; - uint32_t msix_table_size = 0; - uint32_t ioport_bar; - - dev->intr_handle.fd = -1; - dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; - - /* store PCI address string */ - snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT, - loc->domain, loc->bus, loc->devid, loc->function); + int ret; /* get group number */ ret = pci_vfio_get_group_no(pci_addr, &iommu_group_no); @@ -476,8 +461,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev) } /* -* at this point, we know at least one port on this device is bound to VFIO, -* so we can proceed to try and set this particular port up +* at this point, we know that this group is viable (meaning, all devices +* are either bound to VFIO or not bound to anything) */ /* check if the group is viable */ @@ -495,11 +480,6 @@ pci_vfio_map_resource(struct rte_pci_device *dev) return -1; } - /* -* at this point, we know that this group is viable (meaning, all devices -* are either bound to VFIO or not bound to anything) -*/ - /* check if group does not have a container yet */ if (!(group_status.flags & VFIO_GROUP_FLAGS_CONTAINER_SET)) { @@ -546,8 +526,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev) } /* get a file descriptor for the device */ - vfio_dev_fd = ioctl(vfio_group_fd, VFIO_GROUP_GET_DEVICE_FD, pci_addr); - if (vfio_dev_fd < 0) { + *vfio_dev_fd = ioctl(vfio_group_fd, VFIO_GROUP_GET_DEVICE_FD, pci_addr); + if (*vfio_dev_fd < 0) { /* if we cannot get a device fd, this simply means that this * particular port is not bound to VFIO */ @@ -557,14 +537,47 @@ pci_vfio_map_resource(struct rte_pci_device *dev) } /* test and setup the device */ - ret = ioctl(vfio_dev_fd, VFIO_DEVICE_GET_INFO, &device_info); + ret = ioctl(*vfio_dev_fd, VFIO_DEVICE_GET_INFO, device_info); if (ret) { RTE_LOG(ERR, EAL, " %s cannot get device info, " "error %i (%s)\n", pci_addr, errno, strerror(errno)); - close(vfio_dev_fd); + close(*vfio_dev_fd); return -1; } + return 0; +} + +/* + * map the PCI resources of a PCI device in virtual memory (VFIO version). + * primary and secondary processes follow almost exactly the same path + */ +int +pci_vfio_map_resource(struct rte_pci_device *dev) +{ + struct vfio_device_info device_info = { .argsz = sizeof(device_info) }; + char pci_addr[PATH_MAX] = {0}; + int vfio_dev_fd; + struct rte_pci_addr *loc = &dev->addr; + int i, ret, msix_bar; + struct mapped_pci_resource *vfio_res = NULL; + struct mapped_pci_res_list *vfio_res_list = RTE_TAILQ_CAST(rte_vfio_tailq.head, mapped_pci_res_list); + +
[dpdk-dev] [PATCH v3 11/16] vfio: move global vfio_cfg to eal_vfio.c
The vfio_cfg is a module-global variable and so together with this variable, it is necessary to move functions: * pci_vfio_get_group_fd - renamed to vfio_get_group_fd - pci_* version removed (no other call in EAL) * pci_vfio_setup_device - renamed as vfio_setup_device * pci_vfio_enable - renamed as vfio_enable - generalized to check for a specific vfio driver presence - pci_* specialization preserved as a wrapper * pci_vfio_is_enabled - renamed as vfio_is_enabled - generalized to check for a specific vfio driver presence to preserve the semantics of VFIO + PCI - pci_* specialization preserved as a wrapper * clear_current_group - private function, just moved To stop GCC complaining about "defined but not used", the private function pci_vfio_get_group_no has been removed entirely. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal_pci_init.h | 1 - lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 297 + lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c | 2 +- lib/librte_eal/linuxapp/eal/eal_vfio.c | 287 lib/librte_eal/linuxapp/eal/eal_vfio.h | 17 ++ 5 files changed, 309 insertions(+), 295 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h index d34212b..d00bf7d 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h +++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h @@ -94,7 +94,6 @@ int pci_vfio_ioport_unmap(struct rte_pci_ioport *p); /* map VFIO resource prototype */ int pci_vfio_map_resource(struct rte_pci_device *dev); -int pci_vfio_get_group_fd(int iommu_group_fd); #endif diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index 2792c23..46cd683 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -69,9 +69,6 @@ static struct rte_tailq_elem rte_vfio_tailq = { }; EAL_REGISTER_TAILQ(rte_vfio_tailq) -/* per-process VFIO config */ -static struct vfio_config vfio_cfg; - int pci_vfio_read_config(const struct rte_intr_handle *intr_handle, void *buf, size_t len, off_t offs) @@ -299,255 +296,6 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd) return -1; } -/* open group fd or get an existing one */ -int -pci_vfio_get_group_fd(int iommu_group_no) -{ - int i; - int vfio_group_fd; - char filename[PATH_MAX]; - - /* check if we already have the group descriptor open */ - for (i = 0; i < vfio_cfg.vfio_group_idx; i++) - if (vfio_cfg.vfio_groups[i].group_no == iommu_group_no) - return vfio_cfg.vfio_groups[i].fd; - - /* if primary, try to open the group */ - if (internal_config.process_type == RTE_PROC_PRIMARY) { - /* try regular group format */ - snprintf(filename, sizeof(filename), -VFIO_GROUP_FMT, iommu_group_no); - vfio_group_fd = open(filename, O_RDWR); - if (vfio_group_fd < 0) { - /* if file not found, it's not an error */ - if (errno != ENOENT) { - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", filename, - strerror(errno)); - return -1; - } - - /* special case: try no-IOMMU path as well */ - snprintf(filename, sizeof(filename), - VFIO_NOIOMMU_GROUP_FMT, iommu_group_no); - vfio_group_fd = open(filename, O_RDWR); - if (vfio_group_fd < 0) { - if (errno != ENOENT) { - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", filename, - strerror(errno)); - return -1; - } - return 0; - } - /* noiommu group found */ - } - - /* if the fd is valid, create a new group for it */ - if (vfio_cfg.vfio_group_idx == VFIO_MAX_GROUPS) { - RTE_LOG(ERR, EAL, "Maximum number of VFIO groups reached!\n"); - close(vfio_group_fd); - return -1; - } - vfio_cfg.vfio_groups[vfio_cfg.vfio_group_idx].group_no = iommu_group_no; - vfio_cfg.vfio_groups[vfio_cfg.vfio_group_idx].fd = vfio_group_fd; - return vfio_group_fd; - } - /* if we're in a secondary process, request group fd from the primary -* process via our socket -*/ - else { - int socket_fd, ret; - - socket_fd
[dpdk-dev] [PATCH v3 13/16] vfio: make vfio_*_dma_map and iommu_types private
There is no more reason to expose those definitions as nobody uses them. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal_vfio.c | 15 +-- lib/librte_eal/linuxapp/eal/eal_vfio.h | 11 --- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c index d11f723..fcb0ab3 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c @@ -49,6 +49,17 @@ /* per-process VFIO config */ static struct vfio_config vfio_cfg; +static int vfio_type1_dma_map(int); +static int vfio_noiommu_dma_map(int); + +/* IOMMU types we support */ +static const struct vfio_iommu_type iommu_types[] = { + /* x86 IOMMU, otherwise known as type 1 */ + { RTE_VFIO_TYPE1, "Type 1", &vfio_type1_dma_map}, + /* IOMMU-less mode */ + { RTE_VFIO_NOIOMMU, "No-IOMMU", &vfio_noiommu_dma_map}, +}; + int vfio_get_group_fd(int iommu_group_no) { @@ -494,7 +505,7 @@ vfio_get_group_no(const char *sysfs_base, return 1; } -int +static int vfio_type1_dma_map(int vfio_container_fd) { const struct rte_memseg *ms = rte_eal_get_physmem_layout(); @@ -526,7 +537,7 @@ vfio_type1_dma_map(int vfio_container_fd) return 0; } -int +static int vfio_noiommu_dma_map(int __rte_unused vfio_container_fd) { /* No-IOMMU mode does not need DMA mapping */ diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h index 4ca0fa3..f1a5427 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h @@ -149,17 +149,6 @@ int vfio_is_enabled(const char *modname); #define SOCKET_NO_FD 0x1 #define SOCKET_ERR 0xFF -int vfio_type1_dma_map(int); -int vfio_noiommu_dma_map(int); - -/* IOMMU types we support */ -static const struct vfio_iommu_type iommu_types[] = { - /* x86 IOMMU, otherwise known as type 1 */ - { RTE_VFIO_TYPE1, "Type 1", &vfio_type1_dma_map}, - /* IOMMU-less mode */ - { RTE_VFIO_NOIOMMU, "No-IOMMU", &vfio_noiommu_dma_map}, -}; - #define VFIO_PRESENT #endif /* kernel version */ #endif /* RTE_EAL_VFIO */ -- 2.8.0
[dpdk-dev] [PATCH v3 12/16] vfio: fix typo in doc for vfio_setup_device
Signed-off-by: Jan Viktorin Suggested-by: Anatoly Burakov Acked-by: John McNamara --- lib/librte_eal/linuxapp/eal/eal_vfio.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h index d4532a5..4ca0fa3 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h @@ -131,7 +131,7 @@ int vfio_get_group_fd(int iommu_group_no); /** - * Setup vfio_cfg for the device indentified by its address. It discovers + * Setup vfio_cfg for the device identified by its address. It discovers * the configured I/O MMU groups or sets a new one for the device. If a new * groups is assigned, the DMA mapping is performed. * Returns 0 on success, a negative value on failure and a positive value in -- 2.8.0
[dpdk-dev] [PATCH v3 15/16] vfio: initialize vfio out of the PCI subsystem
The VFIO does not depend on the PCI anymore so it can be initialized out of the PCI subsystem. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal.c | 31 ++ lib/librte_eal/linuxapp/eal/eal_pci.c | 17 +--- lib/librte_eal/linuxapp/eal/eal_pci_init.h | 3 --- lib/librte_eal/linuxapp/eal/eal_vfio.h | 3 +++ 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c index 543ef86..606b777 100644 --- a/lib/librte_eal/linuxapp/eal/eal.c +++ b/lib/librte_eal/linuxapp/eal/eal.c @@ -82,6 +82,7 @@ #include "eal_filesystem.h" #include "eal_hugepages.h" #include "eal_options.h" +#include "eal_vfio.h" #define MEMSIZE_IF_NO_HUGE_PAGE (64ULL * 1024ULL * 1024ULL) @@ -701,6 +702,31 @@ rte_eal_iopl_init(void) return 0; } +#ifdef VFIO_PRESENT +static int rte_eal_vfio_setup(void) +{ + if (internal_config.no_pci) + return 0; + + pci_vfio_enable(); + + if (pci_vfio_is_enabled()) { + + /* if we are primary process, create a thread to communicate with +* secondary processes. the thread will use a socket to wait for +* requests from secondary process to send open file descriptors, +* because VFIO does not allow multiple open descriptors on a group or +* VFIO container. +*/ + if (internal_config.process_type == RTE_PROC_PRIMARY && + vfio_mp_sync_setup() < 0) + return -1; + } + + return 0; +} +#endif + /* Launch threads, called at application init(). */ int rte_eal_init(int argc, char **argv) @@ -764,6 +790,11 @@ rte_eal_init(int argc, char **argv) if (rte_eal_pci_init() < 0) rte_panic("Cannot init PCI\n"); +#ifdef VFIO_PRESENT + if (rte_eal_vfio_setup() < 0) + rte_panic("Cannot init VFIO\n"); +#endif + #ifdef RTE_LIBRTE_IVSHMEM if (rte_eal_ivshmem_init() < 0) rte_panic("Cannot init IVSHMEM\n"); diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c index d0a6481..cd9de7c 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c @@ -754,21 +754,6 @@ rte_eal_pci_init(void) RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__); return -1; } -#ifdef VFIO_PRESENT - pci_vfio_enable(); - - if (pci_vfio_is_enabled()) { - - /* if we are primary process, create a thread to communicate with -* secondary processes. the thread will use a socket to wait for -* requests from secondary process to send open file descriptors, -* because VFIO does not allow multiple open descriptors on a group or -* VFIO container. -*/ - if (internal_config.process_type == RTE_PROC_PRIMARY && - vfio_mp_sync_setup() < 0) - return -1; - } -#endif + return 0; } diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h index 62c337f..6a960d1 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h +++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h @@ -74,9 +74,6 @@ int pci_uio_ioport_unmap(struct rte_pci_ioport *p); #ifdef VFIO_PRESENT -int pci_vfio_enable(void); -int pci_vfio_is_enabled(void); - /* access config space */ int pci_vfio_read_config(const struct rte_intr_handle *intr_handle, void *buf, size_t len, off_t offs); diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h index 884884c..43960eb 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h @@ -143,6 +143,9 @@ int vfio_setup_device(const char *sysfs_base, const char *dev_addr, int vfio_enable(const char *modname); int vfio_is_enabled(const char *modname); +int pci_vfio_enable(void); +int pci_vfio_is_enabled(void); + #define SOCKET_REQ_CONTAINER 0x100 #define SOCKET_REQ_GROUP 0x200 #define SOCKET_OK 0x0 -- 2.8.0
[dpdk-dev] [PATCH v3 14/16] vfio: rename and generalize eal_pci_vfio_mp_sync
The module eal_pci_vfio_mp_sync is quite generic so it shouldn't contain the "pci" string in its name. The internal functions don't need the pci_* prefix. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/Makefile | 4 ++-- lib/librte_eal/linuxapp/eal/eal_pci.c | 2 +- lib/librte_eal/linuxapp/eal/eal_pci_init.h| 1 - lib/librte_eal/linuxapp/eal/eal_vfio.h| 2 ++ .../linuxapp/eal/{eal_pci_vfio_mp_sync.c => eal_vfio_mp_sync.c} | 8 5 files changed, 9 insertions(+), 8 deletions(-) rename lib/librte_eal/linuxapp/eal/{eal_pci_vfio_mp_sync.c => eal_vfio_mp_sync.c} (98%) diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile index 517554f..1a97693 100644 --- a/lib/librte_eal/linuxapp/eal/Makefile +++ b/lib/librte_eal/linuxapp/eal/Makefile @@ -67,10 +67,10 @@ endif SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_thread.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_log.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_vfio.c +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_vfio_mp_sync.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_pci.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_pci_uio.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_pci_vfio.c -SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_pci_vfio_mp_sync.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_debug.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_lcore.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_timer.c @@ -111,7 +111,7 @@ CFLAGS_eal_common_cpuflags.o := $(CPUFLAGS_LIST) CFLAGS_eal.o := -D_GNU_SOURCE CFLAGS_eal_interrupts.o := -D_GNU_SOURCE -CFLAGS_eal_pci_vfio_mp_sync.o := -D_GNU_SOURCE +CFLAGS_eal_vfio_mp_sync.o := -D_GNU_SOURCE CFLAGS_eal_timer.o := -D_GNU_SOURCE CFLAGS_eal_lcore.o := -D_GNU_SOURCE CFLAGS_eal_thread.o := -D_GNU_SOURCE diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c index f9c3efd..d0a6481 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c @@ -766,7 +766,7 @@ rte_eal_pci_init(void) * VFIO container. */ if (internal_config.process_type == RTE_PROC_PRIMARY && - pci_vfio_mp_sync_setup() < 0) + vfio_mp_sync_setup() < 0) return -1; } #endif diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h index d00bf7d..62c337f 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h +++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h @@ -76,7 +76,6 @@ int pci_uio_ioport_unmap(struct rte_pci_ioport *p); int pci_vfio_enable(void); int pci_vfio_is_enabled(void); -int pci_vfio_mp_sync_setup(void); /* access config space */ int pci_vfio_read_config(const struct rte_intr_handle *intr_handle, diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h index f1a5427..884884c 100644 --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h @@ -149,6 +149,8 @@ int vfio_is_enabled(const char *modname); #define SOCKET_NO_FD 0x1 #define SOCKET_ERR 0xFF +int vfio_mp_sync_setup(void); + #define VFIO_PRESENT #endif /* kernel version */ #endif /* RTE_EAL_VFIO */ diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c similarity index 98% rename from lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c rename to lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c index b3f4c51..00cf919 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c +++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c @@ -265,7 +265,7 @@ vfio_mp_sync_connect_to_primary(void) * socket listening thread for primary process */ static __attribute__((noreturn)) void * -pci_vfio_mp_sync_thread(void __rte_unused * arg) +vfio_mp_sync_thread(void __rte_unused * arg) { int ret, fd, vfio_group_no; @@ -376,7 +376,7 @@ vfio_mp_sync_socket_setup(void) * set up a local socket and tell it to listen for incoming connections */ int -pci_vfio_mp_sync_setup(void) +vfio_mp_sync_setup(void) { int ret; char thread_name[RTE_MAX_THREAD_NAME_LEN]; @@ -387,7 +387,7 @@ pci_vfio_mp_sync_setup(void) } ret = pthread_create(&socket_thread, NULL, - pci_vfio_mp_sync_thread, NULL); + vfio_mp_sync_thread, NULL); if (ret) { RTE_LOG(ERR, EAL, "Failed to create thread for communication with secondary processes!\n"); @@ -396,7 +396,7 @@ pci_vfio_mp_sync_setup(void) } /* Set thread_name for aid in debugging. */ - snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "pci-vfio-sync"); + snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "vfio-sync"); ret = rte_thread_setname(socket_thread, thread_n
[dpdk-dev] [PATCH v3 16/16] vfio: change VFIO init to be extendable
We can now just OR the vfio_enabled sequentially and so adding new VFIO subsystems (vfio_platform) is possible. Signed-off-by: Jan Viktorin --- lib/librte_eal/linuxapp/eal/eal.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c index 606b777..3fb2188 100644 --- a/lib/librte_eal/linuxapp/eal/eal.c +++ b/lib/librte_eal/linuxapp/eal/eal.c @@ -705,12 +705,14 @@ rte_eal_iopl_init(void) #ifdef VFIO_PRESENT static int rte_eal_vfio_setup(void) { - if (internal_config.no_pci) - return 0; + int vfio_enabled = 0; - pci_vfio_enable(); + if (!internal_config.no_pci) { + pci_vfio_enable(); + vfio_enabled |= pci_vfio_is_enabled(); + } - if (pci_vfio_is_enabled()) { + if (vfio_enabled) { /* if we are primary process, create a thread to communicate with * secondary processes. the thread will use a socket to wait for -- 2.8.0
[dpdk-dev] [PATCH v9 0/7] export PMD infos
On Mon, Jul 04, 2016 at 03:13:58AM +0200, Thomas Monjalon wrote: > This is a respin of the series from Neil. > It was planned to be integrated in 16.07-rc1 but the discovered issues > make a new revision needed. > There are still few things which could be improved but it is not mandatory > to fix them for an integration in 16.07-rc2: > - fix make clean after pmdinfogen > - build installable pmdinfogen for target > - convert pmdinfo.py to Python 3 > - document dependency pyelftools > > Changes done in this v9: > - fix build dependency of drivers on pmdinfogen > - fix build of mlx4, mlx5, aesni > - fix new drivers bnxt, thunderx, kasumi > - fix MAINTAINERS file > - fix coding style in pmdinfogen > - add compiler checks for pmdinfogen > - remove useless functions in pmdinfogen > - fail build if pmdinfogen fails (set -e) > - fix verbose pmdinfogen run > - build pmdinfogen in buildtools directory (was app) > - install pmdinfogen in sdk package (was runtime) > - fix CamelCase in pmdinfo.py > - prefix executables with dpdk- > - rename PMD_REGISTER_DRIVER -> RTE_REGISTER_DRIVER > - separate commit for hostapp.mk refresh > - remove useless hostlib.mk > - spread doc in appropriate patches > > Please review carefully. > Haven't reviewed, but did test applying these patches on FreeBSD to see what happens there. Compilation works fine after applying all patches, and when I run: strings testpmd | grep PMD_INFO_STRING I get the appropriate metadata output of device ids etc. that shows that the data is getting written into the static binaries ok. For a shared library, rather than static build, there was a problem building with clang - probably unrelated to this set, I haven't checked yet - but a gcc shared build worked fine. Checking testpmd showed zero PMD_INFO strings as expected, and librte_pmd_ixgbe.so showed two, again as expected. For the script in the tools directory, the first problem is that python is not to be found in "/usr/bin/python" as on Linux. To make it run on FreeBSD, this should be changed to "/usr/bin/env python", as in dpdk_config.py. For the "pyelftools" dependency, on FreeBSD, this is available in ports as "py-pyelftools" and it installed ok for me. The final issue was the hard-coded path to the pci-ids in /usr/share/hwdata. Patch to fix these script issues is below. Regards, /Bruce diff --git a/tools/dpdk-pmdinfo.py b/tools/dpdk-pmdinfo.py index b8a9be2..8d19b90 100755 --- a/tools/dpdk-pmdinfo.py +++ b/tools/dpdk-pmdinfo.py @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/env python # - # # Utility to dump PMD_INFO_STRING support from an object file @@ -9,6 +9,7 @@ import sys from optparse import OptionParser import string import json +import platform # For running from development directory. It should take precedence over the # installed pyelftools. @@ -556,6 +557,14 @@ def main(stream=None): global raw_output global pcidb +pcifile_default = "./pci.ids" # for unknown OS's assume local file +if platform.system() == 'Linux': +pcifile_default = "/usr/share/hwdata/pci.ids" +elif platform.system() == 'FreeBSD': +pcifile_default = "/usr/local/share/pciids/pci.ids" +if not os.path.exists(pcifile_default): +pcifile_default = "/usr/share/misc/pci_vendors" + optparser = OptionParser( usage='usage: %prog [-hrtp] [-d ', description="Dump pmd hardware support info", @@ -567,7 +576,7 @@ def main(stream=None): optparser.add_option("-d", "--pcidb", dest="pcifile", help="specify a pci database " "to get vendor names from", - default="/usr/share/hwdata/pci.ids", metavar="FILE") + default=pcifile_default, metavar="FILE") optparser.add_option("-t", "--table", dest="tblout", help="output information on hw support as a hex table", action='store_true')
[dpdk-dev] [PATCH v3 00/16] Make VFIO support less dependent on PCI
> From: Jan Viktorin [mailto:viktorin at rehivetech.com] > Sent: Monday, July 4, 2016 4:17 PM > To: dev at dpdk.org > Cc: Jan Viktorin ; Burakov, Anatoly > ; David Marchand > ; Wiles, Keith ; > Santosh Shukla ; Stephen Hemminger > ; Shreyansh Jain > > Subject: [PATCH v3 00/16] Make VFIO support less dependent on PCI > > Hello, > > I've rebased the v2 of this patch set on top of the current master. > It builds well for my setup (both VFIO enabled and disabled). > > Regards > Jan > > > v3: > * 0012: Acked-by: John McNamara > > Jan Viktorin (16): > vfio: fix include of eal_private.h to be local > vfio: move VFIO-specific stuff to eal_vfio.h > vfio: move common vfio constants to eal_vfio.h > vfio: move vfio_iommu_type and dma_map functions to eal_vfio > vfio: generalize pci_vfio_set_iommu_type > vfio: generalize pci_vfio_has_supported_extensions > vfio: move vfio-specific SOCKET_* constants > vfio: generalize pci_vfio_get_container_fd > vfio: generalize pci_vfio_get_group_no > vfio: extract setup logic out of pci_vfio_map_resource > vfio: move global vfio_cfg to eal_vfio.c > vfio: fix typo in doc for vfio_setup_device > vfio: make vfio_*_dma_map and iommu_types private > vfio: rename and generalize eal_pci_vfio_mp_sync > vfio: initialize vfio out of the PCI subsystem > vfio: change VFIO init to be extendable > > lib/librte_eal/linuxapp/eal/Makefile | 5 +- > lib/librte_eal/linuxapp/eal/eal.c | 33 ++ > lib/librte_eal/linuxapp/eal/eal_pci.c | 17 +- > lib/librte_eal/linuxapp/eal/eal_pci_init.h | 41 -- > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 517 +-- > lib/librte_eal/linuxapp/eal/eal_vfio.c | 547 > + > lib/librte_eal/linuxapp/eal/eal_vfio.h | 94 > .../{eal_pci_vfio_mp_sync.c => eal_vfio_mp_sync.c} | 12 +- > 8 files changed, 691 insertions(+), 575 deletions(-) create mode 100644 > lib/librte_eal/linuxapp/eal/eal_vfio.c > rename lib/librte_eal/linuxapp/eal/{eal_pci_vfio_mp_sync.c => > eal_vfio_mp_sync.c} (97%) > > -- > 2.8.0 For the whole patchset: Acked-by: Anatoly Burakov
[dpdk-dev] [PATCH v3 3/6] ena: disable readless communication regarding to HW revision
On Thu, Jun 30, 2016 at 05:04:56PM +0200, Jan Medala wrote: > Depending on HW revision readless communcation between host and device > may be unavailable. > In that case prevent PMD of seting up readless communication mechanism. > The idea of readless communication is a strange one. Can you provide a bit more detail as to what "readless communication" refers to. /Bruce
[dpdk-dev] [PATCH v6 0/4] support reset of VF link
On Mon, 2016-06-20 at 14:24 +0800, Wenzhuo Lu wrote: > If the PF link is down and up, VF link will not work accordingly. > This patch set addes the support of VF link reset. So, when VF > receices the messges of physical link down/up. APP can reset the > VF link and let it recover. > > PS: This patch set is splitted from a previous patch set, > *automatic link recovery on ixgbe/igb VF*, and it's base on the > patch set *support mailbox interruption on ixgbe/igb VF*. > > Wenzhuo Lu (3): > lib/librte_ether: support device reset > ixgbe: implement device reset on VF > igb: implement device reset on VF > > Zhe Tao (1): > i40e: implement device reset on VF > > v1: > - Added the implementation for the VF reset functionality. > v2: > - Changed the i40e related operations during VF reset. > v3: > - Resent the patches because of the mail sent issue. > v4: > - Removed some VF reset emulation code. > v5: > - Removed all the code related with lock. > v6: > - Updated the NIC feature overview matrix. > - Added more explanation in the doxygen comment of reset API. > > doc/guides/nics/overview.rst | 1 + > doc/guides/rel_notes/release_16_07.rst | 13 ++ > drivers/net/e1000/igb_ethdev.c | 59 > drivers/net/i40e/i40e_ethdev.h | 4 ++ > drivers/net/i40e/i40e_ethdev_vf.c | 83 > ++ > drivers/net/i40e/i40e_rxtx.c | 10 > drivers/net/i40e/i40e_rxtx.h | 4 ++ > drivers/net/ixgbe/ixgbe_ethdev.c | 64 +- > drivers/net/ixgbe/ixgbe_ethdev.h | 2 +- > drivers/net/ixgbe/ixgbe_rxtx.c | 12 +++-- > lib/librte_ether/rte_ethdev.c | 17 +++ > lib/librte_ether/rte_ethdev.h | 24 ++ > lib/librte_ether/rte_ether_version.map | 7 +++ > 13 files changed, 295 insertions(+), 5 deletions(-) Hello Wenzhuo, I'm testing this patchset, but I am sporadically running into an issue where the VFs reset fails after the PF flaps. I have a VM running on a KVM box with a X540-AT2, passing 2 VFs in. I am using calling rte_eth_dev_reset in response to a RTE_ETH_EVENT_INTR_RESET callback, and the following errors appear in the log: PMD: ixgbevf_dev_reset(): Ixgbe VF reset: Failed to update link. PMD: ixgbe_alloc_rx_queue_mbufs(): RX mbuf alloc failed queue_id=0 PMD: ixgbevf_dev_start(): Unable to initialize RX hardware (-12) PMD: ixgbevf_dev_reset(): Ixgbe VF reset: Failed to start device. Jumping in with GDB, it seems that the rte_rxmbuf_alloc call in ixgbe_alloc_rx_queue_mbufs returns NULL at iteration 64 out of 2048. The application has ~500 2MB hugepages, and there's 2GB of free memory available on top of that. Have you seen this before? Any pointer or suggestion for debugging? Thanks! -- Kind regards, Luca Boccassi
[dpdk-dev] [PATCH] pdump: close client socket
Close the client socket before returning on error. Coverity issue: 127555 Fixes: f3c1829130ac ("pdump: check missing home environment variable") Signed-off-by: Reshma Pattan --- lib/librte_pdump/rte_pdump.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c index ee566cb..22ed476 100644 --- a/lib/librte_pdump/rte_pdump.c +++ b/lib/librte_pdump/rte_pdump.c @@ -677,7 +677,7 @@ pdump_create_client_socket(struct pdump_request *p) RTE_LOG(ERR, PDUMP, "Failed to get client socket path: %s:%d\n", __func__, __LINE__); - return -1; + goto exit; } addr.sun_family = AF_UNIX; addr_len = sizeof(struct sockaddr_un); @@ -728,6 +728,7 @@ pdump_create_client_socket(struct pdump_request *p) ret = server_resp.err_value; } while (0); +exit: close(socket_fd); unlink(addr.sun_path); return ret; -- 2.5.0
[dpdk-dev] [PATCH v3 5/6] ena: fix memory management issue
On Thu, Jun 30, 2016 at 05:04:58PM +0200, Jan Medala wrote: > After allocating memzone it's required to zeroize memory in it. > Freeing memzone with function dedicated for memoryzones. > Can you provide a fixes line for this patch? > Signed-off-by: Alexander Matushevsky > Signed-off-by: Jakub Palider > Signed-off-by: Jan Medala > --- > drivers/net/ena/base/ena_plat_dpdk.h | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ena/base/ena_plat_dpdk.h > b/drivers/net/ena/base/ena_plat_dpdk.h > index b1ed80c..87c3bf1 100644 > --- a/drivers/net/ena/base/ena_plat_dpdk.h > +++ b/drivers/net/ena/base/ena_plat_dpdk.h > @@ -62,7 +62,7 @@ typedef uint64_t dma_addr_t; > #endif > > #define ena_atomic32_t rte_atomic32_t > -#define ena_mem_handle_t void * > +#define ena_mem_handle_t const struct rte_memzone * > > #define SZ_256 (256U) > #define SZ_4K (4096U) > @@ -188,13 +188,15 @@ typedef uint64_t dma_addr_t; > snprintf(z_name, sizeof(z_name),\ > "ena_alloc_%d", ena_alloc_cnt++); \ > mz = rte_memzone_reserve(z_name, size, SOCKET_ID_ANY, 0); \ > + memset(mz->addr, 0, size); \ > virt = mz->addr;\ > phys = mz->phys_addr; \ > + handle = mz;\ > } while (0) > #define ENA_MEM_FREE_COHERENT(dmadev, size, virt, phys, handle) \ > ({ ENA_TOUCH(size); ENA_TOUCH(phys);\ > ENA_TOUCH(dmadev); \ > -rte_free(virt); }) > +rte_memzone_free(handle); }) > > #define ENA_MEM_ALLOC_COHERENT_NODE(dmadev, size, virt, phys, node, > dev_node) \ > do {\ > -- > 2.8.2 >
[dpdk-dev] [PATCH] pdump: close client socket
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Reshma Pattan > Sent: Monday, July 4, 2016 5:17 PM > To: dev at dpdk.org > Cc: Pattan, Reshma > Subject: [dpdk-dev] [PATCH] pdump: close client socket > > Close the client socket before returning on error. > > Coverity issue: 127555 > > Fixes: f3c1829130ac ("pdump: check missing home environment variable") > > Signed-off-by: Reshma Pattan Acked-by: John McNamara
[dpdk-dev] [PATCH v9 0/7] export PMD infos
On Mon, Jul 04, 2016 at 03:10:14PM +0200, Thomas Monjalon wrote: > Hi Neil, > > I don't really understand why you don't accept I contribute to this > patchset. More details below. > I don't object to your contribution to this patchset. What I object to is you making changes contrary to what I just spent I don't even know how many weeks arguing with you and others about. Given your role as maintainer, it is tantamout to a 'my way or the highway' position. > 2016-07-04 08:34, Neil Horman: > > On Mon, Jul 04, 2016 at 03:13:58AM +0200, Thomas Monjalon wrote: > > > This is a respin of the series from Neil. > > > It was planned to be integrated in 16.07-rc1 but the discovered issues > > > make a new revision needed. > > > There are still few things which could be improved but it is not mandatory > > > to fix them for an integration in 16.07-rc2: > > > - fix make clean after pmdinfogen > > Clean works just fine, whats wrong with it? > > "make clean" do not remove the .pmd.* files. > But it is not a blocking issue. > They don't really need to be removed, as pmdinfogen will overwrite them unilaterally when a rebuild is done. > > > - build installable pmdinfogen for target > > Again, this seems to be working just fine for me, what do you see as wrong > > with > > it? > > If we install the SDK as a binary package for another arch, we cannot use > pmdinfogen as it is not cross-compiled (on purpose). > It was just a thought for possible future enhancement and is not blocking. > Ah, you want cross-compile support, rather than just native target support? I can agree with that. > > > - convert pmdinfo.py to Python 3 > > This was discussed weeks ago earlier in this thread, since there is no > > requirement for python3 we decided not to use it. Why renege on that now? > > Yes we can take the script as is. It is just a note for future enhancement. > Not blocking. Ok, but again, this was discussed weeks ago, the way its written currently should work with python2 or python3 (dependent on the python environment setup). converting any code to make it work better/more efficiently/etc with python3 seems like a step backwards > > > > - document dependency pyelftools > > In what capacity beyond simply noting the fact that it imports that package? > > I don't know where we could document this dependency. > Just a note, not blocking. > Ok, so you dont have any standard here, you just have some vague desire for...something. No other python script bothers with documenting what it imports, so I think you can understand when you list this as an enhancement for my work, but don't worry about any of the other work, its both confusing and frustrating. > > > Changes done in this v9: > > > - fix build dependency of drivers on pmdinfogen > > In what way is it broken? The drivers directory is dependent on building > > pmdinfogen, what more do you want? > > The drivers directory Makefile is a rte.subdir.mk. It does not handle > DEPDIRS. I've fix it in mk/rte.sdkbuild.mk > drivers: | buildtools > > > > - fix build of mlx4, mlx5, aesni > > They weren't broken as of V8, not sure what you're getting at here > > Yes they were because of typos. > I guess you were not compiling them. > No, I specifically compiled them in v8, as they were called out. > > > - fix new drivers bnxt, thunderx, kasumi > > > - fix MAINTAINERS file > > > - fix coding style in pmdinfogen > > > - add compiler checks for pmdinfogen > > > - remove useless functions in pmdinfogen > > > - fail build if pmdinfogen fails (set -e) > > > - fix verbose pmdinfogen run > > > - build pmdinfogen in buildtools directory (was app) > > Are you kidding me!? We just went 10 rounds over where to build this stupid > > application, and after I finally gave in to your demands, you change it one > > last time and call it a fix? Thats a real slap in the face, thanks for > > that. > > We were discussing reusing hostapp.mk. > And I was OK to use app/ as build directory. > After checking sdkinstall.mk, I changed my mind to build in buildtools/. > I do not call it a fix, just a change. Where do you see it is a fix? No slap. > No, its decidedly a slap. You and I argued about this back and forth very publically over the course of a week, and I finally aquiesced to your demands. If you've changed your mind, thats fine I suppose, but to do so after you've gotten your way is fairly frustrating. To then make the change and repost it without any prior consultation sends a very clear signal that my opinion on the subject doesn't really matter. You've put me in a position where I now either need to say whatever change you want to make is ok with me, or I need to argue against the thing that I wanted in the first place. Its very insulting. > > > - install pmdinfogen in sdk package (was runtime) > > > - fix CamelCase in pmdinfo.py > > > - prefix executables with dpdk- > > Again, seriously? Are you just trying to assert some sort of not inven
[dpdk-dev] [PATCH] bnxt: Fix a bug in broadcast/multicast setting
Currently we are wrongly setting HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_MCAST flag in bnxt_hwrm_cfa_l2_set_rx_mask() which is preventing promiscuous and multicast promiscuous settings from working correctly. This patch fixes it. Signed-off-by: Ajit Khaparde --- drivers/net/bnxt/bnxt_hwrm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c index 5d81a60..2ed4c2f 100644 --- a/drivers/net/bnxt/bnxt_hwrm.c +++ b/drivers/net/bnxt/bnxt_hwrm.c @@ -177,8 +177,7 @@ int bnxt_hwrm_cfa_l2_set_rx_mask(struct bnxt *bp, struct bnxt_vnic_info *vnic) mask = HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_PROMISCUOUS; if (vnic->flags & BNXT_VNIC_INFO_ALLMULTI) mask = HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_ALL_MCAST; - req.mask = rte_cpu_to_le_32(HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_MCAST | - HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_BCAST | + req.mask = rte_cpu_to_le_32(HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_BCAST | mask); rc = bnxt_hwrm_send_message(bp, &req, sizeof(req)); -- 2.7.4 (Apple Git-66)
[dpdk-dev] [PATCH v9 0/7] export PMD infos
2016-07-04 12:41, Neil Horman: > On Mon, Jul 04, 2016 at 03:10:14PM +0200, Thomas Monjalon wrote: > > Hi Neil, > > > > I don't really understand why you don't accept I contribute to this > > patchset. More details below. > > > I don't object to your contribution to this patchset. What I object to is you > making changes contrary to what I just spent I don't even know how many weeks > arguing with you and others about. Given your role as maintainer, it is > tantamout to a 'my way or the highway' position. Thank you for explaining your feeling so clearly. I'm sorry for any inconvenience or misunderstanding. I now understand that you prefer I make some patches separately after integrating your version. So I can send a v10 based on your v8 which only fixes compilation. Or you can do it yourself if you prefer. Then I'll send another series for the other changes. Deal? > > 2016-07-04 08:34, Neil Horman: > > > On Mon, Jul 04, 2016 at 03:13:58AM +0200, Thomas Monjalon wrote: > > > > Changes done in this v9: > > > > - fix build dependency of drivers on pmdinfogen > > > In what way is it broken? The drivers directory is dependent on building > > > pmdinfogen, what more do you want? > > > > The drivers directory Makefile is a rte.subdir.mk. It does not handle > > DEPDIRS. I've fix it in mk/rte.sdkbuild.mk > > drivers: | buildtools > > > > > > - fix build of mlx4, mlx5, aesni > > > They weren't broken as of V8, not sure what you're getting at here > > > > Yes they were because of typos. > > I guess you were not compiling them. > > > No, I specifically compiled them in v8, as they were called out. I didn't want to focus on these typos but as you insist, please check v8: http://dpdk.org/ml/archives/dev/2016-June/041994.html Example: -PMD_REGISTER_DRIVER(rte_mlx4_driver) +PMD_REGISTER_DRIVER(rte_mlx4_driveri, mlx4) +DRIVER_REGISTER_PCI_TABLE(mlx4, mlx4_pci_id_map); Which suggests that you are using vi: rte_mlx4_driveri And as far as I remember your rework of PMD_REGISTER_DRIVER requires to end the line with a semi-colon. > > > > - fix new drivers bnxt, thunderx, kasumi > > > > - fix MAINTAINERS file > > > > - fix coding style in pmdinfogen > > > > - add compiler checks for pmdinfogen > > > > - remove useless functions in pmdinfogen > > > > - fail build if pmdinfogen fails (set -e) > > > > - fix verbose pmdinfogen run > > > > - build pmdinfogen in buildtools directory (was app) > > > Are you kidding me!? We just went 10 rounds over where to build this > > > stupid > > > application, and after I finally gave in to your demands, you change it > > > one > > > last time and call it a fix? Thats a real slap in the face, thanks for > > > that. > > > > We were discussing reusing hostapp.mk. > > And I was OK to use app/ as build directory. > > After checking sdkinstall.mk, I changed my mind to build in buildtools/. > > I do not call it a fix, just a change. Where do you see it is a fix? No > > slap. > > > No, its decidedly a slap. You and I argued about this back and forth very > publically over the course of a week, and I finally aquiesced to your demands. > If you've changed your mind, thats fine I suppose, but to do so after you've > gotten your way is fairly frustrating. To then make the change and repost it > without any prior consultation sends a very clear signal that my opinion on > the > subject doesn't really matter. You've put me in a position where I now either > need to say whatever change you want to make is ok with me, or I need to argue > against the thing that I wanted in the first place. Its very insulting. I was in favor of having a generic hostapp.mk. It's still the case. I just changed my mind on building in build/buildtools. I thought it was not a big deal and I'm sorry. > > > > - install pmdinfogen in sdk package (was runtime) > > > > - fix CamelCase in pmdinfo.py > > > > - prefix executables with dpdk- > > > Again, seriously? Are you just trying to assert some sort of not > > > invented here > > > mentality? We had this conversation over and over again weeks ago, but > > > you > > > couldn't possibly chime in then? And you have to slip it in now, because > > > this > > > is the way you want it, as though I did it wrong previously? > > > > You did not do it wrong. > > After more thoughts, I really think we must be careful to have a consistent > > namespace with tools we export publically. > > I'm thinking to re-use the dpdk- prefix for other tools. > > It's open to discussion as everything. > Except that we already had the discussion! Panu, myself, and several others > went over this over a week ago, and I made the stated changes in response. No you said you would prefix pmdinfogen but you did not. As you said me you would add an entry in MAINTAINERS but you did not. That's why I did it, thinking you agreed. > To > have you then post yet another version on my behalf sends a clear signal that > both our opinio