Re: [ath9k-devel] [PATCH] ath9k: ignore radar PHY errors when DFS is not enabled

2015-01-15 Thread Kalle Valo
Simon Wunderlich s...@simonwunderlich.de writes:

 I did not dig how the hw-conf.radar_enabled flag is set in monitor mode,
 but if it is same as for master (i.e. set for DFS channels), then it would
 be a better approach to prevent calling ath9k_dfs_process_phyerr()
 altogether from ath9k_rx_skb_preprocess() if not set.

 Hm, you mean like - if radar_enabled then dfs_process, otherwise fft_process? 
 That would might be more elegant indeed ...

 The monitor mode does not have the radar flag enabled, 
 cfg80211_chandef_dfs_required() returns 0 in this case.

So are you going to send v2 or what's the plan? I didn't quite get the
conclusion from the discussion.

-- 
Kalle Valo
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] [PATCH] ath9k: ignore radar PHY errors when DFS is not enabled

2015-01-15 Thread Simon Wunderlich
Hey Kalle,

On Thursday 15 January 2015 16:30:51 Kalle Valo wrote:
 Simon Wunderlich s...@simonwunderlich.de writes:
  I did not dig how the hw-conf.radar_enabled flag is set in monitor mode,
  but if it is same as for master (i.e. set for DFS channels), then it
  would
  be a better approach to prevent calling ath9k_dfs_process_phyerr()
  altogether from ath9k_rx_skb_preprocess() if not set.
  
  Hm, you mean like - if radar_enabled then dfs_process, otherwise
  fft_process? That would might be more elegant indeed ...
  
  The monitor mode does not have the radar flag enabled,
  cfg80211_chandef_dfs_required() returns 0 in this case.
 
 So are you going to send v2 or what's the plan? I didn't quite get the
 conclusion from the discussion.

sorry for the silence - yes, please drop this version, I'll send v2.

Thanks
Simon

signature.asc
Description: This is a digitally signed message part.
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] [PATCH] ath9k: ignore radar PHY errors when DFS is not enabled

2015-01-13 Thread Zefir Kurtisi
On 01/10/2015 05:26 PM, Simon Wunderlich wrote:
 On Friday 09 January 2015 19:57:37 Arend van Spriel wrote:
 On 01/09/15 17:54, Simon Wunderlich wrote:
 Performing spectral scans on 5 GHz channels may result in PHY errors
 sent by the hardware, even if DFS support is not enabled in the driver
 (e.g. channel scanning or passive monitoring). In that case channels may
 falsely get marked as 'unusable'. To fix that, only process radar PHY
 errors when radar is explicitly enabled in the driver.

 Hi Simon,

 Not an ath9k expert, but I would think those channels would already be
 marked as unusable, because DFS is disabled in the driver. Or does this
 also affect 5G channels that do not require DFS.

 Regards,
 Arend
 
 Hey Arend,
 
 maybe that was not really clear, but this is talking about the DFS state 
 unusable. By default, channels are marked in DFS state usable, and after 
 the clear channel assessment (which is done e.g. when starting AP mode) they 
 are marked as available. As soon as radar is detected they are marked as 
 unusable.
 
 These DFS state changes should only happen while there is something operating 
 with radar enabled, e.g. AP mode. It should not happen if we just have 
 monitor 
 mode or scan for channels. These channels should then stay in their previous 
 DFS state (e.g. 'usable'). This was borked and this patch tries to fix it. :)
 
 Cheers,
  Simon
 
Hi,

the issue here is that DFS and spectral use the same PHY_ERROR reporting
mechanism, and the dfs module is still in its initial state prior the spectral
support was added. With that, feeding the dfs detector with PHY_ERROR frames
generated by spectral scanner might cause false radar detections.

I did not dig how the hw-conf.radar_enabled flag is set in monitor mode, but if
it is same as for master (i.e. set for DFS channels), then it would be a better
approach to prevent calling ath9k_dfs_process_phyerr() altogether from
ath9k_rx_skb_preprocess() if not set.

