[PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Luciano Coelho]

Introduce filtering for scheduled scans to reduce the number of
unnecessary results (which cause useless wake-ups).

This commit adds a new nested attribute for passing generic filters in
scheduled scan requests.  It also adds an implementation for filtering
based on a list of SSIDs.  In the future, more types of filters will
be added.

Signed-off-by: Luciano Coelho <coelho@ti.com>
---
 include/linux/nl80211.h |   33 ++++++++++++++++++++++++++++++++
 include/net/cfg80211.h  |    8 +++++++
 net/wireless/nl80211.c  |   48 +++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 3769303..7d4edb8 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -769,6 +769,8 @@ enum nl80211_commands {
  *	that can be added to a scan request
  * @NL80211_ATTR_MAX_SCHED_SCAN_IE_LEN: maximum length of information
  *	elements that can be added to a scheduled scan request
+ * @NL80211_ATTR_MAX_FILTER_SSIDS: number of SSIDs you can use as
+ *	filters in a scheduled scan request, a wiphy attribute.
  *
  * @NL80211_ATTR_SCAN_FREQUENCIES: nested attribute with frequencies (in MHz)
  * @NL80211_ATTR_SCAN_SSIDS: nested attribute with SSIDs, leave out for passive
@@ -1011,6 +1013,13 @@ enum nl80211_commands {
 
  * @NL80211_ATTR_SCHED_SCAN_INTERVAL: Interval between scheduled scan
  *	cycles, in msecs.
+ * @NL80211_ATTR_SCHED_SCAN_FILTER: Nested attribute with various
+ *	types of filtering to be used with scheduled scans.
+ *	If @NL80211_ATTR_SCAN_SSIDS is passed with values that are not
+ *	included in the filter, the driver may return -EINVAL, since
+ *	it doesn't make sense to send probe requests with SSIDs that
+ *	will be filtered out.  This doesn't apply to the wildcard SSID.
+ *	If ommited, no filtering is done.
  *
  * @NL80211_ATTR_INTERFACE_COMBINATIONS: Nested attribute listing the supported
  *	interface combinations. In each nested item, it contains attributes
@@ -1252,6 +1261,9 @@ enum nl80211_attrs {
 	NL80211_ATTR_IE_PROBE_RESP,
 	NL80211_ATTR_IE_ASSOC_RESP,
 
+	NL80211_ATTR_SCHED_SCAN_FILTER,
+	NL80211_ATTR_MAX_FILTER_SSIDS,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -1711,6 +1723,27 @@ enum nl80211_reg_rule_attr {
 };
 
 /**
+ * enum nl80211_sched_scan_filter_attr - scheduled scan filter attributes
+ * @__NL80211_SCHED_SCAN_FILTER_ATTR_INVALID: attribute number 0 is reserved
+ * @NL80211_SCHED_SCAN_FILTER_SSID: A list of SSIDs to be automatically
+ *	filtered by the hardware and passed up to the host.  SSIDs that are
+ *	not present in this list are ignored (without waking up the host).
+ * @NL80211_SCHED_SCAN_FILTER_ATTR_MAX: highest scheduled scan filter
+ *	attribute number currently defined
+ * @__NL80211_SCHED_SCAN_FILTER_ATTR_AFTER_LAST: internal use
+ */
+enum nl80211_sched_scan_filter_attr {
+	__NL80211_SCHED_SCAN_FILTER_ATTR_INVALID,
+
+	NL80211_ATTR_SCHED_SCAN_FILTER_SSID,
+
+	/* keep last */
+	__NL80211_SCHED_SCAN_FILTER_ATTR_AFTER_LAST,
+	NL80211_SCHED_SCAN_FILTER_ATTR_MAX =
+		__NL80211_SCHED_SCAN_FILTER_ATTR_AFTER_LAST - 1
+};
+
+/**
  * enum nl80211_reg_rule_flags - regulatory rule flags
  *
  * @NL80211_RRF_NO_OFDM: OFDM modulation not allowed
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a37f264..a3e58ff 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -882,6 +882,9 @@ struct cfg80211_scan_request {
  * @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
+ * @filter_ssids: SSIDs to pass to the host (others are filtered out).
+ * 	If ommited, no filtering is done.
+ * @n_filter_ssids: number of filter SSIDs
  * @wiphy: the wiphy this was for
  * @dev: the interface
  * @channels: channels to scan
@@ -893,6 +896,8 @@ struct cfg80211_sched_scan_request {
 	u32 interval;
 	const u8 *ie;
 	size_t ie_len;
+	struct cfg80211_ssid *filter_ssids;
+	int n_filter_ssids;
 
 	/* internal */
 	struct wiphy *wiphy;
@@ -1806,6 +1811,8 @@ struct wiphy_wowlan_support {
  *	any given scan
  * @max_sched_scan_ssids: maximum number of SSIDs the device can scan
  *	for in any given scheduled scan
+ * @max_filter_ssids: maximum number of SSIDs the device can filter when
+ *	performing a scheduled scan, 0 if filtering is not supported
  * @max_scan_ie_len: maximum length of user-controlled IEs device can
  *	add to probe request frames transmitted during a scan, must not
  *	include fixed IEs like supported rates
@@ -1863,6 +1870,7 @@ struct wiphy {
 	int bss_priv_size;
 	u8 max_scan_ssids;
 	u8 max_sched_scan_ssids;
+	u8 max_filter_ssids;
 	u16 max_scan_ie_len;
 	u16 max_sched_scan_ie_len;
 
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 57ecfa4..cd46a07 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -189,6 +189,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
 					 .len = IEEE80211_MAX_DATA_LEN },
 	[NL80211_ATTR_IE_ASSOC_RESP] = { .type = NLA_BINARY,
 					 .len = IEEE80211_MAX_DATA_LEN },
+	[NL80211_ATTR_SCHED_SCAN_FILTER] = { .type = NLA_NESTED },
 };
 
 /* policy for the key attributes */
@@ -714,6 +715,8 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
 		    dev->wiphy.max_scan_ie_len);
 	NLA_PUT_U16(msg, NL80211_ATTR_MAX_SCHED_SCAN_IE_LEN,
 		    dev->wiphy.max_sched_scan_ie_len);
+	NLA_PUT_U8(msg, NL80211_ATTR_MAX_FILTER_SSIDS,
+		   dev->wiphy.max_filter_ssids);
 
 	if (dev->wiphy.flags & WIPHY_FLAG_IBSS_RSN)
 		NLA_PUT_FLAG(msg, NL80211_ATTR_SUPPORT_IBSS_RSN);
@@ -3596,9 +3599,9 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 	struct cfg80211_sched_scan_request *request;
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
 	struct net_device *dev = info->user_ptr[1];
-	struct nlattr *attr;
+	struct nlattr *attr, *attr2;
 	struct wiphy *wiphy;
-	int err, tmp, n_ssids = 0, n_channels, i;
+	int err, tmp, n_ssids = 0, n_filter_ssids = 0, n_channels, i;
 	u32 interval;
 	enum ieee80211_band band;
 	size_t ie_len;
@@ -3640,6 +3643,17 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 	if (n_ssids > wiphy->max_sched_scan_ssids)
 		return -EINVAL;
 
+	if (info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER]) {
+		attr = nla_find_nested(info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER],
+				       NL80211_ATTR_SCHED_SCAN_FILTER_SSID);
+		if (attr)
+			nla_for_each_nested(attr2, attr, tmp)
+				n_filter_ssids++;
+	}
+
+	if (n_filter_ssids > wiphy->max_filter_ssids)
+		return -EINVAL;
+
 	if (info->attrs[NL80211_ATTR_IE])
 		ie_len = nla_len(info->attrs[NL80211_ATTR_IE]);
 	else
