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

[Michal Kazior]

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]

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

[Michal Kazior]

Post by Zefir Kurtisi

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ł

[Zefir Kurtisi]

Post by Michal Kazior

Post by Zefir Kurtisi

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.

Post by Michal Kazior
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