Patchwork Handle EAGAIN in wpa_supplicant_ctrl_iface_send

login
register
mail settings
Submitter Pontus Fuchs
Date Sept. 20, 2013, 7:53 a.m.
Message ID <1379663632-10751-1-git-send-email-pontus.fuchs@gmail.com>
Download mbox | patch
Permalink /patch/276240/
State Superseded
Headers show

Comments

Pontus Fuchs - Sept. 20, 2013, 7:53 a.m.
Commit 4fdc8def changed the ctrl interface socket to be non-blocking,
but didn't update wpa_supplicant_ctrl_iface_send to handle EAGAIN.

If a burst of events are sent, the socket queue can overflow and
sendmsg fails with EAGAIN. When this happens the monitor is silently
detached.

Signed-hostap: Pontus Fuchs <pontus.fuchs@gmail.com>
---
 wpa_supplicant/ctrl_iface_unix.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
Ben Greear - Sept. 20, 2013, 4:05 p.m.
On 09/20/2013 12:53 AM, Pontus Fuchs wrote:
> Commit 4fdc8def changed the ctrl interface socket to be non-blocking,
> but didn't update wpa_supplicant_ctrl_iface_send to handle EAGAIN.
>
> If a burst of events are sent, the socket queue can overflow and
> sendmsg fails with EAGAIN. When this happens the monitor is silently
> detached.

Patch looks OK to me, but since sendmsg uses MSG_DONTWAIT, then it
was always non-blocking and my patch should have not made things any
worse...

Thanks,
Ben

>
> Signed-hostap: Pontus Fuchs <pontus.fuchs@gmail.com>
> ---
>   wpa_supplicant/ctrl_iface_unix.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/wpa_supplicant/ctrl_iface_unix.c b/wpa_supplicant/ctrl_iface_unix.c
> index 49489d6..2c67b9c 100644
> --- a/wpa_supplicant/ctrl_iface_unix.c
> +++ b/wpa_supplicant/ctrl_iface_unix.c
> @@ -623,14 +623,21 @@ static void wpa_supplicant_ctrl_iface_send(const char *ifname, int sock,
>   			msg.msg_name = (void *) &dst->addr;
>   			msg.msg_namelen = dst->addrlen;
>   			if (sendmsg(sock, &msg, MSG_DONTWAIT) < 0) {
> -				int _errno = errno;
> +				int _errno = errno, detach = 0;
>   				wpa_printf(MSG_INFO, "CTRL_IFACE monitor[%d]: "
>   					   "%d - %s",
>   					   idx, errno, strerror(errno));
>   				dst->errors++;
> -				if (dst->errors > 1000 ||
> -				    (_errno != ENOBUFS && dst->errors > 10) ||
> -				    _errno == ENOENT) {
> +				if (dst->errors > 1000 || _errno == ENOENT)
> +					detach = 1;
> +				if (!(_errno == ENOBUFS || _errno == EAGAIN ||
> +				    _errno == EWOULDBLOCK) && dst->errors > 10)
> +					detach = 1;
> +				if (detach) {
> +					wpa_printf(MSG_ERROR, "CTRL_IFACE "
> +						   "monitor[%d]: Too many "
> +						   "errors. Detaching. ",
> +						   idx);
>   					wpa_supplicant_ctrl_iface_detach(
>   						ctrl_dst, &dst->addr,
>   						dst->addrlen);
>
Pontus Fuchs - Sept. 23, 2013, 6:51 a.m.
On 2013-09-20 18:05, Ben Greear wrote:
> On 09/20/2013 12:53 AM, Pontus Fuchs wrote:
>> Commit 4fdc8def changed the ctrl interface socket to be non-blocking,
>> but didn't update wpa_supplicant_ctrl_iface_send to handle EAGAIN.
>>
>> If a burst of events are sent, the socket queue can overflow and
>> sendmsg fails with EAGAIN. When this happens the monitor is silently
>> detached.
>
> Patch looks OK to me, but since sendmsg uses MSG_DONTWAIT, then it
> was always non-blocking and my patch should have not made things any
> worse...
>

Thanks Ben. Missed the MSG_DONTWAIT. I'll have to look for another 
reason to why this started to happen then. Anyway the patch is still 
valid. I'll update the commit msg.

Cheers,

Pontus

Patch

diff --git a/wpa_supplicant/ctrl_iface_unix.c b/wpa_supplicant/ctrl_iface_unix.c
index 49489d6..2c67b9c 100644
--- a/wpa_supplicant/ctrl_iface_unix.c
+++ b/wpa_supplicant/ctrl_iface_unix.c
@@ -623,14 +623,21 @@  static void wpa_supplicant_ctrl_iface_send(const char *ifname, int sock,
 			msg.msg_name = (void *) &dst->addr;
 			msg.msg_namelen = dst->addrlen;
 			if (sendmsg(sock, &msg, MSG_DONTWAIT) < 0) {
-				int _errno = errno;
+				int _errno = errno, detach = 0;
 				wpa_printf(MSG_INFO, "CTRL_IFACE monitor[%d]: "
 					   "%d - %s",
 					   idx, errno, strerror(errno));
 				dst->errors++;
-				if (dst->errors > 1000 ||
-				    (_errno != ENOBUFS && dst->errors > 10) ||
-				    _errno == ENOENT) {
+				if (dst->errors > 1000 || _errno == ENOENT)
+					detach = 1;
+				if (!(_errno == ENOBUFS || _errno == EAGAIN ||
+				    _errno == EWOULDBLOCK) && dst->errors > 10)
+					detach = 1;
+				if (detach) {
+					wpa_printf(MSG_ERROR, "CTRL_IFACE "
+						   "monitor[%d]: Too many "
+						   "errors. Detaching. ",
+						   idx);
 					wpa_supplicant_ctrl_iface_detach(
 						ctrl_dst, &dst->addr,
 						dst->addrlen);