On 12/4/2024 9:05 PM, Johannes Berg wrote:
The subject is a bit misleading IMHO - you don't skip all checks when
there's puncturing ...
maybe just say "wifi: cfg80211: skip regulatory for punctured
subchannels"
However, these checks are also performed on frequencies that
have been punctured, which should not be examined as they are
not in use.
I'd argue subchannels are punctured (or really disabled, the whole
channel is punctured ... but we mix that up already), not frequencies
Thanks for the feedback, will make the necessary changes.
static void cfg80211_set_chans_dfs_state(struct wiphy *wiphy, u32 center_freq,
- u32 bandwidth,
+ u32 bandwidth, u16 punctured,
enum nl80211_dfs_state dfs_state)
{
struct ieee80211_channel *c;
u32 freq;
+ int subchan = 0;
for (freq = center_freq - bandwidth/2 + 10;
freq <= center_freq + bandwidth/2 - 10;
freq += 20) {
+ if (punctured & BIT(subchan))
+ continue;
+ subchan++;
You never tested this code properly, it's clearly broken.
but anyway - for_each_subchan()?
Thank you for pointing that out. You are correct. I apologize for the
mistake.
The for_each_subchan() macro will not work for this. When sub channel is
null, it will terminate the loop, but in this case, we should continue
checking other sub channels.
+#define for_each_subchan(wiphy, center_freq, bandwidth, punctured, \
+ subchan) \
I feel like we really should make this work on a chandef, not all these
arguments ... and cover both center_freq1 and center_freq2, because we
have all these duplicate calls like cfg80211_chandef_dfs_cac_time().
+ for (subchan = ieee80211_next_subchan(wiphy, center_freq, bandwidth, \
+ punctured, NULL); \
It should be especially easy if you're pulling it out into iterator
functions? Worst case, keep some extra state in the loop, like
for (u32 punctured = chandef->punctured,
freq = cfg80211_get_start_freq(chandef, 1),
_cf = 1;
freq = _cf == 1 ? 20, punctured >>= 1;
...)
if (!(punctured & 1))
I don't really mind a bit more complexity here, if it means we can get
rid of all the functions like cfg80211_get_chans_dfs_required() that get
called twice ...
Thank you for the suggestion. I am working on this and will share the
updated patch with you soon.
johannes
