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

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.

> 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