Re: [dpdk-dev] [PATCH] net/i40e: fix unchecked return value

["Xing, Beilei" <beilei.xing@intel.com>]

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Tuesday, February 11, 2020 7:42 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix unchecked return value
> 
> 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 <beilei.xing@intel.com>
> > ---
> >  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.

Yes, agree. Will abandon the patch. Thanks.

> 
> Regards,
> /Bruce