26/07/2020 21:58, Ori Kam:

In this list, I think the alphabetical order was chosen.

It does not comply with Mellanox copyright syntax.
Note: I already did this comment in recent past.

+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+ifeq ($(CONFIG_RTE_LIBRTE_REGEXDEV),y)
+
+#
+# library name
+#
+APP = testregex
+
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -Wno-deprecated-declarations

This flag is not acceptable.

+
+#
+# all source are stored in SRCS-y
+#
+SRCS-y := main.c
+
+include $(RTE_SDK)/mk/rte.app.mk
+
+endif

[...]

space missing

+
+for i in range(REPEAT_COUNT):
+    rand = random.randrange(MAX_LEN)
+    fd_input.write(' ' * rand)
+    current_pos += rand
+    fd_input.write(KEYWORD)
+    match_pos.append(current_pos)
+    fd_res.write('{}\n'.format(str(current_pos)))
+    current_pos += len(KEYWORD)
+
+fd_input.close()
+fd_res.close()

I think there is a more pythonic way of writing in a file.
At least, you can use "with".

Already discussed in a separate thread.
This file should be generated.

In general I like comments explaining what is a file for.
But here it looks useless.

+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdint.h>
+#include <stdarg.h>
+#include <ctype.h>
+#include <errno.h>
+#include <getopt.h>
+#include <signal.h>
+
+#include <rte_eal.h>
+#include <rte_common.h>
+#include <rte_malloc.h>
+#include <rte_mempool.h>
+#include <rte_mbuf.h>
+#include <rte_cycles.h>
+#include <rte_regexdev.h>
+
+#define HELP_VAL 0
+#define RULES_VAL 1
+#define DATA_VAL 2
+#define JOB_VAL 3
+#define PERF_VAL 4
+#define ITER_VAL 5

Please add comments to explain what are these constants for.
I think an enum, and a common prefix would be better.

+#define MAX_FILE_NAME 255
+
+static char rules_file[MAX_FILE_NAME];
+static char data_file[MAX_FILE_NAME];
+static uint32_t jobs;
+static struct rte_mempool *mbuf_mp;
+static uint8_t nb_max_matches;
+static uint16_t nb_max_payload;
+static int perf_test;
+static uint32_t iter;

Please avoid global variables.

+
+static void
+usage(const char *prog_name)
+{
+	printf("%s [EAL options] --\n"
+		" --rules NAME: precompiled rules file\n"
+		" --data NAME: data file to use\n"
+		" --nb_jobs: number of jobs to use\n"
+		" --perf N: only outputs the performance data\n"
+		" --nb_iter N: number of iteration to run\n",
+		prog_name);
+}
+
+static void
+args_parse(int argc, char **argv)
+{
+	char **argvopt;
+	int opt;
+	int opt_idx;
+	size_t len;
+	static struct option lgopts[] = {
+		{ "help",  0, 0, HELP_VAL },
+		{ "rules",  1, 0, RULES_VAL },
+		/* Rules database file to load. */
+		{ "data",  1, 0, DATA_VAL },
+		/* Data file to load. */
+		{ "nb_jobs",  1, 0, JOB_VAL },
+		/* Number of jobs to create. */
+		{ "perf", 0, 0, PERF_VAL},
+		/* Perf test only */
+		{ "nb_iter", 1, 0, ITER_VAL}
+		/* Number of iterations to run with perf test */
+	};
+
+	argvopt = argv;
+

Useless newline.

+	while ((opt = getopt_long(argc, argvopt, "",
+				lgopts, &opt_idx)) != EOF) {
+		switch (opt) {

[...]

+#define MBUF_SIZE (1 << 14)

Why this size?
Add a comment?

+static void
+extbuf_free_cb(void *addr __rte_unused, void *fcb_opaque __rte_unused)
+{
+
+}

Empty function can be dropped.

[...]

+It is based on precomplied rule file, and an input file, both of them can

precompiled

+be selected using command-line options.
+
+In general case, each PMD has it's own rule file.

its

+
+The test outputs the performance, the results matching (rule id, position, len)

length

A list will look better.

+for each job and also a list of matches (rule id, position , len) in absulote

absolute

+position.
+
+
+Limitations
+~~~~~~~~~~~
+
+* Only one queue is supported.
+
+* Supports only precompiled rules.
+
+EAL Options
+~~~~~~~~~~~
+
+The following are the EAL command-line options that can be used in conjunction
+with the ``dpdk-test-regex`` application.
+See the DPDK Getting Started Guides for more information on these options.
+
+
+*   ``-w <PCI>``
+
+	Add a PCI device in white list.

Please drop "EAL options" chapter.
It is not specific to the app.

+Application Options
+~~~~~~~~~~~~~~~~~~~
+
+ ``--rules NAME``: precompiled rule file
+
+ ``--data NAME``: data file to use
+
+ ``--nb_jobs N``: number of jobs to use
+
+ ``--perf N``: only outputs the performance data
+
+ ``--nb_iter N``: number of iteration to run
+
+ ``--help``: prints this help

Please use definition list.

+Compiling the Tool
+------------------
+
+The ``dpdk-test-regex`` application depends on RegEx lib ``rte_regexdev``.

Useless

+
+
+Generating the data
+-------------------
+
+In the current version, the compiled rule file is loaded with a rule that
+matches 'hello world'. To create the data file,
+it is possible to use the included python script ``generate_data_file.py``
+ which generates two files,
+``input.txt`` which holds the input buffer. An input buffer is a random number
+of spaces chars followed by the phrase 'hello world'.
+This sequence is repeated a random number of times.
+The second file is ``res.txt`` which holds the position of each
+of the 'hello world' in the input file.

A script is missing to generate a default set of input data.

+Running the Tool
+----------------
+
+The tool has a number of command line options. Here is the sample command line:
+
+.. code-block:: console
+
+   ./build/app/testregex -w 83:00.0 -- --rules app/test-regex/hello_world.rof2 --data app/test-regex/input.txt --job 100

Bottom line, I think this application is not ready for DPDK 20.08.
It's good to have it available as a patch for first users who
want to play with the new regex library.
However, I propose waiting 20.11 to integrate a better version of it.