Skip to content

Commit bc47f04

Browse files
itsGiaanColinIanKing
authored andcommitted
stress-ovpn: bound the wait for an rtnetlink reply
ovpn_rt_send() retried EINTR unconditionally, so the SIGALRM that ends the run was swallowed on every iteration and the worker never learned it had to stop: a --timeout run outlived its own timeout and needed SIGKILL from outside to clear. EAGAIN cannot occur on a blocking socket, so merging it into the same branch only obscured which case was being handled. The loop had no deadline either, re-entering a blocking recvmsg() whenever a datagram is parsed to the end without a terminating message - latent today, since the only caller passes no callback and NLM_F_ACK is therefore always set. Set SO_RCVTIMEO and split the two cases: EAGAIN now means the deadline expired and is reported as -ETIMEDOUT, while EINTR still retries but gives up once stress_continue_flag() has dropped. Five seconds is far longer than an rtnetlink round trip; across runs of twenty instances, including one under slab fault injection, it never expired. Also correct the comment above the NLM_F_ACK decision, which says the opposite of what the code does. Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com>
1 parent e0601c6 commit bc47f04

1 file changed

Lines changed: 44 additions & 3 deletions

File tree

stress-ovpn.c

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,13 @@ static const stress_help_t help[] = {
9292

9393
#define RT_SNDBUF_SIZE (1024 * 2)
9494
#define RT_RCVBUF_SIZE (1024 * 4)
95+
/*
96+
* A reply is not guaranteed to arrive: the kernel can fail to allocate one
97+
* under memory pressure, and the request may have been aimed at an object that
98+
* went away. Without a deadline the recvmsg() loop in ovpn_rt_send() waits for
99+
* it indefinitely, which hangs the worker.
100+
*/
101+
#define RT_RCVTIMEO_SEC (5)
95102

96103
#define KEY_LEN (256 / 8)
97104
#define NONCE_LEN (8)
@@ -252,6 +259,7 @@ static inline void ovpn_nest_end(struct nlmsghdr *msg, struct rtattr *nest)
252259
static int ovpn_rt_socket(ovpn_ctx_t *ovpn)
253260
{
254261
const char *args_name = ovpn->args_name;
262+
struct timeval rcvtimeo = { RT_RCVTIMEO_SEC, 0 };
255263
int sndbuf = RT_SNDBUF_SIZE;
256264
int rcvbuf = RT_RCVBUF_SIZE;
257265
int fd;
@@ -279,6 +287,18 @@ static int ovpn_rt_socket(ovpn_ctx_t *ovpn)
279287
return -1;
280288
}
281289

290+
/*
291+
* Bound the wait for a reply. This is what turns a lost reply into an
292+
* error the caller can act on, rather than a worker that never returns.
293+
*/
294+
if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &rcvtimeo,
295+
sizeof(rcvtimeo)) < 0) {
296+
pr_dbg("%s: setsockopt SO_RCVTIMEO failed, errno=%d (%s)\n",
297+
args_name, errno, strerror(errno));
298+
(void)close(fd);
299+
return -1;
300+
}
301+
282302
return fd;
283303
}
284304

@@ -357,7 +377,7 @@ static int ovpn_rt_send(
357377

358378
payload->nlmsg_seq = (uint32_t)(time(NULL) & 0xffffffff);
359379

360-
/* no need to send reply */
380+
/* no reply parser, so ask for a plain ack */
361381
if (!cb)
362382
payload->nlmsg_flags |= NLM_F_ACK;
363383

@@ -393,8 +413,29 @@ static int ovpn_rt_send(
393413
iov.iov_len = sizeof(buf);
394414
rcv_len = recvmsg(fd, &nlmsg, 0);
395415
if (rcv_len < 0) {
396-
if (errno == EINTR || errno == EAGAIN) {
397-
pr_dbg("interrupted call\n");
416+
/*
417+
* With SO_RCVTIMEO set, EAGAIN means the deadline
418+
* expired rather than "try again": the reply is not
419+
* coming, so report it instead of asking for it again
420+
* forever.
421+
*/
422+
if ((errno == EAGAIN) || (errno == EWOULDBLOCK)) {
423+
pr_dbg("%s: no netlink reply within %d seconds\n",
424+
args_name, RT_RCVTIMEO_SEC);
425+
ret = -ETIMEDOUT;
426+
goto out;
427+
}
428+
if (errno == EINTR) {
429+
/*
430+
* Retrying is right for a signal, but not once
431+
* the run is winding down: SIGALRM would then
432+
* be swallowed on every iteration and the
433+
* worker would never notice it has to stop.
434+
*/
435+
if (UNLIKELY(!stress_continue_flag())) {
436+
ret = -EINTR;
437+
goto out;
438+
}
398439
continue;
399440
}
400441
pr_dbg("%s: recvmsg() failed\n", args_name);

0 commit comments

Comments
 (0)