[PATCH iwl-next 0/5] Intel driver conversion to new hardware timestamping API

[PATCH iwl-next 0/5] Intel driver conversion to new hardware timestamping API

[Vladimir Oltean]

Since the introduction of the new ndo_hwtstamp_get() and
ndo_hwtstamp_set() operations in v6.6, only e1000e and iavf have
been converted.

There is a push to get rid of the old code path for configuring hardware
timestamping, so the reset of the drivers are converted in this patch
set.

Vladimir Oltean (5):
  ice: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
  igc: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
  igb: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
  ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
  i40e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

 drivers/net/ethernet/intel/i40e/i40e.h        |  9 ++--
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 24 +---------
 drivers/net/ethernet/intel/i40e/i40e_ptp.c    | 43 +++++++++---------
 drivers/net/ethernet/intel/ice/ice_main.c     | 24 +---------
 drivers/net/ethernet/intel/ice/ice_ptp.c      | 45 ++++++++++---------
 drivers/net/ethernet/intel/ice/ice_ptp.h      | 17 ++++---
 drivers/net/ethernet/intel/igb/igb.h          |  9 ++--
 drivers/net/ethernet/intel/igb/igb_main.c     |  6 +--
 drivers/net/ethernet/intel/igb/igb_ptp.c      | 37 +++++++--------
 drivers/net/ethernet/intel/igc/igc.h          |  9 ++--
 drivers/net/ethernet/intel/igc/igc_main.c     | 21 +--------
 drivers/net/ethernet/intel/igc/igc_ptp.c      | 36 +++++++--------
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  9 ++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +--
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c  | 42 ++++++++---------
 15 files changed, 147 insertions(+), 190 deletions(-)

-- 
2.43.0

[PATCH iwl-next 1/5] ice: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Vladimir Oltean]

New timestamping API was introduced in commit 66f7223039c0 ("net: add
NDOs for configuring hardware timestamping") from kernel v6.6.

It is time to convert the Intel ice driver to the new API, so that
timestamping configuration can be removed from the ndo_eth_ioctl() path
completely.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
---
Previously submitted as
https://patchwork.kernel.org/project/netdevbpf/patch/20250512160036.909434-1-vladimir.oltean@nxp.com/

 drivers/net/ethernet/intel/ice/ice_main.c | 24 +-----------
 drivers/net/ethernet/intel/ice/ice_ptp.c  | 45 ++++++++++++-----------
 drivers/net/ethernet/intel/ice/ice_ptp.h  | 17 ++++++---
 3 files changed, 37 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 1fbe13ee93a8..674d90d6a081 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -7880,27 +7880,6 @@ int ice_change_mtu(struct net_device *netdev, int new_mtu)
 	return err;
 }
 
-/**
- * ice_eth_ioctl - Access the hwtstamp interface
- * @netdev: network interface device structure
- * @ifr: interface request data
- * @cmd: ioctl command
- */
-static int ice_eth_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
-{
-	struct ice_netdev_priv *np = netdev_priv(netdev);
-	struct ice_pf *pf = np->vsi->back;
-
-	switch (cmd) {
-	case SIOCGHWTSTAMP:
-		return ice_ptp_get_ts_config(pf, ifr);
-	case SIOCSHWTSTAMP:
-		return ice_ptp_set_ts_config(pf, ifr);
-	default:
-		return -EOPNOTSUPP;
-	}
-}
-
 /**
  * ice_aq_str - convert AQ err code to a string
  * @aq_err: the AQ error code to convert
@@ -9734,7 +9713,6 @@ static const struct net_device_ops ice_netdev_ops = {
 	.ndo_change_mtu = ice_change_mtu,
 	.ndo_get_stats64 = ice_get_stats64,
 	.ndo_set_tx_maxrate = ice_set_tx_maxrate,
-	.ndo_eth_ioctl = ice_eth_ioctl,
 	.ndo_set_vf_spoofchk = ice_set_vf_spoofchk,
 	.ndo_set_vf_mac = ice_set_vf_mac,
 	.ndo_get_vf_config = ice_get_vf_cfg,
@@ -9758,4 +9736,6 @@ static const struct net_device_ops ice_netdev_ops = {
 	.ndo_bpf = ice_xdp,
 	.ndo_xdp_xmit = ice_xdp_xmit,
 	.ndo_xsk_wakeup = ice_xsk_wakeup,
+	.ndo_hwtstamp_get = ice_ptp_hwtstamp_get,
+	.ndo_hwtstamp_set = ice_ptp_hwtstamp_set,
 };
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index b79a148ed0f2..45aa65c190a7 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -2359,23 +2359,24 @@ static int ice_ptp_getcrosststamp(struct ptp_clock_info *info,
 }
 
 /**
- * ice_ptp_get_ts_config - ioctl interface to read the timestamping config
- * @pf: Board private structure
- * @ifr: ioctl data
+ * ice_ptp_hwtstamp_get - interface to read the timestamping config
+ * @netdev: Pointer to network interface device structure
+ * @config: Timestamping configuration structure
  *
  * Copy the timestamping config to user buffer
  */
-int ice_ptp_get_ts_config(struct ice_pf *pf, struct ifreq *ifr)
+int ice_ptp_hwtstamp_get(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config)
 {
-	struct hwtstamp_config *config;
+	struct ice_netdev_priv *np = netdev_priv(netdev);
+	struct ice_pf *pf = np->vsi->back;
 
 	if (pf->ptp.state != ICE_PTP_READY)
 		return -EIO;
 
-	config = &pf->ptp.tstamp_config;
+	*config = pf->ptp.tstamp_config;
 
-	return copy_to_user(ifr->ifr_data, config, sizeof(*config)) ?
-		-EFAULT : 0;
+	return 0;
 }
 
 /**
@@ -2383,8 +2384,8 @@ int ice_ptp_get_ts_config(struct ice_pf *pf, struct ifreq *ifr)
  * @pf: Board private structure
  * @config: hwtstamp settings requested or saved
  */
