26/07/2020 21:58, Ori Kam: > --- a/app/meson.build > +++ b/app/meson.build > @@ -12,6 +12,7 @@ apps = [ > 'test-bbdev', > 'test-cmdline', > 'test-compress-perf', > + 'test-regex', > 'test-crypto-perf', > 'test-eventdev', > 'test-fib',
In this list, I think the alphabetical order was chosen. > diff --git a/app/test-regex/Makefile b/app/test-regex/Makefile > new file mode 100644 > index 0000000..d73e776 > --- /dev/null > +++ b/app/test-regex/Makefile > @@ -0,0 +1,24 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2020 Mellanox Corporation 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 [...] > --- /dev/null > +++ b/app/test-regex/generate_data_file.py > @@ -0,0 +1,29 @@ > +#!/usr/bin/env python > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2020 Mellanox Technologies, Ltd > + > +import random > + > +KEYWORD = 'hello world' > +MAX_COUNT = 10 > +MIN_COUNT = 5 > +MAX_LEN = 1024 > +REPEAT_COUNT = random.randrange(MIN_COUNT, MAX_COUNT) > + > +current_pos = 0; > +match_pos = [] > + > +fd_input = open('input.txt','w') > +fd_res = open('res.txt','w') 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". > --- /dev/null > +++ b/app/test-regex/hello_world.rof2 Already discussed in a separate thread. This file should be generated. > --- /dev/null > +++ b/app/test-regex/main.c > @@ -0,0 +1,429 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright 2020 Mellanox Technologies, Ltd > + * > + * This file contain the application main file > + * This application provides a way to test the RegEx class. 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.