Re: [PATCH v1 19/42] SUNRPC: Fix xdr_get_next_encode_buffer() page boundary handling

[Chuck Lever <chuck.lever@oracle.com>]

> On Mar 3, 2021, at 1:27 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
> 
> 
> 
>> On Mar 3, 2021, at 1:25 PM, Bruce Fields <bfields@fieldses.org> wrote:
>> 
>> On Wed, Mar 03, 2021 at 06:19:33PM +0000, Chuck Lever wrote:
>>> 
>>> 
>>>> On Mar 3, 2021, at 11:52 AM, Bruce Fields <bfields@fieldses.org> wrote:
>>>> 
>>>> On Wed, Mar 03, 2021 at 03:43:28PM +0000, Chuck Lever wrote:
>>>>> Why would that not be OK? the next call to xdr_get_next_encode_buffer()
>>>>> should do the right thing and bounce the new encoded data from the
>>>>> next page into this one again.
>>>>> 
>>>>> So far I have not encountered any problems. Would such a problem show
>>>>> up with some frequency under normal use, or would it be especially
>>>>> subtle?
>>>> 
>>>> I mainly just want to make sure we've got a coherent idea what this code
>>>> is doing....
>>> 
>>> Agreed, that's a good thing.
>> 
>> I'm also a little vague on what exactly the problem is you're running
>> into.  (Probably because I haven't really looked at the v3 readdir
>> encoding.)
>> 
>> Is it approaching the end of a page, or is it running out of buflen?
>> How exactly does it fail?
> 
> I don't recall exactly, it was a late last summer when I wrote all these.
> 
> Approaching the end of a page, IIRC, the unpatched code would leave
> a gap in the directory entry stream.

Well, when I converted the entry encoders to use xdr_stream, it would
have a problem around the end of a page. The existing encoders are
open-coded to deal with this case.


>> --b.
>> 
>>> 
>>> 
>>>> For testing: large replies that aren't just read data are readdir and
>>>> getacl.  So reading large directories with lots of variably-named files
>>>> might be good. Also pynfs could easily send a single compound with lots
>>>> of variable-sized reads, that might be interesting.
>>> 
>>> I've run the full git regression suite over NFSv3 many many times with
>>> this patch applied. That includes unpacking via tar, a full build from
>>> scratch, and then running thousands of individual tests.
>>> 
>>> So that doesn't exercise a particular corner case, but it does reflect
>>> a broad variety of directory operations.
>>> 
>>> 
>>>> Constructing a compound that will result in xdr_reserve_space calls that
>>>> exactly hit the various corner cases may be hard.  But maybe if we just
>>>> send a bunch of compounds that vary over some range we can still
>>>> guarantee hitting those cases.
>>>> 
>>>> --b.
>>> 
>>> --
>>> Chuck Lever
> 
> --
> Chuck Lever

--
Chuck Lever