[RFC PATCH v3 5/5] Convert Intel e1000e NIC driver to use ndo_hwtstamp_get/set callbacks

[RFC PATCH v3 5/5] Convert Intel e1000e NIC driver to use ndo_hwtstamp_get/set callbacks

[Maxim Georgiev]

This patch converts Intel·e1000e·NIC·driver·to·use
the newly introduced ndo_hwtstamp_get/set functions to handle
HW timestamp set and query requests instead of implementing
SIOCGHWTSTAMP/SIOCSHWTSTAMP IOCTLs handling logic.

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Maxim Georgiev <glipus@gmail.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 29 +++++++++++-----------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 6f5c16aebcbf..207e439b949c 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6161,7 +6161,8 @@ static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr,
 /**
  * e1000e_hwtstamp_set - control hardware time stamping
  * @netdev: network interface device structure
- * @ifr: interface request
+ * @kernel_config: kernel version of config parameter structure
+ * @extack: netlink request parameters
  *
  * Outgoing time stamping can be enabled and disabled. Play nice and
  * disable it when requested, although it shouldn't cause any overhead
@@ -6174,15 +6175,15 @@ static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr,
  * specified. Matching the kind of event packet is not supported, with the
  * exception of "all V2 events regardless of level 2 or 4".
  **/
-static int e1000e_hwtstamp_set(struct net_device *netdev, struct ifreq *ifr)
+static int e1000e_hwtstamp_set(struct net_device *netdev,
+			       struct kernel_hwtstamp_config *kernel_config,
+			       struct netlink_ext_ack *extack)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct hwtstamp_config config;
 	int ret_val;
 
-	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
-		return -EFAULT;
-
+	hwtstamp_kernel_to_config(&config, kernel_config);
 	ret_val = e1000e_config_hwtstamp(adapter, &config);
 	if (ret_val)
 		return ret_val;
@@ -6205,16 +6206,18 @@ static int e1000e_hwtstamp_set(struct net_device *netdev, struct ifreq *ifr)
 		break;
 	}
 
-	return copy_to_user(ifr->ifr_data, &config,
-			    sizeof(config)) ? -EFAULT : 0;
+	hwtstamp_config_to_kernel(kernel_config, &config);
+	return 0;
 }
 
-static int e1000e_hwtstamp_get(struct net_device *netdev, struct ifreq *ifr)
+static int e1000e_hwtstamp_get(struct net_device *netdev,
+			       struct kernel_hwtstamp_config *kernel_config,
+			       struct netlink_ext_ack *extack)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 
-	return copy_to_user(ifr->ifr_data, &adapter->hwtstamp_config,
-			    sizeof(adapter->hwtstamp_config)) ? -EFAULT : 0;
+	hwtstamp_config_to_kernel(kernel_config, &adapter->hwtstamp_config);
+	return 0;
 }
 
 static int e1000_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
@@ -6224,10 +6227,6 @@ static int e1000_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 	case SIOCGMIIREG:
 	case SIOCSMIIREG:
 		return e1000_mii_ioctl(netdev, ifr, cmd);
-	case SIOCSHWTSTAMP:
-		return e1000e_hwtstamp_set(netdev, ifr);
-	case SIOCGHWTSTAMP:
-		return e1000e_hwtstamp_get(netdev, ifr);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -7365,6 +7364,8 @@ static const struct net_device_ops e1000e_netdev_ops = {
 	.ndo_set_features = e1000_set_features,
 	.ndo_fix_features = e1000_fix_features,
 	.ndo_features_check	= passthru_features_check,
+	.ndo_hwtstamp_get	= e1000e_hwtstamp_get,
+	.ndo_hwtstamp_set	= e1000e_hwtstamp_set,
 };
 
 /**
-- 
2.39.2

Re: [RFC PATCH v3 5/5] Convert Intel e1000e NIC driver to use ndo_hwtstamp_get/set callbacks

[Vladimir Oltean]

See "git log drivers/net/ethernet/intel/e1000e/netdev.c" for an example
of commit title formatting for this driver.

On Wed, Apr 05, 2023 at 12:33:38AM -0600, Maxim Georgiev wrote:
> This patch converts Intel·e1000e·NIC·driver·to·use

Strange symbols (·) instead of plain spaces here.

> the newly introduced ndo_hwtstamp_get/set functions to handle
> HW timestamp set and query requests instead of implementing
> SIOCGHWTSTAMP/SIOCSHWTSTAMP IOCTLs handling logic.
> 
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Maxim Georgiev <glipus@gmail.com>
> ---
> -static int e1000e_hwtstamp_get(struct net_device *netdev, struct ifreq *ifr)
> +static int e1000e_hwtstamp_get(struct net_device *netdev,
> +			       struct kernel_hwtstamp_config *kernel_config,
> +			       struct netlink_ext_ack *extack)
>  {
>  	struct e1000_adapter *adapter = netdev_priv(netdev);
>  
> -	return copy_to_user(ifr->ifr_data, &adapter->hwtstamp_config,
> -			    sizeof(adapter->hwtstamp_config)) ? -EFAULT : 0;
> +	hwtstamp_config_to_kernel(kernel_config, &adapter->hwtstamp_config);

Why don't you change the type of adapter->hwtstamp_config to struct
kernel_hwtstamp_config and work just with that?

> +	return 0;
>  }

Since AFAIU, none of the CCed people offered to test your patches on
e1000, I guess the options to make progress are either to CC some Intel
people, or to convert some other driver where a volunteer does exist.