2021-09-16 02:55:31

by Aloka Dixit

[permalink] [raw]
Subject: [PATCH v12 0/4] MBSSID and EMA support in AP mode

This patchset adds support for multiple BSSID and
enhanced multi-BSSID advertisements for AP mode.
Individual patches describe the changes in this version.

John Crispin (4):
nl80211: MBSSID and EMA support in AP mode
mac80211: MBSSID support in interface handling
mac80211: MBSSID and EMA support in beacon handling
mac80211: MBSSID channel switch

include/net/cfg80211.h | 44 ++++++
include/net/mac80211.h | 92 ++++++++++++
include/uapi/linux/nl80211.h | 76 +++++++++-
net/mac80211/cfg.c | 192 +++++++++++++++++++++---
net/mac80211/ieee80211_i.h | 2 +
net/mac80211/iface.c | 29 +++-
net/mac80211/tx.c | 173 +++++++++++++++++++---
net/wireless/nl80211.c | 277 ++++++++++++++++++++++++++++++-----
8 files changed, 805 insertions(+), 80 deletions(-)


base-commit: 339133f6c318612f9a4556c300753beda27abc01
--
2.31.1


2021-09-16 02:55:45

by Aloka Dixit

[permalink] [raw]
Subject: [PATCH v12 2/4] mac80211: MBSSID support in interface handling

From: John Crispin <[email protected]>

Configure multiple BSSID and enhanced multi-BSSID advertisement (EMA)
parameters in mac80211 for AP mode.

For each interface, 'mbssid_tx_vif' points to the transmitting interface of
the MBSSID set. The pointer is set to NULL if MBSSID is disabled.

Function ieee80211_stop() is modified to always bring down all the
non-transmitting interfaces first and the transmitting interface last.

Signed-off-by: John Crispin <[email protected]>
Co-developed-by: Aloka Dixit <[email protected]>
Signed-off-by: Aloka Dixit <[email protected]>
---
v12: Only cosmetic changes.
v11: Moved dev_close() for non-transmitting interfaces
before locking wiphy->mtx.
Removed interface count, instead mac80211 passes pointer
'mbssid_tx_vif' to the driver for each interface.
Removed dev_close() calls from ieee80211_del_iface().
Removed IEEE80211_HW_SUPPORTS_MBSSID_AP and
IEEE80211_HW_SUPPORTS_EMA_AP.
Instead of new parameters in struct ieee80211_bss_conf,
reused the existing ones which were used for STA mode.

include/net/mac80211.h | 3 +++
net/mac80211/cfg.c | 38 ++++++++++++++++++++++++++++++++++++++
net/mac80211/iface.c | 29 ++++++++++++++++++++++++++++-
3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index af0fc13cea34..0d405e36c213 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1715,6 +1715,7 @@ enum ieee80211_offload_flags {
* write-protected by sdata_lock and local->mtx so holding either is fine
* for read access.
* @color_change_color: the bss color that will be used after the change.
+ * @mbssid_tx_vif: Pointer to the transmitting interface if MBSSID is enabled.
*/
struct ieee80211_vif {
enum nl80211_iftype type;
@@ -1746,6 +1747,8 @@ struct ieee80211_vif {
bool color_change_active;
u8 color_change_color;

+ struct ieee80211_vif *mbssid_tx_vif;
+
/* must be last */
u8 drv_priv[] __aligned(sizeof(void *));
};
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index d69b31c20fe2..e2b791c37591 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -111,6 +111,36 @@ static int ieee80211_set_mon_options(struct ieee80211_sub_if_data *sdata,
return 0;
}

+static int ieee80211_set_ap_mbssid_options(struct ieee80211_sub_if_data *sdata,
+ struct cfg80211_mbssid_config params)
+{
+ struct ieee80211_sub_if_data *tx_sdata;
+
+ sdata->vif.mbssid_tx_vif = NULL;
+ sdata->vif.bss_conf.bssid_index = 0;
+ sdata->vif.bss_conf.nontransmitted = false;
+ sdata->vif.bss_conf.ema_ap = false;
+
+ if (sdata->vif.type != NL80211_IFTYPE_AP || !params.tx_wdev)
+ return -EINVAL;
+
+ tx_sdata = IEEE80211_WDEV_TO_SUB_IF(params.tx_wdev);
+ if (!tx_sdata)
+ return -EINVAL;
+
+ if (tx_sdata == sdata) {
+ sdata->vif.mbssid_tx_vif = &sdata->vif;
+ } else {
+ sdata->vif.mbssid_tx_vif = &tx_sdata->vif;
+ sdata->vif.bss_conf.nontransmitted = true;
+ sdata->vif.bss_conf.bssid_index = params.index;
+ }
+ if (params.ema)
+ sdata->vif.bss_conf.ema_ap = true;
+
+ return 0;
+}
+
static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy,
const char *name,
unsigned char name_assign_type,
@@ -1105,6 +1135,14 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
changed |= BSS_CHANGED_HE_BSS_COLOR;
}

+ if (sdata->vif.type == NL80211_IFTYPE_AP &&
+ params->mbssid_config.tx_wdev) {
+ err = ieee80211_set_ap_mbssid_options(sdata,
+ params->mbssid_config);
+ if (err)
+ return err;
+ }
+
mutex_lock(&local->mtx);
err = ieee80211_vif_use_channel(sdata, &params->chandef,
IEEE80211_CHANCTX_SHARED);
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 62c95597704b..7c687434c5e9 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -632,17 +632,44 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_do
ieee80211_add_virtual_monitor(local);
}

