Re: [dpdk-dev] [EXT] [PATCH v1 3/4] regexdev: add regexdev core functions

[Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>]

Hi Ori,

>Hi Pavan,
>
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Pavan Nikhilesh
>Bhagavatula
>> Sent: Sunday, April 5, 2020 8:11 PM
>> To: Ori Kam <orika@mellanox.com>; Jerin Jacob Kollanukkaran
>> <jerinj@marvell.com>; xiang.w.wang@intel.com
>> Cc: dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>;
>> hemant.agrawal@nxp.com; Opher Reviv <opher@mellanox.com>;
>Alex
>> Rosenbaum <alexr@mellanox.com>; Dovrat Zifroni
><dovrat@marvell.com>;
>> Prasun Kapoor <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; j.bromhead@titan-ic.com; deri@ntop.org;
>> fc@napatech.com; arthur.su@lionic.com; Thomas Monjalon
>> <thomas@monjalon.net>; Parav Pandit <parav@mellanox.com>
>> Subject: Re: [dpdk-dev] [EXT] [PATCH v1 3/4] regexdev: add regexdev
>core
>> functions
>>
>> Hi Ori,
>>
>> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Pavan
>Nikhilesh
>> >Bhagavatula
>> >>
>> >> Looks like this implementation is incomplete?
>> >> I don't see any pmd specific helper functions for @see
>> >rte_cryptodev_pmd.c,
>> >> rte_eventdev_pmd*
>> >>
>> >I think the current implementation includes all needed functions,
>> >at least for the first stage.
>> >You can find in rte_regexdev_driver.h the functions that should be
>> >called
>> >by the PMD. We have the register / unregister which acts the same
>as
>> >create
>> >and destroy. For parsing argument the PMD may call
>rte_kvargs_parse.
>> >
>>
>> _driver.h should atleast include
>> rte_regex_dev_pci_generic_probe/rte_regex_pmd_vdev_init
>> else there would be a lot of code repetition and possibly udefined
>behavior
>> at the driver layer.
>>
>Why should they be included? At least in this stage, there is no code to
>share
>ethdev why should we add code for the vdev?

Ok I think I failed to communicate my concerns across. 
Let me retry

1. SW based regex devices such as PCRE/Hyperscan rely on vdev framework
i.e. user needs to pass an EAL argument --vdev="regex_pcre" for the driver to 
initialize all the other EAL subsystems (ethdev, eventdev, cryptodev, etc..)support this.

2. HW based independent regex devices that are exposed as PCI devices would need
 pci probe helpers.


>I agree that if we see that there is shared code, we should think about
>creating
>those functions.
>
>> And also why take a different path than the rest of the rte
>subsystems?
>>
>
>Even now if you look at the reference code you will see that there is
>really minimum shared code.
>also this result in that the PMD is not free to allocate resource in the
>order he needs.
>My thinking is that if there are only 2 lines of shared code I prefer to let
>the PMD handle it.
>I know that sharing code should be the first option, but this also makes
>the PMD developer unaware what is going on.
>and lose some control. For example if the PMD programmer wants to
>create hybrid PMD net +
>regex for example, then either he will be forced to do some hacks or
>just by pass the function
>so when this function will be updated his code will break.

Shouldn't the application/end user make that decision rather than PMD programmer?.
If application wants to connect net to regex it should be made possible to do it via rte_flow.

As an example if we see event device spec. It has robust features to connect multiple 
subsystems(ethernet/crypto/timer) to event device and it is controlled from RTE layer.
PMD layer should act on the inputs from RTE layer rather than action on it own.

Thoughts?
Thanks,
Pavan.

 So I prefer if it
>is very short code
>and this code can be developed in different ways to leave it to the
>PMD.
>I suggest that if needed we will add such function. Is that O.K by you?
>
>> >
>> >> >This commit introduce the API that is needed by the RegEx
>devices in
>> >> >order to work with the RegEX lib.
>> >> >
>> >> >During the probe of a RegEx device, the device should configure
>> >itself,
>> >> >and allocate the resources it requires.
>> >> >On completion of the device init, it should call the
>> >> >rte_regex_dev_register in order to register itself as a RegEx
>device.
>> >> >
>> >> >Signed-off-by: Ori Kam <orika@mellanox.com>
>> >> >Signed-off-by: Parav Pandit <parav@mellanox.com>
>> >> >---
>> >> > config/common_base                        |  3 +-
>> >> > config/meson.build                        |  1 +
>> >> > lib/librte_regexdev/Makefile              |  1 +
>> >> > lib/librte_regexdev/meson.build           |  5 ++-
>> >> > lib/librte_regexdev/rte_regexdev.c        | 74
>> >> >++++++++++++++++++++++++++++++-
>> >> > lib/librte_regexdev/rte_regexdev.h        |  7 +++
>> >> > lib/librte_regexdev/rte_regexdev_core.h   |  2 +
>> >> > lib/librte_regexdev/rte_regexdev_driver.h | 50
>> >> >+++++++++++++++++++++
>> >> > meson_options.txt                         |  2 +
>> >> > 9 files changed, 142 insertions(+), 3 deletions(-)
>> >> > create mode 100644 lib/librte_regexdev/rte_regexdev_driver.h
>> >> >
>> >>
>> >> <snip>