Re: [dpdk-dev] [PATCH v5 01/13] regex/mlx5: add RegEx PMD layer and mlx5 driver

[Thomas Monjalon <thomas@monjalon.net>]

19/07/2020 20:09, Ori Kam:
> From: Yuval Avnery <yuvalav@mellanox.com>
> 
> This commit introduce the RegEx pull mode drivers class, and
> adds Mellanox RegEx PMD.
[...]
> --- /dev/null
> +++ b/doc/guides/regexdevs/features_overview.rst
> @@ -0,0 +1,100 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +   Copyright 2020 Mellanox Technologies, Ltd

SPDX is not aligned with the Copyright.
You should not have a double space before SPDX.

[...]
> --- /dev/null
> +++ b/doc/guides/regexdevs/index.rst
> @@ -0,0 +1,15 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +    Copyright 2020 Mellanox Technologies, Ltd

Same here. RST annotations start at the third column.

> +
> +REGEX Device Drivers
> +====================
> +
> +The following are a list of RegEx (Regular Expression) device drivers,
> +which can be used from an application through RegEx API.
> +
> +.. toctree::
> +    :maxdepth: 2
> +    :numbered:

same here

[...]
> +   Due to external dependencies, this driver is disabled in default
> +   configuration of the "make" build. It can be enabled with
> +   ``CONFIG_RTE_LIBRTE_MLX5_REGEX_PMD=y`` or by using "meson" build system which
> +   will detect dependencies.

I would drop this part. I don't think it is an useful info,
especially because make is going to be removed.

> +Mellanox mlx5 PCI device can be probed by number of different pci devices, for example

Inconsistency on the same line: "pci" vs "PCI".

Better to break line after the comma and after a dot.

> +net / vDPA / RegEx. To select the RegEx PMD ``class=regex`` should be specified
> +as device parameter.

This does not explain if RegEx is exclusive with other classes.

[...]
> +- BlueField 2 running Mellonx supported kernel.

typo: Mellonx

> +- Enable the RegEx caps using system call from the BlueField 2.

This is a doc. Please write full words: "capabilities".

[...]
> +These options can be modified in the ``.config`` file.
> +
> +- ``CONFIG_RTE_LIBRTE_MLX5_REGEX_PMD`` (default **n**)
> +
> +  Toggle compilation of librte_pmd_mlx5 itself.

Better to drop this "make" part.
[...]
>  enum mlx5_class {
>  	MLX5_CLASS_INVALID,
>  	MLX5_CLASS_NET = RTE_BIT64(0),
> +	MLX5_CLASS_REGEX = RTE_BIT64(2),
>  	MLX5_CLASS_VDPA = RTE_BIT64(1),

I think regex would be better sorted after vDPA.

[...]
> +++ b/drivers/regex/meson.build
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2020 Mellanox Technologies, Ltd
> +
> +drivers = ['mlx5']
> +std_deps = ['ethdev', 'kvargs'] # 'ethdev' also pulls in mbuf, net, eal etc
> +std_deps += ['bus_pci']         # very many PMDs depend on PCI, so make std
> +std_deps += ['bus_vdev']        # same with vdev bus

I disagree about making bus some standard deps.