Re: [PATCH net-next v5 03/16] net: ethtool: Refactor identical get_ts_info implementations.

[Jay Vosburgh <jay.vosburgh@canonical.com>]

Köry Maincent <kory.maincent@bootlin.com> wrote:

>From: Richard Cochran <richardcochran@gmail.com>
>
>The vlan, macvlan and the bonding drivers call their "real" device driver
>in order to report the time stamping capabilities.  Provide a core
>ethtool helper function to avoid copy/paste in the stack.
>
>Signed-off-by: Richard Cochran <richardcochran@gmail.com>
>Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

	For the bonding portion:

Reviewed-by: Jay Vosburgh <jay.vosburgh@canonical.com>


>---
>
>Change in v5:
>- Fixe typo
>---
> drivers/net/bonding/bond_main.c | 27 ++-------------------------
> drivers/net/macvlan.c           | 14 +-------------
> include/linux/ethtool.h         |  8 ++++++++
> net/8021q/vlan_dev.c            | 15 +--------------
> net/ethtool/common.c            |  6 ++++++
> 5 files changed, 18 insertions(+), 52 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index ed7212e61c54..18af563d20b2 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -5763,29 +5763,12 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
> 	rcu_read_unlock();
> 
> 	if (real_dev) {
>-		ops = real_dev->ethtool_ops;
>-		phydev = real_dev->phydev;
>-
>-		if (phy_has_tsinfo(phydev)) {
>-			ret = phy_ts_info(phydev, info);
>-			goto out;
>-		} else if (ops->get_ts_info) {
>-			ret = ops->get_ts_info(real_dev, info);
>-			goto out;
>-		}
>+		ret = ethtool_get_ts_info_by_layer(real_dev, info);
> 	} else {
> 		/* Check if all slaves support software tx timestamping */
> 		rcu_read_lock();
> 		bond_for_each_slave_rcu(bond, slave, iter) {
>-			ret = -1;
>-			ops = slave->dev->ethtool_ops;
>-			phydev = slave->dev->phydev;
>-
>-			if (phy_has_tsinfo(phydev))
>-				ret = phy_ts_info(phydev, &ts_info);
>-			else if (ops->get_ts_info)
>-				ret = ops->get_ts_info(slave->dev, &ts_info);
>-
>+			ret = ethtool_get_ts_info_by_layer(slave->dev, &ts_info);
> 			if (!ret && (ts_info.so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE)) {
> 				sw_tx_support = true;
> 				continue;
>@@ -5797,15 +5780,9 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
> 		rcu_read_unlock();
> 	}
> 
>-	ret = 0;
>-	info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
>-				SOF_TIMESTAMPING_SOFTWARE;
> 	if (sw_tx_support)
> 		info->so_timestamping |= SOF_TIMESTAMPING_TX_SOFTWARE;
> 
>-	info->phc_index = -1;
>-
>-out:
> 	dev_put(real_dev);
> 	return ret;
> }
>diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>index 02bd201bc7e5..759406fbaea8 100644
>--- a/drivers/net/macvlan.c
>+++ b/drivers/net/macvlan.c
>@@ -1086,20 +1086,8 @@ static int macvlan_ethtool_get_ts_info(struct net_device *dev,
> 				       struct ethtool_ts_info *info)
> {
> 	struct net_device *real_dev = macvlan_dev_real_dev(dev);
>-	const struct ethtool_ops *ops = real_dev->ethtool_ops;
>-	struct phy_device *phydev = real_dev->phydev;
> 
>-	if (phy_has_tsinfo(phydev)) {
>-		return phy_ts_info(phydev, info);
>-	} else if (ops->get_ts_info) {
>-		return ops->get_ts_info(real_dev, info);
>-	} else {
>-		info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
>-			SOF_TIMESTAMPING_SOFTWARE;
>-		info->phc_index = -1;
>-	}
>-
>-	return 0;
>+	return ethtool_get_ts_info_by_layer(real_dev, info);
> }
> 
> static netdev_features_t macvlan_fix_features(struct net_device *dev,
>diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>index 62b61527bcc4..1159daac776e 100644
>--- a/include/linux/ethtool.h
>+++ b/include/linux/ethtool.h
>@@ -1043,6 +1043,14 @@ static inline int ethtool_mm_frag_size_min_to_add(u32 val_min, u32 *val_add,
> 	return -EINVAL;
> }
> 
>+/**
>+ * ethtool_get_ts_info_by_layer - Obtains time stamping capabilities from the MAC or PHY layer.
>+ * @dev: pointer to net_device structure
>+ * @info: buffer to hold the result
>+ * Returns zero on success, non-zero otherwise.
>+ */
>+int ethtool_get_ts_info_by_layer(struct net_device *dev, struct ethtool_ts_info *info);
>+
> /**
>  * ethtool_sprintf - Write formatted string to ethtool string data
>  * @data: Pointer to start of string to update
>diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
>index 2a7f1b15714a..407b2335f091 100644
>--- a/net/8021q/vlan_dev.c
>+++ b/net/8021q/vlan_dev.c
>@@ -702,20 +702,7 @@ static int vlan_ethtool_get_ts_info(struct net_device *dev,
> 				    struct ethtool_ts_info *info)
> {
> 	const struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
>-	const struct ethtool_ops *ops = vlan->real_dev->ethtool_ops;
>-	struct phy_device *phydev = vlan->real_dev->phydev;
>-
>-	if (phy_has_tsinfo(phydev)) {
>-		return phy_ts_info(phydev, info);
>-	} else if (ops->get_ts_info) {
>-		return ops->get_ts_info(vlan->real_dev, info);
>-	} else {
>-		info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
>-			SOF_TIMESTAMPING_SOFTWARE;
>-		info->phc_index = -1;
>-	}
>-
>-	return 0;
>+	return ethtool_get_ts_info_by_layer(vlan->real_dev, info);
> }
> 
> static void vlan_dev_get_stats64(struct net_device *dev,
>diff --git a/net/ethtool/common.c b/net/ethtool/common.c
>index f5598c5f50de..e2315e24d695 100644
>--- a/net/ethtool/common.c
>+++ b/net/ethtool/common.c
>@@ -661,6 +661,12 @@ int ethtool_get_phc_vclocks(struct net_device *dev, int **vclock_index)
> }
> EXPORT_SYMBOL(ethtool_get_phc_vclocks);
> 
>+int ethtool_get_ts_info_by_layer(struct net_device *dev, struct ethtool_ts_info *info)
>+{
>+	return __ethtool_get_ts_info(dev, info);
>+}
>+EXPORT_SYMBOL(ethtool_get_ts_info_by_layer);
>+
> const struct ethtool_phy_ops *ethtool_phy_ops;
> 
> void ethtool_set_ethtool_phy_ops(const struct ethtool_phy_ops *ops)
>-- 
>2.25.1
>
>