[PATCH net 0/3] Add native mode XDP support

[PATCH net 0/3] Add native mode XDP support

[Meghana Malladi]

This series adds native XDP support using page_pool.
XDP zero copy support is not included in this patch series.

Patch 1/3: Replaces skb with page pool for Rx buffer allocation
Patch 2/3: Adds prueth_swdata struct for SWDATA for all swdata cases
Patch 3/3: Introduces native mode XDP support

Roger Quadros (3):
  net: ti: icssg-prueth: Use page_pool API for RX buffer allocation
  net: ti: icssg-prueth: introduce and use prueth_swdata struct for
    SWDATA
  net: ti: icssg-prueth: Add AF_XDP support

 drivers/net/ethernet/ti/Kconfig               |   1 +
 drivers/net/ethernet/ti/icssg/icssg_common.c  | 435 ++++++++++++++----
 drivers/net/ethernet/ti/icssg/icssg_config.h  |   2 +-
 drivers/net/ethernet/ti/icssg/icssg_prueth.c  | 124 ++++-
 drivers/net/ethernet/ti/icssg/icssg_prueth.h  |  51 +-
 .../net/ethernet/ti/icssg/icssg_prueth_sr1.c  |  23 +-
 6 files changed, 543 insertions(+), 93 deletions(-)


base-commit: 49afc040f4d707a4149a05180edc42bc590641a4
-- 
2.25.1

[PATCH net 1/3] net: ti: icssg-prueth: Use page_pool API for RX buffer allocation

[Meghana Malladi]

From: Roger Quadros <rogerq@kernel.org>

This is to prepare for native XDP support.

The page pool API is more faster in allocating pages than
__alloc_skb(). Drawback is that it works at PAGE_SIZE granularity
so we are not efficient in memory usage.
i.e. we are using PAGE_SIZE (4KB) memory for 1.5KB max packet size.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/net/ethernet/ti/Kconfig               |   1 +
 drivers/net/ethernet/ti/icssg/icssg_common.c  | 180 ++++++++++++------
 drivers/net/ethernet/ti/icssg/icssg_prueth.h  |  14 +-
 .../net/ethernet/ti/icssg/icssg_prueth_sr1.c  |  21 +-
 4 files changed, 146 insertions(+), 70 deletions(-)

diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 0d5a862cd78a..b461281d31b6 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -204,6 +204,7 @@ config TI_ICSSG_PRUETH_SR1
 	select PHYLIB
 	select TI_ICSS_IEP
 	select TI_K3_CPPI_DESC_POOL
+	select PAGE_POOL
 	depends on PRU_REMOTEPROC
 	depends on NET_SWITCHDEV
 	depends on ARCH_K3 && OF && TI_K3_UDMA_GLUE_LAYER
diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index 74f0f200a89d..313667ce24c3 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -45,6 +45,11 @@ void prueth_cleanup_rx_chns(struct prueth_emac *emac,
 			    struct prueth_rx_chn *rx_chn,
 			    int max_rflows)
 {
+	if (rx_chn->pg_pool) {
+		page_pool_destroy(rx_chn->pg_pool);
+		rx_chn->pg_pool = NULL;
+	}
+
 	if (rx_chn->desc_pool)
 		k3_cppi_desc_pool_destroy(rx_chn->desc_pool);
 
@@ -461,17 +466,17 @@ int prueth_init_rx_chns(struct prueth_emac *emac,
 }
 EXPORT_SYMBOL_GPL(prueth_init_rx_chns);
 
-int prueth_dma_rx_push(struct prueth_emac *emac,
-		       struct sk_buff *skb,
-		       struct prueth_rx_chn *rx_chn)
+int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
+			      struct prueth_rx_chn *rx_chn,
+			      struct page *page, u32 buf_len)
 {
 	struct net_device *ndev = emac->ndev;
 	struct cppi5_host_desc_t *desc_rx;
-	u32 pkt_len = skb_tailroom(skb);
 	dma_addr_t desc_dma;
 	dma_addr_t buf_dma;
 	void **swdata;
 
+	buf_dma = page_pool_get_dma_addr(page) + PRUETH_HEADROOM;
 	desc_rx = k3_cppi_desc_pool_alloc(rx_chn->desc_pool);
 	if (!desc_rx) {
 		netdev_err(ndev, "rx push: failed to allocate descriptor\n");
@@ -479,25 +484,18 @@ int prueth_dma_rx_push(struct prueth_emac *emac,
 	}
 	desc_dma = k3_cppi_desc_pool_virt2dma(rx_chn->desc_pool, desc_rx);
 
-	buf_dma = dma_map_single(rx_chn->dma_dev, skb->data, pkt_len, DMA_FROM_DEVICE);
-	if (unlikely(dma_mapping_error(rx_chn->dma_dev, buf_dma))) {
-		k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
-		netdev_err(ndev, "rx push: failed to map rx pkt buffer\n");
-		return -EINVAL;
-	}
-
 	cppi5_hdesc_init(desc_rx, CPPI5_INFO0_HDESC_EPIB_PRESENT,
 			 PRUETH_NAV_PS_DATA_SIZE);
 	k3_udma_glue_rx_dma_to_cppi5_addr(rx_chn->rx_chn, &buf_dma);
-	cppi5_hdesc_attach_buf(desc_rx, buf_dma, skb_tailroom(skb), buf_dma, skb_tailroom(skb));
+	cppi5_hdesc_attach_buf(desc_rx, buf_dma, buf_len, buf_dma, buf_len);
 
 	swdata = cppi5_hdesc_get_swdata(desc_rx);
-	*swdata = skb;
+	*swdata = page;
 
-	return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, 0,
+	return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, PRUETH_RX_FLOW_DATA,
 					desc_rx, desc_dma);
 }
