Skip to content

Commit b05fa44

Browse files
committed
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 <[email protected]>
1 parent 80dc863 commit b05fa44

File tree

1 file changed

+37
-1
lines changed

1 file changed

+37
-1
lines changed

lib/event.c

+37-1
Original file line numberDiff line numberDiff line change
@@ -963,12 +963,15 @@ void _event_add_read_write(const struct xref_eventsched *xref,
963963
assert(!"Number of FD's open is greater than FRR currently configured to handle, aborting");
964964

965965
frr_with_mutex (&m->mtx) {
966+
bool queuepos_found = false;
967+
966968
/* Thread is already scheduled; don't reschedule */
967969
if (t_ptr && *t_ptr)
968970
break;
969971

970972
/* default to a new pollfd */
971973
nfds_t queuepos = m->handler.pfdcount;
974+
nfds_t earlierqueuepos = queuepos;
972975

973976
if (dir == EVENT_READ)
974977
thread_array = m->read;
@@ -979,9 +982,10 @@ void _event_add_read_write(const struct xref_eventsched *xref,
979982
* if we already have a pollfd for our file descriptor, find and
980983
* use it
981984
*/
982-
for (nfds_t i = 0; i < m->handler.pfdcount; i++)
985+
for (nfds_t i = 0; i < m->handler.pfdcount; i++) {
983986
if (m->handler.pfds[i].fd == fd) {
984987
queuepos = i;
988+
queuepos_found = true;
985989

986990
#ifdef DEV_BUILD
987991
/*
@@ -993,6 +997,23 @@ void _event_add_read_write(const struct xref_eventsched *xref,
993997
#endif
994998
break;
995999
}
1000+
/*
1001+
* We are setting the fd = -1 for the
1002+
* case when a read/write event is going
1003+
* away. if we find a -1 we can stuff it
1004+
* into that spot, so note it
1005+
*/
1006+
if (m->handler.pfds[i].fd == -1 && earlierqueuepos == m->handler.pfdcount)
1007+
earlierqueuepos = i;
1008+
}
1009+
1010+
/*
1011+
* We will use the queueupos of the fd if we find
1012+
* it, else we can have a -1 so let's use that spot
1013+
* instead.
1014+
*/
1015+
if (!queuepos_found && (earlierqueuepos < queuepos))
1016+
queuepos = earlierqueuepos;
9961017

9971018
/* make sure we have room for this fd + pipe poker fd */
9981019
assert(queuepos + 1 < m->handler.pfdsize);
@@ -1269,6 +1290,14 @@ static void cancel_arg_helper(struct event_loop *master,
12691290
for (i = 0; i < master->handler.pfdcount;) {
12701291
pfd = master->handler.pfds + i;
12711292

1293+
/*
1294+
* Skip this spot, nothing here to see
1295+
*/
1296+
if (pfd->fd == -1) {
1297+
i++;
1298+
continue;
1299+
}
1300+
12721301
if (pfd->events & POLLIN)
12731302
t = master->read[pfd->fd];
12741303
else
@@ -1590,6 +1619,13 @@ static int thread_process_io_helper(struct event_loop *m, struct event *thread,
15901619
* we should.
15911620
*/
15921621
m->handler.pfds[pos].events &= ~(state);
1622+
/*
1623+
* ppoll man page says that a fd of -1 causes the particular
1624+
* array item to be skipped. So let's skip it
1625+
*/
1626+
if (m->handler.pfds[pos].events == 0) {
1627+
m->handler.pfds[pos].fd = -1;
1628+
}
15931629

15941630
if (!thread) {
15951631
if ((actual_state & (POLLHUP|POLLIN)) != POLLHUP)

0 commit comments

Comments
 (0)