Re: [PATCH net-next v1 1/7] net: ethtool: plumb PHY stats to PHY drivers
flat view
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
Date: 2024-12-03 08:30:32
Also in:
linux-doc, lkml
Hi Oleksij, On Tue, 3 Dec 2024 08:56:15 +0100 Oleksij Rempel [off-list ref] wrote:
From: Jakub Kicinski <kuba@kernel.org> Feed the existing IEEE PHY counter struct (which currently only has one entry) and link stats into the PHY driver. The MAC driver can override the value if it somehow has a better idea of PHY stats. Since the stats are "undefined" at input the drivers can't += the values, so we should be safe from double-counting. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
[...]
quoted hunk ↗ jump to hunk
+static void +ethtool_get_phydev_stats(struct net_device *dev, + struct linkstate_reply_data *data) +{ + struct phy_device *phydev = dev->phydev; + + if (!phydev) + return; + + if (dev->phydev) + data->link_stats.link_down_events = + READ_ONCE(dev->phydev->link_down_events); + + if (!phydev->drv || !phydev->drv->get_link_stats) + return; + + mutex_lock(&phydev->lock); + phydev->drv->get_link_stats(phydev, &data->link_stats); + mutex_unlock(&phydev->lock); +} + static int linkstate_prepare_data(const struct ethnl_req_info *req_base, struct ethnl_reply_data *reply_base, const struct genl_info *info)@@ -127,9 +148,7 @@ static int linkstate_prepare_data(const struct ethnl_req_info *req_base, sizeof(data->link_stats) / 8); if (req_base->flags & ETHTOOL_FLAG_STATS) { - if (dev->phydev) - data->link_stats.link_down_events = - READ_ONCE(dev->phydev->link_down_events); + ethtool_get_phydev_stats(dev, data);
I'm sorry to bother you with my multi-phy stuff, but I'd like to avoid directly accessing netdev->phydev at least in the netlink code. Could it be possible for you to pass a phydev directly to the ethtool_get_phydev_stats() helper you're creating ? That way, you could get the stats from other phydevs on the link if userspace passed a phy index in the netlink header. You'd get the phydev that way : phydev = ethnl_req_get_phydev(req_base, tb[ETHTOOL_A_LINKSTATE_HEADER,], info->extack); This is what's done in the pse-pd, plca and cabletest netlink code that deals with phydevs. Note that this helper will fallback to netdev->phydev if user didn't pass any PHY index, which I expect to be what people do most of the time. However should the netdev have more than 1 PHY, we would be able to get the far-away PHY's stats :)
quoted hunk ↗ jump to hunk
if (dev->ethtool_ops->get_link_ext_stats) dev->ethtool_ops->get_link_ext_stats(dev,diff --git a/net/ethtool/stats.c b/net/ethtool/stats.c index 912f0c4fff2f..cf802b1cda6f 100644 --- a/net/ethtool/stats.c +++ b/net/ethtool/stats.c@@ -1,5 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only +#include <linux/phy.h> + #include "netlink.h" #include "common.h" #include "bitset.h"@@ -112,6 +114,19 @@ static int stats_parse_request(struct ethnl_req_info *req_base, return 0; } +static void +ethtool_get_phydev_stats(struct net_device *dev, struct stats_reply_data *data) +{ + struct phy_device *phydev = dev->phydev; + + if (!phydev || !phydev->drv || !phydev->drv->get_phy_stats) + return; + + mutex_lock(&phydev->lock); + phydev->drv->get_phy_stats(phydev, &data->phy_stats); + mutex_unlock(&phydev->lock); +} + static int stats_prepare_data(const struct ethnl_req_info *req_base, struct ethnl_reply_data *reply_base, const struct genl_info *info)@@ -145,6 +160,10 @@ static int stats_prepare_data(const struct ethnl_req_info *req_base, data->ctrl_stats.src = src; data->rmon_stats.src = src; + if (test_bit(ETHTOOL_STATS_ETH_PHY, req_info->stat_mask) && + src == ETHTOOL_MAC_STATS_SRC_AGGREGATE) + ethtool_get_phydev_stats(dev, data);
Same here :) Thanks, Maxime