[PATCH] station: add dependency to device

[PATCH] station: add dependency to device

[Shuai Wang]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

scan_periodic_start triggered by station requires scan_wdev_add to be
called first from device.
---
 src/station.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/station.c b/src/station.c
index c6029423..fd346194 100644
--- a/src/station.c
+++ b/src/station.c
@@ -3225,3 +3225,4 @@ static void station_exit(void)
 
 IWD_MODULE(station, station_init, station_exit)
 IWD_MODULE_DEPENDS(station, netconfig)
+IWD_MODULE_DEPENDS(station, device)
-- 
2.24.0

Re: [PATCH] station: add dependency to device

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]

Hi Shuai,

On 12/5/19 11:56 PM, Shuai Wang wrote:
> scan_periodic_start triggered by station requires scan_wdev_add to be
> called first from device.
> ---
>   src/station.c | 1 +
>   1 file changed, 1 insertion(+)
> 

Thanks for the bug report & fix.  Looks like commit 1057d8aa743a 
("device: Move device creation from netdev.c to event watch")
introduced this regression.

> diff --git a/src/station.c b/src/station.c
> index c6029423..fd346194 100644
> --- a/src/station.c
> +++ b/src/station.c
> @@ -3225,3 +3225,4 @@ static void station_exit(void)
>   
>   IWD_MODULE(station, station_init, station_exit)
>   IWD_MODULE_DEPENDS(station, netconfig)
> +IWD_MODULE_DEPENDS(station, device)
> 

Module dependencies are only really needed if one module _init function 
uses functionality (like event watch registrations, etc) from another 
module at daemon startup time.  For this case the fix is a bit different.

Could you try the attached patch instead?

Regards,
-Denis

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-device-netdev-init-scan-in-netdev-instead-of-device.patch --]
[-- Type: text/x-patch, Size: 2247 bytes --]

From 738184d49167590395439ed677c93db8b58639ac Mon Sep 17 00:00:00 2001
From: Denis Kenzior <denkenz@gmail.com>
Date: Fri, 6 Dec 2019 10:11:51 -0600
Subject: [PATCH] device/netdev: init scan in netdev instead of device

Commit 1057d8aa743a changed the device interface creation logic
from being unconditional inside netdev.c to instead use NETDEV_WATCH_*
events.  However, this broke the assumption that the device interface
was created before all others.  The effect is that the scan_wdev_add
might no longer be called prior to station interface being created.  Fix
this by moving scan_wdev_add/remove calls to netdev.c instead.

Fixes: 1057d8aa743a ("device: Move device creation from netdev.c to event watch")
---
 src/device.c | 4 ----
 src/netdev.c | 4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/device.c b/src/device.c
index 3ed3961920be..0ad1ccbfc809 100644
--- a/src/device.c
+++ b/src/device.c
@@ -322,8 +322,6 @@ static struct device *device_create(struct wiphy *wiphy, struct netdev *netdev)
 		l_info("Unable to register %s interface",
 				L_DBUS_INTERFACE_PROPERTIES);
 
-	scan_wdev_add(netdev_get_wdev_id(device->netdev));
-
 	/*
 	 * register for AP roam transition watch
 	 */
@@ -345,8 +343,6 @@ static void device_free(struct device *device)
 {
 	l_debug("");
 
-	scan_wdev_remove(netdev_get_wdev_id(device->netdev));
-
 	netdev_frame_watch_remove(device->netdev, device->ap_roam_watch);
 	wiphy_state_watch_remove(device->wiphy, device->wiphy_rfkill_watch);
 
diff --git a/src/netdev.c b/src/netdev.c
index af655c842d65..d96ee760c71a 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -626,6 +626,8 @@ static void netdev_free(void *data)
 		WATCHLIST_NOTIFY(&netdev_watches, netdev_watch_func_t,
 					netdev, NETDEV_WATCH_EVENT_DEL);
 
+	scan_wdev_remove(netdev->wdev_id);
+
 	watchlist_destroy(&netdev->frame_watches);
 	watchlist_destroy(&netdev->station_watches);
 
@@ -4204,6 +4206,8 @@ static void netdev_initial_up_cb(int error, uint16_t type, const void *data,
 
 	l_debug("Interface %i initialized", netdev->index);
 
+	scan_wdev_add(netdev->wdev_id);
+
 	WATCHLIST_NOTIFY(&netdev_watches, netdev_watch_func_t,
 				netdev, NETDEV_WATCH_EVENT_NEW);
 	netdev->events_ready = true;
-- 
2.21.0

Re: [PATCH] station: add dependency to device

[Shuai Wang]

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

Hi Denis,

Thanks for the quick response. I tried your patch and it works great :)

Shuai

On Fri, Dec 6, 2019 at 8:27 AM Denis Kenzior <denkenz@gmail.com> wrote:

> Hi Shuai,
>
> On 12/5/19 11:56 PM, Shuai Wang wrote:
> > scan_periodic_start triggered by station requires scan_wdev_add to be
> > called first from device.
> > ---
> >   src/station.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
>
> Thanks for the bug report & fix.  Looks like commit 1057d8aa743a
> ("device: Move device creation from netdev.c to event watch")
> introduced this regression.
>
> > diff --git a/src/station.c b/src/station.c
> > index c6029423..fd346194 100644
> > --- a/src/station.c
> > +++ b/src/station.c
> > @@ -3225,3 +3225,4 @@ static void station_exit(void)
> >
> >   IWD_MODULE(station, station_init, station_exit)
> >   IWD_MODULE_DEPENDS(station, netconfig)
> > +IWD_MODULE_DEPENDS(station, device)
> >
>
> Module dependencies are only really needed if one module _init function
> uses functionality (like event watch registrations, etc) from another
> module at daemon startup time.  For this case the fix is a bit different.
>
> Could you try the attached patch instead?
>
> Regards,
> -Denis
>

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1671 bytes --]

Re: [PATCH] station: add dependency to device

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 172 bytes --]

Hi Shuai,

> Thanks for the quick response. I tried your patch and it works great :)

Great!  Thanks for testing. I pushed this out upstream now.

Regards,
-Denis