@@ -3657,6 +3671,7 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 
 	request = kzalloc(sizeof(*request)
 			+ sizeof(*request->ssids) * n_ssids
+			+ sizeof(*request->filter_ssids) * n_filter_ssids
 			+ sizeof(*request->channels) * n_channels
 			+ ie_len, GFP_KERNEL);
 	if (!request) {
@@ -3674,6 +3689,17 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 			request->ie = (void *)(request->channels + n_channels);
 	}
 
+	if (n_filter_ssids) {
+		if (request->ie)
+			request->filter_ssids = (void *)(request->ie + ie_len);
+		else if (request->ssids)
+			request->filter_ssids =
+				(void *)(request->ssids + n_ssids);
+		else
+			request->filter_ssids =
+				(void *)(request->channels + n_channels);
+	}
+
 	i = 0;
 	if (info->attrs[NL80211_ATTR_SCAN_FREQUENCIES]) {
 		/* user specified, bail out if channel not found */
@@ -3738,6 +3764,24 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 		}
 	}
 
+	i = 0;
+	if (info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER]) {
+		struct nlattr *attr2;
+		attr = nla_find_nested(info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER],
+				       NL80211_ATTR_SCHED_SCAN_FILTER_SSID);
+		if (attr)
+			nla_for_each_nested(attr2, attr, tmp) {
+				if (nla_len(attr2) > IEEE80211_MAX_SSID_LEN) {
+					err = -EINVAL;
+					goto out_free;
+				}
+				memcpy(request->filter_ssids[i].ssid, nla_data(attr2),
+				       nla_len(attr2));
+				request->filter_ssids[i].ssid_len = nla_len(attr2);
+				i++;
+			}
+	}
+
 	if (info->attrs[NL80211_ATTR_IE]) {
 		request->ie_len = nla_len(info->attrs[NL80211_ATTR_IE]);
 		memcpy((void *)request->ie,
-- 
1.7.1

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Johannes Berg]

On Tue, 2011-08-30 at 15:39 +0300, Luciano Coelho wrote:

> + * @NL80211_ATTR_MAX_FILTER_SSIDS: number of SSIDs you can use as
> + *	filters in a scheduled scan request, a wiphy attribute.

When we add more filters, hopefully the number of filters will stay be
this? IOW, shouldn't this be "MAX_FILTERS"? Basically, each filter can
be thought of as a profile, and a profile can contain various attributes
that must match, I think?

> + * @NL80211_ATTR_SCHED_SCAN_FILTER: Nested attribute with various
> + *	types of filtering to be used with scheduled scans.
> + *	If @NL80211_ATTR_SCAN_SSIDS is passed with values that are not
> + *	included in the filter, the driver may return -EINVAL, since
> + *	it doesn't make sense to send probe requests with SSIDs that
> + *	will be filtered out.  This doesn't apply to the wildcard SSID.
> + *	If ommited, no filtering is done.

Maybe cfg80211 should just apply that rule anyway? But is that really
true anyway? Say "DIRECT-" as the P2P wildcard ...

> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index a37f264..a3e58ff 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -882,6 +882,9 @@ struct cfg80211_scan_request {
>   * @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
> + * @filter_ssids: SSIDs to pass to the host (others are filtered out).
> + * 	If ommited, no filtering is done.
> + * @n_filter_ssids: number of filter SSIDs

Maybe we should create a filter struct right away and use that with
filters/n_filters?

> +++ b/net/wireless/nl80211.c
> @@ -189,6 +189,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
>  					 .len = IEEE80211_MAX_DATA_LEN },
>  	[NL80211_ATTR_IE_ASSOC_RESP] = { .type = NLA_BINARY,
>  					 .len = IEEE80211_MAX_DATA_LEN },
> +	[NL80211_ATTR_SCHED_SCAN_FILTER] = { .type = NLA_NESTED },
>  };

Don't we need a policy for the filter attributes?

> +	if (info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER]) {
> +		attr = nla_find_nested(info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER],
> +				       NL80211_ATTR_SCHED_SCAN_FILTER_SSID);
> +		if (attr)
> +			nla_for_each_nested(attr2, attr, tmp)
> +				n_filter_ssids++;
> +	}
> +
> +	if (n_filter_ssids > wiphy->max_filter_ssids)
> +		return -EINVAL;

That looks a little odd. What does that even do? I don't understand the
find_nested at all.

johannes

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Luciano Coelho]

