[PATCH 22.11] Revert "net/iavf: fix abnormal disable HW interrupt"
Kevin Traynor
ktraynor at redhat.com
Mon Jan 15 11:02:43 CET 2024
On 14/01/2024 23:47, Chris Brezovec (cbrezove) wrote:
> Kevin,
>
> I have a question on this issue. The commit you are looking to back out was committed into v22.11.3. Does that mean that the issue exists in v22.11.3? Or were there some other commits between v22.11.3 and v22.11.4 that exposed this issue, or made it more likely to hit?
>
It was reported by our validation team when OVS updated from
22.11.1->22.11.3.
thanks,
Kevin.
> Thanks!
> -Chris
>
> From: Kevin Traynor <ktraynor at redhat.com>
> Date: Friday, January 12, 2024 at 6:21 AM
> To: stable at dpdk.org <stable at dpdk.org>, xuemingl at nvidia.com <xuemingl at nvidia.com>
> Cc: Kevin Traynor <ktraynor at redhat.com>
> Subject: [PATCH 22.11] Revert "net/iavf: fix abnormal disable HW interrupt"
> This reverts commit 2bdeeb53da0dda4e36a022553d3403c1217d677e.
>
> The change in behaviour [0] for handling some admin queue messages
> from polling to interrupt mode has changed the behaviour and is leading
> to a deadlock with OVS.
>
> [0]
> commit 2bdeeb53da0dda4e36a022553d3403c1217d677e
> Author: Mingjin Ye <mingjinx.ye at intel.com>
> Date: Wed Jun 14 09:53:03 2023 +0000
>
> net/iavf: fix abnormal disable HW interrupt
>
> [ upstream commit 675a104e2e940ec476e8b469725e8465d01c0098 ]
>
> Bugzilla ID: 1337
> Reported-by: Kevin Traynor <ktraynor at redhat.com>
> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
> ---
> drivers/net/iavf/iavf_ethdev.c | 25 +++++++++---------
> drivers/net/iavf/iavf_vchnl.c | 48 +++++++++++++++++++++++++++-------
> 2 files changed, 50 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index 62d72a0155..a12ea39444 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -2760,17 +2760,4 @@ iavf_dev_close(struct rte_eth_dev *dev)
>
> ret = iavf_dev_stop(dev);
> -
> - /*
> - * Release redundant queue resource when close the dev
> - * so that other vfs can re-use the queues.
> - */
> - if (vf->lv_enabled) {
> - ret = iavf_request_queues(dev, IAVF_MAX_NUM_QUEUES_DFLT);
> - if (ret)
> - PMD_DRV_LOG(ERR, "Reset the num of queues failed");
> -
> - vf->max_rss_qregion = IAVF_MAX_NUM_QUEUES_DFLT;
> - }
> -
> adapter->closed = true;
>
> @@ -2789,4 +2776,16 @@ iavf_dev_close(struct rte_eth_dev *dev)
> iavf_config_promisc(adapter, false, false);
>
> + /*
> + * Release redundant queue resource when close the dev
> + * so that other vfs can re-use the queues.
> + */
> + if (vf->lv_enabled) {
> + ret = iavf_request_queues(dev, IAVF_MAX_NUM_QUEUES_DFLT);
> + if (ret)
> + PMD_DRV_LOG(ERR, "Reset the num of queues failed");
> +
> + vf->max_rss_qregion = IAVF_MAX_NUM_QUEUES_DFLT;
> + }
> +
> iavf_shutdown_adminq(hw);
> if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
> diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
> index c2f6b617d6..aeffb07cca 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -319,5 +319,4 @@ iavf_execute_vf_cmd(struct iavf_adapter *adapter, struct iavf_cmd_info *args,
> switch (args->ops) {
> case VIRTCHNL_OP_RESET_VF:
> - case VIRTCHNL_OP_REQUEST_QUEUES:
> /*no need to wait for response */
> _clear_cmd(vf);
> @@ -343,4 +342,31 @@ iavf_execute_vf_cmd(struct iavf_adapter *adapter, struct iavf_cmd_info *args,
> _clear_cmd(vf);
> break;
> + case VIRTCHNL_OP_REQUEST_QUEUES:
> + /*
> + * ignore async reply, only wait for system message,
> + * vf_reset = true if get VIRTCHNL_EVENT_RESET_IMPENDING,
> + * if not, means request queues failed.
> + */
> + do {
> + result = iavf_read_msg_from_pf(adapter, args->out_size,
> + args->out_buffer);
> + if (result == IAVF_MSG_SYS && vf->vf_reset) {
> + break;
> + } else if (result == IAVF_MSG_CMD ||
> + result == IAVF_MSG_ERR) {
> + err = -1;
> + break;
> + }
> + iavf_msec_delay(ASQ_DELAY_MS);
> + /* If don't read msg or read sys event, continue */
> + } while (i++ < MAX_TRY_TIMES);
> + if (i >= MAX_TRY_TIMES ||
> + vf->cmd_retval != VIRTCHNL_STATUS_SUCCESS) {
> + err = -1;
> + PMD_DRV_LOG(ERR, "No response or return failure (%d)"
> + " for cmd %d", vf->cmd_retval, args->ops);
> + }
> + _clear_cmd(vf);
> + break;
> default:
> if (rte_thread_is_intr()) {
> @@ -2042,9 +2068,9 @@ iavf_request_queues(struct rte_eth_dev *dev, uint16_t num)
> IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> + struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> struct virtchnl_vf_res_request vfres;
> struct iavf_cmd_info args;
> uint16_t num_queue_pairs;
> int err;
> - int i = 0;
>
> if (!(vf->vf_res->vf_cap_flags &
> @@ -2067,5 +2093,14 @@ iavf_request_queues(struct rte_eth_dev *dev, uint16_t num)
>
> if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
> - err = iavf_execute_vf_cmd_safe(adapter, &args, 0);
> + /* disable interrupt to avoid the admin queue message to be read
> + * before iavf_read_msg_from_pf.
> + *
> + * don't disable interrupt handler until ready to execute vf cmd.
> + */
> + rte_spinlock_lock(&vf->aq_lock);
> + rte_intr_disable(pci_dev->intr_handle);
> + err = iavf_execute_vf_cmd(adapter, &args, 0);
> + rte_intr_enable(pci_dev->intr_handle);
> + rte_spinlock_unlock(&vf->aq_lock);
> } else {
> rte_eal_alarm_cancel(iavf_dev_alarm_handler, dev);
> @@ -2080,11 +2115,4 @@ iavf_request_queues(struct rte_eth_dev *dev, uint16_t num)
> }
>
> - /* wait for interrupt notification vf is resetting */
> - while (i++ < MAX_TRY_TIMES) {
> - if (vf->vf_reset)
> - break;
> - iavf_msec_delay(ASQ_DELAY_MS);
> - }
> -
> /* request queues succeeded, vf is resetting */
> if (vf->vf_reset) {
> --
> 2.43.0
More information about the stable
mailing list