| Message ID | 20230224135933.94104aeda1a0.Ie771c6a66d7d6c3cf67da5f3b0c66cea66fd514c@changeid (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | wifi: wext: warn about usage only once | expand |
Johannes Berg <johannes@sipsolutions.net> writes: > From: Johannes Berg <johannes.berg@intel.com> > > Warn only once since the ratelimit parameters are still > allowing too many messages to happen. This will no longer > tell you all the different processes, but still gives a > heads-up of sorts. > > Also modify the message to note that wext stops working > for future Wi-Fi 7 hardware, this is already implemented > in commit 4ca69027691a ("wifi: wireless: deny wireless > extensions on MLO-capable devices") and is maybe of more > relevance to users than the fact that we'd like to have > wireless extensions deprecated. > > The issue with Wi-Fi 7 is that you can now have multiple > connections to the same AP, so a whole bunch of things > now become per link rather than per netdev, which can't > really be handled in wireless extensions. > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> Linus, do you want to apply this directly or should we send this normally via the wireless tree? For the latter I would assume you would get it sometime next week.
On 2/24/23 06:59, Johannes Berg wrote: > From: Johannes Berg <johannes.berg@intel.com> > > Warn only once since the ratelimit parameters are still > allowing too many messages to happen. This will no longer > tell you all the different processes, but still gives a > heads-up of sorts. > > Also modify the message to note that wext stops working > for future Wi-Fi 7 hardware, this is already implemented > in commit 4ca69027691a ("wifi: wireless: deny wireless > extensions on MLO-capable devices") and is maybe of more > relevance to users than the fact that we'd like to have > wireless extensions deprecated. > > The issue with Wi-Fi 7 is that you can now have multiple > connections to the same AP, so a whole bunch of things > now become per link rather than per netdev, which can't > really be handled in wireless extensions. > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > --- > Not really sure I see a better solution ... > > - tracking it per task would be nice in a way I guess, > but is also awful; > - adjusting the rate limit will lead us into an endless > bikeshedding discussion about the parameters; > - removing the warning will leave us with no indiciation > of what happens with Wi-Fi 7 hardware, although most of > the processes using them now (like Chrome browser??) > probably ignore failures from it > - trying to support a 30+ year old technology on modern > Wi-Fi 7 hardware will be "interesting" and lead to all > kinds of hacks there > --- > net/wireless/wext-core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c > index 13a72b17248e..a125fd1fa134 100644 > --- a/net/wireless/wext-core.c > +++ b/net/wireless/wext-core.c > @@ -641,8 +641,8 @@ static void wireless_warn_cfg80211_wext(void) > { > char name[sizeof(current->comm)]; > > - pr_warn_ratelimited("warning: `%s' uses wireless extensions that are deprecated for modern drivers; use nl80211\n", > - get_task_comm(name, current)); > + pr_warn_once("warning: `%s' uses wireless extensions which will stop working for Wi-Fi 7 hardware; use nl80211\n", > + get_task_comm(name, current)); > } > #endif > Johannes, Although this patch will stop the log spamming that I see, it will not provide much information upstream for fixing the problems. Even if this patch is applied to the kernel, I plan to keep my local, once per task, patch so that I can keep upstream informed, and perhaps get fixes applied there. Larry
On Fri, Feb 24, 2023 at 7:45 AM Larry Finger <Larry.Finger@lwfinger.net> wrote: > > Although this patch will stop the log spamming that I see, it will not provide > much information upstream for fixing the problems. Generally, I think we've been happy with the "warn once". Does it mean that if *multiple* different programs do this, we only get a warning for the first one? Yes. But *users* don't actually care. They can't do much about it. And the people who *can* do things about it - ie the distro maintainers - generally just need to be aware of, and fix, just one or two cases anyway. If there are many more than that, then the warning is bogus anyway, because deprecation is clearly the wrong thing. So I personally think that "warn once" is much better than the "keep some big array of names around" so that you can warn multiple times patch I saw flying past. That said, I would *not* object to "keep a single word of hashed bits" around kind of model. In fact, I've wanted something like that for other situations. So it might be interestring to have a "warn_once_per_comm()" thing, that basically has a u32-sized "these hashes have been warned about", and it does something like - hash current 'comm[]' contents - if the hash bit is not set in that single word, then warn, and set the bit. IOW, it would turn a "warn_once()" into a "warn at most 32 times, and at most once per comm[]" thing. And it would use no more space for the "did I already warn" than just a plain "warn_once()" does. The hash doesn't need to be all that smart, and we would want it to be fairly low-overhead. No need to make it some complicated cryptographically secure hash. It could literally be something like u32 hash_comm(struct task_struct *t, unsigned int bits) { /* No need for locking, we're just hashing anyway */ u32 val = READ_ONCE(*(u32 *)t->comm); return hash_32(val, bits); } and then the "pr_warn_once_per_comm()" would do something like static u32 warned_mask; u32 mask = hash_comm(current, 5); if ((mask & warned_mask) == 0) { warned_mask |= mask; // thread data race - we don't care pr_warn(..) } which is all fairly simple and straightforward and not horribly inefficient. You *could* improve on it further by having some kind of timed rate-limiting, where every 24 hours you'd clear the warning mask, so that you'd warn about these things once a day. That *can* be useful for when people just don't notice the warning the first time around, and "once a day" is not a horribly problem that fills up the logs like the current situation does. But again - I personally think even just a pr_warn_once() is likely good enough. Because all I want is to not have that horrible log-flushing behavior. Linus
On Fri, 24 Feb 2023 17:03:38 +0200 Kalle Valo wrote: > Linus, do you want to apply this directly or should we send this > normally via the wireless tree? For the latter I would assume you would > get it sometime next week. FWIW the net PR will likely be on Monday afternoon, pending this fix and the Kconfig fix from Intel.
Jakub Kicinski <kuba@kernel.org> writes: > On Fri, 24 Feb 2023 17:03:38 +0200 Kalle Valo wrote: >> Linus, do you want to apply this directly or should we send this >> normally via the wireless tree? For the latter I would assume you would >> get it sometime next week. > > FWIW the net PR will likely be on Monday afternoon, pending this fix > and the Kconfig fix from Intel. Ok, I'll try to send a pull request before that. I also have other pending fixes in the queue.
On 2/24/23 13:44, Linus Torvalds wrote: > You *could* improve on it further by having some kind of timed > rate-limiting, where every 24 hours you'd clear the warning mask, so > that you'd warn about these things once a day. That *can* be useful > for when people just don't notice the warning the first time around, > and "once a day" is not a horribly problem that fills up the logs like > the current situation does. > > But again - I personally think even just a pr_warn_once() is likely > good enough. Because all I want is to not have that horrible > log-flushing behavior. To all, I posted my list of 8 different tasks that generated this warning to the openSUSE developers mailing list, and got back a reply from Jan Engelhardt pointed me toward the libqt5-qtbase project, which contains the following snippet: case ARPHRD_ETHER: // check if it's a WiFi interface if (qt_safe_ioctl(socket, SIOCGIWMODE, req) >= 0) return QNetworkInterface::Wifi; return QNetworkInterface::Ethernet; I am not entirely sure why Qt needs to know what type of device the network is using. I tested by replacing this with case ARPHRD_ETHER: return QNetworkInterface::Ethernet; After rebuilding the entire project, and reinstalling all 31 packages generated in a new build, my system now displays only 3 remaining warnings, namely warning: `nspr-2' uses wireless extensions that are deprecated for modern drivers; use nl80211 warning: `ThreadPoolForeg' uses wireless extensions that are deprecated for modern drivers; use nl80211 warning: `nspr-8' uses wireless extensions that are deprecated for modern drivers; use nl80211 To answer Kalle's question, libQt is responsible for most of the warnings that were reported here. In case Qt really needs to know what network it is on, what is a better way to detect if the network is on a Wifi device? Larry
Johannes Berg <johannes@sipsolutions.net> wrote: > From: Johannes Berg <johannes.berg@intel.com> > > Warn only once since the ratelimit parameters are still > allowing too many messages to happen. This will no longer > tell you all the different processes, but still gives a > heads-up of sorts. > > Also modify the message to note that wext stops working > for future Wi-Fi 7 hardware, this is already implemented > in commit 4ca69027691a ("wifi: wireless: deny wireless > extensions on MLO-capable devices") and is maybe of more > relevance to users than the fact that we'd like to have > wireless extensions deprecated. > > The issue with Wi-Fi 7 is that you can now have multiple > connections to the same AP, so a whole bunch of things > now become per link rather than per netdev, which can't > really be handled in wireless extensions. > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> Patch applied to wireless-next.git, thanks. 35c2dcbb64d4 wifi: wext: warn about usage only once
Kalle Valo <kvalo@kernel.org> writes: > Johannes Berg <johannes@sipsolutions.net> wrote: > >> From: Johannes Berg <johannes.berg@intel.com> >> >> Warn only once since the ratelimit parameters are still >> allowing too many messages to happen. This will no longer >> tell you all the different processes, but still gives a >> heads-up of sorts. >> >> Also modify the message to note that wext stops working >> for future Wi-Fi 7 hardware, this is already implemented >> in commit 4ca69027691a ("wifi: wireless: deny wireless >> extensions on MLO-capable devices") and is maybe of more >> relevance to users than the fact that we'd like to have >> wireless extensions deprecated. >> >> The issue with Wi-Fi 7 is that you can now have multiple >> connections to the same AP, so a whole bunch of things >> now become per link rather than per netdev, which can't >> really be handled in wireless extensions. >> >> Signed-off-by: Johannes Berg <johannes.berg@intel.com> > > Patch applied to wireless-next.git, thanks. > > 35c2dcbb64d4 wifi: wext: warn about usage only once Oops, wrong tree. Please ignore the mail, I'll take this into wireless tree.
Johannes Berg <johannes@sipsolutions.net> wrote: > From: Johannes Berg <johannes.berg@intel.com> > > Warn only once since the ratelimit parameters are still > allowing too many messages to happen. This will no longer > tell you all the different processes, but still gives a > heads-up of sorts. > > Also modify the message to note that wext stops working > for future Wi-Fi 7 hardware, this is already implemented > in commit 4ca69027691a ("wifi: wireless: deny wireless > extensions on MLO-capable devices") and is maybe of more > relevance to users than the fact that we'd like to have > wireless extensions deprecated. > > The issue with Wi-Fi 7 is that you can now have multiple > connections to the same AP, so a whole bunch of things > now become per link rather than per netdev, which can't > really be handled in wireless extensions. > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> Patch applied to wireless.git, thanks. 52fd90638a72 wifi: wext: warn about usage only once
On Sat, 25 Feb 2023 20:09:51 -0600 Larry Finger wrote: > In case Qt really needs to know what network it is on, what is a better way to > detect if the network is on a Wifi device? Presupposing lack of love for netlink - /sys/class/net/$ifc/uevent will have DEVTYPE=wlan.
diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c index 13a72b17248e..a125fd1fa134 100644 --- a/net/wireless/wext-core.c +++ b/net/wireless/wext-core.c @@ -641,8 +641,8 @@ static void wireless_warn_cfg80211_wext(void) { char name[sizeof(current->comm)]; - pr_warn_ratelimited("warning: `%s' uses wireless extensions that are deprecated for modern drivers; use nl80211\n", - get_task_comm(name, current)); + pr_warn_once("warning: `%s' uses wireless extensions which will stop working for Wi-Fi 7 hardware; use nl80211\n", + get_task_comm(name, current)); } #endif