Re: [PATCH v3 4/8] net/iavf: rework "async" virtchnl requests

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

On Fri, Mar 06, 2026 at 10:58:20AM +0000, Anatoly Burakov wrote:
> Currently, IPsec crypto requests are called with `async` parameter set to 1
> on account of these requests generating two virtchnl responses instead of
> having just one, like all other requests.
> 
> However, this terminology is misleading, because in actuality *almost all*
> virtchnl requests are asynchronously implemented; that is, almost all of
> them will send a request into virtchnl command queue, and then expect to
> receive a virtchnl response in an interrupt thread, which will update the
> global "pending command" status and write into the global response buffer,
> while the pending request will simply check pending command status on a
> timer. So, for almost all requests, the command status is updated
> asynchronously. The only times this *doesn't* happen is 1) when the
> request is sent from an interrupt thread context, or 2) when the requests
> are sent at init time and the interrupt thread isn't active yet. In both
> of those cases we directly poll the virtchnl queue for responses.
> 
> To make things a little less confusing, remove the usage of "asynchronous"
> terminology across all callsites, and instead make it explicit that what
> we're actually interested in is the number of responses we are waiting for,
> and whether the thread is meant to directly poll the message queue or wait
> for interrupt thread to update command status, not whether the requests
> are "asynchronous". We also make it an implementation detail, not part of
> the API for executing VF commands.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

One minor nit inline below.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>


