Re: [PATCH net v1 1/1] ethtool: fix clearing of WoL flags

From: Florian Fainelli
Date: Thu Oct 19 2023 - 13:34:57 EST


On 10/19/23 09:45, Michal Kubecek wrote:
On Thu, Oct 19, 2023 at 09:20:31AM -0700, Florian Fainelli wrote:
On 10/19/23 06:27, Köry Maincent wrote:
On Thu, 19 Oct 2023 12:50:48 +0200
Michal Kubecek <mkubecek@xxxxxxx> wrote:

On Thu, Oct 19, 2023 at 12:21:14PM +0200, Köry Maincent wrote:
On Thu, 19 Oct 2023 11:51:40 +0200 > Michal Kubecek <mkubecek@xxxxxxx>
wrote:

The issue was indeed introduced by commit 108a36d07c01 ("ethtool: Fix
mod state of verbose no_mask bitset"). The problem is that a "no mask"
verbose bitset only contains bit attributes for bits to be set. This
worked correctly before this commit because we were always updating
a zero bitmap (since commit 6699170376ab ("ethtool: fix application of
verbose no_mask bitset"), that is) so that the rest was left zero
naturally. But now the 1->0 change (old_val is true, bit not present in
netlink nest) no longer works.

Doh I had not seen this issue! Thanks you for reporting it.
I will send the revert then and will update the fix for next merge-window.

Something like the diff below (against current mainline) might do the
trick but it's just an idea, not even build tested.

Seems a good idea without adding too much complexity to the code.
Will try that and send it in next merge window.

Not sure what you mean by next merge window, we need a fix for right now, or
we need to revert 6699170376ab ("ethtool: fix application of verbose no_mask
bitset").

Not that one, that's an old commit that was correct from functional
point of view (the only problem was that it sometimes triggered
a notification even when the value was not changed but that also happens
in other places).

A revert of commit 108a36d07c01 ("ethtool: Fix mod state of verbose
no_mask bitset") is already in net tree as commit 524515020f25 ("Revert
"ethtool: Fix mod state of verbose no_mask bitset"").

Got it, thanks!
--
Florian