Re: [PATCH v3 2/6] net/intel: add read clock feature in ICE

[Bruce Richardson <bruce.richardson@intel.com>]

On Sun, Jun 08, 2025 at 11:32:19AM +0000, Soumyadeep Hore wrote:
> Adding eth_ice_read_clock() feature to get current time
> for scheduling Packets based on Tx time.
> 
> Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
> ---
>  drivers/net/intel/ice/ice_ethdev.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
> index 7cc083ca32..9478ba92df 100644
> --- a/drivers/net/intel/ice/ice_ethdev.c
> +++ b/drivers/net/intel/ice/ice_ethdev.c
> @@ -187,6 +187,7 @@ static int ice_timesync_read_time(struct rte_eth_dev *dev,
>  static int ice_timesync_write_time(struct rte_eth_dev *dev,
>  				   const struct timespec *timestamp);
>  static int ice_timesync_disable(struct rte_eth_dev *dev);
> +static int eth_ice_read_clock(struct rte_eth_dev *dev, uint64_t *clock);
>  static int ice_fec_get_capability(struct rte_eth_dev *dev, struct rte_eth_fec_capa *speed_fec_capa,
>  			   unsigned int num);
>  static int ice_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa);
> @@ -317,6 +318,7 @@ static const struct eth_dev_ops ice_eth_dev_ops = {
>  	.timesync_read_time           = ice_timesync_read_time,
>  	.timesync_write_time          = ice_timesync_write_time,
>  	.timesync_disable             = ice_timesync_disable,
> +	.read_clock					  = eth_ice_read_clock,
>  	.tm_ops_get                   = ice_tm_ops_get,
>  	.fec_get_capability           = ice_fec_get_capability,
>  	.fec_get                      = ice_fec_get,
> @@ -6935,6 +6937,17 @@ ice_timesync_disable(struct rte_eth_dev *dev)
>  	return 0;
>  }
>  
> +static int
> +eth_ice_read_clock(__rte_unused struct rte_eth_dev *dev, uint64_t *clock)
> +{
> +	struct timespec system_time;
> +
> +	clock_gettime(CLOCK_REALTIME, &system_time);
> +	*clock = system_time.tv_sec * NSEC_PER_SEC + system_time.tv_nsec;
> +
> +	return 0;
> +}
> +

I see a number of problems here:

* The "read_clock" API is for reading the time from an ethernet device.
  This is not the same a reading the system time and returning that.
* This uses a potential system call to read the time. (On some linux
  implementations I think it will avoid a system call, but even so, we
  should not add what may be a system call into these fast-path APIs)

/Bruce