On 8/22/26 10:55 PM, Willem de Bruijn wrote:
Ilya Maximets wrote:
quoted
On 8/22/26 2:09 AM, Jakub Kicinski wrote:
quoted
On Fri, 21 Aug 2026 23:45:41 +0200 Ilya Maximets wrote:
quoted
Unfortunately, this needs a rebase now that a conflicting change
for skb_zerocopy() was merged:
Ugh, I was supposed to merge this first, wasn't I? Sorry.
Not a huge deal, I guess, the conflict is mechanical and the patches
are simple. I can take care of manual backports once we get the
'failed to apply' emails. Just a bit of busy work.
quoted
I was hoping for Willem to TAL since skb_tx_error() is a tx ZC
thing, now I realized that he wasn't CCed :S (please do so on v4)
FWIW, I CCed a few people on v1 to have a conversation about a proper
fix, but that wasn't fruitful. So, if I were Norbert, I wouldn't
include them for the new versions either as doing so always feels like
me being annoying. :)
Having a look now.
Thanks!
quoted
For now, the plan is to get v4 of these targeted fixes into net and
stable and then remove skb_tx_error() entirely once net-next is open,
as it seems to have lost all of its prior meaning.
The original use case in tun_net_xmit introduced in commit
149d36f7187c ("tun: report orphan frags errors to zero copy callback")
still exists. Not sure you can remove the function entirely.
The skb_tx_error() prescribes to call kfree_skb() right after it and
all the callers more or less do that (with the fixes applied).
skb_tx_error() does two things:
1. skb_zcopy_downgrade_managed() that takes extra references on frags.
2. Calls skb_zcopy_clear(skb, true);
The kfree_skb() called right after does:
__kfree_skb
skb_release_all
skb_release_data
if (skb_zcopy)
bool skip_unref = shinfo->flags & SKBFL_MANAGED_FRAG_REFS;
skb_zcopy_clear(skb, true);
if (skip_unref)
<skip unreferencing the frags, which is the same as taking
the extra reference>
So, unless I'm missing something, the kfree_skb() already does everything
that skb_tx_error() does.
The fact that skb_tx_error() calls skb_zcopy_clear() with 'true' though
feels weird. I would understand the need for the function, if it was
actually signalling the error and not success. But you switched false
to true in commit 1f8b977ab32d ("sock: enable MSG_ZEROCOPY") nine years
ago and it seems like nobody complained so far...
The bug is hit when this function is called with a cloned or shared
skb. The original zerocopy path through tun_net_xmit was probably
expected to not have this problem.
Commit 0110d6f22f39 ("tun: orphan an skb on tx") explains why this
orphan in tun_net_xmit was needed: a (vhost) zerocopy skb injected in
tap1 arriving at tap2 and not being read there indefinitely. Such
loops are not allowed for zerocopy.
Commit 868eefeb17d4 ("tun: orphan frags on xmit") then added the
frags orphan. Since an skb can be cloned on this tap to tap path,
e.g., with a packet socket, I think the bug goes back to the original
commit that introduced the first caller of skb_tx_error, commit
149d36f7187c ("tun: report orphan frags errors to zero copy callback")
If respinning it may be worthile to link to this thread.