On Tue, 2011-08-30 at 16:16 +0200, Johannes Berg wrote: 
> On Tue, 2011-08-30 at 15:39 +0300, Luciano Coelho wrote:
> 
> > + * @NL80211_ATTR_MAX_FILTER_SSIDS: number of SSIDs you can use as
> > + *	filters in a scheduled scan request, a wiphy attribute.
> 
> When we add more filters, hopefully the number of filters will stay be
> this? IOW, shouldn't this be "MAX_FILTERS"? Basically, each filter can
> be thought of as a profile, and a profile can contain various attributes
> that must match, I think?

Hmmm... I think we shouldn't limit the number of total filters, but the
limits of each kind of filtering.  The firmwares will most likely have
limitation on how many SSIDs can be added to the filter list.  In our
case, we can have max 8 (soon 16) SSIDs in the let-through list.  So I
think this should remain.  Maybe it could be moved to a
FILTER_CAPABILITIES nested attribute instead?


> > + * @NL80211_ATTR_SCHED_SCAN_FILTER: Nested attribute with various
> > + *	types of filtering to be used with scheduled scans.
> > + *	If @NL80211_ATTR_SCAN_SSIDS is passed with values that are not
> > + *	included in the filter, the driver may return -EINVAL, since
> > + *	it doesn't make sense to send probe requests with SSIDs that
> > + *	will be filtered out.  This doesn't apply to the wildcard SSID.
> > + *	If ommited, no filtering is done.
> 
> Maybe cfg80211 should just apply that rule anyway? But is that really
> true anyway? Say "DIRECT-" as the P2P wildcard ...

It would still be true for wildcard filtering, wouldn't it? In wildcard
filtering, you can't pass "DIRECT-" in the SSID list, because that would
be added to the probe requests.  I guess in the P2P wildcard case, we
would want to issue wildcard probe requests (ie. with zero-length SSID)?

I think we can still keep it like this and improve the filter list by
adding a "wildcard" attribute if you want to filter on P2P wildcards?

Now back to "Maybe cfg80211 should apply that rule", I thought about it,
but decided to keep it hw-specific, since I don't know what other HW
will want to do. ;) But I can add it to cfg80211 if you like.


> > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> > index a37f264..a3e58ff 100644
> > --- a/include/net/cfg80211.h
> > +++ b/include/net/cfg80211.h
> > @@ -882,6 +882,9 @@ struct cfg80211_scan_request {
> >   * @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
> > + * @filter_ssids: SSIDs to pass to the host (others are filtered out).
> > + * 	If ommited, no filtering is done.
> > + * @n_filter_ssids: number of filter SSIDs
> 
> Maybe we should create a filter struct right away and use that with
> filters/n_filters?

I can do that too.  This is not really affected by the NL80211 API and,
since we only have SSID filtering so far, I decided not to put it in a
struct yet (this could be changed later).  But I can also add it now, no
problem.


> > +++ b/net/wireless/nl80211.c
> > @@ -189,6 +189,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
> >  					 .len = IEEE80211_MAX_DATA_LEN },
> >  	[NL80211_ATTR_IE_ASSOC_RESP] = { .type = NLA_BINARY,
> >  					 .len = IEEE80211_MAX_DATA_LEN },
> > +	[NL80211_ATTR_SCHED_SCAN_FILTER] = { .type = NLA_NESTED },
> >  };
> 
> Don't we need a policy for the filter attributes?

Hmmm, right.  I missed that.  I have made the filter attributes as
sub-attributes that must be inside the filter nest, so they don't really
fit here.  But I'll figure out how to add policies for them.


> > +	if (info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER]) {
> > +		attr = nla_find_nested(info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER],
> > +				       NL80211_ATTR_SCHED_SCAN_FILTER_SSID);
> > +		if (attr)
> > +			nla_for_each_nested(attr2, attr, tmp)
> > +				n_filter_ssids++;
> > +	}
> > +
> > +	if (n_filter_ssids > wiphy->max_filter_ssids)
> > +		return -EINVAL;
> 
> That looks a little odd. What does that even do? I don't understand the
> find_nested at all.

The idea is that the filter attributes are inside the nested
NL80211_ATTR_SCHED_SCAN_FILTER.  So, what I do here is get the filter
SSID attribute from inside the filter nest.  The SSID filter attribute
itself is also a nest (an array) and that's what I go through in the
nla_for_each_nested().

My idea is that we have this kind of struct:

SCHED_SCAN_START(ssids[], filter{ssid_filters[], bssid_filters[], ...}, ...)

Anyway, I'll recheck if there is a simpler (and maybe correct? ;) way of
doing this.

Thanks for your review, v2 is coming soon. 


-- 
Cheers,
Luca.

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Johannes Berg]

On Tue, 2011-08-30 at 18:27 +0300, Luciano Coelho wrote:
> On Tue, 2011-08-30 at 16:16 +0200, Johannes Berg wrote: 
> > On Tue, 2011-08-30 at 15:39 +0300, Luciano Coelho wrote:
> > 
> > > + * @NL80211_ATTR_MAX_FILTER_SSIDS: number of SSIDs you can use as
> > > + *	filters in a scheduled scan request, a wiphy attribute.
> > 
> > When we add more filters, hopefully the number of filters will stay be
> > this? IOW, shouldn't this be "MAX_FILTERS"? Basically, each filter can
> > be thought of as a profile, and a profile can contain various attributes
> > that must match, I think?
> 
> Hmmm... I think we shouldn't limit the number of total filters, but the
> limits of each kind of filtering.  The firmwares will most likely have
> limitation on how many SSIDs can be added to the filter list.  In our
> case, we can have max 8 (soon 16) SSIDs in the let-through list.  So I
> think this should remain.  Maybe it could be moved to a
> FILTER_CAPABILITIES nested attribute instead?

Well, so the filters I see happening will be along the lines of:
	SSID 'foo' with WPA2-PSK and CCMP

