[v4,1/5] app/test-flow-perf: add flow performance skeleton
Checks
Commit Message
Add flow performance application skeleton.
Signed-off-by: Wisam Jaddo <wisamm@mellanox.com>
---
MAINTAINERS | 5 +
app/Makefile | 1 +
app/meson.build | 1 +
app/test-flow-perf/Makefile | 26 +++
app/test-flow-perf/main.c | 246 +++++++++++++++++++++++++++
app/test-flow-perf/meson.build | 11 ++
app/test-flow-perf/user_parameters.h | 16 ++
config/common_base | 5 +
doc/guides/tools/flow-perf.rst | 69 ++++++++
doc/guides/tools/index.rst | 1 +
10 files changed, 381 insertions(+)
create mode 100644 app/test-flow-perf/Makefile
create mode 100644 app/test-flow-perf/main.c
create mode 100644 app/test-flow-perf/meson.build
create mode 100644 app/test-flow-perf/user_parameters.h
create mode 100644 doc/guides/tools/flow-perf.rst
Comments
On Thu, 20-04-30, 10:33, Wisam Jaddo wrote:
> Add flow performance application skeleton.
>
> Signed-off-by: Wisam Jaddo <wisamm@mellanox.com>
Acked-by: Xiaoyu Min <jackmin@mellanox.com>
On 4/30/20 1:33 PM, Wisam Jaddo wrote:
> Add flow performance application skeleton.
>
> Signed-off-by: Wisam Jaddo <wisamm@mellanox.com>
> ---
> MAINTAINERS | 5 +
> app/Makefile | 1 +
> app/meson.build | 1 +
> app/test-flow-perf/Makefile | 26 +++
> app/test-flow-perf/main.c | 246 +++++++++++++++++++++++++++
> app/test-flow-perf/meson.build | 11 ++
> app/test-flow-perf/user_parameters.h | 16 ++
> config/common_base | 5 +
> doc/guides/tools/flow-perf.rst | 69 ++++++++
> doc/guides/tools/index.rst | 1 +
> 10 files changed, 381 insertions(+)
> create mode 100644 app/test-flow-perf/Makefile
> create mode 100644 app/test-flow-perf/main.c
> create mode 100644 app/test-flow-perf/meson.build
> create mode 100644 app/test-flow-perf/user_parameters.h
> create mode 100644 doc/guides/tools/flow-perf.rst
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d31a809292..b5632c1bf5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1504,6 +1504,11 @@ T: git://dpdk.org/next/dpdk-next-net
> F: app/test-pmd/
> F: doc/guides/testpmd_app_ug/
>
> +Flow performance tool
> +M: Wisam Jaddo <wisamm@mellanox.com>
> +F: app/test-flow-perf
> +F: doc/guides/flow-perf.rst
> +
Shouldn't it be alphabetially sorted? I think by app name.
> Compression performance test application
> T: git://dpdk.org/next/dpdk-next-crypto
> F: app/test-compress-perf/
> diff --git a/app/Makefile b/app/Makefile
> index 823771c5fc..bd823f3db7 100644
> --- a/app/Makefile
> +++ b/app/Makefile
> @@ -9,6 +9,7 @@ DIRS-$(CONFIG_RTE_PROC_INFO) += proc-info
> DIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += pdump
> DIRS-$(CONFIG_RTE_LIBRTE_ACL) += test-acl
> DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += test-cmdline
> +DIRS-$(CONFIG_RTE_TEST_FLOW_PERF) += test-flow-perf
> DIRS-$(CONFIG_RTE_LIBRTE_FIB) += test-fib
> DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += test-pipeline
> DIRS-$(CONFIG_RTE_LIBRTE_IPSEC) += test-sad
> diff --git a/app/meson.build b/app/meson.build
> index 0f7fe94649..e26f5b72f5 100644
> --- a/app/meson.build
> +++ b/app/meson.build
> @@ -14,6 +14,7 @@ apps = [
> 'test-compress-perf',
> 'test-crypto-perf',
> 'test-eventdev',
> + 'test-flow-perf',
I think 'l' goes after 'i'.
> 'test-fib',
> 'test-pipeline',
> 'test-pmd',
> diff --git a/app/test-flow-perf/Makefile b/app/test-flow-perf/Makefile
> new file mode 100644
> index 0000000000..45b1fb1464
> --- /dev/null
> +++ b/app/test-flow-perf/Makefile
> @@ -0,0 +1,26 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2020 Mellanox Technologies, Ltd
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +ifeq ($(CONFIG_RTE_TEST_FLOW_PERF),y)
> +
> +#
> +# library name
> +#
> +APP = flow_perf
> +
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
> +CFLAGS += -O3
> +CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -Wno-deprecated-declarations
> +CFLAGS += -Wno-unused-function
Why is unused function warning disabled?
> +
> +#
> +# all source are stored in SRCS-y
> +#
> +SRCS-y += main.c
> +
> +include $(RTE_SDK)/mk/rte.app.mk
> +
> +endif
> diff --git a/app/test-flow-perf/main.c b/app/test-flow-perf/main.c
> new file mode 100644
> index 0000000000..156b9ef553
> --- /dev/null
> +++ b/app/test-flow-perf/main.c
> @@ -0,0 +1,246 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + *
> + * This file contain the application main file
> + * This application provides the user the ability to test the
> + * insertion rate for specific rte_flow rule under stress state ~4M rule/
> + *
> + * Then it will also provide packet per second measurement after installing
> + * all rules, the user may send traffic to test the PPS that match the rules
> + * after all rules are installed, to check performance or functionality after
> + * the stress.
> + *
> + * The flows insertion will go for all ports first, then it will print the
> + * results, after that the application will go into forwarding packets mode
> + * it will start receiving traffic if any and then forwarding it back and
> + * gives packet per second measurement.
> + *
> + * Copyright 2020 Mellanox Technologies, Ltd
> + */
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <inttypes.h>
> +#include <sys/types.h>
> +#include <sys/queue.h>
> +#include <netinet/in.h>
> +#include <setjmp.h>
> +#include <stdarg.h>
> +#include <ctype.h>
> +#include <errno.h>
> +#include <getopt.h>
> +#include <signal.h>
> +#include <stdbool.h>
> +#include <assert.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <sys/time.h>
> +
> +
> +#include <rte_eal.h>
> +#include <rte_common.h>
> +#include <rte_malloc.h>
> +#include <rte_ether.h>
> +#include <rte_ethdev.h>
> +#include <rte_mempool.h>
> +#include <rte_mbuf.h>
> +#include <rte_net.h>
> +#include <rte_flow.h>
> +#include <rte_cycles.h>
> +#include <rte_memory.h>
It looks like many-many above headers are actulally unused.
Please, remove unused headers.
> +
> +#include "user_parameters.h"
> +
> +static uint32_t nb_lcores;
> +static struct rte_mempool *mbuf_mp;
> +
> +static void usage(char *progname)
> +{
> + printf("\nusage: %s", progname);
Is \n missing at the end of format string?
> +}
> +
> +static void
> +args_parse(int argc, char **argv)
> +{
> + char **argvopt;
> + int opt;
> + int opt_idx;
> + static struct option lgopts[] = {
> + /* Control */
> + { "help", 0, 0, 0 },
> + };
> +
> + argvopt = argv;
> +
> + while ((opt = getopt_long(argc, argvopt, "",
> + lgopts, &opt_idx)) != EOF) {
> + switch (opt) {
> + case 0:
> + if (!strcmp(lgopts[opt_idx].name, "help")) {
> + usage(argv[0]);
> + rte_exit(EXIT_SUCCESS, "Displayed help\n");
> + }
> + break;
> + default:
> + usage(argv[0]);
> + printf("Invalid option: %s\n", argv[optind]);
I think it is more friendly to log errors to stderr and log
invalid option message first before usage.
> + rte_exit(EXIT_SUCCESS, "Invalid option\n");
> + break;
> + }
> + }
> +}
> +
> +static void
> +init_port(void)
> +{
> + int ret;
> + uint16_t i, j;
> + uint16_t port_id;
> + uint16_t nr_ports = rte_eth_dev_count_avail();
> + struct rte_eth_hairpin_conf hairpin_conf = {
> + .peer_count = 1,
> + };
> + struct rte_eth_conf port_conf = {
> + .rxmode = {
> + .split_hdr_size = 0,
I think it is not required, since compiler will
do it for you anyway having below initialization.
> + },
> + .rx_adv_conf = {
> + .rss_conf.rss_hf =
> + ETH_RSS_IP |
> + ETH_RSS_UDP |
May be it is better to remove ETH_RSS_UDP by default,
since it is less common that RSS for TCP because of
possible fragmentation and packets from the same
stream delivered to different CPU cores.
> + ETH_RSS_TCP,
> + }
> + };
> + struct rte_eth_txconf txq_conf;
> + struct rte_eth_rxconf rxq_conf;
> + struct rte_eth_dev_info dev_info;
> +
> + if (nr_ports == 0)
> + rte_exit(EXIT_FAILURE, "Error: no port detected\n");
Please, add empty line here to logically separate above
error from pool creation. Right now it looks misleading.
> + mbuf_mp = rte_pktmbuf_pool_create("mbuf_pool",
> + TOTAL_MBUF_NUM, MBUF_CACHE_SIZE,
> + 0, MBUF_SIZE,
> + rte_socket_id());
> +
> + if (mbuf_mp == NULL)
> + rte_exit(EXIT_FAILURE, "Error: can't init mbuf pool\n");
> +
> + for (port_id = 0; port_id < nr_ports; port_id++) {
> + ret = rte_eth_dev_info_get(port_id, &dev_info);
> + if (ret != 0)
> + rte_exit(EXIT_FAILURE,
> + "Error during getting device (port %u) info: %s\n",
The line looks long. More than 80 symbols.
Please, use just one TAB to indent relative to rte_exit().
It will make the line a bit shorter.
> + port_id, strerror(-ret));
> +
> + port_conf.txmode.offloads &= dev_info.tx_offload_capa;
Taking into account that txmode.offloads are 0 above,
it looks strange. May be it is added to early in the
patch series?
> + printf(":: initializing port: %d\n", port_id);
> + ret = rte_eth_dev_configure(port_id, RXQs + HAIRPIN_QUEUES,
> + TXQs + HAIRPIN_QUEUES, &port_conf);
RXQs and TXQs are bad since mixing upper and lower
cases letters is bad. Macros are in upper case typically.
May be RXQ_NUM? Or NR_RXQ (however, it will be very easy
to missread as NR_RXD and vise versa, so not good)?
> + if (ret < 0)
> + rte_exit(EXIT_FAILURE,
> + ":: cannot configure device: err=%d, port=%u\n",
Too long line, decrease indent
> + ret, port_id);
> +
> + rxq_conf = dev_info.default_rxconf;
> + rxq_conf.offloads = port_conf.rxmode.offloads;
Same here. Also it should take supported offloads
into account.
> + for (i = 0; i < RXQs; i++) {
> + ret = rte_eth_rx_queue_setup(port_id, i, NR_RXD,
> + rte_eth_dev_socket_id(port_id),
> + &rxq_conf,
> + mbuf_mp);
> + if (ret < 0)
> + rte_exit(EXIT_FAILURE,
> + ":: Rx queue setup failed: err=%d, port=%u\n",
Too long line, decrease indent
> + ret, port_id);
> + }
> +
> + txq_conf = dev_info.default_txconf;
> + txq_conf.offloads = port_conf.txmode.offloads;
Same here. Also it should take supported offloads
into account.
> +
> + for (i = 0; i < TXQs; i++) {
> + ret = rte_eth_tx_queue_setup(port_id, i, NR_TXD,
> + rte_eth_dev_socket_id(port_id),
> + &txq_conf);
> + if (ret < 0)
> + rte_exit(EXIT_FAILURE,
> + ":: Tx queue setup failed: err=%d, port=%u\n",
Too long line, decrease indent
> + ret, port_id);
> + }
> +
> + ret = rte_eth_promiscuous_enable(port_id);
> + if (ret != 0)
> + rte_exit(EXIT_FAILURE,
> + ":: promiscuous mode enable failed: err=%s, port=%u\n",
Too long line, decrease indent
> + rte_strerror(-ret), port_id);
> +
> + for (i = RXQs, j = 0; i < RXQs + HAIRPIN_QUEUES; i++, j++) {
> + hairpin_conf.peers[0].port = port_id;
> + hairpin_conf.peers[0].queue = j + TXQs;
> + ret = rte_eth_rx_hairpin_queue_setup(port_id, i,
> + NR_RXD, &hairpin_conf);
> + if (ret != 0)
> + rte_exit(EXIT_FAILURE,
> + ":: Hairpin rx queue setup failed: err=%d, port=%u\n",
> + ret, port_id);
> + }
> +
> + for (i = TXQs, j = 0; i < TXQs + HAIRPIN_QUEUES; i++, j++) {
> + hairpin_conf.peers[0].port = port_id;
> + hairpin_conf.peers[0].queue = j + RXQs;
> + ret = rte_eth_tx_hairpin_queue_setup(port_id, i,
> + NR_TXD, &hairpin_conf);
> + if (ret != 0)
> + rte_exit(EXIT_FAILURE,
> + ":: Hairpin tx queue setup failed: err=%d, port=%u\n",
> + ret, port_id);
> + }
> +
> + ret = rte_eth_dev_start(port_id);
> + if (ret < 0)
> + rte_exit(EXIT_FAILURE,
> + "rte_eth_dev_start:err=%d, port=%u\n",
> + ret, port_id);
> +
> + printf(":: initializing port: %d done\n", port_id);
> + }
> +}
> +
> +int
> +main(int argc, char **argv)
> +{
> + uint16_t lcore_id;
> + uint16_t port;
> + uint16_t nr_ports;
> + int ret;
> + struct rte_flow_error error;
> +
> + nr_ports = rte_eth_dev_count_avail();
Before EAL init? It is defintely unclear.
If it is done by purpose, please, add a comment
to explain why.
> + ret = rte_eal_init(argc, argv);
> + if (ret < 0)
> + rte_exit(EXIT_FAILURE, "EAL init failed\n");
> +
> + argc -= ret;
> + argv += ret;
> +
> + if (argc > 1)
> + args_parse(argc, argv);
> +
> + init_port();
> +
> + nb_lcores = rte_lcore_count();
> +
> + if (nb_lcores <= 1)
> + rte_exit(EXIT_FAILURE, "This app needs at least two cores\n");
> +
> + RTE_LCORE_FOREACH_SLAVE(lcore_id)
> +
> + if (rte_eal_wait_lcore(lcore_id) < 0)
> + break;
Break what? Is it compile tested?
> +
> + for (port = 0; port < nr_ports; port++) {
> + rte_flow_flush(port, &error);
> + rte_eth_dev_stop(port);
> + rte_eth_dev_close(port);
> + }
> + return 0;
> +}
> diff --git a/app/test-flow-perf/meson.build b/app/test-flow-perf/meson.build
> new file mode 100644
> index 0000000000..ec9bb3b3aa
> --- /dev/null
> +++ b/app/test-flow-perf/meson.build
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2020 Mellanox Technologies, Ltd
> +
> +# meson file, for building this example as part of a main DPDK build.
> +#
> +# To build this example as a standalone application with an already-installed
> +# DPDK instance, use 'make'
> +
> +sources = files(
> + 'main.c',
> +)
> diff --git a/app/test-flow-perf/user_parameters.h b/app/test-flow-perf/user_parameters.h
> new file mode 100644
> index 0000000000..56ec7f47b5
> --- /dev/null
> +++ b/app/test-flow-perf/user_parameters.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: BSD-3-Claus
> + *
> + * This file will hold the user parameters values
> + *
> + * Copyright 2020 Mellanox Technologies, Ltd
> + */
> +
> +/** Configuration **/
> +#define RXQs 4
> +#define TXQs 4
> +#define HAIRPIN_QUEUES 4
It makes it Mellanox-speicific from the first patch since
only mlx5 supports hairpin queues. Such things should be
specified using parameters from the very beginning.
> +#define TOTAL_MBUF_NUM 32000
> +#define MBUF_SIZE 2048
> +#define MBUF_CACHE_SIZE 512
> +#define NR_RXD 256
> +#define NR_TXD 256
> diff --git a/config/common_base b/config/common_base
> index 14000ba07e..eaaeaaaee2 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -1124,3 +1124,8 @@ CONFIG_RTE_APP_CRYPTO_PERF=y
> # Compile the eventdev application
> #
> CONFIG_RTE_APP_EVENTDEV=y
> +
> +#
> +# Compile the rte flow perf application
> +#
> +CONFIG_RTE_TEST_FLOW_PERF=y
CONFIG_RTE_APP_FLOW_PERF to follow naming conventions.
> diff --git a/doc/guides/tools/flow-perf.rst b/doc/guides/tools/flow-perf.rst
> new file mode 100644
> index 0000000000..30ce1b6cc0
> --- /dev/null
> +++ b/doc/guides/tools/flow-perf.rst
> @@ -0,0 +1,69 @@
> +.. SPDX-License-Identifier: BSD-3-Clause
> + Copyright 2020 Mellanox Technologies, Ltd
> +
> +RTE Flow performance tool
> +=========================
> +
> +Application for rte_flow performance testing.
> +
> +
> +Compiling the Application
> +=========================
> +The ``test-flow-perf`` application is compiled as part of the main compilation
> +of the DPDK libraries and tools.
> +
> +Refer to the DPDK Getting Started Guides for details.
> +The basic compilation steps are:
> +
> +#. Set the required environmental variables and go to the source directory:
> +
> + .. code-block:: console
> +
> + export RTE_SDK=/path/to/rte_sdk
> + cd $RTE_SDK
> +
> +#. Set the compilation target. For example:
> +
> + .. code-block:: console
> +
> + export RTE_TARGET=x86_64-native-linux-gcc
> +
> +#. Build the application:
> +
> + .. code-block:: console
> +
> + make install T=$RTE_TARGET
> +
> +#. The compiled application will be located at:
> +
> + .. code-block:: console
> +
> + $RTE_SDK/$RTE_TARGET/app/flow-perf
> +
> +
> +Running the Application
> +=======================
> +
> +EAL Command-line Options
> +------------------------
> +
> +Please refer to :doc:`EAL parameters (Linux) <../linux_gsg/linux_eal_parameters>`
> +or :doc:`EAL parameters (FreeBSD) <../freebsd_gsg/freebsd_eal_parameters>` for
> +a list of available EAL command-line options.
> +
> +
> +Flow performance Options
> +------------------------
> +
> +The following are the command-line options for the flow performance application.
> +They must be separated from the EAL options, shown in the previous section, with
> +a ``--`` separator:
> +
> +.. code-block:: console
> +
> + sudo ./test-flow-perf -n 4 -w 08:00.0,dv_flow_en=1 --
> +
> +The command line options are:
> +
> +* ``--help``
> + Display a help message and quit.
> diff --git a/doc/guides/tools/index.rst b/doc/guides/tools/index.rst
> index 782b30864e..7279daebc6 100644
> --- a/doc/guides/tools/index.rst
> +++ b/doc/guides/tools/index.rst
> @@ -16,3 +16,4 @@ DPDK Tools User Guides
> cryptoperf
> comp_perf
> testeventdev
> + flow-perf
>
I think above should be alphabetically sorted as well.
>-----Original Message-----
>From: Andrew Rybchenko <arybchenko@solarflare.com>
>Sent: Monday, May 4, 2020 1:17 PM
>To: Wisam Monther <wisamm@mellanox.com>; dev@dpdk.org; Jack Min
><jackmin@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
>jerinjacobk@gmail.com; gerlitz.or@gmail.com; l.yan@epfl.ch
>Subject: Re: [dpdk-dev] [PATCH v4 1/5] app/test-flow-perf: add flow
>performance skeleton
>
>On 4/30/20 1:33 PM, Wisam Jaddo wrote:
>> Add flow performance application skeleton.
>>
>> Signed-off-by: Wisam Jaddo <wisamm@mellanox.com>
>> ---
>> MAINTAINERS | 5 +
>> app/Makefile | 1 +
>> app/meson.build | 1 +
>> app/test-flow-perf/Makefile | 26 +++
>> app/test-flow-perf/main.c | 246 +++++++++++++++++++++++++++
>> app/test-flow-perf/meson.build | 11 ++
>> app/test-flow-perf/user_parameters.h | 16 ++
>> config/common_base | 5 +
>> doc/guides/tools/flow-perf.rst | 69 ++++++++
>> doc/guides/tools/index.rst | 1 +
>> 10 files changed, 381 insertions(+)
>> create mode 100644 app/test-flow-perf/Makefile create mode 100644
>> app/test-flow-perf/main.c create mode 100644
>> app/test-flow-perf/meson.build create mode 100644
>> app/test-flow-perf/user_parameters.h
>> create mode 100644 doc/guides/tools/flow-perf.rst
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS index d31a809292..b5632c1bf5
>> 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1504,6 +1504,11 @@ T: git://dpdk.org/next/dpdk-next-net
>> F: app/test-pmd/
>> F: doc/guides/testpmd_app_ug/
>>
>> +Flow performance tool
>> +M: Wisam Jaddo <wisamm@mellanox.com>
>> +F: app/test-flow-perf
>> +F: doc/guides/flow-perf.rst
>> +
>
>Shouldn't it be alphabetially sorted? I think by app name.
It looks no,
Current order:
- Sample_packet_forward
- test-pmd
- comp_perf
- eventdev
- proc-info
So I'll move the new app to be in the end of the test apps, since it's been added last, is this ok?
>-----Original Message-----
>From: Andrew Rybchenko <arybchenko@solarflare.com>
>Sent: Monday, May 4, 2020 1:17 PM
>To: Wisam Monther <wisamm@mellanox.com>; dev@dpdk.org; Jack Min
><jackmin@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
>jerinjacobk@gmail.com; gerlitz.or@gmail.com; l.yan@epfl.ch
>Subject: Re: [dpdk-dev] [PATCH v4 1/5] app/test-flow-perf: add flow
>performance skeleton
>
>On 4/30/20 1:33 PM, Wisam Jaddo wrote:
>> Add flow performance application skeleton.
>>
>> Signed-off-by: Wisam Jaddo <wisamm@mellanox.com>
>> ---
>> MAINTAINERS | 5 +
>> app/Makefile | 1 +
>> app/meson.build | 1 +
>> app/test-flow-perf/Makefile | 26 +++
>> app/test-flow-perf/main.c | 246 +++++++++++++++++++++++++++
>> app/test-flow-perf/meson.build | 11 ++
>> app/test-flow-perf/user_parameters.h | 16 ++
>> config/common_base | 5 +
>> doc/guides/tools/flow-perf.rst | 69 ++++++++
>> doc/guides/tools/index.rst | 1 +
>> 10 files changed, 381 insertions(+)
>> create mode 100644 app/test-flow-perf/Makefile create mode 100644
>> app/test-flow-perf/main.c create mode 100644
>> app/test-flow-perf/meson.build create mode 100644
>> app/test-flow-perf/user_parameters.h
>> create mode 100644 doc/guides/tools/flow-perf.rst
>>
>> diff --git a/doc/guides/tools/index.rst b/doc/guides/tools/index.rst
>> index 782b30864e..7279daebc6 100644
>> --- a/doc/guides/tools/index.rst
>> +++ b/doc/guides/tools/index.rst
>> @@ -16,3 +16,4 @@ DPDK Tools User Guides
>> cryptoperf
>> comp_perf
>> testeventdev
>> + flow-perf
>>
>
>I think above should be alphabetically sorted as well.
Same as this, the current is not alphabetically sorted so I have it in the end
05/05/2020 12:45, Wisam Monther:
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> > On 4/30/20 1:33 PM, Wisam Jaddo wrote:
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -1504,6 +1504,11 @@ T: git://dpdk.org/next/dpdk-next-net
> >> F: app/test-pmd/
> >> F: doc/guides/testpmd_app_ug/
> >>
> >> +Flow performance tool
> >> +M: Wisam Jaddo <wisamm@mellanox.com>
> >> +F: app/test-flow-perf
> >> +F: doc/guides/flow-perf.rst
> >> +
> >
> >Shouldn't it be alphabetially sorted? I think by app name.
>
> It looks no,
> Current order:
> - Sample_packet_forward
> - test-pmd
> - comp_perf
> - eventdev
> - proc-info
>
> So I'll move the new app to be in the end of the test apps, since it's been added last, is this ok?
If no alphabetical sorting, there should be a logical one.
Having rte_flow perf testing after general ethdev testing (testpmd),
like you did, is good in my opinion.
::snip::
> > + },
> > + .rx_adv_conf = {
> > + .rss_conf.rss_hf =
> > + ETH_RSS_IP |
> > + ETH_RSS_UDP |
>
> May be it is better to remove ETH_RSS_UDP by default,
> since it is less common that RSS for TCP because of
> possible fragmentation and packets from the same
> stream delivered to different CPU cores.
>
If we want to enable RSS on L4 headers, then UDP and TCP should be fine.
Its an example app anyway?
Otherwise we can just stick with L3 hash like some of the other examples.
::snip::
I agree, since this is a test application,
We can have L4 UDP/TCP rss configuration, since all
Flows/traffic are allowed here and there is nothing to be common here.
From: Ajit Khaparde <ajit.khaparde@broadcom.com>
Sent: Wednesday, May 6, 2020 5:50 AM
To: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: Wisam Monther <wisamm@mellanox.com>; dpdk-dev <dev@dpdk.org>; Jack Min <jackmin@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>; Jerin Jacob <jerinjacobk@gmail.com>; gerlitz.or@gmail.com; l.yan@epfl.ch
Subject: Re: [dpdk-dev] [PATCH v4 1/5] app/test-flow-perf: add flow performance skeleton
::snip::
> + },
> + .rx_adv_conf = {
> + .rss_conf.rss_hf =
> + ETH_RSS_IP |
> + ETH_RSS_UDP |
May be it is better to remove ETH_RSS_UDP by default,
since it is less common that RSS for TCP because of
possible fragmentation and packets from the same
stream delivered to different CPU cores.
If we want to enable RSS on L4 headers, then UDP and TCP should be fine.
Its an example app anyway?
Otherwise we can just stick with L3 hash like some of the other examples.
::snip::
On 5/6/20 10:32 AM, Wisam Monther wrote:
> I agree, since this is a test application,
> We can have L4 UDP/TCP rss configuration, since all
> Flows/traffic are allowed here and there is nothing to be common here.
UDP RSS is less common and could simply be unsupported.
So, it will be harder to use the tool for corresponding NICs.
> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Sent: Wednesday, May 6, 2020 5:50 AM
> To: Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: Wisam Monther <wisamm@mellanox.com>; dpdk-dev <dev@dpdk.org>; Jack Min <jackmin@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>; Jerin Jacob <jerinjacobk@gmail.com>; gerlitz.or@gmail.com; l.yan@epfl.ch
> Subject: Re: [dpdk-dev] [PATCH v4 1/5] app/test-flow-perf: add flow performance skeleton
>
>
>
> ::snip::
>
>
>> + },
>> + .rx_adv_conf = {
>> + .rss_conf.rss_hf =
>> + ETH_RSS_IP |
>> + ETH_RSS_UDP |
> May be it is better to remove ETH_RSS_UDP by default,
> since it is less common that RSS for TCP because of
> possible fragmentation and packets from the same
> stream delivered to different CPU cores.
> If we want to enable RSS on L4 headers, then UDP and TCP should be fine.
> Its an example app anyway?
> Otherwise we can just stick with L3 hash like some of the other examples.
> ::snip::
>
>-----Original Message-----
>From: Andrew Rybchenko <arybchenko@solarflare.com>
>Sent: Wednesday, May 6, 2020 11:48 AM
>To: Wisam Monther <wisamm@mellanox.com>; Ajit Khaparde
><ajit.khaparde@broadcom.com>
>Cc: dpdk-dev <dev@dpdk.org>; Jack Min <jackmin@mellanox.com>; Thomas
>Monjalon <thomas@monjalon.net>; Jerin Jacob <jerinjacobk@gmail.com>;
>gerlitz.or@gmail.com; l.yan@epfl.ch
>Subject: Re: [dpdk-dev] [PATCH v4 1/5] app/test-flow-perf: add flow
>performance skeleton
>
>On 5/6/20 10:32 AM, Wisam Monther wrote:
>> I agree, since this is a test application, We can have L4 UDP/TCP rss
>> configuration, since all Flows/traffic are allowed here and there is
>> nothing to be common here.
>
>UDP RSS is less common and could simply be unsupported.
>So, it will be harder to use the tool for corresponding NICs.
Ok,
So we can go with only IP without TCP
This way we can call the support of hash up to L3 only.
Are we ok with this?
>
>> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
>> Sent: Wednesday, May 6, 2020 5:50 AM
>> To: Andrew Rybchenko <arybchenko@solarflare.com>
>> Cc: Wisam Monther <wisamm@mellanox.com>; dpdk-dev
><dev@dpdk.org>; Jack
>> Min <jackmin@mellanox.com>; Thomas Monjalon
><thomas@monjalon.net>;
>> Jerin Jacob <jerinjacobk@gmail.com>; gerlitz.or@gmail.com;
>> l.yan@epfl.ch
>> Subject: Re: [dpdk-dev] [PATCH v4 1/5] app/test-flow-perf: add flow
>> performance skeleton
>>
>>
>>
>> ::snip::
>>
>>
>>> + },
>>> + .rx_adv_conf = {
>>> + .rss_conf.rss_hf =
>>> + ETH_RSS_IP |
>>> + ETH_RSS_UDP |
>> May be it is better to remove ETH_RSS_UDP by default, since it is less
>> common that RSS for TCP because of possible fragmentation and packets
>> from the same stream delivered to different CPU cores.
>> If we want to enable RSS on L4 headers, then UDP and TCP should be fine.
>> Its an example app anyway?
>> Otherwise we can just stick with L3 hash like some of the other examples.
>> ::snip::
>>
On 5/6/20 11:51 AM, Wisam Monther wrote:
>
>> -----Original Message-----
>> From: Andrew Rybchenko <arybchenko@solarflare.com>
>> Sent: Wednesday, May 6, 2020 11:48 AM
>> To: Wisam Monther <wisamm@mellanox.com>; Ajit Khaparde
>> <ajit.khaparde@broadcom.com>
>> Cc: dpdk-dev <dev@dpdk.org>; Jack Min <jackmin@mellanox.com>; Thomas
>> Monjalon <thomas@monjalon.net>; Jerin Jacob <jerinjacobk@gmail.com>;
>> gerlitz.or@gmail.com; l.yan@epfl.ch
>> Subject: Re: [dpdk-dev] [PATCH v4 1/5] app/test-flow-perf: add flow
>> performance skeleton
>>
>> On 5/6/20 10:32 AM, Wisam Monther wrote:
>>> I agree, since this is a test application, We can have L4 UDP/TCP rss
>>> configuration, since all Flows/traffic are allowed here and there is
>>> nothing to be common here.
>> UDP RSS is less common and could simply be unsupported.
>> So, it will be harder to use the tool for corresponding NICs.
> Ok,
> So we can go with only IP without TCP
> This way we can call the support of hash up to L3 only.
> Are we ok with this?
>
I"m OK with IP+TCP as the most common case.
>>> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>> Sent: Wednesday, May 6, 2020 5:50 AM
>>> To: Andrew Rybchenko <arybchenko@solarflare.com>
>>> Cc: Wisam Monther <wisamm@mellanox.com>; dpdk-dev
>> <dev@dpdk.org>; Jack
>>> Min <jackmin@mellanox.com>; Thomas Monjalon
>> <thomas@monjalon.net>;
>>> Jerin Jacob <jerinjacobk@gmail.com>; gerlitz.or@gmail.com;
>>> l.yan@epfl.ch
>>> Subject: Re: [dpdk-dev] [PATCH v4 1/5] app/test-flow-perf: add flow
>>> performance skeleton
>>>
>>>
>>>
>>> ::snip::
>>>
>>>
>>>> + },
>>>> + .rx_adv_conf = {
>>>> + .rss_conf.rss_hf =
>>>> + ETH_RSS_IP |
>>>> + ETH_RSS_UDP |
>>> May be it is better to remove ETH_RSS_UDP by default, since it is less
>>> common that RSS for TCP because of possible fragmentation and packets
>>> from the same stream delivered to different CPU cores.
>>> If we want to enable RSS on L4 headers, then UDP and TCP should be fine.
>>> Its an example app anyway?
>>> Otherwise we can just stick with L3 hash like some of the other examples.
>>> ::snip::
>>>
@@ -1504,6 +1504,11 @@ T: git://dpdk.org/next/dpdk-next-net
F: app/test-pmd/
F: doc/guides/testpmd_app_ug/
+Flow performance tool
+M: Wisam Jaddo <wisamm@mellanox.com>
+F: app/test-flow-perf
+F: doc/guides/flow-perf.rst
+
Compression performance test application
T: git://dpdk.org/next/dpdk-next-crypto
F: app/test-compress-perf/
@@ -9,6 +9,7 @@ DIRS-$(CONFIG_RTE_PROC_INFO) += proc-info
DIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += pdump
DIRS-$(CONFIG_RTE_LIBRTE_ACL) += test-acl
DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += test-cmdline
+DIRS-$(CONFIG_RTE_TEST_FLOW_PERF) += test-flow-perf
DIRS-$(CONFIG_RTE_LIBRTE_FIB) += test-fib
DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += test-pipeline
DIRS-$(CONFIG_RTE_LIBRTE_IPSEC) += test-sad
@@ -14,6 +14,7 @@ apps = [
'test-compress-perf',
'test-crypto-perf',
'test-eventdev',
+ 'test-flow-perf',
'test-fib',
'test-pipeline',
'test-pmd',
new file mode 100644
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2020 Mellanox Technologies, Ltd
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+ifeq ($(CONFIG_RTE_TEST_FLOW_PERF),y)
+
+#
+# library name
+#
+APP = flow_perf
+
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -Wno-deprecated-declarations
+CFLAGS += -Wno-unused-function
+
+#
+# all source are stored in SRCS-y
+#
+SRCS-y += main.c
+
+include $(RTE_SDK)/mk/rte.app.mk
+
+endif
new file mode 100644
@@ -0,0 +1,246 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ *
+ * This file contain the application main file
+ * This application provides the user the ability to test the
+ * insertion rate for specific rte_flow rule under stress state ~4M rule/
+ *
+ * Then it will also provide packet per second measurement after installing
+ * all rules, the user may send traffic to test the PPS that match the rules
+ * after all rules are installed, to check performance or functionality after
+ * the stress.
+ *
+ * The flows insertion will go for all ports first, then it will print the
+ * results, after that the application will go into forwarding packets mode
+ * it will start receiving traffic if any and then forwarding it back and
+ * gives packet per second measurement.
+ *
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <sys/types.h>
+#include <sys/queue.h>
+#include <netinet/in.h>
+#include <setjmp.h>
+#include <stdarg.h>
+#include <ctype.h>
+#include <errno.h>
+#include <getopt.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/time.h>
+
+
+#include <rte_eal.h>
+#include <rte_common.h>
+#include <rte_malloc.h>
+#include <rte_ether.h>
+#include <rte_ethdev.h>
+#include <rte_mempool.h>
+#include <rte_mbuf.h>
+#include <rte_net.h>
+#include <rte_flow.h>
+#include <rte_cycles.h>
+#include <rte_memory.h>
+
+#include "user_parameters.h"
+
+static uint32_t nb_lcores;
+static struct rte_mempool *mbuf_mp;
+
+static void usage(char *progname)
+{
+ printf("\nusage: %s", progname);
+}
+
+static void
+args_parse(int argc, char **argv)
+{
+ char **argvopt;
+ int opt;
+ int opt_idx;
+ static struct option lgopts[] = {
+ /* Control */
+ { "help", 0, 0, 0 },
+ };
+
+ argvopt = argv;
+
+ while ((opt = getopt_long(argc, argvopt, "",
+ lgopts, &opt_idx)) != EOF) {
+ switch (opt) {
+ case 0:
+ if (!strcmp(lgopts[opt_idx].name, "help")) {
+ usage(argv[0]);
+ rte_exit(EXIT_SUCCESS, "Displayed help\n");
+ }
+ break;
+ default:
+ usage(argv[0]);
+ printf("Invalid option: %s\n", argv[optind]);
+ rte_exit(EXIT_SUCCESS, "Invalid option\n");
+ break;
+ }
+ }
+}
+
+static void
+init_port(void)
+{
+ int ret;
+ uint16_t i, j;
+ uint16_t port_id;
+ uint16_t nr_ports = rte_eth_dev_count_avail();
+ struct rte_eth_hairpin_conf hairpin_conf = {
+ .peer_count = 1,
+ };
+ struct rte_eth_conf port_conf = {
+ .rxmode = {
+ .split_hdr_size = 0,
+ },
+ .rx_adv_conf = {
+ .rss_conf.rss_hf =
+ ETH_RSS_IP |
+ ETH_RSS_UDP |
+ ETH_RSS_TCP,
+ }
+ };
+ struct rte_eth_txconf txq_conf;
+ struct rte_eth_rxconf rxq_conf;
+ struct rte_eth_dev_info dev_info;
+
+ if (nr_ports == 0)
+ rte_exit(EXIT_FAILURE, "Error: no port detected\n");
+ mbuf_mp = rte_pktmbuf_pool_create("mbuf_pool",
+ TOTAL_MBUF_NUM, MBUF_CACHE_SIZE,
+ 0, MBUF_SIZE,
+ rte_socket_id());
+
+ if (mbuf_mp == NULL)
+ rte_exit(EXIT_FAILURE, "Error: can't init mbuf pool\n");
+
+ for (port_id = 0; port_id < nr_ports; port_id++) {
+ ret = rte_eth_dev_info_get(port_id, &dev_info);
+ if (ret != 0)
+ rte_exit(EXIT_FAILURE,
+ "Error during getting device (port %u) info: %s\n",
+ port_id, strerror(-ret));
+
+ port_conf.txmode.offloads &= dev_info.tx_offload_capa;
+ printf(":: initializing port: %d\n", port_id);
+ ret = rte_eth_dev_configure(port_id, RXQs + HAIRPIN_QUEUES,
+ TXQs + HAIRPIN_QUEUES, &port_conf);
+ if (ret < 0)
+ rte_exit(EXIT_FAILURE,
+ ":: cannot configure device: err=%d, port=%u\n",
+ ret, port_id);
+
+ rxq_conf = dev_info.default_rxconf;
+ rxq_conf.offloads = port_conf.rxmode.offloads;
+ for (i = 0; i < RXQs; i++) {
+ ret = rte_eth_rx_queue_setup(port_id, i, NR_RXD,
+ rte_eth_dev_socket_id(port_id),
+ &rxq_conf,
+ mbuf_mp);
+ if (ret < 0)
+ rte_exit(EXIT_FAILURE,
+ ":: Rx queue setup failed: err=%d, port=%u\n",
+ ret, port_id);
+ }
+
+ txq_conf = dev_info.default_txconf;
+ txq_conf.offloads = port_conf.txmode.offloads;
+
+ for (i = 0; i < TXQs; i++) {
+ ret = rte_eth_tx_queue_setup(port_id, i, NR_TXD,
+ rte_eth_dev_socket_id(port_id),
+ &txq_conf);
+ if (ret < 0)
+ rte_exit(EXIT_FAILURE,
+ ":: Tx queue setup failed: err=%d, port=%u\n",
+ ret, port_id);
+ }
+
+ ret = rte_eth_promiscuous_enable(port_id);
+ if (ret != 0)
+ rte_exit(EXIT_FAILURE,
+ ":: promiscuous mode enable failed: err=%s, port=%u\n",
+ rte_strerror(-ret), port_id);
+
+ for (i = RXQs, j = 0; i < RXQs + HAIRPIN_QUEUES; i++, j++) {
+ hairpin_conf.peers[0].port = port_id;
+ hairpin_conf.peers[0].queue = j + TXQs;
+ ret = rte_eth_rx_hairpin_queue_setup(port_id, i,
+ NR_RXD, &hairpin_conf);
+ if (ret != 0)
+ rte_exit(EXIT_FAILURE,
+ ":: Hairpin rx queue setup failed: err=%d, port=%u\n",
+ ret, port_id);
+ }
+
+ for (i = TXQs, j = 0; i < TXQs + HAIRPIN_QUEUES; i++, j++) {
+ hairpin_conf.peers[0].port = port_id;
+ hairpin_conf.peers[0].queue = j + RXQs;
+ ret = rte_eth_tx_hairpin_queue_setup(port_id, i,
+ NR_TXD, &hairpin_conf);
+ if (ret != 0)
+ rte_exit(EXIT_FAILURE,
+ ":: Hairpin tx queue setup failed: err=%d, port=%u\n",
+ ret, port_id);
+ }
+
+ ret = rte_eth_dev_start(port_id);
+ if (ret < 0)
+ rte_exit(EXIT_FAILURE,
+ "rte_eth_dev_start:err=%d, port=%u\n",
+ ret, port_id);
+
+ printf(":: initializing port: %d done\n", port_id);
+ }
+}
+
+int
+main(int argc, char **argv)
+{
+ uint16_t lcore_id;
+ uint16_t port;
+ uint16_t nr_ports;
+ int ret;
+ struct rte_flow_error error;
+
+ nr_ports = rte_eth_dev_count_avail();
+ ret = rte_eal_init(argc, argv);
+ if (ret < 0)
+ rte_exit(EXIT_FAILURE, "EAL init failed\n");
+
+ argc -= ret;
+ argv += ret;
+
+ if (argc > 1)
+ args_parse(argc, argv);
+
+ init_port();
+
+ nb_lcores = rte_lcore_count();
+
+ if (nb_lcores <= 1)
+ rte_exit(EXIT_FAILURE, "This app needs at least two cores\n");
+
+ RTE_LCORE_FOREACH_SLAVE(lcore_id)
+
+ if (rte_eal_wait_lcore(lcore_id) < 0)
+ break;
+
+ for (port = 0; port < nr_ports; port++) {
+ rte_flow_flush(port, &error);
+ rte_eth_dev_stop(port);
+ rte_eth_dev_close(port);
+ }
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Mellanox Technologies, Ltd
+
+# meson file, for building this example as part of a main DPDK build.
+#
+# To build this example as a standalone application with an already-installed
+# DPDK instance, use 'make'
+
+sources = files(
+ 'main.c',
+)
new file mode 100644
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: BSD-3-Claus
+ *
+ * This file will hold the user parameters values
+ *
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+/** Configuration **/
+#define RXQs 4
+#define TXQs 4
+#define HAIRPIN_QUEUES 4
+#define TOTAL_MBUF_NUM 32000
+#define MBUF_SIZE 2048
+#define MBUF_CACHE_SIZE 512
+#define NR_RXD 256
+#define NR_TXD 256
@@ -1124,3 +1124,8 @@ CONFIG_RTE_APP_CRYPTO_PERF=y
# Compile the eventdev application
#
CONFIG_RTE_APP_EVENTDEV=y
+
+#
+# Compile the rte flow perf application
+#
+CONFIG_RTE_TEST_FLOW_PERF=y
new file mode 100644
@@ -0,0 +1,69 @@
+.. SPDX-License-Identifier: BSD-3-Clause
+ Copyright 2020 Mellanox Technologies, Ltd
+
+RTE Flow performance tool
+=========================
+
+Application for rte_flow performance testing.
+
+
+Compiling the Application
+=========================
+The ``test-flow-perf`` application is compiled as part of the main compilation
+of the DPDK libraries and tools.
+
+Refer to the DPDK Getting Started Guides for details.
+The basic compilation steps are:
+
+#. Set the required environmental variables and go to the source directory:
+
+ .. code-block:: console
+
+ export RTE_SDK=/path/to/rte_sdk
+ cd $RTE_SDK
+
+#. Set the compilation target. For example:
+
+ .. code-block:: console
+
+ export RTE_TARGET=x86_64-native-linux-gcc
+
+#. Build the application:
+
+ .. code-block:: console
+
+ make install T=$RTE_TARGET
+
+#. The compiled application will be located at:
+
+ .. code-block:: console
+
+ $RTE_SDK/$RTE_TARGET/app/flow-perf
+
+
+Running the Application
+=======================
+
+EAL Command-line Options
+------------------------
+
+Please refer to :doc:`EAL parameters (Linux) <../linux_gsg/linux_eal_parameters>`
+or :doc:`EAL parameters (FreeBSD) <../freebsd_gsg/freebsd_eal_parameters>` for
+a list of available EAL command-line options.
+
+
+Flow performance Options
+------------------------
+
+The following are the command-line options for the flow performance application.
+They must be separated from the EAL options, shown in the previous section, with
+a ``--`` separator:
+
+.. code-block:: console
+
+ sudo ./test-flow-perf -n 4 -w 08:00.0,dv_flow_en=1 --
+
+The command line options are:
+
+* ``--help``
+ Display a help message and quit.
@@ -16,3 +16,4 @@ DPDK Tools User Guides
cryptoperf
comp_perf
testeventdev
+ flow-perf