Re: [PATCH v2 2/2] eal/linux: handle interrupt epoll events

David Marchand
2026-02-10T09:18:04Z

Hello Kevin,

On Fri, 6 Feb 2026 at 18:21, Kevin Traynor <ktraynor@redhat.com> wrote:
>
> Add handling for epoll error and disconnect conditions EPOLLERR,
> EPOLLHUP and EPOLLRDHUP.
>
> These events indicate that the interrupt file descriptor is in
> an error state or there has been a hangup.
>
> Only do this for interrupts that are read in eal. Interrupts that
> are read outside eal should deal with different interrupt scenarios
> appropriate to their functionality. e.g. virtio interrupt handling
> has reconnect mechanisms for some cases.
>
> Also, treat no bytes read as an error condition.
>
> Bugzilla ID: 1873
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org

Cc: Harman.

>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> lib/eal/linux/eal_interrupts.c | 67 ++++++++++++++++++++++------------
> 1 file changed, 44 insertions(+), 23 deletions(-)
>
> diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
> index 9db978923a..68ca0f929e 100644
> --- a/lib/eal/linux/eal_interrupts.c
> +++ b/lib/eal/linux/eal_interrupts.c
> @@ -887,4 +887,26 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle)
> }
>
> +static void
> +eal_intr_source_remove_and_free(struct rte_intr_source *src)
> +{
> + struct rte_intr_callback *cb, *next;
> +
> + /* Remove the interrupt source */
> + rte_spinlock_lock(&intr_lock);
> + TAILQ_REMOVE(&intr_sources, src, next);
> + rte_spinlock_unlock(&intr_lock);
> +
> + /* Free callbacks */
> + for (cb = TAILQ_FIRST(&src->callbacks); cb; cb = next) {
> + next = TAILQ_NEXT(cb, next);
> + TAILQ_REMOVE(&src->callbacks, cb, next);
> + free(cb);
> + }
> +
> + /* Free the interrupt source */
> + rte_intr_instance_free(src->intr_handle);
> + free(src);
> +}
> +
> static int
> eal_intr_process_interrupts(struct epoll_event *events, int nfds)
> @@ -952,4 +974,16 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
>
> if (bytes_read > 0) {
> + /**
> + * Check for epoll error or disconnect events for
> + * interrupts that are read directly in eal.
> + */
> + if (events[n].events & (EPOLLERR | EPOLLHUP | EPOLLRDHUP)) {
> + EAL_LOG(INFO, "Disconnect condition on fd %d "

This is an anormal situation, I would make this log level the same as
other logs below.

The fact that the interrupt fd gets into this state should be
something to report and investigate.

> + "(events=0x%x), removing from epoll",
> + events[n].data.fd, events[n].events);
> + eal_intr_source_remove_and_free(src);
> + return -1;
> + }
> +
> /**
> * read out to clear the ready-to-be-read flag
> @@ -957,5 +991,7 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
> */
> bytes_read = read(events[n].data.fd, &buf, bytes_read);
> - if (bytes_read < 0) {
> + if (bytes_read > 0) {
> + call = true;
> + } else if (bytes_read < 0) {
> if (errno == EINTR || errno == EWOULDBLOCK)
> continue;
> @@ -965,27 +1001,12 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
> events[n].data.fd,
> strerror(errno));
> - /*
> - * The device is unplugged or buggy, remove
> - * it as an interrupt source and return to
> - * force the wait list to be rebuilt.
> - */
> - rte_spinlock_lock(&intr_lock);
> - TAILQ_REMOVE(&intr_sources, src, next);
> - rte_spinlock_unlock(&intr_lock);
> -
> - for (cb = TAILQ_FIRST(&src->callbacks); cb;
> - cb = next) {
> - next = TAILQ_NEXT(cb, next);
> - TAILQ_REMOVE(&src->callbacks, cb, next);
> - free(cb);
> - }
> - rte_intr_instance_free(src->intr_handle);
> - free(src);
> - return -1;
> - } else if (bytes_read == 0)
> - EAL_LOG(ERR, "Read nothing from file "
> + } else { /* bytes == 0 */

"bytes_read == 0", or remove this comment as the code is quite compact
and leaves little space for wondering what this else block is about.

> + EAL_LOG(WARNING, "Read nothing from file "

I would keep this log at the same level than the < 0 condition.
It seems the same type of error.

> "descriptor %d", events[n].data.fd);

And avoid splitting the format string.

> - else
> - call = true;
> + }
> + if (bytes_read <= 0) {
> + eal_intr_source_remove_and_free(src);
> + return -1;
> + }
> }
>
> --
> 2.52.0
>

Except those nits, the fix looks correct.

Acked-by: David Marchand <david.marchand@redhat.com>

--
David Marchand