[PATCH 1/1] P2P: Avoid resetting pending_listen_freq if p2p_listen is pending

[Jouni Malinen]

On Thu, May 22, 2014 at 12:25:53PM +0530, Jithu Jance wrote:
> I re-organised the code to take care of this case. Hope this is
> fine now.

While this can pass all the hwsim test cases one-by-one separately,
there are still some errors being triggered with "P2P: p2p_listen
command pending already" dropping a P2P_LISTEN command and
wpa_supplicant ending up in state where it is not doing anything after
the previous started operation (e.g., remain-on-channel) completes.

> diff --git a/src/p2p/p2p.c b/src/p2p/p2p.c
> @@ -284,16 +290,21 @@ int p2p_listen(struct p2p_data *p2p, unsigned int timeout)
> 
>  	p2p_dbg(p2p, "Going to listen(only) state");
> 
> +	p2p->pending_listen_sec = timeout / 1000;
> +	p2p->pending_listen_usec = (timeout % 1000) * 1000;
> +
> +	if (p2p->pending_listen_freq) {
> +		/* We have a pending p2p_listen request */
> +		p2p_dbg(p2p, "p2p_listen command pending already");
> +		return -1;
> +	}
> +
>  	freq = p2p_channel_to_freq(p2p->cfg->reg_class, p2p->cfg->channel);
>  	if (freq < 0) {
>  		p2p_dbg(p2p, "Unknown regulatory class/channel");
>  		return -1;
>  	}

This looks a bit odd. Why would either of those return cases update
pending_listen_{sec,usec}? I moved those two back here:

> -	p2p->pending_listen_freq = freq;
> -	p2p->pending_listen_sec = timeout / 1000;
> -	p2p->pending_listen_usec = (timeout % 1000) * 1000;

In other words, only pending_listen_freq moves down to here:

> @@ -308,6 +319,8 @@ int p2p_listen(struct p2p_data *p2p, unsigned int timeout)
>  	if (ies == NULL)
>  		return -1;
> 
> +	p2p->pending_listen_freq = freq;
> +
>  	if (p2p->cfg->start_listen(p2p->cfg->cb_ctx, freq, timeout, ies) < 0) {
>  		p2p_dbg(p2p, "Failed to start listen mode");
>  		p2p->pending_listen_freq = 0;


That said, something is clearly still leaving p2p->pending_listen_freq
set to non-zero value while allowing the previous operation to
terminate. One example that I'm looking at is from
grpform_no_5ghz_add_cli failure case where P2P_FIND is first started and
P2P_LISTEN is then used to move listen-only state. This results in the
previously started p2p_find getting stopped, but p2p_listen execution is
still skipping scheduling of a new operation due to pending_listen_freq
(which is likely left set by an earlier test case).

Maybe p2p_stop_find() would need to clear pending_listen_freq to avoid
this? In this particular error case, the previous find operation is
indeed stopped by wpa_p2p_listen() calling p2p_stop_find just before
calling p2p_listen().
 
-- 
Jouni Malinen                                            PGP id EFC895FA