Messages in this thread | ![/](/images/icornerl.gif) | | From | Mina Almasry <> | Date | Wed, 3 Jan 2024 10:38:31 -0800 | Subject | Re: [RFC PATCH net-next v1 4/4] net: page_pool: use netmem_t instead of struct page in API |
| |
On Wed, Jan 3, 2024 at 1:47 AM Yunsheng Lin <linyunsheng@huawei.com> wrote: > > On 2024/1/3 0:14, Mina Almasry wrote: > > > > The idea being that skb_frag_page() can return NULL if the frag is not > > paged, and the relevant callers are modified to handle that. > > There are many existing drivers which are not expecting NULL returning for > skb_frag_page() as those drivers are not supporting devmem, adding additionl > checking overhead in skb_frag_page() for those drivers does not make much > sense, IMHO, it may make more sense to introduce a new helper for the driver > supporting devmem or networking core that needing dealing with both normal > page and devmem. > > And we are also able to keep the old non-NULL returning semantic for > skb_frag_page().
I think I'm seeing agreement that the direction we're heading into here is that most net stack & drivers should use the abstract netmem type, and only specific code that needs a page or devmem (like tcp_receive_zerocopy or tcp_recvmsg_dmabuf) will be the ones that unpack the netmem and get the underlying page or devmem, using skb_frag_page() or something like skb_frag_dmabuf(), etc.
As Jason says repeatedly, I'm not allowed to blindly cast a netmem to a page and assume netmem==page. Netmem can only be cast to a page after checking the low bits and verifying the netmem is actually a page. I think any suggestions that blindly cast a netmem to page without the checks will get nacked by Jason & Christian, so the checking in the specific cases where the code needs to know the underlying memory type seems necessary.
IMO I'm not sure the checking is expensive. With likely/unlikely & static branches the checks should be very minimal or a straight no-op. For example in RFC v2 where we were doing a lot of checks for devmem (we don't do that anymore for RFCv5), I had run the page_pool perf tests and proved there is little to no perf regression:
https://lore.kernel.org/netdev/CAHS8izM4w2UETAwfnV7w+ZzTMxLkz+FKO+xTgRdtYKzV8RzqXw@mail.gmail.com/
-- Thanks, Mina
| ![\](/images/icornerr.gif) |