linux-kernel - Re: [PATCH V4,net-next] net: mana: Add page pool for RX buffers

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230731173119.3ca14894@kernel.org>
Date:   Mon, 31 Jul 2023 17:31:19 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Haiyang Zhang <haiyangz@...rosoft.com>
Cc:     linux-hyperv@...r.kernel.org, netdev@...r.kernel.org,
        decui@...rosoft.com, kys@...rosoft.com, paulros@...rosoft.com,
        olaf@...fle.de, vkuznets@...hat.com, davem@...emloft.net,
        wei.liu@...nel.org, edumazet@...gle.com, pabeni@...hat.com,
        leon@...nel.org, longli@...rosoft.com, ssengar@...ux.microsoft.com,
        linux-rdma@...r.kernel.org, daniel@...earbox.net,
        john.fastabend@...il.com, bpf@...r.kernel.org, ast@...nel.org,
        sharmaajay@...rosoft.com, hawk@...nel.org, tglx@...utronix.de,
        shradhagupta@...ux.microsoft.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V4,net-next] net: mana: Add page pool for RX buffers

On Fri, 28 Jul 2023 14:46:07 -0700 Haiyang Zhang wrote:
>  static void *mana_get_rxfrag(struct mana_rxq *rxq, struct device *dev,
> -			     dma_addr_t *da, bool is_napi)
> +			     dma_addr_t *da, bool *from_pool, bool is_napi)
>  {
>  	struct page *page;
>  	void *va;
>  
> +	*from_pool = false;
> +
>  	/* Reuse XDP dropped page if available */
>  	if (rxq->xdp_save_va) {
>  		va = rxq->xdp_save_va;
> @@ -1533,17 +1543,22 @@ static void *mana_get_rxfrag(struct mana_rxq *rxq, struct device *dev,
>  			return NULL;
>  		}
>  	} else {
> -		page = dev_alloc_page();
> +		page = page_pool_dev_alloc_pages(rxq->page_pool);
>  		if (!page)
>  			return NULL;
>  
> +		*from_pool = true;
>  		va = page_to_virt(page);
>  	}
>  
>  	*da = dma_map_single(dev, va + rxq->headroom, rxq->datasize,
>  			     DMA_FROM_DEVICE);
>  	if (dma_mapping_error(dev, *da)) {
> -		put_page(virt_to_head_page(va));
> +		if (*from_pool)
> +			page_pool_put_full_page(rxq->page_pool, page, is_napi);

AFAICT you only pass the is_napi to recycle in case of error?
It's fine to always pass in false, passing true enables some
optimizations but it's not worth trying to optimize error paths.

Otherwise you may be passing in true, even tho budget was 0,
see the recently added warnings in this doc:

https://www.kernel.org/doc/html/next/networking/napi.html

In general the driver seems to be processing Rx regardless
of budget? This looks like a bug which should be fixed with
a separate patch for the net tree..
-- 
pw-bot: cr