tools: hv: ignore ENOBUFS in the KVP daemon

PATCH Superseded Dexuan Cui 2 participants 8 messages Nov 19, 2014 Hyper-V/Azure core and drivers

Share & tools

Dexuan Cui

author maintainer

2014-11-19 09:41 UTC #0 lore raw

PATCH 1/1 tools: hv: ignore ENOBUFS in the KVP daemon

Under high memory pressure and very high KVP R/W test pressure, the netlink recvfrom() may transiently return ENOBUFS to the daemon -- we found this during a 2-week stress test.

We'd better not terminate the daemon on this failure, because a typical KVP user can re-try the R/W and hopefully it will succeed next time.

Cc K. Y. Srinivasan <[email protected]>

Signed-off-by Dexuan Cui <[email protected]>

---
 tools/hv/hv_kvp_daemon.c | 7 +++++++
 1 file changed, 7 insertions(+)

tools/hv/hv_kvp_daemon.c +7 -0

index 22b0764..9f4b303 100644
@@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
 				addr_p, &addr_l);
 
 		if (len < 0) {
+			int saved_errno = errno;
 			syslog(LOG_ERR, "recvfrom failed; pid:%u error:%d %s",
 					addr.nl_pid, errno, strerror(errno));
+
+			if (saved_errno == ENOBUFS) {
+				syslog(LOG_ERR, "error = ENOBUFS: ignored");
+				continue;
+			}
+
 			close(fd);
 			return -1;
 		}

Signature · 1 line

1.9.1

Vitaly Kuznetsov

2014-11-19 10:50 UTC #1 lore raw

Quoted 18 lines · Dexuan Cui

Under high memory pressure and very high KVP R/W test pressure, the netlink recvfrom() may transiently return ENOBUFS to the daemon -- we found this during a 2-week stress test.
We'd better not terminate the daemon on this failure, because a typical KVP user can re-try the R/W and hopefully it will succeed next time.
Cc K. Y. Srinivasan <[email protected]>
Signed-off-by Dexuan Cui <[email protected]>
---
 tools/hv/hv_kvp_daemon.c | 7 +++++++
 1 file changed, 7 insertions(+)
tools/hv/hv_kvp_daemon.c +0 -0
index 22b0764..9f4b303 100644

tools/hv/hv_kvp_daemon.c int main(int argc, char *argv[]) · line 1566

Quoted 8 lines

tools/hv/hv_kvp_daemon.c +3 -0

@@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
 				addr_p, &addr_l);
 
 		if (len < 0) {
+			int saved_errno = errno;
 			syslog(LOG_ERR, "recvfrom failed; pid:%u error:%d %s",
 					addr.nl_pid, errno, strerror(errno));
+
+			if (saved_errno == ENOBUFS) {

is it possible to meet EAGAIN (or EWOULDBLOCK) here as well? I'd suggest we ignore these as well in such case. Ignoring ENOMEM here is doubtful, I think. But possible.

Quoted 7 lines

tools/hv/hv_kvp_daemon.c +4 -0

+				syslog(LOG_ERR, "error = ENOBUFS: ignored");
+				continue;
+			}
+
 			close(fd);
 			return -1;
 		}

Signature · 1 line

  Vitaly

Dexuan Cui

author maintainer

2014-11-19 12:31 UTC #2 lore raw

Quoted 40 lines

