Patchwork [2/4] P2P: Add Notice Of Absence interval param

login
register
mail settings
Submitter Janusz.Dziedzic@tieto.com
Date Nov. 28, 2011, 12:11 p.m.
Message ID <3078A9B976EF864C8DDD0C499FFD07911E3B76D9B1@EXMB01.eu.tieto.com>
Download mbox | patch
Permalink /patch/127967/
State Changes Requested
Headers show

Comments

Janusz.Dziedzic@tieto.com - Nov. 28, 2011, 12:11 p.m.
NOA interval param is required during set_noa
and NOA notification.
---
 src/ap/ap_drv_ops.c             |    4 ++--
 src/ap/ap_drv_ops.h             |    2 +-
 src/ap/hostapd.h                |    1 +
 src/ap/p2p_hostapd.c            |   16 ++++++++++------
 src/ap/p2p_hostapd.h            |    2 +-
 src/drivers/driver.h            |    4 +++-
 wpa_supplicant/ctrl_iface.c     |   21 ++++++++++++++++-----
 wpa_supplicant/p2p_supplicant.c |    4 ++--
 wpa_supplicant/p2p_supplicant.h |    2 +-
 9 files changed, 37 insertions(+), 19 deletions(-)
Jouni Malinen - Dec. 6, 2011, 6:42 p.m.
On Mon, Nov 28, 2011 at 02:11:36PM +0200, Janusz.Dziedzic@tieto.com wrote:
> NOA interval param is required during set_noa
> and NOA notification.

Could you please describe the use case for this? set_noa() is really a
testing only functionality in wpa_supplicant and I would expect real
life NoA management to be handled by the driver. For common use cases of
set_noa(), I would expect the interval to be set automatically based on
the Beacon interval. Are you thinking of using this somehow to hardcode
some test cases where the absence periods cross beacon intervals? Is
millisecond resolution suitable for that purpose?
Janusz Dziedzic - Dec. 7, 2011, 6:58 a.m.
2011/12/6 Jouni Malinen <j@w1.fi>:
> On Mon, Nov 28, 2011 at 02:11:36PM +0200, Janusz.Dziedzic@tieto.com wrote:
>> NOA interval param is required during set_noa
>> and NOA notification.
>
> Could you please describe the use case for this? set_noa() is really a
> testing only functionality in wpa_supplicant and I would expect real
What with certification case - we have to setup somehow NOA values
requested from testTool?

> life NoA management to be handled by the driver. For common use cases of
> set_noa(), I would expect the interval to be set automatically based on
> the Beacon interval.
Do you suggest NOA interval should be same as Beacon interval?

> Are you thinking of using this somehow to hardcode
> some test cases where the absence periods cross beacon intervals? Is
> millisecond resolution suitable for that purpose?
>

Yes, this is case for using larger interval.
eg.
interval = 2s
duration = 1s

Regarding resolution, I only add some functionality to API that
currently exists - duration is also in ms.

Anyway I think this could be usefull for test + some certification case.

BR
Janusz
Jouni Malinen - Dec. 11, 2011, 2:58 p.m.
On Wed, Dec 07, 2011 at 07:58:21AM +0100, Janusz Dziedzic wrote:
> 2011/12/6 Jouni Malinen <j@w1.fi>:
> > On Mon, Nov 28, 2011 at 02:11:36PM +0200, Janusz.Dziedzic@tieto.com wrote:
> >> NOA interval param is required during set_noa
> >> and NOA notification.
> >
> > Could you please describe the use case for this? set_noa() is really a
> > testing only functionality in wpa_supplicant and I would expect real
> What with certification case - we have to setup somehow NOA values
> requested from testTool?

Well, I guess that depends on what certification you are thinking of,
but the ones I've seen, do not require this extra parameter.

> > life NoA management to be handled by the driver. For common use cases of
> > set_noa(), I would expect the interval to be set automatically based on
> > the Beacon interval.
> Do you suggest NOA interval should be same as Beacon interval?

Yes.

> Yes, this is case for using larger interval.
> eg.
> interval = 2s
> duration = 1s

OK.. That would be pretty painful for most use cases, but possible, I'd
guess.

> Regarding resolution, I only add some functionality to API that
> currently exists - duration is also in ms.

The problem with this is that you cannot represent the most common test
case I'm aware of (Beacon interval = 100 TU, i.e., 102.4 ms and NoA
interval same as Beacon interval).

> Anyway I think this could be usefull for test + some certification case.

It sounds pretty specific test case, but anyway, I'm not going to accept
this type of modification of the existing command if it breaks use of
current mechanism with interval hardcoded to match with the Beacon
interval.

Patch

