[PATCH 2.6.38] ath9k: remove support for the FIF_PROMISC_IN_BSS filter flag

[PATCH 2.6.38] ath9k: remove support for the FIF_PROMISC_IN_BSS filter flag

[Felix Fietkau]

The hardware rx filter flag triggered by FIF_PROMISC_IN_BSS is overly broad
and covers even frames with PHY errors. When this flag is enabled, this message
shows up frequently during scanning or hardware resets:

ath: Could not stop RX, we could be confusing the DMA engine when we start RX up

Since promiscuous mode is usually not particularly useful, yet enabled by
default by bridging (either used normally in 4-addr mode, or with hacks
for various virtualization software), we should sacrifice it for better
reliability during normal operation.

This patch leaves it enabled if there are active monitor mode interfaces, since
it's very useful for debugging.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Cc: stable@kernel.org
---
 drivers/net/wireless/ath/ath9k/recv.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index cb559e3..a9c3f46 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -413,9 +413,7 @@ u32 ath_calcrxfilter(struct ath_softc *sc)
 	 * mode interface or when in monitor mode. AP mode does not need this
 	 * since it receives all in-BSS frames anyway.
 	 */
-	if (((sc->sc_ah->opmode != NL80211_IFTYPE_AP) &&
-	     (sc->rx.rxfilter & FIF_PROMISC_IN_BSS)) ||
-	    (sc->sc_ah->is_monitoring))
+	if (sc->sc_ah->is_monitoring)
 		rfilt |= ATH9K_RX_FILTER_PROM;
 
 	if (sc->rx.rxfilter & FIF_CONTROL)
-- 
1.7.3.2

Re: [PATCH 2.6.38] ath9k: remove support for the FIF_PROMISC_IN_BSS filter flag

[Ben Greear]

On 03/08/2011 04:48 PM, Felix Fietkau wrote:
> The hardware rx filter flag triggered by FIF_PROMISC_IN_BSS is overly broad
> and covers even frames with PHY errors. When this flag is enabled, this message
> shows up frequently during scanning or hardware resets:
>
> ath: Could not stop RX, we could be confusing the DMA engine when we start RX up
>
> Since promiscuous mode is usually not particularly useful, yet enabled by
> default by bridging (either used normally in 4-addr mode, or with hacks
> for various virtualization software), we should sacrifice it for better
> reliability during normal operation.
>
> This patch leaves it enabled if there are active monitor mode interfaces, since
> it's very useful for debugging.
>
> Signed-off-by: Felix Fietkau<nbd@openwrt.org>
> Cc: stable@kernel.org
> ---
>   drivers/net/wireless/ath/ath9k/recv.c |    4 +---
>   1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> index cb559e3..a9c3f46 100644
> --- a/drivers/net/wireless/ath/ath9k/recv.c
> +++ b/drivers/net/wireless/ath/ath9k/recv.c
> @@ -413,9 +413,7 @@ u32 ath_calcrxfilter(struct ath_softc *sc)
>   	 * mode interface or when in monitor mode. AP mode does not need this
>   	 * since it receives all in-BSS frames anyway.
>   	 */
> -	if (((sc->sc_ah->opmode != NL80211_IFTYPE_AP)&&
> -	     (sc->rx.rxfilter&  FIF_PROMISC_IN_BSS)) ||
> -	    (sc->sc_ah->is_monitoring))
> +	if (sc->sc_ah->is_monitoring)
>   		rfilt |= ATH9K_RX_FILTER_PROM;
>
>   	if (sc->rx.rxfilter&  FIF_CONTROL)

Should we enable this flag if we have multiple STA
interfaces?  I had to add something to ath5k recently
to put it into promisc to properly handle multiple STAs
associated with different APs, for instance.

Do you have any idea *why* enabling this flag causes the DMA error
messages?

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 2.6.38] ath9k: remove support for the FIF_PROMISC_IN_BSS filter flag

[Felix Fietkau]

