| Message ID | 20240315-dir-deleg-v1-12-a1d6209a3654@kernel.org (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | vfs, nfsd, nfs: implement directory delegations | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Fri, Mar 15, 2024 at 12:53:03PM -0400, Jeff Layton wrote: > This adds basic infrastructure for handing GET_DIR_DELEGATION calls from > clients, including the decoders and encoders. For now, the server side > always just returns that the delegation is GDDR_UNAVAIL (and that we > won't call back). > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/nfsd/nfs4proc.c | 30 ++++++++++++++++++++++ > fs/nfsd/nfs4xdr.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > fs/nfsd/xdr4.h | 8 ++++++ > include/linux/nfs4.h | 5 ++++ > 4 files changed, 113 insertions(+), 2 deletions(-) Just a handful of style preferences below. > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > index 2927b1263f08..7973fe17bf3c 100644 > --- a/fs/nfsd/nfs4proc.c > +++ b/fs/nfsd/nfs4proc.c > @@ -2173,6 +2173,18 @@ nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type) > return nfsd4_layout_ops[layout_type]; > } > > +static __be32 > +nfsd4_get_dir_delegation(struct svc_rqst *rqstp, > + struct nfsd4_compound_state *cstate, > + union nfsd4_op_u *u) > +{ > + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; > + > + /* FIXME: actually return a delegation */ > + gdd->nf_status = GDD4_UNAVAIL; > + return nfs_ok; > +} > + > static __be32 > nfsd4_getdeviceinfo(struct svc_rqst *rqstp, > struct nfsd4_compound_state *cstate, union nfsd4_op_u *u) > @@ -3082,6 +3094,18 @@ static u32 nfsd4_copy_notify_rsize(const struct svc_rqst *rqstp, > * sizeof(__be32); > } > > +static u32 nfsd4_get_dir_delegation_rsize(const struct svc_rqst *rqstp, > + const struct nfsd4_op *op) > +{ > + return (op_encode_hdr_size + > + 1 /* gddr_status */ + > + op_encode_verifier_maxsz + > + op_encode_stateid_maxsz + > + 2 /* gddr_notification */ + > + 2 /* gddr_child_attributes */ + > + 2 /* gddr_dir_attributes */); > +} > + > #ifdef CONFIG_NFSD_PNFS > static u32 nfsd4_getdeviceinfo_rsize(const struct svc_rqst *rqstp, > const struct nfsd4_op *op) > @@ -3470,6 +3494,12 @@ static const struct nfsd4_operation nfsd4_ops[] = { > .op_get_currentstateid = nfsd4_get_freestateid, > .op_rsize_bop = nfsd4_only_status_rsize, > }, > + [OP_GET_DIR_DELEGATION] = { > + .op_func = nfsd4_get_dir_delegation, > + .op_flags = OP_MODIFIES_SOMETHING, > + .op_name = "OP_GET_DIR_DELEGATION", > + .op_rsize_bop = nfsd4_get_dir_delegation_rsize, > + }, > #ifdef CONFIG_NFSD_PNFS > [OP_GETDEVICEINFO] = { > .op_func = nfsd4_getdeviceinfo, > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > index fac938f563ad..3718bef74e9f 100644 > --- a/fs/nfsd/nfs4xdr.c > +++ b/fs/nfsd/nfs4xdr.c > @@ -1732,6 +1732,40 @@ nfsd4_decode_free_stateid(struct nfsd4_compoundargs *argp, > return nfsd4_decode_stateid4(argp, &free_stateid->fr_stateid); > } > > +static __be32 > +nfsd4_decode_get_dir_delegation(struct nfsd4_compoundargs *argp, > + union nfsd4_op_u *u) > +{ > + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; > + struct timespec64 ts; > + u32 signal_deleg_avail; > + u32 attrs[1]; I know this isn't how we've done XDR in the past, but I'd rather see these dummy args as fields in struct nfsd4_get_dir_delegation, and also move the comments about whether each argument is supported to the putative nfsd4_proc_get_dir_delegation(). The actual implementation of GET_DIR_DELEGATION is in nfs4proc.c, after all, not here. This is simply a translation function. > + __be32 status; > + > + memset(gdd, 0, sizeof(*gdd)); > + > + /* No signal_avail support for now (and maybe never) */ > + if (xdr_stream_decode_bool(argp->xdr, &signal_deleg_avail) < 0) > + return nfserr_bad_xdr; > + status = nfsd4_decode_bitmap4(argp, gdd->notification_types, > + ARRAY_SIZE(gdd->notification_types)); > + if (status) > + return status; > + > + /* For now, we don't support child or dir attr change notification */ > + status = nfsd4_decode_nfstime4(argp, &ts); > + if (status) > + return status; > + /* No dir attr notification support yet either */ > + status = nfsd4_decode_nfstime4(argp, &ts); > + if (status) > + return status; > + status = nfsd4_decode_bitmap4(argp, attrs, ARRAY_SIZE(attrs)); > + if (status) > + return status; > + return nfsd4_decode_bitmap4(argp, attrs, ARRAY_SIZE(attrs)); > +} > + > #ifdef CONFIG_NFSD_PNFS > static __be32 > nfsd4_decode_getdeviceinfo(struct nfsd4_compoundargs *argp, > @@ -2370,7 +2404,7 @@ static const nfsd4_dec nfsd4_dec_ops[] = { > [OP_CREATE_SESSION] = nfsd4_decode_create_session, > [OP_DESTROY_SESSION] = nfsd4_decode_destroy_session, > [OP_FREE_STATEID] = nfsd4_decode_free_stateid, > - [OP_GET_DIR_DELEGATION] = nfsd4_decode_notsupp, > + [OP_GET_DIR_DELEGATION] = nfsd4_decode_get_dir_delegation, > #ifdef CONFIG_NFSD_PNFS > [OP_GETDEVICEINFO] = nfsd4_decode_getdeviceinfo, > [OP_GETDEVICELIST] = nfsd4_decode_notsupp, > @@ -5002,6 +5036,40 @@ nfsd4_encode_device_addr4(struct xdr_stream *xdr, > return nfserr_toosmall; > } > > +static __be32 > +nfsd4_encode_get_dir_delegation(struct nfsd4_compoundres *resp, __be32 nfserr, > + union nfsd4_op_u *u) > +{ > + struct xdr_stream *xdr = resp->xdr; > + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; > + > + /* Encode the GDDR_* status code */ In other encoders, I've used simply the name of the field as it is in the RFC as a documenting comment. That's more clear, and is easily grep-able. So: /* gddrnf_status */ > + if (xdr_stream_encode_u32(xdr, gdd->nf_status) != XDR_UNIT) > + return nfserr_resource; > + > + /* if it's GDD4_UNAVAIL then we're (almost) done */ > + if (gdd->nf_status == GDD4_UNAVAIL) { I prefer using a switch for XDR unions. That makes our implementation look more like the XDR definition; easier for humans to audit and modify. > + /* We never call back */ > + return nfsd4_encode_bool(xdr, false); Again, let's move this boolean to struct nfsd4_get_dir_delegation to enable nfsd4_proc_get_dir_delegation to decide in the future. > + } > + > + /* GDD4_OK case */ If a switch is used, then this comment becomes a real piece of self-verifying code: case GDD4_OK: > + nfserr = nfsd4_encode_verifier4(xdr, &gdd->cookieverf); > + if (nfserr) > + return nfserr; > + nfserr = nfsd4_encode_stateid4(xdr, &gdd->stateid); > + if (nfserr) > + return nfserr; > + /* No notifications (yet) */ > + nfserr = nfsd4_encode_bitmap4(xdr, 0, 0, 0); > + if (nfserr) > + return nfserr; > + nfserr = nfsd4_encode_bitmap4(xdr, 0, 0, 0); > + if (nfserr) > + return nfserr; > + return nfsd4_encode_bitmap4(xdr, 0, 0, 0); All these as well can go in struct nfsd4_get_dir_delegation. > +} > + > static __be32 > nfsd4_encode_getdeviceinfo(struct nfsd4_compoundres *resp, __be32 nfserr, > union nfsd4_op_u *u) > @@ -5580,7 +5648,7 @@ static const nfsd4_enc nfsd4_enc_ops[] = { > [OP_CREATE_SESSION] = nfsd4_encode_create_session, > [OP_DESTROY_SESSION] = nfsd4_encode_noop, > [OP_FREE_STATEID] = nfsd4_encode_noop, > - [OP_GET_DIR_DELEGATION] = nfsd4_encode_noop, > + [OP_GET_DIR_DELEGATION] = nfsd4_encode_get_dir_delegation, > #ifdef CONFIG_NFSD_PNFS > [OP_GETDEVICEINFO] = nfsd4_encode_getdeviceinfo, > [OP_GETDEVICELIST] = nfsd4_encode_noop, > diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h > index 415516c1b27e..27de75f32dea 100644 > --- a/fs/nfsd/xdr4.h > +++ b/fs/nfsd/xdr4.h > @@ -518,6 +518,13 @@ struct nfsd4_free_stateid { > stateid_t fr_stateid; /* request */ > }; > > +struct nfsd4_get_dir_delegation { > + u32 notification_types[1]; /* request */ > + u32 nf_status; /* response */ > + nfs4_verifier cookieverf; /* response */ > + stateid_t stateid; /* response */ > +}; > + > /* also used for NVERIFY */ > struct nfsd4_verify { > u32 ve_bmval[3]; /* request */ > @@ -797,6 +804,7 @@ struct nfsd4_op { > struct nfsd4_reclaim_complete reclaim_complete; > struct nfsd4_test_stateid test_stateid; > struct nfsd4_free_stateid free_stateid; > + struct nfsd4_get_dir_delegation get_dir_delegation; > struct nfsd4_getdeviceinfo getdeviceinfo; > struct nfsd4_layoutget layoutget; > struct nfsd4_layoutcommit layoutcommit; > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h > index ef8d2d618d5b..11ad088b411d 100644 > --- a/include/linux/nfs4.h > +++ b/include/linux/nfs4.h > @@ -701,6 +701,11 @@ enum state_protect_how4 { > SP4_SSV = 2 > }; > > +enum get_dir_delegation_non_fatal_res { > + GDD4_OK = 0, > + GDD4_UNAVAIL = 1 > +}; > + > enum pnfs_layouttype { > LAYOUT_NFSV4_1_FILES = 1, > LAYOUT_OSD2_OBJECTS = 2, > > -- > 2.44.0 >
On Sun, 2024-03-17 at 11:34 -0400, Chuck Lever wrote: > On Fri, Mar 15, 2024 at 12:53:03PM -0400, Jeff Layton wrote: > > This adds basic infrastructure for handing GET_DIR_DELEGATION calls from > > clients, including the decoders and encoders. For now, the server side > > always just returns that the delegation is GDDR_UNAVAIL (and that we > > won't call back). > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > fs/nfsd/nfs4proc.c | 30 ++++++++++++++++++++++ > > fs/nfsd/nfs4xdr.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > > fs/nfsd/xdr4.h | 8 ++++++ > > include/linux/nfs4.h | 5 ++++ > > 4 files changed, 113 insertions(+), 2 deletions(-) > > Just a handful of style preferences below. > Those comments all make sense. I'll respin along those lines. Also, I may go ahead and send this patch separately from the rest of the series. I think it would be best to have trivial support for GET_DIR_DELEGATION in the kernel server as soon as possible. Eventually, clients may start sending these calls, and it's better if we can just return a non-fatal error instead of sending back NFSERR_NOTSUPP when they do. > > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > > index 2927b1263f08..7973fe17bf3c 100644 > > --- a/fs/nfsd/nfs4proc.c > > +++ b/fs/nfsd/nfs4proc.c > > @@ -2173,6 +2173,18 @@ nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type) > > return nfsd4_layout_ops[layout_type]; > > } > > > > +static __be32 > > +nfsd4_get_dir_delegation(struct svc_rqst *rqstp, > > + struct nfsd4_compound_state *cstate, > > + union nfsd4_op_u *u) > > +{ > > + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; > > + > > + /* FIXME: actually return a delegation */ > > + gdd->nf_status = GDD4_UNAVAIL; > > + return nfs_ok; > > +} > > + > > static __be32 > > nfsd4_getdeviceinfo(struct svc_rqst *rqstp, > > struct nfsd4_compound_state *cstate, union nfsd4_op_u *u) > > @@ -3082,6 +3094,18 @@ static u32 nfsd4_copy_notify_rsize(const struct svc_rqst *rqstp, > > * sizeof(__be32); > > } > > > > +static u32 nfsd4_get_dir_delegation_rsize(const struct svc_rqst *rqstp, > > + const struct nfsd4_op *op) > > +{ > > + return (op_encode_hdr_size + > > + 1 /* gddr_status */ + > > + op_encode_verifier_maxsz + > > + op_encode_stateid_maxsz + > > + 2 /* gddr_notification */ + > > + 2 /* gddr_child_attributes */ + > > + 2 /* gddr_dir_attributes */); > > +} > > + > > #ifdef CONFIG_NFSD_PNFS > > static u32 nfsd4_getdeviceinfo_rsize(const struct svc_rqst *rqstp, > > const struct nfsd4_op *op) > > @@ -3470,6 +3494,12 @@ static const struct nfsd4_operation nfsd4_ops[] = { > > .op_get_currentstateid = nfsd4_get_freestateid, > > .op_rsize_bop = nfsd4_only_status_rsize, > > }, > > + [OP_GET_DIR_DELEGATION] = { > > + .op_func = nfsd4_get_dir_delegation, > > + .op_flags = OP_MODIFIES_SOMETHING, > > + .op_name = "OP_GET_DIR_DELEGATION", > > + .op_rsize_bop = nfsd4_get_dir_delegation_rsize, > > + }, > > #ifdef CONFIG_NFSD_PNFS > > [OP_GETDEVICEINFO] = { > > .op_func = nfsd4_getdeviceinfo, > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > > index fac938f563ad..3718bef74e9f 100644 > > --- a/fs/nfsd/nfs4xdr.c > > +++ b/fs/nfsd/nfs4xdr.c > > @@ -1732,6 +1732,40 @@ nfsd4_decode_free_stateid(struct nfsd4_compoundargs *argp, > > return nfsd4_decode_stateid4(argp, &free_stateid->fr_stateid); > > } > > > > +static __be32 > > +nfsd4_decode_get_dir_delegation(struct nfsd4_compoundargs *argp, > > + union nfsd4_op_u *u) > > +{ > > + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; > > + struct timespec64 ts; > > + u32 signal_deleg_avail; > > + u32 attrs[1]; > > I know this isn't how we've done XDR in the past, but I'd rather > see these dummy args as fields in struct nfsd4_get_dir_delegation, > and also move the comments about whether each argument is supported > to the putative nfsd4_proc_get_dir_delegation(). > > The actual implementation of GET_DIR_DELEGATION is in nfs4proc.c, > after all, not here. This is simply a translation function. > > > > + __be32 status; > > + > > + memset(gdd, 0, sizeof(*gdd)); > > + > > + /* No signal_avail support for now (and maybe never) */ > > + if (xdr_stream_decode_bool(argp->xdr, &signal_deleg_avail) < 0) > > + return nfserr_bad_xdr; > > + status = nfsd4_decode_bitmap4(argp, gdd->notification_types, > > + ARRAY_SIZE(gdd->notification_types)); > > + if (status) > > + return status; > > + > > + /* For now, we don't support child or dir attr change notification */ > > + status = nfsd4_decode_nfstime4(argp, &ts); > > + if (status) > > + return status; > > + /* No dir attr notification support yet either */ > > + status = nfsd4_decode_nfstime4(argp, &ts); > > + if (status) > > + return status; > > + status = nfsd4_decode_bitmap4(argp, attrs, ARRAY_SIZE(attrs)); > > + if (status) > > + return status; > > + return nfsd4_decode_bitmap4(argp, attrs, ARRAY_SIZE(attrs)); > > +} > > + > > #ifdef CONFIG_NFSD_PNFS > > static __be32 > > nfsd4_decode_getdeviceinfo(struct nfsd4_compoundargs *argp, > > @@ -2370,7 +2404,7 @@ static const nfsd4_dec nfsd4_dec_ops[] = { > > [OP_CREATE_SESSION] = nfsd4_decode_create_session, > > [OP_DESTROY_SESSION] = nfsd4_decode_destroy_session, > > [OP_FREE_STATEID] = nfsd4_decode_free_stateid, > > - [OP_GET_DIR_DELEGATION] = nfsd4_decode_notsupp, > > + [OP_GET_DIR_DELEGATION] = nfsd4_decode_get_dir_delegation, > > #ifdef CONFIG_NFSD_PNFS > > [OP_GETDEVICEINFO] = nfsd4_decode_getdeviceinfo, > > [OP_GETDEVICELIST] = nfsd4_decode_notsupp, > > @@ -5002,6 +5036,40 @@ nfsd4_encode_device_addr4(struct xdr_stream *xdr, > > return nfserr_toosmall; > > } > > > > +static __be32 > > +nfsd4_encode_get_dir_delegation(struct nfsd4_compoundres *resp, __be32 nfserr, > > + union nfsd4_op_u *u) > > +{ > > + struct xdr_stream *xdr = resp->xdr; > > + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; > > + > > + /* Encode the GDDR_* status code */ > > In other encoders, I've used simply the name of the field as it is > in the RFC as a documenting comment. That's more clear, and is > easily grep-able. So: > > /* gddrnf_status */ > > > > + if (xdr_stream_encode_u32(xdr, gdd->nf_status) != XDR_UNIT) > > + return nfserr_resource; > > + > > + /* if it's GDD4_UNAVAIL then we're (almost) done */ > > + if (gdd->nf_status == GDD4_UNAVAIL) { > > I prefer using a switch for XDR unions. That makes our > implementation look more like the XDR definition; easier for humans > to audit and modify. > > > > + /* We never call back */ > > + return nfsd4_encode_bool(xdr, false); > > Again, let's move this boolean to struct nfsd4_get_dir_delegation to > enable nfsd4_proc_get_dir_delegation to decide in the future. > > > > + } > > + > > + /* GDD4_OK case */ > > If a switch is used, then this comment becomes a real piece of > self-verifying code: > > case GDD4_OK: > > > > + nfserr = nfsd4_encode_verifier4(xdr, &gdd->cookieverf); > > + if (nfserr) > > + return nfserr; > > + nfserr = nfsd4_encode_stateid4(xdr, &gdd->stateid); > > + if (nfserr) > > + return nfserr; > > + /* No notifications (yet) */ > > + nfserr = nfsd4_encode_bitmap4(xdr, 0, 0, 0); > > + if (nfserr) > > + return nfserr; > > + nfserr = nfsd4_encode_bitmap4(xdr, 0, 0, 0); > > + if (nfserr) > > + return nfserr; > > + return nfsd4_encode_bitmap4(xdr, 0, 0, 0); > > All these as well can go in struct nfsd4_get_dir_delegation. > > > > +} > > + > > static __be32 > > nfsd4_encode_getdeviceinfo(struct nfsd4_compoundres *resp, __be32 nfserr, > > union nfsd4_op_u *u) > > @@ -5580,7 +5648,7 @@ static const nfsd4_enc nfsd4_enc_ops[] = { > > [OP_CREATE_SESSION] = nfsd4_encode_create_session, > > [OP_DESTROY_SESSION] = nfsd4_encode_noop, > > [OP_FREE_STATEID] = nfsd4_encode_noop, > > - [OP_GET_DIR_DELEGATION] = nfsd4_encode_noop, > > + [OP_GET_DIR_DELEGATION] = nfsd4_encode_get_dir_delegation, > > #ifdef CONFIG_NFSD_PNFS > > [OP_GETDEVICEINFO] = nfsd4_encode_getdeviceinfo, > > [OP_GETDEVICELIST] = nfsd4_encode_noop, > > diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h > > index 415516c1b27e..27de75f32dea 100644 > > --- a/fs/nfsd/xdr4.h > > +++ b/fs/nfsd/xdr4.h > > @@ -518,6 +518,13 @@ struct nfsd4_free_stateid { > > stateid_t fr_stateid; /* request */ > > }; > > > > +struct nfsd4_get_dir_delegation { > > + u32 notification_types[1]; /* request */ > > + u32 nf_status; /* response */ > > + nfs4_verifier cookieverf; /* response */ > > + stateid_t stateid; /* response */ > > +}; > > + > > /* also used for NVERIFY */ > > struct nfsd4_verify { > > u32 ve_bmval[3]; /* request */ > > @@ -797,6 +804,7 @@ struct nfsd4_op { > > struct nfsd4_reclaim_complete reclaim_complete; > > struct nfsd4_test_stateid test_stateid; > > struct nfsd4_free_stateid free_stateid; > > + struct nfsd4_get_dir_delegation get_dir_delegation; > > struct nfsd4_getdeviceinfo getdeviceinfo; > > struct nfsd4_layoutget layoutget; > > struct nfsd4_layoutcommit layoutcommit; > > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h > > index ef8d2d618d5b..11ad088b411d 100644 > > --- a/include/linux/nfs4.h > > +++ b/include/linux/nfs4.h > > @@ -701,6 +701,11 @@ enum state_protect_how4 { > > SP4_SSV = 2 > > }; > > > > +enum get_dir_delegation_non_fatal_res { > > + GDD4_OK = 0, > > + GDD4_UNAVAIL = 1 > > +}; > > + > > enum pnfs_layouttype { > > LAYOUT_NFSV4_1_FILES = 1, > > LAYOUT_OSD2_OBJECTS = 2, > > > > -- > > 2.44.0 > > >
On Mon, Mar 18, 2024 at 12:12:06PM -0400, Jeff Layton wrote: > On Sun, 2024-03-17 at 11:34 -0400, Chuck Lever wrote: > > On Fri, Mar 15, 2024 at 12:53:03PM -0400, Jeff Layton wrote: > > > This adds basic infrastructure for handing GET_DIR_DELEGATION calls from > > > clients, including the decoders and encoders. For now, the server side > > > always just returns that the delegation is GDDR_UNAVAIL (and that we > > > won't call back). > > > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > > --- > > > fs/nfsd/nfs4proc.c | 30 ++++++++++++++++++++++ > > > fs/nfsd/nfs4xdr.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > > > fs/nfsd/xdr4.h | 8 ++++++ > > > include/linux/nfs4.h | 5 ++++ > > > 4 files changed, 113 insertions(+), 2 deletions(-) > > > > Just a handful of style preferences below. > > > > Those comments all make sense. I'll respin along those lines. > > Also, I may go ahead and send this patch separately from the rest of the > series. I think it would be best to have trivial support for > GET_DIR_DELEGATION in the kernel server as soon as possible. > > Eventually, clients may start sending these calls, and it's better if we > can just return a non-fatal error instead of sending back NFSERR_NOTSUPP > when they do. I have no objection to the XDR pieces going into NFSD before the rest of the series. Thanks for this implementation! > > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > > > index 2927b1263f08..7973fe17bf3c 100644 > > > --- a/fs/nfsd/nfs4proc.c > > > +++ b/fs/nfsd/nfs4proc.c > > > @@ -2173,6 +2173,18 @@ nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type) > > > return nfsd4_layout_ops[layout_type]; > > > } > > > > > > +static __be32 > > > +nfsd4_get_dir_delegation(struct svc_rqst *rqstp, > > > + struct nfsd4_compound_state *cstate, > > > + union nfsd4_op_u *u) > > > +{ > > > + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; > > > + > > > + /* FIXME: actually return a delegation */ > > > + gdd->nf_status = GDD4_UNAVAIL; > > > + return nfs_ok; > > > +} > > > + > > > static __be32 > > > nfsd4_getdeviceinfo(struct svc_rqst *rqstp, > > > struct nfsd4_compound_state *cstate, union nfsd4_op_u *u) > > > @@ -3082,6 +3094,18 @@ static u32 nfsd4_copy_notify_rsize(const struct svc_rqst *rqstp, > > > * sizeof(__be32); > > > } > > > > > > +static u32 nfsd4_get_dir_delegation_rsize(const struct svc_rqst *rqstp, > > > + const struct nfsd4_op *op) > > > +{ > > > + return (op_encode_hdr_size + > > > + 1 /* gddr_status */ + > > > + op_encode_verifier_maxsz + > > > + op_encode_stateid_maxsz + > > > + 2 /* gddr_notification */ + > > > + 2 /* gddr_child_attributes */ + > > > + 2 /* gddr_dir_attributes */); > > > +} > > > + > > > #ifdef CONFIG_NFSD_PNFS > > > static u32 nfsd4_getdeviceinfo_rsize(const struct svc_rqst *rqstp, > > > const struct nfsd4_op *op) > > > @@ -3470,6 +3494,12 @@ static const struct nfsd4_operation nfsd4_ops[] = { > > > .op_get_currentstateid = nfsd4_get_freestateid, > > > .op_rsize_bop = nfsd4_only_status_rsize, > > > }, > > > + [OP_GET_DIR_DELEGATION] = { > > > + .op_func = nfsd4_get_dir_delegation, > > > + .op_flags = OP_MODIFIES_SOMETHING, > > > + .op_name = "OP_GET_DIR_DELEGATION", > > > + .op_rsize_bop = nfsd4_get_dir_delegation_rsize, > > > + }, > > > #ifdef CONFIG_NFSD_PNFS > > > [OP_GETDEVICEINFO] = { > > > .op_func = nfsd4_getdeviceinfo, > > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > > > index fac938f563ad..3718bef74e9f 100644 > > > --- a/fs/nfsd/nfs4xdr.c > > > +++ b/fs/nfsd/nfs4xdr.c > > > @@ -1732,6 +1732,40 @@ nfsd4_decode_free_stateid(struct nfsd4_compoundargs *argp, > > > return nfsd4_decode_stateid4(argp, &free_stateid->fr_stateid); > > > } > > > > > > +static __be32 > > > +nfsd4_decode_get_dir_delegation(struct nfsd4_compoundargs *argp, > > > + union nfsd4_op_u *u) > > > +{ > > > + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; > > > + struct timespec64 ts; > > > + u32 signal_deleg_avail; > > > + u32 attrs[1]; > > > > I know this isn't how we've done XDR in the past, but I'd rather > > see these dummy args as fields in struct nfsd4_get_dir_delegation, > > and also move the comments about whether each argument is supported > > to the putative nfsd4_proc_get_dir_delegation(). > > > > The actual implementation of GET_DIR_DELEGATION is in nfs4proc.c, > > after all, not here. This is simply a translation function. > > > > > > > + __be32 status; > > > + > > > + memset(gdd, 0, sizeof(*gdd)); > > > + > > > + /* No signal_avail support for now (and maybe never) */ > > > + if (xdr_stream_decode_bool(argp->xdr, &signal_deleg_avail) < 0) > > > + return nfserr_bad_xdr; > > > + status = nfsd4_decode_bitmap4(argp, gdd->notification_types, > > > + ARRAY_SIZE(gdd->notification_types)); > > > + if (status) > > > + return status; > > > + > > > + /* For now, we don't support child or dir attr change notification */ > > > + status = nfsd4_decode_nfstime4(argp, &ts); > > > + if (status) > > > + return status; > > > + /* No dir attr notification support yet either */ > > > + status = nfsd4_decode_nfstime4(argp, &ts); > > > + if (status) > > > + return status; > > > + status = nfsd4_decode_bitmap4(argp, attrs, ARRAY_SIZE(attrs)); > > > + if (status) > > > + return status; > > > + return nfsd4_decode_bitmap4(argp, attrs, ARRAY_SIZE(attrs)); > > > +} > > > + > > > #ifdef CONFIG_NFSD_PNFS > > > static __be32 > > > nfsd4_decode_getdeviceinfo(struct nfsd4_compoundargs *argp, > > > @@ -2370,7 +2404,7 @@ static const nfsd4_dec nfsd4_dec_ops[] = { > > > [OP_CREATE_SESSION] = nfsd4_decode_create_session, > > > [OP_DESTROY_SESSION] = nfsd4_decode_destroy_session, > > > [OP_FREE_STATEID] = nfsd4_decode_free_stateid, > > > - [OP_GET_DIR_DELEGATION] = nfsd4_decode_notsupp, > > > + [OP_GET_DIR_DELEGATION] = nfsd4_decode_get_dir_delegation, > > > #ifdef CONFIG_NFSD_PNFS > > > [OP_GETDEVICEINFO] = nfsd4_decode_getdeviceinfo, > > > [OP_GETDEVICELIST] = nfsd4_decode_notsupp, > > > @@ -5002,6 +5036,40 @@ nfsd4_encode_device_addr4(struct xdr_stream *xdr, > > > return nfserr_toosmall; > > > } > > > > > > +static __be32 > > > +nfsd4_encode_get_dir_delegation(struct nfsd4_compoundres *resp, __be32 nfserr, > > > + union nfsd4_op_u *u) > > > +{ > > > + struct xdr_stream *xdr = resp->xdr; > > > + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; > > > + > > > + /* Encode the GDDR_* status code */ > > > > In other encoders, I've used simply the name of the field as it is > > in the RFC as a documenting comment. That's more clear, and is > > easily grep-able. So: > > > > /* gddrnf_status */ > > > > > > > + if (xdr_stream_encode_u32(xdr, gdd->nf_status) != XDR_UNIT) > > > + return nfserr_resource; > > > + > > > + /* if it's GDD4_UNAVAIL then we're (almost) done */ > > > + if (gdd->nf_status == GDD4_UNAVAIL) { > > > > I prefer using a switch for XDR unions. That makes our > > implementation look more like the XDR definition; easier for humans > > to audit and modify. > > > > > > > + /* We never call back */ > > > + return nfsd4_encode_bool(xdr, false); > > > > Again, let's move this boolean to struct nfsd4_get_dir_delegation to > > enable nfsd4_proc_get_dir_delegation to decide in the future. > > > > > > > + } > > > + > > > + /* GDD4_OK case */ > > > > If a switch is used, then this comment becomes a real piece of > > self-verifying code: > > > > case GDD4_OK: > > > > > > > + nfserr = nfsd4_encode_verifier4(xdr, &gdd->cookieverf); > > > + if (nfserr) > > > + return nfserr; > > > + nfserr = nfsd4_encode_stateid4(xdr, &gdd->stateid); > > > + if (nfserr) > > > + return nfserr; > > > + /* No notifications (yet) */ > > > + nfserr = nfsd4_encode_bitmap4(xdr, 0, 0, 0); > > > + if (nfserr) > > > + return nfserr; > > > + nfserr = nfsd4_encode_bitmap4(xdr, 0, 0, 0); > > > + if (nfserr) > > > + return nfserr; > > > + return nfsd4_encode_bitmap4(xdr, 0, 0, 0); > > > > All these as well can go in struct nfsd4_get_dir_delegation. > > > > > > > +} > > > + > > > static __be32 > > > nfsd4_encode_getdeviceinfo(struct nfsd4_compoundres *resp, __be32 nfserr, > > > union nfsd4_op_u *u) > > > @@ -5580,7 +5648,7 @@ static const nfsd4_enc nfsd4_enc_ops[] = { > > > [OP_CREATE_SESSION] = nfsd4_encode_create_session, > > > [OP_DESTROY_SESSION] = nfsd4_encode_noop, > > > [OP_FREE_STATEID] = nfsd4_encode_noop, > > > - [OP_GET_DIR_DELEGATION] = nfsd4_encode_noop, > > > + [OP_GET_DIR_DELEGATION] = nfsd4_encode_get_dir_delegation, > > > #ifdef CONFIG_NFSD_PNFS > > > [OP_GETDEVICEINFO] = nfsd4_encode_getdeviceinfo, > > > [OP_GETDEVICELIST] = nfsd4_encode_noop, > > > diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h > > > index 415516c1b27e..27de75f32dea 100644 > > > --- a/fs/nfsd/xdr4.h > > > +++ b/fs/nfsd/xdr4.h > > > @@ -518,6 +518,13 @@ struct nfsd4_free_stateid { > > > stateid_t fr_stateid; /* request */ > > > }; > > > > > > +struct nfsd4_get_dir_delegation { > > > + u32 notification_types[1]; /* request */ > > > + u32 nf_status; /* response */ > > > + nfs4_verifier cookieverf; /* response */ > > > + stateid_t stateid; /* response */ > > > +}; > > > + > > > /* also used for NVERIFY */ > > > struct nfsd4_verify { > > > u32 ve_bmval[3]; /* request */ > > > @@ -797,6 +804,7 @@ struct nfsd4_op { > > > struct nfsd4_reclaim_complete reclaim_complete; > > > struct nfsd4_test_stateid test_stateid; > > > struct nfsd4_free_stateid free_stateid; > > > + struct nfsd4_get_dir_delegation get_dir_delegation; > > > struct nfsd4_getdeviceinfo getdeviceinfo; > > > struct nfsd4_layoutget layoutget; > > > struct nfsd4_layoutcommit layoutcommit; > > > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h > > > index ef8d2d618d5b..11ad088b411d 100644 > > > --- a/include/linux/nfs4.h > > > +++ b/include/linux/nfs4.h > > > @@ -701,6 +701,11 @@ enum state_protect_how4 { > > > SP4_SSV = 2 > > > }; > > > > > > +enum get_dir_delegation_non_fatal_res { > > > + GDD4_OK = 0, > > > + GDD4_UNAVAIL = 1 > > > +}; > > > + > > > enum pnfs_layouttype { > > > LAYOUT_NFSV4_1_FILES = 1, > > > LAYOUT_OSD2_OBJECTS = 2, > > > > > > -- > > > 2.44.0 > > > > > > > -- > Jeff Layton <jlayton@kernel.org>
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 2927b1263f08..7973fe17bf3c 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -2173,6 +2173,18 @@ nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type) return nfsd4_layout_ops[layout_type]; } +static __be32 +nfsd4_get_dir_delegation(struct svc_rqst *rqstp, + struct nfsd4_compound_state *cstate, + union nfsd4_op_u *u) +{ + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; + + /* FIXME: actually return a delegation */ + gdd->nf_status = GDD4_UNAVAIL; + return nfs_ok; +} + static __be32 nfsd4_getdeviceinfo(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, union nfsd4_op_u *u) @@ -3082,6 +3094,18 @@ static u32 nfsd4_copy_notify_rsize(const struct svc_rqst *rqstp, * sizeof(__be32); } +static u32 nfsd4_get_dir_delegation_rsize(const struct svc_rqst *rqstp, + const struct nfsd4_op *op) +{ + return (op_encode_hdr_size + + 1 /* gddr_status */ + + op_encode_verifier_maxsz + + op_encode_stateid_maxsz + + 2 /* gddr_notification */ + + 2 /* gddr_child_attributes */ + + 2 /* gddr_dir_attributes */); +} + #ifdef CONFIG_NFSD_PNFS static u32 nfsd4_getdeviceinfo_rsize(const struct svc_rqst *rqstp, const struct nfsd4_op *op) @@ -3470,6 +3494,12 @@ static const struct nfsd4_operation nfsd4_ops[] = { .op_get_currentstateid = nfsd4_get_freestateid, .op_rsize_bop = nfsd4_only_status_rsize, }, + [OP_GET_DIR_DELEGATION] = { + .op_func = nfsd4_get_dir_delegation, + .op_flags = OP_MODIFIES_SOMETHING, + .op_name = "OP_GET_DIR_DELEGATION", + .op_rsize_bop = nfsd4_get_dir_delegation_rsize, + }, #ifdef CONFIG_NFSD_PNFS [OP_GETDEVICEINFO] = { .op_func = nfsd4_getdeviceinfo, diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index fac938f563ad..3718bef74e9f 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -1732,6 +1732,40 @@ nfsd4_decode_free_stateid(struct nfsd4_compoundargs *argp, return nfsd4_decode_stateid4(argp, &free_stateid->fr_stateid); } +static __be32 +nfsd4_decode_get_dir_delegation(struct nfsd4_compoundargs *argp, + union nfsd4_op_u *u) +{ + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; + struct timespec64 ts; + u32 signal_deleg_avail; + u32 attrs[1]; + __be32 status; + + memset(gdd, 0, sizeof(*gdd)); + + /* No signal_avail support for now (and maybe never) */ + if (xdr_stream_decode_bool(argp->xdr, &signal_deleg_avail) < 0) + return nfserr_bad_xdr; + status = nfsd4_decode_bitmap4(argp, gdd->notification_types, + ARRAY_SIZE(gdd->notification_types)); + if (status) + return status; + + /* For now, we don't support child or dir attr change notification */ + status = nfsd4_decode_nfstime4(argp, &ts); + if (status) + return status; + /* No dir attr notification support yet either */ + status = nfsd4_decode_nfstime4(argp, &ts); + if (status) + return status; + status = nfsd4_decode_bitmap4(argp, attrs, ARRAY_SIZE(attrs)); + if (status) + return status; + return nfsd4_decode_bitmap4(argp, attrs, ARRAY_SIZE(attrs)); +} + #ifdef CONFIG_NFSD_PNFS static __be32 nfsd4_decode_getdeviceinfo(struct nfsd4_compoundargs *argp, @@ -2370,7 +2404,7 @@ static const nfsd4_dec nfsd4_dec_ops[] = { [OP_CREATE_SESSION] = nfsd4_decode_create_session, [OP_DESTROY_SESSION] = nfsd4_decode_destroy_session, [OP_FREE_STATEID] = nfsd4_decode_free_stateid, - [OP_GET_DIR_DELEGATION] = nfsd4_decode_notsupp, + [OP_GET_DIR_DELEGATION] = nfsd4_decode_get_dir_delegation, #ifdef CONFIG_NFSD_PNFS [OP_GETDEVICEINFO] = nfsd4_decode_getdeviceinfo, [OP_GETDEVICELIST] = nfsd4_decode_notsupp, @@ -5002,6 +5036,40 @@ nfsd4_encode_device_addr4(struct xdr_stream *xdr, return nfserr_toosmall; } +static __be32 +nfsd4_encode_get_dir_delegation(struct nfsd4_compoundres *resp, __be32 nfserr, + union nfsd4_op_u *u) +{ + struct xdr_stream *xdr = resp->xdr; + struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation; + + /* Encode the GDDR_* status code */ + if (xdr_stream_encode_u32(xdr, gdd->nf_status) != XDR_UNIT) + return nfserr_resource; + + /* if it's GDD4_UNAVAIL then we're (almost) done */ + if (gdd->nf_status == GDD4_UNAVAIL) { + /* We never call back */ + return nfsd4_encode_bool(xdr, false); + } + + /* GDD4_OK case */ + nfserr = nfsd4_encode_verifier4(xdr, &gdd->cookieverf); + if (nfserr) + return nfserr; + nfserr = nfsd4_encode_stateid4(xdr, &gdd->stateid); + if (nfserr) + return nfserr; + /* No notifications (yet) */ + nfserr = nfsd4_encode_bitmap4(xdr, 0, 0, 0); + if (nfserr) + return nfserr; + nfserr = nfsd4_encode_bitmap4(xdr, 0, 0, 0); + if (nfserr) + return nfserr; + return nfsd4_encode_bitmap4(xdr, 0, 0, 0); +} + static __be32 nfsd4_encode_getdeviceinfo(struct nfsd4_compoundres *resp, __be32 nfserr, union nfsd4_op_u *u) @@ -5580,7 +5648,7 @@ static const nfsd4_enc nfsd4_enc_ops[] = { [OP_CREATE_SESSION] = nfsd4_encode_create_session, [OP_DESTROY_SESSION] = nfsd4_encode_noop, [OP_FREE_STATEID] = nfsd4_encode_noop, - [OP_GET_DIR_DELEGATION] = nfsd4_encode_noop, + [OP_GET_DIR_DELEGATION] = nfsd4_encode_get_dir_delegation, #ifdef CONFIG_NFSD_PNFS [OP_GETDEVICEINFO] = nfsd4_encode_getdeviceinfo, [OP_GETDEVICELIST] = nfsd4_encode_noop, diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h index 415516c1b27e..27de75f32dea 100644 --- a/fs/nfsd/xdr4.h +++ b/fs/nfsd/xdr4.h @@ -518,6 +518,13 @@ struct nfsd4_free_stateid { stateid_t fr_stateid; /* request */ }; +struct nfsd4_get_dir_delegation { + u32 notification_types[1]; /* request */ + u32 nf_status; /* response */ + nfs4_verifier cookieverf; /* response */ + stateid_t stateid; /* response */ +}; + /* also used for NVERIFY */ struct nfsd4_verify { u32 ve_bmval[3]; /* request */ @@ -797,6 +804,7 @@ struct nfsd4_op { struct nfsd4_reclaim_complete reclaim_complete; struct nfsd4_test_stateid test_stateid; struct nfsd4_free_stateid free_stateid; + struct nfsd4_get_dir_delegation get_dir_delegation; struct nfsd4_getdeviceinfo getdeviceinfo; struct nfsd4_layoutget layoutget; struct nfsd4_layoutcommit layoutcommit; diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h index ef8d2d618d5b..11ad088b411d 100644 --- a/include/linux/nfs4.h +++ b/include/linux/nfs4.h @@ -701,6 +701,11 @@ enum state_protect_how4 { SP4_SSV = 2 }; +enum get_dir_delegation_non_fatal_res { + GDD4_OK = 0, + GDD4_UNAVAIL = 1 +}; + enum pnfs_layouttype { LAYOUT_NFSV4_1_FILES = 1, LAYOUT_OSD2_OBJECTS = 2,
This adds basic infrastructure for handing GET_DIR_DELEGATION calls from clients, including the decoders and encoders. For now, the server side always just returns that the delegation is GDDR_UNAVAIL (and that we won't call back). Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/nfsd/nfs4proc.c | 30 ++++++++++++++++++++++ fs/nfsd/nfs4xdr.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- fs/nfsd/xdr4.h | 8 ++++++ include/linux/nfs4.h | 5 ++++ 4 files changed, 113 insertions(+), 2 deletions(-)