Re: [dpdk-dev] [PATCH] [new]ixgbe:set txep.mbuf to NULL when calling ixgbe_tx_free_bufs

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

On Sat, Aug 01, 2015 at 09:26:34AM +0800, hepeng wrote:
> In *ixgbe_tx_free_bufs*, after recycling some tx entries, one should set their mbuf pointers to NULL.
> 
> The first path is not correct, the txep->mbuf should be set to NULL no matter if it is recycled into mempool
> Signed-off-by: hepeng <xnhp0320@icloud.com>

Why is this necessary? Setting the mbuf pointer to null introduces an extra write
to the descriptor ring for every packet. Right now the free entries are simply
tracking using the indexes in the tx_queue structure, there is no need to set
the mbufs to NULL as well, as far as I can see.

/Bruce

> ---
>  drivers/net/ixgbe/ixgbe_rxtx_vec.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> index 1c16dec..e7ce740 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> @@ -612,6 +612,7 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
>  	 */
>  	txep = &txq->sw_ring_v[txq->tx_next_dd - (n - 1)];
>  	m = __rte_pktmbuf_prefree_seg(txep[0].mbuf);
> +    txep[0].mbuf = NULL;
>  	if (likely(m != NULL)) {
>  		free[0] = m;
>  		nb_free = 1;
> @@ -632,11 +633,21 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
>  	} else {
>  		for (i = 1; i < n; i++) {
>  			m = __rte_pktmbuf_prefree_seg(txep[i].mbuf);
> -			if (m != NULL)
> +			if (m != NULL) {
>  				rte_mempool_put(m->pool, m);
> +            }
>  		}
>  	}
>  
> +    /*
> +     * No matter the mbufs have been put back to mempool or not,
> +     * we should set the txep[i].mbuf to NULL
> +     */
> +
> +    for( i = 1; i < n; i++) {
> +        txep[i].mbuf = NULL;
> +    }
> +
>  	/* buffers were freed, update counters */
>  	txq->nb_tx_free = (uint16_t)(txq->nb_tx_free + txq->tx_rs_thresh);
>  	txq->tx_next_dd = (uint16_t)(txq->tx_next_dd + txq->tx_rs_thresh);
> -- 
> 1.9.1
>