| Message ID | 1367359813-10928-1-git-send-email-greearb@candelatech.com |
|---|---|
| State | Superseded |
| Headers | show |
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(¶ms.freqs, wpa_s->conf->freq_list) work here to replace both for loops and memory allocation?
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
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, ¶ms.num_filter_ssids); if (extra_ie) {