Re: [dpdk-dev] [PATCH v4] app/test-regex: add RegEx test application

[Thomas Monjalon <thomas@monjalon.net>]

29/07/2020 20:09, Ori Kam:
> From: Yuval Avnery <yuvalav@mellanox.com>
> 
> Following the new RegEx class.
> There is a need to create a dedicated test application in order to
> validate this class and PMD.
> 
> Unlike net device this application loads data from a file.
> 
> This commit introduces the new RegEx test app.
> 
> The basic app flow:
> 1. Configure the RegEx device to use one queue, and set the rule
>    database, using precompiled file.
> 2. Allocate mbufs based on the requested number of jobs, each job will
> i  get one mbuf.
> 3. Enqueue as much as possible jobs.
> 4. Dequeue jobs.
> 5. if the number of dequeue jobs < requested number of jobs job to step
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> Signed-off-by: Yuval Avnery <yuvalav@mellanox.com>
[...]
> +++ b/app/test-regex/Makefile
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2020 Mellanox Technologies, Ltd
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +APP = testregex

In order to match meson and the doc,
I rename to dpdk-test-regex.

> +
> +CFLAGS += -O3
> +CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -DALLOW_EXPERIMENTAL_API

Experimental API is already enabled via rte.vars.mk.
I remove this line.

[...]
> +enum app_args {
> +	ARG_HELP,
> +	ARG_RULES_FILE_NAME,
> +	ARG_DATA_FILE_NAME,
> +	ARG_NUM_OF_JOBS,
> +	ARG_PERF_MODE,
> +	ARG_NUM_OF_ITERATIONS,
> +
> +};

Extra blank line

[...]
> +error:
> +	if (rules)
> +		rte_free(rules);
> +	if (*mbuf_mp)
> +		rte_mempool_free(*mbuf_mp);
> +	return res;
> +
> +}
> +
> +static void
> +extbuf_free_cb(void *addr __rte_unused, void *fcb_opaque __rte_unused)
> +{
> +
> +}

Extra blank lines

[...]
> +       if (ret < 0)
> +               rte_exit(EXIT_FAILURE, "init port failed\n");
> +       ret = run_regex(mbuf_mp, nb_jobs, nb_max_payload, perf_mode,
> +                       nb_iterations, data_file, nb_max_matches);
> +       if (ret < 0) {
> +               rte_mempool_free(mbuf_mp);
> +               rte_exit(EXIT_FAILURE, "RegEx function failed\n");
> +       }
> +       rte_mempool_free(mbuf_mp);
> +       return 0;

Should return EXIT_SUCCESS

[...]
> +``--help``
> +  prints this help

In the doc, it should be better to reword to "print application options".

The MAINTAINERS file must be updated for this new directory.

Applied with above minor changes, thanks.