Re: [PATCH v2 00/15] raw/ntb: post-merge fixes

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 6 Nov 2026 11:20:30 +0530
Raghavendra Ningoji <raghavendra.ningoji@amd.com> wrote:

> This series addresses the post-merge review of the AMD NTB support
> by Stephen. Some of the issues also exist in the Intel code and in
> released versions; those patches are tagged for stable.
> 
> - Patches 1-4 fix the type confusion in rte_pmd_ntb_get_mem_align(),
>   the unbounded peer memory window count, the off-by-one in the
>   address remapping and the memory window leaks.
> 
> - Patch 5 fixes the doorbell interrupts with vfio-pci: only MSI-X
>   vector 0 (DB0) was serviced, so the DB1/DB2 exchange of dev_stop
>   never completed and the stop waited for its 1s timeout.
> 
> - Patch 7 removes the AMD hardware event handling, which could not run
>   as wired, and documents that hardware events are not handled.
> 
> - Patches 8-9 fix the Rx path: a partially consumed packet was leaked
>   and desynchronized the ring, and an mbuf owned by the application
>   could be freed again on device stop.
> 
> - Patches 14-15 fix the peer device status accesses between the
>   interrupt thread and the stop/data paths, and the missing checks at
>   queue setup.
> 
> - The other patches are documentation and cleanup.
> 
> The peer memory window size check suggested for the AMD handshake is
> not added: all the AMD scratchpads are in use, and the window limit
> register programmed with the local memzone length already bounds what
> the peer can reach.
> 
> Tested with vfio-pci on two AMD EPYC Embedded hosts: handshake,
> file transfer, and stop/quit with DB1/DB2 exchange.
> 
> ---
> Changes since v1:
> - Patch 4: embed the memory window size array in struct ntb_hw instead
>   of freeing it on close, as the device stays attached after close and
>   mw_size could then be dereferenced as NULL.
> - Patch 5: allocate the doorbell interrupt handle array with calloc(),
>   as it only holds process-private pointers.
> - Added patch 14 for the peer device status atomic accesses.
> - Added patch 15 for the queue setup checks, also covering the Tx side
>   (unchecked nb_desc, and queue leak on invalid tx_free_thresh).
> 
> Raghavendra Ningoji (15):
>   raw/ntb: fix device check in memory alignment query
>   raw/ntb: fix peer memory window count check
>   raw/ntb: fix memory window bound in address remapping
>   raw/ntb: fix memory leaks
>   raw/ntb: fix doorbell interrupts with VFIO MSI-X
>   raw/ntb: fix AMD processor names
>   raw/ntb: remove AMD hardware event handling
>   raw/ntb: fix Rx of incomplete packets
>   raw/ntb: fix double free of Rx mbufs on stop
>   examples/ntb: fix raw/ntb library linking
>   doc: add NTB device info change to ABI section
>   doc: add IOVA requirement for NTB
>   raw/ntb: remove unused AMD register definitions
>   raw/ntb: fix peer device status access
>   raw/ntb: fix queue setup checks
> 
>  doc/guides/rawdevs/ntb.rst             |  12 +-
>  doc/guides/rel_notes/release_26_11.rst |   6 +-
>  drivers/raw/ntb/ntb.c                  | 212 +++++++++++++++++++------
>  drivers/raw/ntb/ntb.h                  |  17 +-
>  drivers/raw/ntb/ntb_hw_amd.c           |  60 ++-----
>  drivers/raw/ntb/ntb_hw_amd.h           |  25 +--
>  drivers/raw/ntb/ntb_hw_intel.c         |  17 +-
>  examples/ntb/Makefile                  |   3 +-
>  8 files changed, 210 insertions(+), 142 deletions(-)
> 

This version gets clean review from manual AI run.
It did see one pre-existing bug though.

Subject: Re: [PATCH v2 00/15] raw/ntb: post-merge fixes

v2 applies to main (26.11-rc2) and builds at every commit with
-Dwerror=true, and with -Denable_stdatomic=true at the tip.
The v1 comments on 04/15 and 05/15 are addressed; 14/15 and 15/15
cover the pre-existing items.

Patch 15/15 raw/ntb: fix queue setup checks

Info (pre-existing, not introduced by this patch): ntb_queue_setup()
still leaks on two paths the commit message does not cover.

ntb_txq_setup() runs first and installs the queue:

	hw->tx_queues[qp_id] = txq;

so when ntb_rxq_setup() then fails (rx_mp NULL, or allocation
failure), the Tx queue is left installed. Any second setup of the
same queue id (retry after that failure, or reconfiguration)
overwrites hw->tx_queues[qp_id] and hw->rx_queues[qp_id] without
freeing the previous queues.

Validate the shared conf before touching either queue, and release
what is already there:

	if (queue_id >= hw->queue_pairs)
		return -EINVAL;

	if (conf->rx_mp == NULL)
		return -EINVAL;

	if (hw->tx_queues[queue_id] != NULL ||
	    hw->rx_queues[queue_id] != NULL)
		ntb_queue_release(dev, queue_id);

	ret = ntb_txq_setup(dev, queue_id, queue_conf, conf_size);
	if (ret < 0)
		return ret;

	ret = ntb_rxq_setup(dev, queue_id, queue_conf, conf_size);
	if (ret < 0)
		ntb_queue_release(dev, queue_id);
	return ret;

(ntb_txq_release()/ntb_rxq_release() log an error on NULL, so a
NULL check on each would be needed for the last release.) Fine as a
follow-up.