Re: [net-next,v6,10/10] net: ethtool: Introduce ethtool command to list ports

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

Hi

On 06/03/2026 14:58, Simon Horman wrote:
> 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.
> 

> 
>> +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?

hmmm true indeed, however ethtool did show me the replies. I'll fix that
though.

Thanks !

Maxime