On Mon, Jul 8, 2024 at 12:05 AM Rick Macklem <rick.macklem@xxxxxxxxx> wrote:
>
> On Sun, Jul 7, 2024 at 1:21 PM Sagi Grimberg <sagi@xxxxxxxxxxx> wrote:
> >
> >
> >
> > On 07/07/2024 22:05, Rick Macklem wrote:
> > > On Sun, Jul 7, 2024 at 4:07 AM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > >> On Sun, 2024-07-07 at 01:42 +0300, Sagi Grimberg wrote:
> > >>> Many applications open files with O_WRONLY, fully intending to write
> > >>> into the opened file. There is no reason why these applications should
> > >>> not enjoy a write delegation handed to them.
> > >>>
> > >>> Cc: Dai Ngo <dai.ngo@xxxxxxxxxx>
> > >>> Signed-off-by: Sagi Grimberg <sagi@xxxxxxxxxxx>
> > >>> ---
> > >>> Note: I couldn't find any reason to why the initial implementation chose
> > >>> to offer write delegations only to NFS4_SHARE_ACCESS_BOTH, but it seemed
> > >>> like an oversight to me. So I figured why not just send it out and see who
> > >>> objects...
> > >>>
> > >>> fs/nfsd/nfs4state.c | 10 +++++-----
> > >>> 1 file changed, 5 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > >>> index a20c2c9d7d45..69d576b19eb6 100644
> > >>> --- a/fs/nfsd/nfs4state.c
> > >>> +++ b/fs/nfsd/nfs4state.c
> > >>> @@ -5784,15 +5784,15 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > >>> * "An OPEN_DELEGATE_WRITE delegation allows the client to handle,
> > >>> * on its own, all opens."
> > >>> *
> > >>> - * Furthermore the client can use a write delegation for most READ
> > >>> - * operations as well, so we require a O_RDWR file here.
> > >>> - *
> > >>> - * Offer a write delegation in the case of a BOTH open, and ensure
> > >>> - * we get the O_RDWR descriptor.
> > >>> + * Offer a write delegation in the case of a BOTH open (ensure
> > >>> + * a O_RDWR descriptor) Or WRONLY open (with a O_WRONLY descriptor).
> > >>> */
> > >>> if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
> > >>> nf = find_rw_file(fp);
> > >>> dl_type = NFS4_OPEN_DELEGATE_WRITE;
> > >>> + } else if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
> > >>> + nf = find_writeable_file(fp);
> > >>> + dl_type = NFS4_OPEN_DELEGATE_WRITE;
> > >>> }
> > >>>
> > >>> /*
> > >>
> > >> I *think* the main reason we limited this before is because a write
> > >> delegation is really a read/write delegation. There is no such thing as
> > >> a write-only delegation.
> > >>
> > >> Suppose the user is prevented from doing reads against the inode (by
> > >> permission bits or ACLs). The server gives out a WRITE delegation on a
> > >> O_WRONLY open. Will the client allow cached opens for read regardless
> > >> of the server's permissions? Or, does it know to check vs. the server
> > >> if the client tries to do an open for read in this situation?
> > > I was curious and tried a simple test yesterday, using the FreeBSD server
> > > configured to issue a write delegation for a write-only open.
> > > The test simply opened write-only first and then read-only, for a file
> > > with mode 222. It worked as expected for both the Linux and FreeBSD
> > > clients (ie. returned a failure for the read-only open).
> > > I've attached the packet capture for the Linux client, in case you are
> > > interested.
> >
> > Nice, thanks for testing!
> >
> > >
> > > I do believe this is allowed by the RFCs. In fact I think the RFCs
> > > allow a server
> > > to issue a write delegation for a read open (not that I am convinced that is a
> > > good idea). The main thing to check is what the ACE in the write delegation
> > > reply looks like, since my understanding is that the client is expected to do
> > > an Access unless the ACE allows access.
> > > Here's a little snippet:
> > > nfsace4 permissions; /* Defines users who don't
> > > need an ACCESS call as
> > > part of a delegated
> > > open. */
> >
> > Yes, I had the same understanding...
> >
> > >
> > > Now, is it a good idea to do this?
> > > Well, my opinion (as the outsider;-) is that the server should follow whatever
> > > the client requests, as far as practicable. ie. The OPEN_WANT flags:
> > > const OPEN4_SHARE_ACCESS_WANT_NO_PREFERENCE = 0x0000;
> > > const OPEN4_SHARE_ACCESS_WANT_READ_DELEG = 0x0100;
> > > const OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG = 0x0200;
> > > const OPEN4_SHARE_ACCESS_WANT_ANY_DELEG = 0x0300;
> > > const OPEN4_SHARE_ACCESS_WANT_NO_DELEG = 0x0400;
> > > If the client does not provide these (4.0 or 4.1/4.2, but no flags), then I
> > > suppose I might be concerned that there is a buggy client out there that
> > > would do what Jeff suggested (ie. assume it can open the file for reading
> > > after getting a write delegation).
> >
> > Well, that is obviously not what the Linux client is asking for today,
> > but even if it
> > did, what would it set in the OPEN_WANT flags for O_WRONLY open? it would
> > set OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG.
> >
> > I do agree that there is an argument to be made that Linux nfsd should not
> > hand out any delegation that is not explicitly solicited by the client,
> > but I don't see
> > how this particular delegation against a wronly open is any different
> > from any
> > other type of delegation that Linux nfsd hands out today.
> I guess the point I was trying to make was that, if a client explicitly
> asks for a write delegation, then you issue it and the client screws up,
> "the client got what it asked for".
>
> However, if the client has not previously seen a write delegation for this
> case and is not asking for it, then if it is buggy in this respect, a
> server upgrade
> results in a client failure (fun and bothersome to track down, even if
> it really a
> client bug). ie. There is a risk in this change (as Jeff noted) and imho that
> needs to be considered (and tested for as far as practicable).
>
> Anyhow, all the best and good luck with it, rick
> ps: I suspect you guys are like me, in that you do not have easy access
> to other clients, like the resurrected CITI Windows client or the
> VMware one. (I think there is also a NFSv4.1 client in Windows
> server edition?)
Windows Server only has a NFS4.1 server, but no client.
NFSv4.1 clients for Windows are available from OpenText and
https://github.com/kofemann/ms-nfs41-client
Thanks,
Martin
