From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 9C3C3530A for ; Tue, 5 Jul 2016 11:43:49 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP; 05 Jul 2016 02:43:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,579,1459839600"; d="scan'208";a="1001142598" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.220.53]) by fmsmga001.fm.intel.com with SMTP; 05 Jul 2016 02:43:43 -0700 Received: by (sSMTP sendmail emulation); Tue, 05 Jul 2016 10:43:42 +0025 Date: Tue, 5 Jul 2016 10:43:42 +0100 From: Bruce Richardson To: Wang Xiao W Cc: jing.d.chen@intel.com, dev@dpdk.org Message-ID: <20160705094342.GD10232@bricha3-MOBL3> References: <1467618668-28611-1-git-send-email-xiao.w.wang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1467618668-28611-1-git-send-email-xiao.w.wang@intel.com> Organization: Intel Research and =?iso-8859-1?Q?De=ACvel?= =?iso-8859-1?Q?opment?= Ireland Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] net/fm10k: fix Rx descriptor read timing 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: Tue, 05 Jul 2016 09:43:50 -0000 On Mon, Jul 04, 2016 at 03:51:08PM +0800, Wang Xiao W wrote: > We find that when traffic is light, a few amount of packets will be > wrongly parsed (e.g. packet type), however this issue will not happen > when traffic is heavy. > > The root cause is some fields in fm10k_rx_desc are read at wrong timing. > When the input speed is slower than software's capability, fm10k scalar > Rx function accesses descriptors closely to HW, so there's potential > sequence: some fields like pkt_info in fm10k_rx_desc are read before HW > writeback but some fields like DD bit are read after HW writeback, this > will lead to the later packet parsing function using incorrect value. > > This patch fixes this issue by reading and parsing Rx descriptor after > DD bit is set. > > Fixes: 4b61d3bfa941 ("fm10k: add receive and tranmit") > Fixes: c82dd0a7bfa5 ("fm10k: add scatter receive") > > Signed-off-by: Wang Xiao W Fix looks correct to me. The key part I think is that the read of the descriptor at line "desc = q->hw_ring[next_dd];" is not atomic as it's a read of 16B, allowing part of the descriptor to be read either side of a HW write-back. Acked-by: Bruce Richardson > --- > drivers/net/fm10k/fm10k_rxtx.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c > index dd92a91..5b2d04b 100644 > --- a/drivers/net/fm10k/fm10k_rxtx.c > +++ b/drivers/net/fm10k/fm10k_rxtx.c > @@ -114,10 +114,10 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, > > nb_pkts = RTE_MIN(nb_pkts, q->alloc_thresh); > for (count = 0; count < nb_pkts; ++count) { > + if (!(q->hw_ring[next_dd].d.staterr & FM10K_RXD_STATUS_DD)) > + break; > mbuf = q->sw_ring[next_dd]; > desc = q->hw_ring[next_dd]; > - if (!(desc.d.staterr & FM10K_RXD_STATUS_DD)) > - break; > #ifdef RTE_LIBRTE_FM10K_DEBUG_RX > dump_rxd(&desc); > #endif > @@ -228,10 +228,10 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, > > nb_seg = RTE_MIN(nb_pkts, q->alloc_thresh); > for (count = 0; count < nb_seg; count++) { > + if (!(q->hw_ring[next_dd].d.staterr & FM10K_RXD_STATUS_DD)) > + break; > mbuf = q->sw_ring[next_dd]; > desc = q->hw_ring[next_dd]; > - if (!(desc.d.staterr & FM10K_RXD_STATUS_DD)) > - break; > #ifdef RTE_LIBRTE_FM10K_DEBUG_RX > dump_rxd(&desc); > #endif > -- > 1.9.3 >