(because that's what the user configured).

Are you envisioning other filters that make sense w/o an SSID?

> > Maybe cfg80211 should just apply that rule anyway? But is that really
> > true anyway? Say "DIRECT-" as the P2P wildcard ...
> 
> It would still be true for wildcard filtering, wouldn't it? In wildcard
> filtering, you can't pass "DIRECT-" in the SSID list, because that would
> be added to the probe requests.  I guess in the P2P wildcard case, we
> would want to issue wildcard probe requests (ie. with zero-length SSID)?

No, "DIRECT-" *is*  the P2P wildcard, any DIRECT-XY GO will reply to
"DIRECT-". You might want "DIRECT-" in the SSID list even if the GO
you're interested in is "DIRECT-6z".

> I think we can still keep it like this and improve the filter list by
> adding a "wildcard" attribute if you want to filter on P2P wildcards?
> 
> Now back to "Maybe cfg80211 should apply that rule", I thought about it,
> but decided to keep it hw-specific, since I don't know what other HW
> will want to do. ;) But I can add it to cfg80211 if you like.

Is it something you _need_ to make your firmware happy? Otherwise I
think I'd rather leave it out completely :)

> > > + * @filter_ssids: SSIDs to pass to the host (others are filtered out).
> > > + * 	If ommited, no filtering is done.
> > > + * @n_filter_ssids: number of filter SSIDs
> > 
> > Maybe we should create a filter struct right away and use that with
> > filters/n_filters?
> 
> I can do that too.  This is not really affected by the NL80211 API and,
> since we only have SSID filtering so far, I decided not to put it in a
> struct yet (this could be changed later).  But I can also add it now, no
> problem.

Ok, I think I'd prefer that, I know it'll only be a short while until we
add more to it and it would be easier to not have to modify wl12xx at
that time.

> > > +	[NL80211_ATTR_SCHED_SCAN_FILTER] = { .type = NLA_NESTED },
> > >  };
> > 
> > Don't we need a policy for the filter attributes?
> 
> Hmmm, right.  I missed that.  I have made the filter attributes as
> sub-attributes that must be inside the filter nest, so they don't really
> fit here.  But I'll figure out how to add policies for them.

Yeah of course they don't fit but see how we have a separate policy for
key attributes for example.

> > > +	if (info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER]) {
> > > +		attr = nla_find_nested(info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER],
> > > +				       NL80211_ATTR_SCHED_SCAN_FILTER_SSID);
> > > +		if (attr)
> > > +			nla_for_each_nested(attr2, attr, tmp)
> > > +				n_filter_ssids++;
> > > +	}
> > > +
> > > +	if (n_filter_ssids > wiphy->max_filter_ssids)
> > > +		return -EINVAL;
> > 
> > That looks a little odd. What does that even do? I don't understand the
> > find_nested at all.
> 
> The idea is that the filter attributes are inside the nested
> NL80211_ATTR_SCHED_SCAN_FILTER.  So, what I do here is get the filter
> SSID attribute from inside the filter nest.  The SSID filter attribute
> itself is also a nest (an array) and that's what I go through in the
> nla_for_each_nested().
> 
> My idea is that we have this kind of struct:
> 
> SCHED_SCAN_START(ssids[], filter{ssid_filters[], bssid_filters[], ...}, ...)
> 
> Anyway, I'll recheck if there is a simpler (and maybe correct? ;) way of
> doing this.

Oh. Ok you're thinking of this completely differently than I am. With
the filtering I'm looking at (see above) the way to make this make sense
is to do

FILTERS={1={SSID=foo, ...}, 2={SSID=bar, ...}, ... }

IOW have filters be a nested attribute that contains an array that you
walk with for_each_nested() (the attribute ID there 1,2,.. doesn't
matter) and then each of those is nested again containing the filter
attributes. IOW, FILTERS becomes a list of profiles.

johannes

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Luciano Coelho]

On Tue, 2011-08-30 at 18:09 +0200, Johannes Berg wrote: 
> On Tue, 2011-08-30 at 18:27 +0300, Luciano Coelho wrote:
> > On Tue, 2011-08-30 at 16:16 +0200, Johannes Berg wrote: 
> > > On Tue, 2011-08-30 at 15:39 +0300, Luciano Coelho wrote:
> > > 
> > > > + * @NL80211_ATTR_MAX_FILTER_SSIDS: number of SSIDs you can use as
> > > > + *	filters in a scheduled scan request, a wiphy attribute.
> > > 
> > > When we add more filters, hopefully the number of filters will stay be
> > > this? IOW, shouldn't this be "MAX_FILTERS"? Basically, each filter can
> > > be thought of as a profile, and a profile can contain various attributes
> > > that must match, I think?
> > 
> > Hmmm... I think we shouldn't limit the number of total filters, but the
> > limits of each kind of filtering.  The firmwares will most likely have
> > limitation on how many SSIDs can be added to the filter list.  In our
> > case, we can have max 8 (soon 16) SSIDs in the let-through list.  So I
> > think this should remain.  Maybe it could be moved to a
> > FILTER_CAPABILITIES nested attribute instead?
> 
> Well, so the filters I see happening will be along the lines of:
> 	SSID 'foo' with WPA2-PSK and CCMP
> 
> (because that's what the user configured).

Do you see any real advantage in passing the security parameters in the
search? Of course this would filter out a bit more, but in real life how
much would we gain? I guess reporting all matches for that SSID
(regardless of the security parameters) would already be good enough.

If we add these extra parameters, then we have to tell userspace whether
the device supports it or not (wl12xx at the moment doesn't).


> Are you envisioning other filters that make sense w/o an SSID?

I was thinking about other filters such as BSSID blacklisting that you
mentioned.  If we blacklist a BSSID, we probably don't need to care
which SSID it's advertising.

Other filters that come to my mind would be by security features,
regardless of the SSID (eg. "give me all open security SSIDs", or "I
only want WPA2 capable SSIDs").  The wl12xx firmware doesn't support
this at the moment, but I can see some potential use-cases for this.

Yet another possibility is to filter out based on signal strength
(wl12xx supports it).  For instance it may make sense for roaming, eg.
"give me only SSIDs with better signal than the one I'm connected to
right now").

These are all theoretical and at least we don't have any real
requirement for them at the moment.  But this was more or less what I
was preparing for.


> > > Maybe cfg80211 should just apply that rule anyway? But is that really
> > > true anyway? Say "DIRECT-" as the P2P wildcard ...
> > 
> > It would still be true for wildcard filtering, wouldn't it? In wildcard
> > filtering, you can't pass "DIRECT-" in the SSID list, because that would
> > be added to the probe requests.  I guess in the P2P wildcard case, we
> > would want to issue wildcard probe requests (ie. with zero-length SSID)?
> 
> No, "DIRECT-" *is*  the P2P wildcard, any DIRECT-XY GO will reply to
> "DIRECT-". You might want "DIRECT-" in the SSID list even if the GO
> you're interested in is "DIRECT-6z".

Ah, right, I had forgotten how this works.  I *really* need to find the
time to study P2P a bit more. :)

In any case, I think for the filtering, we can implement some kind of
wildcard too.  We could have a bit in the filter saying that it's a
wildcard, so any results that start with the supplied bytes would pass
through.

In this case, I think the same rule of not having SSIDs that don't fit
the filters would still apply.  For instance, if you pass only "DIRECT-"
for the probe_reqs, without broadcast, it doesn't make sense to have a
filter that only matches "FOOBAR".


> > I think we can still keep it like this and improve the filter list by
> > adding a "wildcard" attribute if you want to filter on P2P wildcards?
> > 
> > Now back to "Maybe cfg80211 should apply that rule", I thought about it,
> > but decided to keep it hw-specific, since I don't know what other HW
> > will want to do. ;) But I can add it to cfg80211 if you like.
> 
> Is it something you _need_ to make your firmware happy? Otherwise I
> think I'd rather leave it out completely :)

Kind of, yes.  Actually our firmware is "smart" in the scheduled scan
feature.  You pass a single list of SSIDs to it with attributes such as
open/hidden and it will by itself decide whether to send a probe_req for
it or not.  And it will filter for those SSIDs regardless of whether a
probe_req was sent.  It's a bit more complicated than that, but not
really relevant here.

I could probably manage without this limitation in the API, but then it
would be difficult to handle the max_ssids and max_filter_ssids
limitations (the limitation in our case actually applies to a sum of the
two).  If we have the limitation in the API, then the filter list will
always be a superset of the SSID list, which makes it simpler. ;)


