> Subject: [PATCH v3] net/iavf: fix duplicate VF reset during PF reset recovery
>
> During PF initiated reset recovery, iavf_dev_close() sending
> an extra VIRTCHNL_OP_RESET_VF while recovery is already in progress.
> That second reset can leave PF/VF virtchnl state inconsistent and
> cause VIRTCHNL_OP_CONFIG_VSI_QUEUES to fail with ERR_PARAM after
> ToR link flap/power-cycle, leaving the VF unable to recover.
> This results in connection loss.
>
> Skipped close-time VF reset and related close-time virtchnl
> operations when PF triggered reset recovery is set. This is
> done to avoid a duplicate VF reset, and keep normal behavior
> for application-driven close.
> Handled link-change events through a common static function that
> reads the correct advanced & legacy link fields properly and
> updates no-poll/watchdog/LSC state consistently.
> Also added IAVF_ERR_ADMIN_QUEUE_NO_WORK in virtchnl message
> drain as a normal empty-queue condition and avoid logging it as
> an misleading AQ failure.
>
> Fixes: 675a104e2e94 ("net/iavf: fix abnormal disable HW interrupt")
> Fixes: b34fe66ea893 ("net/iavf: delay VF reset command")
> Fixes: 5e03e316c753 ("net/iavf: handle virtchnl event message without
> interrupt")
> Fixes: 5c8ca9f13c78 ("net/iavf: fix no polling mode switching")
> Fixes: 48de41ca11f0 ("net/avf: enable link status update")
> Fixes: 02d212ca3125 ("net/iavf: rename remaining avf strings")
> Cc: [email protected]
Hi Anurag,
Thanks for the patch. There seems to be multiple logical fixes/changes in
here and I think it would be good to split them into individual patches,
each with their own Fixes tag where relevant. Having multiple fixes in
one patch with multiple Fixes tags makes backporting tricky.
I think at least logic which prevents the RESET_VF during a PF initiated
reset should be split out from the link-change logic.
Thanks,
Ciara
>
> Signed-off-by: Anurag Mandal <[email protected]>
> ---
> V3: Addressed latest ai-code-review comments
> V2: Addressed ai-code-review comments
>
> doc/guides/rel_notes/release_26_07.rst | 3 +
> drivers/net/intel/iavf/iavf_ethdev.c | 37 +++---
> drivers/net/intel/iavf/iavf_vchnl.c | 155 ++++++++++++++++---------
> 3 files changed, 123 insertions(+), 72 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_26_07.rst
> b/doc/guides/rel_notes/release_26_07.rst
> index b8a3e2ced9..e7ac730369 100644
> --- a/doc/guides/rel_notes/release_26_07.rst
> +++ b/doc/guides/rel_notes/release_26_07.rst
> @@ -89,6 +89,9 @@ New Features
>
> * Added support for transmitting LLDP packets based on mbuf packet type.
> * Implemented AVX2 context descriptor transmit paths.
> + * Prevented duplicate 'VIRTCHNL_OP_RESET_VF' during a PF-initiated
> + reset recovery, which earlier caused virtchnl state corruption
> + and connection loss after a top-of-rack (ToR) link flap/power-cycle.
>
> * **Updated PCAP ethernet driver.**
>
> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c
> b/drivers/net/intel/iavf/iavf_ethdev.c
> index bdf650b822..fb6f287d3c 100644
> --- a/drivers/net/intel/iavf/iavf_ethdev.c
> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> @@ -3166,24 +3166,27 @@ 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");
> + /* Skip RESET_VF on a PF-initiated reset */
> + if (!adapter->closed && !vf->in_reset_recovery) {
adapter->closed will always be false here so the check is redundant.
> + /*
> + * 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;
> + }
>