+static void ieee80211_stop_mbssid(struct ieee80211_sub_if_data *sdata)
+{
+ struct ieee80211_sub_if_data *tx_sdata, *non_tx_sdata, *tmp_sdata;
+ struct ieee80211_vif *tx_vif = sdata->vif.mbssid_tx_vif;
+
+ tx_sdata = vif_to_sdata(tx_vif);
+ sdata->vif.mbssid_tx_vif = NULL;
+
+ list_for_each_entry_safe(non_tx_sdata, tmp_sdata,
+ &tx_sdata->local->interfaces, list) {
+ if (non_tx_sdata != sdata && non_tx_sdata != tx_sdata &&
+ non_tx_sdata->vif.mbssid_tx_vif == tx_vif &&
+ ieee80211_sdata_running(non_tx_sdata)) {
+ non_tx_sdata->vif.mbssid_tx_vif = NULL;
+ dev_close(non_tx_sdata->wdev.netdev);
+ }
+ }
+
+ if (sdata != tx_sdata && ieee80211_sdata_running(tx_sdata)) {
+ tx_sdata->vif.mbssid_tx_vif = NULL;
+ dev_close(tx_sdata->wdev.netdev);
+ }
+}
+
static int ieee80211_stop(struct net_device *dev)
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);

- /* close all dependent VLAN interfaces before locking wiphy */
+ /* close dependent VLAN and MBSSID interfaces before locking wiphy */
if (sdata->vif.type == NL80211_IFTYPE_AP) {
struct ieee80211_sub_if_data *vlan, *tmpsdata;

list_for_each_entry_safe(vlan, tmpsdata, &sdata->u.ap.vlans,
u.vlan.list)
dev_close(vlan->dev);
+
+ if (sdata->vif.mbssid_tx_vif)
+ ieee80211_stop_mbssid(sdata);
}

wiphy_lock(sdata->local->hw.wiphy);
--
2.31.1

2021-09-16 02:57:35

by Aloka Dixit

[permalink] [raw]
Subject: [PATCH v12 1/4] nl80211: MBSSID and EMA support in AP mode

From: John Crispin <[email protected]>

This commit adds new attributes to configure support for multiple BSSID
and advanced multi-BSSID advertisements (EMA) in AP mode.

- NL80211_ATTR_MBSSID_CONFIG used for per interface configuration.
- NL80211_ATTR_MBSSID_ELEMS used to MBSSID elements for beacons.

Memory for the elements is allocated dynamically. This change frees
the memory in existing functions which call nl80211_parse_beacon(),
a comment is added to indicate the new references to do the same.

Signed-off-by: John Crispin <[email protected]>
Co-developed-by: Aloka Dixit <[email protected]>
Signed-off-by: Aloka Dixit <[email protected]>
---
v12: Replaced __dev_get_by_index() with dev_get_by_index() and
added dev_put().
nl80211_parse_mbssid_elems()returns ERR_PTR() insead of NULL
in case of error and the caller nl80211_parse_beacon() uses
PTR_ERR() to return the same.

include/net/cfg80211.h | 44 ++++++
include/uapi/linux/nl80211.h | 76 +++++++++-
net/wireless/nl80211.c | 277 ++++++++++++++++++++++++++++++-----
3 files changed, 358 insertions(+), 39 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 62dd8422e0dc..63b8c9890be8 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1040,6 +1040,36 @@ struct cfg80211_crypto_settings {
enum nl80211_sae_pwe_mechanism sae_pwe;
};

