Re: [PATCH v2 0/2] report stop sched scan when actually done

On Sun, Dec 25, 2011 at 1:48 AM, Eyal Shapira [off-list ref] wrote:

On Fri, Dec 23, 2011 at 10:46 AM, Luciano Coelho [off-list ref] wrote:

quoted

On Wed, 2011-12-21 at 16:10 +0100, Johannes Berg wrote:

quoted

On Wed, 2011-12-21 at 12:11 +0200, Eyal Shapira wrote:

quoted

Changes in mac/cfg80211 to notify userspace
of NL80211_CMD_SCHED_SCAN_STOPPED only when the driver
actually reported back that it was stopped. Also
blocks other attempts until we're really done.

This fixes a scenario where stop sched scan
is issued and immediately afterwards a new sched scan
is requested (e.g. with other parameters).
Current state caused a race where the driver started
stopping the sched scan but didn't finish and got
another sched scan request which it couldn't handle.

Luca really needs to take a close look at this, and I think you should
also take a look to see if this kind of API change can be avoided.

I had discussed this with Eyal briefly and at first it seemed ok.  But
actually I now agree that this is an API change.

Looking closer, I see that in the current documentation, there's nothing
stating that after sending sched_scan_stop the userspace needs to wait
for the stopped event.  It just says that "[this] event is also sent
when the %NL80211_CMD_STOP_SCHED_SCAN command is received [...]".
Applications may assume that after the stop command completes, it is
really stopped, without waiting for the stopped event.

Could the code be changed so that we delay the STOP_SCHED_SCAN command
completion instead? Then userspace can rely on that (as it should
anyway, because the command can fail) instead of having to wait for the
stopped event (which is a change in the API).

Sounds like this would be the best way to go and I'll do that. So no need
for these patches. I misunderstood the STOP_SCHED_SCAN NL API to be async (due to the NL events) but it can and should be synchronous AFAIU.
(and might sleep / block).

The only mac80211 change I'd be glad to add is to make sched_scan_stop op return
a value so errors can be reported back like it's being done in the sched_scan_stop cfg80211 op

quoted

--
Cheers,
Luca.