From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id CA7D6CF9 for ; Mon, 19 Oct 2015 20:57:17 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP; 19 Oct 2015 11:57:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,703,1437462000"; d="scan'208";a="797186337" Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by orsmga001.jf.intel.com with ESMTP; 19 Oct 2015 11:57:12 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.75]) by IRSMSX104.ger.corp.intel.com ([169.254.5.150]) with mapi id 14.03.0248.002; Mon, 19 Oct 2015 19:57:12 +0100 From: "Ananyev, Konstantin" To: Zoltan Kiss , "Richardson, Bruce" , "dev@dpdk.org" Thread-Topic: [PATCH] ixgbe: prefetch packet headers in vector PMD receive function Thread-Index: AQHQ5OrcDMYVDKIQM0OQ/IGpqZ+dT54w9hQAgAAIuYCAABXhgIAAByoAgByJi4CAA4aAcIAaL0SAgAA70yCAAPgTAIAAZcdAgAZHagCAADk/QA== Date: Mon, 19 Oct 2015 18:57:11 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836AB17F4@irsmsx105.ger.corp.intel.com> References: <1441135036-7491-1-git-send-email-zoltan.kiss@linaro.org> <55ED8252.1020900@linaro.org> <59AF69C657FD0841A61C55336867B5B0359227DF@IRSMSX103.ger.corp.intel.com> <55ED9BFD.7040009@linaro.org> <59AF69C657FD0841A61C55336867B5B035922A83@IRSMSX103.ger.corp.intel.com> <56059255.5010507@linaro.org> <2601191342CEEE43887BDE71AB97725836A9CE89@irsmsx105.ger.corp.intel.com> <561E7E83.5000401@linaro.org> <2601191342CEEE43887BDE71AB97725836AAFBAB@irsmsx105.ger.corp.intel.com> <561F80CC.4000809@linaro.org> <2601191342CEEE43887BDE71AB97725836AB00DE@irsmsx105.ger.corp.intel.com> <56251A92.70608@linaro.org> In-Reply-To: <56251A92.70608@linaro.org> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] ixgbe: prefetch packet headers in vector PMD receive function X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Oct 2015 18:57:18 -0000 > -----Original Message----- > From: Zoltan Kiss [mailto:zoltan.kiss@linaro.org] > Sent: Monday, October 19, 2015 5:30 PM > To: Ananyev, Konstantin; Richardson, Bruce; dev@dpdk.org > Subject: Re: [PATCH] ixgbe: prefetch packet headers in vector PMD receive= function >=20 >=20 >=20 > On 15/10/15 16:43, Ananyev, Konstantin wrote: > > > > > >> -----Original Message----- > >> From: Zoltan Kiss [mailto:zoltan.kiss@linaro.org] > >> Sent: Thursday, October 15, 2015 11:33 AM > >> To: Ananyev, Konstantin; Richardson, Bruce; dev@dpdk.org > >> Subject: Re: [PATCH] ixgbe: prefetch packet headers in vector PMD rece= ive function > >> > >> > >> > >> On 15/10/15 00:23, Ananyev, Konstantin wrote: > >>> > >>> > >>>> -----Original Message----- > >>>> From: Zoltan Kiss [mailto:zoltan.kiss@linaro.org] > >>>> Sent: Wednesday, October 14, 2015 5:11 PM > >>>> To: Ananyev, Konstantin; Richardson, Bruce; dev@dpdk.org > >>>> Subject: Re: [PATCH] ixgbe: prefetch packet headers in vector PMD re= ceive function > >>>> > >>>> > >>>> > >>>> On 28/09/15 00:19, Ananyev, Konstantin wrote: > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Zoltan Kiss [mailto:zoltan.kiss@linaro.org] > >>>>>> Sent: Friday, September 25, 2015 7:29 PM > >>>>>> To: Richardson, Bruce; dev@dpdk.org > >>>>>> Cc: Ananyev, Konstantin > >>>>>> Subject: Re: [PATCH] ixgbe: prefetch packet headers in vector PMD = receive function > >>>>>> > >>>>>> On 07/09/15 07:41, Richardson, Bruce wrote: > >>>>>>> > >>>>>>>> -----Original Message----- > >>>>>>>> From: Zoltan Kiss [mailto:zoltan.kiss@linaro.org] > >>>>>>>> Sent: Monday, September 7, 2015 3:15 PM > >>>>>>>> To: Richardson, Bruce; dev@dpdk.org > >>>>>>>> Cc: Ananyev, Konstantin > >>>>>>>> Subject: Re: [PATCH] ixgbe: prefetch packet headers in vector PM= D receive > >>>>>>>> function > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> On 07/09/15 13:57, Richardson, Bruce wrote: > >>>>>>>>> > >>>>>>>>>> -----Original Message----- > >>>>>>>>>> From: Zoltan Kiss [mailto:zoltan.kiss@linaro.org] > >>>>>>>>>> Sent: Monday, September 7, 2015 1:26 PM > >>>>>>>>>> To: dev@dpdk.org > >>>>>>>>>> Cc: Ananyev, Konstantin; Richardson, Bruce > >>>>>>>>>> Subject: Re: [PATCH] ixgbe: prefetch packet headers in vector = PMD > >>>>>>>>>> receive function > >>>>>>>>>> > >>>>>>>>>> Hi, > >>>>>>>>>> > >>>>>>>>>> I just realized I've missed the "[PATCH]" tag from the subject= . Did > >>>>>>>>>> anyone had time to review this? > >>>>>>>>>> > >>>>>>>>> Hi Zoltan, > >>>>>>>>> > >>>>>>>>> the big thing that concerns me with this is the addition of new > >>>>>>>>> instructions for each packet in the fast path. Ideally, this > >>>>>>>>> prefetching would be better handled in the application itself, = as for > >>>>>>>>> some apps, e.g. those using pipelining, the core doing the RX f= rom the > >>>>>>>>> NIC may not touch the packet data at all, and the prefetches wi= ll > >>>>>>>> instead cause a performance slowdown. > >>>>>>>>> Is it possible to get the same performance increase - or someth= ing > >>>>>>>>> close to it - by making changes in OVS? > >>>>>>>> OVS already does a prefetch when it's processing the previous pa= cket, but > >>>>>>>> apparently it's not early enough. At least for my test scenario,= where I'm > >>>>>>>> forwarding UDP packets with the least possible overhead. I guess= in tests > >>>>>>>> where OVS does more complex processing it should be fine. > >>>>>>>> I'll try to move the prefetch earlier in OVS codebase, but I'm n= ot sure if > >>>>>>>> it'll help. > >>>>>>> I would suggest trying to prefetch more than one packet ahead. Pr= efetching 4 or > >>>>>>> 8 ahead might work better, depending on the processing being done= . > >>>>>> > >>>>>> I've moved the prefetch earlier, and it seems to work: > >>>>>> > >>>>>> https://patchwork.ozlabs.org/patch/519017/ > >>>>>> > >>>>>> However it raises the question: should we remove header prefetch f= rom > >>>>>> all the other drivers, and the CONFIG_RTE_PMD_PACKET_PREFETCH opti= on? > >>>>> > >>>>> My vote would be for that. > >>>>> Konstantin > >>>> > >>>> After some further thinking I would rather support the > >>>> rte_packet_prefetch() macro (prefetch the header in the driver, and > >>>> configure it through CONFIG_RTE_PMD_PACKET_PREFETCH) > >>>> > >>>> - the prefetch can happen earlier, so if an application needs the he= ader > >>>> right away, this is the fastest > >>>> - if the application doesn't need header prefetch, it can turn it of= f > >>>> compile time. Same as if we wouldn't have this option. > >>>> - if the application has mixed needs (sometimes it needs the header > >>>> right away, sometimes it doesn't), it can turn it off and do what it > >>>> needs. Same as if we wouldn't have this option. > >>>> > >>>> A harder decision would be whether it should be turned on or off by > >>>> default. Currently it's on, and half of the receive functions don't = use it. > >>> > >>> Yep, it is sort of a mess right now. > >>> Another question if we'd like to keep it and standardise it: > >>> at what moment to prefetch: as soon as we realize that HW is done wit= h that buffer, > >>> or as late inside rx_burst() as possible? > >>> I suppose there is no clear answer for that. > >> I think if the application needs the driver start the prefetching, it > >> does it because it's already too late when rte_eth_rx_burst() returns. > >> So I think it's best to do it as soon as we learn that the HW is done. > > > > Probably, but it might be situations when it would be more plausible to= do it later too. > Could you elaborate? > If the application wants prefetch to start earlier than could be done by > itself (right after rte_eth_rx_burst() returns), earlier is better. So > it will have the best chances to have the data in cache. > If it would need the data later, then it could do the prefetch by itself. >=20 > > Again it might depend on particular HW and your memory access pattern. > > > >> > >>> That's why my thought was to just get rid of it. > >>> Though if it would be implemented in some standardized way and disabl= ed by default - > >>> that's probably ok too. > >>> BTW, couldn't that be implemented just via rte_ethdev rx callbacks me= chanism? > >>> Then we can have the code one place and don't need compile option at = all - > >>> could be ebabled/disabled dynamically on a per nic basis. > >>> Or would it be too high overhead for that? > >> > >> I think if we go that way, it's better to pass an option to > >> rte_eth_rx_burst() and tell if you want the header prefetched by the > >> driver. That would be the most flexible. > > > > That would mean ABI breakage for rx_burst()... > > Probably then better a new parameter in rx_conf or something. > > Though it still means that each PMD has to implement it on its own. > That would be the case in any way, as we are are talking about > prefetching in the receive function. >=20 > > And again there might be PMDs that would just ignore that parameter. > If the PMD has a good reason to do that (e.g. prefetch has undesirable > effects), I think it's fine. >=20 > > While for callback it would be all in one and known place. > Who and when would call that callback? If the application after > rte_eth_rx_burst() returned, it wouldn't have too much use, it could > just call prefetch by itself. I am talking about the callbacks called by rx_burst() itself: static inline uint16_t rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id, struct rte_mbuf **rx_pkts, const uint16_t nb_pkts) { struct rte_eth_dev *dev; dev =3D &rte_eth_devices[port_id]; int16_t nb_rx =3D (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_i= d], rx_pkts, nb_pkts); #ifdef RTE_ETHDEV_RXTX_CALLBACKS struct rte_eth_rxtx_callback *cb =3D dev->post_rx_burst_cbs[queue_i= d]; if (unlikely(cb !=3D NULL)) { do { nb_rx =3D cb->fn.rx(port_id, queue_id, rx_pkts, nb_= rx, nb_pkts, cb->param); cb =3D cb->next; } while (cb !=3D NULL); #endif return nb_rx; } Konstantin >=20 > > But again, I didn't measure it, so not sure what will be an impact of t= he callback itself. > > Might be it not worth it. > > Konstantin > > > >> > >>> Konstantin > >>> > >>> > >>> > >>>> > >>>>> > >>>>> > >>>>>> > >>>>>>> > >>>>>>> /Bruce > >>>>>