Re: [dpdk-dev] [PATCH v3] regexdev: introduce regexdev subsystem

[Ori Kam <orika@mellanox.com>]

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday, February 25, 2020 7:57 AM
> To: Ori Kam <orika@mellanox.com>
> Cc: Jerin Jacob <jerinj@marvell.com>; xiang.w.wang@intel.com; dpdk-dev
> <dev@dpdk.org>; Pavan Nikhilesh <pbhagavatula@marvell.com>; Shahaf
> Shuler <shahafs@mellanox.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; Opher Reviv <opher@mellanox.com>; Alex
> Rosenbaum <alexr@mellanox.com>; dovrat@marvell.com; Prasun Kapoor
> <pkapoor@marvell.com>; Nipun Gupta <nipun.gupta@nxp.com>; Richardson,
> Bruce <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>
> Subject: Re: [dpdk-dev] [PATCH v3] regexdev: introduce regexdev subsystem
> 
> > > 4) app/test/test_regexdev.c like app/test/test_eventdev.c
> >
> > We started to create a super basic app, after the API will be finalized and we
> will have HW
> > we can push it. (if you need it faster than feel free)
> 
> A simple Unit test case needs to be present for the APIs. On the
> course of developing common code,
> it can be developed to test the common code with dummy/skeleton driver.
> 

Agree this is what we are currently have.

> >
> > > 5) Need a maintainer for maintaining the regex subsystem
> > >
> > We wish to maintain it if you agree.
> 
> Yes. Please.
> 

Great.

> > > >
> > > > One more thing, regarding the ops structure, I think it is better to split it
> to 2
> > > different
> > > > structures one enque and one for dequeue, since there are no real shared
> > > data and we will
> > > > be able to save memory, what do you think?
> > >
> > > Ops are allocated from mempool so it will be overhead to manage both.
> > > moreover, some
> > > of the fields added in req can be used for resp as info. cryptodev
> > > follows the similar concept,
> > > I think, we can have symmetry with cryptodev wherever is possible to avoid
> > > end-user to learn new API models.
> >
> > True that there will be overhead with 2 mempools (small one)
> > but lets assume 255 results. This means that the buffer should be 255 *
> sizeof(rte_regex_match) = 2K
> > also this will enable us to replace groupX with group[] which will allow even
> more groups.
> > In addition don't think that crypto is a good example.
> > The main difference is that in RegEx the output is different format then the
> input.
> 
> # IMO, Some of the fields may be useful for a response as well. I
> think application may be interested in following
> req filed in the response.
> a) buf_addr

I don't see how this can be used in the response. since if working in out of order result.
you don’t know which result will be returned. 
I also think it is error prone to use the same op for the enqueue and dequeue.

> b) scan_size

Please see above.

> c) user_id (This would be main one)

Agree

> 
> # Having two mempools adds overhead per lcore L1 cache usage and extra
> complexity to the application.
> 
> # IMO, From a performance perspective, one mempool is good due to less
> stress on the cache and it is costly to
> add new mempool for HW mempool implementations.
> 
> # I think, group[] use case we can add it when it required by
> introducing "matches_start_offset" field, which will
> tell the req, where is the end of group[] and where "matches" start
> with single mempool scheme also.
> 
> # I think, one of the other use case for "matches_start_offset" that,
> It may possible to put vendor-specific
> opaque data. It will be filled by driver on response. The application
> can reference the matches as
> 
> struct rte_regex_match *matches = RTE_PTR_ADD(ops, ops-
> 
>matches_start_offset);
> 

O.K for now we will keep  it as is, and we will see what will be in the future.

> >
> > > I assume you will send the v4 with these comments. I think, with v4 we
> > > can start implementing common library code.
> >
> > Just need to agree on the split (one more iteration )
> > and I will start working on the common code.
> 
> Ack.

Great,
I'm starting to work on V4 with all comments so the RFC will be acked and then will start 
coding the rest of the common code.