On 2011-03-09 2:07 AM, Ben Greear wrote:
> On 03/08/2011 04:48 PM, Felix Fietkau wrote:
>> The hardware rx filter flag triggered by FIF_PROMISC_IN_BSS is overly broad
>> and covers even frames with PHY errors. When this flag is enabled, this message
>> shows up frequently during scanning or hardware resets:
>>
>> ath: Could not stop RX, we could be confusing the DMA engine when we start RX up
>>
>> Since promiscuous mode is usually not particularly useful, yet enabled by
>> default by bridging (either used normally in 4-addr mode, or with hacks
>> for various virtualization software), we should sacrifice it for better
>> reliability during normal operation.
>>
>> This patch leaves it enabled if there are active monitor mode interfaces, since
>> it's very useful for debugging.
>>
>> Signed-off-by: Felix Fietkau<nbd@openwrt.org>
>> Cc: stable@kernel.org
>> ---
>>   drivers/net/wireless/ath/ath9k/recv.c |    4 +---
>>   1 files changed, 1 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
>> index cb559e3..a9c3f46 100644
>> --- a/drivers/net/wireless/ath/ath9k/recv.c
>> +++ b/drivers/net/wireless/ath/ath9k/recv.c
>> @@ -413,9 +413,7 @@ u32 ath_calcrxfilter(struct ath_softc *sc)
>>   	 * mode interface or when in monitor mode. AP mode does not need this
>>   	 * since it receives all in-BSS frames anyway.
>>   	 */
>> -	if (((sc->sc_ah->opmode != NL80211_IFTYPE_AP)&&
>> -	     (sc->rx.rxfilter&  FIF_PROMISC_IN_BSS)) ||
>> -	    (sc->sc_ah->is_monitoring))
>> +	if (sc->sc_ah->is_monitoring)
>>   		rfilt |= ATH9K_RX_FILTER_PROM;
>>
>>   	if (sc->rx.rxfilter&  FIF_CONTROL)
> 
> Should we enable this flag if we have multiple STA
> interfaces?  I had to add something to ath5k recently
> to put it into promisc to properly handle multiple STAs
> associated with different APs, for instance.
No, multiple interfaces is handled by the BSSID mask.

> Do you have any idea *why* enabling this flag causes the DMA error
> messages?
I don't know why exactly it happens, but it probably cannot be explained
without going to the details of the inner workings of the MAC/baseband
interaction. But as I mentioned in the description, this flag is overly
broad and doesn't just bypass the address match, but lets all kinds of
other crap through as well - in many situations where even 'normal'
promiscuous behavior would be completely useless.
I think because of that, disabling it for non-monitor operation is the
right thing to do.

- Felix

Re: [PATCH 2.6.38] ath9k: remove support for the FIF_PROMISC_IN_BSS filter flag

[Luis R. Rodriguez]

On Tue, Mar 08, 2011 at 05:15:41PM -0800, Felix Fietkau wrote:
> On 2011-03-09 2:07 AM, Ben Greear wrote:
> > On 03/08/2011 04:48 PM, Felix Fietkau wrote:
> >> The hardware rx filter flag triggered by FIF_PROMISC_IN_BSS is overly broad
> >> and covers even frames with PHY errors. When this flag is enabled, this message
> >> shows up frequently during scanning or hardware resets:
> >>
> >> ath: Could not stop RX, we could be confusing the DMA engine when we start RX up
> >>
> >> Since promiscuous mode is usually not particularly useful, yet enabled by
> >> default by bridging (either used normally in 4-addr mode, or with hacks
> >> for various virtualization software), we should sacrifice it for better
> >> reliability during normal operation.
> >>
> >> This patch leaves it enabled if there are active monitor mode interfaces, since
> >> it's very useful for debugging.
> >>
> >> Signed-off-by: Felix Fietkau<nbd@openwrt.org>
> >> Cc: stable@kernel.org
> >> ---
> >>   drivers/net/wireless/ath/ath9k/recv.c |    4 +---
> >>   1 files changed, 1 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> >> index cb559e3..a9c3f46 100644
> >> --- a/drivers/net/wireless/ath/ath9k/recv.c
> >> +++ b/drivers/net/wireless/ath/ath9k/recv.c
> >> @@ -413,9 +413,7 @@ u32 ath_calcrxfilter(struct ath_softc *sc)
> >>   	 * mode interface or when in monitor mode. AP mode does not need this
> >>   	 * since it receives all in-BSS frames anyway.
> >>   	 */
> >> -	if (((sc->sc_ah->opmode != NL80211_IFTYPE_AP)&&
> >> -	     (sc->rx.rxfilter&  FIF_PROMISC_IN_BSS)) ||
> >> -	    (sc->sc_ah->is_monitoring))
> >> +	if (sc->sc_ah->is_monitoring)
> >>   		rfilt |= ATH9K_RX_FILTER_PROM;
> >>
> >>   	if (sc->rx.rxfilter&  FIF_CONTROL)
> > 
> > Should we enable this flag if we have multiple STA
> > interfaces?  I had to add something to ath5k recently
> > to put it into promisc to properly handle multiple STAs
> > associated with different APs, for instance.
> No, multiple interfaces is handled by the BSSID mask.
> 
> > Do you have any idea *why* enabling this flag causes the DMA error
> > messages?
> I don't know why exactly it happens, but it probably cannot be explained
> without going to the details of the inner workings of the MAC/baseband
> interaction. But as I mentioned in the description, this flag is overly
> broad and doesn't just bypass the address match, but lets all kinds of
> other crap through as well - in many situations where even 'normal'
> promiscuous behavior would be completely useless.
> I think because of that, disabling it for non-monitor operation is the
> right thing to do.

