[arch-projects] [projects][PATCH] wpa_actiond: Wait for three "failed" PONGs before disconnecting
When the system is very low on resources, select() may return 0 even when wpa_supplicant is alive and kicking. Don't disconnect as soon at that occurs - wait three times and clearly log it. Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> --- Hi all, Using [projects] since [wpa_actiond] gets kicked by the email filter. Can anyone update the message to include a list of supported tags, or link to an article/wiki with information? Back on topic: This is a small patch I had for 2 months and I thought about getting upstreamed. In particular - on certain workloads my system can get really low on memory (and available fds). With the patch in place, wpa_actiond no longer kills my connection although in practise I never seen two consecutive "wpa_supplicant failed to reply (PONG)" messages. My patch could be completely bonkers, so any suggestions how to address this correctly will be appreciated. --- wpa_actiond.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/wpa_actiond.c b/wpa_actiond.c index d60d885..03a9d7f 100644 --- a/wpa_actiond.c +++ b/wpa_actiond.c @@ -292,6 +292,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t /* path to control socket */ char *ctrlsock = NULL; int ctrlsocklen; + int pong_failures = 0; ssid[0] = '\0'; old_ssid[0] = '\0'; @@ -359,6 +360,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t } logevent(WPA_ACTIOND_LOG_TERMINATE, iface, ""); state = WPA_ACTIOND_STATE_TERMINATED; + pong_failures = 0; break; case 0: if (state == WPA_ACTIOND_STATE_CONNECTION_LOST) { @@ -372,8 +374,15 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t reply_len = 0; reply[reply_len] = '\0'; if(!str_match(reply, "PONG")) { + if (pong_failures <= 3) { + logevent(WPA_ACTIOND_LOG_CUSTOM_ERROR, iface, "wpa_supplicant failed to reply (PONG)"); + pong_failures++; + break; + } + /* supplicant has been terminated */ if(state == WPA_ACTIOND_STATE_CONNECTED || state == WPA_ACTIOND_STATE_CONNECTION_LOST) { + logevent(WPA_ACTIOND_LOG_CUSTOM_ERROR, iface, "wpa_supplicant failed to reply three times in a row - disconnecting"); logevent(WPA_ACTIOND_LOG_DISCONNECTED, iface, ssid); action(WPA_ACTIOND_ACTION_DISCONNECT, iface, ssid, idstr, wpa_ctrl_get_fd(ctrl), script); } @@ -381,6 +390,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t state = WPA_ACTIOND_STATE_TERMINATED; } } + pong_failures = 0; break; default: /* event received */ @@ -446,6 +456,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t /* we are not interested in this event */ break; } + pong_failures = 0; } } -- 2.14.0
On 21 August 2017 at 10:34, Emil Velikov <emil.l.velikov@gmail.com> wrote:
When the system is very low on resources, select() may return 0 even when wpa_supplicant is alive and kicking.
Don't disconnect as soon at that occurs - wait three times and clearly log it.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> --- Hi all,
Using [projects] since [wpa_actiond] gets kicked by the email filter. Can anyone update the message to include a list of supported tags, or link to an article/wiki with information?
Back on topic:
This is a small patch I had for 2 months and I thought about getting upstreamed.
In particular - on certain workloads my system can get really low on memory (and available fds).
With the patch in place, wpa_actiond no longer kills my connection although in practise I never seen two consecutive "wpa_supplicant failed to reply (PONG)" messages.
My patch could be completely bonkers, so any suggestions how to address this correctly will be appreciated. --- wpa_actiond.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/wpa_actiond.c b/wpa_actiond.c index d60d885..03a9d7f 100644 --- a/wpa_actiond.c +++ b/wpa_actiond.c @@ -292,6 +292,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t /* path to control socket */ char *ctrlsock = NULL; int ctrlsocklen; + int pong_failures = 0;
ssid[0] = '\0'; old_ssid[0] = '\0'; @@ -359,6 +360,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t } logevent(WPA_ACTIOND_LOG_TERMINATE, iface, ""); state = WPA_ACTIOND_STATE_TERMINATED; + pong_failures = 0; break; case 0: if (state == WPA_ACTIOND_STATE_CONNECTION_LOST) { @@ -372,8 +374,15 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t reply_len = 0; reply[reply_len] = '\0'; if(!str_match(reply, "PONG")) { + if (pong_failures <= 3) { + logevent(WPA_ACTIOND_LOG_CUSTOM_ERROR, iface, "wpa_supplicant failed to reply (PONG)"); + pong_failures++; + break; + } + /* supplicant has been terminated */ if(state == WPA_ACTIOND_STATE_CONNECTED || state == WPA_ACTIOND_STATE_CONNECTION_LOST) { + logevent(WPA_ACTIOND_LOG_CUSTOM_ERROR, iface, "wpa_supplicant failed to reply three times in a row - disconnecting"); logevent(WPA_ACTIOND_LOG_DISCONNECTED, iface, ssid); action(WPA_ACTIOND_ACTION_DISCONNECT, iface, ssid, idstr, wpa_ctrl_get_fd(ctrl), script); } @@ -381,6 +390,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t state = WPA_ACTIOND_STATE_TERMINATED; } } + pong_failures = 0; break; default: /* event received */ @@ -446,6 +456,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t /* we are not interested in this event */ break; } + pong_failures = 0; } }
-- Humble ping anyone?
Thanks Emil
On 2017-09-29 15:03, Emil Velikov via arch-projects wrote:
On 21 August 2017 at 10:34, Emil Velikov <emil.l.velikov@gmail.com> wrote:
When the system is very low on resources, select() may return 0 even when wpa_supplicant is alive and kicking.
Don't disconnect as soon at that occurs - wait three times and clearly log it.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> --- Hi all,
Using [projects] since [wpa_actiond] gets kicked by the email filter. Can anyone update the message to include a list of supported tags, or link to an article/wiki with information?
Back on topic:
This is a small patch I had for 2 months and I thought about getting upstreamed.
In particular - on certain workloads my system can get really low on memory (and available fds).
With the patch in place, wpa_actiond no longer kills my connection although in practise I never seen two consecutive "wpa_supplicant failed to reply (PONG)" messages.
My patch could be completely bonkers, so any suggestions how to address this correctly will be appreciated. --- wpa_actiond.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/wpa_actiond.c b/wpa_actiond.c index d60d885..03a9d7f 100644 --- a/wpa_actiond.c +++ b/wpa_actiond.c @@ -292,6 +292,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t /* path to control socket */ char *ctrlsock = NULL; int ctrlsocklen; + int pong_failures = 0;
ssid[0] = '\0'; old_ssid[0] = '\0'; @@ -359,6 +360,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t } logevent(WPA_ACTIOND_LOG_TERMINATE, iface, ""); state = WPA_ACTIOND_STATE_TERMINATED; + pong_failures = 0; break; case 0: if (state == WPA_ACTIOND_STATE_CONNECTION_LOST) { @@ -372,8 +374,15 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t reply_len = 0; reply[reply_len] = '\0'; if(!str_match(reply, "PONG")) { + if (pong_failures <= 3) { + logevent(WPA_ACTIOND_LOG_CUSTOM_ERROR, iface, "wpa_supplicant failed to reply (PONG)"); + pong_failures++; + break; + } + /* supplicant has been terminated */ if(state == WPA_ACTIOND_STATE_CONNECTED || state == WPA_ACTIOND_STATE_CONNECTION_LOST) { + logevent(WPA_ACTIOND_LOG_CUSTOM_ERROR, iface, "wpa_supplicant failed to reply three times in a row - disconnecting"); logevent(WPA_ACTIOND_LOG_DISCONNECTED, iface, ssid); action(WPA_ACTIOND_ACTION_DISCONNECT, iface, ssid, idstr, wpa_ctrl_get_fd(ctrl), script); } @@ -381,6 +390,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t state = WPA_ACTIOND_STATE_TERMINATED; } } + pong_failures = 0; break; default: /* event received */ @@ -446,6 +456,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t /* we are not interested in this event */ break; } + pong_failures = 0; } }
-- Humble ping anyone?
Thanks Emil
I don't think anyone maintains wpa_actiond these days. I will look at your change this week, but forking it completely wouldn't be insane. Bartłomiej
On 9 October 2017 at 09:22, Bartłomiej Piotrowski <bpiotrowski@archlinux.org> wrote:
On 2017-09-29 15:03, Emil Velikov via arch-projects wrote:
On 21 August 2017 at 10:34, Emil Velikov <emil.l.velikov@gmail.com> wrote:
When the system is very low on resources, select() may return 0 even when wpa_supplicant is alive and kicking.
Don't disconnect as soon at that occurs - wait three times and clearly log it.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> --- Hi all,
Using [projects] since [wpa_actiond] gets kicked by the email filter. Can anyone update the message to include a list of supported tags, or link to an article/wiki with information?
Back on topic:
This is a small patch I had for 2 months and I thought about getting upstreamed.
In particular - on certain workloads my system can get really low on memory (and available fds).
With the patch in place, wpa_actiond no longer kills my connection although in practise I never seen two consecutive "wpa_supplicant failed to reply (PONG)" messages.
My patch could be completely bonkers, so any suggestions how to address this correctly will be appreciated. --- wpa_actiond.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/wpa_actiond.c b/wpa_actiond.c index d60d885..03a9d7f 100644 --- a/wpa_actiond.c +++ b/wpa_actiond.c @@ -292,6 +292,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t /* path to control socket */ char *ctrlsock = NULL; int ctrlsocklen; + int pong_failures = 0;
ssid[0] = '\0'; old_ssid[0] = '\0'; @@ -359,6 +360,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t } logevent(WPA_ACTIOND_LOG_TERMINATE, iface, ""); state = WPA_ACTIOND_STATE_TERMINATED; + pong_failures = 0; break; case 0: if (state == WPA_ACTIOND_STATE_CONNECTION_LOST) { @@ -372,8 +374,15 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t reply_len = 0; reply[reply_len] = '\0'; if(!str_match(reply, "PONG")) { + if (pong_failures <= 3) { + logevent(WPA_ACTIOND_LOG_CUSTOM_ERROR, iface, "wpa_supplicant failed to reply (PONG)"); + pong_failures++; + break; + } + /* supplicant has been terminated */ if(state == WPA_ACTIOND_STATE_CONNECTED || state == WPA_ACTIOND_STATE_CONNECTION_LOST) { + logevent(WPA_ACTIOND_LOG_CUSTOM_ERROR, iface, "wpa_supplicant failed to reply three times in a row - disconnecting"); logevent(WPA_ACTIOND_LOG_DISCONNECTED, iface, ssid); action(WPA_ACTIOND_ACTION_DISCONNECT, iface, ssid, idstr, wpa_ctrl_get_fd(ctrl), script); } @@ -381,6 +390,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t state = WPA_ACTIOND_STATE_TERMINATED; } } + pong_failures = 0; break; default: /* event received */ @@ -446,6 +456,7 @@ static void loop(const char *iface, const char *ctrlpath, const int disconnect_t /* we are not interested in this event */ break; } + pong_failures = 0; } }
-- Humble ping anyone?
Thanks Emil
I don't think anyone maintains wpa_actiond these days. I will look at your change this week, but forking it completely wouldn't be insane.
Thanks Bartłomiej. Forking the project for such trivial corner case seems like an overkill. To the best of your knowledge, is Arch moving away from this project/package? Or simply wpa_actiond has reached its peak and development (hence maintainer) is MIA? -Emil
On 2017-10-23 14:56, Emil Velikov via arch-projects wrote:
Thanks Bartłomiej.
Forking the project for such trivial corner case seems like an overkill.
To the best of your knowledge, is Arch moving away from this project/package? Or simply wpa_actiond has reached its peak and development (hence maintainer) is MIA?
-Emil
Long overdue, but pushed your change to master, thanks. I will backport it to the package later. It never was a core project, more a Thomas' side project. I have never used it, but looking at low commit number, it probably just serves its purpose alright. Bartłomiej
participants (2)
-
Bartłomiej Piotrowski
-
Emil Velikov