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 <[email protected]>
One minor nit inline below. Acked-by: Bruce Richardson <[email protected]> > --- > 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>
