On 08/07/2024 0:49, Rick Macklem 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 didn't think to mention that the client was a rather old 6.3 kernel.
If the Linux client is still not setting OPEN4_SHARE_ACCESS_WANT_xxx
flags, then maybe it should do so?
To me, it seems simple enough to make it set the want flags, but I don't
have any context to why it wasn't done up until now...
Perhaps Trond and/or Anna can chime in.
