RE: Re: [PATCH net-next v3] octeontx2-pf: Add support for page pool

[Ratheesh Kannoth]

> -----Original Message-----
> From: Simon Horman <simon.horman@xxxxxxxxxxxx>
> Sent: Friday, May 19, 2023 3:33 PM
> To: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Sunil Kovvuri
> Goutham <sgoutham@xxxxxxxxxxx>; davem@xxxxxxxxxxxxx;
> edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> linyunsheng@xxxxxxxxxx; Subbaraya Sundeep Bhatta
> <sbhatta@xxxxxxxxxxx>; Geethasowjanya Akula <gakula@xxxxxxxxxxx>;
> Srujana Challa <schalla@xxxxxxxxxxx>; Hariprasad Kelam
> <hkelam@xxxxxxxxxxx>
> Subject: [EXT] Re: [PATCH net-next v3] octeontx2-pf: Add support for page
> pool
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Fri, May 19, 2023 at 12:43:52PM +0530, Ratheesh Kannoth wrote:
> > Page pool for each rx queue enhance rx side performance by reclaiming
> > buffers back to each queue specific pool. DMA mapping is done only for
> > first allocation of buffers.
> > As subsequent buffers allocation avoid DMA mapping, it results in
> > performance improvement.
> >
> > Image        |  Performance
> > ------------ | ------------
> > Vannila      |   3Mpps
> >              |
> > with this    |   42Mpps
> > change	     |
> > ---------------------------
> >
> > Signed-off-by: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>
> 
> ...
> 
> > @@ -1205,10 +1226,28 @@ void otx2_sq_free_sqbs(struct otx2_nic *pfvf)
> >  	}
> >  }
> >
> > +void otx2_free_bufs(struct otx2_nic *pfvf, struct otx2_pool *pool,
> > +		    u64 iova, int size)
> > +{
> > +	u64 pa = otx2_iova_to_phys(pfvf->iommu_domain, iova);
> > +	struct page *page = virt_to_head_page(phys_to_virt(pa));
> 
> nit: please arrange local variables in networking code in reverse xmas tree
>      order - longest line to shortest.
Variable "pa" is used in second line.  Are you suggesting to defer assignment later; and only declare variables here in reverse xmas tree style ? 

 
> 
> 	u64 pa = otx2_iova_to_phys(pfvf->iommu_domain, iova);
> 	struct page *page;
> 
> 	page = virt_to_head_page(phys_to_virt(pa));
> 
>      The following tool can check this:
> 
>         https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__github.com_ecree-
> 2Dsolarflare_xmastree&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=aekcsy
> BCH00_LewrEDcQBzsRw8KCpUR0vZb_auTHk4M&m=EAM7f8UtMNRC57jfDp
> gqNsIClttVw6HHyCL7XecFBync-LgDwxIm9_45n-w-
> WvjY&s=x_0EQ5wG1cZv01ySk_vzRX3kc8Bd2OALcpI8NgxYXTI&e=
> 
> ...
> 
> > @@ -1186,11 +1185,13 @@ bool otx2_sq_append_skb(struct net_device
> > *netdev, struct otx2_snd_queue *sq,  }
> > EXPORT_SYMBOL(otx2_sq_append_skb);
> >
> > -void otx2_cleanup_rx_cqes(struct otx2_nic *pfvf, struct otx2_cq_queue
> > *cq)
> > +void otx2_cleanup_rx_cqes(struct otx2_nic *pfvf, struct otx2_cq_queue
> > +*cq, int qidx)
> >  {
> >  	struct nix_cqe_rx_s *cqe;
> >  	int processed_cqe = 0;
> > -	u64 iova, pa;
> > +	struct otx2_pool *pool;
> > +	u16 pool_id;
> > +	u64 iova;
> 
> Likewise here.
Okay
> 
> >
> >  	if (pfvf->xdp_prog)
> >  		xdp_rxq_info_unreg(&cq->xdp_rxq);
> 

-Ratheesh