linux-kernel - Re: [EXTERNAL] Re: [PATCH net 1/3] net: ti: icssg-prueth: Use page

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: <631429d8-9da6-4333-80ce-6ff59e5ecdf6@ti.com>
Date: Thu, 6 Feb 2025 19:31:03 +0530
From: "Malladi, Meghana" <m-malladi@...com>
To: Ido Schimmel <idosch@...sch.org>
CC: <rogerq@...nel.org>, <danishanwar@...com>, <pabeni@...hat.com>,
        <kuba@...nel.org>, <edumazet@...gle.com>, <davem@...emloft.net>,
        <andrew+netdev@...n.ch>, <bpf@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
        <netdev@...r.kernel.org>, <robh@...nel.org>,
        <matthias.schiffer@...tq-group.com>, <dan.carpenter@...aro.org>,
        <rdunlap@...radead.org>, <diogo.ivo@...mens.com>,
        <schnelle@...ux.ibm.com>, <glaroque@...libre.com>,
        <john.fastabend@...il.com>, <hawk@...nel.org>, <daniel@...earbox.net>,
        <ast@...nel.org>, <srk@...com>, Vignesh Raghavendra
	<vigneshr@...com>
Subject: Re: [EXTERNAL] Re: [PATCH net 1/3] net: ti: icssg-prueth: Use
 page_pool API for RX buffer allocation



On 2/5/2025 11:11 PM, Ido Schimmel wrote:
> On Tue, Feb 04, 2025 at 11: 25: 02PM +0530, Malladi, Meghana wrote: > 
> Seems like none of the pages which have been allocated aren't getting > 
> recycled in the rx path after being used unless its some error case. 
> Will > try to fix this. 
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source 
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK! 
> v9dnXdhkXiNQgIoLtH6jcbhWBIydfvayMZ6bf68taZCHXfcLg8XIOscUa_XNxqzQWA$>
> ZjQcmQRYFpfptBannerEnd
> 
> On Tue, Feb 04, 2025 at 11:25:02PM +0530, Malladi, Meghana wrote:
>> Seems like none of the pages which have been allocated aren't getting
>> recycled in the rx path after being used unless its some error case. Will
>> try to fix this.
> 
> skb_mark_for_recycle() should help with page recycling when an skb that
> uses them is freed.
> 
> Anyway, I believe that I don't see put call when tearing down the Rx
> ring because prueth_rx_cleanup() is using page_pool_recycle_direct()
> when it shouldn't. AFAICT, prueth_rx_cleanup() is only called from the
> control path (upon ndo_stop()) and not in NAPI context.
> 

Ok I will use skb_mark_for_recycle()/page_pool_recycle_direct() 
accordingly to recycle the pages.

>> Also I have noticed, in prueth_prepare_rx_chan() pages are allocated per
>> number of descriptors for a channel, but they are not being used when a
>> packet is being recieved (in emac_rx_packet()) and rather new page is
>> allocated for the next upcoming packet. Is this a valid design, what are
>> your thoughts on this ?
> 
> The new page is possibly a page that was recycled into the pool when a
> previous packet was freed / dropped.
> 
> [...]
> 
>> Yes I will add PP_FLAG_DMA_SYNC_DEV as well.
>> I believe page_pool_dma_sync_for_cpu() needs to be called sync Rx page for
>> CPU, am I right ? If so can you tell me, in what all cases should I call
>> this function.
> 
> Before accessing the packet data.
>

Ok, thanks.

>> https://urldefense.com/v3/__https://lkml.iu.edu/hypermail/linux/ 
> kernel/2312.1/06353.html__;!!G3vK!R- 
> autrVAgf5rAbl3CYoqlN5gRE_NqPqYRg1NHkJ405Q33b6uKiHFI73PeRky46dBYBWQpmFThUyD$ <https://urldefense.com/v3/__https://lkml.iu.edu/hypermail/linux/kernel/2312.1/06353.html__;!!G3vK!R-autrVAgf5rAbl3CYoqlN5gRE_NqPqYRg1NHkJ405Q33b6uKiHFI73PeRky46dBYBWQpmFThUyD$>
>> In the above link it is quoted - "Note that this version performs DMA sync
>> unconditionally, even if the associated PP doesn't perform sync-for-device"
>> for the page_pool_dma_sync_for_cpu() function. So does that mean if I am
>> using this function I don't need explicily sync for device call?
> 
> It's explained in the page pool documentation:
> 
> "Driver is always responsible for syncing the pages for the CPU. Drivers
> may choose to take care of syncing for the device as well or set the
> PP_FLAG_DMA_SYNC_DEV flag to request that pages allocated from the page
> pool are already synced for the device."
> 
> https://urldefense.com/v3/__https://docs.kernel.org/networking/ 
> page_pool.html*dma-sync__;Iw!!G3vK!R- 
> autrVAgf5rAbl3CYoqlN5gRE_NqPqYRg1NHkJ405Q33b6uKiHFI73PeRky46dBYBWQphNIm6Qm$ <https://urldefense.com/v3/__https://docs.kernel.org/networking/page_pool.html*dma-sync__;Iw!!G3vK!R-autrVAgf5rAbl3CYoqlN5gRE_NqPqYRg1NHkJ405Q33b6uKiHFI73PeRky46dBYBWQphNIm6Qm$>
>