This reverts commit 108a36d07c01edbc5942d27c92494d1c6e4d45a0.
It was reported that this fix breaks the possibility to remove existing WoL
flags. For example:
~$ ethtool lan2
...
Supports Wake-on: pg
Wake-on: d
...
~$ ethtool -s lan2 wol gp
~$ ethtool lan2
...
Wake-on: pg
...
~$ ethtool -s lan2 wol d
~$ ethtool lan2
...
Wake-on: pg
...
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.
Reported-by: Oleksij Rempel <[email protected]>
Reported-by: Michal Kubecek <[email protected]>
Closes: https://lore.kernel.org/netdev/[email protected]/
Cc: [email protected]
Fixes: 108a36d07c01 ("ethtool: Fix mod state of verbose no_mask bitset")
Signed-off-by: Kory Maincent <[email protected]>
---
This patch is reverted for now as we are approaching the end of the
merge-window. The real fix that fix the mod value will be sent later
on the next merge-window.
---
net/ethtool/bitset.c | 32 ++++++--------------------------
1 file changed, 6 insertions(+), 26 deletions(-)
diff --git a/net/ethtool/bitset.c b/net/ethtool/bitset.c
index 883ed9be81f9..0515d6604b3b 100644
--- a/net/ethtool/bitset.c
+++ b/net/ethtool/bitset.c
@@ -431,10 +431,8 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits,
ethnl_string_array_t names,
struct netlink_ext_ack *extack, bool *mod)
{
- u32 *orig_bitmap, *saved_bitmap = NULL;
struct nlattr *bit_attr;
bool no_mask;
- bool dummy;
int rem;
int ret;
@@ -450,22 +448,8 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits,
}
no_mask = tb[ETHTOOL_A_BITSET_NOMASK];
- if (no_mask) {
- unsigned int nwords = DIV_ROUND_UP(nbits, 32);
- unsigned int nbytes = nwords * sizeof(u32);
-
- /* The bitmap size is only the size of the map part without
- * its mask part.
- */
- saved_bitmap = kcalloc(nwords, sizeof(u32), GFP_KERNEL);
- if (!saved_bitmap)
- return -ENOMEM;
- memcpy(saved_bitmap, bitmap, nbytes);
- ethnl_bitmap32_clear(bitmap, 0, nbits, &dummy);
- orig_bitmap = saved_bitmap;
- } else {
- orig_bitmap = bitmap;
- }
+ if (no_mask)
+ ethnl_bitmap32_clear(bitmap, 0, nbits, mod);
nla_for_each_nested(bit_attr, tb[ETHTOOL_A_BITSET_BITS], rem) {
bool old_val, new_val;
@@ -474,14 +458,13 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits,
if (nla_type(bit_attr) != ETHTOOL_A_BITSET_BITS_BIT) {
NL_SET_ERR_MSG_ATTR(extack, bit_attr,
"only ETHTOOL_A_BITSET_BITS_BIT allowed in ETHTOOL_A_BITSET_BITS");
- ret = -EINVAL;
- goto out;
+ return -EINVAL;
}
ret = ethnl_parse_bit(&idx, &new_val, nbits, bit_attr, no_mask,
names, extack);
if (ret < 0)
- goto out;
- old_val = orig_bitmap[idx / 32] & ((u32)1 << (idx % 32));
+ return ret;
+ old_val = bitmap[idx / 32] & ((u32)1 << (idx % 32));
if (new_val != old_val) {
if (new_val)
bitmap[idx / 32] |= ((u32)1 << (idx % 32));
@@ -491,10 +474,7 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits,
}
}
- ret = 0;
-out:
- kfree(saved_bitmap);
- return ret;
+ return 0;
}
static int ethnl_compact_sanity_checks(unsigned int nbits,
---
base-commit: a602ee3176a81280b829c9f0cf259450f7982168
change-id: 20231019-feature_ptp_bitset_fix-6bf75671b33e
Best regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
On Thu, Oct 19, 2023 at 03:16:41PM +0200, Kory Maincent wrote:
> This reverts commit 108a36d07c01edbc5942d27c92494d1c6e4d45a0.
>
> It was reported that this fix breaks the possibility to remove existing WoL
> flags. For example:
> ~$ ethtool lan2
> ...
> Supports Wake-on: pg
> Wake-on: d
> ...
> ~$ ethtool -s lan2 wol gp
> ~$ ethtool lan2
> ...
> Wake-on: pg
> ...
> ~$ ethtool -s lan2 wol d
> ~$ ethtool lan2
> ...
> Wake-on: pg
> ...
>
> 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.
>
> Reported-by: Oleksij Rempel <[email protected]>
> Reported-by: Michal Kubecek <[email protected]>
> Closes: https://lore.kernel.org/netdev/[email protected]/
> Cc: [email protected]
> Fixes: 108a36d07c01 ("ethtool: Fix mod state of verbose no_mask bitset")
> Signed-off-by: Kory Maincent <[email protected]>
Reviewed-by: Michal Kubecek <[email protected]>
> ---
>
> This patch is reverted for now as we are approaching the end of the
> merge-window. The real fix that fix the mod value will be sent later
> on the next merge-window.
> ---
For the record, the term "merge window" is used for the 2-week interval
between a final and following rc1, not for the whole interval between
two final releases.
Michal
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
On Thu, 19 Oct 2023 15:16:41 +0200 you wrote:
> This reverts commit 108a36d07c01edbc5942d27c92494d1c6e4d45a0.
>
> It was reported that this fix breaks the possibility to remove existing WoL
> flags. For example:
> ~$ ethtool lan2
> ...
> Supports Wake-on: pg
> Wake-on: d
> ...
> ~$ ethtool -s lan2 wol gp
> ~$ ethtool lan2
> ...
> Wake-on: pg
> ...
> ~$ ethtool -s lan2 wol d
> ~$ ethtool lan2
> ...
> Wake-on: pg
> ...
>
> [...]
Here is the summary with links:
- [net] Revert "ethtool: Fix mod state of verbose no_mask bitset"
https://git.kernel.org/netdev/net/c/524515020f25
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html