Hi Maxime, > -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > Sent: Tuesday, March 26, 2019 5:29 PM > To: Lu, Wenzhuo <wenzhuo...@intel.com>; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 6/8] net/ice: support Rx AVX2 vector > > Hi, > > On 3/26/19 2:00 AM, Lu, Wenzhuo wrote: > > Hi Maxime, > > > >> -----Original Message----- > >> From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > >> Sent: Monday, March 25, 2019 4:26 PM > >> To: Lu, Wenzhuo <wenzhuo...@intel.com>; dev@dpdk.org > >> Subject: Re: [dpdk-dev] [PATCH v5 6/8] net/ice: support Rx AVX2 > >> vector > >> > >> Hi, > >> > >> On 3/25/19 3:22 AM, Lu, Wenzhuo wrote: > >>> Hi Maxime, > >>> > >>> > >>>> -----Original Message----- > >>>> From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > >>>> Sent: Friday, March 22, 2019 6:12 PM > >>>> To: Lu, Wenzhuo <wenzhuo...@intel.com>; dev@dpdk.org > >>>> Subject: Re: [dpdk-dev] [PATCH v5 6/8] net/ice: support Rx AVX2 > >>>> vector > >>> > >>> > >>>>> +#ifndef RTE_LIBRTE_ICE_16BYTE_RX_DESC > >>>> > >>>> I see same is done for other Intel NICs, but I wonder what would be > >>>> the performance cost of making it dynamic, if any cost? > >>> Currently we don't have a good idea to make it dynamic. If we use > >>> pointer > >> to point to different functions for 16 byte and 32 byte, there's too > >> much duplicate code to make it hard to maintain. If we use the same > >> function, and check the configure in it. It impacts the performance. > >> > >> Have you done some measurements, what would be the performance > >> impact? > > I mean if we check the configuration is 16 byte or 32 byte, this check will > consume extra CPU cycles. > > That why I think the better way is to have different paths for 16 byte and > 32 byte. We should choose the appropriate path at the beginning. > > > >> > >>> As HW does not support to change the configuration dynamically. The > >> device must be stopped and restarted if the configuration is changed. > >> It's not very helpful to make it a dynamic configuration. We assume > >> that the users can make their choice at the beginning and will not change > it. > >> > >> The problem is that the user has to recompile to switch between the > >> two configurations. And it may not be an option for the user if he > >> uses dpdk packaged by a distribution, for example. > >> > >> Maybe I was not clear, but I don't mean to be able to switch mode > >> while the port is started. I think it would be better to make it > >> possible to switch mode at application startup time. > > Yes, I understand the problem is the recompiling. But we think the users > will not change it after they made decision. That's why's acceptable in > previous drivers. > > The problem is that the user may not be able to change it, if he does not get > DPDK from source but from a distribution like Debian, Ubuntu or Red Hat. > > In this case, it means the user has no choice than sticking to 32 bytes > descriptors. Normally using 32 bytes is the default behavior and it's good to do that. But I have to say I don't quite understand the scenario. DPDK is open source, whatever OS that users are using, nothing prevents them going to dpdk website to get the code and customize it.
> > > Agree it's better to remove all the compile configuration. Looks like that's > what we're trying to do. We'd like to think about how to optimize it later. > > My suggestion would be a devarg, so that you can have a per-port policy > (which is another advantage of doing so). We're thinking about moving some configuration from per port to per queue. To my opinion, it's also a case that maybe it’s better to make it a queue's parameter. Obviously it’s an API change. So we have to be slow and careful :) > > > > > > >> > >>> > >>>> > >>>> Having it dynamic (as a dev arg for instance) would make it > >>>> possible to change the value when the user is using dpdk from a > >>>> distro. It would also help testing coverage. > >>>> > >>>> Btw, how do you select this option with meson build system? > >>> Not very familiar with meson. As I know, we can change the > >>> meson.build > >> to add the configure. > >>> > >> > >> Ok, then please try to do it, because the legacy build system is > >> going to be deprecated.
