diff mbox series

[v3] wifi: mwifiex: avoid AP and STA running on different channel

Message ID 20241008050405.6948-1-yu-hao.lin@nxp.com (mailing list archive)
State Deferred
Delegated to: Kalle Valo
Headers show
Series [v3] wifi: mwifiex: avoid AP and STA running on different channel | expand

Commit Message

David Lin Oct. 8, 2024, 5:04 a.m. UTC 
Current firmware doesn't support AP and STA running on different
channels simultaneously if DRCS is not enabled.
FW crash would occur in such case.
This patch avoids the issue by disabling AP and STA to run on
different channels if DRCS is not running.

Signed-off-by: David Lin <yu-hao.lin@nxp.com>
---

v3:
   - add the check for DRCS mode.
   - add clean comment for wiphy parameters setting.

v2:
   - clean up code.

---
 .../net/wireless/marvell/mwifiex/cfg80211.c   | 32 +++++++++----
 drivers/net/wireless/marvell/mwifiex/util.c   | 47 +++++++++++++++++++
 drivers/net/wireless/marvell/mwifiex/util.h   | 13 +++++
 3 files changed, 82 insertions(+), 10 deletions(-)


base-commit: 5a4d42c1688c88f3be6aef46b0ea6c32694cd2b8

Comments

Sascha Hauer Oct. 15, 2024, 10:20 a.m. UTC | #1 
On Tue, Oct 08, 2024 at 01:04:05PM +0800, David Lin wrote:
> Current firmware doesn't support AP and STA running on different
> channels simultaneously if DRCS is not enabled.
> FW crash would occur in such case.
> This patch avoids the issue by disabling AP and STA to run on
> different channels if DRCS is not running.
> 
> Signed-off-by: David Lin <yu-hao.lin@nxp.com>

Acked-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

