Re: [dpdk-dev] [PATCH v5 1/5] app/flow-perf: add flow performance skeleton

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 5/6/20 3:36 PM, Wisam Jaddo wrote:
> Add flow performance application skeleton.
> 
> Signed-off-by: Wisam Jaddo <wisamm@mellanox.com>
> ---

[snip]

> diff --git a/app/test-flow-perf/main.c b/app/test-flow-perf/main.c
> new file mode 100644
> index 000000000..7a924cdb7
> --- /dev/null
> +++ b/app/test-flow-perf/main.c
> @@ -0,0 +1,200 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2020 Mellanox Technologies, Ltd
> + *
> + * 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.
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <inttypes.h>
> +#include <stdarg.h>
> +#include <errno.h>
> +#include <getopt.h>
> +#include <signal.h>
> +#include <stdbool.h>
> +#include <sys/time.h>
> +
> +#include <rte_malloc.h>
> +#include <rte_mempool.h>
> +#include <rte_mbuf.h>
> +#include <rte_ethdev.h>
> +#include <rte_flow.h>
> +
> +#include "config.h"
> +
> +static uint32_t nb_lcores;
> +static struct rte_mempool *mbuf_mp;
> +
> +static void
> +usage(char *progname)
> +{
> +	printf("\nusage: %s\n", 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")) {

DPDK coding style recommends to compare vs 0 instead of logical
not.

> +				usage(argv[0]);
> +				rte_exit(EXIT_SUCCESS, "Displayed help\n");
> +			}
> +			break;
> +		default:
> +			printf("Invalid option: %s\n", argv[optind]);

Again, sorry if I missed reply: Why error is not logged to
stderr?

> +			usage(argv[0]);
> +			rte_exit(EXIT_SUCCESS, "Invalid option\n");
> +			break;
> +		}
> +	}
> +}
> +
> +static void
> +init_port(void)
> +{
> +	int ret;
> +	uint16_t i;
> +	uint16_t port_id;
> +	uint16_t nr_ports;
> +	struct rte_eth_conf port_conf = {
> +		.rx_adv_conf = {
> +			.rss_conf.rss_hf =
> +				ETH_RSS_IP  |
> +				ETH_RSS_TCP,
> +		}
> +	};
> +	struct rte_eth_txconf txq_conf;
> +	struct rte_eth_rxconf rxq_conf;
> +	struct rte_eth_dev_info dev_info;
> +
> +	nr_ports = rte_eth_dev_count_avail();
> +	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;
> +		port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
> +
> +		printf(":: initializing port: %d\n", port_id);
> +
> +		ret = rte_eth_dev_configure(port_id, RXQ_NUM,
> +				TXQ_NUM, &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;


As far as I know there is no necessity to repeat port offlaod
on queue level. So, the line is not necesary.

> +
> +		for (i = 0; i < RXQ_NUM; 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;

As far as I know there is no necessity to repeat port offlaod
on queue level. So, the line is not necesary.

> +
> +		for (i = 0; i < TXQ_NUM; 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);
> +		}
> +
> +		/* Catch all packets from traffic generator. */
> +		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);
> +
> +		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)
> +{
> +	int ret;
> +	uint16_t port;
> +	struct rte_flow_error error;
> +
> +	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_ETH_FOREACH_DEV(port) {
> +		rte_flow_flush(port, &error);
> +		rte_eth_dev_stop(port);
> +		rte_eth_dev_close(port);
> +	}
> +	return 0;
> +}

[snip]

> diff --git a/config/common_base b/config/common_base
> index 14000ba07..b2edd5267 100644
> diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
> index b124c3f28..258b1e03e 100644
> --- a/doc/guides/rel_notes/release_20_05.rst
> +++ b/doc/guides/rel_notes/release_20_05.rst
> @@ -212,6 +212,16 @@ New Features
>    * Added IPsec inbound load-distribution support for ipsec-secgw application
>      using NIC load distribution feature(Flow Director).
>  
> +* **Added flow performance application.**
> +
> +  Add new application to test rte_flow performance.
> +
> +  Application features:
> +  * Measure rte_flow insertion rate.
> +  * Measure rte_flow deletion rate.
> +  * Dump rte_flow memory consumption.
> +  * Measure packet per second forwarding.

I think above lines should be added in appropriate patches
which really do it.

> +
>  
>  Removed Items
>  -------------

[snip]