From: Andrew Lunn <andrew@lunn.ch>
To: Michal Kubecek <mkubecek@suse.cz>
Cc: David Miller <davem@davemloft.net>,
netdev@vger.kernel.org,
Jakub Kicinski <jakub.kicinski@netronome.com>,
Jiri Pirko <jiri@resnulli.us>,
Florian Fainelli <f.fainelli@gmail.com>,
John Linville <linville@tuxdriver.com>,
Stephen Hemminger <stephen@networkplumber.org>,
Johannes Berg <johannes@sipsolutions.net>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 5/5] ethtool: provide link mode names as a string set
Date: Mon, 9 Dec 2019 21:15:11 +0100 [thread overview]
Message-ID: <20191209201511.GL9099@lunn.ch> (raw)
In-Reply-To: <0c239334df943c3f5f4ca74a2509754e08eda9e3.1575920565.git.mkubecek@suse.cz>
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
next prev parent reply other threads:[~2019-12-09 20:15 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2019-12-09 20:45 ` Michal Kubecek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191209201511.GL9099@lunn.ch \
--to=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=jakub.kicinski@netronome.com \
--cc=jiri@resnulli.us \
--cc=johannes@sipsolutions.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=mkubecek@suse.cz \
--cc=netdev@vger.kernel.org \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is 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).