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

[Ori Kam]

Hi Pavan,

> -----Original Message-----
> From: dev <dev-bounces at dpdk.org> On Behalf Of Pavan Nikhilesh Bhagavatula
> Sent: Sunday, April 5, 2020 8:11 PM
> To: Ori Kam <orika at mellanox.com>; Jerin Jacob Kollanukkaran
> <jerinj at marvell.com>; xiang.w.wang at intel.com
> Cc: dev at dpdk.org; Shahaf Shuler <shahafs at mellanox.com>;
> hemant.agrawal at nxp.com; Opher Reviv <opher at mellanox.com>; Alex
> Rosenbaum <alexr at mellanox.com>; Dovrat Zifroni <dovrat at marvell.com>;
> Prasun Kapoor <pkapoor at marvell.com>; nipun.gupta at nxp.com;
> bruce.richardson at intel.com; yang.a.hong at intel.com; harry.chang at intel.com;
> gu.jian1 at zte.com.cn; shanjiangh at chinatelecom.cn;
> zhangy.yun at chinatelecom.cn; lixingfu at huachentel.com; wushuai at inspur.com;
> yuyingxia at yxlink.com; fanchenggang at sunyainfo.com;
> davidfgao at tencent.com; liuzhong1 at chinaunicom.cn;
> zhaoyong11 at huawei.com; oc at yunify.com; jim at netgate.com;
> hongjun.ni at intel.com; j.bromhead at titan-ic.com; deri at ntop.org;
> fc at napatech.com; arthur.su at lionic.com; Thomas Monjalon
> <thomas at monjalon.net>; Parav Pandit <parav at mellanox.com>
> Subject: Re: [dpdk-dev] [EXT] [PATCH v1 3/4] regexdev: add regexdev core
> functions
> 
> Hi Ori,
> 
> >> From: dev <dev-bounces at 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 at mellanox.com>
> >> >Signed-off-by: Parav Pandit <parav at 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>