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 9F516A051C; Tue, 11 Feb 2020 12:42:27 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 824AA1C06D; Tue, 11 Feb 2020 12:42:27 +0100 (CET) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 8748F1C05C for ; Tue, 11 Feb 2020 12:42:25 +0100 (CET) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Feb 2020 03:42:24 -0800 X-IronPort-AV: E=Sophos;i="5.70,428,1574150400"; d="scan'208";a="221906895" Received: from bricha3-mobl.ger.corp.intel.com ([10.237.221.79]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 11 Feb 2020 03:42:22 -0800 Date: Tue, 11 Feb 2020 11:42:19 +0000 From: Bruce Richardson To: Beilei Xing Cc: dev@dpdk.org, qi.z.zhang@intel.com Message-ID: <20200211114219.GD823@bricha3-MOBL.ger.corp.intel.com> References: <1581447720-29099-1-git-send-email-beilei.xing@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1581447720-29099-1-git-send-email-beilei.xing@intel.com> User-Agent: Mutt/1.12.1 (2019-06-15) Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix unchecked return value 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 Wed, Feb 12, 2020 at 03:02:00AM +0800, Beilei Xing wrote: > Check the return value of the i40e_xmit_cleanup function. > > Coverity issue: 353617 > Fixes: 4861cde46116 ("i40e: new poll mode driver") > > Signed-off-by: Beilei Xing > --- > drivers/net/i40e/i40e_rxtx.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c > index fd1ae80..f43fc0f 100644 > --- a/drivers/net/i40e/i40e_rxtx.c > +++ b/drivers/net/i40e/i40e_rxtx.c > @@ -1038,8 +1038,9 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) > txe = &sw_ring[tx_id]; > > /* Check if the descriptor ring needs to be cleaned. */ > - if (txq->nb_tx_free < txq->tx_free_thresh) > - i40e_xmit_cleanup(txq); > + if ((txq->nb_tx_free < txq->tx_free_thresh) && > + (i40e_xmit_cleanup(txq) != 0)) > + return 0; > I don't think this should be fixed, and the original code is correct. This cleanup is opportunistic and may not cause problems if it fails. For example, if tx_free_thresh is 32, and nb_tx_free is 24, there is no reason to return zero here if the total packets to be sent it 16 - since all packets can feasibly fit. Even if we had 32 to transmit, we still should not quit here, since any packets that can be transmitted should be sent, and there is a subsequent cleanup call at line 1084 to handle failed cleanup when it does become a problem. Regards, /Bruce