[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: Monday, April 6, 2020 3:48 PM
> 
> Hi Ori,
> 
> >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?
> 
> 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.
> 
> 

Like mentions in other treads lets leave it as is for now,
And based on PMD implementation add the required functions.

> >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.
> 
In this case there will be I guess new rte_flow command, and there will be no
no use for the enqueue/deques or there will not be use for rx_burst / tx_burst
This is a story for another day.
In any case the PMD should be as flexiable as possible.  This is why I think that
the most code should be in the PMD since it is PMD implemetion and if we add
common code in rte level this can make the PMD less flexable. For example how
to share resource between net / regex device. For example even in current 
stage the application can receive packets from the net device and send those 
packets to the regex device, if the application uses both devices from the same 
company they can share information that can save time, for example memory 
registration values.
 
> 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.
> 
Like I said above until we support inline regex, the application 
moves the packets just like it moves them between net devices,
so each PMD should have the ability to configure itself as best as it can.
Again I totally in favor that if we see common code we should
move it to rte level. But this should be done only after we have a code
and in any case this doesn't effect the API.
 
>  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>