Nice find, thanks for looking at this. Are we cured now from all of
these rants? Or has anyone seem more?

  Luis

Re: [PATCH 2.6.38] ath9k: remove support for the FIF_PROMISC_IN_BSS filter flag

[Felix Fietkau]

On 2011-03-15 9:51 PM, Luis R. Rodriguez wrote:
> On Tue, Mar 08, 2011 at 05:15:41PM -0800, Felix Fietkau wrote:
>> On 2011-03-09 2:07 AM, Ben Greear wrote:
>> > On 03/08/2011 04:48 PM, Felix Fietkau wrote:
>> >> The hardware rx filter flag triggered by FIF_PROMISC_IN_BSS is overly broad
>> >> and covers even frames with PHY errors. When this flag is enabled, this message
>> >> shows up frequently during scanning or hardware resets:
>> >>
>> >> ath: Could not stop RX, we could be confusing the DMA engine when we start RX up
>> >>
>> >> Since promiscuous mode is usually not particularly useful, yet enabled by
>> >> default by bridging (either used normally in 4-addr mode, or with hacks
>> >> for various virtualization software), we should sacrifice it for better
>> >> reliability during normal operation.
>> >>
>> >> This patch leaves it enabled if there are active monitor mode interfaces, since
>> >> it's very useful for debugging.
>> >>
>> >> Signed-off-by: Felix Fietkau<nbd@openwrt.org>
>> >> Cc: stable@kernel.org
>> >> ---
>> >>   drivers/net/wireless/ath/ath9k/recv.c |    4 +---
>> >>   1 files changed, 1 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
>> >> index cb559e3..a9c3f46 100644
>> >> --- a/drivers/net/wireless/ath/ath9k/recv.c
>> >> +++ b/drivers/net/wireless/ath/ath9k/recv.c
>> >> @@ -413,9 +413,7 @@ u32 ath_calcrxfilter(struct ath_softc *sc)
>> >>   	 * mode interface or when in monitor mode. AP mode does not need this
>> >>   	 * since it receives all in-BSS frames anyway.
>> >>   	 */
>> >> -	if (((sc->sc_ah->opmode != NL80211_IFTYPE_AP)&&
>> >> -	     (sc->rx.rxfilter&  FIF_PROMISC_IN_BSS)) ||
>> >> -	    (sc->sc_ah->is_monitoring))
>> >> +	if (sc->sc_ah->is_monitoring)
>> >>   		rfilt |= ATH9K_RX_FILTER_PROM;
>> >>
>> >>   	if (sc->rx.rxfilter&  FIF_CONTROL)
>> > 
>> > Should we enable this flag if we have multiple STA
>> > interfaces?  I had to add something to ath5k recently
>> > to put it into promisc to properly handle multiple STAs
>> > associated with different APs, for instance.
>> No, multiple interfaces is handled by the BSSID mask.
>> 
>> > Do you have any idea *why* enabling this flag causes the DMA error
>> > messages?
>> I don't know why exactly it happens, but it probably cannot be explained
>> without going to the details of the inner workings of the MAC/baseband
>> interaction. But as I mentioned in the description, this flag is overly
>> broad and doesn't just bypass the address match, but lets all kinds of
>> other crap through as well - in many situations where even 'normal'
>> promiscuous behavior would be completely useless.
>> I think because of that, disabling it for non-monitor operation is the
>> right thing to do.
> 
> Nice find, thanks for looking at this. Are we cured now from all of
> these rants? Or has anyone seem more?
There is definitely more. While this does reduce the occurences of the
DMA RX failure a lot, it neither fixes the root cause of this issue, nor
handles the case where a monitor mode interface is configured.
To fix the issue properly, we need more input from the hw guys.

