Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230524153311.3625329-1-dhowells@redhat.com> <20230524153311.3625329-10-dhowells@redhat.com>
 <20230526180844.73745d78@kernel.org> <499791.1685485603@warthog.procyon.org.uk>
In-Reply-To: <499791.1685485603@warthog.procyon.org.uk>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu, 1 Jun 2023 09:09:16 -0400
Message-ID: <CAHk-=wgeixW3cc=Ys8eL0_+22FUhqeEru=nzRrSXy1U4YQdE-w@mail.gmail.com>
Subject: Re: Bug in short splice to socket?
To:     David Howells <dhowells@redhat.com>
Cc:     Jakub Kicinski <kuba@kernel.org>, netdev@vger.kernel.org,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
        David Ahern <dsahern@kernel.org>,
        Matthew Wilcox <willy@infradead.org>,
        Jens Axboe <axboe@kernel.dk>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, Chuck Lever <chuck.lever@oracle.com>,
        Boris Pismenny <borisp@nvidia.com>,
        John Fastabend <john.fastabend@gmail.com>,
        Christoph Hellwig <hch@infradead.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, May 30, 2023 at 6:26=E2=80=AFPM David Howells <dhowells@redhat.com>=
 wrote:
>
> When used with sendfile(), it sets SPLICE_F_MORE (which causes MSG_MORE t=
o be
> passed to the network protocol) if we haven't yet read everything that th=
e
> user requested and clears it if we fulfilled what the user requested.
>
> This has the weird effect that MSG_MORE gets kind of inverted. [...]

I hate this patch.

The old code is unquestionably garbage, but this just ends up
resulting in more of the same.

The reason the old code is garbage is that it sets SPLICE_F_MORE
entirely in the wrong place. It sets it *after* it has done the
splice(). That's just crazy.

And that craziness is the direct cause of the bug.

You try to fix the bug by just extending on the craziness. No. The
crazy should be removed, not extended upon.

The "flag" setting should be done *before* the send, if we know that
more data is pending even after the "do_splice_to()". Not after.

And the networking code should do its own "oh, splice gave me X bytes,
but I only used Y, so I know more data is pending, so I'll set
MSG_MORE on the *current* packet". But that's entirely inside of
whatever networking code that does th esplice.

So no. I absolutely refuse to entertain this patch because it's
completely broken. The fact that the old code was also completely
broken is *not* an excuse to make for more nonsensical breakage that
may or may just hide the crazy.

               Linus