linux-kernel - Re: [PATCH net-next 3/6] sunrpc: Use sendmsg(MSG_SPLICE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f508459473376e0f08e3a147b292aaab1785f320.camel@kernel.org>
Date:   Sat, 12 Aug 2023 07:45:11 -0400
From:   Jeff Layton <jlayton@...nel.org>
To:     David Howells <dhowells@...hat.com>, netdev@...r.kernel.org
Cc:     Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        David Ahern <dsahern@...nel.org>,
        Matthew Wilcox <willy@...radead.org>,
        Jens Axboe <axboe@...nel.dk>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Chuck Lever <chuck.lever@...cle.com>,
        Trond Myklebust <trond.myklebust@...merspace.com>,
        Anna Schumaker <anna@...nel.org>, linux-nfs@...r.kernel.org
Subject: Re: [PATCH net-next 3/6] sunrpc: Use sendmsg(MSG_SPLICE_PAGES)
 rather then sendpage

On Fri, 2023-08-11 at 19:07 -0400, Jeff Layton wrote:
> On Fri, 2023-08-11 at 18:50 -0400, Jeff Layton wrote:
> > On Fri, 2023-06-09 at 11:02 +0100, David Howells wrote:
> > > When transmitting data, call down into TCP using sendmsg with
> > > MSG_SPLICE_PAGES to indicate that content should be spliced rather than
> > > performing sendpage calls to transmit header, data pages and trailer.
> > > 
> > > Signed-off-by: David Howells <dhowells@...hat.com>
> > > Acked-by: Chuck Lever <chuck.lever@...cle.com>
> > > cc: Trond Myklebust <trond.myklebust@...merspace.com>
> > > cc: Anna Schumaker <anna@...nel.org>
> > > cc: Jeff Layton <jlayton@...nel.org>
> > > cc: "David S. Miller" <davem@...emloft.net>
> > > cc: Eric Dumazet <edumazet@...gle.com>
> > > cc: Jakub Kicinski <kuba@...nel.org>
> > > cc: Paolo Abeni <pabeni@...hat.com>
> > > cc: Jens Axboe <axboe@...nel.dk>
> > > cc: Matthew Wilcox <willy@...radead.org>
> > > cc: linux-nfs@...r.kernel.org
> > > cc: netdev@...r.kernel.org
> > > ---
> > >  include/linux/sunrpc/svc.h | 11 +++++------
> > >  net/sunrpc/svcsock.c       | 38 ++++++++++++--------------------------
> > >  2 files changed, 17 insertions(+), 32 deletions(-)
> > > 
> > 
> > I'm seeing a regression in pynfs runs with v6.5-rc5. 3 tests are failing
> > in a similar fashion. WRT1b is one of them
> > 
> > [vagrant@...yton-kdo-nfsd nfs4.0]$  ./testserver.py --rundeps --maketree --uid=0 --gid=0 localhost:/export/pynfs/4.0/ WRT1b                                                     
> > **************************************************                                                                                                                              
> > WRT1b    st_write.testSimpleWrite2                                : FAILURE                                                                                                     
> >            READ returned                                                                                                                                                        
> >            b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',                                                                                                     
> >            expected b'\x00\x00\x00\x00\x00write data'                                                                                                                           
> > INIT     st_setclientid.testValid                                 : PASS                                                                                                        
> > MKFILE   st_open.testOpen                                         : PASS                                                                                                        
> > **************************************************                                                                                                                              
> > Command line asked for 3 of 679 tests                                                                                                                                           
> > Of those: 0 Skipped, 1 Failed, 0 Warned, 2 Passed                                                   
> 
> FWIW, here's a capture that shows the problem. See frames 109-112 in
> particular. If no one has thoughts on this one, I'll plan to have a look
> early next week.

Since Chuck's nfsd-next branch (which is based on v6.5-rc5) wasn't
showing this issue, I ran a bisect to see what fixes it, and it landed
on this patch:

commit ed9cd98404c8ae5d0bdd6e7ce52e458a8e0841bb
Author: Chuck Lever <chuck.lever@...cle.com>
Date:   Wed Jul 19 14:31:03 2023 -0400

    SUNRPC: Convert svc_tcp_sendmsg to use bio_vecs directly
    
    Add a helper to convert a whole xdr_buf directly into an array of
    bio_vecs, then send this array instead of iterating piecemeal over
    the xdr_buf containing the outbound RPC message.
    
    Reviewed-by: David Howells <dhowells@...hat.com>
    Signed-off-by: Chuck Lever <chuck.lever@...cle.com>


I'll follow up on that thread. I think we may want to pull this patch
into mainline for v6.5.
-- 
Jeff Layton <jlayton@...nel.org>