Message ID | 54B4F062.2090301@neratec.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
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
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 :) -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Simon Wunderlich <sw@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.
Hey Kalle, On Thursday 15 January 2015 16:30:51 Kalle Valo wrote: > Simon Wunderlich <sw@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
From 089ab0d624d4b6f3a206ea8a81b4a3e061cf3edb Mon Sep 17 00:00:00 2001 From: Zefir Kurtisi <zefir.kurtisi@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);