Re: [PATCH RFC net-next v1 4/6] net/mlx5e: Implement ethtool hardware timestamping statistics

["Köry Maincent" <kory.maincent@bootlin.com>]

On Fri, 23 Feb 2024 11:24:48 -0800
Rahul Rameshbabu <rrameshbabu@nvidia.com> wrote:

> Feed driver statistics counters related to hardware timestamping to
> standardized ethtool hardware timestamping statistics group.
> 
> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c index
> bc31196d348a..836198445726 100644 ---
> a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c +++
> b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c @@ -1155,6 +1155,74 @@
> void mlx5e_stats_rmon_get(struct mlx5e_priv *priv, *ranges =
> mlx5e_rmon_ranges; }
>  
> +void mlx5e_stats_ts_get(struct mlx5e_priv *priv,
> +			struct ethtool_ts_stats *ts_stats)
> +{
> +	enum ethtool_ts_stats_layer layer;
> +	struct mlx5e_ptp *ptp;
> +	bool tx_ptp_opened;
> +	int i, j;
> +
> +	mutex_lock(&priv->state_lock);
> +
> +	tx_ptp_opened = priv->tx_ptp_opened;
> +
> +	/* NOTE: this needs to be changed whenever ethtool timestamping
> +	 * layer selection is implemented.
> +	 */
> +	if (ts_stats->layer == ETHTOOL_TS_STATS_LAYER_ACTIVE)
> +		layer = tx_ptp_opened ? ETHTOOL_TS_STATS_LAYER_PHY :
> +					ETHTOOL_TS_STATS_LAYER_DMA;
> +	else
> +		layer = ts_stats->layer;
> +
> +	switch (layer) {
> +	case ETHTOOL_TS_STATS_LAYER_PHY:
> +		if (!tx_ptp_opened)
> +			return;
> +
> +		ptp = priv->channels.ptp;
> +
> +		ts_stats->pkts = 0;
> +		ts_stats->err = 0;
> +		ts_stats->late = 0;
> +		ts_stats->lost = 0;
> +
> +		/* Aggregate stats across all TCs */
> +		for (i = 0; i < ptp->num_tc; i++) {
> +			struct mlx5e_ptp_cq_stats *stats =
> ptp->ptpsq[i].cq_stats; +
> +			ts_stats->pkts += stats->cqe;
> +			ts_stats->err += stats->abort + stats->err_cqe;
> +			ts_stats->late += stats->late_cqe;
> +			ts_stats->lost += stats->lost_cqe;
> +		}
> +		break;
> +	case ETHTOOL_TS_STATS_LAYER_DMA:
> +		/* DMA layer will always successfully timestamp packets.
> Other
> +		 * counters do not make sense for this layer.
> +		 */
> +		ts_stats->pkts = 0;
> +
> +		/* Aggregate stats across all SQs */
> +		mutex_lock(&priv->state_lock);
> +		for (j = 0; j < priv->channels.num; j++) {
> +			struct mlx5e_channel *c = priv->channels.c[j];
> +
> +			for (i = 0; i < c->num_tc; i++) {
> +				struct mlx5e_sq_stats *stats =
> c->sq[i].stats; +
> +				ts_stats->pkts += stats->timestamps;
> +			}
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	mutex_unlock(&priv->state_lock);
> +}
> +

To follow the same logic as my patch series you should use phc qualifier instead
of the layer. See patch 9 of my series.
With HWTSTAMP_PROVIDER_QUALIFIER_PRECISE for the IEEE 1588 which mean the PHY
layer on your case and HWTSTAMP_PROVIDER_QUALIFIER_APPROX for the DMA layer.

Even if the timestamp is made physically on the PHY, this driver does not
register any phy device. The NIC manages all the network architecture by
itself. We decided to use the phc qualifier to fit this use case.

The layer description should only be used internally in the kernel when we are
registering a PHY device and using the phy tsinfo/hwtstamp/rxtstamp/tstsamp
callbacks.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com