| Message ID | 20240830030630.825818-1-yu-hao.lin@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: mwifiex: avoid AP and STA running on different channel | expand |
> From: Francesco Dolcini <francesco@dolcini.it> > Sent: Friday, August 30, 2024 4:11 PM > To: David Lin <yu-hao.lin@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de> > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; > briannorris@chromium.org; kvalo@kernel.org; francesco@dolcini.it; Pete > Hsieh <tsung-hsien.hsieh@nxp.com> > 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
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
> From: Sascha Hauer <s.hauer@pengutronix.de> > Sent: Friday, August 30, 2024 5:19 PM > To: Francesco Dolcini <francesco@dolcini.it> > Cc: David Lin <yu-hao.lin@nxp.com>; linux-wireless@vger.kernel.org; > linux-kernel@vger.kernel.org; briannorris@chromium.org; kvalo@kernel.org; > Pete Hsieh <tsung-hsien.hsieh@nxp.com> > 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
On Fri, Aug 30, 2024 at 10:58:50AM +0000, David Lin wrote: > > From: Sascha Hauer <s.hauer@pengutronix.de> > > Sent: Friday, August 30, 2024 5:19 PM > > To: Francesco Dolcini <francesco@dolcini.it> > > Cc: David Lin <yu-hao.lin@nxp.com>; linux-wireless@vger.kernel.org; > > linux-kernel@vger.kernel.org; briannorris@chromium.org; kvalo@kernel.org; > > Pete Hsieh <tsung-hsien.hsieh@nxp.com> > > 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
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");
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 <yu-hao.lin@nxp.com> --- .../net/wireless/marvell/mwifiex/cfg80211.c | 71 +++++++++++++++++-- 1 file changed, 66 insertions(+), 5 deletions(-) base-commit: ae98f5c9fd8ba84cd408b41faa77e65bf1b4cdfa