From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A9949A053E; Mon, 27 Jul 2020 19:09:46 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 510DD1BFE0; Mon, 27 Jul 2020 19:09:46 +0200 (CEST) Received: from new2-smtp.messagingengine.com (new2-smtp.messagingengine.com [66.111.4.224]) by dpdk.org (Postfix) with ESMTP id ABFD43B5 for ; Mon, 27 Jul 2020 19:09:45 +0200 (CEST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailnew.nyi.internal (Postfix) with ESMTP id E9C215800B1; Mon, 27 Jul 2020 13:09:44 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Mon, 27 Jul 2020 13:09:44 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm1; bh= zkn+1W2kHbLruWs6ng4K13IVOm25G2WF6OSrA9ZtGs4=; b=AsGYtUOGsQ837YAg 37HNlqxhkxJjTuKu0zKy69dZ17wKQkZxgthZ4adoM5yEGIo/PJiMv2zGTF3ED1WY uPb+tvAcTvlII7PRHPHeQ3VWbYs0FAN7HY1ZYFbusfovMWrYnbfemHU0cSTKzwBW 8w2/XKP8z9QSaaaXplS1yrwi3ZrrdZhYnjz5oS3NrLpgflrOAMSOqPzPG3JfT1s9 Kqsfd41s0NNlR2ZrTinH0eFIzeLxi0ojch35WU67UFPeUSEO+YGgoh2hywNaDdF0 4XyMfA0ASQAYbFNDCIA4nQfXm+Nfx/jiX4XKjh1U9eyLf25SxBvbsv6GCJmIQdFN MZXuAg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=zkn+1W2kHbLruWs6ng4K13IVOm25G2WF6OSrA9ZtG s4=; b=ubH13MDm4d0ckHJRSLJbCOeMuJrrVHQI6rDuP9a0qR1vES76dXFiT78At o2dPrNy1KoNajooBkzByVsKt2BwgmvsdOru8cMphKak+yWRXFu4RDGJMdwKH319O ILi7p1l8EufxiS8ppo43YZuh6NLvlCvlbYIYJ7HWzV8gnoMlEjlfdhx1LzSDnEXC yWYs8yLP/C/fQ5nA2Iiz5Et2o42hw5Eypv0I3NgZFoBI7KYhj9wI+/COlGb6Kx96 GT7/IGJe0eCBM8SkrRTMCmZ6l5YNO9OTYD4dZF+mMt0y3xRCi1AY0xseLMegcBAV nk/uz+rT5GrQ4BpMe5A51+PB5x5og== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedriedtgdduuddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepheehgefhleffhedtveffjedukedttddtvddthffhfffhveevvdeu heffgedtvdffnecuffhomhgrihhnpehvrghrshdrmhhknecukfhppeejjedrudefgedrvd dtfedrudekgeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhr ohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id B3718328005A; Mon, 27 Jul 2020 13:09:35 -0400 (EDT) From: Thomas Monjalon To: jerinj@marvell.com, xiang.w.wang@intel.com, matan@mellanox.com, viacheslavo@mellanox.com, John McNamara , Marko Kovacevic , dev@dpdk.org, Ori Kam Cc: guyk@marvell.com, dev@dpdk.org, pbhagavatula@marvell.com, shahafs@mellanox.com, hemant.agrawal@nxp.com, opher@mellanox.com, alexr@mellanox.com, dovrat@marvell.com, pkapoor@marvell.com, nipun.gupta@nxp.com, bruce.richardson@intel.com, yang.a.hong@intel.com, harry.chang@intel.com, gu.jian1@zte.com.cn, shanjiangh@chinatelecom.cn, zhangy.yun@chinatelecom.cn, lixingfu@huachentel.com, wushuai@inspur.com, yuyingxia@yxlink.com, fanchenggang@sunyainfo.com, davidfgao@tencent.com, liuzhong1@chinaunicom.cn, zhaoyong11@huawei.com, oc@yunify.com, jim@netgate.com, hongjun.ni@intel.com, deri@ntop.org, fc@napatech.com, arthur.su@lionic.com, orika@mellanox.com, rasland@mellanox.com, Yuval Avnery Date: Mon, 27 Jul 2020 19:09:34 +0200 Message-ID: <3437146.xsi8y2SdMt@thomas> In-Reply-To: <1595793496-73205-1-git-send-email-orika@mellanox.com> References: <1595793496-73205-1-git-send-email-orika@mellanox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v1] app/test-regex: add RegEx test application X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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 `` > + > + 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.