Re: [PATCH v3 1/2] net/iavf: support VF initiated resets

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

On Thu, Oct 09, 2025 at 10:28:34AM +0000, Ciara Loftus wrote:
> Introduce a function that allows a VF to request the PF to reset the VF
> and then subsequently reinitialise and restart the VF transparently.
> This is useful for example when the application detects that one of the
> queues have hung or any event where a reset is required and the PF is
> unlikely to trigger it.
> 
> This commit leverages the existing graceful reset recovery path that was
> introduced in commit 3e6a5d2d310a ("net/iavf: add devargs to enable VF
> auto-reset").
> 
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> Signed-off-by: Timothy Miskell <timothy.miskell@intel.com>
> ---

Couple of comments inline below.

/Bruce

> v3:
> * Don't set the "vf_reset" flag before iavf_handle_hw_reset. It is only
> for cases when the PF has initiated the reset.
> 
> v2:
> * Always perform the requested reset, regardless of whether the
> auto_reset devarg is set or not.
> * Don't use the event handler to call iavf_handle_hw_reset. Call it
> inline in the rte_pmd_iavf_reset function call.
> * Do not send VIRTCHNL_OP_RESET_VF to the adminq before the
> iavf_handle_hw_reset sequence. That message is already sent within the
> iavf_handle_hw_reset function during iavf_dev_reset.
> * Rename the function to rte_pmd_iavf_restore to reflect that it's not
> just a reset being performed, but a restoration afterwards too.
> ---
>  drivers/net/intel/iavf/iavf.h         |  3 ++-
>  drivers/net/intel/iavf/iavf_ethdev.c  |  9 +++++++--
>  drivers/net/intel/iavf/iavf_vchnl.c   |  2 +-
>  drivers/net/intel/iavf/meson.build    |  1 +
>  drivers/net/intel/iavf/rte_pmd_iavf.c | 28 +++++++++++++++++++++++++++
>  drivers/net/intel/iavf/rte_pmd_iavf.h | 13 +++++++++++++
>  6 files changed, 52 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/net/intel/iavf/rte_pmd_iavf.c
> 
> diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
> index 435902fbc2..3552db7e06 100644
> --- a/drivers/net/intel/iavf/iavf.h
> +++ b/drivers/net/intel/iavf/iavf.h
> @@ -563,6 +563,7 @@ int iavf_flow_sub_check(struct iavf_adapter *adapter,
>  			struct iavf_fsub_conf *filter);
>  void iavf_dev_watchdog_enable(struct iavf_adapter *adapter);
>  void iavf_dev_watchdog_disable(struct iavf_adapter *adapter);
> -void iavf_handle_hw_reset(struct rte_eth_dev *dev);
> +void iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset);
>  void iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change);
> +bool is_iavf_supported(struct rte_eth_dev *dev);
>  #endif /* _IAVF_ETHDEV_H_ */
> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
> index 08c814725d..8fd7d39c6b 100644
> --- a/drivers/net/intel/iavf/iavf_ethdev.c
> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> @@ -3096,7 +3096,7 @@ iavf_is_reset_detected(struct iavf_adapter *adapter)
>   * Handle hardware reset
>   */
>  void
> -iavf_handle_hw_reset(struct rte_eth_dev *dev)
> +iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)
>  {
>  	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>  	struct iavf_adapter *adapter = dev->data->dev_private;
> @@ -3105,7 +3105,7 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev)
>  	if (!dev->data->dev_started)
>  		return;
>  
> -	if (!iavf_is_reset_detected(adapter)) {
> +	if (!vf_initiated_reset && !iavf_is_reset_detected(adapter)) {
>  		PMD_DRV_LOG(DEBUG, "reset not start");
>  		return;
>  	}
> @@ -3212,6 +3212,11 @@ static struct rte_pci_driver rte_iavf_pmd = {
>  	.remove = eth_iavf_pci_remove,
>  };
>  
> +bool is_iavf_supported(struct rte_eth_dev *dev)
> +{
> +	return !strcmp(dev->device->driver->name, rte_iavf_pmd.driver.name);
> +}
> +
>  RTE_PMD_REGISTER_PCI(net_iavf, rte_iavf_pmd);
>  RTE_PMD_REGISTER_PCI_TABLE(net_iavf, pci_id_iavf_map);
>  RTE_PMD_REGISTER_KMOD_DEP(net_iavf, "* igb_uio | vfio-pci");
> diff --git a/drivers/net/intel/iavf/iavf_vchnl.c b/drivers/net/intel/iavf/iavf_vchnl.c
> index b1b7a5bf94..460035d772 100644
> --- a/drivers/net/intel/iavf/iavf_vchnl.c
> +++ b/drivers/net/intel/iavf/iavf_vchnl.c
> @@ -83,7 +83,7 @@ iavf_dev_event_handle(void *param __rte_unused)
>  			struct iavf_adapter *adapter = pos->dev->data->dev_private;
>  			if (pos->event == RTE_ETH_EVENT_INTR_RESET &&
>  			    adapter->devargs.auto_reset) {
> -				iavf_handle_hw_reset(pos->dev);
> +				iavf_handle_hw_reset(pos->dev, false);
>  				rte_free(pos);
>  				continue;
>  			}
> diff --git a/drivers/net/intel/iavf/meson.build b/drivers/net/intel/iavf/meson.build
> index 0db94d6fe6..b39337733f 100644
> --- a/drivers/net/intel/iavf/meson.build
> +++ b/drivers/net/intel/iavf/meson.build
> @@ -24,6 +24,7 @@ sources = files(
>          'iavf_tm.c',
>          'iavf_ipsec_crypto.c',
>          'iavf_fsub.c',
> +        'rte_pmd_iavf.c',
>  )
>  
>  if arch_subdir == 'x86'
> diff --git a/drivers/net/intel/iavf/rte_pmd_iavf.c b/drivers/net/intel/iavf/rte_pmd_iavf.c
> new file mode 100644
> index 0000000000..2a04f9ddba
> --- /dev/null
> +++ b/drivers/net/intel/iavf/rte_pmd_iavf.c
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2010-2025 Intel Corporation
> + */
> +
> +#include <eal_export.h>
> +
> +#include "iavf.h"
> +#include "rte_pmd_iavf.h"
> +
> +RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_pmd_iavf_restore, 25.11)
> +int
> +rte_pmd_iavf_restore(uint16_t port)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> +	dev = &rte_eth_devices[port];
> +
> +	if (!is_iavf_supported(dev)) {
> +		PMD_DRV_LOG(ERR, "Cannot restore VF, port %u is not an IAVF device.", port);
> +		return -ENOTSUP;
> +	}
> +
> +	iavf_handle_hw_reset(dev, true);
> +
> +	return 0;
> +}

