Multiple BSSID support

PATCH In review Aloka Dixit 2 participants 20 messages Mar 10, 2021 – Apr 20, 2021

Subsystems (2)

mac80211 802.11 (including cfg80211/NL80211)

lore

Share & tools

lore

Aloka Dixit

author

2021-03-10 18:27 UTC #0 lore raw

v9 cover Multiple BSSID support

This patchset adds support for multiple BSSID and
enhanced multi-BSSID advertisements.

This version modifies only nl80211 patch (1/4) which describes the difference.

John Crispin (4):
nl80211: add basic multiple bssid support
mac80211: add multiple bssid support to interface handling
mac80211: add multiple bssid/EMA support to beacon handling
mac80211: CSA on non-transmitting interfaces

 include/net/cfg80211.h       |  47 ++++++
 include/net/mac80211.h       | 116 ++++++++++++++-
 include/uapi/linux/nl80211.h |  84 +++++++++++
 net/mac80211/cfg.c           | 178 ++++++++++++++++++++--
 net/mac80211/debugfs.c       |   1 +
 net/mac80211/ieee80211_i.h   |   2 +
 net/mac80211/iface.c         |   6 +
 net/mac80211/tx.c            | 189 ++++++++++++++++++++---
 net/wireless/nl80211.c       | 280 ++++++++++++++++++++++++++++++-----
 9 files changed, 830 insertions(+), 73 deletions(-)

base-commit: 38b5133ad607ecdcc8d24906d1ac9cc8df41acd5

Signature · 1 line

2.25.0

Aloka Dixit

author

2021-03-10 18:27 UTC #1 lore raw

PATCH v9 1/4 nl80211: add basic multiple bssid support

From: John Crispin <[email protected]>

This patch adds support for passing the multiple bssid config to the kernel when an AP gets started. If the BSS is non-transmitting we need to pass

* the ifidx of the transmitting parent
* the BSS index in the set
* the BSS count of the set
* flag indicating if we want to do EMA
* the multiple bssid elements as an array inside the beacon data

This allows use to generate multiple bssid beacons as well as EMA ones.

Signed-off-by John Crispin <[email protected]>

Co-developed-by Aloka Dixit <[email protected]>

Signed-off-by Aloka Dixit <[email protected]>

Reported-by kernel test robot <[email protected]>

Reported-by Dan Carpenter <[email protected]>

---
v9: Added check around kfree(params.acl)
Corrected comment descriptions for NL80211_EXT_FEATURE_MULTIPLE_BSSID_AP
and max_profile_periodicity

v8: Addition of NL80211_EXT_FEATURE_MULTIPLE_BSSID_AP,
NL80211_EXT_FEATURE_EMA_AP and wiphy parameters max_num_vaps,
max_profile_periodicity exposed to the application.

 include/net/cfg80211.h       |  47 ++++++
 include/uapi/linux/nl80211.h |  84 +++++++++++
 net/wireless/nl80211.c       | 280 ++++++++++++++++++++++++++++++-----
 3 files changed, 371 insertions(+), 40 deletions(-)

include/net/cfg80211.h +47 -0

index 911fae42b0c0..7755a04a6de6 100644
@@ -485,6 +485,21 @@ struct ieee80211_supported_band {
 	const struct ieee80211_sband_iftype_data *iftype_data;
 };
 
