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(-)
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
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
On 21 November 2016 at 15:04, Benjamin Berg <[email protected]> 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
On 21 November 2016 at 15:41, Zefir Kurtisi <[email protected]> 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
On 11/21/2016 04:10 PM, Michal Kazior wrote:
> On 21 November 2016 at 15:41, Zefir Kurtisi <[email protected]> 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