> > > > + * @filter_ssids: SSIDs to pass to the host (others are filtered out).
> > > > + * 	If ommited, no filtering is done.
> > > > + * @n_filter_ssids: number of filter SSIDs
> > > 
> > > Maybe we should create a filter struct right away and use that with
> > > filters/n_filters?
> > 
> > I can do that too.  This is not really affected by the NL80211 API and,
> > since we only have SSID filtering so far, I decided not to put it in a
> > struct yet (this could be changed later).  But I can also add it now, no
> > problem.
> 
> Ok, I think I'd prefer that, I know it'll only be a short while until we
> add more to it and it would be easier to not have to modify wl12xx at
> that time.

Indeed sounds better.  I'll do it.


> > > > +	[NL80211_ATTR_SCHED_SCAN_FILTER] = { .type = NLA_NESTED },
> > > >  };
> > > 
> > > Don't we need a policy for the filter attributes?
> > 
> > Hmmm, right.  I missed that.  I have made the filter attributes as
> > sub-attributes that must be inside the filter nest, so they don't really
> > fit here.  But I'll figure out how to add policies for them.
> 
> Yeah of course they don't fit but see how we have a separate policy for
> key attributes for example.

Sure, I tried to look at the key attributes and reg stuff, but they
actually use a different nl callback.  I didn't see how to use it for
the filter stuff, which is actually a subset of attributes for the
existing scan attributes.  Anyway, I'll check the nl API more carefully
and fix this.  I probably need to use nla_parse() and such.


> > > > +	if (info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER]) {
> > > > +		attr = nla_find_nested(info->attrs[NL80211_ATTR_SCHED_SCAN_FILTER],
> > > > +				       NL80211_ATTR_SCHED_SCAN_FILTER_SSID);
> > > > +		if (attr)
> > > > +			nla_for_each_nested(attr2, attr, tmp)
> > > > +				n_filter_ssids++;
> > > > +	}
> > > > +
> > > > +	if (n_filter_ssids > wiphy->max_filter_ssids)
> > > > +		return -EINVAL;
> > > 
> > > That looks a little odd. What does that even do? I don't understand the
> > > find_nested at all.
> > 
> > The idea is that the filter attributes are inside the nested
> > NL80211_ATTR_SCHED_SCAN_FILTER.  So, what I do here is get the filter
> > SSID attribute from inside the filter nest.  The SSID filter attribute
> > itself is also a nest (an array) and that's what I go through in the
> > nla_for_each_nested().
> > 
> > My idea is that we have this kind of struct:
> > 
> > SCHED_SCAN_START(ssids[], filter{ssid_filters[], bssid_filters[], ...}, ...)
> > 
> > Anyway, I'll recheck if there is a simpler (and maybe correct? ;) way of
> > doing this.
> 
> Oh. Ok you're thinking of this completely differently than I am. With
> the filtering I'm looking at (see above) the way to make this make sense
> is to do
> 
> FILTERS={1={SSID=foo, ...}, 2={SSID=bar, ...}, ... }

This makes sense too, but if we want to support other kinds of filters
(such as the ones I mentioned above), we need to have at least the type
of filter for each:

FILTERS={1={type=SSID, SSID=foo},
2={type=SSID, SSID=bar},
3={type=RSSI, min_rssi=-70},
4={type=BLACKLIST, BSSID=bssid1},
5={type=BLACKLIST, BSSID=bssid2},
...}

What I was originally thinking of implementing was this: 

FILTERS={SSID={SSID_LIST={foo, bar, ...}},
	 RSSI={min_rssi},
	 BLACKLIST={BSSID_LIST={bssid1, bssid2, ...}},
	 ...}

...and the SSID_LIST elements could be foo={SSID="FOO", security=WPA2},
bar={SSID="bar", security=OPEN}, for example.  Though I'm not sure this
is necessary (see my question earlier).


