[PATCH] amth10k: fix promisc handling

[PATCH] amth10k: fix promisc handling

[Michal Kazior]

Patch df1404650ccb ("mac80211: remove support for
IFF_PROMISC") removed promiscuous flag propagation
to drivers.

However the patch was designed against ath10k
without 548462133d98 ("ath10k: fix interrupt
storm").

After merge the code drifted into being no longer
correct and due to monitor vdev being
overzealously started caused IBSS to crash on
999.999.0.636 for QCA988X (this firmware revision
is known to have issues with monitor vdev).

This patch keeps expectations of commit
548462133d98 (i.e. reduce irq storm by not
enabling monitor vdev for AP) and doesn't break
existing (known) setups that imply promiscuous
mode on network interfaces.

Contrary to what it looks like 548462133d98
functionality is not reverted since the intention
was a subset of what df1404650ccb did.

Fixes: c17c997d5613 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next")
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 29 +----------------------------
 1 file changed, 1 insertion(+), 28 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 425dbe271495..594eb369ff7f 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1031,22 +1031,6 @@ static int ath10k_monitor_stop(struct ath10k *ar)
 	return 0;
 }
 
-static bool ath10k_mac_should_disable_promisc(struct ath10k *ar)
-{
-	struct ath10k_vif *arvif;
-
-	if (!ar->num_started_vdevs)
-		return false;
-
-	list_for_each_entry(arvif, &ar->arvifs, list)
-		if (arvif->vdev_type != WMI_VDEV_TYPE_AP)
-			return false;
-
-	ath10k_dbg(ar, ATH10K_DBG_MAC,
-		   "mac disabling promiscuous mode because vdev is started\n");
-	return true;
-}
-
 static bool ath10k_mac_monitor_vdev_is_needed(struct ath10k *ar)
 {
 	int num_ctx;
@@ -1065,7 +1049,6 @@ static bool ath10k_mac_monitor_vdev_is_needed(struct ath10k *ar)
 		return false;
 
 	return ar->monitor ||
-	       !ath10k_mac_should_disable_promisc(ar) ||
 	       test_bit(ATH10K_CAC_RUNNING, &ar->dev_flags);
 }
 
@@ -1267,7 +1250,7 @@ static int ath10k_vdev_start_restart(struct ath10k_vif *arvif,
 {
 	struct ath10k *ar = arvif->ar;
 	struct wmi_vdev_start_request_arg arg = {};
-	int ret = 0, ret2;
+	int ret = 0;
 
 	lockdep_assert_held(&ar->conf_mutex);
 
@@ -1326,16 +1309,6 @@ static int ath10k_vdev_start_restart(struct ath10k_vif *arvif,
 	ar->num_started_vdevs++;
 	ath10k_recalc_radar_detection(ar);
 
-	ret = ath10k_monitor_recalc(ar);
-	if (ret) {
-		ath10k_warn(ar, "mac failed to recalc monitor for vdev %i restart %d: %d\n",
-			    arg.vdev_id, restart, ret);
-		ret2 = ath10k_vdev_stop(arvif);
-		if (ret2)
-			ath10k_warn(ar, "mac failed to stop vdev %i restart %d: %d\n",
-				    arg.vdev_id, restart, ret2);
-	}
-
 	return ret;
 }
 
-- 
2.1.4

Re: [PATCH] amth10k: fix promisc handling

[Michal Kazior]

On 12 May 2015 at 14:45, Michal Kazior <michal.kazior@tieto.com> wrote:
> Patch df1404650ccb ("mac80211: remove support for
> IFF_PROMISC") removed promiscuous flag propagation
> to drivers.
>
> However the patch was designed against ath10k
> without 548462133d98 ("ath10k: fix interrupt
> storm").
>
> After merge the code drifted into being no longer
> correct and due to monitor vdev being
> overzealously started caused IBSS to crash on
> 999.999.0.636 for QCA988X (this firmware revision
> is known to have issues with monitor vdev).
>
> This patch keeps expectations of commit
> 548462133d98 (i.e. reduce irq storm by not
> enabling monitor vdev for AP) and doesn't break
> existing (known) setups that imply promiscuous
> mode on network interfaces.
>
> Contrary to what it looks like 548462133d98
> functionality is not reverted since the intention
> was a subset of what df1404650ccb did.
>
> Fixes: c17c997d5613 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next")
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Apparently this also fixes some weird issues with qca6174 hw2.1 notably:
 - ath10k causing disconnecting of other devices in a BSS
 - random Fw crashes

Both problems started to happen because c17c997d5613 enabled monitor
vdev by default on STA interfaces. It seems that qca6174 hw2.1
firmware has issues similar to those of qca988x 999.999.0.636
regarding monitor vdev opration.

Also, I've made a typo in the subject.

I'll post v2 with subject fixed and extended commit log later.


Michał

Re: [PATCH] amth10k: fix promisc handling

[Kalle Valo]

Adding John as this involved wireless-testing

Michal Kazior <michal.kazior@tieto.com> writes:

> On 12 May 2015 at 14:45, Michal Kazior <michal.kazior@tieto.com> wrote:
>> Patch df1404650ccb ("mac80211: remove support for
>> IFF_PROMISC") removed promiscuous flag propagation
>> to drivers.
>>
>> However the patch was designed against ath10k
>> without 548462133d98 ("ath10k: fix interrupt
>> storm").
>>
>> After merge the code drifted into being no longer
>> correct and due to monitor vdev being
>> overzealously started caused IBSS to crash on
>> 999.999.0.636 for QCA988X (this firmware revision
>> is known to have issues with monitor vdev).
>>
>> This patch keeps expectations of commit
>> 548462133d98 (i.e. reduce irq storm by not
>> enabling monitor vdev for AP) and doesn't break
>> existing (known) setups that imply promiscuous
>> mode on network interfaces.
>>
>> Contrary to what it looks like 548462133d98
>> functionality is not reverted since the intention
>> was a subset of what df1404650ccb did.
>>
>> Fixes: c17c997d5613 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next")
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> Apparently this also fixes some weird issues with qca6174 hw2.1 notably:
>  - ath10k causing disconnecting of other devices in a BSS
>  - random Fw crashes
>
> Both problems started to happen because c17c997d5613 enabled monitor
> vdev by default on STA interfaces. It seems that qca6174 hw2.1
> firmware has issues similar to those of qca988x 999.999.0.636
> regarding monitor vdev opration.
>
> Also, I've made a typo in the subject.
>
> I'll post v2 with subject fixed and extended commit log later.

Keep in mind that c17c997d5613 is actually from wireless-testing.git
which means that it will never go to wireless-drivers-next.git nor to
net-next.git. So the merge conflict bug is purely in
wireless-testing.git and in master branch of ath.git (but not in
ath-next branch!).

I think John should apply your v2 patch once you send it. But if you
have something which should be fixed in ath-next remember to send that
in a separate patch so that I can apply that directly to ath-next.

-- 
Kalle Valo

Re: [PATCH] amth10k: fix promisc handling

[Kalle Valo]

Kalle Valo <kvalo@qca.qualcomm.com> writes:

> Adding John as this involved wireless-testing
>
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> On 12 May 2015 at 14:45, Michal Kazior <michal.kazior@tieto.com> wrote:
>>> Patch df1404650ccb ("mac80211: remove support for
>>> IFF_PROMISC") removed promiscuous flag propagation
>>> to drivers.
>>>
>>> However the patch was designed against ath10k
>>> without 548462133d98 ("ath10k: fix interrupt
>>> storm").
>>>
>>> After merge the code drifted into being no longer
>>> correct and due to monitor vdev being
>>> overzealously started caused IBSS to crash on
>>> 999.999.0.636 for QCA988X (this firmware revision
>>> is known to have issues with monitor vdev).
>>>
>>> This patch keeps expectations of commit
>>> 548462133d98 (i.e. reduce irq storm by not
>>> enabling monitor vdev for AP) and doesn't break
>>> existing (known) setups that imply promiscuous
>>> mode on network interfaces.
>>>
>>> Contrary to what it looks like 548462133d98
>>> functionality is not reverted since the intention
>>> was a subset of what df1404650ccb did.
>>>
>>> Fixes: c17c997d5613 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next")
>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>
>> Apparently this also fixes some weird issues with qca6174 hw2.1 notably:
>>  - ath10k causing disconnecting of other devices in a BSS
>>  - random Fw crashes
>>
>> Both problems started to happen because c17c997d5613 enabled monitor
>> vdev by default on STA interfaces. It seems that qca6174 hw2.1
>> firmware has issues similar to those of qca988x 999.999.0.636
>> regarding monitor vdev opration.
>>
>> Also, I've made a typo in the subject.
>>
>> I'll post v2 with subject fixed and extended commit log later.
>
> Keep in mind that c17c997d5613 is actually from wireless-testing.git
> which means that it will never go to wireless-drivers-next.git nor to
> net-next.git. So the merge conflict bug is purely in
> wireless-testing.git and in master branch of ath.git (but not in
> ath-next branch!).
>
> I think John should apply your v2 patch once you send it. But if you
> have something which should be fixed in ath-next remember to send that
> in a separate patch so that I can apply that directly to ath-next.

Actually now that Dave pulled my pull request the issue is fixed in
wireless-drivers-next already. So once John pulls from
wireless-drivers-next and makes sure that ath10k is 100% identical in
both trees the issue should be sorted out and no need for extra patches.

-- 
Kalle Valo

Re: [PATCH] amth10k: fix promisc handling

[Kalle Valo]

Kalle Valo <kvalo@qca.qualcomm.com> writes:

>>>> Fixes: c17c997d5613 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next")
>>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>>
>>> Apparently this also fixes some weird issues with qca6174 hw2.1 notably:
>>>  - ath10k causing disconnecting of other devices in a BSS
>>>  - random Fw crashes
>>>
>>> Both problems started to happen because c17c997d5613 enabled monitor
>>> vdev by default on STA interfaces. It seems that qca6174 hw2.1
>>> firmware has issues similar to those of qca988x 999.999.0.636
>>> regarding monitor vdev opration.
>>>
>>> Also, I've made a typo in the subject.
>>>
>>> I'll post v2 with subject fixed and extended commit log later.
>>
>> Keep in mind that c17c997d5613 is actually from wireless-testing.git
>> which means that it will never go to wireless-drivers-next.git nor to
>> net-next.git. So the merge conflict bug is purely in
>> wireless-testing.git and in master branch of ath.git (but not in
>> ath-next branch!).
>>
>> I think John should apply your v2 patch once you send it. But if you
>> have something which should be fixed in ath-next remember to send that
>> in a separate patch so that I can apply that directly to ath-next.
>
> Actually now that Dave pulled my pull request the issue is fixed in
> wireless-drivers-next already. So once John pulls from
> wireless-drivers-next and makes sure that ath10k is 100% identical in
> both trees the issue should be sorted out and no need for extra patches.

John now fixed this in wireless-testing, thanks John. And I now updated
ath.git master branch so it should be ok as well. Please let me know if
there are still problems.
-- 
Kalle Valo