| Message ID | 20231019-feature_ptp_bitset_fix-v1-1-70f3c429a221@bootlin.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 524515020f2552759a7ef1c9d03e7dac9b1ff3c2 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] Revert "ethtool: Fix mod state of verbose no_mask bitset" | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/apply | success | Patch already applied to net |
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 <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: 108a36d07c01 ("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> > --- > > 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 <kuba@kernel.org>: 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!
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,
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 <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: 108a36d07c01 ("ethtool: Fix mod state of verbose no_mask bitset") Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> --- 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(-) --- base-commit: a602ee3176a81280b829c9f0cf259450f7982168 change-id: 20231019-feature_ptp_bitset_fix-6bf75671b33e Best regards,