From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yw1-f179.google.com (mail-yw1-f179.google.com [209.85.128.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 39591385D7E for ; Mon, 24 Aug 2026 19:14:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787598861; cv=none; b=Gq4kOWnWula6M7fOGmOaCDeo6hyzZgvLt4XuUd2DY1a/Tpq8wOO9WK2GT84RcCfMBxA4mWOyPsg+XLcQ1R7FBuxdmqAsBg7FVlO74uLPNM1EXLBRvmuhtzr0DGmBQ6cRIKfWnbf8MSq7xlz9VlxVzIZCSchHQ89z9ug501ikkqI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787598861; c=relaxed/simple; bh=Mulkdee5bkClRxh8Zt2BR14PzG6qLRAohxmd+dt/PmY=; h=Date:From:To:Cc:Message-ID:In-Reply-To:References:Subject: Mime-Version:Content-Type; b=ODjaTo7cAgmyrfGnrN+o/IveG/a7HBbuSB+MVL1zS6/AT+UjDaQe8TkGa8tPFvSei/g4UCfK33fBrF547O13ruq+Xw1Pmu+70e/TKJ8FX0A4fsvLw9mem3Otqy9ZA3cQ0xroIukVuEvIbJLXEf3CNPQdNRsMwH+1p8YOph4rnNA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=idKRJpJD; arc=none smtp.client-ip=209.85.128.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="idKRJpJD" Received: by mail-yw1-f179.google.com with SMTP id 00721157ae682-836c5b01e82so52377747b3.0 for ; Mon, 24 Aug 2026 12:14:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1787598859; x=1788203659; darn=vger.kernel.org; h=content-transfer-encoding:content-type:mime-version:subject :references:in-reply-to:message-id:cc:to:from:date:from:to:cc :subject:date:message-id:reply-to:content-type; bh=EY28LEF5hNU4O/7Z8bSRN2w17lHi5xeca4NWdtVGjQk=; b=idKRJpJDT0GT4jR0Kg68ajMkEXrivfJF3d/ymLfb0PLiZTCYuQiF82+E3Pox1XjkVb 9X9ETC6cvXCaaljBk0eKGTCjWXwV+aJr/6byhYZ4pbBhVDTdF5BFe1/AdtJovSUr+nmV FgfzM2xAco5pts5GpHRqig6coVZ75QR8SRi0LDg6ZPzj3TJyrEY6U94XCCKz2NNTSh6b gN5N8+f7zcQ/NGTMI7WXyw6gtdGbeiDbg+chxOQPbrBDblpfhvnI+u1jo+5TLPyJhKIk n7csjqiaUMKOuKZG6xx5DlPeQMZRDSnLaqsFLzO+Kb7e9Wr1Rtqvpth1363+dOddVa3R XLdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787598859; x=1788203659; h=content-transfer-encoding:content-type:mime-version:subject :references:in-reply-to:message-id:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=EY28LEF5hNU4O/7Z8bSRN2w17lHi5xeca4NWdtVGjQk=; b=rNB8dJON6u1gAaLlBjzNIH2Co9e5pjhAHgP0ivwRU4omyn4gbvoS77iEVTWX7lqF/9 wLnxdzX3lDbtrOuExSBhgqa16FQNkqg9JchtTpilZ8fxRJMKdBY3EpuNbIpqxTDULC3i x83OoOtzlzVcvBsNp3csTmA2X7dVy7bXMDmZ6dGCFg9KO1O5iY1Rmjb0NZ+Abz3LoN31 Nagqo2kpL9s4cVt9xHSAd2duovLATQlWPe+wsNBEBGZ68gxbpUhjRffh6V6XUzkyzYBd kGOK4UUMGRcKkGhjOKuJ638Heyi6UQt77s78ZWxeJzlemSwEXSD5sDy7cgFaMg/Ct751 ivfA== X-Forwarded-Encrypted: i=1; AHgh+RoJNYc9D7c1KoTYmWoJzdz7+9kZNkrohW74VCOt1AtV33tsZ2E1nXpku3zF9D6a0WXk4mvvV+jkJohwPGc=@vger.kernel.org X-Gm-Message-State: AFuF++k1/BBXff/ez4pkWQ125qASxfUUMH4LE+o60o8fsKHiwDK/Dkx1 NKWlZB7XmGf4d0eJhI22lB5zkXUzUftPhAqsqqU/kDS/7eIwBg3RqSOM X-Gm-Gg: AR+sD12e2uZmQcq0NaXCquMAStgP/a6+/RQHqaaATHXTzAqH+/UF0Tmmln/CpFTYN8f gzOGHOFdfn/SAyopfhUy0qkbv+npe5lwEbue1rlHvTsmVhQPNYUuR/Eo5azyQJJKJ4cRzqFf7mI aHMPo7qDf6YEdu+KNx0wtA2zjPo95r5177i6CBpwEdiqYM420fQP4cCDz3WcoYP7zv4kR9w156p upn2Y9mBZ+JJi06bk4v1OH7XRJANYjdf2AtisJ33lqqVnokzPntcvF2lm15XljxxAXpAdtA73hU BUTlByFYktWOrZGnAat9FSMTKJxnAmcJ0pFXwcKJOpuYRpHNVX6CKawEEIjxZk9uipQ83OkxC6O hdCi6rB7T0k+aHpRe4pGVZOYfy/0oT21J4tdXpIQ78h95wh1lINapG2MGfTA8QIFPdgODeiQN80 aEzWFSKEaUQJqImnogmw0Y6eHU/Bbc5kCXHsGl+WZzr4TyjkjvTcNj/PMCSc+AV8WXzoNbF+XKc umh+Ul65mue6NSm3ebA5a0xXK1BL+FEDYvKLgXy0w== X-Received: by 2002:a05:690c:e1c4:10b0:820:1281:8deb with SMTP id 00721157ae682-849f08113c4mr95407857b3.8.1787598859052; Mon, 24 Aug 2026 12:14:19 -0700 (PDT) Received: from gmail.com (234.207.85.34.bc.googleusercontent.com. [34.85.207.234]) by smtp.gmail.com with ESMTPSA id 00721157ae682-851e1419e0bsm16499657b3.26.2026.08.24.12.14.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Aug 2026 12:14:18 -0700 (PDT) Date: Mon, 24 Aug 2026 15:14:18 -0400 From: Willem de Bruijn To: Ilya Maximets , Willem de Bruijn , Ilya Maximets , Jakub Kicinski Cc: Norbert Szetei , netdev@vger.kernel.org, "David S. Miller" , Eric Dumazet , Paolo Abeni , Simon Horman , Aaron Conole , Eelco Chaudron , Steffen Klassert , Kuan-Ting Chen , "Michael S. Tsirkin" , linux-kernel@vger.kernel.org, dev@openvswitch.org, Jongmin Jang , Willem de Bruijn Message-ID: In-Reply-To: References: <34c64d08-0484-4f3b-b61a-ed68115d01a4@ovn.org> <20260821170950.3e278b1f@kernel.org> <86c61c39-901e-489e-aec9-fafe639276e7@ovn.org> Subject: Re: [PATCH net v3 0/3] net: don't strip zerocopy frag markers from a forwarded skb Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Ilya Maximets wrote: > On 8/22/26 10:55 PM, Willem de Bruijn wrote: > > Ilya Maximets wrote: > >> On 8/22/26 2:09 AM, Jakub Kicinski wrote: > >>> On Fri, 21 Aug 2026 23:45:41 +0200 Ilya Maximets wrote: > >>>> 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. > >> > >>> 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! > > > > >> 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) > the extra reference> > > So, unless I'm missing something, the kfree_skb() already does everything > that skb_tx_error() does. Good point. I agree. > 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... I don't immediately recall the rationale. Probably not intentional and I should have left the original call-sites, notably tun_net_xmit, as is. With MSG_ZEROCOPY, goal is to only set zerocopy_success to false if a transmission could not be fully completed in zerocopy mode. Falling back to copying deep in the stack is usually more expensive than doing it from the start. And if the sendmsg otherwise succeeds, the caller receives no other signal that MSG_ZEROCOPY is counterproductive. skb_copy_ubufs will indeed call skb_zcopy_clear(.., false). General transmit failures are signaled through the normal error path. IMHO this includes allocation failure with GFP_ATOMIC. That said, while I don't fully agree with these skb_tx_error()'s with !zerocopy_success in the skb_orphan_frags() error paths, they did precede my code. If we want to preserve them we would need to keep skb_tx_error, with an extra zerocopy_success argument.