Commit 52451502 authored by Kory Maincent's avatar Kory Maincent Committed by Jakub Kicinski
Browse files

Revert "ethtool: Fix mod state of verbose no_mask bitset" 



This reverts commit 108a36d0.

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 66991703 ("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: default avatarOleksij Rempel <o.rempel@pengutronix.de>
Reported-by: default avatarMichal Kubecek <mkubecek@suse.cz>
Closes: https://lore.kernel.org/netdev/20231019095140.l6fffnszraeb6iiw@lion.mk-sys.cz/


Cc: stable@vger.kernel.org
Fixes: 108a36d0 ("ethtool: Fix mod state of verbose no_mask bitset")
Signed-off-by: default avatarKory Maincent <kory.maincent@bootlin.com>
Reviewed-by: default avatarMichal Kubecek <mkubecek@suse.cz>
Link: https://lore.kernel.org/r/20231019-feature_ptp_bitset_fix-v1-1-70f3c429a221@bootlin.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 1c1f14f9

net/ethtool/bitset.c

+6 −26
Original line number Diff line number Diff line
@@ -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,