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:
- We go over all of them and make sure they're unreachable for unreadable skbs:
if (!skb_frags_readable()) return; // or something
- 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.