回复: [PATCH v3 1/3] ethdev: enable direct rearm with separate API

[Feifei Wang]

Hi, Morten, Ferruh

Thanks very much for your reviewing.

Whatever the worries about direct rearm or comments to improve direct rearm, these urge
us to learn more and think more.  I think it is beneficial and a good achievement for this exploration.

I will update the latest version for techboard code review. During this time,
I need some time to  do performance test for the latest version.
So it should not catch up with this week's meeting and will be done before the techboard
meeting in two weeks.

Best Regards
Feifei

> -----邮件原件-----
> 发件人: Morten Brørup <mb at smartsharesystems.com>
> 发送时间: Monday, March 6, 2023 9:26 PM
> 收件人: Ferruh Yigit <ferruh.yigit at amd.com>; Feifei Wang
> <Feifei.Wang2 at arm.com>; thomas at monjalon.net; Andrew Rybchenko
> <andrew.rybchenko at oktetlabs.ru>; techboard at dpdk.org
> 抄送: dev at dpdk.org; konstantin.v.ananyev at yandex.ru; nd <nd at arm.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com>; Ruifeng Wang
> <Ruifeng.Wang at arm.com>
> 主题: RE: [PATCH v3 1/3] ethdev: enable direct rearm with separate API
> 
> > From: Ferruh Yigit [mailto:ferruh.yigit at amd.com]
> > Sent: Monday, 6 March 2023 13.49
> >
> > On 1/4/2023 8:21 AM, Morten Brørup wrote:
> > >> From: Feifei Wang [mailto:feifei.wang2 at arm.com]
> > >> Sent: Wednesday, 4 January 2023 08.31
> > >>
> > >> Add 'tx_fill_sw_ring' and 'rx_flush_descriptor' API into direct
> > >> rearm mode for separate Rx and Tx Operation. And this can support
> > >> different multiple sources in direct rearm mode. For examples, Rx
> > >> driver is ixgbe, and Tx driver is i40e.
> > >>
> > >> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli at arm.com>
> > >> Suggested-by: Ruifeng Wang <ruifeng.wang at arm.com>
> > >> Signed-off-by: Feifei Wang <feifei.wang2 at arm.com>
> > >> Reviewed-by: Ruifeng Wang <ruifeng.wang at arm.com>
> > >> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli at arm.com>
> > >> ---
> > >
> > > This feature looks very promising for performance. I am pleased to
> > > see
> > progress on it.
> > >
> >
> > Hi Morten,
> >
> > Yes it brings some performance, but not to generic use case, only to
> > specific and constraint use case.
> 
> I got the impression that the supported use case is a prominent and
> important use case.
> 
> This is the primary argument for considering such a complex non-generic
> feature.
> 
> >
> > And changes are relatively invasive comparing the usecase it supports,
> > like it adds new two inline datapath functions and a new dev_ops.
> >
> > I am worried the unnecessary complexity and possible regressions in
> > the fundamental and simple parts of the project, with a good intention
> > to gain a few percentage performance in a specific usecase, can hurt
> > the project.
> >
> >
> > I can see this is compared to MBUF_FAST_FREE feature, but
> > MBUF_FAST_FREE is just an offload benefiting from existing offload
> > infrastructure, which requires very small update and logically change
> > in application and simple to implement in the drivers. So, they are
> > not same from complexity perspective.
> >
> > Briefly, I am not comfortable with this change, I would like to see an
> > explicit approval and code review from techboard to proceed.
> 
> I agree that the complexity is very high, and thus requires extra consideration.
> Your suggested techboard review and approval process seems like a good
> solution.
> 
> And the performance benefit of direct rearm should be compared to the
> performance using the new zero-copy mempool API.
> 
> -Morten