diff mbox series

[net-next,v5,4/5] mvpp2: recycle buffers

Message ID 20210513165846.23722-5-mcroce@linux.microsoft.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series page_pool: recycle buffers | expand

Checks

Context Check Description
netdev/apply fail Patch does not apply to net-next
netdev/tree_selection success Clearly marked for net-next

Commit Message

Matteo Croce May 13, 2021, 4:58 p.m. UTC 
From: Matteo Croce <mcroce@microsoft.com>

Use the new recycling API for page_pool.
In a drop rate test, the packet rate is more than doubled,
from 962 Kpps to 2047 Kpps.

perf top on a stock system shows:

Overhead  Shared Object     Symbol
  30.67%  [kernel]          [k] page_pool_release_page
   8.37%  [kernel]          [k] get_page_from_freelist
   7.34%  [kernel]          [k] free_unref_page
   6.47%  [mvpp2]           [k] mvpp2_rx
   4.69%  [kernel]          [k] eth_type_trans
   4.55%  [kernel]          [k] __netif_receive_skb_core
   4.40%  [kernel]          [k] build_skb
   4.29%  [kernel]          [k] kmem_cache_free
   4.00%  [kernel]          [k] kmem_cache_alloc
   3.81%  [kernel]          [k] dev_gro_receive

With packet rate stable at 962 Kpps:

tx: 0 bps 0 pps rx: 477.4 Mbps 962.6 Kpps
tx: 0 bps 0 pps rx: 477.6 Mbps 962.8 Kpps
tx: 0 bps 0 pps rx: 477.6 Mbps 962.9 Kpps
tx: 0 bps 0 pps rx: 477.2 Mbps 962.1 Kpps
tx: 0 bps 0 pps rx: 477.5 Mbps 962.7 Kpps

And this is the same output with recycling enabled:

Overhead  Shared Object     Symbol
  12.75%  [mvpp2]           [k] mvpp2_rx
   9.56%  [kernel]          [k] __netif_receive_skb_core
   9.29%  [kernel]          [k] build_skb
   9.27%  [kernel]          [k] eth_type_trans
   8.39%  [kernel]          [k] kmem_cache_alloc
   7.85%  [kernel]          [k] kmem_cache_free
   7.36%  [kernel]          [k] page_pool_put_page
   6.45%  [kernel]          [k] dev_gro_receive
   4.72%  [kernel]          [k] __xdp_return
   3.06%  [kernel]          [k] page_pool_refill_alloc_cache

With packet rate above 2000 Kpps:

tx: 0 bps 0 pps rx: 1015 Mbps 2046 Kpps
tx: 0 bps 0 pps rx: 1015 Mbps 2047 Kpps
tx: 0 bps 0 pps rx: 1015 Mbps 2047 Kpps
tx: 0 bps 0 pps rx: 1015 Mbps 2047 Kpps
tx: 0 bps 0 pps rx: 1015 Mbps 2047 Kpps

The major performance increase is explained by the fact that the most CPU
consuming functions (page_pool_release_page, get_page_from_freelist
and free_unref_page) are no longer called on a per packet basis.

The test was done by sending to the macchiatobin 64 byte ethernet frames
with an invalid ethertype, so the packets are dropped early in the RX path.

Signed-off-by: Matteo Croce <mcroce@microsoft.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

Comments

