Patchwork wpa_supplicant: Allow configuring scan frequencies.

login
register
mail settings
Submitter Ben Greear
Date April 30, 2013, 10:10 p.m.
Message ID <1367359813-10928-1-git-send-email-greearb@candelatech.com>
Download mbox | patch
Permalink /patch/240700/
State Superseded
Headers show

Comments

Ben Greear - April 30, 2013, 10:10 p.m.
From: Ben Greear <greearb@candelatech.com>

This allows one to limit the channels that wpa_supplicant will
scan.  This is a useful addition to the freq_list configurable
in the network {} section.

Signed-hostap: Ben Greear <greearb@candelatech.com>
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 638cf40... a1f5010... M	wpa_supplicant/config.c
:100644 100644 2f49e1e... 0c7c477... M	wpa_supplicant/config.h
:100644 100644 f409e98... 51f34df... M	wpa_supplicant/scan.c
 wpa_supplicant/config.c |   15 +++++++++++++++
 wpa_supplicant/config.h |    9 +++++++++
 wpa_supplicant/scan.c   |   14 ++++++++++++++
 3 files changed, 38 insertions(+), 0 deletions(-)
Jouni Malinen - April 30, 2013, 10:48 p.m.
On Tue, Apr 30, 2013 at 03:10:13PM -0700, greearb@candelatech.com wrote:
> This allows one to limit the channels that wpa_supplicant will
> scan.  This is a useful addition to the freq_list configurable
> in the network {} section.

Could you please add an example to wpa_supplicant.conf with some
documentation?

> Signed-hostap: Ben Greear <greearb@candelatech.com>
> Signed-off-by: Ben Greear <greearb@candelatech.com>

Not that the extra line really causes any problems here, but just to
note that I will drop the second one.

> diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
> +static int wpa_config_process_freq_list(const struct parse_data *data,

> +	freqs = wpa_config_parse_int_array(value);
> +	if (freqs == NULL)
> +		return -1;
> +	os_free(config->freq_list);
> +	config->freq_list = freqs;
> +	return 0;
> +}

Where does this config->freq_list get freed when the interface is
removed?

