RE: [PATCH 4/5] net/iavf: change no_poll flag to atomic

> Subject: [PATCH 4/5] net/iavf: change no_poll flag to atomic
> 
> The no_poll gate that pauses the Rx/Tx data path during reset and
> link-down was a plain bool written on the control path and read on
> the data-plane lcores without synchronization, allowing stale reads
> that either keep dropping traffic or trigger spurious reset detection.
> 
> Made no_poll an RTE_ATOMIC(bool) and access it with release stores and
> acquire loads so data-plane lcores reliably observe gate changes.
> 
> Fixes: 5b3124a0a6ef ("net/iavf: support no polling when link down")
> Cc: [email protected]
> 
> Signed-off-by: Anurag Mandal <[email protected]>

Minor nit below. Other than that 

Acked-by: Ciara Loftus <[email protected]>

> ---
>  drivers/net/intel/iavf/iavf.h        | 2 +-
>  drivers/net/intel/iavf/iavf_ethdev.c | 6 +++++-
>  drivers/net/intel/iavf/iavf_rxtx.c   | 8 ++++++--
>  drivers/net/intel/iavf/iavf_vchnl.c  | 4 +++-
>  4 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
> index e76c3bb410..037bc8436f 100644
> --- a/drivers/net/intel/iavf/iavf.h
> +++ b/drivers/net/intel/iavf/iavf.h
> @@ -392,7 +392,7 @@ struct iavf_adapter {
>  	alignas(RTE_CACHE_LINE_MIN_SIZE) uint32_t
> ptype_tbl[IAVF_MAX_PKT_TYPE];
>  	bool stopped;
>  	bool closed;
> -	bool no_poll;
> +	RTE_ATOMIC(bool)no_poll;

Add a space before no_poll

>  	enum iavf_rx_func_type rx_func_type;
>  	enum iavf_tx_func_type tx_func_type;
>  	uint16_t fdir_ref_cnt;
> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c
> b/drivers/net/intel/iavf/iavf_ethdev.c
> index 87b826c873..f6ce339b0c 100644
> --- a/drivers/net/intel/iavf/iavf_ethdev.c
> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> @@ -3604,9 +3604,13 @@ void
>  iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change)
>  {
>  	struct iavf_info *vf = &adapter->vf;
> +	bool no_poll;
> 
> -	adapter->no_poll = (link_change & !vf->link_up) ||
> +	no_poll = (link_change & !vf->link_up) ||
>  		vf->vf_reset || vf->in_reset_recovery;
> +
> +	rte_atomic_store_explicit(&adapter->no_poll, no_poll,
> +				  rte_memory_order_release);
>  }
> 
>  static int
> diff --git a/drivers/net/intel/iavf/iavf_rxtx.c
> b/drivers/net/intel/iavf/iavf_rxtx.c
> index 931bb8420d..104197d082 100644
> --- a/drivers/net/intel/iavf/iavf_rxtx.c
> +++ b/drivers/net/intel/iavf/iavf_rxtx.c
> @@ -3723,7 +3723,9 @@ iavf_recv_pkts_no_poll(void *rx_queue, struct
> rte_mbuf **rx_pkts,
>  	struct ci_rx_queue *rxq = rx_queue;
>  	enum iavf_rx_func_type rx_func_type;
> 
> -	if (!rxq->iavf_vsi || rxq->iavf_vsi->adapter->no_poll)
> +	if (!rxq->iavf_vsi ||
> +	    rte_atomic_load_explicit(&rxq->iavf_vsi->adapter->no_poll,
> +				     rte_memory_order_acquire))
>  		return 0;
> 
>  	rx_func_type = rxq->iavf_vsi->adapter->rx_func_type;
> @@ -3739,7 +3741,9 @@ iavf_xmit_pkts_no_poll(void *tx_queue, struct
> rte_mbuf **tx_pkts,
>  	struct ci_tx_queue *txq = tx_queue;
>  	enum iavf_tx_func_type tx_func_type;
> 
> -	if (!txq->iavf_vsi || txq->iavf_vsi->adapter->no_poll)
> +	if (!txq->iavf_vsi ||
> +	    rte_atomic_load_explicit(&txq->iavf_vsi->adapter->no_poll,
> +				     rte_memory_order_acquire))
>  		return 0;
> 
>  	tx_func_type = txq->iavf_vsi->adapter->tx_func_type;
> diff --git a/drivers/net/intel/iavf/iavf_vchnl.c
> b/drivers/net/intel/iavf/iavf_vchnl.c
> index d22990a524..dee76f97cb 100644
> --- a/drivers/net/intel/iavf/iavf_vchnl.c
> +++ b/drivers/net/intel/iavf/iavf_vchnl.c
> @@ -268,7 +268,9 @@ iavf_handle_link_change_event(struct rte_eth_dev
> *dev,
>  	if (adapter->devargs.no_poll_on_link_down) {
>  		iavf_set_no_poll(adapter, true);
>  		PMD_DRV_LOG(DEBUG, "VF no poll turned %s",
> -			    adapter->no_poll ? "on" : "off");
> +			    rte_atomic_load_explicit(&adapter->no_poll,
> +
> rte_memory_order_relaxed) ?
> +						     "on" : "off");
>  		if (!vf->link_up)
>  			iavf_dev_tx_drain(dev);
>  	}
> --
> 2.34.1