| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9404b230-b9b7-4e0e-8573-621e3e55ed47@csgroup.eu>
Date: Wed, 14 Aug 2024 16:30:09 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>, davem@...emloft.net
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
thomas.petazzoni@...tlin.com, Andrew Lunn <andrew@...n.ch>,
Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>,
Paolo Abeni <pabeni@...hat.com>, Russell King <linux@...linux.org.uk>,
linux-arm-kernel@...ts.infradead.org, Herve Codina
<herve.codina@...tlin.com>, Florian Fainelli <f.fainelli@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>,
Vladimir Oltean <vladimir.oltean@....com>,
Köry Maincent <kory.maincent@...tlin.com>,
Jesse Brandeburg <jesse.brandeburg@...el.com>, Marek Behún
<kabel@...nel.org>, Piergiorgio Beruto <piergiorgio.beruto@...il.com>,
Oleksij Rempel <o.rempel@...gutronix.de>,
Nicolò Veronese <nicveronese@...il.com>,
Simon Horman <horms@...nel.org>, mwojtas@...omium.org,
Nathan Chancellor <nathan@...nel.org>, Antoine Tenart <atenart@...nel.org>,
Marc Kleine-Budde <mkl@...gutronix.de>,
Dan Carpenter <dan.carpenter@...aro.org>,
Romain Gantois <romain.gantois@...tlin.com>
Subject: Re: [PATCH net-next v17 05/14] net: ethtool: Allow passing a phy
index for some commands
Le 09/07/2024 à 08:30, Maxime Chevallier a écrit :
> Some netlink commands are target towards ethernet PHYs, to control some
> of their features. As there's several such commands, add the ability to
> pass a PHY index in the ethnl request, which will populate the generic
> ethnl_req_info with the passed phy_index.
>
> Add a helper that netlink command handlers need to use to grab the
> targeted PHY from the req_info. This helper needs to hold rtnl_lock()
> while interacting with the PHY, as it may be removed at any point.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@...tlin.com>
Reviewed-by: Christophe Leroy <christophe.leroy@...roup.eu>
Tested-by: Christophe Leroy <christophe.leroy@...roup.eu>
> ---
> Documentation/networking/ethtool-netlink.rst | 7 +++
> include/uapi/linux/ethtool_netlink.h | 1 +
> net/ethtool/netlink.c | 57 +++++++++++++++++++-
> net/ethtool/netlink.h | 28 ++++++++++
> 4 files changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
> index 3ab423b80e91..a6ea3716315f 100644
> --- a/Documentation/networking/ethtool-netlink.rst
> +++ b/Documentation/networking/ethtool-netlink.rst
> @@ -57,6 +57,7 @@ Structure of this header is
> ``ETHTOOL_A_HEADER_DEV_INDEX`` u32 device ifindex
> ``ETHTOOL_A_HEADER_DEV_NAME`` string device name
> ``ETHTOOL_A_HEADER_FLAGS`` u32 flags common for all requests
> + ``ETHTOOL_A_HEADER_PHY_INDEX`` u32 phy device index
> ============================== ====== =============================
>
> ``ETHTOOL_A_HEADER_DEV_INDEX`` and ``ETHTOOL_A_HEADER_DEV_NAME`` identify the
> @@ -81,6 +82,12 @@ the behaviour is backward compatible, i.e. requests from old clients not aware
> of the flag should be interpreted the way the client expects. A client must
> not set flags it does not understand.
>
> +``ETHTOOL_A_HEADER_PHY_INDEX`` identifies the Ethernet PHY the message relates to.
> +As there are numerous commands that are related to PHY configuration, and because
> +there may be more than one PHY on the link, the PHY index can be passed in the
> +request for the commands that needs it. It is, however, not mandatory, and if it
> +is not passed for commands that target a PHY, the net_device.phydev pointer
> +is used.
>
> Bit sets
> ========
> diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
> index 6d5bdcc67631..c5af23139e63 100644
> --- a/include/uapi/linux/ethtool_netlink.h
> +++ b/include/uapi/linux/ethtool_netlink.h
> @@ -134,6 +134,7 @@ enum {
> ETHTOOL_A_HEADER_DEV_INDEX, /* u32 */
> ETHTOOL_A_HEADER_DEV_NAME, /* string */
> ETHTOOL_A_HEADER_FLAGS, /* u32 - ETHTOOL_FLAG_* */
> + ETHTOOL_A_HEADER_PHY_INDEX, /* u32 */
>
> /* add new constants above here */
> __ETHTOOL_A_HEADER_CNT,
> diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
> index cb1eea00e349..7b95571a6efb 100644
> --- a/net/ethtool/netlink.c
> +++ b/net/ethtool/netlink.c
> @@ -2,6 +2,7 @@
>
> #include <net/sock.h>
> #include <linux/ethtool_netlink.h>
> +#include <linux/phy_link_topology.h>
> #include <linux/pm_runtime.h>
> #include "netlink.h"
> #include "module_fw.h"
> @@ -31,6 +32,24 @@ const struct nla_policy ethnl_header_policy_stats[] = {
> ETHTOOL_FLAGS_STATS),
> };
>
> +const struct nla_policy ethnl_header_policy_phy[] = {
> + [ETHTOOL_A_HEADER_DEV_INDEX] = { .type = NLA_U32 },
> + [ETHTOOL_A_HEADER_DEV_NAME] = { .type = NLA_NUL_STRING,
> + .len = ALTIFNAMSIZ - 1 },
> + [ETHTOOL_A_HEADER_FLAGS] = NLA_POLICY_MASK(NLA_U32,
> + ETHTOOL_FLAGS_BASIC),
> + [ETHTOOL_A_HEADER_PHY_INDEX] = NLA_POLICY_MIN(NLA_U32, 1),
> +};
> +
> +const struct nla_policy ethnl_header_policy_phy_stats[] = {
> + [ETHTOOL_A_HEADER_DEV_INDEX] = { .type = NLA_U32 },
> + [ETHTOOL_A_HEADER_DEV_NAME] = { .type = NLA_NUL_STRING,
> + .len = ALTIFNAMSIZ - 1 },
> + [ETHTOOL_A_HEADER_FLAGS] = NLA_POLICY_MASK(NLA_U32,
> + ETHTOOL_FLAGS_STATS),
> + [ETHTOOL_A_HEADER_PHY_INDEX] = NLA_POLICY_MIN(NLA_U32, 1),
> +};
> +
> int ethnl_sock_priv_set(struct sk_buff *skb, struct net_device *dev, u32 portid,
> enum ethnl_sock_type type)
> {
> @@ -119,7 +138,7 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
> const struct nlattr *header, struct net *net,
> struct netlink_ext_ack *extack, bool require_dev)
> {
> - struct nlattr *tb[ARRAY_SIZE(ethnl_header_policy)];
> + struct nlattr *tb[ARRAY_SIZE(ethnl_header_policy_phy)];
> const struct nlattr *devname_attr;
> struct net_device *dev = NULL;
> u32 flags = 0;
> @@ -134,7 +153,7 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
> /* No validation here, command policy should have a nested policy set
> * for the header, therefore validation should have already been done.
> */
> - ret = nla_parse_nested(tb, ARRAY_SIZE(ethnl_header_policy) - 1, header,
> + ret = nla_parse_nested(tb, ARRAY_SIZE(ethnl_header_policy_phy) - 1, header,
> NULL, extack);
> if (ret < 0)
> return ret;
> @@ -175,11 +194,45 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
> return -EINVAL;
> }
>
> + if (tb[ETHTOOL_A_HEADER_PHY_INDEX]) {
> + if (dev) {
> + req_info->phy_index = nla_get_u32(tb[ETHTOOL_A_HEADER_PHY_INDEX]);
> + } else {
> + NL_SET_ERR_MSG_ATTR(extack, header,
> + "phy_index set without a netdev");
> + return -EINVAL;
> + }
> + }
> +
> req_info->dev = dev;
> req_info->flags = flags;
> return 0;
> }
>
> +struct phy_device *ethnl_req_get_phydev(const struct ethnl_req_info *req_info,
> + const struct nlattr *header,
> + struct netlink_ext_ack *extack)
> +{
> + struct phy_device *phydev;
> +
> + ASSERT_RTNL();
> +
> + if (!req_info->dev)
> + return NULL;
> +
> + if (!req_info->phy_index)
> + return req_info->dev->phydev;
> +
> + phydev = phy_link_topo_get_phy(req_info->dev, req_info->phy_index);
> + if (!phydev) {
> + NL_SET_ERR_MSG_ATTR(extack, header,
> + "no phy matching phyindex");
> + return ERR_PTR(-ENODEV);
> + }
> +
> + return phydev;
> +}
> +
> /**
> * ethnl_fill_reply_header() - Put common header into a reply message
> * @skb: skb with the message
> diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
> index 46ec273a87c5..4db16048f8d4 100644
> --- a/net/ethtool/netlink.h
> +++ b/net/ethtool/netlink.h
> @@ -251,6 +251,9 @@ static inline unsigned int ethnl_reply_header_size(void)
> * @dev: network device the request is for (may be null)
> * @dev_tracker: refcount tracker for @dev reference
> * @flags: request flags common for all request types
> + * @phy_index: phy_device index connected to @dev this request is for. Can be
> + * 0 if the request doesn't target a phy, or if the @dev's attached
> + * phy is targeted.
> *
> * This is a common base for request specific structures holding data from
> * parsed userspace request. These always embed struct ethnl_req_info at
> @@ -260,6 +263,7 @@ struct ethnl_req_info {
> struct net_device *dev;
> netdevice_tracker dev_tracker;
> u32 flags;
> + u32 phy_index;
> };
>
> static inline void ethnl_parse_header_dev_put(struct ethnl_req_info *req_info)
> @@ -267,6 +271,27 @@ static inline void ethnl_parse_header_dev_put(struct ethnl_req_info *req_info)
> netdev_put(req_info->dev, &req_info->dev_tracker);
> }
>
> +/**
> + * ethnl_req_get_phydev() - Gets the phy_device targeted by this request,
> + * if any. Must be called under rntl_lock().
> + * @req_info: The ethnl request to get the phy from.
> + * @header: The netlink header, used for error reporting.
> + * @extack: The netlink extended ACK, for error reporting.
> + *
> + * The caller must hold RTNL, until it's done interacting with the returned
> + * phy_device.
> + *
> + * Return: A phy_device pointer corresponding either to the passed phy_index
> + * if one is provided. If not, the phy_device attached to the
> + * net_device targeted by this request is returned. If there's no
> + * targeted net_device, or no phy_device is attached, NULL is
> + * returned. If the provided phy_index is invalid, an error pointer
> + * is returned.
> + */
> +struct phy_device *ethnl_req_get_phydev(const struct ethnl_req_info *req_info,
> + const struct nlattr *header,
> + struct netlink_ext_ack *extack);
> +
> /**
> * struct ethnl_reply_data - base type of reply data for GET requests
> * @dev: device for current reply message; in single shot requests it is
> @@ -409,9 +434,12 @@ extern const struct ethnl_request_ops ethnl_rss_request_ops;
> extern const struct ethnl_request_ops ethnl_plca_cfg_request_ops;
> extern const struct ethnl_request_ops ethnl_plca_status_request_ops;
> extern const struct ethnl_request_ops ethnl_mm_request_ops;
> +extern const struct ethnl_request_ops ethnl_phy_request_ops;
>
> extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1];
> extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1];
> +extern const struct nla_policy ethnl_header_policy_phy[ETHTOOL_A_HEADER_PHY_INDEX + 1];
> +extern const struct nla_policy ethnl_header_policy_phy_stats[ETHTOOL_A_HEADER_PHY_INDEX + 1];
> extern const struct nla_policy ethnl_strset_get_policy[ETHTOOL_A_STRSET_COUNTS_ONLY + 1];
> extern const struct nla_policy ethnl_linkinfo_get_policy[ETHTOOL_A_LINKINFO_HEADER + 1];
> extern const struct nla_policy ethnl_linkinfo_set_policy[ETHTOOL_A_LINKINFO_TP_MDIX_CTRL + 1];
Powered by blists - more mailing lists