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

PATCH New Benjamin Berg 3 participants 5 messages Nov 21, 2016 Qualcomm Atheros ATH9K wireless driver

Share & tools

Benjamin Berg

author

2016-11-21 14:04 UTC #0 lore raw

PATCH 1/1 ath9k: Prevent radar detection and spectral scan to be used concurrently

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 <[email protected]>

Signed-off-by Benjamin Berg <[email protected]>

Signed-off-by Simon Wunderlich <[email protected]>

---
 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(-)

drivers/net/wireless/ath/ath9k/common-spectral.c +19 -4

index e2512d5..8444d6d 100644
@@ -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 = {

drivers/net/wireless/ath/ath9k/main.c +6 -0

index e9f32b5..62b86fb 100644
@@ -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);

drivers/net/wireless/ath/ath9k/recv.c +5 -2

index 6697342..ce34add 100644
@@ -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;

Signature · 1 line

2.10.2

Michal Kazior

2016-11-21 14:16 UTC #1 lore raw

PATCH 1/1 ath9k: Prevent radar detection and spectral scan to be used concurrently

Quoted 5 lines · 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.

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?

Zefir Kurtisi

2016-11-21 14:41 UTC #2 lore raw

PATCH 1/1 ath9k: Prevent radar detection and spectral scan to be used concurrently

In reply to Benjamin Berg

Quoted 7 lines · 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.
[...]

Michal Kazior

2016-11-21 15:10 UTC #3 lore raw

PATCH 1/1 ath9k: Prevent radar detection and spectral scan to be used concurrently

Quoted 30 lines · Zefir Kurtisi

Quoted 7 lines · 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.
[...]
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 }

Quoted 5 lines

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?

Zefir Kurtisi

2016-11-21 16:16 UTC #4 lore raw

PATCH 1/1 ath9k: Prevent radar detection and spectral scan to be used concurrently

Quoted 37 lines · Michal Kazior

Quoted 30 lines · Zefir Kurtisi
Quoted 7 lines · 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.
[...]
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 }
Quoted 5 lines
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.

Quoted 7 lines

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.

Quoted 3 lines

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