Re: [PATCH net-next v9 2/9] net: add get_netmem/put_netmem support

[Pavel Begunkov <asml.silence@gmail.com>]

On 4/22/25 22:10, Mina Almasry wrote:
...
>>> Instead of adding net_iov / devmem handling in generic paths affecting
>>> everyone, you could change those functions where it's get_page() are
>>> called legitimately. The niov/devmem part of get/put_netmem doesn't
>>> even have the same semantics as the page counterparts as it cannot
>>> prevent from reallocation. That might be fine, but it's not clear
>>
>> Actually, maybe it's not that exclusive to netiov, same reallocation
>> argument is true for user pages, even though they're reffed
>> separately.
>>
>> It might be fine to leave this approach, while suboptimal it should
>> be easier for you. Depends on how folks feel about the extra
>> overhead in the normal tx path.
>>
> 
> Right, I think there is only 2 ways to handle all the code paths in
> the tcp stack that hit skb_frag_ref:
> 
> 1. We go over all of them and make sure they're unreachable for unreadable skbs:
> 
> if (!skb_frags_readable()) return; // or something
> 
> 2. or, we just add net_iov support in skb_frag_ref.
> 
> This patch series does the latter, which IMO is much preferred.
> 
> FWIW I'm surprised that adding net_iov support to skb_frag_ref/unref
> is facing uncertainty. I've added net_iov support for many skb helpers
> in commit 65249feb6b3df ("net: add support for skbs with unreadable
> frags") and commit 9f6b619edf2e8 ("net: support non paged skb frags").
> skb_frag_ref/unref is just 1 helper I "missed" because it's mostly
> (but not entirely) used by the TX path.

It'd have looked completely different if you did it back then, which
is the same semantics mismatch I mentioned. For pp rx niovs it'd
have incremented the niovs ref pinning that specific niov down
and preventing reallocation (by pp), and that with no devmem specific
code sticking into generic code.
This patch adds a 3rd way to refcount a frag (after page refs and
pp_ref_count), which is why it attracted attention.

-- 
Pavel Begunkov