[PATCH 1/2] P2P: improve deinit for go neg non-zero status

[Jouni Malinen]

On Mon, May 12, 2014 at 11:42:55AM -0400, Eduardo Abinader wrote:
> On Mon, May 12, 2014 at 11:18 AM, Jouni Malinen <j at w1.fi> wrote:
> >
> > On Sun, May 11, 2014 at 12:54:43PM -0400, Eduardo Abinader wrote:
> > > Added cancel timeout and issuing formation failed, in order
> > > to improve detection for group formation failure, upon
> > > non-zero status.

Actually, where does this improve detection?

> > > diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
> > > @@ -1590,6 +1590,8 @@ static void wpas_go_neg_completed(void *ctx, struct p2p_go_neg_results *res)
> > >                              res->status);
> > >               wpas_notify_p2p_go_neg_completed(wpa_s, res);
> > >               wpas_p2p_remove_pending_group_interface(wpa_s);
> > > +             eloop_cancel_timeout(wpas_p2p_long_listen_timeout, wpa_s, NULL);
> > > +             wpas_p2p_group_formation_failed(wpa_s);
> > >               return;
> > >       }

I was trying to go through the wpas_p2p_group_formation_failed() path
here and it ends up doing number of things that do not have any
relevance for this specific non-zero status case:
- eloop_cancel_timeout for wpas_p2p_group_formation_timeout (which has
  not even been started yet)
- p2p_group_formation_failed (which does not do anything new since
  p2p_go_neg_failed() is already taking care of that before calling this
  callback)
- wpas_group_formation_completed (which does number of items related to
  clearing group interface or network block which have not even been
  added yet).

In other words, I'm clearly missing something else about this.. What
does not detect that group formation failed based on the already
existing notification on GO Negotiation failure?

> You're right. We must ensure p2p_long_listen=0.
> 
> I think we could keep this cancel_timeout, in order to earlier free
> the timeout queue and set this p2p_long_listen=0 in patch 2/2 (just
> moving after the formation_failed). This would comply with
> p2p_stop_find_oper and p2p_listen, that do the same.
> 
> Either way, keeping the timeout everything would be fine also.

I was thinking of going ahead with p2p_long_listen = 0 added here, but
then got to the questions above.. Clearly this would do much more than
what could potentially be needed here and as such, I don't think it is
necessary to call wpas_p2p_group_formation_failed(). Instead, a subset
of that functionality could be added here for GO Negotiation failure
case if such subset can be identified.

-- 
Jouni Malinen                                            PGP id EFC895FA