net/mana: add device reset support
Subsystems (2)
v5 cover net/mana: add device reset support
From: Wei Hu <[email protected]>
Add support for handling hardware service reset events in the MANA driver. When the MANA kernel driver receives a hardware service event, it initiates a device reset and notifies userspace via IBV_EVENT_DEVICE_FATAL. The MANA PMD handles this by performing an automatic teardown and recovery sequence.
The driver uses ethdev recovery events (ERR_RECOVERING, RECOVERY_SUCCESS, RECOVERY_FAILED) to notify upper layers of the reset lifecycle, and a PCI device removal event callback to distinguish hot-remove from service reset.
Changes since v4:
- Fixed stale rte_spinlock_unlock call in mana_intr_handler that was missed during the spinlock-to-mutex conversion, causing a -Wincompatible-pointer-types warning
Changes since v3:
- Converted reset_ops_lock from rte_spinlock_t to pthread_mutex_t with PTHREAD_PROCESS_SHARED, since the lock is held across blocking IB verbs calls and IPC with 5s timeout
- Removed rte_dev_event_callback_unregister retry loop to avoid
deadlock: the callback itself blocks on reset_ops_lock, so retrying on -EAGAIN while holding the lock is a deadlock
- Introduced mana_join_reset_thread() helper using CAS on reset_thread_active to prevent double-join undefined behavior
- Added reset thread join in mana_dev_uninit to prevent thread leak on device removal
- Fixed ibv handle leak: priv->ib_ctx is now only set to NULL after ibv_close_device succeeds
- Fixed misleading "All secondary threads are quiescent" log in mana_mp_reset_enter — changed to "Secondary doorbell pages unmapped" since actual quiescence is enforced by the primary's RCU QSBR check before IPC is sent
- Changed event list in mana.rst to RST definition list style
- Squashed documentation into the feature patch per convention
Changes since v2:
- Fixed dev_state_qsv memory leak on device removal
- Fixed reset thread TCB/stack leak: reset_thread_active is now only cleared by the joiner, not the thread itself
- Fixed second reset crash: removed reset thread join logic from mana_dev_close (inner function) to avoid corrupting dev_state when called from mana_reset_enter
- Made reset_thread_active RTE_ATOMIC(bool) with explicit ordering
- Added retry loop for rte_dev_event_callback_unregister on -EAGAIN
- Initialized condvar/mutex with PTHREAD_PROCESS_SHARED since priv is in hugepage shared memory
- Added re-check of dev_state after lock acquisition in mana_intr_handler to prevent racing with pci_remove_event_cb
- Replaced (void *)0 with NULL in mp.c
- Added lock ownership comment block at mana_reset_enter
- Documented rte_dev_event_monitor_start() requirement
- Added mana.rst documentation and release note
Changes since v1:
- Removed net/netvsc patch from this series
- Simplified reset exit: mana_reset_exit calls mana_reset_exit_delay directly instead of spawning a thread
- Added __rte_no_thread_safety_analysis annotations for clang
- Switched to rte_thread_create_internal_control
- Fixed declaration-after-statement style issues
- Removed unnecessary blank lines and stale comments
Wei Hu (1):
net/mana: add device reset support
doc/guides/nics/mana.rst | 38 + doc/guides/rel_notes/release_26_07.rst | 8 + drivers/net/mana/mana.c | 1005 ++++++++++++++++++++++-- drivers/net/mana/mana.h | 33 +- drivers/net/mana/meson.build | 2 +- drivers/net/mana/mp.c | 89 ++- drivers/net/mana/mr.c | 6 +- drivers/net/mana/rx.c | 24 +- drivers/net/mana/tx.c | 40 +- 9 files changed, 1138 insertions(+), 107 deletions(-)
Signature · 1 line
2.34.1
PATCH v5 1/1 net/mana: add device reset support
From: Wei Hu <[email protected]>
Add support for handling hardware reset events in the MANA driver. When the MANA kernel driver receives a hardware service event, it initiates a device reset and notifies userspace via IBV_EVENT_DEVICE_FATAL. The DPDK driver handles this by performing an automatic teardown and recovery sequence.
The reset flow has two phases. In the enter phase, running on the EAL interrupt thread, the driver transitions the device state, waits for data path threads to reach a quiescent state using RCU, stops queues, tears down IB resources, and frees per-queue MR caches. A control thread is then spawned to handle the exit phase:
it waits for the hardware to recover, unregisters the interrupt handler, re-probes the PCI device, reinitializes MR caches, and restarts queues.
A per-device mutex serializes the reset path with ethdev operations. The mutex uses PTHREAD_PROCESS_SHARED for multi-process support and is held across blocking IB verbs calls. Operations that cannot wait (configure, queue setup) return -EBUSY during reset, while dev_stop and dev_close join the reset thread before acquiring the lock to ensure proper sequencing. A CAS-based helper prevents double-join of the reset thread.
Multi-process support is included: secondary processes unmap and remap doorbell pages via IPC during the reset enter and exit phases. Data path functions in both primary and secondary processes check the device state atomically and return early when the device is not active. RCU quiescent state tracking uses per-queue thread IDs in shared hugepage memory, covering both primary and secondary process data path threads.
The driver uses ethdev recovery events to notify upper layers (e.g. netvsc) of the reset lifecycle: RTE_ETH_EVENT_ERR_RECOVERING on entry, RTE_ETH_EVENT_RECOVERY_SUCCESS or RTE_ETH_EVENT_RECOVERY_FAILED on completion. A PCI device removal event callback distinguishes hot-remove from service reset.
Documentation for the device reset feature is added in the MANA NIC guide and the 26.07 release notes.
--- doc/guides/nics/mana.rst | 38 + doc/guides/rel_notes/release_26_07.rst | 8 + drivers/net/mana/mana.c | 1005 ++++++++++++++++++++++-- drivers/net/mana/mana.h | 33 +- drivers/net/mana/meson.build | 2 +- drivers/net/mana/mp.c | 89 ++- drivers/net/mana/mr.c | 6 +- drivers/net/mana/rx.c | 24 +- drivers/net/mana/tx.c | 40 +- 9 files changed, 1138 insertions(+), 107 deletions(-)
doc/guides/nics/mana.rst +38 -0
@@ -71,3 +71,41 @@ The user can specify below argument in devargs. The default value is not set, meaning all the NICs will be probed and loaded. User can specify multiple mac=xx:xx:xx:xx:xx:xx arguments for up to 8 NICs. + +Device Reset Support +-------------------- + +The MANA PMD supports automatic recovery from hardware service reset events. +When the MANA kernel driver receives a hardware service event, +it initiates a device reset and notifies userspace +via ``IBV_EVENT_DEVICE_FATAL``. + +The driver handles this transparently through a two-phase reset flow: + +* **Enter phase**: The driver stops the data path, + waits for all threads to reach a quiescent state using RCU, + tears down IB resources and queues, + and unmaps secondary process doorbell pages. + +* **Exit phase**: After a delay for hardware recovery, + a control thread re-probes the PCI device, + reinstalls the interrupt handler, + reinitializes resources, and restarts queues. + +The driver emits the following ethdev recovery events +to notify upper layers (e.g. netvsc) of the reset lifecycle: + +``RTE_ETH_EVENT_ERR_RECOVERING`` + Reset has started. + +``RTE_ETH_EVENT_RECOVERY_SUCCESS`` + Device has recovered successfully. + +``RTE_ETH_EVENT_RECOVERY_FAILED`` + Recovery failed. + +To distinguish a PCI hot-remove from a service reset, +the driver registers for PCI device removal events. +This requires the application to call ``rte_dev_event_monitor_start()`` +for removal events to be delivered +(e.g. testpmd ``--hot-plug-handling`` option).
doc/guides/rel_notes/release_26_07.rst +8 -0
@@ -77,6 +77,14 @@ New Features Added network driver for the Linkdata Network Adapters. +* **Added device reset support to the MANA PMD.** + + Added automatic recovery from hardware service reset events + in the MANA poll mode driver. The driver uses ethdev recovery events + (``RTE_ETH_EVENT_ERR_RECOVERING``, ``RTE_ETH_EVENT_RECOVERY_SUCCESS``, + ``RTE_ETH_EVENT_RECOVERY_FAILED``) to notify upper layers of the + reset lifecycle. + Removed Items -------------
drivers/net/mana/mana.c +921 -84
1,220 lines not on this page · raw message
drivers/net/mana/mana.h +32 -1
@@ -5,6 +5,8 @@#ifndef __MANA_H__#define __MANA_H__ +#include <pthread.h> +#define PCI_VENDOR_ID_MICROSOFT 0x1414#define PCI_DEVICE_ID_MICROSOFT_MANA_PF 0x00b9#define PCI_DEVICE_ID_MICROSOFT_MANA 0x00ba
@@ -337,6 +339,20 @@ struct mana_process_priv {void *db_page; }; +enum mana_device_state { +/* Normal running */ +MANA_DEV_ACTIVE =0 , +/* In reset enter processing */ +MANA_DEV_RESET_ENTER =1 , +/* +* Reset enter processing completed. +* Waiting for reset exit or in reset exit processing. +*/ +MANA_DEV_RESET_EXIT =2 , +/* Reset failed */ +MANA_DEV_RESET_FAILED =3 , +}; +struct mana_priv {struct rte_eth_dev_data *dev_data;struct mana_process_priv *process_priv;
@@ -368,6 +384,16 @@ struct mana_priv {uint64_t max_mr_size;struct mana_mr_btree mr_btree;rte_spinlock_t mr_btree_lock; +RTE_ATOMIC (enum mana_device_state ) dev_state; +struct rte_rcu_qsbr *dev_state_qsv; +/* mutex for synchronizing mana reset and some mana_dev_ops callbacks */ +pthread_mutex_t reset_ops_lock; +/* Reset thread ID, valid when reset_thread_active is true */ +rte_thread_t reset_thread; +RTE_ATOMIC (bool ) reset_thread_active; +/* Condvar to wake reset thread early on PCI remove */ +pthread_mutex_t reset_cond_mutex; +pthread_cond_t reset_cond; };struct mana_txq_desc {
@@ -427,6 +453,7 @@ struct mana_txq {struct mana_mr_btree mr_btree;struct mana_stats stats;unsigned int socket; +unsigned int txq_idx; };struct mana_rxq {
@@ -462,6 +489,7 @@ struct mana_rxq {struct mana_mr_btree mr_btree;unsigned int socket; +unsigned int rxq_idx; };extern int mana_logtype_driver;
@@ -543,6 +571,8 @@ enum mana_mp_req_type {MANA_MP_REQ_CREATE_MR ,MANA_MP_REQ_START_RXTX ,MANA_MP_REQ_STOP_RXTX , +MANA_MP_REQ_RESET_ENTER , + MANA_MP_REQ_RESET_EXIT, };/* Pameters for IPC. */
@@ -563,8 +593,9 @@ void mana_mp_uninit_primary(void);void mana_mp_uninit_secondary (void );int mana_mp_req_verbs_cmd_fd (struct rte_eth_dev *dev);int mana_mp_req_mr_create (struct mana_priv *priv,uintptr_t addr,uint32_t len); +int mana_map_doorbell_secondary (struct rte_eth_dev *eth_dev,int fd); -void mana_mp_req_on_rxtx (struct rte_eth_dev *dev,enum mana_mp_req_type type); +int mana_mp_req_on_rxtx (struct rte_eth_dev *dev,enum mana_mp_req_type type);void *mana_alloc_verbs_buf (size_t size,void *data); void mana_free_verbs_buf(void *ptr, void *data __rte_unused);
drivers/net/mana/meson.build +1 -1
@@ -7,7 +7,7 @@ if not is_linux or not (dpdk_conf.has('RTE_ARCH_X86') or dpdk_conf.has('RTE_ARCH subdir_done() endif -deps += ['pci', 'bus_pci', 'net', 'eal', 'kvargs'] +deps += ['pci', 'bus_pci', 'net', 'eal', 'kvargs', 'rcu'] sources += files( 'gdma.c',
drivers/net/mana/mp.c +82 -7
153 lines not on this page · raw message
drivers/net/mana/mr.c +4 -2
@@ -314,8 +314,10 @@ mana_mr_btree_init(struct mana_mr_btree *bt, int n, int socket)void mana_mr_btree_free (struct mana_mr_btree *bt) { -rte_free (bt->table); -memset (bt,0 ,sizeof (*bt)); +if (bt && bt->table) { +rte_free (bt->table); +memset (bt,0 ,sizeof (*bt)); + } }int
drivers/net/mana/rx.c +23 -1
@@ -2,6 +2,7 @@* Copyright 2022 Microsoft Corporation */ #include <ethdev_driver.h> +#include <rte_rcu_qsbr.h> #include <infiniband/verbs.h> #include <infiniband/manadv.h>
@@ -36,6 +37,11 @@ mana_rq_ring_doorbell(struct mana_rxq *rxq) db_page = process_priv->db_page; } +if (!db_page) { +DP_LOG (ERR ,"db_page is NULL, cannot ring RX doorbell" ); +return -EINVAL ; + } +/* Hardware Spec specifies that software client should set 0 for * wqe_cnt for Receive Queues. */
@@ -172,7 +178,7 @@ mana_stop_rx_queues(struct rte_eth_dev *dev)for (i =0 ; i < priv->num_queues; i++)if (dev->data->rx_queue_state[i] ==RTE_ETH_QUEUE_STATE_STOPPED ) -return -EINVAL ; +return 0 ;if (priv->rwq_qp) { ret =ibv_destroy_qp (priv->rwq_qp);
@@ -256,6 +262,9 @@ mana_start_rx_queues(struct rte_eth_dev *dev)struct mana_rxq *rxq = dev->data->rx_queues[i];struct ibv_wq_init_attr wq_attr = {}; + rxq->rxq_idx = i; +DRV_LOG (DEBUG ,"assigning rxq_idx to %d" , i); + manadv_set_context_attr(priv->ib_ctx, MANADV_CTX_ATTR_BUF_ALLOCATORS, (void *)((uintptr_t)&(struct manadv_ctx_allocators){
@@ -451,6 +460,17 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)uint32_t pkt_len;uint32_t i;int polled =0 ; +struct rte_rcu_qsbr *dstate_qsv = priv->dev_state_qsv; +unsigned int tid = rxq->rxq_idx; + +rte_rcu_qsbr_thread_online (dstate_qsv, tid); + +if (unlikely (rte_atomic_load_explicit (&priv->dev_state, + rte_memory_order_acquire) !=MANA_DEV_ACTIVE )) { +/* Device reset occurred. */ +rte_rcu_qsbr_thread_offline (dstate_qsv, tid); +return 0 ; + } repoll:/* Polling on new completions if we have no backlog */
@@ -592,6 +612,8 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) wqe_consumed, ret); } +rte_rcu_qsbr_thread_offline (dstate_qsv, tid); +return pkt_received; }
drivers/net/mana/tx.c +29 -11
@@ -3,6 +3,7 @@*/ #include <ethdev_driver.h> +#include <rte_rcu_qsbr.h> #include <infiniband/verbs.h> #include <infiniband/manadv.h>
@@ -17,7 +18,7 @@ mana_stop_tx_queues(struct rte_eth_dev *dev)for (i =0 ; i < priv->num_queues; i++)if (dev->data->tx_queue_state[i] ==RTE_ETH_QUEUE_STATE_STOPPED ) -return -EINVAL ; +return 0 ;for (i =0 ; i < priv->num_queues; i++) {struct mana_txq *txq = dev->data->tx_queues[i];
@@ -83,6 +84,9 @@ mana_start_tx_queues(struct rte_eth_dev *dev) txq = dev->data->tx_queues[i]; + txq->txq_idx = i; +DRV_LOG (DEBUG ,"assigning txq_idx to %d" , txq->txq_idx); + manadv_set_context_attr(priv->ib_ctx, MANADV_CTX_ATTR_BUF_ALLOCATORS, (void *)((uintptr_t)&(struct manadv_ctx_allocators){
@@ -190,10 +194,30 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)void *db_page;uint16_t pkt_sent =0 ;uint32_t num_comp, i; +unsigned int tid = priv->num_queues + txq->txq_idx; +struct rte_rcu_qsbr *dstate_qsv = priv->dev_state_qsv;#ifdef RTE_ARCH_32 uint32_t wqe_count =0 ;#endif + db_page = priv->db_page; +if (rte_eal_process_type () ==RTE_PROC_SECONDARY ) { +struct rte_eth_dev *dev = + &rte_eth_devices[priv->dev_data->port_id]; +struct mana_process_priv *process_priv = dev->process_private; + + db_page = process_priv->db_page; + } + +rte_rcu_qsbr_thread_online (dstate_qsv, tid); + +if (unlikely (rte_atomic_load_explicit (&priv->dev_state, + rte_memory_order_acquire) !=MANA_DEV_ACTIVE || !db_page)) { +/* Device reset event occurred. */ +rte_rcu_qsbr_thread_offline (dstate_qsv, tid); +return 0 ; + } +/* Process send completions from GDMA */ num_comp =gdma_poll_completion_queue (&txq->gdma_cq, txq->gdma_comp_buf, txq->num_desc);
@@ -216,7 +240,8 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) }if (!desc->pkt) { -DP_LOG (ERR ,"mana_txq_desc has a NULL pkt" ); +DP_LOG (ERR ,"mana_txq_desc has a NULL pkt, priv %p, " +"txq = %d" , priv, txq->txq_idx); }else { txq->stats.bytes += desc->pkt->pkt_len;rte_pktmbuf_free (desc->pkt);
@@ -474,15 +499,6 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) }/* Ring hardware door bell */ - db_page = priv->db_page; -if (rte_eal_process_type () ==RTE_PROC_SECONDARY ) { -struct rte_eth_dev *dev = - &rte_eth_devices[priv->dev_data->port_id]; -struct mana_process_priv *process_priv = dev->process_private; - - db_page = process_priv->db_page; - } -if (pkt_sent) {#ifdef RTE_ARCH_32 ret = mana_ring_short_doorbell(db_page, GDMA_QUEUE_SEND,
@@ -501,5 +517,7 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)DP_LOG (ERR ,"mana_ring_doorbell failed ret %d" , ret); } +rte_rcu_qsbr_thread_offline (dstate_qsv, tid); +return pkt_sent; }
Signature · 1 line
2.34.1
Quoted 27 lines · On Fri, 29 May 2026 07:26:48 -0700 Wei Hu <[email protected]> wrote:
drivers/net/mana/mana.c +11
-0@@ -13,7 +13,10 @@#include <ethdev_pci.h> #include <rte_kvargs.h> #include <rte_eal_paging.h> +#include <rte_alarm.h> #include <rte_pci.h> +#include <rte_rcu_qsbr.h> +#include <rte_lock_annotations.h> #include <infiniband/verbs.h> #include <infiniband/manadv.h> @@ -103,6 +106,23 @@ mana_dev_configure(struct rte_eth_dev *dev) RTE_ETH_RX_OFFLOAD_VLAN_STRIP); priv->num_queues = dev->data->nb_rx_queues; +DRV_LOG (DEBUG ,"priv %p, port %u, dev port %u, num_queues: %u" , + priv, priv->port_id, priv->dev_port, priv->num_queues); + +/* +* Register data path thread IDs (rx and tx) with the RCU +* quiescent state variable for device state synchronization. +*/ +for (int i =0 ; i <2 * priv->num_queues; i++) {
drivers/net/mana/mana.c mana_dev_configure(struct rte_eth_dev *dev) · line 126
Quoted 10 lines
drivers/net/mana/mana.c +9
-0+if (rte_rcu_qsbr_thread_register (priv->dev_state_qsv, i) !=0 ) { +DRV_LOG (ERR ,"Failed to register rcu qsv thread %d of total %d" , + i,2 * priv->num_queues -1 ); +return -EINVAL ; + } +DRV_LOG (DEBUG , +"Register thread 0x%x for priv %p, port %u" , + i, priv, priv->port_id); + }
If device driver now has dependency on RCU you need to update meson.build to show that. No other driver does this. Creating threads in driver is discouraged because it can lead to other problems in applications.
If possible, I would figure out how to manage reset without creating a thread per-queue. Or at least one control thread and use epoll() and eventfd's.
Quoted 56 lines
-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Friday, May 29, 2026 11:35 PM
To: Wei Hu <[email protected]>
Cc: [email protected]; Long Li <[email protected]>; Wei Hu <[email protected]>
Subject: [EXTERNAL] Re: [PATCH v5 1/1] net/mana: add device reset supportQuoted 19 lines · On Fri, 29 May 2026 07:26:48 -0700 Wei Hu <[email protected]> wrote:
drivers/net/mana/mana.c index renamed from drivers/net/mana/mana.c +0
-0
67396cda1f..89e45c53c1100644drivers/net/mana/mana.c +3
-0@@ -13,7 +13,10 @@#include <ethdev_pci.h> #include <rte_kvargs.h> #include <rte_eal_paging.h> +#include <rte_alarm.h> #include <rte_pci.h> +#include <rte_rcu_qsbr.h> +#include <rte_lock_annotations.h> #include <infiniband/verbs.h> #include <infiniband/manadv.h> @@ -103,6 +106,23 @@ mana_dev_configure(struct rte_eth_dev *dev) RTE_ETH_RX_OFFLOAD_VLAN_STRIP); priv->num_queues = dev->data->nb_rx_queues;drivers/net/mana/mana.c mana_dev_configure(struct rte_eth_dev *dev) · line 118
Quoted 10 lines
drivers/net/mana/mana.c +10
-0+DRV_LOG (DEBUG ,"priv %p, port %u, dev port %u, num_queues: %u" , + priv, priv->port_id, priv->dev_port, priv->num_queues); + +/* +* Register data path thread IDs (rx and tx) with the RCU +* quiescent state variable for device state synchronization. +*/ +for (int i =0 ; i <2 * priv->num_queues; i++) { +if (rte_rcu_qsbr_thread_register (priv->dev_state_qsv, i) !=0 ) { + DRV_LOG(ERR, "Failed to register rcu qsv thread %d oftotal %d",
drivers/net/mana/mana.c mana_dev_configure(struct rte_eth_dev *dev) · line 126
Quoted 8 lines
drivers/net/mana/mana.c +7
-0+ i, 2 * priv->num_queues - 1); +return -EINVAL ; + } +DRV_LOG (DEBUG , +"Register thread 0x%x for priv %p, port %u" , + i, priv, priv->port_id); + }If device driver now has dependency on RCU you need to update meson.build to show that. No other driver does this. Creating threads in driver is discouraged because it can lead to other problems in applications.
Quoted 2 lines
If possible, I would figure out how to manage reset without creating a thread per-queue. Or at least one control thread and use
epoll()and eventfd's.
Update in meson.build was included in the patch.
I didn't create a thread per-queue. I just added a tid for each queue and use it as an identifier in rte_rcu_qsbr_thread_register(). Calling rte_rcu_qsbr_thread_register() doesn't create any additional thread. It just let the rcu_qsbr know how many tx and rx threads it should expect to be offline before device resetting can proceed.
Wei
Quoted 14 lines · On Fri, 29 May 2026 07:26:48 -0700 Wei Hu <[email protected]> wrote:
drivers/net/mana/mana.c +14
-0+#define MANA_OPS_1_LOCK (_func) \ +static int \ +_func##_lock(struct rte_eth_dev *dev) \ +{ \ + struct mana_priv *priv = dev->data->dev_private; \ + int ret; \ + if (!pthread_mutex_trylock(&priv->reset_ops_lock)) { \ + if (rte_atomic_load_explicit(&priv->dev_state, \ + rte_memory_order_acquire) != \ + MANA_DEV_ACTIVE) { \ + pthread_mutex_unlock(&priv->reset_ops_lock); \ + return -EBUSY; \ + } \ + ret = _func(dev); \
drivers/net/mana/mana.c mana_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) · line 975
Quoted 10 lines
drivers/net/mana/mana.c +10
-0+pthread_mutex_unlock (&priv->reset_ops_lock); \ + }else { \ + ret = -EBUSY ; \ + } \ +return ret; \ +} + +MANA_OPS_1_LOCK (mana_dev_configure) + +MANA_OPS_1_LOCK (mana_dev_start)
I strongly dislike wrapping locking in macros. Macros make code harder to analyze and hide things.
Also, using pthread_mutex here needs seems like a bigger hammer than needed. It looks like reset is just an atomic flag and as long as it was in shared memory simple atomic operators would suffice.
In Linux there are many ways to express the same thing but in general if most drivers follow the same patterns it helps when there is an issue to be able to fix it globally.
Also any code which turns off thread safety analysis is a red flag for me. It needs strong justification.
Overall this reset logic looks more complex than other drivers.
Quoted 39 lines · On Fri, 29 May 2026 07:26:48 -0700 Wei Hu <[email protected]> wrote:
From: Wei Hu <[email protected]>
Add support for handling hardware reset events in the MANA driver. When the MANA kernel driver receives a hardware service event, it initiates a device reset and notifies userspace via
IBV_EVENT_DEVICE_FATAL. The DPDK driver handles this by performing an automatic teardown and recovery sequence.The reset flow has two phases. In the enter phase, running on the EAL interrupt thread, the driver transitions the device state, waits for data path threads to reach a quiescent state using RCU, stops queues, tears down IB resources, and frees per-queue MR caches. A control thread is then spawned to handle the exit phase:
it waits for the hardware to recover, unregisters the interrupt handler, re-probes the PCI device, reinitializes MR caches, and restarts queues.A per-device mutex serializes the reset path with ethdev operations. The mutex uses
PTHREAD_PROCESS_SHAREDfor multi-process support and is held across blocking IB verbs calls. Operations that cannot wait (configure, queue setup) return-EBUSYduring reset, whiledev_stopanddev_closejoin the reset thread before acquiring the lock to ensure proper sequencing. A CAS-based helper prevents double-join of the reset thread.Multi-process support is included: secondary processes unmap and remap doorbell pages via IPC during the reset enter and exit phases. Data path functions in both primary and secondary processes check the device state atomically and return early when the device is not active. RCU quiescent state tracking uses per-queue thread IDs in shared hugepage memory, covering both primary and secondary process data path threads.
The driver uses ethdev recovery events to notify upper layers (e.g. netvsc) of the reset lifecycle:
RTE_ETH_EVENT_ERR_RECOVERINGon entry,RTE_ETH_EVENT_RECOVERY_SUCCESSorRTE_ETH_EVENT_RECOVERY_FAILEDon completion. A PCI device removal event callback distinguishes hot-remove from service reset.
Quoted 5 lines
Documentation for the device reset feature is added in the MANA NIC guide and the 26.07 release notes.
Signed-off-by Wei Hu <[email protected]>---
I went a deeper with AI and tried to figure out a good way to do what the driver is trying to do without reinventing so much.
This reset logic is considerably more complex than other DPDK drivers, and most of the complexity looks self-inflicted. A few specific things.
The RCU use is not really RCU. thread_online/offline are called on every rx/tx burst, and the "thread" token is the queue index, not a thread. So it is a per-queue in-use flag paid for on the hottest path, plus a new library dependency, to express "wait until no queue is mid-burst" -- which the driver already half does by swapping the burst function to mana_*_burst_removed and checking dev_state. Please drop the rcu use. If you must drain readers, a per-queue atomic flag is lighter and local; the fast path already has the dev_state acquire-load it needs.
The data path only needs the atomic, and that part is fine. The lock is legitimate for serializing the teardown/rebuild against control ops, but the way it is used is the problem: it is acquired in mana_intr_handler, released in mana_reset_enter, re-acquired in mana_reset_thread, and released in mana_reset_exit_delay. That cross-function, cross-thread handoff is exactly why every function needs __rte_no_thread_safety_analysis. Acquire and release the lock in the same function and the annotations all go away. Turning off thread-safety analysis needs strong justification and this does not have it.
Wrapping the ops in MANA_OPS_*_LOCK macros hides the lock/state protocol. A single explicit helper at the top of each op is just as terse and stays analyzable.
Two real bugs:
- Recovery and INTR_RMV events are sent via rte_eth_dev_callback_process while reset_ops_lock is held. An app handling INTR_RMV or RECOVERY_FAILED by calling dev_stop/dev_close will re-enter the lock (non-recursive) and deadlock; on the recovery path the callback runs on the reset thread so it also tries to join itself. Emit these events after dropping the lock, as ERR_RECOVERING already does.
- thread_online is taken at the top of the burst functions but thread_offline is only on some return paths. Any early return that misses it leaves a token non-quiescent and rte_rcu_qsbr_check() in mana_reset_enter spins forever. This is the kind of breakage the per-burst bracketing invites -- another reason to drop it.
Also: reset_ops_lock is held across ibv_close_device and the PCI re-probe (blocking under a sleeping mutex), and rte_alarm.h is included but no longer used.
Please look at how hns3 or mlx5 structure reset/recovery. Matching the common pattern means fixes can be made across drivers at once.
Thanks for the review on v5, Stephen.
Quoted 8 lines
-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Tuesday, June 2, 2026 12:58 AM
To: Wei Hu <[email protected]>
Cc: [email protected]; Long Li <[email protected]>; Wei Hu <[email protected]>
Subject: [EXTERNAL] Re: [PATCH v5 1/1] net/mana: add device reset support
Quoted 9 lines
The RCU use is not really RCU. thread_online/offline are called on every rx/tx burst, and the "thread" token is the queue index, not a thread. So it is a per- queue in-use flag paid for on the hottest path, plus a new library dependency, to express "wait until no queue is mid-burst" -- which the driver already half does by swapping the burst function to mana_*_burst_removed and checking
dev_state. Please drop the rcu use. If you must drain readers, a per-queue atomic flag is lighter and local; the fast path already has thedev_stateacquire- load it needs.
I have tested just swapping to mana_*_burst_removed doesn't stop the application from entering the burst functions. I will add per-queue atomic flags and spin poll all queue flags in the slower reset path. I think the actual fast path cost is nearly identical. However, it does remove the rcu library dependency.
Quoted 9 lines
The data path only needs the atomic, and that part is fine. The lock is legitimate for serializing the teardown/rebuild against control ops, but the way it is used is the problem: it is acquired in
mana_intr_handler, released inmana_reset_enter, re-acquired inmana_reset_thread, and released inmana_reset_exit_delay. That cross-function, cross-thread handoff is exactly why every function needs__rte_no_thread_safety_analysis.
Acquire and release the lock in the same function and the annotations all go away. Turning off thread-safety analysis needs strong justification and this does not have it.
I will remove the __rte_no_thread_safety_analysis from the code. It was added when I used spin lock and it caused clang build warnings. Now I have changed it to pthread mutex and clang build is clean without this.
Quoted 6 lines
-
thread_onlineis taken at the top of the burst functions butthread_offlineis only on some return paths. Any early return that misses it leaves a token non-quiescent andrte_rcu_qsbr_check()inmana_reset_enterspins forever. This is the kind of breakage the per-burst bracketing invites -- another reason to drop it.
I have checked mana_rx_burst and mana_tx_burst. There's no actual bug now -- every return path has thread-offline. Changing rcu to per-queue flags would still need to reset the flag in every return path. I agree it is kind of fragile. However, the reset path needs it to make sure there is no thread still in the fast path before it can start to tear down the resources.
I will send out a new revision.
Thanks,
Wei