| Message ID | 1424226915-1100-4-git-send-email-ilan.peer@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Feb 17, 2015 at 09:35:12PM -0500, Ilan Peer wrote: > From: Eliad Peller <eliad@wizery.com> > Read p2p_go_ctwindow from the config file, and pass > it to the driver on GO start. > > Use p2p_go_ctwindow=9 by default. Where does this value of 9 TU come from? The P2P spec states that CTWindow should be at least 10 TU for the GO to be discoverable, so making 9 as the default sounds quite strange. I'm replacing this with 0 (i.e., do not configure CTWindow) to maintain previous behavior and avoid any potential issues with drivers that might not be prepared for the value to be configured in the first place. If you want to use 9 TU for some reason, you can set that in the configuration.
> -----Original Message----- > From: Jouni Malinen [mailto:j@w1.fi] > Sent: Saturday, February 21, 2015 15:38 > To: Peer, Ilan > Cc: hostap@lists.shmoo.com; Peller, EliadX > Subject: Re: [PATCH 4/7] P2P: Allow configuring ctwindow when working as > GO > > On Tue, Feb 17, 2015 at 09:35:12PM -0500, Ilan Peer wrote: > > From: Eliad Peller <eliad@wizery.com> > > Read p2p_go_ctwindow from the config file, and pass it to the driver > > on GO start. > > > > Use p2p_go_ctwindow=9 by default. > > Where does this value of 9 TU come from? The P2P spec states that > CTWindow should be at least 10 TU for the GO to be discoverable, so making 9 > as the default sounds quite strange. I'm replacing this with 0 (i.e., do not > configure CTWindow) to maintain previous behavior and avoid any potential > issues with drivers that might not be prepared for the value to be configured > in the first place. If you want to use 9 TU for some reason, you can set that in > the configuration. > After discussing this with requirement owner, he did recommend changing to 10. Thanks again, Ilan.
On Sun, Feb 22, 2015 at 03:22:55PM +0000, Peer, Ilan wrote: > > From: Jouni Malinen [mailto:j@w1.fi] > > > From: Eliad Peller <eliad@wizery.com> > > > Read p2p_go_ctwindow from the config file, and pass it to the driver > > > on GO start. > > > > > > Use p2p_go_ctwindow=9 by default. > > > > Where does this value of 9 TU come from? The P2P spec states that > > CTWindow should be at least 10 TU for the GO to be discoverable, so making 9 > > as the default sounds quite strange. I'm replacing this with 0 (i.e., do not > > configure CTWindow) to maintain previous behavior and avoid any potential > > issues with drivers that might not be prepared for the value to be configured > > in the first place. If you want to use 9 TU for some reason, you can set that in > > the configuration. > > > > After discussing this with requirement owner, he did recommend changing to 10. Is there a specific reason for that value? I'm not planning on changing this from 0 (which matches the previous behavior) unless there is strong justification for enabling CTWindow configuration on GO by default. 10 TU would sound like a way too small default value, i.e., it would make the GO pretty difficult to discover since the GO could end up being 90% of the time away from the operating channel. That may be justifiable for some use cases (and that is now configurable, so that need is covered), but it does not sound like something I'd use as a default value.
Hi jouni, > -----Original Message----- > From: Jouni Malinen [mailto:j@w1.fi] > Sent: Sunday, February 22, 2015 18:29 > To: Peer, Ilan > Cc: Peller, EliadX; hostap@lists.shmoo.com > Subject: Re: [PATCH 4/7] P2P: Allow configuring ctwindow when working as > GO > > On Sun, Feb 22, 2015 at 03:22:55PM +0000, Peer, Ilan wrote: > > > From: Jouni Malinen [mailto:j@w1.fi] > > > > From: Eliad Peller <eliad@wizery.com> Read p2p_go_ctwindow from > > > > the config file, and pass it to the driver on GO start. > > > > > > > > Use p2p_go_ctwindow=9 by default. > > > > > > Where does this value of 9 TU come from? The P2P spec states that > > > CTWindow should be at least 10 TU for the GO to be discoverable, so > > > making 9 as the default sounds quite strange. I'm replacing this > > > with 0 (i.e., do not configure CTWindow) to maintain previous > > > behavior and avoid any potential issues with drivers that might not > > > be prepared for the value to be configured in the first place. If > > > you want to use 9 TU for some reason, you can set that in the > configuration. > > > > > > > After discussing this with requirement owner, he did recommend changing > to 10. > > Is there a specific reason for that value? I'm not planning on changing this > from 0 (which matches the previous behavior) unless there is strong > justification for enabling CTWindow configuration on GO by default. 10 TU > would sound like a way too small default value, i.e., it would make the GO > pretty difficult to discover since the GO could end up being 90% of the time > away from the operating channel. That may be justifiable for some use cases > (and that is now configurable, so that need is covered), but it does not sound > like something I'd use as a default value. > The P2P GO ctwindow patches were introduced in order to better handle some Miracast adapters that did not handle periodic NOA well (multi-channel scenarios). As far as I understand, the CTWindows does not imply that the P2P GO disappears from the medium once the CTWindows ends (and it can't as long as there are active clients), but guarantees a period of time were clients can start a frame exchange (and override periodic NOA). I'll further check about your concern that this is too small value. I think that this can stay zero for now. Thanks again., Ilan.
diff --git a/src/ap/ap_config.h b/src/ap/ap_config.h index 0f33ac9..ec73771 100644 --- a/src/ap/ap_config.h +++ b/src/ap/ap_config.h @@ -629,6 +629,10 @@ struct hostapd_config { u8 vht_oper_centr_freq_seg0_idx; u8 vht_oper_centr_freq_seg1_idx; +#ifdef CONFIG_P2P + u8 p2p_go_ctwindow; +#endif /* CONFIG_P2P */ + #ifdef CONFIG_TESTING_OPTIONS double ignore_probe_probability; double ignore_auth_probability; diff --git a/src/ap/beacon.c b/src/ap/beacon.c index aa5821b..5299cc0 100644 --- a/src/ap/beacon.c +++ b/src/ap/beacon.c @@ -1007,6 +1007,9 @@ int ieee802_11_build_ap_params(struct hostapd_data *hapd, params->hessid = hapd->conf->hessid; params->access_network_type = hapd->conf->access_network_type; params->ap_max_inactivity = hapd->conf->ap_max_inactivity; +#ifdef CONFIG_P2P + params->p2p_go_ctwindow = hapd->iconf->p2p_go_ctwindow; +#endif /* CONFIG_P2P */ #ifdef CONFIG_HS20 params->disable_dgaf = hapd->conf->disable_dgaf; if (hapd->conf->osen) { diff --git a/src/drivers/driver.h b/src/drivers/driver.h index b1d896c..83a49ea 100644 --- a/src/drivers/driver.h +++ b/src/drivers/driver.h @@ -1010,6 +1010,11 @@ struct wpa_driver_ap_params { int ap_max_inactivity; /** + * ctwindow - Client Traffic Window (in TUs) + */ + u8 p2p_go_ctwindow; + + /** * smps_mode - SMPS mode * * SMPS mode to be used by the AP, specified as the relevant bits of diff --git a/wpa_supplicant/ap.c b/wpa_supplicant/ap.c index fca2137..771c9f4 100644 --- a/wpa_supplicant/ap.c +++ b/wpa_supplicant/ap.c @@ -265,6 +265,17 @@ static int wpa_supplicant_conf_ap(struct wpa_supplicant *wpa_s, else if (wpa_s->conf->beacon_int) conf->beacon_int = wpa_s->conf->beacon_int; +#ifdef CONFIG_P2P + if (wpa_s->conf->p2p_go_ctwindow > conf->beacon_int) { + wpa_printf(MSG_ERROR, + "CTWindow (%d) is bigger than beacon interval (%d) - avoid configuring it!", + wpa_s->conf->p2p_go_ctwindow, conf->beacon_int); + conf->p2p_go_ctwindow = 0; + } else { + conf->p2p_go_ctwindow = wpa_s->conf->p2p_go_ctwindow; + } +#endif /* CONFIG_P2P */ + if ((bss->wpa & 2) && bss->rsn_pairwise == 0) bss->rsn_pairwise = bss->wpa_pairwise; bss->wpa_group = wpa_select_ap_group_cipher(bss->wpa, bss->wpa_pairwise, diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c index ea633a6..85d5a03 100644 --- a/wpa_supplicant/config.c +++ b/wpa_supplicant/config.c @@ -3503,6 +3503,7 @@ struct wpa_config * wpa_config_alloc_empty(const char *ctrl_interface, config->p2p_intra_bss = DEFAULT_P2P_INTRA_BSS; config->p2p_go_max_inactivity = DEFAULT_P2P_GO_MAX_INACTIVITY; config->p2p_optimize_listen_chan = DEFAULT_P2P_OPTIMIZE_LISTEN_CHAN; + config->p2p_go_ctwindow = DEFAULT_P2P_GO_CTWINDOW; config->bss_max_count = DEFAULT_BSS_MAX_COUNT; config->bss_expiration_age = DEFAULT_BSS_EXPIRATION_AGE; config->bss_expiration_scan_count = DEFAULT_BSS_EXPIRATION_SCAN_COUNT; @@ -4135,6 +4136,7 @@ static const struct global_parse_data global_fields[] = { { INT(p2p_go_ht40), 0 }, { INT(p2p_go_vht), 0 }, { INT(p2p_disabled), 0 }, + { INT_RANGE(p2p_go_ctwindow, 0, 127), 0 }, { INT(p2p_no_group_iface), 0 }, { INT_RANGE(p2p_ignore_shared_freq, 0, 1), 0 }, { IPV4(ip_addr_go), 0 }, diff --git a/wpa_supplicant/config.h b/wpa_supplicant/config.h index 2e54994..befcd91 100644 --- a/wpa_supplicant/config.h +++ b/wpa_supplicant/config.h @@ -33,6 +33,7 @@ #define DEFAULT_RAND_ADDR_LIFETIME 60 #define DEFAULT_KEY_MGMT_OFFLOAD 1 #define DEFAULT_CERT_IN_CB 1 +#define DEFAULT_P2P_GO_CTWINDOW 9 #include "config_ssid.h" #include "wps/wps.h" @@ -942,6 +943,13 @@ struct wpa_config { int p2p_go_vht; /** + * p2p_go_ctwindow - CTWindow to use when operating as GO + * + * By default: DEFAULT_P2P_GO_CTWINDOW + */ + int p2p_go_ctwindow; + + /** * p2p_disabled - Whether P2P operations are disabled for this interface */ int p2p_disabled; diff --git a/wpa_supplicant/config_file.c b/wpa_supplicant/config_file.c index cdc6e39..b6e74cc 100644 --- a/wpa_supplicant/config_file.c +++ b/wpa_supplicant/config_file.c @@ -1074,6 +1074,8 @@ static void wpa_config_write_global(FILE *f, struct wpa_config *config) fprintf(f, "p2p_go_ht40=%u\n", config->p2p_go_ht40); if (config->p2p_go_vht) fprintf(f, "p2p_go_vht=%u\n", config->p2p_go_vht); + if (config->p2p_go_ctwindow != DEFAULT_P2P_GO_CTWINDOW) + fprintf(f, "p2p_go_ctwindow=%u\n", config->p2p_go_ctwindow); if (config->p2p_disabled) fprintf(f, "p2p_disabled=%u\n", config->p2p_disabled); if (config->p2p_no_group_iface) diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c index 00b8d85..5e6646e 100644 --- a/wpa_supplicant/p2p_supplicant.c +++ b/wpa_supplicant/p2p_supplicant.c @@ -1852,6 +1852,7 @@ static void wpas_p2p_clone_config(struct wpa_supplicant *dst, d->ignore_old_scan_res = s->ignore_old_scan_res; d->beacon_int = s->beacon_int; d->dtim_period = s->dtim_period; + d->p2p_go_ctwindow = s->p2p_go_ctwindow; d->disassoc_low_ack = s->disassoc_low_ack; d->disable_scan_offload = s->disable_scan_offload; d->passive_scan = s->passive_scan;