netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH ethtool] ethtool: Improve compatibility between netlink and ioctl interfaces
@ 2020-11-02 18:40 Ido Schimmel
  2020-11-02 22:58 ` Michal Kubecek
  0 siblings, 1 reply; 6+ messages in thread
From: Ido Schimmel @ 2020-11-02 18:40 UTC (permalink / raw)
  To: netdev; +Cc: kuba, mkubecek, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@nvidia.com>

With the ioctl interface, when autoneg is enabled, but without
specifying speed, duplex or link modes, the advertised link modes are
set to the supported link modes by the ethtool user space utility.

This does not happen when using the netlink interface. Fix this
incompatibility problem by having ethtool query the supported link modes
from the kernel and advertise all of them when only "autoneg on" is
specified.

Before:

# ethtool -s eth0 advertise 0xC autoneg on
# ethtool -s eth0 autoneg on
# ethtool eth0
Settings for eth0:
	Supported ports: [ TP ]
	Supported link modes:   10baseT/Half 10baseT/Full
	                        100baseT/Half 100baseT/Full
	                        1000baseT/Full
	Supported pause frame use: No
	Supports auto-negotiation: Yes
	Supported FEC modes: Not reported
	Advertised link modes:  100baseT/Half 100baseT/Full
	Advertised pause frame use: No
	Advertised auto-negotiation: Yes
	Advertised FEC modes: Not reported
	Speed: 1000Mb/s
	Duplex: Full
	Auto-negotiation: on
	Port: Twisted Pair
	PHYAD: 0
	Transceiver: internal
	MDI-X: off (auto)
	Supports Wake-on: umbg
	Wake-on: d
        Current message level: 0x00000007 (7)
                               drv probe link
	Link detected: yes

After:

# ethtool -s eth0 advertise 0xC autoneg on
# ethtool -s eth0 autoneg on
# ethtool eth0
Settings for eth0:
	Supported ports: [ TP ]
	Supported link modes:   10baseT/Half 10baseT/Full
	                        100baseT/Half 100baseT/Full
	                        1000baseT/Full
	Supported pause frame use: No
	Supports auto-negotiation: Yes
	Supported FEC modes: Not reported
	Advertised link modes:  10baseT/Half 10baseT/Full
	                        100baseT/Half 100baseT/Full
	                        1000baseT/Full
	Advertised pause frame use: No
	Advertised auto-negotiation: Yes
	Advertised FEC modes: Not reported
	Speed: 1000Mb/s
	Duplex: Full
	Auto-negotiation: on
	Port: Twisted Pair
	PHYAD: 0
	Transceiver: internal
	MDI-X: on (auto)
	Supports Wake-on: umbg
	Wake-on: d
        Current message level: 0x00000007 (7)
                               drv probe link
	Link detected: yes

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
Michal / Jakub, let me know if you see a better way. Sending as RFC
since I want to run it through regression first.
---
 netlink/settings.c | 115 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)

diff --git a/netlink/settings.c b/netlink/settings.c
index 41a2e5af1945..1f856b1b14d5 100644
--- a/netlink/settings.c
+++ b/netlink/settings.c
@@ -1110,6 +1110,113 @@ static const struct param_parser sset_params[] = {
 	{}
 };
 
+static bool sset_is_autoneg_only(const struct nl_context *nlctx)
+{
+	return nlctx->argc == 2 && !strcmp(nlctx->argp[0], "autoneg") &&
+	       !strcmp(nlctx->argp[1], "on");
+}
+
+static int linkmodes_reply_adver_all_cb(const struct nlmsghdr *nlhdr,
+					void *data)
+{
+	const struct nlattr *bitset_tb[ETHTOOL_A_BITSET_MAX + 1] = {};
+	const struct nlattr *tb[ETHTOOL_A_LINKMODES_MAX + 1] = {};
+	DECLARE_ATTR_TB_INFO(bitset_tb);
+	struct nl_context *nlctx = data;
+	struct nl_msg_buff *msgbuff;
+	DECLARE_ATTR_TB_INFO(tb);
+	struct nl_socket *nlsk;
+	struct nlattr *nest;
+	int ret;
+
+	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
+	if (ret < 0)
+		return ret;
+	if (!tb[ETHTOOL_A_LINKMODES_OURS])
+		return -EINVAL;
+
+	ret = mnl_attr_parse_nested(tb[ETHTOOL_A_LINKMODES_OURS], attr_cb,
+				    &bitset_tb_info);
+	if (ret < 0)
+		return ret;
+	if (!bitset_tb[ETHTOOL_A_BITSET_SIZE] ||
+	    !bitset_tb[ETHTOOL_A_BITSET_VALUE] ||
+	    !bitset_tb[ETHTOOL_A_BITSET_MASK])
+		return -EINVAL;
+
+	ret = netlink_init_ethnl2_socket(nlctx);
+	if (ret < 0)
+		return ret;
+
+	nlsk = nlctx->ethnl2_socket;
+	msgbuff = &nlsk->msgbuff;
+
+	ret = msg_init(nlctx, msgbuff, ETHTOOL_MSG_LINKMODES_SET,
+		       NLM_F_REQUEST | NLM_F_ACK);
+	if (ret < 0)
+		return ret;
+	if (ethnla_fill_header(msgbuff, ETHTOOL_A_LINKMODES_HEADER,
+			       nlctx->devname, 0))
+		return -EMSGSIZE;
+
+	if (ethnla_put_u8(msgbuff, ETHTOOL_A_LINKMODES_AUTONEG, AUTONEG_ENABLE))
+		return -EMSGSIZE;
+
+	/* Use the size and mask from the reply and set the value to the mask,
+	 * so that all supported link modes will be advertised.
+	 */
+	ret = -EMSGSIZE;
+	nest = ethnla_nest_start(msgbuff, ETHTOOL_A_LINKMODES_OURS);
+	if (!nest)
+		return -EMSGSIZE;
+
+	if (ethnla_put_u32(msgbuff, ETHTOOL_A_BITSET_SIZE,
+			   mnl_attr_get_u32(bitset_tb[ETHTOOL_A_BITSET_SIZE])))
+		goto err;
+
+	if (ethnla_put(msgbuff, ETHTOOL_A_BITSET_VALUE,
+		       mnl_attr_get_payload_len(bitset_tb[ETHTOOL_A_BITSET_MASK]),
+		       mnl_attr_get_payload(bitset_tb[ETHTOOL_A_BITSET_MASK])))
+		goto err;
+
+	if (ethnla_put(msgbuff, ETHTOOL_A_BITSET_MASK,
+		       mnl_attr_get_payload_len(bitset_tb[ETHTOOL_A_BITSET_MASK]),
+		       mnl_attr_get_payload(bitset_tb[ETHTOOL_A_BITSET_MASK])))
+		goto err;
+
+	ethnla_nest_end(msgbuff, nest);
+
+	ret = nlsock_sendmsg(nlsk, NULL);
+	if (ret < 0)
+		return ret;
+	ret = nlsock_process_reply(nlsk, nomsg_reply_cb, nlctx);
+	if (ret < 0)
+		return ret;
+
+	return MNL_CB_OK;
+
+err:
+	ethnla_nest_cancel(msgbuff, nest);
+	return ret;
+}
+
+static int sset_adver_all(struct nl_context *nlctx)
+{
+	struct nl_socket *nlsk = nlctx->ethnl_socket;
+	int ret;
+
+	if (netlink_cmd_check(nlctx->ctx, ETHTOOL_MSG_LINKMODES_GET, false) ||
+	    netlink_cmd_check(nlctx->ctx, ETHTOOL_MSG_LINKMODES_SET, false))
+		return -EOPNOTSUPP;
+
+	ret = nlsock_prep_get_request(nlsk, ETHTOOL_MSG_LINKMODES_GET,
+				      ETHTOOL_A_LINKMODES_HEADER,
+				      ETHTOOL_FLAG_COMPACT_BITSETS);
+	if (ret < 0)
+		return ret;
+	return nlsock_send_get_request(nlsk, linkmodes_reply_adver_all_cb);
+}
+
 int nl_sset(struct cmd_context *ctx)
 {
 	struct nl_context *nlctx = ctx->nlctx;
@@ -1120,6 +1227,14 @@ int nl_sset(struct cmd_context *ctx)
 	nlctx->argc = ctx->argc;
 	nlctx->devname = ctx->devname;
 
+	/* For compatibility reasons with ioctl-based ethtool, when "autoneg
+	 * on" is specified without "advertise", "speed" and "duplex", we need
+	 * to query the supported link modes from the kernel and advertise all
+	 * of them.
+	 */
+	if (sset_is_autoneg_only(nlctx))
+		return sset_adver_all(nlctx);
+
 	ret = nl_parser(nlctx, sset_params, NULL, PARSER_GROUP_MSG);
 	if (ret < 0)
 		return 1;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-05 12:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-02 18:40 [RFC PATCH ethtool] ethtool: Improve compatibility between netlink and ioctl interfaces Ido Schimmel
2020-11-02 22:58 ` Michal Kubecek
2020-11-03 14:24   ` Ido Schimmel
2020-11-03 23:55     ` Michal Kubecek
2020-11-04 10:05       ` Ido Schimmel
2020-11-05 12:43         ` Ido Schimmel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).