On Wed, Sep 25, 2024 at 11:33 PM Rick Macklem <rick.macklem@xxxxxxxxx> wrote:
>
> On Wed, Sep 25, 2024 at 8:03 AM Martin Wege <martin.l.wege@xxxxxxxxx> wrote:
> >
> > CAUTION: This email originated from outside of the University of Guelph. Do not click links or open attachments unless you recognize the sender and know the content is safe. If in doubt, forward suspicious emails to IThelp@xxxxxxxxxxx.
> >
> >
> > 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
> I'll believe you. (I have no access to Windows servers.)
> What was weird is that I had someone with a problem a few
> years ago, who claimed they were using a NFSv4.1 client in Windows server.
The Windows NFS41 client project was done by CITI at the behest of
Microsoft&SUN Microsystems. Then Oracle took over SUN, stopped
funding, and instead tried to milk money out of the stakeholders for
"support", and the project was quickly abandoned.
https://github.com/kofemann/ms-nfs41-client is based on CITIs original
work where they left off, with major improvements to get it useable
for production systems
> I even "corrected" them, but they claimed they did have it and
> indicated that they had gotten it from Microsoft? Maybe they had the
> OpenText one and he didn't realize it. I hadn't heard of the OpenText
> one (used to be Hummingbird long ago).
Nope, the binaries for the MS NFS41 client popped up around 2008-2010.
Read https://www.heise.de/news/Quelloffener-Windows-Treiber-fuer-NFSv4-1-fertiggestellt-1103643.html
OpenText is NFS4.0, CITI is NFS4.1, the resurrected CITI is NFS4.2
Thanks,
Martin
