* [PATCH v1 1/3] net/ntnic: remove unused unmanaged virt queue setup ops
2026-09-08 16:27 [PATCH v1 0/3] net/ntnic: fix virt queue data in BSS Serhii Iliushyk
@ 2026-09-08 16:27 ` Serhii Iliushyk
2026-09-08 16:27 ` [PATCH v1 2/3] net/ntnic: check virt queue hardware setup result Serhii Iliushyk
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Serhii Iliushyk @ 2026-09-08 16:27 UTC (permalink / raw)
To: stephen, mko-plv, ckm; +Cc: dev
The unmanaged Rx/Tx virt queue setup ops have never had a caller;
only the managed variants are used. Drop them in preparation for
allocating the queue state per queue.
Signed-off-by: Serhii Iliushyk <sil-plv@napatech.com>
---
drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c | 2 --
drivers/net/ntnic/ntnic_mod_reg.h | 30 -------------------
2 files changed, 32 deletions(-)
diff --git a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
index 56eb710d25..7c10acb4e2 100644
--- a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
+++ b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
@@ -1432,8 +1432,6 @@ static void nthw_release_tx_packets(struct nthw_virt_queue *txvq, uint16_t n, ui
}
static struct sg_ops_s sg_ops = {
- .nthw_setup_rx_virt_queue = nthw_setup_rx_virt_queue,
- .nthw_setup_tx_virt_queue = nthw_setup_tx_virt_queue,
.nthw_setup_mngd_rx_virt_queue = nthw_setup_mngd_rx_virt_queue,
.nthw_release_mngd_rx_virt_queue = nthw_release_mngd_rx_virt_queue,
.nthw_setup_mngd_tx_virt_queue = nthw_setup_mngd_tx_virt_queue,
diff --git a/drivers/net/ntnic/ntnic_mod_reg.h b/drivers/net/ntnic/ntnic_mod_reg.h
index 81ba1e44f9..1b5047b3dd 100644
--- a/drivers/net/ntnic/ntnic_mod_reg.h
+++ b/drivers/net/ntnic/ntnic_mod_reg.h
@@ -24,36 +24,6 @@
/* sg ops section */
struct sg_ops_s {
- /* Setup a virtQueue for a VM */
- struct nthw_virt_queue *(*nthw_setup_rx_virt_queue)(nthw_dbs_t *p_nthw_dbs,
- uint32_t index,
- uint16_t start_idx,
- uint16_t start_ptr,
- void *avail_struct_phys_addr,
- void *used_struct_phys_addr,
- void *desc_struct_phys_addr,
- uint16_t queue_size,
- uint32_t host_id,
- uint32_t header,
- uint32_t vq_type,
- int irq_vector,
- uint8_t rx_deferred_start);
- struct nthw_virt_queue *(*nthw_setup_tx_virt_queue)(nthw_dbs_t *p_nthw_dbs,
- uint32_t index,
- uint16_t start_idx,
- uint16_t start_ptr,
- void *avail_struct_phys_addr,
- void *used_struct_phys_addr,
- void *desc_struct_phys_addr,
- uint16_t queue_size,
- uint32_t host_id,
- uint32_t port,
- uint32_t virtual_port,
- uint32_t header,
- uint32_t vq_type,
- int irq_vector,
- uint32_t in_order,
- uint8_t tx_deferred_start);
struct nthw_virt_queue *(*nthw_setup_mngd_rx_virt_queue)(nthw_dbs_t *p_nthw_dbs,
uint32_t index,
uint32_t queue_size,
--
2.45.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v1 2/3] net/ntnic: check virt queue hardware setup result
2026-09-08 16:27 [PATCH v1 0/3] net/ntnic: fix virt queue data in BSS Serhii Iliushyk
2026-09-08 16:27 ` [PATCH v1 1/3] net/ntnic: remove unused unmanaged virt queue setup ops Serhii Iliushyk
@ 2026-09-08 16:27 ` Serhii Iliushyk
2026-09-08 16:27 ` [PATCH v1 3/3] net/ntnic: fix virt queue data in BSS Serhii Iliushyk
2026-09-08 17:18 ` [PATCH v1 0/3] " Stephen Hemminger
3 siblings, 0 replies; 5+ messages in thread
From: Serhii Iliushyk @ 2026-09-08 16:27 UTC (permalink / raw)
To: stephen, mko-plv, ckm; +Cc: dev, stable
The managed virt queue setup ignored the result of programming the
DBS registers and returned the queue as usable.
Return an error from the hardware setup and, when it fails, fail the
managed split and packed setup paths and free the descriptor copy.
Fixes: e13da07fd9fd ("net/ntnic: add split-queue support")
Fixes: af30088786c2 ("net/ntnic: add virtqueue setup managed packed-ring")
Cc: stable@dpdk.org
Signed-off-by: Serhii Iliushyk <sil-plv@napatech.com>
---
drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c | 158 ++++++++++--------
1 file changed, 88 insertions(+), 70 deletions(-)
diff --git a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
index 7c10acb4e2..af200432a2 100644
--- a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
+++ b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
@@ -357,7 +357,8 @@ static uint8_t dbs_qsize_log2(uint16_t qsize)
return qs;
}
-static struct nthw_virt_queue *nthw_setup_rx_virt_queue(nthw_dbs_t *p_nthw_dbs,
+static int nthw_setup_rx_virt_queue(struct nthw_virt_queue *vq,
+ nthw_dbs_t *p_nthw_dbs,
uint32_t index,
uint16_t start_idx,
uint16_t start_ptr,
@@ -383,7 +384,7 @@ static struct nthw_virt_queue *nthw_setup_rx_virt_queue(nthw_dbs_t *p_nthw_dbs,
*/
if (nthw_set_rx_dr_data(p_nthw_dbs, index, (uint64_t)desc_struct_phys_addr,
host_id, qs, header, 0) != 0) {
- return NULL;
+ return -1;
}
/*
@@ -402,7 +403,7 @@ static struct nthw_virt_queue *nthw_setup_rx_virt_queue(nthw_dbs_t *p_nthw_dbs,
if (nthw_set_rx_uw_data(p_nthw_dbs, index,
(uint64_t)used_struct_phys_addr,
host_id, qs, 0, int_enable, vec, istk) != 0) {
- return NULL;
+ return -1;
}
/*
@@ -416,7 +417,7 @@ static struct nthw_virt_queue *nthw_setup_rx_virt_queue(nthw_dbs_t *p_nthw_dbs,
if (nthw_set_rx_am_data(p_nthw_dbs, index, (uint64_t)avail_struct_phys_addr,
RX_AM_DISABLE, host_id, 0,
0) != 0) {
- return NULL;
+ return -1;
}
}
@@ -433,25 +434,24 @@ static struct nthw_virt_queue *nthw_setup_rx_virt_queue(nthw_dbs_t *p_nthw_dbs,
uint32_t enable = rx_deferred_start ? RX_AM_DISABLE : RX_AM_ENABLE;
if (nthw_set_rx_am_data(p_nthw_dbs, index, (uint64_t)avail_struct_phys_addr, enable,
host_id, 0, irq_vector >= 0 ? 1 : 0) != 0) {
- return NULL;
+ return -1;
}
/* Save queue state */
- rxvq[index].usage = NTHW_VIRTQ_UNMANAGED;
- rxvq[index].mp_nthw_dbs = p_nthw_dbs;
- rxvq[index].index = index;
- rxvq[index].queue_size = queue_size;
- rxvq[index].am_enable = (irq_vector < 0) ? RX_AM_ENABLE : RX_AM_DISABLE;
- rxvq[index].host_id = host_id;
- rxvq[index].avail_struct_phys_addr = avail_struct_phys_addr;
- rxvq[index].used_struct_phys_addr = used_struct_phys_addr;
- rxvq[index].desc_struct_phys_addr = desc_struct_phys_addr;
- rxvq[index].vq_type = vq_type;
- rxvq[index].in_order = 0; /* not used */
- rxvq[index].irq_vector = irq_vector;
-
- /* Return queue handle */
- return &rxvq[index];
+ vq->usage = NTHW_VIRTQ_UNMANAGED;
+ vq->mp_nthw_dbs = p_nthw_dbs;
+ vq->index = index;
+ vq->queue_size = queue_size;
+ vq->am_enable = (irq_vector < 0) ? RX_AM_ENABLE : RX_AM_DISABLE;
+ vq->host_id = host_id;
+ vq->avail_struct_phys_addr = avail_struct_phys_addr;
+ vq->used_struct_phys_addr = used_struct_phys_addr;
+ vq->desc_struct_phys_addr = desc_struct_phys_addr;
+ vq->vq_type = vq_type;
+ vq->in_order = 0; /* not used */
+ vq->irq_vector = irq_vector;
+
+ return 0;
}
static int dbs_wait_hw_queue_shutdown(struct nthw_virt_queue *vq, int rx);
@@ -685,7 +685,8 @@ static int nthw_release_mngd_tx_virt_queue(struct nthw_virt_queue *txvq)
return dbs_internal_release_tx_virt_queue(txvq);
}
-static struct nthw_virt_queue *nthw_setup_tx_virt_queue(nthw_dbs_t *p_nthw_dbs,
+static int nthw_setup_tx_virt_queue(struct nthw_virt_queue *vq,
+ nthw_dbs_t *p_nthw_dbs,
uint32_t index,
uint16_t start_idx,
uint16_t start_ptr,
@@ -714,7 +715,7 @@ static struct nthw_virt_queue *nthw_setup_tx_virt_queue(nthw_dbs_t *p_nthw_dbs,
*/
if (nthw_set_tx_dr_data(p_nthw_dbs, index, (uint64_t)desc_struct_phys_addr,
host_id, qs, port, header, 0) != 0) {
- return NULL;
+ return -1;
}
/*
@@ -732,7 +733,7 @@ static struct nthw_virt_queue *nthw_setup_tx_virt_queue(nthw_dbs_t *p_nthw_dbs,
if (nthw_set_tx_uw_data(p_nthw_dbs, index,
(uint64_t)used_struct_phys_addr,
host_id, qs, 0, int_enable, vec, istk, in_order) != 0) {
- return NULL;
+ return -1;
}
/*
@@ -741,7 +742,7 @@ static struct nthw_virt_queue *nthw_setup_tx_virt_queue(nthw_dbs_t *p_nthw_dbs,
*/
if (nthw_set_tx_am_data(p_nthw_dbs, index, (uint64_t)avail_struct_phys_addr, TX_AM_DISABLE,
host_id, 0, irq_vector >= 0 ? 1 : 0) != 0) {
- return NULL;
+ return -1;
}
/*
@@ -751,7 +752,7 @@ static struct nthw_virt_queue *nthw_setup_tx_virt_queue(nthw_dbs_t *p_nthw_dbs,
dbs_init_tx_queue(p_nthw_dbs, index, start_idx, start_ptr);
if (nthw_dbs_set_tx_qp_data(p_nthw_dbs, index, virtual_port) != 0)
- return NULL;
+ return -1;
/*
* 2. Configure the DBS.TX_AM_DATA memory and enable the queues you plan to use;
@@ -765,28 +766,27 @@ static struct nthw_virt_queue *nthw_setup_tx_virt_queue(nthw_dbs_t *p_nthw_dbs,
uint32_t enable = tx_deferred_start ? TX_AM_DISABLE : TX_AM_ENABLE;
if (nthw_set_tx_am_data(p_nthw_dbs, index, (uint64_t)avail_struct_phys_addr,
enable, host_id, 0, 0) != 0) {
- return NULL;
+ return -1;
}
}
/* Save queue state */
- txvq[index].usage = NTHW_VIRTQ_UNMANAGED;
- txvq[index].mp_nthw_dbs = p_nthw_dbs;
- txvq[index].index = index;
- txvq[index].queue_size = queue_size;
- txvq[index].am_enable = (irq_vector < 0) ? TX_AM_ENABLE : TX_AM_DISABLE;
- txvq[index].host_id = host_id;
- txvq[index].port = port;
- txvq[index].virtual_port = virtual_port;
- txvq[index].avail_struct_phys_addr = avail_struct_phys_addr;
- txvq[index].used_struct_phys_addr = used_struct_phys_addr;
- txvq[index].desc_struct_phys_addr = desc_struct_phys_addr;
- txvq[index].vq_type = vq_type;
- txvq[index].in_order = in_order;
- txvq[index].irq_vector = irq_vector;
-
- /* Return queue handle */
- return &txvq[index];
+ vq->usage = NTHW_VIRTQ_UNMANAGED;
+ vq->mp_nthw_dbs = p_nthw_dbs;
+ vq->index = index;
+ vq->queue_size = queue_size;
+ vq->am_enable = (irq_vector < 0) ? TX_AM_ENABLE : TX_AM_DISABLE;
+ vq->host_id = host_id;
+ vq->port = port;
+ vq->virtual_port = virtual_port;
+ vq->avail_struct_phys_addr = avail_struct_phys_addr;
+ vq->used_struct_phys_addr = used_struct_phys_addr;
+ vq->desc_struct_phys_addr = desc_struct_phys_addr;
+ vq->vq_type = vq_type;
+ vq->in_order = in_order;
+ vq->irq_vector = irq_vector;
+
+ return 0;
}
static struct nthw_virt_queue *
@@ -830,12 +830,16 @@ nthw_setup_mngd_rx_virt_queue_split(nthw_dbs_t *p_nthw_dbs,
queue_size * sizeof(*p_packet_buffers));
}
- nthw_setup_rx_virt_queue(p_nthw_dbs, index, 0, 0, (void *)p_virt_struct_area->phys_addr,
- (char *)p_virt_struct_area->phys_addr +
- virtq_struct_layout.used_offset,
- (char *)p_virt_struct_area->phys_addr +
- virtq_struct_layout.desc_offset,
- (uint16_t)queue_size, host_id, header, SPLIT_RING, irq_vector, rx_deferred_start);
+ if (nthw_setup_rx_virt_queue(&rxvq[index], p_nthw_dbs, index, 0, 0,
+ (void *)p_virt_struct_area->phys_addr,
+ (char *)p_virt_struct_area->phys_addr + virtq_struct_layout.used_offset,
+ (char *)p_virt_struct_area->phys_addr + virtq_struct_layout.desc_offset,
+ (uint16_t)queue_size, host_id, header, SPLIT_RING, irq_vector,
+ rx_deferred_start) != 0) {
+ free(rxvq[index].p_virtual_addr);
+ rxvq[index].p_virtual_addr = NULL;
+ return NULL;
+ }
rxvq[index].usage = NTHW_VIRTQ_MANAGED;
@@ -888,13 +892,16 @@ nthw_setup_mngd_tx_virt_queue_split(nthw_dbs_t *p_nthw_dbs,
queue_size * sizeof(*p_packet_buffers));
}
- nthw_setup_tx_virt_queue(p_nthw_dbs, index, 0, 0, (void *)p_virt_struct_area->phys_addr,
- (char *)p_virt_struct_area->phys_addr +
- virtq_struct_layout.used_offset,
- (char *)p_virt_struct_area->phys_addr +
- virtq_struct_layout.desc_offset,
- (uint16_t)queue_size, host_id, port, virtual_port, header,
- SPLIT_RING, irq_vector, in_order, tx_deferred_start);
+ if (nthw_setup_tx_virt_queue(&txvq[index], p_nthw_dbs, index, 0, 0,
+ (void *)p_virt_struct_area->phys_addr,
+ (char *)p_virt_struct_area->phys_addr + virtq_struct_layout.used_offset,
+ (char *)p_virt_struct_area->phys_addr + virtq_struct_layout.desc_offset,
+ (uint16_t)queue_size, host_id, port, virtual_port, header,
+ SPLIT_RING, irq_vector, in_order, tx_deferred_start) != 0) {
+ free(txvq[index].p_virtual_addr);
+ txvq[index].p_virtual_addr = NULL;
+ return NULL;
+ }
txvq[index].usage = NTHW_VIRTQ_MANAGED;
@@ -996,13 +1003,18 @@ nthw_setup_managed_rx_virt_queue_packed(nthw_dbs_t *p_nthw_dbs,
VIRTQ_DESC_F_WRITE | VIRTQ_DESC_F_AVAIL, 1) != 0)
return NULL;
- nthw_setup_rx_virt_queue(p_nthw_dbs, index, 0x8000, 0, /* start wrap ring counter as 1 */
- (void *)((uintptr_t)p_virt_struct_area->phys_addr +
- pvirtq_layout.driver_event_offset),
- (void *)((uintptr_t)p_virt_struct_area->phys_addr +
- pvirtq_layout.device_event_offset),
- p_virt_struct_area->phys_addr, (uint16_t)queue_size, host_id,
- header, PACKED_RING, irq_vector, rx_deferred_start);
+ if (nthw_setup_rx_virt_queue(vq, p_nthw_dbs, index,
+ 0x8000, 0, /* start wrap ring counter as 1 */
+ (void *)((uintptr_t)p_virt_struct_area->phys_addr +
+ pvirtq_layout.driver_event_offset),
+ (void *)((uintptr_t)p_virt_struct_area->phys_addr +
+ pvirtq_layout.device_event_offset),
+ p_virt_struct_area->phys_addr, (uint16_t)queue_size, host_id,
+ header, PACKED_RING, irq_vector, rx_deferred_start) != 0) {
+ free(vq->p_virtual_addr);
+ vq->p_virtual_addr = NULL;
+ return NULL;
+ }
vq->usage = NTHW_VIRTQ_MANAGED;
return vq;
@@ -1031,13 +1043,19 @@ nthw_setup_managed_tx_virt_queue_packed(nthw_dbs_t *p_nthw_dbs,
p_packet_buffers, 0, 0) != 0)
return NULL;
- nthw_setup_tx_virt_queue(p_nthw_dbs, index, 0x8000, 0, /* start wrap ring counter as 1 */
- (void *)((uintptr_t)p_virt_struct_area->phys_addr +
- pvirtq_layout.driver_event_offset),
- (void *)((uintptr_t)p_virt_struct_area->phys_addr +
- pvirtq_layout.device_event_offset),
- p_virt_struct_area->phys_addr, (uint16_t)queue_size, host_id,
- port, virtual_port, header, PACKED_RING, irq_vector, in_order, tx_deferred_start);
+ if (nthw_setup_tx_virt_queue(vq, p_nthw_dbs, index,
+ 0x8000, 0, /* start wrap ring counter as 1 */
+ (void *)((uintptr_t)p_virt_struct_area->phys_addr +
+ pvirtq_layout.driver_event_offset),
+ (void *)((uintptr_t)p_virt_struct_area->phys_addr +
+ pvirtq_layout.device_event_offset),
+ p_virt_struct_area->phys_addr, (uint16_t)queue_size, host_id,
+ port, virtual_port, header, PACKED_RING, irq_vector, in_order,
+ tx_deferred_start) != 0) {
+ free(vq->p_virtual_addr);
+ vq->p_virtual_addr = NULL;
+ return NULL;
+ }
vq->usage = NTHW_VIRTQ_MANAGED;
return vq;
--
2.45.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v1 3/3] net/ntnic: fix virt queue data in BSS
2026-09-08 16:27 [PATCH v1 0/3] net/ntnic: fix virt queue data in BSS Serhii Iliushyk
2026-09-08 16:27 ` [PATCH v1 1/3] net/ntnic: remove unused unmanaged virt queue setup ops Serhii Iliushyk
2026-09-08 16:27 ` [PATCH v1 2/3] net/ntnic: check virt queue hardware setup result Serhii Iliushyk
@ 2026-09-08 16:27 ` Serhii Iliushyk
2026-09-08 17:18 ` [PATCH v1 0/3] " Stephen Hemminger
3 siblings, 0 replies; 5+ messages in thread
From: Serhii Iliushyk @ 2026-09-08 16:27 UTC (permalink / raw)
To: stephen, mko-plv, ckm; +Cc: dev, stable
The virt queue state lived in two static arrays of 128 entries,
indexed by hardware queue id. Although touched on every burst, it
sat in BSS rather than in the hugepage backed EAL heap, wasted
space, was not NUMA aware, was not cache line aligned, was invisible
to a secondary process and was shared by all adapters.
Allocate one cache aligned queue object per queue, and the private
copy of the packet buffer descriptors, with rte_zmalloc_socket() on
the NUMA node of the adapter, and free them on release.
In ethdev, fail setup if allocation fails (a failed malloc() of the
descriptor copy was silently ignored before), release the queue from
the queue release callbacks (the descriptor copy leaked on re-setup
before), clear the handle after release, and register the queue with
ethdev only once setup succeeded.
Fixes: 576e77213f0d ("net/ntnic: add init for virtqueues in DBS")
Cc: stable@dpdk.org
Signed-off-by: Serhii Iliushyk <sil-plv@napatech.com>
---
drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c | 206 +++++++++++-------
drivers/net/ntnic/ntnic_ethdev.c | 52 ++++-
2 files changed, 173 insertions(+), 85 deletions(-)
diff --git a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
index af200432a2..ef8ad29aa1 100644
--- a/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
+++ b/drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c
@@ -3,6 +3,7 @@
*/
#include <rte_common.h>
+#include <rte_malloc.h>
#include <unistd.h>
#include "ntos_drv.h"
@@ -12,7 +13,6 @@
#include "ntlog.h"
#define STRUCT_ALIGNMENT (4 * 1024LU)
-#define MAX_VIRT_QUEUES 128
#define LAST_QUEUE 127
#define DISABLE 0
@@ -97,7 +97,14 @@ enum nthw_virt_queue_usage {
NTHW_VIRTQ_MANAGED
};
-struct nthw_virt_queue {
+/*
+ * Per-queue state. One instance is allocated per virt queue from hugepage
+ * memory on the NUMA node of the adapter, so the state touched in the
+ * Rx/Tx hot path lives next to the rings and packet buffers it describes,
+ * is visible to secondary processes and never shares a cache line with
+ * another queue.
+ */
+struct __rte_cache_aligned nthw_virt_queue {
/* Pointers to virt-queue structs */
union {
struct {
@@ -165,8 +172,54 @@ struct pvirtq_struct_layout_s {
size_t device_event_offset;
};
-static struct nthw_virt_queue rxvq[MAX_VIRT_QUEUES];
-static struct nthw_virt_queue txvq[MAX_VIRT_QUEUES];
+static int dbs_numa_node(const nthw_dbs_t *p_nthw_dbs)
+{
+ return p_nthw_dbs->mp_fpga->p_fpga_info->numa_node;
+}
+
+/* Allocate a zeroed queue object and bind it to its DBS module */
+static struct nthw_virt_queue *dbs_alloc_virt_queue(nthw_dbs_t *p_nthw_dbs)
+{
+ struct nthw_virt_queue *vq = rte_zmalloc_socket("ntnic_vq",
+ sizeof(struct nthw_virt_queue), RTE_CACHE_LINE_SIZE,
+ dbs_numa_node(p_nthw_dbs));
+
+ if (vq != NULL)
+ vq->mp_nthw_dbs = p_nthw_dbs;
+
+ return vq;
+}
+
+/*
+ * Keep a private copy of the packet buffer descriptors. It is indexed on
+ * every Rx/Tx burst, so place it with the rest of the queue state.
+ */
+static int dbs_copy_packet_buffers(struct nthw_virt_queue *vq,
+ const struct nthw_memory_descriptor *p_packet_buffers, uint32_t n)
+{
+ size_t size = (size_t)n * sizeof(*p_packet_buffers);
+
+ if (n == 0)
+ return -1; /* don't allocate memory with size of 0 bytes */
+
+ vq->p_virtual_addr = rte_zmalloc_socket("ntnic_vq_bufs", size, RTE_CACHE_LINE_SIZE,
+ dbs_numa_node(vq->mp_nthw_dbs));
+
+ if (vq->p_virtual_addr == NULL)
+ return -1;
+
+ memcpy(vq->p_virtual_addr, p_packet_buffers, size);
+ return 0;
+}
+
+static void dbs_free_virt_queue(struct nthw_virt_queue *vq)
+{
+ if (vq == NULL)
+ return;
+
+ rte_free(vq->p_virtual_addr);
+ rte_free(vq);
+}
static void dbs_init_rx_queue(nthw_dbs_t *p_nthw_dbs, uint32_t queue, uint32_t start_idx,
uint32_t start_ptr)
@@ -236,11 +289,6 @@ static int nthw_virt_queue_init(struct fpga_info_s *p_fpga_info)
p_fpga_info->mp_nthw_dbs = p_nthw_dbs;
- for (i = 0; i < MAX_VIRT_QUEUES; ++i) {
- rxvq[i].usage = NTHW_VIRTQ_UNUSED;
- txvq[i].usage = NTHW_VIRTQ_UNUSED;
- }
-
nthw_dbs_reset(p_nthw_dbs);
for (i = 0; i < NT_DBS_RX_QUEUES_MAX; ++i)
@@ -358,7 +406,6 @@ static uint8_t dbs_qsize_log2(uint16_t qsize)
}
static int nthw_setup_rx_virt_queue(struct nthw_virt_queue *vq,
- nthw_dbs_t *p_nthw_dbs,
uint32_t index,
uint16_t start_idx,
uint16_t start_ptr,
@@ -372,6 +419,7 @@ static int nthw_setup_rx_virt_queue(struct nthw_virt_queue *vq,
int irq_vector,
uint8_t rx_deferred_start)
{
+ nthw_dbs_t *p_nthw_dbs = vq->mp_nthw_dbs;
uint32_t qs = dbs_qsize_log2(queue_size);
uint32_t int_enable;
uint32_t vec;
@@ -439,7 +487,6 @@ static int nthw_setup_rx_virt_queue(struct nthw_virt_queue *vq,
/* Save queue state */
vq->usage = NTHW_VIRTQ_UNMANAGED;
- vq->mp_nthw_dbs = p_nthw_dbs;
vq->index = index;
vq->queue_size = queue_size;
vq->am_enable = (irq_vector < 0) ? RX_AM_ENABLE : RX_AM_DISABLE;
@@ -580,15 +627,17 @@ static int dbs_internal_release_rx_virt_queue(struct nthw_virt_queue *rxvq)
static int nthw_release_mngd_rx_virt_queue(struct nthw_virt_queue *rxvq)
{
+ int res;
+
if (rxvq == NULL || rxvq->usage != NTHW_VIRTQ_MANAGED)
return -1;
- if (rxvq->p_virtual_addr) {
- free(rxvq->p_virtual_addr);
- rxvq->p_virtual_addr = NULL;
- }
+ res = dbs_internal_release_rx_virt_queue(rxvq);
+
+ /* The queue object is owned here; the caller must drop its handle */
+ dbs_free_virt_queue(rxvq);
- return dbs_internal_release_rx_virt_queue(rxvq);
+ return res;
}
static int dbs_internal_release_tx_virt_queue(struct nthw_virt_queue *txvq)
@@ -674,19 +723,20 @@ static int dbs_internal_release_tx_virt_queue(struct nthw_virt_queue *txvq)
static int nthw_release_mngd_tx_virt_queue(struct nthw_virt_queue *txvq)
{
+ int res;
+
if (txvq == NULL || txvq->usage != NTHW_VIRTQ_MANAGED)
return -1;
- if (txvq->p_virtual_addr) {
- free(txvq->p_virtual_addr);
- txvq->p_virtual_addr = NULL;
- }
+ res = dbs_internal_release_tx_virt_queue(txvq);
- return dbs_internal_release_tx_virt_queue(txvq);
+ /* The queue object is owned here; the caller must drop its handle */
+ dbs_free_virt_queue(txvq);
+
+ return res;
}
static int nthw_setup_tx_virt_queue(struct nthw_virt_queue *vq,
- nthw_dbs_t *p_nthw_dbs,
uint32_t index,
uint16_t start_idx,
uint16_t start_ptr,
@@ -703,6 +753,7 @@ static int nthw_setup_tx_virt_queue(struct nthw_virt_queue *vq,
uint32_t in_order,
uint8_t tx_deferred_start)
{
+ nthw_dbs_t *p_nthw_dbs = vq->mp_nthw_dbs;
uint32_t int_enable;
uint32_t vec;
uint32_t istk;
@@ -772,7 +823,6 @@ static int nthw_setup_tx_virt_queue(struct nthw_virt_queue *vq,
/* Save queue state */
vq->usage = NTHW_VIRTQ_UNMANAGED;
- vq->mp_nthw_dbs = p_nthw_dbs;
vq->index = index;
vq->queue_size = queue_size;
vq->am_enable = (irq_vector < 0) ? TX_AM_ENABLE : TX_AM_DISABLE;
@@ -801,6 +851,10 @@ nthw_setup_mngd_rx_virt_queue_split(nthw_dbs_t *p_nthw_dbs,
uint8_t rx_deferred_start)
{
struct virtq_struct_layout_s virtq_struct_layout = dbs_calc_struct_layout(queue_size);
+ struct nthw_virt_queue *vq = dbs_alloc_virt_queue(p_nthw_dbs);
+
+ if (vq == NULL)
+ return NULL;
dbs_initialize_virt_queue_structs(p_virt_struct_area->virt_addr,
(char *)p_virt_struct_area->virt_addr +
@@ -812,38 +866,33 @@ nthw_setup_mngd_rx_virt_queue_split(nthw_dbs_t *p_nthw_dbs,
p_packet_buffers ? (uint16_t)queue_size : 0,
VIRTQ_DESC_F_WRITE /* Rx */);
- rxvq[index].p_avail = p_virt_struct_area->virt_addr;
- rxvq[index].p_used =
+ vq->p_avail = p_virt_struct_area->virt_addr;
+ vq->p_used =
(void *)((char *)p_virt_struct_area->virt_addr + virtq_struct_layout.used_offset);
- rxvq[index].p_desc =
+ vq->p_desc =
(void *)((char *)p_virt_struct_area->virt_addr + virtq_struct_layout.desc_offset);
- rxvq[index].am_idx = p_packet_buffers ? (uint16_t)queue_size : 0;
- rxvq[index].used_idx = 0;
- rxvq[index].cached_idx = 0;
- rxvq[index].p_virtual_addr = NULL;
+ vq->am_idx = p_packet_buffers ? (uint16_t)queue_size : 0;
- if (p_packet_buffers) {
- rxvq[index].p_virtual_addr = malloc(queue_size * sizeof(*p_packet_buffers));
- if (rxvq[index].p_virtual_addr)
- memcpy(rxvq[index].p_virtual_addr, p_packet_buffers,
- queue_size * sizeof(*p_packet_buffers));
+ if (p_packet_buffers &&
+ dbs_copy_packet_buffers(vq, p_packet_buffers, queue_size) != 0) {
+ dbs_free_virt_queue(vq);
+ return NULL;
}
- if (nthw_setup_rx_virt_queue(&rxvq[index], p_nthw_dbs, index, 0, 0,
+ if (nthw_setup_rx_virt_queue(vq, index, 0, 0,
(void *)p_virt_struct_area->phys_addr,
(char *)p_virt_struct_area->phys_addr + virtq_struct_layout.used_offset,
(char *)p_virt_struct_area->phys_addr + virtq_struct_layout.desc_offset,
(uint16_t)queue_size, host_id, header, SPLIT_RING, irq_vector,
rx_deferred_start) != 0) {
- free(rxvq[index].p_virtual_addr);
- rxvq[index].p_virtual_addr = NULL;
+ dbs_free_virt_queue(vq);
return NULL;
}
- rxvq[index].usage = NTHW_VIRTQ_MANAGED;
+ vq->usage = NTHW_VIRTQ_MANAGED;
- return &rxvq[index];
+ return vq;
}
static struct nthw_virt_queue *
@@ -861,6 +910,10 @@ nthw_setup_mngd_tx_virt_queue_split(nthw_dbs_t *p_nthw_dbs,
uint8_t tx_deferred_start)
{
struct virtq_struct_layout_s virtq_struct_layout = dbs_calc_struct_layout(queue_size);
+ struct nthw_virt_queue *vq = dbs_alloc_virt_queue(p_nthw_dbs);
+
+ if (vq == NULL)
+ return NULL;
dbs_initialize_virt_queue_structs(p_virt_struct_area->virt_addr,
(char *)p_virt_struct_area->virt_addr +
@@ -872,40 +925,32 @@ nthw_setup_mngd_tx_virt_queue_split(nthw_dbs_t *p_nthw_dbs,
0,
0 /* Tx */);
- txvq[index].p_avail = p_virt_struct_area->virt_addr;
- txvq[index].p_used =
+ vq->p_avail = p_virt_struct_area->virt_addr;
+ vq->p_used =
(void *)((char *)p_virt_struct_area->virt_addr + virtq_struct_layout.used_offset);
- txvq[index].p_desc =
+ vq->p_desc =
(void *)((char *)p_virt_struct_area->virt_addr + virtq_struct_layout.desc_offset);
- txvq[index].queue_size = (uint16_t)queue_size;
- txvq[index].am_idx = 0;
- txvq[index].used_idx = 0;
- txvq[index].cached_idx = 0;
- txvq[index].p_virtual_addr = NULL;
-
- txvq[index].tx_descr_avail_idx = 0;
-
- if (p_packet_buffers) {
- txvq[index].p_virtual_addr = malloc(queue_size * sizeof(*p_packet_buffers));
- if (txvq[index].p_virtual_addr)
- memcpy(txvq[index].p_virtual_addr, p_packet_buffers,
- queue_size * sizeof(*p_packet_buffers));
+ vq->queue_size = (uint16_t)queue_size;
+
+ if (p_packet_buffers &&
+ dbs_copy_packet_buffers(vq, p_packet_buffers, queue_size) != 0) {
+ dbs_free_virt_queue(vq);
+ return NULL;
}
- if (nthw_setup_tx_virt_queue(&txvq[index], p_nthw_dbs, index, 0, 0,
+ if (nthw_setup_tx_virt_queue(vq, index, 0, 0,
(void *)p_virt_struct_area->phys_addr,
(char *)p_virt_struct_area->phys_addr + virtq_struct_layout.used_offset,
(char *)p_virt_struct_area->phys_addr + virtq_struct_layout.desc_offset,
(uint16_t)queue_size, host_id, port, virtual_port, header,
SPLIT_RING, irq_vector, in_order, tx_deferred_start) != 0) {
- free(txvq[index].p_virtual_addr);
- txvq[index].p_virtual_addr = NULL;
+ dbs_free_virt_queue(vq);
return NULL;
}
- txvq[index].usage = NTHW_VIRTQ_MANAGED;
+ vq->usage = NTHW_VIRTQ_MANAGED;
- return &txvq[index];
+ return vq;
}
/*
@@ -965,16 +1010,9 @@ static int nthw_setup_managed_virt_queue_packed(struct nthw_virt_queue *vq,
else
vq->used_wrap_count ^= 1; /* pre-fill free buffer IDs */
- if (vq->queue_size == 0)
- return -1; /* don't allocate memory with size of 0 bytes */
-
- vq->p_virtual_addr = malloc(vq->queue_size * sizeof(*p_packet_buffers));
-
- if (vq->p_virtual_addr == NULL)
+ if (dbs_copy_packet_buffers(vq, p_packet_buffers, vq->queue_size) != 0)
return -1;
- memcpy(vq->p_virtual_addr, p_packet_buffers, vq->queue_size * sizeof(*p_packet_buffers));
-
/* Not used yet by FPGA - make sure we disable */
vq->device_event->flags = RING_EVENT_FLAGS_DISABLE;
@@ -993,17 +1031,23 @@ nthw_setup_managed_rx_virt_queue_packed(nthw_dbs_t *p_nthw_dbs,
uint8_t rx_deferred_start)
{
struct pvirtq_struct_layout_s pvirtq_layout;
- struct nthw_virt_queue *vq = &rxvq[index];
+ struct nthw_virt_queue *vq = dbs_alloc_virt_queue(p_nthw_dbs);
+
+ if (vq == NULL)
+ return NULL;
+
/* Set size and setup packed vq ring */
vq->queue_size = queue_size;
/* Use Avail flag bit == 1 because wrap bit is initially set to 1 - and Used is inverse */
if (nthw_setup_managed_virt_queue_packed(vq, &pvirtq_layout, p_virt_struct_area,
p_packet_buffers,
- VIRTQ_DESC_F_WRITE | VIRTQ_DESC_F_AVAIL, 1) != 0)
+ VIRTQ_DESC_F_WRITE | VIRTQ_DESC_F_AVAIL, 1) != 0) {
+ dbs_free_virt_queue(vq);
return NULL;
+ }
- if (nthw_setup_rx_virt_queue(vq, p_nthw_dbs, index,
+ if (nthw_setup_rx_virt_queue(vq, index,
0x8000, 0, /* start wrap ring counter as 1 */
(void *)((uintptr_t)p_virt_struct_area->phys_addr +
pvirtq_layout.driver_event_offset),
@@ -1011,8 +1055,7 @@ nthw_setup_managed_rx_virt_queue_packed(nthw_dbs_t *p_nthw_dbs,
pvirtq_layout.device_event_offset),
p_virt_struct_area->phys_addr, (uint16_t)queue_size, host_id,
header, PACKED_RING, irq_vector, rx_deferred_start) != 0) {
- free(vq->p_virtual_addr);
- vq->p_virtual_addr = NULL;
+ dbs_free_virt_queue(vq);
return NULL;
}
@@ -1035,15 +1078,21 @@ nthw_setup_managed_tx_virt_queue_packed(nthw_dbs_t *p_nthw_dbs,
uint8_t tx_deferred_start)
{
struct pvirtq_struct_layout_s pvirtq_layout;
- struct nthw_virt_queue *vq = &txvq[index];
+ struct nthw_virt_queue *vq = dbs_alloc_virt_queue(p_nthw_dbs);
+
+ if (vq == NULL)
+ return NULL;
+
/* Set size and setup packed vq ring */
vq->queue_size = queue_size;
if (nthw_setup_managed_virt_queue_packed(vq, &pvirtq_layout, p_virt_struct_area,
- p_packet_buffers, 0, 0) != 0)
+ p_packet_buffers, 0, 0) != 0) {
+ dbs_free_virt_queue(vq);
return NULL;
+ }
- if (nthw_setup_tx_virt_queue(vq, p_nthw_dbs, index,
+ if (nthw_setup_tx_virt_queue(vq, index,
0x8000, 0, /* start wrap ring counter as 1 */
(void *)((uintptr_t)p_virt_struct_area->phys_addr +
pvirtq_layout.driver_event_offset),
@@ -1052,8 +1101,7 @@ nthw_setup_managed_tx_virt_queue_packed(nthw_dbs_t *p_nthw_dbs,
p_virt_struct_area->phys_addr, (uint16_t)queue_size, host_id,
port, virtual_port, header, PACKED_RING, irq_vector, in_order,
tx_deferred_start) != 0) {
- free(vq->p_virtual_addr);
- vq->p_virtual_addr = NULL;
+ dbs_free_virt_queue(vq);
return NULL;
}
diff --git a/drivers/net/ntnic/ntnic_ethdev.c b/drivers/net/ntnic/ntnic_ethdev.c
index 7cc90a7a5b..87d3046f29 100644
--- a/drivers/net/ntnic/ntnic_ethdev.c
+++ b/drivers/net/ntnic/ntnic_ethdev.c
@@ -986,6 +986,13 @@ static void eth_tx_queue_release(struct rte_eth_dev *eth_dev, uint16_t queue_id)
{
struct pmd_internals *internals = eth_dev->data->dev_private;
struct ntnic_tx_queue *tx_q = &internals->txq_scg[queue_id];
+
+ /* Stop the HW queue and free its state before the rings are unmapped */
+ if (sg_ops != NULL && tx_q->vq != NULL) {
+ sg_ops->nthw_release_mngd_tx_virt_queue(tx_q->vq);
+ tx_q->vq = NULL;
+ }
+
deallocate_hw_virtio_queues(&tx_q->hwq);
}
@@ -993,6 +1000,13 @@ static void eth_rx_queue_release(struct rte_eth_dev *eth_dev, uint16_t queue_id)
{
struct pmd_internals *internals = eth_dev->data->dev_private;
struct ntnic_rx_queue *rx_q = &internals->rxq_scg[queue_id];
+
+ /* Stop the HW queue and free its state before the rings are unmapped */
+ if (sg_ops != NULL && rx_q->vq != NULL) {
+ sg_ops->nthw_release_mngd_rx_virt_queue(rx_q->vq);
+ rx_q->vq = NULL;
+ }
+
deallocate_hw_virtio_queues(&rx_q->hwq);
}
@@ -1046,8 +1060,6 @@ static int eth_rx_scg_queue_setup(struct rte_eth_dev *eth_dev,
rx_q->mb_pool = mb_pool;
- eth_dev->data->rx_queues[rx_queue_id] = rx_q;
-
mbp_priv = rte_mempool_get_priv(rx_q->mb_pool);
rx_q->buf_size = (uint16_t)(mbp_priv->mbuf_data_room_size - RTE_PKTMBUF_HEADROOM);
rx_q->enabled = !rx_conf->rx_deferred_start;
@@ -1073,6 +1085,20 @@ static int eth_rx_scg_queue_setup(struct rte_eth_dev *eth_dev,
-1,
rx_conf->rx_deferred_start);
+ if (rx_q->vq == NULL) {
+ NT_LOG(ERR, NTNIC, "(%u) NTNIC RX queue %u virt queue setup failed",
+ internals->port, rx_queue_id);
+ deallocate_hw_virtio_queues(&rx_q->hwq);
+ return -1;
+ }
+
+ /*
+ * Hand the queue to ethdev only once it is fully set up. ethdev does not
+ * clear the entry when setup fails, and the release callback would then
+ * tear down rings that were already freed above.
+ */
+ eth_dev->data->rx_queues[rx_queue_id] = rx_q;
+
NT_LOG(DBG, NTNIC, "(%" PRIu32 ") NTNIC RX OVS-SW queues successfully setup",
internals->port);
@@ -1116,8 +1142,6 @@ static int eth_tx_scg_queue_setup(struct rte_eth_dev *eth_dev,
return -1;
}
- eth_dev->data->tx_queues[tx_queue_id] = tx_q;
-
/* Calculate target ID for HW - to be used in NTDVIO0 header bypass_port */
if (tx_q->rss_target_id >= 0) {
/* bypass to a multiqueue port - qsl-hsh index */
@@ -1167,6 +1191,16 @@ static int eth_tx_scg_queue_setup(struct rte_eth_dev *eth_dev,
IN_ORDER,
tx_conf->tx_deferred_start);
+ if (tx_q->vq == NULL) {
+ NT_LOG(ERR, NTNIC, "(%u) NTNIC TX queue %u virt queue setup failed",
+ tx_q->port, tx_queue_id);
+ deallocate_hw_virtio_queues(&tx_q->hwq);
+ return -1;
+ }
+
+ /* See eth_rx_scg_queue_setup() */
+ eth_dev->data->tx_queues[tx_queue_id] = tx_q;
+
tx_q->enabled = !tx_conf->tx_deferred_start;
tx_q->tx_deferred_start = tx_conf->tx_deferred_start;
@@ -1577,11 +1611,15 @@ eth_dev_close(struct rte_eth_dev *eth_dev)
uint q;
if (sg_ops != NULL) {
- for (q = 0; q < internals->nb_rx_queues; q++)
+ for (q = 0; q < internals->nb_rx_queues; q++) {
sg_ops->nthw_release_mngd_rx_virt_queue(rx_q[q].vq);
+ rx_q[q].vq = NULL;
+ }
- for (q = 0; q < internals->nb_tx_queues; q++)
+ for (q = 0; q < internals->nb_tx_queues; q++) {
sg_ops->nthw_release_mngd_tx_virt_queue(tx_q[q].vq);
+ tx_q[q].vq = NULL;
+ }
}
}
@@ -2651,11 +2689,13 @@ nthw_pci_dev_deinit(struct rte_eth_dev *eth_dev __rte_unused)
while (internals) {
for (i = internals->nb_tx_queues - 1; i >= 0; i--) {
sg_ops->nthw_release_mngd_tx_virt_queue(internals->txq_scg[i].vq);
+ internals->txq_scg[i].vq = NULL;
release_hw_virtio_queues(&internals->txq_scg[i].hwq);
}
for (i = internals->nb_rx_queues - 1; i >= 0; i--) {
sg_ops->nthw_release_mngd_rx_virt_queue(internals->rxq_scg[i].vq);
+ internals->rxq_scg[i].vq = NULL;
release_hw_virtio_queues(&internals->rxq_scg[i].hwq);
}
--
2.45.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v1 0/3] net/ntnic: fix virt queue data in BSS
2026-09-08 16:27 [PATCH v1 0/3] net/ntnic: fix virt queue data in BSS Serhii Iliushyk
` (2 preceding siblings ...)
2026-09-08 16:27 ` [PATCH v1 3/3] net/ntnic: fix virt queue data in BSS Serhii Iliushyk
@ 2026-09-08 17:18 ` Stephen Hemminger
3 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-09-08 17:18 UTC (permalink / raw)
To: Serhii Iliushyk; +Cc: mko-plv, ckm, dev
On Tue, 8 Sep 2026 18:27:21 +0200
Serhii Iliushyk <sil-plv@napatech.com> wrote:
> This series addresses the bug report "ntnic: virt queue data in BSS"
> (Bugzilla ID: 1975). The report lists five problems with keeping the
> virt queue state in static arrays; each is addressed as follows.
>
> - BSS memory is slower than hugepages because of TLB overhead, which
> can cause performance loss in the hot path.
> The per-queue state, and the copy of the packet buffer descriptors
> indexed on every burst, are now allocated from the EAL heap.
>
> - Hard coding the size wastes space.
> The 128-entry Rx and Tx arrays are gone; one object is allocated
> per configured queue and freed when the queue is released.
>
> - Data in BSS is not visible to a secondary process.
> The state now lives in EAL memory. The PMD still does not support
> primary/secondary operation, but this no longer stands in the way.
>
> - NUMA aware allocation was not possible.
> Both allocations are made with rte_zmalloc_socket() on the NUMA
> node of the adapter, next to the rings and buffers they describe.
>
> - Cache alignment.
> The queue object is cache line aligned and no longer shares a line
> with a neighboring queue.
>
> Patch 1 drops two never-called setup ops so that only the managed
> setup and release paths own the queue state. Patch 2 makes a failed
> hardware setup visible to the caller, which patch 3 relies on to free
> the queue object on failure. Patch 3 is the fix itself; it also fixes
> a leak of the descriptor copy on queue re-setup and an ignored
> allocation failure that led to a NULL dereference in the burst
> functions.
>
> Serhii Iliushyk (3):
> net/ntnic: remove unused unmanaged virt queue setup ops
> net/ntnic: check virt queue hardware setup result
> net/ntnic: fix virt queue data in BSS
>
> drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c | 334 +++++++++++-------
> drivers/net/ntnic/ntnic_ethdev.c | 52 ++-
> drivers/net/ntnic/ntnic_mod_reg.h | 30 --
> 3 files changed, 245 insertions(+), 171 deletions(-)
>
Looks good, applied to next-net.
^ permalink raw reply [flat|nested] 5+ messages in thread