On Fri, 4 Oct 2024 19:34:45 +0900 Taehee Yoo wrote: > > Our intention with the whole netmem design is that drivers should > > never have to call netmem_to_page(). I.e. the driver should use netmem > > unaware of whether it's page or non-page underneath, to minimize > > complexity driver needs to handle. > > > > This netmem_to_page() call can be removed by using > > skb_frag_fill_netmem_desc() instead of the page variant. But, more > > improtantly, why did the code change here? The code before calls > > skb_frag_fill_page_desc, but the new code sometimes will > > skb_frag_fill_netmem_desc() and sometimes will skb_add_rx_frag_netmem. > > I'm not sure why that logic changed. > > The reason why skb_add_rx_frag_netmem() is used here is to set > skb->unreadable flag. the skb_frag_fill_netmem_desc() doesn't set > skb->unreadable because it doesn't handle skb, it only handles frag. > As far as I know, skb->unreadable should be set to true for devmem > TCP, am I misunderstood? > I tested that don't using skb_add_rx_frag_netmem() here, and it > immediately fails. Yes, but netmem_ref can be either a net_iov or a normal page, and skb_add_rx_frag_netmem() and similar helpers should automatically set skb->unreadable or not. IOW you should be able to always use netmem-aware APIs, no? > > This is not the intended use of PP_FLAG_ALLOW_UNREADABLE_NETMEM. > > > > The driver should set PP_FLAG_ALLOW_UNREADABLE_NETMEM when it's able > > to handle unreadable netmem, it should not worry about whether > > rxq->mp_params.mp_priv is set or not. > > > > You should set PP_FLAG_ALLOW_UNREADABLE_NETMEM when HDS is enabled. > > Let core figure out if mp_params.mp_priv is enabled. All the driver > > needs to report is whether it's configured to be able to handle > > unreadable netmem (which practically means HDS is enabled). > > The reason why the branch exists here is the PP_FLAG_ALLOW_UNREADABLE_NETMEM > flag can't be used with PP_FLAG_DMA_SYNC_DEV. Hm. Isn't the existing check the wrong way around? Is the driver supposed to sync the buffers for device before passing them down?
