* [PATCH net-next 0/5] ethtool netlink interface, preliminary patches
@ 2019-12-09 19:55 Michal Kubecek
2019-12-09 19:55 ` [PATCH net-next 1/5] rtnetlink: provide permanent hardware address in RTM_NEWLINK Michal Kubecek
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Michal Kubecek @ 2019-12-09 19:55 UTC (permalink / raw)
To: David Miller, netdev
Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
As Jakub Kicinski suggested in ethtool netlink v7 discussion, this
submission consists only of preliminary patches which raised no objections;
first four patches already have Acked-by or Reviewed-by.
- patch 1 exposes permanent hardware address (as shown by "ethtool -P")
via rtnetlink
- patch 2 is renames existing netlink helper to a better name
- patch 3 and 4 reorganize existing ethtool code (no functional change)
- patch 5 makes the table of link mode names available as an ethtool string
set (will be needed for the netlink interface)
Once we get these out of the way, v8 of the first part of the ethtool
netlink interface will follow.
Michal Kubecek (5):
rtnetlink: provide permanent hardware address in RTM_NEWLINK
netlink: rename nl80211_validate_nested() to nla_validate_nested()
ethtool: move to its own directory
ethtool: move string arrays into common file
ethtool: provide link mode names as a string set
include/linux/ethtool.h | 4 +
include/net/netlink.h | 8 +-
include/uapi/linux/ethtool.h | 2 +
include/uapi/linux/if_link.h | 1 +
net/Makefile | 2 +-
net/core/Makefile | 2 +-
net/core/rtnetlink.c | 5 +
net/ethtool/Makefile | 3 +
net/ethtool/common.c | 171 ++++++++++++++++++++++++
net/ethtool/common.h | 19 +++
net/{core/ethtool.c => ethtool/ioctl.c} | 89 +-----------
net/wireless/nl80211.c | 3 +-
12 files changed, 219 insertions(+), 90 deletions(-)
create mode 100644 net/ethtool/Makefile
create mode 100644 net/ethtool/common.c
create mode 100644 net/ethtool/common.h
rename net/{core/ethtool.c => ethtool/ioctl.c} (95%)
--
2.24.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH net-next 1/5] rtnetlink: provide permanent hardware address in RTM_NEWLINK 2019-12-09 19:55 [PATCH net-next 0/5] ethtool netlink interface, preliminary patches Michal Kubecek @ 2019-12-09 19:55 ` Michal Kubecek 2019-12-09 23:20 ` Johannes Berg 2019-12-09 19:55 ` [PATCH net-next 2/5] netlink: rename nl80211_validate_nested() to nla_validate_nested() Michal Kubecek ` (3 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Michal Kubecek @ 2019-12-09 19:55 UTC (permalink / raw) To: David Miller, netdev Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg, linux-kernel Permanent hardware address of a network device was traditionally provided via ethtool ioctl interface but as Jiri Pirko pointed out in a review of ethtool netlink interface, rtnetlink is much more suitable for it so let's add it to the RTM_NEWLINK message. Add IFLA_PERM_ADDRESS attribute to RTM_NEWLINK messages unless the permanent address is all zeros (i.e. device driver did not fill it). As permanent address is not modifiable, reject userspace requests containing IFLA_PERM_ADDRESS attribute. Note: we already provide permanent hardware address for bond slaves; unfortunately we cannot drop that attribute for backward compatibility reasons. v5 -> v6: only add the attribute if permanent address is not zero Signed-off-by: Michal Kubecek <mkubecek@suse.cz> Acked-by: Jiri Pirko <jiri@mellanox.com> Acked-by: Stephen Hemminger <stephen@networkplumber.org> --- include/uapi/linux/if_link.h | 1 + net/core/rtnetlink.c | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 8aec8769d944..1d69f637c5d6 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -169,6 +169,7 @@ enum { IFLA_MAX_MTU, IFLA_PROP_LIST, IFLA_ALT_IFNAME, /* Alternative ifname */ + IFLA_PERM_ADDRESS, __IFLA_MAX }; diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 02916f43bf63..20bc406f3871 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -1041,6 +1041,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev, + nla_total_size(4) /* IFLA_MIN_MTU */ + nla_total_size(4) /* IFLA_MAX_MTU */ + rtnl_prop_list_size(dev) + + nla_total_size(MAX_ADDR_LEN) /* IFLA_PERM_ADDRESS */ + 0; } @@ -1757,6 +1758,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, nla_put_s32(skb, IFLA_NEW_IFINDEX, new_ifindex) < 0) goto nla_put_failure; + if (memchr_inv(dev->perm_addr, '\0', dev->addr_len) && + nla_put(skb, IFLA_PERM_ADDRESS, dev->addr_len, dev->perm_addr)) + goto nla_put_failure; rcu_read_lock(); if (rtnl_fill_link_af(skb, dev, ext_filter_mask)) @@ -1822,6 +1826,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = { [IFLA_PROP_LIST] = { .type = NLA_NESTED }, [IFLA_ALT_IFNAME] = { .type = NLA_STRING, .len = ALTIFNAMSIZ - 1 }, + [IFLA_PERM_ADDRESS] = { .type = NLA_REJECT }, }; static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = { -- 2.24.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/5] rtnetlink: provide permanent hardware address in RTM_NEWLINK 2019-12-09 19:55 ` [PATCH net-next 1/5] rtnetlink: provide permanent hardware address in RTM_NEWLINK Michal Kubecek @ 2019-12-09 23:20 ` Johannes Berg 2019-12-09 23:21 ` Johannes Berg 0 siblings, 1 reply; 10+ messages in thread From: Johannes Berg @ 2019-12-09 23:20 UTC (permalink / raw) To: Michal Kubecek, David Miller, netdev Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli, John Linville, Stephen Hemminger, linux-kernel On Mon, 2019-12-09 at 20:55 +0100, Michal Kubecek wrote: > > + if (memchr_inv(dev->perm_addr, '\0', dev->addr_len) && Why not simply !is_zero_ether_addr()? johannes ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/5] rtnetlink: provide permanent hardware address in RTM_NEWLINK 2019-12-09 23:20 ` Johannes Berg @ 2019-12-09 23:21 ` Johannes Berg 0 siblings, 0 replies; 10+ messages in thread From: Johannes Berg @ 2019-12-09 23:21 UTC (permalink / raw) To: Michal Kubecek, David Miller, netdev Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli, John Linville, Stephen Hemminger, linux-kernel On Tue, 2019-12-10 at 00:20 +0100, Johannes Berg wrote: > On Mon, 2019-12-09 at 20:55 +0100, Michal Kubecek wrote: > > + if (memchr_inv(dev->perm_addr, '\0', dev->addr_len) && > > Why not simply !is_zero_ether_addr()? Ugh, sorry, ignore me. I need to go sleep instead of looking at patches. johannes ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next 2/5] netlink: rename nl80211_validate_nested() to nla_validate_nested() 2019-12-09 19:55 [PATCH net-next 0/5] ethtool netlink interface, preliminary patches Michal Kubecek 2019-12-09 19:55 ` [PATCH net-next 1/5] rtnetlink: provide permanent hardware address in RTM_NEWLINK Michal Kubecek @ 2019-12-09 19:55 ` Michal Kubecek 2019-12-09 19:55 ` [PATCH net-next 3/5] ethtool: move to its own directory Michal Kubecek ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Michal Kubecek @ 2019-12-09 19:55 UTC (permalink / raw) To: David Miller, netdev Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg, linux-kernel Function nl80211_validate_nested() is not specific to nl80211, it's a counterpart to nla_validate_nested_deprecated() with strict validation. For consistency with other validation and parse functions, rename it to nla_validate_nested(). Signed-off-by: Michal Kubecek <mkubecek@suse.cz> Acked-by: Jiri Pirko <jiri@mellanox.com> Reviewed-by: Johannes Berg <johannes@sipsolutions.net> --- include/net/netlink.h | 8 ++++---- net/wireless/nl80211.c | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/include/net/netlink.h b/include/net/netlink.h index b140c8f1be22..56c365dc6dc7 100644 --- a/include/net/netlink.h +++ b/include/net/netlink.h @@ -1735,7 +1735,7 @@ static inline void nla_nest_cancel(struct sk_buff *skb, struct nlattr *start) } /** - * nla_validate_nested - Validate a stream of nested attributes + * __nla_validate_nested - Validate a stream of nested attributes * @start: container attribute * @maxtype: maximum attribute type to be expected * @policy: validation policy @@ -1758,9 +1758,9 @@ static inline int __nla_validate_nested(const struct nlattr *start, int maxtype, } static inline int -nl80211_validate_nested(const struct nlattr *start, int maxtype, - const struct nla_policy *policy, - struct netlink_ext_ack *extack) +nla_validate_nested(const struct nlattr *start, int maxtype, + const struct nla_policy *policy, + struct netlink_ext_ack *extack) { return __nla_validate_nested(start, maxtype, policy, NL_VALIDATE_STRICT, extack); diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index da5262b2298b..fa3526592c51 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -12900,8 +12900,7 @@ static int nl80211_vendor_check_policy(const struct wiphy_vendor_command *vcmd, return -EINVAL; } - return nl80211_validate_nested(attr, vcmd->maxattr, vcmd->policy, - extack); + return nla_validate_nested(attr, vcmd->maxattr, vcmd->policy, extack); } static int nl80211_vendor_cmd(struct sk_buff *skb, struct genl_info *info) -- 2.24.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net-next 3/5] ethtool: move to its own directory 2019-12-09 19:55 [PATCH net-next 0/5] ethtool netlink interface, preliminary patches Michal Kubecek 2019-12-09 19:55 ` [PATCH net-next 1/5] rtnetlink: provide permanent hardware address in RTM_NEWLINK Michal Kubecek 2019-12-09 19:55 ` [PATCH net-next 2/5] netlink: rename nl80211_validate_nested() to nla_validate_nested() Michal Kubecek @ 2019-12-09 19:55 ` Michal Kubecek 2019-12-09 19:55 ` [PATCH net-next 4/5] ethtool: move string arrays into common file Michal Kubecek 2019-12-09 19:55 ` [PATCH net-next 5/5] ethtool: provide link mode names as a string set Michal Kubecek 4 siblings, 0 replies; 10+ messages in thread From: Michal Kubecek @ 2019-12-09 19:55 UTC (permalink / raw) To: David Miller, netdev Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg, linux-kernel The ethtool netlink interface is going to be split into multiple files so that it will be more convenient to put all of them in a separate directory net/ethtool. Start by moving current ethtool.c with ioctl interface into this directory and renaming it to ioctl.c. Signed-off-by: Michal Kubecek <mkubecek@suse.cz> Acked-by: Jiri Pirko <jiri@mellanox.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> --- net/Makefile | 2 +- net/core/Makefile | 2 +- net/ethtool/Makefile | 3 +++ net/{core/ethtool.c => ethtool/ioctl.c} | 0 4 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 net/ethtool/Makefile rename net/{core/ethtool.c => ethtool/ioctl.c} (100%) diff --git a/net/Makefile b/net/Makefile index 449fc0b221f8..848303d98d3d 100644 --- a/net/Makefile +++ b/net/Makefile @@ -13,7 +13,7 @@ obj-$(CONFIG_NET) += $(tmp-y) # LLC has to be linked before the files in net/802/ obj-$(CONFIG_LLC) += llc/ -obj-$(CONFIG_NET) += ethernet/ 802/ sched/ netlink/ bpf/ +obj-$(CONFIG_NET) += ethernet/ 802/ sched/ netlink/ bpf/ ethtool/ obj-$(CONFIG_NETFILTER) += netfilter/ obj-$(CONFIG_INET) += ipv4/ obj-$(CONFIG_TLS) += tls/ diff --git a/net/core/Makefile b/net/core/Makefile index a104dc8faafc..3e2c378e5f31 100644 --- a/net/core/Makefile +++ b/net/core/Makefile @@ -8,7 +8,7 @@ obj-y := sock.o request_sock.o skbuff.o datagram.o stream.o scm.o \ obj-$(CONFIG_SYSCTL) += sysctl_net_core.o -obj-y += dev.o ethtool.o dev_addr_lists.o dst.o netevent.o \ +obj-y += dev.o dev_addr_lists.o dst.o netevent.o \ neighbour.o rtnetlink.o utils.o link_watch.o filter.o \ sock_diag.o dev_ioctl.o tso.o sock_reuseport.o \ fib_notifier.o xdp.o flow_offload.o diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile new file mode 100644 index 000000000000..3ebfab2bca66 --- /dev/null +++ b/net/ethtool/Makefile @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 + +obj-y += ioctl.o diff --git a/net/core/ethtool.c b/net/ethtool/ioctl.c similarity index 100% rename from net/core/ethtool.c rename to net/ethtool/ioctl.c -- 2.24.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net-next 4/5] ethtool: move string arrays into common file 2019-12-09 19:55 [PATCH net-next 0/5] ethtool netlink interface, preliminary patches Michal Kubecek ` (2 preceding siblings ...) 2019-12-09 19:55 ` [PATCH net-next 3/5] ethtool: move to its own directory Michal Kubecek @ 2019-12-09 19:55 ` Michal Kubecek 2019-12-09 19:55 ` [PATCH net-next 5/5] ethtool: provide link mode names as a string set Michal Kubecek 4 siblings, 0 replies; 10+ messages in thread From: Michal Kubecek @ 2019-12-09 19:55 UTC (permalink / raw) To: David Miller, netdev Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg, linux-kernel Introduce file net/ethtool/common.c for code shared by ioctl and netlink ethtool interface. Move name tables of features, RSS hash functions, tunables and PHY tunables into this file. Signed-off-by: Michal Kubecek <mkubecek@suse.cz> Reviewed-by: Jiri Pirko <jiri@mellanox.com> --- net/ethtool/Makefile | 2 +- net/ethtool/common.c | 85 ++++++++++++++++++++++++++++++++++++++++++++ net/ethtool/common.h | 17 +++++++++ net/ethtool/ioctl.c | 84 ++----------------------------------------- 4 files changed, 105 insertions(+), 83 deletions(-) create mode 100644 net/ethtool/common.c create mode 100644 net/ethtool/common.h diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile index 3ebfab2bca66..fe70d3a1d545 100644 --- a/net/ethtool/Makefile +++ b/net/ethtool/Makefile @@ -1,3 +1,3 @@ # SPDX-License-Identifier: GPL-2.0 -obj-y += ioctl.o +obj-y += ioctl.o common.o diff --git a/net/ethtool/common.c b/net/ethtool/common.c new file mode 100644 index 000000000000..220d6b539180 --- /dev/null +++ b/net/ethtool/common.c @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note + +#include "common.h" + +const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = { + [NETIF_F_SG_BIT] = "tx-scatter-gather", + [NETIF_F_IP_CSUM_BIT] = "tx-checksum-ipv4", + [NETIF_F_HW_CSUM_BIT] = "tx-checksum-ip-generic", + [NETIF_F_IPV6_CSUM_BIT] = "tx-checksum-ipv6", + [NETIF_F_HIGHDMA_BIT] = "highdma", + [NETIF_F_FRAGLIST_BIT] = "tx-scatter-gather-fraglist", + [NETIF_F_HW_VLAN_CTAG_TX_BIT] = "tx-vlan-hw-insert", + + [NETIF_F_HW_VLAN_CTAG_RX_BIT] = "rx-vlan-hw-parse", + [NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter", + [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert", + [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse", + [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter", + [NETIF_F_VLAN_CHALLENGED_BIT] = "vlan-challenged", + [NETIF_F_GSO_BIT] = "tx-generic-segmentation", + [NETIF_F_LLTX_BIT] = "tx-lockless", + [NETIF_F_NETNS_LOCAL_BIT] = "netns-local", + [NETIF_F_GRO_BIT] = "rx-gro", + [NETIF_F_GRO_HW_BIT] = "rx-gro-hw", + [NETIF_F_LRO_BIT] = "rx-lro", + + [NETIF_F_TSO_BIT] = "tx-tcp-segmentation", + [NETIF_F_GSO_ROBUST_BIT] = "tx-gso-robust", + [NETIF_F_TSO_ECN_BIT] = "tx-tcp-ecn-segmentation", + [NETIF_F_TSO_MANGLEID_BIT] = "tx-tcp-mangleid-segmentation", + [NETIF_F_TSO6_BIT] = "tx-tcp6-segmentation", + [NETIF_F_FSO_BIT] = "tx-fcoe-segmentation", + [NETIF_F_GSO_GRE_BIT] = "tx-gre-segmentation", + [NETIF_F_GSO_GRE_CSUM_BIT] = "tx-gre-csum-segmentation", + [NETIF_F_GSO_IPXIP4_BIT] = "tx-ipxip4-segmentation", + [NETIF_F_GSO_IPXIP6_BIT] = "tx-ipxip6-segmentation", + [NETIF_F_GSO_UDP_TUNNEL_BIT] = "tx-udp_tnl-segmentation", + [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation", + [NETIF_F_GSO_PARTIAL_BIT] = "tx-gso-partial", + [NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation", + [NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation", + [NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation", + + [NETIF_F_FCOE_CRC_BIT] = "tx-checksum-fcoe-crc", + [NETIF_F_SCTP_CRC_BIT] = "tx-checksum-sctp", + [NETIF_F_FCOE_MTU_BIT] = "fcoe-mtu", + [NETIF_F_NTUPLE_BIT] = "rx-ntuple-filter", + [NETIF_F_RXHASH_BIT] = "rx-hashing", + [NETIF_F_RXCSUM_BIT] = "rx-checksum", + [NETIF_F_NOCACHE_COPY_BIT] = "tx-nocache-copy", + [NETIF_F_LOOPBACK_BIT] = "loopback", + [NETIF_F_RXFCS_BIT] = "rx-fcs", + [NETIF_F_RXALL_BIT] = "rx-all", + [NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload", + [NETIF_F_HW_TC_BIT] = "hw-tc-offload", + [NETIF_F_HW_ESP_BIT] = "esp-hw-offload", + [NETIF_F_HW_ESP_TX_CSUM_BIT] = "esp-tx-csum-hw-offload", + [NETIF_F_RX_UDP_TUNNEL_PORT_BIT] = "rx-udp_tunnel-port-offload", + [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record", + [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload", + [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload", +}; + +const char +rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = { + [ETH_RSS_HASH_TOP_BIT] = "toeplitz", + [ETH_RSS_HASH_XOR_BIT] = "xor", + [ETH_RSS_HASH_CRC32_BIT] = "crc32", +}; + +const char +tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = { + [ETHTOOL_ID_UNSPEC] = "Unspec", + [ETHTOOL_RX_COPYBREAK] = "rx-copybreak", + [ETHTOOL_TX_COPYBREAK] = "tx-copybreak", + [ETHTOOL_PFC_PREVENTION_TOUT] = "pfc-prevention-tout", +}; + +const char +phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = { + [ETHTOOL_ID_UNSPEC] = "Unspec", + [ETHTOOL_PHY_DOWNSHIFT] = "phy-downshift", + [ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down", + [ETHTOOL_PHY_EDPD] = "phy-energy-detect-power-down", +}; diff --git a/net/ethtool/common.h b/net/ethtool/common.h new file mode 100644 index 000000000000..41b2efc1e4e1 --- /dev/null +++ b/net/ethtool/common.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ + +#ifndef _ETHTOOL_COMMON_H +#define _ETHTOOL_COMMON_H + +#include <linux/ethtool.h> + +extern const char +netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]; +extern const char +rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN]; +extern const char +tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN]; +extern const char +phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN]; + +#endif /* _ETHTOOL_COMMON_H */ diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index cd9bc67381b2..b262db5a1d91 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -27,6 +27,8 @@ #include <net/xdp_sock.h> #include <net/flow_offload.h> +#include "common.h" + /* * Some useful ethtool_ops methods that're device independent. * If we find that all drivers want to do the same thing here, @@ -54,88 +56,6 @@ EXPORT_SYMBOL(ethtool_op_get_ts_info); #define ETHTOOL_DEV_FEATURE_WORDS ((NETDEV_FEATURE_COUNT + 31) / 32) -static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = { - [NETIF_F_SG_BIT] = "tx-scatter-gather", - [NETIF_F_IP_CSUM_BIT] = "tx-checksum-ipv4", - [NETIF_F_HW_CSUM_BIT] = "tx-checksum-ip-generic", - [NETIF_F_IPV6_CSUM_BIT] = "tx-checksum-ipv6", - [NETIF_F_HIGHDMA_BIT] = "highdma", - [NETIF_F_FRAGLIST_BIT] = "tx-scatter-gather-fraglist", - [NETIF_F_HW_VLAN_CTAG_TX_BIT] = "tx-vlan-hw-insert", - - [NETIF_F_HW_VLAN_CTAG_RX_BIT] = "rx-vlan-hw-parse", - [NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter", - [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert", - [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse", - [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter", - [NETIF_F_VLAN_CHALLENGED_BIT] = "vlan-challenged", - [NETIF_F_GSO_BIT] = "tx-generic-segmentation", - [NETIF_F_LLTX_BIT] = "tx-lockless", - [NETIF_F_NETNS_LOCAL_BIT] = "netns-local", - [NETIF_F_GRO_BIT] = "rx-gro", - [NETIF_F_GRO_HW_BIT] = "rx-gro-hw", - [NETIF_F_LRO_BIT] = "rx-lro", - - [NETIF_F_TSO_BIT] = "tx-tcp-segmentation", - [NETIF_F_GSO_ROBUST_BIT] = "tx-gso-robust", - [NETIF_F_TSO_ECN_BIT] = "tx-tcp-ecn-segmentation", - [NETIF_F_TSO_MANGLEID_BIT] = "tx-tcp-mangleid-segmentation", - [NETIF_F_TSO6_BIT] = "tx-tcp6-segmentation", - [NETIF_F_FSO_BIT] = "tx-fcoe-segmentation", - [NETIF_F_GSO_GRE_BIT] = "tx-gre-segmentation", - [NETIF_F_GSO_GRE_CSUM_BIT] = "tx-gre-csum-segmentation", - [NETIF_F_GSO_IPXIP4_BIT] = "tx-ipxip4-segmentation", - [NETIF_F_GSO_IPXIP6_BIT] = "tx-ipxip6-segmentation", - [NETIF_F_GSO_UDP_TUNNEL_BIT] = "tx-udp_tnl-segmentation", - [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation", - [NETIF_F_GSO_PARTIAL_BIT] = "tx-gso-partial", - [NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation", - [NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation", - [NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation", - - [NETIF_F_FCOE_CRC_BIT] = "tx-checksum-fcoe-crc", - [NETIF_F_SCTP_CRC_BIT] = "tx-checksum-sctp", - [NETIF_F_FCOE_MTU_BIT] = "fcoe-mtu", - [NETIF_F_NTUPLE_BIT] = "rx-ntuple-filter", - [NETIF_F_RXHASH_BIT] = "rx-hashing", - [NETIF_F_RXCSUM_BIT] = "rx-checksum", - [NETIF_F_NOCACHE_COPY_BIT] = "tx-nocache-copy", - [NETIF_F_LOOPBACK_BIT] = "loopback", - [NETIF_F_RXFCS_BIT] = "rx-fcs", - [NETIF_F_RXALL_BIT] = "rx-all", - [NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload", - [NETIF_F_HW_TC_BIT] = "hw-tc-offload", - [NETIF_F_HW_ESP_BIT] = "esp-hw-offload", - [NETIF_F_HW_ESP_TX_CSUM_BIT] = "esp-tx-csum-hw-offload", - [NETIF_F_RX_UDP_TUNNEL_PORT_BIT] = "rx-udp_tunnel-port-offload", - [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record", - [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload", - [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload", -}; - -static const char -rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = { - [ETH_RSS_HASH_TOP_BIT] = "toeplitz", - [ETH_RSS_HASH_XOR_BIT] = "xor", - [ETH_RSS_HASH_CRC32_BIT] = "crc32", -}; - -static const char -tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = { - [ETHTOOL_ID_UNSPEC] = "Unspec", - [ETHTOOL_RX_COPYBREAK] = "rx-copybreak", - [ETHTOOL_TX_COPYBREAK] = "tx-copybreak", - [ETHTOOL_PFC_PREVENTION_TOUT] = "pfc-prevention-tout", -}; - -static const char -phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = { - [ETHTOOL_ID_UNSPEC] = "Unspec", - [ETHTOOL_PHY_DOWNSHIFT] = "phy-downshift", - [ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down", - [ETHTOOL_PHY_EDPD] = "phy-energy-detect-power-down", -}; - static int ethtool_get_features(struct net_device *dev, void __user *useraddr) { struct ethtool_gfeatures cmd = { -- 2.24.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net-next 5/5] ethtool: provide link mode names as a string set 2019-12-09 19:55 [PATCH net-next 0/5] ethtool netlink interface, preliminary patches Michal Kubecek ` (3 preceding siblings ...) 2019-12-09 19:55 ` [PATCH net-next 4/5] ethtool: move string arrays into common file Michal Kubecek @ 2019-12-09 19:55 ` Michal Kubecek 2019-12-09 20:15 ` Andrew Lunn 4 siblings, 1 reply; 10+ messages in thread From: Michal Kubecek @ 2019-12-09 19:55 UTC (permalink / raw) To: David Miller, netdev Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg, linux-kernel Unlike e.g. netdev features, the ethtool ioctl interface requires link mode table to be in sync between kernel and userspace for userspace to be able to display and set all link modes supported by kernel. The way arbitrary length bitsets are implemented in netlink interface, this will be no longer needed. To allow userspace to access all link modes running kernel supports, add table of ethernet link mode names and make it available as a string set to userspace GET_STRSET requests. Add build time check to make sure names are defined for all modes declared in enum ethtool_link_mode_bit_indices. Once the string set is available, make it also accessible via ioctl. Signed-off-by: Michal Kubecek <mkubecek@suse.cz> --- include/linux/ethtool.h | 4 ++ include/uapi/linux/ethtool.h | 2 + net/ethtool/common.c | 86 ++++++++++++++++++++++++++++++++++++ net/ethtool/common.h | 2 + net/ethtool/ioctl.c | 5 +++ 5 files changed, 99 insertions(+) diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 95991e4300bf..5caef65d93d6 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -102,6 +102,10 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings) #define __ETHTOOL_DECLARE_LINK_MODE_MASK(name) \ DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS) +/* compose link mode index from speed, type and duplex */ +#define ETHTOOL_LINK_MODE(speed, type, duplex) \ + ETHTOOL_LINK_MODE_ ## speed ## base ## type ## _ ## duplex ## _BIT + /* drivers must ignore base.cmd and base.link_mode_masks_nwords * fields, but they are allowed to overwrite them (will be ignored). */ diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index d4591792f0b4..f44155840b07 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -593,6 +593,7 @@ struct ethtool_pauseparam { * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS * @ETH_SS_PHY_TUNABLES: PHY tunable names + * @ETH_SS_LINK_MODES: link mode names */ enum ethtool_stringset { ETH_SS_TEST = 0, @@ -604,6 +605,7 @@ enum ethtool_stringset { ETH_SS_TUNABLES, ETH_SS_PHY_STATS, ETH_SS_PHY_TUNABLES, + ETH_SS_LINK_MODES, }; /** diff --git a/net/ethtool/common.c b/net/ethtool/common.c index 220d6b539180..be1b26970eb1 100644 --- a/net/ethtool/common.c +++ b/net/ethtool/common.c @@ -83,3 +83,89 @@ phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = { [ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down", [ETHTOOL_PHY_EDPD] = "phy-energy-detect-power-down", }; + +#define __LINK_MODE_NAME(speed, type, duplex) \ + #speed "base" #type "/" #duplex +#define __DEFINE_LINK_MODE_NAME(speed, type, duplex) \ + [ETHTOOL_LINK_MODE(speed, type, duplex)] = \ + __LINK_MODE_NAME(speed, type, duplex) +#define __DEFINE_SPECIAL_MODE_NAME(_mode, _name) \ + [ETHTOOL_LINK_MODE_ ## _mode ## _BIT] = _name + +const char +link_mode_names[__ETHTOOL_LINK_MODE_MASK_NBITS][ETH_GSTRING_LEN] = { + __DEFINE_LINK_MODE_NAME(10, T, Half), + __DEFINE_LINK_MODE_NAME(10, T, Full), + __DEFINE_LINK_MODE_NAME(100, T, Half), + __DEFINE_LINK_MODE_NAME(100, T, Full), + __DEFINE_LINK_MODE_NAME(1000, T, Half), + __DEFINE_LINK_MODE_NAME(1000, T, Full), + __DEFINE_SPECIAL_MODE_NAME(Autoneg, "Autoneg"), + __DEFINE_SPECIAL_MODE_NAME(TP, "TP"), + __DEFINE_SPECIAL_MODE_NAME(AUI, "AUI"), + __DEFINE_SPECIAL_MODE_NAME(MII, "MII"), + __DEFINE_SPECIAL_MODE_NAME(FIBRE, "FIBRE"), + __DEFINE_SPECIAL_MODE_NAME(BNC, "BNC"), + __DEFINE_LINK_MODE_NAME(10000, T, Full), + __DEFINE_SPECIAL_MODE_NAME(Pause, "Pause"), + __DEFINE_SPECIAL_MODE_NAME(Asym_Pause, "Asym_Pause"), + __DEFINE_LINK_MODE_NAME(2500, X, Full), + __DEFINE_SPECIAL_MODE_NAME(Backplane, "Backplane"), + __DEFINE_LINK_MODE_NAME(1000, KX, Full), + __DEFINE_LINK_MODE_NAME(10000, KX4, Full), + __DEFINE_LINK_MODE_NAME(10000, KR, Full), + [ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = "10000baseR_FEC", + __DEFINE_LINK_MODE_NAME(20000, MLD2, Full), + __DEFINE_LINK_MODE_NAME(20000, KR2, Full), + __DEFINE_LINK_MODE_NAME(40000, KR4, Full), + __DEFINE_LINK_MODE_NAME(40000, CR4, Full), + __DEFINE_LINK_MODE_NAME(40000, SR4, Full), + __DEFINE_LINK_MODE_NAME(40000, LR4, Full), + __DEFINE_LINK_MODE_NAME(56000, KR4, Full), + __DEFINE_LINK_MODE_NAME(56000, CR4, Full), + __DEFINE_LINK_MODE_NAME(56000, SR4, Full), + __DEFINE_LINK_MODE_NAME(56000, LR4, Full), + __DEFINE_LINK_MODE_NAME(25000, CR, Full), + __DEFINE_LINK_MODE_NAME(25000, KR, Full), + __DEFINE_LINK_MODE_NAME(25000, SR, Full), + __DEFINE_LINK_MODE_NAME(50000, CR2, Full), + __DEFINE_LINK_MODE_NAME(50000, KR2, Full), + __DEFINE_LINK_MODE_NAME(100000, KR4, Full), + __DEFINE_LINK_MODE_NAME(100000, SR4, Full), + __DEFINE_LINK_MODE_NAME(100000, CR4, Full), + __DEFINE_LINK_MODE_NAME(100000, LR4_ER4, Full), + __DEFINE_LINK_MODE_NAME(50000, SR2, Full), + __DEFINE_LINK_MODE_NAME(1000, X, Full), + __DEFINE_LINK_MODE_NAME(10000, CR, Full), + __DEFINE_LINK_MODE_NAME(10000, SR, Full), + __DEFINE_LINK_MODE_NAME(10000, LR, Full), + __DEFINE_LINK_MODE_NAME(10000, LRM, Full), + __DEFINE_LINK_MODE_NAME(10000, ER, Full), + __DEFINE_LINK_MODE_NAME(2500, T, Full), + __DEFINE_LINK_MODE_NAME(5000, T, Full), + __DEFINE_SPECIAL_MODE_NAME(FEC_NONE, "None"), + __DEFINE_SPECIAL_MODE_NAME(FEC_RS, "RS"), + __DEFINE_SPECIAL_MODE_NAME(FEC_BASER, "BASER"), + __DEFINE_LINK_MODE_NAME(50000, KR, Full), + __DEFINE_LINK_MODE_NAME(50000, SR, Full), + __DEFINE_LINK_MODE_NAME(50000, CR, Full), + __DEFINE_LINK_MODE_NAME(50000, LR_ER_FR, Full), + __DEFINE_LINK_MODE_NAME(50000, DR, Full), + __DEFINE_LINK_MODE_NAME(100000, KR2, Full), + __DEFINE_LINK_MODE_NAME(100000, SR2, Full), + __DEFINE_LINK_MODE_NAME(100000, CR2, Full), + __DEFINE_LINK_MODE_NAME(100000, LR2_ER2_FR2, Full), + __DEFINE_LINK_MODE_NAME(100000, DR2, Full), + __DEFINE_LINK_MODE_NAME(200000, KR4, Full), + __DEFINE_LINK_MODE_NAME(200000, SR4, Full), + __DEFINE_LINK_MODE_NAME(200000, LR4_ER4_FR4, Full), + __DEFINE_LINK_MODE_NAME(200000, DR4, Full), + __DEFINE_LINK_MODE_NAME(200000, CR4, Full), + __DEFINE_LINK_MODE_NAME(100, T1, Full), + __DEFINE_LINK_MODE_NAME(1000, T1, Full), + __DEFINE_LINK_MODE_NAME(400000, KR8, Full), + __DEFINE_LINK_MODE_NAME(400000, SR8, Full), + __DEFINE_LINK_MODE_NAME(400000, LR8_ER8_FR8, Full), + __DEFINE_LINK_MODE_NAME(400000, DR8, Full), + __DEFINE_LINK_MODE_NAME(400000, CR8, Full), +}; diff --git a/net/ethtool/common.h b/net/ethtool/common.h index 41b2efc1e4e1..351e019b8d85 100644 --- a/net/ethtool/common.h +++ b/net/ethtool/common.h @@ -13,5 +13,7 @@ extern const char tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN]; extern const char phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN]; +extern const char +link_mode_names[__ETHTOOL_LINK_MODE_MASK_NBITS][ETH_GSTRING_LEN]; #endif /* _ETHTOOL_COMMON_H */ diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index b262db5a1d91..9274d70c496b 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -154,6 +154,9 @@ static int __ethtool_get_sset_count(struct net_device *dev, int sset) !ops->get_ethtool_phy_stats) return phy_ethtool_get_sset_count(dev->phydev); + if (sset == ETH_SS_LINK_MODES) + return __ETHTOOL_LINK_MODE_MASK_NBITS; + if (ops->get_sset_count && ops->get_strings) return ops->get_sset_count(dev, sset); else @@ -178,6 +181,8 @@ static void __ethtool_get_strings(struct net_device *dev, else if (stringset == ETH_SS_PHY_STATS && dev->phydev && !ops->get_ethtool_phy_stats) phy_ethtool_get_strings(dev->phydev, data); + else if (stringset == ETH_SS_LINK_MODES) + memcpy(data, link_mode_names, sizeof(link_mode_names)); else /* ops->get_strings is valid because checked earlier */ ops->get_strings(dev, stringset, data); -- 2.24.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 5/5] ethtool: provide link mode names as a string set 2019-12-09 19:55 ` [PATCH net-next 5/5] ethtool: provide link mode names as a string set Michal Kubecek @ 2019-12-09 20:15 ` Andrew Lunn 2019-12-09 20:45 ` Michal Kubecek 0 siblings, 1 reply; 10+ messages in thread From: Andrew Lunn @ 2019-12-09 20:15 UTC (permalink / raw) To: Michal Kubecek Cc: David Miller, netdev, Jakub Kicinski, Jiri Pirko, Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg, linux-kernel On Mon, Dec 09, 2019 at 08:55:45PM +0100, Michal Kubecek wrote: > Unlike e.g. netdev features, the ethtool ioctl interface requires link mode > table to be in sync between kernel and userspace for userspace to be able > to display and set all link modes supported by kernel. The way arbitrary > length bitsets are implemented in netlink interface, this will be no longer > needed. > > To allow userspace to access all link modes running kernel supports, add > table of ethernet link mode names and make it available as a string set to > userspace GET_STRSET requests. Add build time check to make sure names > are defined for all modes declared in enum ethtool_link_mode_bit_indices. Hi Michal Having a build time check is a good idea. However, i don't see it in the code. Please could you point it out. > > Once the string set is available, make it also accessible via ioctl. > > Signed-off-by: Michal Kubecek <mkubecek@suse.cz> > --- > include/linux/ethtool.h | 4 ++ > include/uapi/linux/ethtool.h | 2 + > net/ethtool/common.c | 86 ++++++++++++++++++++++++++++++++++++ > net/ethtool/common.h | 2 + > net/ethtool/ioctl.c | 5 +++ > 5 files changed, 99 insertions(+) > > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h > index 95991e4300bf..5caef65d93d6 100644 > --- a/include/linux/ethtool.h > +++ b/include/linux/ethtool.h > @@ -102,6 +102,10 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings) > #define __ETHTOOL_DECLARE_LINK_MODE_MASK(name) \ > DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS) > > +/* compose link mode index from speed, type and duplex */ > +#define ETHTOOL_LINK_MODE(speed, type, duplex) \ > + ETHTOOL_LINK_MODE_ ## speed ## base ## type ## _ ## duplex ## _BIT > + > /* drivers must ignore base.cmd and base.link_mode_masks_nwords > * fields, but they are allowed to overwrite them (will be ignored). > */ > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h > index d4591792f0b4..f44155840b07 100644 > --- a/include/uapi/linux/ethtool.h > +++ b/include/uapi/linux/ethtool.h > @@ -593,6 +593,7 @@ struct ethtool_pauseparam { > * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names > * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS > * @ETH_SS_PHY_TUNABLES: PHY tunable names > + * @ETH_SS_LINK_MODES: link mode names > */ > enum ethtool_stringset { > ETH_SS_TEST = 0, > @@ -604,6 +605,7 @@ enum ethtool_stringset { > ETH_SS_TUNABLES, > ETH_SS_PHY_STATS, > ETH_SS_PHY_TUNABLES, > + ETH_SS_LINK_MODES, > }; > > /** > diff --git a/net/ethtool/common.c b/net/ethtool/common.c > index 220d6b539180..be1b26970eb1 100644 > --- a/net/ethtool/common.c > +++ b/net/ethtool/common.c > @@ -83,3 +83,89 @@ phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = { > [ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down", > [ETHTOOL_PHY_EDPD] = "phy-energy-detect-power-down", > }; > + > +#define __LINK_MODE_NAME(speed, type, duplex) \ > + #speed "base" #type "/" #duplex > +#define __DEFINE_LINK_MODE_NAME(speed, type, duplex) \ > + [ETHTOOL_LINK_MODE(speed, type, duplex)] = \ > + __LINK_MODE_NAME(speed, type, duplex) > +#define __DEFINE_SPECIAL_MODE_NAME(_mode, _name) \ > + [ETHTOOL_LINK_MODE_ ## _mode ## _BIT] = _name > + > +const char > +link_mode_names[__ETHTOOL_LINK_MODE_MASK_NBITS][ETH_GSTRING_LEN] = { > + __DEFINE_LINK_MODE_NAME(10, T, Half), > + __DEFINE_LINK_MODE_NAME(10, T, Full), > + __DEFINE_LINK_MODE_NAME(100, T, Half), > + __DEFINE_LINK_MODE_NAME(100, T, Full), > + __DEFINE_LINK_MODE_NAME(1000, T, Half), > + __DEFINE_LINK_MODE_NAME(1000, T, Full), > + __DEFINE_SPECIAL_MODE_NAME(Autoneg, "Autoneg"), > + __DEFINE_SPECIAL_MODE_NAME(TP, "TP"), > + __DEFINE_SPECIAL_MODE_NAME(AUI, "AUI"), > + __DEFINE_SPECIAL_MODE_NAME(MII, "MII"), > + __DEFINE_SPECIAL_MODE_NAME(FIBRE, "FIBRE"), > + __DEFINE_SPECIAL_MODE_NAME(BNC, "BNC"), > + __DEFINE_LINK_MODE_NAME(10000, T, Full), > + __DEFINE_SPECIAL_MODE_NAME(Pause, "Pause"), > + __DEFINE_SPECIAL_MODE_NAME(Asym_Pause, "Asym_Pause"), > + __DEFINE_LINK_MODE_NAME(2500, X, Full), > + __DEFINE_SPECIAL_MODE_NAME(Backplane, "Backplane"), > + __DEFINE_LINK_MODE_NAME(1000, KX, Full), > + __DEFINE_LINK_MODE_NAME(10000, KX4, Full), > + __DEFINE_LINK_MODE_NAME(10000, KR, Full), > + [ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = "10000baseR_FEC", Would __DEFINE_SPECIAL_MODE_NAME(10000baseR_FEC, 10000baseR_FEC), work here? Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 5/5] ethtool: provide link mode names as a string set 2019-12-09 20:15 ` Andrew Lunn @ 2019-12-09 20:45 ` Michal Kubecek 0 siblings, 0 replies; 10+ messages in thread From: Michal Kubecek @ 2019-12-09 20:45 UTC (permalink / raw) To: netdev Cc: Andrew Lunn, David Miller, Jakub Kicinski, Jiri Pirko, Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg, linux-kernel On Mon, Dec 09, 2019 at 09:15:11PM +0100, Andrew Lunn wrote: > On Mon, Dec 09, 2019 at 08:55:45PM +0100, Michal Kubecek wrote: > > Unlike e.g. netdev features, the ethtool ioctl interface requires link mode > > table to be in sync between kernel and userspace for userspace to be able > > to display and set all link modes supported by kernel. The way arbitrary > > length bitsets are implemented in netlink interface, this will be no longer > > needed. > > > > To allow userspace to access all link modes running kernel supports, add > > table of ethernet link mode names and make it available as a string set to > > userspace GET_STRSET requests. Add build time check to make sure names > > are defined for all modes declared in enum ethtool_link_mode_bit_indices. > > Hi Michal > > Having a build time check is a good idea. However, i don't see it in > the code. Please could you point it out. Thanks for noticing. The BUILD_BUG_ON() check got lost when I moved the patch to an earlier place and changed the declaration to fixed size array like other name tables have. But you are right, losing the check would be too high price for the uniformity. > > +#define __LINK_MODE_NAME(speed, type, duplex) \ > > + #speed "base" #type "/" #duplex > > +#define __DEFINE_LINK_MODE_NAME(speed, type, duplex) \ > > + [ETHTOOL_LINK_MODE(speed, type, duplex)] = \ > > + __LINK_MODE_NAME(speed, type, duplex) > > +#define __DEFINE_SPECIAL_MODE_NAME(_mode, _name) \ > > + [ETHTOOL_LINK_MODE_ ## _mode ## _BIT] = _name > > + > > +const char > > +link_mode_names[__ETHTOOL_LINK_MODE_MASK_NBITS][ETH_GSTRING_LEN] = { > > + __DEFINE_LINK_MODE_NAME(10, T, Half), > > + __DEFINE_LINK_MODE_NAME(10, T, Full), > > + __DEFINE_LINK_MODE_NAME(100, T, Half), > > + __DEFINE_LINK_MODE_NAME(100, T, Full), > > + __DEFINE_LINK_MODE_NAME(1000, T, Half), > > + __DEFINE_LINK_MODE_NAME(1000, T, Full), > > + __DEFINE_SPECIAL_MODE_NAME(Autoneg, "Autoneg"), > > + __DEFINE_SPECIAL_MODE_NAME(TP, "TP"), > > + __DEFINE_SPECIAL_MODE_NAME(AUI, "AUI"), > > + __DEFINE_SPECIAL_MODE_NAME(MII, "MII"), > > + __DEFINE_SPECIAL_MODE_NAME(FIBRE, "FIBRE"), > > + __DEFINE_SPECIAL_MODE_NAME(BNC, "BNC"), > > + __DEFINE_LINK_MODE_NAME(10000, T, Full), > > + __DEFINE_SPECIAL_MODE_NAME(Pause, "Pause"), > > + __DEFINE_SPECIAL_MODE_NAME(Asym_Pause, "Asym_Pause"), > > + __DEFINE_LINK_MODE_NAME(2500, X, Full), > > + __DEFINE_SPECIAL_MODE_NAME(Backplane, "Backplane"), > > + __DEFINE_LINK_MODE_NAME(1000, KX, Full), > > + __DEFINE_LINK_MODE_NAME(10000, KX4, Full), > > + __DEFINE_LINK_MODE_NAME(10000, KR, Full), > > + [ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = "10000baseR_FEC", > > Would > > __DEFINE_SPECIAL_MODE_NAME(10000baseR_FEC, 10000baseR_FEC), > > work here? Right, it would (with double quotes for second argument). As I wrote the macro for bits which don't represent actual link modes, it never occured to me that it can be also used for this special case. I'll wait until tomorrow to see if there are more comments and send v2. Michal ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-12-09 23:21 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-09 19:55 [PATCH net-next 0/5] ethtool netlink interface, preliminary patches Michal Kubecek 2019-12-09 19:55 ` [PATCH net-next 1/5] rtnetlink: provide permanent hardware address in RTM_NEWLINK Michal Kubecek 2019-12-09 23:20 ` Johannes Berg 2019-12-09 23:21 ` Johannes Berg 2019-12-09 19:55 ` [PATCH net-next 2/5] netlink: rename nl80211_validate_nested() to nla_validate_nested() Michal Kubecek 2019-12-09 19:55 ` [PATCH net-next 3/5] ethtool: move to its own directory Michal Kubecek 2019-12-09 19:55 ` [PATCH net-next 4/5] ethtool: move string arrays into common file Michal Kubecek 2019-12-09 19:55 ` [PATCH net-next 5/5] ethtool: provide link mode names as a string set Michal Kubecek 2019-12-09 20:15 ` Andrew Lunn 2019-12-09 20:45 ` Michal Kubecek
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).