From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 46837A0564; Sat, 7 Mar 2020 00:29:39 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BB0E82BA8; Sat, 7 Mar 2020 00:29:38 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 65F40FEB; Sat, 7 Mar 2020 00:29:35 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Mar 2020 15:29:34 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,523,1574150400"; d="scan'208";a="288102868" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by FMSMGA003.fm.intel.com with ESMTP; 06 Mar 2020 15:29:34 -0800 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 6 Mar 2020 15:29:34 -0800 Received: from shsmsx153.ccr.corp.intel.com (10.239.6.53) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 6 Mar 2020 15:29:33 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.137]) by SHSMSX153.ccr.corp.intel.com ([169.254.12.96]) with mapi id 14.03.0439.000; Sat, 7 Mar 2020 07:29:30 +0800 From: "Zhang, Qi Z" To: "Ye, Xiaolong" CC: "Xing, Beilei" , "dev@dpdk.org" , "stable@dpdk.org" Thread-Topic: [PATCH] net/ice: remove bulk alloc compile option Thread-Index: AQHV8hlcbyE52UjroU2OWaxgIPc2dag6vSIAgAF7l5A= Date: Fri, 6 Mar 2020 23:29:30 +0000 Message-ID: <039ED4275CED7440929022BC67E70611547DB595@SHSMSX103.ccr.corp.intel.com> References: <20200304114038.39644-1-qi.z.zhang@intel.com> <20200306084626.GB18727@intel.com> In-Reply-To: <20200306084626.GB18727@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] net/ice: remove bulk alloc compile option X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Ye, Xiaolong > Sent: Friday, March 6, 2020 4:46 PM > To: Zhang, Qi Z > Cc: Xing, Beilei ; dev@dpdk.org; stable@dpdk.org > Subject: Re: [PATCH] net/ice: remove bulk alloc compile option >=20 > Hi, Qi >=20 > Thanks for the cleanup. >=20 > On 03/04, Qi Zhang wrote: > >Remove CONFIG_RTE_LIBRTE_ICE_RX_ALLOW_BULK_ALLOC with below > >consideration: > > > >1. a default Rx path can always be selected by setting a proper > > rx_free_thresh value at runtime, see > > ice_check_rx_burst_bulk_alloc_preconditions. > > > >2. its not a big deal to always reserve more space for desc ring. > > "ring_size =3D (uint16_t)(rxq->nb_rx_desc + ICE_RX_MAX_BURST);" > > > >3. Fixes a potential invalid memory access in ice_reset_rx_queue. > > if CONFIG_RTE_LIBRTE_ICE_RX_ALLOW_BULK_ALLOC is turned on while > > ice_check_rx_burst_bulk_alloc_preconditions return fail. > > below code will have problem. > > > > for (i =3D 0; i < ICE_RX_MAX_BURST; ++i) > > rxq->sw_ring[rxq->nb_rx_desc + i].mbuf =3D &rxq->fake_mbuf; > > > >Fixes: 50370662b727 ("net/ice: support device and queue ops") > >Cc: stable@dpdk.org > > > >Signed-off-by: Qi Zhang > >--- > > config/common_base | 1 - > > doc/guides/nics/ice.rst | 4 --- > > drivers/net/ice/ice_rxtx.c | 64 > >++++++++++------------------------------------ > > 3 files changed, 13 insertions(+), 56 deletions(-) > > > >diff --git a/config/common_base b/config/common_base index > >7ca2f28b1..c31175f9d 100644 > >--- a/config/common_base > >+++ b/config/common_base > >@@ -337,7 +337,6 @@ CONFIG_RTE_LIBRTE_ICE_PMD=3Dy > >CONFIG_RTE_LIBRTE_ICE_DEBUG_RX=3Dn > CONFIG_RTE_LIBRTE_ICE_DEBUG_TX=3Dn > >CONFIG_RTE_LIBRTE_ICE_DEBUG_TX_FREE=3Dn > >-CONFIG_RTE_LIBRTE_ICE_RX_ALLOW_BULK_ALLOC=3Dy > > CONFIG_RTE_LIBRTE_ICE_16BYTE_RX_DESC=3Dn > > > > # Compile burst-oriented IAVF PMD driver diff --git > >a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst index > >cde3fd620..8af32dabf 100644 > >--- a/doc/guides/nics/ice.rst > >+++ b/doc/guides/nics/ice.rst > >@@ -54,10 +54,6 @@ Please note that enabling debugging options may > affect system performance. > > > > Toggle display of generic debugging messages. > > > >-- ``CONFIG_RTE_LIBRTE_ICE_RX_ALLOW_BULK_ALLOC`` (default ``y``) > >- > >- Toggle bulk allocation for RX. > >- > > - ``CONFIG_RTE_LIBRTE_ICE_16BYTE_RX_DESC`` (default ``n``) > > > > Toggle to use a 16-byte RX descriptor, by default the RX descriptor i= s 32 > byte. > >diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c > >index 60c411bfa..c7e5fc484 100644 > >--- a/drivers/net/ice/ice_rxtx.c > >+++ b/drivers/net/ice/ice_rxtx.c > >@@ -236,17 +236,15 @@ _ice_rx_queue_release_mbufs(struct > ice_rx_queue *rxq) > > rxq->sw_ring[i].mbuf =3D NULL; > > } > > } > >-#ifdef RTE_LIBRTE_ICE_RX_ALLOW_BULK_ALLOC > >- if (rxq->rx_nb_avail =3D=3D 0) > >- return; > >- for (i =3D 0; i < rxq->rx_nb_avail; i++) { > >- struct rte_mbuf *mbuf; > >- > >- mbuf =3D rxq->rx_stage[rxq->rx_next_avail + i]; > >- rte_pktmbuf_free_seg(mbuf); > >- } > >- rxq->rx_nb_avail =3D 0; > >-#endif /* RTE_LIBRTE_ICE_RX_ALLOW_BULK_ALLOC */ > >+ if (rxq->rx_nb_avail =3D=3D 0) > >+ return; > >+ for (i =3D 0; i < rxq->rx_nb_avail; i++) { > >+ struct rte_mbuf *mbuf; > >+ > >+ mbuf =3D rxq->rx_stage[rxq->rx_next_avail + i]; > >+ rte_pktmbuf_free_seg(mbuf); > >+ } >=20 > How about just >=20 > for (i =3D 0; i < rxq->rx_nb_avail; i++) > rte_pktmbuf_free_seg(rxq->rx_stage[rxq->rx_next_avail + i]); I just remove the compile option and keep the code unchanged, but no object= ion for your suggestion.=20 >=20 > [snip] >=20 > For the rest, >=20 > Acked-by: Xiaolong Ye >=20 > And can this cleanup be applied to i40e as well? I think it's good to hav= e less > configurations generally. I40e should be similar with a little bit complexity, I will take chance to = cleanup later. Thanks Qi >=20 > Thanks, > Xiaolong