Re: [PATCH v2] bus/pci: fix automatic interrupt type selection

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 20 Jun 2025 15:50:27 +0300
Pekka Riikonen <priikone@iki.fi> wrote:

> Check if kernel returns 0 interrupt vectors and try another interrupt
> type in that case.  Failing to check the vector count can select an
> interrupt type that's unusable.
> 
> Signed-off-by: Pekka Riikonen <priikone@iki.fi>

Patch looks good, automated review had some feedback.

1. Should add a Fixes: and Cc: stable@dpdk.org

2. There is a pre-existing bugs in the file as well

Error 1: Missing handling when interrupt type was explicitly requested and count is 0

Confidence: ~85%

The existing VFIO_IRQ_INFO_EVENTFD check (lines 223–228) has two branches: if the user explicitly requested this interrupt type (intr_mode != RTE_INTR_MODE_NONE), it returns an error; otherwise it continues to the next type. The new irq.count == 0 check does not mirror this pattern — it unconditionally does continue.

This means if a user explicitly requested e.g. MSI-X via command line (--vfio-intr=msix) but the device reports 0 vectors for MSI-X, the code will silently fall through and try MSI or INTx instead of reporting an error. This is inconsistent with the existing behavior for the eventfd flag check, and arguably a bug: the user asked for a specific interrupt type that doesn't work, and should be told.

Suggested fix:
c

		/* If no vectors, try another type (or fail if explicitly requested) */
		if (irq.count == 0) {
			if (intr_mode != RTE_INTR_MODE_NONE) {
				PCI_LOG(ERR, "Interrupt vector has no entries!");
				return -1;
			}
			continue;
		}

Error 2: Resource leak — eventfd fd leaked on later error paths (pre-existing)

Confidence: ~90%

This is a pre-existing issue, not introduced by this patch, but worth noting since the patch touches this function. At line 240, eventfd() creates a file descriptor. If rte_intr_fd_set() at line 247 fails, the function returns -1 without closing fd. This is a file descriptor leak on that error path.

Similarly, the rte_intr_event_list_update failure path at line 236–237 is fine (no fd allocated yet), but the rte_intr_fd_set failure is a real leak.

This is pre-existing and not caused by this patch, so it's informational rather than something blocking this patch.