+/**
+ * struct cfg80211_mbssid_config - AP settings for multi bssid
+ *
+ * @tx_wdev: pointer to the transmitted interface in the MBSSID set
+ * @index: index of this AP in the multi bssid group.
+ * @ema: set to true if the beacons should be sent out in EMA mode.
+ */
+struct cfg80211_mbssid_config {
+ struct wireless_dev *tx_wdev;
+ u8 index;
+ bool ema;
+};
+
+/**
+ * struct cfg80211_mbssid_elems - Multiple BSSID elements
+ *
+ * @cnt: Number of elements in array %elems.
+ *
+ * @elem: Array of multiple BSSID element(s) to be added into Beacon frames.
+ * @elem.data: Data for multiple BSSID elements.
+ * @elem.len: Length of data.
+ */
+struct cfg80211_mbssid_elems {
+ u8 cnt;
+ struct {
+ u8 *data;
+ size_t len;
+ } elem[];
+};
+
/**
* struct cfg80211_beacon_data - beacon data
* @head: head portion of beacon (before TIM IE)
@@ -1058,6 +1088,7 @@ struct cfg80211_crypto_settings {
* @assocresp_ies_len: length of assocresp_ies in octets
* @probe_resp_len: length of probe response template (@probe_resp)
* @probe_resp: probe response template (AP mode only)
+ * @mbssid_ies: multiple BSSID elements
* @ftm_responder: enable FTM responder functionality; -1 for no change
* (which also implies no change in LCI/civic location data)
* @lci: Measurement Report element content, starting with Measurement Token
@@ -1075,6 +1106,7 @@ struct cfg80211_beacon_data {
const u8 *probe_resp;
const u8 *lci;
const u8 *civicloc;
+ struct cfg80211_mbssid_elems *mbssid_ies;
s8 ftm_responder;

size_t head_len, tail_len;
@@ -1189,6 +1221,7 @@ enum cfg80211_ap_settings_flags {
* @he_oper: HE operation IE (or %NULL if HE isn't enabled)
* @fils_discovery: FILS discovery transmission parameters
* @unsol_bcast_probe_resp: Unsolicited broadcast probe response parameters
+ * @mbssid_config: AP settings for multiple bssid
*/
struct cfg80211_ap_settings {
struct cfg80211_chan_def chandef;
@@ -1221,6 +1254,7 @@ struct cfg80211_ap_settings {
struct cfg80211_he_bss_color he_bss_color;
struct cfg80211_fils_discovery fils_discovery;
struct cfg80211_unsol_bcast_probe_resp unsol_bcast_probe_resp;
+ struct cfg80211_mbssid_config mbssid_config;
};

/**
@@ -4981,6 +5015,13 @@ struct wiphy_iftype_akm_suites {
* %NL80211_TID_CONFIG_ATTR_RETRY_LONG attributes
* @sar_capa: SAR control capabilities
* @rfkill: a pointer to the rfkill structure
+ *
+ * @mbssid_max_interfaces: maximum number of interfaces supported by the driver
+ * in a multiple BSSID set. This field must be set to a non-zero value
+ * by the driver to advertise MBSSID support.
+ * @mbssid_max_ema_profile_periodicity: maximum profile periodicity supported by
+ * the driver. Setting this field to a non-zero value indicates that the
+ * driver supports enhanced multi-BSSID advertisements (EMA AP).
*/
struct wiphy {
struct mutex mtx;
@@ -5125,6 +5166,9 @@ struct wiphy {

struct rfkill *rfkill;

+ u8 mbssid_max_interfaces;
+ u8 ema_max_profile_periodicity;
+
char priv[] __aligned(NETDEV_ALIGN);
};

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c2efea98e060..f1a21314e0f0 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -337,7 +337,10 @@
* @NL80211_CMD_DEL_INTERFACE: Virtual interface was deleted, has attributes
* %NL80211_ATTR_IFINDEX and %NL80211_ATTR_WIPHY. Can also be sent from
* userspace to request deletion of a virtual interface, then requires
- * attribute %NL80211_ATTR_IFINDEX.
+ * attribute %NL80211_ATTR_IFINDEX. If multiple BSSID advertisements are
+ * enabled using %NL80211_ATTR_MBSSID_CONFIG, %NL80211_ATTR_MBSSID_ELEMS,
+ * and if this command is used for the transmitting interface, then all
+ * the non-transmitting interfaces are deleted as well.
*
* @NL80211_CMD_GET_KEY: Get sequence counter information for a key specified
* by %NL80211_ATTR_KEY_IDX and/or %NL80211_ATTR_MAC.
@@ -2593,6 +2596,18 @@ enum nl80211_commands {
* @NL80211_ATTR_COLOR_CHANGE_ELEMS: Nested set of attributes containing the IE
* information for the time while performing a color switch.
*
+ * @NL80211_ATTR_MBSSID_CONFIG: Nested attribute for multiple BSSID
+ * advertisements (MBSSID) parameters in AP mode.
+ * Kernel uses this attribute to indicate the driver's support for MBSSID
+ * and enhanced multi-BSSID advertisements (EMA AP) to the userspace.
+ * Userspace should use this attribute to configure per interface MBSSID
+ * parameters.
+ * See &enum nl80211_mbssid_config_attributes for details.
+ *
+ * @NL80211_ATTR_MBSSID_ELEMS: Nested parameter to pass multiple BSSID elements.
+ * Mandatory parameter for the transmitting interface to enable MBSSID.
+ * Optional for the non-transmitting interfaces.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3096,6 +3111,9 @@ enum nl80211_attrs {
NL80211_ATTR_COLOR_CHANGE_COLOR,
NL80211_ATTR_COLOR_CHANGE_ELEMS,

+ NL80211_ATTR_MBSSID_CONFIG,
+ NL80211_ATTR_MBSSID_ELEMS,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
@@ -7349,4 +7367,60 @@ enum nl80211_sar_specs_attrs {
NL80211_SAR_ATTR_SPECS_MAX = __NL80211_SAR_ATTR_SPECS_LAST - 1,
};

+/**
+ * enum nl80211_mbssid_config_attributes - multiple BSSID (MBSSID) and enhanced
+ * multi-BSSID advertisements (EMA) in AP mode.
+ * Kernel uses some of these attributes to advertise driver's support for
+ * MBSSID and EMA.
+ * Remaining attributes should be used by the userspace to configure the
+ * features.
+ *
+ * @__NL80211_MBSSID_CONFIG_ATTR_INVALID: Invalid
+ *
+ * @NL80211_MBSSID_CONFIG_ATTR_MAX_INTERFACES: Used by the kernel to advertise
+ * the maximum number of MBSSID interfaces supported by the driver.
+ * Driver should indicate MBSSID support by setting
+ * wiphy->mbssid_max_interfaces to a value more than or equal to 2.
+ *
+ * @NL80211_MBSSID_CONFIG_ATTR_MAX_EMA_PROFILE_PERIODICITY: Used by the kernel
+ * to advertise the maximum profile periodicity supported by the driver
+ * if EMA is enabled. Driver should indicate EMA support to the userspace
+ * by setting wiphy->mbssid_max_ema_profile_periodicity to
+ * a non-zero value.
+ *
+ * @NL80211_MBSSID_CONFIG_ATTR_INDEX: Mandatory parameter to pass the index of
+ * this BSS (u8) in the multiple BSSID set.
+ * Value must be set to 0 for the transmitting interface and non-zero for
+ * all non-transmitting interfaces. The userspace will be responsible
+ * for using unique indices for the interfaces.
+ * Range: 0 to wiphy->mbssid_max_interfaces-1.
+ *
+ * @NL80211_MBSSID_CONFIG_ATTR_TX_IFINDEX: Mandatory parameter for
+ * a non-transmitted profile which provides the interface index (u32) of
+ * the transmitted profile. The value must match one of the interface
+ * indices advertised by the kernel. Optional if the interface being set up
+ * is the transmitting one, however, if provided then the value must match
+ * the interface index of the same.
+ *
+ * @NL80211_MBSSID_CONFIG_ATTR_EMA: Flag used to enable EMA AP feature.
+ * Setting this flag is permitted only if the driver advertises EMA support
+ * by setting wiphy->mbssid_max_ema_profile_periodicity to non-zero.
+ *
+ * @__NL80211_MBSSID_CONFIG_ATTR_LAST: Internal
+ * @NL80211_MBSSID_CONFIG_ATTR_MAX: highest attribute
+ */
+enum nl80211_mbssid_config_attributes {
+ __NL80211_MBSSID_CONFIG_ATTR_INVALID,
+
+ NL80211_MBSSID_CONFIG_ATTR_MAX_INTERFACES,
+ NL80211_MBSSID_CONFIG_ATTR_MAX_EMA_PROFILE_PERIODICITY,
+ NL80211_MBSSID_CONFIG_ATTR_INDEX,
+ NL80211_MBSSID_CONFIG_ATTR_TX_IFINDEX,
+ NL80211_MBSSID_CONFIG_ATTR_EMA,
+
+ /* keep last */
+ __NL80211_MBSSID_CONFIG_ATTR_LAST,
+ NL80211_MBSSID_CONFIG_ATTR_MAX = __NL80211_MBSSID_CONFIG_ATTR_LAST - 1,
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index bf7cd4752547..1f6216509124 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -437,6 +437,16 @@ sar_policy[NL80211_SAR_ATTR_MAX + 1] = {
[NL80211_SAR_ATTR_SPECS] = NLA_POLICY_NESTED_ARRAY(sar_specs_policy),
};

+static const struct nla_policy
+nl80211_mbssid_config_policy[NL80211_MBSSID_CONFIG_ATTR_MAX + 1] = {
+ [NL80211_MBSSID_CONFIG_ATTR_MAX_INTERFACES] = NLA_POLICY_MIN(NLA_U8, 2),
+ [NL80211_MBSSID_CONFIG_ATTR_MAX_EMA_PROFILE_PERIODICITY] =
+ NLA_POLICY_MIN(NLA_U8, 1),
+ [NL80211_MBSSID_CONFIG_ATTR_INDEX] = { .type = NLA_U8 },
+ [NL80211_MBSSID_CONFIG_ATTR_TX_IFINDEX] = { .type = NLA_U32 },
+ [NL80211_MBSSID_CONFIG_ATTR_EMA] = { .type = NLA_FLAG },
+};
+
static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[0] = { .strict_start_type = NL80211_ATTR_HE_OBSS_PD },
[NL80211_ATTR_WIPHY] = { .type = NLA_U32 },
@@ -763,6 +773,9 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_COLOR_CHANGE_COUNT] = { .type = NLA_U8 },
[NL80211_ATTR_COLOR_CHANGE_COLOR] = { .type = NLA_U8 },
[NL80211_ATTR_COLOR_CHANGE_ELEMS] = NLA_POLICY_NESTED(nl80211_policy),
+ [NL80211_ATTR_MBSSID_CONFIG] =
+ NLA_POLICY_NESTED(nl80211_mbssid_config_policy),
+ [NL80211_ATTR_MBSSID_ELEMS] = { .type = NLA_NESTED },
};

/* policy for the key attributes */
@@ -2207,6 +2220,35 @@ nl80211_put_sar_specs(struct cfg80211_registered_device *rdev,
return -ENOBUFS;
}

+static int nl80211_put_mbssid_support(struct wiphy *wiphy, struct sk_buff *msg)
+{
+ struct nlattr *config;
+
+ if (!wiphy->mbssid_max_interfaces)
+ return 0;
+
+ config = nla_nest_start(msg, NL80211_ATTR_MBSSID_CONFIG);
+ if (!config)
+ return -ENOBUFS;
+
+ if (nla_put_u8(msg, NL80211_MBSSID_CONFIG_ATTR_MAX_INTERFACES,
+ wiphy->mbssid_max_interfaces))
+ goto fail;
+
+ if (wiphy->ema_max_profile_periodicity &&
+ nla_put_u8(msg,
+ NL80211_MBSSID_CONFIG_ATTR_MAX_EMA_PROFILE_PERIODICITY,
+ wiphy->ema_max_profile_periodicity))
+ goto fail;
+
+ nla_nest_end(msg, config);
+ return 0;
+
+fail:
+ nla_nest_cancel(msg, config);
+ return -ENOBUFS;
+}
+
struct nl80211_dump_wiphy_state {
s64 filter_wiphy;
long start;
@@ -2792,6 +2834,9 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
if (nl80211_put_sar_specs(rdev, msg))
goto nla_put_failure;

+ if (nl80211_put_mbssid_support(&rdev->wiphy, msg))
+ goto nla_put_failure;
+
/* done */
state->split_start = 0;
break;
@@ -4981,6 +5026,95 @@ static int validate_beacon_tx_rate(struct cfg80211_registered_device *rdev,
return 0;
}

+static int nl80211_parse_mbssid_config(struct wiphy *wiphy,
+ struct net_device *dev,
+ struct nlattr *attrs,
+ struct cfg80211_mbssid_config *config,
+ u8 num_elems)
+{
+ struct nlattr *tb[NL80211_MBSSID_CONFIG_ATTR_MAX + 1];
+
+ if (!wiphy->mbssid_max_interfaces)
+ return -EOPNOTSUPP;
+
+ if (nla_parse_nested(tb, NL80211_MBSSID_CONFIG_ATTR_MAX, attrs, NULL,
+ NULL) ||
+ !tb[NL80211_MBSSID_CONFIG_ATTR_INDEX])
+ return -EINVAL;
+
+ config->ema = nla_get_flag(tb[NL80211_MBSSID_CONFIG_ATTR_EMA]);
+ if (config->ema) {
+ if (!wiphy->ema_max_profile_periodicity)
+ return -EOPNOTSUPP;
+
+ if (num_elems > wiphy->ema_max_profile_periodicity)
+ return -EINVAL;
+ }
+
+ config->index = nla_get_u8(tb[NL80211_MBSSID_CONFIG_ATTR_INDEX]);
+ if (config->index >= wiphy->mbssid_max_interfaces ||
+ (!config->index && !num_elems))
+ return -EINVAL;
+
+ if (!config->index)
+ config->tx_wdev = dev->ieee80211_ptr;
+
+ if (tb[NL80211_MBSSID_CONFIG_ATTR_TX_IFINDEX]) {
+ u32 tx_ifindex =
+ nla_get_u32(tb[NL80211_MBSSID_CONFIG_ATTR_TX_IFINDEX]);
+
+ if ((!config->index && tx_ifindex != dev->ifindex) ||
+ (config->index && tx_ifindex == dev->ifindex))
+ return -EINVAL;
+
+ if (tx_ifindex != dev->ifindex) {
+ struct net_device *tx_netdev =
+ dev_get_by_index(wiphy_net(wiphy), tx_ifindex);
+
+ if (!tx_netdev || !tx_netdev->ieee80211_ptr ||
+ tx_netdev->ieee80211_ptr->wiphy != wiphy ||
+ tx_netdev->ieee80211_ptr->iftype !=
+ NL80211_IFTYPE_AP) {
+ dev_put(tx_netdev);
+ return -EINVAL;
+ }
+
+ config->tx_wdev = tx_netdev->ieee80211_ptr;
+ }
+ } else if (config->index) {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static struct cfg80211_mbssid_elems *
+nl80211_parse_mbssid_elems(struct wiphy *wiphy, struct nlattr *attrs)
+{
+ struct nlattr *nl_elems;
+ struct cfg80211_mbssid_elems *elems;
+ int rem_elems;
+ u8 i = 0, num_elems = 0;
+
+ if (!wiphy->mbssid_max_interfaces)
+ return ERR_PTR(-EINVAL);
+
+ nla_for_each_nested(nl_elems, attrs, rem_elems)
+ num_elems++;
+
+ elems = kzalloc(struct_size(elems, elem, num_elems), GFP_KERNEL);
+ if (!elems)
+ return ERR_PTR(-ENOMEM);
+
+ nla_for_each_nested(nl_elems, attrs, rem_elems) {
+ elems->elem[i].data = nla_data(nl_elems);
+ elems->elem[i].len = nla_len(nl_elems);
+ i++;
+ }
+ elems->cnt = num_elems;
+ return elems;
+}
+
static int nl80211_parse_beacon(struct cfg80211_registered_device *rdev,
struct nlattr *attrs[],
struct cfg80211_beacon_data *bcn)
@@ -5061,6 +5195,14 @@ static int nl80211_parse_beacon(struct cfg80211_registered_device *rdev,
bcn->ftm_responder = -1;
}

+ if (attrs[NL80211_ATTR_MBSSID_ELEMS]) {
+ bcn->mbssid_ies =
+ nl80211_parse_mbssid_elems(&rdev->wiphy,
+ attrs[NL80211_ATTR_MBSSID_ELEMS]);
+ if (IS_ERR(bcn->mbssid_ies))
+ return PTR_ERR(bcn->mbssid_ies);
+ }
+
return 0;
}

@@ -5346,7 +5488,7 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)

err = nl80211_parse_beacon(rdev, info->attrs, &params.beacon);
if (err)
- return err;
+ goto out;

params.beacon_interval =
nla_get_u32(info->attrs[NL80211_ATTR_BEACON_INTERVAL]);
@@ -5356,7 +5498,7 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
err = cfg80211_validate_beacon_int(rdev, dev->ieee80211_ptr->iftype,
params.beacon_interval);
if (err)
- return err;
+ goto out;

/*
* In theory, some of these attributes should be required here
@@ -5369,8 +5511,10 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
params.ssid = nla_data(info->attrs[NL80211_ATTR_SSID]);
params.ssid_len =
nla_len(info->attrs[NL80211_ATTR_SSID]);
- if (params.ssid_len == 0)
- return -EINVAL;
+ if (params.ssid_len == 0) {
+ err = -EINVAL;
+ goto out;
+ }
}

if (info->attrs[NL80211_ATTR_HIDDEN_SSID])
@@ -5383,57 +5527,74 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
params.auth_type = nla_get_u32(
info->attrs[NL80211_ATTR_AUTH_TYPE]);
if (!nl80211_valid_auth_type(rdev, params.auth_type,
- NL80211_CMD_START_AP))
- return -EINVAL;
+ NL80211_CMD_START_AP)) {
+ err = -EINVAL;
+ goto out;
+ }
} else
params.auth_type = NL80211_AUTHTYPE_AUTOMATIC;

err = nl80211_crypto_settings(rdev, info, &params.crypto,
NL80211_MAX_NR_CIPHER_SUITES);
if (err)
- return err;
+ goto out;

if (info->attrs[NL80211_ATTR_INACTIVITY_TIMEOUT]) {
- if (!(rdev->wiphy.features & NL80211_FEATURE_INACTIVITY_TIMER))
- return -EOPNOTSUPP;
+ if (!(rdev->wiphy.features &
+ NL80211_FEATURE_INACTIVITY_TIMER)) {
+ err = -EOPNOTSUPP;
+ goto out;
+ }
params.inactivity_timeout = nla_get_u16(
info->attrs[NL80211_ATTR_INACTIVITY_TIMEOUT]);
}

if (info->attrs[NL80211_ATTR_P2P_CTWINDOW]) {
- if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
- return -EINVAL;
+ if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO) {
+ err = -EINVAL;
+ goto out;
+ }
params.p2p_ctwindow =
nla_get_u8(info->attrs[NL80211_ATTR_P2P_CTWINDOW]);
if (params.p2p_ctwindow != 0 &&
- !(rdev->wiphy.features & NL80211_FEATURE_P2P_GO_CTWIN))
- return -EINVAL;
+ !(rdev->wiphy.features & NL80211_FEATURE_P2P_GO_CTWIN)) {
+ err = -EINVAL;
+ goto out;
+ }
}

if (info->attrs[NL80211_ATTR_P2P_OPPPS]) {
u8 tmp;

- if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
- return -EINVAL;
+ if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO) {
+ err = -EINVAL;
+ goto out;
+ }
tmp = nla_get_u8(info->attrs[NL80211_ATTR_P2P_OPPPS]);
params.p2p_opp_ps = tmp;
if (params.p2p_opp_ps != 0 &&
- !(rdev->wiphy.features & NL80211_FEATURE_P2P_GO_OPPPS))
- return -EINVAL;
+ !(rdev->wiphy.features & NL80211_FEATURE_P2P_GO_OPPPS)) {
+ err = -EINVAL;
+ goto out;
+ }
}

if (info->attrs[NL80211_ATTR_WIPHY_FREQ]) {
err = nl80211_parse_chandef(rdev, info, &params.chandef);
if (err)
- return err;
+ goto out;
} else if (wdev->preset_chandef.chan) {
params.chandef = wdev->preset_chandef;
- } else if (!nl80211_get_ap_channel(rdev, &params))
- return -EINVAL;
+ } else if (!nl80211_get_ap_channel(rdev, &params)) {
+ err = -EINVAL;
+ goto out;
+ }

if (!cfg80211_reg_can_beacon_relax(&rdev->wiphy, &params.chandef,
- wdev->iftype))
- return -EINVAL;
+ wdev->iftype)) {
+ err = -EINVAL;
+ goto out;
+ }

if (info->attrs[NL80211_ATTR_TX_RATES]) {
err = nl80211_parse_tx_bitrate_mask(info, info->attrs,
@@ -5441,12 +5602,12 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
&params.beacon_rate,
dev, false);
if (err)
- return err;
+ goto out;

err = validate_beacon_tx_rate(rdev, params.chandef.chan->band,
&params.beacon_rate);
if (err)
- return err;
+ goto out;
}

if (info->attrs[NL80211_ATTR_SMPS_MODE]) {
@@ -5457,29 +5618,38 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
break;
case NL80211_SMPS_STATIC:
if (!(rdev->wiphy.features &
- NL80211_FEATURE_STATIC_SMPS))
- return -EINVAL;
+ NL80211_FEATURE_STATIC_SMPS)) {
+ err = -EINVAL;
+ goto out;
+ }
break;
case NL80211_SMPS_DYNAMIC:
if (!(rdev->wiphy.features &
- NL80211_FEATURE_DYNAMIC_SMPS))
- return -EINVAL;
+ NL80211_FEATURE_DYNAMIC_SMPS)) {
+ err = -EINVAL;
+ goto out;
+ }
break;
default:
- return -EINVAL;
+ err = -EINVAL;
+ goto out;
}
} else {
params.smps_mode = NL80211_SMPS_OFF;
}

params.pbss = nla_get_flag(info->attrs[NL80211_ATTR_PBSS]);
- if (params.pbss && !rdev->wiphy.bands[NL80211_BAND_60GHZ])
- return -EOPNOTSUPP;
+ if (params.pbss && !rdev->wiphy.bands[NL80211_BAND_60GHZ]) {
+ err = -EOPNOTSUPP;
+ goto out;
+ }

if (info->attrs[NL80211_ATTR_ACL_POLICY]) {
params.acl = parse_acl_data(&rdev->wiphy, info);
- if (IS_ERR(params.acl))
- return PTR_ERR(params.acl);
+ if (IS_ERR(params.acl)) {
+ err = PTR_ERR(params.acl);
+ goto out;
+ }
}

params.twt_responder =
@@ -5517,6 +5687,17 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
goto out;
}

+ if (info->attrs[NL80211_ATTR_MBSSID_CONFIG]) {
+ err = nl80211_parse_mbssid_config(&rdev->wiphy, dev,
+ info->attrs[NL80211_ATTR_MBSSID_CONFIG],
+ &params.mbssid_config,
+ (params.beacon.mbssid_ies ?
+ params.beacon.mbssid_ies->cnt :
+ 0));
+ if (err)
+ goto out;
+ }
+
nl80211_calculate_ap_params(&params);

if (info->attrs[NL80211_ATTR_EXTERNAL_AUTH_SUPPORT])
@@ -5537,7 +5718,14 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
wdev_unlock(wdev);

out:
- kfree(params.acl);
+ if (!IS_ERR(params.acl))
+ kfree(params.acl);
+ if (!IS_ERR(params.beacon.mbssid_ies))
+ kfree(params.beacon.mbssid_ies);
+ if (params.mbssid_config.tx_wdev &&
+ params.mbssid_config.tx_wdev->netdev &&
+ params.mbssid_config.tx_wdev->netdev != dev)
+ dev_put(params.mbssid_config.tx_wdev->netdev);

return err;
}
@@ -5562,12 +5750,15 @@ static int nl80211_set_beacon(struct sk_buff *skb, struct genl_info *info)

err = nl80211_parse_beacon(rdev, info->attrs, &params);
if (err)
- return err;
+ goto out;

wdev_lock(wdev);
err = rdev_change_beacon(rdev, dev, &params);
wdev_unlock(wdev);

+out:
+ if (!IS_ERR(params.mbssid_ies))
+ kfree(params.mbssid_ies);
return err;
}

@@ -9244,12 +9435,14 @@ static int nl80211_channel_switch(struct sk_buff *skb, struct genl_info *info)

err = nl80211_parse_beacon(rdev, info->attrs, &params.beacon_after);
if (err)
- return err;
+ goto free;

csa_attrs = kcalloc(NL80211_ATTR_MAX + 1, sizeof(*csa_attrs),
GFP_KERNEL);
- if (!csa_attrs)
- return -ENOMEM;
+ if (!csa_attrs) {
+ err = -ENOMEM;
+ goto free;
+ }

err = nla_parse_nested_deprecated(csa_attrs, NL80211_ATTR_MAX,
info->attrs[NL80211_ATTR_CSA_IES],
@@ -9367,6 +9560,10 @@ static int nl80211_channel_switch(struct sk_buff *skb, struct genl_info *info)
wdev_unlock(wdev);

free:
+ if (!IS_ERR(params.beacon_after.mbssid_ies))
+ kfree(params.beacon_after.mbssid_ies);
+ if (!IS_ERR(params.beacon_csa.mbssid_ies))
+ kfree(params.beacon_csa.mbssid_ies);
kfree(csa_attrs);
return err;
}
@@ -14900,6 +15097,10 @@ static int nl80211_color_change(struct sk_buff *skb, struct genl_info *info)
wdev_unlock(wdev);

out:
+ if (!IS_ERR(params.beacon_next.mbssid_ies))
+ kfree(params.beacon_next.mbssid_ies);
+ if (!IS_ERR(params.beacon_color_change.mbssid_ies))
+ kfree(params.beacon_color_change.mbssid_ies);
kfree(tb);
return err;
}
--
2.31.1

2022-12-18 15:26:26

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH v12 2/4] mac80211: MBSSID support in interface handling

On Wed, Sep 15, 2021 at 07:54:35PM -0700, Aloka Dixit wrote:
> Configure multiple BSSID and enhanced multi-BSSID advertisement (EMA)
> parameters in mac80211 for AP mode.
>
> For each interface, 'mbssid_tx_vif' points to the transmitting interface of
> the MBSSID set. The pointer is set to NULL if MBSSID is disabled.
>
> Function ieee80211_stop() is modified to always bring down all the
> non-transmitting interfaces first and the transmitting interface last.

This has already been applied, but this has some apparent issues that
are now showing up with mac80211_hwsim testing being available..

> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> +static int ieee80211_set_ap_mbssid_options(struct ieee80211_sub_if_data *sdata,
> + struct cfg80211_mbssid_config params)

While that does not really break behavior, why is that params argument
passed by value instead of by reference? I see no point in copying
struct cfg80211_mbssid_config members for this call since the function
is only reading the value.

> + sdata->vif.mbssid_tx_vif = NULL;
> + sdata->vif.bss_conf.bssid_index = 0;
> + sdata->vif.bss_conf.nontransmitted = false;
> + sdata->vif.bss_conf.ema_ap = false;

This cleanup is important, but it is done only here in this helper
function..

> @@ -1105,6 +1135,14 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
> + if (sdata->vif.type == NL80211_IFTYPE_AP &&
> + params->mbssid_config.tx_wdev) {
> + err = ieee80211_set_ap_mbssid_options(sdata,
> + params->mbssid_config);
> + if (err)
> + return err;
> + }

And that is the only place where the help function is called and this
happens only under the params->mbssid_config.tx_wdev condition. In other
words, those bssid_index/nontransmitted/ema_ap values are not cleared in
all cases. This results in issue when the bss_conf (link_conf in the
current kernel snapshot) is left in the previous mbssid configuration.

As an example, this will make the following mac80211_hwsim test case
sequence fail:

hostap/tests/hwsim/vm$ ./vm-run.sh he_ap_ema p2p_group_cli_invalid

This happens because ema_ap is set to true in he_ap_ema and then it is
left set true for p2p_group_cli_invalid and that test case does not
actually end up sending Beacon frames.

This can be fixed by clearing something in the
!params->mbssid_config.tx_wdev case in ieee80211_start_ap(). I'm not
completely sure what is the correct way of doing this, but at least
ema_ap needs to be cleared to false and likely some other cleanup needs
to be done as well.

--
Jouni Malinen PGP id EFC895FA

2022-12-19 19:01:38

by Aloka Dixit

[permalink] [raw]
Subject: Re: [PATCH v12 2/4] mac80211: MBSSID support in interface handling

On 12/18/2022 7:24 AM, Jouni Malinen wrote:
> On Wed, Sep 15, 2021 at 07:54:35PM -0700, Aloka Dixit wrote:
>> Configure multiple BSSID and enhanced multi-BSSID advertisement (EMA)
>> parameters in mac80211 for AP mode.
>>
>> For each interface, 'mbssid_tx_vif' points to the transmitting interface of
>> the MBSSID set. The pointer is set to NULL if MBSSID is disabled.
>>
>> Function ieee80211_stop() is modified to always bring down all the
>> non-transmitting interfaces first and the transmitting interface last.
>
> This has already been applied, but this has some apparent issues that
> are now showing up with mac80211_hwsim testing being available..
>
>> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
>> +static int ieee80211_set_ap_mbssid_options(struct ieee80211_sub_if_data *sdata,
>> + struct cfg80211_mbssid_config params)
>
> While that does not really break behavior, why is that params argument
> passed by value instead of by reference? I see no point in copying
> struct cfg80211_mbssid_config members for this call since the function
> is only reading the value.
>

Hi Jouni, the only reason for value instead of reference is that this
function does not need to change anything in 'params'. I didn't
understand your question. Are you suggesting moving the assignments to
ieee80211_start_ap() instead of this separate function?

>> + sdata->vif.mbssid_tx_vif = NULL;
>> + sdata->vif.bss_conf.bssid_index = 0;
>> + sdata->vif.bss_conf.nontransmitted = false;
>> + sdata->vif.bss_conf.ema_ap = false;
>
> This cleanup is important, but it is done only here in this helper
> function..
> And that is the only place where the help function is called and this
> happens only under the params->mbssid_config.tx_wdev condition. In other
> words, those bssid_index/nontransmitted/ema_ap values are not cleared in
> all cases. This results in issue when the bss_conf (link_conf in the
> current kernel snapshot) is left in the previous mbssid configuration.
>

Will send a patch to fix this part.

Please let me know regarding the first question above so that I can
include that in the same patch.

Thanks.

2022-12-19 19:17:43

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH v12 2/4] mac80211: MBSSID support in interface handling

On Mon, Dec 19, 2022 at 10:53:55AM -0800, Aloka Dixit wrote:
> On 12/18/2022 7:24 AM, Jouni Malinen wrote:
> > On Wed, Sep 15, 2021 at 07:54:35PM -0700, Aloka Dixit wrote:
> > > diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> > > +static int ieee80211_set_ap_mbssid_options(struct ieee80211_sub_if_data *sdata,
> > > + struct cfg80211_mbssid_config params)
> >
> > While that does not really break behavior, why is that params argument
> > passed by value instead of by reference? I see no point in copying
> > struct cfg80211_mbssid_config members for this call since the function
> > is only reading the value.
>
> Hi Jouni, the only reason for value instead of reference is that this
> function does not need to change anything in 'params'. I didn't understand
> your question. Are you suggesting moving the assignments to
> ieee80211_start_ap() instead of this separate function?

I would use a constant pointer (const struct cfg80211_mbssid_config
*params) to avoid the need to copy the full contents of that struct
whenever calling the function.

Maybe the following patch is a clearer way of showing what I was
thinking of (and testing with):

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index e30b2bdb8f01..b0abd99f006e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -138,7 +138,7 @@ static int ieee80211_set_mon_options(struct ieee80211_sub_if_data *sdata,
}

static int ieee80211_set_ap_mbssid_options(struct ieee80211_sub_if_data *sdata,
- struct cfg80211_mbssid_config params,
+ const struct cfg80211_mbssid_config *params,
struct ieee80211_bss_conf *link_conf)
{
struct ieee80211_sub_if_data *tx_sdata;
@@ -148,10 +148,10 @@ static int ieee80211_set_ap_mbssid_options(struct ieee80211_sub_if_data *sdata,
link_conf->nontransmitted = false;
link_conf->ema_ap = false;

- if (sdata->vif.type != NL80211_IFTYPE_AP || !params.tx_wdev)
+ if (sdata->vif.type != NL80211_IFTYPE_AP || !params->tx_wdev)
return -EINVAL;

- tx_sdata = IEEE80211_WDEV_TO_SUB_IF(params.tx_wdev);
+ tx_sdata = IEEE80211_WDEV_TO_SUB_IF(params->tx_wdev);
if (!tx_sdata)
return -EINVAL;

@@ -160,9 +160,9 @@ static int ieee80211_set_ap_mbssid_options(struct ieee80211_sub_if_data *sdata,
} else {
sdata->vif.mbssid_tx_vif = &tx_sdata->vif;
link_conf->nontransmitted = true;
- link_conf->bssid_index = params.index;
+ link_conf->bssid_index = params->index;
}
- if (params.ema)
+ if (params->ema)
link_conf->ema_ap = true;

return 0;
@@ -1268,10 +1268,17 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
if (sdata->vif.type == NL80211_IFTYPE_AP &&
params->mbssid_config.tx_wdev) {
err = ieee80211_set_ap_mbssid_options(sdata,
- params->mbssid_config,
+ &params->mbssid_config,
link_conf);
if (err)
return err;
+ } else {
+ /* FIX: Is this the correct thing to do here and under which
+ * conditions? At least ema_ap needs to be cleared for AP mode
+ * if mbssid_config.tx_wdev is not set. */
+ link_conf->bssid_index = 0;
+ link_conf->nontransmitted = false;
+ link_conf->ema_ap = false;
}

mutex_lock(&local->mtx);


> > This cleanup is important, but it is done only here in this helper
> > function..
> > And that is the only place where the help function is called and this
> > happens only under the params->mbssid_config.tx_wdev condition. In other
> > words, those bssid_index/nontransmitted/ema_ap values are not cleared in
> > all cases. This results in issue when the bss_conf (link_conf in the
> > current kernel snapshot) is left in the previous mbssid configuration.
> >
>
> Will send a patch to fix this part.

Thanks.

> Please let me know regarding the first question above so that I can include
> that in the same patch.

That should not be in the same patch since it is just
cleanup/optimization while the not clearing parameters in some cases is
a visible bug that should be fixed on its own first.

--
Jouni Malinen PGP id EFC895FA

2022-12-19 19:28:25

by Aloka Dixit

[permalink] [raw]
Subject: Re: [PATCH v12 2/4] mac80211: MBSSID support in interface handling

On 12/19/2022 11:15 AM, Jouni Malinen wrote:
> On Mon, Dec 19, 2022 at 10:53:55AM -0800, Aloka Dixit wrote:
>> On 12/18/2022 7:24 AM, Jouni Malinen wrote:
>>> On Wed, Sep 15, 2021 at 07:54:35PM -0700, Aloka Dixit wrote:
>>>> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
>>>> +static int ieee80211_set_ap_mbssid_options(struct ieee80211_sub_if_data *sdata,
>>>> + struct cfg80211_mbssid_config params)
>>>
> @@ -1268,10 +1268,17 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
> if (sdata->vif.type == NL80211_IFTYPE_AP &&
> params->mbssid_config.tx_wdev) {
> err = ieee80211_set_ap_mbssid_options(sdata,
> - params->mbssid_config,
> + &params->mbssid_config,
> link_conf);
> if (err)
> return err;
> + } else {
> + /* FIX: Is this the correct thing to do here and under which
> + * conditions? At least ema_ap needs to be cleared for AP mode
> + * if mbssid_config.tx_wdev is not set. */
> + link_conf->bssid_index = 0;
> + link_conf->nontransmitted = false;
> + link_conf->ema_ap = false;

Will need to clean MBSSID elements in stored beacon as well.
Would the two HWSIM testcases you mentioned earlier be sufficient for
testing? I've never used P2P related tests, and in fact I only even ran
one testcase at a time so stale configurations didn't show up.

Thanks.