diff mbox

Remove P2P Group Client virtual interface on getting a Deauth Event.

Message ID 6C370B347C3FE8438C9692873287D2E119560C6AFE@SJEXCHCCR01.corp.ad.broadcom.com
State Deferred
Headers show

Commit Message

Jithu Jance Oct. 30, 2011, 6:11 p.m. UTC
This patch removes P2P Group Client virtual interface on getting a Deauth Event. Kindly
see whether the patch is okay.


 Signed-off-by: Jithu Jance <jithu@broadcom.com>

---
 wpa_supplicant/events.c         |    4 ++++
 wpa_supplicant/p2p_supplicant.c |   15 +++++++++++++++
 wpa_supplicant/p2p_supplicant.h |    2 ++
 3 files changed, 21 insertions(+), 0 deletions(-)

Comments

Jouni Malinen Oct. 30, 2011, 7:47 p.m. UTC | #1
On Sun, Oct 30, 2011 at 11:11:32AM -0700, Jithu Jance wrote:
> This patch removes P2P Group Client virtual interface on getting a Deauth Event. Kindly
> see whether the patch is okay.

This is an area that I've had some problems in trying to make up my mind
on what would the best approach here. Normal wpa_supplicant behavior is
to try to reconnect to the network whenever an association is lost for
whatever reason. This different P2P behavior does not really sound very
consistent with it and it would have been quite a bit nicer if the GO
behavior for disconnecting a P2P client would have been more explicit by
using a P2P IE with explicit notification of client being kicked out
from the group..

It could be a bit safer to use the group idle mechanism instead of
removing the group immediately, i.e., allow some time (say, 15 seconds)
for wpa_supplicant to try to reconnect and if that does not go through,
remove the group at that point. This should be easily doable by
extending wpas_p2p_set_group_idle_timeout() to take in an argument
specifying that forced idle time and calling it from the deauth/disassoc
event rather than removing the group immediately.

> diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
> @@ -3717,6 +3717,21 @@ void wpas_p2p_deauth_notif(struct wpa_supplicant *wpa_s, const u8 *bssid,
> +void wpas_p2p_group_remove_notif(struct wpa_supplicant *wpa_s, u16 reason_code)
> +{
> +	if(wpa_s->global->p2p_disabled)
> +		return;
> +
> +	/* If we are running a P2P Client and we received a Deauth/Disassoc from the Go, then remove
> +	   the virutal interface on which the client is running. */
> +	if((wpa_s != wpa_s->parent) && (wpa_s->p2p_group_interface == P2P_GROUP_INTERFACE_CLIENT) && (wpa_s->key_mgmt != WPA_KEY_MGMT_WPS)) {

Why would this be limited to case of using a virtual group interface? I
would expect to see same behavior regardless of whether a separate
interface is used. The group idle handles this in consistent way.

In addition, there may not really be need for this new
wpas_p2p_group_remove_notif() since the existing
wpas_p2p_notif_disconnected() could already be enough. Actually, you may
get the desired functionality by just configuring p2p_group_idle=15
which means that the P2P client interface is automatically removed if
the GO has not been visible for 15 seconds.
Jithu Jance Oct. 31, 2011, 10:08 a.m. UTC | #2
Hi Jouni,

Your suggestion of using "p2p_group_idle" seems to be more beneficial. This will allow the framework/upper application to decide whether the Group needs to be removed or not. For applications that wants to disconnect P2P client on deauth can set "p2p_group_idle". This will give more flexibility to the upper framework. Thanks for pointing this out. 

- Jithu Jance.

-----Original Message-----
From: hostap-bounces@lists.shmoo.com [mailto:hostap-bounces@lists.shmoo.com] On Behalf Of Jouni Malinen
Sent: Monday, October 31, 2011 1:17 AM
To: hostap@lists.shmoo.com
Subject: Re: [PATCH] Remove P2P Group Client virtual interface on getting a Deauth Event.

On Sun, Oct 30, 2011 at 11:11:32AM -0700, Jithu Jance wrote:
> This patch removes P2P Group Client virtual interface on getting a Deauth Event. Kindly
> see whether the patch is okay.

This is an area that I've had some problems in trying to make up my mind
on what would the best approach here. Normal wpa_supplicant behavior is
to try to reconnect to the network whenever an association is lost for
whatever reason. This different P2P behavior does not really sound very
consistent with it and it would have been quite a bit nicer if the GO
behavior for disconnecting a P2P client would have been more explicit by
using a P2P IE with explicit notification of client being kicked out
from the group..

It could be a bit safer to use the group idle mechanism instead of
removing the group immediately, i.e., allow some time (say, 15 seconds)
for wpa_supplicant to try to reconnect and if that does not go through,
remove the group at that point. This should be easily doable by
extending wpas_p2p_set_group_idle_timeout() to take in an argument
specifying that forced idle time and calling it from the deauth/disassoc
event rather than removing the group immediately.

> diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
> @@ -3717,6 +3717,21 @@ void wpas_p2p_deauth_notif(struct wpa_supplicant *wpa_s, const u8 *bssid,
> +void wpas_p2p_group_remove_notif(struct wpa_supplicant *wpa_s, u16 reason_code)
> +{
> +	if(wpa_s->global->p2p_disabled)
> +		return;
> +
> +	/* If we are running a P2P Client and we received a Deauth/Disassoc from the Go, then remove
> +	   the virutal interface on which the client is running. */
> +	if((wpa_s != wpa_s->parent) && (wpa_s->p2p_group_interface == P2P_GROUP_INTERFACE_CLIENT) && (wpa_s->key_mgmt != WPA_KEY_MGMT_WPS)) {

Why would this be limited to case of using a virtual group interface? I
would expect to see same behavior regardless of whether a separate
interface is used. The group idle handles this in consistent way.

In addition, there may not really be need for this new
wpas_p2p_group_remove_notif() since the existing
wpas_p2p_notif_disconnected() could already be enough. Actually, you may
get the desired functionality by just configuring p2p_group_idle=15
which means that the P2P client interface is automatically removed if
the GO has not been visible for 15 seconds.
Jouni Malinen Dec. 18, 2011, 8:27 p.m. UTC | #3
On Mon, Oct 31, 2011 at 03:08:34AM -0700, Jithu Jance wrote:
> Your suggestion of using "p2p_group_idle" seems to be more beneficial. This will allow the framework/upper application to decide whether the Group needs to be removed or not. For applications that wants to disconnect P2P client on deauth can set "p2p_group_idle". This will give more flexibility to the upper framework.

After some more thought on this, I ended up adding a hardcoded 10 second
idle timeout for P2P client role since I cannot really come up with a
good use case for P2P client continuing reconnection attempts for long
period. In other words, the P2P client role group will now be terminated
automatically. This doesn't happen immediately on reception of
Deauthentication/Disassociate frame to avoid potential problems with
temporary disconnection. However, the timeout is short enough to make
this happen pretty quickly in the case the GO is indeed tearing down the
group.
diff mbox

Patch

diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 80ca869..11fc39b 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -2024,6 +2024,10 @@  void wpa_supplicant_event(void *ctx, enum wpa_event_type event,
 		}
 #endif /* CONFIG_AP */
 		wpa_supplicant_event_disassoc(wpa_s, reason_code);
+#ifdef CONFIG_P2P
+		wpas_p2p_group_remove_notif(wpa_s, reason_code);
+#endif
+
 		break;
 	case EVENT_MICHAEL_MIC_FAILURE:
 		wpa_supplicant_event_michael_mic_failure(wpa_s, data);
diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
index 3723c50..8b20003 100644
--- a/wpa_supplicant/p2p_supplicant.c
+++ b/wpa_supplicant/p2p_supplicant.c
@@ -3717,6 +3717,21 @@  void wpas_p2p_deauth_notif(struct wpa_supplicant *wpa_s, const u8 *bssid,
 	p2p_deauth_notif(wpa_s->global->p2p, bssid, reason_code, ie, ie_len);
 }
 
+void wpas_p2p_group_remove_notif(struct wpa_supplicant *wpa_s, u16 reason_code)
+{
+	if(wpa_s->global->p2p_disabled)
+		return;
+
+	/* If we are running a P2P Client and we received a Deauth/Disassoc from the Go, then remove
+	   the virutal interface on which the client is running. */
+	if((wpa_s != wpa_s->parent) && (wpa_s->p2p_group_interface == P2P_GROUP_INTERFACE_CLIENT) && (wpa_s->key_mgmt != WPA_KEY_MGMT_WPS)) {
+
+		wpa_printf(MSG_DEBUG, "P2P: [EVENT_DEAUTH] Removing P2P_CLIENT virtual intf.");
+		wpa_supplicant_cancel_scan(wpa_s);
+		wpa_s->removal_reason = P2P_GROUP_REMOVAL_UNAVAILABLE;
+		wpas_p2p_group_delete(wpa_s);
+	}
+}
 
 void wpas_p2p_disassoc_notif(struct wpa_supplicant *wpa_s, const u8 *bssid,
 			     u16 reason_code, const u8 *ie, size_t ie_len)
diff --git a/wpa_supplicant/p2p_supplicant.h b/wpa_supplicant/p2p_supplicant.h
index 9a13f9f..3387722 100644
--- a/wpa_supplicant/p2p_supplicant.h
+++ b/wpa_supplicant/p2p_supplicant.h
@@ -109,6 +109,8 @@  int wpas_p2p_ext_listen(struct wpa_supplicant *wpa_s, unsigned int period,
 			unsigned int interval);
 void wpas_p2p_deauth_notif(struct wpa_supplicant *wpa_s, const u8 *bssid,
 			   u16 reason_code, const u8 *ie, size_t ie_len);
+void wpas_p2p_group_remove_notif(struct wpa_supplicant *wpa_s,
+				u16 reason_code);
 void wpas_p2p_disassoc_notif(struct wpa_supplicant *wpa_s, const u8 *bssid,
 			     u16 reason_code, const u8 *ie, size_t ie_len);
 void wpas_p2p_update_config(struct wpa_supplicant *wpa_s);