> IOW have filters be a nested attribute that contains an array that you
> walk with for_each_nested() (the attribute ID there 1,2,.. doesn't
> matter) and then each of those is nested again containing the filter
> attributes. IOW, FILTERS becomes a list of profiles.

I see.  I'll do something like this and get rid of the
nla_find_nested().  But first we need to agree on the data structs. :)

Too much discussion for my round in this discussion, I'll release and
let you express your thoughts too. ;)

-- 
Cheers,
Luca.

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Johannes Berg]

On Wed, 2011-08-31 at 09:09 +0300, Luciano Coelho wrote:

> > Well, so the filters I see happening will be along the lines of:
> > 	SSID 'foo' with WPA2-PSK and CCMP
> > 
> > (because that's what the user configured).
> 
> Do you see any real advantage in passing the security parameters in the
> search? Of course this would filter out a bit more, but in real life how
> much would we gain? I guess reporting all matches for that SSID
> (regardless of the security parameters) would already be good enough.

I don't really, but it seems that some people still want it.

> If we add these extra parameters, then we have to tell userspace whether
> the device supports it or not (wl12xx at the moment doesn't).

I thought about this, but I'm not convinced we really need to. A device
that doesn't support a given filter can always ignore it and wake up
even when the AP doesn't match all filters, ie. if I ask for
notification on (SSID="foo", WPA2-PSK, CCMP) and get notifications for
SSID="foo", I can still handle that. It's just not as efficient in the
corner case that there are lots of "foo" APs with different settings.

> > Are you envisioning other filters that make sense w/o an SSID?
> 
> I was thinking about other filters such as BSSID blacklisting that you
> mentioned.  If we blacklist a BSSID, we probably don't need to care
> which SSID it's advertising.

Indeed.

> Other filters that come to my mind would be by security features,
> regardless of the SSID (eg. "give me all open security SSIDs", or "I
> only want WPA2 capable SSIDs").  The wl12xx firmware doesn't support
> this at the moment, but I can see some potential use-cases for this.

That might be interesting. But maybe that's a filter a la my previous
model with SSID unset and security set to open etc.? If you add that
filter to wl12xx you'd get wakeup for every network though, so in this
special case it might be interesting to know whether or not the device
supports it. OTOH, if you want to connect to any open network, you'll
need to swallow the wakeup cost anyway.

> Yet another possibility is to filter out based on signal strength
> (wl12xx supports it).  For instance it may make sense for roaming, eg.
> "give me only SSIDs with better signal than the one I'm connected to
> right now").

Wouldn't that also just be an extension? Or would this not be a
per-network filter?

> These are all theoretical and at least we don't have any real
> requirement for them at the moment.  But this was more or less what I
> was preparing for.

Ok. I think the BSSID blacklist would just be a separate attribute, I
see no reason to have a nesting for that.

> In any case, I think for the filtering, we can implement some kind of
> wildcard too.  We could have a bit in the filter saying that it's a
> wildcard, so any results that start with the supplied bytes would pass
> through.
> 
> In this case, I think the same rule of not having SSIDs that don't fit
> the filters would still apply.  For instance, if you pass only "DIRECT-"
> for the probe_reqs, without broadcast, it doesn't make sense to have a
> filter that only matches "FOOBAR".

True. Do we really want to add this wildcard complexity to filters
though? Is it really worth it? OTOH, I guess you'd need it for something
like iPass.

> > > I think we can still keep it like this and improve the filter list by
> > > adding a "wildcard" attribute if you want to filter on P2P wildcards?
> > > 
> > > Now back to "Maybe cfg80211 should apply that rule", I thought about it,
> > > but decided to keep it hw-specific, since I don't know what other HW
> > > will want to do. ;) But I can add it to cfg80211 if you like.
> > 
> > Is it something you _need_ to make your firmware happy? Otherwise I
> > think I'd rather leave it out completely :)
> 
> Kind of, yes.  Actually our firmware is "smart" in the scheduled scan
> feature.  You pass a single list of SSIDs to it with attributes such as
> open/hidden and it will by itself decide whether to send a probe_req for
> it or not.  And it will filter for those SSIDs regardless of whether a
> probe_req was sent.  It's a bit more complicated than that, but not
> really relevant here.
> 
> I could probably manage without this limitation in the API, but then it
> would be difficult to handle the max_ssids and max_filter_ssids
> limitations (the limitation in our case actually applies to a sum of the
> two).  If we have the limitation in the API, then the filter list will
> always be a superset of the SSID list, which makes it simpler. ;)

OK :)

> > Yeah of course they don't fit but see how we have a separate policy for
> > key attributes for example.
> 
> Sure, I tried to look at the key attributes and reg stuff, but they
> actually use a different nl callback.  I didn't see how to use it for
> the filter stuff, which is actually a subset of attributes for the
> existing scan attributes.  Anyway, I'll check the nl API more carefully
> and fix this.  I probably need to use nla_parse() and such.

> > Oh. Ok you're thinking of this completely differently than I am. With
> > the filtering I'm looking at (see above) the way to make this make sense
> > is to do
> > 
> > FILTERS={1={SSID=foo, ...}, 2={SSID=bar, ...}, ... }
> 
> This makes sense too, but if we want to support other kinds of filters
> (such as the ones I mentioned above), we need to have at least the type
> of filter for each:
> 
> FILTERS={1={type=SSID, SSID=foo},
> 2={type=SSID, SSID=bar},
> 3={type=RSSI, min_rssi=-70},
> 4={type=BLACKLIST, BSSID=bssid1},
> 5={type=BLACKLIST, BSSID=bssid2},
> ...}
> 
> What I was originally thinking of implementing was this: 
> 
> FILTERS={SSID={SSID_LIST={foo, bar, ...}},
> 	 RSSI={min_rssi},
> 	 BLACKLIST={BSSID_LIST={bssid1, bssid2, ...}},
> 	 ...}
> 
> ...and the SSID_LIST elements could be foo={SSID="FOO", security=WPA2},
> bar={SSID="bar", security=OPEN}, for example.  Though I'm not sure this
> is necessary (see my question earlier).