- Felix

Re: [PATCH 2.6.38] ath9k: remove support for the FIF_PROMISC_IN_BSS filter flag

[Rajkumar Manoharan]

On Wed, Mar 09, 2011 at 06:18:12AM +0530, Felix Fietkau wrote:
> The hardware rx filter flag triggered by FIF_PROMISC_IN_BSS is overly broad
> and covers even frames with PHY errors. When this flag is enabled, this message
> shows up frequently during scanning or hardware resets:
> 
> ath: Could not stop RX, we could be confusing the DMA engine when we start RX up
> 
> Since promiscuous mode is usually not particularly useful, yet enabled by
> default by bridging (either used normally in 4-addr mode, or with hacks
> for various virtualization software), we should sacrifice it for better
> reliability during normal operation.
> 
> This patch leaves it enabled if there are active monitor mode interfaces, since
> it's very useful for debugging.
> 
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> Cc: stable@kernel.org
> ---
>  drivers/net/wireless/ath/ath9k/recv.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> index cb559e3..a9c3f46 100644
> --- a/drivers/net/wireless/ath/ath9k/recv.c
> +++ b/drivers/net/wireless/ath/ath9k/recv.c
> @@ -413,9 +413,7 @@ u32 ath_calcrxfilter(struct ath_softc *sc)
>  	 * mode interface or when in monitor mode. AP mode does not need this
>  	 * since it receives all in-BSS frames anyway.
>  	 */
> -	if (((sc->sc_ah->opmode != NL80211_IFTYPE_AP) &&
> -	     (sc->rx.rxfilter & FIF_PROMISC_IN_BSS)) ||
> -	    (sc->sc_ah->is_monitoring))
> +	if (sc->sc_ah->is_monitoring)
>  		rfilt |= ATH9K_RX_FILTER_PROM;

Assume there are 2 vifs (STA + MON) and both running. Doing a scan on STA vif, dumps dma messages.

--
Rajkumar

Re: [PATCH 2.6.38] ath9k: remove support for the FIF_PROMISC_IN_BSS filter flag

[Ben Greear]

On 03/08/2011 06:56 PM, Rajkumar Manoharan wrote:
> On Wed, Mar 09, 2011 at 06:18:12AM +0530, Felix Fietkau wrote:
>> The hardware rx filter flag triggered by FIF_PROMISC_IN_BSS is overly broad
>> and covers even frames with PHY errors. When this flag is enabled, this message
>> shows up frequently during scanning or hardware resets:
>>
>> ath: Could not stop RX, we could be confusing the DMA engine when we start RX up
>>
>> Since promiscuous mode is usually not particularly useful, yet enabled by
>> default by bridging (either used normally in 4-addr mode, or with hacks
>> for various virtualization software), we should sacrifice it for better
>> reliability during normal operation.
>>
>> This patch leaves it enabled if there are active monitor mode interfaces, since
>> it's very useful for debugging.
>>
>> Signed-off-by: Felix Fietkau<nbd@openwrt.org>
>> Cc: stable@kernel.org
>> ---
>>   drivers/net/wireless/ath/ath9k/recv.c |    4 +---
>>   1 files changed, 1 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
>> index cb559e3..a9c3f46 100644
>> --- a/drivers/net/wireless/ath/ath9k/recv.c
>> +++ b/drivers/net/wireless/ath/ath9k/recv.c
>> @@ -413,9 +413,7 @@ u32 ath_calcrxfilter(struct ath_softc *sc)
>>   	 * mode interface or when in monitor mode. AP mode does not need this
>>   	 * since it receives all in-BSS frames anyway.
>>   	 */
>> -	if (((sc->sc_ah->opmode != NL80211_IFTYPE_AP)&&
>> -	     (sc->rx.rxfilter&  FIF_PROMISC_IN_BSS)) ||
>> -	    (sc->sc_ah->is_monitoring))
>> +	if (sc->sc_ah->is_monitoring)
>>   		rfilt |= ATH9K_RX_FILTER_PROM;
>
> Assume there are 2 vifs (STA + MON) and both running. Doing a scan on STA vif, dumps dma messages.

Sounds like you got a reproducible test case then!

This patch may be fine, but either way, it is at best making it harder to
hit the DMA issue, not actually fixing the underlying cause.

Thanks,
Ben

>
> --
> Rajkumar
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com