| Message ID | 20221229011243.1527945-3-sudheer.mogilappagari@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Michal Kubecek |
| Headers | show |
| Series | add netlink support for rss get | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Wed, 28 Dec 2022 17:12:43 -0800 Sudheer Mogilappagari wrote: > Add support for netlink based "ethtool -x <dev>" command using > ETHTOOL_MSG_RSS_GET netlink message. It implements same functionality > provided by traditional ETHTOOL_GRSSH subcommand. This displays RSS > table, hash key and hash function along with JSON support. > +void dump_json_rss_info(struct cmd_context *ctx, struct ethtool_rxfh *rss, > + const struct stringset *hash_funcs) > +{ > + unsigned int indir_bytes = rss->indir_size * sizeof(rss->rss_config[0]); > + unsigned int i; > + > + open_json_object(NULL); > + print_string(PRINT_JSON, "ifname", NULL, ctx->devname); > + if (rss->indir_size) { > + open_json_array("rss-indirection-table", NULL); > + for (i = 0; i < rss->indir_size; i++) > + print_uint(PRINT_JSON, NULL, NULL, rss->rss_config[i]); > + close_json_array("\n"); > + } > + > + if (rss->key_size) { > + const char *hkey = ((char *)rss->rss_config + indir_bytes); > + > + open_json_array("rss-hash-key", NULL); > + for (i = 0; i < rss->key_size; i++) > + print_uint(PRINT_JSON, NULL, NULL, (u8)hkey[i]); > + close_json_array("\n"); > + } > + > + if (rss->hfunc) { > + open_json_object("rss-hash-function"); > + for (i = 0; i < get_count(hash_funcs); i++) > + print_bool(PRINT_JSON, get_string(hash_funcs, i), NULL, > + (rss->hfunc & (1 << i))); > + close_json_object(); > + } I believe there can only be a single bit set here, so why not print: "rss-hash-function": "toeplitz" rather than: "rss-hash-function": { "toeplitz": true, "xor": false, "crc32": false } We can make the kernel ensure only one bit is set by checking hweight() == 1 on the value returned by the driver. > + close_json_object(); > +} > + > +int rss_reply_cb(const struct nlmsghdr *nlhdr, void *data) > +{ > + const struct nlattr *tb[ETHTOOL_A_RSS_MAX + 1] = {}; > + unsigned int indir_bytes = 0, hkey_bytes = 0; > + DECLARE_ATTR_TB_INFO(tb); > + struct cb_args *args = data; > + struct nl_context *nlctx = args->nlctx; double space > + const struct stringset *hash_funcs; > + u32 rss_config_size, rss_hfunc; > + const char *indir_table, *hkey; > + > + struct ethtool_rxfh *rss; > + bool silent; > + int err_ret; > + int ret; > + > + silent = nlctx->is_dump || nlctx->is_monitor; > + err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR; > + ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info); > + if (ret < 0) > + return err_ret; > + nlctx->devname = get_dev_name(tb[ETHTOOL_A_RSS_HEADER]); > + if (!dev_ok(nlctx)) > + return err_ret; > + > + if (silent) > + putchar('\n'); show_cr() > + rss_hfunc = mnl_attr_get_u32(tb[ETHTOOL_A_RSS_HFUNC]); > + > + indir_bytes = mnl_attr_get_payload_len(tb[ETHTOOL_A_RSS_INDIR]); > + indir_table = mnl_attr_get_str(tb[ETHTOOL_A_RSS_INDIR]); > + > + hkey_bytes = mnl_attr_get_payload_len(tb[ETHTOOL_A_RSS_HKEY]); > + hkey = mnl_attr_get_str(tb[ETHTOOL_A_RSS_HKEY]); All elements of tb can be NULL, AFAIU. > + rss_config_size = indir_bytes + hkey_bytes; > + > + rss = calloc(1, sizeof(*rss) + rss_config_size); > + if (!rss) { > + perror("Cannot allocate memory for RX flow hash config"); > + return 1; > + } > + > + rss->indir_size = indir_bytes / sizeof(rss->rss_config[0]); > + rss->key_size = hkey_bytes; > + rss->hfunc = rss_hfunc; > + > + memcpy(rss->rss_config, indir_table, indir_bytes); > + memcpy(rss->rss_config + rss->indir_size, hkey, hkey_bytes); Do you only perform this coalescing to reuse the existing print helpers? With a bit of extra refactoring this seems avoidable... > + /* Fetch RSS hash functions and their status and print */ > + > + if (!nlctx->is_monitor) { > + ret = netlink_init_ethnl2_socket(nlctx); > + if (ret < 0) > + return MNL_CB_ERROR; > + } > + hash_funcs = global_stringset(ETH_SS_RSS_HASH_FUNCS, > + nlctx->ethnl2_socket); > + > + ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info); > + if (ret < 0) > + return silent ? MNL_CB_OK : MNL_CB_ERROR; > + nlctx->devname = get_dev_name(tb[ETHTOOL_A_RSS_HEADER]); > + if (!dev_ok(nlctx)) > + return MNL_CB_OK; > + > + if (is_json_context()) { > + dump_json_rss_info(nlctx->ctx, rss, hash_funcs); > + } else { > + print_rss_info(nlctx->ctx, args->num_rings, rss); > + printf("RSS hash function:\n"); > + if (!rss_hfunc) { > + printf(" Operation not supported\n"); > + return 0; > + } > + for (unsigned int i = 0; i < get_count(hash_funcs); i++) { > + printf(" %s: %s\n", get_string(hash_funcs, i), > + (rss_hfunc & (1 << i)) ? "on" : "off"); > + } > + } > + > + free(rss); > + return MNL_CB_OK; > +} > +int get_channels_cb(const struct nlmsghdr *nlhdr, void *data) > +{ > + const struct nlattr *tb[ETHTOOL_A_CHANNELS_MAX + 1] = {}; > + DECLARE_ATTR_TB_INFO(tb); > + struct cb_args *args = data; > + struct nl_context *nlctx = args->nlctx; > + bool silent; > + int err_ret; > + int ret; > + > + silent = nlctx->is_dump || nlctx->is_monitor; > + err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR; > + ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info); > + if (ret < 0) > + return err_ret; > + nlctx->devname = get_dev_name(tb[ETHTOOL_A_CHANNELS_HEADER]); We need to check that the kernel has filled in the attrs before accessing them, no? > + if (!dev_ok(nlctx)) > + return err_ret; > + > + args->num_rings = mnl_attr_get_u8(tb[ETHTOOL_A_CHANNELS_COMBINED_COUNT]); u32, not u8 The correct value is combined + rx, I think I mentioned this before. > + return MNL_CB_OK; I'm also not sure if fetching the channel info shouldn't be done over the nl2 socket, like the string set. If we are in monitor mode we may confuse ourselves trying to parse the wrong messages.
> -----Original Message----- > From: Jakub Kicinski <kuba@kernel.org> > I believe there can only be a single bit set here, so why not print: > > "rss-hash-function": "toeplitz" > > rather than: > > "rss-hash-function": { > "toeplitz": true, > "xor": false, > "crc32": false > } > Was following similar output format as current one. Changed in next version. > > + rss_hfunc = mnl_attr_get_u32(tb[ETHTOOL_A_RSS_HFUNC]); > > + > > + indir_bytes = mnl_attr_get_payload_len(tb[ETHTOOL_A_RSS_INDIR]); > > + indir_table = mnl_attr_get_str(tb[ETHTOOL_A_RSS_INDIR]); > > + > > + hkey_bytes = mnl_attr_get_payload_len(tb[ETHTOOL_A_RSS_HKEY]); > > + hkey = mnl_attr_get_str(tb[ETHTOOL_A_RSS_HKEY]); > > All elements of tb can be NULL, AFAIU. Didn't get this. Do you mean the variables need to be checked for NULL here? If yes, am checking before printing later on. > > + rss->indir_size = indir_bytes / sizeof(rss->rss_config[0]); > > + rss->key_size = hkey_bytes; > > + rss->hfunc = rss_hfunc; > > + > > + memcpy(rss->rss_config, indir_table, indir_bytes); > > + memcpy(rss->rss_config + rss->indir_size, hkey, hkey_bytes); > > Do you only perform this coalescing to reuse the existing print > helpers? With a bit of extra refactoring this seems avoidable... Yes. Was reusing print helpers. Have refactored to avoid need for coalescing. > > +int get_channels_cb(const struct nlmsghdr *nlhdr, void *data) { > > + silent = nlctx->is_dump || nlctx->is_monitor; > > + err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR; > > + ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info); > > + if (ret < 0) > > + return err_ret; > > + nlctx->devname = get_dev_name(tb[ETHTOOL_A_CHANNELS_HEADER]); > > We need to check that the kernel has filled in the attrs before > accessing them, no? Didn't get this one either. similar code isn't doing any checks like you suggested. > > + if (!dev_ok(nlctx)) > > + return err_ret; > > + > > + args->num_rings = > > +mnl_attr_get_u8(tb[ETHTOOL_A_CHANNELS_COMBINED_COUNT]); > > u32, not u8 Fixed. > The correct value is combined + rx, I think I mentioned this before. Have changed it to include rx too like below. args->num_rings = mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_COMBINED_COUNT]); args->num_rings += mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_RX_COUNT]); Slightly unrelated, where can I find the reason behind using combined + rx? Guess it was discussed in mailing list but not able to find it. > + return MNL_CB_OK; > > I'm also not sure if fetching the channel info shouldn't be done over > the nl2 socket, like the string set. If we are in monitor mode we may > confuse ourselves trying to parse the wrong messages. Are you suggesting we need to use ioctl for fetching ring info to avoid mix-up. Is there alternative way to do it ?
On Fri, 6 Jan 2023 17:41:39 +0000 Mogilappagari, Sudheer wrote: > > > + rss_hfunc = mnl_attr_get_u32(tb[ETHTOOL_A_RSS_HFUNC]); > > > + > > > + indir_bytes = mnl_attr_get_payload_len(tb[ETHTOOL_A_RSS_INDIR]); > > > + indir_table = mnl_attr_get_str(tb[ETHTOOL_A_RSS_INDIR]); > > > + > > > + hkey_bytes = mnl_attr_get_payload_len(tb[ETHTOOL_A_RSS_HKEY]); > > > + hkey = mnl_attr_get_str(tb[ETHTOOL_A_RSS_HKEY]); > > > > All elements of tb can be NULL, AFAIU. > > Didn't get this. Do you mean the variables need to be checked > for NULL here? If yes, am checking before printing later on. tb[ETHTOOL_A_RSS_HKEY] can be NULL. I just realized now that the kernel always fills in the attrs: if (nla_put_u32(skb, ETHTOOL_A_RSS_HFUNC, data->hfunc) || nla_put(skb, ETHTOOL_A_RSS_INDIR, sizeof(u32) * data->indir_size, data->indir_table) || nla_put(skb, ETHTOOL_A_RSS_HKEY, data->hkey_size, data->hkey)) return -EMSGSIZE; but that's not a great use of netlink. If there is nothing to report the attribute should simply be skipped, like this: if (nla_put_u32(skb, ETHTOOL_A_RSS_HFUNC, data->hfunc)) || (data->indir_size && nla_put(skb, ETHTOOL_A_RSS_INDIR, sizeof(u32) * data->indir_size, data->indir_table)) || (data->hkey_size && nla_put(skb, ETHTOOL_A_RSS_HKEY, data->hkey_size, data->hkey))) return -EMSGSIZE; I think we should fix the kernel side. > > > +int get_channels_cb(const struct nlmsghdr *nlhdr, void *data) { > > > > + silent = nlctx->is_dump || nlctx->is_monitor; > > > + err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR; > > > + ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info); > > > + if (ret < 0) > > > + return err_ret; > > > + nlctx->devname = get_dev_name(tb[ETHTOOL_A_CHANNELS_HEADER]); > > > > We need to check that the kernel has filled in the attrs before > > accessing them, no? > > Didn't get this one either. similar code isn't doing any checks > like you suggested. Same point, really. Even if the kernel always populates the attribute today, according to netlink rules it's not guaranteed to do so in the future, so any tb[] access should be NULL-checked. > > The correct value is combined + rx, I think I mentioned this before. > > Have changed it to include rx too like below. > args->num_rings = mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_COMBINED_COUNT]); > args->num_rings += mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_RX_COUNT]); Related to previous comments - take a look at channels_fill_reply(). tb[ETHTOOL_A_CHANNELS_RX_COUNT] will be NULL for most devices. mnl_attr_get_u32(NULL) will crash. > Slightly unrelated, where can I find the reason behind using combined + rx? > Guess it was discussed in mailing list but not able to find it. Yes, it's been discussed on the list multiple times but man ethtool is the only source of documentation I know of. The reason is that the channels API refers to interrupts more than queues. So rx means an _irq_ dedicated to Rx processing, not an Rx queue. Unfortunately the author came up with the term "channel" which most people take to mean "queue" :( > > + return MNL_CB_OK; > > > > I'm also not sure if fetching the channel info shouldn't be done over > > the nl2 socket, like the string set. If we are in monitor mode we may > > confuse ourselves trying to parse the wrong messages. > > Are you suggesting we need to use ioctl for fetching ring info to avoid > mix-up. Is there alternative way to do it ? No no, look how the strings for hfunc names are fetched - they are fetched over a different socket, right?
> -----Original Message----- > From: Jakub Kicinski <kuba@kernel.org> > Sent: Friday, January 6, 2023 1:42 PM > To: Mogilappagari, Sudheer <sudheer.mogilappagari@intel.com> > Cc: netdev@vger.kernel.org; mkubecek@suse.cz; andrew@lunn.ch; > Samudrala, Sridhar <sridhar.samudrala@intel.com>; Nguyen, Anthony L > <anthony.l.nguyen@intel.com> > Subject: Re: [PATCH ethtool-next v4 2/2] netlink: add netlink handler > for get rss (-x) > > On Fri, 6 Jan 2023 17:41:39 +0000 Mogilappagari, Sudheer wrote: > > > > + rss_hfunc = mnl_attr_get_u32(tb[ETHTOOL_A_RSS_HFUNC]); > > > > + > > > > + indir_bytes = > mnl_attr_get_payload_len(tb[ETHTOOL_A_RSS_INDIR]); > > > > + indir_table = mnl_attr_get_str(tb[ETHTOOL_A_RSS_INDIR]); > > > > + > > > > + hkey_bytes = > mnl_attr_get_payload_len(tb[ETHTOOL_A_RSS_HKEY]); > > > > + hkey = mnl_attr_get_str(tb[ETHTOOL_A_RSS_HKEY]); > > > > > > All elements of tb can be NULL, AFAIU. > > > > Didn't get this. Do you mean the variables need to be checked for > NULL > > here? If yes, am checking before printing later on. > > tb[ETHTOOL_A_RSS_HKEY] can be NULL. I just realized now that the kernel > always fills in the attrs: > > if (nla_put_u32(skb, ETHTOOL_A_RSS_HFUNC, data->hfunc) || > nla_put(skb, ETHTOOL_A_RSS_INDIR, > sizeof(u32) * data->indir_size, data->indir_table) || > nla_put(skb, ETHTOOL_A_RSS_HKEY, data->hkey_size, data- > >hkey)) > return -EMSGSIZE; > > but that's not a great use of netlink. If there is nothing to report > the attribute should simply be skipped, like this: > > if (nla_put_u32(skb, ETHTOOL_A_RSS_HFUNC, data->hfunc)) || > (data->indir_size && > nla_put(skb, ETHTOOL_A_RSS_INDIR, > sizeof(u32) * data->indir_size, data->indir_table)) > || > (data->hkey_size && > nla_put(skb, ETHTOOL_A_RSS_HKEY, data->hkey_size, data- > >hkey))) > return -EMSGSIZE; > > I think we should fix the kernel side. Will do this. > > > > > +int get_channels_cb(const struct nlmsghdr *nlhdr, void *data) { > > > > > > + silent = nlctx->is_dump || nlctx->is_monitor; > > > > + err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR; > > > > + ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, > &tb_info); > > > > + if (ret < 0) > > > > + return err_ret; > > > > + nlctx->devname = > get_dev_name(tb[ETHTOOL_A_CHANNELS_HEADER]); > > > > > > We need to check that the kernel has filled in the attrs before > > > accessing them, no? > > > > Didn't get this one either. similar code isn't doing any checks like > > you suggested. > > Same point, really. Even if the kernel always populates the attribute > today, according to netlink rules it's not guaranteed to do so in the > future, so any tb[] access should be NULL-checked. > > > > The correct value is combined + rx, I think I mentioned this > before. > > > > Have changed it to include rx too like below. > > args->num_rings = > > args->mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_COMBINED_COUNT]); > > args->num_rings += mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_RX_COUNT]); > > Related to previous comments - take a look at channels_fill_reply(). > tb[ETHTOOL_A_CHANNELS_RX_COUNT] will be NULL for most devices. > mnl_attr_get_u32(NULL) will crash. > > > Slightly unrelated, where can I find the reason behind using combined > + rx? > > Guess it was discussed in mailing list but not able to find it. > > Yes, it's been discussed on the list multiple times but man ethtool is > the only source of documentation I know of. > > The reason is that the channels API refers to interrupts more than > queues. So rx means an _irq_ dedicated to Rx processing, not an Rx > queue. Unfortunately the author came up with the term "channel" which > most people take to mean "queue" :( > > > > + return MNL_CB_OK; > > > > > > I'm also not sure if fetching the channel info shouldn't be done > > > over the nl2 socket, like the string set. If we are in monitor mode > > > we may confuse ourselves trying to parse the wrong messages. > > > > Are you suggesting we need to use ioctl for fetching ring info to > > avoid mix-up. Is there alternative way to do it ? > > No no, look how the strings for hfunc names are fetched - they are > fetched over a different socket, right? global_stringset is using nlctx->ethnl2_socket. Are you suggesting use of it for fetching channels info too ? ret = netlink_init_ethnl2_socket(nlctx); ... hash_funcs = global_stringset(ETH_SS_RSS_HASH_FUNCS, nlctx->ethnl2_socket);
On Mon, 9 Jan 2023 18:07:45 +0000 Mogilappagari, Sudheer wrote: > > > Are you suggesting we need to use ioctl for fetching ring info to > > > avoid mix-up. Is there alternative way to do it ? > > > > No no, look how the strings for hfunc names are fetched - they are > > fetched over a different socket, right? > > global_stringset is using nlctx->ethnl2_socket. Are you suggesting use > of it for fetching channels info too ? > > ret = netlink_init_ethnl2_socket(nlctx); > ... > hash_funcs = global_stringset(ETH_SS_RSS_HASH_FUNCS, nlctx->ethnl2_socket); Yessir.
On Mon, Jan 09, 2023 at 06:07:45PM +0000, Mogilappagari, Sudheer wrote: > > -----Original Message----- > > From: Jakub Kicinski <kuba@kernel.org> > > Sent: Friday, January 6, 2023 1:42 PM > > To: Mogilappagari, Sudheer <sudheer.mogilappagari@intel.com> > > Cc: netdev@vger.kernel.org; mkubecek@suse.cz; andrew@lunn.ch; > > Samudrala, Sridhar <sridhar.samudrala@intel.com>; Nguyen, Anthony L > > <anthony.l.nguyen@intel.com> > > Subject: Re: [PATCH ethtool-next v4 2/2] netlink: add netlink handler > > for get rss (-x) [...] > > No no, look how the strings for hfunc names are fetched - they are > > fetched over a different socket, right? > > global_stringset is using nlctx->ethnl2_socket. Are you suggesting use > of it for fetching channels info too ? > > ret = netlink_init_ethnl2_socket(nlctx); > ... > hash_funcs = global_stringset(ETH_SS_RSS_HASH_FUNCS, nlctx->ethnl2_socket); The purpose of the second socket is to allow sending another request while we are still processing the data from the main request so that we cannot reuse the primary socket. In this case, if we do not support dumps (and do not intend to), we could technically send both request through the same socket (one after the other) but I think it would be easier to use ethnl2_socket for the auxiliary request anyway. Michal
diff --git a/Makefile.am b/Makefile.am index 663f40a..c3e7401 100644 --- a/Makefile.am +++ b/Makefile.am @@ -39,7 +39,7 @@ ethtool_SOURCES += \ netlink/eee.c netlink/tsinfo.c netlink/fec.c \ netlink/stats.c \ netlink/desc-ethtool.c netlink/desc-genlctrl.c \ - netlink/module-eeprom.c netlink/module.c \ + netlink/module-eeprom.c netlink/module.c netlink/rss.c \ netlink/desc-rtnl.c netlink/cable_test.c netlink/tunnels.c \ uapi/linux/ethtool_netlink.h \ uapi/linux/netlink.h uapi/linux/genetlink.h \ diff --git a/ethtool.c b/ethtool.c index 209dbd1..5c16b10 100644 --- a/ethtool.c +++ b/ethtool.c @@ -5850,7 +5850,9 @@ static const struct option args[] = { }, { .opts = "-x|--show-rxfh-indir|--show-rxfh", + .json = true, .func = do_grxfh, + .nlfunc = nl_grss, .help = "Show Rx flow hash indirection table and/or RSS hash key", .xhelp = " [ context %d ]\n" }, diff --git a/netlink/desc-ethtool.c b/netlink/desc-ethtool.c index b3ac64d..ed83dae 100644 --- a/netlink/desc-ethtool.c +++ b/netlink/desc-ethtool.c @@ -442,6 +442,15 @@ static const struct pretty_nla_desc __pse_desc[] = { NLATTR_DESC_U32_ENUM(ETHTOOL_A_PODL_PSE_PW_D_STATUS, pse_pw_d_status), }; +static const struct pretty_nla_desc __rss_desc[] = { + NLATTR_DESC_INVALID(ETHTOOL_A_MODULE_UNSPEC), + NLATTR_DESC_NESTED(ETHTOOL_A_MODULE_HEADER, header), + NLATTR_DESC_U32(ETHTOOL_A_RSS_CONTEXT), + NLATTR_DESC_U32(ETHTOOL_A_RSS_HFUNC), + NLATTR_DESC_BINARY(ETHTOOL_A_RSS_INDIR), + NLATTR_DESC_BINARY(ETHTOOL_A_RSS_HKEY), +}; + const struct pretty_nlmsg_desc ethnl_umsg_desc[] = { NLMSG_DESC_INVALID(ETHTOOL_MSG_USER_NONE), NLMSG_DESC(ETHTOOL_MSG_STRSET_GET, strset), @@ -481,6 +490,7 @@ const struct pretty_nlmsg_desc ethnl_umsg_desc[] = { NLMSG_DESC(ETHTOOL_MSG_MODULE_SET, module), NLMSG_DESC(ETHTOOL_MSG_PSE_GET, pse), NLMSG_DESC(ETHTOOL_MSG_PSE_SET, pse), + NLMSG_DESC(ETHTOOL_MSG_RSS_GET, rss), }; const unsigned int ethnl_umsg_n_desc = ARRAY_SIZE(ethnl_umsg_desc); @@ -524,6 +534,7 @@ const struct pretty_nlmsg_desc ethnl_kmsg_desc[] = { NLMSG_DESC(ETHTOOL_MSG_MODULE_GET_REPLY, module), NLMSG_DESC(ETHTOOL_MSG_MODULE_NTF, module), NLMSG_DESC(ETHTOOL_MSG_PSE_GET_REPLY, pse), + NLMSG_DESC(ETHTOOL_MSG_RSS_GET_REPLY, rss), }; const unsigned int ethnl_kmsg_n_desc = ARRAY_SIZE(ethnl_kmsg_desc); diff --git a/netlink/extapi.h b/netlink/extapi.h index 1bb580a..9b6dd1a 100644 --- a/netlink/extapi.h +++ b/netlink/extapi.h @@ -47,6 +47,7 @@ int nl_gmodule(struct cmd_context *ctx); int nl_smodule(struct cmd_context *ctx); int nl_monitor(struct cmd_context *ctx); int nl_getmodule(struct cmd_context *ctx); +int nl_grss(struct cmd_context *ctx); void nl_monitor_usage(void); @@ -114,6 +115,7 @@ nl_get_eeprom_page(struct cmd_context *ctx __maybe_unused, #define nl_getmodule NULL #define nl_gmodule NULL #define nl_smodule NULL +#define nl_grss NULL #endif /* ETHTOOL_ENABLE_NETLINK */ diff --git a/netlink/rss.c b/netlink/rss.c new file mode 100644 index 0000000..f52c2e7 --- /dev/null +++ b/netlink/rss.c @@ -0,0 +1,241 @@ +/* + * rss.c - netlink implementation of RSS context commands + * + * Implementation of "ethtool -x <dev>" + */ + +#include <errno.h> +#include <string.h> +#include <stdio.h> + +#include "../internal.h" +#include "../common.h" +#include "netlink.h" +#include "strset.h" +#include "parser.h" + +struct cb_args { + struct nl_context *nlctx; + u32 num_rings; +}; + +void dump_json_rss_info(struct cmd_context *ctx, struct ethtool_rxfh *rss, + const struct stringset *hash_funcs) +{ + unsigned int indir_bytes = rss->indir_size * sizeof(rss->rss_config[0]); + unsigned int i; + + open_json_object(NULL); + print_string(PRINT_JSON, "ifname", NULL, ctx->devname); + if (rss->indir_size) { + open_json_array("rss-indirection-table", NULL); + for (i = 0; i < rss->indir_size; i++) + print_uint(PRINT_JSON, NULL, NULL, rss->rss_config[i]); + close_json_array("\n"); + } + + if (rss->key_size) { + const char *hkey = ((char *)rss->rss_config + indir_bytes); + + open_json_array("rss-hash-key", NULL); + for (i = 0; i < rss->key_size; i++) + print_uint(PRINT_JSON, NULL, NULL, (u8)hkey[i]); + close_json_array("\n"); + } + + if (rss->hfunc) { + open_json_object("rss-hash-function"); + for (i = 0; i < get_count(hash_funcs); i++) + print_bool(PRINT_JSON, get_string(hash_funcs, i), NULL, + (rss->hfunc & (1 << i))); + close_json_object(); + } + + close_json_object(); +} + +int rss_reply_cb(const struct nlmsghdr *nlhdr, void *data) +{ + const struct nlattr *tb[ETHTOOL_A_RSS_MAX + 1] = {}; + unsigned int indir_bytes = 0, hkey_bytes = 0; + DECLARE_ATTR_TB_INFO(tb); + struct cb_args *args = data; + struct nl_context *nlctx = args->nlctx; + const struct stringset *hash_funcs; + u32 rss_config_size, rss_hfunc; + const char *indir_table, *hkey; + + struct ethtool_rxfh *rss; + bool silent; + int err_ret; + int ret; + + silent = nlctx->is_dump || nlctx->is_monitor; + err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR; + ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info); + if (ret < 0) + return err_ret; + nlctx->devname = get_dev_name(tb[ETHTOOL_A_RSS_HEADER]); + if (!dev_ok(nlctx)) + return err_ret; + + if (silent) + putchar('\n'); + + rss_hfunc = mnl_attr_get_u32(tb[ETHTOOL_A_RSS_HFUNC]); + + indir_bytes = mnl_attr_get_payload_len(tb[ETHTOOL_A_RSS_INDIR]); + indir_table = mnl_attr_get_str(tb[ETHTOOL_A_RSS_INDIR]); + + hkey_bytes = mnl_attr_get_payload_len(tb[ETHTOOL_A_RSS_HKEY]); + hkey = mnl_attr_get_str(tb[ETHTOOL_A_RSS_HKEY]); + + rss_config_size = indir_bytes + hkey_bytes; + + rss = calloc(1, sizeof(*rss) + rss_config_size); + if (!rss) { + perror("Cannot allocate memory for RX flow hash config"); + return 1; + } + + rss->indir_size = indir_bytes / sizeof(rss->rss_config[0]); + rss->key_size = hkey_bytes; + rss->hfunc = rss_hfunc; + + memcpy(rss->rss_config, indir_table, indir_bytes); + memcpy(rss->rss_config + rss->indir_size, hkey, hkey_bytes); + + /* Fetch RSS hash functions and their status and print */ + + if (!nlctx->is_monitor) { + ret = netlink_init_ethnl2_socket(nlctx); + if (ret < 0) + return MNL_CB_ERROR; + } + hash_funcs = global_stringset(ETH_SS_RSS_HASH_FUNCS, + nlctx->ethnl2_socket); + + ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info); + if (ret < 0) + return silent ? MNL_CB_OK : MNL_CB_ERROR; + nlctx->devname = get_dev_name(tb[ETHTOOL_A_RSS_HEADER]); + if (!dev_ok(nlctx)) + return MNL_CB_OK; + + if (is_json_context()) { + dump_json_rss_info(nlctx->ctx, rss, hash_funcs); + } else { + print_rss_info(nlctx->ctx, args->num_rings, rss); + printf("RSS hash function:\n"); + if (!rss_hfunc) { + printf(" Operation not supported\n"); + return 0; + } + for (unsigned int i = 0; i < get_count(hash_funcs); i++) { + printf(" %s: %s\n", get_string(hash_funcs, i), + (rss_hfunc & (1 << i)) ? "on" : "off"); + } + } + + free(rss); + return MNL_CB_OK; +} + +/* RSS_GET */ +static const struct param_parser grss_params[] = { + { + .arg = "context", + .type = ETHTOOL_A_RSS_CONTEXT, + .handler = nl_parse_direct_u32, + .min_argc = 1, + }, + {} +}; + +int get_channels_cb(const struct nlmsghdr *nlhdr, void *data) +{ + const struct nlattr *tb[ETHTOOL_A_CHANNELS_MAX + 1] = {}; + DECLARE_ATTR_TB_INFO(tb); + struct cb_args *args = data; + struct nl_context *nlctx = args->nlctx; + bool silent; + int err_ret; + int ret; + + silent = nlctx->is_dump || nlctx->is_monitor; + err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR; + ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info); + if (ret < 0) + return err_ret; + nlctx->devname = get_dev_name(tb[ETHTOOL_A_CHANNELS_HEADER]); + if (!dev_ok(nlctx)) + return err_ret; + + args->num_rings = mnl_attr_get_u8(tb[ETHTOOL_A_CHANNELS_COMBINED_COUNT]); + return MNL_CB_OK; +} + +int nl_grss(struct cmd_context *ctx) +{ + struct nl_context *nlctx = ctx->nlctx; + struct nl_socket *nlsk = nlctx->ethnl_socket; + struct nl_msg_buff *msgbuff; + struct cb_args args; + int ret; + + nlctx->cmd = "-x"; + nlctx->argp = ctx->argp; + nlctx->argc = ctx->argc; + nlctx->devname = ctx->devname; + nlsk = nlctx->ethnl_socket; + msgbuff = &nlsk->msgbuff; + + if (netlink_cmd_check(ctx, ETHTOOL_MSG_RSS_GET, true)) + return -EOPNOTSUPP; + + /* save rings information into args.num_rings */ + if (netlink_cmd_check(ctx, ETHTOOL_MSG_CHANNELS_GET, true)) + return -EOPNOTSUPP; + + ret = nlsock_prep_get_request(nlsk, ETHTOOL_MSG_CHANNELS_GET, + ETHTOOL_A_CHANNELS_HEADER, 0); + if (ret < 0) + goto err; + + ret = nlsock_sendmsg(nlsk, NULL); + if (ret < 0) + goto err; + + args.nlctx = nlsk->nlctx; + ret = nlsock_process_reply(nlsk, get_channels_cb, &args); + if (ret < 0) + goto err; + + ret = msg_init(nlctx, msgbuff, ETHTOOL_MSG_RSS_GET, + NLM_F_REQUEST | NLM_F_ACK); + if (ret < 0) + return 1; + if (ethnla_fill_header(msgbuff, ETHTOOL_A_RSS_HEADER, + ctx->devname, 0)) + return -EMSGSIZE; + + ret = nl_parser(nlctx, grss_params, NULL, PARSER_GROUP_NONE, NULL); + if (ret < 0) + goto err; + + new_json_obj(ctx->json); + ret = nlsock_sendmsg(nlsk, NULL); + if (ret < 0) + goto err; + + args.nlctx = nlctx; + ret = nlsock_process_reply(nlsk, rss_reply_cb, &args); + delete_json_obj(); + + if (ret == 0) + return 0; + +err: + return nlctx->exit_code ?: 1; +} +
Add support for netlink based "ethtool -x <dev>" command using ETHTOOL_MSG_RSS_GET netlink message. It implements same functionality provided by traditional ETHTOOL_GRSSH subcommand. This displays RSS table, hash key and hash function along with JSON support. Sample output with json option: $ethtool --json -x eno2 [ { "ifname": "eno2", "rss-indirection-table": [ 0,0,0,0,0,0,0,0,1,1,1,1,1,1,1, ...skip similar lines... 7,7,7,7,7,7,7,7 ], "rss-hash-key": [ 190,195,19,166,..] "rss-hash-function": { "toeplitz": true, "xor": false, "crc32": false } } ] Signed-off-by: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com> --- Makefile.am | 2 +- ethtool.c | 2 + netlink/desc-ethtool.c | 11 ++ netlink/extapi.h | 2 + netlink/rss.c | 241 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 257 insertions(+), 1 deletion(-) create mode 100644 netlink/rss.c