| Message ID | 1683841383-21372-4-git-send-email-dai.ngo@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFSD: add support for NFSv4 write delegation | expand |
Hey Dai- Jeff's a little better with the state-related code, so let me start with a review of the new CB_GETATTR implementation. > On May 11, 2023, at 2:43 PM, Dai Ngo <dai.ngo@oracle.com> wrote: > > Includes: > . CB_GETATTR proc for nfs4_cb_procedures[] > > . XDR encoding and decoding function for CB_GETATTR request/reply > > . add nfs4_cb_fattr to nfs4_delegation for sending CB_GETATTR > and store file attributes from client's reply. > > Signed-off-by: Dai Ngo <dai.ngo@oracle.com> > --- > fs/nfsd/nfs4callback.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++ > fs/nfsd/state.h | 17 +++++++ > fs/nfsd/xdr4cb.h | 19 ++++++++ > 3 files changed, 153 insertions(+) > > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > index 4039ffcf90ba..ca3d72ef5fbc 100644 > --- a/fs/nfsd/nfs4callback.c > +++ b/fs/nfsd/nfs4callback.c > @@ -87,6 +87,43 @@ static void encode_bitmap4(struct xdr_stream *xdr, const __u32 *bitmap, > WARN_ON_ONCE(xdr_stream_encode_uint32_array(xdr, bitmap, len) < 0); > } > > +static void decode_bitmap4(struct xdr_stream *xdr, __u32 *bitmap, > + size_t len) > +{ > + WARN_ON_ONCE(xdr_stream_decode_uint32_array(xdr, bitmap, len) < 0); > +} encode_bitmap4() hides the WARN_ON_ONCE. However, for decoding, we actually want to get the result of the decode, so let's get rid of decode_bitmap4() and simply call xdr_stream_decode_uint32_array() directly from nfs4_xdr_dec_cb_getattr() (and, of course, check it's return code properly, no WARN_ON). > + > +static int decode_attr_length(struct xdr_stream *xdr, uint32_t *attrlen) > +{ > + __be32 *p; > + > + p = xdr_inline_decode(xdr, 4); > + if (unlikely(!p)) > + return -EIO; > + *attrlen = be32_to_cpup(p); > + return 0; > +} > + > +static int decode_cb_getattr(struct xdr_stream *xdr, uint32_t *bitmap, > + struct nfs4_cb_fattr *fattr) > +{ > + __be32 *ptr; > + > + if (likely(bitmap[0] & FATTR4_WORD0_CHANGE)) { > + ptr = xdr_inline_decode(xdr, 8); > + if (unlikely(!ptr)) > + return -EIO; > + xdr_decode_hyper(ptr, &fattr->ncf_cb_cinfo); > + } > + if (likely(bitmap[0] & FATTR4_WORD0_SIZE)) { > + ptr = xdr_inline_decode(xdr, 8); > + if (unlikely(!ptr)) > + return -EIO; > + xdr_decode_hyper(ptr, &fattr->ncf_cb_fsize); > + } Let's use xdr_stream_decode_u64() for these. Also, I don't think the likely() is necessary -- this isn't performance-sensitive code. > + return 0; > +} > + > /* > * nfs_cb_opnum4 > * > @@ -358,6 +395,26 @@ encode_cb_recallany4args(struct xdr_stream *xdr, > } > > /* > + * CB_GETATTR4args > + * struct CB_GETATTR4args { > + * nfs_fh4 fh; > + * bitmap4 attr_request; > + * }; > + * > + * The size and change attributes are the only one > + * guaranteed to be serviced by the client. > + */ > +static void > +encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr, > + struct knfsd_fh *fh, struct nfs4_cb_fattr *fattr) Nit: Can this take just a "struct nfs4_cb_fattr *" parameter instead of the filehandle and fattr? > +{ > + encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR); > + encode_nfs_fh4(xdr, fh); > + encode_bitmap4(xdr, fattr->ncf_cb_bmap, ARRAY_SIZE(fattr->ncf_cb_bmap)); > + hdr->nops++; > +} > + > +/* > * CB_SEQUENCE4args > * > * struct CB_SEQUENCE4args { > @@ -493,6 +550,29 @@ static void nfs4_xdr_enc_cb_null(struct rpc_rqst *req, struct xdr_stream *xdr, > } > > /* > + * 20.1. Operation 3: CB_GETATTR - Get Attributes > + */ > +static void nfs4_xdr_enc_cb_getattr(struct rpc_rqst *req, > + struct xdr_stream *xdr, const void *data) > +{ > + const struct nfsd4_callback *cb = data; > + struct nfs4_cb_fattr *ncf = > + container_of(cb, struct nfs4_cb_fattr, ncf_getattr); > + struct nfs4_delegation *dp = > + container_of(ncf, struct nfs4_delegation, dl_cb_fattr); > + struct nfs4_cb_compound_hdr hdr = { > + .ident = cb->cb_clp->cl_cb_ident, > + .minorversion = cb->cb_clp->cl_minorversion, > + }; > + > + encode_cb_compound4args(xdr, &hdr); > + encode_cb_sequence4args(xdr, cb, &hdr); > + encode_cb_getattr4args(xdr, &hdr, > + &dp->dl_stid.sc_file->fi_fhandle, &dp->dl_cb_fattr); > + encode_cb_nops(&hdr); > +} > + > +/* > * 20.2. Operation 4: CB_RECALL - Recall a Delegation > */ > static void nfs4_xdr_enc_cb_recall(struct rpc_rqst *req, struct xdr_stream *xdr, > @@ -548,6 +628,42 @@ static int nfs4_xdr_dec_cb_null(struct rpc_rqst *req, struct xdr_stream *xdr, > } > > /* > + * 20.1. Operation 3: CB_GETATTR - Get Attributes > + */ > +static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp, > + struct xdr_stream *xdr, > + void *data) > +{ > + struct nfsd4_callback *cb = data; > + struct nfs4_cb_compound_hdr hdr; > + int status; > + u32 bitmap[3] = {0}; > + u32 attrlen; > + struct nfs4_cb_fattr *ncf = > + container_of(cb, struct nfs4_cb_fattr, ncf_getattr); > + > + status = decode_cb_compound4res(xdr, &hdr); > + if (unlikely(status)) > + return status; > + > + status = decode_cb_sequence4res(xdr, cb); > + if (unlikely(status || cb->cb_seq_status)) > + return status; > + > + status = decode_cb_op_status(xdr, OP_CB_GETATTR, &cb->cb_status); > + if (status) > + return status; > + decode_bitmap4(xdr, bitmap, 3); > + status = decode_attr_length(xdr, &attrlen); > + if (status) > + return status; Let's use xdr_stream_decode_u32 directly here, and check that attrlen is a reasonable value. Say, not larger than the full expected array length? > + ncf->ncf_cb_cinfo = 0; > + ncf->ncf_cb_fsize = 0; > + status = decode_cb_getattr(xdr, bitmap, ncf); > + return status; You're actually decoding a fattr4 here, not the whole CB_GETATTR result. Can we call the function decode_cb_fattr4() ? Nit: Let's fold the initialization of cb_cinfo and cb_fsize into decode_cb_fattr4(). > +} > + > +/* > * 20.2. Operation 4: CB_RECALL - Recall a Delegation > */ > static int nfs4_xdr_dec_cb_recall(struct rpc_rqst *rqstp, > @@ -855,6 +971,7 @@ static const struct rpc_procinfo nfs4_cb_procedures[] = { > PROC(CB_NOTIFY_LOCK, COMPOUND, cb_notify_lock, cb_notify_lock), > PROC(CB_OFFLOAD, COMPOUND, cb_offload, cb_offload), > PROC(CB_RECALL_ANY, COMPOUND, cb_recall_any, cb_recall_any), > + PROC(CB_GETATTR, COMPOUND, cb_getattr, cb_getattr), > }; > > static unsigned int nfs4_cb_counts[ARRAY_SIZE(nfs4_cb_procedures)]; > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > index d49d3060ed4f..92349375053a 100644 > --- a/fs/nfsd/state.h > +++ b/fs/nfsd/state.h > @@ -117,6 +117,19 @@ struct nfs4_cpntf_state { > time64_t cpntf_time; /* last time stateid used */ > }; > > +struct nfs4_cb_fattr { > + struct nfsd4_callback ncf_getattr; > + u32 ncf_cb_status; > + u32 ncf_cb_bmap[1]; > + > + /* from CB_GETATTR reply */ > + u64 ncf_cb_cinfo; In fs/nfsd/nfs4xdr.c, we use "cinfo" to mean change info: that's a boolean, and a pair of 64-bit change attribute values. IIUC, I don't think that's what this is; it's just a single change attribute value. To avoid overloading the name "cinfo" can you call this field something like ncf_cb_change ? > + u64 ncf_cb_fsize; > +}; > + > +/* bits for ncf_cb_flags */ > +#define CB_GETATTR_BUSY 0 > + > /* > * Represents a delegation stateid. The nfs4_client holds references to these > * and they are put when it is being destroyed or when the delegation is > @@ -150,6 +163,9 @@ struct nfs4_delegation { > int dl_retries; > struct nfsd4_callback dl_recall; > bool dl_recalled; > + > + /* for CB_GETATTR */ > + struct nfs4_cb_fattr dl_cb_fattr; > }; > > #define cb_to_delegation(cb) \ > @@ -642,6 +658,7 @@ enum nfsd4_cb_op { > NFSPROC4_CLNT_CB_SEQUENCE, > NFSPROC4_CLNT_CB_NOTIFY_LOCK, > NFSPROC4_CLNT_CB_RECALL_ANY, > + NFSPROC4_CLNT_CB_GETATTR, > }; > > /* Returns true iff a is later than b: */ > diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h > index 0d39af1b00a0..3a31bb0a3ded 100644 > --- a/fs/nfsd/xdr4cb.h > +++ b/fs/nfsd/xdr4cb.h > @@ -54,3 +54,22 @@ > #define NFS4_dec_cb_recall_any_sz (cb_compound_dec_hdr_sz + \ > cb_sequence_dec_sz + \ > op_dec_sz) > + > +/* > + * 1: CB_GETATTR opcode (32-bit) > + * N: file_handle > + * 1: number of entry in attribute array (32-bit) > + * 1: entry 0 in attribute array (32-bit) > + */ > +#define NFS4_enc_cb_getattr_sz (cb_compound_enc_hdr_sz + \ > + cb_sequence_enc_sz + \ > + 1 + enc_nfs4_fh_sz + 1 + 1) > +/* > + * 1: attr mask (32-bit bmap) > + * 2: length of attribute array (64-bit) Isn't the array length field 32 bits? > + * 2: change attr (64-bit) > + * 2: size (64-bit) > + */ > +#define NFS4_dec_cb_getattr_sz (cb_compound_dec_hdr_sz + \ > + cb_sequence_dec_sz + 7 + \ Generally we list the length of each item separately to document the individual values, so: 1 + 1 + 2 + 2 is preferred over a single integer. > + op_dec_sz) > -- > 2.9.5 > -- Chuck Lever
Thank you Chuck for your review. I'll make the change in v2. -Dai On 5/12/23 8:30 AM, Chuck Lever III wrote: > Hey Dai- > > Jeff's a little better with the state-related code, so let > me start with a review of the new CB_GETATTR implementation. > > >> On May 11, 2023, at 2:43 PM, Dai Ngo <dai.ngo@oracle.com> wrote: >> >> Includes: >> . CB_GETATTR proc for nfs4_cb_procedures[] >> >> . XDR encoding and decoding function for CB_GETATTR request/reply >> >> . add nfs4_cb_fattr to nfs4_delegation for sending CB_GETATTR >> and store file attributes from client's reply. >> >> Signed-off-by: Dai Ngo <dai.ngo@oracle.com> >> --- >> fs/nfsd/nfs4callback.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++ >> fs/nfsd/state.h | 17 +++++++ >> fs/nfsd/xdr4cb.h | 19 ++++++++ >> 3 files changed, 153 insertions(+) >> >> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c >> index 4039ffcf90ba..ca3d72ef5fbc 100644 >> --- a/fs/nfsd/nfs4callback.c >> +++ b/fs/nfsd/nfs4callback.c >> @@ -87,6 +87,43 @@ static void encode_bitmap4(struct xdr_stream *xdr, const __u32 *bitmap, >> WARN_ON_ONCE(xdr_stream_encode_uint32_array(xdr, bitmap, len) < 0); >> } >> >> +static void decode_bitmap4(struct xdr_stream *xdr, __u32 *bitmap, >> + size_t len) >> +{ >> + WARN_ON_ONCE(xdr_stream_decode_uint32_array(xdr, bitmap, len) < 0); >> +} > encode_bitmap4() hides the WARN_ON_ONCE. > > However, for decoding, we actually want to get the result > of the decode, so let's get rid of decode_bitmap4() and > simply call xdr_stream_decode_uint32_array() directly from > nfs4_xdr_dec_cb_getattr() (and, of course, check it's return > code properly, no WARN_ON). > > >> + >> +static int decode_attr_length(struct xdr_stream *xdr, uint32_t *attrlen) >> +{ >> + __be32 *p; >> + >> + p = xdr_inline_decode(xdr, 4); >> + if (unlikely(!p)) >> + return -EIO; >> + *attrlen = be32_to_cpup(p); >> + return 0; >> +} >> + >> +static int decode_cb_getattr(struct xdr_stream *xdr, uint32_t *bitmap, >> + struct nfs4_cb_fattr *fattr) >> +{ >> + __be32 *ptr; >> + >> + if (likely(bitmap[0] & FATTR4_WORD0_CHANGE)) { >> + ptr = xdr_inline_decode(xdr, 8); >> + if (unlikely(!ptr)) >> + return -EIO; >> + xdr_decode_hyper(ptr, &fattr->ncf_cb_cinfo); >> + } >> + if (likely(bitmap[0] & FATTR4_WORD0_SIZE)) { >> + ptr = xdr_inline_decode(xdr, 8); >> + if (unlikely(!ptr)) >> + return -EIO; >> + xdr_decode_hyper(ptr, &fattr->ncf_cb_fsize); >> + } > Let's use xdr_stream_decode_u64() for these. > > Also, I don't think the likely() is necessary -- this > isn't performance-sensitive code. > > >> + return 0; >> +} >> + >> /* >> * nfs_cb_opnum4 >> * >> @@ -358,6 +395,26 @@ encode_cb_recallany4args(struct xdr_stream *xdr, >> } >> >> /* >> + * CB_GETATTR4args >> + * struct CB_GETATTR4args { >> + * nfs_fh4 fh; >> + * bitmap4 attr_request; >> + * }; >> + * >> + * The size and change attributes are the only one >> + * guaranteed to be serviced by the client. >> + */ >> +static void >> +encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr, >> + struct knfsd_fh *fh, struct nfs4_cb_fattr *fattr) > Nit: Can this take just a "struct nfs4_cb_fattr *" parameter > instead of the filehandle and fattr? > > >> +{ >> + encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR); >> + encode_nfs_fh4(xdr, fh); >> + encode_bitmap4(xdr, fattr->ncf_cb_bmap, ARRAY_SIZE(fattr->ncf_cb_bmap)); >> + hdr->nops++; >> +} >> + >> +/* >> * CB_SEQUENCE4args >> * >> * struct CB_SEQUENCE4args { >> @@ -493,6 +550,29 @@ static void nfs4_xdr_enc_cb_null(struct rpc_rqst *req, struct xdr_stream *xdr, >> } >> >> /* >> + * 20.1. Operation 3: CB_GETATTR - Get Attributes >> + */ >> +static void nfs4_xdr_enc_cb_getattr(struct rpc_rqst *req, >> + struct xdr_stream *xdr, const void *data) >> +{ >> + const struct nfsd4_callback *cb = data; >> + struct nfs4_cb_fattr *ncf = >> + container_of(cb, struct nfs4_cb_fattr, ncf_getattr); >> + struct nfs4_delegation *dp = >> + container_of(ncf, struct nfs4_delegation, dl_cb_fattr); >> + struct nfs4_cb_compound_hdr hdr = { >> + .ident = cb->cb_clp->cl_cb_ident, >> + .minorversion = cb->cb_clp->cl_minorversion, >> + }; >> + >> + encode_cb_compound4args(xdr, &hdr); >> + encode_cb_sequence4args(xdr, cb, &hdr); >> + encode_cb_getattr4args(xdr, &hdr, >> + &dp->dl_stid.sc_file->fi_fhandle, &dp->dl_cb_fattr); >> + encode_cb_nops(&hdr); >> +} >> + >> +/* >> * 20.2. Operation 4: CB_RECALL - Recall a Delegation >> */ >> static void nfs4_xdr_enc_cb_recall(struct rpc_rqst *req, struct xdr_stream *xdr, >> @@ -548,6 +628,42 @@ static int nfs4_xdr_dec_cb_null(struct rpc_rqst *req, struct xdr_stream *xdr, >> } >> >> /* >> + * 20.1. Operation 3: CB_GETATTR - Get Attributes >> + */ >> +static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp, >> + struct xdr_stream *xdr, >> + void *data) >> +{ >> + struct nfsd4_callback *cb = data; >> + struct nfs4_cb_compound_hdr hdr; >> + int status; >> + u32 bitmap[3] = {0}; >> + u32 attrlen; >> + struct nfs4_cb_fattr *ncf = >> + container_of(cb, struct nfs4_cb_fattr, ncf_getattr); >> + >> + status = decode_cb_compound4res(xdr, &hdr); >> + if (unlikely(status)) >> + return status; >> + >> + status = decode_cb_sequence4res(xdr, cb); >> + if (unlikely(status || cb->cb_seq_status)) >> + return status; >> + >> + status = decode_cb_op_status(xdr, OP_CB_GETATTR, &cb->cb_status); >> + if (status) >> + return status; >> + decode_bitmap4(xdr, bitmap, 3); >> + status = decode_attr_length(xdr, &attrlen); >> + if (status) >> + return status; > Let's use xdr_stream_decode_u32 directly here, and > check that attrlen is a reasonable value. Say, not > larger than the full expected array length? > > >> + ncf->ncf_cb_cinfo = 0; >> + ncf->ncf_cb_fsize = 0; >> + status = decode_cb_getattr(xdr, bitmap, ncf); >> + return status; > You're actually decoding a fattr4 here, not the whole > CB_GETATTR result. Can we call the function > decode_cb_fattr4() ? > > Nit: Let's fold the initialization of cb_cinfo and > cb_fsize into decode_cb_fattr4(). > > > >> +} >> + >> +/* >> * 20.2. Operation 4: CB_RECALL - Recall a Delegation >> */ >> static int nfs4_xdr_dec_cb_recall(struct rpc_rqst *rqstp, >> @@ -855,6 +971,7 @@ static const struct rpc_procinfo nfs4_cb_procedures[] = { >> PROC(CB_NOTIFY_LOCK, COMPOUND, cb_notify_lock, cb_notify_lock), >> PROC(CB_OFFLOAD, COMPOUND, cb_offload, cb_offload), >> PROC(CB_RECALL_ANY, COMPOUND, cb_recall_any, cb_recall_any), >> + PROC(CB_GETATTR, COMPOUND, cb_getattr, cb_getattr), >> }; >> >> static unsigned int nfs4_cb_counts[ARRAY_SIZE(nfs4_cb_procedures)]; >> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h >> index d49d3060ed4f..92349375053a 100644 >> --- a/fs/nfsd/state.h >> +++ b/fs/nfsd/state.h >> @@ -117,6 +117,19 @@ struct nfs4_cpntf_state { >> time64_t cpntf_time; /* last time stateid used */ >> }; >> >> +struct nfs4_cb_fattr { >> + struct nfsd4_callback ncf_getattr; >> + u32 ncf_cb_status; >> + u32 ncf_cb_bmap[1]; >> + >> + /* from CB_GETATTR reply */ >> + u64 ncf_cb_cinfo; > In fs/nfsd/nfs4xdr.c, we use "cinfo" to mean change info: > that's a boolean, and a pair of 64-bit change attribute values. > IIUC, I don't think that's what this is; it's just a single > change attribute value. > > To avoid overloading the name "cinfo" can you call this field > something like ncf_cb_change ? > > >> + u64 ncf_cb_fsize; >> +}; >> + >> +/* bits for ncf_cb_flags */ >> +#define CB_GETATTR_BUSY 0 >> + >> /* >> * Represents a delegation stateid. The nfs4_client holds references to these >> * and they are put when it is being destroyed or when the delegation is >> @@ -150,6 +163,9 @@ struct nfs4_delegation { >> int dl_retries; >> struct nfsd4_callback dl_recall; >> bool dl_recalled; >> + >> + /* for CB_GETATTR */ >> + struct nfs4_cb_fattr dl_cb_fattr; >> }; >> >> #define cb_to_delegation(cb) \ >> @@ -642,6 +658,7 @@ enum nfsd4_cb_op { >> NFSPROC4_CLNT_CB_SEQUENCE, >> NFSPROC4_CLNT_CB_NOTIFY_LOCK, >> NFSPROC4_CLNT_CB_RECALL_ANY, >> + NFSPROC4_CLNT_CB_GETATTR, >> }; >> >> /* Returns true iff a is later than b: */ >> diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h >> index 0d39af1b00a0..3a31bb0a3ded 100644 >> --- a/fs/nfsd/xdr4cb.h >> +++ b/fs/nfsd/xdr4cb.h >> @@ -54,3 +54,22 @@ >> #define NFS4_dec_cb_recall_any_sz (cb_compound_dec_hdr_sz + \ >> cb_sequence_dec_sz + \ >> op_dec_sz) >> + >> +/* >> + * 1: CB_GETATTR opcode (32-bit) >> + * N: file_handle >> + * 1: number of entry in attribute array (32-bit) >> + * 1: entry 0 in attribute array (32-bit) >> + */ >> +#define NFS4_enc_cb_getattr_sz (cb_compound_enc_hdr_sz + \ >> + cb_sequence_enc_sz + \ >> + 1 + enc_nfs4_fh_sz + 1 + 1) >> +/* >> + * 1: attr mask (32-bit bmap) >> + * 2: length of attribute array (64-bit) > Isn't the array length field 32 bits? > > >> + * 2: change attr (64-bit) >> + * 2: size (64-bit) >> + */ >> +#define NFS4_dec_cb_getattr_sz (cb_compound_dec_hdr_sz + \ >> + cb_sequence_dec_sz + 7 + \ > Generally we list the length of each item separately > to document the individual values, so: > > 1 + 1 + 2 + 2 > > is preferred over a single integer. > > >> + op_dec_sz) >> -- >> 2.9.5 >> > -- > Chuck Lever > >
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c index 4039ffcf90ba..ca3d72ef5fbc 100644 --- a/fs/nfsd/nfs4callback.c +++ b/fs/nfsd/nfs4callback.c @@ -87,6 +87,43 @@ static void encode_bitmap4(struct xdr_stream *xdr, const __u32 *bitmap, WARN_ON_ONCE(xdr_stream_encode_uint32_array(xdr, bitmap, len) < 0); } +static void decode_bitmap4(struct xdr_stream *xdr, __u32 *bitmap, + size_t len) +{ + WARN_ON_ONCE(xdr_stream_decode_uint32_array(xdr, bitmap, len) < 0); +} + +static int decode_attr_length(struct xdr_stream *xdr, uint32_t *attrlen) +{ + __be32 *p; + + p = xdr_inline_decode(xdr, 4); + if (unlikely(!p)) + return -EIO; + *attrlen = be32_to_cpup(p); + return 0; +} + +static int decode_cb_getattr(struct xdr_stream *xdr, uint32_t *bitmap, + struct nfs4_cb_fattr *fattr) +{ + __be32 *ptr; + + if (likely(bitmap[0] & FATTR4_WORD0_CHANGE)) { + ptr = xdr_inline_decode(xdr, 8); + if (unlikely(!ptr)) + return -EIO; + xdr_decode_hyper(ptr, &fattr->ncf_cb_cinfo); + } + if (likely(bitmap[0] & FATTR4_WORD0_SIZE)) { + ptr = xdr_inline_decode(xdr, 8); + if (unlikely(!ptr)) + return -EIO; + xdr_decode_hyper(ptr, &fattr->ncf_cb_fsize); + } + return 0; +} + /* * nfs_cb_opnum4 * @@ -358,6 +395,26 @@ encode_cb_recallany4args(struct xdr_stream *xdr, } /* + * CB_GETATTR4args + * struct CB_GETATTR4args { + * nfs_fh4 fh; + * bitmap4 attr_request; + * }; + * + * The size and change attributes are the only one + * guaranteed to be serviced by the client. + */ +static void +encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr, + struct knfsd_fh *fh, struct nfs4_cb_fattr *fattr) +{ + encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR); + encode_nfs_fh4(xdr, fh); + encode_bitmap4(xdr, fattr->ncf_cb_bmap, ARRAY_SIZE(fattr->ncf_cb_bmap)); + hdr->nops++; +} + +/* * CB_SEQUENCE4args * * struct CB_SEQUENCE4args { @@ -493,6 +550,29 @@ static void nfs4_xdr_enc_cb_null(struct rpc_rqst *req, struct xdr_stream *xdr, } /* + * 20.1. Operation 3: CB_GETATTR - Get Attributes + */ +static void nfs4_xdr_enc_cb_getattr(struct rpc_rqst *req, + struct xdr_stream *xdr, const void *data) +{ + const struct nfsd4_callback *cb = data; + struct nfs4_cb_fattr *ncf = + container_of(cb, struct nfs4_cb_fattr, ncf_getattr); + struct nfs4_delegation *dp = + container_of(ncf, struct nfs4_delegation, dl_cb_fattr); + struct nfs4_cb_compound_hdr hdr = { + .ident = cb->cb_clp->cl_cb_ident, + .minorversion = cb->cb_clp->cl_minorversion, + }; + + encode_cb_compound4args(xdr, &hdr); + encode_cb_sequence4args(xdr, cb, &hdr); + encode_cb_getattr4args(xdr, &hdr, + &dp->dl_stid.sc_file->fi_fhandle, &dp->dl_cb_fattr); + encode_cb_nops(&hdr); +} + +/* * 20.2. Operation 4: CB_RECALL - Recall a Delegation */ static void nfs4_xdr_enc_cb_recall(struct rpc_rqst *req, struct xdr_stream *xdr, @@ -548,6 +628,42 @@ static int nfs4_xdr_dec_cb_null(struct rpc_rqst *req, struct xdr_stream *xdr, } /* + * 20.1. Operation 3: CB_GETATTR - Get Attributes + */ +static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp, + struct xdr_stream *xdr, + void *data) +{ + struct nfsd4_callback *cb = data; + struct nfs4_cb_compound_hdr hdr; + int status; + u32 bitmap[3] = {0}; + u32 attrlen; + struct nfs4_cb_fattr *ncf = + container_of(cb, struct nfs4_cb_fattr, ncf_getattr); + + status = decode_cb_compound4res(xdr, &hdr); + if (unlikely(status)) + return status; + + status = decode_cb_sequence4res(xdr, cb); + if (unlikely(status || cb->cb_seq_status)) + return status; + + status = decode_cb_op_status(xdr, OP_CB_GETATTR, &cb->cb_status); + if (status) + return status; + decode_bitmap4(xdr, bitmap, 3); + status = decode_attr_length(xdr, &attrlen); + if (status) + return status; + ncf->ncf_cb_cinfo = 0; + ncf->ncf_cb_fsize = 0; + status = decode_cb_getattr(xdr, bitmap, ncf); + return status; +} + +/* * 20.2. Operation 4: CB_RECALL - Recall a Delegation */ static int nfs4_xdr_dec_cb_recall(struct rpc_rqst *rqstp, @@ -855,6 +971,7 @@ static const struct rpc_procinfo nfs4_cb_procedures[] = { PROC(CB_NOTIFY_LOCK, COMPOUND, cb_notify_lock, cb_notify_lock), PROC(CB_OFFLOAD, COMPOUND, cb_offload, cb_offload), PROC(CB_RECALL_ANY, COMPOUND, cb_recall_any, cb_recall_any), + PROC(CB_GETATTR, COMPOUND, cb_getattr, cb_getattr), }; static unsigned int nfs4_cb_counts[ARRAY_SIZE(nfs4_cb_procedures)]; diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index d49d3060ed4f..92349375053a 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -117,6 +117,19 @@ struct nfs4_cpntf_state { time64_t cpntf_time; /* last time stateid used */ }; +struct nfs4_cb_fattr { + struct nfsd4_callback ncf_getattr; + u32 ncf_cb_status; + u32 ncf_cb_bmap[1]; + + /* from CB_GETATTR reply */ + u64 ncf_cb_cinfo; + u64 ncf_cb_fsize; +}; + +/* bits for ncf_cb_flags */ +#define CB_GETATTR_BUSY 0 + /* * Represents a delegation stateid. The nfs4_client holds references to these * and they are put when it is being destroyed or when the delegation is @@ -150,6 +163,9 @@ struct nfs4_delegation { int dl_retries; struct nfsd4_callback dl_recall; bool dl_recalled; + + /* for CB_GETATTR */ + struct nfs4_cb_fattr dl_cb_fattr; }; #define cb_to_delegation(cb) \ @@ -642,6 +658,7 @@ enum nfsd4_cb_op { NFSPROC4_CLNT_CB_SEQUENCE, NFSPROC4_CLNT_CB_NOTIFY_LOCK, NFSPROC4_CLNT_CB_RECALL_ANY, + NFSPROC4_CLNT_CB_GETATTR, }; /* Returns true iff a is later than b: */ diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h index 0d39af1b00a0..3a31bb0a3ded 100644 --- a/fs/nfsd/xdr4cb.h +++ b/fs/nfsd/xdr4cb.h @@ -54,3 +54,22 @@ #define NFS4_dec_cb_recall_any_sz (cb_compound_dec_hdr_sz + \ cb_sequence_dec_sz + \ op_dec_sz) + +/* + * 1: CB_GETATTR opcode (32-bit) + * N: file_handle + * 1: number of entry in attribute array (32-bit) + * 1: entry 0 in attribute array (32-bit) + */ +#define NFS4_enc_cb_getattr_sz (cb_compound_enc_hdr_sz + \ + cb_sequence_enc_sz + \ + 1 + enc_nfs4_fh_sz + 1 + 1) +/* + * 1: attr mask (32-bit bmap) + * 2: length of attribute array (64-bit) + * 2: change attr (64-bit) + * 2: size (64-bit) + */ +#define NFS4_dec_cb_getattr_sz (cb_compound_dec_hdr_sz + \ + cb_sequence_dec_sz + 7 + \ + op_dec_sz)
Includes: . CB_GETATTR proc for nfs4_cb_procedures[] . XDR encoding and decoding function for CB_GETATTR request/reply . add nfs4_cb_fattr to nfs4_delegation for sending CB_GETATTR and store file attributes from client's reply. Signed-off-by: Dai Ngo <dai.ngo@oracle.com> --- fs/nfsd/nfs4callback.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++ fs/nfsd/state.h | 17 +++++++ fs/nfsd/xdr4cb.h | 19 ++++++++ 3 files changed, 153 insertions(+)