[PATCH 04/18] sched_scan: Restart pno/sched_scan on channel list update

[Peer, Ilan]

> -----Original Message-----
> From: Hostap [mailto:hostap-bounces at lists.infradead.org] On Behalf Of
> Jouni Malinen
> Sent: Sunday, September 25, 2016 22:22
> To: Otcheretianski, Andrei <andrei.otcheretianski at intel.com>
> Cc: Arik Nemtsov <arik at wizery.com>; hostap at lists.infradead.org; Nemtsov,
> ArikX <arikx.nemtsov at intel.com>
> Subject: Re: [PATCH 04/18] sched_scan: Restart pno/sched_scan on channel
> list update
> 
> On Mon, Sep 05, 2016 at 05:32:57PM +0300, andrei.otcheretianski at intel.com
> wrote:
> > As the scan channels might need to change when the channel list has
> > been updated by the kernel.
> 
> > diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c @@
> > -3223,14 +3223,22 @@ static void wpa_supplicant_update_channel_list(
> >  		free_hw_features(ifs);
> >  		ifs->hw.modes = wpa_drv_get_hw_feature_data(
> >  			ifs, &ifs->hw.num_modes, &ifs->hw.flags);
> > -	}
> >
> > -	/* Restart sched_scan with updated channel list */
> > -	if (wpa_s->sched_scanning) {
> > -		wpa_dbg(wpa_s, MSG_DEBUG,
> > -			"Channel list changed restart sched scan.");
> > -		wpa_supplicant_cancel_sched_scan(wpa_s);
> > -		wpa_supplicant_req_scan(wpa_s, 0, 0);
> > +		/* Restart pno/sched_scan with updated channel list */
> > +		if (ifs->pno) {
> > +			wpas_stop_pno(ifs);
> > +			wpas_start_pno(ifs);
> > +		} else if (ifs->sched_scanning && !ifs->pno_sched_pending) {
> 
> OK.. So this part looks straightforward..
> 
> > +			/*
> > +			 * simulate a timeout to restart it. reset the state
> > +			 * to start from the beginning.
> > +			 */
> > +			wpa_dbg(ifs, MSG_DEBUG,
> > +				"Channel list changed restart sched scan.");
> > +			ifs->sched_scan_timed_out = 1;
> > +			wpa_s->prev_sched_ssid = NULL;
> > +			wpa_supplicant_cancel_sched_scan(ifs);
> 
> While this does something else than the previous code. Could you please
> clarify why this uses a simulated timeout to restart sched_scan?
> 

The simulated timeout was to handle possible race where an ongoing scheduled scan has stopped asynchronously while trying to restart a new scheduled scan cycle. This is similar to what is done in wpa_supplicant_sched_scan_timeout().

> And is that "wpa_s->prev_sched_ssid = NULL" really supposed to use wpa_s
> instead of ifs? I'd assume this was a missed change from moving to be within
> the dl_list_for_each loop with ifs being the pointer to the specific interface
> while wpa_s is pointing to the interface that received the channel list update
> event.
> 

That's a bug. I'll prepare a fixed version for the patch.

Regards,

Ilan.