> ---
> 
> v3:
>    - add the check for DRCS mode.
>    - add clean comment for wiphy parameters setting.
> 
> v2:
>    - clean up code.
> 
> ---
>  .../net/wireless/marvell/mwifiex/cfg80211.c   | 32 +++++++++----
>  drivers/net/wireless/marvell/mwifiex/util.c   | 47 +++++++++++++++++++
>  drivers/net/wireless/marvell/mwifiex/util.h   | 13 +++++
>  3 files changed, 82 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> index fca3eea7ee84..ebc891d5d6c6 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> @@ -746,13 +746,18 @@ mwifiex_cfg80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
>  
>  	priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
>  
> +	/* Because wiphy parameters are global setting, the setting for
> +	 * the first interface will apply for other interfaces too.
> +	 * If AP and STA are running at same time, these parameters must
> +	 * be the same. If the first interface is running, it means wiphy
> +	 * parameters are already set. The second setting should be dropped
> +	 * without error return, otherwise AP and STA can't run at the same
> +	 * time if wiphy parameters are setting.
> +	 */
>  	switch (priv->bss_role) {
>  	case MWIFIEX_BSS_ROLE_UAP:
> -		if (priv->bss_started) {
> -			mwifiex_dbg(adapter, ERROR,
> -				    "cannot change wiphy params when bss started");
> -			return -EINVAL;
> -		}
> +		if (priv->bss_started)
> +			break;
>  
>  		bss_cfg = kzalloc(sizeof(*bss_cfg), GFP_KERNEL);
>  		if (!bss_cfg)
> @@ -781,11 +786,9 @@ mwifiex_cfg80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
>  		break;
>  
>  	case MWIFIEX_BSS_ROLE_STA:
> -		if (priv->media_connected) {
> -			mwifiex_dbg(adapter, ERROR,
> -				    "cannot change wiphy params when connected");
> -			return -EINVAL;
> -		}
> +		if (priv->media_connected)
> +			break;
> +
>  		if (changed & WIPHY_PARAM_RTS_THRESHOLD) {
>  			ret = mwifiex_set_rts(priv,
>  					      wiphy->rts_threshold);
> @@ -2069,6 +2072,9 @@ static int mwifiex_cfg80211_start_ap(struct wiphy *wiphy,
>  	if (GET_BSS_ROLE(priv) != MWIFIEX_BSS_ROLE_UAP)
>  		return -1;
>  
> +	if (!mwifiex_is_channel_setting_allowable(priv, params->chandef.chan))
> +		return -EOPNOTSUPP;
> +
>  	bss_cfg = kzalloc(sizeof(struct mwifiex_uap_bss_param), GFP_KERNEL);
>  	if (!bss_cfg)
>  		return -ENOMEM;
> @@ -2463,6 +2469,9 @@ mwifiex_cfg80211_connect(struct wiphy *wiphy, struct net_device *dev,
>  		return -EFAULT;
>  	}
>  
> +	if (!mwifiex_is_channel_setting_allowable(priv, sme->channel))
> +		return -EOPNOTSUPP;
> +
>  	mwifiex_dbg(adapter, INFO,
>  		    "info: Trying to associate to bssid %pM\n", sme->bssid);
>  
> @@ -4298,6 +4307,9 @@ mwifiex_cfg80211_authenticate(struct wiphy *wiphy,
>  		return -EINVAL;
>  	}
>  
> +	if (!mwifiex_is_channel_setting_allowable(priv, req->bss->channel))
> +		return -EOPNOTSUPP;
> +
>  	if (priv->auth_alg != WLAN_AUTH_SAE &&
>  	    (priv->auth_flag & HOST_MLME_AUTH_PENDING)) {
>  		mwifiex_dbg(priv->adapter, ERROR, "Pending auth on going\n");
> diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
> index 42c04bf858da..da5eef7b1dec 100644
> --- a/drivers/net/wireless/marvell/mwifiex/util.c
> +++ b/drivers/net/wireless/marvell/mwifiex/util.c
> @@ -323,6 +323,53 @@ int mwifiex_debug_info_to_buffer(struct mwifiex_private *priv, char *buf,
>  	return p - buf;
>  }
>  
> +bool mwifiex_is_channel_setting_allowable(struct mwifiex_private *priv,
> +					  struct ieee80211_channel *check_chan)
> +{
> +	struct mwifiex_adapter *adapter = priv->adapter;
> +	int i;
> +	struct mwifiex_private *tmp_priv;
> +	u8 bss_role = GET_BSS_ROLE(priv);
> +	struct ieee80211_channel *set_chan;
> +
> +	if (adapter->drcs_enabled)
> +		return true;
> +
> +	for (i = 0; i < MWIFIEX_MAX_BSS_NUM; i++) {
> +		tmp_priv = adapter->priv[i];
> +		if (tmp_priv == priv)
> +			continue;
> +
> +		set_chan = NULL;
> +		if (bss_role == MWIFIEX_BSS_ROLE_STA) {
> +			if (GET_BSS_ROLE(tmp_priv) == MWIFIEX_BSS_ROLE_UAP &&
> +			    netif_carrier_ok(tmp_priv->netdev) &&
> +			    cfg80211_chandef_valid(&tmp_priv->bss_chandef))
> +				set_chan = tmp_priv->bss_chandef.chan;
> +		} else if (bss_role == MWIFIEX_BSS_ROLE_UAP) {
> +			struct mwifiex_current_bss_params *bss_params =
> +				&tmp_priv->curr_bss_params;
> +			int channel = bss_params->bss_descriptor.channel;
> +			enum nl80211_band band =
> +				mwifiex_band_to_radio_type(bss_params->band);
> +			int freq =
> +				ieee80211_channel_to_frequency(channel, band);
> +
> +			if (GET_BSS_ROLE(tmp_priv) == MWIFIEX_BSS_ROLE_STA &&
> +			    tmp_priv->media_connected)
> +				set_chan = ieee80211_get_channel(adapter->wiphy, freq);
> +		}
> +
> +		if (set_chan && !ieee80211_channel_equal(check_chan, set_chan)) {
> +			mwifiex_dbg(adapter, ERROR,
> +				    "AP/STA must run on the same channel\n");
> +			return false;
> +		}
> +	}
> +
> +	return true;
> +}
> +
>  static int
>  mwifiex_parse_mgmt_packet(struct mwifiex_private *priv, u8 *payload, u16 len,
>  			  struct rxpd *rx_pd)
> diff --git a/drivers/net/wireless/marvell/mwifiex/util.h b/drivers/net/wireless/marvell/mwifiex/util.h
> index 4699c505c0a0..16f092bb0823 100644
> --- a/drivers/net/wireless/marvell/mwifiex/util.h
> +++ b/drivers/net/wireless/marvell/mwifiex/util.h
> @@ -86,4 +86,17 @@ static inline void le16_unaligned_add_cpu(__le16 *var, u16 val)
>  	put_unaligned_le16(get_unaligned_le16(var) + val, var);
>  }
>  
> +/* Current firmware doesn't support AP and STA running on different
> + * channels simultaneously in normal mode.
> + * FW crash would occur in such case.
> + * This function is used to check if check_chan can be set to FW or not.
> + *
> + * Return:
> + * %true if check_chan can be set to FW without issues.
> + * %false there is already other channel is set to FW, setting of
> + * check_chan is not allowable.
> + */
> +bool mwifiex_is_channel_setting_allowable(struct mwifiex_private *priv,
> +					  struct ieee80211_channel *check_chan);
> +
>  #endif /* !_MWIFIEX_UTIL_H_ */
> 
> base-commit: 5a4d42c1688c88f3be6aef46b0ea6c32694cd2b8
> -- 
> 2.34.1
> 
>
diff mbox series

Patch

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index fca3eea7ee84..ebc891d5d6c6 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -746,13 +746,18 @@  mwifiex_cfg80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
 
 	priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
 
+	/* Because wiphy parameters are global setting, the setting for
+	 * the first interface will apply for other interfaces too.
+	 * If AP and STA are running at same time, these parameters must
+	 * be the same. If the first interface is running, it means wiphy
+	 * parameters are already set. The second setting should be dropped
+	 * without error return, otherwise AP and STA can't run at the same
+	 * time if wiphy parameters are setting.
+	 */
 	switch (priv->bss_role) {
 	case MWIFIEX_BSS_ROLE_UAP:
-		if (priv->bss_started) {
-			mwifiex_dbg(adapter, ERROR,
-				    "cannot change wiphy params when bss started");
-			return -EINVAL;
-		}
+		if (priv->bss_started)
+			break;
 
 		bss_cfg = kzalloc(sizeof(*bss_cfg), GFP_KERNEL);
 		if (!bss_cfg)
@@ -781,11 +786,9 @@  mwifiex_cfg80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
 		break;
 
 	case MWIFIEX_BSS_ROLE_STA:
-		if (priv->media_connected) {
-			mwifiex_dbg(adapter, ERROR,
-				    "cannot change wiphy params when connected");
-			return -EINVAL;
-		}
+		if (priv->media_connected)
+			break;
+
 		if (changed & WIPHY_PARAM_RTS_THRESHOLD) {
 			ret = mwifiex_set_rts(priv,
 					      wiphy->rts_threshold);
@@ -2069,6 +2072,9 @@  static int mwifiex_cfg80211_start_ap(struct wiphy *wiphy,
 	if (GET_BSS_ROLE(priv) != MWIFIEX_BSS_ROLE_UAP)
 		return -1;
 
+	if (!mwifiex_is_channel_setting_allowable(priv, params->chandef.chan))
+		return -EOPNOTSUPP;
+
 	bss_cfg = kzalloc(sizeof(struct mwifiex_uap_bss_param), GFP_KERNEL);
 	if (!bss_cfg)
 		return -ENOMEM;
@@ -2463,6 +2469,9 @@  mwifiex_cfg80211_connect(struct wiphy *wiphy, struct net_device *dev,
 		return -EFAULT;
 	}
 
+	if (!mwifiex_is_channel_setting_allowable(priv, sme->channel))
+		return -EOPNOTSUPP;
+
 	mwifiex_dbg(adapter, INFO,
 		    "info: Trying to associate to bssid %pM\n", sme->bssid);
 
@@ -4298,6 +4307,9 @@  mwifiex_cfg80211_authenticate(struct wiphy *wiphy,
 		return -EINVAL;
 	}
 
+	if (!mwifiex_is_channel_setting_allowable(priv, req->bss->channel))
+		return -EOPNOTSUPP;
+
 	if (priv->auth_alg != WLAN_AUTH_SAE &&
 	    (priv->auth_flag & HOST_MLME_AUTH_PENDING)) {
 		mwifiex_dbg(priv->adapter, ERROR, "Pending auth on going\n");
diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
index 42c04bf858da..da5eef7b1dec 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.c
+++ b/drivers/net/wireless/marvell/mwifiex/util.c
@@ -323,6 +323,53 @@  int mwifiex_debug_info_to_buffer(struct mwifiex_private *priv, char *buf,
 	return p - buf;
 }
 
+bool mwifiex_is_channel_setting_allowable(struct mwifiex_private *priv,
+					  struct ieee80211_channel *check_chan)
+{
+	struct mwifiex_adapter *adapter = priv->adapter;
+	int i;
+	struct mwifiex_private *tmp_priv;
+	u8 bss_role = GET_BSS_ROLE(priv);
+	struct ieee80211_channel *set_chan;
+
+	if (adapter->drcs_enabled)
+		return true;
+
+	for (i = 0; i < MWIFIEX_MAX_BSS_NUM; i++) {
+		tmp_priv = adapter->priv[i];
+		if (tmp_priv == priv)
+			continue;
+
+		set_chan = NULL;
+		if (bss_role == MWIFIEX_BSS_ROLE_STA) {
+			if (GET_BSS_ROLE(tmp_priv) == MWIFIEX_BSS_ROLE_UAP &&
+			    netif_carrier_ok(tmp_priv->netdev) &&
+			    cfg80211_chandef_valid(&tmp_priv->bss_chandef))
+				set_chan = tmp_priv->bss_chandef.chan;
+		} else if (bss_role == MWIFIEX_BSS_ROLE_UAP) {
+			struct mwifiex_current_bss_params *bss_params =
+				&tmp_priv->curr_bss_params;
+			int channel = bss_params->bss_descriptor.channel;
+			enum nl80211_band band =
+				mwifiex_band_to_radio_type(bss_params->band);
+			int freq =
+				ieee80211_channel_to_frequency(channel, band);
+
+			if (GET_BSS_ROLE(tmp_priv) == MWIFIEX_BSS_ROLE_STA &&
+			    tmp_priv->media_connected)
+				set_chan = ieee80211_get_channel(adapter->wiphy, freq);
+		}
+
+		if (set_chan && !ieee80211_channel_equal(check_chan, set_chan)) {
+			mwifiex_dbg(adapter, ERROR,
+				    "AP/STA must run on the same channel\n");
+			return false;
+		}
+	}
+
+	return true;
+}
+
 static int
 mwifiex_parse_mgmt_packet(struct mwifiex_private *priv, u8 *payload, u16 len,
 			  struct rxpd *rx_pd)
diff --git a/drivers/net/wireless/marvell/mwifiex/util.h b/drivers/net/wireless/marvell/mwifiex/util.h
index 4699c505c0a0..16f092bb0823 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.h
+++ b/drivers/net/wireless/marvell/mwifiex/util.h
@@ -86,4 +86,17 @@  static inline void le16_unaligned_add_cpu(__le16 *var, u16 val)
 	put_unaligned_le16(get_unaligned_le16(var) + val, var);
 }
 
+/* Current firmware doesn't support AP and STA running on different
+ * channels simultaneously in normal mode.
+ * FW crash would occur in such case.
+ * This function is used to check if check_chan can be set to FW or not.
+ *
+ * Return:
+ * %true if check_chan can be set to FW without issues.
+ * %false there is already other channel is set to FW, setting of
+ * check_chan is not allowable.
+ */
+bool mwifiex_is_channel_setting_allowable(struct mwifiex_private *priv,
+					  struct ieee80211_channel *check_chan);
+
 #endif /* !_MWIFIEX_UTIL_H_ */