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

[Ori Kam <orika@mellanox.com>]

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? 
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. 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>