And while you're at that: slaves do not need to scan for radar, might be worth
checking if it makes sense to selectively disable radar detection in STA mode. I
am using attached private OpenWRT patch for that - which still would interfere
with spectral scanning. Generally, the PHY_ERROR processing should be reworked 
but
becomes quite complicated when you take into account special use-cases. Think of
radar events being treated differently depending on whether a master or a 
monitor
detected them (OC-CAC vs. ISM).


Cheers,
Zefir

From 089ab0d624d4b6f3a206ea8a81b4a3e061cf3edb Mon Sep 17 00:00:00 2001
From: Zefir Kurtisi zefir.kurt...@neratec.com
Date: Thu, 30 Jan 2014 13:33:50 +0100
Subject: [PATCH] ath9k: do not enable radar pulse detection in STA mode

---
 drivers/net/wireless/ath/ath9k/recv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -384,7 +384,7 @@ u32 ath_calcrxfilter(struct ath_softc *s
 		| ATH9K_RX_FILTER_MCAST;
 
 	/* if operating on a DFS channel, enable radar pulse detection */
-	if (sc-hw-conf.radar_enabled)
+	if (sc-hw-conf.radar_enabled  sc-sc_ah-opmode != NL80211_IFTYPE_STATION)
 		rfilt |= ATH9K_RX_FILTER_PHYRADAR | ATH9K_RX_FILTER_PHYERR;
 
 	spin_lock_bh(sc-chan_lock);
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] [PATCH] ath9k: ignore radar PHY errors when DFS is not enabled

2015-01-13 Thread Simon Wunderlich
On Tuesday 13 January 2015 11:16:02 Zefir Kurtisi wrote:
 On 01/10/2015 05:26 PM, Simon Wunderlich wrote:
  On Friday 09 January 2015 19:57:37 Arend van Spriel wrote:
  On 01/09/15 17:54, Simon Wunderlich wrote:
  Performing spectral scans on 5 GHz channels may result in PHY errors
  sent by the hardware, even if DFS support is not enabled in the driver
  (e.g. channel scanning or passive monitoring). In that case channels may
  falsely get marked as 'unusable'. To fix that, only process radar PHY
  errors when radar is explicitly enabled in the driver.
  
  Hi Simon,
  
  Not an ath9k expert, but I would think those channels would already be
  marked as unusable, because DFS is disabled in the driver. Or does this
  also affect 5G channels that do not require DFS.
  
  Regards,
  Arend
  
  Hey Arend,
  
  maybe that was not really clear, but this is talking about the DFS state
  unusable. By default, channels are marked in DFS state usable, and
  after the clear channel assessment (which is done e.g. when starting AP
  mode) they are marked as available. As soon as radar is detected they
  are marked as unusable.
  
  These DFS state changes should only happen while there is something
  operating with radar enabled, e.g. AP mode. It should not happen if we
  just have monitor mode or scan for channels. These channels should then
  stay in their previous DFS state (e.g. 'usable'). This was borked and
  this patch tries to fix it. :)
  
  Cheers,
  
   Simon
 
 Hi,
 
 the issue here is that DFS and spectral use the same PHY_ERROR reporting
 mechanism, and the dfs module is still in its initial state prior the
 spectral support was added. With that, feeding the dfs detector with
 PHY_ERROR frames generated by spectral scanner might cause false radar
 detections.

Yup, that's right - we noticed that too, and its written in various places 
that the FFT and DFS hardware logic is shared. :)
 
 I did not dig how the hw-conf.radar_enabled flag is set in monitor mode,
 but if it is same as for master (i.e. set for DFS channels), then it would
 be a better approach to prevent calling ath9k_dfs_process_phyerr()
 altogether from ath9k_rx_skb_preprocess() if not set.

Hm, you mean like - if radar_enabled then dfs_process, otherwise fft_process? 
That would might be more elegant indeed ...

The monitor mode does not have the radar flag enabled, 
cfg80211_chandef_dfs_required() returns 0 in this case.

 
 And while you're at that: slaves do not need to scan for radar, might be
 worth checking if it makes sense to selectively disable radar detection in
 STA mode. I am using attached private OpenWRT patch for that - which still
 would interfere with spectral scanning. Generally, the PHY_ERROR processing
 should be reworked but becomes quite complicated when you take into account
 special use-cases. Think of radar events being treated differently
 depending on whether a master or a monitor detected them (OC-CAC vs. ISM).

I didn't check if that is enforced correctly, but 
cfg80211_chandef_dfs_required() returns if radar is required for the various 
interface types - AP, Adhoc and Mesh have it enabled if its a DFS channel, 
client, monitor, etc don't have it enabled. That gets marked in the sdata-
radar_required, and ieee80211_is_radar_required() checks all interfaces if 
there is any interface which needs radar. So that should have been taken care 
of.

Therefore I think that this is already handled in cfg80211/mac80211 and ath9k 
should not check the iftype at all, but only check the radar_enabled flag.

Off-channel CAC is certainly a different beast, but as far as I know we 
currently don't support that anyway. :)

Cheers,
Simon

signature.asc
Description: This is a digitally signed message part.
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] [PATCH] ath9k: ignore radar PHY errors when DFS is not enabled

2015-01-13 Thread Zefir Kurtisi
On 01/13/2015 12:04 PM, Simon Wunderlich wrote:
 On Tuesday 13 January 2015 11:16:02 Zefir Kurtisi wrote:
 [...]

 I did not dig how the hw-conf.radar_enabled flag is set in monitor mode,
 but if it is same as for master (i.e. set for DFS channels), then it would
 be a better approach to prevent calling ath9k_dfs_process_phyerr()
 altogether from ath9k_rx_skb_preprocess() if not set.
 
 Hm, you mean like - if radar_enabled then dfs_process, otherwise fft_process? 
 That would might be more elegant indeed ...
 
More concrete / restrictive:
* if radar_enabled
 - spectral must not be enabled
 - only ath9k_dfs_process_phyerr() has to be processed
* if !radar_enabled
 - don't process ath9k_dfs_process_phyerr()

 The monitor mode does not have the radar flag enabled, 
 cfg80211_chandef_dfs_required() returns 0 in this case.
 
Ah, which then means you can not do (supplemental) CACs with a monitor
out-of-the-box? For that, radar_enabled would need to be set for monitor, which
basically should not harm for a fully passive interface.


 And while you're at that: slaves do not need to scan for radar, might be
 worth checking if it makes sense to selectively disable radar detection in
 STA mode. I am using attached private OpenWRT patch for that - which still
 would interfere with spectral scanning. Generally, the PHY_ERROR processing
 should be reworked but becomes quite complicated when you take into account
 special use-cases. Think of radar events being treated differently
 depending on whether a master or a monitor detected them (OC-CAC vs. ISM).
 
 I didn't check if that is enforced correctly, but 
 cfg80211_chandef_dfs_required() returns if radar is required for the various 
 interface types - AP, Adhoc and Mesh have it enabled if its a DFS channel, 
 client, monitor, etc don't have it enabled. That gets marked in the sdata-
 radar_required, and ieee80211_is_radar_required() checks all interfaces if 
 there is any interface which needs radar. So that should have been taken care 
 of.
 
 Therefore I think that this is already handled in cfg80211/mac80211 and ath9k 
 should not check the iftype at all, but only check the radar_enabled flag.
 
Ok, thanks for clarifying that - one private patch less to handle :)


___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] [PATCH] ath9k: ignore radar PHY errors when DFS is not enabled

2015-01-10 Thread Simon Wunderlich
On Friday 09 January 2015 19:57:37 Arend van Spriel wrote:
 On 01/09/15 17:54, Simon Wunderlich wrote:
  Performing spectral scans on 5 GHz channels may result in PHY errors
  sent by the hardware, even if DFS support is not enabled in the driver
  (e.g. channel scanning or passive monitoring). In that case channels may
  falsely get marked as 'unusable'. To fix that, only process radar PHY
  errors when radar is explicitly enabled in the driver.
 
 Hi Simon,
 
 Not an ath9k expert, but I would think those channels would already be
 marked as unusable, because DFS is disabled in the driver. Or does this
 also affect 5G channels that do not require DFS.
 
 Regards,
 Arend

