[PATCH v8 0/5] Additional processing in beacon updates (v8)

[PATCH v8 0/5] Additional processing in beacon updates (v8)

[Aloka Dixit]

v8: Patches #1, #2 are new in this version which allow resetting
the interval to 0 once set to non-zero which was not possible earlier.
No functional changes to the remaining three patches from v7 here:
https://patchwork.kernel.org/project/linux-wireless/list/?series=693804&state=%2A&archive=both

v7: Resolved conflicts with MLO code changes.

FILS discovery and unsolicited broadcast probe response transmissions
are configured as part of NL80211_CMD_START_AP. The configurations
may get changed whenever there is a change in beacon, e. g. when
a channel switch operation completes the new templates will be sent
by the userspace which reflects the new channel bandwidth. Process the
attributes for these features in NL80211_CMD_SET_BEACON as well.

- Replace the check for interval (for both features) with a new flag
'update' which is set only when the userspace requests an update to
the configuration. This allows the interval to be set to 0 and
templates deleted which wasn't allowed earlier as the attributes got
processed only if the interval was non-zero.
- Modify the local variable in nl80211_set_beacon() and input parameter
to rdev_change_beacon() from type struct cfg80211_beacon_data to
type struct cfg80211_ap_settings to support the new processing.
- Modify ieee80211_change_beacon() to reflect the new input parameter type.
- Modify driver specific functions pointed by change_beacon to reflect
the new input parameter type.
- Add the missing implementation in nl80211 and mac80211 to process
FILS discovery and unsolicited broadcast probe response configurations.


Aloka Dixit (5):
  wifi: nl80211: fixes to FILS discovery updates
  wifi: mac80211: fixes in FILS discovery updates
  wifi: cfg80211: modify prototype for change_beacon
  wifi: nl80211: additions to NL80211_CMD_SET_BEACON
  wifi: mac80211: additions to change_beacon()

 drivers/net/wireless/ath/ath6kl/cfg80211.c    |   4 +-
 drivers/net/wireless/ath/wil6210/cfg80211.c   |   3 +-
 .../broadcom/brcm80211/brcmfmac/cfg80211.c    |   4 +-
 .../net/wireless/marvell/mwifiex/cfg80211.c   |   3 +-
 .../wireless/microchip/wilc1000/cfg80211.c    |   4 +-
 .../net/wireless/quantenna/qtnfmac/cfg80211.c |   4 +-
 .../staging/rtl8723bs/os_dep/ioctl_cfg80211.c |   6 +-
 include/net/cfg80211.h                        |   6 +-
 include/uapi/linux/nl80211.h                  |  11 +-
 net/mac80211/cfg.c                            | 102 ++++++++++--------
 net/wireless/nl80211.c                        |  47 ++++++--
 net/wireless/rdev-ops.h                       |   2 +-
 net/wireless/trace.h                          |  54 +++++-----
 13 files changed, 159 insertions(+), 91 deletions(-)


base-commit: b2090d93d4b6f1c72a9793d5a171806b8468b7cb
-- 
2.39.0

[PATCH v8 1/5] wifi: nl80211: fixes to FILS discovery updates

[Aloka Dixit]

Add a new flag 'update' which is set to true during start_ap() for
only if one of the following two conditions are met:
- Userspace passed an empty nested attribute which indicates that
the feature should be disabled and templates deleted.
- Userspace passed all the parameters for the nested attribute.

Existing configuration will not be changed while the flag
remains false.

Add similar changes for unsolicited broadcast probe response
transmission.

