| Message ID | 1338810072-31040-2-git-send-email-ordex@autistici.org |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, 2012-06-04 at 13:41 +0200, Antonio Quartulli wrote:
> + { INT_RANGE(fixed_freq, 0, 1) },
Fixed freq (and fixed BSSID) are kinda hacks and not really supported by
any other driver/device, I'm not sure they should be supported here?
johannes
On Mon, Jun 04, 2012 at 01:44:39PM +0200, Johannes Berg wrote: > On Mon, 2012-06-04 at 13:41 +0200, Antonio Quartulli wrote: > > > + { INT_RANGE(fixed_freq, 0, 1) }, > > Fixed freq (and fixed BSSID) are kinda hacks and not really supported by > any other driver/device, I'm not sure they should be supported here? mh..is there any way to specific "driver specific" options? Actually this param will be used (if set) by the nl80211 driver and ignored by all the others.. I don't know where I could move this param. Cheers,
On Mon, 2012-06-04 at 14:24 +0200, Antonio Quartulli wrote: > On Mon, Jun 04, 2012 at 01:44:39PM +0200, Johannes Berg wrote: > > On Mon, 2012-06-04 at 13:41 +0200, Antonio Quartulli wrote: > > > > > + { INT_RANGE(fixed_freq, 0, 1) }, > > > > Fixed freq (and fixed BSSID) are kinda hacks and not really supported by > > any other driver/device, I'm not sure they should be supported here? > > mh..is there any way to specific "driver specific" options? Actually this param > will be used (if set) by the nl80211 driver and ignored by all the others.. > > I don't know where I could move this param. For the most part, driver specific parameters are evil and shouldn't be used. What's fixed_freq again? Does that force the device onto a single frequency and disable IBSS merging? Perhaps instead of using "fixed_freq", which seems to overlap a lot with the existing 'frequency' config item, you could just have an option to disable IBSS merging if it's supported by the driver? It's obviously just a hint, since not all devices (even ones that support cfg80211) support disabling IBSS merging. Dan
Hello, On Tue, Jun 05, 2012 at 04:53:01PM -0500, Dan Williams wrote: > On Mon, 2012-06-04 at 14:24 +0200, Antonio Quartulli wrote: > > On Mon, Jun 04, 2012 at 01:44:39PM +0200, Johannes Berg wrote: > > > On Mon, 2012-06-04 at 13:41 +0200, Antonio Quartulli wrote: > > > > > > > + { INT_RANGE(fixed_freq, 0, 1) }, > > > > > > Fixed freq (and fixed BSSID) are kinda hacks and not really supported by > > > any other driver/device, I'm not sure they should be supported here? > > > > mh..is there any way to specific "driver specific" options? Actually this param > > will be used (if set) by the nl80211 driver and ignored by all the others.. > > > > I don't know where I could move this param. > > For the most part, driver specific parameters are evil and shouldn't be > used. I think that most of the params I added are supported by other drivers as well (not 100% sure). We just need to add the support for them in each driver-related code. > What's fixed_freq again? Does that force the device onto a > single frequency and disable IBSS merging? Perhaps instead of using > "fixed_freq", which seems to overlap a lot with the existing 'frequency' > config item, you could just have an option to disable IBSS merging if > it's supported by the driver? It's obviously just a hint, since not all > devices (even ones that support cfg80211) support disabling IBSS > merging. > As far as I can tell, frequency is the initial value where the device will look for other IBSS. Fixed-freq will force the driver to get stuck on that frequency, even if there is another IBSS on another channel. Johannes can probably confirm or disprove what I said... Actually I'm not "adding a new behaviour" to wpa_s, I'm simply allowing the use to configure all the parameters that the ibss join command support. Whether the There are many people out there that use the "iw" command to join an IBSS with the freq and fixed_freq params set. What I'm doing here is allowing these people to configure the same params when using wpa_s. Cheers,
On Wed, Jun 06, 2012 at 08:39:40AM +0200, Antonio Quartulli wrote: > I think that most of the params I added are supported by other drivers as well > (not 100% sure). > We just need to add the support for them in each driver-related code. Well, I would first like to get some justification for this functionality in the first place. This seems to be something that is not compliant with the way IBSS was designed work. As such, there better be some notes on the commit log and wpa_supplicant.conf describing what is the use case for this and warnings on this breaking IBSS compatibility due to missing merges with other parts of the same IBSS with standard compliant STAs. > Actually I'm not "adding a new behaviour" to wpa_s, I'm simply allowing the use > to configure all the parameters that the ibss join command support. This is new functionality from wpa_supplicant view point and needs to be documented in wpa_supplicant.conf. Does the multicast rate really need to be set in wpa_supplicant? If someone needs that special configuration, I would assume that iw can be used after the IBSS has been enabled. By the way, you cannot include drivers/nl80211_copy.h into wpa_supplicant/config_ssid.h since config_ssid.h needs to remain independent of the driver interface. I would suggest reordering the patches in a way that you add the parameters one by one and include all the related changes (driver_nl80211.c, core changes, wpa_supplicant.conf documentation) in each patch. While I don't really in general like the non-compliant IBSS operations, this would make it somewhat more likely that you can convince me to take the fixed_freq and beacon_int (please rename that to beacon_int to match with hostapd and also use it for AP mode configuration instead of just IBSS).
On Wed, Jun 06, 2012 at 02:56:32PM +0300, Jouni Malinen wrote: > Does the multicast rate really need to be set in wpa_supplicant? If > someone needs that special configuration, I would assume that iw can be > used after the IBSS has been enabled. That's the problem. nl80211 doesn't provide any way to modify such params after the ibss has been established. > > By the way, you cannot include drivers/nl80211_copy.h into > wpa_supplicant/config_ssid.h since config_ssid.h needs to remain > independent of the driver interface. ok > > I would suggest reordering the patches in a way that you add the > parameters one by one and include all the related changes > (driver_nl80211.c, core changes, wpa_supplicant.conf documentation) in > each patch. While I don't really in general like the non-compliant IBSS > operations, this would make it somewhat more likely that you can > convince me to take the fixed_freq and beacon_int (please rename that to > beacon_int to match with hostapd and also use it for AP mode > configuration instead of just IBSS). > ok, will do! :) Thanks a lot, Antonio
On Wed, Jun 06, 2012 at 03:11:30PM +0200, Antonio Quartulli wrote: > On Wed, Jun 06, 2012 at 02:56:32PM +0300, Jouni Malinen wrote: > > Does the multicast rate really need to be set in wpa_supplicant? If > > someone needs that special configuration, I would assume that iw can be > > used after the IBSS has been enabled. > > That's the problem. nl80211 doesn't provide any way to modify such params after > the ibss has been established. Ah.. I assumed that that was the case only for parameters than cannot change during IBSS lifetime (e.g., set of supported rates), but it shouldn't be limitation for multicast rate. It looks like the nl80211 interface is not really ideal for this particular parameter.
On Wed, Jun 06, 2012 at 04:52:44PM +0300, Jouni Malinen wrote: > On Wed, Jun 06, 2012 at 03:11:30PM +0200, Antonio Quartulli wrote: > > On Wed, Jun 06, 2012 at 02:56:32PM +0300, Jouni Malinen wrote: > > > Does the multicast rate really need to be set in wpa_supplicant? If > > > someone needs that special configuration, I would assume that iw can be > > > used after the IBSS has been enabled. > > > > That's the problem. nl80211 doesn't provide any way to modify such params after > > the ibss has been established. > > Ah.. I assumed that that was the case only for parameters than cannot > change during IBSS lifetime (e.g., set of supported rates), but it > shouldn't be limitation for multicast rate. It looks like the nl80211 > interface is not really ideal for this particular parameter. I agree...I think that the interface for the IBSS handling in nl80211 is missing something (I think IBSS is not the major objective of most developers out there). However, since the IBSS join command supports such params, why should wpa_s continue to be not able to specify them? Cheers,
On Wed, Jun 06, 2012 at 04:54:07PM +0200, Antonio Quartulli wrote: > However, since the IBSS join command supports such params, why should wpa_s > continue to be not able to specify them? Having pointless parameters available is not a good justification for adding new code to increase complexity and size of wpa_supplicant.. That said, if there is reasonable use case for parameters that have to be set at the time of IBSS join command, it should be doable to come up with such justification.
diff --git a/src/drivers/driver.h b/src/drivers/driver.h index f7fb2ef..dda2fbc 100644 --- a/src/drivers/driver.h +++ b/src/drivers/driver.h @@ -19,6 +19,7 @@ #define WPA_SUPPLICANT_DRIVER_VERSION 4 +#include "drivers/nl80211_copy.h" #include "common/defs.h" #define HOSTAPD_CHAN_DISABLED 0x00000001 @@ -332,6 +333,11 @@ struct wpa_driver_associate_params { */ int freq; + int beacon_interval; + int fixed_freq; + unsigned char rates[NL80211_MAX_SUPP_RATES]; + int mcast_rate; + /** * bg_scan_period - Background scan period in seconds, 0 to disable * background scan, or -1 to indicate no change to default driver diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c index ce763b3..3d6c0e6 100644 --- a/wpa_supplicant/config.c +++ b/wpa_supplicant/config.c @@ -14,6 +14,7 @@ #include "rsn_supp/wpa.h" #include "eap_peer/eap.h" #include "p2p/p2p.h" +#include "drivers/nl80211_copy.h" #include "config.h" @@ -1436,6 +1437,97 @@ static char * wpa_config_write_p2p_client_list(const struct parse_data *data, #endif /* CONFIG_P2P */ +static int wpa_config_parse_mcast_rate(const struct parse_data *data, + struct wpa_ssid *ssid, int line, + const char *value) +{ + ssid->mcast_rate = (int)(strtod(value, NULL) * 10); + + return 0; +} + +#ifndef NO_CONFIG_WRITE +static char * wpa_config_write_mcast_rate(const struct parse_data *data, + struct wpa_ssid *ssid) +{ + char *value; + int res; + + if (!ssid->mcast_rate == 0) + return NULL; + + value = os_malloc(6); /* longest: 300.0 */ + if (value == NULL) + return NULL; + res = os_snprintf(value, 5, "%.1f", (double)ssid->mcast_rate / 10); + if (res < 0) { + os_free(value); + return NULL; + } + return value; +} +#endif /* NO_CONFIG_WRITE */ + +static int wpa_config_parse_rates(const struct parse_data *data, + struct wpa_ssid *ssid, int line, + const char *value) +{ + int i; + char *pos, *r, *sptr, *end; + double rate; + + pos = (char *)value; + r = strtok_r(pos, ",", &sptr); + i = 0; + while (pos && i < NL80211_MAX_SUPP_RATES) { + rate = 0.0; + if (r) + rate = strtod(r, &end); + ssid->rates[i] = rate * 2; + if (*end != '\0' || rate * 2 != ssid->rates[i]) + return 1; + + i++; + r = strtok_r(NULL, ",", &sptr); + } + + return 0; +} + +#ifndef NO_CONFIG_WRITE +static char * wpa_config_write_rates(const struct parse_data *data, + struct wpa_ssid *ssid) +{ + char *value, *pos; + int res, i; + + if (ssid->rates[0] <= 0) + return NULL; + + value = os_malloc(6 * NL80211_MAX_SUPP_RATES + 1); + if (value == NULL) + return NULL; + pos = value; + for (i = 0; i < NL80211_MAX_SUPP_RATES - 1; i++) { + res = os_snprintf(pos, 6, "%.1f,", (double)ssid->rates[i] / 2); + if (res < 0) { + os_free(value); + return NULL; + } + pos += res; + } + res = os_snprintf(pos, 6, "%.1f", + (double)ssid->rates[NL80211_MAX_SUPP_RATES - 1] / 2); + if (res < 0) { + os_free(value); + return NULL; + } + + value[6 * NL80211_MAX_SUPP_RATES] = '\0'; + return value; +} +#endif /* NO_CONFIG_WRITE */ + /* Helper macros for network block parser */ #ifdef OFFSET @@ -1610,6 +1702,10 @@ static const struct parse_data ssid_fields[] = { { STR(ht_mcs) }, #endif /* CONFIG_HT_OVERRIDES */ { INT(ap_max_inactivity) }, + { INT_RANGE(fixed_freq, 0, 1) }, + { INT_RANGE(beacon_interval, 0, 1000) }, + { FUNC(rates) }, + { FUNC(mcast_rate) }, }; #undef OFFSET diff --git a/wpa_supplicant/config_ssid.h b/wpa_supplicant/config_ssid.h index 80d4382..8d152a4 100644 --- a/wpa_supplicant/config_ssid.h +++ b/wpa_supplicant/config_ssid.h @@ -11,6 +11,7 @@ #include "common/defs.h" #include "eap_peer/eap_config.h" +#include "drivers/nl80211_copy.h" #define MAX_SSID_LEN 32 @@ -499,6 +500,11 @@ struct wpa_ssid { * By default: 300 seconds. */ int ap_max_inactivity; + + int fixed_freq; + int beacon_interval; + unsigned char rates[NL80211_MAX_SUPP_RATES]; + double mcast_rate; }; #endif /* CONFIG_SSID_H */ diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c index 3cb954d..59efa16 100644 --- a/wpa_supplicant/wpa_supplicant.c +++ b/wpa_supplicant/wpa_supplicant.c @@ -1363,15 +1363,24 @@ void wpa_supplicant_associate(struct wpa_supplicant *wpa_s, params.ssid_len = ssid->ssid_len; } - if (ssid->mode == WPAS_MODE_IBSS && ssid->bssid_set && - wpa_s->conf->ap_scan == 2) { - params.bssid = ssid->bssid; - params.fixed_bssid = 1; + if (ssid->mode == WPAS_MODE_IBSS) { + if (ssid->bssid_set && wpa_s->conf->ap_scan == 2) { + params.bssid = ssid->bssid; + params.fixed_bssid = 1; + } + if (ssid->frequency > 0 && params.freq == 0) + /* Initial channel for IBSS */ + params.freq = ssid->frequency; + params.fixed_freq = ssid->fixed_freq; + params.beacon_interval = ssid->beacon_interval; + i = 0; + while (i < NL80211_MAX_SUPP_RATES) { + params.rates[i] = ssid->rates[i]; + i++; + } + params.mcast_rate = ssid->mcast_rate; } - if (ssid->mode == WPAS_MODE_IBSS && ssid->frequency > 0 && - params.freq == 0) - params.freq = ssid->frequency; /* Initial channel for IBSS */ params.wpa_ie = wpa_ie; params.wpa_ie_len = wpa_ie_len; params.pairwise_suite = cipher_pairwise;