-----Original Message-----
From: Vitaly Kuznetsov
Sent: Wednesday, November 19, 2014 18:50 PM
To: Dexuan Cui
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
Quoted 1 line · Dexuan Cui
Under high memory pressure and very high KVP R/W test pressure, the
netlink
Quoted 1 line
recvfrom() may transiently return ENOBUFS to the daemon -- we found
this
Quoted 3 lines
during a 2-week stress test.
We'd better not terminate the daemon on this failure, because a typical
KVP
tools/hv/hv_kvp_daemon.c int main(int argc, char *argv[]) · line 1563
Quoted 12 lines
user can re-try the R/W and hopefully it will succeed next time.
tools/hv/hv_kvp_daemon.c +1 -0
index 22b0764..9f4b303 100644
@@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
 				addr_p, &addr_l);
 
 		if (len < 0) {
+			int saved_errno = errno;
 			syslog(LOG_ERR, "recvfrom failed; pid:%u
error:%d %s",
tools/hv/hv_kvp_daemon.c int main(int argc, char *argv[]) · line 1566
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +2 -0
 					addr.nl_pid, errno, strerror(errno));
+
+			if (saved_errno == ENOBUFS) {
is it possible to meet EAGAIN (or EWOULDBLOCK) here as well? I'd suggest we ignore these as well in such case. Ignoring ENOMEM here is doubtful, I think. But possible.

Quoted 1 line

Vitaly

I don't think EAGAIN is possible because "man recvfrom" says
"If no messages are available at the socket, the receive calls wait for a
message to arrive, unless the socket is nonblocking (see fcntl(2)), in which case the value -1 is returned and the external variable errno is set to
EAGAIN or EWOULDBLOCK".

The same man page mention ENOMEM for recvmsg(), but not recvfrom().

-- Dexuan

Vitaly Kuznetsov

2014-11-19 12:41 UTC #3 lore raw

Quoted 38 lines · Dexuan Cui

Quoted 36 lines
-----Original Message-----
From: Vitaly Kuznetsov
Sent: Wednesday, November 19, 2014 18:50 PM
To: Dexuan Cui
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
Quoted 12 lines · Dexuan Cui
Under high memory pressure and very high KVP R/W test pressure, the netlink recvfrom() may transiently return ENOBUFS to the daemon -- we found this during a 2-week stress test.
We'd better not terminate the daemon on this failure, because a typical KVP user can re-try the R/W and hopefully it will succeed next time.
tools/hv/hv_kvp_daemon.c +0 -0
index 22b0764..9f4b303 100644
tools/hv/hv_kvp_daemon.c int main(int argc, char *argv[]) · line 1566
Quoted 8 lines
tools/hv/hv_kvp_daemon.c +3 -0
@@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
 				addr_p, &addr_l);
 
 		if (len < 0) {
+			int saved_errno = errno;
 			syslog(LOG_ERR, "recvfrom failed; pid:%u error:%d %s",
 					addr.nl_pid, errno, strerror(errno));
+
+			if (saved_errno == ENOBUFS) {
is it possible to meet EAGAIN (or EWOULDBLOCK) here as well? I'd suggest we ignore these as well in such case. Ignoring ENOMEM here is doubtful, I think. But possible.
Quoted 1 line
Vitaly

Quoted 7 lines

I don't think EAGAIN is possible because "man recvfrom" says
"If no messages are available at the socket, the receive calls wait for a
message to arrive, unless the socket is nonblocking (see fcntl(2)), in which case the value -1 is returned and the external variable errno is set to
EAGAIN or EWOULDBLOCK".
The same man page mention ENOMEM for recvmsg(), but not recvfrom().

Ah, sorry, I though your patch patches the other place: call to netlink_send() which does sendmsg() (and my EAGAIN/EWOULDBLOCK/ENOMEM comment was about it). It could also make sense to patch them both as I think it is possible to hit these as well.

Quoted 2 lines

-- Dexuan

Signature · 1 line

  Vitaly

Dexuan Cui

author maintainer

2014-11-19 13:12 UTC #4 lore raw

Quoted 70 lines

-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Wednesday, November 19, 2014 20:41 PM
To: Dexuan Cui
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
Quoted 5 lines · Dexuan Cui
Quoted 5 lines
-----Original Message-----
From: Vitaly Kuznetsov
Sent: Wednesday, November 19, 2014 18:50 PM
To: Dexuan Cui
Cc: [email protected]; [email protected];
driverdev-
Quoted 7 lines
Quoted 7 lines
[email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
Quoted 1 line · Dexuan Cui
Under high memory pressure and very high KVP R/W test pressure,
the netlink
Quoted 1 line
Quoted 1 line
Quoted 1 line
recvfrom() may transiently return ENOBUFS to the daemon -- we found
this
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
during a 2-week stress test.
We'd better not terminate the daemon on this failure, because a
typical KVP
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
user can re-try the R/W and hopefully it will succeed next time.
tools/hv/hv_kvp_daemon.c +0 -0
index 22b0764..9f4b303 100644
@@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
 				addr_p, &addr_l);
 
Quoted 3 lines
Quoted 3 lines
tools/hv/hv_kvp_daemon.c int main(int argc, char *argv[]) · line 1563
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +1 -0
 		if (len < 0) {
+			int saved_errno = errno;
 			syslog(LOG_ERR, "recvfrom failed; pid:%u
error:%d %s",
Quoted 5 lines
Quoted 5 lines
tools/hv/hv_kvp_daemon.c int main(int argc, char *argv[]) · line 1566
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +2 -0
 					addr.nl_pid, errno, strerror(errno));
+
+			if (saved_errno == ENOBUFS) {
is it possible to meet EAGAIN (or EWOULDBLOCK) here as well? I'd
suggest
Quoted 8 lines
Quoted 4 lines
we ignore these as well in such case. Ignoring ENOMEM here is doubtful, I think. But possible.
Vitaly
I don't think EAGAIN is possible because "man recvfrom" says
"If no messages are available at the socket, the receive calls wait for a
message to arrive, unless the socket is nonblocking (see fcntl(2)), in
which
Quoted 4 lines
case the value -1 is returned and the external variable errno is set to
EAGAIN or EWOULDBLOCK".
The same man page mention ENOMEM for recvmsg(), but not recvfrom().
Ah, sorry, I though your patch patches the other place: call to netlink_send() which does sendmsg() (and my EAGAIN/EWOULDBLOCK/ENOMEM
comment was about it). It could also make sense to patch them both as I think it is possible to hit these as well.

Quoted 3 lines

Quoted 1 line
-- Dexuan
--
Vitaly

OK, I can add this new check:
(I'll send out the v2 tomorrow in case people have new comments)

tools/hv/hv_kvp_daemon.c +7 -0

@@ -1770,8 +1770,15 @@ kvp_done:
 
                len = netlink_send(fd, incoming_cn_msg);
                if (len < 0) {
+                       int saved_errno = errno;
                        syslog(LOG_ERR, "net_link send failed; error: %d %s", errno,
                                        strerror(errno));
+
+                       if (saved_errno == ENOMEM || saved_errno == EAGAIN) {
+                               syslog(LOG_ERR, "send error: ignored");
+                               continue;
+                       }
+
                        exit(EXIT_FAILURE);
                }
        }

Thanks,
-- Dexuan

Vitaly Kuznetsov

2014-11-19 13:05 UTC #5 lore raw

Quoted 70 lines · Dexuan Cui

Quoted 70 lines
-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Wednesday, November 19, 2014 20:41 PM
To: Dexuan Cui
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
Quoted 5 lines · Dexuan Cui
Quoted 5 lines
-----Original Message-----
From: Vitaly Kuznetsov
Sent: Wednesday, November 19, 2014 18:50 PM
To: Dexuan Cui
Cc: [email protected]; [email protected];
driverdev-
Quoted 7 lines
Quoted 7 lines
[email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
Quoted 1 line · Dexuan Cui
Under high memory pressure and very high KVP R/W test pressure,
the netlink
Quoted 1 line
Quoted 1 line
Quoted 1 line
recvfrom() may transiently return ENOBUFS to the daemon -- we found
this
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
during a 2-week stress test.
We'd better not terminate the daemon on this failure, because a
typical KVP
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
user can re-try the R/W and hopefully it will succeed next time.
tools/hv/hv_kvp_daemon.c +0 -0
index 22b0764..9f4b303 100644
@@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
 				addr_p, &addr_l);
 
Quoted 3 lines
Quoted 3 lines
tools/hv/hv_kvp_daemon.c kvp_done:
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +1 -0
 		if (len < 0) {
+			int saved_errno = errno;
 			syslog(LOG_ERR, "recvfrom failed; pid:%u
error:%d %s",
Quoted 5 lines
Quoted 5 lines
tools/hv/hv_kvp_daemon.c int main(int argc, char *argv[]) · line 1566
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +2 -0
 					addr.nl_pid, errno, strerror(errno));
+
+			if (saved_errno == ENOBUFS) {
is it possible to meet EAGAIN (or EWOULDBLOCK) here as well? I'd
suggest
Quoted 8 lines
Quoted 4 lines
we ignore these as well in such case. Ignoring ENOMEM here is doubtful, I think. But possible.
Vitaly
I don't think EAGAIN is possible because "man recvfrom" says
"If no messages are available at the socket, the receive calls wait for a
message to arrive, unless the socket is nonblocking (see fcntl(2)), in
which
Quoted 4 lines
case the value -1 is returned and the external variable errno is set to
EAGAIN or EWOULDBLOCK".
The same man page mention ENOMEM for recvmsg(), but not recvfrom().
Ah, sorry, I though your patch patches the other place: call to netlink_send() which does sendmsg() (and my EAGAIN/EWOULDBLOCK/ENOMEM
comment was about it). It could also make sense to patch them both as I think it is possible to hit these as well.

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
-- Dexuan
--
Vitaly
OK, I can add this new check:
(I'll send out the v2 tomorrow in case people have new comments)

Thanks!

tools/hv/hv_kvp_daemon.c kvp_done: · line 1777

Quoted 11 lines

tools/hv/hv_kvp_daemon.c +3 -0

@@ -1770,8 +1770,15 @@ kvp_done:
 
                len = netlink_send(fd, incoming_cn_msg);
                if (len < 0) {
+                       int saved_errno = errno;
                        syslog(LOG_ERR, "net_link send failed; error: %d %s", errno,
                                        strerror(errno));
+
+                       if (saved_errno == ENOMEM || saved_errno ==  EAGAIN) {

Sorry for being pushy, but it seems ENOBUFS is also possible here (at least man sendmsg mentions it).

Quoted 10 lines

tools/hv/hv_kvp_daemon.c +4 -0

+                               syslog(LOG_ERR, "send error: ignored");
+                               continue;
+                       }
+
                        exit(EXIT_FAILURE);
                }
        }

Thanks,
-- Dexuan

Signature · 1 line

  Vitaly

Dexuan Cui

author maintainer

2014-11-19 13:14 UTC #6 lore raw

Quoted 15 lines

-----Original Message-----
From: Vitaly Kuznetsov
Quoted 6 lines
Quoted 2 lines
--
Vitaly
OK, I can add this new check:
(I'll send out the v2 tomorrow in case people have new comments)
Thanks!
Quoted 4 lines
tools/hv/hv_kvp_daemon.c +0 -0
@@ -1770,8 +1770,15 @@ kvp_done:
 

Quoted 10 lines

tools/hv/hv_kvp_daemon.c kvp_done: · line 1777

Quoted 7 lines

tools/hv/hv_kvp_daemon.c +3 -0

                len = netlink_send(fd, incoming_cn_msg);
                if (len < 0) {
+                       int saved_errno = errno;
                        syslog(LOG_ERR, "net_link send failed; error: %d %s", errno,
                                        strerror(errno));
+
+                       if (saved_errno == ENOMEM || saved_errno ==  EAGAIN) {

Sorry for being pushy, but it seems ENOBUFS is also possible here (at least man sendmsg mentions it).

OK, I'll add this too. :-)

BTW, I realized sendmsg() can't return EAGAIN here as that's for non-blocking socket.

Here I simply ignore the error, hoping the other end will re-try.

Quoted 9 lines

tools/hv/hv_kvp_daemon.c +4 -0

+                               syslog(LOG_ERR, "send error: ignored");
+                               continue;
+                       }
+
                        exit(EXIT_FAILURE);
                }
        }

Quoted 4 lines

Quoted 2 lines
Thanks,
-- Dexuan
Vitaly

-- Dexuan

Vitaly Kuznetsov

2014-11-19 14:40 UTC #7 lore raw

Quoted 25 lines · Dexuan Cui

Quoted 15 lines
-----Original Message-----
From: Vitaly Kuznetsov
Quoted 6 lines
Quoted 2 lines
--
Vitaly
OK, I can add this new check:
(I'll send out the v2 tomorrow in case people have new comments)
Thanks!
Quoted 4 lines
tools/hv/hv_kvp_daemon.c +0 -0
@@ -1770,8 +1770,15 @@ kvp_done:
 
Quoted 10 lines
tools/hv/hv_kvp_daemon.c kvp_done: · line 1777
Quoted 7 lines
tools/hv/hv_kvp_daemon.c +3 -0
                len = netlink_send(fd, incoming_cn_msg);
                if (len < 0) {
+                       int saved_errno = errno;
                        syslog(LOG_ERR, "net_link send failed; error: %d %s", errno,
                                        strerror(errno));
+
+                       if (saved_errno == ENOMEM || saved_errno ==  EAGAIN) {
Sorry for being pushy, but it seems ENOBUFS is also possible here (at least man sendmsg mentions it).

Quoted 7 lines

OK, I'll add this too. :-)
BTW, I realized sendmsg() can't return EAGAIN here as that's for non-blocking socket.
Here I simply ignore the error, hoping the other end will re-try.

I agree, it's sufficient to ignore ENOBUFS on recieve path and both ENOMEM/ENOBUFS on send.

Thanks!

Quoted 15 lines

Quoted 9 lines

tools/hv/hv_kvp_daemon.c +4 -0

+                               syslog(LOG_ERR, "send error: ignored");
+                               continue;
+                       }
+
                        exit(EXIT_FAILURE);
                }
        }

Quoted 4 lines

Quoted 2 lines
Thanks,
-- Dexuan
Vitaly

-- Dexuan

Signature · 1 line

  Vitaly