wifi: mwifiex: avoid AP and STA running on different channel

Commit Message

David Lin Aug. 30, 2024, 3:06 a.m. UTC

Current firmware doesn't support AP and STA running on different
channels simultaneously.
FW crash would occur in such case.
This patch avoids the issue by disabling AP and STA to run on
different channels.

Signed-off-by: David Lin <[e-mail]>
---
 .../net/wireless/marvell/mwifiex/cfg80211.c   | 71 +++++++++++++++++--
 1 file changed, 66 insertions(+), 5 deletions(-)


base-commit: ae98f5c9fd8ba84cd408b41faa77e65bf1b4cdfa

Comments

Francesco Dolcini Aug. 30, 2024, 8:10 a.m. UTC | #1

+Sascha, that just sent a patch to handle the same issue.

On Fri, Aug 30, 2024 at 11:06:30AM +0800, David Lin wrote:
> Current firmware doesn't support AP and STA running on different
> channels simultaneously.
> FW crash would occur in such case.
> This patch avoids the issue by disabling AP and STA to run on
> different channels.

Is this a generic issue of specific of some firmware version? Asking since the
driver as you know is supporting multiple Wi-Fi device.

Francesco

David Lin Aug. 30, 2024, 8:19 a.m. UTC | #2

> From: Francesco Dolcini <[e-mail]>
> Sent: Friday, August 30, 2024 4:11 PM
> To: David Lin <[e-mail]>; Sascha Hauer <[e-mail]>
> Cc: [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail]; Pete
> Hsieh <[e-mail]>
> Subject: [EXT] Re: [PATCH] wifi: mwifiex: avoid AP and STA running on different
> channel
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> +Sascha, that just sent a patch to handle the same issue.
> 
> On Fri, Aug 30, 2024 at 11:06:30AM +0800, David Lin wrote:
> > Current firmware doesn't support AP and STA running on different
> > channels simultaneously.
> > FW crash would occur in such case.
> > This patch avoids the issue by disabling AP and STA to run on
> > different channels.
> 
> Is this a generic issue of specific of some firmware version? Asking since the
> driver as you know is supporting multiple Wi-Fi device.
> 
> Francesco

Yes. Firmware only allows AP and STA run on the same channel.
Because Mwifiex does not support other ways to make sure this condition.
The code to check the channel setting of AP and STA should be used to avoid
this situation.

David

Sascha Hauer Aug. 30, 2024, 9:18 a.m. UTC | #3

On Fri, Aug 30, 2024 at 10:10:30AM +0200, Francesco Dolcini wrote:
> +Sascha, that just sent a patch to handle the same issue.
> 
> On Fri, Aug 30, 2024 at 11:06:30AM +0800, David Lin wrote:
> > Current firmware doesn't support AP and STA running on different
> > channels simultaneously.
> > FW crash would occur in such case.
> > This patch avoids the issue by disabling AP and STA to run on
> > different channels.
> 
> Is this a generic issue of specific of some firmware version? Asking since the
> driver as you know is supporting multiple Wi-Fi device.

The driver does, unfortunately the hardware does not.

I learned this the hard way. When an accesspoint is running on uap0 and
you want to connect to an accesspoint on mlan0 then it won't work when
that accesspoint is on a different channel.

Likewise, when you are creating an accesspoint on uap0 using channel A
and another one on channel B then you'll notice that both accesspoints
will end up using channel A.

It took me a while to find that out. In the end I found the same channel
check in the nxpwifi driver.

Sascha

David Lin Aug. 30, 2024, 10:58 a.m. UTC | #4

> From: Sascha Hauer <[e-mail]>
> Sent: Friday, August 30, 2024 5:19 PM
> To: Francesco Dolcini <[e-mail]>
> Cc: David Lin <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> Pete Hsieh <[e-mail]>
> Subject: [EXT] Re: [PATCH] wifi: mwifiex: avoid AP and STA running on
> different channel
> 
> On Fri, Aug 30, 2024 at 10:10:30AM +0200, Francesco Dolcini wrote:
> > +Sascha, that just sent a patch to handle the same issue.
> >
> > On Fri, Aug 30, 2024 at 11:06:30AM +0800, David Lin wrote:
> > > Current firmware doesn't support AP and STA running on different
> > > channels simultaneously.
> > > FW crash would occur in such case.
> > > This patch avoids the issue by disabling AP and STA to run on
> > > different channels.
> >
> > Is this a generic issue of specific of some firmware version? Asking
> > since the driver as you know is supporting multiple Wi-Fi device.
> 
> The driver does, unfortunately the hardware does not.
> 
> I learned this the hard way. When an accesspoint is running on uap0 and
> you want to connect to an accesspoint on mlan0 then it won't work when
> that accesspoint is on a different channel.
> 
> Likewise, when you are creating an accesspoint on uap0 using channel A and
> another one on channel B then you'll notice that both accesspoints will end
> up using channel A.
> 
> It took me a while to find that out. In the end I found the same channel
> check in the nxpwifi driver.
> 
> Sascha
> 

