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 96183A04BA; Thu, 1 Oct 2020 17:15:13 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E885A1DAEA; Thu, 1 Oct 2020 17:15:11 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id EFA171DAE9 for ; Thu, 1 Oct 2020 17:15:09 +0200 (CEST) IronPort-SDR: gmBJP1g11HUMgb1iVm+cvBd8U++YUSouqhBnv9ztiCwzOydPKrpkcJOoNvIhJocjCWS0oNZ/+L 5ZYkT+nuyTmw== X-IronPort-AV: E=McAfee;i="6000,8403,9761"; a="162835022" X-IronPort-AV: E=Sophos;i="5.77,323,1596524400"; d="scan'208";a="162835022" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Oct 2020 08:15:03 -0700 IronPort-SDR: 3rzZhrNtr2gX/GOwvx8F0jIqoWqnvRD69pKZNeCWGw79WajMsijLoPjuMltsLz2B9dxikgdGMi x41kiiv+03QA== X-IronPort-AV: E=Sophos;i="5.77,323,1596524400"; d="scan'208";a="504067753" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.253.63]) ([10.213.253.63]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Oct 2020 08:15:02 -0700 To: Jerin Jacob , "Ananyev, Konstantin" Cc: ZY Qiu , Thomas Monjalon , Andrew Rybchenko , "dev@dpdk.org" , "stephen@networkplumber.org" , ZY Qiu References: <20200304173349.26459-1-tgw_team@tencent.com> <20200305164704.29900-1-tgw_team@tencent.com> From: Ferruh Yigit Message-ID: <290cc5e4-eed8-39ba-32ac-6290673ef4cf@intel.com> Date: Thu, 1 Oct 2020 16:14:58 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3] rte_ethdev: safer memory access by calling Rx/Tx callback 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" On 3/11/2020 12:26 PM, Jerin Jacob wrote: > On Wed, Mar 11, 2020 at 5:52 PM Ananyev, Konstantin > wrote: >> >> >> >>> -----Original Message----- >>> From: ZY Qiu >>> Sent: Thursday, March 5, 2020 4:47 PM >>> To: Thomas Monjalon ; Yigit, Ferruh ; Andrew Rybchenko >>> Cc: dev@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin ; ZY Qiu >>> >>> Subject: [PATCH v3] rte_ethdev: safer memory access by calling Rx/Tx callback >>> >>> When compiling with -O0, >>> the compiler does not optimize two memory accesses into one. >>> Leads to accessing a null pointer when queue post Rx burst callback >>> removal while traffic is running. >>> >>> Signed-off-by: ZY Qiu >>> --- >>> lib/librte_ethdev/rte_ethdev.h | 9 +++++---- >>> 1 file changed, 5 insertions(+), 4 deletions(-) >>> >>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h >>> index d1a593ad1..c46612e3e 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.h >>> +++ b/lib/librte_ethdev/rte_ethdev.h >>> @@ -4388,10 +4388,10 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id, >>> rx_pkts, nb_pkts); >>> >>> #ifdef RTE_ETHDEV_RXTX_CALLBACKS >>> - if (unlikely(dev->post_rx_burst_cbs[queue_id] != NULL)) { >>> - struct rte_eth_rxtx_callback *cb = >>> - dev->post_rx_burst_cbs[queue_id]; >>> + struct rte_eth_rxtx_callback *volatile cb = >>> + dev->post_rx_burst_cbs[queue_id]; > > I prefer to change to compiler_barrier() if it working as volatile may have a > performance impact on fast-path code. > Hi ZY Qiu, Did you able to test with compiler barrier, is it solving your problem? If there is no update on the issue, I will mark it as rejected. Thanks, ferruh > >>> >>> + if (unlikely(cb != NULL)) { >>> do { >>> nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx, >>> nb_pkts, cb->param); >>> @@ -4652,7 +4652,8 @@ rte_eth_tx_burst(uint16_t port_id, uint16_t queue_id, >>> #endif >>> >>> #ifdef RTE_ETHDEV_RXTX_CALLBACKS >>> - struct rte_eth_rxtx_callback *cb = dev->pre_tx_burst_cbs[queue_id]; >>> + struct rte_eth_rxtx_callback *volatile cb = >>> + dev->pre_tx_burst_cbs[queue_id]; >>> >>> if (unlikely(cb != NULL)) { >>> do { >>> -- >> >> Acked-by: Konstantin Ananyev >> >>> 2.17.1 >>