Re: NFSv4.2 server replies to Copy with length == 0

On 10/21/2019 10:51 AM, Olga Kornievskaia wrote:
> On Fri, Oct 18, 2019 at 5:50 PM Tom Talpey <[email protected]> wrote:
>>
>> On 10/18/2019 4:50 PM, Olga Kornievskaia wrote:
>>> On Fri, Oct 18, 2019 at 4:26 PM Tom Talpey <[email protected]> wrote:
>>>>
>>>> On 10/17/2019 5:34 PM, Rick Macklem wrote:
>>>>> Olga Kornievskaia wrote:
>>>>>> On Thu, Oct 17, 2019 at 12:20 PM Rick Macklem <[email protected]> >wrote:
>>>>>>>
>>>>>>> Tom Talpey wrote:
>>>>>>>> On 10/17/2019 11:22 AM, J. Bruce Fields wrote:
>>>>>>>>> On Thu, Oct 17, 2019 at 02:16:36AM +0000, Rick Macklem wrote:
>>>>>>>>>> I have now found two cases where the Linux NFSv4.2 server does not
>>>>>>>>>> conform to RFC-7862. One is as above and the other is a reply to Seek
>>>>>>>>>> of NFS4ERR_NXIO when the sa_offset argument == file_size (instead of
>>>>>>>>>> replying NFS_OK along with sr_eof == true).
>>>>>>>>>
>>>>>>>>> Huh.  Looks like that's documented behavior of Linux's seek.  (See the
>>>>>>>>> ERRORS section of the lseek(2) man page.)  Looks like Solaris also
>>>>>>>>> returns -ENXIO in this case:
>>>>>>>>>
>>>>>>>>>          https://docs.oracle.com/cd/E26502_01/html/E29032/lseek-2.html
>>>>>>>>>
>>>>>>>>> And freebsd too:
>>>>>>>>>
>>>>>>>>>          https://www.freebsd.org/cgi/man.cgi?query=lseek&sektion=2
>>>>>>>>>
>>>>>>>>> I wonder where that spec language came from?
>>>>>>>>
>>>>>>>> Those manpages look like ENXIO comes back only on sparse files. Perhaps
>>>>>>>> this is boilerplate from v4.0 before this kind of thing was common.
>>>>>>> As far as I know, a non-sparse file (no holes) behaves the same as a file with
>>>>>>> holes in it. (I'm not sure if the POSIX draft is clear for the case where the
>>>>>>> file system does not support holes, but I think most implementations handle
>>>>>>> that the same way as a file with no holes on a file system that supports holes.)
>>>>>>>
>>>>>>>> This should at least be discussed on [email protected]...
>>>>>>> I think there needs to be a discussion on how to best deal with cases where
>>>>>>> implementations have shipped to users with these glitches in them.
>>>>>>>
>>>>>>> I think it might be better to document them, so that client coders can
>>>>>>> implement work arounds (I am doing so for the three cases I've found)
>>>>>>> instead of the server being patched to change its behaviour, causing
>>>>>>> potentially more interoperability issues.
>>>>>>> (That's why I re-posted this to [email protected].)
>>>>>>>
>>>>>>>>> Our NFS server could translate an -ENXIO return into 0 and sr_eof ==
>>>>>>>>> true easily enough, assuming -ENXIO is really only ever returned in that
>>>>>>>>> case.
>>>>>>>>>
>>>>>>>>> I haven't tested, but from a quick check of the Linux client code I
>>>>>>>>> think that would require a matching fix on the client side to translate
>>>>>>>>> sr_eof == 0 *back* to ENXIO.
>>>>>>>>>
>>>>>>>>> I don't know if it's worth it.
>>>>>>>>
>>>>>>>> What Bad Thing would happen for the difference?
>>>>>>> Well, for a POSIX draft style client, the only effect is that a client may be
>>>>>>> broken (not handle the sr_eof == true reply correctly) without the
>>>>>>> implementors knowing that, since they tested against the Linux server.
>>>>>>> (I believe this is the current status of the Linux client.)
>>>>>>>
>>>>>>> For a server implementor (I get to wear both hats;-), the server can only
>>>>>>> generate one reply or the other. I've implemented both with a tunable
>>>>>>> that can be used to flip between them. I default to the NFS4ERR_NXIO
>>>>>>> reply, since that is what the Linux client expects.
>>>>>>>
>>>>>>> As for the Copy issues, the client can easily handle them, but the client
>>>>>>> coder needs to know about them.
>>>>>>> At this time the FreeBSD server code only implements what is in the RFC.
>>>>>>> This seems sufficient for the testing I've done sofar.
>>>>>>> The case that I think will break would be Linux code that does a
>>>>>>> copy_file_range(infd, NULL, outfd, NULL, INT_MAX, 0) to copy an entire
>>>>>>> file. I'll test this case to-day, but I'm think it will just get a NFS4ERR_INVAL
>>>>>>> reply from the FreeBSD server.
>>>>>>
>>>>>> this might be problematic for the linux client because though there
>>>>>> isn't an official use of it but the libc cp might implement this doing
>>>>>> exactly what you say here copy_file_range(infd, NULL, outfd, NULL,
>>>>>> INT_MAX, 0). It doesn't try to determine the size of the file before
>>>>>> the copy and
>>>>>> just copy max bytes assuming that it'll get a short read at some point.
>>>>> I just tested this and the Linux client does send a Copy with a
>>>>> ca_count == INT_MAX.
>>>>>
>>>>> Truth be told, it worked fine, because my server code neglected to do the
>>>>> required ca_src_offset + ca_count > file_size --> NFS4ERR_INVAL.
>>>>> I'm now adding the check, but it will only be enabled by a tunable not
>>>>> set by default.
>>>>>
>>>>> I am going to post on [email protected], to see if we can just "clarify" the RFC
>>>>> to do what Linux already does.
>>>>
>>>> Technically speaking, this is not Linux behavior but libc's. Of course,
>>>> this may be a moot distinction, because changing applications can be
>>>> even harder than changing kernels. But for IETF discussion purposes
>>>> I don't think it's relevant to lump it into "Linux behavior".
>>>>
>>>> Question though. Could the Linux kernel client recognize this type of
>>>> API request, and morph it into a valid NFS wire operation? Such "torque
>>>> conversion" is basically the client's job, after all.
>>>
>>> I believe the reason the (userland) client would do a copy of INT_MAX
>>> bytes is because it want to avoid trying to query the size of the file
>>> then copying only that amount of bytes since the file size could have
>>> changed between the query and the copy. Same reason even in the kernel
>>> we'd rather not query and then copy.
>>
>> The application (which is not the same as the client) is simply
>> expressing its request to copy the entire file. That's perfectly
>> natural for a local API.
>>
>> The kernel client could just as easily translate this to a valid NFS
>> copy by pulling the size from its attribute cache, or sending a getattr
>> and using the result. After all, the copy operation is expensive and
>> long-running, it's basically no extra overhead.
> 
> Even if the kernel checks the size of the file, there is nothing that
> prevents that size from changing (ie, racing truncate) so even then a
> copy call with a "historically correct" offsets and length would still
> get EINVAL.

That is no different from any other NFS operation. In fact, it is
arguably correct that the operation should fail in such a case.

If the client needs sharing protection, it needs to take a lock, lease
or whatever. The core protocol already defines these.

Tom.

>> In other words, I don't buy the argument that this can't and shouldn't
>> be done.
>>
>>> While linux server doesn't implement count=0 copy and in theory it
>>> probably should, what the (nfs kernel) client can do if it receives a
>>> request from VFS to copy INT_MAX it can translate that to count=0
>>> copy.
>>>
>>> The problem we currently face of having two different ways of doing a
>>> copy: a traditional copy which if at some point requests beyond the
>>> end of the file it doesn't expect to fail and expects a short read vs
>>> doing something like copy_file_range() that uses the nfs copy_offload
>>> that if that were to read beyond the end of the input file and if the
>>> NFS spec reqs are enforced it would fail with EINVAL. That creates
>>> inconsistent results using different copy mechanisms.
>>
>> Ok, this is significant, the operations arguably should have similar
>> semantics. But, if there's a simple set of required processing that
>> the client isn't doing, which will erase the difference, why not do
>> those?
>>
>> Tom.
>>
>>>
>>>
>>>
>>>>
>>>> Tom.
>>>>
>>>>> rick
>>>>>
>>>>>> (I can make this work for the Linux client, but it will take another "cheat"
>>>>>>     enabled via a tunable.)
>>>>>> --> Part of the confusion here comes from the fact that the Linux syscall
>>>>>>           semantics for copy_file_range() has changed. (I also where the
>>>>>>           copy_file_range(2) hat for FreeBSD, so I've got some work to do there, too.)
>>>>>>
>>>>>> In summary, I think that, since Linux has been shipping this for some time,
>>>>>> documenting workarounds is a practical approach.
>>>>>>
>>>>>> rick
>>>>>>
>>>>>> _______________________________________________
>>>>>> nfsv4 mailing list
>>>>>> [email protected]
>>>>>> https://www.ietf.org/mailman/listinfo/nfsv4
>>>>>
>>>>> _______________________________________________
>>>>> nfsv4 mailing list
>>>>> [email protected]
>>>>> https://www.ietf.org/mailman/listinfo/nfsv4
>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> nfsv4 mailing list
>>>> [email protected]
>>>> https://www.ietf.org/mailman/listinfo/nfsv4
>>>
>>> _______________________________________________
>>> nfsv4 mailing list
>>> [email protected]
>>> https://www.ietf.org/mailman/listinfo/nfsv4
>>>
>>>
>>
>> _______________________________________________
>> nfsv4 mailing list
>> [email protected]
>> https://www.ietf.org/mailman/listinfo/nfsv4
> 
> _______________________________________________
> nfsv4 mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/nfsv4
> 
> 

_______________________________________________
nfsv4 mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/nfsv4