Hello community, here is the log from the commit of package ethtool for openSUSE:Factory checked in at 2020-05-17 23:42:39 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/ethtool (Old) and /work/SRC/openSUSE:Factory/.ethtool.new.2738 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "ethtool" Sun May 17 23:42:39 2020 rev:61 rq:805937 version:5.6 Changes: -------- --- /work/SRC/openSUSE:Factory/ethtool/ethtool.changes 2020-01-23 16:10:11.603610216 +0100 +++ /work/SRC/openSUSE:Factory/.ethtool.new.2738/ethtool.changes 2020-05-17 23:42:43.461004189 +0200 @@ -1,0 +2,34 @@ +Fri May 15 20:12:57 UTC 2020 - Michal Kubecek <[email protected]> + +- update upstream references: + features-accept-long-legacy-flag-names-when-setting-.patch + netlink-show-netlink-error-even-without-extack.patch + netlink-use-genetlink-ops-information-to-decide-abou.patch + refactor-interface-between-ioctl-and-netlink-code.patch + +------------------------------------------------------------------- +Tue May 12 21:27:12 UTC 2020 - Michal Kubecek <[email protected]> + +- add fixes and improvements originally intended for 5.6: + netlink-show-netlink-error-even-without-extack.patch + features-accept-long-legacy-flag-names-when-setting-.patch + refactor-interface-between-ioctl-and-netlink-code.patch + netlink-use-genetlink-ops-information-to-decide-abou.patch + +------------------------------------------------------------------- +Tue May 12 21:08:55 UTC 2020 - Michal Kubecek <[email protected]> + +- Update to new upstream release 5.6 + * basic infrastructure for netlink kernel interface + * netlink: support "ethtool <dev>" + * netlink: support "ethtool -s <dev> ..." + * netlink: support "ethtool -P <dev>" + * netlink: monitor support + * add --debug option + * netlink: pretty printing of netlink messages +- add libmnl build time dependency +- fix SLE12 build: + netlink-fix-build-warnings.patch +- minor specfile cleanup + +------------------------------------------------------------------- Old: ---- ethtool-5.4.tar.sign ethtool-5.4.tar.xz New: ---- ethtool-5.6.tar.sign ethtool-5.6.tar.xz features-accept-long-legacy-flag-names-when-setting-.patch netlink-fix-build-warnings.patch netlink-show-netlink-error-even-without-extack.patch netlink-use-genetlink-ops-information-to-decide-abou.patch refactor-interface-between-ioctl-and-netlink-code.patch ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ ethtool.spec ++++++ --- /var/tmp/diff_new_pack.I4WGDJ/_old 2020-05-17 23:42:45.557008397 +0200 +++ /var/tmp/diff_new_pack.I4WGDJ/_new 2020-05-17 23:42:45.557008397 +0200 @@ -1,7 +1,7 @@ # # spec file for package ethtool # -# Copyright (c) 2020 SUSE LINUX GmbH, Nuernberg, Germany. +# Copyright (c) 2020 SUSE LLC # # All modifications and additions to the file contributed by third parties # remain the property of their copyright owners, unless otherwise agreed @@ -17,19 +17,27 @@ Name: ethtool -Version: 5.4 +Version: 5.6 Release: 0 Summary: Utility for examining and tuning Ethernet-based network interfaces License: GPL-2.0-only Group: Productivity/Networking/Diagnostic -Url: http://kernel.org/pub/software/network/ethtool/ +URL: https://www.kernel.org/pub/software/network/ethtool/ #Git-Clone: git://git.kernel.org/pub/scm/network/ethtool/ethtool -Source: http://kernel.org/pub/software/network/ethtool/%{name}-%{version}.tar.xz -Source2: http://kernel.org/pub/software/network/ethtool/%{name}-%{version}.tar.sign +Source: https://www.kernel.org/pub/software/network/ethtool/%{name}-%{version}.tar.xz +Source2: https://www.kernel.org/pub/software/network/ethtool/%{name}-%{version}.tar.sign Source3: %{name}.keyring BuildRoot: %{_tmppath}/%{name}-%{version}-build +BuildRequires: pkgconfig BuildRequires: xz +BuildRequires: pkgconfig(libmnl) + +Patch1: netlink-fix-build-warnings.patch +Patch2: netlink-show-netlink-error-even-without-extack.patch +Patch3: features-accept-long-legacy-flag-names-when-setting-.patch +Patch4: refactor-interface-between-ioctl-and-netlink-code.patch +Patch5: netlink-use-genetlink-ops-information-to-decide-abou.patch %description Ethtool is a small utility for examining and tuning ethernet-based @@ -37,6 +45,11 @@ %prep %setup -q +%patch1 -p1 +%patch2 -p1 +%patch3 -p1 +%patch4 -p1 +%patch5 -p1 %build export CFLAGS="%optflags -W -Wall -Wstrict-prototypes -Wformat-security -Wpointer-arith -Wno-missing-field-initializers" ++++++ ethtool-5.4.tar.xz -> ethtool-5.6.tar.xz ++++++ ++++ 14641 lines of diff (skipped) ++++++ features-accept-long-legacy-flag-names-when-setting-.patch ++++++ From: Michal Kubecek <[email protected]> Date: Mon, 13 Apr 2020 11:46:42 +0200 Subject: features: accept long legacy flag names when setting features Patch-mainline: v5.7 Git-commit: 4b1fa2c2d250441f83b90f9410cd6ce7d879c2fe The legacy feature flags have long names (e.g. "generic-receive-offload") and short names (e.g. "gro"). While "ethtool -k" shows only long names, "ethtool -K" accepts only short names. This is a bit confusing as users have to resort to documentation to see what flag name to use; in particular, if a legacy flag corresponds to only one actual kernel feature, "ethtool -k" shows the output in the same form as if long flag name were a kernel feature name but this name cannot be used to set the flag/feature. Accept both short and long legacy flag names in "ethool -K". Reported-by: Konstantin Kharlamov <[email protected]> Signed-off-by: Michal Kubecek <[email protected]> --- ethtool.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) --- a/ethtool.c +++ b/ethtool.c @@ -2297,26 +2297,33 @@ static int do_sfeatures(struct cmd_context *ctx) /* Generate cmdline_info for legacy flags and kernel-named * features, and parse our arguments. */ - cmdline_features = calloc(ARRAY_SIZE(off_flag_def) + defs->n_features, + cmdline_features = calloc(2 * ARRAY_SIZE(off_flag_def) + + defs->n_features, sizeof(cmdline_features[0])); if (!cmdline_features) { perror("Cannot parse arguments"); rc = 1; goto err; } - for (i = 0; i < ARRAY_SIZE(off_flag_def); i++) + j = 0; + for (i = 0; i < ARRAY_SIZE(off_flag_def); i++) { flag_to_cmdline_info(off_flag_def[i].short_name, off_flag_def[i].value, &off_flags_wanted, &off_flags_mask, - &cmdline_features[i]); + &cmdline_features[j++]); + flag_to_cmdline_info(off_flag_def[i].long_name, + off_flag_def[i].value, + &off_flags_wanted, &off_flags_mask, + &cmdline_features[j++]); + } for (i = 0; i < defs->n_features; i++) flag_to_cmdline_info( defs->def[i].name, FEATURE_FIELD_FLAG(i), &FEATURE_WORD(efeatures->features, i, requested), &FEATURE_WORD(efeatures->features, i, valid), - &cmdline_features[ARRAY_SIZE(off_flag_def) + i]); + &cmdline_features[j++]); parse_generic_cmdline(ctx, &any_changed, cmdline_features, - ARRAY_SIZE(off_flag_def) + defs->n_features); + 2 * ARRAY_SIZE(off_flag_def) + defs->n_features); free(cmdline_features); if (!any_changed) { ++++++ netlink-fix-build-warnings.patch ++++++ From: Michal Kubecek <[email protected]> Date: Tue, 12 May 2020 22:10:19 +0200 Subject: netlink: fix build warnings Patch-mainline: Not yet, going to submit for 5.7 cycle Depending on compiler version and options, some of these warnings may result in build failure. - gcc 4.8 wants __KERNEL_DIV_ROUND_UP defined before including ethtool.h - avoid pointer arithmetic on void * - do not use printf(s) if string is not a string literal Signed-off-by: Michal Kubecek <[email protected]> --- netlink/desc-ethtool.c | 2 +- netlink/parser.c | 2 +- netlink/settings.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) --- a/netlink/desc-ethtool.c +++ b/netlink/desc-ethtool.c @@ -4,9 +4,9 @@ * Descriptions of ethtool netlink messages and attributes for pretty print. */ +#include "../internal.h" #include <linux/ethtool_netlink.h> -#include "../internal.h" #include "prettymsg.h" static const struct pretty_nla_desc __header_desc[] = { --- a/netlink/parser.c +++ b/netlink/parser.c @@ -1016,7 +1016,7 @@ int nl_parser(struct nl_context *nlctx, const struct param_parser *params, buff = tmp_buff_find(buffs, parser->group); msgbuff = buff ? &buff->msgbuff : &nlsk->msgbuff; - param_dest = dest ? (dest + parser->dest_offset) : NULL; + param_dest = dest ? ((char *)dest + parser->dest_offset) : NULL; ret = parser->handler(nlctx, parser->type, parser->handler_data, msgbuff, param_dest); if (ret < 0) --- a/netlink/settings.c +++ b/netlink/settings.c @@ -375,7 +375,7 @@ static int dump_link_modes(struct nl_context *nlctx, after: if (first && if_none) printf("%s", if_none); - printf(after); + fputs(after, stdout); return 0; err: ++++++ netlink-show-netlink-error-even-without-extack.patch ++++++ From: Michal Kubecek <[email protected]> Date: Tue, 10 Mar 2020 13:24:55 +0100 Subject: netlink: show netlink error even without extack Patch-mainline: v5.7 Git-commit: 684f3e6c31bd096ef50725267957e0423b7932c6 Even if the NLMSG_ERROR message has no extack (NLM_F_ACK_TLVS not set, i.e. no error/warning message and bad attribute offset), we still want to display the error code (unless suppressed) and, if pretty printing is enabled, the embedded client message (if present). Fixes: 50efb3cdd2bb ("netlink: netlink socket wrapper and helpers") Signed-off-by: Michal Kubecek <[email protected]> --- netlink/nlsock.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) --- a/netlink/nlsock.c +++ b/netlink/nlsock.c @@ -173,25 +173,25 @@ static int nlsock_process_ack(struct nlmsghdr *nlhdr, ssize_t len, { const struct nlattr *tb[NLMSGERR_ATTR_MAX + 1] = {}; DECLARE_ATTR_TB_INFO(tb); + unsigned int err_offset = 0; unsigned int tlv_offset; struct nlmsgerr *nlerr; bool silent; - if (len < NLMSG_HDRLEN + sizeof(*nlerr)) + if ((len < NLMSG_HDRLEN + sizeof(*nlerr)) || (len < nlhdr->nlmsg_len)) return -EFAULT; nlerr = mnl_nlmsg_get_payload(nlhdr); - silent = (!(nlhdr->nlmsg_flags & NLM_F_ACK_TLVS) || - suppress_nlerr >= 2 || - (suppress_nlerr && nlerr->error == -EOPNOTSUPP)); - if (silent) - goto out; + silent = suppress_nlerr >= 2 || + (suppress_nlerr && nlerr->error == -EOPNOTSUPP); + if (silent || !(nlhdr->nlmsg_flags & NLM_F_ACK_TLVS)) + goto tlv_done; tlv_offset = sizeof(*nlerr); if (!(nlhdr->nlmsg_flags & NLM_F_CAPPED)) tlv_offset += MNL_ALIGN(mnl_nlmsg_get_payload_len(&nlerr->msg)); - if (mnl_attr_parse(nlhdr, tlv_offset, attr_cb, &tb_info) < 0) - goto out; + goto tlv_done; + if (tb[NLMSGERR_ATTR_MSG]) { const char *msg = mnl_attr_get_str(tb[NLMSGERR_ATTR_MSG]); @@ -202,24 +202,21 @@ static int nlsock_process_ack(struct nlmsghdr *nlhdr, ssize_t len, mnl_attr_get_u32(tb[NLMSGERR_ATTR_OFFS])); fputc('\n', stderr); } + if (tb[NLMSGERR_ATTR_OFFS]) + err_offset = mnl_attr_get_u32(tb[NLMSGERR_ATTR_OFFS]); - if (nlerr->error && pretty) { - unsigned int err_offset = 0; - - if (tb[NLMSGERR_ATTR_OFFS]) - err_offset = mnl_attr_get_u32(tb[NLMSGERR_ATTR_OFFS]); +tlv_done: + if (nlerr->error && !silent) { + errno = -nlerr->error; + perror("netlink error"); + } + if (pretty && !(nlhdr->nlmsg_flags & NLM_F_CAPPED) && + nlhdr->nlmsg_len >= NLMSG_HDRLEN + nlerr->msg.nlmsg_len) { fprintf(stderr, "offending message%s:\n", err_offset ? " and attribute" : ""); pretty_print_genlmsg(&nlerr->msg, ethnl_umsg_desc, ethnl_umsg_n_desc, err_offset); } - -out: - if (nlerr->error) { - errno = -nlerr->error; - if (!silent) - perror("netlink error"); - } return nlerr->error; } ++++++ netlink-use-genetlink-ops-information-to-decide-abou.patch ++++++ From: Michal Kubecek <[email protected]> Date: Mon, 13 Apr 2020 17:39:32 +0200 Subject: netlink: use genetlink ops information to decide about fallback Patch-mainline: v5.7 Git-commit: 6c19c0d559c80824383c5f0e2dc229878d9b19f5 Currently ethtool falls back to ioctl when netlink socket initialization fails or if netlink request fails with -EOPNOTSUPP (and we do not know that request cannot possibly work via ioctl, e.g. because of wildcard device name or device name longer than IFNAMSIZ). This logic has one problem: we cannot distinguish if -EOPNOTSUPP error code means that subcommand as such is not implemented in kernel or that it failed for some other reason (e.g. specific combination of parameters is not supported by driver). If the latter is the case, fallback to ioctl is pointless as the same request would fail via ioctl as well. In some cases we can even get a duplicate error message. Fortunately, genetlink provides information about supported family commands in the form of CTRL_ATTR_OPS nested attribute. Parse this information when available and use it to only fall back to ioctl only if kernel support for netlink request as such is missing so that there is a chance that ioctl request will work. In such case, do not send netlink request at all. Signed-off-by: Michal Kubecek <[email protected]> --- netlink/netlink.c | 138 ++++++++++++++++++++++++++++++++++++--------- netlink/netlink.h | 5 ++ netlink/parser.c | 7 +++ netlink/settings.c | 7 +++ 4 files changed, 129 insertions(+), 28 deletions(-) --- a/netlink/netlink.c +++ b/netlink/netlink.c @@ -92,16 +92,88 @@ int get_dev_info(const struct nlattr *nest, int *ifindex, char *ifname) return 0; } +/** + * netlink_cmd_check() - check support for netlink command + * @ctx: ethtool command context + * @cmd: netlink command id + * @devname: device name from user + * @allow_wildcard: wildcard dumps supported + * + * Check if command @cmd is known to be unsupported based on ops information + * from genetlink family id request. Set nlctx->ioctl_fallback if ethtool + * should fall back to ioctl, i.e. when we do not know in advance that + * netlink request is supported. Set nlctx->wildcard_unsupported if "*" was + * used as device name but the request does not support wildcards (on either + * side). + * + * Return: true if we know the netlink request is not supported and should + * fail (and possibly fall back) without actually sending it to kernel. + */ +bool netlink_cmd_check(struct cmd_context *ctx, unsigned int cmd, + bool allow_wildcard) +{ + bool is_dump = !strcmp(ctx->devname, WILDCARD_DEVNAME); + uint32_t cap = is_dump ? GENL_CMD_CAP_DUMP : GENL_CMD_CAP_DO; + struct nl_context *nlctx = ctx->nlctx; + + if (is_dump && !allow_wildcard) { + nlctx->wildcard_unsupported = true; + return true; + } + if (!nlctx->ops_flags) { + nlctx->ioctl_fallback = true; + return false; + } + if (cmd > ETHTOOL_MSG_USER_MAX || !nlctx->ops_flags[cmd]) { + nlctx->ioctl_fallback = true; + return true; + } + + if (is_dump && !(nlctx->ops_flags[cmd] & GENL_CMD_CAP_DUMP)) + nlctx->wildcard_unsupported = true; + + return !(nlctx->ops_flags[cmd] & cap); +} + /* initialization */ -struct fam_info { - const char *fam_name; - const char *grp_name; - uint16_t fam_id; - uint32_t grp_id; -}; +static int genl_read_ops(struct nl_context *nlctx, + const struct nlattr *ops_attr) +{ + struct nlattr *op_attr; + uint32_t *ops_flags; + int ret; + + ops_flags = calloc(__ETHTOOL_MSG_USER_CNT, sizeof(ops_flags[0])); + if (!ops_flags) + return -ENOMEM; + + mnl_attr_for_each_nested(op_attr, ops_attr) { + const struct nlattr *tb[CTRL_ATTR_OP_MAX + 1] = {}; + DECLARE_ATTR_TB_INFO(tb); + uint32_t op_id; + + ret = mnl_attr_parse_nested(op_attr, attr_cb, &tb_info); + if (ret < 0) + goto err; + + if (!tb[CTRL_ATTR_OP_ID] || !tb[CTRL_ATTR_OP_FLAGS]) + continue; + op_id = mnl_attr_get_u32(tb[CTRL_ATTR_OP_ID]); + if (op_id >= __ETHTOOL_MSG_USER_CNT) + continue; + + ops_flags[op_id] = mnl_attr_get_u32(tb[CTRL_ATTR_OP_FLAGS]); + } + + nlctx->ops_flags = ops_flags; + return 0; +err: + free(ops_flags); + return ret; +} -static void find_mc_group(struct nlattr *nest, struct fam_info *info) +static void find_mc_group(struct nl_context *nlctx, struct nlattr *nest) { const struct nlattr *grp_tb[CTRL_ATTR_MCAST_GRP_MAX + 1] = {}; DECLARE_ATTR_TB_INFO(grp_tb); @@ -116,9 +188,9 @@ static void find_mc_group(struct nlattr *nest, struct fam_info *info) !grp_tb[CTRL_ATTR_MCAST_GRP_ID]) continue; if (strcmp(mnl_attr_get_str(grp_tb[CTRL_ATTR_MCAST_GRP_NAME]), - info->grp_name)) + ETHTOOL_MCGRP_MONITOR_NAME)) continue; - info->grp_id = + nlctx->ethnl_mongrp = mnl_attr_get_u32(grp_tb[CTRL_ATTR_MCAST_GRP_ID]); return; } @@ -126,16 +198,21 @@ static void find_mc_group(struct nlattr *nest, struct fam_info *info) static int family_info_cb(const struct nlmsghdr *nlhdr, void *data) { - struct fam_info *info = data; + struct nl_context *nlctx = data; struct nlattr *attr; + int ret; mnl_attr_for_each(attr, nlhdr, GENL_HDRLEN) { switch (mnl_attr_get_type(attr)) { case CTRL_ATTR_FAMILY_ID: - info->fam_id = mnl_attr_get_u16(attr); + nlctx->ethnl_fam = mnl_attr_get_u16(attr); break; + case CTRL_ATTR_OPS: + ret = genl_read_ops(nlctx, attr); + if (ret < 0) + return MNL_CB_ERROR; case CTRL_ATTR_MCAST_GROUPS: - find_mc_group(attr, info); + find_mc_group(nlctx, attr); break; } } @@ -144,41 +221,37 @@ static int family_info_cb(const struct nlmsghdr *nlhdr, void *data) } #ifdef TEST_ETHTOOL -static int get_genl_family(struct nl_socket *nlsk, struct fam_info *info) +static int get_genl_family(struct nl_context *nlctx, struct nl_socket *nlsk) { return 0; } #else -static int get_genl_family(struct nl_socket *nlsk, struct fam_info *info) +static int get_genl_family(struct nl_context *nlctx, struct nl_socket *nlsk) { struct nl_msg_buff *msgbuff = &nlsk->msgbuff; int ret; - nlsk->nlctx->suppress_nlerr = 2; + nlctx->suppress_nlerr = 2; ret = __msg_init(msgbuff, GENL_ID_CTRL, CTRL_CMD_GETFAMILY, NLM_F_REQUEST | NLM_F_ACK, 1); if (ret < 0) goto out; ret = -EMSGSIZE; - if (ethnla_put_strz(msgbuff, CTRL_ATTR_FAMILY_NAME, info->fam_name)) + if (ethnla_put_strz(msgbuff, CTRL_ATTR_FAMILY_NAME, ETHTOOL_GENL_NAME)) goto out; nlsock_sendmsg(nlsk, NULL); - nlsock_process_reply(nlsk, family_info_cb, info); - ret = info->fam_id ? 0 : -EADDRNOTAVAIL; + nlsock_process_reply(nlsk, family_info_cb, nlctx); + ret = nlctx->ethnl_fam ? 0 : -EADDRNOTAVAIL; out: - nlsk->nlctx->suppress_nlerr = 0; + nlctx->suppress_nlerr = 0; return ret; } #endif int netlink_init(struct cmd_context *ctx) { - struct fam_info info = { - .fam_name = ETHTOOL_GENL_NAME, - .grp_name = ETHTOOL_MCGRP_MONITOR_NAME, - }; struct nl_context *nlctx; int ret; @@ -189,11 +262,9 @@ int netlink_init(struct cmd_context *ctx) ret = nlsock_init(nlctx, &nlctx->ethnl_socket, NETLINK_GENERIC); if (ret < 0) goto out_free; - ret = get_genl_family(nlctx->ethnl_socket, &info); + ret = get_genl_family(nlctx, nlctx->ethnl_socket); if (ret < 0) goto out_nlsk; - nlctx->ethnl_fam = info.fam_id; - nlctx->ethnl_mongrp = info.grp_id; ctx->nlctx = nlctx; return 0; @@ -201,6 +272,7 @@ int netlink_init(struct cmd_context *ctx) out_nlsk: nlsock_done(nlctx->ethnl_socket); out_free: + free(nlctx->ops_flags); free(nlctx); return ret; } @@ -210,6 +282,7 @@ static void netlink_done(struct cmd_context *ctx) if (!ctx->nlctx) return; + free(ctx->nlctx->ops_flags); free(ctx->nlctx); ctx->nlctx = NULL; cleanup_all_strings(); @@ -228,6 +301,7 @@ void netlink_run_handler(struct cmd_context *ctx, nl_func_t nlfunc, bool no_fallback) { bool wildcard = ctx->devname && !strcmp(ctx->devname, WILDCARD_DEVNAME); + struct nl_context *nlctx; const char *reason; int ret; @@ -245,12 +319,20 @@ void netlink_run_handler(struct cmd_context *ctx, nl_func_t nlfunc, reason = "netlink interface initialization failed"; goto no_support; } + nlctx = ctx->nlctx; ret = nlfunc(ctx); netlink_done(ctx); - if (ret != -EOPNOTSUPP || no_fallback) + if (no_fallback || ret != -EOPNOTSUPP || !nlctx->ioctl_fallback) { + if (nlctx->wildcard_unsupported) + fprintf(stderr, "%s\n", + "subcommand does not support wildcard dump"); exit(ret >= 0 ? ret : 1); - reason = "kernel netlink support for subcommand missing"; + } + if (nlctx->wildcard_unsupported) + reason = "subcommand does not support wildcard dump"; + else + reason = "kernel netlink support for subcommand missing"; no_support: if (no_fallback) { --- a/netlink/netlink.h +++ b/netlink/netlink.h @@ -25,6 +25,7 @@ struct nl_context { unsigned int suppress_nlerr; uint16_t ethnl_fam; uint32_t ethnl_mongrp; + uint32_t *ops_flags; struct nl_socket *ethnl_socket; struct nl_socket *ethnl2_socket; struct nl_socket *rtnl_socket; @@ -36,6 +37,8 @@ struct nl_context { const char *param; char **argp; int argc; + bool ioctl_fallback; + bool wildcard_unsupported; }; struct attr_tb_info { @@ -50,6 +53,8 @@ int nomsg_reply_cb(const struct nlmsghdr *nlhdr, void *data); int attr_cb(const struct nlattr *attr, void *data); int netlink_init(struct cmd_context *ctx); +bool netlink_cmd_check(struct cmd_context *ctx, unsigned int cmd, + bool allow_wildcard); const char *get_dev_name(const struct nlattr *nest); int get_dev_info(const struct nlattr *nest, int *ifindex, char *ifname); --- a/netlink/parser.c +++ b/netlink/parser.c @@ -1023,6 +1023,13 @@ int nl_parser(struct nl_context *nlctx, const struct param_parser *params, goto out_free; } + if (group_style == PARSER_GROUP_MSG) { + ret = -EOPNOTSUPP; + for (buff = buffs; buff; buff = buff->next) + if (msgbuff_len(&buff->msgbuff) > buff->orig_len && + netlink_cmd_check(nlctx->ctx, buff->id, false)) + goto out_free; + } for (buff = buffs; buff; buff = buff->next) { struct nl_msg_buff *msgbuff = &buff->msgbuff; --- a/netlink/settings.c +++ b/netlink/settings.c @@ -726,6 +726,13 @@ int nl_gset(struct cmd_context *ctx) struct nl_socket *nlsk = nlctx->ethnl_socket; int ret; + if (netlink_cmd_check(ctx, ETHTOOL_MSG_LINKMODES_GET, true) || + netlink_cmd_check(ctx, ETHTOOL_MSG_LINKINFO_GET, true) || + netlink_cmd_check(ctx, ETHTOOL_MSG_WOL_GET, true) || + netlink_cmd_check(ctx, ETHTOOL_MSG_DEBUG_GET, true) || + netlink_cmd_check(ctx, ETHTOOL_MSG_LINKSTATE_GET, true)) + return -EOPNOTSUPP; + nlctx->suppress_nlerr = 1; ret = gset_request(nlsk, ETHTOOL_MSG_LINKMODES_GET, ++++++ refactor-interface-between-ioctl-and-netlink-code.patch ++++++ From: Michal Kubecek <[email protected]> Date: Mon, 13 Apr 2020 13:04:42 +0200 Subject: refactor interface between ioctl and netlink code Patch-mainline: v5.7 Git-commit: ab88f37fa432eea925306b442ac283daf71d8791 Move netlink related code from main() to separate handlers in netlink code. Improve the logic of fallback to ioctl and improve error messages when fallback is not possible (e.g. wildcard device name or name longer than IFNAMSIZ). Also handle the (theoretical for now) case when ioctl handler is not available. Signed-off-by: Michal Kubecek <[email protected]> --- ethtool.c | 51 +++++++++++----------------------------- netlink/extapi.h | 14 +++++++---- netlink/monitor.c | 15 +++++++++--- netlink/netlink.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++- netlink/netlink.h | 1 + 5 files changed, 93 insertions(+), 47 deletions(-) --- a/ethtool.c +++ b/ethtool.c @@ -5164,7 +5164,7 @@ struct option { const char *opts; bool no_dev; int (*func)(struct cmd_context *); - int (*nlfunc)(struct cmd_context *); + nl_func_t nlfunc; const char *help; const char *xhelp; }; @@ -5738,6 +5738,11 @@ static int ioctl_init(struct cmd_context *ctx, bool no_dev) ctx->fd = -1; return 0; } + if (strlen(ctx->devname) >= IFNAMSIZ) { + fprintf(stderr, "Device name longer than %u characters\n", + IFNAMSIZ - 1); + exit_bad_args(); + } /* Setup our control structures. */ memset(&ctx->ifr, 0, sizeof(ctx->ifr)); @@ -5757,9 +5762,9 @@ static int ioctl_init(struct cmd_context *ctx, bool no_dev) int main(int argc, char **argp) { - int (*nlfunc)(struct cmd_context *) = NULL; int (*func)(struct cmd_context *); struct cmd_context ctx = {}; + nl_func_t nlfunc = NULL; bool no_dev; int ret; int k; @@ -5782,19 +5787,12 @@ int main(int argc, char **argp) argp += 2; argc -= 2; } -#ifdef ETHTOOL_ENABLE_NETLINK if (*argp && !strcmp(*argp, "--monitor")) { - if (netlink_init(&ctx)) { - fprintf(stderr, - "Option --monitor is only available with netlink.\n"); - return 1; - } else { - ctx.argp = ++argp; - ctx.argc = --argc; - return nl_monitor(&ctx); - } + ctx.argp = ++argp; + ctx.argc = --argc; + ret = nl_monitor(&ctx); + return ret ? 1 : 0; } -#endif /* First argument must be either a valid option or a device * name to get settings for (which we don't expect to begin @@ -5819,40 +5817,17 @@ int main(int argc, char **argp) no_dev = false; opt_found: - if (nlfunc) { - if (netlink_init(&ctx)) - nlfunc = NULL; /* fallback to ioctl() */ - } - if (!no_dev) { ctx.devname = *argp++; argc--; - /* netlink supports altnames, we will have to recheck against - * IFNAMSIZ later in case of fallback to ioctl - */ - if (!ctx.devname || strlen(ctx.devname) >= ALTIFNAMSIZ) { - netlink_done(&ctx); + if (!ctx.devname) exit_bad_args(); - } } - ctx.argc = argc; ctx.argp = argp; + netlink_run_handler(&ctx, nlfunc, !func); - if (nlfunc) { - ret = nlfunc(&ctx); - netlink_done(&ctx); - if ((ret != -EOPNOTSUPP) || !func) - return (ret >= 0) ? ret : 1; - } - - if (ctx.devname && strlen(ctx.devname) >= IFNAMSIZ) { - fprintf(stderr, - "ethtool: device names longer than %u characters are only allowed with netlink\n", - IFNAMSIZ - 1); - exit_bad_args(); - } ret = ioctl_init(&ctx, no_dev); if (ret) return ret; --- a/netlink/extapi.h +++ b/netlink/extapi.h @@ -10,10 +10,12 @@ struct cmd_context; struct nl_context; +typedef int (*nl_func_t)(struct cmd_context *); + #ifdef ETHTOOL_ENABLE_NETLINK -int netlink_init(struct cmd_context *ctx); -void netlink_done(struct cmd_context *ctx); +void netlink_run_handler(struct cmd_context *ctx, nl_func_t nlfunc, + bool no_fallback); int nl_gset(struct cmd_context *ctx); int nl_sset(struct cmd_context *ctx); @@ -24,13 +26,15 @@ void nl_monitor_usage(void); #else /* ETHTOOL_ENABLE_NETLINK */ -static inline int netlink_init(struct cmd_context *ctx maybe_unused) +static inline void netlink_run_handler(struct cmd_context *ctx, + nl_func_t nlfunc, bool no_fallback) { - return -EOPNOTSUPP; } -static inline void netlink_done(struct cmd_context *ctx maybe_unused) +static inline int nl_monitor(struct cmd_context *ctx) { + fprintf(stderr, "Netlink not supported by ethtool, option --monitor unsupported.\n"); + return -EOPNOTSUPP; } static inline void nl_monitor_usage(void) --- a/netlink/monitor.c +++ b/netlink/monitor.c @@ -167,16 +167,25 @@ static int parse_monitor(struct cmd_context *ctx) int nl_monitor(struct cmd_context *ctx) { - struct nl_context *nlctx = ctx->nlctx; - struct nl_socket *nlsk = nlctx->ethnl_socket; - uint32_t grpid = nlctx->ethnl_mongrp; + struct nl_context *nlctx; + struct nl_socket *nlsk; + uint32_t grpid; bool is_dev; int ret; + ret = netlink_init(ctx); + if (ret < 0) { + fprintf(stderr, "Netlink interface initialization failed, option --monitor not supported.\n"); + return ret; + } + nlctx = ctx->nlctx; + nlsk = nlctx->ethnl_socket; + grpid = nlctx->ethnl_mongrp; if (!grpid) { fprintf(stderr, "multicast group 'monitor' not found\n"); return -EOPNOTSUPP; } + if (parse_monitor(ctx) < 0) return 1; is_dev = ctx->devname && strcmp(ctx->devname, WILDCARD_DEVNAME); --- a/netlink/netlink.c +++ b/netlink/netlink.c @@ -205,7 +205,7 @@ out_free: return ret; } -void netlink_done(struct cmd_context *ctx) +static void netlink_done(struct cmd_context *ctx) { if (!ctx->nlctx) return; @@ -214,3 +214,60 @@ void netlink_done(struct cmd_context *ctx) ctx->nlctx = NULL; cleanup_all_strings(); } + +/** + * netlink_run_handler() - run netlink handler for subcommand + * @ctx: command context + * @nlfunc: subcommand netlink handler to call + * @no_fallback: there is no ioctl fallback handler + * + * This function returns only if ioctl() handler should be run as fallback. + * Otherwise it exits with appropriate return code. + */ +void netlink_run_handler(struct cmd_context *ctx, nl_func_t nlfunc, + bool no_fallback) +{ + bool wildcard = ctx->devname && !strcmp(ctx->devname, WILDCARD_DEVNAME); + const char *reason; + int ret; + + if (ctx->devname && strlen(ctx->devname) >= ALTIFNAMSIZ) { + fprintf(stderr, "device name '%s' longer than %u characters\n", + ctx->devname, ALTIFNAMSIZ - 1); + exit(1); + } + + if (!nlfunc) { + reason = "ethtool netlink support for subcommand missing"; + goto no_support; + } + if (netlink_init(ctx)) { + reason = "netlink interface initialization failed"; + goto no_support; + } + + ret = nlfunc(ctx); + netlink_done(ctx); + if (ret != -EOPNOTSUPP || no_fallback) + exit(ret >= 0 ? ret : 1); + reason = "kernel netlink support for subcommand missing"; + +no_support: + if (no_fallback) { + fprintf(stderr, "%s, subcommand not supported by ioctl\n", + reason); + exit(1); + } + if (wildcard) { + fprintf(stderr, "%s, wildcard dump not supported\n", reason); + exit(1); + } + if (ctx->devname && strlen(ctx->devname) >= IFNAMSIZ) { + fprintf(stderr, + "%s, device name longer than %u not supported\n", + reason, IFNAMSIZ - 1); + exit(1); + } + + /* fallback to ioctl() */ +} --- a/netlink/netlink.h +++ b/netlink/netlink.h @@ -49,6 +49,7 @@ struct attr_tb_info { int nomsg_reply_cb(const struct nlmsghdr *nlhdr, void *data); int attr_cb(const struct nlattr *attr, void *data); +int netlink_init(struct cmd_context *ctx); const char *get_dev_name(const struct nlattr *nest); int get_dev_info(const struct nlattr *nest, int *ifindex, char *ifname);
