Hi Joey,

On Wed, Oct 13, 2010 at 09:58:14AM -0600, Joey Lee wrote:
> Hi Sameul, 
> 
> 於 三,2010-10-13 於 16:07 +0200,Samuel Ortiz 提到:
> > Hi guys,
> > 
> > On Wed, Oct 13, 2010 at 10:33:04AM +0800, [email protected] wrote:
> > > From: Lee, Chun-Yi <[email protected]>
> > > 
> > > There have a race condition issue in connman between wifi driver object 
> > > with
> > > element->driver reference.
> > > On some machine, the wifi driver object doesn't generate before we try to
> > > register a element. It causes element->driver is NULL, and will not do
> > > driver->remove when remove element.
> > Do you mean that the wifi device is not probed when the wifi device driver 
> > is
> > probed ?
> > That is to say, connman_device_register() for the wifi device was not called
> > yet when connman_device_driver_register() is called for the wifi driver ?
> > 
> > I suppose you're running a pre 0.62 ConnMan release. In that case, udev.c
> > enumerates all devices and registers them. For all available network 
> > devices,
> > add_net_device() eventually calls connman_device_register(). Only after that
> > happens, all plugins init routines are called and e.g.
> > connman_device_driver_register() is called from the wifi-legacy plugin.
> > 
> > Could you please send a complete connman.log with full debug of your failure
> > case, from when connmand starts up until you stop it ? 
> 
> The attached file is the fail case connman log:
> 
> Log line description:
> 
> 1168: boot until stable
>       - WiFi APs list show up
> 1174: press Fn+F11 wifi key to turn off wifi
> 1329: press Fn+F11 wifi key to turn on wifi until stable
>       - WiFi switch still show enabled on carrick GUI, but APs list removed.
> 1331: press Fn+F11 wifi key to turn on wifi
> 1437: press Fn+F11 wifi key to turn on wifi until stable
> 
> I also attached it on 
>  http://bugs.meego.com/show_bug.cgi?id=8075
So I've been looking at this issue and this race condition is indeed real, and
not related to your specific hardware. It will be visible to HW that is either
removable (e.g. a US dongle) or completely removed from the bus upon rfkill
commands (e.g. pcie hotpluggable devices).
When the device shows up through udev/rtnl before wpa_supplicant is started,
the corresponding element->driver pointer will not be set, and we'll be left
with a stale connman_device upon removal of the physical device..

Although your patch is correct and does fix the issue, I don't really like
exporting the whole element probing routine, that should be kept internal to
the element code.

What I propose is to create a simple __connman_element_set_driver() routine
that will just set the right driver for a given element. The patch below works
fine for me, could you please give it a try (It may not apply cleanly
depending on the ConnMan version you're using) ?


diff --git a/src/connman.h b/src/connman.h
index 5c17a68..8aaac3a 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -190,6 +190,8 @@ gboolean __connman_element_device_isfiltered(const char 
*devname);
 int __connman_detect_init(void);
 void __connman_detect_cleanup(void);
 
+void __connman_element_set_driver(struct connman_element *element);
+
 #include <connman/ipconfig.h>
 
 int __connman_ipconfig_init(void);
diff --git a/src/device.c b/src/device.c
index 373eb5f..1b721ee 100644
--- a/src/device.c
+++ b/src/device.c
@@ -586,6 +586,8 @@ static void probe_driver(struct connman_element *element, 
gpointer user_data)
 
        element->device->driver = driver;
 
+       __connman_element_set_driver(element);
+
        setup_device(element->device);
 }
 
diff --git a/src/element.c b/src/element.c
index 74383cc..a5700f2 100644
--- a/src/element.c
+++ b/src/element.c
@@ -1322,6 +1322,28 @@ void connman_element_set_error(struct connman_element 
*element,
        __connman_service_indicate_error(service, convert_error(error));
 }
 
+void __connman_element_set_driver(struct connman_element *element)
+{
+       GSList *list;
+
+       DBG("element %p name %s driver %p", element, element->name,
+                                               element->driver);
+
+       if (element->driver)
+               return;
+
+       for (list = driver_list; list; list = list->next) {
+               struct connman_driver *driver = list->data;
+
+               if (match_driver(element, driver) == FALSE)
+                       continue;
+
+               element->driver = driver;
+
+               break;
+       }
+}
+
 int __connman_element_init(const char *device, const char *nodevice)
 {
        struct connman_element *element;




-- 
Intel Open Source Technology Centre
http://oss.intel.com/
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman

Reply via email to