From: "Kanchanapally, Vidyullatha" <[email protected]>
The driver extended capabilities may differ for different
interface types which the userspace needs to know (for
example the fine timing measurement initiator and responder
bits might differ for a station and AP). Add a new nl80211
attribute to provide extended capabilities per interface type
to userspace.
Reviewed-by: Jouni Malinen <[email protected]>
Signed-off-by: Vidyullatha Kanchanapally <[email protected]>
---
include/net/cfg80211.h | 28 +++++++++++++++++++++++++++-
include/uapi/linux/nl80211.h | 7 +++++++
net/wireless/nl80211.c | 33 +++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 1e008cd..c66a374 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3080,6 +3080,24 @@ struct wiphy_vendor_command {
};
/**
+ * struct wiphy_iftype_ext_capab - extended capabilities per interface type
+ * @iftype: interface type
+ * @ext_capab: extended capabilities supported by the driver,
+ * additional capabilities might be supported by userspace; these are
+ * the 802.11 extended capabilities ("Extended Capabilities element")
+ * and are in the same format as in the information element. See
+ * IEEE Std 802.11-2012 8.4.2.29 for the defined fields.
+ * @ext_capab_mask: mask of the valid values
+ * @ext_capab_len: length of the extended capabilities
+ */
+struct wiphy_iftype_ext_capab {
+ enum nl80211_iftype iftype;
+ const u8 *ext_capab;
+ const u8 *ext_capab_mask;
+ u8 ext_capab_len;
+};
+
+/**
* struct wiphy - wireless hardware description
* @reg_notifier: the driver's regulatory notification callback,
* note that if your driver uses wiphy_apply_custom_regulatory()
@@ -3196,9 +3214,14 @@ struct wiphy_vendor_command {
* additional capabilities might be supported by userspace; these are
* the 802.11 extended capabilities ("Extended Capabilities element")
* and are in the same format as in the information element. See
- * 802.11-2012 8.4.2.29 for the defined fields.
+ * 802.11-2012 8.4.2.29 for the defined fields. These are the default
+ * extended capabilities to be used if the capabilities are not specified
+ * for a specific interface type in iftype_ext_capab.
* @extended_capabilities_mask: mask of the valid values
* @extended_capabilities_len: length of the extended capabilities
+ * @iftype_ext_capab: array of extended capabilities per interface type
+ * @num_iftype_ext_capab: number of interface types for which extended
+ * capabilities are specified separately.
* @coalesce: packet coalescing support information
*
* @vendor_commands: array of vendor commands supported by the hardware
@@ -3298,6 +3321,9 @@ struct wiphy {
const u8 *extended_capabilities, *extended_capabilities_mask;
u8 extended_capabilities_len;
+ struct wiphy_iftype_ext_capab *iftype_ext_capab;
+ unsigned int num_iftype_ext_capab;
+
/* If multiple wiphys are registered and you're handed e.g.
* a regular netdev with assigned ieee80211_ptr, you won't
* know whether it points to a wiphy your driver has registered
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 9baa20b..8c3eae1 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1817,6 +1817,11 @@ enum nl80211_commands {
* @NL80211_ATTR_STA_SUPPORT_P2P_PS: whether P2P PS mechanism supported
* or not. u8, one of the values of &enum nl80211_sta_p2p_ps_status
*
+ * @NL80211_ATTR_IFTYPE_EXT_CAPA: Nested attribute of the following attributes:
+ * %NL80211_ATTR_IFTYPE, %NL80211_ATTR_EXT_CAPA,
+ * %NL80211_ATTR_EXT_CAPA_MASK, to specify the extended capabilities per
+ * interface type.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2197,6 +2202,8 @@ enum nl80211_attrs {
NL80211_ATTR_STA_SUPPORT_P2P_PS,
+ NL80211_ATTR_IFTYPE_EXT_CAPA,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 3514450..94b9791 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1761,6 +1761,39 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
nla_nest_end(msg, nested);
}
+ state->split_start++;
+ break;
+ case 13:
+ if (rdev->wiphy.num_iftype_ext_capab &&
+ rdev->wiphy.iftype_ext_capab) {
+ struct nlattr *nested_ext_capab, *nested;
+
+ nested = nla_nest_start(msg,
+ NL80211_ATTR_IFTYPE_EXT_CAPA);
+ if (!nested)
+ goto nla_put_failure;
+
+ for (i = 0; i < rdev->wiphy.num_iftype_ext_capab; i++) {
+ struct wiphy_iftype_ext_capab *capab;
+
+ capab = &rdev->wiphy.iftype_ext_capab[i];
+ nested_ext_capab = nla_nest_start(msg, i);
+ if (!nested_ext_capab ||
+ nla_put_u32(msg, NL80211_ATTR_IFTYPE,
+ capab->iftype) ||
+ nla_put(msg, NL80211_ATTR_EXT_CAPA,
+ capab->ext_capab_len,
+ capab->ext_capab) ||
+ nla_put(msg, NL80211_ATTR_EXT_CAPA_MASK,
+ capab->ext_capab_len,
+ capab->ext_capab_mask))
+ goto nla_put_failure;
+
+ nla_nest_end(msg, nested_ext_capab);
+ }
+ nla_nest_end(msg, nested);
+ }
+
/* done */
state->split_start = 0;
break;
--
1.8.2.1
On Fri, 2016-04-15 at 16:57 +0530, Kanchanapally, Vidyullatha wrote:
>
> +struct wiphy_iftype_ext_capab {
> + enum nl80211_iftype iftype;
> + const u8 *ext_capab;
> + const u8 *ext_capab_mask;
> + u8 ext_capab_len;
I think you should reuse the struct member names that we used before -
that will make grepping for them easier.
> + struct wiphy_iftype_ext_capab *iftype_ext_capab;
const
> + * @NL80211_ATTR_IFTYPE_EXT_CAPA: Nested attribute of the following
> attributes:
> + * %NL80211_ATTR_IFTYPE, %NL80211_ATTR_EXT_CAPA,
> + * %NL80211_ATTR_EXT_CAPA_MASK, to specify the extended
> capabilities per
> + * interface type.
That's a bit awkward to parse, since you have to parse the big
attribute list again, but I guess it still makes more sense than having
entirely different attributes.
> + state->split_start++;
> + break;
> + case 13:
> + if (rdev->wiphy.num_iftype_ext_capab &&
> + rdev->wiphy.iftype_ext_capab) {
> + struct nlattr *nested_ext_capab, *nested;
> +
> + nested = nla_nest_start(msg,
> + NL80211_ATTR_IFTYPE_
> EXT_CAPA);
> + if (!nested)
> + goto nla_put_failure;
> +
> + for (i = 0; i < rdev-
> >wiphy.num_iftype_ext_capab; i++) {
> + struct wiphy_iftype_ext_capab
> *capab;
> +
> + capab = &rdev-
> >wiphy.iftype_ext_capab[i];
> + nested_ext_capab =
> nla_nest_start(msg, i);
> + if (!nested_ext_capab ||
> + nla_put_u32(msg,
> NL80211_ATTR_IFTYPE,
> + capab->iftype) ||
> + nla_put(msg,
> NL80211_ATTR_EXT_CAPA,
> + capab->ext_capab_len,
> + capab->ext_capab) ||
> + nla_put(msg,
> NL80211_ATTR_EXT_CAPA_MASK,
> + capab->ext_capab_len,
> + capab->ext_capab_mask))
> + goto nla_put_failure;
> +
> + nla_nest_end(msg, nested_ext_capab);
> + }
> + nla_nest_end(msg, nested);
> + }
I think we should consider allowing this to be split multiple messages
(for different interface types), since this can potentially get rather
long (interface types x 2 x capa len). OTOH, we'd have to get a LOT of
interface types x capabilities to hit the limit of 4k, not sure. But it
should just require a few lines of code to do this.
Also, if it's possible, perhaps we should consider checking that nobody
globally advertises any capabilities they don't advertise on all
possible interface types?
I'm assuming that not listing an interface type would mean that the
global defaults apply, but old userspace will also not know about the
per-interface, so you shouldn't list anything there.
So I'm thinking something like
supported_on_all = iftype_ext_capab[0]
for i in 1..num_iftype_ext_capab-1:
supported_on_all &= iftype_ext_capab[i]
WARN_ON(wiphy->ext_capa_mask & ~supported_on_all)
(which is obviously some kind of pseudo-code.)
johannes
SGkgSm9oYW5uZXMsDQoNCj5BbHNvLCBpZiBpdCdzIHBvc3NpYmxlLCBwZXJoYXBzIHdlIHNob3Vs
ZCBjb25zaWRlciBjaGVja2luZyB0aGF0IG5vYm9keSBnbG9iYWxseSBhZHZlcnRpc2VzIGFueSBj
YXBhYmlsaXRpZXMgdGhleSBkb24ndCBhZHZlcnRpc2Ugb24gYWxsIHBvc3NpYmxlIGludGVyZmFj
ZSB0eXBlcz8NCg0KPkknbSBhc3N1bWluZyB0aGF0IG5vdCBsaXN0aW5nIGFuIGludGVyZmFjZSB0
eXBlIHdvdWxkIG1lYW4gdGhhdCB0aGUgZ2xvYmFsIGRlZmF1bHRzIGFwcGx5LCBidXQgb2xkIHVz
ZXJzcGFjZSB3aWxsIGFsc28gbm90IGtub3cgYWJvdXQgdGhlIHBlci1pbnRlcmZhY2UsIHNvIHlv
dSBzaG91bGRuJ3QgbGlzdCBhbnl0aGluZyB0aGVyZS4NCg0KPlNvIEknbSB0aGlua2luZyBzb21l
dGhpbmcgbGlrZQ0KDQo+c3VwcG9ydGVkX29uX2FsbCA9IGlmdHlwZV9leHRfY2FwYWJbMF0NCj5m
b3IgaSBpbiAxLi5udW1faWZ0eXBlX2V4dF9jYXBhYi0xOg0KPiAgICBzdXBwb3J0ZWRfb25fYWxs
ICY9IGlmdHlwZV9leHRfY2FwYWJbaV0gV0FSTl9PTih3aXBoeS0+ZXh0X2NhcGFfbWFzayAmIH5z
dXBwb3J0ZWRfb25fYWxsKQ0KDQpXZSB3ZXJlIHRoaW5raW5nIHdoZXRoZXIgdGhpcyBpcyBhbiBh
cHByb3ByaWF0ZSB2YWxpZGF0aW9uIG9yIG5vdCBzaW5jZSB3ZSBjYW5ub3QgYmUgc3VyZSBob3cg
dGhlIEV4dGVuZGVkIENhcGFiaWxpdGllcyBhcmUgZGVmaW5lZC4NClRoZXkgbmVlZCBub3QgbmVj
ZXNzYXJpbHkgYmUgYWxsIHBvc2l0aXZlIGNhcGFiaWxpdGllcywgdGhleSBjb3VsZCBjb3VwbGUg
Ym90aCB0aGUgcG9zaXRpdmUgYW5kIG5lZ2F0aXZlIGNhcGFiaWxpdGllcyBhcyB3ZWxsLg0KUGxl
YXNlIGxldCB1cyBrbm93IGlmIHRoaXMgY2hhbmdlIGlzIHJlYWxseSBuZWVkZWQuDQoNClRoYW5r
cw0KVmlkeXVsbGF0aGENCg0KLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IEpvaGFu
bmVzIEJlcmcgW21haWx0bzpqb2hhbm5lc0BzaXBzb2x1dGlvbnMubmV0XSANClNlbnQ6IFN1bmRh
eSwgQXByaWwgMTcsIDIwMTYgMzo0MiBBTQ0KVG86IEthbmNoYW5hcGFsbHksIFZpZHl1bGxhdGhh
IDx2a2FuY2hhbkBxdGkucXVhbGNvbW0uY29tPg0KQ2M6IGxpbnV4LXdpcmVsZXNzQHZnZXIua2Vy
bmVsLm9yZzsgTWFsaW5lbiwgSm91bmkgPGpvdW5pQHFjYS5xdWFsY29tbS5jb20+OyBIdWxsdXIg
U3VicmFtYW55YW0sIEFtYXJuYXRoIDxhbWFybmF0aEBxY2EucXVhbGNvbW0uY29tPjsgVW5kZWth
cmksIFN1bmlsIER1dHQgPHVzZHV0dEBxdGkucXVhbGNvbW0uY29tPg0KU3ViamVjdDogUmU6IFtQ
QVRDSF0gY2ZnODAyMTE6IEFkdmVydGlzZSBleHRlbmRlZCBjYXBhYmlsaXRpZXMgcGVyIGludGVy
ZmFjZSB0eXBlIHRvIHVzZXJzcGFjZQ0KDQpPbiBGcmksIDIwMTYtMDQtMTUgYXQgMTY6NTcgKzA1
MzAsIEthbmNoYW5hcGFsbHksIFZpZHl1bGxhdGhhIHdyb3RlOg0KPsKgDQo+ICtzdHJ1Y3Qgd2lw
aHlfaWZ0eXBlX2V4dF9jYXBhYiB7DQo+ICsJZW51bSBubDgwMjExX2lmdHlwZSBpZnR5cGU7DQo+
ICsJY29uc3QgdTggKmV4dF9jYXBhYjsNCj4gKwljb25zdCB1OCAqZXh0X2NhcGFiX21hc2s7DQo+
ICsJdTggZXh0X2NhcGFiX2xlbjsNCg0KSSB0aGluayB5b3Ugc2hvdWxkIHJldXNlIHRoZSBzdHJ1
Y3QgbWVtYmVyIG5hbWVzIHRoYXQgd2UgdXNlZCBiZWZvcmUgLSB0aGF0IHdpbGwgbWFrZSBncmVw
cGluZyBmb3IgdGhlbSBlYXNpZXIuDQoNCj4gKwlzdHJ1Y3Qgd2lwaHlfaWZ0eXBlX2V4dF9jYXBh
YiAqaWZ0eXBlX2V4dF9jYXBhYjsNCg0KY29uc3QNCg0KPiArICogQE5MODAyMTFfQVRUUl9JRlRZ
UEVfRVhUX0NBUEE6IE5lc3RlZCBhdHRyaWJ1dGUgb2YgdGhlIGZvbGxvd2luZw0KPiBhdHRyaWJ1
dGVzOg0KPiArICoJJU5MODAyMTFfQVRUUl9JRlRZUEUsICVOTDgwMjExX0FUVFJfRVhUX0NBUEEs
DQo+ICsgKgklTkw4MDIxMV9BVFRSX0VYVF9DQVBBX01BU0ssIHRvIHNwZWNpZnkgdGhlIGV4dGVu
ZGVkDQo+IGNhcGFiaWxpdGllcyBwZXINCj4gKyAqCWludGVyZmFjZSB0eXBlLg0KDQpUaGF0J3Mg
YSBiaXQgYXdrd2FyZCB0byBwYXJzZSwgc2luY2UgeW91IGhhdmUgdG8gcGFyc2UgdGhlIGJpZyBh
dHRyaWJ1dGUgbGlzdCBhZ2FpbiwgYnV0IEkgZ3Vlc3MgaXQgc3RpbGwgbWFrZXMgbW9yZSBzZW5z
ZSB0aGFuIGhhdmluZyBlbnRpcmVseSBkaWZmZXJlbnQgYXR0cmlidXRlcy4NCsKgDQo+ICsJCXN0
YXRlLT5zcGxpdF9zdGFydCsrOw0KPiArCQlicmVhazsNCj4gKwljYXNlIDEzOg0KPiArCQlpZiAo
cmRldi0+d2lwaHkubnVtX2lmdHlwZV9leHRfY2FwYWIgJiYNCj4gKwkJwqDCoMKgwqByZGV2LT53
aXBoeS5pZnR5cGVfZXh0X2NhcGFiKSB7DQo+ICsJCQlzdHJ1Y3QgbmxhdHRyICpuZXN0ZWRfZXh0
X2NhcGFiLCAqbmVzdGVkOw0KPiArDQo+ICsJCQluZXN0ZWQgPSBubGFfbmVzdF9zdGFydChtc2cs
DQo+ICsJCQkJCQlOTDgwMjExX0FUVFJfSUZUWVBFXw0KPiBFWFRfQ0FQQSk7DQo+ICsJCQlpZiAo
IW5lc3RlZCkNCj4gKwkJCQlnb3RvIG5sYV9wdXRfZmFpbHVyZTsNCj4gKw0KPiArCQkJZm9yIChp
ID0gMDsgaSA8IHJkZXYtDQo+ID53aXBoeS5udW1faWZ0eXBlX2V4dF9jYXBhYjsgaSsrKSB7DQo+
ICsJCQkJc3RydWN0IHdpcGh5X2lmdHlwZV9leHRfY2FwYWINCj4gKmNhcGFiOw0KPiArDQo+ICsJ
CQkJY2FwYWIgPSAmcmRldi0NCj4gPndpcGh5LmlmdHlwZV9leHRfY2FwYWJbaV07DQo+ICsJCQkJ
bmVzdGVkX2V4dF9jYXBhYiA9DQo+IG5sYV9uZXN0X3N0YXJ0KG1zZywgaSk7DQo+ICsJCQkJaWYg
KCFuZXN0ZWRfZXh0X2NhcGFiIHx8DQo+ICsJCQkJwqDCoMKgwqBubGFfcHV0X3UzMihtc2csDQo+
IE5MODAyMTFfQVRUUl9JRlRZUEUsDQo+ICsJCQkJCQljYXBhYi0+aWZ0eXBlKSB8fA0KPiArCQkJ
CcKgwqDCoMKgbmxhX3B1dChtc2csDQo+IE5MODAyMTFfQVRUUl9FWFRfQ0FQQSwNCj4gKwkJCQkJ
wqDCoMKgwqBjYXBhYi0+ZXh0X2NhcGFiX2xlbiwNCj4gKwkJCQkJwqDCoMKgwqBjYXBhYi0+ZXh0
X2NhcGFiKSB8fA0KPiArCQkJCcKgwqDCoMKgbmxhX3B1dChtc2csDQo+IE5MODAyMTFfQVRUUl9F
WFRfQ0FQQV9NQVNLLA0KPiArCQkJCQnCoMKgwqDCoGNhcGFiLT5leHRfY2FwYWJfbGVuLA0KPiAr
CQkJCQnCoMKgwqDCoGNhcGFiLT5leHRfY2FwYWJfbWFzaykpDQo+ICsJCQkJCWdvdG8gbmxhX3B1
dF9mYWlsdXJlOw0KPiArDQo+ICsJCQkJbmxhX25lc3RfZW5kKG1zZywgbmVzdGVkX2V4dF9jYXBh
Yik7DQo+ICsJCQl9DQo+ICsJCQlubGFfbmVzdF9lbmQobXNnLCBuZXN0ZWQpOw0KPiArCQl9DQoN
CkkgdGhpbmsgd2Ugc2hvdWxkIGNvbnNpZGVyIGFsbG93aW5nIHRoaXMgdG8gYmUgc3BsaXQgbXVs
dGlwbGUgbWVzc2FnZXMgKGZvciBkaWZmZXJlbnQgaW50ZXJmYWNlIHR5cGVzKSwgc2luY2UgdGhp
cyBjYW4gcG90ZW50aWFsbHkgZ2V0IHJhdGhlciBsb25nIChpbnRlcmZhY2UgdHlwZXMgeCAyIHgg
Y2FwYSBsZW4pLiBPVE9ILCB3ZSdkIGhhdmUgdG8gZ2V0IGEgTE9UIG9mIGludGVyZmFjZSB0eXBl
cyB4IGNhcGFiaWxpdGllcyB0byBoaXQgdGhlIGxpbWl0IG9mIDRrLCBub3Qgc3VyZS4gQnV0IGl0
IHNob3VsZCBqdXN0IHJlcXVpcmUgYSBmZXcgbGluZXMgb2YgY29kZSB0byBkbyB0aGlzLg0KDQoN
CkFsc28sIGlmIGl0J3MgcG9zc2libGUsIHBlcmhhcHMgd2Ugc2hvdWxkIGNvbnNpZGVyIGNoZWNr
aW5nIHRoYXQgbm9ib2R5IGdsb2JhbGx5IGFkdmVydGlzZXMgYW55IGNhcGFiaWxpdGllcyB0aGV5
IGRvbid0IGFkdmVydGlzZSBvbiBhbGwgcG9zc2libGUgaW50ZXJmYWNlIHR5cGVzPw0KDQpJJ20g
YXNzdW1pbmcgdGhhdCBub3QgbGlzdGluZyBhbiBpbnRlcmZhY2UgdHlwZSB3b3VsZCBtZWFuIHRo
YXQgdGhlIGdsb2JhbCBkZWZhdWx0cyBhcHBseSwgYnV0IG9sZCB1c2Vyc3BhY2Ugd2lsbCBhbHNv
IG5vdCBrbm93IGFib3V0IHRoZSBwZXItaW50ZXJmYWNlLCBzbyB5b3Ugc2hvdWxkbid0IGxpc3Qg
YW55dGhpbmcgdGhlcmUuDQoNClNvIEknbSB0aGlua2luZyBzb21ldGhpbmcgbGlrZQ0KDQpzdXBw
b3J0ZWRfb25fYWxsID0gaWZ0eXBlX2V4dF9jYXBhYlswXQ0KZm9yIGkgaW4gMS4ubnVtX2lmdHlw
ZV9leHRfY2FwYWItMToNCsKgIMKgIHN1cHBvcnRlZF9vbl9hbGwgJj0gaWZ0eXBlX2V4dF9jYXBh
YltpXSBXQVJOX09OKHdpcGh5LT5leHRfY2FwYV9tYXNrICYgfnN1cHBvcnRlZF9vbl9hbGwpDQoN
Cih3aGljaCBpcyBvYnZpb3VzbHkgc29tZSBraW5kIG9mIHBzZXVkby1jb2RlLikNCg0Kam9oYW5u
ZXMNCg==
On Fri, 2016-04-22 at 14:52 +0000, Kanchanapally, Vidyullatha wrote:
>
> > So I'm thinking something like
> >
> > supported_on_all = iftype_ext_capab[0]
> > for i in 1..num_iftype_ext_capab-1:
> > supported_on_all &= iftype_ext_capab[i]
> > WARN_ON(wiphy->ext_capa_mask & ~supported_on_all)
> We were thinking whether this is an appropriate validation or not
> since we cannot be sure how the Extended Capabilities are defined.
> They need not necessarily be all positive capabilities, they could
> couple both the positive and negative capabilities as well.
> Please let us know if this change is really needed.
I'm ambivalent about this. I don't think it makes sense to see drivers
that register both, i.e. would result in the warning I suggested. I
don't think *negative* capabilities can really be added, since the spec
always assumes that capabilities that you don't list are zero.
In considering multi-bit values, you might have a point, if - for
example - you had a MAX-MSDU-IN-AMSDU value of 0b11 for some
interfaces, and 0b01 for all the others.
Realistically though, does that make sense?
I would expect this to be the only multi-bit field that might ever be
supported this way, since in the future drivers would likely only
specify the current subset of capabilities in the wiphy ext_capab, and
put all newer extensions into the per-iftype ext_capa, assuming that
they're running on a newer supplicant.
So on the whole, I don't really see a good reason to not do pretty much
exactly the (pseudo)code I wrote above; perhaps you can come up with a
better example than I tried?
johannes