diff --git a/src/ap/ap_drv_ops.c b/src/ap/ap_drv_ops.c
index 77d647b..161f411 100644
--- a/src/ap/ap_drv_ops.c
+++ b/src/ap/ap_drv_ops.c
@@ -549,11 +549,11 @@  struct wpa_scan_results * hostapd_driver_get_scan_results(
 
 
 int hostapd_driver_set_noa(struct hostapd_data *hapd, u8 count, int start,
-			   int duration)
+			   int duration, int interval)
 {
 	if (hapd->driver && hapd->driver->set_noa)
 		return hapd->driver->set_noa(hapd->drv_priv, count, start,
-					     duration);
+					     duration, interval);
 	return -1;
 }
 
diff --git a/src/ap/ap_drv_ops.h b/src/ap/ap_drv_ops.h
index c918a4a..6de375b 100644
--- a/src/ap/ap_drv_ops.h
+++ b/src/ap/ap_drv_ops.h
@@ -81,7 +81,7 @@  int hostapd_driver_scan(struct hostapd_data *hapd,
 struct wpa_scan_results * hostapd_driver_get_scan_results(
 	struct hostapd_data *hapd);
 int hostapd_driver_set_noa(struct hostapd_data *hapd, u8 count, int start,
-			   int duration);
+			   int duration, int interval);
 int hostapd_drv_set_key(const char *ifname,
 			struct hostapd_data *hapd,
 			enum wpa_alg alg, const u8 *addr,
diff --git a/src/ap/hostapd.h b/src/ap/hostapd.h
index 5401e80..a85aece 100644
--- a/src/ap/hostapd.h
+++ b/src/ap/hostapd.h
@@ -167,6 +167,7 @@  struct hostapd_data {
 	int noa_enabled;
 	int noa_start;
 	int noa_duration;
+	int noa_interval;
 #endif /* CONFIG_P2P */
 };
 
diff --git a/src/ap/p2p_hostapd.c b/src/ap/p2p_hostapd.c
index 6f8b778..69137b8 100644
--- a/src/ap/p2p_hostapd.c
+++ b/src/ap/p2p_hostapd.c
@@ -37,31 +37,35 @@  int hostapd_p2p_get_mib_sta(struct hostapd_data *hapd, struct sta_info *sta,
 
 
 int hostapd_p2p_set_noa(struct hostapd_data *hapd, u8 count, int start,
-			int duration)
+			int duration, int interval)
 {
 	wpa_printf(MSG_DEBUG, "P2P: Set NoA parameters: count=%u start=%d "
-		   "duration=%d", count, start, duration);
+		   "duration=%d interval=%d", count, start, duration, interval);
 
 	if (count == 0) {
 		hapd->noa_enabled = 0;
 		hapd->noa_start = 0;
 		hapd->noa_duration = 0;
+		hapd->noa_interval = 0;
 	}
 
 	if (count != 255) {
 		wpa_printf(MSG_DEBUG, "P2P: Non-periodic NoA - set "
 			   "NoA parameters");
-		return hostapd_driver_set_noa(hapd, count, start, duration);
+		return hostapd_driver_set_noa(hapd, count, start,
+					      duration, interval);
 	}
 
 	hapd->noa_enabled = 1;
 	hapd->noa_start = start;
 	hapd->noa_duration = duration;
+	hapd->noa_interval = interval;
 
 	if (hapd->num_sta_no_p2p == 0) {
 		wpa_printf(MSG_DEBUG, "P2P: No legacy STAs connected - update "
 			   "periodic NoA parameters");
-		return hostapd_driver_set_noa(hapd, count, start, duration);
+		return hostapd_driver_set_noa(hapd, count, start,
+					      duration, interval);
 	}
 
 	wpa_printf(MSG_DEBUG, "P2P: Legacy STA(s) connected - do not enable "
@@ -77,7 +81,7 @@  void hostapd_p2p_non_p2p_sta_connected(struct hostapd_data *hapd)
 
 	if (hapd->noa_enabled) {
 		wpa_printf(MSG_DEBUG, "P2P: Disable periodic NoA");
-		hostapd_driver_set_noa(hapd, 0, 0, 0);
+		hostapd_driver_set_noa(hapd, 0, 0, 0, 0);
 	}
 }
 
@@ -89,7 +93,7 @@  void hostapd_p2p_non_p2p_sta_disconnected(struct hostapd_data *hapd)
 	if (hapd->noa_enabled) {
 		wpa_printf(MSG_DEBUG, "P2P: Enable periodic NoA");
 		hostapd_driver_set_noa(hapd, 255, hapd->noa_start,
-				       hapd->noa_duration);
+				       hapd->noa_duration, hapd->noa_interval);
 	}
 }
 
