net/iavf: harden reset recovery and data path on link flap
cover net/iavf: harden reset recovery and data path on link flap
During PF-initiated reset or a remote/ToR switch link-flap, the VF might miss the reset event, race on the no_poll gate, leak in-flight Tx descriptors, and stay down if dev_start ran before the PF VSI was ready.
This patch series build on the earlier reset-recovery fixes with the following:
- Reset detection: complement the ARQLEN1 check with VFGEN_RSTAT (VIRTCHNL_VFR_INPROGRESS) and poll at a 5 ms interval, matching the kernel iavf driver, so fast ARQ flips are not missed. When the VFR is still not observed, recover anyway instead of bailing out, keeping PF and VF state in sync.
- no_poll: make the flag atomic (RTE_ATOMIC) with release/acquire ordering so the data-plane lcores observe gate changes reliably.
- Tx drain: add iavf_dev_tx_drain() to flush in-flight Tx descriptors on link-down and impending-reset events before teardown, preventing MDD events and descriptor leaks.
- Deferred start: when dev_start fails during recovery (PF VSI inactive), defer it via start_pending and resume on the next link-up event so the VF comes back without manual intervention.
- AdminQ: discard zeroed (opcode 0) descriptors seen during PF-initiated resets to avoid the "Request 0 is not supported" log flood.
Anurag Mandal (5):
net/iavf: discard empty AdminQ descriptors on reset net/iavf: defer device start when PF VSI not ready net/iavf: drain in-flight Tx before reset net/iavf: change no_poll flag to atomic net/iavf: improve VF reset detection on fast ARQ flip
drivers/net/intel/iavf/iavf.h | 5 +- drivers/net/intel/iavf/iavf_ethdev.c | 103 ++++++++++++++++++++---- drivers/net/intel/iavf/iavf_rxtx.c | 112 ++++++++++++++++++++++++++- drivers/net/intel/iavf/iavf_rxtx.h | 6 ++ drivers/net/intel/iavf/iavf_vchnl.c | 40 +++++++++- 5 files changed, 246 insertions(+), 20 deletions(-)
Signature · 1 line
2.34.1
PATCH 1/5 net/iavf: discard empty AdminQ descriptors on reset
During PF-initiated resets iavf_clean_arq_element() has been observed to return success with a fully zeroed descriptor (opcode 0). These fell through to the default arm of the dispatch switch and produced a "Request 0 is not supported yet" log flood in the field.
Skip descriptors with opcode 0 so they are silently discarded and do not flood log.
--- drivers/net/intel/iavf/iavf_vchnl.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
drivers/net/intel/iavf/iavf_vchnl.c +13 -0
@@ -614,6 +614,19 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)break ; } aq_opc =rte_le_to_cpu_16 (info.desc.opcode); + +/* +* During PF-initiated resets, iavf_clean_arq_element() has +* been observed to return IAVF_SUCCESS with a fully zeroed +* descriptor (opcode 0). +* Without this guard, such descriptors would fall through +* to the default case of the dispatch switch below and the +* "Request 0 is not supported yet" log flood reported in +* the field would be produced. These are discarded now. +*/ +if (aq_opc ==0 ) +continue ; +/* For the message sent from pf to vf, opcode is stored in * cookie_high of struct iavf_aq_desc, while return error code * are stored in cookie_low, Which is done by PF driver.
Signature · 1 line
2.34.1
RE: [PATCH 1/5] net/iavf: discard empty AdminQ descriptors on reset
Quoted 24 lines
Subject: [PATCH 1/5] net/iavf: discard empty AdminQ descriptors on reset
During PF-initiated resets
iavf_clean_arq_element()has been observed to return success with a fully zeroed descriptor (opcode 0). These fell through to the default arm of the dispatch switch and produced a "Request 0 is not supported yet" log flood in the field.Skip descriptors with opcode 0 so they are silently discarded and do not flood log.
Signed-off-by Anurag Mandal <[email protected]>--- drivers/net/intel/iavf/iavf_vchnl.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)drivers/net/intel/iavf/iavf_vchnl.c +0
-0@@ -614,6 +614,19 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)break ; } aq_opc =rte_le_to_cpu_16 (info.desc.opcode);
drivers/net/intel/iavf/iavf_vchnl.c iavf_handle_virtchnl_msg(struct rte_eth_dev *dev) · line 626
Quoted 10 lines
drivers/net/intel/iavf/iavf_vchnl.c +10
-0+ +/* +* During PF-initiated resets, iavf_clean_arq_element() has +* been observed to return IAVF_SUCCESS with a fully zeroed +* descriptor (opcode 0). +* Without this guard, such descriptors would fall through +* to the default case of the dispatch switch below and the +* "Request 0 is not supported yet" log flood reported in +* the field would be produced. These are discarded now. +*/
This comment is too lengthy. This information is captured appropriately in the commit message.
Something like "opcode 0 means the descriptor is empty so it can be skipped" or similar. Assuming an opcode of zero always means an empty/zeroed descriptor?
Quoted 8 lines
drivers/net/intel/iavf/iavf_vchnl.c +3
-1+if (aq_opc ==0 ) +continue ; +/* For the message sent from pf to vf, opcode is stored in * cookie_high of struct iavf_aq_desc, while return error code * are stored in cookie_low, Which is done by PF driver. -- 2.34.1
PATCH 2/5 net/iavf: defer device start when PF VSI not ready
In reply to Anurag MandalDuring reset recovery iavf_dev_start() might fail (typically -EIO from VIRTCHNL_OP_CONFIG_VSI_QUEUES) because the PF VSI is not yet active, leaving the VF down and requiring manual intervention to recover.
Added a start_pending flag: when device start fails during recovery, defer it instead of erroring out and resume it from newly added iavf_resume_pending_start() on the next link-up event, so the VF comes back automatically.
--- drivers/net/intel/iavf/iavf.h | 2 + drivers/net/intel/iavf/iavf_ethdev.c | 56 ++++++++++++++++++++++++++-- drivers/net/intel/iavf/iavf_vchnl.c | 16 +++++++- 3 files changed, 69 insertions(+), 5 deletions(-)
drivers/net/intel/iavf/iavf.h +2 -0
@@ -293,6 +293,7 @@ struct iavf_info {bool in_reset_recovery;bool reset_pending;bool pf_reset_in_progress; +bool start_pending;uint32_t ptp_caps;rte_spinlock_t phc_time_aq_lock;
@@ -533,4 +534,5 @@ 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);void iavf_hash_uninit (struct iavf_adapter *ad); +void iavf_resume_pending_start (struct rte_eth_dev *dev);#endif /* _IAVF_ETHDEV_H_ */
drivers/net/intel/iavf/iavf_ethdev.c +52 -4
86 lines not on this page · raw message
drivers/net/intel/iavf/iavf_vchnl.c +15 -1
@@ -260,7 +260,7 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,* (link is down or a VF reset is in progress); the watchdog drives * auto-reset recovery, so it must remain armed in those cases. */ -if (vf->link_up && !vf->vf_reset) +if (vf->link_up && !vf->vf_reset && !vf->in_reset_recovery)iavf_dev_watchdog_disable (adapter);else iavf_dev_watchdog_enable (adapter);
@@ -271,6 +271,20 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev, adapter->no_poll ? "on" : "off"); } +/* +* Resume a deferred dev_start. +* iavf_handle_hw_reset() sets vf->start_pending when +* reset recovery completed dev_init() but iavf_dev_start() +* itself failed (typically -EIO from VIRTCHNL_OP_CONFIG_VSI_QUEUES +* when the PF VSI was inactive). +* A link-up event implies the PF VSI is active again, so retry now. +* Run before the LSC event post so the port is ready to accept Tx +* by the time the app's link-up callback fires; no_poll has already +* been cleared above so bursts go through as soon as +* dev_start sets dev_started=1. +*/ +iavf_resume_pending_start (dev); +iavf_dev_event_post (dev,RTE_ETH_EVENT_INTR_LSC ,NULL ,0 ); PMD_DRV_LOG(INFO, "Link status update:%s",
Signature · 1 line
2.34.1
RE: [PATCH 2/5] net/iavf: defer device start when PF VSI not ready
Quoted 11 lines
Subject: [PATCH 2/5] net/iavf: defer device start when PF VSI not ready
During reset recovery
iavf_dev_start()might fail (typically-EIOfromVIRTCHNL_OP_CONFIG_VSI_QUEUES) because the PF VSI is not yet active, leaving the VF down and requiring manual intervention to recover.Added a
start_pendingflag: when device start fails during recovery, defer it instead of erroring out and resume it from newly addediavf_resume_pending_start()on the next link-up event, so the VF comes back automatically.
It sounds like a fix, can you add a Fixes tag?
Quoted 106 lines
Signed-off-by Anurag Mandal <[email protected]>--- drivers/net/intel/iavf/iavf.h | 2 + drivers/net/intel/iavf/iavf_ethdev.c | 56 ++++++++++++++++++++++++++-- drivers/net/intel/iavf/iavf_vchnl.c | 16 +++++++- 3 files changed, 69 insertions(+), 5 deletions(-)drivers/net/intel/iavf/iavf.h +2
-0@@ -293,6 +293,7 @@ struct iavf_info {bool in_reset_recovery;bool reset_pending;bool pf_reset_in_progress; +bool start_pending;uint32_t ptp_caps;rte_spinlock_t phc_time_aq_lock;@@ -533,4 +534,5 @@ 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);void iavf_hash_uninit (struct iavf_adapter *ad); +void iavf_resume_pending_start (struct rte_eth_dev *dev);#endif /* _IAVF_ETHDEV_H_ */
drivers/net/intel/iavf/iavf_ethdev.c +7
-0@@ -1093,6 +1093,9 @@ iavf_dev_start(struct rte_eth_dev *dev)iavf_phc_sync_alarm_start (dev); +/* An explicit start supersedes any pending deferred start */ + vf->start_pending = false; +return 0 ; error:@@ -1131,6 +1134,9 @@ iavf_dev_stop(struct rte_eth_dev *dev) adapter->stopped =1 ; dev->data->dev_started =0 ; +/* An explicit stop cancels any pending deferred start */ + vf->start_pending = false; +return 0 ; }@@ -3425,6 +3431,7 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset) vf->in_reset_recovery = true; vf->pf_reset_in_progress = !vf_initiated_reset; + vf->start_pending = false;iavf_set_no_poll (adapter, false);drivers/net/intel/iavf/iavf_ethdev.c +5
-4/* Call the pre reset callback */ @@ -3445,10 +3452,17 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)if (!vf_initiated_reset || restart_device) {/* start the device */ ret =iavf_dev_start (dev); -if (ret) - goto error; - - dev->data->dev_started =1 ; +if (ret ==0 ) { + dev->data->dev_started =1 ; + }else { + PMD_DRV_LOG(WARNING, + "dev_start failed during reset recovery(rc=%d);"
drivers/net/intel/iavf/iavf_ethdev.c +21
-0+"deferring to next link-up event" , + ret); + vf->start_pending = true; + dev->data->dev_started =0 ; + ret =0 ; + } }/* Restore settings after the reset */ @@ -3662,6 +3676,40 @@ bool is_iavf_supported(struct rte_eth_dev *dev)return !strcmp (dev->device->driver->name, rte_iavf_pmd.driver.name); } +void +iavf_resume_pending_start (struct rte_eth_dev *dev) +{ +struct iavf_info *vf =IAVF_DEV_PRIVATE_TO_VF (dev->data- >dev_private); +int ret; + +if (!vf->start_pending) +return ; +/* +* If the application has already (re)started the port itself, the +* deferred start is stale, the application's action is honoured +* and resume pending is dropped to avoid starting an +* already-running port a second time. +*/ +if (dev->data->dev_started) {
drivers/net/intel/iavf/iavf_ethdev.c bool is_iavf_supported(struct rte_eth_dev *dev) · line 3705
Quoted 10 lines
drivers/net/intel/iavf/iavf_ethdev.c +9
-0+ vf->start_pending = false; +return ; + } + +if (!vf->link_up) +return ; + +PMD_DRV_LOG (DEBUG ,"PF link back up; resuming deferred dev_start"); + ret =iavf_dev_start (dev);
This call to iavf_dev_start could result in another nested call to iavf_resume_pending_start which would be problematic. iavf_dev_start polls the ARQ eg.
iavf_dev_start -> iavf_configure_queues -> iavf_execute_vf_cmd_safe -> iavf_wait_for_msg -> iavf_read_msg_from_pf -> iavf_clean_arq_element
If another LSC happens during this time, you could enter iavf_handle_link_change_event again and end up calling iavf_resume_pending_start again. It can maybe be prevented by clearing the pending flag at the beginning of iavf_resume_pending_start and setting it again if the start fails.
Quoted 20 lines
drivers/net/intel/iavf/iavf_ethdev.c +6
-0+if (ret ==0 ) { + dev->data->dev_started =1 ; + vf->start_pending = false; + }else { + PMD_DRV_LOG(ERR, + "deferred dev_start failed (ret=%d); will retry onnext link-up",
drivers/net/intel/iavf/iavf_ethdev.c +4
-0+ ret); + } +} +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" );drivers/net/intel/iavf/iavf_vchnl.c +0
-0
drivers/net/intel/iavf/iavf_vchnl.c iavf_handle_link_change_event(struct rte_eth_dev · line 264
Quoted 6 lines
drivers/net/intel/iavf/iavf_vchnl.c +1
-1@@ -260,7 +260,7 @@ iavf_handle_link_change_event(struct rte_eth_dev*dev, * (link is down or a VF reset is in progress); the watchdog drives * auto-reset recovery, so it must remain armed in those cases. */ -if (vf->link_up && !vf->vf_reset) +if (vf->link_up && !vf->vf_reset && !vf->in_reset_recovery)
Is this change relevant to the rest of this patch?
Quoted 14 lines
drivers/net/intel/iavf/iavf_vchnl.c +0
-0iavf_dev_watchdog_disable (adapter); elsedrivers/net/intel/iavf/iavf_vchnl.c +5
-0iavf_dev_watchdog_enable (adapter);@@ -271,6 +271,20 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev, adapter->no_poll ? "on" : "off"); } +/* +* Resume a deferred dev_start. +* iavf_handle_hw_reset() sets vf->start_pending when +* reset recovery completed dev_init() but iavf_dev_start() +* itself failed (typically -EIO from
VIRTCHNL_OP_CONFIG_VSI_QUEUES
drivers/net/intel/iavf/iavf_vchnl.c iavf_handle_link_change_event(struct rte_eth_dev *dev, · line 285
Quoted 7 lines
drivers/net/intel/iavf/iavf_vchnl.c +7
-0+* when the PF VSI was inactive). +* A link-up event implies the PF VSI is active again, so retry now. +* Run before the LSC event post so the port is ready to accept Tx +* by the time the app's link-up callback fires; no_poll has already +* been cleared above so bursts go through as soon as +* dev_start sets dev_started=1. +*/
I think this comment is verbose, consider shortening it.
Quoted 7 lines
drivers/net/intel/iavf/iavf_vchnl.c +2
-1+iavf_resume_pending_start (dev); +iavf_dev_event_post (dev,RTE_ETH_EVENT_INTR_LSC ,NULL ,0 ); PMD_DRV_LOG(INFO, "Link status update:%s", --2.34.1
PATCH 3/5 net/iavf: drain in-flight Tx before reset
In reply to Anurag MandalOn a link-down or impending PF reset, in-flight Tx descriptors were left pending when the queues were torn down, which could trigger Malicious Driver Detection (MDD) events and leak descriptors.
Added iavf_dev_tx_drain() to let already-posted Tx bursts complete and flush the rings within a bounded budget, and call it on link-down and reset-impending events before teardown, preventing MDD events and descriptor leaks. The drain selects the cleanup routine that matches the active Tx path: the scalar path uses ci_tx_xmit_cleanup(), while the vector and CTX paths use ci_tx_free_bufs_vec(). This matters because the scalar and vector paths track their software rings differently
(ci_tx_entry vs ci_tx_entry_vec) and using the scalar routine on a vector queue would walk the wrong ring and free the wrong mbufs.
--- drivers/net/intel/iavf/iavf_rxtx.c | 104 ++++++++++++++++++++++++++++ drivers/net/intel/iavf/iavf_rxtx.h | 6 ++ drivers/net/intel/iavf/iavf_vchnl.c | 7 ++ 3 files changed, 117 insertions(+)
drivers/net/intel/iavf/iavf_rxtx.c +104 -0
@@ -32,6 +32,7 @@#include "iavf.h" #include "iavf_rxtx.h" +#include "iavf_rxtx_vec_common.h" #include "iavf_ipsec_crypto.h" #include "rte_pmd_iavf.h"
@@ -4025,6 +4026,109 @@ iavf_tx_done_cleanup_full(struct ci_tx_queue *txq,return (int )pkt_cnt; } +/* +* Reclaim completed Tx descriptors for a single queue using the cleanup +* routine that matches the active Tx path. +* The scalar and vector paths track their software rings differently +* (ci_tx_entry vs ci_tx_entry_vec) and keep separate completion +* bookkeeping, so using the scalar routine on a vector queue +* (or vice versa) would free the wrong mbufs. +* Returns true if any descriptors were reclaimed. +*/ +static bool +iavf_tx_drain_cleanup (struct ci_tx_queue *txq, +enum iavf_tx_func_type tx_func_type) +{ +switch (tx_func_type) { +case IAVF_TX_AVX2_CTX : +case IAVF_TX_AVX2_CTX_OFFLOAD : +case IAVF_TX_AVX512_CTX : +case IAVF_TX_AVX512_CTX_OFFLOAD : +return ci_tx_free_bufs_vec (txq, iavf_tx_desc_done, true) !=0 ; +case IAVF_TX_NEON : +case IAVF_TX_AVX2 : +case IAVF_TX_AVX2_OFFLOAD : +case IAVF_TX_AVX512 : +case IAVF_TX_AVX512_OFFLOAD : +return ci_tx_free_bufs_vec (txq, iavf_tx_desc_done, false) !=0 ; +case IAVF_TX_DEFAULT : +default : +return ci_tx_xmit_cleanup (txq) ==0 ; + } +} + +/* +* iavf_dev_tx_drain - drain in-flight Tx descriptors after a link-down or +* impending PF reset event. +*/ +void +iavf_dev_tx_drain (struct rte_eth_dev *dev) +{ +struct iavf_adapter *adapter = +IAVF_DEV_PRIVATE_TO_ADAPTER (dev->data->dev_private); +enum iavf_tx_func_type tx_func_type = adapter->tx_func_type; +struct ci_tx_queue *txq; +uint64_t hz, deadline; +int idle_iters =0 ; +uint16_t qid; + +/* +* Allow any Tx burst already in flight on a data-plane lcore to +* write its remaining descriptors and notify. After +* this window, the no_poll gate set by the caller is observed at +* the next burst-entry and no new descriptors will be posted. +*/ +rte_delay_us_block (IAVF_TX_DRAIN_SETTLE_US ); + + hz =rte_get_timer_hz (); + deadline =rte_get_timer_cycles () + + (hz *IAVF_TX_DRAIN_TIMEOUT_US ) /1000000ULL ; + +while (rte_get_timer_cycles () < deadline) { +bool any_pending = false; +bool any_progress = false; + +for (qid =0 ; qid < dev->data->nb_tx_queues; qid++) { + txq = dev->data->tx_queues[qid]; +if (txq ==NULL || + dev->data->tx_queue_state[qid] != +RTE_ETH_QUEUE_STATE_STARTED ) +continue ; + +/* +* nb_tx_free == nb_tx_desc - 1 means the ring is +* empty (one descriptor is always reserved). +*/ +if (txq->nb_tx_free >= txq->nb_tx_desc -1 ) +continue ; + + any_pending = true; +if (iavf_tx_drain_cleanup (txq, tx_func_type)) + any_progress = true; + } + +if (!any_pending) +return ; + +if (any_progress) { + idle_iters =0 ; + }else if (++idle_iters >=IAVF_TX_DRAIN_IDLE_MAX ) { +/* +* HW has not advanced the RS-bit write-back for +* several polling intervals; either the queue is +* quiescent except for the sub-rs_thresh tail +* (which we cannot observe here) or HW is no +* longer fetching. Further polling is unlikely to +* help, and the PF teardown path has its own +* grace period for the remainder. +*/ +break ; + } + +rte_delay_us_block (IAVF_TX_DRAIN_POLL_US ); + } +} +int iavf_dev_tx_done_cleanup (void *txq,uint32_t free_cnt) {
drivers/net/intel/iavf/iavf_rxtx.h +6 -0
@@ -506,6 +506,11 @@ enum iavf_tx_ctx_desc_tunnel_l4_tunnel_type {/* Valid indicator bit for the time_stamp_low field */ #define IAVF_RX_FLX_DESC_TS_VALID (0x1UL) +#define IAVF_TX_DRAIN_TIMEOUT_US 10000/* total drain budget: 10 ms */ +#define IAVF_TX_DRAIN_SETTLE_US 100/* let in-flight burst land */ +#define IAVF_TX_DRAIN_POLL_US 50/* poll interval */ +#define IAVF_TX_DRAIN_IDLE_MAX 20/* ~1 ms of no RS write-back */ + int iavf_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, uint16_t nb_desc,
@@ -641,6 +646,7 @@ void iavf_set_default_ptype_table(struct rte_eth_dev *dev);void iavf_rx_queue_release_mbufs_vec (struct ci_rx_queue *rxq);void iavf_rx_queue_release_mbufs_neon (struct ci_rx_queue *rxq);enum rte_vect_max_simd iavf_get_max_simd_bitwidth (void ); +void iavf_dev_tx_drain (struct rte_eth_dev *dev); static inline void iavf_dump_rx_descriptor(struct ci_rx_queue *rxq,
drivers/net/intel/iavf/iavf_vchnl.c +7 -0
@@ -269,6 +269,8 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,iavf_set_no_poll (adapter, true);PMD_DRV_LOG (DEBUG ,"VF no poll turned %s" , adapter->no_poll ?"on" :"off" ); +if (!vf->link_up) +iavf_dev_tx_drain (dev); }/*
@@ -341,6 +343,8 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +if (adapter->devargs.no_poll_on_link_down) +iavf_dev_tx_drain (vf->eth_dev);iavf_dev_event_post (vf->eth_dev,RTE_ETH_EVENT_INTR_RESET ,NULL ,0 );
@@ -579,6 +583,9 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +iavf_dev_watchdog_enable (adapter); +if (adapter->devargs.no_poll_on_link_down) +iavf_dev_tx_drain (dev);iavf_dev_event_post (dev,RTE_ETH_EVENT_INTR_RESET ,NULL ,0 ); }
Signature · 1 line
2.34.1
RE: [PATCH 3/5] net/iavf: drain in-flight Tx before reset
Quoted 16 lines
-----Original Message-----
From: Mandal, Anurag <[email protected]>
Sent: 06 August 2026 09:26
To: [email protected]
Cc: Richardson, Bruce <[email protected]>; Medvedkin, Vladimir <[email protected]>; Loftus, Ciara <[email protected]>; Mandal, Anurag <[email protected]>
Subject: [PATCH 3/5] net/iavf: drain in-flight Tx before resetOn a link-down or impending PF reset, in-flight Tx descriptors were left pending when the queues were torn down, which could trigger Malicious Driver Detection (MDD) events and leak descriptors.
Quoted 12 lines
Added
iavf_dev_tx_drain()to let already-posted Tx bursts complete and flush the rings within a bounded budget, and call it on link-down and reset-impending events before teardown, preventing MDD events and descriptor leaks. The drain selects the cleanup routine that matches the active Tx path: the scalar path usesci_tx_xmit_cleanup(), while the vector and CTX paths useci_tx_free_bufs_vec(). This matters because the scalar and vector paths track their software rings differently
(ci_tx_entryvsci_tx_entry_vec) and using the scalar routine on a vector queue would walk the wrong ring and free the wrong mbufs.
I think " The drain selects the cleanup routine that matches the active Tx path" is sufficient detail, you can clip the rest out.
Quoted 24 lines
Signed-off-by Anurag Mandal <[email protected]>--- drivers/net/intel/iavf/iavf_rxtx.c | 104 ++++++++++++++++++++++++++++ drivers/net/intel/iavf/iavf_rxtx.h | 6 ++ drivers/net/intel/iavf/iavf_vchnl.c | 7 ++ 3 files changed, 117 insertions(+)drivers/net/intel/iavf/iavf_rxtx.c +1
-0@@ -32,6 +32,7 @@#include "iavf.h" #include "iavf_rxtx.h" +#include "iavf_rxtx_vec_common.h" #include "iavf_ipsec_crypto.h" #include "rte_pmd_iavf.h" @@ -4025,6 +4026,109 @@ iavf_tx_done_cleanup_full(struct ci_tx_queue *txq, return (int)pkt_cnt;
drivers/net/intel/iavf/iavf_rxtx.c iavf_tx_done_cleanup_full(struct ci_tx_queue · line 4037
Quoted 10 lines
drivers/net/intel/iavf/iavf_rxtx.c +7
-0} +/* +* Reclaim completed Tx descriptors for a single queue using the cleanup +* routine that matches the active Tx path. +* The scalar and vector paths track their software rings differently +* (ci_tx_entry vs ci_tx_entry_vec) and keep separate completion +* bookkeeping, so using the scalar routine on a vector queue +* (or vice versa) would free the wrong mbufs.
I think the above sentence can be removed (verbose).
Quoted 24 lines
drivers/net/intel/iavf/iavf_rxtx.c +24
-0+* Returns true if any descriptors were reclaimed. +*/ +static bool +iavf_tx_drain_cleanup (struct ci_tx_queue *txq, +enum iavf_tx_func_type tx_func_type) +{ +switch (tx_func_type) { +case IAVF_TX_AVX2_CTX : +case IAVF_TX_AVX2_CTX_OFFLOAD : +case IAVF_TX_AVX512_CTX : +case IAVF_TX_AVX512_CTX_OFFLOAD : +return ci_tx_free_bufs_vec (txq, iavf_tx_desc_done, true) !=0 ; +case IAVF_TX_NEON : +case IAVF_TX_AVX2 : +case IAVF_TX_AVX2_OFFLOAD : +case IAVF_TX_AVX512 : +case IAVF_TX_AVX512_OFFLOAD : +return ci_tx_free_bufs_vec (txq, iavf_tx_desc_done, false) !=0 ; +case IAVF_TX_DEFAULT : +default : +return ci_tx_xmit_cleanup (txq) ==0 ; + } +} +
[snip]
Quoted 12 lines
drivers/net/intel/iavf/iavf_rxtx.c +2
-0/* @@ -341,6 +343,8 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len, if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +if (adapter->devargs.no_poll_on_link_down) +iavf_dev_tx_drain (vf->eth_dev); iavf_dev_event_post(vf->eth_dev, RTE_ETH_EVENT_INTR_RESET,drivers/net/intel/iavf/iavf_rxtx.c +0
-0NULL, 0);
drivers/net/intel/iavf/iavf_rxtx.c iavf_handle_pf_event_msg(struct rte_eth_dev *dev, · line 587
Quoted 5 lines
drivers/net/intel/iavf/iavf_rxtx.c +1
-0@@ -579,6 +583,9 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg, if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +iavf_dev_watchdog_enable (adapter);
Is this enabling of the watchdog intended?
Quoted 8 lines
drivers/net/intel/iavf/iavf_rxtx.c +2
-0+if (adapter->devargs.no_poll_on_link_down) +iavf_dev_tx_drain (dev); iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,drivers/net/intel/iavf/iavf_rxtx.c +0
-1NULL, 0); } --2.34.1
PATCH 4/5 net/iavf: change no_poll flag to atomic
In reply to Anurag MandalThe no_poll gate that pauses the Rx/Tx data path during reset and link-down was a plain bool written on the control path and read on the data-plane lcores without synchronization, allowing stale reads that either keep dropping traffic or trigger spurious reset detection.
Made no_poll an RTE_ATOMIC(bool) and access it with release stores and acquire loads so data-plane lcores reliably observe gate changes.
--- drivers/net/intel/iavf/iavf.h | 2 +- drivers/net/intel/iavf/iavf_ethdev.c | 6 +++++- drivers/net/intel/iavf/iavf_rxtx.c | 8 ++++++-- drivers/net/intel/iavf/iavf_vchnl.c | 4 +++- 4 files changed, 15 insertions(+), 5 deletions(-)
drivers/net/intel/iavf/iavf.h +1 -1
@@ -392,7 +392,7 @@ struct iavf_adapter { alignas(RTE_CACHE_LINE_MIN_SIZE )uint32_t ptype_tbl[IAVF_MAX_PKT_TYPE ];bool stopped;bool closed; -bool no_poll; +RTE_ATOMIC (bool )no_poll;enum iavf_rx_func_type rx_func_type;enum iavf_tx_func_type tx_func_type;uint16_t fdir_ref_cnt;
drivers/net/intel/iavf/iavf_ethdev.c +5 -1
@@ -3604,9 +3604,13 @@ void iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change) {struct iavf_info *vf = &adapter->vf; +bool no_poll; - adapter->no_poll = (link_change & !vf->link_up) || + no_poll = (link_change & !vf->link_up) || vf->vf_reset || vf->in_reset_recovery; + +rte_atomic_store_explicit (&adapter->no_poll, no_poll, + rte_memory_order_release); } static int
drivers/net/intel/iavf/iavf_rxtx.c +6 -2
@@ -3723,7 +3723,9 @@ iavf_recv_pkts_no_poll(void *rx_queue, struct rte_mbuf **rx_pkts,struct ci_rx_queue *rxq = rx_queue;enum iavf_rx_func_type rx_func_type; -if (!rxq->iavf_vsi || rxq->iavf_vsi->adapter->no_poll) +if (!rxq->iavf_vsi || +rte_atomic_load_explicit (&rxq->iavf_vsi->adapter->no_poll, + rte_memory_order_acquire))return 0 ; rx_func_type = rxq->iavf_vsi->adapter->rx_func_type;
@@ -3739,7 +3741,9 @@ iavf_xmit_pkts_no_poll(void *tx_queue, struct rte_mbuf **tx_pkts,struct ci_tx_queue *txq = tx_queue;enum iavf_tx_func_type tx_func_type; -if (!txq->iavf_vsi || txq->iavf_vsi->adapter->no_poll) +if (!txq->iavf_vsi || +rte_atomic_load_explicit (&txq->iavf_vsi->adapter->no_poll, + rte_memory_order_acquire))return 0 ; tx_func_type = txq->iavf_vsi->adapter->tx_func_type;
drivers/net/intel/iavf/iavf_vchnl.c +3 -1
@@ -268,7 +268,9 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,if (adapter->devargs.no_poll_on_link_down) {iavf_set_no_poll (adapter, true);PMD_DRV_LOG (DEBUG ,"VF no poll turned %s" , - adapter->no_poll ?"on" :"off" ); +rte_atomic_load_explicit (&adapter->no_poll, + rte_memory_order_relaxed) ? +"on" :"off" );if (!vf->link_up)iavf_dev_tx_drain (dev); }
Signature · 1 line
2.34.1
RE: [PATCH 4/5] net/iavf: change no_poll flag to atomic
Quoted 7 lines
Subject: [PATCH 4/5] net/iavf: change
no_pollflag to atomicThe
no_pollgate that pauses the Rx/Tx data path during reset and link-down was a plain bool written on the control path and read on the data-plane lcores without synchronization, allowing stale reads that either keep dropping traffic or trigger spurious reset detection.
Quoted 7 lines
Made
no_pollanRTE_ATOMIC(bool)and access it with release stores and acquire loads so data-plane lcores reliably observe gate changes.Fixes 5b3124a0a6ef ("net/iavf: support no polling when link down")Signed-off-by Anurag Mandal <[email protected]>
Minor nit below. Other than that
Quoted 12 lines
--- drivers/net/intel/iavf/iavf.h | 2 +- drivers/net/intel/iavf/iavf_ethdev.c | 6 +++++- drivers/net/intel/iavf/iavf_rxtx.c | 8 ++++++-- drivers/net/intel/iavf/iavf_vchnl.c | 4 +++- 4 files changed, 15 insertions(+), 5 deletions(-)drivers/net/intel/iavf/iavf.h +0
-0
drivers/net/intel/iavf/iavf.h struct iavf_adapter { · line 396
Quoted 6 lines
drivers/net/intel/iavf/iavf.h +1
-1@@ -392,7 +392,7 @@ struct iavf_adapter { alignas(RTE_CACHE_LINE_MIN_SIZE )uint32_t ptype_tbl[IAVF_MAX_PKT_TYPE ];bool stopped;bool closed; -bool no_poll; +RTE_ATOMIC (bool )no_poll;
Add a space before no_poll
Quoted 72 lines
drivers/net/intel/iavf/iavf.h +0
-0enum iavf_rx_func_type rx_func_type;enum iavf_tx_func_type tx_func_type;drivers/net/intel/iavf/iavf.h +0
-0uint16_t fdir_ref_cnt;drivers/net/intel/iavf/iavf_ethdev.c +5
-1@@ -3604,9 +3604,13 @@ void iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change) {struct iavf_info *vf = &adapter->vf; +bool no_poll; - adapter->no_poll = (link_change & !vf->link_up) || + no_poll = (link_change & !vf->link_up) || vf->vf_reset || vf->in_reset_recovery; + +rte_atomic_store_explicit (&adapter->no_poll, no_poll, + rte_memory_order_release); } static intdrivers/net/intel/iavf/iavf_rxtx.c +3
-1@@ -3723,7 +3723,9 @@ iavf_recv_pkts_no_poll(void *rx_queue, structrte_mbuf **rx_pkts, struct ci_rx_queue *rxq = rx_queue;enum iavf_rx_func_type rx_func_type; -if (!rxq->iavf_vsi || rxq->iavf_vsi->adapter->no_poll) +if (!rxq->iavf_vsi || +rte_atomic_load_explicit (&rxq->iavf_vsi->adapter->no_poll, + rte_memory_order_acquire))return 0 ;drivers/net/intel/iavf/iavf_rxtx.c +3
-1rx_func_type = rxq->iavf_vsi->adapter->rx_func_type;@@ -3739,7 +3741,9 @@ iavf_xmit_pkts_no_poll(void *tx_queue, structrte_mbuf **tx_pkts, struct ci_tx_queue *txq = tx_queue;enum iavf_tx_func_type tx_func_type; -if (!txq->iavf_vsi || txq->iavf_vsi->adapter->no_poll) +if (!txq->iavf_vsi || +rte_atomic_load_explicit (&txq->iavf_vsi->adapter->no_poll, + rte_memory_order_acquire))return 0 ;drivers/net/intel/iavf/iavf_rxtx.c +0
-0tx_func_type = txq->iavf_vsi->adapter->tx_func_type;drivers/net/intel/iavf/iavf_vchnl.c +2
-1@@ -268,7 +268,9 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev, if (adapter->devargs.no_poll_on_link_down) {iavf_set_no_poll (adapter, true); PMD_DRV_LOG(DEBUG, "VF no poll turned %s", - adapter->no_poll ?"on" :"off" ); + rte_atomic_load_explicit(&adapter->no_poll, +
rte_memory_order_relaxed) ?drivers/net/intel/iavf/iavf_vchnl.c +1
-1+ "on" : "off"); if (!vf->link_up)iavf_dev_tx_drain (dev); } --2.34.1
PATCH 5/5 net/iavf: improve VF reset detection on fast ARQ flip
In reply to Anurag MandalDuring PF-initiated reset or a remote/ToR switch link-flap, the PF toggles the admin receive queue enable bit (ARQLEN1) so quickly around a VF reset that the VF's sampling window misses it, leaving the PF and the VF states out of sync and the data path stalled.
Complement the ARQLEN1 check with VFGEN_RSTAT (VIRTCHNL_VFR_INPROGRESS) and shorten the poll interval to 5 ms (with a proportionally larger count, keeping the ~10 s budget), matching the Linux kernel iavf driver. When the VFR is still not observed, proceed with recovery instead of bailing out so the PF and the VF states converge.
--- drivers/net/intel/iavf/iavf.h | 1 + drivers/net/intel/iavf/iavf_ethdev.c | 41 +++++++++++++++++++++------- 2 files changed, 32 insertions(+), 10 deletions(-)
drivers/net/intel/iavf/iavf.h +1 -0
@@ -21,6 +21,7 @@#define IAVF_AQ_BUF_SZ 4096#define IAVF_RESET_WAIT_CNT 2000#define IAVF_RESET_DETECTED_CNT 500 +#define IAVF_RESET_POLL_SCALE 4/* Poll-interval scale for reset detection */ #define IAVF_BUF_SIZE_MIN 1024#define IAVF_FRAME_SIZE_MAX 9728#define IAVF_QUEUE_BASE_ADDR_UNIT 128
drivers/net/intel/iavf/iavf_ethdev.c +31 -10
67 lines not on this page · raw message
Signature · 1 line
2.34.1
RE: [PATCH 5/5] net/iavf: improve VF reset detection on fast ARQ flip
Quoted 44 lines
Subject: [PATCH 5/5] net/iavf: improve VF reset detection on fast ARQ flip
During PF-initiated reset or a remote/ToR switch link-flap, the PF toggles the admin receive queue enable bit (ARQLEN1) so quickly around a VF reset that the VF's sampling window misses it, leaving the PF and the VF states out of sync and the data path stalled.
Complement the ARQLEN1 check with
VFGEN_RSTAT(VIRTCHNL_VFR_INPROGRESS) and shorten the poll interval to 5 ms (with a proportionally larger count, keeping the ~10 s budget), matching the Linux kernel iavf driver. When the VFR is still not observed, proceed with recovery instead of bailing out so the PF and the VF states converge.Fixes ada64daa1a5b ("net/iavf: fix VF reset for flow director rule")Fixes 80fb3c920458 ("net/iavf: fix crash on VF start")Signed-off-by Anurag Mandal <[email protected]>--- drivers/net/intel/iavf/iavf.h | 1 + drivers/net/intel/iavf/iavf_ethdev.c | 41 +++++++++++++++++++++------- 2 files changed, 32 insertions(+), 10 deletions(-)drivers/net/intel/iavf/iavf.h +1
-0@@ -21,6 +21,7 @@#define IAVF_AQ_BUF_SZ 4096#define IAVF_RESET_WAIT_CNT 2000#define IAVF_RESET_DETECTED_CNT 500 +#define IAVF_RESET_POLL_SCALE 4/* Poll-interval scale for reset detection */ #define IAVF_BUF_SIZE_MIN 1024#define IAVF_FRAME_SIZE_MAX 9728#define
IAVF_QUEUE_BASE_ADDR_UNIT128drivers/net/intel/iavf/iavf_ethdev.c +0
-0
drivers/net/intel/iavf/iavf_ethdev.c iavf_dev_close(struct rte_eth_dev *dev) · line 3240
Quoted 7 lines
drivers/net/intel/iavf/iavf_ethdev.c +3
-1@@ -3235,7 +3235,9 @@ iavf_dev_close(struct rte_eth_dev *dev)/* remove RSS configuration */ iavf_hash_uninit (adapter); -iavf_flow_flush (dev,NULL ); +/* Skip the virtchnl-emitting teardown on a PF-initiated reset */ +if (!vf->pf_reset_in_progress) +iavf_flow_flush (dev,NULL );
This seems like a valid change but unrelated to the rest of the commit. Consider splitting it out.
Quoted 69 lines
drivers/net/intel/iavf/iavf_ethdev.c +26
-5iavf_flow_uninit (adapter);/* @@ -3358,8 +3360,26 @@ iavf_dev_reset(struct rte_eth_dev *dev)static inline bool iavf_is_reset (struct iavf_hw *hw) { -return !(IAVF_READ_REG (hw,IAVF_VF_ARQLEN1 ) & -IAVF_VF_ARQLEN1_ARQENABLE_MASK ); +uint32_t rstat; + +/* ARQ has been disabled by the PF as part of the VFR. */ +if (!(IAVF_READ_REG (hw,IAVF_VF_ARQLEN1 ) & +IAVF_VF_ARQLEN1_ARQENABLE_MASK )) +return true; + +/* +* VFGEN_RSTAT reports VIRTCHNL_VFR_INPROGRESS. +* At times, the PF flips ARQENABLE so quickly +* around a VFR that the ARQLEN1 sample window +* misses it. Using VFGEN_RSTAT, as a +* complementary indicator, prevents from +* missing a reset that really did happen. +*/ + rstat = (IAVF_READ_REG (hw,IAVF_VFGEN_RSTAT ) & +IAVF_VFGEN_RSTAT_VFR_STATE_MASK ) >> +IAVF_VFGEN_RSTAT_VFR_STATE_SHIFT ; + +return rstat ==VIRTCHNL_VFR_INPROGRESS ; } static bool@@ -3368,11 +3388,14 @@ iavf_is_reset_detected(struct iavf_adapter *adapter)struct iavf_hw *hw =IAVF_DEV_PRIVATE_TO_HW (adapter);int i; -/* poll until we see the reset actually happen */ -for (i =0 ; i <IAVF_RESET_DETECTED_CNT ; i++) { +/* +* Poll until the reset actually happen. +* Poll every 5 ms to catch the fast ARQ flips. +*/ +for (i =0 ; i <IAVF_RESET_DETECTED_CNT *IAVF_RESET_POLL_SCALE ; i++) {if (iavf_is_reset (hw))return true; -rte_delay_ms (20 ); +rte_delay_us (5000 ); }drivers/net/intel/iavf/iavf_ethdev.c +2
-4return false;@@ -3423,10 +3446,8 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)if (!dev->data->dev_started)return ; -if (!iavf_is_reset_detected (adapter)) { -PMD_DRV_LOG (DEBUG ,"reset not start" ); -return ; - } + if (!iavf_is_reset_detected(adapter)) + PMD_DRV_LOG(WARNING, "VFR not observed; recovering anyway"); }
vf->in_reset_recovery= true;
--
2.34.1
v2 cover net/iavf: harden reset recovery and data path on link flap
In reply to Anurag MandalV2-changes:
- Addressed Ciara Loftus's review comments.
- Split Watchdog and flow flush from other patches.
- Shortened multiple comments and commit msg
- Prevented iavf_resume_pending_start() looping
During PF-initiated reset or a remote/ToR switch link-flap, the VF might miss the reset event, race on the no_poll gate, leak in-flight Tx descriptors, and stay down if dev_start ran before the PF VSI was ready.
This patch series build on the earlier reset-recovery fixes with the following:
- Reset detection: complement the ARQLEN1 check with VFGEN_RSTAT (VIRTCHNL_VFR_INPROGRESS) and poll at a 5 ms interval, matching the kernel iavf driver, so fast ARQ flips are not missed. When the VFR is still not observed, recover anyway instead of bailing out, keeping PF and VF state in sync.
- no_poll: make the flag atomic (RTE_ATOMIC) with release/acquire ordering so the data-plane lcores observe gate changes reliably.
- Tx drain: add iavf_dev_tx_drain() to flush in-flight Tx descriptors on link-down and impending-reset events before teardown, preventing MDD events and descriptor leaks.
- Deferred start: when dev_start fails during recovery (PF VSI inactive), defer it via start_pending and resume on the next link-up event so the VF comes back without manual intervention.
- AdminQ: discard zeroed (opcode 0) descriptors seen during PF-initiated resets to avoid the "Request 0 is not supported" log flood.
- Watchdog: keep watchdog armed for the whole reset window
- Flow flush: skip flow flush during PF-initiated reset
Anurag Mandal (7):
net/iavf: discard empty AdminQ descriptors on reset net/iavf: defer device start when PF VSI not ready net/iavf: drain in-flight Tx before reset net/iavf: change no_poll flag to atomic net/iavf: improve VF reset detection on fast ARQ flip net/iavf: keep watchdog armed for the whole reset window net/iavf: skip flow flush during PF-initiated reset
drivers/net/intel/iavf/iavf.h | 5 +- drivers/net/intel/iavf/iavf_ethdev.c | 108 +++++++++++++++++++++++---- drivers/net/intel/iavf/iavf_rxtx.c | 108 ++++++++++++++++++++++++++- drivers/net/intel/iavf/iavf_rxtx.h | 6 ++ drivers/net/intel/iavf/iavf_vchnl.c | 21 +++++- 5 files changed, 228 insertions(+), 20 deletions(-)
Signature · 1 line
2.34.1
PATCH v2 1/7 net/iavf: discard empty AdminQ descriptors on reset
During PF-initiated resets iavf_clean_arq_element() has been observed to return success with a fully zeroed descriptor (opcode 0). These fell through to the default arm of the dispatch switch and produced a "Request 0 is not supported yet" log flood in the field.
Skip descriptors with opcode 0 so they are silently discarded and do not flood log.
--- drivers/net/intel/iavf/iavf_vchnl.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/net/intel/iavf/iavf_vchnl.c +5 -0
@@ -614,6 +614,11 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)break ; } aq_opc =rte_le_to_cpu_16 (info.desc.opcode); + +/* opcode 0 means the descriptor is empty/zeroed, skip it */ +if (aq_opc ==0 ) +continue ; +/* For the message sent from pf to vf, opcode is stored in * cookie_high of struct iavf_aq_desc, while return error code * are stored in cookie_low, Which is done by PF driver.
Signature · 1 line
2.34.1
RE: [PATCH v2 1/7] net/iavf: discard empty AdminQ descriptors on reset
Quoted 11 lines
Subject: [PATCH v2 1/7] net/iavf: discard empty AdminQ descriptors on reset
During PF-initiated resets
iavf_clean_arq_element()has been observed to return success with a fully zeroed descriptor (opcode 0). These fell through to the default arm of the dispatch switch and produced a "Request 0 is not supported yet" log flood in the field.Skip descriptors with opcode 0 so they are silently discarded and do not flood log.
Signed-off-by Anurag Mandal <[email protected]>
Quoted 23 lines
--- drivers/net/intel/iavf/iavf_vchnl.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/net/intel/iavf/iavf_vchnl.c +5
-1@@ -614,6 +614,11 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)break ; } aq_opc =rte_le_to_cpu_16 (info.desc.opcode); + +/* opcode 0 means the descriptor is empty/zeroed, skip it */ +if (aq_opc ==0 ) +continue ; +/* For the message sent from pf to vf, opcode is stored in * cookie_high of struct iavf_aq_desc, while return error code * are stored in cookie_low, Which is done by PF driver. -- 2.34.1
PATCH v2 2/7 net/iavf: defer device start when PF VSI not ready
In reply to Anurag MandalDuring reset recovery iavf_dev_start() might fail (typically -EIO from VIRTCHNL_OP_CONFIG_VSI_QUEUES) because the PF VSI is not yet active, leaving the VF down and requiring manual intervention to recover.
Added a start_pending flag: when device start fails during recovery, defer it instead of erroring out and resume it from newly added iavf_resume_pending_start() on the next link-up event, so the VF comes back automatically.
--- drivers/net/intel/iavf/iavf.h | 2 + drivers/net/intel/iavf/iavf_ethdev.c | 61 ++++++++++++++++++++++++++-- drivers/net/intel/iavf/iavf_vchnl.c | 3 ++ 3 files changed, 62 insertions(+), 4 deletions(-)
drivers/net/intel/iavf/iavf.h +2 -0
@@ -293,6 +293,7 @@ struct iavf_info {bool in_reset_recovery;bool reset_pending;bool pf_reset_in_progress; +bool start_pending;uint32_t ptp_caps;rte_spinlock_t phc_time_aq_lock;
@@ -533,4 +534,5 @@ 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);void iavf_hash_uninit (struct iavf_adapter *ad); +void iavf_resume_pending_start (struct rte_eth_dev *dev);#endif /* _IAVF_ETHDEV_H_ */
drivers/net/intel/iavf/iavf_ethdev.c +57 -4
91 lines not on this page · raw message
drivers/net/intel/iavf/iavf_vchnl.c +3 -0
@@ -271,6 +271,9 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev, adapter->no_poll ? "on" : "off"); } +/* Retry a dev_start deferred by reset recovery, before the LSC post. */ +iavf_resume_pending_start (dev); +iavf_dev_event_post (dev,RTE_ETH_EVENT_INTR_LSC ,NULL ,0 ); PMD_DRV_LOG(INFO, "Link status update:%s",
Signature · 1 line
2.34.1
RE: [PATCH v2 2/7] net/iavf: defer device start when PF VSI not ready
Quoted 7 lines
Subject: [PATCH v2 2/7] net/iavf: defer device start when PF VSI not ready
During reset recovery
iavf_dev_start()might fail (typically-EIOfromVIRTCHNL_OP_CONFIG_VSI_QUEUES) because the PF VSI is not yet active, leaving the VF down and requiring manual intervention to recover.
Quoted 6 lines
Added a
start_pendingflag: when device start fails during recovery, defer it instead of erroring out and resume it from newly addediavf_resume_pending_start()on the next link-up event, so the VF comes back automatically.Fixes 105d9fcefbee ("net/iavf: fix race condition on link status change")
I don't think this is the correct Fixes tag. It seems like an issue that has been present since the recovery path was introduced. Maybe:3e6a5d2d310a ("net/iavf: add devargs to enable VF auto-reset") ?
Quoted 3 lines
Signed-off-by Anurag Mandal <[email protected]>---
Re: [PATCH v2 2/7] net/iavf: defer device start when PF VSI not ready
Quoted 12 lines · Ciara
Quoted 12 lines
Subject: [PATCH v2 2/7] net/iavf: defer device start when PF VSI not ready
During reset recovery
iavf_dev_start()might fail (typically-EIOfromVIRTCHNL_OP_CONFIG_VSI_QUEUES) because the PF VSI is not yet active, leaving the VF down and requiring manual intervention to recover.Added a
start_pendingflag: when device start fails during recovery, defer it instead of erroring out and resume it from newly addediavf_resume_pending_start()on the next link-up event, so the VF comes back automatically.
Quoted 10 lines
Quoted 1 line
Fixes 105d9fcefbee ("net/iavf: fix race condition on link status change")I don't think this is the correct Fixes tag. It seems like an issue that has been present since the recovery path was introduced. Maybe:
3e6a5d2d310a("net/iavf: add devargs to enable VF auto-reset") ?Quoted 3 lines
Signed-off-by Anurag Mandal <[email protected]>---
If the rest of the series is ok, I can fix up things like fixes tags and log message changes on commit, so no need to do up a new version just for this alone
/Bruce
PATCH v2 3/7 net/iavf: drain in-flight Tx before reset
In reply to Anurag MandalOn a link-down or impending PF reset, in-flight Tx descriptors were left pending when the queues were torn down, which could trigger Malicious Driver Detection (MDD) events and leak descriptors.
Added iavf_dev_tx_drain() to let already-posted Tx bursts complete and flush the rings within a bounded budget, and call it on link-down and reset-impending events before teardown, preventing MDD events and descriptor leaks. The drain selects the cleanup routine that matches the active Tx path.
--- drivers/net/intel/iavf/iavf_rxtx.c | 100 ++++++++++++++++++++++++++++ drivers/net/intel/iavf/iavf_rxtx.h | 6 ++ drivers/net/intel/iavf/iavf_vchnl.c | 6 ++ 3 files changed, 112 insertions(+)
drivers/net/intel/iavf/iavf_rxtx.c +100 -0
@@ -32,6 +32,7 @@#include "iavf.h" #include "iavf_rxtx.h" +#include "iavf_rxtx_vec_common.h" #include "iavf_ipsec_crypto.h" #include "rte_pmd_iavf.h"
@@ -4025,6 +4026,105 @@ iavf_tx_done_cleanup_full(struct ci_tx_queue *txq,return (int )pkt_cnt; } +/* +* Reclaim completed Tx descriptors for a single queue using the cleanup +* routine that matches the active Tx path. +* Returns true if any descriptors were reclaimed. +*/ +static bool +iavf_tx_drain_cleanup (struct ci_tx_queue *txq, +enum iavf_tx_func_type tx_func_type) +{ +switch (tx_func_type) { +case IAVF_TX_AVX2_CTX : +case IAVF_TX_AVX2_CTX_OFFLOAD : +case IAVF_TX_AVX512_CTX : +case IAVF_TX_AVX512_CTX_OFFLOAD : +return ci_tx_free_bufs_vec (txq, iavf_tx_desc_done, true) !=0 ; +case IAVF_TX_NEON : +case IAVF_TX_AVX2 : +case IAVF_TX_AVX2_OFFLOAD : +case IAVF_TX_AVX512 : +case IAVF_TX_AVX512_OFFLOAD : +return ci_tx_free_bufs_vec (txq, iavf_tx_desc_done, false) !=0 ; +case IAVF_TX_DEFAULT : +default : +return ci_tx_xmit_cleanup (txq) ==0 ; + } +} + +/* +* iavf_dev_tx_drain - drain in-flight Tx descriptors after a link-down or +* impending PF reset event. +*/ +void +iavf_dev_tx_drain (struct rte_eth_dev *dev) +{ +struct iavf_adapter *adapter = +IAVF_DEV_PRIVATE_TO_ADAPTER (dev->data->dev_private); +enum iavf_tx_func_type tx_func_type = adapter->tx_func_type; +struct ci_tx_queue *txq; +uint64_t hz, deadline; +int idle_iters =0 ; +uint16_t qid; + +/* +* Allow any Tx burst already in flight on a data-plane lcore to +* write its remaining descriptors and notify. After +* this window, the no_poll gate set by the caller is observed at +* the next burst-entry and no new descriptors will be posted. +*/ +rte_delay_us_block (IAVF_TX_DRAIN_SETTLE_US ); + + hz =rte_get_timer_hz (); + deadline =rte_get_timer_cycles () + + (hz *IAVF_TX_DRAIN_TIMEOUT_US ) /1000000ULL ; + +while (rte_get_timer_cycles () < deadline) { +bool any_pending = false; +bool any_progress = false; + +for (qid =0 ; qid < dev->data->nb_tx_queues; qid++) { + txq = dev->data->tx_queues[qid]; +if (txq ==NULL || + dev->data->tx_queue_state[qid] != +RTE_ETH_QUEUE_STATE_STARTED ) +continue ; + +/* +* nb_tx_free == nb_tx_desc - 1 means the ring is +* empty (one descriptor is always reserved). +*/ +if (txq->nb_tx_free >= txq->nb_tx_desc -1 ) +continue ; + + any_pending = true; +if (iavf_tx_drain_cleanup (txq, tx_func_type)) + any_progress = true; + } + +if (!any_pending) +return ; + +if (any_progress) { + idle_iters =0 ; + }else if (++idle_iters >=IAVF_TX_DRAIN_IDLE_MAX ) { +/* +* HW has not advanced the RS-bit write-back for +* several polling intervals; either the queue is +* quiescent except for the sub-rs_thresh tail +* (which we cannot observe here) or HW is no +* longer fetching. Further polling is unlikely to +* help, and the PF teardown path has its own +* grace period for the remainder. +*/ +break ; + } + +rte_delay_us_block (IAVF_TX_DRAIN_POLL_US ); + } +} +int iavf_dev_tx_done_cleanup (void *txq,uint32_t free_cnt) {
drivers/net/intel/iavf/iavf_rxtx.h +6 -0
@@ -506,6 +506,11 @@ enum iavf_tx_ctx_desc_tunnel_l4_tunnel_type {/* Valid indicator bit for the time_stamp_low field */ #define IAVF_RX_FLX_DESC_TS_VALID (0x1UL) +#define IAVF_TX_DRAIN_TIMEOUT_US 10000/* total drain budget: 10 ms */ +#define IAVF_TX_DRAIN_SETTLE_US 100/* let in-flight burst land */ +#define IAVF_TX_DRAIN_POLL_US 50/* poll interval */ +#define IAVF_TX_DRAIN_IDLE_MAX 20/* ~1 ms of no RS write-back */ + int iavf_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, uint16_t nb_desc,
@@ -641,6 +646,7 @@ void iavf_set_default_ptype_table(struct rte_eth_dev *dev);void iavf_rx_queue_release_mbufs_vec (struct ci_rx_queue *rxq);void iavf_rx_queue_release_mbufs_neon (struct ci_rx_queue *rxq);enum rte_vect_max_simd iavf_get_max_simd_bitwidth (void ); +void iavf_dev_tx_drain (struct rte_eth_dev *dev); static inline void iavf_dump_rx_descriptor(struct ci_rx_queue *rxq,
drivers/net/intel/iavf/iavf_vchnl.c +6 -0
@@ -269,6 +269,8 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,iavf_set_no_poll (adapter, true);PMD_DRV_LOG (DEBUG ,"VF no poll turned %s" , adapter->no_poll ?"on" :"off" ); +if (!vf->link_up) +iavf_dev_tx_drain (dev); }/* Retry a dev_start deferred by reset recovery, before the LSC post. */
@@ -330,6 +332,8 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +if (adapter->devargs.no_poll_on_link_down) +iavf_dev_tx_drain (vf->eth_dev);iavf_dev_event_post (vf->eth_dev,RTE_ETH_EVENT_INTR_RESET ,NULL ,0 );
@@ -568,6 +572,8 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +if (adapter->devargs.no_poll_on_link_down) +iavf_dev_tx_drain (dev);iavf_dev_event_post (dev,RTE_ETH_EVENT_INTR_RESET ,NULL ,0 ); }
Signature · 1 line
2.34.1
RE: [PATCH v2 3/7] net/iavf: drain in-flight Tx before reset
Quoted 7 lines
Subject: [PATCH v2 3/7] net/iavf: drain in-flight Tx before reset
On a link-down or impending PF reset, in-flight Tx descriptors were left pending when the queues were torn down, which could trigger Malicious Driver Detection (MDD) events and leak descriptors.
Quoted 8 lines
Added
iavf_dev_tx_drain()to let already-posted Tx bursts complete and flush the rings within a bounded budget, and call it on link-down and reset-impending events before teardown, preventing MDD events and descriptor leaks. The drain selects the cleanup routine that matches the active Tx path.Signed-off-by Anurag Mandal <[email protected]>
PATCH v2 4/7 net/iavf: change no_poll flag to atomic
In reply to Anurag MandalThe no_poll gate that pauses the Rx/Tx data path during reset and link-down was a plain bool written on the control path and read on the data-plane lcores without synchronization, allowing stale reads that either keep dropping traffic or trigger spurious reset detection.
Made no_poll an RTE_ATOMIC(bool) and access it with release stores and acquire loads so data-plane lcores reliably observe gate changes.
--- drivers/net/intel/iavf/iavf.h | 2 +- drivers/net/intel/iavf/iavf_ethdev.c | 6 +++++- drivers/net/intel/iavf/iavf_rxtx.c | 8 ++++++-- drivers/net/intel/iavf/iavf_vchnl.c | 4 +++- 4 files changed, 15 insertions(+), 5 deletions(-)
drivers/net/intel/iavf/iavf.h +1 -1
@@ -392,7 +392,7 @@ struct iavf_adapter { alignas(RTE_CACHE_LINE_MIN_SIZE )uint32_t ptype_tbl[IAVF_MAX_PKT_TYPE ];bool stopped;bool closed; -bool no_poll; +RTE_ATOMIC (bool ) no_poll;enum iavf_rx_func_type rx_func_type;enum iavf_tx_func_type tx_func_type;uint16_t fdir_ref_cnt;
drivers/net/intel/iavf/iavf_ethdev.c +5 -1
@@ -3604,9 +3604,13 @@ void iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change) {struct iavf_info *vf = &adapter->vf; +bool no_poll; - adapter->no_poll = (link_change & !vf->link_up) || + no_poll = (link_change & !vf->link_up) || vf->vf_reset || vf->in_reset_recovery; + +rte_atomic_store_explicit (&adapter->no_poll, no_poll, + rte_memory_order_release); } static int
drivers/net/intel/iavf/iavf_rxtx.c +6 -2
@@ -3723,7 +3723,9 @@ iavf_recv_pkts_no_poll(void *rx_queue, struct rte_mbuf **rx_pkts,struct ci_rx_queue *rxq = rx_queue;enum iavf_rx_func_type rx_func_type; -if (!rxq->iavf_vsi || rxq->iavf_vsi->adapter->no_poll) +if (!rxq->iavf_vsi || +rte_atomic_load_explicit (&rxq->iavf_vsi->adapter->no_poll, + rte_memory_order_acquire))return 0 ; rx_func_type = rxq->iavf_vsi->adapter->rx_func_type;
@@ -3739,7 +3741,9 @@ iavf_xmit_pkts_no_poll(void *tx_queue, struct rte_mbuf **tx_pkts,struct ci_tx_queue *txq = tx_queue;enum iavf_tx_func_type tx_func_type; -if (!txq->iavf_vsi || txq->iavf_vsi->adapter->no_poll) +if (!txq->iavf_vsi || +rte_atomic_load_explicit (&txq->iavf_vsi->adapter->no_poll, + rte_memory_order_acquire))return 0 ; tx_func_type = txq->iavf_vsi->adapter->tx_func_type;
drivers/net/intel/iavf/iavf_vchnl.c +3 -1
@@ -268,7 +268,9 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,if (adapter->devargs.no_poll_on_link_down) {iavf_set_no_poll (adapter, true);PMD_DRV_LOG (DEBUG ,"VF no poll turned %s" , - adapter->no_poll ?"on" :"off" ); +rte_atomic_load_explicit (&adapter->no_poll, + rte_memory_order_relaxed) ? +"on" :"off" );if (!vf->link_up)iavf_dev_tx_drain (dev); }
Signature · 1 line
2.34.1
PATCH v2 5/7 net/iavf: improve VF reset detection on fast ARQ flip
In reply to Anurag MandalDuring PF-initiated reset or a remote/ToR switch link-flap, the PF toggles the admin receive queue enable bit (ARQLEN1) so quickly around a VF reset that the VF's sampling window misses it, leaving the PF and the VF states out of sync and the data path stalled.
Complement the ARQLEN1 check with VFGEN_RSTAT (VIRTCHNL_VFR_INPROGRESS) and shorten the poll interval to 5 ms (with a proportionally larger count, keeping the ~10 s budget), matching the Linux kernel iavf driver. When the VFR is still not observed, proceed with recovery instead of bailing out so the PF and the VF states converge.
--- drivers/net/intel/iavf/iavf.h | 1 + drivers/net/intel/iavf/iavf_ethdev.c | 37 +++++++++++++++++++++------- 2 files changed, 29 insertions(+), 9 deletions(-)
drivers/net/intel/iavf/iavf.h +1 -0
@@ -21,6 +21,7 @@#define IAVF_AQ_BUF_SZ 4096#define IAVF_RESET_WAIT_CNT 2000#define IAVF_RESET_DETECTED_CNT 500 +#define IAVF_RESET_POLL_SCALE 4/* Poll-interval scale for reset detection */ #define IAVF_BUF_SIZE_MIN 1024#define IAVF_FRAME_SIZE_MAX 9728#define IAVF_QUEUE_BASE_ADDR_UNIT 128
drivers/net/intel/iavf/iavf_ethdev.c +28 -9
@@ -3358,8 +3358,26 @@ iavf_dev_reset(struct rte_eth_dev *dev)static inline bool iavf_is_reset (struct iavf_hw *hw) { -return !(IAVF_READ_REG (hw,IAVF_VF_ARQLEN1 ) & -IAVF_VF_ARQLEN1_ARQENABLE_MASK ); +uint32_t rstat; + +/* ARQ has been disabled by the PF as part of the VFR. */ +if (!(IAVF_READ_REG (hw,IAVF_VF_ARQLEN1 ) & +IAVF_VF_ARQLEN1_ARQENABLE_MASK )) +return true; + +/* +* VFGEN_RSTAT reports VIRTCHNL_VFR_INPROGRESS. +* At times, the PF flips ARQENABLE so quickly +* around a VFR that the ARQLEN1 sample window +* misses it. Using VFGEN_RSTAT, as a +* complementary indicator, prevents from +* missing a reset that really did happen. +*/ + rstat = (IAVF_READ_REG (hw,IAVF_VFGEN_RSTAT ) & +IAVF_VFGEN_RSTAT_VFR_STATE_MASK ) >> +IAVF_VFGEN_RSTAT_VFR_STATE_SHIFT ; + +return rstat ==VIRTCHNL_VFR_INPROGRESS ; } static bool
@@ -3368,11 +3386,14 @@ iavf_is_reset_detected(struct iavf_adapter *adapter)struct iavf_hw *hw =IAVF_DEV_PRIVATE_TO_HW (adapter);int i; -/* poll until we see the reset actually happen */ -for (i =0 ; i <IAVF_RESET_DETECTED_CNT ; i++) { +/* +* Poll until the reset actually happen. +* Poll every 5 ms to catch the fast ARQ flips. +*/ +for (i =0 ; i <IAVF_RESET_DETECTED_CNT *IAVF_RESET_POLL_SCALE ; i++) {if (iavf_is_reset (hw))return true; -rte_delay_ms (20 ); +rte_delay_us (5000 ); }return false;
@@ -3423,10 +3444,8 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)if (!dev->data->dev_started)return ; -if (!iavf_is_reset_detected (adapter)) { -PMD_DRV_LOG (DEBUG ,"reset not start" ); -return ; - } +if (!iavf_is_reset_detected (adapter)) +PMD_DRV_LOG (WARNING ,"VFR not observed; recovering anyway" ); } vf->in_reset_recovery = true;
Signature · 1 line
2.34.1
RE: [PATCH v2 5/7] net/iavf: improve VF reset detection on fast ARQ flip
Quoted 8 lines
Subject: [PATCH v2 5/7] net/iavf: improve VF reset detection on fast ARQ flip
During PF-initiated reset or a remote/ToR switch link-flap, the PF toggles the admin receive queue enable bit (ARQLEN1) so quickly around a VF reset that the VF's sampling window misses it, leaving the PF and the VF states out of sync and the data path stalled.
Quoted 6 lines
Complement the ARQLEN1 check with
VFGEN_RSTAT(VIRTCHNL_VFR_INPROGRESS) and shorten the poll interval to 5 ms (with a proportionally larger count, keeping the ~10 s budget), matching the Linux kernel iavf driver. When the VFR is still not observed, proceed with recovery instead of bailing out so the PF and the VF states converge.
Double space between "and" and "the"
Quoted 2 lines
Fixes ada64daa1a5b ("net/iavf: fix VF reset for flow director rule")
I think the above Fixes tag can be dropped
Quoted 1 line
Fixes 80fb3c920458 ("net/iavf: fix crash on VF start")
Cc stable?
Quoted 2 lines
Signed-off-by Anurag Mandal <[email protected]>
PATCH v2 6/7 net/iavf: keep watchdog armed for the whole reset window
In reply to Anurag MandalThe device watchdog polls VFGEN_RSTAT to detect a VF reset and drives the auto-reset recovery path.
It is disabled when a link-up event arrives, on the assumption that a working link means no reset is in flight.
That assumption does not hold during recovery.
Keep the watchdog armed while vf->in_reset_recovery is set, and enable it on VIRTCHNL_EVENT_RESET_IMPENDING, so it stays armed from the PF reset notification until recovery completes.
--- drivers/net/intel/iavf/iavf_vchnl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/intel/iavf/iavf_vchnl.c +2 -1
@@ -260,7 +260,7 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,* (link is down or a VF reset is in progress); the watchdog drives * auto-reset recovery, so it must remain armed in those cases. */ -if (vf->link_up && !vf->vf_reset) +if (vf->link_up && !vf->vf_reset && !vf->in_reset_recovery)iavf_dev_watchdog_disable (adapter);else iavf_dev_watchdog_enable (adapter);
@@ -574,6 +574,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +iavf_dev_watchdog_enable (adapter);if (adapter->devargs.no_poll_on_link_down)iavf_dev_tx_drain (dev); iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
Signature · 1 line
2.34.1
RE: [PATCH v2 6/7] net/iavf: keep watchdog armed for the whole reset window
Quoted 9 lines
Subject: [PATCH v2 6/7] net/iavf: keep watchdog armed for the whole reset window
The device watchdog polls
VFGEN_RSTATto detect a VF reset and drives the auto-reset recovery path.
It is disabled when a link-up event arrives, on the assumption that a working link means no reset is in flight.
That assumption does not hold during recovery.
Quoted 3 lines
Keep the watchdog armed while
vf->in_reset_recoveryis set, and enable it onVIRTCHNL_EVENT_RESET_IMPENDING, so it stays armed from the PF reset notification until recovery completes.
Is there an appropriate fixes tag that could be added to this commit?
Quoted 22 lines
Signed-off-by Anurag Mandal <[email protected]>--- drivers/net/intel/iavf/iavf_vchnl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/net/intel/iavf/iavf_vchnl.c +1
-1@@ -260,7 +260,7 @@ iavf_handle_link_change_event(struct rte_eth_dev*dev, * (link is down or a VF reset is in progress); the watchdog drives * auto-reset recovery, so it must remain armed in those cases. */ -if (vf->link_up && !vf->vf_reset) +if (vf->link_up && !vf->vf_reset && !vf->in_reset_recovery)iavf_dev_watchdog_disable (adapter);else drivers/net/intel/iavf/iavf_vchnl.c +0
-0iavf_dev_watchdog_enable (adapter);
drivers/net/intel/iavf/iavf_vchnl.c iavf_handle_pf_event_msg(struct rte_eth_dev *dev, · line 578
Quoted 5 lines
drivers/net/intel/iavf/iavf_vchnl.c +1
-0@@ -574,6 +574,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg, if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +iavf_dev_watchdog_enable (adapter);
Should the watchdog be enabled in iavf_read_msg_from_pf where the RESET_IMPENDING message is handled also?
Quoted 6 lines
drivers/net/intel/iavf/iavf_vchnl.c +0
-0if (adapter->devargs.no_poll_on_link_down)iavf_dev_tx_drain (dev);iavf_dev_event_post(dev,
RTE_ETH_EVENT_INTR_RESET,
--
2.34.1
PATCH v2 7/7 net/iavf: skip flow flush during PF-initiated reset
In reply to Anurag MandalSkip the virtchnl-emitting teardown on a PF-initiated reset.
--- drivers/net/intel/iavf/iavf_ethdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/net/intel/iavf/iavf_ethdev.c +3 -1
@@ -3235,7 +3235,9 @@ iavf_dev_close(struct rte_eth_dev *dev)/* remove RSS configuration */ iavf_hash_uninit (adapter); -iavf_flow_flush (dev,NULL ); +/* Skip the virtchnl-emitting teardown on a PF-initiated reset */ +if (!vf->pf_reset_in_progress) +iavf_flow_flush (dev,NULL );iavf_flow_uninit (adapter);/*
Signature · 1 line
2.34.1
RE: [PATCH v2 7/7] net/iavf: skip flow flush during PF-initiated reset
Quoted 7 lines
Subject: [PATCH v2 7/7] net/iavf: skip flow flush during PF-initiated reset
Skip the virtchnl-emitting teardown on a PF-initiated reset.
Fixes ada64daa1a5b ("net/iavf: fix VF reset for flow director rule")Signed-off-by Anurag Mandal <[email protected]>
Quoted 22 lines
--- drivers/net/intel/iavf/iavf_ethdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/net/intel/iavf/iavf_ethdev.c +3
-2@@ -3235,7 +3235,9 @@ iavf_dev_close(struct rte_eth_dev *dev)/* remove RSS configuration */ iavf_hash_uninit (adapter); -iavf_flow_flush (dev,NULL ); +/* Skip the virtchnl-emitting teardown on a PF-initiated reset */ +if (!vf->pf_reset_in_progress) +iavf_flow_flush (dev,NULL );iavf_flow_uninit (adapter);/* -- 2.34.1
v3 cover net/iavf: harden reset recovery and data path on link flap
In reply to Anurag MandalV3-changes:
- Addressed Ciara Loftus's review comments on Fixes tag
- Added missing watchdog enable in Patch-6
- Removed extra space in commit msg of Patch-5
V2-changes:
- Addressed Ciara Loftus's review comments.
- Split Watchdog and flow flush from other patches.
- Shortened multiple comments and commit msg
- Prevented iavf_resume_pending_start() looping
During PF-initiated reset or a remote/ToR switch link-flap, the VF might miss the reset event, race on the no_poll gate, leak in-flight Tx descriptors, and stay down if dev_start ran before the PF VSI was ready.
This patch series build on the earlier reset-recovery fixes with the following:
- Reset detection: complement the ARQLEN1 check with VFGEN_RSTAT (VIRTCHNL_VFR_INPROGRESS) and poll at a 5 ms interval, matching the kernel iavf driver, so fast ARQ flips are not missed. When the VFR is still not observed, recover anyway instead of bailing out, keeping PF and VF state in sync.
- no_poll: make the flag atomic (RTE_ATOMIC) with release/acquire ordering so the data-plane lcores observe gate changes reliably.
- Tx drain: add iavf_dev_tx_drain() to flush in-flight Tx descriptors on link-down and impending-reset events before teardown, preventing MDD events and descriptor leaks.
- Deferred start: when dev_start fails during recovery (PF VSI inactive), defer it via start_pending and resume on the next link-up event so the VF comes back without manual intervention.
- AdminQ: discard zeroed (opcode 0) descriptors seen during PF-initiated resets to avoid the "Request 0 is not supported" log flood.
- Watchdog: keep watchdog armed for the whole reset window
- Flow flush: skip flow flush during PF-initiated reset
Anurag Mandal (7):
net/iavf: discard empty AdminQ descriptors on reset net/iavf: defer device start when PF VSI not ready net/iavf: drain in-flight Tx before reset net/iavf: change no_poll flag to atomic net/iavf: improve VF reset detection on fast ARQ flip net/iavf: keep watchdog armed for the whole reset window net/iavf: skip flow flush during PF-initiated reset
drivers/net/intel/iavf/iavf.h | 5 +- drivers/net/intel/iavf/iavf_ethdev.c | 108 +++++++++++++++++++++++---- drivers/net/intel/iavf/iavf_rxtx.c | 108 ++++++++++++++++++++++++++- drivers/net/intel/iavf/iavf_rxtx.h | 6 ++ drivers/net/intel/iavf/iavf_vchnl.c | 22 +++++- 5 files changed, 229 insertions(+), 20 deletions(-)
Signature · 1 line
2.34.1
PATCH v3 1/7 net/iavf: discard empty AdminQ descriptors on reset
During PF-initiated resets iavf_clean_arq_element() has been observed to return success with a fully zeroed descriptor (opcode 0). These fell through to the default arm of the dispatch switch and produced a "Request 0 is not supported yet" log flood in the field.
Skip descriptors with opcode 0 so they are silently discarded and do not flood log.
--- drivers/net/intel/iavf/iavf_vchnl.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/net/intel/iavf/iavf_vchnl.c +5 -0
@@ -614,6 +614,11 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)break ; } aq_opc =rte_le_to_cpu_16 (info.desc.opcode); + +/* opcode 0 means the descriptor is empty/zeroed, skip it */ +if (aq_opc ==0 ) +continue ; +/* For the message sent from pf to vf, opcode is stored in * cookie_high of struct iavf_aq_desc, while return error code * are stored in cookie_low, Which is done by PF driver.
Signature · 1 line
2.34.1
RE: [PATCH v3 1/7] net/iavf: discard empty AdminQ descriptors on reset
Quoted 14 lines
-----Original Message-----
From: Mandal, Anurag <[email protected]>
Sent: 17 August 2026 12:19
To: [email protected]
Cc: Richardson, Bruce <[email protected]>; Medvedkin, Vladimir <[email protected]>; Loftus, Ciara <[email protected]>; Mandal, Anurag <[email protected]>
Subject: [PATCH v3 1/7] net/iavf: discard empty AdminQ descriptors on resetDuring PF-initiated resets
iavf_clean_arq_element()has been observed to return success with a fully zeroed descriptor (opcode 0).
These fell through to the default arm of the dispatch switch and produced a "Request 0 is not supported yet" log flood in the field.
Quoted 4 lines
Skip descriptors with opcode 0 so they are silently discarded and do not flood log.
Signed-off-by Anurag Mandal <[email protected]>
Quoted 24 lines
--- drivers/net/intel/iavf/iavf_vchnl.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/net/intel/iavf/iavf_vchnl.c +5
-0@@ -614,6 +614,11 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)break ; } aq_opc =rte_le_to_cpu_16 (info.desc.opcode); + +/* opcode 0 means the descriptor is empty/zeroed, skip it */ +if (aq_opc ==0 ) +continue ; +/* For the message sent from pf to vf, opcode is stored in * cookie_high of struct iavf_aq_desc, while return error code
* are stored incookie_low, Which is done by PF driver.
--
2.34.1
PATCH v3 2/7 net/iavf: defer device start when PF VSI not ready
In reply to Anurag MandalDuring reset recovery iavf_dev_start() might fail (typically -EIO from VIRTCHNL_OP_CONFIG_VSI_QUEUES) because the PF VSI is not yet active, leaving the VF down and requiring manual intervention to recover.
Added a start_pending flag: when device start fails during recovery, defer it instead of erroring out and resume it from newly added iavf_resume_pending_start() on the next link-up event, so the VF comes back automatically.
--- drivers/net/intel/iavf/iavf.h | 2 + drivers/net/intel/iavf/iavf_ethdev.c | 61 ++++++++++++++++++++++++++-- drivers/net/intel/iavf/iavf_vchnl.c | 3 ++ 3 files changed, 62 insertions(+), 4 deletions(-)
drivers/net/intel/iavf/iavf.h +2 -0
@@ -293,6 +293,7 @@ struct iavf_info {bool in_reset_recovery;bool reset_pending;bool pf_reset_in_progress; +bool start_pending;uint32_t ptp_caps;rte_spinlock_t phc_time_aq_lock;
@@ -533,4 +534,5 @@ 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);void iavf_hash_uninit (struct iavf_adapter *ad); +void iavf_resume_pending_start (struct rte_eth_dev *dev);#endif /* _IAVF_ETHDEV_H_ */
drivers/net/intel/iavf/iavf_ethdev.c +57 -4
91 lines not on this page · raw message
drivers/net/intel/iavf/iavf_vchnl.c +3 -0
@@ -271,6 +271,9 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev, adapter->no_poll ? "on" : "off"); } +/* Retry a dev_start deferred by reset recovery, before the LSC post. */ +iavf_resume_pending_start (dev); +iavf_dev_event_post (dev,RTE_ETH_EVENT_INTR_LSC ,NULL ,0 ); PMD_DRV_LOG(INFO, "Link status update:%s",
Signature · 1 line
2.34.1
RE: [PATCH v3 2/7] net/iavf: defer device start when PF VSI not ready
Quoted 16 lines
-----Original Message-----
From: Mandal, Anurag <[email protected]>
Sent: 17 August 2026 07:49
To: [email protected]
Cc: Richardson, Bruce <[email protected]>; Medvedkin, Vladimir <[email protected]>; Loftus, Ciara <[email protected]>; Mandal, Anurag <[email protected]>
Subject: [PATCH v3 2/7] net/iavf: defer device start when PF VSI not readyDuring reset recovery
iavf_dev_start()might fail (typically-EIOfromVIRTCHNL_OP_CONFIG_VSI_QUEUES) because the PF VSI is not yet active, leaving the VF down and requiring manual intervention to recover.
Quoted 8 lines
Added a
start_pendingflag: when device start fails during recovery, defer it instead of erroring out and resume it from newly addediavf_resume_pending_start()on the next link-up event, so the VF comes back automatically.Fixes 3e6a5d2d310a ("net/iavf: add devargs to enable VF auto-reset")Signed-off-by Anurag Mandal <[email protected]>
Quoted 6 lines
--- drivers/net/intel/iavf/iavf.h | 2 + drivers/net/intel/iavf/iavf_ethdev.c | 61 ++++++++++++++++++++++++++-- drivers/net/intel/iavf/iavf_vchnl.c | 3 ++ 3 files changed, 62 insertions(+), 4 deletions(-)
PATCH v3 3/7 net/iavf: drain in-flight Tx before reset
In reply to Anurag MandalOn a link-down or impending PF reset, in-flight Tx descriptors were left pending when the queues were torn down, which could trigger Malicious Driver Detection (MDD) events and leak descriptors.
Added iavf_dev_tx_drain() to let already-posted Tx bursts complete and flush the rings within a bounded budget, and call it on link-down and reset-impending events before teardown, preventing MDD events and descriptor leaks. The drain selects the cleanup routine that matches the active Tx path.
--- drivers/net/intel/iavf/iavf_rxtx.c | 100 ++++++++++++++++++++++++++++ drivers/net/intel/iavf/iavf_rxtx.h | 6 ++ drivers/net/intel/iavf/iavf_vchnl.c | 6 ++ 3 files changed, 112 insertions(+)
drivers/net/intel/iavf/iavf_rxtx.c +100 -0
@@ -32,6 +32,7 @@#include "iavf.h" #include "iavf_rxtx.h" +#include "iavf_rxtx_vec_common.h" #include "iavf_ipsec_crypto.h" #include "rte_pmd_iavf.h"
@@ -4025,6 +4026,105 @@ iavf_tx_done_cleanup_full(struct ci_tx_queue *txq,return (int )pkt_cnt; } +/* +* Reclaim completed Tx descriptors for a single queue using the cleanup +* routine that matches the active Tx path. +* Returns true if any descriptors were reclaimed. +*/ +static bool +iavf_tx_drain_cleanup (struct ci_tx_queue *txq, +enum iavf_tx_func_type tx_func_type) +{ +switch (tx_func_type) { +case IAVF_TX_AVX2_CTX : +case IAVF_TX_AVX2_CTX_OFFLOAD : +case IAVF_TX_AVX512_CTX : +case IAVF_TX_AVX512_CTX_OFFLOAD : +return ci_tx_free_bufs_vec (txq, iavf_tx_desc_done, true) !=0 ; +case IAVF_TX_NEON : +case IAVF_TX_AVX2 : +case IAVF_TX_AVX2_OFFLOAD : +case IAVF_TX_AVX512 : +case IAVF_TX_AVX512_OFFLOAD : +return ci_tx_free_bufs_vec (txq, iavf_tx_desc_done, false) !=0 ; +case IAVF_TX_DEFAULT : +default : +return ci_tx_xmit_cleanup (txq) ==0 ; + } +} + +/* +* iavf_dev_tx_drain - drain in-flight Tx descriptors after a link-down or +* impending PF reset event. +*/ +void +iavf_dev_tx_drain (struct rte_eth_dev *dev) +{ +struct iavf_adapter *adapter = +IAVF_DEV_PRIVATE_TO_ADAPTER (dev->data->dev_private); +enum iavf_tx_func_type tx_func_type = adapter->tx_func_type; +struct ci_tx_queue *txq; +uint64_t hz, deadline; +int idle_iters =0 ; +uint16_t qid; + +/* +* Allow any Tx burst already in flight on a data-plane lcore to +* write its remaining descriptors and notify. After +* this window, the no_poll gate set by the caller is observed at +* the next burst-entry and no new descriptors will be posted. +*/ +rte_delay_us_block (IAVF_TX_DRAIN_SETTLE_US ); + + hz =rte_get_timer_hz (); + deadline =rte_get_timer_cycles () + + (hz *IAVF_TX_DRAIN_TIMEOUT_US ) /1000000ULL ; + +while (rte_get_timer_cycles () < deadline) { +bool any_pending = false; +bool any_progress = false; + +for (qid =0 ; qid < dev->data->nb_tx_queues; qid++) { + txq = dev->data->tx_queues[qid]; +if (txq ==NULL || + dev->data->tx_queue_state[qid] != +RTE_ETH_QUEUE_STATE_STARTED ) +continue ; + +/* +* nb_tx_free == nb_tx_desc - 1 means the ring is +* empty (one descriptor is always reserved). +*/ +if (txq->nb_tx_free >= txq->nb_tx_desc -1 ) +continue ; + + any_pending = true; +if (iavf_tx_drain_cleanup (txq, tx_func_type)) + any_progress = true; + } + +if (!any_pending) +return ; + +if (any_progress) { + idle_iters =0 ; + }else if (++idle_iters >=IAVF_TX_DRAIN_IDLE_MAX ) { +/* +* HW has not advanced the RS-bit write-back for +* several polling intervals; either the queue is +* quiescent except for the sub-rs_thresh tail +* (which we cannot observe here) or HW is no +* longer fetching. Further polling is unlikely to +* help, and the PF teardown path has its own +* grace period for the remainder. +*/ +break ; + } + +rte_delay_us_block (IAVF_TX_DRAIN_POLL_US ); + } +} +int iavf_dev_tx_done_cleanup (void *txq,uint32_t free_cnt) {
drivers/net/intel/iavf/iavf_rxtx.h +6 -0
@@ -506,6 +506,11 @@ enum iavf_tx_ctx_desc_tunnel_l4_tunnel_type {/* Valid indicator bit for the time_stamp_low field */ #define IAVF_RX_FLX_DESC_TS_VALID (0x1UL) +#define IAVF_TX_DRAIN_TIMEOUT_US 10000/* total drain budget: 10 ms */ +#define IAVF_TX_DRAIN_SETTLE_US 100/* let in-flight burst land */ +#define IAVF_TX_DRAIN_POLL_US 50/* poll interval */ +#define IAVF_TX_DRAIN_IDLE_MAX 20/* ~1 ms of no RS write-back */ + int iavf_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, uint16_t nb_desc,
@@ -641,6 +646,7 @@ void iavf_set_default_ptype_table(struct rte_eth_dev *dev);void iavf_rx_queue_release_mbufs_vec (struct ci_rx_queue *rxq);void iavf_rx_queue_release_mbufs_neon (struct ci_rx_queue *rxq);enum rte_vect_max_simd iavf_get_max_simd_bitwidth (void ); +void iavf_dev_tx_drain (struct rte_eth_dev *dev); static inline void iavf_dump_rx_descriptor(struct ci_rx_queue *rxq,
drivers/net/intel/iavf/iavf_vchnl.c +6 -0
@@ -269,6 +269,8 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,iavf_set_no_poll (adapter, true);PMD_DRV_LOG (DEBUG ,"VF no poll turned %s" , adapter->no_poll ?"on" :"off" ); +if (!vf->link_up) +iavf_dev_tx_drain (dev); }/* Retry a dev_start deferred by reset recovery, before the LSC post. */
@@ -330,6 +332,8 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +if (adapter->devargs.no_poll_on_link_down) +iavf_dev_tx_drain (vf->eth_dev);iavf_dev_event_post (vf->eth_dev,RTE_ETH_EVENT_INTR_RESET ,NULL ,0 );
@@ -568,6 +572,8 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +if (adapter->devargs.no_poll_on_link_down) +iavf_dev_tx_drain (dev);iavf_dev_event_post (dev,RTE_ETH_EVENT_INTR_RESET ,NULL ,0 ); }
Signature · 1 line
2.34.1
Re: [PATCH v3 3/7] net/iavf: drain in-flight Tx before reset
Quoted 5 lines · Anurag Mandal
On a link-down or impending PF reset, in-flight Tx descriptors were left pending when the queues were torn down, which could trigger Malicious Driver Detection (MDD) events and leak descriptors.
I agree that this is a good idea on reset, but what about on link down? In most of our drivers, if the link goes down, the Tx packets are left in the descriptor ring to be sent when the link comes back up again. Can we manage without the cleanup on link down? If the link is down to reconfigure, the cleanup should be done as part of the reconfigure, I would think. Am I missing something?
Also, if this prevents MDD events should it have a fixes tag and be backported?
/Bruce
PATCH v3 4/7 net/iavf: change no_poll flag to atomic
In reply to Anurag MandalThe no_poll gate that pauses the Rx/Tx data path during reset and link-down was a plain bool written on the control path and read on the data-plane lcores without synchronization, allowing stale reads that either keep dropping traffic or trigger spurious reset detection.
Made no_poll an RTE_ATOMIC(bool) and access it with release stores and acquire loads so data-plane lcores reliably observe gate changes.
--- drivers/net/intel/iavf/iavf.h | 2 +- drivers/net/intel/iavf/iavf_ethdev.c | 6 +++++- drivers/net/intel/iavf/iavf_rxtx.c | 8 ++++++-- drivers/net/intel/iavf/iavf_vchnl.c | 4 +++- 4 files changed, 15 insertions(+), 5 deletions(-)
drivers/net/intel/iavf/iavf.h +1 -1
@@ -392,7 +392,7 @@ struct iavf_adapter { alignas(RTE_CACHE_LINE_MIN_SIZE )uint32_t ptype_tbl[IAVF_MAX_PKT_TYPE ];bool stopped;bool closed; -bool no_poll; +RTE_ATOMIC (bool ) no_poll;enum iavf_rx_func_type rx_func_type;enum iavf_tx_func_type tx_func_type;uint16_t fdir_ref_cnt;
drivers/net/intel/iavf/iavf_ethdev.c +5 -1
@@ -3604,9 +3604,13 @@ void iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change) {struct iavf_info *vf = &adapter->vf; +bool no_poll; - adapter->no_poll = (link_change & !vf->link_up) || + no_poll = (link_change & !vf->link_up) || vf->vf_reset || vf->in_reset_recovery; + +rte_atomic_store_explicit (&adapter->no_poll, no_poll, + rte_memory_order_release); } static int
drivers/net/intel/iavf/iavf_rxtx.c +6 -2
@@ -3723,7 +3723,9 @@ iavf_recv_pkts_no_poll(void *rx_queue, struct rte_mbuf **rx_pkts,struct ci_rx_queue *rxq = rx_queue;enum iavf_rx_func_type rx_func_type; -if (!rxq->iavf_vsi || rxq->iavf_vsi->adapter->no_poll) +if (!rxq->iavf_vsi || +rte_atomic_load_explicit (&rxq->iavf_vsi->adapter->no_poll, + rte_memory_order_acquire))return 0 ; rx_func_type = rxq->iavf_vsi->adapter->rx_func_type;
@@ -3739,7 +3741,9 @@ iavf_xmit_pkts_no_poll(void *tx_queue, struct rte_mbuf **tx_pkts,struct ci_tx_queue *txq = tx_queue;enum iavf_tx_func_type tx_func_type; -if (!txq->iavf_vsi || txq->iavf_vsi->adapter->no_poll) +if (!txq->iavf_vsi || +rte_atomic_load_explicit (&txq->iavf_vsi->adapter->no_poll, + rte_memory_order_acquire))return 0 ; tx_func_type = txq->iavf_vsi->adapter->tx_func_type;
drivers/net/intel/iavf/iavf_vchnl.c +3 -1
@@ -268,7 +268,9 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,if (adapter->devargs.no_poll_on_link_down) {iavf_set_no_poll (adapter, true);PMD_DRV_LOG (DEBUG ,"VF no poll turned %s" , - adapter->no_poll ?"on" :"off" ); +rte_atomic_load_explicit (&adapter->no_poll, + rte_memory_order_relaxed) ? +"on" :"off" );if (!vf->link_up)iavf_dev_tx_drain (dev); }
Signature · 1 line
2.34.1
PATCH v3 5/7 net/iavf: improve VF reset detection on fast ARQ flip
In reply to Anurag MandalDuring PF-initiated reset or a remote/ToR switch link-flap, the PF toggles the admin receive queue enable bit (ARQLEN1) so quickly around a VF reset that the VF's sampling window misses it, leaving the PF and the VF states out of sync and the data path stalled.
Complement the ARQLEN1 check with VFGEN_RSTAT (VIRTCHNL_VFR_INPROGRESS) and shorten the poll interval to 5 ms (with a proportionally larger count, keeping the ~10 s budget), matching the Linux kernel iavf driver. When the VFR is still not observed, proceed with recovery instead of bailing out so the PF and the VF states converge.
--- drivers/net/intel/iavf/iavf.h | 1 + drivers/net/intel/iavf/iavf_ethdev.c | 37 +++++++++++++++++++++------- 2 files changed, 29 insertions(+), 9 deletions(-)
drivers/net/intel/iavf/iavf.h +1 -0
@@ -21,6 +21,7 @@#define IAVF_AQ_BUF_SZ 4096#define IAVF_RESET_WAIT_CNT 2000#define IAVF_RESET_DETECTED_CNT 500 +#define IAVF_RESET_POLL_SCALE 4/* Poll-interval scale for reset detection */ #define IAVF_BUF_SIZE_MIN 1024#define IAVF_FRAME_SIZE_MAX 9728#define IAVF_QUEUE_BASE_ADDR_UNIT 128
drivers/net/intel/iavf/iavf_ethdev.c +28 -9
@@ -3358,8 +3358,26 @@ iavf_dev_reset(struct rte_eth_dev *dev)static inline bool iavf_is_reset (struct iavf_hw *hw) { -return !(IAVF_READ_REG (hw,IAVF_VF_ARQLEN1 ) & -IAVF_VF_ARQLEN1_ARQENABLE_MASK ); +uint32_t rstat; + +/* ARQ has been disabled by the PF as part of the VFR. */ +if (!(IAVF_READ_REG (hw,IAVF_VF_ARQLEN1 ) & +IAVF_VF_ARQLEN1_ARQENABLE_MASK )) +return true; + +/* +* VFGEN_RSTAT reports VIRTCHNL_VFR_INPROGRESS. +* At times, the PF flips ARQENABLE so quickly +* around a VFR that the ARQLEN1 sample window +* misses it. Using VFGEN_RSTAT, as a +* complementary indicator, prevents from +* missing a reset that really did happen. +*/ + rstat = (IAVF_READ_REG (hw,IAVF_VFGEN_RSTAT ) & +IAVF_VFGEN_RSTAT_VFR_STATE_MASK ) >> +IAVF_VFGEN_RSTAT_VFR_STATE_SHIFT ; + +return rstat ==VIRTCHNL_VFR_INPROGRESS ; } static bool
@@ -3368,11 +3386,14 @@ iavf_is_reset_detected(struct iavf_adapter *adapter)struct iavf_hw *hw =IAVF_DEV_PRIVATE_TO_HW (adapter);int i; -/* poll until we see the reset actually happen */ -for (i =0 ; i <IAVF_RESET_DETECTED_CNT ; i++) { +/* +* Poll until the reset actually happen. +* Poll every 5 ms to catch the fast ARQ flips. +*/ +for (i =0 ; i <IAVF_RESET_DETECTED_CNT *IAVF_RESET_POLL_SCALE ; i++) {if (iavf_is_reset (hw))return true; -rte_delay_ms (20 ); +rte_delay_us (5000 ); }return false;
@@ -3423,10 +3444,8 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)if (!dev->data->dev_started)return ; -if (!iavf_is_reset_detected (adapter)) { -PMD_DRV_LOG (DEBUG ,"reset not start" ); -return ; - } +if (!iavf_is_reset_detected (adapter)) +PMD_DRV_LOG (WARNING ,"VFR not observed; recovering anyway" ); } vf->in_reset_recovery = true;
Signature · 1 line
2.34.1
PATCH v3 6/7 net/iavf: keep watchdog armed for the whole reset window
In reply to Anurag MandalThe device watchdog polls VFGEN_RSTAT to detect a VF reset and drives the auto-reset recovery path.
It is disabled when a link-up event arrives, on the assumption that a working link means no reset is in flight.
That assumption does not hold during recovery.
Keep the watchdog armed while vf->in_reset_recovery is set, and enable it on VIRTCHNL_EVENT_RESET_IMPENDING, so it stays armed from the PF reset notification until recovery completes.
--- drivers/net/intel/iavf/iavf_vchnl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/net/intel/iavf/iavf_vchnl.c +3 -1
@@ -260,7 +260,7 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,* (link is down or a VF reset is in progress); the watchdog drives * auto-reset recovery, so it must remain armed in those cases. */ -if (vf->link_up && !vf->vf_reset) +if (vf->link_up && !vf->vf_reset && !vf->in_reset_recovery)iavf_dev_watchdog_disable (adapter);else iavf_dev_watchdog_enable (adapter);
@@ -334,6 +334,7 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +iavf_dev_watchdog_enable (adapter);if (adapter->devargs.no_poll_on_link_down)iavf_dev_tx_drain (vf->eth_dev); iavf_dev_event_post(vf->eth_dev,
@@ -574,6 +575,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +iavf_dev_watchdog_enable (adapter);if (adapter->devargs.no_poll_on_link_down)iavf_dev_tx_drain (dev); iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
Signature · 1 line
2.34.1
RE: [PATCH v3 6/7] net/iavf: keep watchdog armed for the whole reset window
Quoted 9 lines
Subject: [PATCH v3 6/7] net/iavf: keep watchdog armed for the whole reset window
The device watchdog polls
VFGEN_RSTATto detect a VF reset and drives the auto-reset recovery path.
It is disabled when a link-up event arrives, on the assumption that a working link means no reset is in flight.
That assumption does not hold during recovery.
Quoted 8 lines
Keep the watchdog armed while
vf->in_reset_recoveryis set, and enable it onVIRTCHNL_EVENT_RESET_IMPENDING, so it stays armed from the PF reset notification until recovery completes.Fixes ece7d7eef04f ("net/iavf: fix duplicate VF reset during PF resetrecovery")
Signed-off-by Anurag Mandal <[email protected]>
Quoted 4 lines
--- drivers/net/intel/iavf/iavf_vchnl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
PATCH v3 7/7 net/iavf: skip flow flush during PF-initiated reset
In reply to Anurag MandalSkip the virtchnl-emitting teardown on a PF-initiated reset.
--- drivers/net/intel/iavf/iavf_ethdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/net/intel/iavf/iavf_ethdev.c +3 -1
@@ -3235,7 +3235,9 @@ iavf_dev_close(struct rte_eth_dev *dev)/* remove RSS configuration */ iavf_hash_uninit (adapter); -iavf_flow_flush (dev,NULL ); +/* Skip the virtchnl-emitting teardown on a PF-initiated reset */ +if (!vf->pf_reset_in_progress) +iavf_flow_flush (dev,NULL );iavf_flow_uninit (adapter);/*
Signature · 1 line
2.34.1
v4 cover net/iavf: harden reset recovery and data path on link flap
In reply to Anurag MandalV4-changes:
- Addressed Bruce Richardson's review comment on Patch-3
- Removed Tx drain on link down
V3-changes:
- Addressed Ciara Loftus's review comments on Fixes tag
- Added missing watchdog enable in Patch-6
- Removed extra space in commit msg of Patch-5
V2-changes:
- Addressed Ciara Loftus's review comments.
- Split Watchdog and flow flush from other patches.
- Shortened multiple comments and commit msg
- Prevented iavf_resume_pending_start() looping
During PF-initiated reset or a remote/ToR switch link-flap, the VF might miss the reset event, race on the no_poll gate, leak in-flight Tx descriptors, and stay down if dev_start ran before the PF VSI was ready.
This patch series build on the earlier reset-recovery fixes with the following:
- Reset detection: complement the ARQLEN1 check with VFGEN_RSTAT (VIRTCHNL_VFR_INPROGRESS) and poll at a 5 ms interval, matching the kernel iavf driver, so fast ARQ flips are not missed. When the VFR is still not observed, recover anyway instead of bailing out, keeping PF and VF state in sync.
- no_poll: make the flag atomic (RTE_ATOMIC) with release/acquire ordering so the data-plane lcores observe gate changes reliably.
- Tx drain: add iavf_dev_tx_drain() to flush in-flight Tx descriptors on impending-reset events before teardown, preventing MDD events and descriptor leaks.
- Deferred start: when dev_start fails during recovery (PF VSI inactive), defer it via start_pending and resume on the next link-up event so the VF comes back without manual intervention.
- AdminQ: discard zeroed (opcode 0) descriptors seen during PF-initiated resets to avoid the "Request 0 is not supported" log flood.
- Watchdog: keep watchdog armed for the whole reset window
- Flow flush: skip flow flush during PF-initiated reset
Anurag Mandal (7):
net/iavf: discard empty AdminQ descriptors on reset net/iavf: defer device start when PF VSI not ready net/iavf: drain in-flight Tx before reset net/iavf: change no_poll flag to atomic net/iavf: improve VF reset detection on fast ARQ flip net/iavf: keep watchdog armed for the whole reset window net/iavf: skip flow flush during PF-initiated reset
drivers/net/intel/iavf/iavf.h | 5 +- drivers/net/intel/iavf/iavf_ethdev.c | 108 +++++++++++++++++++++++---- drivers/net/intel/iavf/iavf_rxtx.c | 108 ++++++++++++++++++++++++++- drivers/net/intel/iavf/iavf_rxtx.h | 6 ++ drivers/net/intel/iavf/iavf_vchnl.c | 20 ++++- 5 files changed, 227 insertions(+), 20 deletions(-)
Signature · 1 line
2.34.1
PATCH v4 1/7 net/iavf: discard empty AdminQ descriptors on reset
During PF-initiated resets iavf_clean_arq_element() has been observed to return success with a fully zeroed descriptor (opcode 0). These fell through to the default arm of the dispatch switch and produced a "Request 0 is not supported yet" log flood in the field.
Skip descriptors with opcode 0 so they are silently discarded and do not flood log.
--- drivers/net/intel/iavf/iavf_vchnl.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/net/intel/iavf/iavf_vchnl.c +5 -0
@@ -614,6 +614,11 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)break ; } aq_opc =rte_le_to_cpu_16 (info.desc.opcode); + +/* opcode 0 means the descriptor is empty/zeroed, skip it */ +if (aq_opc ==0 ) +continue ; +/* For the message sent from pf to vf, opcode is stored in * cookie_high of struct iavf_aq_desc, while return error code * are stored in cookie_low, Which is done by PF driver.
Signature · 1 line
2.34.1
PATCH v4 2/7 net/iavf: defer device start when PF VSI not ready
In reply to Anurag MandalDuring reset recovery iavf_dev_start() might fail (typically -EIO from VIRTCHNL_OP_CONFIG_VSI_QUEUES) because the PF VSI is not yet active, leaving the VF down and requiring manual intervention to recover.
Added a start_pending flag: when device start fails during recovery, defer it instead of erroring out and resume it from newly added iavf_resume_pending_start() on the next link-up event, so the VF comes back automatically.
--- drivers/net/intel/iavf/iavf.h | 2 + drivers/net/intel/iavf/iavf_ethdev.c | 61 ++++++++++++++++++++++++++-- drivers/net/intel/iavf/iavf_vchnl.c | 3 ++ 3 files changed, 62 insertions(+), 4 deletions(-)
drivers/net/intel/iavf/iavf.h +2 -0
@@ -293,6 +293,7 @@ struct iavf_info {bool in_reset_recovery;bool reset_pending;bool pf_reset_in_progress; +bool start_pending;uint32_t ptp_caps;rte_spinlock_t phc_time_aq_lock;
@@ -533,4 +534,5 @@ 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);void iavf_hash_uninit (struct iavf_adapter *ad); +void iavf_resume_pending_start (struct rte_eth_dev *dev);#endif /* _IAVF_ETHDEV_H_ */
drivers/net/intel/iavf/iavf_ethdev.c +57 -4
91 lines not on this page · raw message
drivers/net/intel/iavf/iavf_vchnl.c +3 -0
@@ -271,6 +271,9 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev, adapter->no_poll ? "on" : "off"); } +/* Retry a dev_start deferred by reset recovery, before the LSC post. */ +iavf_resume_pending_start (dev); +iavf_dev_event_post (dev,RTE_ETH_EVENT_INTR_LSC ,NULL ,0 ); PMD_DRV_LOG(INFO, "Link status update:%s",
Signature · 1 line
2.34.1
PATCH v4 3/7 net/iavf: drain in-flight Tx before reset
In reply to Anurag MandalOn an impending PF reset, in-flight Tx descriptors were left pending when the queues were torn down, which could trigger Malicious Driver Detection (MDD) events and leak descriptors.
Added iavf_dev_tx_drain() to let already-posted Tx bursts complete and flush the rings within a bounded budget, and call it on reset-impending events before teardown, preventing MDD events and descriptor leaks.
The drain selects the cleanup routine that matches the active Tx path.
--- drivers/net/intel/iavf/iavf_rxtx.c | 100 ++++++++++++++++++++++++++++ drivers/net/intel/iavf/iavf_rxtx.h | 6 ++ drivers/net/intel/iavf/iavf_vchnl.c | 4 ++ 3 files changed, 110 insertions(+)
drivers/net/intel/iavf/iavf_rxtx.c +100 -0
@@ -32,6 +32,7 @@#include "iavf.h" #include "iavf_rxtx.h" +#include "iavf_rxtx_vec_common.h" #include "iavf_ipsec_crypto.h" #include "rte_pmd_iavf.h"
@@ -4025,6 +4026,105 @@ iavf_tx_done_cleanup_full(struct ci_tx_queue *txq,return (int )pkt_cnt; } +/* +* Reclaim completed Tx descriptors for a single queue using the cleanup +* routine that matches the active Tx path. +* Returns true if any descriptors were reclaimed. +*/ +static bool +iavf_tx_drain_cleanup (struct ci_tx_queue *txq, +enum iavf_tx_func_type tx_func_type) +{ +switch (tx_func_type) { +case IAVF_TX_AVX2_CTX : +case IAVF_TX_AVX2_CTX_OFFLOAD : +case IAVF_TX_AVX512_CTX : +case IAVF_TX_AVX512_CTX_OFFLOAD : +return ci_tx_free_bufs_vec (txq, iavf_tx_desc_done, true) !=0 ; +case IAVF_TX_NEON : +case IAVF_TX_AVX2 : +case IAVF_TX_AVX2_OFFLOAD : +case IAVF_TX_AVX512 : +case IAVF_TX_AVX512_OFFLOAD : +return ci_tx_free_bufs_vec (txq, iavf_tx_desc_done, false) !=0 ; +case IAVF_TX_DEFAULT : +default : +return ci_tx_xmit_cleanup (txq) ==0 ; + } +} + +/* +* iavf_dev_tx_drain - drain in-flight Tx descriptors after an +* impending PF reset event. +*/ +void +iavf_dev_tx_drain (struct rte_eth_dev *dev) +{ +struct iavf_adapter *adapter = +IAVF_DEV_PRIVATE_TO_ADAPTER (dev->data->dev_private); +enum iavf_tx_func_type tx_func_type = adapter->tx_func_type; +struct ci_tx_queue *txq; +uint64_t hz, deadline; +int idle_iters =0 ; +uint16_t qid; + +/* +* Allow any Tx burst already in flight on a data-plane lcore to +* write its remaining descriptors and notify. After +* this window, the no_poll gate set by the caller is observed at +* the next burst-entry and no new descriptors will be posted. +*/ +rte_delay_us_block (IAVF_TX_DRAIN_SETTLE_US ); + + hz =rte_get_timer_hz (); + deadline =rte_get_timer_cycles () + + (hz *IAVF_TX_DRAIN_TIMEOUT_US ) /1000000ULL ; + +while (rte_get_timer_cycles () < deadline) { +bool any_pending = false; +bool any_progress = false; + +for (qid =0 ; qid < dev->data->nb_tx_queues; qid++) { + txq = dev->data->tx_queues[qid]; +if (txq ==NULL || + dev->data->tx_queue_state[qid] != +RTE_ETH_QUEUE_STATE_STARTED ) +continue ; + +/* +* nb_tx_free == nb_tx_desc - 1 means the ring is +* empty (one descriptor is always reserved). +*/ +if (txq->nb_tx_free >= txq->nb_tx_desc -1 ) +continue ; + + any_pending = true; +if (iavf_tx_drain_cleanup (txq, tx_func_type)) + any_progress = true; + } + +if (!any_pending) +return ; + +if (any_progress) { + idle_iters =0 ; + }else if (++idle_iters >=IAVF_TX_DRAIN_IDLE_MAX ) { +/* +* HW has not advanced the RS-bit write-back for +* several polling intervals; either the queue is +* quiescent except for the sub-rs_thresh tail +* (which we cannot observe here) or HW is no +* longer fetching. Further polling is unlikely to +* help, and the PF teardown path has its own +* grace period for the remainder. +*/ +break ; + } + +rte_delay_us_block (IAVF_TX_DRAIN_POLL_US ); + } +} +int iavf_dev_tx_done_cleanup (void *txq,uint32_t free_cnt) {
drivers/net/intel/iavf/iavf_rxtx.h +6 -0
@@ -506,6 +506,11 @@ enum iavf_tx_ctx_desc_tunnel_l4_tunnel_type {/* Valid indicator bit for the time_stamp_low field */ #define IAVF_RX_FLX_DESC_TS_VALID (0x1UL) +#define IAVF_TX_DRAIN_TIMEOUT_US 10000/* total drain budget: 10 ms */ +#define IAVF_TX_DRAIN_SETTLE_US 100/* let in-flight burst land */ +#define IAVF_TX_DRAIN_POLL_US 50/* poll interval */ +#define IAVF_TX_DRAIN_IDLE_MAX 20/* ~1 ms of no RS write-back */ + int iavf_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, uint16_t nb_desc,
@@ -641,6 +646,7 @@ void iavf_set_default_ptype_table(struct rte_eth_dev *dev);void iavf_rx_queue_release_mbufs_vec (struct ci_rx_queue *rxq);void iavf_rx_queue_release_mbufs_neon (struct ci_rx_queue *rxq);enum rte_vect_max_simd iavf_get_max_simd_bitwidth (void ); +void iavf_dev_tx_drain (struct rte_eth_dev *dev); static inline void iavf_dump_rx_descriptor(struct ci_rx_queue *rxq,
drivers/net/intel/iavf/iavf_vchnl.c +4 -0
@@ -330,6 +330,8 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +if (adapter->devargs.no_poll_on_link_down) +iavf_dev_tx_drain (vf->eth_dev);iavf_dev_event_post (vf->eth_dev,RTE_ETH_EVENT_INTR_RESET ,NULL ,0 );
@@ -568,6 +570,8 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +if (adapter->devargs.no_poll_on_link_down) +iavf_dev_tx_drain (dev);iavf_dev_event_post (dev,RTE_ETH_EVENT_INTR_RESET ,NULL ,0 ); }
Signature · 1 line
2.34.1
PATCH v4 4/7 net/iavf: change no_poll flag to atomic
In reply to Anurag MandalThe no_poll gate that pauses the Rx/Tx data path during reset and link-down was a plain bool written on the control path and read on the data-plane lcores without synchronization, allowing stale reads that either keep dropping traffic or trigger spurious reset detection.
Made no_poll an RTE_ATOMIC(bool) and access it with release stores and acquire loads so data-plane lcores reliably observe gate changes.
--- drivers/net/intel/iavf/iavf.h | 2 +- drivers/net/intel/iavf/iavf_ethdev.c | 6 +++++- drivers/net/intel/iavf/iavf_rxtx.c | 8 ++++++-- drivers/net/intel/iavf/iavf_vchnl.c | 4 +++- 4 files changed, 15 insertions(+), 5 deletions(-)
drivers/net/intel/iavf/iavf.h +1 -1
@@ -392,7 +392,7 @@ struct iavf_adapter { alignas(RTE_CACHE_LINE_MIN_SIZE )uint32_t ptype_tbl[IAVF_MAX_PKT_TYPE ];bool stopped;bool closed; -bool no_poll; +RTE_ATOMIC (bool ) no_poll;enum iavf_rx_func_type rx_func_type;enum iavf_tx_func_type tx_func_type;uint16_t fdir_ref_cnt;
drivers/net/intel/iavf/iavf_ethdev.c +5 -1
@@ -3604,9 +3604,13 @@ void iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change) {struct iavf_info *vf = &adapter->vf; +bool no_poll; - adapter->no_poll = (link_change & !vf->link_up) || + no_poll = (link_change & !vf->link_up) || vf->vf_reset || vf->in_reset_recovery; + +rte_atomic_store_explicit (&adapter->no_poll, no_poll, + rte_memory_order_release); } static int
drivers/net/intel/iavf/iavf_rxtx.c +6 -2
@@ -3723,7 +3723,9 @@ iavf_recv_pkts_no_poll(void *rx_queue, struct rte_mbuf **rx_pkts,struct ci_rx_queue *rxq = rx_queue;enum iavf_rx_func_type rx_func_type; -if (!rxq->iavf_vsi || rxq->iavf_vsi->adapter->no_poll) +if (!rxq->iavf_vsi || +rte_atomic_load_explicit (&rxq->iavf_vsi->adapter->no_poll, + rte_memory_order_acquire))return 0 ; rx_func_type = rxq->iavf_vsi->adapter->rx_func_type;
@@ -3739,7 +3741,9 @@ iavf_xmit_pkts_no_poll(void *tx_queue, struct rte_mbuf **tx_pkts,struct ci_tx_queue *txq = tx_queue;enum iavf_tx_func_type tx_func_type; -if (!txq->iavf_vsi || txq->iavf_vsi->adapter->no_poll) +if (!txq->iavf_vsi || +rte_atomic_load_explicit (&txq->iavf_vsi->adapter->no_poll, + rte_memory_order_acquire))return 0 ; tx_func_type = txq->iavf_vsi->adapter->tx_func_type;
drivers/net/intel/iavf/iavf_vchnl.c +3 -1
@@ -268,7 +268,9 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,if (adapter->devargs.no_poll_on_link_down) {iavf_set_no_poll (adapter, true);PMD_DRV_LOG (DEBUG ,"VF no poll turned %s" , - adapter->no_poll ?"on" :"off" ); +rte_atomic_load_explicit (&adapter->no_poll, + rte_memory_order_relaxed) ? +"on" :"off" ); }/* Retry a dev_start deferred by reset recovery, before the LSC post. */
Signature · 1 line
2.34.1
PATCH v4 5/7 net/iavf: improve VF reset detection on fast ARQ flip
In reply to Anurag MandalDuring PF-initiated reset or a remote/ToR switch link-flap, the PF toggles the admin receive queue enable bit (ARQLEN1) so quickly around a VF reset that the VF's sampling window misses it, leaving the PF and the VF states out of sync and the data path stalled.
Complement the ARQLEN1 check with VFGEN_RSTAT (VIRTCHNL_VFR_INPROGRESS) and shorten the poll interval to 5 ms (with a proportionally larger count, keeping the ~10 s budget), matching the Linux kernel iavf driver. When the VFR is still not observed, proceed with recovery instead of bailing out so the PF and the VF states converge.
--- drivers/net/intel/iavf/iavf.h | 1 + drivers/net/intel/iavf/iavf_ethdev.c | 37 +++++++++++++++++++++------- 2 files changed, 29 insertions(+), 9 deletions(-)
drivers/net/intel/iavf/iavf.h +1 -0
@@ -21,6 +21,7 @@#define IAVF_AQ_BUF_SZ 4096#define IAVF_RESET_WAIT_CNT 2000#define IAVF_RESET_DETECTED_CNT 500 +#define IAVF_RESET_POLL_SCALE 4/* Poll-interval scale for reset detection */ #define IAVF_BUF_SIZE_MIN 1024#define IAVF_FRAME_SIZE_MAX 9728#define IAVF_QUEUE_BASE_ADDR_UNIT 128
drivers/net/intel/iavf/iavf_ethdev.c +28 -9
@@ -3358,8 +3358,26 @@ iavf_dev_reset(struct rte_eth_dev *dev)static inline bool iavf_is_reset (struct iavf_hw *hw) { -return !(IAVF_READ_REG (hw,IAVF_VF_ARQLEN1 ) & -IAVF_VF_ARQLEN1_ARQENABLE_MASK ); +uint32_t rstat; + +/* ARQ has been disabled by the PF as part of the VFR. */ +if (!(IAVF_READ_REG (hw,IAVF_VF_ARQLEN1 ) & +IAVF_VF_ARQLEN1_ARQENABLE_MASK )) +return true; + +/* +* VFGEN_RSTAT reports VIRTCHNL_VFR_INPROGRESS. +* At times, the PF flips ARQENABLE so quickly +* around a VFR that the ARQLEN1 sample window +* misses it. Using VFGEN_RSTAT, as a +* complementary indicator, prevents from +* missing a reset that really did happen. +*/ + rstat = (IAVF_READ_REG (hw,IAVF_VFGEN_RSTAT ) & +IAVF_VFGEN_RSTAT_VFR_STATE_MASK ) >> +IAVF_VFGEN_RSTAT_VFR_STATE_SHIFT ; + +return rstat ==VIRTCHNL_VFR_INPROGRESS ; } static bool
@@ -3368,11 +3386,14 @@ iavf_is_reset_detected(struct iavf_adapter *adapter)struct iavf_hw *hw =IAVF_DEV_PRIVATE_TO_HW (adapter);int i; -/* poll until we see the reset actually happen */ -for (i =0 ; i <IAVF_RESET_DETECTED_CNT ; i++) { +/* +* Poll until the reset actually happen. +* Poll every 5 ms to catch the fast ARQ flips. +*/ +for (i =0 ; i <IAVF_RESET_DETECTED_CNT *IAVF_RESET_POLL_SCALE ; i++) {if (iavf_is_reset (hw))return true; -rte_delay_ms (20 ); +rte_delay_us (5000 ); }return false;
@@ -3423,10 +3444,8 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)if (!dev->data->dev_started)return ; -if (!iavf_is_reset_detected (adapter)) { -PMD_DRV_LOG (DEBUG ,"reset not start" ); -return ; - } +if (!iavf_is_reset_detected (adapter)) +PMD_DRV_LOG (WARNING ,"VFR not observed; recovering anyway" ); } vf->in_reset_recovery = true;
Signature · 1 line
2.34.1
PATCH v4 6/7 net/iavf: keep watchdog armed for the whole reset window
In reply to Anurag MandalThe device watchdog polls VFGEN_RSTAT to detect a VF reset and drives the auto-reset recovery path.
It is disabled when a link-up event arrives, on the assumption that a working link means no reset is in flight.
That assumption does not hold during recovery.
Keep the watchdog armed while vf->in_reset_recovery is set, and enable it on VIRTCHNL_EVENT_RESET_IMPENDING, so it stays armed from the PF reset notification until recovery completes.
--- drivers/net/intel/iavf/iavf_vchnl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/net/intel/iavf/iavf_vchnl.c +3 -1
@@ -260,7 +260,7 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,* (link is down or a VF reset is in progress); the watchdog drives * auto-reset recovery, so it must remain armed in those cases. */ -if (vf->link_up && !vf->vf_reset) +if (vf->link_up && !vf->vf_reset && !vf->in_reset_recovery)iavf_dev_watchdog_disable (adapter);else iavf_dev_watchdog_enable (adapter);
@@ -332,6 +332,7 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +iavf_dev_watchdog_enable (adapter);if (adapter->devargs.no_poll_on_link_down)iavf_dev_tx_drain (vf->eth_dev); iavf_dev_event_post(vf->eth_dev,
@@ -572,6 +573,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,if (!vf->vf_reset) { vf->vf_reset = true;iavf_set_no_poll (adapter, false); +iavf_dev_watchdog_enable (adapter);if (adapter->devargs.no_poll_on_link_down)iavf_dev_tx_drain (dev); iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
Signature · 1 line
2.34.1
PATCH v4 7/7 net/iavf: skip flow flush during PF-initiated reset
In reply to Anurag MandalSkip the virtchnl-emitting teardown on a PF-initiated reset.
--- drivers/net/intel/iavf/iavf_ethdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/net/intel/iavf/iavf_ethdev.c +3 -1
@@ -3235,7 +3235,9 @@ iavf_dev_close(struct rte_eth_dev *dev)/* remove RSS configuration */ iavf_hash_uninit (adapter); -iavf_flow_flush (dev,NULL ); +/* Skip the virtchnl-emitting teardown on a PF-initiated reset */ +if (!vf->pf_reset_in_progress) +iavf_flow_flush (dev,NULL );iavf_flow_uninit (adapter);/*
Signature · 1 line
2.34.1
Quoted 47 lines · Anurag Mandal
V4-changes:
- Addressed Bruce Richardson's review comment on Patch-3
- Removed Tx drain on link downV3-changes:
- Addressed Ciara Loftus's review comments on Fixes tag
- Added missing watchdog enable in Patch-6
- Removed extra space in commit msg of Patch-5V2-changes:
- Addressed Ciara Loftus's review comments.
- Split Watchdog and flow flush from other patches.
- Shortened multiple comments and commit msg
- Preventediavf_resume_pending_start()loopingDuring PF-initiated reset or a remote/ToR switch link-flap, the VF might miss the reset event, race on the
no_pollgate, leak in-flight Tx descriptors, and stay down ifdev_startran before the PF VSI was ready.This patch series build on the earlier reset-recovery fixes with the following:
- Reset detection: complement the ARQLEN1 check with
VFGEN_RSTAT(VIRTCHNL_VFR_INPROGRESS) and poll at a 5 ms interval, matching the kernel iavf driver, so fast ARQ flips are not missed. When the VFR is still not observed, recover anyway instead of bailing out, keeping PF and VF state in sync.-
no_poll: make the flag atomic (RTE_ATOMIC) with release/acquire ordering so the data-plane lcores observe gate changes reliably.- Tx drain: add
iavf_dev_tx_drain()to flush in-flight Tx descriptors on impending-reset events before teardown, preventing MDD events and descriptor leaks.- Deferred start: when
dev_startfails during recovery (PF VSI inactive), defer it viastart_pendingand resume on the next link-up event so the VF comes back without manual intervention.- AdminQ: discard zeroed (opcode 0) descriptors seen during PF-initiated resets to avoid the "Request 0 is not supported" log flood.
- Watchdog: keep watchdog armed for the whole reset window
- Flow flush: skip flow flush during PF-initiated reset
Quoted 9 lines
Anurag Mandal (7):
net/iavf: discard empty AdminQ descriptors on reset net/iavf: defer device start when PF VSI not ready net/iavf: drain in-flight Tx before reset net/iavf: changeno_pollflag to atomic net/iavf: improve VF reset detection on fast ARQ flip net/iavf: keep watchdog armed for the whole reset window net/iavf: skip flow flush during PF-initiated reset
Series applied to dpdk-next-net-intel.
Thanks,
/Bruce