[PATCH v2] cfg80211: Advertise extended capabilities per interface type to userspace
From: "Kanchanapally, Vidyullatha" 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. Signed-off-by: Vidyullatha Kanchanapally Reviewed-by: Jouni Malinen --- v2: * Split NL80211_ATTR_IFTYPE_EXT_CAPA into multiple messages for each interface. * Add check to warn if the globally advertised capabilities are not advertised on all possible interface types. include/net/cfg80211.h | 28 +++- include/uapi/linux/nl80211.h | 7 +++ net/wireless/core.c | 30 ++ net/wireless/nl80211.c | 43 ++- 4 files changed, 106 insertions(+), 2 deletions(-) diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 1e008cd..e6cafa0 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 + * @extended_capabilities: 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. + * @extended_capabilities_mask: mask of the valid values + * @extended_capabilities_len: length of the extended capabilities + */ +struct wiphy_iftype_ext_capab { + enum nl80211_iftype iftype; + const u8 *extended_capabilities; + const u8 *extended_capabilities_mask; + u8 extended_capabilities_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; + const 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 e23d786..a83c62a 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -1819,6 +1819,11 @@ enum nl80211_commands { * * @NL80211_ATTR_PAD: attribute used for padding for 64-bit alignment * + * @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 @@ -2201,6 +2206,8 @@ enum nl80211_attrs { NL80211_ATTR_PAD, + NL80211_ATTR_IFTYPE_EXT_CAPA, + /* add attributes here, update the policy in nl80211.c */ __NL80211_ATTR_AFTER_LAST, diff --git a/net/wireless/core.c b/net/wireless/core.c index 7f7b940..d493da9 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -747,6 +747,36 @@ int wiphy_register(struct wiphy *wiphy) nl80211_send_reg_change_event(&request); } + /* Check that nobody globally advertises any capabilities they do not +* advertise on all possible interface types. +*/ + if (wiph
RE: [PATCH] cfg80211: Advertise extended capabilities per interface type to userspace
Hi Johannes, >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) 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. Thanks Vidyullatha -Original Message- From: Johannes Berg [mailto:johan...@sipsolutions.net] Sent: Sunday, April 17, 2016 3:42 AM To: Kanchanapally, Vidyullatha Cc: linux-wireless@vger.kernel.org; Malinen, Jouni ; Hullur Subramanyam, Amarnath ; Undekari, Sunil Dutt Subject: Re: [PATCH] cfg80211: Advertise extended capabilities per interface type to userspace 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
[PATCH] cfg80211: Advertise extended capabilities per interface type to userspace
From: "Kanchanapally, Vidyullatha" 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 Signed-off-by: Vidyullatha Kanchanapally --- 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, +
RE: [PATCH 1/3] cfg80211: Add option to report the bss entry in connect result
Hi Arend, The bss table entry in kernel will have the channel indicated in the beacon/probe response itself. But here we are addressing a case where the AP is changing its channel (not through CSA but is disconnecting clients on old channel and is coming up on a new channel). In such a scenario, because of the kernel's scan timeout logic, there is a small amount of time where the bss table entry is present for the AP on both the old and new channel. If a connect happens with in this time, the kernel might map the "wdev->current_bss" to a wrong bss entry, which is obtained in " __cfg80211_connect_result" through "cfg80211_get_bss(wdev->wiphy, NULL, ...)". The channel pointer passed here is NULL. The "wdev->current_bss" is held by the kernel through the connection and the bss entry shall not get flushed until a disconnect, although the AP is not seen any more in scan results. The kernel for such a connection will report a wrong channel as associated channel. The current cfg80211 connect indication "cfg80211_connect_result" does not have a provision to indicate either the channel or the bss entry. Hence we are introducing a new cfg80211 api called "cfg80211_connect_bss" which includes a bss entry. Thanks Vidyullatha -Original Message- From: Arend van Spriel [mailto:arend.vanspr...@broadcom.com] Sent: Tuesday, April 12, 2016 2:22 PM To: Kanchanapally, Vidyullatha ; johan...@sipsolutions.net Cc: linux-wireless@vger.kernel.org; Malinen, Jouni ; Undekari, Sunil Dutt ; Hullur Subramanyam, Amarnath Subject: Re: [PATCH 1/3] cfg80211: Add option to report the bss entry in connect result On 11-04-16 11:46, Kanchanapally, Vidyullatha wrote: > From: "Kanchanapally, Vidyullatha" > > Since cfg80211 maintains separate BSS table entries for APs if the > same BSSID, SSID pair is seen on multiple channels, it is possible > that it can map the current_bss to a BSS entry on the wrong channel. > This current_bss will not get flushed unless disconnected and cfg80211 > reports a wrong channel as the associated channel. > > Fix this by introducing a new cfg80211_connect_bss() function which is > similar to cfg80211_connect_result(), but it includes an additional > parameter: the bss the STA is connected to. This allows drivers to > provide the exact bss entry that matches the BSS to which the > connection was completed. How does this work? Is the bss table entry filed with channel it sees the bss during scan or does it use the channel indicated in the beacon/probe reponse. I would expect the latter. Is the AP beaconing on multiple channels using the same BSSID? It would help understand this change with a driver patch. Regards, Arend > Reviewed-by: Jouni Malinen > Signed-off-by: Vidyullatha Kanchanapally > Signed-off-by: Sunil Dutt > --- > include/net/cfg80211.h | 39 +++ > net/wireless/core.h| 1 + > net/wireless/sme.c | 28 ++-- > net/wireless/util.c| 2 +- > 4 files changed, 59 insertions(+), 11 deletions(-) > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index > 858a97ec..51f177c 100644 > --- a/include/net/cfg80211.h > +++ b/include/net/cfg80211.h > @@ -4648,6 +4648,32 @@ static inline void > cfg80211_testmode_event(struct sk_buff *skb, gfp_t gfp) #endif > > /** > + * cfg80211_connect_bss - notify cfg80211 of connection result > + * > + * @dev: network device > + * @bssid: the BSSID of the AP > + * @bss: entry of bss to which STA got connected to, can be obtained > + * through cfg80211_get_bss (may be %NULL) > + * @req_ie: association request IEs (maybe be %NULL) > + * @req_ie_len: association request IEs length > + * @resp_ie: association response IEs (may be %NULL) > + * @resp_ie_len: assoc response IEs length > + * @status: status code, 0 for successful connection, use > + * %WLAN_STATUS_UNSPECIFIED_FAILURE if your device cannot give you > + * the real status code for failures. > + * @gfp: allocation flags > + * > + * It should be called by the underlying driver whenever connect() > +has > + * succeeded. This is similar to cfg80211_connect_result(), but with > +the > + * option of identifying the exact bss entry for the connection. Only > +one of > + * these functions should be called. > + */ > +void cfg80211_connect_bss(struct net_device *dev, const u8 *bssid, > + struct cfg80211_bss *bss, const u8 *req_ie, > + size_t req_ie_len, const u8 *resp_ie, > + size_t resp_ie_len, u16 status, gfp_t gfp); > + > +/** > * cfg80211_connect_result - notify cfg80211 of connection result > * > * @dev: network device
[PATCH 1/3] cfg80211: Add option to report the bss entry in connect result
From: "Kanchanapally, Vidyullatha" Since cfg80211 maintains separate BSS table entries for APs if the same BSSID, SSID pair is seen on multiple channels, it is possible that it can map the current_bss to a BSS entry on the wrong channel. This current_bss will not get flushed unless disconnected and cfg80211 reports a wrong channel as the associated channel. Fix this by introducing a new cfg80211_connect_bss() function which is similar to cfg80211_connect_result(), but it includes an additional parameter: the bss the STA is connected to. This allows drivers to provide the exact bss entry that matches the BSS to which the connection was completed. Reviewed-by: Jouni Malinen Signed-off-by: Vidyullatha Kanchanapally Signed-off-by: Sunil Dutt --- include/net/cfg80211.h | 39 +++ net/wireless/core.h| 1 + net/wireless/sme.c | 28 ++-- net/wireless/util.c| 2 +- 4 files changed, 59 insertions(+), 11 deletions(-) diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 858a97ec..51f177c 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -4648,6 +4648,32 @@ static inline void cfg80211_testmode_event(struct sk_buff *skb, gfp_t gfp) #endif /** + * cfg80211_connect_bss - notify cfg80211 of connection result + * + * @dev: network device + * @bssid: the BSSID of the AP + * @bss: entry of bss to which STA got connected to, can be obtained + * through cfg80211_get_bss (may be %NULL) + * @req_ie: association request IEs (maybe be %NULL) + * @req_ie_len: association request IEs length + * @resp_ie: association response IEs (may be %NULL) + * @resp_ie_len: assoc response IEs length + * @status: status code, 0 for successful connection, use + * %WLAN_STATUS_UNSPECIFIED_FAILURE if your device cannot give you + * the real status code for failures. + * @gfp: allocation flags + * + * It should be called by the underlying driver whenever connect() has + * succeeded. This is similar to cfg80211_connect_result(), but with the + * option of identifying the exact bss entry for the connection. Only one of + * these functions should be called. + */ +void cfg80211_connect_bss(struct net_device *dev, const u8 *bssid, + struct cfg80211_bss *bss, const u8 *req_ie, + size_t req_ie_len, const u8 *resp_ie, + size_t resp_ie_len, u16 status, gfp_t gfp); + +/** * cfg80211_connect_result - notify cfg80211 of connection result * * @dev: network device @@ -4664,10 +4690,15 @@ static inline void cfg80211_testmode_event(struct sk_buff *skb, gfp_t gfp) * It should be called by the underlying driver whenever connect() has * succeeded. */ -void cfg80211_connect_result(struct net_device *dev, const u8 *bssid, -const u8 *req_ie, size_t req_ie_len, -const u8 *resp_ie, size_t resp_ie_len, -u16 status, gfp_t gfp); +static inline void +cfg80211_connect_result(struct net_device *dev, const u8 *bssid, + const u8 *req_ie, size_t req_ie_len, + const u8 *resp_ie, size_t resp_ie_len, + u16 status, gfp_t gfp) +{ + cfg80211_connect_bss(dev, bssid, NULL, req_ie, req_ie_len, resp_ie, +resp_ie_len, status, gfp); +} /** * cfg80211_roamed - notify cfg80211 of roaming diff --git a/net/wireless/core.h b/net/wireless/core.h index 64c70e4..f3b0a07 100644 --- a/net/wireless/core.h +++ b/net/wireless/core.h @@ -214,6 +214,7 @@ struct cfg80211_event { const u8 *resp_ie; size_t req_ie_len; size_t resp_ie_len; + struct cfg80211_bss *bss; u16 status; } cr; struct { diff --git a/net/wireless/sme.c b/net/wireless/sme.c index 1fba416..da97424d 100644 --- a/net/wireless/sme.c +++ b/net/wireless/sme.c @@ -753,19 +753,32 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid, kfree(country_ie); } -void cfg80211_connect_result(struct net_device *dev, const u8 *bssid, -const u8 *req_ie, size_t req_ie_len, -const u8 *resp_ie, size_t resp_ie_len, -u16 status, gfp_t gfp) +/* Consumes bss object one way or another */ +void cfg80211_connect_bss(struct net_device *dev, const u8 *bssid, + struct cfg80211_bss *bss, const u8 *req_ie, + size_t req_ie_len, const u8 *resp_ie, + size_t resp_ie_len, u16 status, gfp_t gfp) { struct wireless_dev *wdev = dev->ieee80211_ptr; struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy); struct cfg80211_event *ev; unsigned long flags; + if (bss) { + /* M