Initialize fd, dev_fd, and all efds array elements as invalid in rte_intr_instance_alloc(). This follows Unix convention, avoiding confusion with fd 0 which could result from zero-initialization.
Update the API documentation to reflect this behavior and add a unit test check to verify the fd is invalid after allocation. Update the FreeBSD PCI implementation that checked against a 0 FD. Add a hack in the unit test to let the UIO simulated tests pass. Remove now-redundant initialization calls that were setting the fd invalid right after allocation. Signed-off-by: David Marchand <[email protected]> --- Changes since RFC v1: - squashed FreeBSD PCI update, - updated unit test, --- app/test/test_interrupts.c | 12 ++++++++++-- doc/guides/rel_notes/release_26_11.rst | 5 +++++ drivers/bus/cdx/cdx_vfio.c | 6 ------ drivers/bus/pci/bsd/pci.c | 2 +- drivers/bus/pci/linux/pci_vfio.c | 11 ----------- drivers/bus/pci/pci_common_uio.c | 6 ------ drivers/bus/vmbus/vmbus_common_uio.c | 6 ------ drivers/net/failsafe/failsafe.c | 3 --- drivers/net/mana/mana.c | 4 ---- drivers/net/memif/rte_eth_memif.c | 6 ------ drivers/net/mlx4/mlx4.c | 3 --- drivers/net/tap/rte_eth_tap.c | 1 - lib/eal/common/eal_common_interrupts.c | 6 ++++++ lib/eal/freebsd/eal_alarm.c | 3 --- lib/eal/include/rte_interrupts.h | 2 ++ lib/eal/linux/eal_dev.c | 4 ---- 16 files changed, 24 insertions(+), 56 deletions(-) diff --git a/app/test/test_interrupts.c b/app/test/test_interrupts.c index 3a5be92cd7..747dfc4f48 100644 --- a/app/test/test_interrupts.c +++ b/app/test/test_interrupts.c @@ -73,11 +73,13 @@ test_interrupt_init(void) rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE); if (!intr_handles[i]) return -1; + if (rte_intr_fd_get(intr_handles[i]) != -1) + return -1; + if (rte_intr_dev_fd_get(intr_handles[i]) != -1) + return -1; } test_intr_handle = intr_handles[TEST_INTERRUPT_HANDLE_INVALID]; - if (rte_intr_fd_set(test_intr_handle, -1)) - return -1; if (rte_intr_type_set(test_intr_handle, RTE_INTR_HANDLE_UNKNOWN)) return -1; @@ -90,6 +92,9 @@ test_interrupt_init(void) test_intr_handle = intr_handles[TEST_INTERRUPT_HANDLE_VALID_UIO]; if (rte_intr_fd_set(test_intr_handle, pfds.readfd)) return -1; + /* HACK: UIO type does not require a device FD, but a valid handle should contain one */ + if (rte_intr_dev_fd_set(test_intr_handle, INT_MAX)) + return -1; if (rte_intr_type_set(test_intr_handle, RTE_INTR_HANDLE_UIO)) return -1; @@ -108,6 +113,9 @@ test_interrupt_init(void) test_intr_handle = intr_handles[TEST_INTERRUPT_HANDLE_CASE1]; if (rte_intr_fd_set(test_intr_handle, pfds.writefd)) return -1; + /* HACK: UIO type does not require a device FD, but a valid handle should contain one */ + if (rte_intr_dev_fd_set(test_intr_handle, INT_MAX)) + return -1; if (rte_intr_type_set(test_intr_handle, RTE_INTR_HANDLE_UIO)) return -1; diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst index c8cc86295d..daafb4c6ed 100644 --- a/doc/guides/rel_notes/release_26_11.rst +++ b/doc/guides/rel_notes/release_26_11.rst @@ -93,6 +93,11 @@ API Changes Also, make sure to start the actual text at the margin. ======================================================= +* interrupts: Updated interrupt handle file descriptor management. + + * ``rte_intr_instance_alloc()`` now initializes file descriptors + to invalid values instead of zero. + ABI Changes ----------- diff --git a/drivers/bus/cdx/cdx_vfio.c b/drivers/bus/cdx/cdx_vfio.c index 11fe3265d2..0e4af4e940 100644 --- a/drivers/bus/cdx/cdx_vfio.c +++ b/drivers/bus/cdx/cdx_vfio.c @@ -397,9 +397,6 @@ cdx_vfio_map_resource_primary(struct rte_cdx_device *dev) struct cdx_map *maps; int vfio_dev_fd, i, ret; - if (rte_intr_fd_set(dev->intr_handle, -1)) - return -1; - ret = rte_vfio_setup_device(RTE_CDX_BUS_DEVICES_PATH, dev_name, &vfio_dev_fd, &device_info); if (ret) @@ -493,9 +490,6 @@ cdx_vfio_map_resource_secondary(struct rte_cdx_device *dev) const char *dev_name = dev->device.name; struct cdx_map *maps; - if (rte_intr_fd_set(dev->intr_handle, -1)) - return -1; - /* if we're in a secondary process, just find our tailq entry */ TAILQ_FOREACH(vfio_res, vfio_res_list, next) { if (strcmp(vfio_res->name, dev_name)) diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c index c6df31d486..bbe08605af 100644 --- a/drivers/bus/pci/bsd/pci.c +++ b/drivers/bus/pci/bsd/pci.c @@ -93,7 +93,7 @@ pci_uio_free_resource(struct rte_pci_device *dev, { rte_free(uio_res); - if (rte_intr_fd_get(dev->intr_handle)) { + if (rte_intr_fd_get(dev->intr_handle) >= 0) { close(rte_intr_fd_get(dev->intr_handle)); rte_intr_fd_set(dev->intr_handle, -1); rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_UNKNOWN); diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c index bc5c5c2499..a92a0d86ec 100644 --- a/drivers/bus/pci/linux/pci_vfio.c +++ b/drivers/bus/pci/linux/pci_vfio.c @@ -742,12 +742,6 @@ pci_vfio_map_resource_primary(struct rte_pci_device *dev) struct pci_map *maps; - if (rte_intr_fd_set(dev->intr_handle, -1)) - return -1; - - if (rte_intr_fd_set(dev->vfio_req_intr_handle, -1)) - return -1; - /* store PCI address string */ snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT, loc->domain, loc->bus, loc->devid, loc->function); @@ -939,11 +933,6 @@ pci_vfio_map_resource_secondary(struct rte_pci_device *dev) struct pci_map *maps; - if (rte_intr_fd_set(dev->intr_handle, -1)) - return -1; - if (rte_intr_fd_set(dev->vfio_req_intr_handle, -1)) - return -1; - /* store PCI address string */ snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT, loc->domain, loc->bus, loc->devid, loc->function); diff --git a/drivers/bus/pci/pci_common_uio.c b/drivers/bus/pci/pci_common_uio.c index 71974e9f56..47d50721dc 100644 --- a/drivers/bus/pci/pci_common_uio.c +++ b/drivers/bus/pci/pci_common_uio.c @@ -101,12 +101,6 @@ pci_uio_map_resource(struct rte_pci_device *dev) struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list); - if (rte_intr_fd_set(dev->intr_handle, -1)) - return -1; - - if (rte_intr_dev_fd_set(dev->intr_handle, -1)) - return -1; - /* allocate uio resource */ ret = pci_uio_alloc_resource(dev, &uio_res); if (ret) diff --git a/drivers/bus/vmbus/vmbus_common_uio.c b/drivers/bus/vmbus/vmbus_common_uio.c index c6e7e07302..7459f4ea7a 100644 --- a/drivers/bus/vmbus/vmbus_common_uio.c +++ b/drivers/bus/vmbus/vmbus_common_uio.c @@ -173,12 +173,6 @@ vmbus_uio_map_resource(struct rte_vmbus_device *dev) int ret; /* TODO: handle rescind */ - if (rte_intr_fd_set(dev->intr_handle, -1)) - return -1; - - if (rte_intr_dev_fd_set(dev->intr_handle, -1)) - return -1; - if (rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_UNKNOWN)) return -1; diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c index aa3fe53b22..3ba143924d 100644 --- a/drivers/net/failsafe/failsafe.c +++ b/drivers/net/failsafe/failsafe.c @@ -279,9 +279,6 @@ fs_eth_dev_create(struct rte_vdev_device *vdev) goto cancel_alarm; } - if (rte_intr_fd_set(PRIV(dev)->intr_handle, -1)) - goto cancel_alarm; - if (rte_intr_type_set(PRIV(dev)->intr_handle, RTE_INTR_HANDLE_EXT)) goto cancel_alarm; diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c index 0b72f711a1..1864ba2a2b 100644 --- a/drivers/net/mana/mana.c +++ b/drivers/net/mana/mana.c @@ -1946,10 +1946,6 @@ mana_intr_install(struct rte_eth_dev *eth_dev, struct mana_priv *priv) return -ENOMEM; } - ret = rte_intr_fd_set(priv->intr_handle, -1); - if (ret) - goto free_intr; - ret = mana_fd_set_non_blocking(ctx->async_fd); if (ret) { DRV_LOG(ERR, "Failed to change async_fd to NONBLOCK"); diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index 5d153c3a5a..04e12b8ffa 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -1496,9 +1496,6 @@ memif_tx_queue_setup(struct rte_eth_dev *dev, mq->n_pkts = 0; mq->n_bytes = 0; - if (rte_intr_fd_set(mq->intr_handle, -1)) - return -rte_errno; - if (rte_intr_type_set(mq->intr_handle, RTE_INTR_HANDLE_EXT)) return -rte_errno; @@ -1536,9 +1533,6 @@ memif_rx_queue_setup(struct rte_eth_dev *dev, mq->n_pkts = 0; mq->n_bytes = 0; - if (rte_intr_fd_set(mq->intr_handle, -1)) - return -rte_errno; - if (rte_intr_type_set(mq->intr_handle, RTE_INTR_HANDLE_EXT)) return -rte_errno; diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c index 28f1116891..dbcd98d49f 100644 --- a/drivers/net/mlx4/mlx4.c +++ b/drivers/net/mlx4/mlx4.c @@ -1062,9 +1062,6 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev) goto port_error; } - if (rte_intr_fd_set(priv->intr_handle, -1)) - goto port_error; - if (rte_intr_type_set(priv->intr_handle, RTE_INTR_HANDLE_EXT)) goto port_error; diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c index b93452f168..13114edba5 100644 --- a/drivers/net/tap/rte_eth_tap.c +++ b/drivers/net/tap/rte_eth_tap.c @@ -2191,7 +2191,6 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name, dev->tx_pkt_burst = pmd_tx_burst; rte_intr_type_set(pmd->intr_handle, RTE_INTR_HANDLE_EXT); - rte_intr_fd_set(pmd->intr_handle, -1); dev->intr_handle = pmd->intr_handle; /* Presetup the fds to -1 as being not valid */ diff --git a/lib/eal/common/eal_common_interrupts.c b/lib/eal/common/eal_common_interrupts.c index a2a310750a..4e31a71319 100644 --- a/lib/eal/common/eal_common_interrupts.c +++ b/lib/eal/common/eal_common_interrupts.c @@ -68,6 +68,8 @@ struct rte_intr_handle *rte_intr_instance_alloc(uint32_t flags) rte_errno = ENOMEM; goto fail; } + for (int i = 0; i < RTE_MAX_RXTX_INTR_VEC_ID; i++) + intr_handle->efds[i] = -1; if (uses_rte_memory) { intr_handle->elist = rte_zmalloc(NULL, @@ -83,6 +85,8 @@ struct rte_intr_handle *rte_intr_instance_alloc(uint32_t flags) goto fail; } + intr_handle->fd = -1; + intr_handle->dev_fd = -1; intr_handle->alloc_flags = flags; intr_handle->nb_intr = RTE_MAX_RXTX_INTR_VEC_ID; @@ -153,6 +157,8 @@ int rte_intr_event_list_update(struct rte_intr_handle *intr_handle, int size) goto fail; } intr_handle->efds = tmp_efds; + for (int i = intr_handle->nb_intr; i < size; i++) + intr_handle->efds[i] = -1; if (uses_rte_memory) { tmp_elist = rte_realloc(intr_handle->elist, diff --git a/lib/eal/freebsd/eal_alarm.c b/lib/eal/freebsd/eal_alarm.c index c03e281e67..f7480d2802 100644 --- a/lib/eal/freebsd/eal_alarm.c +++ b/lib/eal/freebsd/eal_alarm.c @@ -73,9 +73,6 @@ rte_eal_alarm_init(void) if (rte_intr_type_set(intr_handle, RTE_INTR_HANDLE_ALARM)) goto error; - if (rte_intr_fd_set(intr_handle, -1)) - goto error; - /* on FreeBSD, timers don't use fd's, and their identifiers are stored * in separate namespace from fd's, so using any value is OK. however, * EAL interrupts handler expects fd's to be unique, so use an actual fd diff --git a/lib/eal/include/rte_interrupts.h b/lib/eal/include/rte_interrupts.h index 0ae16bbe19..1cfb9d3f8e 100644 --- a/lib/eal/include/rte_interrupts.h +++ b/lib/eal/include/rte_interrupts.h @@ -225,6 +225,8 @@ uint32_t rte_intr_active_events_flags(void); * can be realloced later based on size of MSIX interrupts supported by a PCI * device. * + * All file descriptors (fd, dev_fd, efds) are initialized to -1. + * * This function should be called from application or driver, before calling * any of the interrupt APIs. * diff --git a/lib/eal/linux/eal_dev.c b/lib/eal/linux/eal_dev.c index ec408649d0..c78e565427 100644 --- a/lib/eal/linux/eal_dev.c +++ b/lib/eal/linux/eal_dev.c @@ -326,10 +326,6 @@ rte_dev_event_monitor_start(void) if (ret) goto exit; - ret = rte_intr_fd_set(intr_handle, -1); - if (ret) - goto exit; - ret = dev_uev_socket_fd_create(); if (ret) { EAL_LOG(ERR, "error create device event fd."); -- 2.54.0
