[arch-projects] [projects][PATCH] wpa_actiond: Wait for three "failed" PONGs before disconnecting

[Bartłomiej Piotrowski]

On 2017-09-29 15:03, Emil Velikov via arch-projects wrote:
> On 21 August 2017 at 10:34, Emil Velikov <emil.l.velikov at 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 at 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