> ---
>  drivers/net/intel/iavf/iavf.h       |  53 --------
>  drivers/net/intel/iavf/iavf_vchnl.c | 187 ++++++++++++++++++----------
>  2 files changed, 121 insertions(+), 119 deletions(-)
> 
> diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
> index cf98d12247..440376c4ca 100644
> --- a/drivers/net/intel/iavf/iavf.h
> +++ b/drivers/net/intel/iavf/iavf.h
> @@ -432,59 +432,6 @@ struct iavf_cmd_info {
>  	uint32_t out_size;      /* buffer size for response */
>  };
>  
> -/* notify current command done. Only call in case execute
> - * _atomic_set_cmd successfully.
> - */
> -static inline void
> -_notify_cmd(struct iavf_info *vf, int msg_ret)
> -{
> -	vf->cmd_retval = msg_ret;
> -	rte_wmb();
> -	vf->pend_cmd = VIRTCHNL_OP_UNKNOWN;
> -}
> -
> -/* clear current command. Only call in case execute
> - * _atomic_set_cmd successfully.
> - */
> -static inline void
> -_clear_cmd(struct iavf_info *vf)
> -{
> -	rte_wmb();
> -	vf->pend_cmd = VIRTCHNL_OP_UNKNOWN;
> -	vf->cmd_retval = VIRTCHNL_STATUS_SUCCESS;
> -}
> -
> -/* Check there is pending cmd in execution. If none, set new command. */
> -static inline int
> -_atomic_set_cmd(struct iavf_info *vf, enum virtchnl_ops ops)
> -{
> -	enum virtchnl_ops op_unk = VIRTCHNL_OP_UNKNOWN;
> -	int ret = rte_atomic_compare_exchange_strong_explicit(&vf->pend_cmd, &op_unk, ops,
> -			rte_memory_order_acquire, rte_memory_order_acquire);
> -
> -	if (!ret)
> -		PMD_DRV_LOG(ERR, "There is incomplete cmd %d", vf->pend_cmd);
> -
> -	rte_atomic_store_explicit(&vf->pend_cmd_count, 1, rte_memory_order_relaxed);
> -
> -	return !ret;
> -}
> -
> -/* Check there is pending cmd in execution. If none, set new command. */
> -static inline int
> -_atomic_set_async_response_cmd(struct iavf_info *vf, enum virtchnl_ops ops)
> -{
> -	enum virtchnl_ops op_unk = VIRTCHNL_OP_UNKNOWN;
> -	int ret = rte_atomic_compare_exchange_strong_explicit(&vf->pend_cmd, &op_unk, ops,
> -			rte_memory_order_acquire, rte_memory_order_acquire);
> -
> -	if (!ret)
> -		PMD_DRV_LOG(ERR, "There is incomplete cmd %d", vf->pend_cmd);
> -
> -	rte_atomic_store_explicit(&vf->pend_cmd_count, 2, rte_memory_order_relaxed);
> -
> -	return !ret;
> -}
>  int iavf_check_api_version(struct iavf_adapter *adapter);
>  int iavf_get_vf_resource(struct iavf_adapter *adapter);
>  void iavf_dev_event_post(struct rte_eth_dev *dev,
> diff --git a/drivers/net/intel/iavf/iavf_vchnl.c b/drivers/net/intel/iavf/iavf_vchnl.c
> index d97bdf0dc1..d240745f5c 100644
> --- a/drivers/net/intel/iavf/iavf_vchnl.c
> +++ b/drivers/net/intel/iavf/iavf_vchnl.c
> @@ -309,42 +309,97 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
>  }
>  
>  static int
> -iavf_execute_vf_cmd(struct iavf_adapter *adapter, struct iavf_cmd_info *args,
> -	int async)
> +iavf_set_pending_cmd(struct iavf_info *vf, enum virtchnl_ops ops,
> +		    uint32_t resp_count)
> +{
> +	enum virtchnl_ops op_unk = VIRTCHNL_OP_UNKNOWN;
> +	int ret = rte_atomic_compare_exchange_strong_explicit(&vf->pend_cmd,
> +			&op_unk, ops, rte_memory_order_acquire,
> +			rte_memory_order_acquire);
> +
> +	if (ret == 0) {
> +		PMD_DRV_LOG(ERR, "There is incomplete cmd %d", vf->pend_cmd);
> +		return -1;
> +	}
> +
> +	rte_atomic_store_explicit(&vf->pend_cmd_count, resp_count,
> +			rte_memory_order_relaxed);
> +
> +	return 0;
> +}
> +
> +static inline void
> +iavf_clear_pending_cmd(struct iavf_info *vf)
> +{
> +	rte_wmb();
> +	vf->pend_cmd = VIRTCHNL_OP_UNKNOWN;
> +	vf->cmd_retval = VIRTCHNL_STATUS_SUCCESS;
> +}
> +
> +static inline void
> +iavf_notify_pending_cmd(struct iavf_info *vf, int msg_ret)
> +{
> +	vf->cmd_retval = msg_ret;
> +	rte_wmb();
> +	vf->pend_cmd = VIRTCHNL_OP_UNKNOWN;
> +}
> +
> +static int
> +iavf_get_cmd_resp_count(enum virtchnl_ops op)
> +{
> +	switch (op) {
> +	case VIRTCHNL_OP_RESET_VF:
> +	case VIRTCHNL_OP_REQUEST_QUEUES:
> +		/* These commands trigger reset and are not waited for */
> +		return 0;
> +	case VIRTCHNL_OP_INLINE_IPSEC_CRYPTO:
> +		/* IPsec crypto commands generate two responses */
> +		return 2;
> +	default:
> +		/* All other commands generate one response */
> +		return 1;
> +	}
> +}
> +
> +static int
> +iavf_execute_vf_cmd(struct iavf_adapter *adapter, struct iavf_cmd_info *args)
>  {
>  	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(adapter);
>  	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
>  	enum iavf_aq_result result;
>  	enum iavf_status ret;
> +	uint32_t resp_count;
>  	int err = 0;
>  	int i = 0;
>  
>  	if (vf->vf_reset)
>  		return -EIO;
>  
> +	resp_count = iavf_get_cmd_resp_count(args->ops);
>  
> -	if (async) {
> -		if (_atomic_set_async_response_cmd(vf, args->ops))
> -			return -1;
> -	} else {
> -		if (_atomic_set_cmd(vf, args->ops))
> -			return -1;
> -	}
> +	/*
> +	 * For some commands, we are not waiting for responses because they
> +	 * produce a reset event. However, we still need to set pending command
> +	 * to avoid sending commands while another one is already in progress.
> +	 */
> +	if (iavf_set_pending_cmd(vf, args->ops, RTE_MAX(1U, resp_count)))

Nit: according to coding style this should be explicitly "!= 0" 

> +		return -1;
>  
<snip>