Skip to content

Commit

Permalink
Revert "ethtool: Fix mod state of verbose no_mask bitset"
Browse files Browse the repository at this point in the history
This reverts commit 108a36d.

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 6699170 ("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 <o.rempel@pengutronix.de>
Reported-by: Michal Kubecek <mkubecek@suse.cz>
Closes: https://lore.kernel.org/netdev/20231019095140.l6fffnszraeb6iiw@lion.mk-sys.cz/
Cc: stable@vger.kernel.org
Fixes: 108a36d ("ethtool: Fix mod state of verbose no_mask bitset")
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
Link: https://lore.kernel.org/r/20231019-feature_ptp_bitset_fix-v1-1-70f3c429a221@bootlin.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Kory Maincent authored and Jakub Kicinski committed Oct 19, 2023
1 parent 1c1f14f commit 5245150
Showing 1 changed file with 6 additions and 26 deletions.
32 changes: 6 additions & 26 deletions net/ethtool/bitset.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand All @@ -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));
Expand All @@ -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,
Expand Down

0 comments on commit 5245150

Please sign in to comment.