[PATCH 4/6] P2P: Add p2p group formation tiemout after p2p invitation result

[Jean-Michel Bachot]

Le 25/06/2011 10:29, Jouni Malinen a ?crit :
> On Thu, Jun 09, 2011 at 04:40:58PM +0000, Sane, Jayant wrote:
>> Register timeout after a successful p2p invitation result
>> cancel timeout when p2p client is connected.
>> Timeout expires if STA is not authorized ( authorized = 0 in sta_authorized_cb)
>> Timeout value is 15 seconds as for the GO negociation timeout.
> This commit message is a bit misleading since the change itself is only
> for the case of persistent groups. Had this been for any invitation, I
> would have rejected this without much thought, but even in the
> persistent group case this is a bit problematic. While the common
> sequence would be to connect immediately, there does not seem to be any
> strict requirement in the P2P specification to do so and the 15 second
> group formation timeout is not applicable to this case either.
>
> Is there a particular reason for this change? Please note that this
> catches only the case where a persistent group is being reinvoked
> between devices that support persistent reconnect. Without that,
> persistent group re-invocation would follow different path due to user
> acceptance being required.
>
Indeed , this timeout has been set only for the case a persistent group 
is reinvoked between devices
that support persistent reconnect. The reason of this timeout is that it 
allows to report the GroupFinished
signal on D-bus interface upon the GO formation timeout expiration (like 
for the GO negocation). If timeout expires, it means that
a connection failure occurs . It can be for example  the 4  way 
handshake that fails. Issue is that no event exists in current 
implementation
that reports failure between the Invitation succes and the real p2p 
connection establishment.
I believe it would be great to implement this kind of event, for example 
a 4-way handhake failure to inform upper layer ... . Would  it be correct ?


>> diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
>> index 0626684..e5534f4 100644
>> +void wpas_p2p_cancel_connect_timeout_after_invite(void *ctx)
>> +{
>> +	struct wpa_supplicant *wpa_s = ctx;
>> +	if (wpa_s->connect_timeout_after_invite) {
>> +		eloop_cancel_timeout(wpas_p2p_group_formation_timeout,
>> +				     wpa_s->parent, NULL);
>> +		wpa_s->connect_timeout_after_invite = 0;
>> +	}
>> +}
> That connect_timeout_after_invite variable should not be needed at all,
> i.e., it is fine to try to cancel an eloop timeout that has not been
> registered. However, you do need to cancel the timeout somewhere on the
> deinit path to avoid leaving pending timeouts with references to
> pointers that may not be valid anymore.
>
>
>
>> @@ -2156,6 +2165,16 @@ static void wpas_invitation_result(void *ctx, int status, const u8 *bssid)
>>
>>   	wpas_p2p_group_add_persistent(wpa_s, ssid,
>>   				      ssid->mode == WPAS_MODE_P2P_GO, 0);
>> +
>> +	/*
>> +	 * Allow the same timeout for re-invoking a persistent group than
>> +	 * normal 15 second group formation timeout.
>> +	 */
>> +	wpa_s->connect_timeout_after_invite = 1;
>> +	eloop_cancel_timeout(wpas_p2p_group_formation_timeout, wpa_s, NULL);
>> +	eloop_register_timeout(15, 0, wpas_p2p_group_formation_timeout, wpa_s,
>> +				   NULL);
> While suitability of the 15 second group formation timeout here is a bit
> unclear, this would at least need to add the maximum configuration
> timeout value from the invitation request/response exchange in the same
> way as is done in the group formation case.
>