> diff --git a/wpa_supplicant/config.h b/wpa_supplicant/config.h
> @@ -648,6 +648,15 @@ struct wpa_config {
>  	unsigned int max_num_sta;
>  
>  	/**
> +	 * freq_list - Array of allowed scan frequencies or %NULL for all
> +	 *
> +	 * This is an optional zero-terminated array of frequencies in
> +	 * megahertz (MHz) to allow for narrowing scanning range.
> +	 * TODO:  Could make it limit ssid freq_lists as well.
> +	 */
> +	int *freq_list;

Could you please clarify what that TODO item means? I have a slight
preference of not included it there as part of wpa_config documentation.

> diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
> @@ -750,6 +750,20 @@ ssid_list_set:
> +	/* See if user specified frequencies..if so, scan only those */
> +	if ((!params.freqs) && wpa_s->conf->freq_list) {

No need for the extra parenthesis around !freqs.

> +		for (i = 0; wpa_s->conf->freq_list[i]; i++) { }

Hmm.. This style is not used elsewhere in hostap.git, I think and I
cannot really say that I would be a fan of that "{ }" on the same line
with a for loop.. Anyway, this should use int_array_len().

> +		params.freqs = os_zalloc(sizeof(int) * (i + 1));
> +		for (i = 0; wpa_s->conf->freq_list[i]; i++)
> +			params.freqs[i] = wpa_s->conf->freq_list[i];

os_zalloc() may return NULL..

Wouldn't int_array_concat(&params.freqs, wpa_s->conf->freq_list) work
here to replace both for loops and memory allocation?
Ben Greear - April 30, 2013, 11:02 p.m.
On 04/30/2013 03:48 PM, Jouni Malinen wrote:
> On Tue, Apr 30, 2013 at 03:10:13PM -0700, greearb@candelatech.com wrote:
>> This allows one to limit the channels that wpa_supplicant will
>> scan.  This is a useful addition to the freq_list configurable
>> in the network {} section.
>
> Could you please add an example to wpa_supplicant.conf with some
> documentation?
>
>> Signed-hostap: Ben Greear <greearb@candelatech.com>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>
> Not that the extra line really causes any problems here, but just to
> note that I will drop the second one.
>
>> diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
>> +static int wpa_config_process_freq_list(const struct parse_data *data,
>
>> +	freqs = wpa_config_parse_int_array(value);
>> +	if (freqs == NULL)
>> +		return -1;
>> +	os_free(config->freq_list);
>> +	config->freq_list = freqs;
>> +	return 0;
>> +}
>
> Where does this config->freq_list get freed when the interface is
> removed?
>
>> diff --git a/wpa_supplicant/config.h b/wpa_supplicant/config.h
>> @@ -648,6 +648,15 @@ struct wpa_config {
>>   	unsigned int max_num_sta;
>>
>>   	/**
>> +	 * freq_list - Array of allowed scan frequencies or %NULL for all
>> +	 *
>> +	 * This is an optional zero-terminated array of frequencies in
>> +	 * megahertz (MHz) to allow for narrowing scanning range.
>> +	 * TODO:  Could make it limit ssid freq_lists as well.
>> +	 */
>> +	int *freq_list;
>
> Could you please clarify what that TODO item means? I have a slight
> preference of not included it there as part of wpa_config documentation.

I think that the wpa_s->conf->freq_list could force all network
blocks to use use a subset of that freq_list (including the full
freq_list).

I'm not sure if it's worth coding up though, since users can just
manually set the freq_list in the network block(s).

I'll remove the TODO.

I'll work on the rest of the comments and re-post shortly.

Thanks,
Ben

Patch

diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
index 638cf40..a1f5010 100644
--- a/wpa_supplicant/config.c
+++ b/wpa_supplicant/config.c
@@ -897,6 +897,20 @@  static int wpa_config_parse_freq_list(const struct parse_data *data,
 	return 0;
 }
 
+static int wpa_config_process_freq_list(const struct parse_data *data,
+					struct wpa_config *config, int line,
+					const char *value)
+{
+	int *freqs;
+
+	freqs = wpa_config_parse_int_array(value);
+	if (freqs == NULL)
+		return -1;
+	os_free(config->freq_list);
+	config->freq_list = freqs;
+	return 0;
+}
+
 
 #ifndef NO_CONFIG_WRITE
 static char * wpa_config_write_freqs(const struct parse_data *data,
@@ -3026,6 +3040,7 @@  static const struct global_parse_data global_fields[] = {
 	{ STR(pcsc_reader), 0 },
 	{ STR(pcsc_pin), 0 },
 	{ STR(driver_param), 0 },
+	{ FUNC(freq_list), 0 },
 	{ INT(dot11RSNAConfigPMKLifetime), 0 },
 	{ INT(dot11RSNAConfigPMKReauthThreshold), 0 },
 	{ INT(dot11RSNAConfigSATimeout), 0 },
diff --git a/wpa_supplicant/config.h b/wpa_supplicant/config.h
index 2f49e1e..0c7c477 100644
--- a/wpa_supplicant/config.h
+++ b/wpa_supplicant/config.h
@@ -648,6 +648,15 @@  struct wpa_config {
 	unsigned int max_num_sta;
 
 	/**
+	 * freq_list - Array of allowed scan frequencies or %NULL for all
+	 *
+	 * This is an optional zero-terminated array of frequencies in
+	 * megahertz (MHz) to allow for narrowing scanning range.
+	 * TODO:  Could make it limit ssid freq_lists as well.
+	 */
+	int *freq_list;
+
+	/**
 	 * changed_parameters - Bitmap of changed parameters since last update
 	 */
 	unsigned int changed_parameters;
diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
index f409e98..51f34df 100644
--- a/wpa_supplicant/scan.c
+++ b/wpa_supplicant/scan.c
@@ -750,6 +750,20 @@  ssid_list_set:
 		os_free(wpa_s->next_scan_freqs);
 	wpa_s->next_scan_freqs = NULL;
 
+	/* See if user specified frequencies..if so, scan only those */
+	if ((!params.freqs) && wpa_s->conf->freq_list) {
+		int cnt = 0;
+		int i;
+
+		wpa_dbg(wpa_s, MSG_DEBUG,
+			"Optimize scan based on conf->freq_list");
+		for (i = 0; wpa_s->conf->freq_list[i]; i++) { }
+		
+		params.freqs = os_zalloc(sizeof(int) * (i + 1));
+		for (i = 0; wpa_s->conf->freq_list[i]; i++)
+			params.freqs[i] = wpa_s->conf->freq_list[i];
+	}
+
 	params.filter_ssids = wpa_supplicant_build_filter_ssids(
 		wpa_s->conf, &params.num_filter_ssids);
 	if (extra_ie) {