[net-next,v5,2/3] net: stmmac: add Rx HWTS metadata to XDP receive pkt

  Add receive hardware timestamp metadata support via kfunc to XDP receive
packets.

Suggested-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
Acked-by: Stanislav Fomichev <sdf@google.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  3 ++
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 40 ++++++++++++++++++-
 2 files changed, 42 insertions(+), 1 deletion(-)
  

On 14/04/2023 07.26, Song Yoong Siang wrote:
> Add receive hardware timestamp metadata support via kfunc to XDP receive
> packets.
> 
> Suggested-by: Stanislav Fomichev <sdf@google.com>
> Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
> Acked-by: Stanislav Fomichev <sdf@google.com>
> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  3 ++
>   .../net/ethernet/stmicro/stmmac/stmmac_main.c | 40 ++++++++++++++++++-
>   2 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index ac8ccf851708..826ac0ec88c6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -94,6 +94,9 @@ struct stmmac_rx_buffer {
>   
>   struct stmmac_xdp_buff {
>   	struct xdp_buff xdp;
> +	struct stmmac_priv *priv;
> +	struct dma_desc *p;
> +	struct dma_desc *np;

Hmm, I don't like the naming of the descriptors as "p" and "np".
If you insist on this naming, at least we need comments describing that 
this is.

Does "desc" and "ndesc" make sense?  (where "n" means "next")

>   };
>   
>   struct stmmac_rx_queue {
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 10b9f8912bb2..74f78e5537a3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5313,10 +5313,15 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>   
>   			xdp_init_buff(&ctx.xdp, buf_sz, &rx_q->xdp_rxq);
>   			xdp_prepare_buff(&ctx.xdp, page_address(buf->page),
> -					 buf->page_offset, buf1_len, false);
> +					 buf->page_offset, buf1_len, true);
>   
>   			pre_len = ctx.xdp.data_end - ctx.xdp.data_hard_start -
>   				  buf->page_offset;
> +
> +			ctx.priv = priv;
> +			ctx.p = p;
> +			ctx.np = np;
> +
>   			skb = stmmac_xdp_run_prog(priv, &ctx.xdp);
>   			/* Due xdp_adjust_tail: DMA sync for_device
>   			 * cover max len CPU touch
> @@ -7060,6 +7065,37 @@ void stmmac_fpe_handshake(struct stmmac_priv *priv, bool enable)
>   	}
>   }
>   
> +static int stmmac_xdp_rx_timestamp(const struct xdp_md *_ctx, u64 *timestamp)
> +{
> +	const struct stmmac_xdp_buff *ctx = (void *)_ctx;
> +	struct stmmac_priv *priv = ctx->priv;
> +	struct dma_desc *desc = ctx->p;
> +	struct dma_desc *np = ctx->np;
> +	struct dma_desc *p = ctx->p;
> +	u64 ns = 0;
> +
> +	if (!priv->hwts_rx_en)
> +		return -ENODATA;
> +
> +	/* For GMAC4, the valid timestamp is from CTX next desc. */
> +	if (priv->plat->has_gmac4 || priv->plat->has_xgmac)
> +		desc = np;
> +
> +	/* Check if timestamp is available */
> +	if (stmmac_get_rx_timestamp_status(priv, p, np, priv->adv_ts)) {
> +		stmmac_get_timestamp(priv, desc, priv->adv_ts, &ns);
> +		ns -= priv->plat->cdc_error_adj;
> +		*timestamp = ns_to_ktime(ns);
> +		return 0;
> +	}
> +
> +	return -ENODATA;
> +}
> +
> +static const struct xdp_metadata_ops stmmac_xdp_metadata_ops = {
> +	.xmo_rx_timestamp		= stmmac_xdp_rx_timestamp,
> +};
> +
>   /**
>    * stmmac_dvr_probe
>    * @device: device pointer
> @@ -7167,6 +7203,8 @@ int stmmac_dvr_probe(struct device *device,
>   
>   	ndev->netdev_ops = &stmmac_netdev_ops;
>   
> +	ndev->xdp_metadata_ops = &stmmac_xdp_metadata_ops;
> +
>   	ndev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
>   			    NETIF_F_RXCSUM;
>   	ndev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
  

On Saturday, April 15, 2023 12:39 AM, Jesper Dangaard Brouer <jbrouer@redhat.com> wrote:
>On 14/04/2023 07.26, Song Yoong Siang wrote:
>> Add receive hardware timestamp metadata support via kfunc to XDP
>> receive packets.
>>
>> Suggested-by: Stanislav Fomichev <sdf@google.com>
>> Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
>> Acked-by: Stanislav Fomichev <sdf@google.com>
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  3 ++
>>   .../net/ethernet/stmicro/stmmac/stmmac_main.c | 40 ++++++++++++++++++-
>>   2 files changed, 42 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> index ac8ccf851708..826ac0ec88c6 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> @@ -94,6 +94,9 @@ struct stmmac_rx_buffer {
>>
>>   struct stmmac_xdp_buff {
>>   	struct xdp_buff xdp;
>> +	struct stmmac_priv *priv;
>> +	struct dma_desc *p;
>> +	struct dma_desc *np;
>
>Hmm, I don't like the naming of the descriptors as "p" and "np".
>If you insist on this naming, at least we need comments describing that this is.
>
>Does "desc" and "ndesc" make sense?  (where "n" means "next")
>

Yup, make sense to have desc and ndesc. I will update the naming in V6.

Thanks & Regards
Siang

>>   };
>>
>>   struct stmmac_rx_queue {
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 10b9f8912bb2..74f78e5537a3 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -5313,10 +5313,15 @@ static int stmmac_rx(struct stmmac_priv *priv,
>> int limit, u32 queue)
>>
>>   			xdp_init_buff(&ctx.xdp, buf_sz, &rx_q->xdp_rxq);
>>   			xdp_prepare_buff(&ctx.xdp, page_address(buf->page),
>> -					 buf->page_offset, buf1_len, false);
>> +					 buf->page_offset, buf1_len, true);
>>
>>   			pre_len = ctx.xdp.data_end - ctx.xdp.data_hard_start -
>>   				  buf->page_offset;
>> +
>> +			ctx.priv = priv;
>> +			ctx.p = p;
>> +			ctx.np = np;
>> +
>>   			skb = stmmac_xdp_run_prog(priv, &ctx.xdp);
>>   			/* Due xdp_adjust_tail: DMA sync for_device
>>   			 * cover max len CPU touch
>> @@ -7060,6 +7065,37 @@ void stmmac_fpe_handshake(struct stmmac_priv *priv, bool enable)
>>   	}
>>   }
>>
>> +static int stmmac_xdp_rx_timestamp(const struct xdp_md *_ctx, u64
>> +*timestamp) {
>> +	const struct stmmac_xdp_buff *ctx = (void *)_ctx;
>> +	struct stmmac_priv *priv = ctx->priv;
>> +	struct dma_desc *desc = ctx->p;
>> +	struct dma_desc *np = ctx->np;
>> +	struct dma_desc *p = ctx->p;
>> +	u64 ns = 0;
>> +
>> +	if (!priv->hwts_rx_en)
>> +		return -ENODATA;
>> +
>> +	/* For GMAC4, the valid timestamp is from CTX next desc. */
>> +	if (priv->plat->has_gmac4 || priv->plat->has_xgmac)
>> +		desc = np;
>> +
>> +	/* Check if timestamp is available */
>> +	if (stmmac_get_rx_timestamp_status(priv, p, np, priv->adv_ts)) {
>> +		stmmac_get_timestamp(priv, desc, priv->adv_ts, &ns);
>> +		ns -= priv->plat->cdc_error_adj;
>> +		*timestamp = ns_to_ktime(ns);
>> +		return 0;
>> +	}
>> +
>> +	return -ENODATA;
>> +}
>> +
>> +static const struct xdp_metadata_ops stmmac_xdp_metadata_ops = {
>> +	.xmo_rx_timestamp		= stmmac_xdp_rx_timestamp,
>> +};
>> +
>>   /**
>>    * stmmac_dvr_probe
>>    * @device: device pointer
>> @@ -7167,6 +7203,8 @@ int stmmac_dvr_probe(struct device *device,
>>
>>   	ndev->netdev_ops = &stmmac_netdev_ops;
>>
>> +	ndev->xdp_metadata_ops = &stmmac_xdp_metadata_ops;
>> +
>>   	ndev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
>>   			    NETIF_F_RXCSUM;
>>   	ndev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |