On 1/3/2023 8:07 AM, Johannes Berg wrote:
Hi,[ 104.897615] brcmfmac: cfg80211_set_channel: set chanspec 0x100e fail, reason -52[...]All of these 10 errors are repeated every 60 sec.Catching up after the holidays ;-) Above chanspec values are invalid. 0x100e = channel 14/bw 20MHz. The 'iw list' output shows all these channels are disabled. So who/what is trying to set these channels. Scanning sets the channel in firmware. Is this initiated from hostapd?Yeah, what userspace is running here? Looks like cfg80211_set_channel() is only used for survey? Couple of observations on the side: * might be nice to have some "brcm" indication in that name :P * dump_survey should just dump data, not actually implement the data collection, I think?
commit 6c04deae1438e5df59fc4848795248fc34961f51 Author: Wright Feng <wright.feng@xxxxxxxxxxx> Commit: Kalle Valo <kvalo@xxxxxxxxxx> brcmfmac: Add dump_survey cfg80211 ops for HostApd AutoChannelSelectionTo enable ACS feature in Hostap daemon, dump_survey cfg80211 ops and dump obss survey command in firmware side are needed. This patch is for adding
dump_survey feature and adding DUMP_OBSS feature flag to check if firmware supports dump_obss iovar. Signed-off-by: Wright Feng <wright.feng@xxxxxxxxxxx> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@xxxxxxxxxxx> Signed-off-by: Ian Lin <ian.lin@xxxxxxxxxxxx> Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxx>Link: https://lore.kernel.org/r/20220929012527.4152-2-ian.lin@xxxxxxxxxxxx
I did comment on one of the patches in the series, but I now I looked at the diff and it is giving me the infamous eyeball cancer paraphrasing John Linville.
Maybe trying ACS?Seems it must be something like that.As these are marked as disabled user-space should not use them. What I don't understand is why these pass the cfg80211 layer so adding Johannes here.Well that goes back to my earlier observation above: dump_survey() should just dump all *available* data, not actually try to *collect* data. So if userspace requests data for a channel that's disabled, that's actually OK, but you shouldn't _have_ any data for that channel since it's disabled. Also nl80211 won't send the data out if it exists, but there's no check to see if asking the driver makes sense since if it's a channel that exists, it should be valid to ask the driver if it has data - it just shouldn't have any. The way it works in mac80211 is that survey data is collected during scan, I think?
My assumption is that our scan does not collect the survey data so this implementation initiates a dedicated scan/measurement/survey/whatever and returns the results. Might have been better to propose a scan flag so this could be initiated upon a scan request.
Anyway, I see in nl80211.c we are skipping sending up survey data for disable channels. So let me send a quick patch for avoiding the error messages and renaming the function. Thanks for your thought. Always appreciated. Best wishes for 2023.
Gr. AvS
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature