[PATCH] wifi: wext: warn about usage only once

[PATCH] wifi: wext: warn about usage only once

[Johannes Berg]

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
 
-- 
2.39.2

Re: [PATCH] wifi: wext: warn about usage only once

[Kalle Valo]

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.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] wifi: wext: warn about usage only once

[Larry Finger]

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

Re: [PATCH] wifi: wext: warn about usage only once

[Linus Torvalds]

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

Re: [PATCH] wifi: wext: warn about usage only once

[Jakub Kicinski]

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.

Re: [PATCH] wifi: wext: warn about usage only once

[Kalle Valo]

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.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] wifi: wext: warn about usage only once

[Larry Finger]

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

Re: [PATCH] wifi: wext: warn about usage only once

[Kalle Valo]

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

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20230224135933.94104aeda1a0.Ie771c6a66d7d6c3cf67da5f3b0c66cea66fd514c@changeid/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] wifi: wext: warn about usage only once

[Kalle Valo]

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.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] wifi: wext: warn about usage only once

[Kalle Valo]

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

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20230224135933.94104aeda1a0.Ie771c6a66d7d6c3cf67da5f3b0c66cea66fd514c@changeid/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] wifi: wext: warn about usage only once

[Jakub Kicinski]

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.