| Message ID | 200909091309.55068.hs4233@mail.mn-solutions.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, 2009-09-09 at 13:09 +0200, Holger Schurig wrote: > Handles the case when SIOCSIWSCAN specified iw_scan_req.num_channels and > iw_scan_req.channels[]. > + > + /* If we have a wireless request structure and the > + * wireless request specifies frequencies, then search > + * for the matching hardware channel. > + */ > + if (wreq && wreq->num_channels) { > + int k; > + int wiphy_freq = wiphy->bands[band]->channels[j].center_freq; > + for (k = 0; k < wreq->num_channels; k++) { > + int wext_freq = wreq->channel_list[k].m / 100000; > + if (wext_freq == wiphy_freq) > + goto wext_freq_found; > + } > + goto wext_freq_not_found; This is a bit weird -- this way you don't report errors if the user specified frequencies that don't exist. Also, are you sure that it has to be a frequency as opposed to a channel number? johannes
> This is a bit weird -- this way you don't report errors if the > user specified frequencies that don't exist. Couldn't / shouldn't this done in user-space, e.g. after checking IWRANGE. > Also, are you sure that it has to be a frequency as opposed to > a channel number? I'm not totally sure, because there's nowhere any documentation about the exact meanings of fields in "struct iw_freq". My patch for wpa_supplicant's driver_wext.c only put's a frequency there: + req.channel_list[i].m = params->freqs[i] * 100000; + req.channel_list[i].e = 1; + req.channel_list[i].i = i; + req.channel_list[i].flags = 0; But now that you bring this into my mind, I should have looked at another consumer of "struct iw_freq", e.g. I should use cfg80211_wext_freq() like cfg80211_wext_siwfreq() ?!? I'll prepare some patch.
> This is a bit weird -- this way you don't report errors if the > user specified frequencies that don't exist. The old code did this: Loop over all bands Loop over all channels Stick channel to scan request I simply added this: Loop over all bands Loop over all channels If scan-request hasn't this channel freq: continue Stick channel to scan request Now, if I want to report an -EINVAL for every possibly invalid scan-request channel, I'd have to do this: If scan-request has freqs: Loop over all scan-request freqs Loop over all bands Loop over all channels search for freq if found: Stick channel to scan request else: err = -EINVAL else: Loop over all bands Loop over all channels Stick channel to scan request This is considerable code-bloat for such a seldom-used function. I'd rather do it like this: Loop over all bands Loop over all channels If scan-request hasn't this channel freq: continue Stick channel to scan request if no channels: err = -EINVAL That's a compromise :-)
On Fri, 2009-09-11 at 09:52 +0200, Holger Schurig wrote: > Now, if I want to report an -EINVAL for every possibly invalid > scan-request channel, I'd have to do this: > > > If scan-request has freqs: > Loop over all scan-request freqs > Loop over all bands > Loop over all channels > search for freq > if found: > Stick channel to scan request > else: > err = -EINVAL > else: > Loop over all bands > Loop over all channels > Stick channel to scan request > > This is considerable code-bloat for such a seldom-used function. Doesn't seem that bad considering that the inner loop is already in an existing function. > I'd rather do it like this: > > > Loop over all bands > Loop over all channels > If scan-request hasn't this channel freq: continue > Stick channel to scan request > if no channels: > err = -EINVAL > > That's a compromise :-) I guess. I'd still prefer the other way, but it's wext, so I don't really care :) johannes
Index: linux-wl/net/wireless/scan.c =================================================================== --- linux-wl.orig/net/wireless/scan.c 2009-09-09 10:38:58.000000000 +0200 +++ linux-wl/net/wireless/scan.c 2009-09-09 11:59:39.000000000 +0200 @@ -607,6 +607,9 @@ int cfg80211_wext_siwscan(struct net_dev if (!netif_running(dev)) return -ENETDOWN; + if (wrqu->data.length == sizeof(struct iw_scan_req)) + wreq = (struct iw_scan_req *)extra; + rdev = cfg80211_get_dev_from_ifindex(dev_net(dev), dev->ifindex); if (IS_ERR(rdev)) @@ -619,9 +622,14 @@ int cfg80211_wext_siwscan(struct net_dev wiphy = &rdev->wiphy; - for (band = 0; band < IEEE80211_NUM_BANDS; band++) - if (wiphy->bands[band]) - n_channels += wiphy->bands[band]->n_channels; + /* Determine number of channels, needed to allocate creq */ + if (wreq && wreq->num_channels) + n_channels = wreq->num_channels; + else { + for (band = 0; band < IEEE80211_NUM_BANDS; band++) + if (wiphy->bands[band]) + n_channels += wiphy->bands[band]->n_channels; + } creq = kzalloc(sizeof(*creq) + sizeof(struct cfg80211_ssid) + n_channels * sizeof(void *), @@ -638,22 +646,41 @@ int cfg80211_wext_siwscan(struct net_dev creq->n_channels = n_channels; creq->n_ssids = 1; - /* all channels */ + /* translate "Scan on frequencies" request */ i = 0; for (band = 0; band < IEEE80211_NUM_BANDS; band++) { int j; if (!wiphy->bands[band]) continue; for (j = 0; j < wiphy->bands[band]->n_channels; j++) { + + /* If we have a wireless request structure and the + * wireless request specifies frequencies, then search + * for the matching hardware channel. + */ + if (wreq && wreq->num_channels) { + int k; + int wiphy_freq = wiphy->bands[band]->channels[j].center_freq; + for (k = 0; k < wreq->num_channels; k++) { + int wext_freq = wreq->channel_list[k].m / 100000; + if (wext_freq == wiphy_freq) + goto wext_freq_found; + } + goto wext_freq_not_found; + } + + wext_freq_found: creq->channels[i] = &wiphy->bands[band]->channels[j]; i++; + wext_freq_not_found: ; } } - /* translate scan request */ - if (wrqu->data.length == sizeof(struct iw_scan_req)) { - wreq = (struct iw_scan_req *)extra; + /* Set real number of channels specified in creq->channels[] */ + creq->n_channels = i; + /* translate "Scan for SSID" request */ + if (wreq) { if (wrqu->data.flags & IW_SCAN_THIS_ESSID) { if (wreq->essid_len > IEEE80211_MAX_SSID_LEN) return -EINVAL;
Handles the case when SIOCSIWSCAN specified iw_scan_req.num_channels and iw_scan_req.channels[]. Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de> -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html