diff mbox

[v2,4/4] P2P: Handle driver NOA notification

Message ID 3078A9B976EF864C8DDD0C499FFD07911E3B76DBDF@EXMB01.eu.tieto.com
State Changes Requested
Headers show

Commit Message

Janusz.Dziedzic@tieto.com Nov. 29, 2011, 5:24 a.m. UTC
Update NOA attribute in beacon and probe response
frames after we will get NOA change/set notification
from the driver.
---
 src/p2p/p2p.h                   |   22 +++++++++++++++++
 src/p2p/p2p_group.c             |   49 +++++++++++++++++++++++++++++++++++++++
 wpa_supplicant/events.c         |   10 +++++--
 wpa_supplicant/p2p_supplicant.c |   19 +++++++++++++++
 wpa_supplicant/p2p_supplicant.h |    3 ++
 5 files changed, 100 insertions(+), 3 deletions(-)

--
1.7.0.4

Comments

Jouni Malinen Dec. 6, 2011, 6:33 p.m. UTC | #1
On Tue, Nov 29, 2011 at 07:24:37AM +0200, Janusz.Dziedzic@tieto.com wrote:
>  Update NOA attribute in beacon and probe response
> frames after we will get NOA change/set notification
> from the driver.

Could you please provide some more details on how drivers would use this
mechanism and why Beacon and Probe Response updates for NoA should go
through wpa_supplicant? Wouldn't it be cleaner and more efficient to
handle this within the driver especially to handle non-periodic NoA
cases depending on offchannel operations?

It looks like this designs seems to assume that there is only a single
NoA timing schedule in the NoA attribute. What if the driver wants to
use two concurrent schedules?


PS.

As far as contributions to hostap.git are concerned, I would appreciate
an explicit acknowledgment of the licensing terms as described in the
CONTRIBUTIONS file:
http://w1.fi/gitweb/gitweb.cgi?p=hostap.git;a=blob_plain;f=CONTRIBUTIONS
Janusz Dziedzic Dec. 7, 2011, 7:23 a.m. UTC | #2
2011/12/6 Jouni Malinen <j@w1.fi>:
> On Tue, Nov 29, 2011 at 07:24:37AM +0200, Janusz.Dziedzic@tieto.com wrote:
>>  Update NOA attribute in beacon and probe response
>> frames after we will get NOA change/set notification
>> from the driver.
>
> Could you please provide some more details on how drivers would use this
> mechanism and why Beacon and Probe Response updates for NoA should go
> through wpa_supplicant?
In case driver/firmware will not update beacon/probe_response frames
by itself can just use
this NOA change/set notification. After that supplicant will set correct IEs.
Extra start time can be used here to be sure beacon/probe_resp will be
updated correctly and all clients will sync ...

> Wouldn't it be cleaner and more efficient to
> handle this within the driver especially to handle non-periodic NoA
> cases depending on offchannel operations?
>
Driver/firmware could menage NOA itselft and report NOA change/set in
case will not update IEs itself.
In case will do that internally don't need to call this NOA
notification. So, nothing will change for drivers/firmware handle this
internally.

Currently in our environtment we are testing two cases for P2P_GO and NOA:
1) when firmware using beacon/probe_resp templates - and in such case
after we set NOA firmware will update templates and add NOA attr. This
have one limitation. Supplicant will never get probe_requests and will
not build p2p_peers table correctly (invite), but there is one pro
(big pro in case of mobile market) - we will not send probe_requests
to the driver (firmware will handle this internally) - so our platform
could be in deep sleep in case no traffic.
So, probably best here could be implement p2p_find (scan) in GO (but I
think this is not so trivial) and using this firmware
beacon/probe_resp templates.

2) probe_resp is handled by wpa_supplicant/driver. So we have two
options here, first catch this probe_resp frames in the driver and add
NOA attr. Second send NOA notification to wpa_supplicant and update
probe_Response IEs. I choose second option because supplicant already
have an API for create/update NOA Attrs. So, was easy to do.
I also test this with mac80211 driver succesfully.

