[xdp-hints] [PATCH net-next v4 0/3] XDP Rx HWTS metadata for stmmac driver

[xdp-hints] [PATCH net-next v4 0/3] XDP Rx HWTS metadata for stmmac driver

[Song Yoong Siang]

Implemented XDP receive hardware timestamp metadata for stmmac driver.

This patchset is tested with tools/testing/selftests/bpf/xdp_hw_metadata.
Below are the test steps and results.

Command on DUT:
	sudo ./xdp_hw_metadata <interface name>

Command on Link Partner:
	echo -n xdp | nc -u -q1 <destination IPv4 addr> 9091
	echo -n skb | nc -u -q1 <destination IPv4 addr> 9092

Result for port 9091:
	0x55fdb5f006d0: rx_desc[3]->addr=1000000003bd000 addr=3bd100 comp_addr=3bd000
	rx_timestamp: 1677762474360150047
	rx_hash: 0
	0x55fdb5f006d0: complete idx=515 addr=3bd000

Result for port 9092:
	found skb hwtstamp = 1677762476.320146161

Changes since v3:
 * directly retrieve Rx HWTS in stmmac_xdp_rx_timestamp(), instead of reuse
   stmmac_get_rx_hwtstamp()

Changes since v2:
 * To reduce packet processing cost, get the Rx HWTS only when xmo_rx_timestamp()
   is called

Changes since v1:
 * Add static to stmmac_xdp_metadata_ops declaration

---

Song Yoong Siang (3):
  net: stmmac: introduce wrapper for struct xdp_buff
  net: stmmac: add Rx HWTS metadata to XDP receive pkt
  net: stmmac: add Rx HWTS metadata to XDP ZC receive pkt

 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  7 ++
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 80 ++++++++++++++++---
 2 files changed, 77 insertions(+), 10 deletions(-)

-- 
2.34.1

[xdp-hints] [PATCH net-next v4 1/3] net: stmmac: introduce wrapper for struct xdp_buff

[Song Yoong Siang]

Introduce struct stmmac_xdp_buff as a preparation to support XDP Rx
metadata via kfuncs.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h   |  4 ++++
 .../net/ethernet/stmicro/stmmac/stmmac_main.c  | 18 +++++++++---------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 3d15e1e92e18..ac8ccf851708 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -92,6 +92,10 @@ struct stmmac_rx_buffer {
 	dma_addr_t sec_addr;
 };
 
