[dpdk-dev] [RFC 0/4] Use Google Test as DPDK unit test framework
2016-08-02 21:37, Declan Doherty: > I've been trying out using google test as a possible replacement for our unit > test framework and have put to together this series of patches with help from > Anatoly as RFC to get peoples thoughts on migrating to google test. Thanks for exploring new possibilities. > To facilitate google test this rfc patch set contains build system changes to > allow C++ to built within the DPDK framework, this intended for unit test code > only, and to support google test which is a C++ framework. Don't worry, I'm > not > advocating making DPDK a C++ project :) You are not advocating but the unit test must be written in C++. I don't think it is a good idea to force people to write and maintain the tests in a different language than the code it tests. > Some of the major advantages of google test that I see over continuing to use > the > current test include giving a consist feel to all tests, a powerful test > execution framework which allow individual test suites or tests to be > specified > from the command line, support for a standard xunit output which can be > integrated > into a continuous build systems, and a very powerful mocking library > which allows much more control over testing failure conditions. It would be interesting to better describe in details what is missing currently and what such a framework can bring. (I agree there is a huge room for improvements on unit tests)
[dpdk-dev] [PATCH v2] lpm: remove redundant check when adding lpm rule
2016-08-02 17:04, Bruce Richardson: > Having to make this change twice shows up the fact that we are still carrying > around some version changes for older releases. Given that we are now past the > 16.07 release, the old code can probably be removed. Any volunteers to maybe > do up a patch for that. The first step would be to announce an ABI breakage. Do you plan to do other breaking changes? We may try to group them.
[dpdk-dev] [RFC 4/4] app/test-gtest: example google test application
Adds a new application to demonstrate how google test could be used as a new unit test framework. To compile both ${GTEST_DIR} and ${GMOCK_DIR} the install paths to google test and google mock directories respectively must be defined/exported. Signed-off-by: Declan Doherty --- app/Makefile| 1 + app/test-gtest/Makefile | 67 ++ app/test-gtest/main.cpp | 20 +++ app/test-gtest/test_mempool.cpp | 281 config/common_base | 5 + 5 files changed, 374 insertions(+) create mode 100644 app/test-gtest/Makefile create mode 100644 app/test-gtest/main.cpp create mode 100644 app/test-gtest/test_mempool.cpp diff --git a/app/Makefile b/app/Makefile index 30ec292..71cf31e 100644 --- a/app/Makefile +++ b/app/Makefile @@ -38,5 +38,6 @@ DIRS-$(CONFIG_RTE_TEST_PMD) += test-pmd DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_test DIRS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += proc_info DIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += pdump +DIRS-$(CONFIG_RTE_TEST_GTEST) += test-gtest include $(RTE_SDK)/mk/rte.subdir.mk diff --git a/app/test-gtest/Makefile b/app/test-gtest/Makefile new file mode 100644 index 000..66dbe15 --- /dev/null +++ b/app/test-gtest/Makefile @@ -0,0 +1,67 @@ +# BSD LICENSE +# BSD LICENSE +# +# Copyright(c) 2016 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. + + +ifneq ($(MAKECMDGOALS),clean) +ifeq ($(GTEST_DIR),) +$(error "Please define GTEST_DIR environment variable") +endif + +ifeq ($(GMOCK_DIR),) +$(error "Please define GMOCK_DIR environment variable") +endif +endif + +include $(RTE_SDK)/mk/rte.vars.mk + +# binary name +APP = test-gtest + +# all source are stored in SRCS-y +SRCS-y := main.cpp +SRCS-y += test_mempool.cpp + +CFLAGS += -O0 +CFLAGS += $(WERROR_FLAGS) + +CXXFLAGS += -std=c++11 +CXXFLAGS += -I$(GTEST_DIR)/include +CXXFLAGS += -I$(GMOCK_DIR)/include + +LDFLAGS += -lstdc++ +LDFLAGS += -L$(GMOCK_DIR) -lgmock + +# this application needs libraries first +DEPDIRS-y += lib drivers + +include $(RTE_SDK)/mk/rte.app.mk + diff --git a/app/test-gtest/main.cpp b/app/test-gtest/main.cpp new file mode 100644 index 000..a221823 --- /dev/null +++ b/app/test-gtest/main.cpp @@ -0,0 +1,20 @@ +#include +#include + +#include + +#include +#include + +int main(int argc, char *argv[]) +{ + /* Initialise DPDK EAL */ + int ret = rte_eal_init(argc, argv); + if (ret < 0) + rte_exit(EXIT_FAILURE, "Invalid EAL arguments\n"); + argc -= ret; + argv += ret; + + testing::InitGoogleMock(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/app/test-gtest/test_mempool.cpp b/app/test-gtest/test_mempool.cpp new file mode 100644 index 000..4c3f7e2 --- /dev/null +++ b/app/test-gtest/test_mempool.cpp @@ -0,0 +1,281 @@ +#include +#include + +#include + +#include +#include +#include +#include + +class MemoryPool : public ::testing::Test { + +protected: + + virtual void SetUp() { + mp = NULL; + mempool_elements = (rte_lcore_count() * (lcore_cache_size + + RTE_MEMPOOL_CACHE_MAX_SIZE) -1); + } + + virtual void TearDown() { + if (mp) + rte_mempool_free(mp); + + } + + /* +* save the object number in the first 4 bytes of object data. All +* other bytes are set
[dpdk-dev] [RFC 3/4] eal: add command line option to log output to stdout
Adds new command line options which allows the user to stop application echoing log output to stdout, logs are still written to syslog. Signed-off-by: Declan Doherty --- lib/librte_eal/common/eal_common_log.c | 14 ++ lib/librte_eal/common/eal_common_options.c | 8 lib/librte_eal/common/eal_options.h| 2 ++ lib/librte_eal/common/include/rte_log.h| 15 +++ lib/librte_eal/linuxapp/eal/eal.c | 1 + lib/librte_eal/linuxapp/eal/eal_log.c | 23 ++- 6 files changed, 54 insertions(+), 9 deletions(-) diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c index 7916c78..8319823 100644 --- a/lib/librte_eal/common/eal_common_log.c +++ b/lib/librte_eal/common/eal_common_log.c @@ -45,6 +45,7 @@ struct rte_logs rte_logs = { .type = ~0, .level = RTE_LOG_DEBUG, + .silent = 1, .file = NULL, }; @@ -102,6 +103,18 @@ rte_get_log_level(void) return rte_logs.level; } +void +rte_log_silence_stdout(void) +{ + rte_logs.silent = 0; +} + +int +rte_log_stdout(void) +{ + return rte_logs.silent; +} + /* Set global log type */ void rte_set_log_type(uint32_t type, int enable) @@ -125,6 +138,7 @@ int rte_log_cur_msg_loglevel(void) return RTE_PER_LCORE(log_cur_msg).loglevel; } + /* get the current logtype for the message beeing processed */ int rte_log_cur_msg_logtype(void) { diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c index 1a1bab3..186cfeb 100644 --- a/lib/librte_eal/common/eal_common_options.c +++ b/lib/librte_eal/common/eal_common_options.c @@ -69,6 +69,7 @@ eal_short_options[] = "r:" /* memory ranks */ "v" /* version */ "w:" /* pci-whitelist */ + "s" /* silence log output to stdout */ ; const struct option @@ -95,6 +96,7 @@ eal_long_options[] = { {OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM}, {OPT_VMWARE_TSC_MAP,0, NULL, OPT_VMWARE_TSC_MAP_NUM }, {OPT_XEN_DOM0, 0, NULL, OPT_XEN_DOM0_NUM }, + {OPT_SILENT_STDOUT, 0, NULL, OPT_SILENT_STDOUT_NUM}, {0, 0, NULL, 0} }; @@ -842,6 +844,10 @@ eal_parse_common_option(int opt, const char *optarg, RTE_LOG(CRIT, EAL, "RTE Version: '%s'\n", rte_version()); break; + case OPT_SILENT_STDOUT_NUM: + rte_log_silence_stdout(); + break; + /* long options */ case OPT_HUGE_UNLINK_NUM: conf->hugepage_unlink = 1; @@ -906,6 +912,7 @@ eal_parse_common_option(int opt, const char *optarg, conf->log_level = log; break; } + case OPT_LCORES_NUM: if (eal_parse_lcores(optarg) < 0) { RTE_LOG(ERR, EAL, "invalid parameter for --" @@ -1028,6 +1035,7 @@ eal_common_usage(void) " --"OPT_PROC_TYPE" Type of this process (primary|secondary|auto)\n" " --"OPT_SYSLOG"Set syslog facility\n" " --"OPT_LOG_LEVEL" Set default log level\n" + " -s, --"OPT_SILENT_STDOUT" Silent mode, supress output to STDOUT\n" " -v Display version information on startup\n" " -h, --help This help\n" "\nEAL options for DEBUG use only:\n" diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h index a881c62..ce6547c 100644 --- a/lib/librte_eal/common/eal_options.h +++ b/lib/librte_eal/common/eal_options.h @@ -41,6 +41,8 @@ enum { OPT_PCI_BLACKLIST_NUM = 'b', #define OPT_PCI_WHITELIST "pci-whitelist" OPT_PCI_WHITELIST_NUM = 'w', +#define OPT_SILENT_STDOUT "log-stdout-silent" + OPT_SILENT_STDOUT_NUM = 's', /* first long only option value must be >= 256, so that we won't * conflict with short options */ diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h index b1add04..74acb91 100644 --- a/lib/librte_eal/common/include/rte_log.h +++ b/lib/librte_eal/common/include/rte_log.h @@ -56,6 +56,7 @@ extern "C" { struct rte_logs { uint32_t type; /**< Bitfield with enabled logs. */ uint32_t level; /**< Log level. */ + uint32_t silent:1; /**< Silent mode - Don't print to STDOUT */ FILE *file; /**< Pointer to current FILE* for logs. */ }; @@ -181,6 +182,20 @@ int rte_log_cur_msg_loglevel(void); int rte_log_cur_msg_logtype(void); /** + * Silence output to stdout by logging facilities + */ +void rte_log_silence_stdout(void); + +/** + * Check if echoing log output to stdout is enabled. + * + * @return + * - Returns 1 if echoing to stdout is enabled + * - Returns 0 if logging is in silent mode + */ +int rte_log_stdout(void); + +/** * @depreca
[dpdk-dev] [RFC 2/4] examples: add c++ example application
From: Anatoly Burakov Example application for C++ compilation Signed-off-by: Declan Doherty --- examples/Makefile| 1 + examples/helloworld-cpp/Makefile | 51 + examples/helloworld-cpp/c_code.c | 5 ++ examples/helloworld-cpp/c_header.h | 6 +++ examples/helloworld-cpp/cpp_code.cpp | 89 5 files changed, 152 insertions(+) create mode 100644 examples/helloworld-cpp/Makefile create mode 100644 examples/helloworld-cpp/c_code.c create mode 100644 examples/helloworld-cpp/c_header.h create mode 100644 examples/helloworld-cpp/cpp_code.cpp diff --git a/examples/Makefile b/examples/Makefile index 18b41b9..cb81c6c 100644 --- a/examples/Makefile +++ b/examples/Makefile @@ -46,6 +46,7 @@ endif DIRS-y += ethtool DIRS-y += exception_path DIRS-y += helloworld +DIRS-y += helloworld-cpp DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += ip_pipeline ifeq ($(CONFIG_RTE_LIBRTE_LPM),y) DIRS-$(CONFIG_RTE_IP_FRAG) += ip_reassembly diff --git a/examples/helloworld-cpp/Makefile b/examples/helloworld-cpp/Makefile new file mode 100644 index 000..f8f281c --- /dev/null +++ b/examples/helloworld-cpp/Makefile @@ -0,0 +1,51 @@ +# 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. + +ifeq ($(RTE_SDK),) +$(error "Please define RTE_SDK environment variable") +endif + +# Default target, can be overriden by command line or environment +RTE_TARGET ?= x86_64-native-linuxapp-gcc + +include $(RTE_SDK)/mk/rte.vars.mk + +# binary name +APP = helloworld-cpp + +# all source are stored in SRCS-y +SRCS-y := c_code.c cpp_code.cpp + +CFLAGS += -O3 +CFLAGS += $(WERROR_FLAGS) +LDFLAGS += -lstdc++ + +include $(RTE_SDK)/mk/rte.extapp.mk diff --git a/examples/helloworld-cpp/c_code.c b/examples/helloworld-cpp/c_code.c new file mode 100644 index 000..d349a3b --- /dev/null +++ b/examples/helloworld-cpp/c_code.c @@ -0,0 +1,5 @@ +#include "c_header.h" + +int func(void) { + return 100500; +} diff --git a/examples/helloworld-cpp/c_header.h b/examples/helloworld-cpp/c_header.h new file mode 100644 index 000..3668b60 --- /dev/null +++ b/examples/helloworld-cpp/c_header.h @@ -0,0 +1,6 @@ +#ifndef HEADER_H +#define HEADER_H + +int func(void); + +#endif diff --git a/examples/helloworld-cpp/cpp_code.cpp b/examples/helloworld-cpp/cpp_code.cpp new file mode 100644 index 000..6919048 --- /dev/null +++ b/examples/helloworld-cpp/cpp_code.cpp @@ -0,0 +1,89 @@ +/*- + * 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
[dpdk-dev] [RFC 1/4] mk: Add support for C++ compilation
From: Anatoly Burakov Adding support for compiling C++ files as part of the build system Signed-off-by: Declan Doherty --- mk/internal/rte.compile-pre.mk | 52 ++ mk/target/generic/rte.vars.mk | 3 +++ mk/toolchain/clang/rte.vars.mk | 14 +--- mk/toolchain/gcc/rte.vars.mk | 14 +--- mk/toolchain/icc/rte.vars.mk | 10 +--- 5 files changed, 80 insertions(+), 13 deletions(-) diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk index f740179..2342942 100644 --- a/mk/internal/rte.compile-pre.mk +++ b/mk/internal/rte.compile-pre.mk @@ -32,12 +32,12 @@ # # Common to rte.lib.mk, rte.app.mk, rte.obj.mk # - SRCS-all := $(SRCS-y) $(SRCS-n) $(SRCS-) # convert source to obj file src2obj = $(strip $(patsubst %.c,%.o,\ - $(patsubst %.S,%_s.o,$(1 + $(patsubst %.S,%_s.o,\ + $(patsubst %.cpp,%_cpp.o,$(1) # add a dot in front of the file name dotfile = $(strip $(foreach f,$(1),\ @@ -46,12 +46,14 @@ dotfile = $(strip $(foreach f,$(1),\ # convert source/obj files into dot-dep filename (does not # include .S files) src2dep = $(strip $(call dotfile,$(patsubst %.c,%.o.d, \ - $(patsubst %.S,,$(1) + $(patsubst %.cpp,%_cpp.o.d, \ + $(patsubst %.S,,$(1)) obj2dep = $(strip $(call dotfile,$(patsubst %.o,%.o.d,$(1 # convert source/obj files into dot-cmd filename src2cmd = $(strip $(call dotfile,$(patsubst %.c,%.o.cmd, \ - $(patsubst %.S,%_s.o.cmd,$(1) + $(patsubst %.cpp,%_cpp.o.cmd, \ + $(patsubst %.S,%_s.o.cmd,$(1)) obj2cmd = $(strip $(call dotfile,$(patsubst %.o,%.o.cmd,$(1 OBJS-y := $(call src2obj,$(SRCS-y)) @@ -186,3 +188,45 @@ S_TO_O_DO = @set -e; \ $(depfile_missing),\ $(depfile_newer)),\ $(S_TO_O_DO)) + + +# command to compile a .cpp file to generate an object +ifeq ($(USE_HOST),1) +CXX_TO_O = $(HOSTCXX) -Wp,-MD,$(call obj2dep,$(@)).tmp $(HOST_CXXFLAGS) \ + $(CXXFLAGS_$(@)) $(HOST_EXTRA_CXXFLAGS) -o $@ -c $< +CXX_TO_O_STR = $(subst ','\'',$(CXX_TO_O)) #'# fix syntax highlight +CXX_TO_O_DISP = $(if $(V),"$(CXX_TO_O_STR)"," HOSTCXX $(@)") +else +CXX_TO_O = $(CXX) -Wp,-MD,$(call obj2dep,$(@)).tmp $(CXXFLAGS) \ + $(CXXFLAGS_$(@)) $(EXTRA_CXXFLAGS) -o $@ -c $< +CXX_TO_O_STR = $(subst ','\'',$(CXX_TO_O)) #'# fix syntax highlight +CXX_TO_O_DISP = $(if $(V),"$(CXX_TO_O_STR)"," CXX $(@)") +endif +CXX_TO_O_CMD = 'cmd_$@ = $(CXX_TO_O_STR)' +CXX_TO_O_DO = @set -e; \ + echo $(CXX_TO_O_DISP); \ + $(CXX_TO_O) && \ + echo $(CXX_TO_O_CMD) > $(call obj2cmd,$(@)) && \ + sed 's,'$@':,dep_'$@' =,' $(call obj2dep,$(@)).tmp > $(call obj2dep,$(@)) && \ + rm -f $(call obj2dep,$(@)).tmp + +# +# Compile .cpp file if needed +# Note: dep_$$@ is from the .d file and DEP_$$@ can be specified by +# user (by default it is empty) +# +.SECONDEXPANSION: +%_cpp.o: %.cpp $$(wildcard $$(dep_$$@)) $$(DEP_$$(@)) 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,$(CXX_TO_O))) " ; \ + echo -n "depfile_missing=$(call boolean,$(depfile_missing)) " ; \ + echo "depfile_newer=$(call boolean,$(depfile_newer))") + $(if $(or \ + $(file_missing),\ + $(call cmdline_changed,$(CXX_TO_O)),\ + $(depfile_missing),\ + $(depfile_newer)),\ + $(CXX_TO_O_DO)) \ No newline at end of file diff --git a/mk/target/generic/rte.vars.mk b/mk/target/generic/rte.vars.mk index 75a616a..128d4e2 100644 --- a/mk/target/generic/rte.vars.mk +++ b/mk/target/generic/rte.vars.mk @@ -138,7 +138,10 @@ endif LDFLAGS += -L$(RTE_SDK_BIN)/lib endif +CXXFLAGS := $(CFLAGS) + export CFLAGS +export CXXFLAGS export LDFLAGS endif diff --git a/mk/toolchain/clang/rte.vars.mk b/mk/toolchain/clang/rte.vars.mk index 7749b99..2a7105e 100644 --- a/mk/toolchain/clang/rte.vars.mk +++ b/mk/toolchain/clang/rte.vars.mk @@ -32,8 +32,9 @@ # # toolchain: # -# - define CC, LD, AR, AS, ... (overriden by cmdline value) +# - define CC, CXX, LD, AR, AS, ... (overriden by cmdline value) # - define TOOLCHAIN_CFLAGS variable (overriden by cmdline value) +# - define TOOLCHAIN_CXXFLAGS variable (overriden by cmdline value) # - define TOOLCHAIN_LDFLAGS variable (overriden by cmdline value) # - define TOOLCHAIN_ASFLAGS variable (overriden by cmdline value) # @@ -41,6 +42,7 @@ CC= $(CROSS)clang KERNELCC = $(CROSS)gcc CPP = $(CROSS)cpp +CXX = $(CROSS)clang++ # for now, we don't use as but nasm. # AS = $(CROSS)as AS= nasm @@ -57,10 +59,16 @@ HOSTCC= $(CC) else HOSTCC= clang endif +ifeq ("$(origin CXX)", "command line")
[dpdk-dev] [RFC 0/4] Use Google Test as DPDK unit test framework
I've been trying out using google test as a possible replacement for our unit test framework and have put to together this series of patches with help from Anatoly as RFC to get peoples thoughts on migrating to google test. To facilitate google test this rfc patch set contains build system changes to allow C++ to built within the DPDK framework, this intended for unit test code only, and to support google test which is a C++ framework. Don't worry, I'm not advocating making DPDK a C++ project :) A new EAL option to allow the user to silence logging to stdout while still logging to syslog. This was to facilitate a eclipse plugin I was using which parses googe test output, so it is not strictly necessary. A example application which uses the C++ build changes. And finally a google test application which some mempool tests ported from the current test app to give a feel of what google test is like to use for testing C. Some of the major advantages of google test that I see over continuing to use the current test include giving a consist feel to all tests, a powerful test execution framework which allow individual test suites or tests to be specified from the command line, support for a standard xunit output which can be integrated into a continuous build systems, and a very powerful mocking library which allows much more control over testing failure conditions. Have a look at the patch set keeping in mind that this isn't intended to be production code, I imagine that build systems changes for C++ would require some refinement etc Regards Declan Anatoly Burakov (2): mk: Add support for C++ compilation examples: add c++ example application Declan Doherty (2): eal: add command line option to log output to stdout app/test-gtest: example google test application app/Makefile | 1 + app/test-gtest/Makefile| 67 +++ app/test-gtest/main.cpp| 20 ++ app/test-gtest/test_mempool.cpp| 281 + config/common_base | 5 + examples/Makefile | 1 + examples/helloworld-cpp/Makefile | 51 ++ examples/helloworld-cpp/c_code.c | 5 + examples/helloworld-cpp/c_header.h | 6 + examples/helloworld-cpp/cpp_code.cpp | 89 + lib/librte_eal/common/eal_common_log.c | 14 ++ lib/librte_eal/common/eal_common_options.c | 8 + lib/librte_eal/common/eal_options.h| 2 + lib/librte_eal/common/include/rte_log.h| 15 ++ lib/librte_eal/linuxapp/eal/eal.c | 1 + lib/librte_eal/linuxapp/eal/eal_log.c | 23 ++- mk/internal/rte.compile-pre.mk | 52 +- mk/target/generic/rte.vars.mk | 3 + mk/toolchain/clang/rte.vars.mk | 14 +- mk/toolchain/gcc/rte.vars.mk | 14 +- mk/toolchain/icc/rte.vars.mk | 10 +- 21 files changed, 660 insertions(+), 22 deletions(-) create mode 100644 app/test-gtest/Makefile create mode 100644 app/test-gtest/main.cpp create mode 100644 app/test-gtest/test_mempool.cpp create mode 100644 examples/helloworld-cpp/Makefile create mode 100644 examples/helloworld-cpp/c_code.c create mode 100644 examples/helloworld-cpp/c_header.h create mode 100644 examples/helloworld-cpp/cpp_code.cpp -- 2.5.5
[dpdk-dev] [PATCH v3] net/mlx5: fix possible NULL deref in Rx path
The user is allowed to call ->rx_pkt_burst() even without free mbufs in the pool. In this scenario we'll fail allocating a rep mbuf on the first iteration (where pkt is still NULL). This would cause us to deref a NULL pkt (reset refcount and free). Fix this by checking the pkt before freeing it. Fixes: a1bdb71a32da ("net/mlx5: fix crash in Rx") Signed-off-by: Sagi Grimberg Acked-by: Adrien Mazarguil --- Changes from v2: - fix check-git-log.sh complaints - collected acked-by tag Changes from v1: - check pkt only once in case we failed to allocate a buffer drivers/net/mlx5/mlx5_rxtx.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index fce3381ae87a..37573668e43e 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -1572,6 +1572,14 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) rte_prefetch0(wqe); rep = rte_mbuf_raw_alloc(rxq->mp); if (unlikely(rep == NULL)) { + ++rxq->stats.rx_nombuf; + if (!pkt) { + /* +* no buffers before we even started, +* bail out silently. +*/ + break; + } while (pkt != seg) { assert(pkt != (*rxq->elts)[idx]); seg = NEXT(pkt); @@ -1579,7 +1587,6 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) __rte_mbuf_raw_free(pkt); pkt = seg; } - ++rxq->stats.rx_nombuf; break; } if (!pkt) { -- 1.9.1
[dpdk-dev] [RFC] scripts: make load-devel-config not to appear as executable
2016-08-02 15:54, Christian Ehrhardt: > Quoting the first line of the script: "#! /bin/echo must be loaded with ." > Given that we should drop the .sh file ending as well as the executable > flag - both are not needed to source the file. Hmmm, it is still a file containing some shell commands, right? So why removing the .sh extension?
[dpdk-dev] [PATCH v2] lpm: remove redundant check when adding lpm rule
On Tue, Aug 02, 2016 at 10:09:25AM +0800, Wei Dai wrote: > When a rule with depth > 24 is added into an existing > rule with depth <=24, a new tbl8 is allocated, the existing > rule first fulfill whole new tbl8, so the filed vaild of typo "valid" > each entry in this tbl8 is always true and depth of each > entry is always < 24 before adding new rule with depth > 24. > > Signed-off-by: Wei Dai Acked-by: Bruce Richardson Having to make this change twice shows up the fact that we are still carrying around some version changes for older releases. Given that we are now past the 16.07 release, the old code can probably be removed. Any volunteers to maybe do up a patch for that. Regards, /Bruce
[dpdk-dev] [PATCH 2/2] app/test: add a case to verify lpm tlb8 recycle
On Mon, Aug 01, 2016 at 03:03:44PM +0800, Wei Dai wrote: > As a bug-fix for lpm tlb8 recycle is introduced, > add a test case to verify tlb8 group is correctly > freed when it only includes a rule with depth=24. > > Signed-off-by: Wei Dai Acked-by: Bruce Richardson
[dpdk-dev] [PATCH 1/2] lpm: fix tlb8 only not freed for depth=24
On Mon, Aug 01, 2016 at 03:03:20PM +0800, Wei Dai wrote: > When all rules with depth > 24 are deleted in a same tlb8 group > and only leave a rule with depth <= 24 in this group, the tlb8 > group should be recycled. > The title needs a bit of rewording, I think e.g. lpm: fix freeing unused sub-table on rule delete Otherwise: Acked-by: Bruce Richardson
[dpdk-dev] [RFC] scripts: make load-devel-config not to appear as executable
Quoting the first line of the script: "#! /bin/echo must be loaded with ." Given that we should drop the .sh file ending as well as the executable flag - both are not needed to source the file. Signed-off-by: Christian Ehrhardt --- MAINTAINERS | 2 +- scripts/checkpatches.sh | 2 +- scripts/load-devel-config| 14 ++ scripts/load-devel-config.sh | 14 -- scripts/test-build.sh| 4 ++-- 5 files changed, 18 insertions(+), 18 deletions(-) create mode 100644 scripts/load-devel-config delete mode 100755 scripts/load-devel-config.sh diff --git a/MAINTAINERS b/MAINTAINERS index 6536c6b..f8b99ee 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -27,7 +27,7 @@ F: MAINTAINERS F: scripts/check-maintainers.sh F: scripts/check-git-log.sh F: scripts/checkpatches.sh -F: scripts/load-devel-config.sh +F: scripts/load-devel-config F: scripts/test-build.sh Stable Branches diff --git a/scripts/checkpatches.sh b/scripts/checkpatches.sh index 7111558..b596b4e 100755 --- a/scripts/checkpatches.sh +++ b/scripts/checkpatches.sh @@ -33,7 +33,7 @@ # Load config options: # - DPDK_CHECKPATCH_PATH # - DPDK_CHECKPATCH_LINE_LENGTH -. $(dirname $(readlink -e $0))/load-devel-config.sh +. $(dirname $(readlink -e $0))/load-devel-config length=${DPDK_CHECKPATCH_LINE_LENGTH:-80} diff --git a/scripts/load-devel-config b/scripts/load-devel-config new file mode 100644 index 000..489f007 --- /dev/null +++ b/scripts/load-devel-config @@ -0,0 +1,14 @@ +#! /bin/echo must be loaded with . + +# Load DPDK devel config and allow override +# from system file +test ! -r /etc/dpdk/devel.config || +. /etc/dpdk/devel.config +# from user file +test ! -r ~/.config/dpdk/devel.config || +. ~/.config/dpdk/devel.config +# from local file +test ! -r $(dirname $(readlink -m $0))/../.develconfig || +. $(dirname $(readlink -m $0))/../.develconfig + +# The config files must export variables in the shell style diff --git a/scripts/load-devel-config.sh b/scripts/load-devel-config.sh deleted file mode 100755 index 489f007..000 --- a/scripts/load-devel-config.sh +++ /dev/null @@ -1,14 +0,0 @@ -#! /bin/echo must be loaded with . - -# Load DPDK devel config and allow override -# from system file -test ! -r /etc/dpdk/devel.config || -. /etc/dpdk/devel.config -# from user file -test ! -r ~/.config/dpdk/devel.config || -. ~/.config/dpdk/devel.config -# from local file -test ! -r $(dirname $(readlink -m $0))/../.develconfig || -. $(dirname $(readlink -m $0))/../.develconfig - -# The config files must export variables in the shell style diff --git a/scripts/test-build.sh b/scripts/test-build.sh index d2cafc1..e8971fe 100755 --- a/scripts/test-build.sh +++ b/scripts/test-build.sh @@ -48,7 +48,7 @@ default_path=$PATH # - DPDK_NOTIFY (notify-send) # - LIBSSO_SNOW3G_PATH # - LIBSSO_KASUMI_PATH -. $(dirname $(readlink -e $0))/load-devel-config.sh +. $(dirname $(readlink -e $0))/load-devel-config print_usage () { echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2] ...]]" @@ -211,7 +211,7 @@ for conf in $configs ; do # reload config with DPDK_TARGET set DPDK_TARGET=$target reset_env - . $(dirname $(readlink -e $0))/load-devel-config.sh + . $(dirname $(readlink -e $0))/load-devel-config options=$(echo $conf | sed 's,[^~+]*,,') dir=$conf -- 2.7.4
[dpdk-dev] eal: map shared config into exact same address as primary process (for freebsd)
Hello I am getting a segmentation fault on freebsd 10 due to the shared config mismatch between the primary and secondary process - The following commit seems to have fixed this issue long time back on Linux but was never ported to freebsd. http://dpdk.org/browse/dpdk/commit/?id=6258f1c942c3e0b7bb715158cc266eede6521ddf Was there any specific reason for not fixing this on freebsd? FYI - secondary process : (gdb) p rte_config $2 = {master_lcore = 7, lcore_count = 2, lcore_role = {ROLE_OFF, ROLE_OFF, ROLE_OFF, ROLE_OFF, ROLE_OFF, ROLE_OFF, ROLE_RTE, ROLE_RTE, ROLE_OFF }, process_type = RTE_PROC_SECONDARY, mem_config = 0x80067} primary proces : (gdb) p rte_config $2 = {master_lcore = 0, lcore_count = 6, lcore_role = {ROLE_RTE, ROLE_RTE, ROLE_RTE, ROLE_RTE, ROLE_RTE, ROLE_RTE, ROLE_OFF }, process_type = RTE_PROC_PRIMARY, mem_config = 0x800675000} --tyc
[dpdk-dev] how to design high performance QoS support for a large amount of subscribers
Hi, I am trying to add QoS support for a high performance VNF with large amount of subscribers (millions). It requires to support guaranteed bit rate for different service level of subscribers. I.e. four service levels need to be supported: * Diamond, 500M * Gold, 100M * Silver, 50M * Bronze, 10M Here is the current pipeline design using DPDK: * 4 RX threads, does packet classification and load balancing * 10-20 worker thread, does application subscriber management * 4 TX threads, sends packets to TX NICs. * Ring buffers used among RX threads, Worker threads, and TX threads I read DPDK program guide for QoS framework regarding hierarchical scheduler: Port, sub-port, pipe, TC and queues, I am looking for advice on how to design QoS scheduler to support millions of subscribers (pipes) which traffic are processed in tens of worker threads where subscriber management processing are handled? One design thought is as the following: 8 ports (each one is associated with one physical port), 16-20 sub-ports (each is used by one Worker thread), each sub-port supports 250K pipes for subscribers. Each worker thread manages one sub-port and does metering for the sub-port to get color, and after identity subscriber flow pick a unused pipe, and do sched enqueuer/de-queue and then put into TX rings to TX threads, and TX threads send the packets to TX NICs. Are there functional and performance issues with above approach? Any advice and input are appreciated. Regards, Yuyong
[dpdk-dev] [PATCH v2] net/mlx5: Fix possible NULL deref in RX path
The user is allowed to call ->rx_pkt_burst() even without free mbufs in the pool. In this scenario we'll fail allocating a rep mbuf on the first iteration (where pkt is still NULL). This would cause us to deref a NULL pkt (reset refcount and free). Fix this by checking the pkt before freeing it. Fixes: a1bdb71a32da ("net/mlx5: fix crash in Rx") Signed-off-by: Sagi Grimberg --- Changes from v1: - check pkt only once in case we failed to allocate a buffer drivers/net/mlx5/mlx5_rxtx.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index fce3381ae87a..37573668e43e 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -1572,6 +1572,14 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) rte_prefetch0(wqe); rep = rte_mbuf_raw_alloc(rxq->mp); if (unlikely(rep == NULL)) { + ++rxq->stats.rx_nombuf; + if (!pkt) { + /* +* no buffers before we even started, +* bail out silently. +*/ + break; + } while (pkt != seg) { assert(pkt != (*rxq->elts)[idx]); seg = NEXT(pkt); @@ -1579,7 +1587,6 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) __rte_mbuf_raw_free(pkt); pkt = seg; } - ++rxq->stats.rx_nombuf; break; } if (!pkt) { -- 1.9.1
[dpdk-dev] DPDK Community Survey - about to close
Just 2 minutes for DPDK ;) http://surveymonkey.com/r/DPDK_Community_Survey 2016-08-02 14:39, Thomas Monjalon: > Hi all, > > That's the first time a DPDK survey is published. > It will help us in our future progress to decide what are the most > important stuff to work on. > > Please do not wait to fill it out. It closes on August 4. > Thanks for taking 2 minutes now to give your feedback. > When you will have done your duty, you might peacefully do > your next task or just enjoy a nice August month :) > > Each voice counts! Thanks > > > 2016-07-28 16:45, Glynn, Michael J: > > Hi all > > > > As part of our ongoing efforts to improve DPDK, we'd like to hear > > your feedback! > > > > We have created a number of DPDK-related questions here > > https://www.surveymonkey.com/r/DPDK_Community_Survey > > and want to hear your views!! > > > > The survey will close at midnight GMT on Thursday August 4th > > > > Thanks in advance for your feedback - the more responses we get > > the more data we have to drive further features, improvements, etc... > > so please respond!! > > > > Regards > > Mike >
[dpdk-dev] [dpdk-announce] DPDK Community Survey - about to close
Hi all, That's the first time a DPDK survey is published. It will help us in our future progress to decide what are the most important stuff to work on. Please do not wait to fill it out. It closes on August 4. Thanks for taking 2 minutes now to give your feedback. When you will have done your duty, you might peacefully do your next task or just enjoy a nice August month :) Each voice counts! Thanks 2016-07-28 16:45, Glynn, Michael J: > Hi all > > As part of our ongoing efforts to improve DPDK, we'd like to hear > your feedback! > > We have created a number of DPDK-related questions here > https://www.surveymonkey.com/r/DPDK_Community_Survey > and want to hear your views!! > > The survey will close at midnight GMT on Thursday August 4th > > Thanks in advance for your feedback - the more responses we get > the more data we have to drive further features, improvements, etc... > so please respond!! > > Regards > Mike
[dpdk-dev] [PATCH v2] net/mlx5: Fix possible NULL deref in RX path
On Tue, Aug 02, 2016 at 03:02:18PM +0300, Sagi Grimberg wrote: > The user is allowed to call ->rx_pkt_burst() even without free > mbufs in the pool. In this scenario we'll fail allocating a rep mbuf > on the first iteration (where pkt is still NULL). This would cause us > to deref a NULL pkt (reset refcount and free). > > Fix this by checking the pkt before freeing it. > > Fixes: a1bdb71a32da ("net/mlx5: fix crash in Rx") > Signed-off-by: Sagi Grimberg > --- > Changes from v1: > - check pkt only once in case we failed to allocate a buffer > > drivers/net/mlx5/mlx5_rxtx.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c > index fce3381ae87a..37573668e43e 100644 > --- a/drivers/net/mlx5/mlx5_rxtx.c > +++ b/drivers/net/mlx5/mlx5_rxtx.c > @@ -1572,6 +1572,14 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, > uint16_t pkts_n) > rte_prefetch0(wqe); > rep = rte_mbuf_raw_alloc(rxq->mp); > if (unlikely(rep == NULL)) { > + ++rxq->stats.rx_nombuf; > + if (!pkt) { > + /* > + * no buffers before we even started, > + * bail out silently. > + */ > + break; > + } > while (pkt != seg) { > assert(pkt != (*rxq->elts)[idx]); > seg = NEXT(pkt); > @@ -1579,7 +1587,6 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, > uint16_t pkts_n) > __rte_mbuf_raw_free(pkt); > pkt = seg; > } > - ++rxq->stats.rx_nombuf; > break; > } > if (!pkt) { > -- > 1.9.1 A few nit-picks from check-git-log.sh: Wrong headline uppercase: net/mlx5: Fix possible NULL deref in RX path Wrong headline lowercase: net/mlx5: Fix possible NULL deref in RX path Missing blank line after 'Fixes' tag: Fixes: a1bdb71a32da ("net/mlx5: fix crash in Rx") Otherwise, Acked-by: Adrien Mazarguil -- Adrien Mazarguil 6WIND
[dpdk-dev] [PATCH] net/mlx5: Fix possible NULL deref in RX path
On 02/08/16 12:58, Adrien Mazarguil wrote: > On Tue, Aug 02, 2016 at 12:31:35PM +0300, Sagi Grimberg wrote: >> >> >> On 01/08/16 19:43, Adrien Mazarguil wrote: >>> Hi Sagi, >>> >>> On Mon, Aug 01, 2016 at 11:44:21AM +0300, Sagi Grimberg wrote: The user is allowed to call ->rx_pkt_burst() even without free mbufs in the pool. In this scenario we'll fail allocating a rep mbuf on the first iteration (where pkt is still NULL). This would cause us to deref a NULL pkt (reset refcount and free). Fix this by checking the pkt before freeing it. >>> >>> Just to be sure, did you get an actual NULL deref crash here or is that an >>> assumed possibility? >>> >>> I'm asking because this problem was supposed to be addressed by: >>> >>> a1bdb71a32da ("net/mlx5: fix crash in Rx") >> >> I actually got the NULL deref. This happens when the application doesn't >> restore mbufs to the pool correctly. In the case rte_mbuf_raw_alloc >> will fail on the first iteration (pkt wasn't assigned) unlike the >> condition handled in a1bdb71a32da. >> >> With this applied, I didn't see the crash. > > Thanks for confirming this, Hey Adrien, I just noticed that I missed the rest of your response in the previous message (pre-coffee mail browsing...) You analysis was on spot. > now what about the different approach I > suggested in my previous message to avoid the extra check in the inner loop: > > if (!pkt) > pkt = seg; > while (pkt != seg) { > ... > } We can go this way, but it looks kinda confusing to set pkt = seg and then iterate on pkt != seg. How about a more explicit approach: -- diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index fce3381ae87a..37573668e43e 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -1572,6 +1572,14 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) rte_prefetch0(wqe); rep = rte_mbuf_raw_alloc(rxq->mp); if (unlikely(rep == NULL)) { + ++rxq->stats.rx_nombuf; + if (!pkt) { + /* +* no buffers before we even started, +* bail out silently. +*/ + break; + } while (pkt != seg) { assert(pkt != (*rxq->elts)[idx]); seg = NEXT(pkt); @@ -1579,7 +1587,6 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) __rte_mbuf_raw_free(pkt); pkt = seg; } - ++rxq->stats.rx_nombuf; break; } if (!pkt) { -- > Also the fixes line in your commit message? I'll add it in v2. Thanks.
[dpdk-dev] [PATCH] net/mlx5: Fix possible NULL deref in RX path
On Tue, Aug 02, 2016 at 01:47:55PM +0300, Sagi Grimberg wrote: > > > On 02/08/16 12:58, Adrien Mazarguil wrote: > >On Tue, Aug 02, 2016 at 12:31:35PM +0300, Sagi Grimberg wrote: > >> > >> > >>On 01/08/16 19:43, Adrien Mazarguil wrote: > >>>Hi Sagi, > >>> > >>>On Mon, Aug 01, 2016 at 11:44:21AM +0300, Sagi Grimberg wrote: > The user is allowed to call ->rx_pkt_burst() even without free > mbufs in the pool. In this scenario we'll fail allocating a rep mbuf > on the first iteration (where pkt is still NULL). This would cause us > to deref a NULL pkt (reset refcount and free). > > Fix this by checking the pkt before freeing it. > >>> > >>>Just to be sure, did you get an actual NULL deref crash here or is that an > >>>assumed possibility? > >>> > >>>I'm asking because this problem was supposed to be addressed by: > >>> > >>>a1bdb71a32da ("net/mlx5: fix crash in Rx") > >> > >>I actually got the NULL deref. This happens when the application doesn't > >>restore mbufs to the pool correctly. In the case rte_mbuf_raw_alloc > >>will fail on the first iteration (pkt wasn't assigned) unlike the > >>condition handled in a1bdb71a32da. > >> > >>With this applied, I didn't see the crash. > > > >Thanks for confirming this, > > Hey Adrien, I just noticed that I missed the rest of > your response in the previous message (pre-coffee mail > browsing...) > > You analysis was on spot. > > >now what about the different approach I > >suggested in my previous message to avoid the extra check in the inner loop: > > > > if (!pkt) > > pkt = seg; > > while (pkt != seg) { > > ... > > } > > We can go this way, but it looks kinda confusing to set pkt = seg and > then iterate on pkt != seg. > > How about a more explicit approach: > -- > diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c > index fce3381ae87a..37573668e43e 100644 > --- a/drivers/net/mlx5/mlx5_rxtx.c > +++ b/drivers/net/mlx5/mlx5_rxtx.c > @@ -1572,6 +1572,14 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, > uint16_t pkts_n) > rte_prefetch0(wqe); > rep = rte_mbuf_raw_alloc(rxq->mp); > if (unlikely(rep == NULL)) { > + ++rxq->stats.rx_nombuf; > + if (!pkt) { > + /* > +* no buffers before we even started, > +* bail out silently. > +*/ > + break; > + } > while (pkt != seg) { > assert(pkt != (*rxq->elts)[idx]); > seg = NEXT(pkt); > @@ -1579,7 +1587,6 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, > uint16_t pkts_n) > __rte_mbuf_raw_free(pkt); > pkt = seg; > } > - ++rxq->stats.rx_nombuf; > break; > } > if (!pkt) { > -- Yes, that's also fine. > >Also the fixes line in your commit message? > > I'll add it in v2. Thanks. Go ahead, thanks! -- Adrien Mazarguil 6WIND
[dpdk-dev] [PATCH v3 4/4] eal: fix end character check in --lcores argument
2016-08-02 08:22, Dai, Wei: > Hi, Thomas, Yigit > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > 2016-07-28 16:26, Ferruh Yigit: > > > On 7/27/2016 12:27 PM, Wei Dai wrote: > > > > With --lcores 'a-b at c-d', eal_parse_cores() fails because > > > > eal_parse_set() fails due to the next character after lcore set a-b, > > > > which is '@'and not ',' or '\0'. > > > > There is also a right check immediately after this incorrect check. > > > > > > > > Fixes: 53e54bf81700 ("eal: new option --lcores for cpu assignment") > > > > > > > > Signed-off-by: Wei Dai > > > > > > I am not sure if a-b at c-d syntax should be supported. (a-b)@(c-d) is > > > supported and already working. > > > > Agreed. > > > > Series applied, except this last patch 4, thanks > > Sorry, I can't find any document to clarify whether a-b at c-d shoule be > supported. > So I design following case to verify all kinds of combination of - and @. > I tested my patch with --lcores '0-3 at 12-15, 4-7@(8-11), (8-11)@4-7, > (12-15)@(0-3), 16-19, (20-23) ', OK. So do you agree we can discard this patch and forget this syntax?
[dpdk-dev] [PATCH] net/i40e: fix null pointer dereferences when using VMDQ+RSS
When using VMDQ+RSS, the queue ids used by the application are not contiguous (see i40e_pf_config_rss). Most of the driver already handled this, but there were a few cases where it assumed all configured queues had been setup. Fixes: 4861cde46116 ("i40e: new poll mode driver") Fixes: 6b4537128394 ("i40e: free queue memory when closing") Fixes: 8e109464c022 ("i40e: allow vector Rx and Tx usage") Signed-off-by: Rich Lane --- drivers/net/i40e/i40e_rxtx.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index 554d167..0556a4d 100644 --- a/drivers/net/i40e/i40e_rxtx.c +++ b/drivers/net/i40e/i40e_rxtx.c @@ -2948,11 +2948,15 @@ i40e_dev_clear_queues(struct rte_eth_dev *dev) PMD_INIT_FUNC_TRACE(); for (i = 0; i < dev->data->nb_tx_queues; i++) { + if (!dev->data->tx_queues[i]) + continue; i40e_tx_queue_release_mbufs(dev->data->tx_queues[i]); i40e_reset_tx_queue(dev->data->tx_queues[i]); } for (i = 0; i < dev->data->nb_rx_queues; i++) { + if (!dev->data->rx_queues[i]) + continue; i40e_rx_queue_release_mbufs(dev->data->rx_queues[i]); i40e_reset_rx_queue(dev->data->rx_queues[i]); } @@ -2966,12 +2970,16 @@ i40e_dev_free_queues(struct rte_eth_dev *dev) PMD_INIT_FUNC_TRACE(); for (i = 0; i < dev->data->nb_rx_queues; i++) { + if (!dev->data->rx_queues[i]) + continue; i40e_dev_rx_queue_release(dev->data->rx_queues[i]); dev->data->rx_queues[i] = NULL; } dev->data->nb_rx_queues = 0; for (i = 0; i < dev->data->nb_tx_queues; i++) { + if (!dev->data->tx_queues[i]) + continue; i40e_dev_tx_queue_release(dev->data->tx_queues[i]); dev->data->tx_queues[i] = NULL; } @@ -3154,7 +3162,7 @@ i40e_set_rx_function(struct rte_eth_dev *dev) struct i40e_rx_queue *rxq = dev->data->rx_queues[i]; - if (i40e_rxq_vec_setup(rxq)) { + if (rxq && i40e_rxq_vec_setup(rxq)) { ad->rx_vec_allowed = false; break; } @@ -3216,7 +3224,8 @@ i40e_set_rx_function(struct rte_eth_dev *dev) for (i = 0; i < dev->data->nb_rx_queues; i++) { struct i40e_rx_queue *rxq = dev->data->rx_queues[i]; - rxq->rx_using_sse = rx_using_sse; + if (rxq) + rxq->rx_using_sse = rx_using_sse; } } } @@ -3255,7 +3264,7 @@ i40e_set_tx_function(struct rte_eth_dev *dev) struct i40e_tx_queue *txq = dev->data->tx_queues[i]; - if (i40e_txq_vec_setup(txq)) { + if (txq && i40e_txq_vec_setup(txq)) { ad->tx_vec_allowed = false; break; } -- 1.9.1
[dpdk-dev] [PATCH] net/mlx5: Fix possible NULL deref in RX path
On 01/08/16 19:43, Adrien Mazarguil wrote: > Hi Sagi, > > On Mon, Aug 01, 2016 at 11:44:21AM +0300, Sagi Grimberg wrote: >> The user is allowed to call ->rx_pkt_burst() even without free >> mbufs in the pool. In this scenario we'll fail allocating a rep mbuf >> on the first iteration (where pkt is still NULL). This would cause us >> to deref a NULL pkt (reset refcount and free). >> >> Fix this by checking the pkt before freeing it. > > Just to be sure, did you get an actual NULL deref crash here or is that an > assumed possibility? > > I'm asking because this problem was supposed to be addressed by: > > a1bdb71a32da ("net/mlx5: fix crash in Rx") I actually got the NULL deref. This happens when the application doesn't restore mbufs to the pool correctly. In the case rte_mbuf_raw_alloc will fail on the first iteration (pkt wasn't assigned) unlike the condition handled in a1bdb71a32da. With this applied, I didn't see the crash.
[dpdk-dev] [PATCH] i40e: enable i40e pmd on ARM platform
And add read memory barrier to avoid status inconsistency between two RX descriptors readings. Signed-off-by: Jianbo Liu --- config/defconfig_arm64-armv8a-linuxapp-gcc | 2 +- doc/guides/nics/overview.rst | 2 +- drivers/net/i40e/i40e_rxtx.c | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/config/defconfig_arm64-armv8a-linuxapp-gcc b/config/defconfig_arm64-armv8a-linuxapp-gcc index 1a17126..08f282b 100644 --- a/config/defconfig_arm64-armv8a-linuxapp-gcc +++ b/config/defconfig_arm64-armv8a-linuxapp-gcc @@ -46,6 +46,6 @@ CONFIG_RTE_EAL_IGB_UIO=n CONFIG_RTE_LIBRTE_IVSHMEM=n CONFIG_RTE_LIBRTE_FM10K_PMD=n -CONFIG_RTE_LIBRTE_I40E_PMD=n +CONFIG_RTE_LIBRTE_I40E_INC_VECTOR=n CONFIG_RTE_SCHED_VECTOR=n diff --git a/doc/guides/nics/overview.rst b/doc/guides/nics/overview.rst index 6abbae6..5175591 100644 --- a/doc/guides/nics/overview.rst +++ b/doc/guides/nics/overview.rst @@ -138,7 +138,7 @@ Most of these differences are summarized below. Linux VFIO Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Other kdrv Y Y Y ARMv7 Y Y Y - ARMv8 Y Y Y Y Y Y Y Y + ARMv8 Y Y Y Y Y Y Y Y Y Power8 Y Y Y TILE-Gx Y x86-32 Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index 554d167..4004b8e 100644 --- a/drivers/net/i40e/i40e_rxtx.c +++ b/drivers/net/i40e/i40e_rxtx.c @@ -994,6 +994,8 @@ i40e_rx_scan_hw_ring(struct i40e_rx_queue *rxq) I40E_RXD_QW1_STATUS_SHIFT; } + rte_rmb(); + /* Compute how many status bits were set */ for (j = 0, nb_dd = 0; j < I40E_LOOK_AHEAD; j++) nb_dd += s[j] & (1 << I40E_RX_DESC_STATUS_DD_SHIFT); -- 2.4.11
[dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
Hello Wei, On Tue, Aug 2, 2016 at 2:59 AM, Zhao1, Wei wrote: > Hi, Wujingjing and Kyle Larose > > > >> -Original Message- >> From: Zhao1, Wei >> Sent: Tuesday, August 2, 2016 11:27 AM >> To: Wu, Jingjing ; Lu, Wenzhuo >> >> Cc: dev at dpdk.org >> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent >> >> Hi,Wu jingjing and wenzhuo >> >> > -Original Message- >> > From: Zhao1, Wei >> > Sent: Monday, August 1, 2016 4:58 PM >> > To: 'Kyle Larose' >> > Cc: dev at dpdk.org >> > Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic >> > inconsistent >> > >> > Hi, Kyle Larose >> >The core problem is i40e has no statistic of discard bytes, that >> > means even if when ports are not stopped, the statistic rx_good_bytes >> > is consist of discard >> > bytes?is that reasonable? In other words, I can just minus discard >> > bytes from rx_good_bytes if I can get discard bytes number, that is much >> better. >> > >> > -Original Message- >> > From: Kyle Larose [mailto:eomereadig at gmail.com] >> > Sent: Saturday, July 30, 2016 1:17 AM >> > To: Zhao1, Wei >> > Cc: dev at dpdk.org >> > Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic >> > inconsistent >> > >> > On Fri, Jul 29, 2016 at 4:50 AM, Wei Zhao1 wrote: >> > > rx_good_bytes and rx_good_packets statistic is inconsistent when >> > > port stopped,ipackets statistic is minus the discard packets but >> > > rx_bytes statistic not.Also,i40e has no statistic of discard bytes, >> > > so we have to delete discard packets item from rx_good_packets statistic. >> > > >> > > Fixes: 9aace75fc82e ("i40e: fix statistics") >> > > >> > > Signed-off-by: Wei Zhao1 >> > > --- >> > > drivers/net/i40e/i40e_ethdev.c | 3 +-- >> > > 1 file changed, 1 insertion(+), 2 deletions(-) >> > > >> > > diff --git a/drivers/net/i40e/i40e_ethdev.c >> > > b/drivers/net/i40e/i40e_ethdev.c index 11a5804..553dfd9 100644 >> > > --- a/drivers/net/i40e/i40e_ethdev.c >> > > +++ b/drivers/net/i40e/i40e_ethdev.c >> > > @@ -2319,8 +2319,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, >> > > struct rte_eth_stats *stats) >> > > >> > > stats->ipackets = pf->main_vsi->eth_stats.rx_unicast + >> > > pf->main_vsi->eth_stats.rx_multicast + >> > > - pf->main_vsi->eth_stats.rx_broadcast - >> > > - pf->main_vsi->eth_stats.rx_discards; >> > > + pf->main_vsi->eth_stats.rx_broadcast; >> > > stats->opackets = pf->main_vsi->eth_stats.tx_unicast + >> > > pf->main_vsi->eth_stats.tx_multicast + >> > > pf->main_vsi->eth_stats.tx_broadcast; >> > > -- >> > > 2.5.5 >> > > >> > >> > Is it not worse to report a received packet when no packet was >> > actually received by the upper layers under normal operations than to >> > ensure that packets and bytes are consistent when an interface is >> > stopped? It seems like the first case is much more likely to occur than the >> second. >> > >> > Are we just introducing a new issue to fix another? >> > >> > How does this behaviour compare to other NICs? Does the ixgbe report >> > discarded packets in its ipackets? My reading of the driver is that it >> > does not. >> > In fact, it does something interesting to deal with the >> > problem: >> > >> > from: >> > http://dpdk.org/browse/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c >> > >> > /* >> > * An errata states that gprc actually counts good + missed packets: >> > * Workaround to set gprc to summated queue packet receives */ >> > hw_stats- >> > >gprc = *total_qprc; >> > >> > total_gprc is equal to the sum of the qprc per queue. Can we do >> > something similar on the i40e instead of adding unicast, mulitcast and >> broadcast? >> >> >> I have checked ixgbe code about Rx statistic, in function >> ixgbe_read_stats_registers() we can find the rx_good_bytes and >> rx_good_packets statistic. >> It is listed below, we can see rx_good_packets is also just addition of >> Queue >> Packets Received Count and not minused discard packet number. >> Is there some wrong of understanding? My understanding of the problem can be broken into three parts: 1) In Unicast/Multicast/Broadcast packet counters are counting packets which were discarded 2) The corresponding byte counters count packets which were discarded. 3) There are no discarded byte counters. Our in bytes counter consists of the sum of in unicast, in multicast, and in broadcast. This sum includes discarded bytes, which we do not want, for two reasons. First, it would lead to misleading bitrate reports: people expect to see the amount of traffic actually handled. Second, it conflicts with the current packet counters (the counters without your change). Obviously if we could count the discarded bytes, we could subtract them. Alternatively, if we could count only received bytes which were not discarded, then we would not need to subtract discarded bytes from the re
[dpdk-dev] [PATCH] net/mlx5: Fix possible NULL deref in RX path
On Tue, Aug 02, 2016 at 12:31:35PM +0300, Sagi Grimberg wrote: > > > On 01/08/16 19:43, Adrien Mazarguil wrote: > >Hi Sagi, > > > >On Mon, Aug 01, 2016 at 11:44:21AM +0300, Sagi Grimberg wrote: > >>The user is allowed to call ->rx_pkt_burst() even without free > >>mbufs in the pool. In this scenario we'll fail allocating a rep mbuf > >>on the first iteration (where pkt is still NULL). This would cause us > >>to deref a NULL pkt (reset refcount and free). > >> > >>Fix this by checking the pkt before freeing it. > > > >Just to be sure, did you get an actual NULL deref crash here or is that an > >assumed possibility? > > > >I'm asking because this problem was supposed to be addressed by: > > > > a1bdb71a32da ("net/mlx5: fix crash in Rx") > > I actually got the NULL deref. This happens when the application doesn't > restore mbufs to the pool correctly. In the case rte_mbuf_raw_alloc > will fail on the first iteration (pkt wasn't assigned) unlike the > condition handled in a1bdb71a32da. > > With this applied, I didn't see the crash. Thanks for confirming this, now what about the different approach I suggested in my previous message to avoid the extra check in the inner loop: if (!pkt) pkt = seg; while (pkt != seg) { ... } Also the fixes line in your commit message? -- Adrien Mazarguil 6WIND
[dpdk-dev] [PATCH] table: add missing exports
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Aleksey Katargin > Sent: Monday, August 1, 2016 10:01 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH] table: add missing exports > > Signed-off-by: Aleksey Katargin > --- > lib/librte_table/rte_table_version.map | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/librte_table/rte_table_version.map > b/lib/librte_table/rte_table_version.map > index 2138698..459c2da 100644 > --- a/lib/librte_table/rte_table_version.map > +++ b/lib/librte_table/rte_table_version.map > @@ -3,6 +3,7 @@ DPDK_2.0 { > Acked-by: Cristian Dumitrescu Hi Aleksey, I checked all DPDK releases 2.0 -> 16.07 and yes, you're exactly right. Ouch!!! Thanks very much for fixing this! Regards, Cristian
[dpdk-dev] [RFC] Generic flow director/filtering/classification API
On 16-07-23 02:10 PM, John Fastabend wrote: > On 16-07-21 12:20 PM, Adrien Mazarguil wrote: >> Hi Jerin, >> >> Sorry, looks like I missed your reply. Please see below. >> > > Hi Adrian, > > Sorry for a bit delay but a few comments that may be worth considering. > > To start with completely agree on the general problem statement and the > nice summary of all the current models. Also good start on this. > >> >> Considering that allowed pattern/actions combinations cannot be known in >> advance and would result in an unpractically large number of capabilities to >> expose, a method is provided to validate a given rule from the current >> device configuration state without actually adding it (akin to a "dry run" >> mode). > > Rather than have a query/validate process why did we jump over having an > intermediate representation of the capabilities? Here you state it is > unpractical but we know how to represent parse graphs and the drivers > could report their supported parse graph via a single query to a middle > layer. > > This will actually reduce the msg chatter imagine many applications at > init time or in boundary cases where a large set of applications come > online at once and start banging on the interface all at once seems less > than ideal. > A bit more details on possible interface for capabilities query, One way I've used to describe these graphs from driver to software stacks is to use a set of structures to build the graph. For fixed graphs this could just be *.h file for programmable hardware (typically coming from fw update on nics) the driver can read the parser details out of firmware and render the structures. I've done this two ways: one is to define all the fields in their own structures using something like, struct field { char *name; u32 uid; u32 bitwidth; }; This gives a unique id (uid) for each field along with its width and a user friendly name. The fields are organized into headers via a header structure, struct header_node { char *name; u32 uid; u32 *fields; struct parse_graph *jump; }; Each node has a unique id and then a list of fields. Where 'fields' is a list of uid's of fields its also easy enough to embed the field struct in the header_node if that is simpler its really a style question. The 'struct parse_graph' gives the list of edges from this header node to other header nodes. Using a parse graph structure defined struct parse_graph { struct field_reference ref; __u32 jump_uid; }; Again as a matter of style you can embed the parse graph in the header node as I did above or do it as its own object. The field_reference noted below gives the id of the field and the value e.g. the tuple (ipv4.protocol, 6) then jump_uid would be the uid of TCP. struct field_reference { __u32 header_uid; __u32 field_uid; __u32 mask_type; __u32 type; __u8 *value; __u8 *mask; }; The cost doing all this is some additional overhead at init time. But building generic function over this and having a set of predefined uids for well-known protocols such ip, udp, tcp, etc helps. What you get for the cost is a few things that I think are worth it. (i) Now new protocols can be added/removed without recompiling DPDK (ii) a software package can use the capability query to verify the required protocols are off-loadable vs a possibly large set of test queries and (iii) when we do the programming of the device we can provide a tuple (table-uid, header-uid, field-uid, value, mask, priority) and the middle layer "knowing" the above graph can verify the command so drivers only ever see "good" commands, (iv) finally it should be faster in terms of cmds per second because the drivers can map the tuple (table, header, field, priority) to a slot efficiently vs parsing. IMO point (iii) and (iv) will in practice make the code much simpler because we can maintain common middle layer and not require parsing by drivers. Making each driver simpler by abstracting into common layer. > Worse in my opinion it requires all drivers to write mostly duplicating > validation code where a common layer could easily do this if every > driver reported a common data structure representing its parse graph > instead. The nice fallout of this initial effort upfront is the driver > no longer needs to do error handling/checking/etc and can assume all > rules are correct and valid. It makes driver code much simpler to > support. And IMO at least by doing this we get some other nice benefits > described below. > > Another related question is about performance. > >> Creation >> >> >> Creating a flow rule is similar to validating one, except the rule is >> actually created. >> >> :: >> >> struct rte_flow * >> rte_flow_create(uint8_t port_id, >> const struct rte_flow_pattern *pattern, >> const struct rte_flow_actions *actions); > > I gather this implies that
[dpdk-dev] [PATCH v2] lpm: remove redundant check when adding lpm rule
When a rule with depth > 24 is added into an existing rule with depth <=24, a new tbl8 is allocated, the existing rule first fulfill whole new tbl8, so the filed vaild of each entry in this tbl8 is always true and depth of each entry is always < 24 before adding new rule with depth > 24. Signed-off-by: Wei Dai --- lib/librte_lpm/rte_lpm.c | 22 ++ 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c index 24fec4b..ec67765 100644 --- a/lib/librte_lpm/rte_lpm.c +++ b/lib/librte_lpm/rte_lpm.c @@ -931,32 +931,27 @@ add_depth_big_v20(struct rte_lpm_v20 *lpm, uint32_t ip_masked, uint8_t depth, /* Populate new tbl8 with tbl24 value. */ for (i = tbl8_group_start; i < tbl8_group_end; i++) { lpm->tbl8[i].valid = VALID; lpm->tbl8[i].depth = lpm->tbl24[tbl24_index].depth; lpm->tbl8[i].next_hop = lpm->tbl24[tbl24_index].next_hop; } tbl8_index = tbl8_group_start + (ip_masked & 0xFF); /* Insert new rule into the tbl8 entry. */ for (i = tbl8_index; i < tbl8_index + tbl8_range; i++) { - if (!lpm->tbl8[i].valid || - lpm->tbl8[i].depth <= depth) { - lpm->tbl8[i].valid = VALID; - lpm->tbl8[i].depth = depth; - lpm->tbl8[i].next_hop = next_hop; - - continue; - } + lpm->tbl8[i].valid = VALID; + lpm->tbl8[i].depth = depth; + lpm->tbl8[i].next_hop = next_hop; } /* * Update tbl24 entry to point to new tbl8 entry. Note: The * ext_flag and tbl8_index need to be updated simultaneously, * so assign whole structure in one go. */ struct rte_lpm_tbl_entry_v20 new_tbl24_entry = { { .group_idx = (uint8_t)tbl8_group_index, }, .valid = VALID, .valid_group = 1, @@ -1062,32 +1057,27 @@ add_depth_big_v1604(struct rte_lpm *lpm, uint32_t ip_masked, uint8_t depth, /* Populate new tbl8 with tbl24 value. */ for (i = tbl8_group_start; i < tbl8_group_end; i++) { lpm->tbl8[i].valid = VALID; lpm->tbl8[i].depth = lpm->tbl24[tbl24_index].depth; lpm->tbl8[i].next_hop = lpm->tbl24[tbl24_index].next_hop; } tbl8_index = tbl8_group_start + (ip_masked & 0xFF); /* Insert new rule into the tbl8 entry. */ for (i = tbl8_index; i < tbl8_index + tbl8_range; i++) { - if (!lpm->tbl8[i].valid || - lpm->tbl8[i].depth <= depth) { - lpm->tbl8[i].valid = VALID; - lpm->tbl8[i].depth = depth; - lpm->tbl8[i].next_hop = next_hop; - - continue; - } + lpm->tbl8[i].valid = VALID; + lpm->tbl8[i].depth = depth; + lpm->tbl8[i].next_hop = next_hop; } /* * Update tbl24 entry to point to new tbl8 entry. Note: The * ext_flag and tbl8_index need to be updated simultaneously, * so assign whole structure in one go. */ struct rte_lpm_tbl_entry new_tbl24_entry = { .group_idx = (uint8_t)tbl8_group_index, .valid = VALID, .valid_group = 1, -- 2.5.5
[dpdk-dev] [PATCH v2] examples: fix unusual-interpreter
> -Original Message- > From: Christian Ehrhardt [mailto:christian.ehrhardt at canonical.com] > Sent: Tuesday, August 2, 2016 7:40 AM > To: christian.ehrhardt at canonical.com; thomas.monjalon at 6wind.com; > Dumitrescu, Cristian ; dev at dpdk.org > Subject: [PATCH v2] examples: fix unusual-interpreter > > *update in v2* > - use #!/usr/bin/env python as usually recommended and suggested in the > discussion > > Due to regular lintian checks in Debian packaging it surfaced that these > two scripts had a space in their #! statement which renders it to be > human, but not shell readable. > > Fixes: 8673a3e8 ("examples/ip_pipeline: add config diagram generator") > Fixes: fa667b46 ("examples/ip_pipeline: add core mappings script") > > Signed-off-by: Christian Ehrhardt > --- > examples/ip_pipeline/config/diagram-generator.py| 2 +- > examples/ip_pipeline/config/pipeline-to-core-mapping.py | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > Acked-by: Cristian Dumitrescu
[dpdk-dev] [PATCH] qat: change the session structure to be variable sized
This patch changes the qat session data structure sent to qat from a fixed size to a variable size which is dependent on the size of the chosen algorithm. This reduces the amount of bytes which are transferred across PCIe and thus helps to increase qat performance when the accelerator is bound by PCIe. Signed-off-by: John Griffin --- drivers/crypto/qat/qat_adf/qat_algs.h| 5 +- drivers/crypto/qat/qat_adf/qat_algs_build_desc.c | 462 +-- drivers/crypto/qat/qat_crypto.c | 15 +- 3 files changed, 183 insertions(+), 299 deletions(-) diff --git a/drivers/crypto/qat/qat_adf/qat_algs.h b/drivers/crypto/qat/qat_adf/qat_algs.h index 243c1b4..6a86053 100644 --- a/drivers/crypto/qat/qat_adf/qat_algs.h +++ b/drivers/crypto/qat/qat_adf/qat_algs.h @@ -87,8 +87,10 @@ struct qat_session { enum icp_qat_hw_cipher_mode qat_mode; enum icp_qat_hw_auth_algo qat_hash_alg; struct qat_alg_cd cd; + uint8_t *cd_cur_ptr; phys_addr_t cd_paddr; struct icp_qat_fw_la_bulk_req fw_req; + uint8_t aad_len; struct qat_crypto_instance *inst; uint8_t salt[ICP_QAT_HW_AES_BLK_SZ]; rte_spinlock_t lock;/* protects this struct */ @@ -115,7 +117,8 @@ int qat_alg_aead_session_create_content_desc_auth(struct qat_session *cdesc, uint32_t digestsize, unsigned int operation); -void qat_alg_init_common_hdr(struct icp_qat_fw_comn_req_hdr *header); +void qat_alg_init_common_hdr(struct icp_qat_fw_comn_req_hdr *header, + uint16_t proto); void qat_alg_ablkcipher_init_enc(struct qat_alg_ablkcipher_cd *cd, int alg, const uint8_t *key, diff --git a/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c b/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c index 185bb33..6de8919 100644 --- a/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c +++ b/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c @@ -344,7 +344,8 @@ static int qat_alg_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, return 0; } -void qat_alg_init_common_hdr(struct icp_qat_fw_comn_req_hdr *header) +void qat_alg_init_common_hdr(struct icp_qat_fw_comn_req_hdr *header, + uint16_t proto) { PMD_INIT_FUNC_TRACE(); header->hdr_flags = @@ -358,7 +359,7 @@ void qat_alg_init_common_hdr(struct icp_qat_fw_comn_req_hdr *header) ICP_QAT_FW_LA_CIPH_IV_FLD_FLAG_SET(header->serv_specif_flags, ICP_QAT_FW_CIPH_IV_16BYTE_DATA); ICP_QAT_FW_LA_PROTO_SET(header->serv_specif_flags, - ICP_QAT_FW_LA_NO_PROTO); + proto); ICP_QAT_FW_LA_UPDATE_STATE_SET(header->serv_specif_flags, ICP_QAT_FW_LA_NO_UPDATE_STATE); } @@ -375,127 +376,88 @@ int qat_alg_aead_session_create_content_desc_cipher(struct qat_session *cdesc, struct icp_qat_fw_cipher_cd_ctrl_hdr *cipher_cd_ctrl = ptr; struct icp_qat_fw_auth_cd_ctrl_hdr *hash_cd_ctrl = ptr; enum icp_qat_hw_cipher_convert key_convert; + uint32_t total_key_size; uint16_t proto = ICP_QAT_FW_LA_NO_PROTO;/* no CCM/GCM/Snow3G */ - uint16_t cipher_offset = 0; + uint16_t cipher_offset, cd_size; PMD_INIT_FUNC_TRACE(); - if (cdesc->qat_cmd == ICP_QAT_FW_LA_CMD_HASH_CIPHER && - cdesc->qat_hash_alg != ICP_QAT_HW_AUTH_ALGO_SNOW_3G_UIA2) { - cipher = - (struct icp_qat_hw_cipher_algo_blk *)((char *)&cdesc->cd + - sizeof(struct icp_qat_hw_auth_algo_blk)); - cipher_offset = sizeof(struct icp_qat_hw_auth_algo_blk); - } else { - cipher = (struct icp_qat_hw_cipher_algo_blk *)&cdesc->cd; - cipher_offset = 0; - } - /* CD setup */ - if (cdesc->qat_dir == ICP_QAT_HW_CIPHER_ENCRYPT) { - ICP_QAT_FW_LA_RET_AUTH_SET(header->serv_specif_flags, - ICP_QAT_FW_LA_RET_AUTH_RES); - ICP_QAT_FW_LA_CMP_AUTH_SET(header->serv_specif_flags, - ICP_QAT_FW_LA_NO_CMP_AUTH_RES); - } else { + if (cdesc->qat_cmd == ICP_QAT_FW_LA_CMD_CIPHER) { + cd_pars->u.s.content_desc_addr = cdesc->cd_paddr; + ICP_QAT_FW_COMN_CURR_ID_SET(cipher_cd_ctrl, + ICP_QAT_FW_SLICE_CIPHER); + ICP_QAT_FW_COMN_NEXT_ID_SET(cipher_cd_ctrl, + ICP_QAT_FW_SLICE_DRAM_WR); ICP_QAT_FW_LA_RET_AUTH_SET(header->serv_specif_flags, ICP_QAT_FW_LA_NO_RET_AUTH_RES); ICP_QAT_FW_LA_CMP_AUTH_SET(header->serv_specif_flags, -
[dpdk-dev] [PATCH v2] examples: fix unusual-interpreter
*update in v2* - use #!/usr/bin/env python as usually recommended and suggested in the discussion Due to regular lintian checks in Debian packaging it surfaced that these two scripts had a space in their #! statement which renders it to be human, but not shell readable. Fixes: 8673a3e8 ("examples/ip_pipeline: add config diagram generator") Fixes: fa667b46 ("examples/ip_pipeline: add core mappings script") Signed-off-by: Christian Ehrhardt --- examples/ip_pipeline/config/diagram-generator.py| 2 +- examples/ip_pipeline/config/pipeline-to-core-mapping.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/ip_pipeline/config/diagram-generator.py b/examples/ip_pipeline/config/diagram-generator.py index f20cbcb..6b7170b 100755 --- a/examples/ip_pipeline/config/diagram-generator.py +++ b/examples/ip_pipeline/config/diagram-generator.py @@ -1,4 +1,4 @@ -#! /usr/bin/python2 +#!/usr/bin/env python # BSD LICENSE # diff --git a/examples/ip_pipeline/config/pipeline-to-core-mapping.py b/examples/ip_pipeline/config/pipeline-to-core-mapping.py index 37b131c..c2050b8 100755 --- a/examples/ip_pipeline/config/pipeline-to-core-mapping.py +++ b/examples/ip_pipeline/config/pipeline-to-core-mapping.py @@ -1,4 +1,4 @@ -#! /usr/bin/python2 +#!/usr/bin/env python # BSD LICENSE # -- 2.7.4
[dpdk-dev] [PATCH v3 4/4] eal: fix end character check in --lcores argument
Hi, Thomas, Yigit > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Friday, July 29, 2016 12:06 AM > To: Dai, Wei > Cc: dev at dpdk.org; Yigit, Ferruh > Subject: Re: [dpdk-dev] [PATCH v3 4/4] eal: fix end character check in > --lcores > argument > > 2016-07-28 16:26, Ferruh Yigit: > > On 7/27/2016 12:27 PM, Wei Dai wrote: > > > With --lcores 'a-b at c-d', eal_parse_cores() fails because > > > eal_parse_set() fails due to the next character after lcore set a-b, > > > which is '@'and not ',' or '\0'. > > > There is also a right check immediately after this incorrect check. > > > > > > Fixes: 53e54bf81700 ("eal: new option --lcores for cpu assignment") > > > > > > Signed-off-by: Wei Dai > > > > I am not sure if a-b at c-d syntax should be supported. (a-b)@(c-d) is > > supported and already working. > > Agreed. > > Series applied, except this last patch 4, thanks Sorry, I can't find any document to clarify whether a-b at c-d shoule be supported. So I design following case to verify all kinds of combination of - and @. I tested my patch with --lcores '0-3 at 12-15, 4-7@(8-11), (8-11)@4-7, (12-15)@(0-3), 16-19, (20-23) ', After calling eal_parse_args(argc, argv), I observed lcore_config[i].cpuset to check cpuset of each lcore. I added following codes immediately after calling fctret = eal_parse_args(argc, argv); in function rte_eal_init( ) in eal.c to check this patch. { struct rte_config *p; const char *rte_role_str[2] = {"ROLE_RTE", "ROLE_OFF"}; p = rte_eal_get_configuration(); for (i=0; i<28; i++) { printf("lcore %2d status = %s, runs on cpuset = 0x%08lX\n", i, rte_role_str[p->lcore_role[i]], lcore_config[i].cpuset.__bits[0]); } } The result is as follow: lcore 0 status = ROLE_RTE, runs on cpuset = 0xF000 lcore 1 status = ROLE_RTE, runs on cpuset = 0xF000 lcore 2 status = ROLE_RTE, runs on cpuset = 0xF000 lcore 3 status = ROLE_RTE, runs on cpuset = 0xF000 lcore 4 status = ROLE_RTE, runs on cpuset = 0x0F00 lcore 5 status = ROLE_RTE, runs on cpuset = 0x0F00 lcore 6 status = ROLE_RTE, runs on cpuset = 0x0F00 lcore 7 status = ROLE_RTE, runs on cpuset = 0x0F00 lcore 8 status = ROLE_RTE, runs on cpuset = 0x00F0 lcore 9 status = ROLE_RTE, runs on cpuset = 0x00F0 lcore 10 status = ROLE_RTE, runs on cpuset = 0x00F0 lcore 11 status = ROLE_RTE, runs on cpuset = 0x00F0 lcore 12 status = ROLE_RTE, runs on cpuset = 0x000F lcore 13 status = ROLE_RTE, runs on cpuset = 0x000F lcore 14 status = ROLE_RTE, runs on cpuset = 0x000F lcore 15 status = ROLE_RTE, runs on cpuset = 0x000F lcore 16 status = ROLE_RTE, runs on cpuset = 0x0001 lcore 17 status = ROLE_RTE, runs on cpuset = 0x0002 lcore 18 status = ROLE_RTE, runs on cpuset = 0x0004 lcore 19 status = ROLE_RTE, runs on cpuset = 0x0008 lcore 20 status = ROLE_RTE, runs on cpuset = 0x00F0 lcore 21 status = ROLE_RTE, runs on cpuset = 0x00F0 lcore 22 status = ROLE_RTE, runs on cpuset = 0x00F0 lcore 23 status = ROLE_RTE, runs on cpuset = 0x00F0 lcore 24 status = ROLE_OFF, runs on cpuset = 0x lcore 25 status = ROLE_OFF, runs on cpuset = 0x lcore 26 status = ROLE_OFF, runs on cpuset = 0x lcore 27 status = ROLE_OFF, runs on cpuset = 0x
[dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
Hi, Wujingjing and Kyle Larose > -Original Message- > From: Zhao1, Wei > Sent: Tuesday, August 2, 2016 11:27 AM > To: Wu, Jingjing ; Lu, Wenzhuo > > Cc: dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent > > Hi,Wu jingjing and wenzhuo > > > -Original Message- > > From: Zhao1, Wei > > Sent: Monday, August 1, 2016 4:58 PM > > To: 'Kyle Larose' > > Cc: dev at dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic > > inconsistent > > > > Hi, Kyle Larose > >The core problem is i40e has no statistic of discard bytes, that > > means even if when ports are not stopped, the statistic rx_good_bytes > > is consist of discard > > bytes?is that reasonable? In other words, I can just minus discard > > bytes from rx_good_bytes if I can get discard bytes number, that is much > better. > > > > -Original Message- > > From: Kyle Larose [mailto:eomereadig at gmail.com] > > Sent: Saturday, July 30, 2016 1:17 AM > > To: Zhao1, Wei > > Cc: dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic > > inconsistent > > > > On Fri, Jul 29, 2016 at 4:50 AM, Wei Zhao1 wrote: > > > rx_good_bytes and rx_good_packets statistic is inconsistent when > > > port stopped,ipackets statistic is minus the discard packets but > > > rx_bytes statistic not.Also,i40e has no statistic of discard bytes, > > > so we have to delete discard packets item from rx_good_packets statistic. > > > > > > Fixes: 9aace75fc82e ("i40e: fix statistics") > > > > > > Signed-off-by: Wei Zhao1 > > > --- > > > drivers/net/i40e/i40e_ethdev.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/drivers/net/i40e/i40e_ethdev.c > > > b/drivers/net/i40e/i40e_ethdev.c index 11a5804..553dfd9 100644 > > > --- a/drivers/net/i40e/i40e_ethdev.c > > > +++ b/drivers/net/i40e/i40e_ethdev.c > > > @@ -2319,8 +2319,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, > > > struct rte_eth_stats *stats) > > > > > > stats->ipackets = pf->main_vsi->eth_stats.rx_unicast + > > > pf->main_vsi->eth_stats.rx_multicast + > > > - pf->main_vsi->eth_stats.rx_broadcast - > > > - pf->main_vsi->eth_stats.rx_discards; > > > + pf->main_vsi->eth_stats.rx_broadcast; > > > stats->opackets = pf->main_vsi->eth_stats.tx_unicast + > > > pf->main_vsi->eth_stats.tx_multicast + > > > pf->main_vsi->eth_stats.tx_broadcast; > > > -- > > > 2.5.5 > > > > > > > Is it not worse to report a received packet when no packet was > > actually received by the upper layers under normal operations than to > > ensure that packets and bytes are consistent when an interface is > > stopped? It seems like the first case is much more likely to occur than the > second. > > > > Are we just introducing a new issue to fix another? > > > > How does this behaviour compare to other NICs? Does the ixgbe report > > discarded packets in its ipackets? My reading of the driver is that it does > > not. > > In fact, it does something interesting to deal with the > > problem: > > > > from: > > http://dpdk.org/browse/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c > > > > /* > > * An errata states that gprc actually counts good + missed packets: > > * Workaround to set gprc to summated queue packet receives */ > > hw_stats- > > >gprc = *total_qprc; > > > > total_gprc is equal to the sum of the qprc per queue. Can we do > > something similar on the i40e instead of adding unicast, mulitcast and > broadcast? > > > I have checked ixgbe code about Rx statistic, in function > ixgbe_read_stats_registers() we can find the rx_good_bytes and > rx_good_packets statistic. > It is listed below, we can see rx_good_packets is also just addition of Queue > Packets Received Count and not minused discard packet number. > Is there some wrong of understanding? > > for (i = 0; i < IXGBE_QUEUE_STAT_COUNTERS; i++) { > .. > *total_qprc += hw_stats->qprc[i]; > *total_qbrc += hw_stats->qbrc[i]; > .. > } The problem is i40e has no statistic of discard bytes, so it is impossible to minus discard bytes from rx_good_bytes . If you think it's not reasonable to Delete rx_discards iterm from rx_good_packets statistic, this patch will be superseded. Because I didn't find other way to correct this problem at present.
[dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
Hi,Wu jingjing and wenzhuo > -Original Message- > From: Zhao1, Wei > Sent: Monday, August 1, 2016 4:58 PM > To: 'Kyle Larose' > Cc: dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent > > Hi, Kyle Larose >The core problem is i40e has no statistic of discard bytes, that means > even if > when ports are not stopped, the statistic rx_good_bytes is consist of discard > bytes?is that reasonable? In other words, I can just minus discard bytes > from rx_good_bytes if I can get discard bytes number, that is much better. > > -Original Message- > From: Kyle Larose [mailto:eomereadig at gmail.com] > Sent: Saturday, July 30, 2016 1:17 AM > To: Zhao1, Wei > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent > > On Fri, Jul 29, 2016 at 4:50 AM, Wei Zhao1 wrote: > > rx_good_bytes and rx_good_packets statistic is inconsistent when port > > stopped,ipackets statistic is minus the discard packets but rx_bytes > > statistic not.Also,i40e has no statistic of discard bytes, so we have > > to delete discard packets item from rx_good_packets statistic. > > > > Fixes: 9aace75fc82e ("i40e: fix statistics") > > > > Signed-off-by: Wei Zhao1 > > --- > > drivers/net/i40e/i40e_ethdev.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/net/i40e/i40e_ethdev.c > > b/drivers/net/i40e/i40e_ethdev.c index 11a5804..553dfd9 100644 > > --- a/drivers/net/i40e/i40e_ethdev.c > > +++ b/drivers/net/i40e/i40e_ethdev.c > > @@ -2319,8 +2319,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, > > struct rte_eth_stats *stats) > > > > stats->ipackets = pf->main_vsi->eth_stats.rx_unicast + > > pf->main_vsi->eth_stats.rx_multicast + > > - pf->main_vsi->eth_stats.rx_broadcast - > > - pf->main_vsi->eth_stats.rx_discards; > > + pf->main_vsi->eth_stats.rx_broadcast; > > stats->opackets = pf->main_vsi->eth_stats.tx_unicast + > > pf->main_vsi->eth_stats.tx_multicast + > > pf->main_vsi->eth_stats.tx_broadcast; > > -- > > 2.5.5 > > > > Is it not worse to report a received packet when no packet was actually > received by the upper layers under normal operations than to ensure that > packets and bytes are consistent when an interface is stopped? It seems like > the first case is much more likely to occur than the second. > > Are we just introducing a new issue to fix another? > > How does this behaviour compare to other NICs? Does the ixgbe report > discarded packets in its ipackets? My reading of the driver is that it does > not. > In fact, it does something interesting to deal with the > problem: > > from: http://dpdk.org/browse/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c > > /* > * An errata states that gprc actually counts good + missed packets: > * Workaround to set gprc to summated queue packet receives */ hw_stats- > >gprc = *total_qprc; > > total_gprc is equal to the sum of the qprc per queue. Can we do something > similar on the i40e instead of adding unicast, mulitcast and broadcast? I have checked ixgbe code about Rx statistic, in function ixgbe_read_stats_registers() we can find the rx_good_bytes and rx_good_packets statistic. It is listed below, we can see rx_good_packets is also just addition of Queue Packets Received Count and not minused discard packet number. Is there some wrong of understanding? for (i = 0; i < IXGBE_QUEUE_STAT_COUNTERS; i++) { .. *total_qprc += hw_stats->qprc[i]; *total_qbrc += hw_stats->qbrc[i]; .. }