Yes, this patch is backported from nxpwifi (same as another patch for AP DFS mode).
Because Mwifiex supports connect/disconnect and separate auth/assoc, I think I will
clean up the code and create patch v2 later.

David

Sascha Hauer Aug. 30, 2024, 11:13 a.m. UTC | #5

On Fri, Aug 30, 2024 at 10:58:50AM +0000, David Lin wrote:
> > From: Sascha Hauer <[e-mail]>
> > Sent: Friday, August 30, 2024 5:19 PM
> > To: Francesco Dolcini <[e-mail]>
> > Cc: David Lin <[e-mail]>; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > Pete Hsieh <[e-mail]>
> > Subject: [EXT] Re: [PATCH] wifi: mwifiex: avoid AP and STA running on
> > different channel
> > 
> > On Fri, Aug 30, 2024 at 10:10:30AM +0200, Francesco Dolcini wrote:
> > > +Sascha, that just sent a patch to handle the same issue.
> > >
> > > On Fri, Aug 30, 2024 at 11:06:30AM +0800, David Lin wrote:
> > > > Current firmware doesn't support AP and STA running on different
> > > > channels simultaneously.
> > > > FW crash would occur in such case.
> > > > This patch avoids the issue by disabling AP and STA to run on
> > > > different channels.
> > >
> > > Is this a generic issue of specific of some firmware version? Asking
> > > since the driver as you know is supporting multiple Wi-Fi device.
> > 
> > The driver does, unfortunately the hardware does not.
> > 
> > I learned this the hard way. When an accesspoint is running on uap0 and
> > you want to connect to an accesspoint on mlan0 then it won't work when
> > that accesspoint is on a different channel.
> > 
> > Likewise, when you are creating an accesspoint on uap0 using channel A and
> > another one on channel B then you'll notice that both accesspoints will end
> > up using channel A.
> > 
> > It took me a while to find that out. In the end I found the same channel
> > check in the nxpwifi driver.
> > 
> > Sascha
> > 
> 
> Yes, this patch is backported from nxpwifi (same as another patch for AP DFS mode).
> Because Mwifiex supports connect/disconnect and separate auth/assoc, I think I will
> clean up the code and create patch v2 later.

Gna, I didn't notice that I haven't answered to my own patch, but to
your patch addressing the same problem a few hours later ;)

Sascha

David Lin Aug. 30, 2024, 11:25 a.m. UTC | #6

> From: Sascha Hauer <[e-mail]>
> Sent: Friday, August 30, 2024 7:14 PM
> To: David Lin <[e-mail]>
> Cc: Francesco Dolcini <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> Pete Hsieh <[e-mail]>
> Subject: Re: [EXT] Re: [PATCH] wifi: mwifiex: avoid AP and STA running on
> different channel
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On Fri, Aug 30, 2024 at 10:58:50AM +0000, David Lin wrote:
> > > From: Sascha Hauer <[e-mail]>
> > > Sent: Friday, August 30, 2024 5:19 PM
> > > To: Francesco Dolcini <[e-mail]>
> > > Cc: David Lin <[e-mail]>; [e-mail];
> > > [e-mail]; [e-mail];
> > > [e-mail]; Pete Hsieh <[e-mail]>
> > > Subject: [EXT] Re: [PATCH] wifi: mwifiex: avoid AP and STA running
> > > on different channel
> > >
> > > On Fri, Aug 30, 2024 at 10:10:30AM +0200, Francesco Dolcini wrote:
> > > > +Sascha, that just sent a patch to handle the same issue.
> > > >
> > > > On Fri, Aug 30, 2024 at 11:06:30AM +0800, David Lin wrote:
> > > > > Current firmware doesn't support AP and STA running on different
> > > > > channels simultaneously.
> > > > > FW crash would occur in such case.
> > > > > This patch avoids the issue by disabling AP and STA to run on
> > > > > different channels.
> > > >
> > > > Is this a generic issue of specific of some firmware version?
> > > > Asking since the driver as you know is supporting multiple Wi-Fi device.
> > >
> > > The driver does, unfortunately the hardware does not.
> > >
> > > I learned this the hard way. When an accesspoint is running on uap0
> > > and you want to connect to an accesspoint on mlan0 then it won't
> > > work when that accesspoint is on a different channel.
> > >
> > > Likewise, when you are creating an accesspoint on uap0 using channel
> > > A and another one on channel B then you'll notice that both
> > > accesspoints will end up using channel A.
> > >
> > > It took me a while to find that out. In the end I found the same
> > > channel check in the nxpwifi driver.
> > >
> > > Sascha
> > >
> >
> > Yes, this patch is backported from nxpwifi (same as another patch for AP
> DFS mode).
> > Because Mwifiex supports connect/disconnect and separate auth/assoc, I
> > think I will clean up the code and create patch v2 later.
> 
> Gna, I didn't notice that I haven't answered to my own patch, but to your
> patch addressing the same problem a few hours later ;)
> 
> Sascha
> 

