Re: [dpdk-dev] [EXT] [PATCH v1 4/4] regexdev: implement regex rte level functions

[Ori Kam <orika@mellanox.com>]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Thomas Monjalon
> Sent: Monday, April 6, 2020 5:00 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Ori Kam
> <orika@mellanox.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; xiang.w.wang@intel.com;
> 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; david.marchand@redhat.com
> Subject: Re: [dpdk-dev] [EXT] [PATCH v1 4/4] regexdev: implement regex rte
> level functions
> 
> 06/04/2020 15:50, Pavan Nikhilesh Bhagavatula:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > >> >06/04/2020 14:33, Pavan Nikhilesh Bhagavatula:
> > >> >> >> From: Pavan Nikhilesh Bhagavatula
> > >> >> >> > >+uint16_t
> > >> >> >> > >+rte_regexdev_enqueue_burst(uint8_t dev_id, uint16_t
> > >> >qp_id,
> > >> >> >> > >+			   struct rte_regex_ops **ops, uint16_t
> > >> >nb_ops)
> > >> >> >> > >+{
> > >> >> >> > >+	return regex_devices[dev_id]-
> > >> >> >> > >>enqueue(regex_devices[dev_id], qp_id,
> > >> >> >> > >+					      ops, nb_ops);
> > >> >> >> > >+}
> > >> >> >> >
> > >> >> >> > Move these functions to .h in-lining them.
> > >> >> >> > Also, please add debug checks @see
> > >> >> >rte_eth_rx_burst/rte_eth_tx_burst.
> > >> >> >>
> > >> >> >> O.K will update.
> > >> >> >
> > >> >> >In general, inlining is a pain for ABI compatibility.
> > >> >> >Please inline only if the gain is very significant.
> > >> >> >
> > >> >>
> > >> >> The performance gain mostly comes from hoisting
> > >> >`regex_devices[dev_id]` load above the
> > >> >> poll loop.
> > >> >> Since, the performance measurement application is still in pipeline
> > >> >and regexdev would be
> > >> >> experimental for next couple of releases I suggest inlining it now
> > >and
> > >> >worrying about ABI when
> > >> >> experimental tag needs to be removed.
> > >> >
> > >> >No, we must worry about ABI from the beginning.
> > >>
> > >> I though performance was the primary objective :-).
> > >
> > >It is.
> > >
> > >> >> We can follow the same path as done by ethdev
> > >>
> >[https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Furldefe
> nse.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-
> &amp;data=02%7C01%7Corika%40mellanox.com%7Cc3e07910f52846f64b5008
> d7da32d82e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6372177
> 84252928490&amp;sdata=8Pxq8ciUVW7bMiB1XmQBNm%2Fsmy8m1Wa7yKXK
> JRL4d%2B4%3D&amp;reserved=0
> >
> >3A__https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> w.mail-
> %2F&amp;data=02%7C01%7Corika%40mellanox.com%7Cc3e07910f52846f64b
> 5008d7da32d82e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637
> 217784252928490&amp;sdata=sTDsN85xShTU7WOLQQ2iF8eh%2FyoVHYwDq%
> 2FTdgdHdlbM%3D&amp;reserved=0
> > >> >2Darchive.com_dev-
> > >>
> > >>40dpdk.org_msg142392.html&d=DwICAg&c=nKjWec2b6R0mOyPaz7xt
> > >f
> > >> >Q&r=E3SgYMjtKCMVsB-fmvgGV3o-
> > >> >g_fjLhk5Pupi9ijohpc&m=7Gqb6WKmZV5uY3xa7FRVrRVDz8Usrsd-
> > >>
> > >>rDjIKr6CUQQ&s=sQo2Kx9fzTNXwiQ2Fzki3s5GSuiiAEzz2VtN68_KKXo&e
> > >=
> > >> >]
> > >> >
> > >> >ethdev is not an argument.
> > >>
> > >> What about ring?
> [https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Furldefens
> e.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttp-
> &amp;data=02%7C01%7Corika%40mellanox.com%7Cc3e07910f52846f64b5008
> d7da32d82e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6372177
> 84252928490&amp;sdata=t2qUIj51WPzqQhJngheYh1s7DWzOxd%2FcAyAQK39C
> HgQ%3D&amp;reserved=0
> > >3A__mails.dpdk.org_archives_dev_2020-
> > >2DApril_161506.html&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3S
> > >gYMjtKCMVsB-fmvgGV3o-
> > >g_fjLhk5Pupi9ijohpc&m=u9gnM_YrOJDusN4yR8YUcUuUkri4tOjnHrQ0A
> > >Qd5zTw&s=uv6AQA-
> > >Zu7o6ugyhpGHLxFOk4SfEdkHfFGDmhzANRME&e= ]
> > >>
> > >> Why do we need to prove the same performance advantage using in-
> > >lining for datapath
> > >> critical functions again and again?
> > >
> > >Because every libraries have not the same usage and load.
> > >We should compare how much cycle is saved with inline vs
> > >how much cycles is, "in average", a regex burst?
> > >
> > >If you tell me regex processing is fast, OK, let's inline.
> > >
> >
> > Regex processing speed would still be dependent on underlying device
> capabilities.
> >
> > All we are trying to do is reduce the enqueue/dequeue completion time
> which would
> > bring down the overall latency.
> 
> Take your regex HW and do a simple regex processing burst.
> How many cycles it takes to complete?
> How many cycles you lose if not inline?
> If the ratio is lower than 1/200, I think inline is not a must.
> 
> Ori, please consider the same measure for your HW.
> 
I think that the default for data path should be inline.
In this specific case maybe the benefit is small.
I guess that by definition the RegEx is slower than the net device.
It is much harder to check the performance of RegEx device due to the 
fact that it depends on the data itself and on the rules.

I don't have HW to check the numbers right now. Even if I have them
this is for Mellanox implementation other PMD may have different values.
I suggest that we change to inline, and after getting the HW we will run some tests
and based on them we can decide if the API should be changed.
Please note that the API change will be before the first integration of this patch
so it is not real API change.