* [PATCH 1/9] mwifiex: Small cleanup for handling virtual interface type changes
2021-09-14 19:59 [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Jonas Dreßler
@ 2021-09-14 19:59 ` Jonas Dreßler
2021-09-21 15:02 ` Kalle Valo
2021-09-14 19:59 ` [PATCH 2/9] mwifiex: Use function to check whether interface type change is allowed Jonas Dreßler
` (8 subsequent siblings)
9 siblings, 1 reply; 12+ messages in thread
From: Jonas Dreßler @ 2021-09-14 19:59 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski
Cc: Jonas Dreßler, Tsuchiya Yuto, linux-wireless, netdev,
linux-kernel, Maximilian Luz, Andy Shevchenko, Pali Rohár
Handle the obvious invalid virtual interface type changes with a general
check instead of looking at the individual change.
For type changes from P2P_CLIENT to P2P_GO and the other way round, this
changes the behavior slightly: We now still do nothing, but return
-EOPNOTSUPP instead of 0. Now that behavior was incorrect before and
still is, because type changes between these two types are actually
possible and supported, which we'll fix in a following commit.
Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
---
.../net/wireless/marvell/mwifiex/cfg80211.c | 39 +++++++------------
1 file changed, 14 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 0961f4a5e415..e8deba119ff1 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1141,6 +1141,20 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
return -EBUSY;
}
+ if (type == NL80211_IFTYPE_UNSPECIFIED) {
+ mwifiex_dbg(priv->adapter, INFO,
+ "%s: no new type specified, keeping old type %d\n",
+ dev->name, curr_iftype);
+ return 0;
+ }
+
+ if (curr_iftype == type) {
+ mwifiex_dbg(priv->adapter, INFO,
+ "%s: interface already is of type %d\n",
+ dev->name, curr_iftype);
+ return 0;
+ }
+
switch (curr_iftype) {
case NL80211_IFTYPE_ADHOC:
switch (type) {
@@ -1160,12 +1174,6 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
case NL80211_IFTYPE_AP:
return mwifiex_change_vif_to_ap(dev, curr_iftype, type,
params);
- case NL80211_IFTYPE_UNSPECIFIED:
- mwifiex_dbg(priv->adapter, INFO,
- "%s: kept type as IBSS\n", dev->name);
- fallthrough;
- case NL80211_IFTYPE_ADHOC: /* This shouldn't happen */
- return 0;
default:
mwifiex_dbg(priv->adapter, ERROR,
"%s: changing to %d not supported\n",
@@ -1191,12 +1199,6 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
case NL80211_IFTYPE_AP:
return mwifiex_change_vif_to_ap(dev, curr_iftype, type,
params);
- case NL80211_IFTYPE_UNSPECIFIED:
- mwifiex_dbg(priv->adapter, INFO,
- "%s: kept type as STA\n", dev->name);
- fallthrough;
- case NL80211_IFTYPE_STATION: /* This shouldn't happen */
- return 0;
default:
mwifiex_dbg(priv->adapter, ERROR,
"%s: changing to %d not supported\n",
@@ -1214,12 +1216,6 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
case NL80211_IFTYPE_P2P_GO:
return mwifiex_change_vif_to_p2p(dev, curr_iftype,
type, params);
- case NL80211_IFTYPE_UNSPECIFIED:
- mwifiex_dbg(priv->adapter, INFO,
- "%s: kept type as AP\n", dev->name);
- fallthrough;
- case NL80211_IFTYPE_AP: /* This shouldn't happen */
- return 0;
default:
mwifiex_dbg(priv->adapter, ERROR,
"%s: changing to %d not supported\n",
@@ -1254,13 +1250,6 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
return -EFAULT;
return mwifiex_change_vif_to_ap(dev, curr_iftype, type,
params);
- case NL80211_IFTYPE_UNSPECIFIED:
- mwifiex_dbg(priv->adapter, INFO,
- "%s: kept type as P2P\n", dev->name);
- fallthrough;
- case NL80211_IFTYPE_P2P_CLIENT:
- case NL80211_IFTYPE_P2P_GO:
- return 0;
default:
mwifiex_dbg(priv->adapter, ERROR,
"%s: changing to %d not supported\n",
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/9] mwifiex: Small cleanup for handling virtual interface type changes
2021-09-14 19:59 ` [PATCH 1/9] mwifiex: Small cleanup for handling virtual interface type changes Jonas Dreßler
@ 2021-09-21 15:02 ` Kalle Valo
0 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2021-09-21 15:02 UTC (permalink / raw)
To: Jonas Dreßler
Cc: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, David S. Miller, Jakub Kicinski, Jonas Dreßler,
Tsuchiya Yuto, linux-wireless, netdev, linux-kernel,
Maximilian Luz, Andy Shevchenko, Pali Rohár
Jonas Dreßler <verdre@v0yd.nl> wrote:
> Handle the obvious invalid virtual interface type changes with a general
> check instead of looking at the individual change.
>
> For type changes from P2P_CLIENT to P2P_GO and the other way round, this
> changes the behavior slightly: We now still do nothing, but return
> -EOPNOTSUPP instead of 0. Now that behavior was incorrect before and
> still is, because type changes between these two types are actually
> possible and supported, which we'll fix in a following commit.
>
> Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
9 patches applied to wireless-drivers-next.git, thanks.
babe2a332dc4 mwifiex: Small cleanup for handling virtual interface type changes
abe3a2c9ead8 mwifiex: Use function to check whether interface type change is allowed
c2e9666cdffd mwifiex: Run SET_BSS_MODE when changing from P2P to STATION vif-type
54350dac4e6a mwifiex: Use helper function for counting interface types
fae2aac8c740 mwifiex: Update virtual interface counters right after setting bss_type
25bbec30a2c7 mwifiex: Allow switching interface type from P2P_CLIENT to P2P_GO
5e2e1a4bf4a1 mwifiex: Handle interface type changes from AP to STATION
c606008b7062 mwifiex: Properly initialize private structure on interface type changes
72e717500f99 mwifiex: Fix copy-paste mistake when creating virtual interface
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210914195909.36035-2-verdre@v0yd.nl/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/9] mwifiex: Use function to check whether interface type change is allowed
2021-09-14 19:59 [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Jonas Dreßler
2021-09-14 19:59 ` [PATCH 1/9] mwifiex: Small cleanup for handling virtual interface type changes Jonas Dreßler
@ 2021-09-14 19:59 ` Jonas Dreßler
2021-09-14 19:59 ` [PATCH 3/9] mwifiex: Run SET_BSS_MODE when changing from P2P to STATION vif-type Jonas Dreßler
` (7 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jonas Dreßler @ 2021-09-14 19:59 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski
Cc: Jonas Dreßler, Tsuchiya Yuto, linux-wireless, netdev,
linux-kernel, Maximilian Luz, Andy Shevchenko, Pali Rohár
Instead of bailing out in the function which is supposed to do the type
change, detect invalid changes beforehand using a generic function and
return an error if the change is not allowed.
Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
---
.../net/wireless/marvell/mwifiex/cfg80211.c | 139 ++++++++++++------
1 file changed, 92 insertions(+), 47 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index e8deba119ff1..dabc59c47de3 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -939,6 +939,76 @@ mwifiex_init_new_priv_params(struct mwifiex_private *priv,
return 0;
}
+static bool
+is_vif_type_change_allowed(struct mwifiex_adapter *adapter,
+ enum nl80211_iftype old_iftype,
+ enum nl80211_iftype new_iftype)
+{
+ switch (old_iftype) {
+ case NL80211_IFTYPE_ADHOC:
+ switch (new_iftype) {
+ case NL80211_IFTYPE_STATION:
+ return true;
+ case NL80211_IFTYPE_P2P_CLIENT:
+ case NL80211_IFTYPE_P2P_GO:
+ return adapter->curr_iface_comb.p2p_intf !=
+ adapter->iface_limit.p2p_intf;
+ case NL80211_IFTYPE_AP:
+ return adapter->curr_iface_comb.uap_intf !=
+ adapter->iface_limit.uap_intf;
+ default:
+ return false;
+ }
+
+ case NL80211_IFTYPE_STATION:
+ switch (new_iftype) {
+ case NL80211_IFTYPE_ADHOC:
+ return true;
+ case NL80211_IFTYPE_P2P_CLIENT:
+ case NL80211_IFTYPE_P2P_GO:
+ return adapter->curr_iface_comb.p2p_intf !=
+ adapter->iface_limit.p2p_intf;
+ case NL80211_IFTYPE_AP:
+ return adapter->curr_iface_comb.uap_intf !=
+ adapter->iface_limit.uap_intf;
+ default:
+ return false;
+ }
+
+ case NL80211_IFTYPE_AP:
+ switch (new_iftype) {
+ case NL80211_IFTYPE_ADHOC:
+ case NL80211_IFTYPE_STATION:
+ return adapter->curr_iface_comb.sta_intf !=
+ adapter->iface_limit.sta_intf;
+ case NL80211_IFTYPE_P2P_CLIENT:
+ case NL80211_IFTYPE_P2P_GO:
+ return adapter->curr_iface_comb.p2p_intf !=
+ adapter->iface_limit.p2p_intf;
+ default:
+ return false;
+ }
+
+ case NL80211_IFTYPE_P2P_CLIENT:
+ case NL80211_IFTYPE_P2P_GO:
+ switch (new_iftype) {
+ case NL80211_IFTYPE_ADHOC:
+ case NL80211_IFTYPE_STATION:
+ return true;
+ case NL80211_IFTYPE_AP:
+ return adapter->curr_iface_comb.uap_intf !=
+ adapter->iface_limit.uap_intf;
+ default:
+ return false;
+ }
+
+ default:
+ break;
+ }
+
+ return false;
+}
+
static int
mwifiex_change_vif_to_p2p(struct net_device *dev,
enum nl80211_iftype curr_iftype,
@@ -955,13 +1025,6 @@ mwifiex_change_vif_to_p2p(struct net_device *dev,
adapter = priv->adapter;
- if (adapter->curr_iface_comb.p2p_intf ==
- adapter->iface_limit.p2p_intf) {
- mwifiex_dbg(adapter, ERROR,
- "cannot create multiple P2P ifaces\n");
- return -1;
- }
-
mwifiex_dbg(adapter, INFO,
"%s: changing role to p2p\n", dev->name);
@@ -1027,15 +1090,6 @@ mwifiex_change_vif_to_sta_adhoc(struct net_device *dev,
adapter = priv->adapter;
- if ((curr_iftype != NL80211_IFTYPE_P2P_CLIENT &&
- curr_iftype != NL80211_IFTYPE_P2P_GO) &&
- (adapter->curr_iface_comb.sta_intf ==
- adapter->iface_limit.sta_intf)) {
- mwifiex_dbg(adapter, ERROR,
- "cannot create multiple station/adhoc ifaces\n");
- return -1;
- }
-
if (type == NL80211_IFTYPE_STATION)
mwifiex_dbg(adapter, INFO,
"%s: changing role to station\n", dev->name);
@@ -1086,13 +1140,6 @@ mwifiex_change_vif_to_ap(struct net_device *dev,
adapter = priv->adapter;
- if (adapter->curr_iface_comb.uap_intf ==
- adapter->iface_limit.uap_intf) {
- mwifiex_dbg(adapter, ERROR,
- "cannot create multiple AP ifaces\n");
- return -1;
- }
-
mwifiex_dbg(adapter, INFO,
"%s: changing role to AP\n", dev->name);
@@ -1155,6 +1202,13 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
return 0;
}
+ if (!is_vif_type_change_allowed(priv->adapter, curr_iftype, type)) {
+ mwifiex_dbg(priv->adapter, ERROR,
+ "%s: change from type %d to %d is not allowed\n",
+ dev->name, curr_iftype, type);
+ return -EOPNOTSUPP;
+ }
+
switch (curr_iftype) {
case NL80211_IFTYPE_ADHOC:
switch (type) {
@@ -1175,12 +1229,9 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
return mwifiex_change_vif_to_ap(dev, curr_iftype, type,
params);
default:
- mwifiex_dbg(priv->adapter, ERROR,
- "%s: changing to %d not supported\n",
- dev->name, type);
- return -EOPNOTSUPP;
+ goto errnotsupp;
}
- break;
+
case NL80211_IFTYPE_STATION:
switch (type) {
case NL80211_IFTYPE_ADHOC:
@@ -1200,12 +1251,9 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
return mwifiex_change_vif_to_ap(dev, curr_iftype, type,
params);
default:
- mwifiex_dbg(priv->adapter, ERROR,
- "%s: changing to %d not supported\n",
- dev->name, type);
- return -EOPNOTSUPP;
+ goto errnotsupp;
}
- break;
+
case NL80211_IFTYPE_AP:
switch (type) {
case NL80211_IFTYPE_ADHOC:
@@ -1217,12 +1265,9 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
return mwifiex_change_vif_to_p2p(dev, curr_iftype,
type, params);
default:
- mwifiex_dbg(priv->adapter, ERROR,
- "%s: changing to %d not supported\n",
- dev->name, type);
- return -EOPNOTSUPP;
+ goto errnotsupp;
}
- break;
+
case NL80211_IFTYPE_P2P_CLIENT:
case NL80211_IFTYPE_P2P_GO:
switch (type) {
@@ -1251,21 +1296,21 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
return mwifiex_change_vif_to_ap(dev, curr_iftype, type,
params);
default:
- mwifiex_dbg(priv->adapter, ERROR,
- "%s: changing to %d not supported\n",
- dev->name, type);
- return -EOPNOTSUPP;
+ goto errnotsupp;
}
- break;
+
default:
- mwifiex_dbg(priv->adapter, ERROR,
- "%s: unknown iftype: %d\n",
- dev->name, dev->ieee80211_ptr->iftype);
- return -EOPNOTSUPP;
+ goto errnotsupp;
}
return 0;
+
+errnotsupp:
+ mwifiex_dbg(priv->adapter, ERROR,
+ "unsupported interface type transition: %d to %d\n",
+ curr_iftype, type);
+ return -EOPNOTSUPP;
}
static void
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/9] mwifiex: Run SET_BSS_MODE when changing from P2P to STATION vif-type
2021-09-14 19:59 [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Jonas Dreßler
2021-09-14 19:59 ` [PATCH 1/9] mwifiex: Small cleanup for handling virtual interface type changes Jonas Dreßler
2021-09-14 19:59 ` [PATCH 2/9] mwifiex: Use function to check whether interface type change is allowed Jonas Dreßler
@ 2021-09-14 19:59 ` Jonas Dreßler
2021-09-14 19:59 ` [PATCH 4/9] mwifiex: Use helper function for counting interface types Jonas Dreßler
` (6 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jonas Dreßler @ 2021-09-14 19:59 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski
Cc: Jonas Dreßler, Tsuchiya Yuto, linux-wireless, netdev,
linux-kernel, Maximilian Luz, Andy Shevchenko, Pali Rohár
We currently handle changing from the P2P to the STATION virtual
interface type slightly different than changing from P2P to ADHOC: When
changing to STATION, we don't send the SET_BSS_MODE command. We do send
that command on all other type-changes though, and it probably makes
sense to send the command since after all we just changed our BSS_MODE.
Looking at prior changes to this part of the code, it seems that this is
simply a leftover from old refactorings.
Since sending the SET_BSS_MODE command is the only difference between
mwifiex_change_vif_to_sta_adhoc() and the current code, we can now use
mwifiex_change_vif_to_sta_adhoc() for both switching to ADHOC and
STATION interface type.
This does not fix any particular bug and just "looked right", so there's
a small chance it might be a regression.
Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
---
.../net/wireless/marvell/mwifiex/cfg80211.c | 22 ++++---------------
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index dabc59c47de3..146aabe14753 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1270,29 +1270,15 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
case NL80211_IFTYPE_P2P_CLIENT:
case NL80211_IFTYPE_P2P_GO:
+ if (mwifiex_cfg80211_deinit_p2p(priv))
+ return -EFAULT;
+
switch (type) {
- case NL80211_IFTYPE_STATION:
- if (mwifiex_cfg80211_deinit_p2p(priv))
- return -EFAULT;
- priv->adapter->curr_iface_comb.p2p_intf--;
- priv->adapter->curr_iface_comb.sta_intf++;
- dev->ieee80211_ptr->iftype = type;
- if (mwifiex_deinit_priv_params(priv))
- return -1;
- if (mwifiex_init_new_priv_params(priv, dev, type))
- return -1;
- if (mwifiex_sta_init_cmd(priv, false, false))
- return -1;
- break;
case NL80211_IFTYPE_ADHOC:
- if (mwifiex_cfg80211_deinit_p2p(priv))
- return -EFAULT;
+ case NL80211_IFTYPE_STATION:
return mwifiex_change_vif_to_sta_adhoc(dev, curr_iftype,
type, params);
- break;
case NL80211_IFTYPE_AP:
- if (mwifiex_cfg80211_deinit_p2p(priv))
- return -EFAULT;
return mwifiex_change_vif_to_ap(dev, curr_iftype, type,
params);
default:
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/9] mwifiex: Use helper function for counting interface types
2021-09-14 19:59 [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Jonas Dreßler
` (2 preceding siblings ...)
2021-09-14 19:59 ` [PATCH 3/9] mwifiex: Run SET_BSS_MODE when changing from P2P to STATION vif-type Jonas Dreßler
@ 2021-09-14 19:59 ` Jonas Dreßler
2021-09-14 19:59 ` [PATCH 5/9] mwifiex: Update virtual interface counters right after setting bss_type Jonas Dreßler
` (5 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jonas Dreßler @ 2021-09-14 19:59 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski
Cc: Jonas Dreßler, Tsuchiya Yuto, linux-wireless, netdev,
linux-kernel, Maximilian Luz, Andy Shevchenko, Pali Rohár
Use a small helper function to increment and decrement the counter of
the interface types we currently manage. This makes the code that
actually changes and sets up the interface type a bit less messy and
also helps avoiding mistakes in case someone increments/decrements a
counter wrongly.
Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
---
.../net/wireless/marvell/mwifiex/cfg80211.c | 110 ++++++------------
1 file changed, 35 insertions(+), 75 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 146aabe14753..8b9517c243c8 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1009,6 +1009,32 @@ is_vif_type_change_allowed(struct mwifiex_adapter *adapter,
return false;
}
+static void
+update_vif_type_counter(struct mwifiex_adapter *adapter,
+ enum nl80211_iftype iftype,
+ int change)
+{
+ switch (iftype) {
+ case NL80211_IFTYPE_UNSPECIFIED:
+ case NL80211_IFTYPE_ADHOC:
+ case NL80211_IFTYPE_STATION:
+ adapter->curr_iface_comb.sta_intf += change;
+ break;
+ case NL80211_IFTYPE_AP:
+ adapter->curr_iface_comb.uap_intf += change;
+ break;
+ case NL80211_IFTYPE_P2P_CLIENT:
+ case NL80211_IFTYPE_P2P_GO:
+ adapter->curr_iface_comb.p2p_intf += change;
+ break;
+ default:
+ mwifiex_dbg(adapter, ERROR,
+ "%s: Unsupported iftype passed: %d\n",
+ __func__, iftype);
+ break;
+ }
+}
+
static int
mwifiex_change_vif_to_p2p(struct net_device *dev,
enum nl80211_iftype curr_iftype,
@@ -1056,19 +1082,8 @@ mwifiex_change_vif_to_p2p(struct net_device *dev,
if (mwifiex_sta_init_cmd(priv, false, false))
return -1;
- switch (curr_iftype) {
- case NL80211_IFTYPE_STATION:
- case NL80211_IFTYPE_ADHOC:
- adapter->curr_iface_comb.sta_intf--;
- break;
- case NL80211_IFTYPE_AP:
- adapter->curr_iface_comb.uap_intf--;
- break;
- default:
- break;
- }
-
- adapter->curr_iface_comb.p2p_intf++;
+ update_vif_type_counter(adapter, curr_iftype, -1);
+ update_vif_type_counter(adapter, type, +1);
dev->ieee80211_ptr->iftype = type;
return 0;
@@ -1107,20 +1122,10 @@ mwifiex_change_vif_to_sta_adhoc(struct net_device *dev,
if (mwifiex_sta_init_cmd(priv, false, false))
return -1;
- switch (curr_iftype) {
- case NL80211_IFTYPE_P2P_CLIENT:
- case NL80211_IFTYPE_P2P_GO:
- adapter->curr_iface_comb.p2p_intf--;
- break;
- case NL80211_IFTYPE_AP:
- adapter->curr_iface_comb.uap_intf--;
- break;
- default:
- break;
- }
-
- adapter->curr_iface_comb.sta_intf++;
+ update_vif_type_counter(adapter, curr_iftype, -1);
+ update_vif_type_counter(adapter, type, +1);
dev->ieee80211_ptr->iftype = type;
+
return 0;
}
@@ -1153,20 +1158,8 @@ mwifiex_change_vif_to_ap(struct net_device *dev,
if (mwifiex_sta_init_cmd(priv, false, false))
return -1;
- switch (curr_iftype) {
- case NL80211_IFTYPE_P2P_CLIENT:
- case NL80211_IFTYPE_P2P_GO:
- adapter->curr_iface_comb.p2p_intf--;
- break;
- case NL80211_IFTYPE_STATION:
- case NL80211_IFTYPE_ADHOC:
- adapter->curr_iface_comb.sta_intf--;
- break;
- default:
- break;
- }
-
- adapter->curr_iface_comb.uap_intf++;
+ update_vif_type_counter(adapter, curr_iftype, -1);
+ update_vif_type_counter(adapter, type, +1);
dev->ieee80211_ptr->iftype = type;
return 0;
}
@@ -3128,23 +3121,7 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
mwifiex_dev_debugfs_init(priv);
#endif
- switch (type) {
- case NL80211_IFTYPE_UNSPECIFIED:
- case NL80211_IFTYPE_STATION:
- case NL80211_IFTYPE_ADHOC:
- adapter->curr_iface_comb.sta_intf++;
- break;
- case NL80211_IFTYPE_AP:
- adapter->curr_iface_comb.uap_intf++;
- break;
- case NL80211_IFTYPE_P2P_CLIENT:
- adapter->curr_iface_comb.p2p_intf++;
- break;
- default:
- /* This should be dead code; checked above */
- mwifiex_dbg(adapter, ERROR, "type not supported\n");
- return ERR_PTR(-EINVAL);
- }
+ update_vif_type_counter(adapter, type, +1);
return &priv->wdev;
@@ -3210,24 +3187,7 @@ int mwifiex_del_virtual_intf(struct wiphy *wiphy, struct wireless_dev *wdev)
/* Clear the priv in adapter */
priv->netdev = NULL;
- switch (priv->bss_mode) {
- case NL80211_IFTYPE_UNSPECIFIED:
- case NL80211_IFTYPE_STATION:
- case NL80211_IFTYPE_ADHOC:
- adapter->curr_iface_comb.sta_intf--;
- break;
- case NL80211_IFTYPE_AP:
- adapter->curr_iface_comb.uap_intf--;
- break;
- case NL80211_IFTYPE_P2P_CLIENT:
- case NL80211_IFTYPE_P2P_GO:
- adapter->curr_iface_comb.p2p_intf--;
- break;
- default:
- mwifiex_dbg(adapter, ERROR,
- "del_virtual_intf: type not supported\n");
- break;
- }
+ update_vif_type_counter(adapter, priv->bss_mode, -1);
priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED;
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/9] mwifiex: Update virtual interface counters right after setting bss_type
2021-09-14 19:59 [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Jonas Dreßler
` (3 preceding siblings ...)
2021-09-14 19:59 ` [PATCH 4/9] mwifiex: Use helper function for counting interface types Jonas Dreßler
@ 2021-09-14 19:59 ` Jonas Dreßler
2021-09-14 19:59 ` [PATCH 6/9] mwifiex: Allow switching interface type from P2P_CLIENT to P2P_GO Jonas Dreßler
` (4 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jonas Dreßler @ 2021-09-14 19:59 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski
Cc: Jonas Dreßler, Tsuchiya Yuto, linux-wireless, netdev,
linux-kernel, Maximilian Luz, Andy Shevchenko, Pali Rohár
In mwifiex_init_new_priv_params() we update our private driver state to
reflect the currently selected virtual interface type. Most notably we
set the bss_mode to the mode we're going to put the firmware in.
Now after we updated the driver state we actually start talking to the
firmware and instruct it to set up the new mode. Those commands can and
will sometimes fail, in which case we return with an error from
mwifiex_change_vif_to_*. We currently update our virtual interface type
counters after this return, which means the code is never reached when a
firmware error happens and we never update the counters. Since we have
updated our bss_mode earlier though, the counters now no longer reflect
the actual state of the driver.
This will break things on the next virtual interface change, because the
virtual interface type we're switching away from didn't get its counter
incremented, and we end up decrementing a 0-counter.
To fix this, simply update the virtual interface type counters right
after updating our driver structures, so that they are always in sync.
Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
---
.../net/wireless/marvell/mwifiex/cfg80211.c | 25 +++++++++++--------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 8b9517c243c8..f2797102c5a2 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1059,6 +1059,10 @@ mwifiex_change_vif_to_p2p(struct net_device *dev,
if (mwifiex_init_new_priv_params(priv, dev, type))
return -1;
+ update_vif_type_counter(adapter, curr_iftype, -1);
+ update_vif_type_counter(adapter, type, +1);
+ dev->ieee80211_ptr->iftype = type;
+
switch (type) {
case NL80211_IFTYPE_P2P_CLIENT:
if (mwifiex_cfg80211_init_p2p_client(priv))
@@ -1082,10 +1086,6 @@ mwifiex_change_vif_to_p2p(struct net_device *dev,
if (mwifiex_sta_init_cmd(priv, false, false))
return -1;
- update_vif_type_counter(adapter, curr_iftype, -1);
- update_vif_type_counter(adapter, type, +1);
- dev->ieee80211_ptr->iftype = type;
-
return 0;
}
@@ -1116,16 +1116,17 @@ mwifiex_change_vif_to_sta_adhoc(struct net_device *dev,
return -1;
if (mwifiex_init_new_priv_params(priv, dev, type))
return -1;
+
+ update_vif_type_counter(adapter, curr_iftype, -1);
+ update_vif_type_counter(adapter, type, +1);
+ dev->ieee80211_ptr->iftype = type;
+
if (mwifiex_send_cmd(priv, HostCmd_CMD_SET_BSS_MODE,
HostCmd_ACT_GEN_SET, 0, NULL, true))
return -1;
if (mwifiex_sta_init_cmd(priv, false, false))
return -1;
- update_vif_type_counter(adapter, curr_iftype, -1);
- update_vif_type_counter(adapter, type, +1);
- dev->ieee80211_ptr->iftype = type;
-
return 0;
}
@@ -1152,15 +1153,17 @@ mwifiex_change_vif_to_ap(struct net_device *dev,
return -1;
if (mwifiex_init_new_priv_params(priv, dev, type))
return -1;
+
+ update_vif_type_counter(adapter, curr_iftype, -1);
+ update_vif_type_counter(adapter, type, +1);
+ dev->ieee80211_ptr->iftype = type;
+
if (mwifiex_send_cmd(priv, HostCmd_CMD_SET_BSS_MODE,
HostCmd_ACT_GEN_SET, 0, NULL, true))
return -1;
if (mwifiex_sta_init_cmd(priv, false, false))
return -1;
- update_vif_type_counter(adapter, curr_iftype, -1);
- update_vif_type_counter(adapter, type, +1);
- dev->ieee80211_ptr->iftype = type;
return 0;
}
/*
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/9] mwifiex: Allow switching interface type from P2P_CLIENT to P2P_GO
2021-09-14 19:59 [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Jonas Dreßler
` (4 preceding siblings ...)
2021-09-14 19:59 ` [PATCH 5/9] mwifiex: Update virtual interface counters right after setting bss_type Jonas Dreßler
@ 2021-09-14 19:59 ` Jonas Dreßler
2021-09-14 19:59 ` [PATCH 7/9] mwifiex: Handle interface type changes from AP to STATION Jonas Dreßler
` (3 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jonas Dreßler @ 2021-09-14 19:59 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski
Cc: Jonas Dreßler, Tsuchiya Yuto, linux-wireless, netdev,
linux-kernel, Maximilian Luz, Andy Shevchenko, Pali Rohár
It's possible to change virtual interface type between P2P_CLIENT and
P2P_GO, the card supports that just fine, and it happens for example
when using miracast with the miraclecast software.
So allow type changes between P2P_CLIENT and P2P_GO and simply call into
mwifiex_change_vif_to_p2p(), which handles this just fine. We have to
call mwifiex_cfg80211_deinit_p2p() before though to make sure the old
p2p mode is properly uninitialized.
Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
---
.../net/wireless/marvell/mwifiex/cfg80211.c | 36 +++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index f2797102c5a2..ed4041ff9c89 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -990,11 +990,26 @@ is_vif_type_change_allowed(struct mwifiex_adapter *adapter,
}
case NL80211_IFTYPE_P2P_CLIENT:
+ switch (new_iftype) {
+ case NL80211_IFTYPE_ADHOC:
+ case NL80211_IFTYPE_STATION:
+ return true;
+ case NL80211_IFTYPE_P2P_GO:
+ return true;
+ case NL80211_IFTYPE_AP:
+ return adapter->curr_iface_comb.uap_intf !=
+ adapter->iface_limit.uap_intf;
+ default:
+ return false;
+ }
+
case NL80211_IFTYPE_P2P_GO:
switch (new_iftype) {
case NL80211_IFTYPE_ADHOC:
case NL80211_IFTYPE_STATION:
return true;
+ case NL80211_IFTYPE_P2P_CLIENT:
+ return true;
case NL80211_IFTYPE_AP:
return adapter->curr_iface_comb.uap_intf !=
adapter->iface_limit.uap_intf;
@@ -1265,6 +1280,24 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
}
case NL80211_IFTYPE_P2P_CLIENT:
+ if (mwifiex_cfg80211_deinit_p2p(priv))
+ return -EFAULT;
+
+ switch (type) {
+ case NL80211_IFTYPE_ADHOC:
+ case NL80211_IFTYPE_STATION:
+ return mwifiex_change_vif_to_sta_adhoc(dev, curr_iftype,
+ type, params);
+ case NL80211_IFTYPE_P2P_GO:
+ return mwifiex_change_vif_to_p2p(dev, curr_iftype,
+ type, params);
+ case NL80211_IFTYPE_AP:
+ return mwifiex_change_vif_to_ap(dev, curr_iftype, type,
+ params);
+ default:
+ goto errnotsupp;
+ }
+
case NL80211_IFTYPE_P2P_GO:
if (mwifiex_cfg80211_deinit_p2p(priv))
return -EFAULT;
@@ -1274,6 +1307,9 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
case NL80211_IFTYPE_STATION:
return mwifiex_change_vif_to_sta_adhoc(dev, curr_iftype,
type, params);
+ case NL80211_IFTYPE_P2P_CLIENT:
+ return mwifiex_change_vif_to_p2p(dev, curr_iftype,
+ type, params);
case NL80211_IFTYPE_AP:
return mwifiex_change_vif_to_ap(dev, curr_iftype, type,
params);
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/9] mwifiex: Handle interface type changes from AP to STATION
2021-09-14 19:59 [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Jonas Dreßler
` (5 preceding siblings ...)
2021-09-14 19:59 ` [PATCH 6/9] mwifiex: Allow switching interface type from P2P_CLIENT to P2P_GO Jonas Dreßler
@ 2021-09-14 19:59 ` Jonas Dreßler
2021-09-14 19:59 ` [PATCH 8/9] mwifiex: Properly initialize private structure on interface type changes Jonas Dreßler
` (2 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jonas Dreßler @ 2021-09-14 19:59 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski
Cc: Jonas Dreßler, Tsuchiya Yuto, linux-wireless, netdev,
linux-kernel, Maximilian Luz, Andy Shevchenko, Pali Rohár
Looks like this case was simply overseen, so handle it, too.
Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index ed4041ff9c89..64caa5c4350d 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1268,6 +1268,7 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
case NL80211_IFTYPE_AP:
switch (type) {
case NL80211_IFTYPE_ADHOC:
+ case NL80211_IFTYPE_STATION:
return mwifiex_change_vif_to_sta_adhoc(dev, curr_iftype,
type, params);
break;
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 8/9] mwifiex: Properly initialize private structure on interface type changes
2021-09-14 19:59 [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Jonas Dreßler
` (6 preceding siblings ...)
2021-09-14 19:59 ` [PATCH 7/9] mwifiex: Handle interface type changes from AP to STATION Jonas Dreßler
@ 2021-09-14 19:59 ` Jonas Dreßler
2021-09-14 19:59 ` [PATCH 9/9] mwifiex: Fix copy-paste mistake when creating virtual interface Jonas Dreßler
2021-09-15 18:46 ` [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Pali Rohár
9 siblings, 0 replies; 12+ messages in thread
From: Jonas Dreßler @ 2021-09-14 19:59 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski
Cc: Jonas Dreßler, Tsuchiya Yuto, linux-wireless, netdev,
linux-kernel, Maximilian Luz, Andy Shevchenko, Pali Rohár
When creating a new virtual interface in mwifiex_add_virtual_intf(), we
update our internal driver states like bss_type, bss_priority, bss_role
and bss_mode to reflect the mode the firmware will be set to.
When switching virtual interface mode using
mwifiex_init_new_priv_params() though, we currently only update bss_mode
and bss_role. In order for the interface mode switch to actually work,
we also need to update bss_type to its proper value, so do that.
This fixes a crash of the firmware (because the driver tries to execute
commands that are invalid in AP mode) when switching from station mode
to AP mode.
Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 64caa5c4350d..0eb31201a82b 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -908,16 +908,20 @@ mwifiex_init_new_priv_params(struct mwifiex_private *priv,
switch (type) {
case NL80211_IFTYPE_STATION:
case NL80211_IFTYPE_ADHOC:
- priv->bss_role = MWIFIEX_BSS_ROLE_STA;
+ priv->bss_role = MWIFIEX_BSS_ROLE_STA;
+ priv->bss_type = MWIFIEX_BSS_TYPE_STA;
break;
case NL80211_IFTYPE_P2P_CLIENT:
- priv->bss_role = MWIFIEX_BSS_ROLE_STA;
+ priv->bss_role = MWIFIEX_BSS_ROLE_STA;
+ priv->bss_type = MWIFIEX_BSS_TYPE_P2P;
break;
case NL80211_IFTYPE_P2P_GO:
- priv->bss_role = MWIFIEX_BSS_ROLE_UAP;
+ priv->bss_role = MWIFIEX_BSS_ROLE_UAP;
+ priv->bss_type = MWIFIEX_BSS_TYPE_P2P;
break;
case NL80211_IFTYPE_AP:
priv->bss_role = MWIFIEX_BSS_ROLE_UAP;
+ priv->bss_type = MWIFIEX_BSS_TYPE_UAP;
break;
default:
mwifiex_dbg(adapter, ERROR,
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 9/9] mwifiex: Fix copy-paste mistake when creating virtual interface
2021-09-14 19:59 [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Jonas Dreßler
` (7 preceding siblings ...)
2021-09-14 19:59 ` [PATCH 8/9] mwifiex: Properly initialize private structure on interface type changes Jonas Dreßler
@ 2021-09-14 19:59 ` Jonas Dreßler
2021-09-15 18:46 ` [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Pali Rohár
9 siblings, 0 replies; 12+ messages in thread
From: Jonas Dreßler @ 2021-09-14 19:59 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski
Cc: Jonas Dreßler, Tsuchiya Yuto, linux-wireless, netdev,
linux-kernel, Maximilian Luz, Andy Shevchenko, Pali Rohár
The BSS priority here for a new P2P_CLIENT device was accidentally set
to an enum that's certainly not meant for this. Since
MWIFIEX_BSS_ROLE_STA is 0 anyway, we can just set the bss_priority to 0
instead here.
Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 0eb31201a82b..d62a20de3ada 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -3054,7 +3054,7 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
priv->bss_type = MWIFIEX_BSS_TYPE_P2P;
priv->frame_type = MWIFIEX_DATA_FRAME_TYPE_ETH_II;
- priv->bss_priority = MWIFIEX_BSS_ROLE_STA;
+ priv->bss_priority = 0;
priv->bss_role = MWIFIEX_BSS_ROLE_STA;
priv->bss_started = 0;
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode
2021-09-14 19:59 [PATCH 0/9] mwifiex: Fixes for wifi p2p and AP mode Jonas Dreßler
` (8 preceding siblings ...)
2021-09-14 19:59 ` [PATCH 9/9] mwifiex: Fix copy-paste mistake when creating virtual interface Jonas Dreßler
@ 2021-09-15 18:46 ` Pali Rohár
9 siblings, 0 replies; 12+ messages in thread
From: Pali Rohár @ 2021-09-15 18:46 UTC (permalink / raw)
To: Jonas Dreßler
Cc: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski,
Tsuchiya Yuto, linux-wireless, netdev, linux-kernel,
Maximilian Luz, Andy Shevchenko
On Tuesday 14 September 2021 21:59:00 Jonas Dreßler wrote:
> A bunch of bugfixes for running mwifiex in the P2P and AP mode, for some prior
> discussions, see https://github.com/linux-surface/kernel/pull/71.
Changes look good,
Acked-by: Pali Rohár <pali@kernel.org>
> Jonas Dreßler (9):
> mwifiex: Small cleanup for handling virtual interface type changes
> mwifiex: Use function to check whether interface type change is
> allowed
> mwifiex: Run SET_BSS_MODE when changing from P2P to STATION vif-type
> mwifiex: Use helper function for counting interface types
> mwifiex: Update virtual interface counters right after setting
> bss_type
> mwifiex: Allow switching interface type from P2P_CLIENT to P2P_GO
> mwifiex: Handle interface type changes from AP to STATION
> mwifiex: Properly initialize private structure on interface type
> changes
> mwifiex: Fix copy-paste mistake when creating virtual interface
>
> .../net/wireless/marvell/mwifiex/cfg80211.c | 370 ++++++++++--------
> 1 file changed, 197 insertions(+), 173 deletions(-)
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread