[ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[Benjamin Berg]

In the case that a spectral scan is enabled the PHY errors sent by the
hardware as part of the scanning might trigger the radar detection and
channels might be marked as 'unusable' incorrectly. This patch fixes
the issue by preventing the spectral scan to be enabled if DFS is used
and only analysing the PHY errors for DFS if radar detection is enabled.

Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fit.fraunhofer.de>
Signed-off-by: Benjamin Berg <benjamin@sipsolutions.net>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 drivers/net/wireless/ath/ath9k/common-spectral.c | 23 +++++++++++++++++++----
 drivers/net/wireless/ath/ath9k/main.c            |  6 ++++++
 drivers/net/wireless/ath/ath9k/recv.c            |  7 +++++--
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/common-spectral.c b/drivers/net/wireless/ath/ath9k/common-spectral.c
index e2512d5..8444d6d 100644
--- a/drivers/net/wireless/ath/ath9k/common-spectral.c
+++ b/drivers/net/wireless/ath/ath9k/common-spectral.c
@@ -802,16 +802,27 @@ static ssize_t write_file_spec_scan_ctl(struct file *file,
 					size_t count, loff_t *ppos)
 {
 	struct ath_spec_scan_priv *spec_priv = file->private_data;
+	struct ath_hw *ah = spec_priv->ah;
+	struct ath_softc *sc = ah->hw->priv;
 	struct ath_common *common = ath9k_hw_common(spec_priv->ah);
 	char buf[32];
 	ssize_t len;
+	ssize_t result = count;
 
 	if (IS_ENABLED(CONFIG_ATH9K_TX99))
 		return -EOPNOTSUPP;
 
+	mutex_lock(&sc->mutex);
+	if (ah->hw->conf.radar_enabled) {
+		result = -EINVAL;
+		goto unlock;
+	}
+
 	len = min(count, sizeof(buf) - 1);
-	if (copy_from_user(buf, user_buf, len))
-		return -EFAULT;
+	if (copy_from_user(buf, user_buf, len)) {
+		result = -EFAULT;
+		goto unlock;
+	}
 
 	buf[len] = '\0';
 
@@ -830,10 +841,14 @@ static ssize_t write_file_spec_scan_ctl(struct file *file,
 		ath9k_cmn_spectral_scan_config(common, spec_priv, SPECTRAL_DISABLED);
 		ath_dbg(common, CONFIG, "spectral scan: disabled\n");
 	} else {
-		return -EINVAL;
+		result = -EINVAL;
+		goto unlock;
 	}
 
-	return count;
+unlock:
+	mutex_unlock(&sc->mutex);
+
+	return result;
 }
 
 static const struct file_operations fops_spec_scan_ctl = {
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index e9f32b5..62b86fb 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1459,6 +1459,12 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
 	if (!ath9k_is_chanctx_enabled() && (changed & IEEE80211_CONF_CHANGE_CHANNEL)) {
 		ctx->offchannel = !!(conf->flags & IEEE80211_CONF_OFFCHANNEL);
 		ath_chanctx_set_channel(sc, ctx, &hw->conf.chandef);
+
+		/* We need to ensure that spectral scan is disabled. */
+		if (conf->radar_enabled &&
+		    sc->spec_priv.spectral_mode != SPECTRAL_DISABLED)
+			ath9k_cmn_spectral_scan_config(common, &sc->spec_priv,
+						       SPECTRAL_DISABLED);
 	}
 
 	mutex_unlock(&sc->mutex);
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 6697342..ce34add 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -867,8 +867,11 @@ static int ath9k_rx_skb_preprocess(struct ath_softc *sc,
 	 * can be dropped.
 	 */
 	if (rx_stats->rs_status & ATH9K_RXERR_PHY) {
-		ath9k_dfs_process_phyerr(sc, hdr, rx_stats, rx_status->mactime);
-		if (ath_cmn_process_fft(&sc->spec_priv, hdr, rx_stats, rx_status->mactime))
+		if (hw->conf.radar_enabled)
+			ath9k_dfs_process_phyerr(sc, hdr, rx_stats,
+						 rx_status->mactime);
+		else if (ath_cmn_process_fft(&sc->spec_priv, hdr, rx_stats,
+					     rx_status->mactime))
 			RX_STAT_INC(rx_spectral);
 
 		return -EINVAL;
-- 
2.10.2

[PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[Benjamin Berg]

In the case that a spectral scan is enabled the PHY errors sent by the
hardware as part of the scanning might trigger the radar detection and
channels might be marked as 'unusable' incorrectly. This patch fixes
the issue by preventing the spectral scan to be enabled if DFS is used
and only analysing the PHY errors for DFS if radar detection is enabled.

Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fit.fraunhofer.de>
Signed-off-by: Benjamin Berg <benjamin@sipsolutions.net>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 drivers/net/wireless/ath/ath9k/common-spectral.c | 23 +++++++++++++++++++----
 drivers/net/wireless/ath/ath9k/main.c            |  6 ++++++
 drivers/net/wireless/ath/ath9k/recv.c            |  7 +++++--
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/common-spectral.c b/drivers/net/wireless/ath/ath9k/common-spectral.c
index e2512d5..8444d6d 100644
--- a/drivers/net/wireless/ath/ath9k/common-spectral.c
+++ b/drivers/net/wireless/ath/ath9k/common-spectral.c
@@ -802,16 +802,27 @@ static ssize_t write_file_spec_scan_ctl(struct file *file,
 					size_t count, loff_t *ppos)
 {
 	struct ath_spec_scan_priv *spec_priv = file->private_data;
+	struct ath_hw *ah = spec_priv->ah;
+	struct ath_softc *sc = ah->hw->priv;
 	struct ath_common *common = ath9k_hw_common(spec_priv->ah);
 	char buf[32];
 	ssize_t len;
+	ssize_t result = count;
 
 	if (IS_ENABLED(CONFIG_ATH9K_TX99))
 		return -EOPNOTSUPP;
 
+	mutex_lock(&sc->mutex);
+	if (ah->hw->conf.radar_enabled) {
+		result = -EINVAL;
+		goto unlock;
+	}
+
 	len = min(count, sizeof(buf) - 1);
-	if (copy_from_user(buf, user_buf, len))
-		return -EFAULT;
+	if (copy_from_user(buf, user_buf, len)) {
+		result = -EFAULT;
+		goto unlock;
+	}
 
 	buf[len] = '\0';
 
@@ -830,10 +841,14 @@ static ssize_t write_file_spec_scan_ctl(struct file *file,
 		ath9k_cmn_spectral_scan_config(common, spec_priv, SPECTRAL_DISABLED);
 		ath_dbg(common, CONFIG, "spectral scan: disabled\n");
 	} else {
-		return -EINVAL;
+		result = -EINVAL;
+		goto unlock;
 	}
 
-	return count;
+unlock:
+	mutex_unlock(&sc->mutex);
+
+	return result;
 }
 
 static const struct file_operations fops_spec_scan_ctl = {
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index e9f32b5..62b86fb 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1459,6 +1459,12 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
 	if (!ath9k_is_chanctx_enabled() && (changed & IEEE80211_CONF_CHANGE_CHANNEL)) {
 		ctx->offchannel = !!(conf->flags & IEEE80211_CONF_OFFCHANNEL);
 		ath_chanctx_set_channel(sc, ctx, &hw->conf.chandef);
+
+		/* We need to ensure that spectral scan is disabled. */
+		if (conf->radar_enabled &&
+		    sc->spec_priv.spectral_mode != SPECTRAL_DISABLED)
+			ath9k_cmn_spectral_scan_config(common, &sc->spec_priv,
+						       SPECTRAL_DISABLED);
 	}
 
 	mutex_unlock(&sc->mutex);
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 6697342..ce34add 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -867,8 +867,11 @@ static int ath9k_rx_skb_preprocess(struct ath_softc *sc,
 	 * can be dropped.
 	 */
 	if (rx_stats->rs_status & ATH9K_RXERR_PHY) {
-		ath9k_dfs_process_phyerr(sc, hdr, rx_stats, rx_status->mactime);
-		if (ath_cmn_process_fft(&sc->spec_priv, hdr, rx_stats, rx_status->mactime))
+		if (hw->conf.radar_enabled)
+			ath9k_dfs_process_phyerr(sc, hdr, rx_stats,
+						 rx_status->mactime);
+		else if (ath_cmn_process_fft(&sc->spec_priv, hdr, rx_stats,
+					     rx_status->mactime))
 			RX_STAT_INC(rx_spectral);
 
 		return -EINVAL;
-- 
2.10.2

[ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[Michal Kazior]

On 21 November 2016 at 15:04, Benjamin Berg <benjamin@sipsolutions.net> wrote:
> In the case that a spectral scan is enabled the PHY errors sent by the
> hardware as part of the scanning might trigger the radar detection and
> channels might be marked as 'unusable' incorrectly. This patch fixes
> the issue by preventing the spectral scan to be enabled if DFS is used
> and only analysing the PHY errors for DFS if radar detection is enabled.

According to the comment in ath_cmn_process_fft() this doesn't seem to
be necessary for all chips:

 515         /* AR9280 and before report via ATH9K_PHYERR_RADAR,
AR93xx and newer
 516          * via ATH9K_PHYERR_SPECTRAL. Haven't seen
ATH9K_PHYERR_FALSE_RADAR_EXT
 517          * yet, but this is supposed to be possible as well.
 518          */
 519         if (rs->rs_phyerr != ATH9K_PHYERR_RADAR &&
 520             rs->rs_phyerr != ATH9K_PHYERR_FALSE_RADAR_EXT &&
 521             rs->rs_phyerr != ATH9K_PHYERR_SPECTRAL)
 522                 return 0;


Micha?

Re: [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[Michal Kazior]

On 21 November 2016 at 15:04, Benjamin Berg <benjamin@sipsolutions.net> wro=
te:
> In the case that a spectral scan is enabled the PHY errors sent by the
> hardware as part of the scanning might trigger the radar detection and
> channels might be marked as 'unusable' incorrectly. This patch fixes
> the issue by preventing the spectral scan to be enabled if DFS is used
> and only analysing the PHY errors for DFS if radar detection is enabled.

According to the comment in ath_cmn_process_fft() this doesn't seem to
be necessary for all chips:

 515         /* AR9280 and before report via ATH9K_PHYERR_RADAR,
AR93xx and newer
 516          * via ATH9K_PHYERR_SPECTRAL. Haven't seen
ATH9K_PHYERR_FALSE_RADAR_EXT
 517          * yet, but this is supposed to be possible as well.
 518          */
 519         if (rs->rs_phyerr !=3D ATH9K_PHYERR_RADAR &&
 520             rs->rs_phyerr !=3D ATH9K_PHYERR_FALSE_RADAR_EXT &&
 521             rs->rs_phyerr !=3D ATH9K_PHYERR_SPECTRAL)
 522                 return 0;


Micha=C5=82

[ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[Zefir Kurtisi]

On 11/21/2016 03:04 PM, Benjamin Berg wrote:
> In the case that a spectral scan is enabled the PHY errors sent by the
> hardware as part of the scanning might trigger the radar detection and
> channels might be marked as 'unusable' incorrectly. This patch fixes
> the issue by preventing the spectral scan to be enabled if DFS is used
> and only analysing the PHY errors for DFS if radar detection is enabled.
> 
> [...]

Re: [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[Zefir Kurtisi]

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

On 11/21/2016 03:04 PM, Benjamin Berg wrote:
> In the case that a spectral scan is enabled the PHY errors sent by the
> hardware as part of the scanning might trigger the radar detection and
> channels might be marked as 'unusable' incorrectly. This patch fixes
> the issue by preventing the spectral scan to be enabled if DFS is used
> and only analysing the PHY errors for DFS if radar detection is enabled.
> 
> [...]

>From the relevant source code portion in channel.c:ath_set_channel()

80         /* Enable radar pulse detection if on a DFS channel. Spectral
81          * scanning and radar detection can not be used concurrently.
82          */
83         if (hw->conf.radar_enabled) {
84                 u32 rxfilter;
85
86                 rxfilter = ath9k_hw_getrxfilter(ah);
87                 rxfilter |= ATH9K_RX_FILTER_PHYRADAR |
88                                 ATH9K_RX_FILTER_PHYERR;
89                 ath9k_hw_setrxfilter(ah, rxfilter);
90                 ath_dbg(common, DFS, "DFS enabled at freq %d\n",
91                         chan->center_freq);
92         } else {
93                 /* perform spectral scan if requested. */
94                 if (test_bit(ATH_OP_SCANNING, &common->op_flags) &&
95                         sc->spec_priv.spectral_mode == SPECTRAL_CHANSCAN)
96                         ath9k_cmn_spectral_scan_trigger(common, &sc->spec_priv);
97         }

it seems that spectral can't ever be activated while operating on a DFS channel.

If you need to catch the opposite case, i.e. prevent feeding pseudo-radar pulses
into the pattern detector, you just need to ensure that they depend on
hw->conf.radar_enabled. A patch like the attached one should be enough.


Cheers,
Zefir

[-- Attachment #2: 0001-ath9k-feed-DFS-detector-only-if-operating-on-radar-c.patch --]
[-- Type: text/x-patch, Size: 986 bytes --]

>From c24edf82e1f509490ba9dd3e34eec3ac3b309321 Mon Sep 17 00:00:00 2001
From: Zefir Kurtisi <zefir.kurtisi@neratec.com>
Date: Mon, 21 Nov 2016 15:33:45 +0100
Subject: [PATCH] ath9k: feed DFS detector only if operating on radar channel

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

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 6697342..e4701a7 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -867,7 +867,8 @@ static int ath9k_rx_skb_preprocess(struct ath_softc *sc,
 	 * can be dropped.
 	 */
 	if (rx_stats->rs_status & ATH9K_RXERR_PHY) {
-		ath9k_dfs_process_phyerr(sc, hdr, rx_stats, rx_status->mactime);
+		if (hw->conf.radar_enabled)
+			ath9k_dfs_process_phyerr(sc, hdr, rx_stats, rx_status->mactime);
 		if (ath_cmn_process_fft(&sc->spec_priv, hdr, rx_stats, rx_status->mactime))
 			RX_STAT_INC(rx_spectral);
 
-- 
2.7.4

[ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[Michal Kazior]

On 21 November 2016 at 15:41, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> On 11/21/2016 03:04 PM, Benjamin Berg wrote:
>> In the case that a spectral scan is enabled the PHY errors sent by the
>> hardware as part of the scanning might trigger the radar detection and
>> channels might be marked as 'unusable' incorrectly. This patch fixes
>> the issue by preventing the spectral scan to be enabled if DFS is used
>> and only analysing the PHY errors for DFS if radar detection is enabled.
>>
>> [...]
>
> From the relevant source code portion in channel.c:ath_set_channel()
>
> 80         /* Enable radar pulse detection if on a DFS channel. Spectral
> 81          * scanning and radar detection can not be used concurrently.
> 82          */
> 83         if (hw->conf.radar_enabled) {
> 84                 u32 rxfilter;
> 85
> 86                 rxfilter = ath9k_hw_getrxfilter(ah);
> 87                 rxfilter |= ATH9K_RX_FILTER_PHYRADAR |
> 88                                 ATH9K_RX_FILTER_PHYERR;
> 89                 ath9k_hw_setrxfilter(ah, rxfilter);
> 90                 ath_dbg(common, DFS, "DFS enabled at freq %d\n",
> 91                         chan->center_freq);
> 92         } else {
> 93                 /* perform spectral scan if requested. */
> 94                 if (test_bit(ATH_OP_SCANNING, &common->op_flags) &&
> 95                         sc->spec_priv.spectral_mode == SPECTRAL_CHANSCAN)
> 96                         ath9k_cmn_spectral_scan_trigger(common, &sc->spec_priv);
> 97         }
>
> it seems that spectral can't ever be activated while operating on a DFS channel.
>
> If you need to catch the opposite case, i.e. prevent feeding pseudo-radar pulses
> into the pattern detector, you just need to ensure that they depend on
> hw->conf.radar_enabled. A patch like the attached one should be enough.

Good point. I guess set_channel could be oversimplified as well. I
mean, it makes sense to consider radar and spectral mutually exclusive
if they use the same phyerr code. However some chips actually seem (as
per the comment I mentioned) to distinguish the two so I don't know if
the "mutually exclusive" is true for all chips per se. Just thinking
out loud.

I also wonder if calling ieee80211_radar_detect() should have any
effect if there are no radar operated interfaces in the first place?


Micha?

Re: [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[Michal Kazior]

On 21 November 2016 at 15:41, Zefir Kurtisi <zefir.kurtisi@neratec.com> wro=
te:
> On 11/21/2016 03:04 PM, Benjamin Berg wrote:
>> In the case that a spectral scan is enabled the PHY errors sent by the
>> hardware as part of the scanning might trigger the radar detection and
>> channels might be marked as 'unusable' incorrectly. This patch fixes
>> the issue by preventing the spectral scan to be enabled if DFS is used
>> and only analysing the PHY errors for DFS if radar detection is enabled.
>>
>> [...]
>
> From the relevant source code portion in channel.c:ath_set_channel()
>
> 80         /* Enable radar pulse detection if on a DFS channel. Spectral
> 81          * scanning and radar detection can not be used concurrently.
> 82          */
> 83         if (hw->conf.radar_enabled) {
> 84                 u32 rxfilter;
> 85
> 86                 rxfilter =3D ath9k_hw_getrxfilter(ah);
> 87                 rxfilter |=3D ATH9K_RX_FILTER_PHYRADAR |
> 88                                 ATH9K_RX_FILTER_PHYERR;
> 89                 ath9k_hw_setrxfilter(ah, rxfilter);
> 90                 ath_dbg(common, DFS, "DFS enabled at freq %d\n",
> 91                         chan->center_freq);
> 92         } else {
> 93                 /* perform spectral scan if requested. */
> 94                 if (test_bit(ATH_OP_SCANNING, &common->op_flags) &&
> 95                         sc->spec_priv.spectral_mode =3D=3D SPECTRAL_CH=
ANSCAN)
> 96                         ath9k_cmn_spectral_scan_trigger(common, &sc->s=
pec_priv);
> 97         }
>
> it seems that spectral can't ever be activated while operating on a DFS c=
hannel.
>
> If you need to catch the opposite case, i.e. prevent feeding pseudo-radar=
 pulses
> into the pattern detector, you just need to ensure that they depend on
> hw->conf.radar_enabled. A patch like the attached one should be enough.

Good point. I guess set_channel could be oversimplified as well. I
mean, it makes sense to consider radar and spectral mutually exclusive
if they use the same phyerr code. However some chips actually seem (as
per the comment I mentioned) to distinguish the two so I don't know if
the "mutually exclusive" is true for all chips per se. Just thinking
out loud.

I also wonder if calling ieee80211_radar_detect() should have any
effect if there are no radar operated interfaces in the first place?


Micha=C5=82

[ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[Zefir Kurtisi]

On 11/21/2016 04:10 PM, Michal Kazior wrote:
> On 21 November 2016 at 15:41, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>> On 11/21/2016 03:04 PM, Benjamin Berg wrote:
>>> In the case that a spectral scan is enabled the PHY errors sent by the
>>> hardware as part of the scanning might trigger the radar detection and
>>> channels might be marked as 'unusable' incorrectly. This patch fixes
>>> the issue by preventing the spectral scan to be enabled if DFS is used
>>> and only analysing the PHY errors for DFS if radar detection is enabled.
>>>
>>> [...]
>>
>> From the relevant source code portion in channel.c:ath_set_channel()
>>
>> 80         /* Enable radar pulse detection if on a DFS channel. Spectral
>> 81          * scanning and radar detection can not be used concurrently.
>> 82          */
>> 83         if (hw->conf.radar_enabled) {
>> 84                 u32 rxfilter;
>> 85
>> 86                 rxfilter = ath9k_hw_getrxfilter(ah);
>> 87                 rxfilter |= ATH9K_RX_FILTER_PHYRADAR |
>> 88                                 ATH9K_RX_FILTER_PHYERR;
>> 89                 ath9k_hw_setrxfilter(ah, rxfilter);
>> 90                 ath_dbg(common, DFS, "DFS enabled at freq %d\n",
>> 91                         chan->center_freq);
>> 92         } else {
>> 93                 /* perform spectral scan if requested. */
>> 94                 if (test_bit(ATH_OP_SCANNING, &common->op_flags) &&
>> 95                         sc->spec_priv.spectral_mode == SPECTRAL_CHANSCAN)
>> 96                         ath9k_cmn_spectral_scan_trigger(common, &sc->spec_priv);
>> 97         }
>>
>> it seems that spectral can't ever be activated while operating on a DFS channel.
>>
>> If you need to catch the opposite case, i.e. prevent feeding pseudo-radar pulses
>> into the pattern detector, you just need to ensure that they depend on
>> hw->conf.radar_enabled. A patch like the attached one should be enough.
> 
> Good point. I guess set_channel could be oversimplified as well. I
> mean, it makes sense to consider radar and spectral mutually exclusive
> if they use the same phyerr code. However some chips actually seem (as
> per the comment I mentioned) to distinguish the two so I don't know if
> the "mutually exclusive" is true for all chips per se. Just thinking
> out loud.
> 
You're right, blindly feeding PHYERR frames to spectral when spectral is not
enabled is as wrong as feeding them to the dfs-detector when not on a radar
channel. Therefore, the patch I proposed should be extended to make calling
ath_cmn_process_fft() depending on
a) not operating on radar channel, and
b) spectral scan is enabled

Updated patch attached as proposal.

> I also wonder if calling ieee80211_radar_detect() should have any
> effect if there are no radar operated interfaces in the first place?
> 
True, calling ieee80211_radar_detect() for a non-DFS channel is (or should be)
ignored by mac. But feeding the dfs-detector with invalid pulses causes quite some
computational overhead - dropping them if no radar detection is required is a good
thing to do.


Cheers,
Zefir




-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ath9k-feed-DFS-detector-only-if-operating-on-radar-c.patch
Type: text/x-patch
Size: 0 bytes
Desc: not available
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20161121/ab36ab4e/attachment-0001.bin 

Re: [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[Zefir Kurtisi]

[-- Attachment #1: Type: text/plain, Size: 3091 bytes --]

On 11/21/2016 04:10 PM, Michal Kazior wrote:
> On 21 November 2016 at 15:41, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>> On 11/21/2016 03:04 PM, Benjamin Berg wrote:
>>> In the case that a spectral scan is enabled the PHY errors sent by the
>>> hardware as part of the scanning might trigger the radar detection and
>>> channels might be marked as 'unusable' incorrectly. This patch fixes
>>> the issue by preventing the spectral scan to be enabled if DFS is used
>>> and only analysing the PHY errors for DFS if radar detection is enabled.
>>>
>>> [...]
>>
>> From the relevant source code portion in channel.c:ath_set_channel()
>>
>> 80         /* Enable radar pulse detection if on a DFS channel. Spectral
>> 81          * scanning and radar detection can not be used concurrently.
>> 82          */
>> 83         if (hw->conf.radar_enabled) {
>> 84                 u32 rxfilter;
>> 85
>> 86                 rxfilter = ath9k_hw_getrxfilter(ah);
>> 87                 rxfilter |= ATH9K_RX_FILTER_PHYRADAR |
>> 88                                 ATH9K_RX_FILTER_PHYERR;
>> 89                 ath9k_hw_setrxfilter(ah, rxfilter);
>> 90                 ath_dbg(common, DFS, "DFS enabled at freq %d\n",
>> 91                         chan->center_freq);
>> 92         } else {
>> 93                 /* perform spectral scan if requested. */
>> 94                 if (test_bit(ATH_OP_SCANNING, &common->op_flags) &&
>> 95                         sc->spec_priv.spectral_mode == SPECTRAL_CHANSCAN)
>> 96                         ath9k_cmn_spectral_scan_trigger(common, &sc->spec_priv);
>> 97         }
>>
>> it seems that spectral can't ever be activated while operating on a DFS channel.
>>
>> If you need to catch the opposite case, i.e. prevent feeding pseudo-radar pulses
>> into the pattern detector, you just need to ensure that they depend on
>> hw->conf.radar_enabled. A patch like the attached one should be enough.
> 
> Good point. I guess set_channel could be oversimplified as well. I
> mean, it makes sense to consider radar and spectral mutually exclusive
> if they use the same phyerr code. However some chips actually seem (as
> per the comment I mentioned) to distinguish the two so I don't know if
> the "mutually exclusive" is true for all chips per se. Just thinking
> out loud.
> 
You're right, blindly feeding PHYERR frames to spectral when spectral is not
enabled is as wrong as feeding them to the dfs-detector when not on a radar
channel. Therefore, the patch I proposed should be extended to make calling
ath_cmn_process_fft() depending on
a) not operating on radar channel, and
b) spectral scan is enabled

Updated patch attached as proposal.

> I also wonder if calling ieee80211_radar_detect() should have any
> effect if there are no radar operated interfaces in the first place?
> 
True, calling ieee80211_radar_detect() for a non-DFS channel is (or should be)
ignored by mac. But feeding the dfs-detector with invalid pulses causes quite some
computational overhead - dropping them if no radar detection is required is a good
thing to do.


Cheers,
Zefir





[-- Attachment #2: 0001-ath9k-feed-DFS-detector-only-if-operating-on-radar-c.patch --]
[-- Type: text/x-patch, Size: 1455 bytes --]

>From aaa28dcaf0879c6bfc7be96077c79894bb230dfb Mon Sep 17 00:00:00 2001
From: Zefir Kurtisi <zefir.kurtisi@neratec.com>
Date: Mon, 21 Nov 2016 15:33:45 +0100
Subject: [PATCH] ath9k: feed DFS detector only if operating on radar channel

---
 drivers/net/wireless/ath/ath9k/recv.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 6697342..48f8af1 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -867,10 +867,21 @@ static int ath9k_rx_skb_preprocess(struct ath_softc *sc,
 	 * can be dropped.
 	 */
 	if (rx_stats->rs_status & ATH9K_RXERR_PHY) {
-		ath9k_dfs_process_phyerr(sc, hdr, rx_stats, rx_status->mactime);
-		if (ath_cmn_process_fft(&sc->spec_priv, hdr, rx_stats, rx_status->mactime))
+		/*
+		 * DFS and spectral are mutually exclusive
+		 *
+		 * Since some chips use RXERR_PHY as indication for both, we
+		 * need to double check which feature is enabled to prevent
+		 * feeding spectral or dfs-detector with wrong frames.
+		 */
+		if (hw->conf.radar_enabled) {
+			ath9k_dfs_process_phyerr(sc, hdr, rx_stats,
+						 rx_status->mactime);
+		} else if (sc->spec_priv.spectral_mode != SPECTRAL_DISABLED &&
+			   ath_cmn_process_fft(&sc->spec_priv, hdr, rx_stats,
+					       rx_status->mactime)) {
 			RX_STAT_INC(rx_spectral);
-
+		}
 		return -EINVAL;
 	}
 
-- 
2.7.4