[PATCH] carl9170: remove P2P_GO support

[PATCH] carl9170: remove P2P_GO support

[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.

Cc: <stable@vger.kernel.org>
Link: <https://lkml.kernel.org/r/3a9d86b6-744f-e670-8792-9167257edef8@googlemail.com>
Reported-by: Frank Schäfer <fschaefer.oss@googlemail.com>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
 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

Re: [PATCH] carl9170: remove P2P_GO support

[Frank Schäfer]

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: <stable@vger.kernel.org>
Are you sure about that ?
People might be using it...

Regards,
Frank

> Link: <https://lkml.kernel.org/r/3a9d86b6-744f-e670-8792-9167257edef8@googlemail.com>
> Reported-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
>   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();

Re: [PATCH] carl9170: remove P2P_GO support

[Kalle Valo]

Frank Schäfer <fschaefer.oss@googlemail.com> 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: <stable@vger.kernel.org>
>
> 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

Re: [PATCH] carl9170: remove P2P_GO support

[Christian Lamparter]

Hello,

On Tuesday, 5 May 2020 09:20:20 CEST Kalle Valo wrote:
> Frank Schäfer <fschaefer.oss@googlemail.com> 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: <stable@vger.kernel.org>
> >
> > 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

Re: [PATCH] carl9170: remove P2P_GO support

[Kalle Valo]

Christian Lamparter <chunkeey@gmail.com> writes:

> On Tuesday, 5 May 2020 09:20:20 CEST Kalle Valo wrote:
>> Frank Schäfer <fschaefer.oss@googlemail.com> 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: <stable@vger.kernel.org>
>> >
>> > 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

Re: [PATCH] carl9170: remove P2P_GO support

[Kalle Valo]

Christian Lamparter <chunkeey@gmail.com> 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: <stable@vger.kernel.org>
> Link: <https://lkml.kernel.org/r/3a9d86b6-744f-e670-8792-9167257edef8@googlemail.com>
> Reported-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

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