> It looks like this designs seems to assume that there is only a single
> NoA timing schedule in the NoA attribute. What if the driver wants to
> use two concurrent schedules?
Yes, this is first version and we can improve that.

>
>
> PS.
>
> As far as contributions to hostap.git are concerned, I would appreciate
> an explicit acknowledgment of the licensing terms as described in the
> CONTRIBUTIONS file:
> http://w1.fi/gitweb/gitweb.cgi?p=hostap.git;a=blob_plain;f=CONTRIBUTIONS
>
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP@lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap
>
Jouni Malinen Dec. 11, 2011, 3:21 p.m. UTC | #3
On Wed, Dec 07, 2011 at 08:23:55AM +0100, Janusz Dziedzic wrote:
> In case driver/firmware will not update beacon/probe_response frames
> by itself can just use
> this NOA change/set notification. After that supplicant will set correct IEs.
> Extra start time can be used here to be sure beacon/probe_resp will be
> updated correctly and all clients will sync ...

Wouldn't it be simpler to just do this within the driver? It is trivial
to add NoA to the frame when used and you'll get rid of the extra
latency on going through wpa_supplicant.

> Driver/firmware could menage NOA itselft and report NOA change/set in
> case will not update IEs itself.
> In case will do that internally don't need to call this NOA
> notification. So, nothing will change for drivers/firmware handle this
> internally.

Well, the question here is whether to add the extra complexity into
wpa_supplicant and kernel interface for something that would seem to be
better implemented somewhere else.

> Currently in our environtment we are testing two cases for P2P_GO and NOA:
> 1) when firmware using beacon/probe_resp templates - and in such case
> after we set NOA firmware will update templates and add NOA attr. This
> have one limitation. Supplicant will never get probe_requests and will
> not build p2p_peers table correctly (invite), but there is one pro
> (big pro in case of mobile market) - we will not send probe_requests
> to the driver (firmware will handle this internally) - so our platform
> could be in deep sleep in case no traffic.
> So, probably best here could be implement p2p_find (scan) in GO (but I
> think this is not so trivial) and using this firmware
> beacon/probe_resp templates.

You can reply to the Probe Request frames in firmware, but you will need
to provide notification of receive Probe Request frames (at least some
kind of summary, if you want to reduce frequency of such indications) to
wpa_supplicant to be able to use P2P management code there. In addition,
the firmware will need to be aware of all filtering rules on when to
reply to a Probe Request based on the device information from the
connected P2P clients.

> 2) probe_resp is handled by wpa_supplicant/driver. So we have two
> options here, first catch this probe_resp frames in the driver and add
> NOA attr. Second send NOA notification to wpa_supplicant and update
> probe_Response IEs. I choose second option because supplicant already
> have an API for create/update NOA Attrs. So, was easy to do.
> I also test this with mac80211 driver succesfully.

While this would be possible in theory, it has drawbacks and I'm not
aware of anyone else trying to move to that direction.. All NoA handling
within wpa_supplicant was really meant only for testing purposes. The
driver/firmware is expected to add the NoA attribute based on internal
state.
diff mbox

Patch

diff --git a/src/p2p/p2p.h b/src/p2p/p2p.h
index 51a46c8..e64361d 100644
--- a/src/p2p/p2p.h
+++ b/src/p2p/p2p.h
@@ -1547,4 +1547,26 @@  int p2p_in_progress(struct p2p_data *p2p);
  */
 int p2p_other_scan_completed(struct p2p_data *p2p);
 
+/**
+ * p2p_group_handle_driver_notify_noa - Handle driver notification about
+ *	NOA change/set
+ * @p2p_group: P2P group context from p2p_group_init()
+ * @index: Instance of NOA timeing
+ * @oppps_ctwindow: Opportunistic power save capability
+ *		    BIT(7): 1 - enabled, 0 disabled
+ *		    BITS(0-6) - Client Traffic Window in TU
+ * @count_type: Indicates the number of absence intervals
+ *		255 - mean a continous schedule
+ *		0 - mean NOA disabled
+ * @duration: Duration in units of microseconds that P2P_GO
+ *	      can remain absent following the start of NOA
+ *	      interval
+ * @interval: NOA interval in units of microseconds
+ * @start_time: The start time for the schedule expressed in terms
+ *		of lower 4 bytes of the TSF timer
+ */
+void p2p_group_handle_driver_notify_noa(struct p2p_group *group, u8 index,
+					u8 oppps_ctwindow, u8 count_type,
+					u32 duration, u32 interval,
+					u32 start_time);
 #endif /* P2P_H */
diff --git a/src/p2p/p2p_group.c b/src/p2p/p2p_group.c
index be5075a..a9d3158 100644
--- a/src/p2p/p2p_group.c
+++ b/src/p2p/p2p_group.c
@@ -696,3 +696,52 @@  const u8 * p2p_iterate_group_members(struct p2p_group *group, void **next)
 
 	return iter->addr;
 }
+
+
+void p2p_group_handle_driver_notify_noa(struct p2p_group *group, u8 index,
+					u8 oppps_ctwindow, u8 count_type,
+					u32 duration, u32 interval,
+					u32 start_time)
+{
+	struct wpabuf *req;
+	struct p2p_noa_desc desc = {0}, *noa_desc;
+
+	wpa_printf(MSG_DEBUG, "%s called, index: %d, oppps: %d, count: %d, "
+			      "duration: %d, interval: %d, start: %d\n",
+			      __func__, index, oppps_ctwindow, count_type,
+			      duration, interval, start_time);
+
+	noa_desc = &desc;
+	req = wpabuf_alloc(100);
+	if (req == NULL)
+		return;
+
+	desc.count_type = count_type;
+	desc.duration = duration;
+	desc.interval = interval;
+	desc.start_time = start_time;
+
+	/* Check if NOA disabled */
+	if(count_type == 0)
+		noa_desc = NULL;
+
+	/* Build NOA attr */
+	p2p_buf_add_noa(req,
+			index,
+			!!(oppps_ctwindow & BIT(7)),
+			oppps_ctwindow & ~BIT(7),
+			noa_desc, NULL);
+
+	/* Check if we should clear NOA attr */
+	if (!(oppps_ctwindow & BIT(7)) && !noa_desc) {
+		/* Remove NOA attr */
+		wpabuf_free(req);
+		req = NULL;
+	}
+
+	/* Update beacon, probe_resp IEs */
+	p2p_group_notif_noa(group, req);
+
+	if (req)
+		wpabuf_free(req);
+}
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 509b05e..d3e8086 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -2471,10 +2471,14 @@  void wpa_supplicant_event(void *ctx, enum wpa_event_type event,
 		break;
 	case EVENT_DRIVER_NOTIFY_NOA:
 #ifdef CONFIG_P2P
-		/* TODO: Update Beacon and probe response here
-		 *	 p2p_group_notif_noa()
-		 */
 		wpa_dbg(wpa_s, MSG_DEBUG, "EVENT_DRIVER_NOTIFY_NOA");
+		wpas_handle_notify_noa(wpa_s,
+				data->driver_notify_noa.index,
+				data->driver_notify_noa.oppps_ctwindow,
+				data->driver_notify_noa.count_type,
+				data->driver_notify_noa.duration,
+				data->driver_notify_noa.interval,
+				data->driver_notify_noa.start_time);
 #endif /* CONFIG_P2P */
 		break;
 	default:
diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
index c82b7f8..087250d 100644
--- a/wpa_supplicant/p2p_supplicant.c
+++ b/wpa_supplicant/p2p_supplicant.c
@@ -4161,3 +4161,22 @@  int wpas_p2p_in_progress(struct wpa_supplicant *wpa_s)
 
 	return p2p_in_progress(wpa_s->global->p2p);
 }
+
+
+void wpas_handle_notify_noa(void *ctx, u8 index, u8 oppps_ctwindow,
+			    u8 count_type, u32 duration, u32 interval,
+			    u32 start_time)
+{
+	struct wpa_supplicant *wpa_s = ctx;
+
+	if (wpa_s->global->p2p_disabled || wpa_s->global->p2p == NULL)
+		return;
+
+	if (!wpa_s->ap_iface)
+		return;
+
+	p2p_group_handle_driver_notify_noa(wpa_s->ap_iface->bss[0]->p2p_group,
+					   index, oppps_ctwindow,
+					   count_type, duration, interval,
+					   start_time);
+}
diff --git a/wpa_supplicant/p2p_supplicant.h b/wpa_supplicant/p2p_supplicant.h
index 3adb076..271e312 100644
--- a/wpa_supplicant/p2p_supplicant.h
+++ b/wpa_supplicant/p2p_supplicant.h
@@ -129,4 +129,7 @@  void wpas_p2p_wps_failed(struct wpa_supplicant *wpa_s,
 			 struct wps_event_fail *fail);
 int wpas_p2p_in_progress(struct wpa_supplicant *wpa_s);
 
+void wpas_handle_notify_noa(void *ctx, u8 index, u8 oppps_ctwindow,
+			    u8 count_type, u32 duration, u32 interval,
+			    u32 start_time);
 #endif /* P2P_SUPPLICANT_H */
-- 
1.7.0.4



-----Original Message-----
From: hostap-bounces@lists.shmoo.com [mailto:hostap-bounces@lists.shmoo.com] On Behalf Of Janusz.Dziedzic@tieto.com
Sent: 28 listopada 2011 13:12
To: hostap@lists.shmoo.com
Cc: j@w1.fi; johannes@sipsolutions.net
Subject: [PATCH 4/4] P2P: Handle driver NOA notification


Update NOA attribute in beacon and probe response frames after we will get NOA change/set notification from the driver.
---
 src/p2p/p2p.h                   |   22 +++++++++++++++++
 src/p2p/p2p_group.c             |   49 +++++++++++++++++++++++++++++++++++++++
 wpa_supplicant/events.c         |   10 +++++--
 wpa_supplicant/p2p_supplicant.c |   19 +++++++++++++++
 wpa_supplicant/p2p_supplicant.h |    3 ++
 5 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/src/p2p/p2p.h b/src/p2p/p2p.h index 51a46c8..e64361d 100644
--- a/src/p2p/p2p.h
+++ b/src/p2p/p2p.h
@@ -1547,4 +1547,26 @@  int p2p_in_progress(struct p2p_data *p2p);
  */
 int p2p_other_scan_completed(struct p2p_data *p2p);
 
+/**
+ * p2p_group_handle_driver_notify_noa - Handle driver notification about
+ *	NOA change/set
+ * @p2p_group: P2P group context from p2p_group_init()
+ * @index: Instance of NOA timeing
+ * @oppps_ctwindow: Opportunistic power save capability
+ *		    BIT(7): 1 - enabled, 0 disabled
+ *		    BITS(0-6) - Client Traffic Window in TU
+ * @count_type: Indicates the number of absence intervals
+ *		255 - mean a continous schedule
+ *		0 - mean NOA disabled
+ * @duration: Duration in units of microseconds that P2P_GO
+ *	      can remain absent following the start of NOA
+ *	      interval
+ * @interval: NOA interval in units of microseconds
+ * @start_time: The start time for the schedule expressed in terms
+ *		of lower 4 bytes of the TSF timer
+ */
+void p2p_group_handle_driver_notify_noa(struct p2p_group *group, u8 index,
+					u8 oppps_ctwindow, u8 count_type,
+					u32 duration, u32 interval,
+					u32 start_time);
 #endif /* P2P_H */
diff --git a/src/p2p/p2p_group.c b/src/p2p/p2p_group.c index be5075a..560a2f6 100644
--- a/src/p2p/p2p_group.c
+++ b/src/p2p/p2p_group.c
@@ -696,3 +696,52 @@  const u8 * p2p_iterate_group_members(struct p2p_group *group, void **next)
 
 	return iter->addr;
 }