diff --git a/src/ap/p2p_hostapd.h b/src/ap/p2p_hostapd.h
index 95b31d9..779c4a4 100644
--- a/src/ap/p2p_hostapd.h
+++ b/src/ap/p2p_hostapd.h
@@ -20,7 +20,7 @@ 
 int hostapd_p2p_get_mib_sta(struct hostapd_data *hapd, struct sta_info *sta,
 			    char *buf, size_t buflen);
 int hostapd_p2p_set_noa(struct hostapd_data *hapd, u8 count, int start,
-			int duration);
+			int duration, int interval);
 void hostapd_p2p_non_p2p_sta_connected(struct hostapd_data *hapd);
 void hostapd_p2p_non_p2p_sta_disconnected(struct hostapd_data *hapd);
 
diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index 44127f4..3550cbe 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -2032,13 +2032,15 @@  struct wpa_driver_ops {
 	 * @count: Count
 	 * @start: Start time in ms from next TBTT
 	 * @duration: Duration in ms
+	 * @interval: Interval in ms
 	 * Returns: 0 on success or -1 on failure
 	 *
 	 * This function is used to set Notice of Absence parameters for GO. It
 	 * is used only for testing. To disable NoA, all parameters are set to
 	 * 0.
 	 */
-	int (*set_noa)(void *priv, u8 count, int start, int duration);
+	int (*set_noa)(void *priv, u8 count, int start, int duration,
+		       int interval);
 
 	/**
 	 * set_p2p_powersave - Set P2P power save options
diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c
index b2789be..8d23029 100644
--- a/wpa_supplicant/ctrl_iface.c
+++ b/wpa_supplicant/ctrl_iface.c
@@ -2947,8 +2947,10 @@  static int p2p_ctrl_set(struct wpa_supplicant *wpa_s, char *cmd)
 
 	if (os_strcmp(cmd, "noa") == 0) {
 		char *pos;
-		int count, start, duration;
-		/* GO NoA parameters: count,start_offset(ms),duration(ms) */
+		int count, start, duration, interval;
+		/* GO NoA parameters:
+		 *    count,start_offset(ms),duration(ms), interval(ms)
+		 */
 		count = atoi(param);
 		pos = os_strchr(param, ',');
 		if (pos == NULL)
@@ -2960,13 +2962,22 @@  static int p2p_ctrl_set(struct wpa_supplicant *wpa_s, char *cmd)
 			return -1;
 		pos++;
 		duration = atoi(pos);
-		if (count < 0 || count > 255 || start < 0 || duration < 0)
+		pos = os_strchr(pos, ',');
+		if (pos == NULL)
+			return -1;
+		pos++;
+		interval = atoi(pos);
+		if (count < 0 || count > 255 || start < 0 ||
+		    duration < 0 || interval < 0)
 			return -1;
 		if (count == 0 && duration > 0)
 			return -1;
+		if (interval < duration)
+			return -1;
 		wpa_printf(MSG_DEBUG, "CTRL_IFACE: P2P_SET GO NoA: count=%d "
-			   "start=%d duration=%d", count, start, duration);
-		return wpas_p2p_set_noa(wpa_s, count, start, duration);
+			   "start=%d duration=%d interval=%d", count, start,
+			   duration, interval);
+		return wpas_p2p_set_noa(wpa_s, count, start, duration, interval);
 	}
 
 	if (os_strcmp(cmd, "ps") == 0)
diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
index c6484af..c82b7f8 100644
--- a/wpa_supplicant/p2p_supplicant.c
+++ b/wpa_supplicant/p2p_supplicant.c
@@ -3859,12 +3859,12 @@  void wpas_p2p_update_config(struct wpa_supplicant *wpa_s)
 
 
 int wpas_p2p_set_noa(struct wpa_supplicant *wpa_s, u8 count, int start,
-		     int duration)
+		     int duration, int interval)
 {
 	if (!wpa_s->ap_iface)
 		return -1;
 	return hostapd_p2p_set_noa(wpa_s->ap_iface->bss[0], count, start,
-				   duration);
+				   duration, interval);
 }
 
 
diff --git a/wpa_supplicant/p2p_supplicant.h b/wpa_supplicant/p2p_supplicant.h
index 9a13f9f..3adb076 100644
--- a/wpa_supplicant/p2p_supplicant.h
+++ b/wpa_supplicant/p2p_supplicant.h
@@ -113,7 +113,7 @@  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);
 int wpas_p2p_set_noa(struct wpa_supplicant *wpa_s, u8 count, int start,
-		     int duration);
+		     int duration, int interval);
 int wpas_p2p_set_cross_connect(struct wpa_supplicant *wpa_s, int enabled);
 void wpas_p2p_notif_connected(struct wpa_supplicant *wpa_s);
 void wpas_p2p_notif_disconnected(struct wpa_supplicant *wpa_s);