Hey Arend,

maybe that was not really clear, but this is talking about the DFS state 
unusable. By default, channels are marked in DFS state usable, and after 
the clear channel assessment (which is done e.g. when starting AP mode) they 
are marked as available. As soon as radar is detected they are marked as 
unusable.

These DFS state changes should only happen while there is something operating 
with radar enabled, e.g. AP mode. It should not happen if we just have monitor 
mode or scan for channels. These channels should then stay in their previous 
DFS state (e.g. 'usable'). This was borked and this patch tries to fix it. :)

Cheers,
 Simon

signature.asc
Description: This is a digitally signed message part.
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] [PATCH] ath9k: ignore radar PHY errors when DFS is not enabled

2015-01-09 Thread Arend van Spriel
On 01/09/15 17:54, Simon Wunderlich wrote:
 Performing spectral scans on 5 GHz channels may result in PHY errors
 sent by the hardware, even if DFS support is not enabled in the driver
 (e.g. channel scanning or passive monitoring). In that case channels may
 falsely get marked as 'unusable'. To fix that, only process radar PHY
 errors when radar is explicitly enabled in the driver.

Hi Simon,

Not an ath9k expert, but I would think those channels would already be 
marked as unusable, because DFS is disabled in the driver. Or does this 
also affect 5G channels that do not require DFS.

Regards,
Arend

 Cc: Stablesta...@vger.kernel.org  [v3.10+]
 Reported-by: Mathias Kretschmermathias.kretsch...@fokus.fraunhofer.de
 Signed-off-by: Simon Wunderlichs...@simonwunderlich.de
 ---
   drivers/net/wireless/ath/ath9k/dfs.c | 6 ++
   1 file changed, 6 insertions(+)

 diff --git a/drivers/net/wireless/ath/ath9k/dfs.c 
 b/drivers/net/wireless/ath/ath9k/dfs.c
 index 726271c..3d04905 100644
 --- a/drivers/net/wireless/ath/ath9k/dfs.c
 +++ b/drivers/net/wireless/ath/ath9k/dfs.c
 @@ -152,6 +152,12 @@ void ath9k_dfs_process_phyerr(struct ath_softc *sc, void 
 *data,
   return;
   }

 + if (!sc-hw-conf.radar_enabled) {
 + ath_dbg(common, DFS,
 + Error: received radar phyerr while radar was 
 disabled\n);
 + return;
 + }
 +
   datalen = rs-rs_datalen;
   if (datalen == 0) {
   DFS_STAT_INC(sc, datalen_discards);

___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


[ath9k-devel] [PATCH] ath9k: ignore radar PHY errors when DFS is not enabled

2015-01-09 Thread Simon Wunderlich
Performing spectral scans on 5 GHz channels may result in PHY errors
sent by the hardware, even if DFS support is not enabled in the driver
(e.g. channel scanning or passive monitoring). In that case channels may
falsely get marked as 'unusable'. To fix that, only process radar PHY
errors when radar is explicitly enabled in the driver.

Cc: Stable sta...@vger.kernel.org [v3.10+]
Reported-by: Mathias Kretschmer mathias.kretsch...@fokus.fraunhofer.de
Signed-off-by: Simon Wunderlich s...@simonwunderlich.de
---
 drivers/net/wireless/ath/ath9k/dfs.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/dfs.c 
b/drivers/net/wireless/ath/ath9k/dfs.c
index 726271c..3d04905 100644
--- a/drivers/net/wireless/ath/ath9k/dfs.c
+++ b/drivers/net/wireless/ath/ath9k/dfs.c
@@ -152,6 +152,12 @@ void ath9k_dfs_process_phyerr(struct ath_softc *sc, void 
*data,
return;
}
 
+   if (!sc-hw-conf.radar_enabled) {
+   ath_dbg(common, DFS,
+   Error: received radar phyerr while radar was 
disabled\n);
+   return;
+   }
+
datalen = rs-rs_datalen;
if (datalen == 0) {
DFS_STAT_INC(sc, datalen_discards);
-- 
2.1.4

___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel