linux-kernel - Re: [PATCH net-next v5 2/3] net: stmmac: add Rx HWTS metadata to XDP receive pkt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d74d570c-3001-4c92-7516-eb20ecb479d7@redhat.com>
Date:   Fri, 14 Apr 2023 18:38:42 +0200
From:   Jesper Dangaard Brouer <jbrouer@...hat.com>
To:     Song Yoong Siang <yoong.siang.song@...el.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Jose Abreu <joabreu@...opsys.com>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        Stanislav Fomichev <sdf@...gle.com>,
        Alexander Duyck <alexanderduyck@...com>,
        Ong Boon Leong <boon.leong.ong@...el.com>,
        Jacob Keller <jacob.e.keller@...el.com>
Cc:     brouer@...hat.com, netdev@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        bpf@...r.kernel.org, xdp-hints@...-project.net
Subject: Re: [PATCH net-next v5 2/3] net: stmmac: add Rx HWTS metadata to XDP
 receive pkt


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@...gle.com>
> Signed-off-by: Song Yoong Siang <yoong.siang.song@...el.com>
> Acked-by: Stanislav Fomichev <sdf@...gle.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 |