public inbox for linux-wireless@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers
@ 2025-06-03 10:32 Gokul Sivakumar
  2025-06-03 10:32 ` [PATCH wireless-next v2 1/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to driver Gokul Sivakumar
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Gokul Sivakumar @ 2025-06-03 10:32 UTC (permalink / raw)
  To: Johannes Berg, Arend van Spriel, Ajay Singh, Claudiu Beznea
  Cc: linux-wireless, brcm80211, wlan-kernel-dev-list,
	gokulkumar.sivakumar

In cfg80211, added provision for the drivers which registers the callback 
.change_bss() cfg80211_ops, to find which set of AP BSS params are changed
by the userpsace in the SET_BSS operation. With this the drivers can decide
to reject the operation if any of the changed AP BSS param is not supported
for explicit configuration. This helps ensuring that the driver does not
partially handle the AP BSS params and avoid misleading the userspace that
the SET_BSS operation is fully successful.

Also introduced the "changed" bitmask check in all the drivers that are
currently registering to .change_bss() cfg80211_ops. This would make the
condition for handling SET_BSS operation equally stirct across all drivers.

CHANGE LOG:
v2:
 - Rephrased the subject line for this v2 patchset cover letter to include
   new changes introduced for addressing review comments. Kindly refer the
   below mentioned v1 section for the reference to the previously submitted
   patch Link.

 - cfg80211: in a new patch in v2, introduced the "changed" bitmask for
   AP BSS parameters as suggested in Johannes's review comment in v1 patch.
   Link: https://lore.kernel.org/linux-wireless/29fa5ea7f4cc177bed823ec3489d610e1d69a08f.camel@sipsolutions.net/

 - brcmfmac & wil6210: added "changed" bitmask check for the supported
   "ap_isolate" param and reject the entire SET_BSS operation if the
   userspace passes any other unsupported AP BSS parameter.

 - brcmfmac: added changes to get the current "ap_isolate" param value from
   firmware before attempting to set the new value from userspace. If the
   new value is already set, skip this operation.

 - wilc1000: in a new patch in v2, added "changed" bitmask check for the
   SET_BSS operation, and return -EOPNOTSUPP instead of 0, if the userspace
   attempts to change any AP BSS param. This helps to avoid misleading the
   userspace that the operation is fully successful.

 - mac80211: in a new patch in v2, added "changed" bitmask check for the
   supported AP BSS params, and return -EOPNOTSUPP if any unsupported
   param is set by the userspace.

v1:
 - brcmfmac: support AP isolation to restrict reachability between stations
   Link: https://lore.kernel.org/linux-wireless/20250423175125.7233-1-gokulkumar.sivakumar@infineon.com/


Gokul Sivakumar (4):
  wifi: cfg80211: Add support to indicate changed AP BSS parameters to
    driver
  wifi: wil6210: reject SET_BSS if any changed AP BSS param is not
    supported
  wifi: wilc1000: reject SET_BSS if any changed AP BSS param is not
    supported
  wifi: mac80211: reject SET_BSS if any changed AP BSS param is not
    supported

Wright Feng (1):
  wifi: brcmfmac: support AP isolation to restrict reachability between
    stations

 drivers/net/wireless/ath/wil6210/cfg80211.c   |  7 +++
 .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 47 +++++++++++++++++++
 .../wireless/microchip/wilc1000/cfg80211.c    |  6 +++
 include/net/cfg80211.h                        | 30 ++++++++++++
 net/mac80211/cfg.c                            | 14 ++++++
 net/wireless/nl80211.c                        | 28 +++++++++--
 6 files changed, 127 insertions(+), 5 deletions(-)

-- 
2.47.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH wireless-next v2 1/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to driver
  2025-06-03 10:32 [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Gokul Sivakumar
@ 2025-06-03 10:32 ` Gokul Sivakumar
  2025-06-03 10:32 ` [PATCH wireless-next v2 2/5] wifi: brcmfmac: support AP isolation to restrict reachability between stations Gokul Sivakumar
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Gokul Sivakumar @ 2025-06-03 10:32 UTC (permalink / raw)
  To: Johannes Berg, Arend van Spriel, Ajay Singh, Claudiu Beznea
  Cc: linux-wireless, brcm80211, wlan-kernel-dev-list,
	gokulkumar.sivakumar

With the userspace applications like hostapd, incase if the user explicitly
enabled an AP BSS param related config file option, like "ap_isolate", or
if the hostapd implicitly sets an AP BSS param like "cts_protection", then
those params are sent to the cfg80211 as part of SET_BSS operation request.

cfg80211 would then set the bits corresponding to the AP BSS parameters in
the newly introduced "changed" bitmap. Drivers which have registered for
the .change_bss() cfg80211_ops, on receiving this SET_BSS request, can now
understand which specific subset of params are changed by the userspace
using this bitmask. If the driver allows explicit configuration of changed
AP BSS params and if the values of those params are different from default
value in driver/firmware, then the driver can update it accordingly.

This helps ensuring that the driver does not partially handle the AP BSS
params and avoid misleading the userspace that the SET_BSS operation is
fully successful.

Signed-off-by: Gokul Sivakumar <gokulkumar.sivakumar@infineon.com>
---
 include/net/cfg80211.h | 30 ++++++++++++++++++++++++++++++
 net/wireless/nl80211.c | 28 +++++++++++++++++++++++-----
 2 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d1848dc8ec99..1f069ccb411e 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2326,12 +2326,41 @@ struct mpath_info {
 	int generation;
 };
 
+/**
+ * enum cfg80211_bss_params_changed - BSS parameters that are being changed
+ *
+ * This enum provides information of all BSS parameters that
+ * have to be updated as part of change_bss() call.
+ *
+ * @CFG80211_BSS_PARAM_CHANGED_CTS_PROT: Indicates that CTS Protection changed.
+ * @CFG80211_BSS_PARAM_CHANGED_SHORT_PREAMBLE: Indicates that preamble changed.
+ * @CFG80211_BSS_PARAM_CHANGED_SHORT_SLOT_TIME: Indicates that slot timing changed.
+ * @CFG80211_BSS_PARAM_CHANGED_BASIC_RATES: Indicatesthat Basic Rateset changed.
+ * @CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE: Indicates that AP Isolation setting changed.
+ * @CFG80211_BSS_PARAM_CHANGED_HT_OPMODE: Indicates that HT mode to be changed.
+ * @CFG80211_BSS_PARAM_CHANGED_P2P_CTWINDOW: Indicates that P2P CTWindow changed.
+ * @CFG80211_BSS_PARAM_CHANGED_P2P_OPPPS: Indicates that P2P Opportunistic
+ *	Power Save Mode changed.
+ */
+enum cfg80211_bss_params_changed {
+	CFG80211_BSS_PARAM_CHANGED_CTS_PROT		= BIT(0),
+	CFG80211_BSS_PARAM_CHANGED_SHORT_PREAMBLE	= BIT(1),
+	CFG80211_BSS_PARAM_CHANGED_SHORT_SLOT_TIME	= BIT(2),
+	CFG80211_BSS_PARAM_CHANGED_BASIC_RATES		= BIT(3),
+	CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE		= BIT(4),
+	CFG80211_BSS_PARAM_CHANGED_HT_OPMODE		= BIT(5),
+	CFG80211_BSS_PARAM_CHANGED_P2P_CTWINDOW		= BIT(6),
+	CFG80211_BSS_PARAM_CHANGED_P2P_OPPPS		= BIT(7),
+};
+
 /**
  * struct bss_parameters - BSS parameters
  *
  * Used to change BSS parameters (mainly for AP mode).
  *
  * @link_id: link_id or -1 for non-MLD
+ * @changed: bitmask of BSS parameters being changed by the user,
+ *	see &enum cfg80211_bss_params_changed.
  * @use_cts_prot: Whether to use CTS protection
  *	(0 = no, 1 = yes, -1 = do not change)
  * @use_short_preamble: Whether the use of short preambles is allowed
@@ -2350,6 +2379,7 @@ struct mpath_info {
  */
 struct bss_parameters {
 	int link_id;
+	u32 changed;
 	int use_cts_prot;
 	int use_short_preamble;
 	int use_short_slot_time;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index fd5f79266471..719517f58110 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -8243,26 +8243,42 @@ static int nl80211_set_bss(struct sk_buff *skb, struct genl_info *info)
 	params.p2p_ctwindow = -1;
 	params.p2p_opp_ps = -1;
 
-	if (info->attrs[NL80211_ATTR_BSS_CTS_PROT])
+	if (info->attrs[NL80211_ATTR_BSS_CTS_PROT]) {
 		params.use_cts_prot =
 		    nla_get_u8(info->attrs[NL80211_ATTR_BSS_CTS_PROT]);
-	if (info->attrs[NL80211_ATTR_BSS_SHORT_PREAMBLE])
+		params.changed |= CFG80211_BSS_PARAM_CHANGED_CTS_PROT;
+	}
+
+	if (info->attrs[NL80211_ATTR_BSS_SHORT_PREAMBLE]) {
 		params.use_short_preamble =
 		    nla_get_u8(info->attrs[NL80211_ATTR_BSS_SHORT_PREAMBLE]);
-	if (info->attrs[NL80211_ATTR_BSS_SHORT_SLOT_TIME])
+		params.changed |= CFG80211_BSS_PARAM_CHANGED_SHORT_PREAMBLE;
+	}
+
+	if (info->attrs[NL80211_ATTR_BSS_SHORT_SLOT_TIME]) {
 		params.use_short_slot_time =
 		    nla_get_u8(info->attrs[NL80211_ATTR_BSS_SHORT_SLOT_TIME]);
+		params.changed |= CFG80211_BSS_PARAM_CHANGED_SHORT_SLOT_TIME;
+	}
+
 	if (info->attrs[NL80211_ATTR_BSS_BASIC_RATES]) {
 		params.basic_rates =
 			nla_data(info->attrs[NL80211_ATTR_BSS_BASIC_RATES]);
 		params.basic_rates_len =
 			nla_len(info->attrs[NL80211_ATTR_BSS_BASIC_RATES]);
+		params.changed |= CFG80211_BSS_PARAM_CHANGED_BASIC_RATES;
 	}
-	if (info->attrs[NL80211_ATTR_AP_ISOLATE])
+
+	if (info->attrs[NL80211_ATTR_AP_ISOLATE]) {
 		params.ap_isolate = !!nla_get_u8(info->attrs[NL80211_ATTR_AP_ISOLATE]);
-	if (info->attrs[NL80211_ATTR_BSS_HT_OPMODE])
+		params.changed |= CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE;
+	}
+
+	if (info->attrs[NL80211_ATTR_BSS_HT_OPMODE]) {
 		params.ht_opmode =
 			nla_get_u16(info->attrs[NL80211_ATTR_BSS_HT_OPMODE]);
+		params.changed |= CFG80211_BSS_PARAM_CHANGED_HT_OPMODE;
+	}
 
 	if (info->attrs[NL80211_ATTR_P2P_CTWINDOW]) {
 		if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
@@ -8272,6 +8288,7 @@ static int nl80211_set_bss(struct sk_buff *skb, struct genl_info *info)
 		if (params.p2p_ctwindow != 0 &&
 		    !(rdev->wiphy.features & NL80211_FEATURE_P2P_GO_CTWIN))
 			return -EINVAL;
+		params.changed |= CFG80211_BSS_PARAM_CHANGED_P2P_CTWINDOW;
 	}
 
 	if (info->attrs[NL80211_ATTR_P2P_OPPPS]) {
@@ -8284,6 +8301,7 @@ static int nl80211_set_bss(struct sk_buff *skb, struct genl_info *info)
 		if (params.p2p_opp_ps &&
 		    !(rdev->wiphy.features & NL80211_FEATURE_P2P_GO_OPPPS))
 			return -EINVAL;
+		params.changed |= CFG80211_BSS_PARAM_CHANGED_P2P_OPPPS;
 	}
 
 	if (!rdev->ops->change_bss)
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH wireless-next v2 2/5] wifi: brcmfmac: support AP isolation to restrict reachability between stations
  2025-06-03 10:32 [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Gokul Sivakumar
  2025-06-03 10:32 ` [PATCH wireless-next v2 1/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to driver Gokul Sivakumar
@ 2025-06-03 10:32 ` Gokul Sivakumar
  2025-06-03 10:32 ` [PATCH wireless-next v2 3/5] wifi: wil6210: reject SET_BSS if any changed AP BSS param is not supported Gokul Sivakumar
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Gokul Sivakumar @ 2025-06-03 10:32 UTC (permalink / raw)
  To: Johannes Berg, Arend van Spriel, Ajay Singh, Claudiu Beznea
  Cc: linux-wireless, brcm80211, wlan-kernel-dev-list,
	gokulkumar.sivakumar

From: Wright Feng <wright.feng@cypress.com>

hostapd & wpa_supplicant userspace daemons exposes an AP mode specific
config file parameter "ap_isolate" to the user, which is used to control
low-level bridging of frames between the stations associated in the BSS.

In driver, handle this user setting in the newly defined cfg80211_ops
function brcmf_cfg80211_change_bss() by enabling "ap_isolate" IOVAR in
the firmware.

In AP mode, the "ap_isolate" value from the cfg80211 layer represents,
 0 = allow low-level bridging of frames between associated stations
 1 = restrict low-level bridging of frames to isolate associated stations
-1 = do not change existing setting

The userspace can change more than one AP BSS Parameter in the SET_BSS
operation. Incase if any BSS param other than the currently supported
"ap_isolate" is passed by userspace, reject the entire SET_BSS operation
instead of misleading the userspace that the operation is fully successful.

Signed-off-by: Wright Feng <wright.feng@cypress.com>
Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com>
Signed-off-by: Gokul Sivakumar <gokulkumar.sivakumar@infineon.com>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index dc2383faddd1..31067c640480 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5933,6 +5933,52 @@ static int brcmf_cfg80211_del_pmk(struct wiphy *wiphy, struct net_device *dev,
 	return brcmf_set_pmk(ifp, NULL, 0);
 }
 
+static int brcmf_set_ap_bssparam_isolate(struct brcmf_if *ifp, int param_ap_isolate)
+{
+	int ret = 0, cur_ap_isolate = 0;
+
+	/* In cfg80211, for AP mode, the "param_ap_isolate" value represents
+	 *  0 = allow low-level bridging of frames between associated stations
+	 *  1 = restrict low-level bridging of frames to isolate associated stations
+	 * -1 = do not change existing setting
+	 */
+	if (param_ap_isolate >= 0) {
+		ret = brcmf_fil_iovar_int_get(ifp, "ap_isolate", &cur_ap_isolate);
+		if (ret < 0)
+			return ret;
+
+		if (cur_ap_isolate != param_ap_isolate) {
+			ret = brcmf_fil_iovar_int_set(ifp, "ap_isolate", param_ap_isolate);
+			if (ret < 0) {
+				brcmf_err("ap_isolate iovar failed: ret=%d\n", ret);
+				return ret;
+			}
+		}
+	}
+
+	return ret;
+}
+
+static int brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev,
+				     struct bss_parameters *params)
+{
+	struct brcmf_if *ifp = netdev_priv(dev);
+	int ret = 0;
+
+	/* Reject the operation if any of the AP BSS params that got changed are not
+	 * supported by the driver for explicit configuration.
+	 */
+	if (params->changed &
+	    ~(CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE))
+		return -EOPNOTSUPP;
+
+	ret = brcmf_set_ap_bssparam_isolate(ifp, params->ap_isolate);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
 static struct cfg80211_ops brcmf_cfg80211_ops = {
 	.add_virtual_intf = brcmf_cfg80211_add_iface,
 	.del_virtual_intf = brcmf_cfg80211_del_iface,
@@ -5980,6 +6026,7 @@ static struct cfg80211_ops brcmf_cfg80211_ops = {
 	.update_connect_params = brcmf_cfg80211_update_conn_params,
 	.set_pmk = brcmf_cfg80211_set_pmk,
 	.del_pmk = brcmf_cfg80211_del_pmk,
+	.change_bss = brcmf_cfg80211_change_bss,
 };
 
 struct cfg80211_ops *brcmf_cfg80211_get_ops(struct brcmf_mp_device *settings)
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH wireless-next v2 3/5] wifi: wil6210: reject SET_BSS if any changed AP BSS param is not supported
  2025-06-03 10:32 [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Gokul Sivakumar
  2025-06-03 10:32 ` [PATCH wireless-next v2 1/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to driver Gokul Sivakumar
  2025-06-03 10:32 ` [PATCH wireless-next v2 2/5] wifi: brcmfmac: support AP isolation to restrict reachability between stations Gokul Sivakumar
@ 2025-06-03 10:32 ` Gokul Sivakumar
  2025-06-03 10:32 ` [PATCH wireless-next v2 4/5] wifi: wilc1000: " Gokul Sivakumar
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Gokul Sivakumar @ 2025-06-03 10:32 UTC (permalink / raw)
  To: Johannes Berg, Arend van Spriel, Ajay Singh, Claudiu Beznea
  Cc: linux-wireless, brcm80211, wlan-kernel-dev-list,
	gokulkumar.sivakumar

The userspace can change more than one AP BSS Parameter in the SET_BSS
operation. Incase if any BSS param other than the currently supported
"ap_isolate" is passed by userspace, reject the entire SET_BSS operation
instead of misleading the userspace that the operation is fully successful.

Signed-off-by: Gokul Sivakumar <gokulkumar.sivakumar@infineon.com>
---
 drivers/net/wireless/ath/wil6210/cfg80211.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index 5473c01cbe66..fbf7e21d91fe 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -2416,6 +2416,13 @@ static int wil_cfg80211_change_bss(struct wiphy *wiphy,
 	struct wil6210_priv *wil = wiphy_to_wil(wiphy);
 	struct wil6210_vif *vif = ndev_to_vif(dev);
 
+	/* Reject the operation if any of the AP BSS params that got changed are not
+	 * supported by the driver for explicit configuration.
+	 */
+	if (params->changed &
+	    ~(CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE))
+		return -EOPNOTSUPP;
+
 	if (params->ap_isolate >= 0) {
 		wil_dbg_misc(wil, "change_bss: ap_isolate MID %d, %d => %d\n",
 			     vif->mid, vif->ap_isolate, params->ap_isolate);
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH wireless-next v2 4/5] wifi: wilc1000: reject SET_BSS if any changed AP BSS param is not supported
  2025-06-03 10:32 [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Gokul Sivakumar
                   ` (2 preceding siblings ...)
  2025-06-03 10:32 ` [PATCH wireless-next v2 3/5] wifi: wil6210: reject SET_BSS if any changed AP BSS param is not supported Gokul Sivakumar
@ 2025-06-03 10:32 ` Gokul Sivakumar
  2025-06-03 10:32 ` [PATCH wireless-next v2 5/5] wifi: mac80211: " Gokul Sivakumar
  2025-06-03 10:56 ` [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Johannes Berg
  5 siblings, 0 replies; 11+ messages in thread
From: Gokul Sivakumar @ 2025-06-03 10:32 UTC (permalink / raw)
  To: Johannes Berg, Arend van Spriel, Ajay Singh, Claudiu Beznea
  Cc: linux-wireless, brcm80211, wlan-kernel-dev-list,
	gokulkumar.sivakumar

Since the driver currently does not support setting any of the AP BSS
parameters, reject the entire SET_BSS operation instead of misleading the
userspace that the operation is fully successful.

Signed-off-by: Gokul Sivakumar <gokulkumar.sivakumar@infineon.com>
---
 drivers/net/wireless/microchip/wilc1000/cfg80211.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
index e7aa0f991923..98776ef9195b 100644
--- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
+++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
@@ -797,6 +797,12 @@ static int get_station(struct wiphy *wiphy, struct net_device *dev,
 static int change_bss(struct wiphy *wiphy, struct net_device *dev,
 		      struct bss_parameters *params)
 {
+	/* Reject the operation if any of the AP BSS params that got changed are not
+	 * supported by the driver for explicit configuration.
+	 */
+	if (params->changed)
+		return -EOPNOTSUPP;
+
 	return 0;
 }
 
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH wireless-next v2 5/5] wifi: mac80211: reject SET_BSS if any changed AP BSS param is not supported
  2025-06-03 10:32 [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Gokul Sivakumar
                   ` (3 preceding siblings ...)
  2025-06-03 10:32 ` [PATCH wireless-next v2 4/5] wifi: wilc1000: " Gokul Sivakumar
@ 2025-06-03 10:32 ` Gokul Sivakumar
  2025-06-03 10:56 ` [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Johannes Berg
  5 siblings, 0 replies; 11+ messages in thread
From: Gokul Sivakumar @ 2025-06-03 10:32 UTC (permalink / raw)
  To: Johannes Berg, Arend van Spriel, Ajay Singh, Claudiu Beznea
  Cc: linux-wireless, brcm80211, wlan-kernel-dev-list,
	gokulkumar.sivakumar

The userspace can change more than one AP BSS Parameter in the SET_BSS
operation. Incase if any BSS param other than the list of parameters
currently supported for explicit configuration are passed by userspace,
reject the entire SET_BSS operation.

Signed-off-by: Gokul Sivakumar <gokulkumar.sivakumar@infineon.com>
---
 net/mac80211/cfg.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index d9d88f2f2831..c57d6f995fa7 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2751,6 +2751,20 @@ static int ieee80211_change_bss(struct wiphy *wiphy,
 	struct ieee80211_supported_band *sband;
 	u64 changed = 0;
 
+	/* Reject the operation if any of the AP BSS params that got changed are not
+	 * supported by the driver for explicit configuration.
+	 */
+	if (params->changed &
+	    ~(CFG80211_BSS_PARAM_CHANGED_CTS_PROT |
+	      CFG80211_BSS_PARAM_CHANGED_SHORT_PREAMBLE |
+	      CFG80211_BSS_PARAM_CHANGED_SHORT_SLOT_TIME |
+	      CFG80211_BSS_PARAM_CHANGED_BASIC_RATES |
+	      CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE |
+	      CFG80211_BSS_PARAM_CHANGED_HT_OPMODE |
+	      CFG80211_BSS_PARAM_CHANGED_P2P_CTWINDOW |
+	      CFG80211_BSS_PARAM_CHANGED_P2P_OPPPS))
+		return -EOPNOTSUPP;
+
 	link = ieee80211_link_or_deflink(sdata, params->link_id, true);
 	if (IS_ERR(link))
 		return PTR_ERR(link);
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers
  2025-06-03 10:32 [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Gokul Sivakumar
                   ` (4 preceding siblings ...)
  2025-06-03 10:32 ` [PATCH wireless-next v2 5/5] wifi: mac80211: " Gokul Sivakumar
@ 2025-06-03 10:56 ` Johannes Berg
  2025-06-03 12:12   ` Gokul Sivakumar
  5 siblings, 1 reply; 11+ messages in thread
From: Johannes Berg @ 2025-06-03 10:56 UTC (permalink / raw)
  To: Gokul Sivakumar, Arend van Spriel, Ajay Singh, Claudiu Beznea
  Cc: linux-wireless, brcm80211, wlan-kernel-dev-list

On Tue, 2025-06-03 at 16:02 +0530, Gokul Sivakumar wrote:
> In cfg80211, added provision for the drivers which registers the callback 
> .change_bss() cfg80211_ops, to find which set of AP BSS params are changed
> by the userpsace in the SET_BSS operation. With this the drivers can decide
> to reject the operation if any of the changed AP BSS param is not supported
> for explicit configuration. This helps ensuring that the driver does not
> partially handle the AP BSS params and avoid misleading the userspace that
> the SET_BSS operation is fully successful.
> 
> Also introduced the "changed" bitmask check in all the drivers that are
> currently registering to .change_bss() cfg80211_ops. This would make the
> condition for handling SET_BSS operation equally stirct across all drivers.
> 

Oh, nice! I'll have to look at it in more detail later, but one thing I
saw now is in patch 2 you have this:

> In AP mode, the "ap_isolate" value from the cfg80211 layer represents,
>  0 = allow low-level bridging of frames between associated stations
>  1 = restrict low-level bridging of frames to isolate associated stations
> -1 = do not change existing setting

Is that -1 still true? Seems like now it should just be that
CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE isn't set and then you don't touch
it?

johannes

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers
  2025-06-03 10:56 ` [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Johannes Berg
@ 2025-06-03 12:12   ` Gokul Sivakumar
  2025-06-03 12:29     ` Johannes Berg
  0 siblings, 1 reply; 11+ messages in thread
From: Gokul Sivakumar @ 2025-06-03 12:12 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Arend van Spriel, Ajay Singh, Claudiu Beznea, linux-wireless,
	brcm80211, wlan-kernel-dev-list, Gokul Sivakumar

On 06/03, Johannes Berg wrote:
> On Tue, 2025-06-03 at 16:02 +0530, Gokul Sivakumar wrote:
> > In cfg80211, added provision for the drivers which registers the callback
> > .change_bss() cfg80211_ops, to find which set of AP BSS params are changed
> > by the userpsace in the SET_BSS operation. With this the drivers can decide
> > to reject the operation if any of the changed AP BSS param is not supported
> > for explicit configuration. This helps ensuring that the driver does not
> > partially handle the AP BSS params and avoid misleading the userspace that
> > the SET_BSS operation is fully successful.
> >
> > Also introduced the "changed" bitmask check in all the drivers that are
> > currently registering to .change_bss() cfg80211_ops. This would make the
> > condition for handling SET_BSS operation equally stirct across all drivers.
> >
> 
> Oh, nice! I'll have to look at it in more detail later, but one thing I
> saw now is in patch 2 you have this:
> 
> > In AP mode, the "ap_isolate" value from the cfg80211 layer represents,
> >  0 = allow low-level bridging of frames between associated stations
> >  1 = restrict low-level bridging of frames to isolate associated stations
> > -1 = do not change existing setting
> 
> Is that -1 still true? Seems like now it should just be that
> CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE isn't set and then you don't touch
> it?
> 

Apologies for the HTML content in my earlier reply which got rejected by
the mailing list.

Kindly note that the driver is rejecting the SET_BSS operation if an
unsupported AP BSS param is passed by userspace, while the opposite is not
true. ie. the operation would not be rejected by the driver, when a
supported AP BSS param is not passed by the userspace.

So yes, the significance of "-1" still holds true, because if suppose the
userspace skipped this param in the SET_BSS request, the driver when
receiving the request will have the ap_isolate param with the default
value "-1". The driver is checking if the param value is >=0 before
proceeding with handling it. And will ignore the param, only if it is -1.

Gokul

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers
  2025-06-03 12:12   ` Gokul Sivakumar
@ 2025-06-03 12:29     ` Johannes Berg
  2025-06-03 13:17       ` Arend Van Spriel
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Berg @ 2025-06-03 12:29 UTC (permalink / raw)
  To: Gokul Sivakumar
  Cc: Arend van Spriel, Ajay Singh, Claudiu Beznea, linux-wireless,
	brcm80211, wlan-kernel-dev-list

On Tue, 2025-06-03 at 17:42 +0530, Gokul Sivakumar wrote:
> > 
> > > In AP mode, the "ap_isolate" value from the cfg80211 layer represents,
> > >  0 = allow low-level bridging of frames between associated stations
> > >  1 = restrict low-level bridging of frames to isolate associated stations
> > > -1 = do not change existing setting
> > 
> > Is that -1 still true? Seems like now it should just be that
> > CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE isn't set and then you don't touch
> > it?
> 

> Kindly note that the driver is rejecting the SET_BSS operation if an
> unsupported AP BSS param is passed by userspace, while the opposite is not
> true. ie. the operation would not be rejected by the driver, when a
> supported AP BSS param is not passed by the userspace.

Sure. That's a different question though.

> So yes, the significance of "-1" still holds true, because if suppose the
> userspace skipped this param in the SET_BSS request, the driver when
> receiving the request will have the ap_isolate param with the default
> value "-1". The driver is checking if the param value is >=0 before
> proceeding with handling it. And will ignore the param, only if it is -1.

Why though? It seems odd. Basically in this case cfg80211 is saying "I'm
not changing this parameter" but then anyway you check it's value, why?

While we're changing all the drivers, it would seem better to me to just
change them in a way that they don't look at the value if the
corresponding change flag isn't set?

johannes

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers
  2025-06-03 12:29     ` Johannes Berg
@ 2025-06-03 13:17       ` Arend Van Spriel
  2025-06-03 14:07         ` Gokul Sivakumar
  0 siblings, 1 reply; 11+ messages in thread
From: Arend Van Spriel @ 2025-06-03 13:17 UTC (permalink / raw)
  To: Johannes Berg, Gokul Sivakumar
  Cc: Ajay Singh, Claudiu Beznea, linux-wireless, brcm80211,
	wlan-kernel-dev-list

On June 3, 2025 2:29:54 PM Johannes Berg <johannes@sipsolutions.net> wrote:

> On Tue, 2025-06-03 at 17:42 +0530, Gokul Sivakumar wrote:
>>>
>>>> In AP mode, the "ap_isolate" value from the cfg80211 layer represents,
>>>> 0 = allow low-level bridging of frames between associated stations
>>>> 1 = restrict low-level bridging of frames to isolate associated stations
>>>> -1 = do not change existing setting
>>>
>>> Is that -1 still true? Seems like now it should just be that
>>> CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE isn't set and then you don't touch
>>> it?
>
>> Kindly note that the driver is rejecting the SET_BSS operation if an
>> unsupported AP BSS param is passed by userspace, while the opposite is not
>> true. ie. the operation would not be rejected by the driver, when a
>> supported AP BSS param is not passed by the userspace.
>
> Sure. That's a different question though.
>
>> So yes, the significance of "-1" still holds true, because if suppose the
>> userspace skipped this param in the SET_BSS request, the driver when
>> receiving the request will have the ap_isolate param with the default
>> value "-1". The driver is checking if the param value is >=0 before
>> proceeding with handling it. And will ignore the param, only if it is -1.
>
> Why though? It seems odd. Basically in this case cfg80211 is saying "I'm
> not changing this parameter" but then anyway you check it's value, why?
>
> While we're changing all the drivers, it would seem better to me to just
> change them in a way that they don't look at the value if the
> corresponding change flag isn't set?

I do have a couple of patches prepped for this, but did want to run few 
tests before sending it out. Now that this is submitted I am not sure what 
to do with that. I can send it as RFC so the idea is clear.

Regards,
Arend



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers
  2025-06-03 13:17       ` Arend Van Spriel
@ 2025-06-03 14:07         ` Gokul Sivakumar
  0 siblings, 0 replies; 11+ messages in thread
From: Gokul Sivakumar @ 2025-06-03 14:07 UTC (permalink / raw)
  To: Johannes Berg, Arend Van Spriel
  Cc: Ajay Singh, Claudiu Beznea, linux-wireless, brcm80211,
	wlan-kernel-dev-list

On 06/03, Arend Van Spriel wrote:
> On June 3, 2025 2:29:54 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> 
> > On Tue, 2025-06-03 at 17:42 +0530, Gokul Sivakumar wrote:
> > > > 
> > > > > In AP mode, the "ap_isolate" value from the cfg80211 layer represents,
> > > > > 0 = allow low-level bridging of frames between associated stations
> > > > > 1 = restrict low-level bridging of frames to isolate associated stations
> > > > > -1 = do not change existing setting
> > > > 
> > > > Is that -1 still true? Seems like now it should just be that
> > > > CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE isn't set and then you don't touch
> > > > it?
> > 
> > > Kindly note that the driver is rejecting the SET_BSS operation if an
> > > unsupported AP BSS param is passed by userspace, while the opposite is not
> > > true. ie. the operation would not be rejected by the driver, when a
> > > supported AP BSS param is not passed by the userspace.
> > 
> > Sure. That's a different question though.
> > 
> > > So yes, the significance of "-1" still holds true, because if suppose the
> > > userspace skipped this param in the SET_BSS request, the driver when
> > > receiving the request will have the ap_isolate param with the default
> > > value "-1". The driver is checking if the param value is >=0 before
> > > proceeding with handling it. And will ignore the param, only if it is -1.
> > 
> > Why though? It seems odd. Basically in this case cfg80211 is saying "I'm
> > not changing this parameter" but then anyway you check it's value, why?

yes, if the driver individually checks for the CFG80211_BSS_PARAM_CHANGED_*
flag before handling the corresponding AP BSS param, then the check for the
param value as -1 is not necessary.

> > While we're changing all the drivers, it would seem better to me to just
> > change them in a way that they don't look at the value if the
> > corresponding change flag isn't set?

your suggested approach is fine, will update this and submit a v3 patchset.

> I do have a couple of patches prepped for this, but did want to run few
> tests before sending it out. Now that this is submitted I am not sure what
> to do with that. I can send it as RFC so the idea is clear.

Ok. I have submitted this patchset based on the suggestions in our v1 patch
review discussion, to address johannes's comment using the newly introduced
CHANGED_* flags which are helpful to the drivers.

Gokul

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-06-03 14:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-03 10:32 [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Gokul Sivakumar
2025-06-03 10:32 ` [PATCH wireless-next v2 1/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to driver Gokul Sivakumar
2025-06-03 10:32 ` [PATCH wireless-next v2 2/5] wifi: brcmfmac: support AP isolation to restrict reachability between stations Gokul Sivakumar
2025-06-03 10:32 ` [PATCH wireless-next v2 3/5] wifi: wil6210: reject SET_BSS if any changed AP BSS param is not supported Gokul Sivakumar
2025-06-03 10:32 ` [PATCH wireless-next v2 4/5] wifi: wilc1000: " Gokul Sivakumar
2025-06-03 10:32 ` [PATCH wireless-next v2 5/5] wifi: mac80211: " Gokul Sivakumar
2025-06-03 10:56 ` [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Johannes Berg
2025-06-03 12:12   ` Gokul Sivakumar
2025-06-03 12:29     ` Johannes Berg
2025-06-03 13:17       ` Arend Van Spriel
2025-06-03 14:07         ` Gokul Sivakumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox