> On Dec 9, 2020, at 11:39 AM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote: > > On Wed, 2020-12-09 at 11:16 -0500, Chuck Lever wrote: >> Hey- >> >>> On Dec 9, 2020, at 9:48 AM, trondmy@xxxxxxxxxx wrote: >>> >>> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> >>> >>> Ensure that we encode the data payload + padding, and that we >>> truncate >>> the preallocated buffer to the actual read size. >> >> Did you intend to merge 15/16 and 16/16 through your tree? > > No. They can go through the nfsd tree. I included them here because > they are necessary in order to pass the xfstests. Would it be OK if they went in 5.11-rc? I've got the initial merge tag prepared already. If they can't wait, let me know. >> Can the patch descriptions say a little more about why these >> changes are necessary? If they fix a misbehavior, describe >> the problem. > > It's the same problem and solution that exists in the READ code. > > nfsd_readv() doesn't necessarily return the same number of bytes that > we requested and preallocated buffer space for. So to deal with that, > we have to truncate the preallocated buffer. Huh. I thought it was doing that already? Oh, that's just for the cases where the server returns an error status. The READ_PLUS encoder incorrectly does not do that truncation for short READs... got it. > Finally, we have to write zeros into the padding bytes after the read > buffer. Right. Then the problem statement is that the server's READ_PLUS XDR encoder isn't padding the read buffer properly. Quibble: perhaps these are two separate issues that each deserve their own patches with Fixes: tags (and if you re-post these, please add a Fixes: tag to 16/16 too). Thanks! >>> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> >>> --- >>> fs/nfsd/nfs4xdr.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c >>> index 833a2c64dfe8..26f6e277101d 100644 >>> --- a/fs/nfsd/nfs4xdr.c >>> +++ b/fs/nfsd/nfs4xdr.c >>> @@ -4632,6 +4632,7 @@ nfsd4_encode_read_plus_data(struct >>> nfsd4_compoundres *resp, >>> resp->rqstp->rq_vec, read->rd_vlen, >>> maxcount, eof); >>> if (nfserr) >>> return nfserr; >>> + xdr_truncate_encode(xdr, starting_len + 16 + >>> xdr_align_size(*maxcount)); >>> >>> tmp = htonl(NFS4_CONTENT_DATA); >>> write_bytes_to_xdr_buf(xdr->buf, starting_len, &tmp, >>> 4); >>> @@ -4639,6 +4640,10 @@ nfsd4_encode_read_plus_data(struct >>> nfsd4_compoundres *resp, >>> write_bytes_to_xdr_buf(xdr->buf, starting_len + 4, &tmp64, >>> 8); >>> tmp = htonl(*maxcount); >>> write_bytes_to_xdr_buf(xdr->buf, starting_len + 12, &tmp, >>> 4); >>> + >>> + tmp = xdr_zero; >>> + write_bytes_to_xdr_buf(xdr->buf, starting_len + 16 + >>> *maxcount, &tmp, >>> + xdr_pad_size(*maxcount)); >>> return nfs_ok; >>> } > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@xxxxxxxxxxxxxxx -- Chuck Lever chucklever@xxxxxxxxx
