| Message ID | 20200706115219.663650-1-john@phrozen.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [V2,01/10] nl80211: add basic multiple bssid support | expand |
> +++ b/include/net/cfg80211.h > @@ -604,6 +604,8 @@ struct vif_params { > u8 macaddr[ETH_ALEN]; > const u8 *vht_mumimo_groups; > const u8 *vht_mumimo_follow_addr; > + enum nl80211_multi_bssid_mode multi_bssid_mode; > + u32 multi_bssid_parent; Can you put this into a(n anonymous) sub-structure? This also missed kernel-doc. > * unprotected beacon report > + * @multi_bssid_mode: Is this a legacy, transmitted or non-transmitted bssid > + * @multi_bssid_parent: a non-transmitted bssid has a transmitted parent > + * @multi_bssid_list: linked list for tracking parent - child relations. FWIW, you can (now?) write * @multi_bssid.list: ... to document data for anonymous sub-structures. I started applying this and changed it myself, but am having second thoughts on later patches in this series. > + * @NL80211_ATTR_MULTI_BSSID_MODE: Set the (Non-)Transmitted flag for this > + * BSSIDs beacon. > + * > + * @NL80211_ATTR_MULTI_BSSID_PARENT: If this is a Non-Transmitted BSSID, define > + * the parent interface. Maybe clarify the parent (transmitted BSSID) interface or so? > +/** > + * enum nl80211_multi_bssid_mode - Multiple BSSID beacon type > + * > + * Used by cfg80211_ap_settings That'd be weird, but it's not true, you have it for NL80211_ATTR_MULTI_BSSID_MODE. Actually, that documentation should point here and say the values are from this enum. > + * @MULTIPLE_BSSID_LEGACY: This BSS is not part of a multiple BSSID group > + * @MULTIPLE_BSSID_TRANSMITTED: This BSS is broadcasting a multiple BSSID > + * beacon Please just use a single tab there :) > + [NL80211_ATTR_MULTI_BSSID_MODE] = NLA_POLICY_RANGE(NLA_U8, > + NL80211_MULTIPLE_BSSID_LEGACY, > + NL80211_MULTIPLE_BSSID_NON_TRANSMITTED), Maybe nicer as [...] = NLA_POLICY_RANGE(... johannes
On Mon, 2020-07-06 at 13:52 +0200, John Crispin wrote: > /** > @@ -5109,6 +5111,9 @@ struct cfg80211_cqm_config; > * @pmsr_free_wk: (private) peer measurements cleanup work > * @unprot_beacon_reported: (private) timestamp of last > * unprotected beacon report > + * @multi_bssid_mode: Is this a legacy, transmitted or non-transmitted bssid > + * @multi_bssid_parent: a non-transmitted bssid has a transmitted parent > + * @multi_bssid_list: linked list for tracking parent - child relations. > */ > struct wireless_dev { > struct wiphy *wiphy; > @@ -5188,6 +5193,10 @@ struct wireless_dev { > struct work_struct pmsr_free_wk; > > unsigned long unprot_beacon_reported; > + > + enum nl80211_multi_bssid_mode multi_bssid_mode; > + struct wireless_dev *multi_bssid_parent; > + struct list_head multi_bssid_list; Ah, I forgot - this was where I thought it'd be better to change ... after reading the later patches. Or let me just ask: why is that here? You're passing it down in the params (code I removed above), and then you're effectively making it the driver's (mac80211's) responsibility to track it. So why should it be here? cfg80211 isn't really using this much. I saw there's a small validation thing here, but that could also be pushed down, and then this can live in mac80211? Or do you have any future plans for using this in cfg80211? johannes
On Mon, 2020-07-06 at 13:52 +0200, John Crispin wrote: > > @@ -3761,6 +3765,16 @@ static int nl80211_new_interface(struct sk_buff *skb, struct genl_info *info) > if (err < 0) > return err; > > + if (info->attrs[NL80211_ATTR_MULTI_BSSID_MODE]) > + params.multi_bssid_mode = > + nla_get_u8(info->attrs[NL80211_ATTR_MULTI_BSSID_MODE]); [...] Oh .. missed this completely until I got to the iw patch :) Why are you adding this in *new interface? IMHO it would be more applicable to "start_ap"? I don't see a reason why an interface couldn't change the role here regarding multi-BSSID while it's down? That might also address some of the whole "cfg80211" vs. "mac80211" thing I raised previously, since now cfg80211 would have a lot more knowledge about things if the interface is already operating, i.e. it could track and validate more of this? johannes
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index fc7e8807838d..ba8f3c8df0ca 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -604,6 +604,8 @@ struct vif_params { u8 macaddr[ETH_ALEN]; const u8 *vht_mumimo_groups; const u8 *vht_mumimo_follow_addr; + enum nl80211_multi_bssid_mode multi_bssid_mode; + u32 multi_bssid_parent; }; /** @@ -5109,6 +5111,9 @@ struct cfg80211_cqm_config; * @pmsr_free_wk: (private) peer measurements cleanup work * @unprot_beacon_reported: (private) timestamp of last * unprotected beacon report + * @multi_bssid_mode: Is this a legacy, transmitted or non-transmitted bssid + * @multi_bssid_parent: a non-transmitted bssid has a transmitted parent + * @multi_bssid_list: linked list for tracking parent - child relations. */ struct wireless_dev { struct wiphy *wiphy; @@ -5188,6 +5193,10 @@ struct wireless_dev { struct work_struct pmsr_free_wk; unsigned long unprot_beacon_reported; + + enum nl80211_multi_bssid_mode multi_bssid_mode; + struct wireless_dev *multi_bssid_parent; + struct list_head multi_bssid_list; }; static inline u8 *wdev_address(struct wireless_dev *wdev) diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index 4e6339ab1fce..b7a5d1de7aff 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -2505,6 +2505,12 @@ enum nl80211_commands { * @NL80211_ATTR_HE_6GHZ_CAPABILITY: HE 6 GHz Band Capability element (from * association request when used with NL80211_CMD_NEW_STATION). * + * @NL80211_ATTR_MULTI_BSSID_MODE: Set the (Non-)Transmitted flag for this + * BSSIDs beacon. + * + * @NL80211_ATTR_MULTI_BSSID_PARENT: If this is a Non-Transmitted BSSID, define + * the parent interface. + * * @NUM_NL80211_ATTR: total number of nl80211_attrs available * @NL80211_ATTR_MAX: highest attribute number currently defined * @__NL80211_ATTR_AFTER_LAST: internal use @@ -2987,6 +2993,9 @@ enum nl80211_attrs { NL80211_ATTR_HE_6GHZ_CAPABILITY, + NL80211_ATTR_MULTI_BSSID_MODE, + NL80211_ATTR_MULTI_BSSID_PARENT, + /* add attributes here, update the policy in nl80211.c */ __NL80211_ATTR_AFTER_LAST, @@ -6769,6 +6778,22 @@ enum nl80211_peer_measurement_ftm_failure_reasons { NL80211_PMSR_FTM_FAILURE_BAD_CHANGED_PARAMS, }; +/** + * enum nl80211_multi_bssid_mode - Multiple BSSID beacon type + * + * Used by cfg80211_ap_settings + * + * @MULTIPLE_BSSID_LEGACY: This BSS is not part of a multiple BSSID group + * @MULTIPLE_BSSID_TRANSMITTED: This BSS is broadcasting a multiple BSSID + * beacon + * @MULTIPLE_BSSID_NON_TRANSMITTED: This BSS is not broadcasting a beacon + */ +enum nl80211_multi_bssid_mode { + NL80211_MULTIPLE_BSSID_LEGACY = 0, + NL80211_MULTIPLE_BSSID_TRANSMITTED, + NL80211_MULTIPLE_BSSID_NON_TRANSMITTED, +}; + /** * enum nl80211_peer_measurement_ftm_resp - FTM response attributes * @__NL80211_PMSR_FTM_RESP_ATTR_INVALID: invalid diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 263ae395ad44..e29f834f8d12 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -658,6 +658,10 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { .type = NLA_EXACT_LEN, .len = sizeof(struct ieee80211_he_6ghz_capa), }, + [NL80211_ATTR_MULTI_BSSID_MODE] = NLA_POLICY_RANGE(NLA_U8, + NL80211_MULTIPLE_BSSID_LEGACY, + NL80211_MULTIPLE_BSSID_NON_TRANSMITTED), + [NL80211_ATTR_MULTI_BSSID_PARENT] = { .type = NLA_U32 }, }; /* policy for the key attributes */ @@ -3761,6 +3765,16 @@ static int nl80211_new_interface(struct sk_buff *skb, struct genl_info *info) if (err < 0) return err; + if (info->attrs[NL80211_ATTR_MULTI_BSSID_MODE]) + params.multi_bssid_mode = + nla_get_u8(info->attrs[NL80211_ATTR_MULTI_BSSID_MODE]); + if (info->attrs[NL80211_ATTR_MULTI_BSSID_PARENT]) + params.multi_bssid_parent = + nla_get_u8(info->attrs[NL80211_ATTR_MULTI_BSSID_PARENT]); + if (params.multi_bssid_mode == NL80211_MULTIPLE_BSSID_NON_TRANSMITTED && + !params.multi_bssid_parent) + return -EOPNOTSUPP; + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); if (!msg) return -ENOMEM;
This patch adds support for passing the multiple bssid config to the kernel when adding an interface. A bss can be legacy, transmitting or non-transmitting. A non-transmitting BSSID will have a parent interface, which needs to be transmitting. Signed-off-by: John Crispin <john@phrozen.org> --- include/net/cfg80211.h | 9 +++++++++ include/uapi/linux/nl80211.h | 25 +++++++++++++++++++++++++ net/wireless/nl80211.c | 14 ++++++++++++++ 3 files changed, 48 insertions(+)