From 4f735b42bf5de38cdacd6b29710364e62ec3caf2 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Mon, 7 Oct 2024 21:46:33 -0400
Subject: [PATCH] lib: Correctly handle ppoll pfds.events == 0

The frrevent system is spitting out this message in bgpd:
20:40:15 mem1-roc-f2-b1-r5-t2-d4 bgpd[13166]: [XETTR-D5MR0][EC 100663316] Attempting to process an I/O event but for fd: 214(8) no thread to handle this!

This is because as each io event is processed, it is possible that a
.events is set to 0.  This can leave a situation where we ask
ppoll to handle anything that happens on a fd with a .events of 0,
in this situation ppoll can return POLLERR, which indicates that
something bad has happened on the fd.

Let's set the ppoll fds.fd value to -1 when there are no more
events to be processed.  ppoll specifically calls out that
it will just skip this particular one.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
 lib/event.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/lib/event.c b/lib/event.c
index d925d0d5f0fc..871e2067e6da 100644
--- a/lib/event.c
+++ b/lib/event.c
@@ -963,12 +963,15 @@ void _event_add_read_write(const struct xref_eventsched *xref,
 		assert(!"Number of FD's open is greater than FRR currently configured to handle, aborting");
 
 	frr_with_mutex (&m->mtx) {
+		bool queuepos_found = false;
+
 		/* Thread is already scheduled; don't reschedule */
 		if (t_ptr && *t_ptr)
 			break;
 
 		/* default to a new pollfd */
 		nfds_t queuepos = m->handler.pfdcount;
+		nfds_t earlierqueuepos = queuepos;
 
 		if (dir == EVENT_READ)
 			thread_array = m->read;
@@ -979,9 +982,10 @@ void _event_add_read_write(const struct xref_eventsched *xref,
 		 * if we already have a pollfd for our file descriptor, find and
 		 * use it
 		 */
-		for (nfds_t i = 0; i < m->handler.pfdcount; i++)
+		for (nfds_t i = 0; i < m->handler.pfdcount; i++) {
 			if (m->handler.pfds[i].fd == fd) {
 				queuepos = i;
+				queuepos_found = true;
 
 #ifdef DEV_BUILD
 				/*
@@ -993,6 +997,23 @@ void _event_add_read_write(const struct xref_eventsched *xref,
 #endif
 				break;
 			}
+			/*
+			 * We are setting the fd = -1 for the
+			 * case when a read/write event is going
+			 * away.  if we find a -1 we can stuff it
+			 * into that spot, so note it
+			 */
+			if (m->handler.pfds[i].fd == -1 && earlierqueuepos == m->handler.pfdcount)
+				earlierqueuepos = i;
+		}
+
+		/*
+		 * We will use the queueupos of the fd if we find
+		 * it, else we can have a -1 so let's use that spot
+		 * instead.
+		 */
+		if (!queuepos_found && (earlierqueuepos < queuepos))
+			queuepos = earlierqueuepos;
 
 		/* make sure we have room for this fd + pipe poker fd */
 		assert(queuepos + 1 < m->handler.pfdsize);
@@ -1269,6 +1290,14 @@ static void cancel_arg_helper(struct event_loop *master,
 	for (i = 0; i < master->handler.pfdcount;) {
 		pfd = master->handler.pfds + i;
 
+		/*
+		 * Skip this spot, nothing here to see
+		 */
+		if (pfd->fd == -1) {
+			i++;
+			continue;
+		}
+
 		if (pfd->events & POLLIN)
 			t = master->read[pfd->fd];
 		else
@@ -1590,6 +1619,12 @@ static int thread_process_io_helper(struct event_loop *m, struct event *thread,
 	 * we should.
 	 */
 	m->handler.pfds[pos].events &= ~(state);
+	/*
+	 * ppoll man page says that a fd of -1 causes the particular
+	 * array item to be skipped.  So let's skip it
+	 */
+	if (m->handler.pfds[pos].events == 0)
+		m->handler.pfds[pos].fd = -1;
 
 	if (!thread) {
 		if ((actual_state & (POLLHUP|POLLIN)) != POLLHUP)