[RFC,net-next,v2,05/10] net: ethtool: Allow passing a phy index for some commands
Commit Message
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 relevant phydev when the command targets a PHY.
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
include/uapi/linux/ethtool_netlink.h | 1 +
net/ethtool/netlink.c | 22 ++++++++++++++++++++++
net/ethtool/netlink.h | 8 ++++++--
3 files changed, 29 insertions(+), 2 deletions(-)
Comments
> + if (dev) {
> + if (tb[ETHTOOL_A_HEADER_PHY_INDEX]) {
> + u32 phy_index = nla_get_u32(tb[ETHTOOL_A_HEADER_PHY_INDEX]);
> +
> + phydev = link_topo_get_phy(&dev->link_topo, phy_index);
struct phy_device *link_topo_get_phy(struct link_topology *lt, int phyindex)
We have u32 vs int here for phyindex. It would be good to have the
same type everywhere.
> + if (!phydev) {
> + NL_SET_ERR_MSG_ATTR(extack, header, "no phy matches phy index");
> + return -EINVAL;
> + }
> + } else {
> + /* If we need a PHY but no phy index is specified, fallback
> + * to dev->phydev
> + */
> + phydev = dev->phydev;
> + }
> + }
> +
> + req_info->phydev = phydev;
Don't forget to update Documentation/networking/ethtool-netlink.rst.
Andrew
On Tue, 21 Nov 2023 02:08:37 +0100
Andrew Lunn <andrew@lunn.ch> wrote:
> > + if (dev) {
> > + if (tb[ETHTOOL_A_HEADER_PHY_INDEX]) {
> > + u32 phy_index = nla_get_u32(tb[ETHTOOL_A_HEADER_PHY_INDEX]);
> > +
> > + phydev = link_topo_get_phy(&dev->link_topo, phy_index);
>
> struct phy_device *link_topo_get_phy(struct link_topology *lt, int phyindex)
>
> We have u32 vs int here for phyindex. It would be good to have the
> same type everywhere.
Indeed I messed-up the typing for that variable, shame as it's the core
of that series :(
I'll get it right for the next version.
>
> > + if (!phydev) {
> > + NL_SET_ERR_MSG_ATTR(extack, header, "no phy matches phy index");
> > + return -EINVAL;
> > + }
> > + } else {
> > + /* If we need a PHY but no phy index is specified, fallback
> > + * to dev->phydev
> > + */
> > + phydev = dev->phydev;
> > + }
> > + }
> > +
> > + req_info->phydev = phydev;
>
> Don't forget to update Documentation/networking/ethtool-netlink.rst.
Yep I'll squeeze the documentation bit here.
Thanks for the review,
Maxime
> Andrew
@@ -133,6 +133,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,
@@ -4,6 +4,7 @@
#include <linux/ethtool_netlink.h>
#include <linux/pm_runtime.h>
#include "netlink.h"
+#include <linux/link_topology.h>
static struct genl_family ethtool_genl_family;
@@ -20,6 +21,7 @@ const struct nla_policy ethnl_header_policy[] = {
.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_stats[] = {
@@ -28,6 +30,7 @@ const struct nla_policy ethnl_header_policy_stats[] = {
.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_ops_begin(struct net_device *dev)
@@ -91,6 +94,7 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
{
struct nlattr *tb[ARRAY_SIZE(ethnl_header_policy)];
const struct nlattr *devname_attr;
+ struct phy_device *phydev = NULL;
struct net_device *dev = NULL;
u32 flags = 0;
int ret;
@@ -145,6 +149,24 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
return -EINVAL;
}
+ if (dev) {
+ if (tb[ETHTOOL_A_HEADER_PHY_INDEX]) {
+ u32 phy_index = nla_get_u32(tb[ETHTOOL_A_HEADER_PHY_INDEX]);
+
+ phydev = link_topo_get_phy(&dev->link_topo, phy_index);
+ if (!phydev) {
+ NL_SET_ERR_MSG_ATTR(extack, header, "no phy matches phy index");
+ return -EINVAL;
+ }
+ } else {
+ /* If we need a PHY but no phy index is specified, fallback
+ * to dev->phydev
+ */
+ phydev = dev->phydev;
+ }
+ }
+
+ req_info->phydev = phydev;
req_info->dev = dev;
req_info->flags = flags;
return 0;
@@ -250,6 +250,7 @@ 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
+ * @phydev: phy_device connected to @dev this request is for (may be null)
*
* This is a common base for request specific structures holding data from
* parsed userspace request. These always embed struct ethnl_req_info at
@@ -259,6 +260,7 @@ struct ethnl_req_info {
struct net_device *dev;
netdevice_tracker dev_tracker;
u32 flags;
+ struct phy_device *phydev;
};
static inline void ethnl_parse_header_dev_put(struct ethnl_req_info *req_info)
@@ -395,9 +397,10 @@ 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[ETHTOOL_A_HEADER_PHY_INDEX + 1];
+extern const struct nla_policy ethnl_header_policy_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];
@@ -441,6 +444,7 @@ extern const struct nla_policy ethnl_plca_set_cfg_policy[ETHTOOL_A_PLCA_MAX + 1]
extern const struct nla_policy ethnl_plca_get_status_policy[ETHTOOL_A_PLCA_HEADER + 1];
extern const struct nla_policy ethnl_mm_get_policy[ETHTOOL_A_MM_HEADER + 1];
extern const struct nla_policy ethnl_mm_set_policy[ETHTOOL_A_MM_MAX + 1];
+extern const struct nla_policy ethnl_phy_get_policy[ETHTOOL_A_PHY_HEADER + 1];
int ethnl_set_features(struct sk_buff *skb, struct genl_info *info);
int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info);