diff mbox series

[net-next,v5,1/6] octeontx2-pf: use xdp_return_frame() to free xdp buffers

Message ID 20250206085034.1978172-2-sumang@marvell.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add af_xdp support for cn10k | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 15 of 15 maintainers
netdev/build_clang fail Errors and warnings before: 20 this patch: 21
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 9 this patch: 10
netdev/checkpatch warning WARNING: line length of 87 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Suman Ghosh Feb. 6, 2025, 8:50 a.m. UTC 
xdp_return_frames() will help to free the xdp frames and their
associated pages back to page pool.

Signed-off-by: Geetha sowjanya <gakula@marvell.com>
Signed-off-by: Suman Ghosh <sumang@marvell.com>
---
 .../marvell/octeontx2/nic/otx2_common.h       |  4 +-
 .../ethernet/marvell/octeontx2/nic/otx2_pf.c  |  7 ++-
 .../marvell/octeontx2/nic/otx2_txrx.c         | 49 ++++++++++++-------
 .../marvell/octeontx2/nic/otx2_txrx.h         |  1 +
 4 files changed, 39 insertions(+), 22 deletions(-)

Comments

Simon Horman Feb. 10, 2025, 4:25 p.m. UTC | #1 
On Thu, Feb 06, 2025 at 02:20:29PM +0530, Suman Ghosh wrote:
> xdp_return_frames() will help to free the xdp frames and their
> associated pages back to page pool.
> 
> Signed-off-by: Geetha sowjanya <gakula@marvell.com>
> Signed-off-by: Suman Ghosh <sumang@marvell.com>

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> index 224cef938927..d46f05993d3f 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> @@ -96,20 +96,22 @@ static unsigned int frag_num(unsigned int i)
>  
>  static void otx2_xdp_snd_pkt_handler(struct otx2_nic *pfvf,
>  				     struct otx2_snd_queue *sq,
> -				 struct nix_cqe_tx_s *cqe)
> +				     struct nix_cqe_tx_s *cqe)
>  {
>  	struct nix_send_comp_s *snd_comp = &cqe->comp;
>  	struct sg_list *sg;
>  	struct page *page;
> -	u64 pa;
> +	u64 pa, iova;
>  
>  	sg = &sq->sg[snd_comp->sqe_id];
>  
> -	pa = otx2_iova_to_phys(pfvf->iommu_domain, sg->dma_addr[0]);
> -	otx2_dma_unmap_page(pfvf, sg->dma_addr[0],
> -			    sg->size[0], DMA_TO_DEVICE);
> +	iova = sg->dma_addr[0];
> +	pa = otx2_iova_to_phys(pfvf->iommu_domain, iova);
>  	page = virt_to_page(phys_to_virt(pa));
> -	put_page(page);

Hi Suman,

With this patch applied page is assigned but otherwise unused in this
function. So unless there are some side effects of the above, I think
page and in turn pa and iova can be removed.

> +	if (sg->flags & XDP_REDIRECT)
> +		otx2_dma_unmap_page(pfvf, sg->dma_addr[0], sg->size[0], DMA_TO_DEVICE);
> +	xdp_return_frame((struct xdp_frame *)sg->skb);
> +	sg->skb = (u64)NULL;
>  }
>  
>  static void otx2_snd_pkt_handler(struct otx2_nic *pfvf,

...
Simon Horman Feb. 10, 2025, 5:53 p.m. UTC | #2 
On Mon, Feb 10, 2025 at 04:25:43PM +0000, Simon Horman wrote:
> On Thu, Feb 06, 2025 at 02:20:29PM +0530, Suman Ghosh wrote:
> > xdp_return_frames() will help to free the xdp frames and their
> > associated pages back to page pool.
> > 
> > Signed-off-by: Geetha sowjanya <gakula@marvell.com>
> > Signed-off-by: Suman Ghosh <sumang@marvell.com>
> 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> > index 224cef938927..d46f05993d3f 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> > @@ -96,20 +96,22 @@ static unsigned int frag_num(unsigned int i)
> >  
> >  static void otx2_xdp_snd_pkt_handler(struct otx2_nic *pfvf,
> >  				     struct otx2_snd_queue *sq,
> > -				 struct nix_cqe_tx_s *cqe)
> > +				     struct nix_cqe_tx_s *cqe)
> >  {
> >  	struct nix_send_comp_s *snd_comp = &cqe->comp;
> >  	struct sg_list *sg;
> >  	struct page *page;
> > -	u64 pa;
> > +	u64 pa, iova;
> >  
> >  	sg = &sq->sg[snd_comp->sqe_id];
> >  
> > -	pa = otx2_iova_to_phys(pfvf->iommu_domain, sg->dma_addr[0]);
> > -	otx2_dma_unmap_page(pfvf, sg->dma_addr[0],
> > -			    sg->size[0], DMA_TO_DEVICE);
> > +	iova = sg->dma_addr[0];
> > +	pa = otx2_iova_to_phys(pfvf->iommu_domain, iova);
> >  	page = virt_to_page(phys_to_virt(pa));
> > -	put_page(page);
> 
> Hi Suman,
> 
> With this patch applied page is assigned but otherwise unused in this
> function. So unless there are some side effects of the above, I think
> page and in turn pa and iova can be removed.

I now see that page and pa are removed in patch 6/6, although iova
is left behind. I think it would be best to move the cleanup forward
to this patch.

> 
> > +	if (sg->flags & XDP_REDIRECT)
> > +		otx2_dma_unmap_page(pfvf, sg->dma_addr[0], sg->size[0], DMA_TO_DEVICE);
> > +	xdp_return_frame((struct xdp_frame *)sg->skb);
> > +	sg->skb = (u64)NULL;
> >  }
> >  
> >  static void otx2_snd_pkt_handler(struct otx2_nic *pfvf,
> 
> ...
>
Simon Horman Feb. 10, 2025, 5:54 p.m. UTC | #3 
On Thu, Feb 06, 2025 at 02:20:29PM +0530, Suman Ghosh wrote:
> xdp_return_frames() will help to free the xdp frames and their
> associated pages back to page pool.
> 
> Signed-off-by: Geetha sowjanya <gakula@marvell.com>
> Signed-off-by: Suman Ghosh <sumang@marvell.com>

...

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c

...

> @@ -1465,10 +1476,14 @@ static bool otx2_xdp_rcv_pkt_handler(struct otx2_nic *pfvf,
>  		trace_xdp_exception(pfvf->netdev, prog, act);
>  		break;
>  	case XDP_DROP:
> +		cq->pool_ptrs++;
> +		if (page->pp) {
> +			page_pool_recycle_direct(pool->page_pool, page);
> +			return true;
> +		}
>  		otx2_dma_unmap_page(pfvf, iova, pfvf->rbsize,
>  				    DMA_FROM_DEVICE);
>  		put_page(page);
> -		cq->pool_ptrs++;
>  		return true;

The above seems to get shuffled around in the next patch anyway, so
maybe it's best to do this here (completely untested):

	case XDP_DROP:
		cq->pool_ptrs++;
		if (page->pp) {
			page_pool_recycle_direct(pool->page_pool, page);
		} else {
			otx2_dma_unmap_page(pfvf, iova, pfvf->rbsize,
					    DMA_FROM_DEVICE);
			put_page(page);
		}
		return true;

>  	}
>  	return false;

...
Suman Ghosh Feb. 12, 2025, 6:44 a.m. UTC | #4 
>>  static void otx2_xdp_snd_pkt_handler(struct otx2_nic *pfvf,
>>  				     struct otx2_snd_queue *sq,
>> -				 struct nix_cqe_tx_s *cqe)
>> +				     struct nix_cqe_tx_s *cqe)
>>  {
>>  	struct nix_send_comp_s *snd_comp = &cqe->comp;
>>  	struct sg_list *sg;
>>  	struct page *page;
>> -	u64 pa;
>> +	u64 pa, iova;
>>
>>  	sg = &sq->sg[snd_comp->sqe_id];
>>
>> -	pa = otx2_iova_to_phys(pfvf->iommu_domain, sg->dma_addr[0]);
>> -	otx2_dma_unmap_page(pfvf, sg->dma_addr[0],
>> -			    sg->size[0], DMA_TO_DEVICE);
>> +	iova = sg->dma_addr[0];
>> +	pa = otx2_iova_to_phys(pfvf->iommu_domain, iova);
>>  	page = virt_to_page(phys_to_virt(pa));
>> -	put_page(page);
>
>Hi Suman,
>
>With this patch applied page is assigned but otherwise unused in this
>function. So unless there are some side effects of the above, I think
>page and in turn pa and iova can be removed.
[Suman] ack, will update in v6
>
>> +	if (sg->flags & XDP_REDIRECT)
>> +		otx2_dma_unmap_page(pfvf, sg->dma_addr[0], sg->size[0],
>DMA_TO_DEVICE);
>> +	xdp_return_frame((struct xdp_frame *)sg->skb);
>> +	sg->skb = (u64)NULL;
>>  }
>>
>>  static void otx2_snd_pkt_handler(struct otx2_nic *pfvf,
>
>...
Suman Ghosh Feb. 12, 2025, 6:45 a.m. UTC | #5 
>> > -	u64 pa;
>> > +	u64 pa, iova;
>> >
>> >  	sg = &sq->sg[snd_comp->sqe_id];
>> >
>> > -	pa = otx2_iova_to_phys(pfvf->iommu_domain, sg->dma_addr[0]);
>> > -	otx2_dma_unmap_page(pfvf, sg->dma_addr[0],
>> > -			    sg->size[0], DMA_TO_DEVICE);
>> > +	iova = sg->dma_addr[0];
>> > +	pa = otx2_iova_to_phys(pfvf->iommu_domain, iova);
>> >  	page = virt_to_page(phys_to_virt(pa));
>> > -	put_page(page);
>>
>> Hi Suman,
>>
>> With this patch applied page is assigned but otherwise unused in this
>> function. So unless there are some side effects of the above, I think
>> page and in turn pa and iova can be removed.
>
>I now see that page and pa are removed in patch 6/6, although iova is
>left behind. I think it would be best to move the cleanup forward to
>this patch.
[Suman] ack, will update in v6
Suman Ghosh Feb. 12, 2025, 6:52 a.m. UTC | #6 
>> @@ -1465,10 +1476,14 @@ static bool otx2_xdp_rcv_pkt_handler(struct
>otx2_nic *pfvf,
>>  		trace_xdp_exception(pfvf->netdev, prog, act);
>>  		break;
>>  	case XDP_DROP:
>> +		cq->pool_ptrs++;
>> +		if (page->pp) {
>> +			page_pool_recycle_direct(pool->page_pool, page);
>> +			return true;
>> +		}
>>  		otx2_dma_unmap_page(pfvf, iova, pfvf->rbsize,
>>  				    DMA_FROM_DEVICE);
>>  		put_page(page);
>> -		cq->pool_ptrs++;
>>  		return true;
>
>The above seems to get shuffled around in the next patch anyway, so
>maybe it's best to do this here (completely untested):
>
>	case XDP_DROP:
>		cq->pool_ptrs++;
>		if (page->pp) {
>			page_pool_recycle_direct(pool->page_pool, page);
>		} else {
>			otx2_dma_unmap_page(pfvf, iova, pfvf->rbsize,
>					    DMA_FROM_DEVICE);
>			put_page(page);
>		}
>		return true;
>
>>  	}
>>  	return false;
[Suman] ack, will update in v6
>
>...
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index 65814e3dc93f..d5fbccb289df 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -21,6 +21,7 @@ 
 #include <linux/time64.h>
 #include <linux/dim.h>
 #include <uapi/linux/if_macsec.h>
+#include <net/page_pool/helpers.h>
 
 #include <mbox.h>
 #include <npc.h>
@@ -1094,7 +1095,8 @@  int otx2_del_macfilter(struct net_device *netdev, const u8 *mac);
 int otx2_add_macfilter(struct net_device *netdev, const u8 *mac);
 int otx2_enable_rxvlan(struct otx2_nic *pf, bool enable);
 int otx2_install_rxvlan_offload_flow(struct otx2_nic *pfvf);
-bool otx2_xdp_sq_append_pkt(struct otx2_nic *pfvf, u64 iova, int len, u16 qidx);
+bool otx2_xdp_sq_append_pkt(struct otx2_nic *pfvf, struct xdp_frame *xdpf,
+			    u64 iova, int len, u16 qidx, u16 flags);
 u16 otx2_get_max_mtu(struct otx2_nic *pfvf);
 int otx2_handle_ntuple_tc_features(struct net_device *netdev,
 				   netdev_features_t features);
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index e1dde93e8af8..4347a3c95350 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -2691,7 +2691,6 @@  static int otx2_get_vf_config(struct net_device *netdev, int vf,
 static int otx2_xdp_xmit_tx(struct otx2_nic *pf, struct xdp_frame *xdpf,
 			    int qidx)
 {
-	struct page *page;
 	u64 dma_addr;
 	int err = 0;
 
@@ -2701,11 +2700,11 @@  static int otx2_xdp_xmit_tx(struct otx2_nic *pf, struct xdp_frame *xdpf,
 	if (dma_mapping_error(pf->dev, dma_addr))
 		return -ENOMEM;
 
-	err = otx2_xdp_sq_append_pkt(pf, dma_addr, xdpf->len, qidx);
+	err = otx2_xdp_sq_append_pkt(pf, xdpf, dma_addr, xdpf->len,
+				     qidx, XDP_REDIRECT);
 	if (!err) {
 		otx2_dma_unmap_page(pf, dma_addr, xdpf->len, DMA_TO_DEVICE);
-		page = virt_to_page(xdpf->data);
-		put_page(page);
+		xdp_return_frame(xdpf);
 		return -ENOMEM;
 	}
 	return 0;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
index 224cef938927..d46f05993d3f 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
@@ -96,20 +96,22 @@  static unsigned int frag_num(unsigned int i)
 
 static void otx2_xdp_snd_pkt_handler(struct otx2_nic *pfvf,
 				     struct otx2_snd_queue *sq,
-				 struct nix_cqe_tx_s *cqe)
+				     struct nix_cqe_tx_s *cqe)
 {
 	struct nix_send_comp_s *snd_comp = &cqe->comp;
 	struct sg_list *sg;
 	struct page *page;
-	u64 pa;
+	u64 pa, iova;
 
 	sg = &sq->sg[snd_comp->sqe_id];
 
-	pa = otx2_iova_to_phys(pfvf->iommu_domain, sg->dma_addr[0]);
-	otx2_dma_unmap_page(pfvf, sg->dma_addr[0],
-			    sg->size[0], DMA_TO_DEVICE);
+	iova = sg->dma_addr[0];
+	pa = otx2_iova_to_phys(pfvf->iommu_domain, iova);
 	page = virt_to_page(phys_to_virt(pa));
-	put_page(page);
+	if (sg->flags & XDP_REDIRECT)
+		otx2_dma_unmap_page(pfvf, sg->dma_addr[0], sg->size[0], DMA_TO_DEVICE);
+	xdp_return_frame((struct xdp_frame *)sg->skb);
+	sg->skb = (u64)NULL;
 }
 
 static void otx2_snd_pkt_handler(struct otx2_nic *pfvf,
@@ -1359,8 +1361,9 @@  void otx2_free_pending_sqe(struct otx2_nic *pfvf)
 	}
 }
 
-static void otx2_xdp_sqe_add_sg(struct otx2_snd_queue *sq, u64 dma_addr,
-				int len, int *offset)
+static void otx2_xdp_sqe_add_sg(struct otx2_snd_queue *sq,
+				struct xdp_frame *xdpf,
+				u64 dma_addr, int len, int *offset, u16 flags)
 {
 	struct nix_sqe_sg_s *sg = NULL;
 	u64 *iova = NULL;
@@ -1377,9 +1380,12 @@  static void otx2_xdp_sqe_add_sg(struct otx2_snd_queue *sq, u64 dma_addr,
 	sq->sg[sq->head].dma_addr[0] = dma_addr;
 	sq->sg[sq->head].size[0] = len;
 	sq->sg[sq->head].num_segs = 1;
+	sq->sg[sq->head].flags = flags;
+	sq->sg[sq->head].skb = (u64)xdpf;
 }
 
-bool otx2_xdp_sq_append_pkt(struct otx2_nic *pfvf, u64 iova, int len, u16 qidx)
+bool otx2_xdp_sq_append_pkt(struct otx2_nic *pfvf, struct xdp_frame *xdpf,
+			    u64 iova, int len, u16 qidx, u16 flags)
 {
 	struct nix_sqe_hdr_s *sqe_hdr;
 	struct otx2_snd_queue *sq;
@@ -1405,7 +1411,7 @@  bool otx2_xdp_sq_append_pkt(struct otx2_nic *pfvf, u64 iova, int len, u16 qidx)
 
 	offset = sizeof(*sqe_hdr);
 
-	otx2_xdp_sqe_add_sg(sq, iova, len, &offset);
+	otx2_xdp_sqe_add_sg(sq, xdpf, iova, len, &offset, flags);
 	sqe_hdr->sizem1 = (offset / 16) - 1;
 	pfvf->hw_ops->sqe_flush(pfvf, sq, offset, qidx);
 
@@ -1419,6 +1425,8 @@  static bool otx2_xdp_rcv_pkt_handler(struct otx2_nic *pfvf,
 				     bool *need_xdp_flush)
 {
 	unsigned char *hard_start;
+	struct otx2_pool *pool;
+	struct xdp_frame *xdpf;
 	int qidx = cq->cq_idx;
 	struct xdp_buff xdp;
 	struct page *page;
@@ -1426,6 +1434,7 @@  static bool otx2_xdp_rcv_pkt_handler(struct otx2_nic *pfvf,
 	u32 act;
 	int err;
 
+	pool = &pfvf->qset.pool[qidx];
 	iova = cqe->sg.seg_addr - OTX2_HEAD_ROOM;
 	pa = otx2_iova_to_phys(pfvf->iommu_domain, iova);
 	page = virt_to_page(phys_to_virt(pa));
@@ -1444,19 +1453,21 @@  static bool otx2_xdp_rcv_pkt_handler(struct otx2_nic *pfvf,
 	case XDP_TX:
 		qidx += pfvf->hw.tx_queues;
 		cq->pool_ptrs++;
-		return otx2_xdp_sq_append_pkt(pfvf, iova,
-					      cqe->sg.seg_size, qidx);
+		xdpf = xdp_convert_buff_to_frame(&xdp);
+		return otx2_xdp_sq_append_pkt(pfvf, xdpf, cqe->sg.seg_addr,
+					      cqe->sg.seg_size, qidx, XDP_TX);
 	case XDP_REDIRECT:
 		cq->pool_ptrs++;
 		err = xdp_do_redirect(pfvf->netdev, &xdp, prog);
-
-		otx2_dma_unmap_page(pfvf, iova, pfvf->rbsize,
-				    DMA_FROM_DEVICE);
 		if (!err) {
 			*need_xdp_flush = true;
 			return true;
 		}
-		put_page(page);
+
+		otx2_dma_unmap_page(pfvf, iova, pfvf->rbsize,
+				    DMA_FROM_DEVICE);
+		xdpf = xdp_convert_buff_to_frame(&xdp);
+		xdp_return_frame(xdpf);
 		break;
 	default:
 		bpf_warn_invalid_xdp_action(pfvf->netdev, prog, act);
@@ -1465,10 +1476,14 @@  static bool otx2_xdp_rcv_pkt_handler(struct otx2_nic *pfvf,
 		trace_xdp_exception(pfvf->netdev, prog, act);
 		break;
 	case XDP_DROP:
+		cq->pool_ptrs++;
+		if (page->pp) {
+			page_pool_recycle_direct(pool->page_pool, page);
+			return true;
+		}
 		otx2_dma_unmap_page(pfvf, iova, pfvf->rbsize,
 				    DMA_FROM_DEVICE);
 		put_page(page);
-		cq->pool_ptrs++;
 		return true;
 	}
 	return false;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.h
index d23810963fdb..92e1e84cad75 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.h
@@ -76,6 +76,7 @@  struct otx2_rcv_queue {
 
 struct sg_list {
 	u16	num_segs;
+	u16	flags;
 	u64	skb;
 	u64	size[OTX2_MAX_FRAGS_IN_SQE];
 	u64	dma_addr[OTX2_MAX_FRAGS_IN_SQE];