+struct stmmac_xdp_buff {
+	struct xdp_buff xdp;
+};
+
 struct stmmac_rx_queue {
 	u32 rx_count_frames;
 	u32 queue_index;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d7fcab057032..6ffce52ca837 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5188,9 +5188,9 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 	int status = 0, coe = priv->hw->rx_csum;
 	unsigned int next_entry = rx_q->cur_rx;
 	enum dma_data_direction dma_dir;
+	struct stmmac_xdp_buff ctx = {};
 	unsigned int desc_size;
 	struct sk_buff *skb = NULL;
-	struct xdp_buff xdp;
 	int xdp_status = 0;
 	int buf_sz;
 
@@ -5311,17 +5311,17 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 			dma_sync_single_for_cpu(priv->device, buf->addr,
 						buf1_len, dma_dir);
 
-			xdp_init_buff(&xdp, buf_sz, &rx_q->xdp_rxq);
-			xdp_prepare_buff(&xdp, page_address(buf->page),
+			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);
 
-			pre_len = xdp.data_end - xdp.data_hard_start -
+			pre_len = ctx.xdp.data_end - ctx.xdp.data_hard_start -
 				  buf->page_offset;
-			skb = stmmac_xdp_run_prog(priv, &xdp);
+			skb = stmmac_xdp_run_prog(priv, &ctx.xdp);
 			/* Due xdp_adjust_tail: DMA sync for_device
 			 * cover max len CPU touch
 			 */
-			sync_len = xdp.data_end - xdp.data_hard_start -
+			sync_len = ctx.xdp.data_end - ctx.xdp.data_hard_start -
 				   buf->page_offset;
 			sync_len = max(sync_len, pre_len);
 
@@ -5331,7 +5331,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 
 				if (xdp_res & STMMAC_XDP_CONSUMED) {
 					page_pool_put_page(rx_q->page_pool,
-							   virt_to_head_page(xdp.data),
+							   virt_to_head_page(ctx.xdp.data),
 							   sync_len, true);
 					buf->page = NULL;
 					priv->dev->stats.rx_dropped++;
@@ -5359,7 +5359,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 
 		if (!skb) {
 			/* XDP program may expand or reduce tail */
-			buf1_len = xdp.data_end - xdp.data;
+			buf1_len = ctx.xdp.data_end - ctx.xdp.data;
 
 			skb = napi_alloc_skb(&ch->rx_napi, buf1_len);
 			if (!skb) {
@@ -5369,7 +5369,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 			}
 
 			/* XDP program may adjust header */
-			skb_copy_to_linear_data(skb, xdp.data, buf1_len);
+			skb_copy_to_linear_data(skb, ctx.xdp.data, buf1_len);
 			skb_put(skb, buf1_len);
 
 			/* Data payload copied into SKB, page ready for recycle */
-- 
2.34.1

[xdp-hints] [PATCH net-next v4 2/3] net: stmmac: add Rx HWTS metadata to XDP receive pkt

[Song Yoong Siang]

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>
---
 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;
 };
 
 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 6ffce52ca837..831a3e22e0d8 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 |
-- 
2.34.1

[xdp-hints] [PATCH net-next v4 3/3] net: stmmac: add Rx HWTS metadata to XDP ZC receive pkt

[Song Yoong Siang]

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

Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 831a3e22e0d8..25ec39aa3bf0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1614,6 +1614,12 @@ static int stmmac_alloc_rx_buffers_zc(struct stmmac_priv *priv,
 	struct stmmac_rx_queue *rx_q = &dma_conf->rx_queue[queue];
 	int i;
 
+	/* struct stmmac_xdp_buff is using cb field (maximum size of 24 bytes)
+	 * in struct xdp_buff_xsk to stash driver specific information. Thus,
+	 * use this macro to make sure no size violations.
+	 */
+	XSK_CHECK_PRIV_TYPE(struct stmmac_xdp_buff);
+
 	for (i = 0; i < dma_conf->dma_rx_size; i++) {
 		struct stmmac_rx_buffer *buf;
 		dma_addr_t dma_addr;
@@ -4998,6 +5004,16 @@ static bool stmmac_rx_refill_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
 	return ret;
 }
 
+static struct stmmac_xdp_buff *xsk_buff_to_stmmac_ctx(struct xdp_buff *xdp)
+{
+	/* In XDP zero copy data path, xdp field in struct xdp_buff_xsk is used
+	 * to represent incoming packet, whereas cb field in the same structure
+	 * is used to store driver specific info. Thus, struct stmmac_xdp_buff
+	 * is laid on top of xdp and cb fields of struct xdp_buff_xsk.
+	 */
+	return (struct stmmac_xdp_buff *)xdp;
+}
+
 static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 {
 	struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue];
@@ -5027,6 +5043,7 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 	}
 	while (count < limit) {
 		struct stmmac_rx_buffer *buf;
+		struct stmmac_xdp_buff *ctx;
 		unsigned int buf1_len = 0;
 		struct dma_desc *np, *p;
 		int entry;
@@ -5112,6 +5129,11 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 			goto read_again;
 		}
 
+		ctx = xsk_buff_to_stmmac_ctx(buf->xdp);
+		ctx->priv = priv;
+		ctx->p = p;
+		ctx->np = np;
+
 		/* XDP ZC Frame only support primary buffers for now */
 		buf1_len = stmmac_rx_buf1_len(priv, p, status, len);
 		len += buf1_len;
-- 
2.34.1

[xdp-hints] Re: [PATCH net-next v4 2/3] net: stmmac: add Rx HWTS metadata to XDP receive pkt

[Stanislav Fomichev]

On Wed, Apr 12, 2023 at 8:26 PM Song Yoong Siang
<yoong.siang.song@intel.com> 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>

Conceptually looks good, thanks!
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;
>  };
>
>  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 6ffce52ca837..831a3e22e0d8 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 |
> --
> 2.34.1
>

[xdp-hints] Re: [PATCH net-next v4 1/3] net: stmmac: introduce wrapper for struct xdp_buff

[Jesper Dangaard Brouer]

On 13/04/2023 05.25, Song Yoong Siang wrote:
> Introduce struct stmmac_xdp_buff as a preparation to support XDP Rx
> metadata via kfuncs.
> 
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac.h   |  4 ++++
>   .../net/ethernet/stmicro/stmmac/stmmac_main.c  | 18 +++++++++---------
>   2 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index 3d15e1e92e18..ac8ccf851708 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -92,6 +92,10 @@ struct stmmac_rx_buffer {
>   	dma_addr_t sec_addr;
>   };
>   
> +struct stmmac_xdp_buff {
> +	struct xdp_buff xdp;
> +};
> +
>   struct stmmac_rx_queue {
>   	u32 rx_count_frames;
>   	u32 queue_index;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index d7fcab057032..6ffce52ca837 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5188,9 +5188,9 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>   	int status = 0, coe = priv->hw->rx_csum;
>   	unsigned int next_entry = rx_q->cur_rx;
>   	enum dma_data_direction dma_dir;
> +	struct stmmac_xdp_buff ctx = {};

This code trick {} will zero out the struct.

Is this done purpose and really needed?

On x86_64 this unfortunately generates an asm instruction: rep stos

A repeated store string operation, for zeroing out memory, which is
slow. (Because[1] it supports be suspended by an exception or interrupt,
which requires it to store/restore CPU flags).

[1] https://www.felixcloutier.com/x86/rep:repe:repz:repne:repnz#tbl-4-22


>   	unsigned int desc_size;
>   	struct sk_buff *skb = NULL;
> -	struct xdp_buff xdp;
>   	int xdp_status = 0;
>   	int buf_sz;
>   
> @@ -5311,17 +5311,17 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>   			dma_sync_single_for_cpu(priv->device, buf->addr,
>   						buf1_len, dma_dir);
>   
> -			xdp_init_buff(&xdp, buf_sz, &rx_q->xdp_rxq);
> -			xdp_prepare_buff(&xdp, page_address(buf->page),
> +			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);
>   
> -			pre_len = xdp.data_end - xdp.data_hard_start -
> +			pre_len = ctx.xdp.data_end - ctx.xdp.data_hard_start -
>   				  buf->page_offset;
> -			skb = stmmac_xdp_run_prog(priv, &xdp);
> +			skb = stmmac_xdp_run_prog(priv, &ctx.xdp);
>   			/* Due xdp_adjust_tail: DMA sync for_device
>   			 * cover max len CPU touch
>   			 */
> -			sync_len = xdp.data_end - xdp.data_hard_start -
> +			sync_len = ctx.xdp.data_end - ctx.xdp.data_hard_start -
>   				   buf->page_offset;
>   			sync_len = max(sync_len, pre_len);
>   
> @@ -5331,7 +5331,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>   
>   				if (xdp_res & STMMAC_XDP_CONSUMED) {
>   					page_pool_put_page(rx_q->page_pool,
> -							   virt_to_head_page(xdp.data),
> +							   virt_to_head_page(ctx.xdp.data),
>   							   sync_len, true);
>   					buf->page = NULL;
>   					priv->dev->stats.rx_dropped++;
> @@ -5359,7 +5359,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>   
>   		if (!skb) {
>   			/* XDP program may expand or reduce tail */
> -			buf1_len = xdp.data_end - xdp.data;
> +			buf1_len = ctx.xdp.data_end - ctx.xdp.data;
>   
>   			skb = napi_alloc_skb(&ch->rx_napi, buf1_len);
>   			if (!skb) {
> @@ -5369,7 +5369,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>   			}
>   
>   			/* XDP program may adjust header */
> -			skb_copy_to_linear_data(skb, xdp.data, buf1_len);
> +			skb_copy_to_linear_data(skb, ctx.xdp.data, buf1_len);
>   			skb_put(skb, buf1_len);
>   
>   			/* Data payload copied into SKB, page ready for recycle */

[xdp-hints] Re: [PATCH net-next v4 1/3] net: stmmac: introduce wrapper for struct xdp_buff

[Song, Yoong Siang]

On Friday, April 14, 2023 1:10 AM , Jesper Dangaard Brouer <jbrouer@redhat.com> wrote:
>On 13/04/2023 05.25, Song Yoong Siang wrote:
>> Introduce struct stmmac_xdp_buff as a preparation to support XDP Rx
>> metadata via kfuncs.
>>
>> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
>> Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/stmmac.h   |  4 ++++
>>   .../net/ethernet/stmicro/stmmac/stmmac_main.c  | 18 +++++++++---------
>>   2 files changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> index 3d15e1e92e18..ac8ccf851708 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> @@ -92,6 +92,10 @@ struct stmmac_rx_buffer {
>>   	dma_addr_t sec_addr;
>>   };
>>
>> +struct stmmac_xdp_buff {
>> +	struct xdp_buff xdp;
>> +};
>> +
>>   struct stmmac_rx_queue {
>>   	u32 rx_count_frames;
>>   	u32 queue_index;
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index d7fcab057032..6ffce52ca837 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -5188,9 +5188,9 @@ static int stmmac_rx(struct stmmac_priv *priv, int
>limit, u32 queue)
>>   	int status = 0, coe = priv->hw->rx_csum;
>>   	unsigned int next_entry = rx_q->cur_rx;
>>   	enum dma_data_direction dma_dir;
>> +	struct stmmac_xdp_buff ctx = {};
>
>This code trick {} will zero out the struct.
>
>Is this done purpose and really needed?
>
>On x86_64 this unfortunately generates an asm instruction: rep stos
>
>A repeated store string operation, for zeroing out memory, which is slow.
>(Because[1] it supports be suspended by an exception or interrupt, which requires
>it to store/restore CPU flags).
>
>[1] https://www.felixcloutier.com/x86/rep:repe:repz:repne:repnz#tbl-4-22

Thanks for the useful information and link. I will submit v5 to remove {}.
>
>
>>   	unsigned int desc_size;
>>   	struct sk_buff *skb = NULL;
>> -	struct xdp_buff xdp;
>>   	int xdp_status = 0;
>>   	int buf_sz;
>>
>> @@ -5311,17 +5311,17 @@ static int stmmac_rx(struct stmmac_priv *priv, int
>limit, u32 queue)
>>   			dma_sync_single_for_cpu(priv->device, buf->addr,
>>   						buf1_len, dma_dir);
>>
>> -			xdp_init_buff(&xdp, buf_sz, &rx_q->xdp_rxq);
>> -			xdp_prepare_buff(&xdp, page_address(buf->page),
>> +			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);
>>
>> -			pre_len = xdp.data_end - xdp.data_hard_start -
>> +			pre_len = ctx.xdp.data_end - ctx.xdp.data_hard_start -
>>   				  buf->page_offset;
>> -			skb = stmmac_xdp_run_prog(priv, &xdp);
>> +			skb = stmmac_xdp_run_prog(priv, &ctx.xdp);
>>   			/* Due xdp_adjust_tail: DMA sync for_device
>>   			 * cover max len CPU touch
>>   			 */
>> -			sync_len = xdp.data_end - xdp.data_hard_start -
>> +			sync_len = ctx.xdp.data_end - ctx.xdp.data_hard_start -
>>   				   buf->page_offset;
>>   			sync_len = max(sync_len, pre_len);
>>
>> @@ -5331,7 +5331,7 @@ static int stmmac_rx(struct stmmac_priv *priv,
>> int limit, u32 queue)
>>
>>   				if (xdp_res & STMMAC_XDP_CONSUMED) {
>>   					page_pool_put_page(rx_q->page_pool,
>> -
>virt_to_head_page(xdp.data),
>> +
>virt_to_head_page(ctx.xdp.data),
>>   							   sync_len, true);
>>   					buf->page = NULL;
>>   					priv->dev->stats.rx_dropped++;
>> @@ -5359,7 +5359,7 @@ static int stmmac_rx(struct stmmac_priv *priv,
>> int limit, u32 queue)
>>
>>   		if (!skb) {
>>   			/* XDP program may expand or reduce tail */
>> -			buf1_len = xdp.data_end - xdp.data;
>> +			buf1_len = ctx.xdp.data_end - ctx.xdp.data;
>>
>>   			skb = napi_alloc_skb(&ch->rx_napi, buf1_len);
>>   			if (!skb) {
>> @@ -5369,7 +5369,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int
>limit, u32 queue)
>>   			}
>>
>>   			/* XDP program may adjust header */
>> -			skb_copy_to_linear_data(skb, xdp.data, buf1_len);
>> +			skb_copy_to_linear_data(skb, ctx.xdp.data, buf1_len);
>>   			skb_put(skb, buf1_len);
>>
>>   			/* Data payload copied into SKB, page ready for recycle
>*/