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

[Jouni Malinen]

On Mon, May 19, 2014 at 06:04:15PM +0530, Jithu Jance wrote:
> I ran hwsim and fixed a case where the listen was getting stuck (the
> p2p_find case that you mentioned). Please
> find the revised patch below. But when I run the hwsim there were other
> failures which seems to occur with or without this patch.
> I couldn't figure out the cause of the failure as the group formation seems
> to succeed, but the result shows as failed. :(

I'm not sure whether the comments I have below addresses the failures
you saw, but there is something wrong with this patch. I did not run it
through the hwsim test cases, though.

> @@ -238,6 +238,12 @@ static void p2p_listen_in_find(struct p2p_data *p2p,
> int dev_disc)
>   p2p_dbg(p2p, "Starting short listen state (state=%s)",
>   p2p_state_txt(p2p->state));
> 
> + if (p2p->pending_listen_freq) {
> + /* We have a pending p2p_listen request */
> + p2p_dbg(p2p, "p2p_listen command pending already");
> + return;
> + }

This is whitespace damaged (tabs removed?).

> - p2p->pending_listen_freq = freq;
> - p2p->pending_listen_sec = 0;
> - p2p->pending_listen_usec = 1024 * tu;
> -
>   ies = p2p_build_probe_resp_ies(p2p);
>   if (ies == NULL)
>   return;
> 
> + p2p->pending_listen_freq = freq;
> + p2p->pending_listen_sec = 0;
> + p2p->pending_listen_usec = 1024 * tu;

This looks fine.

> @@ -284,16 +290,18 @@ int p2p_listen(struct p2p_data *p2p, unsigned int
>   p2p_dbg(p2p, "Going to listen(only) state");
> 
> + if (p2p->pending_listen_freq) {
> + /* We have a pending p2p_listen request */
> + p2p_dbg(p2p, "p2p_listen command pending already");
> + return -1;
> + }

This part would look reasonable as well on its own. However, the
following change results in issues:

> - p2p->pending_listen_freq = freq;
> - p2p->pending_listen_sec = timeout / 1000;
> - p2p->pending_listen_usec = (timeout % 1000) * 1000;
> -
>   if (p2p->p2p_scan_running) {
>   if (p2p->start_after_scan == P2P_AFTER_SCAN_CONNECT) {
>   p2p_dbg(p2p, "p2p_scan running - connect is already pending - skip
> listen");
> @@ -308,6 +316,10 @@ int p2p_listen(struct p2p_data *p2p, unsigned int
> timeout)
>   if (ies == NULL)
>   return -1;
> 
> + p2p->pending_listen_freq = freq;
> + p2p->pending_listen_sec = timeout / 1000;
> + p2p->pending_listen_usec = (timeout % 1000) * 1000;

It's not shown in the context here, but this moving of
p2p->pending_listen_{sec,usec} would lose information from the
p2p->p2p_scan_running case. That uses p2p->start_after_scan to postpone
start of the listen operation and p2p_run_after_scan() expects
p2p->pending_listen_{sec,usec} to be set fror the P2P_AFTER_SCAN_LISTEN
case.
 
-- 
Jouni Malinen                                            PGP id EFC895FA