I wouldn't bother nesting those though, so I'd do

NETWORK_FILTER=(1=(SSID=...), ...)
BSSID_BLACKLIST=(1=...,2=...,...)
etc.
That way only the network attributes that belong to a "profile" like
SSID, security, ... are nested.

johannes

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Luciano Coelho]

On Wed, 2011-08-31 at 08:20 +0200, Johannes Berg wrote: 
> > Other filters that come to my mind would be by security features,
> > regardless of the SSID (eg. "give me all open security SSIDs", or "I
> > only want WPA2 capable SSIDs").  The wl12xx firmware doesn't support
> > this at the moment, but I can see some potential use-cases for this.
> 
> That might be interesting. But maybe that's a filter a la my previous
> model with SSID unset and security set to open etc.? If you add that
> filter to wl12xx you'd get wakeup for every network though, so in this
> special case it might be interesting to know whether or not the device
> supports it. OTOH, if you want to connect to any open network, you'll
> need to swallow the wakeup cost anyway.
> 
> > Yet another possibility is to filter out based on signal strength
> > (wl12xx supports it).  For instance it may make sense for roaming, eg.
> > "give me only SSIDs with better signal than the one I'm connected to
> > right now").
> 
> Wouldn't that also just be an extension? Or would this not be a
> per-network filter?

Right, we can do it as an extension.


> > These are all theoretical and at least we don't have any real
> > requirement for them at the moment.  But this was more or less what I
> > was preparing for.
> 
> Ok. I think the BSSID blacklist would just be a separate attribute, I
> see no reason to have a nesting for that.

Actually, we could make the filter and blacklisting very generic.  We
can use the same attributes for both.  The only difference would be that
the former would include anything that matches and the latter would
exclude anything that matches.


> > In any case, I think for the filtering, we can implement some kind of
> > wildcard too.  We could have a bit in the filter saying that it's a
> > wildcard, so any results that start with the supplied bytes would pass
> > through.
> > 
> > In this case, I think the same rule of not having SSIDs that don't fit
> > the filters would still apply.  For instance, if you pass only "DIRECT-"
> > for the probe_reqs, without broadcast, it doesn't make sense to have a
> > filter that only matches "FOOBAR".
> 
> True. Do we really want to add this wildcard complexity to filters
> though? Is it really worth it? OTOH, I guess you'd need it for something
> like iPass.

Yeah, it was the hotspot case that I had in mind.  But since we don't
need this right now, let's leave it out.  It can be added easily later
on.


> > > > I think we can still keep it like this and improve the filter list by
> > > > adding a "wildcard" attribute if you want to filter on P2P wildcards?
> > > > 
> > > > Now back to "Maybe cfg80211 should apply that rule", I thought about it,
> > > > but decided to keep it hw-specific, since I don't know what other HW
> > > > will want to do. ;) But I can add it to cfg80211 if you like.
> > > 
> > > Is it something you _need_ to make your firmware happy? Otherwise I
> > > think I'd rather leave it out completely :)
> > 
> > Kind of, yes.  Actually our firmware is "smart" in the scheduled scan
> > feature.  You pass a single list of SSIDs to it with attributes such as
> > open/hidden and it will by itself decide whether to send a probe_req for
> > it or not.  And it will filter for those SSIDs regardless of whether a
> > probe_req was sent.  It's a bit more complicated than that, but not
> > really relevant here.
> > 
> > I could probably manage without this limitation in the API, but then it
> > would be difficult to handle the max_ssids and max_filter_ssids
> > limitations (the limitation in our case actually applies to a sum of the
> > two).  If we have the limitation in the API, then the filter list will
> > always be a superset of the SSID list, which makes it simpler. ;)
> 
> OK :)

I'll add the check in cfg80211 then.  There are probably other
combinations that don't make sense either (eg. sending an SSID in the
probes and blacklisting it).


> > > Yeah of course they don't fit but see how we have a separate policy for
> > > key attributes for example.
> > 
> > Sure, I tried to look at the key attributes and reg stuff, but they
> > actually use a different nl callback.  I didn't see how to use it for
> > the filter stuff, which is actually a subset of attributes for the
> > existing scan attributes.  Anyway, I'll check the nl API more carefully
> > and fix this.  I probably need to use nla_parse() and such.
> 
> > > Oh. Ok you're thinking of this completely differently than I am. With
> > > the filtering I'm looking at (see above) the way to make this make sense
> > > is to do
> > > 
> > > FILTERS={1={SSID=foo, ...}, 2={SSID=bar, ...}, ... }
> > 
> > This makes sense too, but if we want to support other kinds of filters
> > (such as the ones I mentioned above), we need to have at least the type
> > of filter for each:
> > 
> > FILTERS={1={type=SSID, SSID=foo},
> > 2={type=SSID, SSID=bar},
> > 3={type=RSSI, min_rssi=-70},
> > 4={type=BLACKLIST, BSSID=bssid1},
> > 5={type=BLACKLIST, BSSID=bssid2},
> > ...}
> > 
> > What I was originally thinking of implementing was this: 
> > 
> > FILTERS={SSID={SSID_LIST={foo, bar, ...}},
> > 	 RSSI={min_rssi},
> > 	 BLACKLIST={BSSID_LIST={bssid1, bssid2, ...}},
> > 	 ...}
> > 
> > ...and the SSID_LIST elements could be foo={SSID="FOO", security=WPA2},
> > bar={SSID="bar", security=OPEN}, for example.  Though I'm not sure this
> > is necessary (see my question earlier).
> 
> I wouldn't bother nesting those though, so I'd do
> 
> NETWORK_FILTER=(1=(SSID=...), ...)
> BSSID_BLACKLIST=(1=...,2=...,...)
> etc.
> That way only the network attributes that belong to a "profile" like
> SSID, security, ... are nested.

Yeah, this is okay.  To make it more generic though, should we make it
like this?

MATCH={1={SSID=foo, SECURITY=open, BSSID=00:01:02:03:04:05, ...}, 2={...}}
NOT_MATCH={1={SSID=bar, ...}, 2={BSSID=00:02:03:04:05:06, ...}, ...}

