Re: [net-next,v6,10/10] net: ethtool: Introduce ethtool command to list ports
From: Simon Horman <horms@kernel.org>
Date: 2026-03-06 13:59:24
Also in:
lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- net: ethtool: Introduce ethtool command to list ports This commit introduces a new ethtool netlink command to expose phy_port information to userspace, allowing enumeration of available ports and their capabilities on a given interface.
quoted hunk ↗ jump to hunk
diff --git a/MAINTAINERS b/MAINTAINERS index 2265e2c9bfbe..668af8739c21 100644 --- a/MAINTAINERS +++ b/MAINTAINERS@@ -18421,6 +18421,7 @@ F: Documentation/devicetree/bindings/net/ethernet-connector.yaml F: Documentation/networking/phy-port.rst F: drivers/net/phy/phy_port.c F: include/linux/phy_port.h +F: net/ethtool/port.c K: struct\s+phy_port|phy_port_ NETWORKING [GENERAL]
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/net/ethtool/port.c b/net/ethtool/port.c new file mode 100644 index 000000000000..d870ed70769e --- /dev/null +++ b/net/ethtool/port.c@@ -0,0 +1,376 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright 2026 Bootlin + * + */ +#include "common.h" +#include "bitset.h" +#include "netlink.h" + +#include <linux/phy.h> +#include <linux/phy_link_topology.h> +#include <linux/phy_port.h> +#include <net/netdev_lock.h>
[ ... ]
+static int port_dump_one(struct sk_buff *skb, struct net_device *dev,
+ struct netlink_callback *cb)
+{
+ struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
+ void *ehdr;
+ int ret;
+
+ ehdr = ethnl_dump_put(skb, cb, ETHTOOL_A_PORT_HEADER);
+ if (!ehdr)
+ return -EMSGSIZE;
Is ETHTOOL_A_PORT_HEADER the correct constant here?
The third parameter to ethnl_dump_put() should be the genetlink command
ID that gets stored in the message header's cmd field. Looking at other
dump implementations in net/ethtool/:
net/ethtool/tunnels.c:
ehdr = ethnl_dump_put(skb, cb, ETHTOOL_MSG_TUNNEL_INFO_GET_REPLY);
net/ethtool/tsinfo.c:
ehdr = ethnl_dump_put(skb, cb, ETHTOOL_MSG_TSINFO_GET_REPLY);
net/ethtool/rss.c:
ehdr = ethnl_dump_put(skb, cb, ETHTOOL_MSG_RSS_GET_REPLY);
These all pass ETHTOOL_MSG_*_GET_REPLY command constants, not
ETHTOOL_A_* attribute constants.
ETHTOOL_A_PORT_HEADER is a netlink attribute with value 1 (from
include/uapi/linux/ethtool_netlink.h). Meanwhile, this file's
ethnl_port_request_ops structure correctly uses
ETHTOOL_MSG_PORT_GET_REPLY for .reply_cmd, suggesting the same
constant should be used here. Can this cause the wrong command ID
to be sent in every dump response message?
+ + memset(ctx->reply_data, 0, sizeof(struct port_reply_data)); + ctx->reply_data->base.dev = dev; + + rtnl_lock(); + netdev_lock_ops(dev); + + ret = port_prepare_data(&ctx->req_info->base, &ctx->reply_data->base, + genl_info_dump(cb)); + + netdev_unlock_ops(dev); + rtnl_unlock(); + + if (ret < 0) + goto out; + + ret = ethnl_fill_reply_header(skb, dev, ETHTOOL_A_PORT_HEADER); + if (ret < 0) + goto out; + + ret = port_fill_reply(skb, &ctx->req_info->base, &ctx->reply_data->base); + +out: + ctx->reply_data->base.dev = NULL; + if (ret < 0) + genlmsg_cancel(skb, ehdr); + else + genlmsg_end(skb, ehdr); + + return ret; +}
[ ... ]
+const struct ethnl_request_ops ethnl_port_request_ops = {
+ .request_cmd = ETHTOOL_MSG_PORT_GET,
+ .reply_cmd = ETHTOOL_MSG_PORT_GET_REPLY,
+ .hdr_attr = ETHTOOL_A_PORT_HEADER,
+ .req_info_size = sizeof(struct port_req_info),
+ .reply_data_size = sizeof(struct port_reply_data),
+
+ .parse_request = port_parse_request,
+ .prepare_data = port_prepare_data,
+ .reply_size = port_reply_size,
+ .fill_reply = port_fill_reply,
+};