| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c50a0969-4b17-f2c2-6ad6-b085b8ac4043@huawei.com>
Date: Tue, 16 May 2023 09:14:01 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Ratheesh Kannoth <rkannoth@...vell.com>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
CC: <sgoutham@...vell.com>, <davem@...emloft.net>, <edumazet@...gle.com>,
<kuba@...nel.org>, <pabeni@...hat.com>
Subject: Re: [PATCH net-next] octeontx2-pf: Add support for page pool
On 2023/5/15 13:56, 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.
Any performance data to share here?
....
> @@ -1170,15 +1199,24 @@ void otx2_free_aura_ptr(struct otx2_nic *pfvf, int type)
> /* Free SQB and RQB pointers from the aura pool */
> for (pool_id = pool_start; pool_id < pool_end; pool_id++) {
> iova = otx2_aura_allocptr(pfvf, pool_id);
> + pool = &pfvf->qset.pool[pool_id];
> while (iova) {
> if (type == AURA_NIX_RQ)
> iova -= OTX2_HEAD_ROOM;
>
> pa = otx2_iova_to_phys(pfvf->iommu_domain, iova);
> - dma_unmap_page_attrs(pfvf->dev, iova, size,
> - DMA_FROM_DEVICE,
> - DMA_ATTR_SKIP_CPU_SYNC);
> - put_page(virt_to_page(phys_to_virt(pa)));
> + page = virt_to_page(phys_to_virt(pa));
virt_to_page() seems ok for order-0 page allocated from page
pool as it does now, but it may break for order-1+ page as
page_pool_put_page() expects head page of compound page or base
page. Maybe add a comment for that or use virt_to_head_page()
explicitly.
> +
> + if (pool->page_pool) {
> + page_pool_put_page(pool->page_pool, page, size, true);
page_pool_put_full_page() seems more appropriate here, as the
PP_FLAG_DMA_SYNC_DEV flag is not set, even if it is set, it seems
the whole page need to be synced instead of a frag.
> + } else {
> + dma_unmap_page_attrs(pfvf->dev, iova, size,
> + DMA_FROM_DEVICE,
> + DMA_ATTR_SKIP_CPU_SYNC);
> +
> + put_page(page);
> + }
> +
> iova = otx2_aura_allocptr(pfvf, pool_id);
> }
> }
> @@ -1196,6 +1234,8 @@ void otx2_aura_pool_free(struct otx2_nic *pfvf)
> pool = &pfvf->qset.pool[pool_id];
> qmem_free(pfvf->dev, pool->stack);
> qmem_free(pfvf->dev, pool->fc_addr);
> + page_pool_destroy(pool->page_pool);
> + pool->page_pool = NULL;
> }
> devm_kfree(pfvf->dev, pfvf->qset.pool);
> pfvf->qset.pool = NULL;
> @@ -1279,8 +1319,10 @@ static int otx2_aura_init(struct otx2_nic *pfvf, int aura_id,
> }
>
> static int otx2_pool_init(struct otx2_nic *pfvf, u16 pool_id,
> - int stack_pages, int numptrs, int buf_size)
> + int stack_pages, int numptrs, int buf_size,
> + int type)
> {
> + struct page_pool_params pp_params = { 0 };
> struct npa_aq_enq_req *aq;
> struct otx2_pool *pool;
> int err;
> @@ -1324,6 +1366,22 @@ static int otx2_pool_init(struct otx2_nic *pfvf, u16 pool_id,
> aq->ctype = NPA_AQ_CTYPE_POOL;
> aq->op = NPA_AQ_INSTOP_INIT;
>
> + if (type != AURA_NIX_RQ) {
> + pool->page_pool = NULL;
> + return 0;
> + }
> +
> + pp_params.flags = PP_FLAG_PAGE_FRAG | PP_FLAG_DMA_MAP;
> + pp_params.pool_size = numptrs;
> + pp_params.nid = NUMA_NO_NODE;
> + pp_params.dev = pfvf->dev;
> + pp_params.dma_dir = DMA_FROM_DEVICE;
> + pool->page_pool = page_pool_create(&pp_params);
> + if (!pool->page_pool) {
> + netdev_err(pfvf->netdev, "Creation of page pool failed\n");
> + return -EFAULT;
> + }
> +
> return 0;
> }
>
> @@ -1358,7 +1416,7 @@ int otx2_sq_aura_pool_init(struct otx2_nic *pfvf)
>
> /* Initialize pool context */
> err = otx2_pool_init(pfvf, pool_id, stack_pages,
> - num_sqbs, hw->sqb_size);
> + num_sqbs, hw->sqb_size, AURA_NIX_SQ);
> if (err)
> goto fail;
> }
> @@ -1421,7 +1479,7 @@ int otx2_rq_aura_pool_init(struct otx2_nic *pfvf)
> }
> for (pool_id = 0; pool_id < hw->rqpool_cnt; pool_id++) {
> err = otx2_pool_init(pfvf, pool_id, stack_pages,
> - num_ptrs, pfvf->rbsize);
> + num_ptrs, pfvf->rbsize, AURA_NIX_RQ);
> if (err)
> goto fail;
> }
...
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> index 7045fedfd73a..df5f45aa6980 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> @@ -217,9 +217,10 @@ static bool otx2_skb_add_frag(struct otx2_nic *pfvf, struct sk_buff *skb,
> skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
> va - page_address(page) + off,
> len - off, pfvf->rbsize);
> -
> +#ifndef CONFIG_PAGE_POOL
Most driver does 'select PAGE_POOL' in config when adding page
pool support, is there any reason it does not do it here?
> otx2_dma_unmap_page(pfvf, iova - OTX2_HEAD_ROOM,
> pfvf->rbsize, DMA_FROM_DEVICE);
> +#endif
> return true;
> }
>
> @@ -382,6 +383,8 @@ static void otx2_rcv_pkt_handler(struct otx2_nic *pfvf,
> if (pfvf->netdev->features & NETIF_F_RXCSUM)
> skb->ip_summed = CHECKSUM_UNNECESSARY;
>
> + skb_mark_for_recycle(skb);
> +
> napi_gro_frags(napi);
> }
>
> @@ -1180,11 +1183,14 @@ 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;
> + struct otx2_pool *pool;
> + struct page *page;
> u64 iova, pa;
> + u16 pool_id;
>
> if (pfvf->xdp_prog)
> xdp_rxq_info_unreg(&cq->xdp_rxq);
> @@ -1192,6 +1198,9 @@ void otx2_cleanup_rx_cqes(struct otx2_nic *pfvf, struct otx2_cq_queue *cq)
> if (otx2_nix_cq_op_status(pfvf, cq) || !cq->pend_cqe)
> return;
>
> + pool_id = otx2_get_pool_idx(pfvf, AURA_NIX_RQ, qidx);
> + pool = &pfvf->qset.pool[pool_id];
> +
> while (cq->pend_cqe) {
> cqe = (struct nix_cqe_rx_s *)otx2_get_next_cqe(cq);
> processed_cqe++;
> @@ -1205,8 +1214,14 @@ void otx2_cleanup_rx_cqes(struct otx2_nic *pfvf, struct otx2_cq_queue *cq)
> }
> iova = cqe->sg.seg_addr - OTX2_HEAD_ROOM;
> pa = otx2_iova_to_phys(pfvf->iommu_domain, iova);
> - otx2_dma_unmap_page(pfvf, iova, pfvf->rbsize, DMA_FROM_DEVICE);
> - put_page(virt_to_page(phys_to_virt(pa)));
> + page = virt_to_page(phys_to_virt(pa));
> +
> + if (pool->page_pool) {
> + page_pool_put_page(pool->page_pool, page, pfvf->rbsize, true);
> + } else {
> + otx2_dma_unmap_page(pfvf, iova, pfvf->rbsize, DMA_FROM_DEVICE);
> + put_page(page);
> + }
Maybe add a helper for the above as there is a similiar code block
in the otx2_free_aura_ptr()
>
Powered by blists - more mailing lists