I don't think we need a brand new file for this. Can it not go in an
existing iavf C file?

> diff --git a/drivers/net/intel/iavf/rte_pmd_iavf.h b/drivers/net/intel/iavf/rte_pmd_iavf.h
> index 04b86a5dd7..50a8cd0070 100644
> --- a/drivers/net/intel/iavf/rte_pmd_iavf.h
> +++ b/drivers/net/intel/iavf/rte_pmd_iavf.h
> @@ -96,6 +96,19 @@ extern uint64_t rte_pmd_ifd_dynflag_proto_xtr_tcp_mask;
>  extern uint64_t rte_pmd_ifd_dynflag_proto_xtr_ip_offset_mask;
>  extern uint64_t rte_pmd_ifd_dynflag_proto_xtr_ipsec_crypto_said_mask;
>  
> +/**
> + * Request PF driver to initiate a PF-to-VF RESET and restore the device
> + * following the reset by reconfiguring and restarting the device. The
> + * port must be started before calling this function.
> + *
> + * @param port
> + *   The port identifier of the Ethernet device.
> + * @return
> + *   0 if successful, otherwise if a failure occurs
> + */
> +__rte_experimental
> +int rte_pmd_iavf_restore(uint16_t port);
> +

I wonder if "reinitialize" or "reinit" is a better name for this function.
After all, it does a reset and then a restore - more like a reboot!

>  /**
>   * The mbuf dynamic field pointer for flexible descriptor's extraction metadata.
>   */
> -- 
> 2.34.1
>