Signed-off-by: Aloka Dixit <quic_alokad@quicinc.com>
---
v8: New patch in this series.

 include/net/cfg80211.h       |  4 ++++
 include/uapi/linux/nl80211.h | 11 +++++++----
 net/wireless/nl80211.c       | 16 +++++++++++++++-
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7c7d03aa9d06..5317f67da67f 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1286,6 +1286,7 @@ struct cfg80211_acl_data {
  * struct cfg80211_fils_discovery - FILS discovery parameters from
  * IEEE Std 802.11ai-2016, Annex C.3 MIB detail.
  *
+ * @update: Set to true if the feature configuration should be updated.
  * @min_interval: Minimum packet interval in TUs (0 - 10000)
  * @max_interval: Maximum packet interval in TUs (0 - 10000)
  * @tmpl_len: Template length
@@ -1293,6 +1294,7 @@ struct cfg80211_acl_data {
  *	frame headers.
  */
 struct cfg80211_fils_discovery {
+	bool update;
 	u32 min_interval;
 	u32 max_interval;
 	size_t tmpl_len;
@@ -1303,6 +1305,7 @@ struct cfg80211_fils_discovery {
  * struct cfg80211_unsol_bcast_probe_resp - Unsolicited broadcast probe
  *	response parameters in 6GHz.
  *
+ * @update: Set to true if the feature configuration should be updated.
  * @interval: Packet interval in TUs. Maximum allowed is 20 TU, as mentioned
  *	in IEEE P802.11ax/D6.0 26.17.2.3.2 - AP behavior for fast passive
  *	scanning
@@ -1310,6 +1313,7 @@ struct cfg80211_fils_discovery {
  * @tmpl: Template data for probe response
  */
 struct cfg80211_unsol_bcast_probe_resp {
+	bool update;
 	u32 interval;
 	size_t tmpl_len;
 	const u8 *tmpl;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 88eb85c63029..97909e5fa959 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2690,11 +2690,13 @@ enum nl80211_commands {
  *
  * @NL80211_ATTR_FILS_DISCOVERY: Optional parameter to configure FILS
  *	discovery. It is a nested attribute, see
- *	&enum nl80211_fils_discovery_attributes.
+ *	&enum nl80211_fils_discovery_attributes. Userspace should pass an empty
+ *	nested attribute to disable this feature and delete the templates.
  *
  * @NL80211_ATTR_UNSOL_BCAST_PROBE_RESP: Optional parameter to configure
  *	unsolicited broadcast probe response. It is a nested attribute, see
- *	&enum nl80211_unsol_bcast_probe_resp_attributes.
+ *	&enum nl80211_unsol_bcast_probe_resp_attributes. Userspace should pass an empty
+ *	nested attribute to disable this feature and delete the templates.
  *
  * @NL80211_ATTR_S1G_CAPABILITY: S1G Capability information element (from
  *	association request when used with NL80211_CMD_NEW_STATION)
@@ -7606,7 +7608,7 @@ enum nl80211_iftype_akm_attributes {
  * @NL80211_FILS_DISCOVERY_ATTR_INT_MIN: Minimum packet interval (u32, TU).
  *	Allowed range: 0..10000 (TU = Time Unit)
  * @NL80211_FILS_DISCOVERY_ATTR_INT_MAX: Maximum packet interval (u32, TU).
- *	Allowed range: 0..10000 (TU = Time Unit)
+ *	Allowed range: 0..10000 (TU = Time Unit). If set to 0, the feature is disabled.
  * @NL80211_FILS_DISCOVERY_ATTR_TMPL: Template data for FILS discovery action
  *	frame including the headers.
  *
@@ -7639,7 +7641,8 @@ enum nl80211_fils_discovery_attributes {
  *
  * @NL80211_UNSOL_BCAST_PROBE_RESP_ATTR_INT: Maximum packet interval (u32, TU).
  *	Allowed range: 0..20 (TU = Time Unit). IEEE P802.11ax/D6.0
- *	26.17.2.3.2 (AP behavior for fast passive scanning).
+ *	26.17.2.3.2 (AP behavior for fast passive scanning). If set to 0, the feature is
+ *	disabled.
  * @NL80211_UNSOL_BCAST_PROBE_RESP_ATTR_TMPL: Unsolicited broadcast probe response
  *	frame template (binary).
  *
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 0da2e6a2a7ea..487a34cbc6fb 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5683,6 +5683,13 @@ static int nl80211_parse_fils_discovery(struct cfg80211_registered_device *rdev,
 	if (ret)
 		return ret;
 
+	if (!tb[NL80211_FILS_DISCOVERY_ATTR_INT_MIN] &&
+	    !tb[NL80211_FILS_DISCOVERY_ATTR_INT_MAX] &&
+	    !tb[NL80211_FILS_DISCOVERY_ATTR_TMPL]) {
+		fd->update = true;
+		return 0;
+	}
+
 	if (!tb[NL80211_FILS_DISCOVERY_ATTR_INT_MIN] ||
 	    !tb[NL80211_FILS_DISCOVERY_ATTR_INT_MAX] ||
 	    !tb[NL80211_FILS_DISCOVERY_ATTR_TMPL])
@@ -5692,7 +5699,7 @@ static int nl80211_parse_fils_discovery(struct cfg80211_registered_device *rdev,
 	fd->tmpl = nla_data(tb[NL80211_FILS_DISCOVERY_ATTR_TMPL]);
 	fd->min_interval = nla_get_u32(tb[NL80211_FILS_DISCOVERY_ATTR_INT_MIN]);
 	fd->max_interval = nla_get_u32(tb[NL80211_FILS_DISCOVERY_ATTR_INT_MAX]);
-
+	fd->update = true;
 	return 0;
 }
 
@@ -5715,6 +5722,12 @@ nl80211_parse_unsol_bcast_probe_resp(struct cfg80211_registered_device *rdev,
 	if (ret)
 		return ret;
 
+	if (!tb[NL80211_UNSOL_BCAST_PROBE_RESP_ATTR_INT] &&
+	    !tb[NL80211_UNSOL_BCAST_PROBE_RESP_ATTR_TMPL]) {
+		presp->update = true;
+		return 0;
+	}
+
 	if (!tb[NL80211_UNSOL_BCAST_PROBE_RESP_ATTR_INT] ||
 	    !tb[NL80211_UNSOL_BCAST_PROBE_RESP_ATTR_TMPL])
 		return -EINVAL;
@@ -5722,6 +5735,7 @@ nl80211_parse_unsol_bcast_probe_resp(struct cfg80211_registered_device *rdev,
 	presp->tmpl = nla_data(tb[NL80211_UNSOL_BCAST_PROBE_RESP_ATTR_TMPL]);
 	presp->tmpl_len = nla_len(tb[NL80211_UNSOL_BCAST_PROBE_RESP_ATTR_TMPL]);
 	presp->interval = nla_get_u32(tb[NL80211_UNSOL_BCAST_PROBE_RESP_ATTR_INT]);
+	presp->update = true;
 	return 0;
 }
 
-- 
2.39.0

[PATCH v8 2/5] wifi: mac80211: fixes in FILS discovery updates

[Aloka Dixit]

FILS discovery configuration gets updated only if the maximum interval
is set to a non-zero value, hence there is no way to reset this value
to 0 once set. Replace the check for interval with a new flag which is
set only if the configuration should be updated.

Add similar changes for the unsolicited broadcast probe response handling.

Signed-off-by: Aloka Dixit <quic_alokad@quicinc.com>
---
v8: New patch in this series.

 net/mac80211/cfg.c | 75 +++++++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 37 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index e7ac24603892..dcf152114652 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -984,25 +984,28 @@ static int ieee80211_set_fils_discovery(struct ieee80211_sub_if_data *sdata,
 	struct fils_discovery_data *new, *old = NULL;
 	struct ieee80211_fils_discovery *fd;
 
-	if (!params->tmpl || !params->tmpl_len)
-		return -EINVAL;
+	if (!params->update)
+		return 0;
 
 	fd = &link_conf->fils_discovery;
 	fd->min_interval = params->min_interval;
 	fd->max_interval = params->max_interval;
 
 	old = sdata_dereference(link->u.ap.fils_discovery, sdata);
-	new = kzalloc(sizeof(*new) + params->tmpl_len, GFP_KERNEL);
-	if (!new)
-		return -ENOMEM;
-	new->len = params->tmpl_len;
-	memcpy(new->data, params->tmpl, params->tmpl_len);
-	rcu_assign_pointer(link->u.ap.fils_discovery, new);
-
 	if (old)
 		kfree_rcu(old, rcu_head);
 
-	return 0;
+	RCU_INIT_POINTER(link->u.ap.fils_discovery, NULL);
+	if (params->tmpl && params->tmpl_len) {
+		new = kzalloc(sizeof(*new) + params->tmpl_len, GFP_KERNEL);
+		if (!new)
+			return -ENOMEM;
+		new->len = params->tmpl_len;
+		memcpy(new->data, params->tmpl, params->tmpl_len);
+		rcu_assign_pointer(link->u.ap.fils_discovery, new);
+	}
+
+	return BSS_CHANGED_FILS_DISCOVERY;
 }
 
 static int
@@ -1013,23 +1016,26 @@ ieee80211_set_unsol_bcast_probe_resp(struct ieee80211_sub_if_data *sdata,
 {
 	struct unsol_bcast_probe_resp_data *new, *old = NULL;
 
-	if (!params->tmpl || !params->tmpl_len)
-		return -EINVAL;
+	if (!params->update)
+		return 0;
 
-	old = sdata_dereference(link->u.ap.unsol_bcast_probe_resp, sdata);
-	new = kzalloc(sizeof(*new) + params->tmpl_len, GFP_KERNEL);
-	if (!new)
-		return -ENOMEM;
-	new->len = params->tmpl_len;
-	memcpy(new->data, params->tmpl, params->tmpl_len);
-	rcu_assign_pointer(link->u.ap.unsol_bcast_probe_resp, new);
+	link_conf->unsol_bcast_probe_resp_interval = params->interval;
 
+	old = sdata_dereference(link->u.ap.unsol_bcast_probe_resp, sdata);
 	if (old)
 		kfree_rcu(old, rcu_head);
 
-	link_conf->unsol_bcast_probe_resp_interval = params->interval;
+	RCU_INIT_POINTER(link->u.ap.unsol_bcast_probe_resp, NULL);
+	if (params->tmpl && params->tmpl_len) {
+		new = kzalloc(sizeof(*new) + params->tmpl_len, GFP_KERNEL);
+		if (!new)
+			return -ENOMEM;
+		new->len = params->tmpl_len;
+		memcpy(new->data, params->tmpl, params->tmpl_len);
+		rcu_assign_pointer(link->u.ap.unsol_bcast_probe_resp, new);
+	}
 
-	return 0;
+	return BSS_CHANGED_UNSOL_BCAST_PROBE_RESP;
 }
 
 static int ieee80211_set_ftm_responder_params(
@@ -1460,23 +1466,18 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	if (err < 0)
 		goto error;
 
-	if (params->fils_discovery.max_interval) {
-		err = ieee80211_set_fils_discovery(sdata,
-						   &params->fils_discovery,
-						   link, link_conf);
-		if (err < 0)
-			goto error;
-		changed |= BSS_CHANGED_FILS_DISCOVERY;
-	}
+	err = ieee80211_set_fils_discovery(sdata, &params->fils_discovery,
+					   link, link_conf);
+	if (err < 0)
+		goto error;
+	changed |= err;
 
-	if (params->unsol_bcast_probe_resp.interval) {
-		err = ieee80211_set_unsol_bcast_probe_resp(sdata,
-							   &params->unsol_bcast_probe_resp,
-							   link, link_conf);
-		if (err < 0)
-			goto error;
-		changed |= BSS_CHANGED_UNSOL_BCAST_PROBE_RESP;
-	}
+	err = ieee80211_set_unsol_bcast_probe_resp(sdata,
+						   &params->unsol_bcast_probe_resp,
+						   link, link_conf);
+	if (err < 0)
+		goto error;
+	changed |= err;
 
 	err = drv_start_ap(sdata->local, sdata, link_conf);
 	if (err) {
-- 
2.39.0

[PATCH v8 3/5] wifi: cfg80211: modify prototype for change_beacon

[Aloka Dixit]

Modify the prototype for change_beacon() in struct cfg80211_op to
accept cfg80211_ap_settings instead of cfg80211_beacon_data so that
it can process data in addition to beacons.
Modify the prototypes of ieee80211_change_beacon() and driver specific
functions accordingly.

Signed-off-by: Aloka Dixit <quic_alokad@quicinc.com>
---
v8: Only cosmetic changes.

 drivers/net/wireless/ath/ath6kl/cfg80211.c    |  4 +-
 drivers/net/wireless/ath/wil6210/cfg80211.c   |  3 +-
 .../broadcom/brcm80211/brcmfmac/cfg80211.c    |  4 +-
 .../net/wireless/marvell/mwifiex/cfg80211.c   |  3 +-
 .../wireless/microchip/wilc1000/cfg80211.c    |  4 +-
 .../net/wireless/quantenna/qtnfmac/cfg80211.c |  4 +-
 .../staging/rtl8723bs/os_dep/ioctl_cfg80211.c |  6 ++-
 include/net/cfg80211.h                        |  2 +-
 net/mac80211/cfg.c                            | 14 ++---
 net/wireless/nl80211.c                        | 16 ++++--
 net/wireless/rdev-ops.h                       |  2 +-
 net/wireless/trace.h                          | 54 ++++++++++---------
 12 files changed, 67 insertions(+), 49 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index 0c2b8b1a10d5..e7ce1de77f09 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -2954,7 +2954,7 @@ static int ath6kl_start_ap(struct wiphy *wiphy, struct net_device *dev,
 }
 
 static int ath6kl_change_beacon(struct wiphy *wiphy, struct net_device *dev,
-				struct cfg80211_beacon_data *beacon)
+				struct cfg80211_ap_settings *params)
 {
 	struct ath6kl_vif *vif = netdev_priv(dev);
 
@@ -2964,7 +2964,7 @@ static int ath6kl_change_beacon(struct wiphy *wiphy, struct net_device *dev,
 	if (vif->next_mode != AP_NETWORK)
 		return -EOPNOTSUPP;
 
-	return ath6kl_set_ies(vif, beacon);
+	return ath6kl_set_ies(vif, &params->beacon);
 }
 
 static int ath6kl_stop_ap(struct wiphy *wiphy, struct net_device *dev,
diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index 40f9a7ef8980..dfbb478ae274 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -2082,11 +2082,12 @@ void wil_cfg80211_ap_recovery(struct wil6210_priv *wil)
 
 static int wil_cfg80211_change_beacon(struct wiphy *wiphy,
 				      struct net_device *ndev,
-				      struct cfg80211_beacon_data *bcon)
+				      struct cfg80211_ap_settings *params)
 {
 	struct wil6210_priv *wil = wiphy_to_wil(wiphy);
 	struct wireless_dev *wdev = ndev->ieee80211_ptr;
 	struct wil6210_vif *vif = ndev_to_vif(ndev);
+	struct cfg80211_beacon_data *bcon = &params->beacon;
 	int rc;
 	u32 privacy = 0;
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index de8a2e27f49c..6a71fb22aa12 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5411,13 +5411,13 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
 
 static s32
 brcmf_cfg80211_change_beacon(struct wiphy *wiphy, struct net_device *ndev,
-			     struct cfg80211_beacon_data *info)
+			     struct cfg80211_ap_settings *info)
 {
 	struct brcmf_if *ifp = netdev_priv(ndev);
 
 	brcmf_dbg(TRACE, "Enter\n");
 
-	return brcmf_config_ap_mgmt_ie(ifp->vif, info);
+	return brcmf_config_ap_mgmt_ie(ifp->vif, &info->beacon);
 }
 
 static int
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index ba4e29713a8c..70473be42d7b 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1835,10 +1835,11 @@ static int mwifiex_cfg80211_set_cqm_rssi_config(struct wiphy *wiphy,
  */
 static int mwifiex_cfg80211_change_beacon(struct wiphy *wiphy,
 					  struct net_device *dev,
-					  struct cfg80211_beacon_data *data)
+					  struct cfg80211_ap_settings *params)
 {
 	struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
 	struct mwifiex_adapter *adapter = priv->adapter;
+	struct cfg80211_beacon_data *data = &params->beacon;
 
 	mwifiex_cancel_scan(adapter);
 
diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
index b545d93c6e37..3447470d3d02 100644
--- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
+++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
@@ -1441,11 +1441,11 @@ static int start_ap(struct wiphy *wiphy, struct net_device *dev,
 }
 
 static int change_beacon(struct wiphy *wiphy, struct net_device *dev,
-			 struct cfg80211_beacon_data *beacon)
+			 struct cfg80211_ap_settings *params)
 {
 	struct wilc_vif *vif = netdev_priv(dev);
 
-	return wilc_add_beacon(vif, 0, 0, beacon);
+	return wilc_add_beacon(vif, 0, 0, &params->beacon);
 }
 
 static int stop_ap(struct wiphy *wiphy, struct net_device *dev,
diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 73e6f9408b51..9388adcdcac1 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -331,11 +331,11 @@ static int qtnf_mgmt_set_appie(struct qtnf_vif *vif,
 }
 
 static int qtnf_change_beacon(struct wiphy *wiphy, struct net_device *dev,
-			      struct cfg80211_beacon_data *info)
+			      struct cfg80211_ap_settings *info)
 {
 	struct qtnf_vif *vif = qtnf_netdev_get_priv(dev);
 
-	return qtnf_mgmt_set_appie(vif, info);
+	return qtnf_mgmt_set_appie(vif, &info->beacon);
 }
 
 static int qtnf_start_ap(struct wiphy *wiphy, struct net_device *dev,
diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index 2ae7843abdf7..5ddc2d9a6060 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -2319,11 +2319,13 @@ static int cfg80211_rtw_start_ap(struct wiphy *wiphy, struct net_device *ndev,
 }
 
 static int cfg80211_rtw_change_beacon(struct wiphy *wiphy, struct net_device *ndev,
-		struct cfg80211_beacon_data *info)
+		struct cfg80211_ap_settings *info)
 {
 	struct adapter *adapter = rtw_netdev_priv(ndev);
 
-	return rtw_add_beacon(adapter, info->head, info->head_len, info->tail, info->tail_len);
+	return rtw_add_beacon(adapter, info->beacon.head,
+			      info->beacon.head_len, info->beacon.tail,
+			      info->beacon.tail_len);
 }
 
 static int cfg80211_rtw_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 5317f67da67f..a4552d8f8200 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4452,7 +4452,7 @@ struct cfg80211_ops {
 	int	(*start_ap)(struct wiphy *wiphy, struct net_device *dev,
 			    struct cfg80211_ap_settings *settings);
 	int	(*change_beacon)(struct wiphy *wiphy, struct net_device *dev,
-				 struct cfg80211_beacon_data *info);
+				 struct cfg80211_ap_settings *info);
 	int	(*stop_ap)(struct wiphy *wiphy, struct net_device *dev,
 			   unsigned int link_id);
 
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index dcf152114652..3fe424992893 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1509,10 +1509,12 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 }
 
 static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
-				   struct cfg80211_beacon_data *params)
+				   struct cfg80211_ap_settings *params)
+
 {
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_link_data *link;
+	struct cfg80211_beacon_data *beacon = &params->beacon;
 	struct beacon_data *old;
 	int err;
 	struct ieee80211_bss_conf *link_conf;
@@ -1520,7 +1522,7 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
 
 	sdata_assert_lock(sdata);
 
-	link = sdata_dereference(sdata->link[params->link_id], sdata);
+	link = sdata_dereference(sdata->link[beacon->link_id], sdata);
 	if (!link)
 		return -ENOLINK;
 
@@ -1536,14 +1538,14 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
 	if (!old)
 		return -ENOENT;
 
-	err = ieee80211_assign_beacon(sdata, link, params, NULL, NULL,
+	err = ieee80211_assign_beacon(sdata, link, beacon, NULL, NULL,
 				      &changed);
 	if (err < 0)
 		return err;
 
-	if (params->he_bss_color_valid &&
-	    params->he_bss_color.enabled != link_conf->he_bss_color.enabled) {
-		link_conf->he_bss_color.enabled = params->he_bss_color.enabled;
+	if (beacon->he_bss_color_valid &&
+	    beacon->he_bss_color.enabled != link_conf->he_bss_color.enabled) {
+		link_conf->he_bss_color.enabled = beacon->he_bss_color.enabled;
 		changed |= BSS_CHANGED_HE_BSS_COLOR;
 	}
 
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 487a34cbc6fb..5e087534a12e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6238,7 +6238,7 @@ static int nl80211_set_beacon(struct sk_buff *skb, struct genl_info *info)
 	unsigned int link_id = nl80211_link_id(info->attrs);
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
-	struct cfg80211_beacon_data params;
+	struct cfg80211_ap_settings *params;
 	int err;
 
 	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
@@ -6251,17 +6251,23 @@ static int nl80211_set_beacon(struct sk_buff *skb, struct genl_info *info)
 	if (!wdev->links[link_id].ap.beacon_interval)
 		return -EINVAL;
 
-	err = nl80211_parse_beacon(rdev, info->attrs, &params, info->extack);
+	params = kzalloc(sizeof(*params), GFP_KERNEL);
+	if (!params)
+		return -ENOMEM;
+
+	err = nl80211_parse_beacon(rdev, info->attrs, &params->beacon,
+				   info->extack);
 	if (err)
 		goto out;
 
 	wdev_lock(wdev);
-	err = rdev_change_beacon(rdev, dev, &params);
+	err = rdev_change_beacon(rdev, dev, params);
 	wdev_unlock(wdev);
 
 out:
-	kfree(params.mbssid_ies);
-	kfree(params.rnr_ies);
+	kfree(params->beacon.mbssid_ies);
+	kfree(params->beacon.rnr_ies);
+	kfree(params);
 	return err;
 }
 
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 90bb7ac4b930..c6a2c07e380b 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -173,7 +173,7 @@ static inline int rdev_start_ap(struct cfg80211_registered_device *rdev,
 
 static inline int rdev_change_beacon(struct cfg80211_registered_device *rdev,
 				     struct net_device *dev,
-				     struct cfg80211_beacon_data *info)
+				     struct cfg80211_ap_settings *info)
 {
 	int ret;
 	trace_rdev_change_beacon(&rdev->wiphy, dev, info);
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 617c0d0dfa96..5579b6a4f314 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -615,46 +615,52 @@ TRACE_EVENT(rdev_start_ap,
 
 TRACE_EVENT(rdev_change_beacon,
 	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
-		 struct cfg80211_beacon_data *info),
+		 struct cfg80211_ap_settings *info),
 	TP_ARGS(wiphy, netdev, info),
 	TP_STRUCT__entry(
 		WIPHY_ENTRY
 		NETDEV_ENTRY
 		__field(int, link_id)
-		__dynamic_array(u8, head, info ? info->head_len : 0)
-		__dynamic_array(u8, tail, info ? info->tail_len : 0)
-		__dynamic_array(u8, beacon_ies, info ? info->beacon_ies_len : 0)
+		__dynamic_array(u8, head, info ? info->beacon.head_len : 0)
+		__dynamic_array(u8, tail, info ? info->beacon.tail_len : 0)
+		__dynamic_array(u8, beacon_ies,
+				info ? info->beacon.beacon_ies_len : 0)
 		__dynamic_array(u8, proberesp_ies,
-				info ? info->proberesp_ies_len : 0)
+				info ? info->beacon.proberesp_ies_len : 0)
 		__dynamic_array(u8, assocresp_ies,
-				info ? info->assocresp_ies_len : 0)
-		__dynamic_array(u8, probe_resp, info ? info->probe_resp_len : 0)
+				info ? info->beacon.assocresp_ies_len : 0)
+		__dynamic_array(u8, probe_resp,
+				info ? info->beacon.probe_resp_len : 0)
 	),
 	TP_fast_assign(
 		WIPHY_ASSIGN;
 		NETDEV_ASSIGN;
 		if (info) {
-			__entry->link_id = info->link_id;
-			if (info->head)
-				memcpy(__get_dynamic_array(head), info->head,
-				       info->head_len);
-			if (info->tail)
-				memcpy(__get_dynamic_array(tail), info->tail,
-				       info->tail_len);
-			if (info->beacon_ies)
+			__entry->link_id = info->beacon.link_id;
+			if (info->beacon.head)
+				memcpy(__get_dynamic_array(head),
+				       info->beacon.head,
+				       info->beacon.head_len);
+			if (info->beacon.tail)
+				memcpy(__get_dynamic_array(tail),
+				       info->beacon.tail,
+				       info->beacon.tail_len);
+			if (info->beacon.beacon_ies)
 				memcpy(__get_dynamic_array(beacon_ies),
-				       info->beacon_ies, info->beacon_ies_len);
-			if (info->proberesp_ies)
+				       info->beacon.beacon_ies,
+				       info->beacon.beacon_ies_len);
+			if (info->beacon.proberesp_ies)
 				memcpy(__get_dynamic_array(proberesp_ies),
-				       info->proberesp_ies,
-				       info->proberesp_ies_len);
-			if (info->assocresp_ies)
+				       info->beacon.proberesp_ies,
+				       info->beacon.proberesp_ies_len);
+			if (info->beacon.assocresp_ies)
 				memcpy(__get_dynamic_array(assocresp_ies),
-				       info->assocresp_ies,
-				       info->assocresp_ies_len);
-			if (info->probe_resp)
+				       info->beacon.assocresp_ies,
+				       info->beacon.assocresp_ies_len);
+			if (info->beacon.probe_resp)
 				memcpy(__get_dynamic_array(probe_resp),
-				       info->probe_resp, info->probe_resp_len);
+				       info->beacon.probe_resp,
+				       info->beacon.probe_resp_len);
 		} else {
 			__entry->link_id = -1;
 		}
-- 
2.39.0

[PATCH v8 4/5] wifi: nl80211: additions to NL80211_CMD_SET_BEACON

[Aloka Dixit]

FILS discovery and unsolicited broadcast probe response templates
need to be updated along with beacon templates in some cases such as
the channel switch operation. Add the missing implementation.

Signed-off-by: Aloka Dixit <quic_alokad@quicinc.com>
---
v8: No changes.

 net/wireless/nl80211.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5e087534a12e..bb1ab78d3f27 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6239,6 +6239,7 @@ static int nl80211_set_beacon(struct sk_buff *skb, struct genl_info *info)
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_ap_settings *params;
+	struct nlattr *attrs;
 	int err;
 
 	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
@@ -6260,6 +6261,20 @@ static int nl80211_set_beacon(struct sk_buff *skb, struct genl_info *info)
 	if (err)
 		goto out;
 
+	attrs = info->attrs[NL80211_ATTR_FILS_DISCOVERY];
+	if (attrs) {
+		err = nl80211_parse_fils_discovery(rdev, attrs, params);
+		if (err)
+			goto out;
+	}
+
+	attrs = info->attrs[NL80211_ATTR_UNSOL_BCAST_PROBE_RESP];
+	if (attrs) {
+		err = nl80211_parse_unsol_bcast_probe_resp(rdev, attrs, params);
+		if (err)
+			goto out;
+	}
+
 	wdev_lock(wdev);
 	err = rdev_change_beacon(rdev, dev, params);
 	wdev_unlock(wdev);
-- 
2.39.0

[PATCH v8 5/5] wifi: mac80211: additions to change_beacon()

[Aloka Dixit]

Process FILS discovery and unsolicited broadcast probe response
transmission configurations in ieee80211_change_beacon().

Signed-off-by: Aloka Dixit <quic_alokad@quicinc.com>
---
v8: Do not check for a non-zero interval value to decide
whether to process the configuration.

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

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 3fe424992893..12e6c782a3c2 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1543,6 +1543,19 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
 	if (err < 0)
 		return err;
 
+	err = ieee80211_set_fils_discovery(sdata, &params->fils_discovery,
+					   link, link_conf);
+	if (err < 0)
+		return err;
+	changed |= err;
+
+	err = ieee80211_set_unsol_bcast_probe_resp(sdata,
+						   &params->unsol_bcast_probe_resp,
+						   link, link_conf);
+	if (err < 0)
+		return err;
+	changed |= err;
+
 	if (beacon->he_bss_color_valid &&
 	    beacon->he_bss_color.enabled != link_conf->he_bss_color.enabled) {
 		link_conf->he_bss_color.enabled = beacon->he_bss_color.enabled;
-- 
2.39.0

Re: [PATCH v8 0/5] Additional processing in beacon updates (v8)

[Jeff Johnson]

On 7/27/2023 10:40 AM, Aloka Dixit wrote:
> v8: Patches #1, #2 are new in this version which allow resetting
> the interval to 0 once set to non-zero which was not possible earlier.
> No functional changes to the remaining three patches from v7 here:
> https://patchwork.kernel.org/project/linux-wireless/list/?series=693804&state=%2A&archive=both
> 
> v7: Resolved conflicts with MLO code changes.
> 
> FILS discovery and unsolicited broadcast probe response transmissions
> are configured as part of NL80211_CMD_START_AP. The configurations
> may get changed whenever there is a change in beacon, e. g. when
> a channel switch operation completes the new templates will be sent
> by the userspace which reflects the new channel bandwidth. Process the
> attributes for these features in NL80211_CMD_SET_BEACON as well.
> 
> - Replace the check for interval (for both features) with a new flag
> 'update' which is set only when the userspace requests an update to
> the configuration. This allows the interval to be set to 0 and
> templates deleted which wasn't allowed earlier as the attributes got
> processed only if the interval was non-zero.
> - Modify the local variable in nl80211_set_beacon() and input parameter
> to rdev_change_beacon() from type struct cfg80211_beacon_data to
> type struct cfg80211_ap_settings to support the new processing.
> - Modify ieee80211_change_beacon() to reflect the new input parameter type.
> - Modify driver specific functions pointed by change_beacon to reflect
> the new input parameter type.
> - Add the missing implementation in nl80211 and mac80211 to process
> FILS discovery and unsolicited broadcast probe response configurations.
> 
> 
> Aloka Dixit (5):
>    wifi: nl80211: fixes to FILS discovery updates
>    wifi: mac80211: fixes in FILS discovery updates
>    wifi: cfg80211: modify prototype for change_beacon
>    wifi: nl80211: additions to NL80211_CMD_SET_BEACON
>    wifi: mac80211: additions to change_beacon()
> 
>   drivers/net/wireless/ath/ath6kl/cfg80211.c    |   4 +-
>   drivers/net/wireless/ath/wil6210/cfg80211.c   |   3 +-
>   .../broadcom/brcm80211/brcmfmac/cfg80211.c    |   4 +-
>   .../net/wireless/marvell/mwifiex/cfg80211.c   |   3 +-
>   .../wireless/microchip/wilc1000/cfg80211.c    |   4 +-
>   .../net/wireless/quantenna/qtnfmac/cfg80211.c |   4 +-
>   .../staging/rtl8723bs/os_dep/ioctl_cfg80211.c |   6 +-
>   include/net/cfg80211.h                        |   6 +-
>   include/uapi/linux/nl80211.h                  |  11 +-
>   net/mac80211/cfg.c                            | 102 ++++++++++--------
>   net/wireless/nl80211.c                        |  47 ++++++--
>   net/wireless/rdev-ops.h                       |   2 +-
>   net/wireless/trace.h                          |  54 +++++-----
>   13 files changed, 159 insertions(+), 91 deletions(-)
> 
> 
> base-commit: b2090d93d4b6f1c72a9793d5a171806b8468b7cb
for the series:

Reviewed-by: Jeff Johnson <quic_jjohnson@quicinc.com>

Re: [PATCH v8 0/5] Additional processing in beacon updates (v8)

[Aloka Dixit]

On 8/16/2023 11:38 AM, Jeff Johnson wrote:
> On 7/27/2023 10:40 AM, Aloka Dixit wrote:
>> v8: Patches #1, #2 are new in this version which allow resetting
>> the interval to 0 once set to non-zero which was not possible earlier.
>> No functional changes to the remaining three patches from v7 here:
>> https://patchwork.kernel.org/project/linux-wireless/list/?series=693804&state=%2A&archive=both
>>
>> v7: Resolved conflicts with MLO code changes.
>>
>> FILS discovery and unsolicited broadcast probe response transmissions
>> are configured as part of NL80211_CMD_START_AP. The configurations
>> may get changed whenever there is a change in beacon, e. g. when
>> a channel switch operation completes the new templates will be sent
>> by the userspace which reflects the new channel bandwidth. Process the
>> attributes for these features in NL80211_CMD_SET_BEACON as well.
>>
>> - Replace the check for interval (for both features) with a new flag
>> 'update' which is set only when the userspace requests an update to
>> the configuration. This allows the interval to be set to 0 and
>> templates deleted which wasn't allowed earlier as the attributes got
>> processed only if the interval was non-zero.
>> - Modify the local variable in nl80211_set_beacon() and input parameter
>> to rdev_change_beacon() from type struct cfg80211_beacon_data to
>> type struct cfg80211_ap_settings to support the new processing.
>> - Modify ieee80211_change_beacon() to reflect the new input parameter 
>> type.
>> - Modify driver specific functions pointed by change_beacon to reflect
>> the new input parameter type.
>> - Add the missing implementation in nl80211 and mac80211 to process
>> FILS discovery and unsolicited broadcast probe response configurations.
>>
>>
>> Aloka Dixit (5):
>>    wifi: nl80211: fixes to FILS discovery updates
>>    wifi: mac80211: fixes in FILS discovery updates
>>    wifi: cfg80211: modify prototype for change_beacon
>>    wifi: nl80211: additions to NL80211_CMD_SET_BEACON
>>    wifi: mac80211: additions to change_beacon()
>>
>>   drivers/net/wireless/ath/ath6kl/cfg80211.c    |   4 +-
>>   drivers/net/wireless/ath/wil6210/cfg80211.c   |   3 +-
>>   .../broadcom/brcm80211/brcmfmac/cfg80211.c    |   4 +-
>>   .../net/wireless/marvell/mwifiex/cfg80211.c   |   3 +-
>>   .../wireless/microchip/wilc1000/cfg80211.c    |   4 +-
>>   .../net/wireless/quantenna/qtnfmac/cfg80211.c |   4 +-
>>   .../staging/rtl8723bs/os_dep/ioctl_cfg80211.c |   6 +-
>>   include/net/cfg80211.h                        |   6 +-
>>   include/uapi/linux/nl80211.h                  |  11 +-
>>   net/mac80211/cfg.c                            | 102 ++++++++++--------
>>   net/wireless/nl80211.c                        |  47 ++++++--
>>   net/wireless/rdev-ops.h                       |   2 +-
>>   net/wireless/trace.h                          |  54 +++++-----
>>   13 files changed, 159 insertions(+), 91 deletions(-)
>>
>>
>> base-commit: b2090d93d4b6f1c72a9793d5a171806b8468b7cb
> for the series:
> 
> Reviewed-by: Jeff Johnson <quic_jjohnson@quicinc.com>
> 

Thanks for the review!

Re: [PATCH v8 0/5] Additional processing in beacon updates (v8)

[Aloka Dixit]

On 8/17/2023 11:04 AM, Aloka Dixit wrote:
> On 8/16/2023 11:38 AM, Jeff Johnson wrote:
>> On 7/27/2023 10:40 AM, Aloka Dixit wrote:
>>> v8: Patches #1, #2 are new in this version which allow resetting
>>> the interval to 0 once set to non-zero which was not possible earlier.
>>> No functional changes to the remaining three patches from v7 here:
>>> https://patchwork.kernel.org/project/linux-wireless/list/?series=693804&state=%2A&archive=both
>>>
>>> v7: Resolved conflicts with MLO code changes.
>>>
>>> FILS discovery and unsolicited broadcast probe response transmissions
>>> are configured as part of NL80211_CMD_START_AP. The configurations
>>> may get changed whenever there is a change in beacon, e. g. when
>>> a channel switch operation completes the new templates will be sent
>>> by the userspace which reflects the new channel bandwidth. Process the
>>> attributes for these features in NL80211_CMD_SET_BEACON as well.
>>>
>>> - Replace the check for interval (for both features) with a new flag
>>> 'update' which is set only when the userspace requests an update to
>>> the configuration. This allows the interval to be set to 0 and
>>> templates deleted which wasn't allowed earlier as the attributes got
>>> processed only if the interval was non-zero.
>>> - Modify the local variable in nl80211_set_beacon() and input parameter
>>> to rdev_change_beacon() from type struct cfg80211_beacon_data to
>>> type struct cfg80211_ap_settings to support the new processing.
>>> - Modify ieee80211_change_beacon() to reflect the new input parameter 
>>> type.
>>> - Modify driver specific functions pointed by change_beacon to reflect
>>> the new input parameter type.
>>> - Add the missing implementation in nl80211 and mac80211 to process
>>> FILS discovery and unsolicited broadcast probe response configurations.
>>>
>>> base-commit: b2090d93d4b6f1c72a9793d5a171806b8468b7cb
>> for the series:
>>
>> Reviewed-by: Jeff Johnson <quic_jjohnson@quicinc.com>
>>
> 
> Thanks for the review!


Hello Johannes,

Please review this series.

I know this version is too late to recollect the context of earlier 
patches but hopefully following is helpful.

Versions 1 to 7 tried to fix this issue - FILS discovery transmission 
stops after CSA. I had tried to fix it in mac80211 which did not set 
BSS_CHANGED_FILS_DISCOVERY unless new parameters were sent by user-space.

For v7, you mentioned that while the flag=0 indicates that FILS 
configurations did not change, it does not indicate that it got deleted 
so the driver should decide depending on the existing configuration and
not depend only on the flag. I have already validated this ath12k patch 
which fixes the above issue, without cfg80211 and mac80211 patches in 
this series: 
https://patchwork.kernel.org/project/linux-wireless/patch/20230905174324.25296-1-quic_alokad@quicinc.com/

And I have changed this series to let the user-space give 'interval=0' 
as the parameter which was basically a no-op earlier. This way the 
transmission can be stopped explicitly and include the additional 
processing in the change_beacon from the previous versions which was 
anyway required.

Thanks.

Re: [PATCH v8 0/5] Additional processing in beacon updates (v8)

[Johannes Berg]

Hi Aloka,

> Please review this series.

Yes, sorry for the delay.

> I know this version is too late to recollect the context of earlier 
> patches but hopefully following is helpful.
> 
> Versions 1 to 7 tried to fix this issue - FILS discovery transmission 
> stops after CSA. I had tried to fix it in mac80211 which did not set 
> BSS_CHANGED_FILS_DISCOVERY unless new parameters were sent by user-space.
> 
> For v7, you mentioned that while the flag=0 indicates that FILS 
> configurations did not change, it does not indicate that it got deleted 
> so the driver should decide depending on the existing configuration and
> not depend only on the flag. I have already validated this ath12k patch 
> which fixes the above issue, without cfg80211 and mac80211 patches in 
> this series: 
> https://patchwork.kernel.org/project/linux-wireless/patch/20230905174324.25296-1-quic_alokad@quicinc.com/
> 
> And I have changed this series to let the user-space give 'interval=0' 
> as the parameter which was basically a no-op earlier. This way the 
> transmission can be stopped explicitly and include the additional 
> processing in the change_beacon from the previous versions which was 
> anyway required.
> 

Yep, thanks a lot!

I've applied the series since I was rebasing it on the current tree with
the locking changes and while that wasn't hard, I didn't want to
needlessly double the work and have you do it for a resend as well.

I've made some small tweaks and fixes, so please take a look at it, I
hope I didn't mess anything up.

Also, I'd like you to send a follow-up patch that updates the
documentation: now that we pass the whole settings to change_beacon(), I
think we need to document - perhaps as part of the kernel-doc for struct
cfg80211_ap_settings - which of the parameters are actually changing
there. Right now given your patches, it's clear that only beacon,
unsol_bcast_probe_resp and fils_discovery are (currently) allowed to
change.


Alternatively, maybe we should indeed change the prototype again and
introduce a new struct cfg80211_ap_update that contains only the
parameters that change?

That feels a bit harder, but really it isn't by that much - in mac80211
ieee80211_set_fils_discovery() etc. already take the sub-parameters
(&params->fils_discovery), so not a problem there, and in nl80211 we
could as well pass struct cfg80211_fils_discovery directly to
nl80211_parse_fils_discovery() rather than the entire struct
cfg80211_ap_settings, which wouldn't be a massive change.


I think maybe I even prefer the latter if I'm looking at it now, but I'm
not sure I'm not missing something from earlier discussions on this.

What do you think?

johannes

Re: [PATCH v8 0/5] Additional processing in beacon updates (v8)

[Aloka Dixit]

On 9/13/2023 3:26 AM, Johannes Berg wrote:
> Hi Aloka,
> 
> I've applied the series since I was rebasing it on the current tree with
> the locking changes and while that wasn't hard, I didn't want to
> needlessly double the work and have you do it for a resend as well.
> 
> I've made some small tweaks and fixes, so please take a look at it, I
> hope I didn't mess anything up.
> 

Thanks!

> Also, I'd like you to send a follow-up patch that updates the
> documentation: now that we pass the whole settings to change_beacon(), I
> think we need to document - perhaps as part of the kernel-doc for struct
> cfg80211_ap_settings - which of the parameters are actually changing
> there. Right now given your patches, it's clear that only beacon,
> unsol_bcast_probe_resp and fils_discovery are (currently) allowed to
> change.
> 
> 
> Alternatively, maybe we should indeed change the prototype again and
> introduce a new struct cfg80211_ap_update that contains only the
> parameters that change?
> 
> That feels a bit harder, but really it isn't by that much - in mac80211
> ieee80211_set_fils_discovery() etc. already take the sub-parameters
> (&params->fils_discovery), so not a problem there, and in nl80211 we
> could as well pass struct cfg80211_fils_discovery directly to
> nl80211_parse_fils_discovery() rather than the entire struct
> cfg80211_ap_settings, which wouldn't be a massive change.
> 
> 
> I think maybe I even prefer the latter if I'm looking at it now, but I'm
> not sure I'm not missing something from earlier discussions on this.
> 
> What do you think?
> 
> johannes

The second option will take couple of weeks due to current work load. 
How about I do the first option, kernel-doc, until then?

Thanks.

Re: [PATCH v8 0/5] Additional processing in beacon updates (v8)

[Johannes Berg]

On Mon, 2023-09-18 at 15:29 -0700, Aloka Dixit wrote:
> > Alternatively, maybe we should indeed change the prototype again and
> > introduce a new struct cfg80211_ap_update that contains only the
> > parameters that change?
> > 
> > That feels a bit harder, but really it isn't by that much - in mac80211
> > ieee80211_set_fils_discovery() etc. already take the sub-parameters
> > (&params->fils_discovery), so not a problem there, and in nl80211 we
> > could as well pass struct cfg80211_fils_discovery directly to
> > nl80211_parse_fils_discovery() rather than the entire struct
> > cfg80211_ap_settings, which wouldn't be a massive change.
> > 
> > 
> > I think maybe I even prefer the latter if I'm looking at it now, but I'm
> > not sure I'm not missing something from earlier discussions on this.
> 
> The second option will take couple of weeks due to current work load. 

Since you didn't seem to be opposed, I just did the second thing myself
now, it wasn't really hard. :)

johannes