* [PATCH] nl80211: Count only ssid attr to know number of match sets
@ 2014-01-23 4:27 Raja Mani
2014-01-23 8:17 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Raja Mani @ 2014-01-23 4:27 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, johannes, Raja Mani
NL80211_ATTR_SCHED_SCAN_MATCH can have NL80211_SCHED_SCAN_MATCH_ATTR_SSID
as well as NL80211_SCHED_SCAN_MATCH_ATTR_RSSI in it. Each nested attributes
which are part of NL80211_ATTR_SCHED_SCAN_MATCH are counted to find out
number of match sets in nl80211_start_sched_scan().
This is the problem if NL80211_SCHED_SCAN_MATCH_ATTR_RSSI also part of
NL80211_ATTR_SCHED_SCAN_MATCH. Fix this incorrect calculation by considering
only NL80211_SCHED_SCAN_MATCH_ATTR_SSID to know match set count.
Signed-off-by: Raja Mani <rmani@qti.qualcomm.com>
---
net/wireless/nl80211.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d0afd82..2d3a86f 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5504,11 +5504,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_MATCH])
+ if (info->attrs[NL80211_ATTR_SCHED_SCAN_MATCH]) {
nla_for_each_nested(attr,
info->attrs[NL80211_ATTR_SCHED_SCAN_MATCH],
- tmp)
- n_match_sets++;
+ tmp) {
+ nla_parse(tb, NL80211_SCHED_SCAN_MATCH_ATTR_MAX,
+ nla_data(attr), nla_len(attr),
+ nl80211_match_policy);
+ if (tb[NL80211_SCHED_SCAN_MATCH_ATTR_SSID])
+ n_match_sets++;
+ }
+ }
if (n_match_sets > wiphy->max_match_sets)
return -EINVAL;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] nl80211: Count only ssid attr to know number of match sets
2014-01-23 4:27 [PATCH] nl80211: Count only ssid attr to know number of match sets Raja Mani
@ 2014-01-23 8:17 ` Johannes Berg
2014-01-23 9:53 ` Mani, Raja
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2014-01-23 8:17 UTC (permalink / raw)
To: Raja Mani; +Cc: linville, linux-wireless
On Thu, 2014-01-23 at 09:57 +0530, Raja Mani wrote:
> NL80211_ATTR_SCHED_SCAN_MATCH can have NL80211_SCHED_SCAN_MATCH_ATTR_SSID
> as well as NL80211_SCHED_SCAN_MATCH_ATTR_RSSI in it. Each nested attributes
> which are part of NL80211_ATTR_SCHED_SCAN_MATCH are counted to find out
> number of match sets in nl80211_start_sched_scan().
>
> This is the problem if NL80211_SCHED_SCAN_MATCH_ATTR_RSSI also part of
> NL80211_ATTR_SCHED_SCAN_MATCH. Fix this incorrect calculation by considering
> only NL80211_SCHED_SCAN_MATCH_ATTR_SSID to know match set count.
Err, you failed to explain *why* this is a problem. I'm not even
convinced you're right anyway.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] nl80211: Count only ssid attr to know number of match sets
2014-01-23 8:17 ` Johannes Berg
@ 2014-01-23 9:53 ` Mani, Raja
2014-01-23 9:59 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Mani, Raja @ 2014-01-23 9:53 UTC (permalink / raw)
To: Johannes Berg; +Cc: linville, linux-wireless
Pj4gTkw4MDIxMV9BVFRSX1NDSEVEX1NDQU5fTUFUQ0ggY2FuIGhhdmUgDQo+PiBOTDgwMjExX1ND
SEVEX1NDQU5fTUFUQ0hfQVRUUl9TU0lEDQo+PiBhcyB3ZWxsIGFzIE5MODAyMTFfU0NIRURfU0NB
Tl9NQVRDSF9BVFRSX1JTU0kgaW4gaXQuIEVhY2ggbmVzdGVkIA0KPj4gYXR0cmlidXRlcyB3aGlj
aCBhcmUgcGFydCBvZiBOTDgwMjExX0FUVFJfU0NIRURfU0NBTl9NQVRDSCBhcmUgY291bnRlZCAN
Cj4+IHRvIGZpbmQgb3V0IG51bWJlciBvZiBtYXRjaCBzZXRzIGluIG5sODAyMTFfc3RhcnRfc2No
ZWRfc2NhbigpLg0KPj4gDQo+PiBUaGlzIGlzIHRoZSBwcm9ibGVtIGlmIE5MODAyMTFfU0NIRURf
U0NBTl9NQVRDSF9BVFRSX1JTU0kgYWxzbyBwYXJ0IG9mIA0KPj4gTkw4MDIxMV9BVFRSX1NDSEVE
X1NDQU5fTUFUQ0guIEZpeCB0aGlzIGluY29ycmVjdCBjYWxjdWxhdGlvbiBieSANCj4+IGNvbnNp
ZGVyaW5nIG9ubHkgTkw4MDIxMV9TQ0hFRF9TQ0FOX01BVENIX0FUVFJfU1NJRCB0byBrbm93IG1h
dGNoIHNldCBjb3VudC4NCj4NCj5FcnIsIHlvdSBmYWlsZWQgdG8gZXhwbGFpbiAqd2h5KiB0aGlz
IGlzIGEgcHJvYmxlbS4gSSdtIG5vdCBldmVuIGNvbnZpbmNlZCB5b3UncmUgcmlnaHQgYW55d2F5
Lg0KPg0KPmpvaGFubmVzDQoNClRvIGtub3cgbWF0Y2ggc2V0IGNvdW50LCBubDgwMjExIHNob3Vs
ZCBjb3VudCBvbmx5IE5MODAyMTFfU0NIRURfKl9BVFRSX1NTSUQuDQpTb21lIGNhc2VzLCB3cGFf
c3VwcGxpY2FudCB3b3VsZCBwYXNzIE5MODAyMTFfU0NIRURfKl9BVFRSX1JTU0kgYWxzbyBhbG9u
ZSB3aXRoDQpOTDgwMjExX1NDSEVEXypfQVRUUl9TU0lELiAgRm9yIGV4YW1wbGUsIGFzc3VtZSAg
b25lIE5MODAyMTFfU0NIRURfKl9BVFRSX1NTSUQgYW5kDQpvbmUgTkw4MDIxMV9TQ0hFRF8qX0FU
VFJfUlNTSSBwYXNzZWQgZnJvbSB3cGFfc3VwcGxpY2FudC4gSW4gdGhpcyBjYXNlLCBubDgwMjEx
IGNvdW50DQpOTDgwMjExX1NDSEVEXypfQVRUUl9SU1NJIGFsc28gb25lIG9mIHRoZSBwcm9maWxl
ICwgRmluYWxseSBtYXRjaCBzZXQgY291bnQgd2lsbCBiZSBjb21lDQoyIGluIHRoaXMgY2FzZS4g
KElkZWFsbHksIG1hdGNoIHNldCBjb3VudCBzaG91bGQgYmUgMSkuICBUaGUgc2FtZSBjb3VudCB2
YWx1ZSBpcyBwYXNzZWQgdG8gZHJpdmVyLA0KYW5kIGRyaXZlciBhbHNvIHdvdWxkIGFzc3VtZSB0
aGVyZSBhcmUgdHdvIFNTSUQgYXZhaWxhYmxlIGluIG1hdGNoIHNldCBhcnJheSAocmVxdWVzdC0+
bWF0Y2hfc2V0cykuDQpCdXQgb25seSBvbmUgU1NJRCBpcyB2YWxpZCBpbiByZXF1ZXN0LT5tYXRj
aF9zZXRzIGFycmF5IGluIHRoaXMgY2FzZS4NCg0KVGhhbmtzLA0KUmFqYQ0KDQoNCg==
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nl80211: Count only ssid attr to know number of match sets
2014-01-23 9:53 ` Mani, Raja
@ 2014-01-23 9:59 ` Johannes Berg
2014-01-23 10:46 ` Mani, Raja
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2014-01-23 9:59 UTC (permalink / raw)
To: Mani, Raja; +Cc: linville, linux-wireless
On Thu, 2014-01-23 at 09:53 +0000, Mani, Raja wrote:
> >Err, you failed to explain *why* this is a problem. I'm not even convinced you're right anyway.
> To know match set count, nl80211 should count only NL80211_SCHED_*_ATTR_SSID.
> Some cases, wpa_supplicant would pass NL80211_SCHED_*_ATTR_RSSI also alone with
> NL80211_SCHED_*_ATTR_SSID. For example, assume one NL80211_SCHED_*_ATTR_SSID and
> one NL80211_SCHED_*_ATTR_RSSI passed from wpa_supplicant. In this case, nl80211 count
> NL80211_SCHED_*_ATTR_RSSI also one of the profile , Finally match set count will be come
> 2 in this case. (Ideally, match set count should be 1). The same count value is passed to driver,
> and driver also would assume there are two SSID available in match set array (request->match_sets).
> But only one SSID is valid in request->match_sets array in this case.
You still haven't explained *why* you think that a match set should only
be considered to be one when it includes an SSID. That doesn't seem to
be the definition of the API.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] nl80211: Count only ssid attr to know number of match sets
2014-01-23 9:59 ` Johannes Berg
@ 2014-01-23 10:46 ` Mani, Raja
2014-01-23 10:50 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Mani, Raja @ 2014-01-23 10:46 UTC (permalink / raw)
To: Johannes Berg; +Cc: linville, linux-wireless
Pj4gPkVyciwgeW91IGZhaWxlZCB0byBleHBsYWluICp3aHkqIHRoaXMgaXMgYSBwcm9ibGVtLiBJ
J20gbm90IGV2ZW4gY29udmluY2VkIHlvdSdyZSByaWdodCBhbnl3YXkuDQoNCj4+IFRvIGtub3cg
bWF0Y2ggc2V0IGNvdW50LCBubDgwMjExIHNob3VsZCBjb3VudCBvbmx5IE5MODAyMTFfU0NIRURf
Kl9BVFRSX1NTSUQuDQo+PiBTb21lIGNhc2VzLCB3cGFfc3VwcGxpY2FudCB3b3VsZCBwYXNzIE5M
ODAyMTFfU0NIRURfKl9BVFRSX1JTU0kgYWxzbyANCj4+IGFsb25lIHdpdGggTkw4MDIxMV9TQ0hF
RF8qX0FUVFJfU1NJRC4gIEZvciBleGFtcGxlLCBhc3N1bWUgIG9uZSANCj4+IE5MODAyMTFfU0NI
RURfKl9BVFRSX1NTSUQgYW5kIG9uZSBOTDgwMjExX1NDSEVEXypfQVRUUl9SU1NJIHBhc3NlZCAN
Cj4+IGZyb20gd3BhX3N1cHBsaWNhbnQuIEluIHRoaXMgY2FzZSwgbmw4MDIxMSBjb3VudCANCj4+
IE5MODAyMTFfU0NIRURfKl9BVFRSX1JTU0kgYWxzbyBvbmUgb2YgdGhlIHByb2ZpbGUgLCBGaW5h
bGx5IG1hdGNoIHNldCANCj4+IGNvdW50IHdpbGwgYmUgY29tZQ0KPj4gMiBpbiB0aGlzIGNhc2Uu
IChJZGVhbGx5LCBtYXRjaCBzZXQgY291bnQgc2hvdWxkIGJlIDEpLiAgVGhlIHNhbWUgDQo+PiBj
b3VudCB2YWx1ZSBpcyBwYXNzZWQgdG8gZHJpdmVyLCBhbmQgZHJpdmVyIGFsc28gd291bGQgYXNz
dW1lIHRoZXJlIGFyZSB0d28gU1NJRCBhdmFpbGFibGUgaW4gbWF0Y2ggc2V0IGFycmF5IChyZXF1
ZXN0LT5tYXRjaF9zZXRzKS4NCj4+IEJ1dCBvbmx5IG9uZSBTU0lEIGlzIHZhbGlkIGluIHJlcXVl
c3QtPm1hdGNoX3NldHMgYXJyYXkgaW4gdGhpcyBjYXNlLg0KPg0KPllvdSBzdGlsbCBoYXZlbid0
IGV4cGxhaW5lZCAqd2h5KiB5b3UgdGhpbmsgdGhhdCBhIG1hdGNoIHNldCBzaG91bGQgb25seSBi
ZSBjb25zaWRlcmVkIHRvIGJlIG9uZSB3aGVuIGl0IGluY2x1ZGVzIGFuIFNTSUQuIFRoYXQgZG9l
c24ndCBzZWVtIHRvIGJlIHRoZSBkZWZpbml0aW9uIG9mIHRoZSBBUEkuDQo+DQo+Sm9oYW5uZXMN
Cg0KRHJpdmVyIGl0ZXJhdGVzIG1hdGNoIHNldCBhcnJheSAocmVxdWVzdC0+bWF0Y2hfc2V0cykg
YmFzZWQgb24gbWF0Y2ggc2V0IGNvdW50IChyZXF1ZXN0LT5uX21hdGNoX3NldHMpLg0KSW4gYWJv
dmUgZXhhbXBsZSwgb25seSByZXF1ZXN0LT5uX21hdGNoX3NldHNbMF0gd2lsbCBoYXZlIHZhbGlk
IFNTSUQgYW5kICByZXF1ZXN0LT5uX21hdGNoX3NldHNbMV0pIHdpbGwgaGF2ZSBaRVJPIGluIGl0
cyBlbGVtZW50IGFsd2F5cy4NCkJ5IGhhdmluZyBjb3VudCBiZWluZyAyIGFuZCBoYXZpbmcgaW52
YWxpZCBTU0lEIGluIHJlcXVlc3QtPm5fbWF0Y2hfc2V0c1sxXSBpcyB0aGUgcHJvYmxlbS4gIA0K
DQpNZW1vcnkgZm9yIHJlcXVlc3QtPm1hdGNoX3NldHMgaXMgYWxsb2NhdGVkIGJhc2VkIG9uIG1h
dGNoIHNldCBjb3VudCBpbiBubDgwMjExLiAgQnkgcmVkdWNpbmcgdGhlIGNvdW50ICwgIG5sODAy
MTEgZG9uJ3QgaGF2ZSANClRvIGFsbG9jYXRlIG1lbW9yeSBmb3IgcmVxdWVzdC0+bl9tYXRjaF9z
ZXRzWzFdIGFuZCBhbHNvIGRyaXZlciBjYW4gZXhhY3RseSBrbm93IHZhbGlkIFNTSUQgY291bnQg
YXZhaWxhYmxlIGluIG1hdGNoIHNldCBhcnJheSB3aXRoIG15IGNoYW5nZS4NCg==
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nl80211: Count only ssid attr to know number of match sets
2014-01-23 10:46 ` Mani, Raja
@ 2014-01-23 10:50 ` Johannes Berg
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2014-01-23 10:50 UTC (permalink / raw)
To: Mani, Raja; +Cc: linville, linux-wireless
On Thu, 2014-01-23 at 10:46 +0000, Mani, Raja wrote:
> >> >Err, you failed to explain *why* this is a problem. I'm not even convinced you're right anyway.
>
> >> To know match set count, nl80211 should count only NL80211_SCHED_*_ATTR_SSID.
> >> Some cases, wpa_supplicant would pass NL80211_SCHED_*_ATTR_RSSI also
> >> alone with NL80211_SCHED_*_ATTR_SSID. For example, assume one
> >> NL80211_SCHED_*_ATTR_SSID and one NL80211_SCHED_*_ATTR_RSSI passed
> >> from wpa_supplicant. In this case, nl80211 count
> >> NL80211_SCHED_*_ATTR_RSSI also one of the profile , Finally match set
> >> count will be come
> >> 2 in this case. (Ideally, match set count should be 1). The same
> >> count value is passed to driver, and driver also would assume there are two SSID available in match set array (request->match_sets).
> >> But only one SSID is valid in request->match_sets array in this case.
> >
> >You still haven't explained *why* you think that a match set should only be considered to be one when it includes an SSID. That doesn't seem to be the definition of the API.
> >
> >Johannes
>
> Driver iterates match set array (request->match_sets) based on match set count (request->n_match_sets).
> In above example, only request->n_match_sets[0] will have valid SSID and request->n_match_sets[1]) will have ZERO in its element always.
> By having count being 2 and having invalid SSID in request->n_match_sets[1] is the problem.
>
> Memory for request->match_sets is allocated based on match set count in nl80211. By reducing the count , nl80211 don't have
> To allocate memory for request->n_match_sets[1] and also driver can exactly know valid SSID count available in match set array with my change.
So essentially you're saying the match set API is completely stupid...
yeah that seems to match the code in nl80211_start_sched_scan() where it
looks at the match set...
Anyway, you need a far better commit log.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-01-23 10:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-23 4:27 [PATCH] nl80211: Count only ssid attr to know number of match sets Raja Mani
2014-01-23 8:17 ` Johannes Berg
2014-01-23 9:53 ` Mani, Raja
2014-01-23 9:59 ` Johannes Berg
2014-01-23 10:46 ` Mani, Raja
2014-01-23 10:50 ` Johannes Berg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.