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 D0323A0542; Wed, 15 Jul 2020 19:20:25 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A61254C8A; Wed, 15 Jul 2020 19:20:25 +0200 (CEST) Received: from new2-smtp.messagingengine.com (new2-smtp.messagingengine.com [66.111.4.224]) by dpdk.org (Postfix) with ESMTP id 4967F2B9A for ; Wed, 15 Jul 2020 19:20:24 +0200 (CEST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailnew.nyi.internal (Postfix) with ESMTP id D116C580FE6; Wed, 15 Jul 2020 13:20:22 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Wed, 15 Jul 2020 13:20:22 -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= Bk+eOGM3VGHFKRRoW3ps3YrTqroX4LY+tDmxqXN+pxo=; b=gkBQaU6PzE3ZOBpi UKuSyEc34jU3o5lqvspePbcdCV76eXeV4F2iYV74tkczin9+KE6Vu/BJkqJIs429 +d1t9m9e3f3SuvPvU2jqQAxJcRQtW57Knj77nKi/CgLVfQ5rlBH0tnCmQO+qpP3U MQQ/7jW3lWCOcztBMLWIscKJkD4xLugo/dzb//LG9r8U3WrU1936f4YnMmgnmJZw YHw+lmyx+sVfyCKewyB+MbUzpPmDEr3YpeoI3ayNeEON51mFyqeAGMX9xD5paI20 8Tswakrc9xPJcC5OBYz5DqGHjMs/4T+bzur2uzrqAw0FGb0KOLT5bkGJEV+ctzol FQKQqg== 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=Bk+eOGM3VGHFKRRoW3ps3YrTqroX4LY+tDmxqXN+p xo=; b=BHHcMhRYp/oTmL2dcwb0sDEXBQMBBchEKMPVrwwau0mJBecznzsHFb1bx ftHKfAfClS1aZfLRvRGO1bp+pHNJxHvAtjQz1SfawVdHZ3zX3NXfKUjBPpa4M6fj nKvTwSn2JtMXz7QEeC9xldLCboAI0BFtfvx6UvT1q+h7PJz2QlegOKia2cW655xI WvD1cCg/7Q7RBlt91gPJGvtIhZaAGqu8YlT7ABDxUIJmv3Uu6c4y6CfikXyPTzCz l9haCh6+Xj7TEHUpn0mmd3vZPoRMJvpoOvyEEuMzy7xSkRZ50lKta7XW4JmqBFt0 TLD6KSeswaR8v/OylyExmSZGYxdjg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedrfedvgdduudekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepveetvdetjeetgeevieegjeejuefhuedvfeekledtveeugfevveel feeikeeiveffnecuffhomhgrihhnpehptghrvgdrohhrghenucfkphepjeejrddufeegrd dvtdefrddukeegnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhf rhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth 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 E468D30600A6; Wed, 15 Jul 2020 13:20:11 -0400 (EDT) From: Thomas Monjalon To: Ori Kam Cc: jerinj@marvell.com, xiang.w.wang@intel.com, matan@mellanox.com, viacheslavo@mellanox.com, John McNamara , Marko Kovacevic , Shahaf Shuler , Ray Kinsella , Neil Horman , dev@dpdk.org, guyk@marvell.com, pbhagavatula@marvell.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 , honnappa.nagarahalli@arm.com, asafp@mellanox.com, shys@mellanox.com Date: Wed, 15 Jul 2020 19:20:10 +0200 Message-ID: <1981010.RyStOxmPgm@thomas> In-Reply-To: <1594587541-110442-2-git-send-email-orika@mellanox.com> References: <1593941027-86651-1-git-send-email-orika@mellanox.com> <1594587541-110442-1-git-send-email-orika@mellanox.com> <1594587541-110442-2-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 v2 01/20] regex/mlx5: add RegEx PMD layer and mlx5 driver 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" 12/07/2020 22:58, Ori Kam: > From: Yuval Avnery > > This commit introduce the RegEx poll mode drivers class, and > adds Mellanox RegEx PMD. > > Signed-off-by: Yuval Avnery > Signed-off-by: Ori Kam > --- > v2: > * Add documantion. First typo. Bad start for a doc update... > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -453,7 +453,9 @@ F: doc/guides/compressdevs/features/default.ini > RegEx API - EXPERIMENTAL > M: Ori Kam > F: lib/librte_regexdev/ > +F: drivers/regex/ No, please don't. > F: doc/guides/prog_guide/regexdev.rst > +F: doc/guides/regexdevs/features/default.ini > +RegEx Drivers > +------------------ Too much underlines [...] > +; Features of a default RegEx driver. > +; > +; This file defines the features that are valid for inclusion in > +; the other driver files and also the order that they appear in > +; the features table in the documentation. The feature description > +; string should not exceed feature_str_len defined in conf.py. > +; > +[Features] > +ARMv7 = obsolete > +ARMv8 = Should be "Armv8" (with lower cases) or aarch64. > +Power8 = not very used > +x86-32 = not very used > +x86-64 = You can add simply "x86". > +Usage doc = > +Design doc = > +Perf doc = I think you drop docs from the features list. [...] > +++ b/doc/guides/regexdevs/features_overview.rst > @@ -0,0 +1,118 @@ > +.. SPDX-License-Identifier: BSD-3-Clause > + Copyright 2020 Mellanox Technologies, Ltd Why 4 spaces? 3 are enough (and recommended in code style I think). > + > +Overview of RegEx Drivers Features > +================================== > + > +This section explains the supported features that are listed in the table below. > + > +Cross buffer > + Support cross buffer detection. > + > +PCRE start anchor > + Support PCRE start anchor. > + > +PCRE atomic grouping > + Support PCRE atomic grouping. > + > +PCRE back reference > + Support PCRE back regerence. > + > +PCRE back tracking ctrl > + Support PCRE back tracking ctrl. > + > +PCRE call outs > + Support PCRE call outes. > + > +PCRE forward reference > + Support Forward reference. > + > +PCRE greedy > + Support PCRE greedy mode. > + > +PCRE match all > + Support PCRE match all. > + > +PCRE match as end > + Support match as end. > + > +PCRE match point rst > + Support PCRE match point reset directive. > + > +PCRE New line conventions > + Support new line conventions. > + > +PCRE new line SEQ > + Support new line sequence. > + > +PCRE look around > + Support PCRE look around. > + > +PCRE possessive qualifiers > + Support PCRE possessive qualifiers. > + > +PCRE subroutine references > + Support PCRE subroutine references. > + > +PCRE UTF 8 > + Support UTF-8. > + > +PCRE UTF 16 > + Support UTF-16. > + > +PCRE UTF 32 > + Support UTF-32. > + > +PCRE word boundary > + Support word boundaries. > + > +Run time compilation > + Support compilation during run time. > + All these features are not in features.ini > +.. note:: > + > + Most of the features capabilities should be provided by the drivers via the > + next vDPA operations: ``get_features`` and ``get_protocol_features``. Now I understand why we don't have doc reviews: even the author is not reading its own doc! > + > + > +References > +========== > + > + * `PCRE: PCRE patteren man page `_ Typo > +++ b/doc/guides/regexdevs/index.rst > @@ -0,0 +1,15 @@ > +.. SPDX-License-Identifier: BSD-3-Clause > + Copyright 2020 Mellanox Technologies, Ltd 3 spaces are enough > +++ b/doc/guides/regexdevs/mlx5.rst > @@ -0,0 +1,95 @@ > +.. SPDX-License-Identifier: BSD-3-Clause > + Copyright 2020 Mellanox Technologies, Ltd 3 spaces are enough > +Design > +------ > + > +This PMD is configuring the RegEx HW engine. > +For the PMD to work, the application must supply > +a precompiled rule file in rof2 format. A link to rof2? > + > +The PMD can use libibverbs and libmlx5 to access the device firmware > +or directly the hardware components. can? or always? s/can use/uses/ > +There are different levels of objects and bypassing abilities > +to get the best performances: > + > +- Verbs is a complete high-level generic API > +- Direct Verbs is a device-specific API > +- DevX allows to access firmware objects > +- Direct Rules manages flow steering at low-level hardware layer How flow steering is related to RegEx engine? > + > +Enabling librte_pmd_mlx5_regex causes DPDK applications to be linked against > +libibverbs. > + > +A Mellanox mlx5 PCI device can be probed by either net/mlx5 driver or regex/mlx5 > +driver but not in parallel. Hence, the user should decide the driver by dissabling disabling > +the net device using ``CONFIG_RTE_LIBRTE_MLX5_PMD``. The meson disabling option is missing. Isn't is possible to decide at runtime with devargs? > + > +Supported NICs > +-------------- > + > +* Mellanox\ |reg| BlueField 2 SmartNIC > + > +Prerequisites > +------------- > + > +- Mellanox OFED version: **5.0** > + see :doc:`../../nics/mlx5` guide for more Mellanox OFED details. Which upstream rdma-core version? > +- Enable the RegEx caps using system call from the BlueField 2. > + Contact Mellanox support for detail explanation. Why not giving the details here? Or link to an external doc? > + > +Compilation options > +~~~~~~~~~~~~~~~~~~~ > + > +These options can be modified in the ``.config`` file. > + > +- ``CONFIG_RTE_LIBRTE_MLX5_REGEX_PMD`` (default **n**) > + > + Toggle compilation of librte_pmd_mlx5 itself. > + > +- ``CONFIG_RTE_IBVERBS_LINK_DLOPEN`` (default **n**) > + > + Build PMD with additional code to make it loadable without hard > + dependencies on **libibverbs** nor **libmlx5**, which may not be installed > + on the target system. > + > + In this mode, their presence is still required for it to run properly, > + however their absence won't prevent a DPDK application from starting (with > + ``CONFIG_RTE_BUILD_SHARED_LIB`` disabled) and they won't show up as > + missing with ``ldd(1)``. > + > + It works by moving these dependencies to a purpose-built rdma-core "glue" > + plug-in which must either be installed in a directory whose name is based > + on ``CONFIG_RTE_EAL_PMD_PATH`` suffixed with ``-glue`` if set, or in a > + standard location for the dynamic linker (e.g. ``/lib``) if left to the > + default empty string (``""``). > + > + This option has no performance impact. > + > +- ``CONFIG_RTE_IBVERBS_LINK_STATIC`` (default **n**) > + > + Embed static flavor of the dependencies **libibverbs** and **libmlx5** > + in the PMD shared library or the executable static binary. These options are not specific to this driver. We should link to the original explanations in the net PMD. By the way it is missing meson explanations. [...] > --- /dev/null > +++ b/doc/guides/regexdevs/overview_feature_table.txt This generated file should not be committed in the repo. You are missing an update in doc/guides/conf.py and .gitignore. [...] > +* **Added the RegEx Library, a generic RegEx service library.** Redundant with previous lib addition. > + > + Added Mellanox MLX5 RegEx PMD driver, which implements the RegEx library > + and allows to offload RegEx searches. > + Please prefer this rebased change: Added the RegEx library which provides an API for offload of regular expressions search operations to hardware or software accelerator devices. + Added Mellanox RegEx PMD, allowing to offload RegEx searches. + > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -24,5 +24,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += event > DEPDIRS-event := common bus mempool net crypto > DIRS-$(CONFIG_RTE_LIBRTE_RAWDEV) += raw > DEPDIRS-raw := common bus mempool net event > +DIRS-$(CONFIG_RTE_LIBRTE_REGEXDEV) += regex > +DEPDIRS-regex := common Please keep same order everywhere: regex between compressdev and eventdev. (yes vdpa should have been just after net) [...] > --- a/drivers/meson.build > +++ b/drivers/meson.build > @@ -11,7 +11,8 @@ dpdk_driver_classes = ['common', > 'compress', # depends on common, bus, mempool. > 'vdpa', # depends on common, bus and mempool. > 'event', # depends on common, bus, mempool and net. > - 'baseband'] # depends on common and bus. > + 'baseband', # depends on common and bus. > + 'regex'] # depends on common, bus, regexdev. Again, please add after compress. [...] > +if get_option('buildtype').contains('debug') > + cflags += [ '-pedantic', '-DPEDANTIC' ] > +else > + cflags += [ '-UPEDANTIC' ] > +endif Please let's stop with pedantic now. By the way, it does not compile: drivers/regex/mlx5/mlx5_regex.c:6: error: ISO C forbids an empty translation unit [-Werror=pedantic]