+/**
+ * struct cfg80211_multiple_bssid - AP settings for multi bssid
+ *
+ * @index: the index of this AP in the multi bssid group.
+ * @count: the total number of multi bssid peer APs.
+ * @parent: non-transmitted BSSs transmitted parents index
+ * @ema: Shall the beacons be sent out in EMA mode.
+ */
+struct cfg80211_multiple_bssid {
+	u8 index;
+	u8 count;
+	u32 parent;
+	bool ema;
+};
+
 /**
  * ieee80211_get_sband_iftype_data - return sband data for a given iftype
  * @sband: the sband to search for the STA on

@@ -1032,6 +1047,23 @@ struct cfg80211_crypto_settings {
 	enum nl80211_sae_pwe_mechanism sae_pwe;
 };
 
+/**
+ * struct cfg80211_multiple_bssid_data - Multiple BSSID elements
+ *
+ * @cnt: Number of elements in array %elems.
+ *
+ * @elems: Array of multiple BSSID element(s) to be added into Beacon frames.
+ * @elems.data: Data for multiple BSSID elements.
+ * @elems.len: Length of data.
+ */
+struct cfg80211_multiple_bssid_data {
+	u8 cnt;
+	struct {
+		u8 *data;
+		size_t len;
+	} elems[];
+};
+
 /**
  * struct cfg80211_beacon_data - beacon data
  * @head: head portion of beacon (before TIM IE)

@@ -1058,6 +1090,7 @@ struct cfg80211_crypto_settings {
  *	Token (measurement type 11)
  * @lci_len: LCI data length
  * @civicloc_len: Civic location data length
+ * @multiple_bssid: multiple_bssid elements
  */
 struct cfg80211_beacon_data {
 	const u8 *head, *tail;

@@ -1076,6 +1109,8 @@ struct cfg80211_beacon_data {
 	size_t probe_resp_len;
 	size_t lci_len;
 	size_t civicloc_len;
+
+	struct cfg80211_multiple_bssid_data *multiple_bssid;
 };
 
 struct mac_address {

@@ -1181,6 +1216,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
+ * @multiple_bssid: AP settings for multiple bssid.
  */
 struct cfg80211_ap_settings {
 	struct cfg80211_chan_def chandef;

@@ -1213,6 +1249,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_multiple_bssid multiple_bssid;
 };
 
 /**

@@ -4941,6 +4978,11 @@ struct wiphy_iftype_akm_suites {
  *	configuration through the %NL80211_TID_CONFIG_ATTR_RETRY_SHORT and
  *	%NL80211_TID_CONFIG_ATTR_RETRY_LONG attributes
  * @sar_capa: SAR control capabilities
+ *
+ * @multiple_bssid: Describes device's multiple BSSID config support
+ * @multiple_bssid.max_num_vaps: Maximum number of VAPS supported by the driver
+ * @multiple_bssid.max_profile_periodicity: Maximum EMA profile periodicity
+ *	supported by the driver
  */
 struct wiphy {
 	struct mutex mtx;

@@ -5083,6 +5125,11 @@ struct wiphy {
 
 	const struct cfg80211_sar_capa *sar_capa;
 
+	struct {
+		u8 max_num_vaps;
+		u8 max_profile_periodicity;
+	} multiple_bssid;
+
 	char priv[] __aligned(NETDEV_ALIGN);
 };

include/uapi/linux/nl80211.h +84 -0

112 lines not on this page · raw message

net/wireless/nl80211.c +240 ~~-40~~

441 lines not on this page · raw message

Signature · 1 line

2.25.0

Johannes Berg

maintainer

2021-04-08 12:05 UTC #2 lore raw

Re: [PATCH v9 1/4] nl80211: add basic multiple bssid support

Quoted 5 lines · Aloka Dixit

include/net/cfg80211.h +4 -0

+/**
+ * struct cfg80211_multiple_bssid - AP settings for multi bssid
+ *
+ * @index: the index of this AP in the multi bssid group.

include/net/cfg80211.h struct ieee80211_supported_band { · line 501

Quoted 10 lines

include/net/cfg80211.h +10 -0

+ * @count: the total number of multi bssid peer APs.
+ * @parent: non-transmitted BSSs transmitted parents index
+ * @ema: Shall the beacons be sent out in EMA mode.
+ */
+struct cfg80211_multiple_bssid {
+	u8 index;
+	u8 count;
+	u32 parent;
+	bool ema;
+};

Can reorder this to make it smaller: bool before u32), or even u32 element first.

Quoted 6 lines

include/net/cfg80211.h +6 -0

+/**
+ * struct cfg80211_multiple_bssid_data - Multiple BSSID elements
+ *
+ * @cnt: Number of elements in array %elems.
+ *
+ * @elems: Array of multiple BSSID element(s) to be added into Beacon frames.

include/net/cfg80211.h struct cfg80211_crypto_settings { · line 1065

Quoted 10 lines

include/net/cfg80211.h +10 -0

+ * @elems.data: Data for multiple BSSID elements.
+ * @elems.len: Length of data.
+ */
+struct cfg80211_multiple_bssid_data {
+	u8 cnt;
+	struct {
+		u8 *data;
+		size_t len;
+	} elems[];
+};

Why are the elements separate, rather than a single big buffer with all elements?

Quoted 1 line

+ * @multiple_bssid.max_num_vaps: Maximum number of VAPS supported by the driver

Please use upstream terminology, we don't use the term "VAP" (and "VAPS" would be wrong anyway, "VAPs").

I'd probably call this "max_interfaces", maybe we'll extend this to other things (mesh?) later?

Quoted 8 lines

include/uapi/linux/nl80211.h +6 -0

+/**
+ * enum nl80211_multiple_bssid_elems_attributes - Attributes used to pass
+ *	multiple BSSID elements data.
+ *
+ * @__NL80211_MULTIPLE_BSSID_ELEMS_ATTR_INVALID: Invalid
+ *

include/uapi/linux/nl80211.h enum nl80211_sar_specs_attrs { · line 7357

Quoted 10 lines

include/uapi/linux/nl80211.h +10 -0

+ * @NL80211_MULTIPLE_BSSID_ELEMS_ATTR_COUNT: Number of multiple BSSID
+ *	elements (u8).
+ *
+ * @NL80211_MULTIPLE_BSSID_ELEMS_ATTR_DATA: Array of multiple BSSID elements.
+ */
+enum nl80211_multiple_bssid_elems_attributes {
+	__NL80211_MULTIPLE_BSSID_ELEMS_ATTR_INVALID,
+
+	NL80211_MULTIPLE_BSSID_ELEMS_ATTR_COUNT,
+	NL80211_MULTIPLE_BSSID_ELEMS_ATTR_DATA,

Can you clarify why this is an array? See also above.

johannes

Aloka Dixit

author

2021-04-08 17:10 UTC #3 lore raw

Re: [PATCH v9 1/4] nl80211: add basic multiple bssid support

Quoted 11 lines · Johannes Berg

Quoted 10 lines · Aloka Dixit

include/net/cfg80211.h +9 -0

+/**
+ * struct cfg80211_multiple_bssid - AP settings for multi bssid
+ *
+ * @index: the index of this AP in the multi bssid group.
+ * @count: the total number of multi bssid peer APs.
+ * @parent: non-transmitted BSSs transmitted parents index
+ * @ema: Shall the beacons be sent out in EMA mode.
+ */
+struct cfg80211_multiple_bssid {

Quoted 9 lines

include/net/cfg80211.h struct ieee80211_supported_band { · line 501
Quoted 5 lines
include/net/cfg80211.h +5 -0
+	u8 index;
+	u8 count;
+	u32 parent;
+	bool ema;
+};
Can reorder this to make it smaller: bool before u32), or even u32 element first.

Will do.

Quoted 12 lines

Quoted 12 lines
+/**
+ * struct cfg80211_multiple_bssid_data - Multiple BSSID elements
+ *
+ * @cnt: Number of elements in array %elems.
+ *
+ * @elems: Array of multiple BSSID element(s) to be added into Beacon frames.
+ * @elems.data: Data for multiple BSSID elements.
+ * @elems.len: Length of data.
+ */
include/net/cfg80211.h +2 -0
+struct cfg80211_multiple_bssid_data {
+	u8 cnt;

Quoted 10 lines

include/net/cfg80211.h struct cfg80211_crypto_settings { · line 1065
Quoted 5 lines
include/net/cfg80211.h +5 -0
+	struct {
+		u8 *data;
+		size_t len;
+	} elems[];
+};
Why are the elements separate, rather than a single big buffer with all elements?

This is for EMA AP where each element will hold different non-transmitted profiles. While copying elements in ieee80211_assign_beacon(), all are copied one by one. However during beacon generation in __ieee80211_beacon_get(), only the element at a given index is actually added. Hence separate elements.

Quoted 9 lines

Quoted 2 lines
+ * @multiple_bssid.max_num_vaps: Maximum number of VAPS supported by the driver
Please use upstream terminology, we don't use the term "VAP" (and "VAPS" would be wrong anyway, "VAPs").
I'd probably call this "max_interfaces", maybe we'll extend this to other things (mesh?) later?
Quoted 1 line

Will change.

Quoted 23 lines

Quoted 14 lines
+/**
+ * enum nl80211_multiple_bssid_elems_attributes - Attributes used to pass
+ * multiple BSSID elements data.
+ *
+ * @__NL80211_MULTIPLE_BSSID_ELEMS_ATTR_INVALID: Invalid
+ *
+ * @NL80211_MULTIPLE_BSSID_ELEMS_ATTR_COUNT: Number of multiple BSSID
+ * elements (u8).
+ *
+ * @NL80211_MULTIPLE_BSSID_ELEMS_ATTR_DATA: Array of multiple BSSID elements.
+ */
include/uapi/linux/nl80211.h enum nl80211_sar_specs_attrs { · line 7357
Quoted 5 lines
include/uapi/linux/nl80211.h +5 -0
+enum nl80211_multiple_bssid_elems_attributes {
+	__NL80211_MULTIPLE_BSSID_ELEMS_ATTR_INVALID,
+
+	NL80211_MULTIPLE_BSSID_ELEMS_ATTR_COUNT,
+	NL80211_MULTIPLE_BSSID_ELEMS_ATTR_DATA,
Can you clarify why this is an array? See also above.

Quoted 1 line

johannes

Array to pass separate MBSSID elements for EMA which will hold different profiles in the same MBSSID set.

Aloka Dixit

author

2021-03-10 18:27 UTC #4 lore raw

PATCH v9 2/4 mac80211: add multiple bssid support to interface handling

In reply to Aloka Dixit

From: John Crispin <[email protected]>

Add a new helper ieee80211_set_multiple_bssid_options() takes propagating the cfg80211 data down the stack.

The patch also makes sure that all members of the bss set will get closed when either of them is shutdown.

Signed-off-by John Crispin <[email protected]>

Co-developed-by Aloka Dixit <[email protected]>

Signed-off-by Aloka Dixit <[email protected]>

---
 include/net/mac80211.h | 30 +++++++++++++++++++++++-
 net/mac80211/cfg.c     | 53 ++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/debugfs.c |  1 +
 net/mac80211/iface.c   |  6 +++++
 4 files changed, 89 insertions(+), 1 deletion(-)

include/net/mac80211.h +29 -1

index 2d1d629e5d14..b9f51515c2e8 100644
@@ -631,6 +631,7 @@ struct ieee80211_fils_discovery {
  * @s1g: BSS is S1G BSS (affects Association Request format).
  * @beacon_tx_rate: The configured beacon transmit rate that needs to be passed
  *	to driver when rate control is offloaded to firmware.
+ * @multiple_bssid: the multiple bssid settings of the AP.
  */
 struct ieee80211_bss_conf {
 	const u8 *bssid;

@@ -700,6 +701,7 @@ struct ieee80211_bss_conf {
 	u32 unsol_bcast_probe_resp_interval;
 	bool s1g;
 	struct cfg80211_bitrate_mask beacon_tx_rate;
+	struct cfg80211_multiple_bssid multiple_bssid;
 };
 
 /**

@@ -1663,6 +1665,19 @@ enum ieee80211_offload_flags {
 	IEEE80211_OFFLOAD_DECAP_ENABLED		= BIT(2),
 };
 
+/**
+ * enum ieee80211_vif_multiple_bssid_flags - virtual interface multiple bssid flags
+ *
+ * @IEEE80211_VIF_MBSS_TRANSMITTING: this BSS is transmitting beacons
+ * @IEEE80211_VIF_MBSS_NON_TRANSMITTING: this BSS is not transmitting beacons
+ * @IEEE80211_VIF_MBSS_EMA_BEACON: beacons should be send out in EMA mode
+ */
+enum ieee80211_vif_multiple_bssid_flags {
+	IEEE80211_VIF_MBSS_TRANSMITTING         = BIT(1),
+	IEEE80211_VIF_MBSS_NON_TRANSMITTING     = BIT(2),
+	IEEE80211_VIF_MBSS_EMA_BEACON           = BIT(3),
+};
+
 /**
  * struct ieee80211_vif - per-interface data
  *

@@ -1709,6 +1724,11 @@ enum ieee80211_offload_flags {
  *	protected by fq->lock.
  * @offload_flags: 802.3 -> 802.11 enapsulation offload flags, see
  *	&enum ieee80211_offload_flags.
+ *
+ * @multiple_bssid: Multiple BSSID configurations.
+ * @multiple_bssid.parent: Interface index of the transmitted BSS.
+ * @multiple_bssid.flags: multiple bssid flags, see
+ *	enum ieee80211_vif_multiple_bssid_flags.
  */
 struct ieee80211_vif {
 	enum nl80211_iftype type;

@@ -1737,6 +1757,11 @@ struct ieee80211_vif {
 
 	bool txqs_stopped[IEEE80211_NUM_ACS];
 
+	struct {
+		struct ieee80211_vif *parent;
+		u32 flags;
+	} multiple_bssid;
+
 	/* must be last */
 	u8 drv_priv[] __aligned(sizeof(void *));
 };

@@ -2384,7 +2409,7 @@ struct ieee80211_txq {
  * @IEEE80211_HW_TX_STATUS_NO_AMPDU_LEN: Driver does not report accurate A-MPDU
  *	length in tx status information
  *
- * @IEEE80211_HW_SUPPORTS_MULTI_BSSID: Hardware supports multi BSSID
+ * @IEEE80211_HW_SUPPORTS_MULTI_BSSID: Hardware supports multi BSSID in STA mode
  *
  * @IEEE80211_HW_SUPPORTS_ONLY_HE_MULTI_BSSID: Hardware supports multi BSSID
  *	only for HE APs. Applies if @IEEE80211_HW_SUPPORTS_MULTI_BSSID is set.

@@ -2399,6 +2424,8 @@ struct ieee80211_txq {
  * @IEEE80211_HW_SUPPORTS_RX_DECAP_OFFLOAD: Hardware supports rx decapsulation
  *	offload
  *
+ * @IEEE80211_HW_SUPPORTS_MULTI_BSSID_AP: Hardware supports multi BSSID in
+ *	AP mode
  * @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
  */
 enum ieee80211_hw_flags {

@@ -2453,6 +2480,7 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_AMPDU_KEYBORDER_SUPPORT,
 	IEEE80211_HW_SUPPORTS_TX_ENCAP_OFFLOAD,
 	IEEE80211_HW_SUPPORTS_RX_DECAP_OFFLOAD,
+	IEEE80211_HW_SUPPORTS_MULTI_BSSID_AP,
 
 	/* keep last, obviously */
 	NUM_IEEE80211_HW_FLAGS

net/mac80211/cfg.c +53 -0

71 lines not on this page · raw message

net/mac80211/debugfs.c +1 -0

index 5296898875ff..249b8f4e6a54 100644
@@ -456,6 +456,7 @@ static const char *hw_flag_names[] = {
 	FLAG(AMPDU_KEYBORDER_SUPPORT),
 	FLAG(SUPPORTS_TX_ENCAP_OFFLOAD),
 	FLAG(SUPPORTS_RX_DECAP_OFFLOAD),
+	FLAG(SUPPORTS_MULTI_BSSID_AP),
 #undef FLAG
 };

net/mac80211/iface.c +6 -0

index b80c9b016b2b..6bb48bf87aca 100644
@@ -376,6 +376,12 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_do
 	bool cancel_scan;
 	struct cfg80211_nan_func *func;
 
+	/* make sure the parent is already down */
+	if (sdata->vif.type == NL80211_IFTYPE_AP &&
+	    sdata->vif.multiple_bssid.parent &&
+	    ieee80211_sdata_running(vif_to_sdata(sdata->vif.multiple_bssid.parent)))
+		dev_close(vif_to_sdata(sdata->vif.multiple_bssid.parent)->wdev.netdev);
+
 	clear_bit(SDATA_STATE_RUNNING, &sdata->state);
 
 	cancel_scan = rcu_access_pointer(local->scan_sdata) == sdata;

Signature · 1 line

2.25.0

Johannes Berg

maintainer

2021-04-08 12:06 UTC #5 lore raw

Re: [PATCH v9 2/4] mac80211: add multiple bssid support to interface handling

Quoted 7 lines · Aloka Dixit

From: John Crispin <[email protected]>
Add a new helper ieee80211_set_multiple_bssid_options() takes propagating the cfg80211 data down the stack.
The patch also makes sure that all members of the bss set will get closed when either of them is shutdown.

s/either/any/

net/mac80211/cfg.c static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy, · line 179

Quoted 6 lines

static int ieee80211_del_iface(struct wiphy *wiphy, struct wireless_dev *wdev) {
net/mac80211/cfg.c +3 -0
+	struct ieee80211_sub_if_data *sdata;
+
+	sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);

can be one line

net/mac80211/cfg.c static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy, · line 188

Quoted 9 lines

net/mac80211/cfg.c +9 -0

+	if (sdata && sdata->vif.type == NL80211_IFTYPE_AP) {
+		if (sdata->vif.multiple_bssid.flags & IEEE80211_VIF_MBSS_TRANSMITTING) {
+			struct ieee80211_sub_if_data *child;
+
+			rcu_read_lock();
+			list_for_each_entry_rcu(child, &sdata->local->interfaces, list)
+				if (child->vif.multiple_bssid.parent == &sdata->vif)
+					dev_close(child->wdev.netdev);
+			rcu_read_unlock();

You never tested this properly, this is wrong.

johannes

Aloka Dixit

author

2021-04-16 21:35 UTC #6 lore raw

Re: [PATCH v9 2/4] mac80211: add multiple bssid support to interface handling

Quoted 11 lines · Johannes Berg

Quoted 7 lines · Aloka Dixit
From: John Crispin <[email protected]>
Add a new helper ieee80211_set_multiple_bssid_options() takes propagating the cfg80211 data down the stack.
The patch also makes sure that all members of the bss set will get closed when either of them is shutdown.
s/either/any/
Quoted 1 line

Quoted 8 lines

net/mac80211/cfg.c static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy, · line 179
Quoted 5 lines
static int ieee80211_del_iface(struct wiphy *wiphy, struct wireless_dev *wdev) {
net/mac80211/cfg.c +3 -0
+	struct ieee80211_sub_if_data *sdata;
+
+	sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
can be one line

Okay

Quoted 13 lines

net/mac80211/cfg.c static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy, · line 188
Quoted 10 lines
(quoted diff) +2 -0
+	if (sdata && sdata->vif.type == NL80211_IFTYPE_AP) {
+		if (sdata->vif.multiple_bssid.flags & 
IEEE80211_VIF_MBSS_TRANSMITTING) {
net/mac80211/cfg.c +7 -0
+			struct ieee80211_sub_if_data *child;
+
+			rcu_read_lock();
+			list_for_each_entry_rcu(child, &sdata->local->interfaces, list)
+				if (child->vif.multiple_bssid.parent == &sdata->vif)
+					dev_close(child->wdev.netdev);
+			rcu_read_unlock();
You never tested this properly, this is wrong.

Quoted 1 line

johannes

This was added for graceful shutdown of non-transmitting interfaces whenever the transmitting one is being brought down. But I see that dev_close() is happening twice now.

Inclining towards removing this and just return error to application if it tries to remove transmitting before all non-transmitting are deleted. However, currently the "parent" pointer to indicate the transmitting interface is maintained in mac80211, nothing in cfg80211.

Which option would be better,
(1) Add new function to mac80211_config_ops to return yes/no to whether cfg80211 can delete a particular interface when MBSSID is in use.
(2) Add a new pointer in struct wireless_dev to track the transmitting interface of the set. Then the yes/no decision can be taken in cfg80211 itself.

Thanks.

Johannes Berg

maintainer

2021-04-19 11:28 UTC #7 lore raw

Re: [PATCH v9 2/4] mac80211: add multiple bssid support to interface handling

Hi,

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
(quoted diff) +2 -0
+	if (sdata && sdata->vif.type == NL80211_IFTYPE_AP) {
+		if (sdata->vif.multiple_bssid.flags & 
IEEE80211_VIF_MBSS_TRANSMITTING) {
(quoted diff) +2 -0
+			struct ieee80211_sub_if_data *child;
+

Quoted 9 lines

Quoted 5 lines
net/mac80211/cfg.c static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy, · line 188
Quoted 5 lines
net/mac80211/cfg.c +5 -0
+			rcu_read_lock();
+			list_for_each_entry_rcu(child, &sdata->local->interfaces, list)
+				if (child->vif.multiple_bssid.parent == &sdata->vif)
+					dev_close(child->wdev.netdev);
+			rcu_read_unlock();
This was added for graceful shutdown of non-transmitting interfaces whenever the transmitting one is being brought down.

I know, I asked you to.

Quoted 3 lines

But I see that
dev_close() is happening twice now.

That wouldn't be an issue.

The issue is that dev_close() needs to be able to sleep, and it even contains a might_sleep(), so you can't do it under the RCU protection you used here.

Quoted 4 lines

Inclining towards removing this and just return error to application if it tries to remove transmitting before all non-transmitting are deleted. However, currently the "parent" pointer to indicate the transmitting interface is maintained in mac80211, nothing in cfg80211.

That seems kinda awkward, considering e.g. if hostapd crashes and then a new instance has to clean up, it might not really have much knowledge of the order in which it should be doing that.

I think it's better if you just fix the locking here?

johannes

Aloka Dixit

author

2021-04-19 22:35 UTC #8 lore raw

Re: [PATCH v9 2/4] mac80211: add multiple bssid support to interface handling

Quoted 34 lines · Johannes Berg

Hi,
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
(quoted diff) +2 -0
+	if (sdata && sdata->vif.type == NL80211_IFTYPE_AP) {
+		if (sdata->vif.multiple_bssid.flags &
IEEE80211_VIF_MBSS_TRANSMITTING) {
(quoted diff) +2 -0
+			struct ieee80211_sub_if_data *child;
+
Quoted 9 lines
Quoted 5 lines
net/mac80211/cfg.c static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy, · line 188
Quoted 5 lines
net/mac80211/cfg.c +5 -0
+			rcu_read_lock();
+			list_for_each_entry_rcu(child, &sdata->local->interfaces, list)
+				if (child->vif.multiple_bssid.parent == &sdata->vif)
+					dev_close(child->wdev.netdev);
+			rcu_read_unlock();
This was added for graceful shutdown of non-transmitting interfaces whenever the transmitting one is being brought down.
I know, I asked you to.
Quoted 3 lines
But I see that
dev_close() is happening twice now.
That wouldn't be an issue.
The issue is that dev_close() needs to be able to sleep, and it even contains a might_sleep(), so you can't do it under the RCU protection you used here.
Quoted 4 lines
Inclining towards removing this and just return error to application if it tries to remove transmitting before all non-transmitting are deleted. However, currently the "parent" pointer to indicate the transmitting interface is maintained in mac80211, nothing in cfg80211.

Quoted 7 lines

That seems kinda awkward, considering e.g. if hostapd crashes and then a new instance has to clean up, it might not really have much knowledge of the order in which it should be doing that.
I think it's better if you just fix the locking here?
johannes

Hi Johannes,
Thanks for the response, I need more help.

Is rcu_read_lock is not allowed around dev_close() because it might block or *ANY* lock? Because both functions with new dev_close() themselves are called with locks held,
(1) ieee80211_do_stop() happens inside wiphy_lock(sdata->local->hw.wiphy)
(2) ieee80211_del_iface() happens inside mutex_lock(&rdev->wiphy.mtx). Should these be unlocked temporarily too before calling dev_close()? Didn't find any instance of wiphy.mtx lock/unlock in mac80211.

Also, in cfg.c, list_for_each_entry(sdata, &local->interfaces, list) is called with two different murexes: (1) local->iflist_mtx
(2) local->mtx

Which is the correct one for this purpose among above two and rcu_read_lock()? Once that decided, would following be sufficient -

    lock()
    list_for_each_entry(sdata, &local->interfaces, list) {
        get_child_pointer
        unlock()
        dev_close()
        lock()
    }
    unlock

Thanks.

Aloka Dixit

author

2021-04-19 22:42 UTC #9 lore raw

Re: [PATCH v9 2/4] mac80211: add multiple bssid support to interface handling

Quoted 46 lines · Aloka Dixit

Quoted 34 lines · Johannes Berg
Hi,
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
(quoted diff) +2 -0
+	if (sdata && sdata->vif.type == NL80211_IFTYPE_AP) {
+		if (sdata->vif.multiple_bssid.flags &
IEEE80211_VIF_MBSS_TRANSMITTING) {
(quoted diff) +2 -0
+			struct ieee80211_sub_if_data *child;
+
Quoted 9 lines
Quoted 5 lines
net/mac80211/cfg.c static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy, · line 188
Quoted 5 lines
net/mac80211/cfg.c +5 -0
+			rcu_read_lock();
+			list_for_each_entry_rcu(child, &sdata->local->interfaces, list)
+				if (child->vif.multiple_bssid.parent == &sdata->vif)
+					dev_close(child->wdev.netdev);
+			rcu_read_unlock();
This was added for graceful shutdown of non-transmitting interfaces whenever the transmitting one is being brought down.
I know, I asked you to.
Quoted 3 lines
But I see that
dev_close() is happening twice now.
That wouldn't be an issue.
The issue is that dev_close() needs to be able to sleep, and it even contains a might_sleep(), so you can't do it under the RCU protection you used here.
Quoted 4 lines
Inclining towards removing this and just return error to application if it tries to remove transmitting before all non-transmitting are deleted. However, currently the "parent" pointer to indicate the transmitting interface is maintained in mac80211, nothing in cfg80211.
Quoted 7 lines
That seems kinda awkward, considering e.g. if hostapd crashes and then a new instance has to clean up, it might not really have much knowledge of the order in which it should be doing that.
I think it's better if you just fix the locking here?
johannes
Hi Johannes,
Thanks for the response, I need more help.

Quoted 9 lines

Is rcu_read_lock is not allowed around dev_close() because it might block or *ANY* lock?
Because both functions with new dev_close() themselves are called with locks held,
(1) ieee80211_do_stop() happens inside wiphy_lock(sdata->local->hw.wiphy)
(2) ieee80211_del_iface() happens inside mutex_lock(&rdev->wiphy.mtx). Should these be unlocked temporarily too before calling dev_close()? Didn't find any instance of wiphy.mtx lock/unlock in mac80211.

My bad, wiphy_lock() internally uses wiphy.mtx so mac80211 does have a way, then should it be unlocked temporarily before dev_close()?

Quoted 16 lines

Also, in cfg.c, list_for_each_entry(sdata, &local->interfaces, list) is called with two different murexes: (1) local->iflist_mtx
(2) local->mtx
Which is the correct one for this purpose among above two and rcu_read_lock()? Once that decided, would following be sufficient -
    lock()
    list_for_each_entry(sdata, &local->interfaces, list) {
        get_child_pointer
        unlock()
        dev_close()
        lock()
    }
    unlock
Thanks.

Johannes Berg

maintainer

2021-04-20 08:25 UTC #10 lore raw

Re: [PATCH v9 2/4] mac80211: add multiple bssid support to interface handling

In reply to Aloka Dixit

Hi Aloka,

Quoted 3 lines

Is rcu_read_lock is not allowed around dev_close() because it might block or *ANY* lock?

Well, I guess it's more nuanced than that.

rcu_read_lock() is not allowed because it enters an atomic context. Similarly not allowed would be spinlocks, or local_bh_disable, or any similar thing that makes the context atomic.

From a locking perspective, normal mutexes *would* be allowed.

However, you'd have to be extremely careful to not allow recursion, since dev_close() will call back into cfg80211/mac80211.

Quoted 4 lines

Because both functions with new dev_close() themselves are called with locks held,
(1) ieee80211_do_stop() happens inside wiphy_lock(sdata->local->hw.wiphy)

This is probably already problematic.

Quoted 1 line

(2) ieee80211_del_iface() happens inside mutex_lock(&rdev->wiphy.mtx).

As you discovered, that's the same lock.

Quoted 1 line

Should these be unlocked temporarily too before calling dev_close()?

I don't think temporarily dropping locks is ever a good idea, it makes it really hard to reason about the code.

But we already do this for AP-VLAN interfaces, so not sure why this is so different?

Quoted 7 lines

Also, in cfg.c, list_for_each_entry(sdata, &local->interfaces, list) is called with two different murexes: (1) local->iflist_mtx
(2) local->mtx

Quoted 8 lines

Which is the correct one for this purpose among above two and rcu_read_lock()?
Once that decided, would following be sufficient - lock()
     list_for_each_entry(sdata, &local->interfaces, list) { get_child_pointer
         unlock()
         dev_close()

What guarantees you don't lose the device after the unlock()? I think you'd risk list corruption here this way...

Look at the other instance of dev_close() here - as long as you can guarantee there won't be recursion (and you do, because non-transmitting interfaces don't have other non-transmitting below them, though they may actually have AP-VLANs!), we should be fine just doing it like that code?

johannes

Aloka Dixit

author

2021-03-10 18:27 UTC #11 lore raw

PATCH v9 3/4 mac80211: add multiple bssid/EMA support to beacon handling

In reply to Aloka Dixit

From: John Crispin <[email protected]>

With beacon_data now holding the additional information about the multiple bssid elements, we need to honour these in the various beacon handling code paths.

Extend ieee80211_beacon_get_template() to allow generation of multiple bssid/EMA beacons. The API provides support for HW that can offload the EMA beaconing aswell as HW that will require periodic updates of the beacon template upon completion events.

In case the HW can do full EMA offload, functions are provided that allow the driver to get a list of the periodicity number of beacons and their matching mutable offsets.

Signed-off-by John Crispin <[email protected]>

Co-developed-by Aloka Dixit <[email protected]>

Signed-off-by Aloka Dixit <[email protected]>

---
 include/net/mac80211.h     |  86 +++++++++++++++++
 net/mac80211/cfg.c         | 114 +++++++++++++++++++---
 net/mac80211/ieee80211_i.h |   2 +
 net/mac80211/tx.c          | 189 ++++++++++++++++++++++++++++++++-----
 4 files changed, 359 insertions(+), 32 deletions(-)

include/net/mac80211.h +86 -0

index b9f51515c2e8..fc456262b335 100644
@@ -4889,12 +4889,17 @@ void ieee80211_report_low_ack(struct ieee80211_sta *sta, u32 num_packets);
  * @cntdwn_counter_offs: array of IEEE80211_MAX_CNTDWN_COUNTERS_NUM offsets
  *	to countdown counters.  This array can contain zero values which
  *	should be ignored.
+ * @multiple_bssid_offset: position of the multiple bssid element
+ * @multiple_bssid_length: size of the multiple bssid element
  */
 struct ieee80211_mutable_offsets {
 	u16 tim_offset;
 	u16 tim_length;
 
 	u16 cntdwn_counter_offs[IEEE80211_MAX_CNTDWN_COUNTERS_NUM];
+
+	u16 multiple_bssid_offset;
+	u16 multiple_bssid_length;
 };
 
 /**

@@ -4921,6 +4926,87 @@ ieee80211_beacon_get_template(struct ieee80211_hw *hw,
 			      struct ieee80211_vif *vif,
 			      struct ieee80211_mutable_offsets *offs);
 
+/**
+ * enum ieee80211_bcn_tmpl_ema - EMA beacon generation type
+ * @IEEE80211_BCN_EMA_NONE: don't generate an EMA beacon.
+ * @IEEE80211_BCN_EMA_NEXT: generate the next periodicity beacon.
+ * @IEEE80211_BCN_EMA_INDEX: generate beacon by periodicity index
+ *	if the value is >= this enum value.
+ */
+enum ieee80211_bcn_tmpl_ema {
+	IEEE80211_BCN_EMA_NONE	= -2,
+	IEEE80211_BCN_EMA_NEXT	= -1,
+	IEEE80211_BCN_EMA_INDEX	= 0,
+};
+
+/**
+ * ieee80211_beacon_get_template_ema_next - EMA beacon template generation
+ *	function for drivers using the sw offload path.
+ * @hw: pointer obtained from ieee80211_alloc_hw().
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ * @offs: &struct ieee80211_mutable_offsets pointer to struct that will
+ *	receive the offsets that may be updated by the driver.
+ *
+ * This function differs from ieee80211_beacon_get_template in the sense that
+ * it generates EMA VAP templates. When we use multiple_bssid, the beacons can
+ * get very large costing a lot of airtime. To work around this, we iterate
+ * over the multiple bssid elements and only send one inside the beacon for
+ * 1..n. Calling this function will auto-increment the periodicity counter.
+ *
+ * This function needs to follow the same rules as ieee80211_beacon_get_template
+ *
+ * Return: The beacon template. %NULL on error.
+ */
+struct sk_buff *ieee80211_beacon_get_template_ema_next(struct ieee80211_hw *hw,
+						       struct ieee80211_vif *vif,
+						       struct ieee80211_mutable_offsets *offs);
+
+/**
+ * struct ieee80211_ema_bcn_list - list entry of an EMA beacon
+ * @list: the list pointer.
+ * @skb: the skb containing this specific beacon
+ * @offs: &struct ieee80211_mutable_offsets pointer to struct that will
+ *	receive the offsets that may be updated by the driver.
+ */
+struct ieee80211_ema_bcn_list {
+	struct list_head list;
+	struct sk_buff *skb;
+	struct ieee80211_mutable_offsets offs;
+};
+
+/**
+ * ieee80211_beacon_get_template_ema_list - EMA beacon template generation
+ *	function for drivers using the hw offload.
+ * @hw: pointer obtained from ieee80211_alloc_hw().
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ * @head: linked list head that will get populated with
+ *	&struct ieee80211_ema_bcn_list pointers.
+ *
+ * This function differs from ieee80211_beacon_get_template in the sense that
+ * it generates EMA VAP templates. When we use multiple_bssid, the beacons can
+ * get very large costing a lot of airtime. To work around this, we iterate
+ * over the multiple bssid elements and only send one inside the beacon for
+ * 1..n. This function will populate a linked list that the driver can pass
+ * to the HW.
+ *
+ * This function needs to follow the same rules as ieee80211_beacon_get_template
+ *
+ * Return: The nuber of entries in the list or 0 on error.
+ */
+
+int ieee80211_beacon_get_template_ema_list(struct ieee80211_hw *hw,
+					   struct ieee80211_vif *vif,
+					   struct list_head *head);
+
+/**
+ * ieee80211_beacon_free_ema_list - free an EMA beacon template list
+ * @head: linked list head containing &struct ieee80211_ema_bcn_list pointers.
+ *
+ * This function will free a list previously acquired by calling
+ * ieee80211_beacon_get_template_ema_list()
+ */
+void ieee80211_beacon_free_ema_list(struct list_head *head);
+
 /**
  * ieee80211_beacon_get_tim - beacon generation function
  * @hw: pointer obtained from ieee80211_alloc_hw().

net/mac80211/cfg.c +103 ~~-11~~

212 lines not on this page · raw message

net/mac80211/ieee80211_i.h +2 -0

index ecda126a7026..d30d53cfec1a 100644
@@ -261,6 +261,8 @@ struct beacon_data {
 	struct ieee80211_meshconf_ie *meshconf;
 	u16 cntdwn_counter_offsets[IEEE80211_MAX_CNTDWN_COUNTERS_NUM];
 	u8 cntdwn_current_counter;
+	struct cfg80211_multiple_bssid_data *multiple_bssid;
+	u16 ema_index;
 	struct rcu_head rcu_head;
 };

net/mac80211/tx.c +168 ~~-21~~

289 lines not on this page · raw message

Signature · 1 line

2.25.0

Johannes Berg

maintainer

2021-04-08 12:11 UTC #12 lore raw

Re: [PATCH v9 3/4] mac80211: add multiple bssid/EMA support to beacon handling

include/net/mac80211.h ieee80211_beacon_get_template(struct ieee80211_hw *hw, · line 4939

Quoted 12 lines · Aloka Dixit

include/net/mac80211.h +11 -0

+/**
+ * enum ieee80211_bcn_tmpl_ema - EMA beacon generation type
+ * @IEEE80211_BCN_EMA_NONE: don't generate an EMA beacon.
+ * @IEEE80211_BCN_EMA_NEXT: generate the next periodicity beacon.
+ * @IEEE80211_BCN_EMA_INDEX: generate beacon by periodicity index
+ *	if the value is >= this enum value.
+ */
+enum ieee80211_bcn_tmpl_ema {
+	IEEE80211_BCN_EMA_NONE	= -2,
+	IEEE80211_BCN_EMA_NEXT	= -1,
+	IEEE80211_BCN_EMA_INDEX	= 0,

Maybe call it _BASE instead of _INDEX, it's not really meant to be used as is?

Quoted 7 lines

net/mac80211/cfg.c +7 -0

+static u8 *ieee80211_copy_multiple_bssid_beacon(u8 *offset,
+						struct cfg80211_multiple_bssid_data *dest,
+						struct cfg80211_multiple_bssid_data *src)
+{
+	int i;
+
+	if (!dest || !src)

net/mac80211/cfg.c static int ieee80211_set_ftm_responder_params( · line 1016

Quoted 10 lines

net/mac80211/cfg.c +10 -0

+		return offset;
+
+	dest->cnt = src->cnt;
+	for (i = 0; i < dest->cnt; i++) {
+		dest->elems[i].len = src->elems[i].len;
+		dest->elems[i].data = offset;
+		memcpy(dest->elems[i].data, src->elems[i].data,
+		       dest->elems[i].len);
+		offset += dest->elems[i].len;
+	}

Following my earlier question - here you just copy all the elements one after another, as far as I can tell, so why did they need to be separate in the first place? Might be a lot simpler everywhere if all of this was just a single buffer, starting from the userspace API?

net/mac80211/tx.c __ieee80211_beacon_get(struct ieee80211_hw *hw, · line 4804

Quoted 8 lines

net/mac80211/tx.c +0 -2

@@ -4740,13 +4800,11 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
  	struct ieee80211_chanctx_conf *chanctx_conf;
  	int csa_off_base = 0;
 
-	rcu_read_lock();
-
  	sdata = vif_to_sdata(vif);
  	chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);

OK, but ...

net/mac80211/tx.c __ieee80211_beacon_get(struct ieee80211_hw *hw, · line 5075

Quoted 8 lines

net/mac80211/tx.c +2 -1

  struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
  					 struct ieee80211_vif *vif,
  					 u16 *tim_offset, u16 *tim_length)
  {
  	struct ieee80211_mutable_offsets offs = {};
-	struct sk_buff *bcn = __ieee80211_beacon_get(hw, vif, &offs, false);
+	struct sk_buff *bcn = __ieee80211_beacon_get(hw, vif, &offs, false,
+						     IEEE80211_BCN_EMA_NONE);

You didn't add the protection everywhere.

johannes

Aloka Dixit

author

2021-04-08 17:17 UTC #13 lore raw

Re: [PATCH v9 3/4] mac80211: add multiple bssid/EMA support to beacon handling

Quoted 17 lines · Johannes Berg

include/net/mac80211.h ieee80211_beacon_get_template(struct ieee80211_hw *hw, · line 4939
Quoted 12 lines · Aloka Dixit
include/net/mac80211.h +11 -0
+/**
+ * enum ieee80211_bcn_tmpl_ema - EMA beacon generation type
+ * @IEEE80211_BCN_EMA_NONE: don't generate an EMA beacon.
+ * @IEEE80211_BCN_EMA_NEXT: generate the next periodicity beacon.
+ * @IEEE80211_BCN_EMA_INDEX: generate beacon by periodicity index
+ *	if the value is >= this enum value.
+ */
+enum ieee80211_bcn_tmpl_ema {
+	IEEE80211_BCN_EMA_NONE	= -2,
+	IEEE80211_BCN_EMA_NEXT	= -1,
+	IEEE80211_BCN_EMA_INDEX	= 0,
Maybe call it _BASE instead of _INDEX, it's not really meant to be used as is?

Sure

Quoted 15 lines

net/mac80211/cfg.c +15 -0

+static u8 *ieee80211_copy_multiple_bssid_beacon(u8 *offset,
+						struct cfg80211_multiple_bssid_data *dest,
+						struct cfg80211_multiple_bssid_data *src)
+{
+	int i;
+
+	if (!dest || !src)
+		return offset;
+
+	dest->cnt = src->cnt;
+	for (i = 0; i < dest->cnt; i++) {
+		dest->elems[i].len = src->elems[i].len;
+		dest->elems[i].data = offset;
+		memcpy(dest->elems[i].data, src->elems[i].data,
+		       dest->elems[i].len);

Quoted 10 lines

Review
Quoted 2 lines
(quoted diff) +2 -0
+		offset += dest->elems[i].len;
+	}
Following my earlier question - here you just copy all the elements one after another, as far as I can tell, so why did they need to be separate in the first place? Might be a lot simpler everywhere if all of this was just a single buffer, starting from the userspace API?

Separate buffers required as only one buffer is copied when (ema_index >= IEEE80211_BCN_EMA_INDEX) in __ieee80211_beacon_get().

Quoted 12 lines

net/mac80211/tx.c __ieee80211_beacon_get(struct ieee80211_hw · line 4805

Quoted 9 lines

net/mac80211/tx.c +0 -2

@@ -4740,13 +4800,11 @@ __ieee80211_beacon_get(struct ieee80211_hw 
 *hw,
  	struct ieee80211_chanctx_conf *chanctx_conf;
  	int csa_off_base = 0;
 
-	rcu_read_lock();
-
  	sdata = vif_to_sdata(vif);
  	chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);

OK, but ...

Quoted 11 lines

net/mac80211/tx.c __ieee80211_beacon_get(struct ieee80211_hw *hw, · line 5075

Quoted 8 lines

net/mac80211/tx.c +2 -1

  struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
  					 struct ieee80211_vif *vif,
  					 u16 *tim_offset, u16 *tim_length)
  {
  	struct ieee80211_mutable_offsets offs = {};
-	struct sk_buff *bcn = __ieee80211_beacon_get(hw, vif, &offs, false);
+	struct sk_buff *bcn = __ieee80211_beacon_get(hw, vif, &offs, false,
+						     IEEE80211_BCN_EMA_NONE);

You didn't add the protection everywhere.

Will revise this part, thanks for bringing it up.

Quoted 1 line

johannes

Aloka Dixit

author

2021-03-10 18:27 UTC #14 lore raw

PATCH v9 4/4 mac80211: CSA on non-transmitting interfaces

In reply to Aloka Dixit

From: John Crispin <[email protected]>

Trigger ieee80211_csa_finish() on the non-transmitting interfaces when CSA concludes on the tranmitting interface.

Modify offset in beacon generation to reflect multiple BSSID element length.

Signed-off-by John Crispin <[email protected]>

Co-developed-by Aloka Dixit <[email protected]>

Signed-off-by Aloka Dixit <[email protected]>

---
 net/mac80211/cfg.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

net/mac80211/cfg.c +11 -0

index e72c0a4a26c3..ccafa39f7ad6 100644
@@ -3238,6 +3238,17 @@ void ieee80211_csa_finish(struct ieee80211_vif *vif)
 {
 	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
 
+	if (sdata->vif.multiple_bssid.flags & IEEE80211_VIF_MBSS_TRANSMITTING) {
+		struct ieee80211_sub_if_data *child;
+
+		rcu_read_lock();
+		list_for_each_entry_rcu(child, &sdata->local->interfaces, list)
+			if (child->vif.multiple_bssid.parent == &sdata->vif)
+				ieee80211_queue_work(&child->local->hw,
+						     &child->csa_finalize_work);
+		rcu_read_unlock();
+	}
+
 	ieee80211_queue_work(&sdata->local->hw,
 			     &sdata->csa_finalize_work);
 }

Signature · 1 line

2.25.0

Johannes Berg

maintainer

2021-04-08 11:53 UTC #15 lore raw

In reply to Aloka Dixit

Quoted 2 lines · Aloka Dixit

This patchset adds support for multiple BSSID and
enhanced multi-BSSID advertisements.

All of this, in particular the subjects, should really mention that it's for AP side.

Does it apply also for mesh, btw?

johannes

Aloka Dixit

author

2021-04-09 18:05 UTC #16 lore raw

Quoted 10 lines · Johannes Berg

Quoted 2 lines · Aloka Dixit
This patchset adds support for multiple BSSID and
enhanced multi-BSSID advertisements.
All of this, in particular the subjects, should really mention that it's for AP side.
Does it apply also for mesh, btw?
johannes

No, this patchset is only for AP mode.
Will mention it in the titles/commit_logs/comments. Thanks.

Johannes Berg

maintainer

2021-04-08 12:17 UTC #17 lore raw

In reply to Aloka Dixit

Quoted 11 lines · Aloka Dixit

This patchset adds support for multiple BSSID and
enhanced multi-BSSID advertisements.
This version modifies only nl80211 patch (1/4) which describes the difference.
John Crispin (4):
  nl80211: add basic multiple bssid support
  mac80211: add multiple bssid support to interface handling
  mac80211: add multiple bssid/EMA support to beacon handling
  mac80211: CSA on non-transmitting interfaces

Also, can we have hwsim support for proper testing? Seems with all the logic in mac80211 to give you the right beacons etc. that should be really simple?

johannes

Aloka Dixit

author

2021-04-09 18:31 UTC #18 lore raw

Quoted 13 lines · Johannes Berg

Quoted 11 lines · Aloka Dixit
This patchset adds support for multiple BSSID and
enhanced multi-BSSID advertisements.
This version modifies only nl80211 patch (1/4) which describes the difference.
John Crispin (4):
  nl80211: add basic multiple bssid support
  mac80211: add multiple bssid support to interface handling
  mac80211: add multiple bssid/EMA support to beacon handling
  mac80211: CSA on non-transmitting interfaces

Quoted 5 lines

Also, can we have hwsim support for proper testing? Seems with all the logic in mac80211 to give you the right beacons etc. that should be really simple?
johannes

Sure, will add hwsim tests.

BTW, do you see value in replacing all occurrences of *_multiple_bssid_* with *_mbssid_* (also in new nl80211 attributes).
Most lines needed to be split because of 80 character boundary, so this replacement will make that code look cleaner. I have avoided it till now to reduce number of changes between versions.
Thanks.

Johannes Berg

maintainer

2021-04-09 19:28 UTC #19 lore raw

Quoted 8 lines · Aloka Dixit

BTW, do you see value in replacing all occurrences of *_multiple_bssid_* with *_mbssid_* (also in new nl80211 attributes).
Most lines needed to be split because of 80 character boundary, so this replacement will make that code look cleaner. I have avoided it till now to reduce number of changes between versions.

Might be nicer, but I can live with the longer identifiers too. I don't really mind if you do it between versions though, so feel free.

johannes