* [PATCH] wifi: mwifiex: Ensure all STA and AP use the same channel
@ 2024-08-30 6:56 Sascha Hauer
2024-08-30 9:49 ` Kalle Valo
2024-09-03 7:31 ` [EXT] " David Lin
0 siblings, 2 replies; 6+ messages in thread
From: Sascha Hauer @ 2024-08-30 6:56 UTC (permalink / raw)
To: Brian Norris, Francesco Dolcini, Kalle Valo
Cc: linux-wireless, linux-kernel, Sascha Hauer, stable
The mwifiex chips support simultaneous Accesspoint and station mode,
but this only works when all are using the same channel. The downstream
driver uses ECSA which makes the Accesspoint automatically switch to the
channel the station is going to use. Until this is implemented in the
mwifiex driver at least catch this situation and bail out with an error.
Userspace doesn't have a meaningful way to figure out what went wrong,
so print an error message to give the user a clue.
Without this patch the driver would timeout on the
HostCmd_CMD_802_11_ASSOCIATE command when creating a station with a
channel different from the one that an existing accesspoint uses.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Cc: stable@vger.kernel.org
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 52 ++++++++++++++++++++++++
drivers/net/wireless/marvell/mwifiex/main.h | 1 +
drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 3 ++
3 files changed, 56 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 5697a02e6b8d3..0d3bf624cd3de 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -2054,6 +2054,55 @@ static int mwifiex_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
return 0;
}
+bool mwifiex_channel_conflict(struct mwifiex_private *priv, struct ieee80211_channel *ch)
+{
+ struct mwifiex_adapter *adapter = priv->adapter;
+ struct mwifiex_current_bss_params *bss_params;
+ u8 band;
+ int freq, i;
+
+ for (i = 0; i < adapter->priv_num; i++) {
+ struct mwifiex_private *p = adapter->priv[i];
+ struct ieee80211_channel *used = NULL;
+
+ if (p == priv)
+ continue;
+
+ switch (GET_BSS_ROLE(p)) {
+ case MWIFIEX_BSS_ROLE_UAP:
+ if (!netif_carrier_ok(p->netdev))
+ break;
+
+ if (!cfg80211_chandef_valid(&p->bss_chandef))
+ break;
+
+ used = p->bss_chandef.chan;
+
+ break;
+ case MWIFIEX_BSS_ROLE_STA:
+ if (!p->media_connected)
+ break;
+
+ bss_params = &p->curr_bss_params;
+ band = mwifiex_band_to_radio_type(bss_params->band);
+ freq = ieee80211_channel_to_frequency(bss_params->bss_descriptor.channel,
+ band);
+
+ used = ieee80211_get_channel(priv->wdev.wiphy, freq);
+
+ break;
+ }
+
+ if (used && !ieee80211_channel_equal(used, ch)) {
+ mwifiex_dbg(priv->adapter, MSG,
+ "all AP and STA must operate on same channel\n");
+ return false;
+ }
+ }
+
+ return true;
+}
+
/* cfg80211 operation handler for start_ap.
* Function sets beacon period, DTIM period, SSID and security into
* AP config structure.
@@ -2069,6 +2118,9 @@ static int mwifiex_cfg80211_start_ap(struct wiphy *wiphy,
if (GET_BSS_ROLE(priv) != MWIFIEX_BSS_ROLE_UAP)
return -1;
+ if (!mwifiex_channel_conflict(priv, params->chandef.chan))
+ return -EBUSY;
+
bss_cfg = kzalloc(sizeof(struct mwifiex_uap_bss_param), GFP_KERNEL);
if (!bss_cfg)
return -ENOMEM;
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 529863edd7a25..b68dbf884156b 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -1697,6 +1697,7 @@ int mwifiex_set_mac_address(struct mwifiex_private *priv,
struct net_device *dev,
bool external, u8 *new_mac);
void mwifiex_devdump_tmo_func(unsigned long function_context);
+bool mwifiex_channel_conflict(struct mwifiex_private *priv, struct ieee80211_channel *ch);
#ifdef CONFIG_DEBUG_FS
void mwifiex_debugfs_init(void);
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
index d3cba6895f8ce..9794816d8a0c6 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
@@ -291,6 +291,9 @@ int mwifiex_bss_start(struct mwifiex_private *priv, struct cfg80211_bss *bss,
if (!bss_desc)
return -1;
+ if (!mwifiex_channel_conflict(priv, bss->channel))
+ return -EBUSY;
+
if (mwifiex_band_to_radio_type(bss_desc->bss_band) ==
HostCmd_SCAN_RADIO_TYPE_BG) {
config_bands = BAND_B | BAND_G | BAND_GN;
---
base-commit: 67a72043aa2e6f60f7bbe7bfa598ba168f16d04f
change-id: 20240830-mwifiex-check-channel-f411a156bbe0
Best regards,
--
Sascha Hauer <s.hauer@pengutronix.de>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] wifi: mwifiex: Ensure all STA and AP use the same channel
2024-08-30 6:56 [PATCH] wifi: mwifiex: Ensure all STA and AP use the same channel Sascha Hauer
@ 2024-08-30 9:49 ` Kalle Valo
2024-08-30 11:04 ` Sascha Hauer
2024-09-03 7:31 ` [EXT] " David Lin
1 sibling, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2024-08-30 9:49 UTC (permalink / raw)
To: Sascha Hauer
Cc: Brian Norris, Francesco Dolcini, linux-wireless, linux-kernel,
stable
Sascha Hauer <s.hauer@pengutronix.de> writes:
> The mwifiex chips support simultaneous Accesspoint and station mode,
> but this only works when all are using the same channel. The downstream
> driver uses ECSA which makes the Accesspoint automatically switch to the
> channel the station is going to use. Until this is implemented in the
> mwifiex driver at least catch this situation and bail out with an error.
> Userspace doesn't have a meaningful way to figure out what went wrong,
> so print an error message to give the user a clue.
>
> Without this patch the driver would timeout on the
> HostCmd_CMD_802_11_ASSOCIATE command when creating a station with a
> channel different from the one that an existing accesspoint uses.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: stable@vger.kernel.org
Does this mean that iface combination definitions are wrong? For example:
static const struct
ieee80211_iface_combination mwifiex_iface_comb_ap_sta_drcs = {
.limits = mwifiex_ap_sta_limits,
.num_different_channels = 2,
.n_limits = ARRAY_SIZE(mwifiex_ap_sta_limits),
.max_interfaces = MWIFIEX_MAX_BSS_NUM,
.beacon_int_infra_match = true,
};
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
https://docs.kernel.org/process/submitting-patches.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wifi: mwifiex: Ensure all STA and AP use the same channel
2024-08-30 9:49 ` Kalle Valo
@ 2024-08-30 11:04 ` Sascha Hauer
2024-08-30 11:58 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2024-08-30 11:04 UTC (permalink / raw)
To: Kalle Valo
Cc: Brian Norris, Francesco Dolcini, linux-wireless, linux-kernel,
stable
On Fri, Aug 30, 2024 at 12:49:34PM +0300, Kalle Valo wrote:
> Sascha Hauer <s.hauer@pengutronix.de> writes:
>
> > The mwifiex chips support simultaneous Accesspoint and station mode,
> > but this only works when all are using the same channel. The downstream
> > driver uses ECSA which makes the Accesspoint automatically switch to the
> > channel the station is going to use. Until this is implemented in the
> > mwifiex driver at least catch this situation and bail out with an error.
> > Userspace doesn't have a meaningful way to figure out what went wrong,
> > so print an error message to give the user a clue.
> >
> > Without this patch the driver would timeout on the
> > HostCmd_CMD_802_11_ASSOCIATE command when creating a station with a
> > channel different from the one that an existing accesspoint uses.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: stable@vger.kernel.org
>
> Does this mean that iface combination definitions are wrong? For example:
>
> static const struct
> ieee80211_iface_combination mwifiex_iface_comb_ap_sta_drcs = {
> .limits = mwifiex_ap_sta_limits,
> .num_different_channels = 2,
> .n_limits = ARRAY_SIZE(mwifiex_ap_sta_limits),
> .max_interfaces = MWIFIEX_MAX_BSS_NUM,
> .beacon_int_infra_match = true,
> };
I wasn't aware of DRCS as it's disabled by default in the mwifiex
driver. From a quick test I can say that indeed with DRCS two channels
are supported. It seems we have to relax the same channel enforcement
when DRCS is enabled.
This brings up the question why DRCS is disabled by default. Wouldn't it
make sense to always enable it when available?
Related: num_different_channels is exposed to userspace, but outside the
MAC80211 layer there is nothing in the kernel that enforces this
restriction. Am I missing something or is this just an open patch
opportunity?
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wifi: mwifiex: Ensure all STA and AP use the same channel
2024-08-30 11:04 ` Sascha Hauer
@ 2024-08-30 11:58 ` Johannes Berg
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2024-08-30 11:58 UTC (permalink / raw)
To: Sascha Hauer, Kalle Valo
Cc: Brian Norris, Francesco Dolcini, linux-wireless, linux-kernel,
stable
On Fri, 2024-08-30 at 13:04 +0200, Sascha Hauer wrote:
>
> Related: num_different_channels is exposed to userspace, but outside the
> MAC80211 layer there is nothing in the kernel that enforces this
> restriction. Am I missing something or is this just an open patch
> opportunity?
>
It is, or was least was, non-trivial to do in cfg80211 since it
isn't/wasn't always fully aware of what's going on. mac80211 does this
by calling into some helper functions with an appropriate description of
what's happening, any other driver could do that too.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXT] [PATCH] wifi: mwifiex: Ensure all STA and AP use the same channel
2024-08-30 6:56 [PATCH] wifi: mwifiex: Ensure all STA and AP use the same channel Sascha Hauer
2024-08-30 9:49 ` Kalle Valo
@ 2024-09-03 7:31 ` David Lin
2024-09-09 17:17 ` Francesco Dolcini
1 sibling, 1 reply; 6+ messages in thread
From: David Lin @ 2024-09-03 7:31 UTC (permalink / raw)
To: Sascha Hauer, Brian Norris, Francesco Dolcini, Kalle Valo
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, Pete Hsieh
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Friday, August 30, 2024 2:57 PM
> To: Brian Norris <briannorris@chromium.org>; Francesco Dolcini
> <francesco@dolcini.it>; Kalle Valo <kvalo@kernel.org>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; Sascha Hauer
> <s.hauer@pengutronix.de>; stable@vger.kernel.org
> Subject: [EXT] [PATCH] wifi: mwifiex: Ensure all STA and AP use the same
> 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
>
>
> The mwifiex chips support simultaneous Accesspoint and station mode, but this
> only works when all are using the same channel. The downstream driver uses
> ECSA which makes the Accesspoint automatically switch to the channel the
> station is going to use. Until this is implemented in the mwifiex driver at
> least catch this situation and bail out with an error.
> Userspace doesn't have a meaningful way to figure out what went wrong, so
> print an error message to give the user a clue.
>
> Without this patch the driver would timeout on the
> HostCmd_CMD_802_11_ASSOCIATE command when creating a station with a
> channel different from the one that an existing accesspoint uses.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: stable@vger.kernel.org
> ---
> drivers/net/wireless/marvell/mwifiex/cfg80211.c | 52
> ++++++++++++++++++++++++
> drivers/net/wireless/marvell/mwifiex/main.h | 1 +
> drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 3 ++
> 3 files changed, 56 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> index 5697a02e6b8d3..0d3bf624cd3de 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> @@ -2054,6 +2054,55 @@ static int mwifiex_cfg80211_stop_ap(struct wiphy
> *wiphy, struct net_device *dev,
> return 0;
> }
>
> +bool mwifiex_channel_conflict(struct mwifiex_private *priv, struct
> +ieee80211_channel *ch) {
> + struct mwifiex_adapter *adapter = priv->adapter;
> + struct mwifiex_current_bss_params *bss_params;
> + u8 band;
> + int freq, i;
> +
> + for (i = 0; i < adapter->priv_num; i++) {
> + struct mwifiex_private *p = adapter->priv[i];
> + struct ieee80211_channel *used = NULL;
> +
> + if (p == priv)
> + continue;
> +
> + switch (GET_BSS_ROLE(p)) {
> + case MWIFIEX_BSS_ROLE_UAP:
> + if (!netif_carrier_ok(p->netdev))
> + break;
> +
> + if (!cfg80211_chandef_valid(&p->bss_chandef))
> + break;
> +
> + used = p->bss_chandef.chan;
> +
> + break;
> + case MWIFIEX_BSS_ROLE_STA:
> + if (!p->media_connected)
> + break;
> +
> + bss_params = &p->curr_bss_params;
> + band =
> mwifiex_band_to_radio_type(bss_params->band);
> + freq =
> ieee80211_channel_to_frequency(bss_params->bss_descriptor.channel,
> +
> band);
> +
> + used =
> ieee80211_get_channel(priv->wdev.wiphy,
> + freq);
> +
> + break;
> + }
> +
> + if (used && !ieee80211_channel_equal(used, ch)) {
> + mwifiex_dbg(priv->adapter, MSG,
> + "all AP and STA must operate on
> same channel\n");
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +
> /* cfg80211 operation handler for start_ap.
> * Function sets beacon period, DTIM period, SSID and security into
> * AP config structure.
> @@ -2069,6 +2118,9 @@ static int mwifiex_cfg80211_start_ap(struct wiphy
> *wiphy,
> if (GET_BSS_ROLE(priv) != MWIFIEX_BSS_ROLE_UAP)
> return -1;
>
> + if (!mwifiex_channel_conflict(priv, params->chandef.chan))
> + return -EBUSY;
> +
> bss_cfg = kzalloc(sizeof(struct mwifiex_uap_bss_param),
> GFP_KERNEL);
> if (!bss_cfg)
> return -ENOMEM;
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.h
> b/drivers/net/wireless/marvell/mwifiex/main.h
> index 529863edd7a25..b68dbf884156b 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.h
> +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> @@ -1697,6 +1697,7 @@ int mwifiex_set_mac_address(struct
> mwifiex_private *priv,
> struct net_device *dev,
> bool external, u8 *new_mac); void
> mwifiex_devdump_tmo_func(unsigned long function_context);
> +bool mwifiex_channel_conflict(struct mwifiex_private *priv, struct
> +ieee80211_channel *ch);
>
> #ifdef CONFIG_DEBUG_FS
> void mwifiex_debugfs_init(void);
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> index d3cba6895f8ce..9794816d8a0c6 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> @@ -291,6 +291,9 @@ int mwifiex_bss_start(struct mwifiex_private *priv,
> struct cfg80211_bss *bss,
> if (!bss_desc)
> return -1;
>
> + if (!mwifiex_channel_conflict(priv, bss->channel))
> + return -EBUSY;
> +
> if (mwifiex_band_to_radio_type(bss_desc->bss_band) ==
>
> HostCmd_SCAN_RADIO_TYPE_BG) {
> config_bands = BAND_B | BAND_G | BAND_GN;
>
> ---
> base-commit: 67a72043aa2e6f60f7bbe7bfa598ba168f16d04f
> change-id: 20240830-mwifiex-check-channel-f411a156bbe0
>
> Best regards,
> --
> Sascha Hauer <s.hauer@pengutronix.de>
Please use https://patchwork.kernel.org/project/linux-wireless/patch/20240902084311.2607-1-yu-hao.lin@nxp.com/ to replace this patch.
This patch can't let AP and STA running on the same channel if some wiphy parameters are set.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXT] [PATCH] wifi: mwifiex: Ensure all STA and AP use the same channel
2024-09-03 7:31 ` [EXT] " David Lin
@ 2024-09-09 17:17 ` Francesco Dolcini
0 siblings, 0 replies; 6+ messages in thread
From: Francesco Dolcini @ 2024-09-09 17:17 UTC (permalink / raw)
To: David Lin, Sascha Hauer
Cc: Brian Norris, Francesco Dolcini, Kalle Valo,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, Pete Hsieh
Hello Sascha,
On Tue, Sep 03, 2024 at 07:31:34AM +0000, David Lin wrote:
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> >
> > The mwifiex chips support simultaneous Accesspoint and station mode, but this
> > only works when all are using the same channel. The downstream driver uses
> > ECSA which makes the Accesspoint automatically switch to the channel the
> > station is going to use. Until this is implemented in the mwifiex driver at
> > least catch this situation and bail out with an error.
> > Userspace doesn't have a meaningful way to figure out what went wrong, so
> > print an error message to give the user a clue.
> >
> > Without this patch the driver would timeout on the
> > HostCmd_CMD_802_11_ASSOCIATE command when creating a station with a
> > channel different from the one that an existing accesspoint uses.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: stable@vger.kernel.org
...
> Please use
> https://patchwork.kernel.org/project/linux-wireless/patch/20240902084311.2607-1-yu-hao.lin@nxp.com/
> to replace this patch.
>
> This patch can't let AP and STA running on the same channel if some
> wiphy parameters are set.
>
Sasha, any comment on this? It seems you are solving the same issue (I
did not look into any of the 2 patches myself so far).
Francesco
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-09 17:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-30 6:56 [PATCH] wifi: mwifiex: Ensure all STA and AP use the same channel Sascha Hauer
2024-08-30 9:49 ` Kalle Valo
2024-08-30 11:04 ` Sascha Hauer
2024-08-30 11:58 ` Johannes Berg
2024-09-03 7:31 ` [EXT] " David Lin
2024-09-09 17:17 ` Francesco Dolcini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox