Re: [PATCH 1/2] wifi: add a disconnected timer
Samuel Ortiz writes: > Hi Kalle, Hi Samuel, > On Tue, Oct 26, 2010 at 07:30:53PM +0300, Kalle Valo wrote: >> In a big network, for example at Ubuntu Developer Summit which has >10 APs, >> connman and wpasupplicant got out of sync very easily. connman claimed it >> was connected even though wpasupplicant (and the kernel driver) was actually >> connected to the AP. > > Did you mean connman claimed it was _disconnected_ ? Yes, that's what I mean. Sorry for the confusion. Just to be clear: connman claimed that wifi connection was disconnected, but 'iw wlan0 link' showed that I was connected to Ubuntu network. >> The problem is that while roaming between APs inside ESS >> wpasupplicant states go like this: >> >> COMPLETED -> DISCONNECTED -> SCANNING -> AUTHENTICATING ... -> COMPLETED >> >> So what happens is that connman unnecessarily marks the network disconnected >> even though wpasupplicant is just roaming to a different AP within ESS. > > Well it really got disconnected from the AP, so ConnMan should just track > that. Well, it depends how we want to handle intra-ESS roaming. I see two choises: 1) wpasupplicant manages roaming, connman just provides SSID and other settings 2) connman manages roaming We should definitely go with option 1, it's faster and wpasupplicant has the best information to choose which AP to connect to. So roaming between APs on an ESS would be transparent from connman point of view. > I'm really not a big fan of trying to be smarter than wpa_supplicant. In my > experience, it's the shortest paths to new bugs The problem here is that when wpasupplicant is roaming between APs it goes to the disconnected state and then immediately to the scanning state. The connman wifi plugin doesn't handle this properly. >> To fix this add a timer which waits 10 seconds after a disconnected state. >> If wpasupplicant hasn't connected to a network at time only then set the >> network disconnected. > > It looks a bit like a hack to me, the fundamental issue (ConnMan not tracking > the wpa_supplicant states properly, it seems) is not fixed. I agree my patch is a bit hackish, but it makes a huge difference here at UDS. Without these two patches (and Mohamed's busy loop fix) connman is unusable with a large wifi network. Currently wifi plugin makes wrong assumptions how wpasupplicant works and what the different wpasupplicant states mean. The proper fix would be to change wpasupplicant to not use disconnected state when roaming. The disconnected state should be used only when wpasupplicant is certain that it cannot connect to the ESS anymore, for example after scanning two times and not finding any suitable APs. I think I can work on changing wpasupplicant at some point, but it will take few months before I can find the time. My short term goal is to make connman wifi reliable enough for the users. Thank you for the comments. -- Kalle Valo ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH 1/2] wifi: add a disconnected timer
Hi Kalle, On Tue, Oct 26, 2010 at 07:30:53PM +0300, Kalle Valo wrote: > In a big network, for example at Ubuntu Developer Summit which has >10 APs, > connman and wpasupplicant got out of sync very easily. connman claimed it > was connected even though wpasupplicant (and the kernel driver) was actually > connected to the AP. Did you mean connman claimed it was _disconnected_ ? > The problem is that while roaming between APs inside ESS wpasupplicant states > go like this: > > COMPLETED -> DISCONNECTED -> SCANNING -> AUTHENTICATING ... -> COMPLETED > > So what happens is that connman unnecessarily marks the network disconnected > even though wpasupplicant is just roaming to a different AP within ESS. Well it really got disconnected from the AP, so ConnMan should just track that. I'm really not a big fan of trying to be smarter than wpa_supplicant. In my experience, it's the shortest paths to new bugs > To fix this add a timer which waits 10 seconds after a disconnected state. > If wpasupplicant hasn't connected to a network at time only then set the > network disconnected. It looks a bit like a hack to me, the fundamental issue (ConnMan not tracking the wpa_supplicant states properly, it seems) is not fixed. Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
[PATCH 1/2] wifi: add a disconnected timer
In a big network, for example at Ubuntu Developer Summit which has >10 APs,
connman and wpasupplicant got out of sync very easily. connman claimed it
was connected even though wpasupplicant (and the kernel driver) was actually
connected to the AP.
The problem is that while roaming between APs inside ESS wpasupplicant states
go like this:
COMPLETED -> DISCONNECTED -> SCANNING -> AUTHENTICATING ... -> COMPLETED
So what happens is that connman unnecessarily marks the network disconnected
even though wpasupplicant is just roaming to a different AP within ESS.
To fix this add a timer which waits 10 seconds after a disconnected state.
If wpasupplicant hasn't connected to a network at time only then set the
network disconnected.
---
plugins/wifi.c | 71 ++-
1 files changed, 64 insertions(+), 7 deletions(-)
diff --git a/plugins/wifi.c b/plugins/wifi.c
index 37f6e32..d036a92 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -61,6 +61,7 @@ struct wifi_data {
int index;
unsigned flags;
unsigned int watch;
+ guint disconnect_timer;
};
static int get_bssid(struct connman_device *device,
@@ -309,6 +310,61 @@ static const gchar *state2str(GSupplicantState state)
return "UNKNOWN";
}
+static gboolean disconnected_timeout(gpointer user_data)
+{
+ GSupplicantInterface *interface;
+ struct connman_network *network;
+ struct wifi_data *wifi;
+
+ DBG("");
+
+ interface = user_data;
+
+ wifi = g_supplicant_interface_get_data(interface);
+
+ if (wifi == NULL)
+ return FALSE;
+
+ network = wifi->network;
+
+ connman_network_set_associating(network, FALSE);
+ connman_network_set_connected(network, FALSE);
+
+ connman_network_unref(wifi->network);
+ wifi->network = NULL;
+
+ return FALSE;
+}
+
+static void start_disconnected_timer(GSupplicantInterface *interface)
+{
+ struct wifi_data *wifi = g_supplicant_interface_get_data(interface);
+
+ if (wifi == NULL)
+ return;
+
+ if (wifi->disconnect_timer != 0)
+ return;
+
+ wifi->disconnect_timer = g_timeout_add(1, disconnected_timeout,
+ interface);
+}
+
+static void stop_disconnected_timer(GSupplicantInterface *interface)
+{
+ struct wifi_data *wifi = g_supplicant_interface_get_data(interface);
+
+ if (wifi == NULL)
+ return;
+
+ if (wifi->disconnect_timer == 0)
+ return;
+
+ g_source_remove(wifi->disconnect_timer);
+ wifi->disconnect_timer = 0;
+}
+
+
static void interface_state(GSupplicantInterface *interface)
{
struct connman_network *network;
@@ -328,8 +384,10 @@ static void interface_state(GSupplicantInterface
*interface)
network = wifi->network;
device = wifi->device;
- if (device == NULL || network == NULL)
+ if (device == NULL || network == NULL) {
+ stop_disconnected_timer(interface);
return;
+ }
switch (state) {
case G_SUPPLICANT_STATE_SCANNING:
@@ -345,6 +403,8 @@ static void interface_state(GSupplicantInterface *interface)
/* reset scan trigger and schedule background scan */
connman_device_schedule_scan(device);
+ stop_disconnected_timer(interface);
+
if (get_bssid(device, bssid, &bssid_len) == 0)
connman_network_set_address(network,
bssid, bssid_len);
@@ -352,12 +412,7 @@ static void interface_state(GSupplicantInterface
*interface)
break;
case G_SUPPLICANT_STATE_DISCONNECTED:
- connman_network_set_associating(network, FALSE);
- connman_network_set_connected(network, FALSE);
-
- connman_network_unref(wifi->network);
- wifi->network = NULL;
-
+ start_disconnected_timer(interface);
break;
case G_SUPPLICANT_STATE_INACTIVE:
@@ -640,6 +695,8 @@ static int network_disconnect(struct connman_network
*network)
connman_network_set_associating(network, FALSE);
+ stop_disconnected_timer(wifi->interface);
+
return g_supplicant_interface_disconnect(wifi->interface,
disconnect_callback, wifi);
}
--
1.7.1
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