Meaning that we just have MATCH/NO_MATCH and use the same set of
attributes for both? Maybe someone will want to blacklist an SSID or
other attributes too.  Who wants to see APs called "Free open wifi"? :D

I also prefer using match/not_match (couldn't find a better antonym),
because "filter" is ambiguous (let-through or leave-out?).

-- 
Cheers,
Luca.

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Johannes Berg]

On Wed, 2011-08-31 at 10:30 +0300, Luciano Coelho wrote:

> > Ok. I think the BSSID blacklist would just be a separate attribute, I
> > see no reason to have a nesting for that.
> 
> Actually, we could make the filter and blacklisting very generic.  We
> can use the same attributes for both.  The only difference would be that
> the former would include anything that matches and the latter would
> exclude anything that matches.

Hm, true, but does it really make sense to blacklist SSIDs when we have
a whitelist for them? Or would you want to whitelist a wildcard and then
blacklist a specific one?


> I also prefer using match/not_match (couldn't find a better antonym),
> because "filter" is ambiguous (let-through or leave-out?).

agree.

johannes

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Luciano Coelho]

On Wed, 2011-08-31 at 09:39 +0200, Johannes Berg wrote: 
> On Wed, 2011-08-31 at 10:30 +0300, Luciano Coelho wrote:
> 
> > > Ok. I think the BSSID blacklist would just be a separate attribute, I
> > > see no reason to have a nesting for that.
> > 
> > Actually, we could make the filter and blacklisting very generic.  We
> > can use the same attributes for both.  The only difference would be that
> > the former would include anything that matches and the latter would
> > exclude anything that matches.
> 
> Hm, true, but does it really make sense to blacklist SSIDs when we have
> a whitelist for them? Or would you want to whitelist a wildcard and then
> blacklist a specific one?

Whitelisting and blacklisting the same SSID would be one of the
"illegal" cases that we may want to reject with -EINVAL.

But my idea was that you could do this:

MATCH={SSID=foo} and
NOT_MATCH={BSSID=00:01:02:03:04:05}

or

MATCH={BSSID=00:01:02:03:04:05} and
NOT_MATCH={SSID=foo}

or, making more sense:

MATCH={BSSID=00:01:02:03:04:05} and
NOT_MATCH={MIN_RSSI=-70}

My point is that many of these combinations might not make sense, but
generalizing the API allows for cases that *do* make sense but we have
not thought of yet.


> > I also prefer using match/not_match (couldn't find a better antonym),
> > because "filter" is ambiguous (let-through or leave-out?).
> 
> agree.

We could also use a single list like this:

FILTER={1={MATCH=true, SSID=foo}, 2={MATCH=false, BSSID=...}

But then we're becoming a bit too "netfiltery". :P What do you think?

-- 
Cheers,
Luca.

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Johannes Berg]

On Wed, 2011-08-31 at 10:49 +0300, Luciano Coelho wrote:

> Whitelisting and blacklisting the same SSID would be one of the
> "illegal" cases that we may want to reject with -EINVAL.
> 
> But my idea was that you could do this:
> 
> MATCH={SSID=foo} and
> NOT_MATCH={BSSID=00:01:02:03:04:05}
> 
> or
> 
> MATCH={BSSID=00:01:02:03:04:05} and
> NOT_MATCH={SSID=foo}
> 
> or, making more sense:
> 
> MATCH={BSSID=00:01:02:03:04:05} and
> NOT_MATCH={MIN_RSSI=-70}
> 
> My point is that many of these combinations might not make sense, but
> generalizing the API allows for cases that *do* make sense but we have
> not thought of yet.

Good point :)

> > > I also prefer using match/not_match (couldn't find a better antonym),
> > > because "filter" is ambiguous (let-through or leave-out?).
> > 
> > agree.
> 
> We could also use a single list like this:
> 
> FILTER={1={MATCH=true, SSID=foo}, 2={MATCH=false, BSSID=...}
> 
> But then we're becoming a bit too "netfiltery". :P What do you think?

Heh. Don't really care :)

I guess the thing to keep in mind is that it's all an optimisation and
if a device can't support certain combinations it doesn't really
matter :)

johannes

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Johannes Berg]

On Wed, 2011-08-31 at 09:53 +0200, Johannes Berg wrote:

> > MATCH={BSSID=00:01:02:03:04:05} and
> > NOT_MATCH={MIN_RSSI=-70}

Actually does that really make sense?

I suspect you'd want something like

MATCH SSID="foo" && RSSI >= -70

where the && part isn't really expressed now?

johannes

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Luciano Coelho]

On Wed, 2011-08-31 at 09:54 +0200, Johannes Berg wrote: 
> On Wed, 2011-08-31 at 09:53 +0200, Johannes Berg wrote:
> 
> > > MATCH={BSSID=00:01:02:03:04:05} and
> > > NOT_MATCH={MIN_RSSI=-70}
> 
> Actually does that really make sense?
> 
> I suspect you'd want something like
> 
> MATCH SSID="foo" && RSSI >= -70
> 
> where the && part isn't really expressed now?

Ahmmm... yes, the actual parameters don't make much sense. :D

But about &&, I guess it's should be like this:

MATCH={1=... OR 2=... OR 3=...} AND NOT_MATCH={1=... NOR 2=...}

So the netfiltery way of doing it would be much more complicated,
because we would have to specify ANDs and ORs explicitly in the list
somehow.

-- 
Cheers,
Luca.

Re: [PATCH] nl80211/cfg80211: add ssid filtering for sched_scan

[Luciano Coelho]

On Wed, 2011-08-31 at 09:53 +0200, Johannes Berg wrote: 
> I guess the thing to keep in mind is that it's all an optimisation and
> if a device can't support certain combinations it doesn't really
> matter :)

Yeah, it must be clear in the implementation that the device may send
things that are not really following the filtering rules.  We can't
avoid that anyway, because some other entity may start a scan and mangle
with our BSS cache.

-- 
Cheers,
Luca.