wpa_supplicant: Should disconnect on deinit whatever WOWLAN is enable or disable

Dear Hostap organization
--------------------------------------------------------
[wpa_supplicant] Rollback change list:02c21c02d09fdce55c0048cc58ff870cab77c9e9

[Description]

　　The function wpa_drv_get_wowlan() is to get signal of whether wowlan is triggered by wpa itself to kernel through wpa_drv_wowlan() function. Trigger action depends on the flag<wowlan_triggers = xx> in wpa_supp.conf. 

　　It must need to disconnect on interface deinit whatever WOWLAN is enable or disable, because deinit means that interface is removed or wpa_supplicant process killed.
　　If don't do so, the status between kernel/wifi driver and wpa_supplicant will be different and it will occur wifi unexpected behavior.
       E.g: 
            when supplicant re-started up, the status of supplicant is disconnected while kernel and wifi driver is connected because  wifi don't accept any disconnected or deauth signal before.
            And then, it could't scan any aps with <NL_SCAN_FLAGE_RANDOM_ADDR> scan cmd since kernel thinks wifi is already connected and block this scan flag.

Signed-off-by: xing wan <18071720608@163.com>

--------------------------------------------------------

Hi,

On Sat, Nov 07, 2020 at 06:32:07PM +0800, xing wan wrote:
> 
> Dear Hostap organization
> --------------------------------------------------------

This patch doesn't really look like it went through git format-patch.
You might look at other patches on the mailing list to see what the
formatting normally looks like:
https://patchwork.ozlabs.org/project/hostap/list/

> [wpa_supplicant] Rollback change list:02c21c02d09fdce55c0048cc58ff870cab77c9e9

If you're reverting a feature, it's usually nice to CC the author in
case they're still around and care. They might be able to find some
constructive middle ground that resolves both your issue and their
intended feature.

I've CC'd Alfonso.

> [Description]
> 
> 　　The function wpa_drv_get_wowlan() is to get signal of whether wowlan is triggered by wpa itself to kernel through wpa_drv_wowlan() function. Trigger action depends on the flag<wowlan_triggers = xx> in wpa_supp.conf. 
> 
> 　　It must need to disconnect on interface deinit whatever WOWLAN is enable or disable, because deinit means that interface is removed or wpa_supplicant process killed.
> 　　If don't do so, the status between kernel/wifi driver and wpa_supplicant will be different and it will occur wifi unexpected behavior.
>        E.g: 
>             when supplicant re-started up, the status of supplicant is disconnected while kernel and wifi driver is connected because  wifi don't accept any disconnected or deauth signal before.

This last part sounds like your real bug: wpa_supplicant should have
better startup behavior. It should either synchronize the existing state
(and log a state of "connected") or else it should force a disconnect
for any interfaces it's managing.

It's possible to hit this scenario in other ways, I think, like if
wpa_supplicant shuts down uncleanly (e.g., crash), so it seems like a
good idea to solve it generally rather than revert this one feature that
makes it easier to hit.

Brian

>             And then, it could't scan any aps with <NL_SCAN_FLAGE_RANDOM_ADDR> scan cmd since kernel thinks wifi is already connected and block this scan flag.
>
> Signed-off-by: xing wan <18071720608@163.com>

Hi,

On Fri, Nov 13, 2020 at 11:04 PM Brian Norris <briannorris@chromium.org> wrote:
>
> Hi,
>
> On Sat, Nov 07, 2020 at 06:32:07PM +0800, xing wan wrote:
> >
> > Dear Hostap organization
> > --------------------------------------------------------
>
> This patch doesn't really look like it went through git format-patch.
> You might look at other patches on the mailing list to see what the
> formatting normally looks like:
> https://patchwork.ozlabs.org/project/hostap/list/
>
> > [wpa_supplicant] Rollback change list:02c21c02d09fdce55c0048cc58ff870cab77c9e9
>
> If you're reverting a feature, it's usually nice to CC the author in
> case they're still around and care. They might be able to find some
> constructive middle ground that resolves both your issue and their
> intended feature.
>
> I've CC'd Alfonso.

Thanks for including me in the discussion (I am resending to the list
as I had not included it in my previous message).

>
> > [Description]
> >
> >　　 The function wpa_drv_get_wowlan() is to get signal of whether wowlan is triggered by wpa itself to kernel through wpa_drv_wowlan() function. Trigger action depends on the flag<wowlan_triggers = xx> in wpa_supp.conf.
> >
> >　　 It must need to disconnect on interface deinit whatever WOWLAN is enable or disable, because deinit means that interface is removed or wpa_supplicant process killed.

The patch I introduced keeps the WiFi connection alive only if WoWLAN is
enabled, even after wpa_supplicant stops. The purpose of WoWLAN is to
resume/power on the device by using special packages. In the case of
power off (S5),
the full OS is shut down, so we want to keep wifi connection even when
wpa_supplicant stops, to be able to still receive these packages. This
is the reason
for keeping the connection.

> >　　 If don't do so, the status between kernel/wifi driver and wpa_supplicant will be different and it will occur wifi unexpected behavior.
> >        E.g:
> >             when supplicant re-started up, the status of supplicant is disconnected while kernel and wifi driver is connected because  wifi don't accept any disconnected or deauth signal before.
>
> This last part sounds like your real bug: wpa_supplicant should have
> better startup behavior. It should either synchronize the existing state
> (and log a state of "connected") or else it should force a disconnect
> for any interfaces it's managing.

I agree - the fix should be trying to synchronize the state when
wpa_supplicant starts.

>
> It's possible to hit this scenario in other ways, I think, like if
> wpa_supplicant shuts down uncleanly (e.g., crash), so it seems like a
> good idea to solve it generally rather than revert this one feature that
> makes it easier to hit.

Other cases when this might happen is if you have started a connection by using
another tool like "iw".

>
> Brian
>
> >             And then, it could't scan any aps with <NL_SCAN_FLAGE_RANDOM_ADDR> scan cmd since kernel thinks wifi is already connected and block this scan flag.
> >
> > Signed-off-by: xing wan <18071720608@163.com>

Best regards,
Alfonso