All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacob Keller <jacob.e.keller@intel.com>
To: Andrew Lunn <andrew@lunn.ch>, Michal Kubecek <mkubecek@suse.cz>,
	netdev@vger.kernel.org
Cc: Jacob Keller <jacob.e.keller@intel.com>,
	Jamie Gloudon <jamie.gloudon@gmx.fr>
Subject: [ethtool] ethtool: fix netlink bitmasks when sent as NOMASK
Date: Mon, 27 Jul 2020 14:47:00 -0700	[thread overview]
Message-ID: <20200727214700.5915-1-jacob.e.keller@intel.com> (raw)

The ethtool netlink API can send bitsets without an associated bitmask.
These do not get displayed properly, because the dump_link_modes, and
bitset_get_bit to not check whether the provided bitset is a NOMASK
bitset. This results in the inability to display peer advertised link
modes.

The dump_link_modes and bitset_get_bit functions are designed so they
can print either the values or the mask. For a nomask bitmap, this
doesn't make sense. There is no mask.

Modify dump_link_modes to check ETHTOOL_A_BITSET_NOMASK. For compact
bitmaps, always check and print the ETHTOOL_A_BITSET_VALUE bits,
regardless of the request to display the mask or the value. For full
size bitmaps, the set of provided bits indicates the valid values,
without using ETHTOOL_A_BITSET_VALUE fields. Thus, do not skip printing
bits without this attribute if nomask is set. This essentially means
that dump_link_modes will treat a NOMASK bitset as having a mask
equivalent to all of its set bits.

For bitset_get_bit, also check for ETHTOOL_A_BITSET_NOMASK. For compact
bitmaps, always use ETHTOOL_A_BITSET_BIT_VALUE as in dump_link_modes.
For full bitmaps, if nomask is set, then always return true of the bit
is in the set, rather than only if it provides an
ETHTOOL_A_BITSET_BIT_VALUE. This will then correctly report the set
bits.

This fixes display of link partner advertised fields when using the
netlink API.

Reported-by: Jamie Gloudon <jamie.gloudon@gmx.fr>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 netlink/bitset.c   | 9 ++++++---
 netlink/settings.c | 8 +++++---
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/netlink/bitset.c b/netlink/bitset.c
index 130bcdb5b52c..ba5d3ea77ff7 100644
--- a/netlink/bitset.c
+++ b/netlink/bitset.c
@@ -50,6 +50,7 @@ bool bitset_get_bit(const struct nlattr *bitset, bool mask, unsigned int idx,
 	DECLARE_ATTR_TB_INFO(bitset_tb);
 	const struct nlattr *bits;
 	const struct nlattr *bit;
+	bool nomask;
 	int ret;
 
 	*retptr = 0;
@@ -57,8 +58,10 @@ bool bitset_get_bit(const struct nlattr *bitset, bool mask, unsigned int idx,
 	if (ret < 0)
 		goto err;
 
-	bits = mask ? bitset_tb[ETHTOOL_A_BITSET_MASK] :
-		      bitset_tb[ETHTOOL_A_BITSET_VALUE];
+	nomask = bitset_tb[ETHTOOL_A_BITSET_NOMASK];
+
+	bits = mask && !nomask ? bitset_tb[ETHTOOL_A_BITSET_MASK] :
+		                 bitset_tb[ETHTOOL_A_BITSET_VALUE];
 	if (bits) {
 		const uint32_t *bitmap =
 			(const uint32_t *)mnl_attr_get_payload(bits);
@@ -87,7 +90,7 @@ bool bitset_get_bit(const struct nlattr *bitset, bool mask, unsigned int idx,
 
 		my_idx = mnl_attr_get_u32(tb[ETHTOOL_A_BITSET_BIT_INDEX]);
 		if (my_idx == idx)
-			return mask || tb[ETHTOOL_A_BITSET_BIT_VALUE];
+			return mask || nomask || tb[ETHTOOL_A_BITSET_BIT_VALUE];
 	}
 
 	return false;
diff --git a/netlink/settings.c b/netlink/settings.c
index 35ba2f5dd6d5..29557653336e 100644
--- a/netlink/settings.c
+++ b/netlink/settings.c
@@ -280,9 +280,11 @@ int dump_link_modes(struct nl_context *nlctx, const struct nlattr *bitset,
 	const struct nlattr *bit;
 	bool first = true;
 	int prev = -2;
+	bool nomask;
 	int ret;
 
 	ret = mnl_attr_parse_nested(bitset, attr_cb, &bitset_tb_info);
+	nomask = bitset_tb[ETHTOOL_A_BITSET_NOMASK];
 	bits = bitset_tb[ETHTOOL_A_BITSET_BITS];
 	if (ret < 0)
 		goto err_nonl;
@@ -297,8 +299,8 @@ int dump_link_modes(struct nl_context *nlctx, const struct nlattr *bitset,
 			goto err_nonl;
 		lm_strings = global_stringset(ETH_SS_LINK_MODES,
 					      nlctx->ethnl2_socket);
-		bits = mask ? bitset_tb[ETHTOOL_A_BITSET_MASK] :
-			      bitset_tb[ETHTOOL_A_BITSET_VALUE];
+		bits = mask && !nomask ? bitset_tb[ETHTOOL_A_BITSET_MASK] :
+			                 bitset_tb[ETHTOOL_A_BITSET_VALUE];
 		ret = -EFAULT;
 		if (!bits || !bitset_tb[ETHTOOL_A_BITSET_SIZE])
 			goto err_nonl;
@@ -354,7 +356,7 @@ int dump_link_modes(struct nl_context *nlctx, const struct nlattr *bitset,
 		if (!tb[ETHTOOL_A_BITSET_BIT_INDEX] ||
 		    !tb[ETHTOOL_A_BITSET_BIT_NAME])
 			goto err;
-		if (!mask && !tb[ETHTOOL_A_BITSET_BIT_VALUE])
+		if (!mask && !nomask && !tb[ETHTOOL_A_BITSET_BIT_VALUE])
 			continue;
 
 		idx = mnl_attr_get_u32(tb[ETHTOOL_A_BITSET_BIT_INDEX]);
-- 
2.26.2


             reply	other threads:[~2020-07-27 21:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-27 21:47 Jacob Keller [this message]
2020-07-27 21:51 ` [ethtool] ethtool: fix netlink bitmasks when sent as NOMASK Jacob Keller
2020-07-27 22:02   ` Jamie Gloudon
2020-07-27 22:11 ` Andrew Lunn
2020-07-27 22:22   ` Jacob Keller
2020-07-27 22:26   ` Michal Kubecek
2020-07-27 22:32     ` Keller, Jacob E
2020-07-27 22:40       ` Michal Kubecek
2020-07-27 22:21 ` Michal Kubecek
2020-07-27 22:32   ` Jacob Keller
2020-07-27 22:53     ` Michal Kubecek
2020-07-27 23:18       ` Jacob Keller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200727214700.5915-1-jacob.e.keller@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=andrew@lunn.ch \
    --cc=jamie.gloudon@gmx.fr \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.