Russell King (Oracle) May 13, 2021, 6:20 p.m. UTC | #1 
On Thu, May 13, 2021 at 06:58:45PM +0200, Matteo Croce wrote:
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index b2259bf1d299..9dceabece56c 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -3847,6 +3847,7 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
>  	struct mvpp2_pcpu_stats ps = {};
>  	enum dma_data_direction dma_dir;
>  	struct bpf_prog *xdp_prog;
> +	struct xdp_rxq_info *rxqi;
>  	struct xdp_buff xdp;
>  	int rx_received;
>  	int rx_done = 0;
> @@ -3912,15 +3913,15 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
>  		else
>  			frag_size = bm_pool->frag_size;
>  
> +		if (bm_pool->pkt_size == MVPP2_BM_SHORT_PKT_SIZE)
> +			rxqi = &rxq->xdp_rxq_short;
> +		else
> +			rxqi = &rxq->xdp_rxq_long;
>  
> +		if (xdp_prog) {
> +			xdp.rxq = rxqi;
>  
> +			xdp_init_buff(&xdp, PAGE_SIZE, rxqi);
>  			xdp_prepare_buff(&xdp, data,
>  					 MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM,
>  					 rx_bytes, false);
> @@ -3964,7 +3965,7 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
>  		}
>  
>  		if (pp)
> +			skb_mark_for_recycle(skb, virt_to_page(data), pp);
>  		else
>  			dma_unmap_single_attrs(dev->dev.parent, dma_addr,
>  					       bm_pool->buf_size, DMA_FROM_DEVICE,

Looking at the above, which I've only quoted the _resulting_ code after
your patch above, I don't see why you have moved the
"bm_pool->pkt_size == MVPP2_BM_SHORT_PKT_SIZE" conditional outside of
the test for xdp_prog - I don't see rxqi being used except within that
conditional. Please can you explain the reasoning there?

Thanks.
Matteo Croce May 13, 2021, 11:52 p.m. UTC | #2 
On Thu, May 13, 2021 at 8:21 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Thu, May 13, 2021 at 06:58:45PM +0200, Matteo Croce wrote:
> > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > index b2259bf1d299..9dceabece56c 100644
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -3847,6 +3847,7 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
> >       struct mvpp2_pcpu_stats ps = {};
> >       enum dma_data_direction dma_dir;
> >       struct bpf_prog *xdp_prog;
> > +     struct xdp_rxq_info *rxqi;
> >       struct xdp_buff xdp;
> >       int rx_received;
> >       int rx_done = 0;
> > @@ -3912,15 +3913,15 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
> >               else
> >                       frag_size = bm_pool->frag_size;
> >
> > +             if (bm_pool->pkt_size == MVPP2_BM_SHORT_PKT_SIZE)
> > +                     rxqi = &rxq->xdp_rxq_short;
> > +             else
> > +                     rxqi = &rxq->xdp_rxq_long;
> >
> > +             if (xdp_prog) {
> > +                     xdp.rxq = rxqi;
> >
> > +                     xdp_init_buff(&xdp, PAGE_SIZE, rxqi);
> >                       xdp_prepare_buff(&xdp, data,
> >                                        MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM,
> >                                        rx_bytes, false);
> > @@ -3964,7 +3965,7 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
> >               }
> >
> >               if (pp)
> > +                     skb_mark_for_recycle(skb, virt_to_page(data), pp);
> >               else
> >                       dma_unmap_single_attrs(dev->dev.parent, dma_addr,
> >                                              bm_pool->buf_size, DMA_FROM_DEVICE,
>
> Looking at the above, which I've only quoted the _resulting_ code after
> your patch above, I don't see why you have moved the
> "bm_pool->pkt_size == MVPP2_BM_SHORT_PKT_SIZE" conditional outside of
> the test for xdp_prog - I don't see rxqi being used except within that
> conditional. Please can you explain the reasoning there?
>

Back in v3, skb_mark_for_recycle() was accepting an xdp_mem_info*, so
I needed rxqi out of that conditional scope to get that pointer.
Now we just need a page_pool*, so I can restore the original chunk.
Nice catch.

Thanks,
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index b2259bf1d299..9dceabece56c 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3847,6 +3847,7 @@  static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
 	struct mvpp2_pcpu_stats ps = {};
 	enum dma_data_direction dma_dir;
 	struct bpf_prog *xdp_prog;
+	struct xdp_rxq_info *rxqi;
 	struct xdp_buff xdp;
 	int rx_received;
 	int rx_done = 0;
@@ -3912,15 +3913,15 @@  static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
 		else
 			frag_size = bm_pool->frag_size;
 
-		if (xdp_prog) {
-			struct xdp_rxq_info *xdp_rxq;
+		if (bm_pool->pkt_size == MVPP2_BM_SHORT_PKT_SIZE)
+			rxqi = &rxq->xdp_rxq_short;
+		else
+			rxqi = &rxq->xdp_rxq_long;
 
-			if (bm_pool->pkt_size == MVPP2_BM_SHORT_PKT_SIZE)
-				xdp_rxq = &rxq->xdp_rxq_short;
-			else
-				xdp_rxq = &rxq->xdp_rxq_long;
+		if (xdp_prog) {
+			xdp.rxq = rxqi;
 
-			xdp_init_buff(&xdp, PAGE_SIZE, xdp_rxq);
+			xdp_init_buff(&xdp, PAGE_SIZE, rxqi);
 			xdp_prepare_buff(&xdp, data,
 					 MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM,
 					 rx_bytes, false);
@@ -3964,7 +3965,7 @@  static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
 		}
 
 		if (pp)
-			page_pool_release_page(pp, virt_to_page(data));
+			skb_mark_for_recycle(skb, virt_to_page(data), pp);
 		else
 			dma_unmap_single_attrs(dev->dev.parent, dma_addr,
 					       bm_pool->buf_size, DMA_FROM_DEVICE,