Tidy up the server-side XDR encoders for NFSv4 OPEN and LOCK
results. Series applies to nfsd-next. See topic branch
"nfsd4-encoder-overhaul" in this repo:
https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
---
Chuck Lever (7):
NFSD: Add nfsd4_encode_lock_owner4()
NFSD: Refactor nfsd4_encode_lock_denied()
NFSD: Add nfsd4_encode_open_read_delegation4()
NFSD: Add nfsd4_encode_open_write_delegation4()
NFSD: Add nfsd4_encode_open_none_delegation4()
NFSD: Add nfsd4_encode_open_delegation4()
NFSD: Clean up nfsd4_encode_open()
fs/nfsd/nfs4state.c | 6 +-
fs/nfsd/nfs4xdr.c | 305 +++++++++++++++++++++++++++-----------------
fs/nfsd/xdr4.h | 2 +-
3 files changed, 189 insertions(+), 124 deletions(-)
--
Chuck Lever
From: Chuck Lever <[email protected]>
To better align our implementation with the XDR specification,
refactor the part of nfsd4_encode_open() that encodes the
open_none_delegation4 type.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4xdr.c | 42 ++++++++++++++++++++++++------------------
1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f7e5f54fda00..b6c5ccb9351f 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4149,6 +4149,27 @@ nfsd4_encode_open_write_delegation4(struct xdr_stream *xdr,
return nfsd4_encode_open_nfsace4(xdr);
}
+static __be32
+nfsd4_encode_open_none_delegation4(struct xdr_stream *xdr,
+ struct nfsd4_open *open)
+{
+ __be32 status = nfs_ok;
+
+ /* ond_why */
+ if (xdr_stream_encode_u32(xdr, open->op_why_no_deleg) != XDR_UNIT)
+ return nfserr_resource;
+ switch (open->op_why_no_deleg) {
+ case WND4_CONTENTION:
+ /* ond_server_will_push_deleg */
+ status = nfsd4_encode_bool(xdr, false);
+ break;
+ case WND4_RESOURCE:
+ /* ond_server_will_signal_avail */
+ status = nfsd4_encode_bool(xdr, false);
+ }
+ return status;
+}
+
static __be32
nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
union nfsd4_op_u *u)
@@ -4185,24 +4206,9 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
case NFS4_OPEN_DELEGATE_WRITE:
/* write */
return nfsd4_encode_open_write_delegation4(xdr, open);
- case NFS4_OPEN_DELEGATE_NONE_EXT: /* 4.1 */
- switch (open->op_why_no_deleg) {
- case WND4_CONTENTION:
- case WND4_RESOURCE:
- p = xdr_reserve_space(xdr, 8);
- if (!p)
- return nfserr_resource;
- *p++ = cpu_to_be32(open->op_why_no_deleg);
- /* deleg signaling not supported yet: */
- *p++ = cpu_to_be32(0);
- break;
- default:
- p = xdr_reserve_space(xdr, 4);
- if (!p)
- return nfserr_resource;
- *p++ = cpu_to_be32(open->op_why_no_deleg);
- }
- break;
+ case NFS4_OPEN_DELEGATE_NONE_EXT:
+ /* od_whynone */
+ return nfsd4_encode_open_none_delegation4(xdr, open);
default:
BUG();
}
From: Chuck Lever <[email protected]>
Use the modern XDR utility functions.
The LOCK and LOCKT encoder functions need to return nfserr_denied
when a lock is denied, but nfsd4_encode_lock4denied() should return
a status code that is consistent with other XDR encoders.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4xdr.c | 73 ++++++++++++++++++++++++++---------------------------
1 file changed, 36 insertions(+), 37 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 9f7a6924ef5f..ee8a7989f54f 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3990,40 +3990,26 @@ nfsd4_encode_lock_owner4(struct xdr_stream *xdr, const clientid_t *clientid,
return nfsd4_encode_opaque(xdr, owner->data, owner->len);
}
-/*
-* Including all fields other than the name, a LOCK4denied structure requires
-* 8(clientid) + 4(namelen) + 8(offset) + 8(length) + 4(type) = 32 bytes.
-*/
static __be32
-nfsd4_encode_lock_denied(struct xdr_stream *xdr, struct nfsd4_lock_denied *ld)
+nfsd4_encode_lock4denied(struct xdr_stream *xdr,
+ const struct nfsd4_lock_denied *ld)
{
- struct xdr_netobj *conf = &ld->ld_owner;
- __be32 *p, status;
+ __be32 status;
-again:
- p = xdr_reserve_space(xdr, XDR_UNIT * 5);
- if (!p) {
- /*
- * Don't fail to return the result just because we can't
- * return the conflicting open:
- */
- if (conf->len) {
- kfree(conf->data);
- conf->len = 0;
- conf->data = NULL;
- goto again;
- }
- return nfserr_resource;
- }
- p = xdr_encode_hyper(p, ld->ld_start);
- p = xdr_encode_hyper(p, ld->ld_length);
- *p++ = cpu_to_be32(ld->ld_type);
- status = nfsd4_encode_lock_owner4(xdr, &ld->ld_clientid,
- &ld->ld_owner);
+ /* offset */
+ status = nfsd4_encode_offset4(xdr, ld->ld_start);
if (status != nfs_ok)
return status;
-
- return nfserr_denied;
+ /* length */
+ status = nfsd4_encode_length4(xdr, ld->ld_length);
+ if (status != nfs_ok)
+ return status;
+ /* locktype */
+ if (xdr_stream_encode_u32(xdr, ld->ld_type) != XDR_UNIT)
+ return nfserr_resource;
+ /* owner */
+ return nfsd4_encode_lock_owner4(xdr, &ld->ld_clientid,
+ &ld->ld_owner);
}
static __be32
@@ -4032,13 +4018,21 @@ nfsd4_encode_lock(struct nfsd4_compoundres *resp, __be32 nfserr,
{
struct nfsd4_lock *lock = &u->lock;
struct xdr_stream *xdr = resp->xdr;
+ __be32 status;
- if (!nfserr)
- nfserr = nfsd4_encode_stateid4(xdr, &lock->lk_resp_stateid);
- else if (nfserr == nfserr_denied)
- nfserr = nfsd4_encode_lock_denied(xdr, &lock->lk_denied);
-
- return nfserr;
+ switch (nfserr) {
+ case nfs_ok:
+ /* resok4 */
+ status = nfsd4_encode_stateid4(xdr, &lock->lk_resp_stateid);
+ break;
+ case nfserr_denied:
+ /* denied */
+ status = nfsd4_encode_lock4denied(xdr, &lock->lk_denied);
+ break;
+ default:
+ return nfserr;
+ }
+ return status != nfs_ok ? status : nfserr;
}
static __be32
@@ -4047,9 +4041,14 @@ nfsd4_encode_lockt(struct nfsd4_compoundres *resp, __be32 nfserr,
{
struct nfsd4_lockt *lockt = &u->lockt;
struct xdr_stream *xdr = resp->xdr;
+ __be32 status;
- if (nfserr == nfserr_denied)
- nfsd4_encode_lock_denied(xdr, &lockt->lt_denied);
+ if (nfserr == nfserr_denied) {
+ /* denied */
+ status = nfsd4_encode_lock4denied(xdr, &lockt->lt_denied);
+ if (status != nfs_ok)
+ return status;
+ }
return nfserr;
}
From: Chuck Lever <[email protected]>
Make it easier to adjust the XDR encoder to handle new features
related to write delegations.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4xdr.c | 59 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 33 insertions(+), 26 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f411fcc435f6..f7e5f54fda00 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4118,6 +4118,37 @@ nfsd4_encode_open_read_delegation4(struct xdr_stream *xdr, struct nfsd4_open *op
return nfsd4_encode_open_nfsace4(xdr);
}
+static __be32
+nfsd4_encode_nfs_space_limit4(struct xdr_stream *xdr, u64 filesize)
+{
+ /* limitby */
+ if (xdr_stream_encode_u32(xdr, NFS4_LIMIT_SIZE) != XDR_UNIT)
+ return nfserr_resource;
+ /* filesize */
+ return nfsd4_encode_uint64_t(xdr, filesize);
+}
+
+static __be32
+nfsd4_encode_open_write_delegation4(struct xdr_stream *xdr,
+ struct nfsd4_open *open)
+{
+ __be32 status;
+
+ /* stateid */
+ status = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
+ if (status != nfs_ok)
+ return status;
+ /* recall */
+ status = nfsd4_encode_bool(xdr, open->op_recall);
+ if (status != nfs_ok)
+ return status;
+ /* space_limit */
+ status = nfsd4_encode_nfs_space_limit4(xdr, 0);
+ if (status != nfs_ok)
+ return status;
+ return nfsd4_encode_open_nfsace4(xdr);
+}
+
static __be32
nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
union nfsd4_op_u *u)
@@ -4152,32 +4183,8 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
/* read */
return nfsd4_encode_open_read_delegation4(xdr, open);
case NFS4_OPEN_DELEGATE_WRITE:
- nfserr = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
- if (nfserr)
- return nfserr;
-
- p = xdr_reserve_space(xdr, XDR_UNIT * 8);
- if (!p)
- return nfserr_resource;
- *p++ = cpu_to_be32(open->op_recall);
-
- /*
- * Always flush on close
- *
- * TODO: space_limit's in delegations
- */
- *p++ = cpu_to_be32(NFS4_LIMIT_SIZE);
- *p++ = xdr_zero;
- *p++ = xdr_zero;
-
- /*
- * TODO: ACE's in delegations
- */
- *p++ = cpu_to_be32(NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE);
- *p++ = cpu_to_be32(0);
- *p++ = cpu_to_be32(0);
- *p++ = cpu_to_be32(0); /* XXX: is NULL principal ok? */
- break;
+ /* write */
+ return nfsd4_encode_open_write_delegation4(xdr, open);
case NFS4_OPEN_DELEGATE_NONE_EXT: /* 4.1 */
switch (open->op_why_no_deleg) {
case WND4_CONTENTION:
From: Chuck Lever <[email protected]>
Refactor nfsd4_encode_open() so the open_read_delegation4 type is
encoded in a separate function. This makes it more straightforward
to later add support for returning an nfsace4 in OPEN responses that
offer a delegation.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4state.c | 6 +++--
fs/nfsd/nfs4xdr.c | 61 ++++++++++++++++++++++++++++++++++++++-------------
fs/nfsd/xdr4.h | 2 +-
3 files changed, 49 insertions(+), 20 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 22e95b9ae82f..b1118050ff52 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5688,11 +5688,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
struct path path;
cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
- open->op_recall = 0;
+ open->op_recall = false;
switch (open->op_claim_type) {
case NFS4_OPEN_CLAIM_PREVIOUS:
if (!cb_up)
- open->op_recall = 1;
+ open->op_recall = true;
break;
case NFS4_OPEN_CLAIM_NULL:
parent = currentfh;
@@ -5746,7 +5746,7 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS &&
open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE) {
dprintk("NFSD: WARNING: refusing delegation reclaim\n");
- open->op_recall = 1;
+ open->op_recall = true;
}
/* 4.1 client asking for a delegation? */
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index ee8a7989f54f..f411fcc435f6 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4074,6 +4074,49 @@ nfsd4_encode_link(struct nfsd4_compoundres *resp, __be32 nfserr,
return nfsd4_encode_change_info4(xdr, &link->li_cinfo);
}
+/*
+ * This implementation does not yet support returning an ACE in an
+ * OPEN that offers a delegation.
+ */
+static __be32
+nfsd4_encode_open_nfsace4(struct xdr_stream *xdr)
+{
+ __be32 status;
+
+ /* type */
+ status = nfsd4_encode_acetype4(xdr, NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE);
+ if (status != nfs_ok)
+ return nfserr_resource;
+ /* flag */
+ status = nfsd4_encode_aceflag4(xdr, 0);
+ if (status != nfs_ok)
+ return nfserr_resource;
+ /* access mask */
+ status = nfsd4_encode_acemask4(xdr, 0);
+ if (status != nfs_ok)
+ return nfserr_resource;
+ /* who - empty for now */
+ if (xdr_stream_encode_u32(xdr, 0) != XDR_UNIT)
+ return nfserr_resource;
+ return nfs_ok;
+}
+
+static __be32
+nfsd4_encode_open_read_delegation4(struct xdr_stream *xdr, struct nfsd4_open *open)
+{
+ __be32 status;
+
+ /* stateid */
+ status = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
+ if (status != nfs_ok)
+ return status;
+ /* recall */
+ status = nfsd4_encode_bool(xdr, open->op_recall);
+ if (status != nfs_ok)
+ return status;
+ /* permissions */
+ return nfsd4_encode_open_nfsace4(xdr);
+}
static __be32
nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
@@ -4106,22 +4149,8 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
case NFS4_OPEN_DELEGATE_NONE:
break;
case NFS4_OPEN_DELEGATE_READ:
- nfserr = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
- if (nfserr)
- return nfserr;
- p = xdr_reserve_space(xdr, 20);
- if (!p)
- return nfserr_resource;
- *p++ = cpu_to_be32(open->op_recall);
-
- /*
- * TODO: ACE's in delegations
- */
- *p++ = cpu_to_be32(NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE);
- *p++ = cpu_to_be32(0);
- *p++ = cpu_to_be32(0);
- *p++ = cpu_to_be32(0); /* XXX: is NULL principal ok? */
- break;
+ /* read */
+ return nfsd4_encode_open_read_delegation4(xdr, open);
case NFS4_OPEN_DELEGATE_WRITE:
nfserr = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
if (nfserr)
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index aba07d5378fc..c142a9b5ab98 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -389,7 +389,7 @@ struct nfsd4_open {
u32 op_deleg_want; /* request */
stateid_t op_stateid; /* response */
__be32 op_xdr_error; /* see nfsd4_open_omfg() */
- u32 op_recall; /* recall */
+ bool op_recall; /* response */
struct nfsd4_change_info op_cinfo; /* response */
u32 op_rflags; /* response */
bool op_truncate; /* used during processing */
From: Chuck Lever <[email protected]>
To better align our implementation with the XDR specification,
refactor the part of nfsd4_encode_open() that encodes delegation
metadata.
As part of that refactor, remove an unnecessary BUG() call site and
a comment that appears to be stale.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4xdr.c | 56 +++++++++++++++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 23 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index b6c5ccb9351f..f37c370ceded 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4170,13 +4170,43 @@ nfsd4_encode_open_none_delegation4(struct xdr_stream *xdr,
return status;
}
+static __be32
+nfsd4_encode_open_delegation4(struct xdr_stream *xdr, struct nfsd4_open *open)
+{
+ __be32 status;
+
+ /* delegation_type */
+ if (xdr_stream_encode_u32(xdr, open->op_delegate_type) != XDR_UNIT)
+ return nfserr_resource;
+ switch (open->op_delegate_type) {
+ case NFS4_OPEN_DELEGATE_NONE:
+ status = nfs_ok;
+ break;
+ case NFS4_OPEN_DELEGATE_READ:
+ /* read */
+ status = nfsd4_encode_open_read_delegation4(xdr, open);
+ break;
+ case NFS4_OPEN_DELEGATE_WRITE:
+ /* write */
+ status = nfsd4_encode_open_write_delegation4(xdr, open);
+ break;
+ case NFS4_OPEN_DELEGATE_NONE_EXT:
+ /* od_whynone */
+ status = nfsd4_encode_open_none_delegation4(xdr, open);
+ break;
+ default:
+ status = nfserr_serverfault;
+ }
+
+ return status;
+}
+
static __be32
nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
union nfsd4_op_u *u)
{
struct nfsd4_open *open = &u->open;
struct xdr_stream *xdr = resp->xdr;
- __be32 *p;
nfserr = nfsd4_encode_stateid4(xdr, &open->op_stateid);
if (nfserr)
@@ -4192,28 +4222,8 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
if (nfserr)
return nfserr;
- p = xdr_reserve_space(xdr, 4);
- if (!p)
- return nfserr_resource;
-
- *p++ = cpu_to_be32(open->op_delegate_type);
- switch (open->op_delegate_type) {
- case NFS4_OPEN_DELEGATE_NONE:
- break;
- case NFS4_OPEN_DELEGATE_READ:
- /* read */
- return nfsd4_encode_open_read_delegation4(xdr, open);
- case NFS4_OPEN_DELEGATE_WRITE:
- /* write */
- return nfsd4_encode_open_write_delegation4(xdr, open);
- case NFS4_OPEN_DELEGATE_NONE_EXT:
- /* od_whynone */
- return nfsd4_encode_open_none_delegation4(xdr, open);
- default:
- BUG();
- }
- /* XXX save filehandle here */
- return 0;
+ /* delegation */
+ return nfsd4_encode_open_delegation4(xdr, open);
}
static __be32
From: Chuck Lever <[email protected]>
Finish cleaning up nfsd4_encode_open().
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4xdr.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f37c370ceded..143efc24551d 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4208,20 +4208,23 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
struct nfsd4_open *open = &u->open;
struct xdr_stream *xdr = resp->xdr;
+ /* stateid */
nfserr = nfsd4_encode_stateid4(xdr, &open->op_stateid);
- if (nfserr)
+ if (nfserr != nfs_ok)
return nfserr;
+ /* cinfo */
nfserr = nfsd4_encode_change_info4(xdr, &open->op_cinfo);
- if (nfserr)
+ if (nfserr != nfs_ok)
return nfserr;
- if (xdr_stream_encode_u32(xdr, open->op_rflags) < 0)
- return nfserr_resource;
-
+ /* rflags */
+ nfserr = nfsd4_encode_uint32_t(xdr, open->op_rflags);
+ if (nfserr != nfs_ok)
+ return nfserr;
+ /* attrset */
nfserr = nfsd4_encode_bitmap4(xdr, open->op_bmval[0],
open->op_bmval[1], open->op_bmval[2]);
- if (nfserr)
+ if (nfserr != nfs_ok)
return nfserr;
-
/* delegation */
return nfsd4_encode_open_delegation4(xdr, open);
}
From: Chuck Lever <[email protected]>
To improve readability and better align the LOCK encoders with the
XDR specification, add an explicit encoder named for the lock_owner4
type.
In particular, to avoid code duplication, use
nfsd4_encode_clientid4() to encode the clientid in the lock owner
rather than open-coding it.
It looks to me like nfs4_set_lock_denied() already clears the
clientid if it won't return an owner (cf: the nevermind: label). The
code in the XDR encoder appears to be redundant and can safely be
removed.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4xdr.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index d21aaa56c49a..9f7a6924ef5f 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3976,6 +3976,20 @@ nfsd4_encode_getfh(struct nfsd4_compoundres *resp, __be32 nfserr,
return nfsd4_encode_nfs_fh4(xdr, &fhp->fh_handle);
}
+static __be32
+nfsd4_encode_lock_owner4(struct xdr_stream *xdr, const clientid_t *clientid,
+ const struct xdr_netobj *owner)
+{
+ __be32 status;
+
+ /* clientid */
+ status = nfsd4_encode_clientid4(xdr, clientid);
+ if (status != nfs_ok)
+ return status;
+ /* owner */
+ return nfsd4_encode_opaque(xdr, owner->data, owner->len);
+}
+
/*
* Including all fields other than the name, a LOCK4denied structure requires
* 8(clientid) + 4(namelen) + 8(offset) + 8(length) + 4(type) = 32 bytes.
@@ -3984,10 +3998,10 @@ static __be32
nfsd4_encode_lock_denied(struct xdr_stream *xdr, struct nfsd4_lock_denied *ld)
{
struct xdr_netobj *conf = &ld->ld_owner;
- __be32 *p;
+ __be32 *p, status;
again:
- p = xdr_reserve_space(xdr, 32 + XDR_LEN(conf->len));
+ p = xdr_reserve_space(xdr, XDR_UNIT * 5);
if (!p) {
/*
* Don't fail to return the result just because we can't
@@ -4004,14 +4018,11 @@ nfsd4_encode_lock_denied(struct xdr_stream *xdr, struct nfsd4_lock_denied *ld)
p = xdr_encode_hyper(p, ld->ld_start);
p = xdr_encode_hyper(p, ld->ld_length);
*p++ = cpu_to_be32(ld->ld_type);
- if (conf->len) {
- p = xdr_encode_opaque_fixed(p, &ld->ld_clientid, 8);
- p = xdr_encode_opaque(p, conf->data, conf->len);
- kfree(conf->data);
- } else { /* non - nfsv4 lock in conflict, no clientid nor owner */
- p = xdr_encode_hyper(p, (u64)0); /* clientid */
- *p++ = cpu_to_be32(0); /* length of owner name */
- }
+ status = nfsd4_encode_lock_owner4(xdr, &ld->ld_clientid,
+ &ld->ld_owner);
+ if (status != nfs_ok)
+ return status;
+
return nfserr_denied;
}
On Fri, 2023-09-29 at 09:58 -0400, Chuck Lever wrote:
> Tidy up the server-side XDR encoders for NFSv4 OPEN and LOCK
> results. Series applies to nfsd-next. See topic branch
> "nfsd4-encoder-overhaul" in this repo:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>
> ---
>
> Chuck Lever (7):
> NFSD: Add nfsd4_encode_lock_owner4()
> NFSD: Refactor nfsd4_encode_lock_denied()
> NFSD: Add nfsd4_encode_open_read_delegation4()
> NFSD: Add nfsd4_encode_open_write_delegation4()
> NFSD: Add nfsd4_encode_open_none_delegation4()
> NFSD: Add nfsd4_encode_open_delegation4()
> NFSD: Clean up nfsd4_encode_open()
>
>
> fs/nfsd/nfs4state.c | 6 +-
> fs/nfsd/nfs4xdr.c | 305 +++++++++++++++++++++++++++-----------------
> fs/nfsd/xdr4.h | 2 +-
> 3 files changed, 189 insertions(+), 124 deletions(-)
>
> --
> Chuck Lever
>
Looks pretty straightforward. I had one minor nit about the struct
packing but the rest looks fine. You can add:
Reviewed-by: Jeff Layton <[email protected]>
On Fri, 2023-09-29 at 09:59 -0400, Chuck Lever wrote:
> From: Chuck Lever <[email protected]>
>
> Refactor nfsd4_encode_open() so the open_read_delegation4 type is
> encoded in a separate function. This makes it more straightforward
> to later add support for returning an nfsace4 in OPEN responses that
> offer a delegation.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 6 +++--
> fs/nfsd/nfs4xdr.c | 61 ++++++++++++++++++++++++++++++++++++++-------------
> fs/nfsd/xdr4.h | 2 +-
> 3 files changed, 49 insertions(+), 20 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 22e95b9ae82f..b1118050ff52 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5688,11 +5688,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> struct path path;
>
> cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
> - open->op_recall = 0;
> + open->op_recall = false;
> switch (open->op_claim_type) {
> case NFS4_OPEN_CLAIM_PREVIOUS:
> if (!cb_up)
> - open->op_recall = 1;
> + open->op_recall = true;
> break;
> case NFS4_OPEN_CLAIM_NULL:
> parent = currentfh;
> @@ -5746,7 +5746,7 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS &&
> open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE) {
> dprintk("NFSD: WARNING: refusing delegation reclaim\n");
> - open->op_recall = 1;
> + open->op_recall = true;
> }
>
> /* 4.1 client asking for a delegation? */
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index ee8a7989f54f..f411fcc435f6 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -4074,6 +4074,49 @@ nfsd4_encode_link(struct nfsd4_compoundres *resp, __be32 nfserr,
> return nfsd4_encode_change_info4(xdr, &link->li_cinfo);
> }
>
> +/*
> + * This implementation does not yet support returning an ACE in an
> + * OPEN that offers a delegation.
> + */
> +static __be32
> +nfsd4_encode_open_nfsace4(struct xdr_stream *xdr)
> +{
> + __be32 status;
> +
> + /* type */
> + status = nfsd4_encode_acetype4(xdr, NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE);
> + if (status != nfs_ok)
> + return nfserr_resource;
> + /* flag */
> + status = nfsd4_encode_aceflag4(xdr, 0);
> + if (status != nfs_ok)
> + return nfserr_resource;
> + /* access mask */
> + status = nfsd4_encode_acemask4(xdr, 0);
> + if (status != nfs_ok)
> + return nfserr_resource;
> + /* who - empty for now */
> + if (xdr_stream_encode_u32(xdr, 0) != XDR_UNIT)
> + return nfserr_resource;
> + return nfs_ok;
> +}
> +
> +static __be32
> +nfsd4_encode_open_read_delegation4(struct xdr_stream *xdr, struct nfsd4_open *open)
> +{
> + __be32 status;
> +
> + /* stateid */
> + status = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
> + if (status != nfs_ok)
> + return status;
> + /* recall */
> + status = nfsd4_encode_bool(xdr, open->op_recall);
> + if (status != nfs_ok)
> + return status;
> + /* permissions */
> + return nfsd4_encode_open_nfsace4(xdr);
> +}
>
> static __be32
> nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
> @@ -4106,22 +4149,8 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
> case NFS4_OPEN_DELEGATE_NONE:
> break;
> case NFS4_OPEN_DELEGATE_READ:
> - nfserr = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
> - if (nfserr)
> - return nfserr;
> - p = xdr_reserve_space(xdr, 20);
> - if (!p)
> - return nfserr_resource;
> - *p++ = cpu_to_be32(open->op_recall);
> -
> - /*
> - * TODO: ACE's in delegations
> - */
> - *p++ = cpu_to_be32(NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE);
> - *p++ = cpu_to_be32(0);
> - *p++ = cpu_to_be32(0);
> - *p++ = cpu_to_be32(0); /* XXX: is NULL principal ok? */
> - break;
> + /* read */
> + return nfsd4_encode_open_read_delegation4(xdr, open);
> case NFS4_OPEN_DELEGATE_WRITE:
> nfserr = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
> if (nfserr)
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index aba07d5378fc..c142a9b5ab98 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -389,7 +389,7 @@ struct nfsd4_open {
> u32 op_deleg_want; /* request */
> stateid_t op_stateid; /* response */
> __be32 op_xdr_error; /* see nfsd4_open_omfg() */
> - u32 op_recall; /* recall */
> + bool op_recall; /* response */
nit: this would probably pack better if you moved op_recall down beside
op_truncate.
> struct nfsd4_change_info op_cinfo; /* response */
> u32 op_rflags; /* response */
> bool op_truncate; /* used during processing */
>
>
--
Jeff Layton <[email protected]>
On Fri, Sep 29, 2023 at 12:21:28PM -0400, Jeff Layton wrote:
> On Fri, 2023-09-29 at 09:59 -0400, Chuck Lever wrote:
> > From: Chuck Lever <[email protected]>
> >
> > Refactor nfsd4_encode_open() so the open_read_delegation4 type is
> > encoded in a separate function. This makes it more straightforward
> > to later add support for returning an nfsace4 in OPEN responses that
> > offer a delegation.
> >
> > Signed-off-by: Chuck Lever <[email protected]>
> > ---
> > fs/nfsd/nfs4state.c | 6 +++--
> > fs/nfsd/nfs4xdr.c | 61 ++++++++++++++++++++++++++++++++++++++-------------
> > fs/nfsd/xdr4.h | 2 +-
> > 3 files changed, 49 insertions(+), 20 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 22e95b9ae82f..b1118050ff52 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -5688,11 +5688,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > struct path path;
> >
> > cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
> > - open->op_recall = 0;
> > + open->op_recall = false;
> > switch (open->op_claim_type) {
> > case NFS4_OPEN_CLAIM_PREVIOUS:
> > if (!cb_up)
> > - open->op_recall = 1;
> > + open->op_recall = true;
> > break;
> > case NFS4_OPEN_CLAIM_NULL:
> > parent = currentfh;
> > @@ -5746,7 +5746,7 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS &&
> > open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE) {
> > dprintk("NFSD: WARNING: refusing delegation reclaim\n");
> > - open->op_recall = 1;
> > + open->op_recall = true;
> > }
> >
> > /* 4.1 client asking for a delegation? */
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index ee8a7989f54f..f411fcc435f6 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -4074,6 +4074,49 @@ nfsd4_encode_link(struct nfsd4_compoundres *resp, __be32 nfserr,
> > return nfsd4_encode_change_info4(xdr, &link->li_cinfo);
> > }
> >
> > +/*
> > + * This implementation does not yet support returning an ACE in an
> > + * OPEN that offers a delegation.
> > + */
> > +static __be32
> > +nfsd4_encode_open_nfsace4(struct xdr_stream *xdr)
> > +{
> > + __be32 status;
> > +
> > + /* type */
> > + status = nfsd4_encode_acetype4(xdr, NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE);
> > + if (status != nfs_ok)
> > + return nfserr_resource;
> > + /* flag */
> > + status = nfsd4_encode_aceflag4(xdr, 0);
> > + if (status != nfs_ok)
> > + return nfserr_resource;
> > + /* access mask */
> > + status = nfsd4_encode_acemask4(xdr, 0);
> > + if (status != nfs_ok)
> > + return nfserr_resource;
> > + /* who - empty for now */
> > + if (xdr_stream_encode_u32(xdr, 0) != XDR_UNIT)
> > + return nfserr_resource;
> > + return nfs_ok;
> > +}
> > +
> > +static __be32
> > +nfsd4_encode_open_read_delegation4(struct xdr_stream *xdr, struct nfsd4_open *open)
> > +{
> > + __be32 status;
> > +
> > + /* stateid */
> > + status = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
> > + if (status != nfs_ok)
> > + return status;
> > + /* recall */
> > + status = nfsd4_encode_bool(xdr, open->op_recall);
> > + if (status != nfs_ok)
> > + return status;
> > + /* permissions */
> > + return nfsd4_encode_open_nfsace4(xdr);
> > +}
> >
> > static __be32
> > nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
> > @@ -4106,22 +4149,8 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
> > case NFS4_OPEN_DELEGATE_NONE:
> > break;
> > case NFS4_OPEN_DELEGATE_READ:
> > - nfserr = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
> > - if (nfserr)
> > - return nfserr;
> > - p = xdr_reserve_space(xdr, 20);
> > - if (!p)
> > - return nfserr_resource;
> > - *p++ = cpu_to_be32(open->op_recall);
> > -
> > - /*
> > - * TODO: ACE's in delegations
> > - */
> > - *p++ = cpu_to_be32(NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE);
> > - *p++ = cpu_to_be32(0);
> > - *p++ = cpu_to_be32(0);
> > - *p++ = cpu_to_be32(0); /* XXX: is NULL principal ok? */
> > - break;
> > + /* read */
> > + return nfsd4_encode_open_read_delegation4(xdr, open);
> > case NFS4_OPEN_DELEGATE_WRITE:
> > nfserr = nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
> > if (nfserr)
> > diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> > index aba07d5378fc..c142a9b5ab98 100644
> > --- a/fs/nfsd/xdr4.h
> > +++ b/fs/nfsd/xdr4.h
> > @@ -389,7 +389,7 @@ struct nfsd4_open {
> > u32 op_deleg_want; /* request */
> > stateid_t op_stateid; /* response */
> > __be32 op_xdr_error; /* see nfsd4_open_omfg() */
> > - u32 op_recall; /* recall */
> > + bool op_recall; /* response */
>
> nit: this would probably pack better if you moved op_recall down beside
> op_truncate.
Fixed. All 15 pushed to nfsd-next with your Reviewed-by. Thanks!
> > struct nfsd4_change_info op_cinfo; /* response */
> > u32 op_rflags; /* response */
> > bool op_truncate; /* used during processing */
> >
> >
>
> --
> Jeff Layton <[email protected]>
--
Chuck Lever