January 2021 - connman

Re: [PATCH] clock: Add TimeSynced signal emitted when the system time has been synced

by Daniel Wagner

Hi Emmanuel, On Thu, Jan 21, 2021 at 04:35:38PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote: > Hello, > > Finally, after running several campaigns, I faced (rarely) no time update after updating the timeservers lists. > It seems to work better with this second version. > > From ced5a1988f5b0e7a44a278b0e270577e6b427f88 Mon Sep 17 00:00:00 2001 > From: Emmanuel VAUTRIN <Emmanuel.VAUTRIN(a)cpexterne.org> > Date: Thu, 7 Jan 2021 17:27:39 +0100 > Subject: [PATCH v2] clock: Implement Clock API time synchronization > > Implement Clock API TimeserverSynced property, which indicates if the > current system time is synced via NTP servers. > Trigger a time synchronization when the TimeUpdate property is set to > auto. > Fix time synchronization when starting with an empty Timeservers list. This really needs to be a separate patch. > Emit a Time PropertyChanged signal on NTP synchronization success. > --- > doc/clock-api.txt | 5 +++++ > src/clock.c | 15 +++++++++++++++ > src/connman.h | 3 +++ > src/timeserver.c | 43 +++++++++++++++++++++++++++++++++++++++++-- > 4 files changed, 64 insertions(+), 2 deletions(-) > > diff --git a/doc/clock-api.txt b/doc/clock-api.txt > index 6818f5a8..48d29970 100644 > --- a/doc/clock-api.txt > +++ b/doc/clock-api.txt > @@ -85,3 +85,8 @@ Properties uint64 Time [readonly or readwrite] [experimental] > > This list of servers is used when TimeUpdates is set > to auto. > + > + boolean TimeserverSynced [readonly] [experimental] > + > + This value indicates if the current system time > + is synced via NTP servers. Add something like 'Only when the Time property is set to auto and the system time is in sync with an NTP server this property is set to true' > diff --git a/src/clock.c b/src/clock.c The D-Bus bits look good. > diff --git a/src/connman.h b/src/connman.h > index e51d94d0..50d52bbe 100644 > --- a/src/connman.h > +++ b/src/connman.h > @@ -452,6 +452,9 @@ GSList *__connman_timeserver_get_all(struct connman_service *service); > void __connman_timeserver_sync(struct connman_service *service); > void __connman_timeserver_conf_update(struct connman_service *service); > > +bool __connman_timeserver_is_synced(void); > +void __connman_timeserver_set_synced(bool status); > + > enum __connman_dhcpv6_status { > CONNMAN_DHCPV6_STATUS_FAIL = 0, > CONNMAN_DHCPV6_STATUS_SUCCEED = 1, > diff --git a/src/timeserver.c b/src/timeserver.c > index 5f0aca68..2675113b 100644 > --- a/src/timeserver.c > +++ b/src/timeserver.c > @@ -29,6 +29,7 @@ > #include <stdlib.h> > #include <gweb/gresolv.h> > #include <netdb.h> > +#include <sys/time.h> > > #include "connman.h" > > @@ -40,6 +41,7 @@ static GSList *ts_list = NULL; > static char *ts_current = NULL; > static int ts_recheck_id = 0; > static int ts_backoff_id = 0; > +static bool ts_is_synced = false; > > static GResolv *resolv = NULL; > static int resolv_id = 0; > @@ -55,8 +57,24 @@ static void ntp_callback(bool success, void *user_data) > { > DBG("success %d", success); > > + __connman_timeserver_set_synced(success); > if (!success) > sync_next(); Use early exit to avoid big indents: if (!success) { sync_next(); return; } if (!gettimeofday(...)) { connman_warn(...); return; } .... > + else { > + struct timeval tv; > + > + if (gettimeofday(&tv, NULL) == 0) { > + dbus_uint64_t timestamp; > + > + timestamp = tv.tv_sec; > + connman_dbus_property_changed_basic( > + CONNMAN_MANAGER_PATH, > + CONNMAN_CLOCK_INTERFACE, "Time", > + DBUS_TYPE_UINT64, &timestamp); > + } else { > + connman_warn("Failed to get current time"); > + } > + } > } > > static void save_timeservers(char **servers) > @@ -337,6 +355,8 @@ static void ts_reset(struct connman_service *service) > if (!resolv) > return; > > + __connman_timeserver_set_synced(false); > + > /* > * Before we start creating the new timeserver list we must stop > * any ongoing ntp query and server resolution. > @@ -366,6 +386,8 @@ static void ts_reset(struct connman_service *service) > > __connman_service_timeserver_changed(service, timeservers_list); > > + ts_service = service; > + > if (!timeservers_list) { > DBG("No timeservers set."); > return; > @@ -373,7 +395,6 @@ static void ts_reset(struct connman_service *service) > > ts_recheck_enable(); > > - ts_service = service; This looks very suspicious. You need to explain why you need to change this as separate patch. I know why I put the ts_service assignment after the timeserver_list check. ts_service points to the currently used service to probe for NTP. If the timeserver_list is empty, there can't be any service we probe. > timeserver_sync_start(); > } > > @@ -394,6 +415,24 @@ void __connman_timeserver_conf_update(struct connman_service *service) > } > > > +bool __connman_timeserver_is_synced(void) > +{ > + return ts_is_synced; > +} > + > +void __connman_timeserver_set_synced(bool status) > +{ > + if (ts_is_synced != status) { > + dbus_bool_t is_synced; > + > + ts_is_synced = status; > + is_synced = status; > + connman_dbus_property_changed_basic(CONNMAN_MANAGER_PATH, > + CONNMAN_CLOCK_INTERFACE, "TimeserverSynced", > + DBUS_TYPE_BOOLEAN, &is_synced); > + } Again use early exit if (ts_is_synced == status) return ... > +} > + > static int timeserver_start(struct connman_service *service) > { > char **nameservers; > @@ -467,7 +506,7 @@ int __connman_timeserver_system_set(char **servers) > save_timeservers(servers); > > service = connman_service_get_default(); > - __connman_timeserver_conf_update(service); > + ts_reset(service); > > return 0; Apart my style nitpicks (early exit) and the ts_service thing the patch looks good. Thanks, Daniel

1 month

2
1
0 / 0

Re: [PATCH] service: Fix autoconnect after rejection

by Daniel Wagner

Hi, On Mon, Jan 25, 2021 at 07:56:41PM +0000, Matthijs Vader wrote: > There is a patch set, dated 2017 that adds a setting to configure the > maximum number of retries [1]. Including an option to set it negative, > to retry indefinitely. The patch adds this feature directly to the wifi plugin. Emmanuel's approach is to change the main service state machine. This has the benefit that it would work for all technologies. > Since then, we now have 10.000s of devices out there, many of which > use wifi. Many of those devices are completely unattended (ie. in an > installed system with no-one watching over it) or at the very least > can be considered unattended compared to what I understood was the > original ConnMan use case: a phone. The difference in perspective > between a phone in someone his/her hands vs. an unattended device with > hours of travel to get to, makes quite the difference for this. This is correct. > We decided to make it retry the connection forever, which worked out > well. At least much better than the initial situation, where it would > stop retrying after a few attempts. Glad to hear. I really think we should add something like this. In a controlled environment, it totally makes sense to have such an option. Thanks, Daniel

1 month

1
0
0 / 0

RE: [PATCH] service: Fix autoconnect after rejection

by Matthijs Vader

Hello Emmanuel, Daniel, and others, Emmanuel wrote: > > All right Daniel, > > It is clearer now. > > Daniel wrote: > > Yes, this is a design decision. When we get disconnected from an AP we > > can't assume it's just because we had a bad connection. The AP could > > actively disconnect the station because the password has changed. And if > > we keep retry to connect we could get blocked completely. That is why > > there is no automatic state transition from the FAILURE state to IDLE. > > I understand that. My main concern was to be able to leave the Failure state. > It has not to be automatic if it is a security limitation. > > > As this is a recurring topic, I think we could try to be a bit > > user friendly as not all AP will do this. > > What about adding a retry counter and limit the retries? To prevent the > > situation above, we could export the retry limit to the D-Bus API. So if > > an user has such a network setup where constant retry will result in a > > blocked account just try N times. What do you think? Would that work for > > you? > > No problem if you think it is cleaner to require an external intervention. > In this case, I can update our REST api to manage this, for example on > state update signals. I will follow your decision on the best (cleaner / safer) way > to follow. There is a patch set, dated 2017 that adds a setting to configure the maximum number of retries [1]. Including an option to set it negative, to retry indefinitely. Since then, we now have 10.000s of devices out there, many of which use wifi. Many of those devices are completely unattended (ie. in an installed system with no-one watching over it) or at the very least can be considered unattended compared to what I understood was the original ConnMan use case: a phone. The difference in perspective between a phone in someone his/her hands vs. an unattended device with hours of travel to get to, makes quite the difference for this. We decided to make it retry the connection forever, which worked out well. At least much better than the initial situation, where it would stop retrying after a few attempts. For details on how we're currently using (a by now perhaps slightly outdated) ConnMan, please see [2]. Hope this helps, and have a good evening. Matthijs [1] https://lists.01.org/hyperkitty/list/connman@lists.01.org/thread/F5N4TBQK... [2] https://github.com/victronenergy/meta-victronenergy/tree/master/meta-venu...

1 month

1
0
0 / 0

Re: [PATCH] service: Fix autoconnect after rejection

by Daniel Wagner

Hi Emmanuel, On Wed, Jan 13, 2021 at 07:00:18PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote: > Hi Daniel, > > I have been able to reproduce this issue, in a wifi environment with only 1 AP, > that I turn off during the association step, to set the service in failure state. ("State": "failure", > "Error": "blocked" or "Error": "connect-failed"). In this case, it > stays blocked in this state. Yes, this is a design decision. When we get disconnected from an AP we can't assume it's just because we had a bad connection. The AP could actively disconnect the station because the password has changed. And if we keep retry to connect we could get blocked completely. That is why there is no automatic state transition from the FAILURE state to IDLE. As this is a recurring topic, I think we could try to be a bit user friendly as not all AP will do this. > > IIRC, the reason why we don't have this in ConnMan already, is the > > case, where you want to connect to a network with outdated > > credentials. In this case your account could be blocked. Or at least > > that is what I remember. > > I have followed your recommandation, and propose the following patch, fixing it: > ----------------------------------------------------------------------------------------- > From 9df3909aa487eb1f8c8796d82b0ee09adcead36b Mon Sep 17 00:00:00 2001 > From: Emmanuel VAUTRIN <Emmanuel.VAUTRIN(a)cpexterne.org> > Date: Wed, 13 Jan 2021 18:19:55 +0100 > Subject: [PATCH] service: Fix autoconnection of a service in failure state > > Prevents a service to be blocked in failure state, when its > autoconnection fails. > --- > src/service.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/src/service.c b/src/service.c > index 5a582cdf..64ee1f44 100644 > --- a/src/service.c > +++ b/src/service.c > @@ -4186,6 +4186,12 @@ static bool auto_connect_service(GList *services, > return autoconnecting; > } > > + if (service->state == CONNMAN_SERVICE_STATE_FAILURE) { > + /* Back to idle to unblock next autoconnection. */ > + service->state = CONNMAN_SERVICE_STATE_IDLE; > + continue; > + } What about adding a retry counter and limit the retries? To prevent the situation above, we could export the retry limit to the D-Bus API. So if an user has such a network setup where constant retry will result in a blocked account just try N times. What do you think? Would that work for you? Thanks, Daniel

1 month

2
1
0 / 0

Re: [PATCH] wifi: Fix connection before disconnection completed

by Daniel Wagner

Hi, On Mon, Jan 18, 2021 at 01:05:11PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote: > From 08aefcb3bad90041505acaf95400c5a1ac9c711a Mon Sep 17 00:00:00 2001 > From: Gabriel FORTE <gforte(a)wyplay.com> > Date: Fri, 15 Jan 2021 19:40:00 +0100 > Subject: [PATCH] wifi: Fix connection before disconnection completed > > Prevents an input/output error when a network tries to connect before > the complete disconnection of the former connected one. > --- > plugins/wifi.c | 51 +++++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 44 insertions(+), 7 deletions(-) > > diff --git a/plugins/wifi.c b/plugins/wifi.c > index fc304e3b..8975a9ff 100644 > --- a/plugins/wifi.c > +++ b/plugins/wifi.c > @@ -165,6 +165,11 @@ struct wifi_data { > int assoc_code; > }; > > +struct disconnect_cb_data { s/disconnect_cb_data/disconnect_data/ > + struct wifi_data *wifi; > + struct connman_network *network; > +}; > + > static GList *iface_list = NULL; > > static GList *pending_wifi_device = NULL; > @@ -2220,18 +2225,32 @@ static int network_connect(struct connman_network *network) > static void disconnect_callback(int result, GSupplicantInterface *interface, > void *user_data) > { > - struct wifi_data *wifi = user_data; > + struct disconnect_cb_data *cb_data = user_data; > + struct wifi_data *wifi = cb_data->wifi; > + struct connman_network *network = cb_data->network; Generally we try to follow the inverse christmas tree style. > + > + g_free(cb_data); > > - DBG("result %d supplicant interface %p wifi %p", > - result, interface, wifi); > + DBG("result %d supplicant interface %p wifi %p networks: current %p \ > + pending %p disconnected %p", result, interface, wifi, > + wifi->network, wifi->pending_network, network); > > if (result == -ECONNABORTED) { > DBG("wifi interface no longer available"); > return; > } > > - if (wifi->network && wifi->network != wifi->pending_network) > - connman_network_set_connected(wifi->network, false); > + if (network) Useless, the network pointer wont ever be NULL. > + connman_network_set_connected(network, false); > + > + if (network != wifi->network) { > + if (network == wifi->pending_network) { > + wifi->pending_network = NULL; > + } > + DBG("current wifi network has changed since disconnection"); > + return; > + } > + > wifi->network = NULL; > > wifi->disconnecting = false; > @@ -2250,6 +2269,7 @@ static int network_disconnect(struct connman_network *network) > struct connman_device *device = connman_network_get_device(network); > struct wifi_data *wifi; > int err; > + struct disconnect_cb_data *cb_data; > > DBG("network %p", network); > > @@ -2264,8 +2284,16 @@ static int network_disconnect(struct connman_network *network) > > wifi->disconnecting = true; > > + cb_data = g_try_malloc0(sizeof(*cb_data)); Just use g_malloc0((), there is little point in trying to catch failing small memory allocation. The system will be completely useless before we ever see this allocation failing. GLib will fail internally for any small allocation too. > + if (!cb_data) { > + connman_error("Failed to allocate cb_data"); > + return -ENOMEM; > + } > + > + cb_data->wifi = wifi; > + cb_data->network = network; > err = g_supplicant_interface_disconnect(wifi->interface, > - disconnect_callback, wifi); > + disconnect_callback, cb_data); > if (err < 0) > wifi->disconnecting = false; Leaks cb_data. > > @@ -2387,9 +2415,18 @@ static bool handle_wps_completion(GSupplicantInterface *interface, > > if (!wps_ssid || wps_ssid_len != ssid_len || > memcmp(ssid, wps_ssid, ssid_len) != 0) { > + struct disconnect_cb_data *cb_data = > + g_try_malloc0(sizeof(*cb_data)); > + if (!cb_data) { > + connman_error("Failed to allocate cb_data"); > + return false; > + } > + > + cb_data->wifi = wifi; > + cb_data->network = network; > connman_network_set_associating(network, false); > g_supplicant_interface_disconnect(wifi->interface, > - disconnect_callback, wifi); > + disconnect_callback, cb_data); > return false; > } > I've fixed the patch accordingly and pushed the patch. Thanks, Daniel

1 month

2
1
0 / 0

RE: [PATCH] clock: Add TimeSynced signal emitted when the system time has been synced

by VAUTRIN Emmanuel (Canal Plus Prestataire)

Hello, Finally, after running several campaigns, I faced (rarely) no time update after updating the timeservers lists. It seems to work better with this second version. From ced5a1988f5b0e7a44a278b0e270577e6b427f88 Mon Sep 17 00:00:00 2001 From: Emmanuel VAUTRIN <Emmanuel.VAUTRIN(a)cpexterne.org> Date: Thu, 7 Jan 2021 17:27:39 +0100 Subject: [PATCH v2] clock: Implement Clock API time synchronization Implement Clock API TimeserverSynced property, which indicates if the current system time is synced via NTP servers. Trigger a time synchronization when the TimeUpdate property is set to auto. Fix time synchronization when starting with an empty Timeservers list. Emit a Time PropertyChanged signal on NTP synchronization success. --- doc/clock-api.txt | 5 +++++ src/clock.c | 15 +++++++++++++++ src/connman.h | 3 +++ src/timeserver.c | 43 +++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/doc/clock-api.txt b/doc/clock-api.txt index 6818f5a8..48d29970 100644 --- a/doc/clock-api.txt +++ b/doc/clock-api.txt @@ -85,3 +85,8 @@ Properties uint64 Time [readonly or readwrite] [experimental] This list of servers is used when TimeUpdates is set to auto. + + boolean TimeserverSynced [readonly] [experimental] + + This value indicates if the current system time + is synced via NTP servers. diff --git a/src/clock.c b/src/clock.c index 0fde2c34..906538ac 100644 --- a/src/clock.c +++ b/src/clock.c @@ -173,6 +173,7 @@ static DBusMessage *get_properties(DBusConnection *conn, { DBusMessage *reply; DBusMessageIter array, dict; + dbus_bool_t is_synced; struct timeval tv; const char *str; @@ -210,6 +211,10 @@ static DBusMessage *get_properties(DBusConnection *conn, connman_dbus_dict_append_array(&dict, "Timeservers", DBUS_TYPE_STRING, append_timeservers, NULL); + is_synced = __connman_timeserver_is_synced(); + connman_dbus_dict_append_basic(&dict, "TimeserverSynced", + DBUS_TYPE_BOOLEAN, &is_synced); + connman_dbus_dict_close(&array, &dict); return reply; @@ -258,6 +263,7 @@ static DBusMessage *set_property(DBusConnection *conn, if (settimeofday(&tv, NULL) < 0) return __connman_error_invalid_arguments(msg); + __connman_timeserver_set_synced(false); connman_dbus_property_changed_basic(CONNMAN_MANAGER_PATH, CONNMAN_CLOCK_INTERFACE, "Time", DBUS_TYPE_UINT64, &newval); @@ -283,6 +289,13 @@ static DBusMessage *set_property(DBusConnection *conn, connman_dbus_property_changed_basic(CONNMAN_MANAGER_PATH, CONNMAN_CLOCK_INTERFACE, "TimeUpdates", DBUS_TYPE_STRING, &strval); + + if (newval == TIME_UPDATES_AUTO) { + struct connman_service *service; + + service = connman_service_get_default(); + __connman_timeserver_conf_update(service); + } } else if (g_str_equal(name, "Timezone")) { const char *strval; @@ -362,6 +375,8 @@ static DBusMessage *set_property(DBusConnection *conn, connman_dbus_property_changed_array(CONNMAN_MANAGER_PATH, CONNMAN_CLOCK_INTERFACE, "Timeservers", DBUS_TYPE_STRING, append_timeservers, NULL); + } else if (g_str_equal(name, "TimeserverSynced")) { + return __connman_error_permission_denied(msg); } else return __connman_error_invalid_property(msg); diff --git a/src/connman.h b/src/connman.h index e51d94d0..50d52bbe 100644 --- a/src/connman.h +++ b/src/connman.h @@ -452,6 +452,9 @@ GSList *__connman_timeserver_get_all(struct connman_service *service); void __connman_timeserver_sync(struct connman_service *service); void __connman_timeserver_conf_update(struct connman_service *service); +bool __connman_timeserver_is_synced(void); +void __connman_timeserver_set_synced(bool status); + enum __connman_dhcpv6_status { CONNMAN_DHCPV6_STATUS_FAIL = 0, CONNMAN_DHCPV6_STATUS_SUCCEED = 1, diff --git a/src/timeserver.c b/src/timeserver.c index 5f0aca68..2675113b 100644 --- a/src/timeserver.c +++ b/src/timeserver.c @@ -29,6 +29,7 @@ #include <stdlib.h> #include <gweb/gresolv.h> #include <netdb.h> +#include <sys/time.h> #include "connman.h" @@ -40,6 +41,7 @@ static GSList *ts_list = NULL; static char *ts_current = NULL; static int ts_recheck_id = 0; static int ts_backoff_id = 0; +static bool ts_is_synced = false; static GResolv *resolv = NULL; static int resolv_id = 0; @@ -55,8 +57,24 @@ static void ntp_callback(bool success, void *user_data) { DBG("success %d", success); + __connman_timeserver_set_synced(success); if (!success) sync_next(); + else { + struct timeval tv; + + if (gettimeofday(&tv, NULL) == 0) { + dbus_uint64_t timestamp; + + timestamp = tv.tv_sec; + connman_dbus_property_changed_basic( + CONNMAN_MANAGER_PATH, + CONNMAN_CLOCK_INTERFACE, "Time", + DBUS_TYPE_UINT64, &timestamp); + } else { + connman_warn("Failed to get current time"); + } + } } static void save_timeservers(char **servers) @@ -337,6 +355,8 @@ static void ts_reset(struct connman_service *service) if (!resolv) return; + __connman_timeserver_set_synced(false); + /* * Before we start creating the new timeserver list we must stop * any ongoing ntp query and server resolution. @@ -366,6 +386,8 @@ static void ts_reset(struct connman_service *service) __connman_service_timeserver_changed(service, timeservers_list); + ts_service = service; + if (!timeservers_list) { DBG("No timeservers set."); return; @@ -373,7 +395,6 @@ static void ts_reset(struct connman_service *service) ts_recheck_enable(); - ts_service = service; timeserver_sync_start(); } @@ -394,6 +415,24 @@ void __connman_timeserver_conf_update(struct connman_service *service) } +bool __connman_timeserver_is_synced(void) +{ + return ts_is_synced; +} + +void __connman_timeserver_set_synced(bool status) +{ + if (ts_is_synced != status) { + dbus_bool_t is_synced; + + ts_is_synced = status; + is_synced = status; + connman_dbus_property_changed_basic(CONNMAN_MANAGER_PATH, + CONNMAN_CLOCK_INTERFACE, "TimeserverSynced", + DBUS_TYPE_BOOLEAN, &is_synced); + } +} + static int timeserver_start(struct connman_service *service) { char **nameservers; @@ -467,7 +506,7 @@ int __connman_timeserver_system_set(char **servers) save_timeservers(servers); service = connman_service_get_default(); - __connman_timeserver_conf_update(service); + ts_reset(service); return 0; } -- 2.25.1 B.R. Emmanuel

1 month, 1 week

1
0
0 / 0

RE: strange connection problem

by Thomas Green

That appears to work for me. I recommend it for approval Thank you so much Emmanual. Tom -----Original Message----- From: VAUTRIN Emmanuel (Canal Plus Prestataire) <Emmanuel.VAUTRIN(a)cpexterne.org> Sent: Monday, January 18, 2021 6:37 AM To: Thomas Green <TGreen2(a)Sorenson.com>; connman(a)lists.01.org Subject: RE: strange connection problem [EXTERNAL] Thomas, I have just proposed our fix ("[PATCH] wifi: Fix connection before disconnection completed"). Can you apply the patch, and check if it solves your bug? B.R. Emmanuel _______________________________________________ connman mailing list -- connman(a)lists.01.org To unsubscribe send an email to connman-leave(a)lists.01.org

1 month, 1 week

1
0
0 / 0

RE: strange connection problem

by VAUTRIN Emmanuel (Canal Plus Prestataire)

Thomas, I have just proposed our fix ("[PATCH] wifi: Fix connection before disconnection completed"). Can you apply the patch, and check if it solves your bug? B.R. Emmanuel

1 month, 1 week

1
0
0 / 0

[PATCH] wifi: Fix connection before disconnection completed

by VAUTRIN Emmanuel (Canal Plus Prestataire)

From 08aefcb3bad90041505acaf95400c5a1ac9c711a Mon Sep 17 00:00:00 2001 From: Gabriel FORTE <gforte(a)wyplay.com> Date: Fri, 15 Jan 2021 19:40:00 +0100 Subject: [PATCH] wifi: Fix connection before disconnection completed Prevents an input/output error when a network tries to connect before the complete disconnection of the former connected one. --- plugins/wifi.c | 51 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/plugins/wifi.c b/plugins/wifi.c index fc304e3b..8975a9ff 100644 --- a/plugins/wifi.c +++ b/plugins/wifi.c @@ -165,6 +165,11 @@ struct wifi_data { int assoc_code; }; +struct disconnect_cb_data { + struct wifi_data *wifi; + struct connman_network *network; +}; + static GList *iface_list = NULL; static GList *pending_wifi_device = NULL; @@ -2220,18 +2225,32 @@ static int network_connect(struct connman_network *network) static void disconnect_callback(int result, GSupplicantInterface *interface, void *user_data) { - struct wifi_data *wifi = user_data; + struct disconnect_cb_data *cb_data = user_data; + struct wifi_data *wifi = cb_data->wifi; + struct connman_network *network = cb_data->network; + + g_free(cb_data); - DBG("result %d supplicant interface %p wifi %p", - result, interface, wifi); + DBG("result %d supplicant interface %p wifi %p networks: current %p \ + pending %p disconnected %p", result, interface, wifi, + wifi->network, wifi->pending_network, network); if (result == -ECONNABORTED) { DBG("wifi interface no longer available"); return; } - if (wifi->network && wifi->network != wifi->pending_network) - connman_network_set_connected(wifi->network, false); + if (network) + connman_network_set_connected(network, false); + + if (network != wifi->network) { + if (network == wifi->pending_network) { + wifi->pending_network = NULL; + } + DBG("current wifi network has changed since disconnection"); + return; + } + wifi->network = NULL; wifi->disconnecting = false; @@ -2250,6 +2269,7 @@ static int network_disconnect(struct connman_network *network) struct connman_device *device = connman_network_get_device(network); struct wifi_data *wifi; int err; + struct disconnect_cb_data *cb_data; DBG("network %p", network); @@ -2264,8 +2284,16 @@ static int network_disconnect(struct connman_network *network) wifi->disconnecting = true; + cb_data = g_try_malloc0(sizeof(*cb_data)); + if (!cb_data) { + connman_error("Failed to allocate cb_data"); + return -ENOMEM; + } + + cb_data->wifi = wifi; + cb_data->network = network; err = g_supplicant_interface_disconnect(wifi->interface, - disconnect_callback, wifi); + disconnect_callback, cb_data); if (err < 0) wifi->disconnecting = false; @@ -2387,9 +2415,18 @@ static bool handle_wps_completion(GSupplicantInterface *interface, if (!wps_ssid || wps_ssid_len != ssid_len || memcmp(ssid, wps_ssid, ssid_len) != 0) { + struct disconnect_cb_data *cb_data = + g_try_malloc0(sizeof(*cb_data)); + if (!cb_data) { + connman_error("Failed to allocate cb_data"); + return false; + } + + cb_data->wifi = wifi; + cb_data->network = network; connman_network_set_associating(network, false); g_supplicant_interface_disconnect(wifi->interface, - disconnect_callback, wifi); + disconnect_callback, cb_data); return false; } -- 2.25.1

1 month, 1 week

1
0
0 / 0

RE: strange connection problem

by VAUTRIN Emmanuel (Canal Plus Prestataire)

Hello Thomas, > I've been able to work around this by putting a delay between the disconnection of the old AP > and the connection to the new AP in our application, but it seems an inelegant solution. Good you have found a work-around to unblock your situation. > If there is something better, I would like to know. As said before, we have faced similar problems by our side and fixed them. And luckily, I have just begun to check the patch I hope will solve your problem. Our issue is that the wifi disconnect callback misbehaves if the current network has been updated meanwhile. I still have a doubt after my code review (but the patch works fine since a long time), and I want to propose the best solution, but normally, I will propose something quickly, next week. So, just a bit more patience. :) B.R. Emmanuel

1 month, 1 week

1
0
0 / 0

2021

2020

2019

2018

2017

2016

2015

connman January 2021