After Francesco mentioned, then I know you have the patch to fix the same issue a few hours later after this patch.
I think we will commit to backport needed patches from nxpwifi. BTW, are you sure your patch can work to run AP and STA on
the same channel if some wiphy parameters are setting?

David

Patch

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 722ead51e912..53fce5eccc69 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -781,11 +781,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);
@@ -2065,10 +2063,37 @@  static int mwifiex_cfg80211_start_ap(struct wiphy *wiphy,
 {
 	struct mwifiex_uap_bss_param *bss_cfg;
 	struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
+	struct mwifiex_adapter *adapter = priv->adapter;
+	struct mwifiex_private *tmp_priv;
+	int i;
+	struct mwifiex_current_bss_params *bss_params;
+	enum nl80211_band band;
+	int freq;
 
 	if (GET_BSS_ROLE(priv) != MWIFIEX_BSS_ROLE_UAP)
 		return -1;
 
+	for (i = 0; i < MWIFIEX_MAX_BSS_NUM; i++) {
+		tmp_priv = adapter->priv[i];
+		if (tmp_priv == priv)
+			continue;
+		if (GET_BSS_ROLE(tmp_priv) == MWIFIEX_BSS_ROLE_STA &&
+		    tmp_priv->media_connected) {
+			bss_params = &tmp_priv->curr_bss_params;
+			band = mwifiex_band_to_radio_type(bss_params->band);
+			freq = ieee80211_channel_to_frequency
+			       (bss_params->bss_descriptor.channel, band);
+			if (!ieee80211_channel_equal
+			     (params->chandef.chan,
+			      ieee80211_get_channel(wiphy, freq))) {
+				mwifiex_dbg
+				(priv->adapter, MSG,
+				 "AP and STA must operate on same channel\n");
+				return -EOPNOTSUPP;
+			}
+		}
+	}
+
 	bss_cfg = kzalloc(sizeof(struct mwifiex_uap_bss_param), GFP_KERNEL);
 	if (!bss_cfg)
 		return -ENOMEM;
@@ -2437,6 +2462,8 @@  mwifiex_cfg80211_connect(struct wiphy *wiphy, struct net_device *dev,
 	struct mwifiex_adapter *adapter = priv->adapter;
 	struct cfg80211_bss *bss = NULL;
 	int ret;
+	struct mwifiex_private *tmp_priv;
+	int i;
 
 	if (GET_BSS_ROLE(priv) != MWIFIEX_BSS_ROLE_STA) {
 		mwifiex_dbg(adapter, ERROR,
@@ -2451,6 +2478,22 @@  mwifiex_cfg80211_connect(struct wiphy *wiphy, struct net_device *dev,
 		return -EALREADY;
 	}
 
+	for (i = 0; i < MWIFIEX_MAX_BSS_NUM; i++) {
+		tmp_priv = adapter->priv[i];
+		if (tmp_priv == priv)
+			continue;
+		if (GET_BSS_ROLE(tmp_priv) == MWIFIEX_BSS_ROLE_UAP &&
+		    netif_carrier_ok(tmp_priv->netdev) &&
+		    cfg80211_chandef_valid(&tmp_priv->bss_chandef)) {
+			if (!ieee80211_channel_equal
+			     (sme->channel, tmp_priv->bss_chandef.chan)) {
+				mwifiex_dbg(adapter, ERROR,
+					    "STA/AP must on the same channel\n");
+				return -EOPNOTSUPP;
+			}
+		}
+	}
+
 	if (priv->scan_block)
 		priv->scan_block = false;
 
@@ -4285,6 +4328,8 @@  mwifiex_cfg80211_authenticate(struct wiphy *wiphy,
 	u32 tx_control = 0, pkt_type = PKT_TYPE_MGMT;
 	u8 trans = 1, status_code = 0;
 	u8 *varptr;
+	struct mwifiex_private *tmp_priv;
+	int i;
 
 	if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP) {
 		mwifiex_dbg(priv->adapter, ERROR, "Interface role is AP\n");
@@ -4298,6 +4343,22 @@  mwifiex_cfg80211_authenticate(struct wiphy *wiphy,
 		return -EINVAL;
 	}
 
+	for (i = 0; i < MWIFIEX_MAX_BSS_NUM; i++) {
+		tmp_priv = adapter->priv[i];
+		if (tmp_priv == priv)
+			continue;
+		if (GET_BSS_ROLE(tmp_priv) == MWIFIEX_BSS_ROLE_UAP &&
+		    netif_carrier_ok(tmp_priv->netdev) &&
+		    cfg80211_chandef_valid(&tmp_priv->bss_chandef)) {
+			if (!ieee80211_channel_equal
+			    (req->bss->channel, tmp_priv->bss_chandef.chan)) {
+				mwifiex_dbg(adapter, ERROR,
+					    "STA/AP must on the same channel\n");
+				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");