Re: [PATCH net-next v10 05/13] net: ethtool: Allow passing a phy index for some commands
From: Jakub Kicinski <kuba@kernel.org>
Date: 2024-03-09 05:01:23
Also in:
linux-arm-kernel, lkml
On Mon, 4 Mar 2024 16:10:01 +0100 Maxime Chevallier wrote:
+``ETHTOOL_A_HEADER_PHY_INDEX`` identify the ethernet PHY the message relates to.
identifies Ethernet
+As there are numerous commands that are related to PHY configuration, and because +we can have more than one PHY on the link, the PHY index can be passed in the
we can have -> there may be
+request for the commands that needs it. It is however not mandatory, and if it
commas around however
+is not passed for commands that target a PHY, the net_device.phydev pointer +is used, as a fallback that keeps the legacy behaviour.
s/ that keeps the legacy behaviour// feels more like a default than legacy TBH
quoted hunk ↗ jump to hunk
@@ -104,7 +124,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;@@ -145,6 +165,26 @@ 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 = phy_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;
You can drop the msg, and use ENODEV? Also point at the index, not header: struct nl_attr *phyid; phyid = tb[ETHTOOL_A_HEADER_PHY_INDEX]; phydev = phy_link_topo_get_phy(dev->link_topo, nla_get_u32(phyid)); if (!... NL_SET_ERR_ATTR(extack, phyid); return -ENODEV;
+ }
+ } else {
+ /* If we need a PHY but no phy index is specified, fallback
+ * to dev->phydev
+ */
+ phydev = dev->phydev;
+ }
+ }
else if (tb[ETHTOOL_A_HEADER_PHY_INDEX]) {
NL_SET_ERR_MSG_ATTR("can't target a PHY without a netdev")
...
? just in case someone calls this with _phy policy and no dev required?