From: Jerin Jacob <jerinjacobk@gmail.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
Ori Kam <orika@mellanox.com>,
"xiang.w.wang@intel.com" <xiang.w.wang@intel.com>,
Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>,
"dev@dpdk.org" <dev@dpdk.org>,
Shahaf Shuler <shahafs@mellanox.com>,
"hemant.agrawal@nxp.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" <nipun.gupta@nxp.com>,
"bruce.richardson@intel.com" <bruce.richardson@intel.com>,
"yang.a.hong@intel.com" <yang.a.hong@intel.com>,
"harry.chang@intel.com" <harry.chang@intel.com>,
"gu.jian1@zte.com.cn" <gu.jian1@zte.com.cn>,
"shanjiangh@chinatelecom.cn" <shanjiangh@chinatelecom.cn>,
"zhangy.yun@chinatelecom.cn" <zhangy.yun@chinatelecom.cn>,
"lixingfu@huachentel.com" <lixingfu@huachentel.com>,
"wushuai@inspur.com" <wushuai@inspur.com>,
"yuyingxia@yxlink.com" <yuyingxia@yxlink.com>,
"fanchenggang@sunyainfo.com" <fanchenggang@sunyainfo.com>,
"davidfgao@tencent.com" <davidfgao@tencent.com>,
"liuzhong1@chinaunicom.cn" <liuzhong1@chinaunicom.cn>,
"zhaoyong11@huawei.com" <zhaoyong11@huawei.com>,
"oc@yunify.com" <oc@yunify.com>,
"jim@netgate.com" <jim@netgate.com>,
"hongjun.ni@intel.com" <hongjun.ni@intel.com>,
"j.bromhead@titan-ic.com" <j.bromhead@titan-ic.com>,
"deri@ntop.org" <deri@ntop.org>,
"fc@napatech.com" <fc@napatech.com>,
"arthur.su@lionic.com" <arthur.su@lionic.com>,
Parav Pandit <parav@mellanox.com>
Subject: Re: [dpdk-dev] [EXT] [PATCH v1 3/4] regexdev: add regexdev core functions
Date: Tue, 7 Apr 2020 18:24:36 +0530 [thread overview]
Message-ID: <CALBAE1OEf+Wk_D5iH1hDh3gcdBNOj8NKk0HX9Y5TRTThrhL7yA@mail.gmail.com> (raw)
In-Reply-To: <2907264.PWNTSA8uO9@thomas>
On Tue, Apr 7, 2020 at 5:57 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 07/04/2020 07:49, Jerin Jacob:
> > > >
> > > > If it abstracts it properly adding vdev and PCI is a simple change.
> > > > See
> > > >
> > > > lib/librte_eventdev/rte_eventdev_pmd_vdev.h
> > > > lib/librte_eventdev/rte_eventdev_pmd_pci.h
> > > >
> > > > I think, the common code should take from other matured subsystem instead if
> > > > writing from scratch,
> > > >
> > > I agree with you about the rewrite, but this is why I don't want to add this code
> > > until I know what this code should do and how it should be used.
> > > I don't agree, that one subsystem is like other one by default, and that if something
> > > is done for one subsystem it should be done for other.
> > > Not always what was done before is the best.
> > >
> > > Some time back there was a long thread about ethdev and the rte device
> > > where should it be released and by whom.
> > > My basic thinking is that unless proven otherwise the code should be in the PMD
> > > this is also why it is important for me to get this rte level API acked.
> > > when starting to implement the code for the PMD it will be cleared what
> > > is the shared code and how it is best to configure the system.
> > > Also this is not external API so it can be changed at any time.
> > > Saying that I don't think we should wait long before adding such code.
> > > I think that when we will have our first PMD we know better if such
> > > function is needed.
> > > Also think about that maybe this PMD will be shared with the
> > > net PMD so such function will also introduce more complexity.
> >
> >
> > My thought process was I like this when I added the common code for eventdev.
> > I have checked ethdev, cryptodev and followed a similar scheme
> > wherever it applicable for eventdev.
> > If we see the regexdev API, it is similar to ethdev. cryptodev and
> > eventdev API. From the device
> > API PoV, the framework needs to follow the same behavior to avoid
> > having new behavior for regexdev,
> > Especially in configure->queue_setup->start->rx_burst->tx_burst->stop->reconfigure->start
> > sequence.
> >
> >
> > Ethdev may be bloated by features,
>
> No, ethdev has more features and is evolving as a better API.
I meant the same, where ethdev filled with "ethdev specific" features,
better take another subsystem for devices
specific thing.
>
> > My request is to take cryptodev and
> > eventdev as a base
> > change to accordingly.
>
> When writing a new API, we should learn from past lessons,
> including eventdev and cryptodev, but also all the things evolving
> in ethdev.
It looks out of context.
>
> > That makes review process easy, As reviewer
> > needs only think, The rationale behind,
> > Why it regexdev common code chosen a different approach. Writing from
> > scratch makes the reviewer's job
> > difficult.
> >
> > We spend a lot of time reviewing and make mature cryptodev and
> > eventdev common code, Please leverage that.
>
> Again, leveraging doesn't mean to implement all in one patch.
> Instead of asking for adding more, please focus on what must be changed
> in what is proposed in this series.
Look at the configure function[1], Does it take care of reconfiguration?
Common code takes care of allocating memory for point for queue
objects. Where is that?
http://patches.dpdk.org/patch/67311/
I will put in other way out, We know how other devices works in DPDK,
if we are changing
the means then please mention in the cover letter for the proposed
design and why would
think, the new solution is better.
Regexdev specification(patch 1/4) is not different than other devices'
fundamental view.
In my understanding, we don't put any effort to understand, why other
subsystems were chosen the specific
implementation in the common code. For me, it worth spending my time
on the review once basic stuff is there.
int
rte_regexdev_configure(uint8_t dev_id, const struct rte_regexdev_config *cfg)
{
if (dev_id >= RTE_MAX_REGEXDEV_DEVS)
return -EINVAL;
if (regex_devices[dev_id] == NULL)
return -EINVAL;
if (cfg == NULL)
return -EINVAL;
if (regex_devices[dev_id]->dev_ops->dev_configure == NULL)
return -ENOTSUP;
return regex_devices[dev_id]->dev_ops->dev_configure
(regex_devices[dev_id], cfg);
}
>
>
next prev parent reply other threads:[~2020-04-07 12:55 UTC|newest]
Thread overview: 96+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-29 6:47 [dpdk-dev] [PATCH v1 0/4] add RegEx class Ori Kam
2020-03-29 6:47 ` [dpdk-dev] [PATCH v1 1/4] regexdev: introduce regexdev subsystem Ori Kam
2020-04-04 15:04 ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2020-04-05 15:03 ` Ori Kam
2020-03-29 6:47 ` [dpdk-dev] [PATCH v1 2/4] regexdev: add regex core h file Ori Kam
[not found] ` <DM5PR18MB221411AAEAFFA9D9F373292BC6C20@DM5PR18MB2214.namprd18.prod.outlook.com>
2020-04-07 8:53 ` Guy Kaneti
2020-04-07 16:16 ` Ori Kam
2020-04-07 16:27 ` Jerin Jacob
2020-04-08 7:37 ` Ori Kam
2020-04-08 7:48 ` Jerin Jacob
2020-04-08 8:31 ` Ori Kam
2020-04-08 8:38 ` Jerin Jacob
2020-04-08 9:51 ` Ori Kam
2020-03-29 6:47 ` [dpdk-dev] [PATCH v1 3/4] regexdev: add regexdev core functions Ori Kam
2020-04-04 15:01 ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2020-04-05 15:05 ` Ori Kam
2020-04-05 17:10 ` Pavan Nikhilesh Bhagavatula
2020-04-05 20:02 ` Ori Kam
2020-04-06 12:48 ` Pavan Nikhilesh Bhagavatula
2020-04-06 13:29 ` Thomas Monjalon
2020-04-06 13:38 ` Jerin Jacob
2020-04-06 19:11 ` Ori Kam
2020-04-07 5:49 ` Jerin Jacob
2020-04-07 6:46 ` Ori Kam
2020-04-07 7:22 ` Jerin Jacob
2020-04-07 12:27 ` Thomas Monjalon
2020-04-07 12:54 ` Jerin Jacob [this message]
2020-04-07 14:21 ` Guy Kaneti
2020-04-07 16:28 ` Ori Kam
2020-04-07 16:37 ` Guy Kaneti
2020-04-08 6:52 ` Ori Kam
2020-04-08 8:39 ` Ori Kam
2020-04-19 10:38 ` Guy Kaneti
2020-04-22 21:36 ` Ori Kam
2020-04-08 9:41 ` Ori Kam
2020-03-29 6:47 ` [dpdk-dev] [PATCH v1 4/4] regexdev: implement regex rte level functions Ori Kam
2020-04-04 14:27 ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2020-04-05 15:04 ` Ori Kam
2020-04-05 16:48 ` Pavan Nikhilesh Bhagavatula
2020-04-05 19:46 ` Ori Kam
2020-04-06 11:16 ` Thomas Monjalon
2020-04-06 12:33 ` Pavan Nikhilesh Bhagavatula
2020-04-06 13:14 ` Thomas Monjalon
2020-04-06 13:20 ` Jerin Jacob
2020-04-06 13:22 ` Pavan Nikhilesh Bhagavatula
2020-04-06 13:36 ` Thomas Monjalon
2020-04-06 13:50 ` Pavan Nikhilesh Bhagavatula
2020-04-06 14:00 ` Thomas Monjalon
2020-04-06 18:53 ` Ori Kam
2020-04-04 13:06 ` [dpdk-dev] [EXT] [PATCH v1 0/4] add RegEx class Pavan Nikhilesh Bhagavatula
2020-04-05 15:03 ` Ori Kam
2020-04-17 12:43 ` [dpdk-dev] [PATCH v2 " Ori Kam
2020-04-17 12:43 ` [dpdk-dev] [PATCH v2 1/4] regexdev: introduce regexdev subsystem Ori Kam
2020-04-17 12:43 ` [dpdk-dev] [PATCH v2 2/4] regexdev: add regex core h file Ori Kam
2020-04-20 10:48 ` Guy Kaneti
2020-04-20 15:49 ` Ori Kam
2020-04-17 12:43 ` [dpdk-dev] [PATCH v2 3/4] regexdev: add regexdev core functions Ori Kam
2020-04-17 12:43 ` [dpdk-dev] [PATCH v2 4/4] regexdev: implement regex rte level functions Ori Kam
2020-04-21 11:12 ` Guy Kaneti
2020-04-21 11:20 ` Ori Kam
2020-04-21 11:36 ` Guy Kaneti
2020-04-22 20:33 ` Ori Kam
2020-05-07 9:45 ` [dpdk-dev] [PATCH v3 0/4] add RegEx class Ori Kam
2020-05-07 9:45 ` [dpdk-dev] [PATCH v3 1/4] regexdev: introduce regexdev subsystem Ori Kam
2020-06-21 11:18 ` Ori Kam
2020-06-30 19:57 ` Ori Kam
2020-05-07 9:45 ` [dpdk-dev] [PATCH v3 2/4] regexdev: add regex core h file Ori Kam
2020-06-03 6:47 ` [dpdk-dev] [EXT] " Guy Kaneti
2020-05-07 9:45 ` [dpdk-dev] [PATCH v3 3/4] regexdev: add regexdev core functions Ori Kam
2020-06-03 6:57 ` [dpdk-dev] [EXT] " Guy Kaneti
2020-05-07 9:45 ` [dpdk-dev] [PATCH v3 4/4] regexdev: implement regex rte level functions Ori Kam
2020-06-03 6:59 ` [dpdk-dev] [EXT] " Guy Kaneti
2020-06-28 13:45 ` Guy Kaneti
2020-06-28 14:10 ` Ori Kam
2020-05-24 20:24 ` [dpdk-dev] [PATCH v3 0/4] add RegEx class Ori Kam
2020-07-02 7:45 ` [dpdk-dev] [PATCH v4 " Ori Kam
2020-07-02 7:46 ` [dpdk-dev] [PATCH v4 1/4] regexdev: introduce regexdev subsystem Ori Kam
2020-07-05 21:18 ` Thomas Monjalon
2020-07-06 7:02 ` Ori Kam
2020-07-02 7:46 ` [dpdk-dev] [PATCH v4 2/4] regexdev: add regex core h file Ori Kam
2020-07-02 7:46 ` [dpdk-dev] [PATCH v4 3/4] regexdev: add regexdev core functions Ori Kam
2020-07-05 21:08 ` Thomas Monjalon
2020-07-06 6:07 ` Ori Kam
2020-07-06 7:03 ` Thomas Monjalon
2020-07-06 8:00 ` Bruce Richardson
2020-07-02 7:46 ` [dpdk-dev] [PATCH v4 4/4] regexdev: implement regex rte level functions Ori Kam
2020-07-05 21:21 ` [dpdk-dev] [PATCH v4 0/4] add RegEx class Thomas Monjalon
2020-07-06 7:03 ` Ori Kam
2020-07-06 17:36 ` [dpdk-dev] [PATCH v5 " Ori Kam
2020-07-06 17:36 ` [dpdk-dev] [PATCH v5 1/4] regexdev: introduce regexdev subsystem Ori Kam
2020-07-06 19:38 ` Thomas Monjalon
2020-09-11 12:46 ` David Marchand
2020-07-06 17:36 ` [dpdk-dev] [PATCH v5 2/4] regexdev: add regex core h file Ori Kam
2020-07-06 17:36 ` [dpdk-dev] [PATCH v5 3/4] regexdev: add regexdev core functions Ori Kam
2020-07-06 17:36 ` [dpdk-dev] [PATCH v5 4/4] regexdev: implement regex rte level functions Ori Kam
2020-07-06 22:30 ` [dpdk-dev] [PATCH v5 0/4] add RegEx class Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CALBAE1OEf+Wk_D5iH1hDh3gcdBNOj8NKk0HX9Y5TRTThrhL7yA@mail.gmail.com \
--to=jerinjacobk@gmail.com \
--cc=alexr@mellanox.com \
--cc=arthur.su@lionic.com \
--cc=bruce.richardson@intel.com \
--cc=davidfgao@tencent.com \
--cc=deri@ntop.org \
--cc=dev@dpdk.org \
--cc=dovrat@marvell.com \
--cc=fanchenggang@sunyainfo.com \
--cc=fc@napatech.com \
--cc=gu.jian1@zte.com.cn \
--cc=harry.chang@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=hongjun.ni@intel.com \
--cc=j.bromhead@titan-ic.com \
--cc=jerinj@marvell.com \
--cc=jim@netgate.com \
--cc=liuzhong1@chinaunicom.cn \
--cc=lixingfu@huachentel.com \
--cc=nipun.gupta@nxp.com \
--cc=oc@yunify.com \
--cc=opher@mellanox.com \
--cc=orika@mellanox.com \
--cc=parav@mellanox.com \
--cc=pbhagavatula@marvell.com \
--cc=pkapoor@marvell.com \
--cc=shahafs@mellanox.com \
--cc=shanjiangh@chinatelecom.cn \
--cc=thomas@monjalon.net \
--cc=wushuai@inspur.com \
--cc=xiang.w.wang@intel.com \
--cc=yang.a.hong@intel.com \
--cc=yuyingxia@yxlink.com \
--cc=zhangy.yun@chinatelecom.cn \
--cc=zhaoyong11@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).