On Tue, Jan 13, 2026 at 03:17:57PM +0000, Ciara Loftus wrote:
> The user may want to perform some actions before and/or after a VF
> reset, for example storing settings that may be lost during the reset
> and restoring them after the reset. To facilitate this, introduce a
> new function which allows the user to register either a pre or post
> reset callback, which will be executed before or after the VF has been
> reset. To unregister the callback, simply use the same function with a
> null callback as argument.
>
> Signed-off-by: Ciara Loftus <[email protected]>
Some review comments and suggestions inline below.
/Bruce
> ---
> doc/guides/rel_notes/release_26_03.rst | 4 ++
> drivers/net/intel/iavf/iavf.h | 15 +++++++
> drivers/net/intel/iavf/iavf_ethdev.c | 62 ++++++++++++++++++++++++++
> drivers/net/intel/iavf/rte_pmd_iavf.h | 24 ++++++++++
> 4 files changed, 105 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_26_03.rst
> b/doc/guides/rel_notes/release_26_03.rst
> index 15dabee7a1..770f9933ee 100644
> --- a/doc/guides/rel_notes/release_26_03.rst
> +++ b/doc/guides/rel_notes/release_26_03.rst
> @@ -55,6 +55,10 @@ New Features
> Also, make sure to start the actual text at the margin.
> =======================================================
>
> +* **Updated Intel iavf driver.**
> +
> + * Added support for pre and post VF reset callbacks.
> +
>
> Removed Items
> -------------
> diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
> index d78582e05c..5482472549 100644
> --- a/drivers/net/intel/iavf/iavf.h
> +++ b/drivers/net/intel/iavf/iavf.h
> @@ -224,6 +224,17 @@ struct iavf_qtc_map {
> uint16_t queue_count;
> };
>
> +enum iavf_reset_cb_type {
> + IAVF_RESET_CB_TYPE_PRE,
> + IAVF_RESET_CB_TYPE_POST,
> +};
I don't particularly like having the names end with "PRE" and "POST" on
their own. How about renaming to drop the "TYPE" and then adding in the
reset again at the end, so that it's pre-reset and post-reset, which tends
to read better IMHO: IAVF_RESET_CB_PRE_RESET, IAVF_RESET_CB_POST_RESET?
Perhaps there is better naming again that can be suggested?
> +
> +struct iavf_reset_cb_arg {
> + uint16_t port_id;
> + int reset_status;
> + void *user_state;
> +};
> +
I don't think we need this structure. When the user is setting up the
callback the only parameter they provide in this struct is the user_state.
Then for each callback, I'd make it have each parameter separately rather
than using a struct.
Semi-related, since the reset_status is going to be undefined for the
pre-reset callback, did you consider removing the reset_cb_type enum and
just using separate types and APIs for configuring pre and post-reset
callbacks?
void (*pre_reset_cb)(uint16_t port_id, void *user_param);
void (*post_reset_cb)(uint16_t port_id, int reset_state, void *user_param);
and then APIs
rte_pmd_iavf_register_pre_reset_cb(...)
rte_pmd_iavf_register_post_reset_cb(...)
Just a suggestion - if you prefer to keep patch as is, I have no strong
objections.
> /* Structure to store private data specific for VF instance. */
> struct iavf_info {
> uint16_t num_queue_pairs;
> @@ -257,6 +268,10 @@ struct iavf_info {
>
> struct iavf_vsi vsi;
> bool vf_reset; /* true for VF reset pending, false for no VF reset */
> + void (*pre_reset_cb)(struct iavf_reset_cb_arg *arg); /* Pre reset
> callback function ptr */
> + void (*post_reset_cb)(struct iavf_reset_cb_arg *arg); /* Post reset
> callback function ptr */
Normally we use typedefs for function pointers in DPDK. However, there may
be cases where we don't too, so no strong objection here.
> + struct iavf_reset_cb_arg *pre_reset_cb_arg; /* Pre reset function
> argument */
> + struct iavf_reset_cb_arg *post_reset_cb_arg; /* Post reset function
> argument */
> uint64_t flags;
>
> uint8_t *rss_lut;
> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c
> b/drivers/net/intel/iavf/iavf_ethdev.c
> index 15e49fe248..553f38a286 100644
> --- a/drivers/net/intel/iavf/iavf_ethdev.c
> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> @@ -3120,6 +3120,10 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool
> vf_initiated_reset)
> vf->in_reset_recovery = true;
> iavf_set_no_poll(adapter, false);
>
> + /* Call the pre reset callback */
> + if (vf->pre_reset_cb)
DPDK style guide says to always compare pointers explicitly with NULL.
> + (*vf->pre_reset_cb)(vf->pre_reset_cb_arg);
> +
Don't think we need the "*" or the brackets around the function pointer
here.
> ret = iavf_dev_reset(dev);
> if (ret)
> goto error;
> @@ -3144,6 +3148,13 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool
> vf_initiated_reset)
> error:
> PMD_DRV_LOG(DEBUG, "RESET recover with error code=%dn", ret);
> exit:
> + /* Update the reset status */
> + if (vf->post_reset_cb_arg)
> + vf->post_reset_cb_arg->reset_status = ret;
> + /* Call the post reset callback */
> + if (vf->post_reset_cb)
> + (*vf->post_reset_cb)(vf->post_reset_cb_arg);
> +
> vf->in_reset_recovery = false;
> iavf_set_no_poll(adapter, false);
>
> @@ -3183,6 +3194,57 @@ rte_pmd_iavf_reinit(uint16_t port)
> return 0;
> }
>
> +RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_pmd_iavf_reset_cb_register, 26.03)
> +int
> +rte_pmd_iavf_reset_cb_register(uint16_t port,
> + enum iavf_reset_cb_type cb_type,
> + void (*reset_cb)(struct iavf_reset_cb_arg
> *arg),
> + struct iavf_reset_cb_arg *reset_arg)
> +{
> + struct rte_eth_dev *dev;
> + struct iavf_info *vf;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> + dev = &rte_eth_devices[port];
> +
> + if (!is_iavf_supported(dev)) {
> + PMD_DRV_LOG(ERR, "Cannot register callback, port %u is not an
> IAVF device.", port);
> + return -ENOTSUP;
> + }
> +
> + if (reset_cb != NULL && reset_arg == NULL) {
> + PMD_DRV_LOG(ERR, "Cannot register callback on port %u, arg is
> NULL.", port);
> + return -EINVAL;
> + }
> +
Minor nit, but I would check the parameters before doing any other later
checks, so suggest moving this up as a parameter check right after the
port id check.
> + if (reset_arg != NULL)
> + reset_arg->port_id = port;
> +
> + vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> +
> + if (vf->in_reset_recovery) {
> + PMD_DRV_LOG(ERR, "Cannot modify reset callback on port %u, VF
> is resetting.", port);
> + return -EBUSY;
> + }
> +
> + switch (cb_type) {
> + case IAVF_RESET_CB_TYPE_PRE:
> + vf->pre_reset_cb = reset_cb;
> + vf->pre_reset_cb_arg = reset_arg;
> + break;
> + case IAVF_RESET_CB_TYPE_POST:
> + vf->post_reset_cb = reset_cb;
> + vf->post_reset_cb_arg = reset_arg;
> + break;
> + default:
> + PMD_DRV_LOG(ERR, "Invalid reset callback type: %d", cb_type);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> void
> iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change)
> {
> diff --git a/drivers/net/intel/iavf/rte_pmd_iavf.h
> b/drivers/net/intel/iavf/rte_pmd_iavf.h
> index dea1bd2789..365f8a3e6a 100644
> --- a/drivers/net/intel/iavf/rte_pmd_iavf.h
> +++ b/drivers/net/intel/iavf/rte_pmd_iavf.h
> @@ -20,6 +20,8 @@
> #include <rte_mbuf.h>
> #include <rte_mbuf_dyn.h>
>
> +#include "iavf.h"
> +
> #ifdef __cplusplus
> extern "C" {
> #endif
> @@ -109,6 +111,28 @@ extern uint64_t
> rte_pmd_ifd_dynflag_proto_xtr_ipsec_crypto_said_mask;
> __rte_experimental
> int rte_pmd_iavf_reinit(uint16_t port);
>
> +/**
> + * Register or unregister a pre or post reset event callback
> + *
> + * @param port
> + * The port identifier of the Ethernet device.
> + * @param cb_type
> + * The callback type, pre-reset (0) or post-reset (1).
> + * @param reset_cb
> + * The callback function that will be invoked by the driver.
> + * Pass NULL to unregister an existing callback.
> + * @param reset_arg
> + * The argument passed to the callback function.
> + * Can be NULL when unregistering (reset_cb is NULL).
> + * @return
> + * 0 if successful, otherwise if a failure occurs.
> + */
> +__rte_experimental
> +int rte_pmd_iavf_reset_cb_register(uint16_t port,
> + enum iavf_reset_cb_type cb_type,
> + void (*reset_cb)(struct iavf_reset_cb_arg *arg),
> + struct iavf_reset_cb_arg *reset_arg);
> +
> /**
> * The mbuf dynamic field pointer for flexible descriptor's extraction
> metadata.
> */
> --
> 2.43.0
>
