Re: [PATCH net-next v18 08/10] netlink: specs: Add ethernet port listing with ethtool

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] ETHTOOL_A_PORT_SUPPORTED_INTERFACES is added to the spec
  (Documentation/netlink/specs/ethtool.yaml, attribute-set 'port',…
- [Low] Baseline rejected this as a false positive.

--

netlink: specs: Add ethernet port listing with ethtool

This commit adds the YAML spec, the generated uAPI header entries and the
documentation for a new ETHTOOL_MSG_PORT_GET netlink command. The command
reports the front-facing ports (phy_port) of a network interface, including
their type, supported link modes and supported MII interfaces.

> diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
> index 5dd4d1b5d94bb..35e028c0be296 100644
> --- a/Documentation/netlink/specs/ethtool.yaml
> +++ b/Documentation/netlink/specs/ethtool.yaml

[ ... ]

> @@ -1905,6 +1910,32 @@ attribute-sets:
>          name: link
>          type: nest
>          nested-attributes: mse-snapshot
> +  -
> +    name: port
> +    attr-cnt-name: --ethtool-a-port-cnt
> +    attributes:

[ ... ]

> +      -
> +        name: supported-modes
> +        type: nest
> +        nested-attributes: bitset
> +      -
> +        name: supported-interfaces
> +        type: nest
> +        nested-attributes: bitset

[Severity: Medium]
How is userspace expected to decode the bits in this bitset?

Nothing in the uAPI says which bit index maps to which interface. The
implementation added later in the series, port_fill_reply() in
net/ethtool/port.c, emits it as:

	ret = ethnl_put_bitset(skb, ETHTOOL_A_PORT_SUPPORTED_INTERFACES,
			       reply->interfaces, NULL,
			       PHY_INTERFACE_MODE_MAX,
			       phy_interface_names, compact);

That means the bit positions are the kernel-internal phy_interface_t values
from include/linux/phy.h. The names come from phy_interface_names[] in
net/ethtool/netlink.c, which is not exposed as an ETH_SS_* string set.
enum ethtool_stringset has ETH_SS_LINK_MODES for the sibling
supported-modes bitset, but it has no entry for PHY interface modes.

If the request header sets ETHTOOL_FLAG_COMPACT_BITSETS, ethnl_put_bitset()
sends only the size and value words, without the per-bit names. Would the
client then get raw phy_interface_t indices that it has no uAPI way to map
back to interface names?

It would also make the ordering of phy_interface_t de facto ABI. Adding or
reordering a mode would then change what compact-bitset users see.

Should this bitset get a matching string set? If not, should the spec and
ethtool-netlink.rst say that only the verbose form can be decoded? Right
now the documentation only says "bitset of supported MII interfaces".

[ ... ]

> @@ -2859,6 +2890,30 @@ operations:
>              - worst-channel
>              - link
>        dump: *mse-get-op
> +    -
> +      name: port-get
> +      doc: Get ports attached to an interface
> +
> +      attribute-set: port

[Severity: Low]
This commit adds the port-get operation, the ETHTOOL_MSG_PORT_GET and
ETHTOOL_MSG_PORT_GET_REPLY IDs and the ETHTOOL_A_PORT_* attributes to the
uAPI. There is no kernel handler for them yet at this point in the series.
Would a kernel built at this commit return -EOPNOTSUPP for PORT_GET?

The next patch in the series, "net: ethtool: Introduce ethtool command to
list ports", adds net/ethtool/port.c. It also adds the
ethnl_default_requests[] entry and the ethtool_genl_ops entry for
ETHTOOL_MSG_PORT_GET. By the end of the series the command works, so this
only matters when bisecting to this intermediate commit.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927133619.955236-1-maxime.chevallier%40bootlin.com