2020-04-25 09:29:05

by Christian Lamparter

[permalink] [raw]
Subject: [PATCH] carl9170: remove P2P_GO support

This patch follows up on a bug-report by Frank Schäfer that
discovered P2P GO wasn't working with wpa_supplicant.
This patch removes part of the broken P2P GO support but
keeps the vif switchover code in place.

Cc: <[email protected]>
Link: <https://lkml.kernel.org/r/[email protected]>
Reported-by: Frank Schäfer <[email protected]>
Signed-off-by: Christian Lamparter <[email protected]>
---
drivers/net/wireless/ath/carl9170/fw.c | 4 +---
drivers/net/wireless/ath/carl9170/main.c | 21 ++++-----------------
2 files changed, 5 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/fw.c b/drivers/net/wireless/ath/carl9170/fw.c
index 51934d191f33..1ab09e1c9ec5 100644
--- a/drivers/net/wireless/ath/carl9170/fw.c
+++ b/drivers/net/wireless/ath/carl9170/fw.c
@@ -338,9 +338,7 @@ static int carl9170_fw(struct ar9170 *ar, const __u8 *data, size_t len)
ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_ADHOC);

if (SUPP(CARL9170FW_WLANTX_CAB)) {
- if_comb_types |=
- BIT(NL80211_IFTYPE_AP) |
- BIT(NL80211_IFTYPE_P2P_GO);
+ if_comb_types |= BIT(NL80211_IFTYPE_AP);

#ifdef CONFIG_MAC80211_MESH
if_comb_types |=
diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
index 5914926a5c5b..816929fb5b14 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -582,11 +582,10 @@ static int carl9170_init_interface(struct ar9170 *ar,
ar->disable_offload |= ((vif->type != NL80211_IFTYPE_STATION) &&
(vif->type != NL80211_IFTYPE_AP));

- /* While the driver supports HW offload in a single
- * P2P client configuration, it doesn't support HW
- * offload in the favourit, concurrent P2P GO+CLIENT
- * configuration. Hence, HW offload will always be
- * disabled for P2P.
+ /* The driver used to have P2P GO+CLIENT support,
+ * but since this was dropped and we don't know if
+ * there are any gremlins lurking in the shadows,
+ * so best we keep HW offload disabled for P2P.
*/
ar->disable_offload |= vif->p2p;

@@ -639,18 +638,6 @@ static int carl9170_op_add_interface(struct ieee80211_hw *hw,
if (vif->type == NL80211_IFTYPE_STATION)
break;

- /* P2P GO [master] use-case
- * Because the P2P GO station is selected dynamically
- * by all participating peers of a WIFI Direct network,
- * the driver has be able to change the main interface
- * operating mode on the fly.
- */
- if (main_vif->p2p && vif->p2p &&
- vif->type == NL80211_IFTYPE_AP) {
- old_main = main_vif;
- break;
- }
-
err = -EBUSY;
rcu_read_unlock();

--
2.26.2


2020-04-26 10:33:38

by Frank Schäfer

[permalink] [raw]
Subject: Re: [PATCH] carl9170: remove P2P_GO support

Hi Christian,

Am 25.04.20 um 11:28 schrieb Christian Lamparter:
> This patch follows up on a bug-report by Frank Schäfer that
> discovered P2P GO wasn't working with wpa_supplicant.
> This patch removes part of the broken P2P GO support but
> keeps the vif switchover code in place.
Hmm... no way to fix it ?
P2P-GO seems to work fine with p2p_no_group_iface=1, so do you really
think it's a good idea / required to remove the whole thing ?

> Cc: <[email protected]>
Are you sure about that ?
People might be using it...

Regards,
Frank

> Link: <https://lkml.kernel.org/r/[email protected]>
> Reported-by: Frank Schäfer <[email protected]>
> Signed-off-by: Christian Lamparter <[email protected]>
> ---
> drivers/net/wireless/ath/carl9170/fw.c | 4 +---
> drivers/net/wireless/ath/carl9170/main.c | 21 ++++-----------------
> 2 files changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/carl9170/fw.c b/drivers/net/wireless/ath/carl9170/fw.c
> index 51934d191f33..1ab09e1c9ec5 100644
> --- a/drivers/net/wireless/ath/carl9170/fw.c
> +++ b/drivers/net/wireless/ath/carl9170/fw.c
> @@ -338,9 +338,7 @@ static int carl9170_fw(struct ar9170 *ar, const __u8 *data, size_t len)
> ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_ADHOC);
>
> if (SUPP(CARL9170FW_WLANTX_CAB)) {
> - if_comb_types |=
> - BIT(NL80211_IFTYPE_AP) |
> - BIT(NL80211_IFTYPE_P2P_GO);
> + if_comb_types |= BIT(NL80211_IFTYPE_AP);
>
> #ifdef CONFIG_MAC80211_MESH
> if_comb_types |=
> diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
> index 5914926a5c5b..816929fb5b14 100644
> --- a/drivers/net/wireless/ath/carl9170/main.c
> +++ b/drivers/net/wireless/ath/carl9170/main.c
> @@ -582,11 +582,10 @@ static int carl9170_init_interface(struct ar9170 *ar,
> ar->disable_offload |= ((vif->type != NL80211_IFTYPE_STATION) &&
> (vif->type != NL80211_IFTYPE_AP));
>
> - /* While the driver supports HW offload in a single
> - * P2P client configuration, it doesn't support HW
> - * offload in the favourit, concurrent P2P GO+CLIENT
> - * configuration. Hence, HW offload will always be
> - * disabled for P2P.
> + /* The driver used to have P2P GO+CLIENT support,
> + * but since this was dropped and we don't know if
> + * there are any gremlins lurking in the shadows,
> + * so best we keep HW offload disabled for P2P.
> */
> ar->disable_offload |= vif->p2p;
>
> @@ -639,18 +638,6 @@ static int carl9170_op_add_interface(struct ieee80211_hw *hw,
> if (vif->type == NL80211_IFTYPE_STATION)
> break;
>
> - /* P2P GO [master] use-case
> - * Because the P2P GO station is selected dynamically
> - * by all participating peers of a WIFI Direct network,
> - * the driver has be able to change the main interface
> - * operating mode on the fly.
> - */
> - if (main_vif->p2p && vif->p2p &&
> - vif->type == NL80211_IFTYPE_AP) {
> - old_main = main_vif;
> - break;
> - }
> -
> err = -EBUSY;
> rcu_read_unlock();

2020-05-05 07:23:24

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] carl9170: remove P2P_GO support

Frank Schäfer <[email protected]> writes:

> Hi Christian,
>
> Am 25.04.20 um 11:28 schrieb Christian Lamparter:
>> This patch follows up on a bug-report by Frank Schäfer that
>> discovered P2P GO wasn't working with wpa_supplicant.
>> This patch removes part of the broken P2P GO support but
>> keeps the vif switchover code in place.
>
> Hmm... no way to fix it ?
> P2P-GO seems to work fine with p2p_no_group_iface=1, so do you really
> think it's a good idea / required to remove the whole thing ?
>
>> Cc: <[email protected]>
>
> Are you sure about that ?
> People might be using it...

Christian, what should I do? Take the patch or drop it?

--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2020-05-05 18:32:11

by Christian Lamparter

[permalink] [raw]
Subject: Re: [PATCH] carl9170: remove P2P_GO support

Hello,

On Tuesday, 5 May 2020 09:20:20 CEST Kalle Valo wrote:
> Frank Sch?fer <[email protected]> writes:
> >
> > Am 25.04.20 um 11:28 schrieb Christian Lamparter:
> >> This patch follows up on a bug-report by Frank Sch?fer that
> >> discovered P2P GO wasn't working with wpa_supplicant.
> >> This patch removes part of the broken P2P GO support but
> >> keeps the vif switchover code in place.
> >
> > Hmm... no way to fix it ?
> > P2P-GO seems to work fine with p2p_no_group_iface=1, so do you really
> > think it's a good idea / required to remove the whole thing ?
> >
> >> Cc: <[email protected]>
> >
> > Are you sure about that ?
> > People might be using it...
>
> Christian, what should I do? Take the patch or drop it?

Well, the way I see it: AR9170's silicon die has been cast before
P2P was a thing. And while it was nice back in '10 then to had have
something to test ath9k with, by todays standards the lack of features
like separate TSF sync and the cacheless BA makes it so that it's holding
back the performance and connection quality of all the clients which are
connected to it.

So, I think we'll just kicking down the can on that and I'm sorry that
I gave the people the wrong impression. There are much better chips now
that came with P2P in mind.

So: please take the patch.

Cheers
Christian



2020-05-06 06:05:51

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] carl9170: remove P2P_GO support

Christian Lamparter <[email protected]> writes:

> On Tuesday, 5 May 2020 09:20:20 CEST Kalle Valo wrote:
>> Frank Schäfer <[email protected]> writes:
>> >
>> > Am 25.04.20 um 11:28 schrieb Christian Lamparter:
>> >> This patch follows up on a bug-report by Frank Schäfer that
>> >> discovered P2P GO wasn't working with wpa_supplicant.
>> >> This patch removes part of the broken P2P GO support but
>> >> keeps the vif switchover code in place.
>> >
>> > Hmm... no way to fix it ?
>> > P2P-GO seems to work fine with p2p_no_group_iface=1, so do you really
>> > think it's a good idea / required to remove the whole thing ?
>> >
>> >> Cc: <[email protected]>
>> >
>> > Are you sure about that ?
>> > People might be using it...
>>
>> Christian, what should I do? Take the patch or drop it?
>
> Well, the way I see it: AR9170's silicon die has been cast before
> P2P was a thing. And while it was nice back in '10 then to had have
> something to test ath9k with, by todays standards the lack of features
> like separate TSF sync and the cacheless BA makes it so that it's holding
> back the performance and connection quality of all the clients which are
> connected to it.
>
> So, I think we'll just kicking down the can on that and I'm sorry that
> I gave the people the wrong impression. There are much better chips now
> that came with P2P in mind.
>
> So: please take the patch.

Sounds good to me, thanks. I'll take the patch.

--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2020-05-06 06:15:54

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] carl9170: remove P2P_GO support

Christian Lamparter <[email protected]> wrote:

> This patch follows up on a bug-report by Frank Schäfer that
> discovered P2P GO wasn't working with wpa_supplicant.
> This patch removes part of the broken P2P GO support but
> keeps the vif switchover code in place.
>
> Cc: <[email protected]>
> Link: <https://lkml.kernel.org/r/[email protected]>
> Reported-by: Frank Schäfer <[email protected]>
> Signed-off-by: Christian Lamparter <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

b14fba7ebd04 carl9170: remove P2P_GO support

--
https://patchwork.kernel.org/patch/11509803/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches