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

[Bruce Richardson <bruce.richardson@intel.com>]

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.

Regards,
/Bruce