-static int
-ice_ptp_set_timestamp_mode(struct ice_pf *pf, struct hwtstamp_config *config)
+static int ice_ptp_set_timestamp_mode(struct ice_pf *pf,
+				      struct kernel_hwtstamp_config *config)
 {
 	switch (config->tx_type) {
 	case HWTSTAMP_TX_OFF:
@@ -2428,32 +2429,32 @@ ice_ptp_set_timestamp_mode(struct ice_pf *pf, struct hwtstamp_config *config)
 }
 
 /**
- * ice_ptp_set_ts_config - ioctl interface to control the timestamping
- * @pf: Board private structure
- * @ifr: ioctl data
+ * ice_ptp_hwtstamp_set - interface to control the timestamping
+ * @netdev: Pointer to network interface device structure
+ * @config: Timestamping configuration structure
+ * @extack: Netlink extended ack structure for error reporting
  *
  * Get the user config and store it
  */
-int ice_ptp_set_ts_config(struct ice_pf *pf, struct ifreq *ifr)
+int ice_ptp_hwtstamp_set(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config,
+			 struct netlink_ext_ack *extack)
 {
-	struct hwtstamp_config config;
+	struct ice_netdev_priv *np = netdev_priv(netdev);
+	struct ice_pf *pf = np->vsi->back;
 	int err;
 
 	if (pf->ptp.state != ICE_PTP_READY)
 		return -EAGAIN;
 
-	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
-		return -EFAULT;
-
-	err = ice_ptp_set_timestamp_mode(pf, &config);
+	err = ice_ptp_set_timestamp_mode(pf, config);
 	if (err)
 		return err;
 
 	/* Return the actual configuration set */
-	config = pf->ptp.tstamp_config;
+	*config = pf->ptp.tstamp_config;
 
-	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
-		-EFAULT : 0;
+	return 0;
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index 3b769a0cad00..da9bfd46d750 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -262,7 +262,7 @@ struct ice_ptp {
 	struct ptp_extts_request extts_rqs[GLTSYN_EVNT_H_IDX_MAX];
 	struct ptp_clock_info info;
 	struct ptp_clock *clock;
-	struct hwtstamp_config tstamp_config;
+	struct kernel_hwtstamp_config tstamp_config;
 	u64 reset_time;
 	u32 tx_hwtstamp_skipped;
 	u32 tx_hwtstamp_timeouts;
@@ -294,8 +294,11 @@ struct ice_ptp {
 #if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
 int ice_ptp_clock_index(struct ice_pf *pf);
 struct ice_pf;
-int ice_ptp_set_ts_config(struct ice_pf *pf, struct ifreq *ifr);
-int ice_ptp_get_ts_config(struct ice_pf *pf, struct ifreq *ifr);
+int ice_ptp_hwtstamp_get(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config);
+int ice_ptp_hwtstamp_set(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config,
+			 struct netlink_ext_ack *extack);
 void ice_ptp_restore_timestamp_mode(struct ice_pf *pf);
 
 void ice_ptp_extts_event(struct ice_pf *pf);
@@ -316,12 +319,16 @@ void ice_ptp_init(struct ice_pf *pf);
 void ice_ptp_release(struct ice_pf *pf);
 void ice_ptp_link_change(struct ice_pf *pf, bool linkup);
 #else /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */
-static inline int ice_ptp_set_ts_config(struct ice_pf *pf, struct ifreq *ifr)
+
+static inline int ice_ptp_hwtstamp_get(struct net_device *netdev,
+				       struct kernel_hwtstamp_config *config)
 {
 	return -EOPNOTSUPP;
 }
 
-static inline int ice_ptp_get_ts_config(struct ice_pf *pf, struct ifreq *ifr)
+static inline int ice_ptp_hwtstamp_set(struct net_device *netdev,
+				       struct kernel_hwtstamp_config *config,
+				       struct netlink_ext_ack *extack)
 {
 	return -EOPNOTSUPP;
 }
-- 
2.43.0

[PATCH iwl-next 2/5] igc: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Vladimir Oltean]

New timestamping API was introduced in commit 66f7223039c0 ("net: add
NDOs for configuring hardware timestamping") from kernel v6.6.

It is time to convert the Intel igc driver to the new API, so that
timestamping configuration can be removed from the ndo_eth_ioctl() path
completely.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/intel/igc/igc.h      |  9 ++++--
 drivers/net/ethernet/intel/igc/igc_main.c | 21 ++-----------
 drivers/net/ethernet/intel/igc/igc_ptp.c  | 36 +++++++++++------------
 3 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 859a15e4ccba..3b61b3447c52 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -313,7 +313,7 @@ struct igc_adapter {
 	 */
 	spinlock_t ptp_tx_lock;
 	struct igc_tx_timestamp_request tx_tstamp[IGC_MAX_TX_TSTAMP_REGS];
-	struct hwtstamp_config tstamp_config;
+	struct kernel_hwtstamp_config tstamp_config;
 	unsigned int ptp_flags;
 	/* System time value lock */
 	spinlock_t tmreg_lock;
@@ -752,8 +752,11 @@ void igc_ptp_reset(struct igc_adapter *adapter);
 void igc_ptp_suspend(struct igc_adapter *adapter);
 void igc_ptp_stop(struct igc_adapter *adapter);
 ktime_t igc_ptp_rx_pktstamp(struct igc_adapter *adapter, __le32 *buf);
-int igc_ptp_set_ts_config(struct net_device *netdev, struct ifreq *ifr);
-int igc_ptp_get_ts_config(struct net_device *netdev, struct ifreq *ifr);
+int igc_ptp_hwtstamp_get(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config);
+int igc_ptp_hwtstamp_set(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config,
+			 struct netlink_ext_ack *extack);
 void igc_ptp_tx_hang(struct igc_adapter *adapter);
 void igc_ptp_read(struct igc_adapter *adapter, struct timespec64 *ts);
 void igc_ptp_tx_tstamp_event(struct igc_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 27575a1e1777..698d5ca64a33 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6294,24 +6294,6 @@ int igc_close(struct net_device *netdev)
 	return 0;
 }
 
-/**
- * igc_ioctl - Access the hwtstamp interface
- * @netdev: network interface device structure
- * @ifr: interface request data
- * @cmd: ioctl command
- **/
-static int igc_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
-{
-	switch (cmd) {
-	case SIOCGHWTSTAMP:
-		return igc_ptp_get_ts_config(netdev, ifr);
-	case SIOCSHWTSTAMP:
-		return igc_ptp_set_ts_config(netdev, ifr);
-	default:
-		return -EOPNOTSUPP;
-	}
-}
-
 static int igc_save_launchtime_params(struct igc_adapter *adapter, int queue,
 				      bool enable)
 {
@@ -6940,12 +6922,13 @@ static const struct net_device_ops igc_netdev_ops = {
 	.ndo_fix_features	= igc_fix_features,
 	.ndo_set_features	= igc_set_features,
 	.ndo_features_check	= igc_features_check,
-	.ndo_eth_ioctl		= igc_ioctl,
 	.ndo_setup_tc		= igc_setup_tc,
 	.ndo_bpf		= igc_bpf,
 	.ndo_xdp_xmit		= igc_xdp_xmit,
 	.ndo_xsk_wakeup		= igc_xsk_wakeup,
 	.ndo_get_tstamp		= igc_get_tstamp,
+	.ndo_hwtstamp_get	= igc_ptp_hwtstamp_get,
+	.ndo_hwtstamp_set	= igc_ptp_hwtstamp_set,
 };
 
 u32 igc_rd32(struct igc_hw *hw, u32 reg)
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index f4f5c28615d3..b7b46d863bee 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -626,7 +626,7 @@ static void igc_ptp_enable_tx_timestamp(struct igc_adapter *adapter)
  * Return: 0 in case of success, negative errno code otherwise.
  */
 static int igc_ptp_set_timestamp_mode(struct igc_adapter *adapter,
-				      struct hwtstamp_config *config)
+				      struct kernel_hwtstamp_config *config)
 {
 	switch (config->tx_type) {
 	case HWTSTAMP_TX_OFF:
@@ -853,48 +853,46 @@ void igc_ptp_tx_tstamp_event(struct igc_adapter *adapter)
 }
 
 /**
- * igc_ptp_set_ts_config - set hardware time stamping config
+ * igc_ptp_hwtstamp_set - set hardware time stamping config
  * @netdev: network interface device structure
- * @ifr: interface request data
+ * @config: timestamping configuration structure
+ * @extack: netlink extended ack structure for error reporting
  *
  **/
-int igc_ptp_set_ts_config(struct net_device *netdev, struct ifreq *ifr)
+int igc_ptp_hwtstamp_set(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config,
+			 struct netlink_ext_ack *extack)
 {
 	struct igc_adapter *adapter = netdev_priv(netdev);
-	struct hwtstamp_config config;
 	int err;
 
-	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
-		return -EFAULT;
-
-	err = igc_ptp_set_timestamp_mode(adapter, &config);
+	err = igc_ptp_set_timestamp_mode(adapter, config);
 	if (err)
 		return err;
 
 	/* save these settings for future reference */
-	memcpy(&adapter->tstamp_config, &config,
-	       sizeof(adapter->tstamp_config));
+	adapter->tstamp_config = *config;
 
-	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
-		-EFAULT : 0;
+	return 0;
 }
 
 /**
- * igc_ptp_get_ts_config - get hardware time stamping config
+ * igc_ptp_hwtstamp_get - get hardware time stamping config
  * @netdev: network interface device structure
- * @ifr: interface request data
+ * @config: timestamping configuration structure
  *
  * Get the hwtstamp_config settings to return to the user. Rather than attempt
  * to deconstruct the settings from the registers, just return a shadow copy
  * of the last known settings.
  **/
-int igc_ptp_get_ts_config(struct net_device *netdev, struct ifreq *ifr)
+int igc_ptp_hwtstamp_get(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config)
 {
 	struct igc_adapter *adapter = netdev_priv(netdev);
-	struct hwtstamp_config *config = &adapter->tstamp_config;
 
-	return copy_to_user(ifr->ifr_data, config, sizeof(*config)) ?
-		-EFAULT : 0;
+	*config = adapter->tstamp_config;
+
+	return 0;
 }
 
 /* The two conditions below must be met for cross timestamping via
-- 
2.43.0

[PATCH iwl-next 3/5] igb: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Vladimir Oltean]

New timestamping API was introduced in commit 66f7223039c0 ("net: add
NDOs for configuring hardware timestamping") from kernel v6.6.

It is time to convert the Intel igb driver to the new API, so that
timestamping configuration can be removed from the ndo_eth_ioctl() path
completely.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/intel/igb/igb.h      |  9 ++++--
 drivers/net/ethernet/intel/igb/igb_main.c |  6 ++--
 drivers/net/ethernet/intel/igb/igb_ptp.c  | 37 +++++++++++------------
 3 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index f34ead8243e9..c3f4f7cd264e 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -626,7 +626,7 @@ struct igb_adapter {
 	struct delayed_work ptp_overflow_work;
 	struct work_struct ptp_tx_work;
 	struct sk_buff *ptp_tx_skb;
-	struct hwtstamp_config tstamp_config;
+	struct kernel_hwtstamp_config tstamp_config;
 	unsigned long ptp_tx_start;
 	unsigned long last_rx_ptp_check;
 	unsigned long last_rx_timestamp;
@@ -771,8 +771,11 @@ void igb_ptp_tx_hang(struct igb_adapter *adapter);
 void igb_ptp_rx_rgtstamp(struct igb_q_vector *q_vector, struct sk_buff *skb);
 int igb_ptp_rx_pktstamp(struct igb_q_vector *q_vector, void *va,
 			ktime_t *timestamp);
-int igb_ptp_set_ts_config(struct net_device *netdev, struct ifreq *ifr);
-int igb_ptp_get_ts_config(struct net_device *netdev, struct ifreq *ifr);
+int igb_ptp_hwtstamp_get(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config);
+int igb_ptp_hwtstamp_set(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config,
+			 struct netlink_ext_ack *extack);
 void igb_set_flag_queue_pairs(struct igb_adapter *, const u32);
 unsigned int igb_get_max_rss_queues(struct igb_adapter *);
 #ifdef CONFIG_IGB_HWMON
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 9e9a5900e6e5..abed0704ed04 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3062,6 +3062,8 @@ static const struct net_device_ops igb_netdev_ops = {
 	.ndo_bpf		= igb_xdp,
 	.ndo_xdp_xmit		= igb_xdp_xmit,
 	.ndo_xsk_wakeup         = igb_xsk_wakeup,
+	.ndo_hwtstamp_get	= igb_ptp_hwtstamp_get,
+	.ndo_hwtstamp_set	= igb_ptp_hwtstamp_set,
 };
 
 /**
@@ -9315,10 +9317,6 @@ static int igb_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 	case SIOCGMIIREG:
 	case SIOCSMIIREG:
 		return igb_mii_ioctl(netdev, ifr, cmd);
-	case SIOCGHWTSTAMP:
-		return igb_ptp_get_ts_config(netdev, ifr);
-	case SIOCSHWTSTAMP:
-		return igb_ptp_set_ts_config(netdev, ifr);
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 793c96016288..05d30aba66db 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -1094,21 +1094,22 @@ void igb_ptp_rx_rgtstamp(struct igb_q_vector *q_vector, struct sk_buff *skb)
 }
 
 /**
- * igb_ptp_get_ts_config - get hardware time stamping config
+ * igb_ptp_hwtstamp_get - get hardware time stamping config
  * @netdev: netdev struct
- * @ifr: interface struct
+ * @config: timestamping configuration structure
  *
  * Get the hwtstamp_config settings to return to the user. Rather than attempt
  * to deconstruct the settings from the registers, just return a shadow copy
  * of the last known settings.
  **/
-int igb_ptp_get_ts_config(struct net_device *netdev, struct ifreq *ifr)
+int igb_ptp_hwtstamp_get(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config)
 {
 	struct igb_adapter *adapter = netdev_priv(netdev);
-	struct hwtstamp_config *config = &adapter->tstamp_config;
 
-	return copy_to_user(ifr->ifr_data, config, sizeof(*config)) ?
-		-EFAULT : 0;
+	*config = adapter->tstamp_config;
+
+	return 0;
 }
 
 /**
@@ -1129,7 +1130,7 @@ int igb_ptp_get_ts_config(struct net_device *netdev, struct ifreq *ifr)
  * level 2 or 4".
  */
 static int igb_ptp_set_timestamp_mode(struct igb_adapter *adapter,
-				      struct hwtstamp_config *config)
+				      struct kernel_hwtstamp_config *config)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	u32 tsync_tx_ctl = E1000_TSYNCTXCTL_ENABLED;
@@ -1275,30 +1276,26 @@ static int igb_ptp_set_timestamp_mode(struct igb_adapter *adapter,
 }
 
 /**
- * igb_ptp_set_ts_config - set hardware time stamping config
+ * igb_ptp_hwtstamp_set - set hardware time stamping config
  * @netdev: netdev struct
- * @ifr: interface struct
- *
+ * @config: timestamping configuration structure
+ * @extack: netlink extended ack structure for error reporting
  **/
-int igb_ptp_set_ts_config(struct net_device *netdev, struct ifreq *ifr)
+int igb_ptp_hwtstamp_set(struct net_device *netdev,
+			 struct kernel_hwtstamp_config *config,
+			 struct netlink_ext_ack *extack)
 {
 	struct igb_adapter *adapter = netdev_priv(netdev);
-	struct hwtstamp_config config;
 	int err;
 
-	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
-		return -EFAULT;
-
-	err = igb_ptp_set_timestamp_mode(adapter, &config);
+	err = igb_ptp_set_timestamp_mode(adapter, config);
 	if (err)
 		return err;
 
 	/* save these settings for future reference */
-	memcpy(&adapter->tstamp_config, &config,
-	       sizeof(adapter->tstamp_config));
+	adapter->tstamp_config = *config;
 
-	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
-		-EFAULT : 0;
+	return 0;
 }
 
 /**
-- 
2.43.0

[PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Vladimir Oltean]

New timestamping API was introduced in commit 66f7223039c0 ("net: add
NDOs for configuring hardware timestamping") from kernel v6.6.

It is time to convert the Intel ixgbe driver to the new API, so that
timestamping configuration can be removed from the ndo_eth_ioctl() path
completely.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  9 ++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +--
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c  | 42 +++++++++----------
 3 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 47311b134a7a..9432ad7f2352 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -784,7 +784,7 @@ struct ixgbe_adapter {
 	struct ptp_clock_info ptp_caps;
 	struct work_struct ptp_tx_work;
 	struct sk_buff *ptp_tx_skb;
-	struct hwtstamp_config tstamp_config;
+	struct kernel_hwtstamp_config tstamp_config;
 	unsigned long ptp_tx_start;
 	unsigned long last_overflow_check;
 	unsigned long last_rx_ptp_check;
@@ -1079,8 +1079,11 @@ static inline void ixgbe_ptp_rx_hwtstamp(struct ixgbe_ring *rx_ring,
 	rx_ring->last_rx_timestamp = jiffies;
 }
 
-int ixgbe_ptp_set_ts_config(struct ixgbe_adapter *adapter, struct ifreq *ifr);
-int ixgbe_ptp_get_ts_config(struct ixgbe_adapter *adapter, struct ifreq *ifr);
+int ixgbe_ptp_hwtstamp_get(struct net_device *netdev,
+			   struct kernel_hwtstamp_config *config);
+int ixgbe_ptp_hwtstamp_set(struct net_device *netdev,
+			   struct kernel_hwtstamp_config *config,
+			   struct netlink_ext_ack *extack);
 void ixgbe_ptp_start_cyclecounter(struct ixgbe_adapter *adapter);
 void ixgbe_ptp_reset(struct ixgbe_adapter *adapter);
 void ixgbe_ptp_check_pps_event(struct ixgbe_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 03d31e5b131d..85aee90ba87e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -9438,10 +9438,6 @@ static int ixgbe_ioctl(struct net_device *netdev, struct ifreq *req, int cmd)
 	struct ixgbe_adapter *adapter = ixgbe_from_netdev(netdev);
 
 	switch (cmd) {
-	case SIOCSHWTSTAMP:
-		return ixgbe_ptp_set_ts_config(adapter, req);
-	case SIOCGHWTSTAMP:
-		return ixgbe_ptp_get_ts_config(adapter, req);
 	case SIOCGMIIPHY:
 		if (!adapter->hw.phy.ops.read_reg)
 			return -EOPNOTSUPP;
@@ -10905,6 +10901,8 @@ static const struct net_device_ops ixgbe_netdev_ops = {
 	.ndo_bpf		= ixgbe_xdp,
 	.ndo_xdp_xmit		= ixgbe_xdp_xmit,
 	.ndo_xsk_wakeup         = ixgbe_xsk_wakeup,
+	.ndo_hwtstamp_get	= ixgbe_ptp_hwtstamp_get,
+	.ndo_hwtstamp_set	= ixgbe_ptp_hwtstamp_set,
 };
 
 static void ixgbe_disable_txr_hw(struct ixgbe_adapter *adapter,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index eef25e11d938..ac7abd40c33b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -936,20 +936,22 @@ void ixgbe_ptp_rx_rgtstamp(struct ixgbe_q_vector *q_vector,
 }
 
 /**
- * ixgbe_ptp_get_ts_config - get current hardware timestamping configuration
- * @adapter: pointer to adapter structure
- * @ifr: ioctl data
+ * ixgbe_ptp_hwtstamp_get - get current hardware timestamping configuration
+ * @netdev: pointer to net device structure
+ * @config: timestamping configuration structure
  *
  * This function returns the current timestamping settings. Rather than
  * attempt to deconstruct registers to fill in the values, simply keep a copy
  * of the old settings around, and return a copy when requested.
  */
-int ixgbe_ptp_get_ts_config(struct ixgbe_adapter *adapter, struct ifreq *ifr)
+int ixgbe_ptp_hwtstamp_get(struct net_device *netdev,
+			   struct kernel_hwtstamp_config *config)
 {
-	struct hwtstamp_config *config = &adapter->tstamp_config;
+	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 
-	return copy_to_user(ifr->ifr_data, config,
-			    sizeof(*config)) ? -EFAULT : 0;
+	*config = adapter->tstamp_config;
+
+	return 0;
 }
 
 /**
@@ -978,7 +980,7 @@ int ixgbe_ptp_get_ts_config(struct ixgbe_adapter *adapter, struct ifreq *ifr)
  * mode, if required to support the specifically requested mode.
  */
 static int ixgbe_ptp_set_timestamp_mode(struct ixgbe_adapter *adapter,
-				 struct hwtstamp_config *config)
+					struct kernel_hwtstamp_config *config)
 {
 	struct ixgbe_hw *hw = &adapter->hw;
 	u32 tsync_tx_ctl = IXGBE_TSYNCTXCTL_ENABLED;
@@ -1129,31 +1131,29 @@ static int ixgbe_ptp_set_timestamp_mode(struct ixgbe_adapter *adapter,
 }
 
 /**
- * ixgbe_ptp_set_ts_config - user entry point for timestamp mode
- * @adapter: pointer to adapter struct
- * @ifr: ioctl data
+ * ixgbe_ptp_hwtstamp_set - user entry point for timestamp mode
+ * @netdev: pointer to net device structure
+ * @config: timestamping configuration structure
+ * @extack: netlink extended ack structure for error reporting
  *
  * Set hardware to requested mode. If unsupported, return an error with no
  * changes. Otherwise, store the mode for future reference.
  */
-int ixgbe_ptp_set_ts_config(struct ixgbe_adapter *adapter, struct ifreq *ifr)
+int ixgbe_ptp_hwtstamp_set(struct net_device *netdev,
+			   struct kernel_hwtstamp_config *config,
+			   struct netlink_ext_ack *extack)
 {
-	struct hwtstamp_config config;
+	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 	int err;
 
-	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
-		return -EFAULT;
-
-	err = ixgbe_ptp_set_timestamp_mode(adapter, &config);
+	err = ixgbe_ptp_set_timestamp_mode(adapter, config);
 	if (err)
 		return err;
 
 	/* save these settings for future reference */
-	memcpy(&adapter->tstamp_config, &config,
-	       sizeof(adapter->tstamp_config));
+	adapter->tstamp_config = *config;
 
-	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
-		-EFAULT : 0;
+	return 0;
 }
 
 static void ixgbe_ptp_link_speed_adjust(struct ixgbe_adapter *adapter,
-- 
2.43.0

[PATCH iwl-next 5/5] i40e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Vladimir Oltean]

New timestamping API was introduced in commit 66f7223039c0 ("net: add
NDOs for configuring hardware timestamping") from kernel v6.6.

It is time to convert the Intel i40e driver to the new API, so that
timestamping configuration can be removed from the ndo_eth_ioctl() path
completely.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h      |  9 +++--
 drivers/net/ethernet/intel/i40e/i40e_main.c | 24 +-----------
 drivers/net/ethernet/intel/i40e/i40e_ptp.c  | 43 +++++++++++----------
 3 files changed, 31 insertions(+), 45 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index c67963bfe14e..6250f5203c15 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -660,7 +660,7 @@ struct i40e_pf {
 	struct ptp_clock_info ptp_caps;
 	struct sk_buff *ptp_tx_skb;
 	unsigned long ptp_tx_start;
-	struct hwtstamp_config tstamp_config;
+	struct kernel_hwtstamp_config tstamp_config;
 	struct timespec64 ptp_prev_hw_time;
 	struct work_struct ptp_extts0_work;
 	ktime_t ptp_reset_start;
@@ -1302,8 +1302,11 @@ void i40e_ptp_tx_hang(struct i40e_pf *pf);
 void i40e_ptp_tx_hwtstamp(struct i40e_pf *pf);
 void i40e_ptp_rx_hwtstamp(struct i40e_pf *pf, struct sk_buff *skb, u8 index);
 void i40e_ptp_set_increment(struct i40e_pf *pf);
-int i40e_ptp_set_ts_config(struct i40e_pf *pf, struct ifreq *ifr);
-int i40e_ptp_get_ts_config(struct i40e_pf *pf, struct ifreq *ifr);
+int i40e_ptp_hwtstamp_get(struct net_device *netdev,
+			  struct kernel_hwtstamp_config *config);
+int i40e_ptp_hwtstamp_set(struct net_device *netdev,
+			  struct kernel_hwtstamp_config *config,
+			  struct netlink_ext_ack *extack);
 void i40e_ptp_save_hw_time(struct i40e_pf *pf);
 void i40e_ptp_restore_hw_time(struct i40e_pf *pf);
 void i40e_ptp_init(struct i40e_pf *pf);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 120d68654e3f..73af81e0c229 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -2953,27 +2953,6 @@ static int i40e_change_mtu(struct net_device *netdev, int new_mtu)
 	return 0;
 }
 
-/**
- * i40e_ioctl - Access the hwtstamp interface
- * @netdev: network interface device structure
- * @ifr: interface request data
- * @cmd: ioctl command
- **/
-int i40e_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
-{
-	struct i40e_netdev_priv *np = netdev_priv(netdev);
-	struct i40e_pf *pf = np->vsi->back;
-
-	switch (cmd) {
-	case SIOCGHWTSTAMP:
-		return i40e_ptp_get_ts_config(pf, ifr);
-	case SIOCSHWTSTAMP:
-		return i40e_ptp_set_ts_config(pf, ifr);
-	default:
-		return -EOPNOTSUPP;
-	}
-}
-
 /**
  * i40e_vlan_stripping_enable - Turn on vlan stripping for the VSI
  * @vsi: the vsi being adjusted
@@ -13622,7 +13601,6 @@ static const struct net_device_ops i40e_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_set_mac_address	= i40e_set_mac,
 	.ndo_change_mtu		= i40e_change_mtu,
-	.ndo_eth_ioctl		= i40e_ioctl,
 	.ndo_tx_timeout		= i40e_tx_timeout,
 	.ndo_vlan_rx_add_vid	= i40e_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid	= i40e_vlan_rx_kill_vid,
@@ -13650,6 +13628,8 @@ static const struct net_device_ops i40e_netdev_ops = {
 	.ndo_xsk_wakeup	        = i40e_xsk_wakeup,
 	.ndo_dfwd_add_station	= i40e_fwd_add,
 	.ndo_dfwd_del_station	= i40e_fwd_del,
+	.ndo_hwtstamp_get	= i40e_ptp_hwtstamp_get,
+	.ndo_hwtstamp_set	= i40e_ptp_hwtstamp_set,
 };
 
 /**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index b72a4b5d76b9..1d04ea7df552 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -912,23 +912,26 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
 }
 
 /**
- * i40e_ptp_get_ts_config - ioctl interface to read the HW timestamping
- * @pf: Board private structure
- * @ifr: ioctl data
+ * i40e_ptp_hwtstamp_get - interface to read the HW timestamping
+ * @netdev: Network device structure
+ * @config: Timestamping configuration structure
  *
  * Obtain the current hardware timestamping settigs as requested. To do this,
  * keep a shadow copy of the timestamp settings rather than attempting to
  * deconstruct it from the registers.
  **/
-int i40e_ptp_get_ts_config(struct i40e_pf *pf, struct ifreq *ifr)
+int i40e_ptp_hwtstamp_get(struct net_device *netdev,
+			  struct kernel_hwtstamp_config *config)
 {
-	struct hwtstamp_config *config = &pf->tstamp_config;
+	struct i40e_netdev_priv *np = netdev_priv(netdev);
+	struct i40e_pf *pf = np->vsi->back;
 
 	if (!test_bit(I40E_FLAG_PTP_ENA, pf->flags))
 		return -EOPNOTSUPP;
 
-	return copy_to_user(ifr->ifr_data, config, sizeof(*config)) ?
-		-EFAULT : 0;
+	*config = pf->tstamp_config;
+
+	return 0;
 }
 
 /**
@@ -1167,7 +1170,7 @@ int i40e_ptp_alloc_pins(struct i40e_pf *pf)
  * more broad if the specific filter is not directly supported.
  **/
 static int i40e_ptp_set_timestamp_mode(struct i40e_pf *pf,
-				       struct hwtstamp_config *config)
+				       struct kernel_hwtstamp_config *config)
 {
 	struct i40e_hw *hw = &pf->hw;
 	u32 tsyntype, regval;
@@ -1290,9 +1293,10 @@ static int i40e_ptp_set_timestamp_mode(struct i40e_pf *pf,
 }
 
 /**
- * i40e_ptp_set_ts_config - ioctl interface to control the HW timestamping
- * @pf: Board private structure
- * @ifr: ioctl data
+ * i40e_ptp_hwtstamp_set - interface to control the HW timestamping
+ * @netdev: Network device structure
+ * @config: Timestamping configuration structure
+ * @extack: Netlink extended ack structure for error reporting
  *
  * Respond to the user filter requests and make the appropriate hardware
  * changes here. The XL710 cannot support splitting of the Tx/Rx timestamping
@@ -1303,26 +1307,25 @@ static int i40e_ptp_set_timestamp_mode(struct i40e_pf *pf,
  * as the user receives the timestamps they care about and the user is notified
  * the filter has been broadened.
  **/
-int i40e_ptp_set_ts_config(struct i40e_pf *pf, struct ifreq *ifr)
+int i40e_ptp_hwtstamp_set(struct net_device *netdev,
+			  struct kernel_hwtstamp_config *config,
+			  struct netlink_ext_ack *extack)
 {
-	struct hwtstamp_config config;
+	struct i40e_netdev_priv *np = netdev_priv(netdev);
+	struct i40e_pf *pf = np->vsi->back;
 	int err;
 
 	if (!test_bit(I40E_FLAG_PTP_ENA, pf->flags))
 		return -EOPNOTSUPP;
 
-	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
-		return -EFAULT;
-
-	err = i40e_ptp_set_timestamp_mode(pf, &config);
+	err = i40e_ptp_set_timestamp_mode(pf, config);
 	if (err)
 		return err;
 
 	/* save these settings for future reference */
-	pf->tstamp_config = config;
+	pf->tstamp_config = *config;
 
-	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
-		-EFAULT : 0;
+	return 0;
 }
 
 /**
-- 
2.43.0

Re: [PATCH iwl-next 1/5] ice: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Jacob Keller]

On 5/13/2025 3:11 AM, Vladimir Oltean wrote:
> New timestamping API was introduced in commit 66f7223039c0 ("net: add
> NDOs for configuring hardware timestamping") from kernel v6.6.
> 
> It is time to convert the Intel ice driver to the new API, so that
> timestamping configuration can be removed from the ndo_eth_ioctl() path
> completely.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Acked-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> Previously submitted as
> https://patchwork.kernel.org/project/netdevbpf/patch/20250512160036.909434-1-vladimir.oltean@nxp.com/
> 
I had a minor conflict when applying to Tony's dev-queue, but it was
trivial to fix. The series has been applied and marked for testing.

Thanks,
Jake

Re: [PATCH iwl-next 0/5] Intel driver conversion to new hardware timestamping API

[Vadim Fedorenko]

On 13/05/2025 11:11, Vladimir Oltean wrote:
> Since the introduction of the new ndo_hwtstamp_get() and
> ndo_hwtstamp_set() operations in v6.6, only e1000e and iavf have
> been converted.
> 
> There is a push to get rid of the old code path for configuring hardware
> timestamping, so the reset of the drivers are converted in this patch
> set.
> 
> Vladimir Oltean (5):
>    ice: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>    igc: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>    igb: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>    ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>    i40e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
> 
>   drivers/net/ethernet/intel/i40e/i40e.h        |  9 ++--
>   drivers/net/ethernet/intel/i40e/i40e_main.c   | 24 +---------
>   drivers/net/ethernet/intel/i40e/i40e_ptp.c    | 43 +++++++++---------
>   drivers/net/ethernet/intel/ice/ice_main.c     | 24 +---------
>   drivers/net/ethernet/intel/ice/ice_ptp.c      | 45 ++++++++++---------
>   drivers/net/ethernet/intel/ice/ice_ptp.h      | 17 ++++---
>   drivers/net/ethernet/intel/igb/igb.h          |  9 ++--
>   drivers/net/ethernet/intel/igb/igb_main.c     |  6 +--
>   drivers/net/ethernet/intel/igb/igb_ptp.c      | 37 +++++++--------
>   drivers/net/ethernet/intel/igc/igc.h          |  9 ++--
>   drivers/net/ethernet/intel/igc/igc_main.c     | 21 +--------
>   drivers/net/ethernet/intel/igc/igc_ptp.c      | 36 +++++++--------
>   drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  9 ++--
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +--
>   drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c  | 42 ++++++++---------
>   15 files changed, 147 insertions(+), 190 deletions(-)
> 

The changes are pretty straight-forward.

For series:
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

RE: [PATCH iwl-next 0/5] Intel driver conversion to new hardware timestamping API

[Olech, Milena]

On 05/13/2025 12:11 PM, Vladimir Oltean wrote:

>Since the introduction of the new ndo_hwtstamp_get() and
>ndo_hwtstamp_set() operations in v6.6, only e1000e and iavf have
>been converted.
>
>There is a push to get rid of the old code path for configuring hardware
>timestamping, so the reset of the drivers are converted in this patch
>set.
>

Thanks for introducing these changes!
Reviewed-by: Milena Olech <milena.olech@intel.com>

+ I'm not a part of val team, but I've made some engineering basic
tests on ICE and it seems to work.

>Vladimir Oltean (5):
>  ice: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>  igc: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>  igb: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>  ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>  i40e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>
> drivers/net/ethernet/intel/i40e/i40e.h        |  9 ++--
> drivers/net/ethernet/intel/i40e/i40e_main.c   | 24 +---------
> drivers/net/ethernet/intel/i40e/i40e_ptp.c    | 43 +++++++++---------
> drivers/net/ethernet/intel/ice/ice_main.c     | 24 +---------
> drivers/net/ethernet/intel/ice/ice_ptp.c      | 45 ++++++++++---------
> drivers/net/ethernet/intel/ice/ice_ptp.h      | 17 ++++---
> drivers/net/ethernet/intel/igb/igb.h          |  9 ++--
> drivers/net/ethernet/intel/igb/igb_main.c     |  6 +--
> drivers/net/ethernet/intel/igb/igb_ptp.c      | 37 +++++++--------
> drivers/net/ethernet/intel/igc/igc.h          |  9 ++--
> drivers/net/ethernet/intel/igc/igc_main.c     | 21 +--------
> drivers/net/ethernet/intel/igc/igc_ptp.c      | 36 +++++++--------
> drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  9 ++--
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +--
> drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c  | 42 ++++++++---------
> 15 files changed, 147 insertions(+), 190 deletions(-)
>
>-- 
>2.43.0

Re: [PATCH iwl-next 1/5] ice: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Simon Horman]

On Tue, May 13, 2025 at 01:11:28PM +0300, Vladimir Oltean wrote:
> New timestamping API was introduced in commit 66f7223039c0 ("net: add
> NDOs for configuring hardware timestamping") from kernel v6.6.
> 
> It is time to convert the Intel ice driver to the new API, so that
> timestamping configuration can be removed from the ndo_eth_ioctl() path
> completely.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Acked-by: Jacob Keller <jacob.e.keller@intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH iwl-next 2/5] igc: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Simon Horman]

On Tue, May 13, 2025 at 01:11:29PM +0300, Vladimir Oltean wrote:
> New timestamping API was introduced in commit 66f7223039c0 ("net: add
> NDOs for configuring hardware timestamping") from kernel v6.6.
> 
> It is time to convert the Intel igc driver to the new API, so that
> timestamping configuration can be removed from the ndo_eth_ioctl() path
> completely.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH iwl-next 3/5] igb: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Simon Horman]

On Tue, May 13, 2025 at 01:11:30PM +0300, Vladimir Oltean wrote:
> New timestamping API was introduced in commit 66f7223039c0 ("net: add
> NDOs for configuring hardware timestamping") from kernel v6.6.
> 
> It is time to convert the Intel igb driver to the new API, so that
> timestamping configuration can be removed from the ndo_eth_ioctl() path
> completely.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Simon Horman]

On Tue, May 13, 2025 at 01:11:31PM +0300, Vladimir Oltean wrote:
> New timestamping API was introduced in commit 66f7223039c0 ("net: add
> NDOs for configuring hardware timestamping") from kernel v6.6.
> 
> It is time to convert the Intel ixgbe driver to the new API, so that
> timestamping configuration can be removed from the ndo_eth_ioctl() path
> completely.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH iwl-next 5/5] i40e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Simon Horman]

On Tue, May 13, 2025 at 01:11:32PM +0300, Vladimir Oltean wrote:
> New timestamping API was introduced in commit 66f7223039c0 ("net: add
> NDOs for configuring hardware timestamping") from kernel v6.6.
> 
> It is time to convert the Intel i40e driver to the new API, so that
> timestamping configuration can be removed from the ndo_eth_ioctl() path
> completely.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [Intel-wired-lan] [PATCH iwl-next 2/5] igc: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Lifshits, Vitaly]

On 5/16/2025 3:26 PM, Simon Horman wrote:
> On Tue, May 13, 2025 at 01:11:29PM +0300, Vladimir Oltean wrote:
>> New timestamping API was introduced in commit 66f7223039c0 ("net: add
>> NDOs for configuring hardware timestamping") from kernel v6.6.
>>
>> It is time to convert the Intel igc driver to the new API, so that
>> timestamping configuration can be removed from the ndo_eth_ioctl() path
>> completely.
>>
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> 

Reviewed-by: Vitaly Lifshits <vitaly.lifshits@intel.com>

Re: [Intel-wired-lan] [PATCH iwl-next 2/5] igc: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Mor Bar-Gabay]

On 13/05/2025 13:11, Vladimir Oltean wrote:
> New timestamping API was introduced in commit 66f7223039c0 ("net: add
> NDOs for configuring hardware timestamping") from kernel v6.6.
> 
> It is time to convert the Intel igc driver to the new API, so that
> timestamping configuration can be removed from the ndo_eth_ioctl() path
> completely.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Reviewed-by: Simon Horman <horms@kernel.org>
> Reviewed-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
> ---
>   drivers/net/ethernet/intel/igc/igc.h      |  9 ++++--
>   drivers/net/ethernet/intel/igc/igc_main.c | 21 ++-----------
>   drivers/net/ethernet/intel/igc/igc_ptp.c  | 36 +++++++++++------------
>   3 files changed, 25 insertions(+), 41 deletions(-)
> 
Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>

RE: [Intel-wired-lan] [PATCH iwl-next 5/5] i40e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Rinitha, SX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Vladimir Oltean
> Sent: 13 May 2025 15:42
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Keller, Jacob E <jacob.e.keller@intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Gomes, Vinicius <vinicius.gomes@intel.com>; Vadim Fedorenko <vadim.fedorenko@linux.dev>; Richard Cochran <richardcochran@gmail.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next 5/5] i40e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>
> New timestamping API was introduced in commit 66f7223039c0 ("net: add NDOs for configuring hardware timestamping") from kernel v6.6.
>
> It is time to convert the Intel i40e driver to the new API, so that timestamping configuration can be removed from the ndo_eth_ioctl() path completely.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e.h      |  9 +++--
> drivers/net/ethernet/intel/i40e/i40e_main.c | 24 +-----------  drivers/net/ethernet/intel/i40e/i40e_ptp.c  | 43 +++++++++++----------
> 3 files changed, 31 insertions(+), 45 deletions(-)
>

Tested-by: Rinitha S <sx.rinitha@intel.com> (A Contingent worker at Intel)

RE: [Intel-wired-lan] [PATCH iwl-next 1/5] ice: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Rinitha, SX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Vladimir Oltean
> Sent: 13 May 2025 15:41
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Keller, Jacob E <jacob.e.keller@intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Gomes, Vinicius <vinicius.gomes@intel.com>; Vadim Fedorenko <vadim.fedorenko@linux.dev>; Richard Cochran <richardcochran@gmail.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next 1/5] ice: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>
> New timestamping API was introduced in commit 66f7223039c0 ("net: add NDOs for configuring hardware timestamping") from kernel v6.6.
>
> It is time to convert the Intel ice driver to the new API, so that timestamping configuration can be removed from the ndo_eth_ioctl() path completely.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Acked-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> Previously submitted as
https://patchwork.kernel.org/project/netdevbpf/patch/20250512160036.909434-1-vladimir.oltean@nxp.com/
>
> drivers/net/ethernet/intel/ice/ice_main.c | 24 +-----------  drivers/net/ethernet/intel/ice/ice_ptp.c  | 45 ++++++++++++-----------  drivers/net/ethernet/intel/ice/ice_ptp.h  | 17 ++++++---
> 3 files changed, 37 insertions(+), 49 deletions(-)
>

Tested-by: Rinitha S <sx.rinitha@intel.com> (A Contingent worker at Intel)

RE: [Intel-wired-lan] [PATCH iwl-next 3/5] igb: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Rinitha, SX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Vladimir Oltean
> Sent: 13 May 2025 15:42
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Keller, Jacob E <jacob.e.keller@intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Gomes, Vinicius <vinicius.gomes@intel.com>; > Vadim Fedorenko <vadim.fedorenko@linux.dev>; Richard Cochran <richardcochran@gmail.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next 3/5] igb: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>
> New timestamping API was introduced in commit 66f7223039c0 ("net: add NDOs for configuring hardware timestamping") from kernel v6.6.
>
> It is time to convert the Intel igb driver to the new API, so that timestamping configuration can be removed from the ndo_eth_ioctl() path completely.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/ethernet/intel/igb/igb.h      |  9 ++++--
> drivers/net/ethernet/intel/igb/igb_main.c |  6 ++--  drivers/net/ethernet/intel/igb/igb_ptp.c  | 37 +++++++++++------------
> 3 files changed, 25 insertions(+), 27 deletions(-)
>

Tested-by: Rinitha S <sx.rinitha@intel.com> (A Contingent worker at Intel)

Re: [PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 4607 bytes --]



On 5/13/2025 3:11 AM, Vladimir Oltean wrote:
> New timestamping API was introduced in commit 66f7223039c0 ("net: add
> NDOs for configuring hardware timestamping") from kernel v6.6.
> 
> It is time to convert the Intel ixgbe driver to the new API, so that
> timestamping configuration can be removed from the ndo_eth_ioctl() path
> completely.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Ugh. Apologies for the late reply here, but this took for ever to track
down what was wrong in our testing.

The ixgbe patch has a somewhat subtle bug which lead to failed timestamp
configuration and likely other forms of memory corruption.

>  drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  9 ++--
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +--
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c  | 42 +++++++++----------
>  3 files changed, 29 insertions(+), 28 deletions(-)
> 

>  
>  /**
> - * ixgbe_ptp_get_ts_config - get current hardware timestamping configuration
> - * @adapter: pointer to adapter structure
> - * @ifr: ioctl data
> + * ixgbe_ptp_hwtstamp_get - get current hardware timestamping configuration
> + * @netdev: pointer to net device structure
> + * @config: timestamping configuration structure
>   *
>   * This function returns the current timestamping settings. Rather than
>   * attempt to deconstruct registers to fill in the values, simply keep a copy
>   * of the old settings around, and return a copy when requested.
>   */
> -int ixgbe_ptp_get_ts_config(struct ixgbe_adapter *adapter, struct ifreq *ifr)
> +int ixgbe_ptp_hwtstamp_get(struct net_device *netdev,
> +			   struct kernel_hwtstamp_config *config)
>  {
> -	struct hwtstamp_config *config = &adapter->tstamp_config;
> +	struct ixgbe_adapter *adapter = netdev_priv(netdev);
>  

ixgbe doesn't directly assign the adapter to netdev_priv and this needs
to be ixgbe_from_netdev, since there is a wrapper ixgbe_netdev_priv
structure. I didn't dig into why, but both get and set are wrong here,
and are misinterpreting the ixgbe_netdev_priv structure as
ixgbe_adapter, which is obviously wrong.

See its definition quoted here:
> static inline struct ixgbe_adapter *ixgbe_from_netdev(struct net_device *netdev)
> {
>         struct ixgbe_netdevice_priv *priv = netdev_priv(netdev);
> 
>         return priv->adapter;
> }
> 

Whats odd is that the netdev priv structure is just a wrapper around a
pointer to the adapter:

> struct ixgbe_netdevice_priv {
>         struct ixgbe_adapter *adapter;
> };


> -	return copy_to_user(ifr->ifr_data, config,
> -			    sizeof(*config)) ? -EFAULT : 0;
> +	*config = adapter->tstamp_config;
> +
> +	return 0;
>  }

Because we're completely pointing to the wrong memory, this overwrites
who knows what since the ixgbe_netdev_priv is just the pointer address.

>  
>  /**
> @@ -978,7 +980,7 @@ int ixgbe_ptp_get_ts_config(struct ixgbe_adapter *adapter, struct ifreq *ifr)
>   * mode, if required to support the specifically requested mode.
>   */
>  static int ixgbe_ptp_set_timestamp_mode(struct ixgbe_adapter *adapter,
> -				 struct hwtstamp_config *config)
> +					struct kernel_hwtstamp_config *config)
>  {
>  	struct ixgbe_hw *hw = &adapter->hw;
>  	u32 tsync_tx_ctl = IXGBE_TSYNCTXCTL_ENABLED;
> @@ -1129,31 +1131,29 @@ static int ixgbe_ptp_set_timestamp_mode(struct ixgbe_adapter *adapter,
>  }
>  
>  /**
> - * ixgbe_ptp_set_ts_config - user entry point for timestamp mode
> - * @adapter: pointer to adapter struct
> - * @ifr: ioctl data
> + * ixgbe_ptp_hwtstamp_set - user entry point for timestamp mode
> + * @netdev: pointer to net device structure
> + * @config: timestamping configuration structure
> + * @extack: netlink extended ack structure for error reporting
>   *
>   * Set hardware to requested mode. If unsupported, return an error with no
>   * changes. Otherwise, store the mode for future reference.
>   */
> -int ixgbe_ptp_set_ts_config(struct ixgbe_adapter *adapter, struct ifreq *ifr)
> +int ixgbe_ptp_hwtstamp_set(struct net_device *netdev,
> +			   struct kernel_hwtstamp_config *config,
> +			   struct netlink_ext_ack *extack)
>  {
> -	struct hwtstamp_config config;
> +	struct ixgbe_adapter *adapter = netdev_priv(netdev);
>  	int err;


Same here, the current code would need ixgbe_from_netdev()

I think the right thing to do is submit a patch/fix which drops the
completely useless ixgbe_netdevice_priv structure, but I'll have to dig
into git history to see why it was ever there in the first place.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 3820 bytes --]



On 6/30/2025 11:56 AM, Jacob Keller wrote:
> 
> 
> On 5/13/2025 3:11 AM, Vladimir Oltean wrote:
>> New timestamping API was introduced in commit 66f7223039c0 ("net: add
>> NDOs for configuring hardware timestamping") from kernel v6.6.
>>
>> It is time to convert the Intel ixgbe driver to the new API, so that
>> timestamping configuration can be removed from the ndo_eth_ioctl() path
>> completely.
>>
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>> ---
> 
> Ugh. Apologies for the late reply here, but this took for ever to track
> down what was wrong in our testing.
> 
> The ixgbe patch has a somewhat subtle bug which lead to failed timestamp
> configuration and likely other forms of memory corruption.
> 
>>  drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  9 ++--
>>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +--
>>  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c  | 42 +++++++++----------
>>  3 files changed, 29 insertions(+), 28 deletions(-)
>>
> 
>>  
>>  /**
>> - * ixgbe_ptp_get_ts_config - get current hardware timestamping configuration
>> - * @adapter: pointer to adapter structure
>> - * @ifr: ioctl data
>> + * ixgbe_ptp_hwtstamp_get - get current hardware timestamping configuration
>> + * @netdev: pointer to net device structure
>> + * @config: timestamping configuration structure
>>   *
>>   * This function returns the current timestamping settings. Rather than
>>   * attempt to deconstruct registers to fill in the values, simply keep a copy
>>   * of the old settings around, and return a copy when requested.
>>   */
>> -int ixgbe_ptp_get_ts_config(struct ixgbe_adapter *adapter, struct ifreq *ifr)
>> +int ixgbe_ptp_hwtstamp_get(struct net_device *netdev,
>> +			   struct kernel_hwtstamp_config *config)
>>  {
>> -	struct hwtstamp_config *config = &adapter->tstamp_config;
>> +	struct ixgbe_adapter *adapter = netdev_priv(netdev);
>>  
> 
> ixgbe doesn't directly assign the adapter to netdev_priv and this needs
> to be ixgbe_from_netdev, since there is a wrapper ixgbe_netdev_priv
> structure. I didn't dig into why, but both get and set are wrong here,
> and are misinterpreting the ixgbe_netdev_priv structure as
> ixgbe_adapter, which is obviously wrong.
> 
> See its definition quoted here:
>> static inline struct ixgbe_adapter *ixgbe_from_netdev(struct net_device *netdev)
>> {
>>         struct ixgbe_netdevice_priv *priv = netdev_priv(netdev);
>>
>>         return priv->adapter;
>> }
>>
> 
> Whats odd is that the netdev priv structure is just a wrapper around a
> pointer to the adapter:
> 
>> struct ixgbe_netdevice_priv {
>>         struct ixgbe_adapter *adapter;
>> };
> 
> 
>> -	return copy_to_user(ifr->ifr_data, config,
>> -			    sizeof(*config)) ? -EFAULT : 0;
>> +	*config = adapter->tstamp_config;
>> +
>> +	return 0;
>>  }
> 
> Because we're completely pointing to the wrong memory, this overwrites
> who knows what since the ixgbe_netdev_priv is just the pointer address.
> 
This is an artifact of the work to refactor ixgbe to support devlink:

Both netdev and devlink want a private structure allocated as a flexible
array member of their parent structure. They cannot both directly be
ice_adapter, so we chose to have devlink be ice_adapter, and netdev gets
the wrapper structure. I suspect the patches you wrote were based on a
tree before this refactor, and/or you just did not spot the refactor
happened.

a0285236ab93 ("ixgbe: add initial devlink support") is where the change
took place, which merged relatively recently.

@Tony, I think this is a pretty trivial fixup on your tree if you want
to handle it instead of forcing Vladimir to make a v2?

its really just switching netdev_priv to ixgbe_from_netdev in these two
functions.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Vladimir Oltean]

On Mon, Jun 30, 2025 at 01:47:07PM -0700, Jacob Keller wrote:
> On 6/30/2025 11:56 AM, Jacob Keller wrote:
> > On 5/13/2025 3:11 AM, Vladimir Oltean wrote:
> >> New timestamping API was introduced in commit 66f7223039c0 ("net: add
> >> NDOs for configuring hardware timestamping") from kernel v6.6.
> >>
> >> It is time to convert the Intel ixgbe driver to the new API, so that
> >> timestamping configuration can be removed from the ndo_eth_ioctl() path
> >> completely.
> >>
> >> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> >> ---
> > 
> > Ugh. Apologies for the late reply here, but this took for ever to track
> > down what was wrong in our testing.
> > 
> > The ixgbe patch has a somewhat subtle bug which lead to failed timestamp
> > configuration and likely other forms of memory corruption.
> > 
> >>  drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  9 ++--
> >>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +--
> >>  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c  | 42 +++++++++----------
> >>  3 files changed, 29 insertions(+), 28 deletions(-)
> >>
> > 
> >>  
> >>  /**
> >> - * ixgbe_ptp_get_ts_config - get current hardware timestamping configuration
> >> - * @adapter: pointer to adapter structure
> >> - * @ifr: ioctl data
> >> + * ixgbe_ptp_hwtstamp_get - get current hardware timestamping configuration
> >> + * @netdev: pointer to net device structure
> >> + * @config: timestamping configuration structure
> >>   *
> >>   * This function returns the current timestamping settings. Rather than
> >>   * attempt to deconstruct registers to fill in the values, simply keep a copy
> >>   * of the old settings around, and return a copy when requested.
> >>   */
> >> -int ixgbe_ptp_get_ts_config(struct ixgbe_adapter *adapter, struct ifreq *ifr)
> >> +int ixgbe_ptp_hwtstamp_get(struct net_device *netdev,
> >> +			   struct kernel_hwtstamp_config *config)
> >>  {
> >> -	struct hwtstamp_config *config = &adapter->tstamp_config;
> >> +	struct ixgbe_adapter *adapter = netdev_priv(netdev);
> >>  
> > 
> > ixgbe doesn't directly assign the adapter to netdev_priv and this needs
> > to be ixgbe_from_netdev, since there is a wrapper ixgbe_netdev_priv
> > structure. I didn't dig into why, but both get and set are wrong here,
> > and are misinterpreting the ixgbe_netdev_priv structure as
> > ixgbe_adapter, which is obviously wrong.
> > 
> > See its definition quoted here:
> >> static inline struct ixgbe_adapter *ixgbe_from_netdev(struct net_device *netdev)
> >> {
> >>         struct ixgbe_netdevice_priv *priv = netdev_priv(netdev);
> >>
> >>         return priv->adapter;
> >> }
> >>
> > 
> > Whats odd is that the netdev priv structure is just a wrapper around a
> > pointer to the adapter:
> > 
> >> struct ixgbe_netdevice_priv {
> >>         struct ixgbe_adapter *adapter;
> >> };
> > 
> > 
> >> -	return copy_to_user(ifr->ifr_data, config,
> >> -			    sizeof(*config)) ? -EFAULT : 0;
> >> +	*config = adapter->tstamp_config;
> >> +
> >> +	return 0;
> >>  }
> > 
> > Because we're completely pointing to the wrong memory, this overwrites
> > who knows what since the ixgbe_netdev_priv is just the pointer address.
> > 
> This is an artifact of the work to refactor ixgbe to support devlink:
> 
> Both netdev and devlink want a private structure allocated as a flexible
> array member of their parent structure. They cannot both directly be
> ice_adapter, so we chose to have devlink be ice_adapter, and netdev gets
> the wrapper structure. I suspect the patches you wrote were based on a
> tree before this refactor, and/or you just did not spot the refactor
> happened.
> 
> a0285236ab93 ("ixgbe: add initial devlink support") is where the change
> took place, which merged relatively recently.
> 
> @Tony, I think this is a pretty trivial fixup on your tree if you want
> to handle it instead of forcing Vladimir to make a v2?
> 
> its really just switching netdev_priv to ixgbe_from_netdev in these two
> functions.

Ugh :-/ sorry for the trouble, and thanks for doing the hard work of
characterizing this.

Indeed, my first conversion of ixgbe was in August 2023, as this commit
can attest:
https://github.com/vladimiroltean/linux/commit/0351ebf1eee3381ccfba9d31a924d1dd887a316f

At that time, Przemyslaw's commit fd5ef5203ce6 ("ixgbe: wrap
netdev_priv() usage") didn't exist, and "struct ixgbe_adapter *adapter =
netdev_priv(netdev);" was the de facto idiom in the driver, which I then
replicated two more times, in the new ixgbe_ptp_hwtstamp_set() and
ixgbe_ptp_hwtstamp_get() functions. Not only did I not notice that this
change took place, but it also compiled just fine, making me completely
unsuspecting...

Tony, let me know how you would like to proceed.

RE: [PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Nguyen, Anthony L]

> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> Sent: Monday, June 30, 2025 1:57 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; netdev@vger.kernel.org; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Gomes, Vinicius
> <vinicius.gomes@intel.com>; Vadim Fedorenko
> <vadim.fedorenko@linux.dev>; Richard Cochran <richardcochran@gmail.com>
> Subject: Re: [PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and
> ndo_hwtstamp_set()
> 
> On Mon, Jun 30, 2025 at 01:47:07PM -0700, Jacob Keller wrote:
> > On 6/30/2025 11:56 AM, Jacob Keller wrote:
> > > On 5/13/2025 3:11 AM, Vladimir Oltean wrote:
> > >> New timestamping API was introduced in commit 66f7223039c0 ("net:
> > >> add NDOs for configuring hardware timestamping") from kernel v6.6.
> > >>
> > >> It is time to convert the Intel ixgbe driver to the new API, so
> > >> that timestamping configuration can be removed from the
> > >> ndo_eth_ioctl() path completely.
> > >>
> > >> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > >> ---
> > >
> > > Ugh. Apologies for the late reply here, but this took for ever to
> > > track down what was wrong in our testing.
> > >
> > > The ixgbe patch has a somewhat subtle bug which lead to failed
> > > timestamp configuration and likely other forms of memory corruption.
> > >
> > >>  drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  9 ++--
> > >>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +--
> > >> drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c  | 42
> > >> +++++++++----------
> > >>  3 files changed, 29 insertions(+), 28 deletions(-)
> > >>
> > >
> > >>
> > >>  /**
> > >> - * ixgbe_ptp_get_ts_config - get current hardware timestamping
> > >> configuration
> > >> - * @adapter: pointer to adapter structure
> > >> - * @ifr: ioctl data
> > >> + * ixgbe_ptp_hwtstamp_get - get current hardware timestamping
> > >> + configuration
> > >> + * @netdev: pointer to net device structure
> > >> + * @config: timestamping configuration structure
> > >>   *
> > >>   * This function returns the current timestamping settings. Rather than
> > >>   * attempt to deconstruct registers to fill in the values, simply keep a copy
> > >>   * of the old settings around, and return a copy when requested.
> > >>   */
> > >> -int ixgbe_ptp_get_ts_config(struct ixgbe_adapter *adapter, struct
> > >> ifreq *ifr)
> > >> +int ixgbe_ptp_hwtstamp_get(struct net_device *netdev,
> > >> +			   struct kernel_hwtstamp_config *config)
> > >>  {
> > >> -	struct hwtstamp_config *config = &adapter->tstamp_config;
> > >> +	struct ixgbe_adapter *adapter = netdev_priv(netdev);
> > >>
> > >
> > > ixgbe doesn't directly assign the adapter to netdev_priv and this
> > > needs to be ixgbe_from_netdev, since there is a wrapper
> > > ixgbe_netdev_priv structure. I didn't dig into why, but both get and
> > > set are wrong here, and are misinterpreting the ixgbe_netdev_priv
> > > structure as ixgbe_adapter, which is obviously wrong.
> > >
> > > See its definition quoted here:
> > >> static inline struct ixgbe_adapter *ixgbe_from_netdev(struct
> > >> net_device *netdev) {
> > >>         struct ixgbe_netdevice_priv *priv = netdev_priv(netdev);
> > >>
> > >>         return priv->adapter;
> > >> }
> > >>
> > >
> > > Whats odd is that the netdev priv structure is just a wrapper around
> > > a pointer to the adapter:
> > >
> > >> struct ixgbe_netdevice_priv {
> > >>         struct ixgbe_adapter *adapter; };
> > >
> > >
> > >> -	return copy_to_user(ifr->ifr_data, config,
> > >> -			    sizeof(*config)) ? -EFAULT : 0;
> > >> +	*config = adapter->tstamp_config;
> > >> +
> > >> +	return 0;
> > >>  }
> > >
> > > Because we're completely pointing to the wrong memory, this overwrites
> > > who knows what since the ixgbe_netdev_priv is just the pointer address.
> > >
> > This is an artifact of the work to refactor ixgbe to support devlink:
> >
> > Both netdev and devlink want a private structure allocated as a flexible
> > array member of their parent structure. They cannot both directly be
> > ice_adapter, so we chose to have devlink be ice_adapter, and netdev gets
> > the wrapper structure. I suspect the patches you wrote were based on a
> > tree before this refactor, and/or you just did not spot the refactor
> > happened.
> >
> > a0285236ab93 ("ixgbe: add initial devlink support") is where the change
> > took place, which merged relatively recently.
> >
> > @Tony, I think this is a pretty trivial fixup on your tree if you want
> > to handle it instead of forcing Vladimir to make a v2?
> >
> > its really just switching netdev_priv to ixgbe_from_netdev in these two
> > functions.
> 
> Ugh :-/ sorry for the trouble, and thanks for doing the hard work of
> characterizing this.
> 
> Indeed, my first conversion of ixgbe was in August 2023, as this commit
> can attest:
> https://github.com/vladimiroltean/linux/commit/0351ebf1eee3381ccfba9d3
> 1a924d1dd887a316f
> 
> At that time, Przemyslaw's commit fd5ef5203ce6 ("ixgbe: wrap
> netdev_priv() usage") didn't exist, and "struct ixgbe_adapter *adapter =
> netdev_priv(netdev);" was the de facto idiom in the driver, which I then
> replicated two more times, in the new ixgbe_ptp_hwtstamp_set() and
> ixgbe_ptp_hwtstamp_get() functions. Not only did I not notice that this
> change took place, but it also compiled just fine, making me completely
> unsuspecting...
> 
> Tony, let me know how you would like to proceed.

I think it's easiest for me to make the change.

Thanks,
Tony

Re: [PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 1688 bytes --]



On 6/30/2025 1:56 PM, Vladimir Oltean wrote:
> 
> Ugh :-/ sorry for the trouble, and thanks for doing the hard work of
> characterizing this.
> 

No worries.

> Indeed, my first conversion of ixgbe was in August 2023, as this commit
> can attest:
> https://github.com/vladimiroltean/linux/commit/0351ebf1eee3381ccfba9d31a924d1dd887a316f
> 
> At that time, Przemyslaw's commit fd5ef5203ce6 ("ixgbe: wrap
> netdev_priv() usage") didn't exist, and "struct ixgbe_adapter *adapter =
> netdev_priv(netdev);" was the de facto idiom in the driver, which I then
> replicated two more times, in the new ixgbe_ptp_hwtstamp_set() and
> ixgbe_ptp_hwtstamp_get() functions. Not only did I not notice that this
> change took place, but it also compiled just fine, making me completely
> unsuspecting...
> 

It would be nice if we had a mechanism to provide type safety for the
netdev_priv.. but with C I don't really know of a good way. If you
provide your own macro you can do type checks on that.. but I'm not sure
how else we could implement this to avoid the type issues.

The use of such a wrapper private structure is essentially required
either in devlink code or netdev code because both of the parent structs
want flexible array private members, so they can't both be ixgbe_adapter.

Of course validating with KASAN would have caught this faster.. I am
actually quite surprised that we manage to get into
ice_ptp_set_timestamp_mode() and invoke the register access functions
without immediately crashing.

At least its caught before this merged and caused memory corruption on
production systems!

> Tony, let me know how you would like to proceed.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

RE: [Intel-wired-lan] [PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Rinitha, SX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Vladimir Oltean
> Sent: 13 May 2025 15:42
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Keller, Jacob E <jacob.e.keller@intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Gomes, Vinicius <vinicius.gomes@intel.com>; Vadim Fedorenko <vadim.fedorenko@linux.dev>; Richard Cochran <richardcochran@gmail.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>
> New timestamping API was introduced in commit 66f7223039c0 ("net: add NDOs for configuring hardware timestamping") from kernel v6.6.
>
> It is time to convert the Intel ixgbe driver to the new API, so that timestamping configuration can be removed from the ndo_eth_ioctl() path completely.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  9 ++--
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +--  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c  | 42 +++++++++----------
> 3 files changed, 29 insertions(+), 28 deletions(-)
>

With the ixgbe_from_netdev() change

Tested-by: Rinitha S <sx.rinitha@intel.com> (A Contingent worker at Intel)