-EXPORT_SYMBOL_GPL(prueth_dma_rx_push);
+EXPORT_SYMBOL_GPL(prueth_dma_rx_push_mapped);
 
 u64 icssg_ts_to_ns(u32 hi_sw, u32 hi, u32 lo, u32 cycle_time_ns)
 {
@@ -535,18 +533,31 @@ void emac_rx_timestamp(struct prueth_emac *emac,
 	ssh->hwtstamp = ns_to_ktime(ns);
 }
 
+unsigned int prueth_rxbuf_total_len(unsigned int len)
+{
+	len += PRUETH_HEADROOM;
+	len += SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+
+	return SKB_DATA_ALIGN(len);
+}
+EXPORT_SYMBOL_GPL(prueth_rxbuf_total_len);
+
 static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
 {
 	struct prueth_rx_chn *rx_chn = &emac->rx_chns;
 	u32 buf_dma_len, pkt_len, port_id = 0;
 	struct net_device *ndev = emac->ndev;
 	struct cppi5_host_desc_t *desc_rx;
-	struct sk_buff *skb, *new_skb;
 	dma_addr_t desc_dma, buf_dma;
+	struct page *page, *new_page;
+	struct page_pool *pool;
+	struct sk_buff *skb;
 	void **swdata;
 	u32 *psdata;
+	void *pa;
 	int ret;
 
+	pool = rx_chn->pg_pool;
 	ret = k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, flow_id, &desc_dma);
 	if (ret) {
 		if (ret != -ENODATA)
@@ -558,14 +569,8 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
 		return 0;
 
 	desc_rx = k3_cppi_desc_pool_dma2virt(rx_chn->desc_pool, desc_dma);
-
 	swdata = cppi5_hdesc_get_swdata(desc_rx);
-	skb = *swdata;
-
-	psdata = cppi5_hdesc_get_psdata(desc_rx);
-	/* RX HW timestamp */
-	if (emac->rx_ts_enabled)
-		emac_rx_timestamp(emac, skb, psdata);
+	page = *swdata;
 
 	cppi5_hdesc_get_obuf(desc_rx, &buf_dma, &buf_dma_len);
 	k3_udma_glue_rx_cppi5_to_dma_addr(rx_chn->rx_chn, &buf_dma);
@@ -574,32 +579,50 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
 	pkt_len -= 4;
 	cppi5_desc_get_tags_ids(&desc_rx->hdr, &port_id, NULL);
 
-	dma_unmap_single(rx_chn->dma_dev, buf_dma, buf_dma_len, DMA_FROM_DEVICE);
 	k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
 
-	skb->dev = ndev;
-	new_skb = netdev_alloc_skb_ip_align(ndev, PRUETH_MAX_PKT_SIZE);
 	/* if allocation fails we drop the packet but push the
-	 * descriptor back to the ring with old skb to prevent a stall
+	 * descriptor back to the ring with old page to prevent a stall
 	 */
-	if (!new_skb) {
+	new_page = page_pool_dev_alloc_pages(pool);
+	if (unlikely(!new_page)) {
+		new_page = page;
 		ndev->stats.rx_dropped++;
-		new_skb = skb;
-	} else {
-		/* send the filled skb up the n/w stack */
-		skb_put(skb, pkt_len);
-		if (emac->prueth->is_switch_mode)
-			skb->offload_fwd_mark = emac->offload_fwd_mark;
-		skb->protocol = eth_type_trans(skb, ndev);
-		napi_gro_receive(&emac->napi_rx, skb);
-		ndev->stats.rx_bytes += pkt_len;
-		ndev->stats.rx_packets++;
+		goto requeue;
 	}
 
+	/* prepare skb and send to n/w stack */
+	pa = page_address(page);
+	skb = build_skb(pa, prueth_rxbuf_total_len(pkt_len));
+	if (!skb) {
+		ndev->stats.rx_dropped++;
+		page_pool_recycle_direct(pool, page);
+		goto requeue;
+	}
+
+	skb_reserve(skb, PRUETH_HEADROOM);
+	skb_put(skb, pkt_len);
+	skb->dev = ndev;
+
+	psdata = cppi5_hdesc_get_psdata(desc_rx);
+	/* RX HW timestamp */
+	if (emac->rx_ts_enabled)
+		emac_rx_timestamp(emac, skb, psdata);
+
+	if (emac->prueth->is_switch_mode)
+		skb->offload_fwd_mark = emac->offload_fwd_mark;
+	skb->protocol = eth_type_trans(skb, ndev);
+
+	netif_receive_skb(skb);
+	ndev->stats.rx_bytes += pkt_len;
+	ndev->stats.rx_packets++;
+
+requeue:
 	/* queue another RX DMA */
-	ret = prueth_dma_rx_push(emac, new_skb, &emac->rx_chns);
+	ret = prueth_dma_rx_push_mapped(emac, &emac->rx_chns, new_page,
+					PRUETH_MAX_PKT_SIZE);
 	if (WARN_ON(ret < 0)) {
-		dev_kfree_skb_any(new_skb);
+		page_pool_recycle_direct(pool, new_page);
 		ndev->stats.rx_errors++;
 		ndev->stats.rx_dropped++;
 	}
@@ -611,22 +634,17 @@ static void prueth_rx_cleanup(void *data, dma_addr_t desc_dma)
 {
 	struct prueth_rx_chn *rx_chn = data;
 	struct cppi5_host_desc_t *desc_rx;
-	struct sk_buff *skb;
-	dma_addr_t buf_dma;
-	u32 buf_dma_len;
+	struct page_pool *pool;
+	struct page *page;
 	void **swdata;
 
+	pool = rx_chn->pg_pool;
+
 	desc_rx = k3_cppi_desc_pool_dma2virt(rx_chn->desc_pool, desc_dma);
 	swdata = cppi5_hdesc_get_swdata(desc_rx);
-	skb = *swdata;
-	cppi5_hdesc_get_obuf(desc_rx, &buf_dma, &buf_dma_len);
-	k3_udma_glue_rx_cppi5_to_dma_addr(rx_chn->rx_chn, &buf_dma);
-
-	dma_unmap_single(rx_chn->dma_dev, buf_dma, buf_dma_len,
-			 DMA_FROM_DEVICE);
+	page = *swdata;
+	page_pool_recycle_direct(pool, page);
 	k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
-
-	dev_kfree_skb_any(skb);
 }
 
 static int prueth_tx_ts_cookie_get(struct prueth_emac *emac)
@@ -907,29 +925,70 @@ int icssg_napi_rx_poll(struct napi_struct *napi_rx, int budget)
 }
 EXPORT_SYMBOL_GPL(icssg_napi_rx_poll);
 
+static struct page_pool *prueth_create_page_pool(struct prueth_emac *emac,
+						 struct device *dma_dev,
+						 int size)
+{
+	struct page_pool_params pp_params = { 0 };
+	struct page_pool *pool;
+
+	pp_params.order = 0;
+	pp_params.flags = PP_FLAG_DMA_MAP;
+	pp_params.pool_size = size;
+	pp_params.nid = dev_to_node(emac->prueth->dev);
+	pp_params.dma_dir = DMA_BIDIRECTIONAL;
+	pp_params.dev = dma_dev;
+	pp_params.napi = &emac->napi_rx;
+
+	pool = page_pool_create(&pp_params);
+	if (IS_ERR(pool))
+		netdev_err(emac->ndev, "cannot create rx page pool\n");
+
+	return pool;
+}
+
 int prueth_prepare_rx_chan(struct prueth_emac *emac,
 			   struct prueth_rx_chn *chn,
 			   int buf_size)
 {
-	struct sk_buff *skb;
+	struct page_pool *pool;
+	struct page *page;
 	int i, ret;
 
+	pool = prueth_create_page_pool(emac, chn->dma_dev, chn->descs_num);
+	if (IS_ERR(pool))
+		return PTR_ERR(pool);
+
+	chn->pg_pool = pool;
+
 	for (i = 0; i < chn->descs_num; i++) {
-		skb = __netdev_alloc_skb_ip_align(NULL, buf_size, GFP_KERNEL);
-		if (!skb)
-			return -ENOMEM;
+		/* NOTE: we're not using memory efficiently here.
+		 * 1 full page (4KB?) used here instead of
+		 * PRUETH_MAX_PKT_SIZE (~1.5KB?)
+		 */
+		page = page_pool_dev_alloc_pages(pool);
+		if (!page) {
+			netdev_err(emac->ndev, "couldn't allocate rx page\n");
+			ret = -ENOMEM;
+			goto recycle_alloc_pg;
+		}
 
-		ret = prueth_dma_rx_push(emac, skb, chn);
+		ret = prueth_dma_rx_push_mapped(emac, chn, page, buf_size);
 		if (ret < 0) {
 			netdev_err(emac->ndev,
-				   "cannot submit skb for rx chan %s ret %d\n",
+				   "cannot submit page for rx chan %s ret %d\n",
 				   chn->name, ret);
-			kfree_skb(skb);
-			return ret;
+			page_pool_recycle_direct(pool, page);
+			goto recycle_alloc_pg;
 		}
 	}
 
 	return 0;
+
+recycle_alloc_pg:
+	prueth_reset_rx_chan(&emac->rx_chns, PRUETH_MAX_RX_FLOWS, false);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(prueth_prepare_rx_chan);
 
@@ -958,6 +1017,9 @@ void prueth_reset_rx_chan(struct prueth_rx_chn *chn,
 					  prueth_rx_cleanup, !!i);
 	if (disable)
 		k3_udma_glue_disable_rx_chn(chn->rx_chn);
+
+	page_pool_destroy(chn->pg_pool);
+	chn->pg_pool = NULL;
 }
 EXPORT_SYMBOL_GPL(prueth_reset_rx_chan);
 
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index 5473315ea204..62f3d04af222 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -33,6 +33,8 @@
 #include <linux/dma/k3-udma-glue.h>
 
 #include <net/devlink.h>
+#include <net/xdp.h>
+#include <net/page_pool/helpers.h>
 
 #include "icssg_config.h"
 #include "icss_iep.h"
@@ -125,6 +127,7 @@ struct prueth_rx_chn {
 	u32 descs_num;
 	unsigned int irq[ICSSG_MAX_RFLOWS];	/* separate irq per flow */
 	char name[32];
+	struct page_pool *pg_pool;
 };
 
 /* There are 4 Tx DMA channels, but the highest priority is CH3 (thread 3)
@@ -202,6 +205,10 @@ struct prueth_emac {
 	unsigned long rx_pace_timeout_ns;
 };
 
+/* The buf includes headroom compatible with both skb and xdpf */
+#define PRUETH_HEADROOM_NA (max(XDP_PACKET_HEADROOM, NET_SKB_PAD) + NET_IP_ALIGN)
+#define PRUETH_HEADROOM  ALIGN(PRUETH_HEADROOM_NA, sizeof(long))
+
 /**
  * struct prueth_pdata - PRUeth platform data
  * @fdqring_mode: Free desc queue mode
@@ -402,9 +409,10 @@ int prueth_init_rx_chns(struct prueth_emac *emac,
 			struct prueth_rx_chn *rx_chn,
 			char *name, u32 max_rflows,
 			u32 max_desc_num);
-int prueth_dma_rx_push(struct prueth_emac *emac,
-		       struct sk_buff *skb,
-		       struct prueth_rx_chn *rx_chn);
+int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
+			      struct prueth_rx_chn *rx_chn,
+			      struct page *page, u32 buf_len);
+unsigned int prueth_rxbuf_total_len(unsigned int len);
 void emac_rx_timestamp(struct prueth_emac *emac,
 		       struct sk_buff *skb, u32 *psdata);
 enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
index 3dc86397c367..c2bc7169355a 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
@@ -274,10 +274,12 @@ static struct sk_buff *prueth_process_rx_mgm(struct prueth_emac *emac,
 	struct prueth_rx_chn *rx_chn = &emac->rx_mgm_chn;
 	struct net_device *ndev = emac->ndev;
 	struct cppi5_host_desc_t *desc_rx;
-	struct sk_buff *skb, *new_skb;
+	struct page *page, *new_page;
 	dma_addr_t desc_dma, buf_dma;
 	u32 buf_dma_len, pkt_len;
+	struct sk_buff *skb;
 	void **swdata;
+	void *pa;
 	int ret;
 
 	ret = k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, flow_id, &desc_dma);
@@ -299,32 +301,35 @@ static struct sk_buff *prueth_process_rx_mgm(struct prueth_emac *emac,
 	}
 
 	swdata = cppi5_hdesc_get_swdata(desc_rx);
-	skb = *swdata;
+	page = *swdata;
 	cppi5_hdesc_get_obuf(desc_rx, &buf_dma, &buf_dma_len);
 	pkt_len = cppi5_hdesc_get_pktlen(desc_rx);
 
 	dma_unmap_single(rx_chn->dma_dev, buf_dma, buf_dma_len, DMA_FROM_DEVICE);
 	k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
 
-	new_skb = netdev_alloc_skb_ip_align(ndev, PRUETH_MAX_PKT_SIZE);
+	new_page = page_pool_dev_alloc_pages(rx_chn->pg_pool);
 	/* if allocation fails we drop the packet but push the
 	 * descriptor back to the ring with old skb to prevent a stall
 	 */
-	if (!new_skb) {
+	if (!new_page) {
 		netdev_err(ndev,
-			   "skb alloc failed, dropped mgm pkt from flow %d\n",
+			   "page alloc failed, dropped mgm pkt from flow %d\n",
 			   flow_id);
-		new_skb = skb;
+		new_page = page;
 		skb = NULL;	/* return NULL */
 	} else {
 		/* return the filled skb */
+		pa = page_address(page);
+		skb = build_skb(pa, prueth_rxbuf_total_len(pkt_len));
 		skb_put(skb, pkt_len);
 	}
 
 	/* queue another DMA */
-	ret = prueth_dma_rx_push(emac, new_skb, &emac->rx_mgm_chn);
+	ret = prueth_dma_rx_push_mapped(emac, &emac->rx_chns, new_page,
+					PRUETH_MAX_PKT_SIZE);
 	if (WARN_ON(ret < 0))
-		dev_kfree_skb_any(new_skb);
+		page_pool_recycle_direct(rx_chn->pg_pool, new_page);
 
 	return skb;
 }
-- 
2.25.1

Re: [PATCH net 1/3] net: ti: icssg-prueth: Use page_pool API for RX buffer allocation

[Ido Schimmel]

On Wed, Jan 22, 2025 at 06:19:49PM +0530, Meghana Malladi wrote:
> From: Roger Quadros <rogerq@kernel.org>
> 
> This is to prepare for native XDP support.
> 
> The page pool API is more faster in allocating pages than
> __alloc_skb(). Drawback is that it works at PAGE_SIZE granularity
> so we are not efficient in memory usage.
> i.e. we are using PAGE_SIZE (4KB) memory for 1.5KB max packet size.
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>

Patch is missing your SoB

> ---
>  drivers/net/ethernet/ti/Kconfig               |   1 +
>  drivers/net/ethernet/ti/icssg/icssg_common.c  | 180 ++++++++++++------
>  drivers/net/ethernet/ti/icssg/icssg_prueth.h  |  14 +-
>  .../net/ethernet/ti/icssg/icssg_prueth_sr1.c  |  21 +-
>  4 files changed, 146 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
> index 0d5a862cd78a..b461281d31b6 100644
> --- a/drivers/net/ethernet/ti/Kconfig
> +++ b/drivers/net/ethernet/ti/Kconfig
> @@ -204,6 +204,7 @@ config TI_ICSSG_PRUETH_SR1
>  	select PHYLIB
>  	select TI_ICSS_IEP
>  	select TI_K3_CPPI_DESC_POOL
> +	select PAGE_POOL
>  	depends on PRU_REMOTEPROC
>  	depends on NET_SWITCHDEV
>  	depends on ARCH_K3 && OF && TI_K3_UDMA_GLUE_LAYER
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index 74f0f200a89d..313667ce24c3 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
> @@ -45,6 +45,11 @@ void prueth_cleanup_rx_chns(struct prueth_emac *emac,
>  			    struct prueth_rx_chn *rx_chn,
>  			    int max_rflows)
>  {
> +	if (rx_chn->pg_pool) {
> +		page_pool_destroy(rx_chn->pg_pool);
> +		rx_chn->pg_pool = NULL;
> +	}
> +
>  	if (rx_chn->desc_pool)
>  		k3_cppi_desc_pool_destroy(rx_chn->desc_pool);
>  
> @@ -461,17 +466,17 @@ int prueth_init_rx_chns(struct prueth_emac *emac,
>  }
>  EXPORT_SYMBOL_GPL(prueth_init_rx_chns);
>  
> -int prueth_dma_rx_push(struct prueth_emac *emac,
> -		       struct sk_buff *skb,
> -		       struct prueth_rx_chn *rx_chn)
> +int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
> +			      struct prueth_rx_chn *rx_chn,
> +			      struct page *page, u32 buf_len)
>  {
>  	struct net_device *ndev = emac->ndev;
>  	struct cppi5_host_desc_t *desc_rx;
> -	u32 pkt_len = skb_tailroom(skb);
>  	dma_addr_t desc_dma;
>  	dma_addr_t buf_dma;
>  	void **swdata;
>  
> +	buf_dma = page_pool_get_dma_addr(page) + PRUETH_HEADROOM;
>  	desc_rx = k3_cppi_desc_pool_alloc(rx_chn->desc_pool);
>  	if (!desc_rx) {
>  		netdev_err(ndev, "rx push: failed to allocate descriptor\n");
> @@ -479,25 +484,18 @@ int prueth_dma_rx_push(struct prueth_emac *emac,
>  	}
>  	desc_dma = k3_cppi_desc_pool_virt2dma(rx_chn->desc_pool, desc_rx);
>  
> -	buf_dma = dma_map_single(rx_chn->dma_dev, skb->data, pkt_len, DMA_FROM_DEVICE);
> -	if (unlikely(dma_mapping_error(rx_chn->dma_dev, buf_dma))) {
> -		k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
> -		netdev_err(ndev, "rx push: failed to map rx pkt buffer\n");
> -		return -EINVAL;
> -	}
> -
>  	cppi5_hdesc_init(desc_rx, CPPI5_INFO0_HDESC_EPIB_PRESENT,
>  			 PRUETH_NAV_PS_DATA_SIZE);
>  	k3_udma_glue_rx_dma_to_cppi5_addr(rx_chn->rx_chn, &buf_dma);
> -	cppi5_hdesc_attach_buf(desc_rx, buf_dma, skb_tailroom(skb), buf_dma, skb_tailroom(skb));
> +	cppi5_hdesc_attach_buf(desc_rx, buf_dma, buf_len, buf_dma, buf_len);
>  
>  	swdata = cppi5_hdesc_get_swdata(desc_rx);
> -	*swdata = skb;
> +	*swdata = page;
>  
> -	return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, 0,
> +	return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, PRUETH_RX_FLOW_DATA,
>  					desc_rx, desc_dma);
>  }
> -EXPORT_SYMBOL_GPL(prueth_dma_rx_push);
> +EXPORT_SYMBOL_GPL(prueth_dma_rx_push_mapped);
>  
>  u64 icssg_ts_to_ns(u32 hi_sw, u32 hi, u32 lo, u32 cycle_time_ns)
>  {
> @@ -535,18 +533,31 @@ void emac_rx_timestamp(struct prueth_emac *emac,
>  	ssh->hwtstamp = ns_to_ktime(ns);
>  }
>  
> +unsigned int prueth_rxbuf_total_len(unsigned int len)
> +{
> +	len += PRUETH_HEADROOM;
> +	len += SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> +
> +	return SKB_DATA_ALIGN(len);
> +}
> +EXPORT_SYMBOL_GPL(prueth_rxbuf_total_len);
> +
>  static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
>  {
>  	struct prueth_rx_chn *rx_chn = &emac->rx_chns;
>  	u32 buf_dma_len, pkt_len, port_id = 0;
>  	struct net_device *ndev = emac->ndev;
>  	struct cppi5_host_desc_t *desc_rx;
> -	struct sk_buff *skb, *new_skb;
>  	dma_addr_t desc_dma, buf_dma;
> +	struct page *page, *new_page;
> +	struct page_pool *pool;
> +	struct sk_buff *skb;
>  	void **swdata;
>  	u32 *psdata;
> +	void *pa;
>  	int ret;
>  
> +	pool = rx_chn->pg_pool;
>  	ret = k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, flow_id, &desc_dma);
>  	if (ret) {
>  		if (ret != -ENODATA)
> @@ -558,14 +569,8 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
>  		return 0;
>  
>  	desc_rx = k3_cppi_desc_pool_dma2virt(rx_chn->desc_pool, desc_dma);
> -
>  	swdata = cppi5_hdesc_get_swdata(desc_rx);
> -	skb = *swdata;
> -
> -	psdata = cppi5_hdesc_get_psdata(desc_rx);
> -	/* RX HW timestamp */
> -	if (emac->rx_ts_enabled)
> -		emac_rx_timestamp(emac, skb, psdata);
> +	page = *swdata;
>  
>  	cppi5_hdesc_get_obuf(desc_rx, &buf_dma, &buf_dma_len);
>  	k3_udma_glue_rx_cppi5_to_dma_addr(rx_chn->rx_chn, &buf_dma);
> @@ -574,32 +579,50 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
>  	pkt_len -= 4;
>  	cppi5_desc_get_tags_ids(&desc_rx->hdr, &port_id, NULL);
>  
> -	dma_unmap_single(rx_chn->dma_dev, buf_dma, buf_dma_len, DMA_FROM_DEVICE);
>  	k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
>  
> -	skb->dev = ndev;
> -	new_skb = netdev_alloc_skb_ip_align(ndev, PRUETH_MAX_PKT_SIZE);
>  	/* if allocation fails we drop the packet but push the
> -	 * descriptor back to the ring with old skb to prevent a stall
> +	 * descriptor back to the ring with old page to prevent a stall
>  	 */
> -	if (!new_skb) {
> +	new_page = page_pool_dev_alloc_pages(pool);

I don't see where the reference is released via a put call

> +	if (unlikely(!new_page)) {
> +		new_page = page;
>  		ndev->stats.rx_dropped++;
> -		new_skb = skb;
> -	} else {
> -		/* send the filled skb up the n/w stack */
> -		skb_put(skb, pkt_len);
> -		if (emac->prueth->is_switch_mode)
> -			skb->offload_fwd_mark = emac->offload_fwd_mark;
> -		skb->protocol = eth_type_trans(skb, ndev);
> -		napi_gro_receive(&emac->napi_rx, skb);
> -		ndev->stats.rx_bytes += pkt_len;
> -		ndev->stats.rx_packets++;
> +		goto requeue;
>  	}
>  
> +	/* prepare skb and send to n/w stack */
> +	pa = page_address(page);
> +	skb = build_skb(pa, prueth_rxbuf_total_len(pkt_len));

napi_build_skb()? See commit 53ee91427177 ("net/mlx5e: Switch to using
napi_build_skb()")

Also, I believe the frag size is incorrect. It is used to initialize
skb->truesize which should signal the size of the buffer that was
allocated for the packet (PAGE_SIZE in this case).

> +	if (!skb) {
> +		ndev->stats.rx_dropped++;
> +		page_pool_recycle_direct(pool, page);
> +		goto requeue;
> +	}
> +
> +	skb_reserve(skb, PRUETH_HEADROOM);
> +	skb_put(skb, pkt_len);
> +	skb->dev = ndev;
> +
> +	psdata = cppi5_hdesc_get_psdata(desc_rx);
> +	/* RX HW timestamp */
> +	if (emac->rx_ts_enabled)
> +		emac_rx_timestamp(emac, skb, psdata);
> +
> +	if (emac->prueth->is_switch_mode)
> +		skb->offload_fwd_mark = emac->offload_fwd_mark;
> +	skb->protocol = eth_type_trans(skb, ndev);

skb_mark_for_recycle() ?

> +
> +	netif_receive_skb(skb);

The code was previously using napi_gro_receive(), why give up on GRO
support?

> +	ndev->stats.rx_bytes += pkt_len;
> +	ndev->stats.rx_packets++;
> +
> +requeue:
>  	/* queue another RX DMA */
> -	ret = prueth_dma_rx_push(emac, new_skb, &emac->rx_chns);
> +	ret = prueth_dma_rx_push_mapped(emac, &emac->rx_chns, new_page,
> +					PRUETH_MAX_PKT_SIZE);
>  	if (WARN_ON(ret < 0)) {
> -		dev_kfree_skb_any(new_skb);
> +		page_pool_recycle_direct(pool, new_page);
>  		ndev->stats.rx_errors++;
>  		ndev->stats.rx_dropped++;
>  	}
> @@ -611,22 +634,17 @@ static void prueth_rx_cleanup(void *data, dma_addr_t desc_dma)
>  {
>  	struct prueth_rx_chn *rx_chn = data;
>  	struct cppi5_host_desc_t *desc_rx;
> -	struct sk_buff *skb;
> -	dma_addr_t buf_dma;
> -	u32 buf_dma_len;
> +	struct page_pool *pool;
> +	struct page *page;
>  	void **swdata;
>  
> +	pool = rx_chn->pg_pool;
> +
>  	desc_rx = k3_cppi_desc_pool_dma2virt(rx_chn->desc_pool, desc_dma);
>  	swdata = cppi5_hdesc_get_swdata(desc_rx);
> -	skb = *swdata;
> -	cppi5_hdesc_get_obuf(desc_rx, &buf_dma, &buf_dma_len);
> -	k3_udma_glue_rx_cppi5_to_dma_addr(rx_chn->rx_chn, &buf_dma);
> -
> -	dma_unmap_single(rx_chn->dma_dev, buf_dma, buf_dma_len,
> -			 DMA_FROM_DEVICE);
> +	page = *swdata;
> +	page_pool_recycle_direct(pool, page);
>  	k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
> -
> -	dev_kfree_skb_any(skb);
>  }
>  
>  static int prueth_tx_ts_cookie_get(struct prueth_emac *emac)
> @@ -907,29 +925,70 @@ int icssg_napi_rx_poll(struct napi_struct *napi_rx, int budget)
>  }
>  EXPORT_SYMBOL_GPL(icssg_napi_rx_poll);
>  
> +static struct page_pool *prueth_create_page_pool(struct prueth_emac *emac,
> +						 struct device *dma_dev,
> +						 int size)
> +{
> +	struct page_pool_params pp_params = { 0 };
> +	struct page_pool *pool;
> +
> +	pp_params.order = 0;
> +	pp_params.flags = PP_FLAG_DMA_MAP;

What about PP_FLAG_DMA_SYNC_DEV ? I don't see a sync for device call. I
also don't see a sync for CPU...

> +	pp_params.pool_size = size;
> +	pp_params.nid = dev_to_node(emac->prueth->dev);
> +	pp_params.dma_dir = DMA_BIDIRECTIONAL;
> +	pp_params.dev = dma_dev;
> +	pp_params.napi = &emac->napi_rx;
> +
> +	pool = page_pool_create(&pp_params);
> +	if (IS_ERR(pool))
> +		netdev_err(emac->ndev, "cannot create rx page pool\n");
> +
> +	return pool;
> +}
> +
>  int prueth_prepare_rx_chan(struct prueth_emac *emac,
>  			   struct prueth_rx_chn *chn,
>  			   int buf_size)
>  {
> -	struct sk_buff *skb;
> +	struct page_pool *pool;
> +	struct page *page;
>  	int i, ret;
>  
> +	pool = prueth_create_page_pool(emac, chn->dma_dev, chn->descs_num);
> +	if (IS_ERR(pool))
> +		return PTR_ERR(pool);
> +
> +	chn->pg_pool = pool;
> +
>  	for (i = 0; i < chn->descs_num; i++) {
> -		skb = __netdev_alloc_skb_ip_align(NULL, buf_size, GFP_KERNEL);
> -		if (!skb)
> -			return -ENOMEM;
> +		/* NOTE: we're not using memory efficiently here.
> +		 * 1 full page (4KB?) used here instead of
> +		 * PRUETH_MAX_PKT_SIZE (~1.5KB?)
> +		 */

What about using page_pool_alloc_frag()? Never used it, but the
documentation says:

2. page_pool_alloc_frag(): allocate memory with page splitting when
driver knows that the memory it need is always smaller than or equal to
half of the page allocated from page pool. Page splitting enables memory
saving and thus avoids TLB/cache miss for data access, but there also is
some cost to implement page splitting, mainly some cache line
dirtying/bouncing for ‘struct page’ and atomic operation for
page->pp_ref_count.

https://docs.kernel.org/networking/page_pool.html

> +		page = page_pool_dev_alloc_pages(pool);
> +		if (!page) {
> +			netdev_err(emac->ndev, "couldn't allocate rx page\n");
> +			ret = -ENOMEM;
> +			goto recycle_alloc_pg;
> +		}
>  
> -		ret = prueth_dma_rx_push(emac, skb, chn);
> +		ret = prueth_dma_rx_push_mapped(emac, chn, page, buf_size);
>  		if (ret < 0) {
>  			netdev_err(emac->ndev,
> -				   "cannot submit skb for rx chan %s ret %d\n",
> +				   "cannot submit page for rx chan %s ret %d\n",
>  				   chn->name, ret);
> -			kfree_skb(skb);
> -			return ret;
> +			page_pool_recycle_direct(pool, page);
> +			goto recycle_alloc_pg;
>  		}
>  	}
>  
>  	return 0;
> +
> +recycle_alloc_pg:
> +	prueth_reset_rx_chan(&emac->rx_chns, PRUETH_MAX_RX_FLOWS, false);
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(prueth_prepare_rx_chan);
>  
> @@ -958,6 +1017,9 @@ void prueth_reset_rx_chan(struct prueth_rx_chn *chn,
>  					  prueth_rx_cleanup, !!i);
>  	if (disable)
>  		k3_udma_glue_disable_rx_chn(chn->rx_chn);
> +
> +	page_pool_destroy(chn->pg_pool);
> +	chn->pg_pool = NULL;
>  }
>  EXPORT_SYMBOL_GPL(prueth_reset_rx_chan);
>  
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> index 5473315ea204..62f3d04af222 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> @@ -33,6 +33,8 @@
>  #include <linux/dma/k3-udma-glue.h>
>  
>  #include <net/devlink.h>
> +#include <net/xdp.h>
> +#include <net/page_pool/helpers.h>
>  
>  #include "icssg_config.h"
>  #include "icss_iep.h"
> @@ -125,6 +127,7 @@ struct prueth_rx_chn {
>  	u32 descs_num;
>  	unsigned int irq[ICSSG_MAX_RFLOWS];	/* separate irq per flow */
>  	char name[32];
> +	struct page_pool *pg_pool;
>  };
>  
>  /* There are 4 Tx DMA channels, but the highest priority is CH3 (thread 3)
> @@ -202,6 +205,10 @@ struct prueth_emac {
>  	unsigned long rx_pace_timeout_ns;
>  };
>  
> +/* The buf includes headroom compatible with both skb and xdpf */
> +#define PRUETH_HEADROOM_NA (max(XDP_PACKET_HEADROOM, NET_SKB_PAD) + NET_IP_ALIGN)
> +#define PRUETH_HEADROOM  ALIGN(PRUETH_HEADROOM_NA, sizeof(long))
> +
>  /**
>   * struct prueth_pdata - PRUeth platform data
>   * @fdqring_mode: Free desc queue mode
> @@ -402,9 +409,10 @@ int prueth_init_rx_chns(struct prueth_emac *emac,
>  			struct prueth_rx_chn *rx_chn,
>  			char *name, u32 max_rflows,
>  			u32 max_desc_num);
> -int prueth_dma_rx_push(struct prueth_emac *emac,
> -		       struct sk_buff *skb,
> -		       struct prueth_rx_chn *rx_chn);
> +int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
> +			      struct prueth_rx_chn *rx_chn,
> +			      struct page *page, u32 buf_len);
> +unsigned int prueth_rxbuf_total_len(unsigned int len);
>  void emac_rx_timestamp(struct prueth_emac *emac,
>  		       struct sk_buff *skb, u32 *psdata);
>  enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev);
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> index 3dc86397c367..c2bc7169355a 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> @@ -274,10 +274,12 @@ static struct sk_buff *prueth_process_rx_mgm(struct prueth_emac *emac,
>  	struct prueth_rx_chn *rx_chn = &emac->rx_mgm_chn;
>  	struct net_device *ndev = emac->ndev;
>  	struct cppi5_host_desc_t *desc_rx;
> -	struct sk_buff *skb, *new_skb;
> +	struct page *page, *new_page;
>  	dma_addr_t desc_dma, buf_dma;
>  	u32 buf_dma_len, pkt_len;
> +	struct sk_buff *skb;
>  	void **swdata;
> +	void *pa;
>  	int ret;
>  
>  	ret = k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, flow_id, &desc_dma);
> @@ -299,32 +301,35 @@ static struct sk_buff *prueth_process_rx_mgm(struct prueth_emac *emac,
>  	}
>  
>  	swdata = cppi5_hdesc_get_swdata(desc_rx);
> -	skb = *swdata;
> +	page = *swdata;
>  	cppi5_hdesc_get_obuf(desc_rx, &buf_dma, &buf_dma_len);
>  	pkt_len = cppi5_hdesc_get_pktlen(desc_rx);
>  
>  	dma_unmap_single(rx_chn->dma_dev, buf_dma, buf_dma_len, DMA_FROM_DEVICE);
>  	k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
>  
> -	new_skb = netdev_alloc_skb_ip_align(ndev, PRUETH_MAX_PKT_SIZE);
> +	new_page = page_pool_dev_alloc_pages(rx_chn->pg_pool);
>  	/* if allocation fails we drop the packet but push the
>  	 * descriptor back to the ring with old skb to prevent a stall
>  	 */
> -	if (!new_skb) {
> +	if (!new_page) {
>  		netdev_err(ndev,
> -			   "skb alloc failed, dropped mgm pkt from flow %d\n",
> +			   "page alloc failed, dropped mgm pkt from flow %d\n",
>  			   flow_id);
> -		new_skb = skb;
> +		new_page = page;
>  		skb = NULL;	/* return NULL */
>  	} else {
>  		/* return the filled skb */
> +		pa = page_address(page);
> +		skb = build_skb(pa, prueth_rxbuf_total_len(pkt_len));
>  		skb_put(skb, pkt_len);
>  	}
>  
>  	/* queue another DMA */
> -	ret = prueth_dma_rx_push(emac, new_skb, &emac->rx_mgm_chn);
> +	ret = prueth_dma_rx_push_mapped(emac, &emac->rx_chns, new_page,
> +					PRUETH_MAX_PKT_SIZE);
>  	if (WARN_ON(ret < 0))
> -		dev_kfree_skb_any(new_skb);
> +		page_pool_recycle_direct(rx_chn->pg_pool, new_page);
>  
>  	return skb;
>  }
> -- 
> 2.25.1
> 
> 

Re: [EXTERNAL] Re: [PATCH net 1/3] net: ti: icssg-prueth: Use page_pool API for RX buffer allocation

[Malladi, Meghana]

Thank for reviewing this patch series.

On 1/23/2025 10:42 PM, Ido Schimmel wrote:
> On Wed, Jan 22, 2025 at 06: 19: 49PM +0530, Meghana Malladi wrote: > 
> From: Roger Quadros <rogerq@ kernel. org> > > This is to prepare for 
> native XDP support. > > The page pool API is more faster in allocating 
> pages than >
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source 
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK! 
> v9dnXbjP_SZUaouOFJtJEp11QLjwx1v0fjJpnJ8CktxnLhoq3emLChPo8_sdyEkE_w$>
> ZjQcmQRYFpfptBannerEnd
> 
> On Wed, Jan 22, 2025 at 06:19:49PM +0530, Meghana Malladi wrote:
>> From: Roger Quadros <rogerq@kernel.org>
>> 
>> This is to prepare for native XDP support.
>> 
>> The page pool API is more faster in allocating pages than
>> __alloc_skb(). Drawback is that it works at PAGE_SIZE granularity
>> so we are not efficient in memory usage.
>> i.e. we are using PAGE_SIZE (4KB) memory for 1.5KB max packet size.
>> 
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> 
> Patch is missing your SoB
> 

Yes I have missed it, will add it for v2.

>> ---
>>  drivers/net/ethernet/ti/Kconfig               |   1 +
>>  drivers/net/ethernet/ti/icssg/icssg_common.c  | 180 ++++++++++++------
>>  drivers/net/ethernet/ti/icssg/icssg_prueth.h  |  14 +-
>>  .../net/ethernet/ti/icssg/icssg_prueth_sr1.c  |  21 +-
>>  4 files changed, 146 insertions(+), 70 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
>> index 0d5a862cd78a..b461281d31b6 100644
>> --- a/drivers/net/ethernet/ti/Kconfig
>> +++ b/drivers/net/ethernet/ti/Kconfig
>> @@ -204,6 +204,7 @@ config TI_ICSSG_PRUETH_SR1
>>  	select PHYLIB
>>  	select TI_ICSS_IEP
>>  	select TI_K3_CPPI_DESC_POOL
>> +	select PAGE_POOL
>>  	depends on PRU_REMOTEPROC
>>  	depends on NET_SWITCHDEV
>>  	depends on ARCH_K3 && OF && TI_K3_UDMA_GLUE_LAYER
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
>> index 74f0f200a89d..313667ce24c3 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
>> @@ -45,6 +45,11 @@ void prueth_cleanup_rx_chns(struct prueth_emac *emac,
>>  			    struct prueth_rx_chn *rx_chn,
>>  			    int max_rflows)
>>  {
>> +	if (rx_chn->pg_pool) {
>> +		page_pool_destroy(rx_chn->pg_pool);
>> +		rx_chn->pg_pool = NULL;
>> +	}
>> +
>>  	if (rx_chn->desc_pool)
>>  		k3_cppi_desc_pool_destroy(rx_chn->desc_pool);
>>  
>> @@ -461,17 +466,17 @@ int prueth_init_rx_chns(struct prueth_emac *emac,
>>  }
>>  EXPORT_SYMBOL_GPL(prueth_init_rx_chns);
>>  
>> -int prueth_dma_rx_push(struct prueth_emac *emac,
>> -		       struct sk_buff *skb,
>> -		       struct prueth_rx_chn *rx_chn)
>> +int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
>> +			      struct prueth_rx_chn *rx_chn,
>> +			      struct page *page, u32 buf_len)
>>  {
>>  	struct net_device *ndev = emac->ndev;
>>  	struct cppi5_host_desc_t *desc_rx;
>> -	u32 pkt_len = skb_tailroom(skb);
>>  	dma_addr_t desc_dma;
>>  	dma_addr_t buf_dma;
>>  	void **swdata;
>>  
>> +	buf_dma = page_pool_get_dma_addr(page) + PRUETH_HEADROOM;
>>  	desc_rx = k3_cppi_desc_pool_alloc(rx_chn->desc_pool);
>>  	if (!desc_rx) {
>>  		netdev_err(ndev, "rx push: failed to allocate descriptor\n");
>> @@ -479,25 +484,18 @@ int prueth_dma_rx_push(struct prueth_emac *emac,
>>  	}
>>  	desc_dma = k3_cppi_desc_pool_virt2dma(rx_chn->desc_pool, desc_rx);
>>  
>> -	buf_dma = dma_map_single(rx_chn->dma_dev, skb->data, pkt_len, DMA_FROM_DEVICE);
>> -	if (unlikely(dma_mapping_error(rx_chn->dma_dev, buf_dma))) {
>> -		k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
>> -		netdev_err(ndev, "rx push: failed to map rx pkt buffer\n");
>> -		return -EINVAL;
>> -	}
>> -
>>  	cppi5_hdesc_init(desc_rx, CPPI5_INFO0_HDESC_EPIB_PRESENT,
>>  			 PRUETH_NAV_PS_DATA_SIZE);
>>  	k3_udma_glue_rx_dma_to_cppi5_addr(rx_chn->rx_chn, &buf_dma);
>> -	cppi5_hdesc_attach_buf(desc_rx, buf_dma, skb_tailroom(skb), buf_dma, skb_tailroom(skb));
>> +	cppi5_hdesc_attach_buf(desc_rx, buf_dma, buf_len, buf_dma, buf_len);
>>  
>>  	swdata = cppi5_hdesc_get_swdata(desc_rx);
>> -	*swdata = skb;
>> +	*swdata = page;
>>  
>> -	return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, 0,
>> +	return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, PRUETH_RX_FLOW_DATA,
>>  					desc_rx, desc_dma);
>>  }
>> -EXPORT_SYMBOL_GPL(prueth_dma_rx_push);
>> +EXPORT_SYMBOL_GPL(prueth_dma_rx_push_mapped);
>>  
>>  u64 icssg_ts_to_ns(u32 hi_sw, u32 hi, u32 lo, u32 cycle_time_ns)
>>  {
>> @@ -535,18 +533,31 @@ void emac_rx_timestamp(struct prueth_emac *emac,
>>  	ssh->hwtstamp = ns_to_ktime(ns);
>>  }
>>  
>> +unsigned int prueth_rxbuf_total_len(unsigned int len)
>> +{
>> +	len += PRUETH_HEADROOM;
>> +	len += SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>> +
>> +	return SKB_DATA_ALIGN(len);
>> +}
>> +EXPORT_SYMBOL_GPL(prueth_rxbuf_total_len);
>> +
>>  static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
>>  {
>>  	struct prueth_rx_chn *rx_chn = &emac->rx_chns;
>>  	u32 buf_dma_len, pkt_len, port_id = 0;
>>  	struct net_device *ndev = emac->ndev;
>>  	struct cppi5_host_desc_t *desc_rx;
>> -	struct sk_buff *skb, *new_skb;
>>  	dma_addr_t desc_dma, buf_dma;
>> +	struct page *page, *new_page;
>> +	struct page_pool *pool;
>> +	struct sk_buff *skb;
>>  	void **swdata;
>>  	u32 *psdata;
>> +	void *pa;
>>  	int ret;
>>  
>> +	pool = rx_chn->pg_pool;
>>  	ret = k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, flow_id, &desc_dma);
>>  	if (ret) {
>>  		if (ret != -ENODATA)
>> @@ -558,14 +569,8 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
>>  		return 0;
>>  
>>  	desc_rx = k3_cppi_desc_pool_dma2virt(rx_chn->desc_pool, desc_dma);
>> -
>>  	swdata = cppi5_hdesc_get_swdata(desc_rx);
>> -	skb = *swdata;
>> -
>> -	psdata = cppi5_hdesc_get_psdata(desc_rx);
>> -	/* RX HW timestamp */
>> -	if (emac->rx_ts_enabled)
>> -		emac_rx_timestamp(emac, skb, psdata);
>> +	page = *swdata;
>>  
>>  	cppi5_hdesc_get_obuf(desc_rx, &buf_dma, &buf_dma_len);
>>  	k3_udma_glue_rx_cppi5_to_dma_addr(rx_chn->rx_chn, &buf_dma);
>> @@ -574,32 +579,50 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
>>  	pkt_len -= 4;
>>  	cppi5_desc_get_tags_ids(&desc_rx->hdr, &port_id, NULL);
>>  
>> -	dma_unmap_single(rx_chn->dma_dev, buf_dma, buf_dma_len, DMA_FROM_DEVICE);
>>  	k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
>>  
>> -	skb->dev = ndev;
>> -	new_skb = netdev_alloc_skb_ip_align(ndev, PRUETH_MAX_PKT_SIZE);
>>  	/* if allocation fails we drop the packet but push the
>> -	 * descriptor back to the ring with old skb to prevent a stall
>> +	 * descriptor back to the ring with old page to prevent a stall
>>  	 */
>> -	if (!new_skb) {
>> +	new_page = page_pool_dev_alloc_pages(pool);
> 
> I don't see where the reference is released via a put call
> 

Seems like none of the pages which have been allocated aren't getting 
recycled in the rx path after being used unless its some error case. 
Will try to fix this.

Also I have noticed, in prueth_prepare_rx_chan() pages are allocated per 
number of descriptors for a channel, but they are not being used when a 
packet is being recieved (in emac_rx_packet()) and rather new page is 
allocated for the next upcoming packet. Is this a valid design, what are 
your thoughts on this ?
)
>> +	if (unlikely(!new_page)) {
>> +		new_page = page;
>>  		ndev->stats.rx_dropped++;
>> -		new_skb = skb;
>> -	} else {
>> -		/* send the filled skb up the n/w stack */
>> -		skb_put(skb, pkt_len);
>> -		if (emac->prueth->is_switch_mode)
>> -			skb->offload_fwd_mark = emac->offload_fwd_mark;
>> -		skb->protocol = eth_type_trans(skb, ndev);
>> -		napi_gro_receive(&emac->napi_rx, skb);
>> -		ndev->stats.rx_bytes += pkt_len;
>> -		ndev->stats.rx_packets++;
>> +		goto requeue;
>>  	}
>>  
>> +	/* prepare skb and send to n/w stack */
>> +	pa = page_address(page);
>> +	skb = build_skb(pa, prueth_rxbuf_total_len(pkt_len));
> 
> napi_build_skb()? See commit 53ee91427177 ("net/mlx5e: Switch to using
> napi_build_skb()")
> 

Ok, I will update it

> Also, I believe the frag size is incorrect. It is used to initialize
> skb->truesize which should signal the size of the buffer that was
> allocated for the packet (PAGE_SIZE in this case).
> 

Yes you are correct, I will replace it with PAGE_SIZE.

>> +	if (!skb) {
>> +		ndev->stats.rx_dropped++;
>> +		page_pool_recycle_direct(pool, page);
>> +		goto requeue;
>> +	}
>> +
>> +	skb_reserve(skb, PRUETH_HEADROOM);
>> +	skb_put(skb, pkt_len);
>> +	skb->dev = ndev;
>> +
>> +	psdata = cppi5_hdesc_get_psdata(desc_rx);
>> +	/* RX HW timestamp */
>> +	if (emac->rx_ts_enabled)
>> +		emac_rx_timestamp(emac, skb, psdata);
>> +
>> +	if (emac->prueth->is_switch_mode)
>> +		skb->offload_fwd_mark = emac->offload_fwd_mark;
>> +	skb->protocol = eth_type_trans(skb, ndev);
> 
> skb_mark_for_recycle() ?
> 
>> +
>> +	netif_receive_skb(skb);
> 
> The code was previously using napi_gro_receive(), why give up on GRO
> support?
> 

will update with napi_gro_receive() and skb_mark_for_recycle() changes 
for v2.

>> +	ndev->stats.rx_bytes += pkt_len;
>> +	ndev->stats.rx_packets++;
>> +
>> +requeue:
>>  	/* queue another RX DMA */
>> -	ret = prueth_dma_rx_push(emac, new_skb, &emac->rx_chns);
>> +	ret = prueth_dma_rx_push_mapped(emac, &emac->rx_chns, new_page,
>> +					PRUETH_MAX_PKT_SIZE);
>>  	if (WARN_ON(ret < 0)) {
>> -		dev_kfree_skb_any(new_skb);
>> +		page_pool_recycle_direct(pool, new_page);
>>  		ndev->stats.rx_errors++;
>>  		ndev->stats.rx_dropped++;
>>  	}
>> @@ -611,22 +634,17 @@ static void prueth_rx_cleanup(void *data, dma_addr_t desc_dma)
>>  {
>>  	struct prueth_rx_chn *rx_chn = data;
>>  	struct cppi5_host_desc_t *desc_rx;
>> -	struct sk_buff *skb;
>> -	dma_addr_t buf_dma;
>> -	u32 buf_dma_len;
>> +	struct page_pool *pool;
>> +	struct page *page;
>>  	void **swdata;
>>  
>> +	pool = rx_chn->pg_pool;
>> +
>>  	desc_rx = k3_cppi_desc_pool_dma2virt(rx_chn->desc_pool, desc_dma);
>>  	swdata = cppi5_hdesc_get_swdata(desc_rx);
>> -	skb = *swdata;
>> -	cppi5_hdesc_get_obuf(desc_rx, &buf_dma, &buf_dma_len);
>> -	k3_udma_glue_rx_cppi5_to_dma_addr(rx_chn->rx_chn, &buf_dma);
>> -
>> -	dma_unmap_single(rx_chn->dma_dev, buf_dma, buf_dma_len,
>> -			 DMA_FROM_DEVICE);
>> +	page = *swdata;
>> +	page_pool_recycle_direct(pool, page);
>>  	k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
>> -
>> -	dev_kfree_skb_any(skb);
>>  }
>>  
>>  static int prueth_tx_ts_cookie_get(struct prueth_emac *emac)
>> @@ -907,29 +925,70 @@ int icssg_napi_rx_poll(struct napi_struct *napi_rx, int budget)
>>  }
>>  EXPORT_SYMBOL_GPL(icssg_napi_rx_poll);
>>  
>> +static struct page_pool *prueth_create_page_pool(struct prueth_emac *emac,
>> +						 struct device *dma_dev,
>> +						 int size)
>> +{
>> +	struct page_pool_params pp_params = { 0 };
>> +	struct page_pool *pool;
>> +
>> +	pp_params.order = 0;
>> +	pp_params.flags = PP_FLAG_DMA_MAP;
> 
> What about PP_FLAG_DMA_SYNC_DEV ? I don't see a sync for device call. I
> also don't see a sync for CPU...
> 

Yes I will add PP_FLAG_DMA_SYNC_DEV as well.
I believe page_pool_dma_sync_for_cpu() needs to be called sync Rx page 
for CPU, am I right ? If so can you tell me, in what all cases should I 
call this function.

https://lkml.iu.edu/hypermail/linux/kernel/2312.1/06353.html
In the above link it is quoted - "Note that this version performs DMA 
sync unconditionally, even if the associated PP doesn't perform 
sync-for-device" for the page_pool_dma_sync_for_cpu() function. So does 
that mean if I am using this function I don't need explicily sync for 
device call?

And if thats not the case, can you point me to some reference on how I 
can acheive sync for device.

>> +	pp_params.pool_size = size;
>> +	pp_params.nid = dev_to_node(emac->prueth->dev);
>> +	pp_params.dma_dir = DMA_BIDIRECTIONAL;
>> +	pp_params.dev = dma_dev;
>> +	pp_params.napi = &emac->napi_rx;
>> +
>> +	pool = page_pool_create(&pp_params);
>> +	if (IS_ERR(pool))
>> +		netdev_err(emac->ndev, "cannot create rx page pool\n");
>> +
>> +	return pool;
>> +}
>> +
>>  int prueth_prepare_rx_chan(struct prueth_emac *emac,
>>  			   struct prueth_rx_chn *chn,
>>  			   int buf_size)
>>  {
>> -	struct sk_buff *skb;
>> +	struct page_pool *pool;
>> +	struct page *page;
>>  	int i, ret;
>>  
>> +	pool = prueth_create_page_pool(emac, chn->dma_dev, chn->descs_num);
>> +	if (IS_ERR(pool))
>> +		return PTR_ERR(pool);
>> +
>> +	chn->pg_pool = pool;
>> +
>>  	for (i = 0; i < chn->descs_num; i++) {
>> -		skb = __netdev_alloc_skb_ip_align(NULL, buf_size, GFP_KERNEL);
>> -		if (!skb)
>> -			return -ENOMEM;
>> +		/* NOTE: we're not using memory efficiently here.
>> +		 * 1 full page (4KB?) used here instead of
>> +		 * PRUETH_MAX_PKT_SIZE (~1.5KB?)
>> +		 */
> 
> What about using page_pool_alloc_frag()? Never used it, but the
> documentation says:
> 
> 2. page_pool_alloc_frag(): allocate memory with page splitting when
> driver knows that the memory it need is always smaller than or equal to
> half of the page allocated from page pool. Page splitting enables memory
> saving and thus avoids TLB/cache miss for data access, but there also is
> some cost to implement page splitting, mainly some cache line
> dirtying/bouncing for ‘struct page’ and atomic operation for
> page->pp_ref_count.
> 

I believe the reason for not using it is similar to the explanation in 
the patch here: 
https://lore.kernel.org/all/20240424203559.3420468-4-anthony.l.nguyen@intel.com/
In a nutshell, to keep it simple and compact.

> https://urldefense.com/v3/__https://docs.kernel.org/networking/ 
> page_pool.html__;!!G3vK!WHMkn8KHknjjz91hGnt0Ywf5z4VLXzXTK5Bu2T6CV2n- 
> zHUE1iqgRJsrKQYQwFswRSFrHOYym_qb$ <https://urldefense.com/v3/__https://docs.kernel.org/networking/page_pool.html__;!!G3vK!WHMkn8KHknjjz91hGnt0Ywf5z4VLXzXTK5Bu2T6CV2n-zHUE1iqgRJsrKQYQwFswRSFrHOYym_qb$>
> 
>> +		page = page_pool_dev_alloc_pages(pool);
>> +		if (!page) {
>> +			netdev_err(emac->ndev, "couldn't allocate rx page\n");
>> +			ret = -ENOMEM;
>> +			goto recycle_alloc_pg;
>> +		}
>>  
>> -		ret = prueth_dma_rx_push(emac, skb, chn);
>> +		ret = prueth_dma_rx_push_mapped(emac, chn, page, buf_size);
>>  		if (ret < 0) {
>>  			netdev_err(emac->ndev,
>> -				   "cannot submit skb for rx chan %s ret %d\n",
>> +				   "cannot submit page for rx chan %s ret %d\n",
>>  				   chn->name, ret);
>> -			kfree_skb(skb);
>> -			return ret;
>> +			page_pool_recycle_direct(pool, page);
>> +			goto recycle_alloc_pg;
>>  		}
>>  	}
>>  
>>  	return 0;
>> +
>> +recycle_alloc_pg:
>> +	prueth_reset_rx_chan(&emac->rx_chns, PRUETH_MAX_RX_FLOWS, false);
>> +
>> +	return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(prueth_prepare_rx_chan);
>>  
>> @@ -958,6 +1017,9 @@ void prueth_reset_rx_chan(struct prueth_rx_chn *chn,
>>  					  prueth_rx_cleanup, !!i);
>>  	if (disable)
>>  		k3_udma_glue_disable_rx_chn(chn->rx_chn);
>> +
>> +	page_pool_destroy(chn->pg_pool);
>> +	chn->pg_pool = NULL;
>>  }
>>  EXPORT_SYMBOL_GPL(prueth_reset_rx_chan);
>>  
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> index 5473315ea204..62f3d04af222 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> @@ -33,6 +33,8 @@
>>  #include <linux/dma/k3-udma-glue.h>
>>  
>>  #include <net/devlink.h>
>> +#include <net/xdp.h>
>> +#include <net/page_pool/helpers.h>
>>  
>>  #include "icssg_config.h"
>>  #include "icss_iep.h"
>> @@ -125,6 +127,7 @@ struct prueth_rx_chn {
>>  	u32 descs_num;
>>  	unsigned int irq[ICSSG_MAX_RFLOWS];	/* separate irq per flow */
>>  	char name[32];
>> +	struct page_pool *pg_pool;
>>  };
>>  
>>  /* There are 4 Tx DMA channels, but the highest priority is CH3 (thread 3)
>> @@ -202,6 +205,10 @@ struct prueth_emac {
>>  	unsigned long rx_pace_timeout_ns;
>>  };
>>  
>> +/* The buf includes headroom compatible with both skb and xdpf */
>> +#define PRUETH_HEADROOM_NA (max(XDP_PACKET_HEADROOM, NET_SKB_PAD) + NET_IP_ALIGN)
>> +#define PRUETH_HEADROOM  ALIGN(PRUETH_HEADROOM_NA, sizeof(long))
>> +
>>  /**
>>   * struct prueth_pdata - PRUeth platform data
>>   * @fdqring_mode: Free desc queue mode
>> @@ -402,9 +409,10 @@ int prueth_init_rx_chns(struct prueth_emac *emac,
>>  			struct prueth_rx_chn *rx_chn,
>>  			char *name, u32 max_rflows,
>>  			u32 max_desc_num);
>> -int prueth_dma_rx_push(struct prueth_emac *emac,
>> -		       struct sk_buff *skb,
>> -		       struct prueth_rx_chn *rx_chn);
>> +int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
>> +			      struct prueth_rx_chn *rx_chn,
>> +			      struct page *page, u32 buf_len);
>> +unsigned int prueth_rxbuf_total_len(unsigned int len);
>>  void emac_rx_timestamp(struct prueth_emac *emac,
>>  		       struct sk_buff *skb, u32 *psdata);
>>  enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev);
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
>> index 3dc86397c367..c2bc7169355a 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
>> @@ -274,10 +274,12 @@ static struct sk_buff *prueth_process_rx_mgm(struct prueth_emac *emac,
>>  	struct prueth_rx_chn *rx_chn = &emac->rx_mgm_chn;
>>  	struct net_device *ndev = emac->ndev;
>>  	struct cppi5_host_desc_t *desc_rx;
>> -	struct sk_buff *skb, *new_skb;
>> +	struct page *page, *new_page;
>>  	dma_addr_t desc_dma, buf_dma;
>>  	u32 buf_dma_len, pkt_len;
>> +	struct sk_buff *skb;
>>  	void **swdata;
>> +	void *pa;
>>  	int ret;
>>  
>>  	ret = k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, flow_id, &desc_dma);
>> @@ -299,32 +301,35 @@ static struct sk_buff *prueth_process_rx_mgm(struct prueth_emac *emac,
>>  	}
>>  
>>  	swdata = cppi5_hdesc_get_swdata(desc_rx);
>> -	skb = *swdata;
>> +	page = *swdata;
>>  	cppi5_hdesc_get_obuf(desc_rx, &buf_dma, &buf_dma_len);
>>  	pkt_len = cppi5_hdesc_get_pktlen(desc_rx);
>>  
>>  	dma_unmap_single(rx_chn->dma_dev, buf_dma, buf_dma_len, DMA_FROM_DEVICE);
>>  	k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
>>  
>> -	new_skb = netdev_alloc_skb_ip_align(ndev, PRUETH_MAX_PKT_SIZE);
>> +	new_page = page_pool_dev_alloc_pages(rx_chn->pg_pool);
>>  	/* if allocation fails we drop the packet but push the
>>  	 * descriptor back to the ring with old skb to prevent a stall
>>  	 */
>> -	if (!new_skb) {
>> +	if (!new_page) {
>>  		netdev_err(ndev,
>> -			   "skb alloc failed, dropped mgm pkt from flow %d\n",
>> +			   "page alloc failed, dropped mgm pkt from flow %d\n",
>>  			   flow_id);
>> -		new_skb = skb;
>> +		new_page = page;
>>  		skb = NULL;	/* return NULL */
>>  	} else {
>>  		/* return the filled skb */
>> +		pa = page_address(page);
>> +		skb = build_skb(pa, prueth_rxbuf_total_len(pkt_len));
>>  		skb_put(skb, pkt_len);
>>  	}
>>  
>>  	/* queue another DMA */
>> -	ret = prueth_dma_rx_push(emac, new_skb, &emac->rx_mgm_chn);
>> +	ret = prueth_dma_rx_push_mapped(emac, &emac->rx_chns, new_page,
>> +					PRUETH_MAX_PKT_SIZE);
>>  	if (WARN_ON(ret < 0))
>> -		dev_kfree_skb_any(new_skb);
>> +		page_pool_recycle_direct(rx_chn->pg_pool, new_page);
>>  
>>  	return skb;
>>  }
>> -- 
>> 2.25.1
>> 
>> 
> 

Re: [EXTERNAL] Re: [PATCH net 1/3] net: ti: icssg-prueth: Use page_pool API for RX buffer allocation

[Ido Schimmel]

On Tue, Feb 04, 2025 at 11:25:02PM +0530, Malladi, Meghana wrote:
> Seems like none of the pages which have been allocated aren't getting
> recycled in the rx path after being used unless its some error case. Will
> try to fix this.

skb_mark_for_recycle() should help with page recycling when an skb that
uses them is freed.

Anyway, I believe that I don't see put call when tearing down the Rx
ring because prueth_rx_cleanup() is using page_pool_recycle_direct()
when it shouldn't. AFAICT, prueth_rx_cleanup() is only called from the
control path (upon ndo_stop()) and not in NAPI context.

> Also I have noticed, in prueth_prepare_rx_chan() pages are allocated per
> number of descriptors for a channel, but they are not being used when a
> packet is being recieved (in emac_rx_packet()) and rather new page is
> allocated for the next upcoming packet. Is this a valid design, what are
> your thoughts on this ?

The new page is possibly a page that was recycled into the pool when a
previous packet was freed / dropped.

[...]

> Yes I will add PP_FLAG_DMA_SYNC_DEV as well.
> I believe page_pool_dma_sync_for_cpu() needs to be called sync Rx page for
> CPU, am I right ? If so can you tell me, in what all cases should I call
> this function.

Before accessing the packet data.

> https://lkml.iu.edu/hypermail/linux/kernel/2312.1/06353.html
> In the above link it is quoted - "Note that this version performs DMA sync
> unconditionally, even if the associated PP doesn't perform sync-for-device"
> for the page_pool_dma_sync_for_cpu() function. So does that mean if I am
> using this function I don't need explicily sync for device call?

It's explained in the page pool documentation:

"Driver is always responsible for syncing the pages for the CPU. Drivers
may choose to take care of syncing for the device as well or set the
PP_FLAG_DMA_SYNC_DEV flag to request that pages allocated from the page
pool are already synced for the device."

https://docs.kernel.org/networking/page_pool.html#dma-sync

Re: [EXTERNAL] Re: [PATCH net 1/3] net: ti: icssg-prueth: Use page_pool API for RX buffer allocation

[Malladi, Meghana]

On 2/5/2025 11:11 PM, Ido Schimmel wrote:
> On Tue, Feb 04, 2025 at 11: 25: 02PM +0530, Malladi, Meghana wrote: > 
> Seems like none of the pages which have been allocated aren't getting > 
> recycled in the rx path after being used unless its some error case. 
> Will > try to fix this. 
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source 
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK! 
> v9dnXdhkXiNQgIoLtH6jcbhWBIydfvayMZ6bf68taZCHXfcLg8XIOscUa_XNxqzQWA$>
> ZjQcmQRYFpfptBannerEnd
> 
> On Tue, Feb 04, 2025 at 11:25:02PM +0530, Malladi, Meghana wrote:
>> Seems like none of the pages which have been allocated aren't getting
>> recycled in the rx path after being used unless its some error case. Will
>> try to fix this.
> 
> skb_mark_for_recycle() should help with page recycling when an skb that
> uses them is freed.
> 
> Anyway, I believe that I don't see put call when tearing down the Rx
> ring because prueth_rx_cleanup() is using page_pool_recycle_direct()
> when it shouldn't. AFAICT, prueth_rx_cleanup() is only called from the
> control path (upon ndo_stop()) and not in NAPI context.
> 

Ok I will use skb_mark_for_recycle()/page_pool_recycle_direct() 
accordingly to recycle the pages.

>> Also I have noticed, in prueth_prepare_rx_chan() pages are allocated per
>> number of descriptors for a channel, but they are not being used when a
>> packet is being recieved (in emac_rx_packet()) and rather new page is
>> allocated for the next upcoming packet. Is this a valid design, what are
>> your thoughts on this ?
> 
> The new page is possibly a page that was recycled into the pool when a
> previous packet was freed / dropped.
> 
> [...]
> 
>> Yes I will add PP_FLAG_DMA_SYNC_DEV as well.
>> I believe page_pool_dma_sync_for_cpu() needs to be called sync Rx page for
>> CPU, am I right ? If so can you tell me, in what all cases should I call
>> this function.
> 
> Before accessing the packet data.
>

Ok, thanks.

>> https://urldefense.com/v3/__https://lkml.iu.edu/hypermail/linux/ 
> kernel/2312.1/06353.html__;!!G3vK!R- 
> autrVAgf5rAbl3CYoqlN5gRE_NqPqYRg1NHkJ405Q33b6uKiHFI73PeRky46dBYBWQpmFThUyD$ <https://urldefense.com/v3/__https://lkml.iu.edu/hypermail/linux/kernel/2312.1/06353.html__;!!G3vK!R-autrVAgf5rAbl3CYoqlN5gRE_NqPqYRg1NHkJ405Q33b6uKiHFI73PeRky46dBYBWQpmFThUyD$>
>> In the above link it is quoted - "Note that this version performs DMA sync
>> unconditionally, even if the associated PP doesn't perform sync-for-device"
>> for the page_pool_dma_sync_for_cpu() function. So does that mean if I am
>> using this function I don't need explicily sync for device call?
> 
> It's explained in the page pool documentation:
> 
> "Driver is always responsible for syncing the pages for the CPU. Drivers
> may choose to take care of syncing for the device as well or set the
> PP_FLAG_DMA_SYNC_DEV flag to request that pages allocated from the page
> pool are already synced for the device."
> 
> https://urldefense.com/v3/__https://docs.kernel.org/networking/ 
> page_pool.html*dma-sync__;Iw!!G3vK!R- 
> autrVAgf5rAbl3CYoqlN5gRE_NqPqYRg1NHkJ405Q33b6uKiHFI73PeRky46dBYBWQphNIm6Qm$ <https://urldefense.com/v3/__https://docs.kernel.org/networking/page_pool.html*dma-sync__;Iw!!G3vK!R-autrVAgf5rAbl3CYoqlN5gRE_NqPqYRg1NHkJ405Q33b6uKiHFI73PeRky46dBYBWQphNIm6Qm$>
> 

[PATCH net 2/3] net: ti: icssg-prueth: introduce and use prueth_swdata struct for SWDATA

[Meghana Malladi]

From: Roger Quadros <rogerq@kernel.org>

We have different cases for SWDATA (skb, page, cmd, etc)
so it is better to have a dedicated data structure for that.
We can embed the type field inside the struct and use it
to interpret the data in completion handlers.

Increase SWDATA size to 48 so we have some room to add
more data if required.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/net/ethernet/ti/icssg/icssg_common.c  | 47 ++++++++++++-------
 drivers/net/ethernet/ti/icssg/icssg_config.h  |  2 +-
 drivers/net/ethernet/ti/icssg/icssg_prueth.c  |  6 +++
 drivers/net/ethernet/ti/icssg/icssg_prueth.h  | 18 +++++++
 .../net/ethernet/ti/icssg/icssg_prueth_sr1.c  |  4 +-
 5 files changed, 58 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index 313667ce24c3..63b5d66aab99 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -136,12 +136,12 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
 	struct net_device *ndev = emac->ndev;
 	struct cppi5_host_desc_t *desc_tx;
 	struct netdev_queue *netif_txq;
+	struct prueth_swdata *swdata;
 	struct prueth_tx_chn *tx_chn;
 	unsigned int total_bytes = 0;
 	struct sk_buff *skb;
 	dma_addr_t desc_dma;
 	int res, num_tx = 0;
-	void **swdata;
 
 	tx_chn = &emac->tx_chns[chn];
 
@@ -163,12 +163,18 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
 		swdata = cppi5_hdesc_get_swdata(desc_tx);
 
 		/* was this command's TX complete? */
-		if (emac->is_sr1 && *(swdata) == emac->cmd_data) {
+		if (emac->is_sr1 && (void *)(swdata) == emac->cmd_data) {
 			prueth_xmit_free(tx_chn, desc_tx);
 			continue;
 		}
 
-		skb = *(swdata);
+		if (swdata->type != PRUETH_SWDATA_SKB) {
+			netdev_err(ndev, "tx_complete: invalid swdata type %d\n", swdata->type);
+			budget++;
+			continue;
+		}
+
+		skb = swdata->data.skb;
 		prueth_xmit_free(tx_chn, desc_tx);
 
 		ndev = skb->dev;
@@ -472,9 +478,9 @@ int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
 {
 	struct net_device *ndev = emac->ndev;
 	struct cppi5_host_desc_t *desc_rx;
+	struct prueth_swdata *swdata;
 	dma_addr_t desc_dma;
 	dma_addr_t buf_dma;
-	void **swdata;
 
 	buf_dma = page_pool_get_dma_addr(page) + PRUETH_HEADROOM;
 	desc_rx = k3_cppi_desc_pool_alloc(rx_chn->desc_pool);
@@ -490,7 +496,8 @@ int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
 	cppi5_hdesc_attach_buf(desc_rx, buf_dma, buf_len, buf_dma, buf_len);
 
 	swdata = cppi5_hdesc_get_swdata(desc_rx);
-	*swdata = page;
+	swdata->type = PRUETH_SWDATA_PAGE;
+	swdata->data.page = page;
 
 	return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, PRUETH_RX_FLOW_DATA,
 					desc_rx, desc_dma);
@@ -548,11 +555,11 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
 	u32 buf_dma_len, pkt_len, port_id = 0;
 	struct net_device *ndev = emac->ndev;
 	struct cppi5_host_desc_t *desc_rx;
+	struct prueth_swdata *swdata;
 	dma_addr_t desc_dma, buf_dma;
 	struct page *page, *new_page;
 	struct page_pool *pool;
 	struct sk_buff *skb;
-	void **swdata;
 	u32 *psdata;
 	void *pa;
 	int ret;
@@ -570,7 +577,11 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
 
 	desc_rx = k3_cppi_desc_pool_dma2virt(rx_chn->desc_pool, desc_dma);
 	swdata = cppi5_hdesc_get_swdata(desc_rx);
-	page = *swdata;
+	if (swdata->type != PRUETH_SWDATA_PAGE) {
+		netdev_err(ndev, "rx_pkt: invalid swdata->type %d\n", swdata->type);
+		return 0;
+	}
+	page = swdata->data.page;
 
 	cppi5_hdesc_get_obuf(desc_rx, &buf_dma, &buf_dma_len);
 	k3_udma_glue_rx_cppi5_to_dma_addr(rx_chn->rx_chn, &buf_dma);
@@ -634,16 +645,18 @@ static void prueth_rx_cleanup(void *data, dma_addr_t desc_dma)
 {
 	struct prueth_rx_chn *rx_chn = data;
 	struct cppi5_host_desc_t *desc_rx;
+	struct prueth_swdata *swdata;
 	struct page_pool *pool;
 	struct page *page;
-	void **swdata;
 
 	pool = rx_chn->pg_pool;
 
 	desc_rx = k3_cppi_desc_pool_dma2virt(rx_chn->desc_pool, desc_dma);
 	swdata = cppi5_hdesc_get_swdata(desc_rx);
-	page = *swdata;
-	page_pool_recycle_direct(pool, page);
+	if (swdata->type == PRUETH_SWDATA_PAGE) {
+		page = swdata->data.page;
+		page_pool_recycle_direct(pool, page);
+	}
 	k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
 }
 
@@ -680,13 +693,13 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
 	struct prueth_emac *emac = netdev_priv(ndev);
 	struct prueth *prueth = emac->prueth;
 	struct netdev_queue *netif_txq;
+	struct prueth_swdata *swdata;
 	struct prueth_tx_chn *tx_chn;
 	dma_addr_t desc_dma, buf_dma;
 	u32 pkt_len, dst_tag_id;
 	int i, ret = 0, q_idx;
 	bool in_tx_ts = 0;
 	int tx_ts_cookie;
-	void **swdata;
 	u32 *epib;
 
 	pkt_len = skb_headlen(skb);
@@ -748,7 +761,8 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
 	k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
 	cppi5_hdesc_attach_buf(first_desc, buf_dma, pkt_len, buf_dma, pkt_len);
 	swdata = cppi5_hdesc_get_swdata(first_desc);
-	*swdata = skb;
+	swdata->type = PRUETH_SWDATA_SKB;
+	swdata->data.skb = skb;
 
 	/* Handle the case where skb is fragmented in pages */
 	cur_desc = first_desc;
@@ -851,15 +865,16 @@ static void prueth_tx_cleanup(void *data, dma_addr_t desc_dma)
 {
 	struct prueth_tx_chn *tx_chn = data;
 	struct cppi5_host_desc_t *desc_tx;
+	struct prueth_swdata *swdata;
 	struct sk_buff *skb;
-	void **swdata;
 
 	desc_tx = k3_cppi_desc_pool_dma2virt(tx_chn->desc_pool, desc_dma);
 	swdata = cppi5_hdesc_get_swdata(desc_tx);
-	skb = *(swdata);
+	if (swdata->type == PRUETH_SWDATA_SKB) {
+		skb = swdata->data.skb;
+		dev_kfree_skb_any(skb);
+	}
 	prueth_xmit_free(tx_chn, desc_tx);
-
-	dev_kfree_skb_any(skb);
 }
 
 irqreturn_t prueth_rx_irq(int irq, void *dev_id)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
index c884e9fa099e..eab84e11d80e 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
@@ -20,7 +20,7 @@ struct icssg_flow_cfg {
 
 #define PRUETH_PKT_TYPE_CMD	0x10
 #define PRUETH_NAV_PS_DATA_SIZE	16	/* Protocol specific data size */
-#define PRUETH_NAV_SW_DATA_SIZE	16	/* SW related data size */
+#define PRUETH_NAV_SW_DATA_SIZE	48	/* SW related data size */
 #define PRUETH_MAX_TX_DESC	512
 #define PRUETH_MAX_RX_DESC	512
 #define PRUETH_MAX_RX_FLOWS	1	/* excluding default flow */
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index d76fe6d05e10..30bcd6aa966e 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -1441,6 +1441,12 @@ static int prueth_probe(struct platform_device *pdev)
 
 	np = dev->of_node;
 
+	if (sizeof(struct prueth_swdata) > PRUETH_NAV_SW_DATA_SIZE) {
+		dev_err(dev, "insufficient SW_DATA size: %d vs %ld\n",
+			PRUETH_NAV_SW_DATA_SIZE, sizeof(struct prueth_swdata));
+		return -ENOMEM;
+	}
+
 	prueth = devm_kzalloc(dev, sizeof(*prueth), GFP_KERNEL);
 	if (!prueth)
 		return -ENOMEM;
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index 62f3d04af222..187c4e062a15 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -130,6 +130,24 @@ struct prueth_rx_chn {
 	struct page_pool *pg_pool;
 };
 
+enum prueth_swdata_type {
+	PRUETH_SWDATA_INVALID = 0,
+	PRUETH_SWDATA_SKB,
+	PRUETH_SWDATA_PAGE,
+	PRUETH_SWDATA_CMD,
+};
+
+union prueth_data {
+	struct sk_buff *skb;
+	struct page *page;
+	u32 cmd;
+};
+
+struct prueth_swdata {
+	union prueth_data data;
+	enum prueth_swdata_type type;
+};
+
 /* There are 4 Tx DMA channels, but the highest priority is CH3 (thread 3)
  * and lower three are lower priority channels or threads.
  */
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
index c2bc7169355a..c05abef35e8e 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
@@ -275,10 +275,10 @@ static struct sk_buff *prueth_process_rx_mgm(struct prueth_emac *emac,
 	struct net_device *ndev = emac->ndev;
 	struct cppi5_host_desc_t *desc_rx;
 	struct page *page, *new_page;
+	struct prueth_swdata *swdata;
 	dma_addr_t desc_dma, buf_dma;
 	u32 buf_dma_len, pkt_len;
 	struct sk_buff *skb;
-	void **swdata;
 	void *pa;
 	int ret;
 
@@ -301,7 +301,7 @@ static struct sk_buff *prueth_process_rx_mgm(struct prueth_emac *emac,
 	}
 
 	swdata = cppi5_hdesc_get_swdata(desc_rx);
-	page = *swdata;
+	page = swdata->data.page;
 	cppi5_hdesc_get_obuf(desc_rx, &buf_dma, &buf_dma_len);
 	pkt_len = cppi5_hdesc_get_pktlen(desc_rx);
 
-- 
2.25.1

[PATCH net 3/3] net: ti: icssg-prueth: Add AF_XDP support

[Meghana Malladi]

From: Roger Quadros <rogerq@kernel.org>

Add native XDP support. We do not support zero copy yet.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/net/ethernet/ti/icssg/icssg_common.c | 224 ++++++++++++++++++-
 drivers/net/ethernet/ti/icssg/icssg_prueth.c | 118 +++++++++-
 drivers/net/ethernet/ti/icssg/icssg_prueth.h |  19 ++
 3 files changed, 348 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index 63b5d66aab99..b36ad2ba627d 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -98,11 +98,19 @@ void prueth_xmit_free(struct prueth_tx_chn *tx_chn,
 {
 	struct cppi5_host_desc_t *first_desc, *next_desc;
 	dma_addr_t buf_dma, next_desc_dma;
+	struct prueth_swdata *swdata;
 	u32 buf_dma_len;
 
 	first_desc = desc;
 	next_desc = first_desc;
 
+	swdata = cppi5_hdesc_get_swdata(desc);
+	if (swdata->type == PRUETH_SWDATA_PAGE) {
+		page_pool_recycle_direct(swdata->rx_chn->pg_pool,
+					 swdata->data.page);
+		goto free_desc;
+	}
+
 	cppi5_hdesc_get_obuf(first_desc, &buf_dma, &buf_dma_len);
 	k3_udma_glue_tx_cppi5_to_dma_addr(tx_chn->tx_chn, &buf_dma);
 
@@ -126,6 +134,7 @@ void prueth_xmit_free(struct prueth_tx_chn *tx_chn,
 		k3_cppi_desc_pool_free(tx_chn->desc_pool, next_desc);
 	}
 
+free_desc:
 	k3_cppi_desc_pool_free(tx_chn->desc_pool, first_desc);
 }
 EXPORT_SYMBOL_GPL(prueth_xmit_free);
@@ -139,6 +148,7 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
 	struct prueth_swdata *swdata;
 	struct prueth_tx_chn *tx_chn;
 	unsigned int total_bytes = 0;
+	struct xdp_frame *xdpf;
 	struct sk_buff *skb;
 	dma_addr_t desc_dma;
 	int res, num_tx = 0;
@@ -168,20 +178,29 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
 			continue;
 		}
 
-		if (swdata->type != PRUETH_SWDATA_SKB) {
+		switch (swdata->type) {
+		case PRUETH_SWDATA_SKB:
+			skb = swdata->data.skb;
+			ndev->stats.tx_bytes += skb->len;
+			ndev->stats.tx_packets++;
+			total_bytes += skb->len;
+			napi_consume_skb(skb, budget);
+			break;
+		case PRUETH_SWDATA_XDPF:
+			xdpf = swdata->data.xdpf;
+			ndev->stats.tx_bytes += xdpf->len;
+			ndev->stats.tx_packets++;
+			total_bytes += xdpf->len;
+			xdp_return_frame(xdpf);
+			break;
+		default:
 			netdev_err(ndev, "tx_complete: invalid swdata type %d\n", swdata->type);
+			prueth_xmit_free(tx_chn, desc_tx);
 			budget++;
 			continue;
 		}
 
-		skb = swdata->data.skb;
 		prueth_xmit_free(tx_chn, desc_tx);
-
-		ndev = skb->dev;
-		ndev->stats.tx_packets++;
-		ndev->stats.tx_bytes += skb->len;
-		total_bytes += skb->len;
-		napi_consume_skb(skb, budget);
 		num_tx++;
 	}
 
@@ -498,6 +517,7 @@ int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
 	swdata = cppi5_hdesc_get_swdata(desc_rx);
 	swdata->type = PRUETH_SWDATA_PAGE;
 	swdata->data.page = page;
+	swdata->rx_chn = rx_chn;
 
 	return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, PRUETH_RX_FLOW_DATA,
 					desc_rx, desc_dma);
@@ -549,7 +569,159 @@ unsigned int prueth_rxbuf_total_len(unsigned int len)
 }
 EXPORT_SYMBOL_GPL(prueth_rxbuf_total_len);
 
-static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
+/**
+ * emac_xmit_xdp_frame - transmits an XDP frame
+ * @emac: emac device
+ * @xdpf: data to transmit
+ * @page: page from page pool if already DMA mapped
+ * @q_idx: queue id
+ *
+ * Return: XDP state
+ */
+int emac_xmit_xdp_frame(struct prueth_emac *emac,
+			struct xdp_frame *xdpf,
+			struct page *page,
+			unsigned int q_idx)
+{
+	struct cppi5_host_desc_t *first_desc;
+	struct net_device *ndev = emac->ndev;
+	struct prueth_tx_chn *tx_chn;
+	dma_addr_t desc_dma, buf_dma;
+	struct prueth_swdata *swdata;
+	u32 *epib;
+	int ret;
+
+	void *data = xdpf->data;
+	u32 pkt_len = xdpf->len;
+
+	if (q_idx >= PRUETH_MAX_TX_QUEUES) {
+		netdev_err(ndev, "xdp tx: invalid q_id %d\n", q_idx);
+		return ICSSG_XDP_CONSUMED;	/* drop */
+	}
+
+	tx_chn = &emac->tx_chns[q_idx];
+
+	if (page) { /* already DMA mapped by page_pool */
+		buf_dma = page_pool_get_dma_addr(page);
+		buf_dma += xdpf->headroom + sizeof(struct xdp_frame);
+	} else { /* Map the linear buffer */
+		buf_dma = dma_map_single(tx_chn->dma_dev, data, pkt_len, DMA_TO_DEVICE);
+		if (dma_mapping_error(tx_chn->dma_dev, buf_dma)) {
+			netdev_err(ndev, "xdp tx: failed to map data buffer\n");
+			return ICSSG_XDP_CONSUMED;	/* drop */
+		}
+	}
+
+	first_desc = k3_cppi_desc_pool_alloc(tx_chn->desc_pool);
+	if (!first_desc) {
+		netdev_dbg(ndev, "xdp tx: failed to allocate descriptor\n");
+		if (!page)
+			dma_unmap_single(tx_chn->dma_dev, buf_dma, pkt_len, DMA_TO_DEVICE);
+		goto drop_free_descs;	/* drop */
+	}
+
+	cppi5_hdesc_init(first_desc, CPPI5_INFO0_HDESC_EPIB_PRESENT,
+			 PRUETH_NAV_PS_DATA_SIZE);
+	cppi5_hdesc_set_pkttype(first_desc, 0);
+	epib = first_desc->epib;
+	epib[0] = 0;
+	epib[1] = 0;
+
+	/* set dst tag to indicate internal qid at the firmware which is at
+	 * bit8..bit15. bit0..bit7 indicates port num for directed
+	 * packets in case of switch mode operation
+	 */
+	cppi5_desc_set_tags_ids(&first_desc->hdr, 0, (emac->port_id | (q_idx << 8)));
+	k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
+	cppi5_hdesc_attach_buf(first_desc, buf_dma, pkt_len, buf_dma, pkt_len);
+	swdata = cppi5_hdesc_get_swdata(first_desc);
+	if (page) {
+		swdata->type = PRUETH_SWDATA_PAGE;
+		swdata->data.page = page;
+		/* we assume page came from RX channel page pool */
+		swdata->rx_chn = &emac->rx_chns;
+	} else {
+		swdata->type = PRUETH_SWDATA_XDPF;
+		swdata->data.xdpf = xdpf;
+	}
+
+	cppi5_hdesc_set_pktlen(first_desc, pkt_len);
+	desc_dma = k3_cppi_desc_pool_virt2dma(tx_chn->desc_pool, first_desc);
+
+	ret = k3_udma_glue_push_tx_chn(tx_chn->tx_chn, first_desc, desc_dma);
+	if (ret) {
+		netdev_err(ndev, "xdp tx: push failed: %d\n", ret);
+		goto drop_free_descs;
+	}
+
+	return ICSSG_XDP_TX;
+
+drop_free_descs:
+	prueth_xmit_free(tx_chn, first_desc);
+	return ICSSG_XDP_CONSUMED;
+}
+EXPORT_SYMBOL_GPL(emac_xmit_xdp_frame);
+
+/**
+ * emac_run_xdp - run an XDP program
+ * @emac: emac device
+ * @xdp: XDP buffer containing the frame
+ * @page: page with RX data if already DMA mapped
+ *
+ * Return: XDP state
+ */
+static int emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp,
+			struct page *page)
+{
+	int err, result = ICSSG_XDP_PASS;
+	struct bpf_prog *xdp_prog;
+	struct xdp_frame *xdpf;
+	int q_idx;
+	u32 act;
+
+	xdp_prog = READ_ONCE(emac->xdp_prog);
+
+	if (!xdp_prog)
+		return result;
+
+	act = bpf_prog_run_xdp(xdp_prog, xdp);
+	switch (act) {
+	case XDP_PASS:
+		break;
+	case XDP_TX:
+		/* Send packet to TX ring for immediate transmission */
+		xdpf = xdp_convert_buff_to_frame(xdp);
+		if (unlikely(!xdpf))
+			goto drop;
+
+		q_idx = smp_processor_id() % emac->tx_ch_num;
+		result = emac_xmit_xdp_frame(emac, xdpf, page, q_idx);
+		if (result == ICSSG_XDP_CONSUMED)
+			goto drop;
+		break;
+	case XDP_REDIRECT:
+		err = xdp_do_redirect(emac->ndev, xdp, xdp_prog);
+		if (err)
+			goto drop;
+		result = ICSSG_XDP_REDIR;
+		break;
+	default:
+		bpf_warn_invalid_xdp_action(emac->ndev, xdp_prog, act);
+		fallthrough;
+	case XDP_ABORTED:
+drop:
+		trace_xdp_exception(emac->ndev, xdp_prog, act);
+		fallthrough; /* handle aborts by dropping packet */
+	case XDP_DROP:
+		result = ICSSG_XDP_CONSUMED;
+		page_pool_recycle_direct(emac->rx_chns.pg_pool, page);
+		break;
+	}
+
+	return result;
+}
+
+static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id, int *xdp_state)
 {
 	struct prueth_rx_chn *rx_chn = &emac->rx_chns;
 	u32 buf_dma_len, pkt_len, port_id = 0;
@@ -560,10 +732,12 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
 	struct page *page, *new_page;
 	struct page_pool *pool;
 	struct sk_buff *skb;
+	struct xdp_buff xdp;
 	u32 *psdata;
 	void *pa;
 	int ret;
 
+	*xdp_state = 0;
 	pool = rx_chn->pg_pool;
 	ret = k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, flow_id, &desc_dma);
 	if (ret) {
@@ -602,8 +776,17 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
 		goto requeue;
 	}
 
-	/* prepare skb and send to n/w stack */
 	pa = page_address(page);
+	if (emac->xdp_prog) {
+		xdp_init_buff(&xdp, PAGE_SIZE, &rx_chn->xdp_rxq);
+		xdp_prepare_buff(&xdp, pa, PRUETH_HEADROOM, pkt_len, false);
+
+		*xdp_state = emac_run_xdp(emac, &xdp, page);
+		if (*xdp_state != ICSSG_XDP_PASS)
+			goto requeue;
+	}
+
+	/* prepare skb and send to n/w stack */
 	skb = build_skb(pa, prueth_rxbuf_total_len(pkt_len));
 	if (!skb) {
 		ndev->stats.rx_dropped++;
@@ -866,14 +1049,25 @@ static void prueth_tx_cleanup(void *data, dma_addr_t desc_dma)
 	struct prueth_tx_chn *tx_chn = data;
 	struct cppi5_host_desc_t *desc_tx;
 	struct prueth_swdata *swdata;
+	struct xdp_frame *xdpf;
 	struct sk_buff *skb;
 
 	desc_tx = k3_cppi_desc_pool_dma2virt(tx_chn->desc_pool, desc_dma);
 	swdata = cppi5_hdesc_get_swdata(desc_tx);
-	if (swdata->type == PRUETH_SWDATA_SKB) {
+
+	switch (swdata->type) {
+	case PRUETH_SWDATA_SKB:
 		skb = swdata->data.skb;
 		dev_kfree_skb_any(skb);
+		break;
+	case PRUETH_SWDATA_XDPF:
+		xdpf = swdata->data.xdpf;
+		xdp_return_frame(xdpf);
+		break;
+	default:
+		break;
 	}
+
 	prueth_xmit_free(tx_chn, desc_tx);
 }
 
@@ -908,15 +1102,18 @@ int icssg_napi_rx_poll(struct napi_struct *napi_rx, int budget)
 		PRUETH_RX_FLOW_DATA_SR1 : PRUETH_RX_FLOW_DATA;
 	int flow = emac->is_sr1 ?
 		PRUETH_MAX_RX_FLOWS_SR1 : PRUETH_MAX_RX_FLOWS;
+	int xdp_state_or = 0;
 	int num_rx = 0;
 	int cur_budget;
+	int xdp_state;
 	int ret;
 
 	while (flow--) {
 		cur_budget = budget - num_rx;
 
 		while (cur_budget--) {
-			ret = emac_rx_packet(emac, flow);
+			ret = emac_rx_packet(emac, flow, &xdp_state);
+			xdp_state_or |= xdp_state;
 			if (ret)
 				break;
 			num_rx++;
@@ -926,6 +1123,9 @@ int icssg_napi_rx_poll(struct napi_struct *napi_rx, int budget)
 			break;
 	}
 
+	if (xdp_state_or & ICSSG_XDP_REDIR)
+		xdp_do_flush();
+
 	if (num_rx < budget && napi_complete_done(napi_rx, num_rx)) {
 		if (unlikely(emac->rx_pace_timeout_ns)) {
 			hrtimer_start(&emac->rx_hrtimer,
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 30bcd6aa966e..6a45d3c132fb 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -559,6 +559,33 @@ const struct icss_iep_clockops prueth_iep_clockops = {
 	.perout_enable = prueth_perout_enable,
 };
 
+static int prueth_create_xdp_rxqs(struct prueth_emac *emac)
+{
+	struct xdp_rxq_info *rxq = &emac->rx_chns.xdp_rxq;
+	struct page_pool *pool = emac->rx_chns.pg_pool;
+	int ret;
+
+	ret = xdp_rxq_info_reg(rxq, emac->ndev, 0, rxq->napi_id);
+	if (ret)
+		return ret;
+
+	ret = xdp_rxq_info_reg_mem_model(rxq, MEM_TYPE_PAGE_POOL, pool);
+	if (ret)
+		xdp_rxq_info_unreg(rxq);
+
+	return ret;
+}
+
+static void prueth_destroy_xdp_rxqs(struct prueth_emac *emac)
+{
+	struct xdp_rxq_info *rxq = &emac->rx_chns.xdp_rxq;
+
+	if (!xdp_rxq_info_is_reg(rxq))
+		return;
+
+	xdp_rxq_info_unreg(rxq);
+}
+
 static int icssg_prueth_add_mcast(struct net_device *ndev, const u8 *addr)
 {
 	struct prueth_emac *emac = netdev_priv(ndev);
@@ -707,10 +734,14 @@ static int emac_ndo_open(struct net_device *ndev)
 	if (ret)
 		goto free_tx_ts_irq;
 
-	ret = k3_udma_glue_enable_rx_chn(emac->rx_chns.rx_chn);
+	ret = prueth_create_xdp_rxqs(emac);
 	if (ret)
 		goto reset_rx_chn;
 
+	ret = k3_udma_glue_enable_rx_chn(emac->rx_chns.rx_chn);
+	if (ret)
+		goto destroy_xdp_rxqs;
+
 	for (i = 0; i < emac->tx_ch_num; i++) {
 		ret = k3_udma_glue_enable_tx_chn(emac->tx_chns[i].tx_chn);
 		if (ret)
@@ -736,6 +767,8 @@ static int emac_ndo_open(struct net_device *ndev)
 	 * any SKB for completion. So set false to free_skb
 	 */
 	prueth_reset_tx_chan(emac, i, false);
+destroy_xdp_rxqs:
+	prueth_destroy_xdp_rxqs(emac);
 reset_rx_chn:
 	prueth_reset_rx_chan(&emac->rx_chns, max_rx_flows, false);
 free_tx_ts_irq:
@@ -807,6 +840,8 @@ static int emac_ndo_stop(struct net_device *ndev)
 
 	prueth_reset_rx_chan(&emac->rx_chns, max_rx_flows, true);
 
+	prueth_destroy_xdp_rxqs(emac);
+
 	napi_disable(&emac->napi_rx);
 	hrtimer_cancel(&emac->rx_hrtimer);
 
@@ -943,6 +978,85 @@ static int emac_ndo_vlan_rx_del_vid(struct net_device *ndev,
 	return 0;
 }
 
+/**
+ * emac_xdp_xmit - Implements ndo_xdp_xmit
+ * @dev: netdev
+ * @n: number of frames
+ * @frames: array of XDP buffer pointers
+ * @flags: XDP extra info
+ *
+ * Return: number of frames successfully sent. Failed frames
+ * will be free'ed by XDP core.
+ *
+ * For error cases, a negative errno code is returned and no-frames
+ * are transmitted (caller must handle freeing frames).
+ **/
+static int emac_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames,
+			 u32 flags)
+{
+	struct prueth_emac *emac = netdev_priv(dev);
+	unsigned int q_idx;
+	int nxmit = 0;
+	int i;
+
+	q_idx = smp_processor_id() % emac->tx_ch_num;
+
+	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
+		return -EINVAL;
+
+	for (i = 0; i < n; i++) {
+		struct xdp_frame *xdpf = frames[i];
+		int err;
+
+		err = emac_xmit_xdp_frame(emac, xdpf, NULL, q_idx);
+		if (err != ICSSG_XDP_TX)
+			break;
+		nxmit++;
+	}
+
+	return nxmit;
+}
+
+/**
+ * emac_xdp_setup - add/remove an XDP program
+ * @emac: emac device
+ * @bpf: XDP program
+ *
+ * Return: Always 0 (Success)
+ **/
+static int emac_xdp_setup(struct prueth_emac *emac, struct netdev_bpf *bpf)
+{
+	struct bpf_prog *prog = bpf->prog;
+
+	if (!emac->xdpi.prog && !prog)
+		return 0;
+
+	WRITE_ONCE(emac->xdp_prog, prog);
+
+	xdp_attachment_setup(&emac->xdpi, bpf);
+
+	return 0;
+}
+
+/**
+ * emac_ndo_bpf - implements ndo_bpf for icssg_prueth
+ * @ndev: network adapter device
+ * @bpf: XDP program
+ *
+ * Return: 0 on success, error code on failure.
+ **/
+static int emac_ndo_bpf(struct net_device *ndev, struct netdev_bpf *bpf)
+{
+	struct prueth_emac *emac = netdev_priv(ndev);
+
+	switch (bpf->command) {
+	case XDP_SETUP_PROG:
+		return emac_xdp_setup(emac, bpf);
+	default:
+		return -EINVAL;
+	}
+}
+
 static const struct net_device_ops emac_netdev_ops = {
 	.ndo_open = emac_ndo_open,
 	.ndo_stop = emac_ndo_stop,
@@ -957,6 +1071,8 @@ static const struct net_device_ops emac_netdev_ops = {
 	.ndo_fix_features = emac_ndo_fix_features,
 	.ndo_vlan_rx_add_vid = emac_ndo_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid = emac_ndo_vlan_rx_del_vid,
+	.ndo_bpf = emac_ndo_bpf,
+	.ndo_xdp_xmit = emac_xdp_xmit,
 };
 
 static int prueth_netdev_init(struct prueth *prueth,
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index 187c4e062a15..15b6a49bdb1a 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -8,6 +8,8 @@
 #ifndef __NET_TI_ICSSG_PRUETH_H
 #define __NET_TI_ICSSG_PRUETH_H
 
+#include <linux/bpf.h>
+#include <linux/bpf_trace.h>
 #include <linux/etherdevice.h>
 #include <linux/genalloc.h>
 #include <linux/if_vlan.h>
@@ -128,6 +130,7 @@ struct prueth_rx_chn {
 	unsigned int irq[ICSSG_MAX_RFLOWS];	/* separate irq per flow */
 	char name[32];
 	struct page_pool *pg_pool;
+	struct xdp_rxq_info xdp_rxq;
 };
 
 enum prueth_swdata_type {
@@ -135,16 +138,19 @@ enum prueth_swdata_type {
 	PRUETH_SWDATA_SKB,
 	PRUETH_SWDATA_PAGE,
 	PRUETH_SWDATA_CMD,
+	PRUETH_SWDATA_XDPF,
 };
 
 union prueth_data {
 	struct sk_buff *skb;
 	struct page *page;
 	u32 cmd;
+	struct xdp_frame *xdpf;
 };
 
 struct prueth_swdata {
 	union prueth_data data;
+	struct prueth_rx_chn *rx_chn;
 	enum prueth_swdata_type type;
 };
 
@@ -155,6 +161,12 @@ struct prueth_swdata {
 
 #define PRUETH_MAX_TX_TS_REQUESTS	50 /* Max simultaneous TX_TS requests */
 
+/* XDP BPF state */
+#define ICSSG_XDP_PASS           0
+#define ICSSG_XDP_CONSUMED       BIT(0)
+#define ICSSG_XDP_TX             BIT(1)
+#define ICSSG_XDP_REDIR          BIT(2)
+
 /* Minimum coalesce time in usecs for both Tx and Rx */
 #define ICSSG_MIN_COALESCE_USECS 20
 
@@ -221,6 +233,9 @@ struct prueth_emac {
 	/* RX IRQ Coalescing Related */
 	struct hrtimer rx_hrtimer;
 	unsigned long rx_pace_timeout_ns;
+
+	struct bpf_prog *xdp_prog;
+	struct xdp_attachment_info xdpi;
 };
 
 /* The buf includes headroom compatible with both skb and xdpf */
@@ -459,5 +474,9 @@ void prueth_put_cores(struct prueth *prueth, int slice);
 
 /* Revision specific helper */
 u64 icssg_ts_to_ns(u32 hi_sw, u32 hi, u32 lo, u32 cycle_time_ns);
+int emac_xmit_xdp_frame(struct prueth_emac *emac,
+			struct xdp_frame *xdpf,
+			struct page *page,
+			unsigned int q_idx);
 
 #endif /* __NET_TI_ICSSG_PRUETH_H */
-- 
2.25.1

Re: [PATCH net 3/3] net: ti: icssg-prueth: Add AF_XDP support

[Ido Schimmel]

s/AF_XDP/XDP/ ?

On Wed, Jan 22, 2025 at 06:19:51PM +0530, Meghana Malladi wrote:
> From: Roger Quadros <rogerq@kernel.org>
> 
> Add native XDP support. We do not support zero copy yet.

There are various XDP features (e.g., NETDEV_XDP_ACT_BASIC) to tell the
stack what the driver supports. I don't see any of them being set here.

> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>

[...]

> +static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id, int *xdp_state)
>  {
>  	struct prueth_rx_chn *rx_chn = &emac->rx_chns;
>  	u32 buf_dma_len, pkt_len, port_id = 0;
> @@ -560,10 +732,12 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
>  	struct page *page, *new_page;
>  	struct page_pool *pool;
>  	struct sk_buff *skb;
> +	struct xdp_buff xdp;
>  	u32 *psdata;
>  	void *pa;
>  	int ret;
>  
> +	*xdp_state = 0;
>  	pool = rx_chn->pg_pool;
>  	ret = k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, flow_id, &desc_dma);
>  	if (ret) {
> @@ -602,8 +776,17 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
>  		goto requeue;
>  	}
>  
> -	/* prepare skb and send to n/w stack */
>  	pa = page_address(page);
> +	if (emac->xdp_prog) {
> +		xdp_init_buff(&xdp, PAGE_SIZE, &rx_chn->xdp_rxq);
> +		xdp_prepare_buff(&xdp, pa, PRUETH_HEADROOM, pkt_len, false);
> +
> +		*xdp_state = emac_run_xdp(emac, &xdp, page);
> +		if (*xdp_state != ICSSG_XDP_PASS)
> +			goto requeue;
> +	}
> +
> +	/* prepare skb and send to n/w stack */
>  	skb = build_skb(pa, prueth_rxbuf_total_len(pkt_len));
>  	if (!skb) {
>  		ndev->stats.rx_dropped++;

XDP program could have changed the packet length, but driver seems to be
building the skb using original length read from the descriptor.
Consider using xdp_build_skb_from_buff()

Re: [EXTERNAL] Re: [PATCH net 3/3] net: ti: icssg-prueth: Add AF_XDP support

[Malladi, Meghana]

On 1/23/2025 10:55 PM, Ido Schimmel wrote:
> s/AF_XDP/XDP/ ? On Wed, Jan 22, 2025 at 06: 19: 51PM +0530, Meghana 
> Malladi wrote: > From: Roger Quadros <rogerq@ kernel. org> > > Add 
> native XDP support. We do not support zero copy yet. There are various 
> XDP features (e. g. , NETDEV_XDP_ACT_BASIC)
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source 
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK! 
> v9dnXbjPnQP15quJdjhJcrR0CoEiGINyhi1SO3vz-ZB8Sgif7YzLnG8ayC2XmAQz6g$>
> ZjQcmQRYFpfptBannerEnd
> 
> s/AF_XDP/XDP/ ?
> 

Will fix the typo.

> On Wed, Jan 22, 2025 at 06:19:51PM +0530, Meghana Malladi wrote:
>> From: Roger Quadros <rogerq@kernel.org>
>> 
>> Add native XDP support. We do not support zero copy yet.
> 
> There are various XDP features (e.g., NETDEV_XDP_ACT_BASIC) to tell the
> stack what the driver supports. I don't see any of them being set here.
> 

Ok, I will add the feature flags in the v2 patch series.

>> 
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> 
> [...]
> 
>> +static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id, int *xdp_state)
>>  {
>>  	struct prueth_rx_chn *rx_chn = &emac->rx_chns;
>>  	u32 buf_dma_len, pkt_len, port_id = 0;
>> @@ -560,10 +732,12 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
>>  	struct page *page, *new_page;
>>  	struct page_pool *pool;
>>  	struct sk_buff *skb;
>> +	struct xdp_buff xdp;
>>  	u32 *psdata;
>>  	void *pa;
>>  	int ret;
>>  
>> +	*xdp_state = 0;
>>  	pool = rx_chn->pg_pool;
>>  	ret = k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, flow_id, &desc_dma);
>>  	if (ret) {
>> @@ -602,8 +776,17 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
>>  		goto requeue;
>>  	}
>>  
>> -	/* prepare skb and send to n/w stack */
>>  	pa = page_address(page);
>> +	if (emac->xdp_prog) {
>> +		xdp_init_buff(&xdp, PAGE_SIZE, &rx_chn->xdp_rxq);
>> +		xdp_prepare_buff(&xdp, pa, PRUETH_HEADROOM, pkt_len, false);
>> +
>> +		*xdp_state = emac_run_xdp(emac, &xdp, page);
>> +		if (*xdp_state != ICSSG_XDP_PASS)
>> +			goto requeue;
>> +	}
>> +
>> +	/* prepare skb and send to n/w stack */
>>  	skb = build_skb(pa, prueth_rxbuf_total_len(pkt_len));
>>  	if (!skb) {
>>  		ndev->stats.rx_dropped++;
> 
> XDP program could have changed the packet length, but driver seems to be

This will be true given, emac->xdp_prog is not NULL. What about when XDP 
is not enabled ?

> building the skb using original length read from the descriptor.
> Consider using xdp_build_skb_from_buff()
> 

Re: [EXTERNAL] Re: [PATCH net 3/3] net: ti: icssg-prueth: Add AF_XDP support

[Ido Schimmel]

On Tue, Feb 04, 2025 at 11:25:39PM +0530, Malladi, Meghana wrote:
> On 1/23/2025 10:55 PM, Ido Schimmel wrote:
> > XDP program could have changed the packet length, but driver seems to be
> 
> This will be true given, emac->xdp_prog is not NULL. What about when XDP is
> not enabled ?

I don't understand the question. My point is that the packet doesn't
necessarily look the same after XDP ran.

> 
> > building the skb using original length read from the descriptor.
> > Consider using xdp_build_skb_from_buff()
> > 
> 

Re: [EXTERNAL] Re: [PATCH net 3/3] net: ti: icssg-prueth: Add AF_XDP support

[Malladi, Meghana]

On 2/5/2025 11:16 PM, Ido Schimmel wrote:
> On Tue, Feb 04, 2025 at 11: 25: 39PM +0530, Malladi, Meghana wrote: > On 
> 1/23/2025 10: 55 PM, Ido Schimmel wrote: > > XDP program could have 
> changed the packet length, but driver seems to be > > This will be true 
> given, emac->xdp_prog
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source 
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK! 
> v9dnXdhkNoe0hkqlFZKoUhMAqXZwolk5zXhypw1qeZY8pxUHTuuleZBOKulyBnK9eA$>
> ZjQcmQRYFpfptBannerEnd
> 
> On Tue, Feb 04, 2025 at 11:25:39PM +0530, Malladi, Meghana wrote:
>> On 1/23/2025 10:55 PM, Ido Schimmel wrote:
>> > XDP program could have changed the packet length, but driver seems to be
>> 
>> This will be true given, emac->xdp_prog is not NULL. What about when XDP is
>> not enabled ?
> 
> I don't understand the question. My point is that the packet doesn't
> necessarily look the same after XDP ran.
> 

emac_rx_packet() is a common function for both XDP and non-XDP use 
cases. XDP will only run when emac->xdp_prog is not NULL. I understand 
that when XDP ran, it can change the contents of the packet hence it is 
advisable to use "xdp_build_skb_from_buff(const struct xdp_buff *xdp)", 
but for cases when xdp doesn't run - the xdp struct has junk/zero value 
which cannot be converted into some valid skb. But I think I will do 
something like this:

if (emac->xdp_prog)
	xdp_build_skb_from_buff(xdp);
else
	skb = napi_build_skb(pa, PAGE_SIZE);

Hope this will address your comment.

>> 
>> > building the skb using original length read from the descriptor.
>> > Consider using xdp_build_skb_from_buff()
>> > 
>> 
> 

Re: [PATCH net 0/3] Add native mode XDP support

[Simon Horman]

On Wed, Jan 22, 2025 at 06:19:48PM +0530, Meghana Malladi wrote:
> This series adds native XDP support using page_pool.
> XDP zero copy support is not included in this patch series.
> 
> Patch 1/3: Replaces skb with page pool for Rx buffer allocation
> Patch 2/3: Adds prueth_swdata struct for SWDATA for all swdata cases
> Patch 3/3: Introduces native mode XDP support

Hi Meghana,

Unless I am mistaken this patchset should be targeted at net-next,
as a new feature, rather than net, as bug fixes.

With that in mind:

## Form letter - net-next-closed

The merge window for v6.14 has begun. Therefore net-next is closed
for new drivers, features, code refactoring and optimizations.
We are currently accepting bug fixes only.

Please repost when net-next reopens after Feb 3rd.

RFC patches sent for review only are obviously welcome at any time.

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle

--
pw-bot: deferred

Re: [PATCH net 0/3] Add native mode XDP support

[Meghana Malladi]

On 22/01/25 18:43, Simon Horman wrote:
> On Wed, Jan 22, 2025 at 06:19:48PM +0530, Meghana Malladi wrote:
>> This series adds native XDP support using page_pool.
>> XDP zero copy support is not included in this patch series.
>>
>> Patch 1/3: Replaces skb with page pool for Rx buffer allocation
>> Patch 2/3: Adds prueth_swdata struct for SWDATA for all swdata cases
>> Patch 3/3: Introduces native mode XDP support
> 
> Hi Meghana,
> 
> Unless I am mistaken this patchset should be targeted at net-next,
> as a new feature, rather than net, as bug fixes.
> 

Yes Indeed. Thanks for pointing this to me. I will be more vigilant next 
time I am posting to avoid such mistakes. :)

> With that in mind:
> 
> ## Form letter - net-next-closed
> 
> The merge window for v6.14 has begun. Therefore net-next is closed
> for new drivers, features, code refactoring and optimizations.
> We are currently accepting bug fixes only.
> 
> Please repost when net-next reopens after Feb 3rd.
> 

My bad for not checking this before posting. I will re-post it after Feb 
3rd. Thanks a lot.

> RFC patches sent for review only are obviously welcome at any time.
> 
> See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
> 
> --
> pw-bot: deferred