| Message ID | 1348772354-15936-2-git-send-email-bzhao@marvell.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote: > From: Sam Leffler <sleffler@chromium.org> > > Add a flags word to direct and scheduled scan requests; it will > be used for control of optional behaviours such as flushing the > bss cache prior to doing a scan. Why for scheduled scan as well? > + * @NL80211_ATTR_SCAN_FLAGS: scan request control flags (u32) One thing that might be useful is to advertise which flags are even supported at all by a driver, if we add different ones? We might then ignore the flags that we don't support anyway, but at least userspace would know that it can't expect flushing (for example) on an older kernel version and might have to use some workarounds or whatever. > +/** > + * enum nl80211_scan_flags - scan request control flags > + * > + * Scan request control flags are used to control the handling > + * of NL80211_CMD_TRIGGER_SCAN and NL80211_CMD_START_SCHED_SCAN > + * requests. > + */ > +enum nl80211_scan_flags { > +}; > + > /** > + * enum cfg80211_scan_flags - scan request control flags > + */ > +enum cfg80211_scan_flags { > +}; That doesn't make a lot of sense? A single enum seems sufficient? > + nla_put_u32(msg, NL80211_ATTR_SCAN_FLAGS, req->flags); Missing error check, also, is there nothing that re-publishes information about scheduled scans? johannes -- 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
On Fri, Sep 28, 2012 at 4:01 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote: >> From: Sam Leffler <sleffler@chromium.org> >> >> Add a flags word to direct and scheduled scan requests; it will >> be used for control of optional behaviours such as flushing the >> bss cache prior to doing a scan. > > Why for scheduled scan as well? Because I thought the scheduled scan mechanism is used to trigger periodic scans w/o involving user space and so might be used to do bgscan's. Please enlighten me. > >> + * @NL80211_ATTR_SCAN_FLAGS: scan request control flags (u32) > > One thing that might be useful is to advertise which flags are even > supported at all by a driver, if we add different ones? We might then > ignore the flags that we don't support anyway, but at least userspace > would know that it can't expect flushing (for example) on an older > kernel version and might have to use some workarounds or whatever. Yes I considered this. I don't know what the model is for nl80211 in this regard (think we've talked about this in the past). The low priority scan flag is somewhat advisory so applications should be able to deal w/ it being ignored. The flush flag however may be cause some misbehaviour--e.g. you wakeup, scan, and then act on stale results that may cause you try and associate to an ap that's out of range in which case you'll end up doing the equivalent to the flush in user space to make sure the right thing happens. > >> +/** >> + * enum nl80211_scan_flags - scan request control flags >> + * >> + * Scan request control flags are used to control the handling >> + * of NL80211_CMD_TRIGGER_SCAN and NL80211_CMD_START_SCHED_SCAN >> + * requests. >> + */ >> +enum nl80211_scan_flags { >> +}; >> + > >> /** >> + * enum cfg80211_scan_flags - scan request control flags >> + */ >> +enum cfg80211_scan_flags { >> +}; > > That doesn't make a lot of sense? A single enum seems sufficient? I thought cfg80211's name space was separate from nl80211's? Was just trying to follow what I found elsewhere... > >> + nla_put_u32(msg, NL80211_ATTR_SCAN_FLAGS, req->flags); > > Missing error check, also, is there nothing that re-publishes > information about scheduled scans? > > johannes > > -- > 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 -- 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
On Fri, 2012-09-28 at 13:36 -0700, Sam Leffler wrote: > On Fri, Sep 28, 2012 at 4:01 AM, Johannes Berg > <johannes@sipsolutions.net> wrote: > > On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote: > >> From: Sam Leffler <sleffler@chromium.org> > >> > >> Add a flags word to direct and scheduled scan requests; it will > >> be used for control of optional behaviours such as flushing the > >> bss cache prior to doing a scan. > > > > Why for scheduled scan as well? > > Because I thought the scheduled scan mechanism is used to trigger > periodic scans w/o involving user space and so might be used to do > bgscan's. Please enlighten me. It could be used for that, but I don't see any handling for scheduled scans in this code. And this also goes into the question about advertising the features, since scheduled scan is always implemented by the driver -- there's little value in implementing it in mac80211 since then it's software anyway and wpa_s can just do the triggering. Then the question will be: does the driver support it or not, and should we advertise whether it's supported or not. Also for scheduled scan, I'm not sure how the flushing would work? You don't seem to have implemented that. > >> + * @NL80211_ATTR_SCAN_FLAGS: scan request control flags (u32) > > > > One thing that might be useful is to advertise which flags are even > > supported at all by a driver, if we add different ones? We might then > > ignore the flags that we don't support anyway, but at least userspace > > would know that it can't expect flushing (for example) on an older > > kernel version and might have to use some workarounds or whatever. > > Yes I considered this. I don't know what the model is for nl80211 in > this regard (think we've talked about this in the past). The low > priority scan flag is somewhat advisory so applications should be able > to deal w/ it being ignored. The flush flag however may be cause some > misbehaviour--e.g. you wakeup, scan, and then act on stale results > that may cause you try and associate to an ap that's out of range in > which case you'll end up doing the equivalent to the flush in user > space to make sure the right thing happens. I'd say there are different possible implementations here. One way would be to advertise the nl80211 scan flags attribute with the device information and set all the supported bits, at least the ones that are important like the flush one, and document the advisory ones as such. For other more generic features we'd probably use nl80211_feature_flags, but here I'd suggest the above, putting the attribute into the device information with the supported bits set, which today would be only the flush one which is handled in cfg80211 so we don't (yet) need driver information for it if low-prio is advisory. There's one complication though with scheduled scan, since that'd require a different feature attribute unless we want to mandate that all flags must always be supported in both or neither, which I'm not sure is realistic. How if "flush" supposed to behave in scheduled scan anyway? > >> +/** > >> + * enum nl80211_scan_flags - scan request control flags > >> + * enum cfg80211_scan_flags - scan request control flags > > That doesn't make a lot of sense? A single enum seems sufficient? > > I thought cfg80211's name space was separate from nl80211's? Was just > trying to follow what I found elsewhere... Yeah there are a few (old) examples of this (e.g. band enum), but they aren't really separate any more, we have the namespaces more like this (in maths/latex notation): nl80211 $\subset$ cfg80211 $\subset$ mac80211 At some point maybe I'll clean up the old examples. I think there might also be a few where the cfg80211 value is BIT() of the nl80211 value, but I'm not really sure. johannes -- 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
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h index 7df9b50..109f75d 100644 --- a/include/linux/nl80211.h +++ b/include/linux/nl80211.h @@ -1273,6 +1273,8 @@ enum nl80211_commands { * the connection request from a station. nl80211_connect_failed_reason * enum has different reasons of connection failure. * + * @NL80211_ATTR_SCAN_FLAGS: scan request control flags (u32) + * * @NL80211_ATTR_MAX: highest attribute number currently defined * @__NL80211_ATTR_AFTER_LAST: internal use */ @@ -1530,6 +1532,8 @@ enum nl80211_attrs { NL80211_ATTR_CONN_FAILED_REASON, + NL80211_ATTR_SCAN_FLAGS, + /* add attributes here, update the policy in nl80211.c */ __NL80211_ATTR_AFTER_LAST, @@ -3069,4 +3073,14 @@ enum nl80211_connect_failed_reason { NL80211_CONN_FAIL_BLOCKED_CLIENT, }; +/** + * enum nl80211_scan_flags - scan request control flags + * + * Scan request control flags are used to control the handling + * of NL80211_CMD_TRIGGER_SCAN and NL80211_CMD_START_SCHED_SCAN + * requests. + */ +enum nl80211_scan_flags { +}; + #endif /* __LINUX_NL80211_H */ diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index ab78b53..2c0c14a 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -992,6 +992,12 @@ struct cfg80211_ssid { }; /** + * enum cfg80211_scan_flags - scan request control flags + */ +enum cfg80211_scan_flags { +}; + +/** * struct cfg80211_scan_request - scan request description * * @ssids: SSIDs to scan for (active scan only) @@ -1000,6 +1006,7 @@ struct cfg80211_ssid { * @n_channels: total number of channels to scan * @ie: optional information element(s) to add into Probe Request or %NULL * @ie_len: length of ie in octets + * @flags: bit field of flags controlling operation * @rates: bitmap of rates to advertise for each band * @wiphy: the wiphy this was for * @wdev: the wireless device to scan for @@ -1012,6 +1019,7 @@ struct cfg80211_scan_request { u32 n_channels; const u8 *ie; size_t ie_len; + u32 flags; u32 rates[IEEE80211_NUM_BANDS]; @@ -1044,6 +1052,7 @@ struct cfg80211_match_set { * @interval: interval between each scheduled scan cycle * @ie: optional information element(s) to add into Probe Request or %NULL * @ie_len: length of ie in octets + * @flags: bit field of flags controlling operation * @match_sets: sets of parameters to be matched for a scan result * entry to be considered valid and to be passed to the host * (others are filtered out). @@ -1061,6 +1070,7 @@ struct cfg80211_sched_scan_request { u32 interval; const u8 *ie; size_t ie_len; + u32 flags; struct cfg80211_match_set *match_sets; int n_match_sets; s32 rssi_thold; diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index f1047ae..e4312f6 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -355,6 +355,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = { [NL80211_ATTR_BG_SCAN_PERIOD] = { .type = NLA_U16 }, [NL80211_ATTR_WDEV] = { .type = NLA_U64 }, [NL80211_ATTR_USER_REG_HINT_TYPE] = { .type = NLA_U32 }, + [NL80211_ATTR_SCAN_FLAGS] = { .type = NLA_U32 }, }; /* policy for the key attributes */ @@ -4337,6 +4338,10 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info) } } + if (info->attrs[NL80211_ATTR_SCAN_FLAGS]) + request->flags = nla_get_u32( + info->attrs[NL80211_ATTR_SCAN_FLAGS]); + request->no_cck = nla_get_flag(info->attrs[NL80211_ATTR_TX_NO_CCK_RATE]); @@ -4568,6 +4573,10 @@ static int nl80211_start_sched_scan(struct sk_buff *skb, request->ie_len); } + if (info->attrs[NL80211_ATTR_SCAN_FLAGS]) + request->flags = nla_get_u32( + info->attrs[NL80211_ATTR_SCAN_FLAGS]); + request->dev = dev; request->wiphy = &rdev->wiphy; request->interval = interval; @@ -7622,6 +7631,8 @@ static int nl80211_add_scan_req(struct sk_buff *msg, nla_put(msg, NL80211_ATTR_IE, req->ie_len, req->ie)) goto nla_put_failure; + nla_put_u32(msg, NL80211_ATTR_SCAN_FLAGS, req->flags); + return 0; nla_put_failure: return -ENOBUFS;