+
+
+void p2p_group_handle_driver_notify_noa(struct p2p_group *group, u8 index,
+					u8 oppps_ctwindow, u8 count_type,
+					u32 duration, u32 interval,
+					u32 start_time)
+{
+	struct wpabuf *req;
+	struct p2p_noa_desc desc = {0}, *noa_desc;
+
+	wpa_printf(MSG_DEBUG, "%s called, index: %d, oppps: %d, count: %d, "
+			      "duration: %d, interval: %d, start: %d\n",
+			      __func__, index, oppps_ctwindow, count_type,
+			      duration, interval, start_time);
+
+	noa_desc = &desc;
+	req = wpabuf_alloc(100);
+	if (req == NULL)
+		return;
+
+	desc.count_type = count_type;
+	desc.duration = duration;
+	desc.interval = interval;
+	desc.start_time = start_time;
+
+	/* Check if NOA disabled */
+	if(count_type == 0)
+		noa_desc = NULL;
+
+	/* Build NOA attr */
+	p2p_buf_add_noa(req,
+			index,
+			0,
+			0,
+			noa_desc, NULL);
+
+	/* Check if we should clear NOA attr */
+	if (!(oppps_ctwindow & BIT(7)) && !noa_desc) {
+		/* Remove NOA attr */
+		wpabuf_free(req);
+		req = NULL;
+	}
+
+	/* Update beacon, probe_resp IEs */
+	p2p_group_notif_noa(group, req);
+
+	if (req)
+		wpabuf_free(req);
+}
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c index 509b05e..d3e8086 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -2471,10 +2471,14 @@  void wpa_supplicant_event(void *ctx, enum wpa_event_type event,
 		break;
 	case EVENT_DRIVER_NOTIFY_NOA:
 #ifdef CONFIG_P2P
-		/* TODO: Update Beacon and probe response here
-		 *	 p2p_group_notif_noa()
-		 */
 		wpa_dbg(wpa_s, MSG_DEBUG, "EVENT_DRIVER_NOTIFY_NOA");
+		wpas_handle_notify_noa(wpa_s,
+				data->driver_notify_noa.index,
+				data->driver_notify_noa.oppps_ctwindow,
+				data->driver_notify_noa.count_type,
+				data->driver_notify_noa.duration,
+				data->driver_notify_noa.interval,
+				data->driver_notify_noa.start_time);
 #endif /* CONFIG_P2P */
 		break;
 	default:
diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c index c82b7f8..087250d 100644
--- a/wpa_supplicant/p2p_supplicant.c
+++ b/wpa_supplicant/p2p_supplicant.c
@@ -4161,3 +4161,22 @@  int wpas_p2p_in_progress(struct wpa_supplicant *wpa_s)
 
 	return p2p_in_progress(wpa_s->global->p2p);
 }
+
+
+void wpas_handle_notify_noa(void *ctx, u8 index, u8 oppps_ctwindow,
+			    u8 count_type, u32 duration, u32 interval,
+			    u32 start_time)
+{
+	struct wpa_supplicant *wpa_s = ctx;
+
+	if (wpa_s->global->p2p_disabled || wpa_s->global->p2p == NULL)
+		return;
+
+	if (!wpa_s->ap_iface)
+		return;
+
+	p2p_group_handle_driver_notify_noa(wpa_s->ap_iface->bss[0]->p2p_group,
+					   index, oppps_ctwindow,
+					   count_type, duration, interval,
+					   start_time);
+}
diff --git a/wpa_supplicant/p2p_supplicant.h b/wpa_supplicant/p2p_supplicant.h index 3adb076..271e312 100644
--- a/wpa_supplicant/p2p_supplicant.h
+++ b/wpa_supplicant/p2p_supplicant.h
@@ -129,4 +129,7 @@  void wpas_p2p_wps_failed(struct wpa_supplicant *wpa_s,
 			 struct wps_event_fail *fail);
 int wpas_p2p_in_progress(struct wpa_supplicant *wpa_s);
 
+void wpas_handle_notify_noa(void *ctx, u8 index, u8 oppps_ctwindow,
+			    u8 count_type, u32 duration, u32 interval,
+			    u32 start_time);
 #endif /* P2P_SUPPLICANT_H */