From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 17D495949 for ; Wed, 22 Jul 2015 15:35:45 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga103.fm.intel.com with ESMTP; 22 Jul 2015 06:35:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,523,1432623600"; d="scan'208";a="733418966" Received: from irsmsx105.ger.corp.intel.com ([163.33.3.28]) by orsmga001.jf.intel.com with ESMTP; 22 Jul 2015 06:35:44 -0700 Received: from irsmsx103.ger.corp.intel.com ([169.254.3.216]) by irsmsx105.ger.corp.intel.com ([169.254.7.245]) with mapi id 14.03.0224.002; Wed, 22 Jul 2015 14:35:43 +0100 From: "Richardson, Bruce" To: Zoltan Kiss Thread-Topic: [dpdk-dev] [PATCH v2] ixgbe: fix check for split packets Thread-Index: AQHQxF69TL4oJ8lDzE+VhuFxS+bK1J3nLV0AgAAULACAACchgIAAFKaw Date: Wed, 22 Jul 2015 13:35:41 +0000 Message-ID: <59AF69C657FD0841A61C55336867B5B03457D2BE@IRSMSX103.ger.corp.intel.com> References: <1437492307-29754-1-git-send-email-bruce.richardson@intel.com> <1437556410-14102-1-git-send-email-bruce.richardson@intel.com> <55AF66B6.1000305@linaro.org> <20150722095946.GB11652@bricha3-MOBL3> <55AF9875.7010001@linaro.org> In-Reply-To: <55AF9875.7010001@linaro.org> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2] ixgbe: fix check for split packets 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: Wed, 22 Jul 2015 13:35:46 -0000 > -----Original Message----- > From: Zoltan Kiss [mailto:zoltan.kiss@linaro.org] > Sent: Wednesday, July 22, 2015 2:20 PM > To: Richardson, Bruce > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2] ixgbe: fix check for split packets >=20 >=20 >=20 > On 22/07/15 10:59, Bruce Richardson wrote: > > On Wed, Jul 22, 2015 at 10:47:34AM +0100, Zoltan Kiss wrote: > >> Hi, > >> > >> And what happens if someone changes RTE_IXGBE_VPMD_RX_BURST to > >> something else than 32? I guess this bug were introduced when someone > >> raised it from > >> 16 to 32 > > > > Actually, no, this bug is purely due to me getting my maths wrong when > > I wrote this function. The vector PMD has always worked in bursts of > > 32 at a time. > > > >> I think you are better off with a for loop which uses this value. Or > >> at least make a comment around RTE_IXGBE_VPMD_RX_BURST that if you > >> change that value this check should be modified as well. > > > > The vector PMD always works off a fixed 32 burst size. Any change to > > that will lead to many changes in the code, so I don't believe a loop i= s > necessary. >=20 > Ok, then I suggest to make a comment around RTE_IXGBE_VPMD_RX_BURST that > changing it needs a lot of other changes in the code elsewhere, e.g in > this split_flags check. > Btw. vPMD was a bit misleading abbreviation for me, it took me a while > until I realized 'v' stands for 'vector', not 'virtualization' as in most > cases nowadays. Good idea. I'll try to submit a patch to add a comment if I get the chance = - otherwise feel free to do so yourself. As for the naming, yes, I suppose it can be confusing. :-) We possibly need= to start calling these pieces of code SSE or AVX rather than just vector, = since for some we may end up with multiple vector versions. /Bruce >=20 > > > > Regards, > > /Bruce > > > >> > >> Regards, > >> > >> Zoltan > >> > >> On 22/07/15 10:13, Bruce Richardson wrote: > >>> The check for split packets to be reassembled in the vector ixgbe > >>> PMD was incorrectly only checking the first 16 elements of the array > >>> instead of all 32. This is fixed by changing the uint32_t values to > >>> be uint64_t instead. > >>> > >>> Fixes: cf4b4708a88a ("ixgbe: improve slow-path perf with vector > >>> scattered Rx") > >>> > >>> Reported-by: Zoltan Kiss > >>> Signed-off-by: Bruce Richardson > >>> > >>> --- > >>> V2: Rename variable from split_fl32 to split_fl64 to match type > change. > >>> --- > >>> drivers/net/ixgbe/ixgbe_rxtx_vec.c | 6 +++--- > >>> 1 file changed, 3 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c > >>> b/drivers/net/ixgbe/ixgbe_rxtx_vec.c > >>> index d3ac74a..f2052c6 100644 > >>> --- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c > >>> +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c > >>> @@ -549,10 +549,10 @@ ixgbe_recv_scattered_pkts_vec(void *rx_queue, > struct rte_mbuf **rx_pkts, > >>> return 0; > >>> > >>> /* happy day case, full burst + no packets to be joined */ > >>> - const uint32_t *split_fl32 =3D (uint32_t *)split_flags; > >>> + const uint64_t *split_fl64 =3D (uint64_t *)split_flags; > >>> if (rxq->pkt_first_seg =3D=3D NULL && > >>> - split_fl32[0] =3D=3D 0 && split_fl32[1] =3D=3D 0 && > >>> - split_fl32[2] =3D=3D 0 && split_fl32[3] =3D=3D 0) > >>> + split_fl64[0] =3D=3D 0 && split_fl64[1] =3D=3D 0 && > >>> + split_fl64[2] =3D=3D 0 && split_fl64[3] =3D=3D 0) > >>> return nb_bufs; > >>> > >>> /* reassemble any packets that need reassembly*/ > >>>