| Message ID | 20190916211353.18802-19-olga.kornievskaia@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | client and server support for "inter" SSC copy | expand |
On Mon, Sep 16, 2019 at 05:13:52PM -0400, Olga Kornievskaia wrote: > @@ -1956,6 +1964,45 @@ static void svcxdr_init_encode(struct svc_rqst *rqstp, > - rqstp->rq_auth_slack; > } > > +#ifdef CONFIG_NFSD_V4_2_INTER_SSC > +static __be32 > +check_if_stalefh_allowed(struct nfsd4_compoundargs *args) > +{ > + struct nfsd4_op *op, *current_op, *saved_op; current_op and saved_op need to be initialized to NULL here. > + struct nfsd4_copy *copy; > + struct nfsd4_putfh *putfh; > + int i; > + > + /* traverse all operation and if it's a COPY compound, mark the > + * source filehandle to skip verification > + */ > + for (i = 0; i < args->opcnt; i++) { > + op = &args->ops[i]; > + if (op->opnum == OP_PUTFH) > + current_op = op; > + else if (op->opnum == OP_SAVEFH) > + saved_op = current_op; > + else if (op->opnum == OP_RESTOREFH) > + current_op = saved_op; > + else if (op->opnum == OP_COPY) { > + copy = (struct nfsd4_copy *)&op->u; > + if (!saved_op) > + return nfserr_nofilehandle; Looks like this results in returning an empty compound result with just the bare result. I believe what we need to do is execute all the ops preceding the COPY normally, then return the nofilehandle error on the COPY. One approach might be if (!saved_op) { op->status = nfserr_nofilehandle; return; } and change check_if_stalefh_allowed to have no return value. --b. > + putfh = (struct nfsd4_putfh *)&saved_op->u; > + if (!copy->cp_intra) > + putfh->no_verify = true; > + } > + } > + return nfs_ok; > +} ... > @@ -2004,6 +2051,9 @@ static void svcxdr_init_encode(struct svc_rqst *rqstp, > resp->opcnt = 1; > goto encode_op; > } > + status = check_if_stalefh_allowed(args); > + if (status) > + goto out; > > trace_nfsd_compound(rqstp, args->opcnt); > while (!status && resp->opcnt < args->opcnt) {
On Wed, Oct 2, 2019 at 3:55 PM J. Bruce Fields <bfields@fieldses.org> wrote: > > On Mon, Sep 16, 2019 at 05:13:52PM -0400, Olga Kornievskaia wrote: > > @@ -1956,6 +1964,45 @@ static void svcxdr_init_encode(struct svc_rqst *rqstp, > > - rqstp->rq_auth_slack; > > } > > > > +#ifdef CONFIG_NFSD_V4_2_INTER_SSC > > +static __be32 > > +check_if_stalefh_allowed(struct nfsd4_compoundargs *args) > > +{ > > + struct nfsd4_op *op, *current_op, *saved_op; > > current_op and saved_op need to be initialized to NULL here. > > > + struct nfsd4_copy *copy; > > + struct nfsd4_putfh *putfh; > > + int i; > > + > > + /* traverse all operation and if it's a COPY compound, mark the > > + * source filehandle to skip verification > > + */ > > + for (i = 0; i < args->opcnt; i++) { > > + op = &args->ops[i]; > > + if (op->opnum == OP_PUTFH) > > + current_op = op; > > + else if (op->opnum == OP_SAVEFH) > > + saved_op = current_op; > > + else if (op->opnum == OP_RESTOREFH) > > + current_op = saved_op; > > + else if (op->opnum == OP_COPY) { > > + copy = (struct nfsd4_copy *)&op->u; > > + if (!saved_op) > > + return nfserr_nofilehandle; > > Looks like this results in returning an empty compound result with just > the bare result. I believe what we need to do is execute all the > ops preceding the COPY normally, then return the nofilehandle error on > the COPY. > > One approach might be > > if (!saved_op) { > op->status = nfserr_nofilehandle; > return; > } > > and change check_if_stalefh_allowed to have no return value. Ok thanks. I'm just curious if op->status assignment is necessary or I can just do a return. When nfsd4_copy() calls nfs4_verify_copy() it would check if (!cstate->save_fh.fh_dentry) return nfserr_nofilehandle; > > --b. > > > + putfh = (struct nfsd4_putfh *)&saved_op->u; > > + if (!copy->cp_intra) > > + putfh->no_verify = true; > > + } > > + } > > + return nfs_ok; > > +} > ... > > @@ -2004,6 +2051,9 @@ static void svcxdr_init_encode(struct svc_rqst *rqstp, > > resp->opcnt = 1; > > goto encode_op; > > } > > + status = check_if_stalefh_allowed(args); > > + if (status) > > + goto out; > > > > > trace_nfsd_compound(rqstp, args->opcnt); > > while (!status && resp->opcnt < args->opcnt) {
On Mon, Oct 07, 2019 at 10:31:04AM -0400, Olga Kornievskaia wrote: > On Wed, Oct 2, 2019 at 3:55 PM J. Bruce Fields <bfields@fieldses.org> wrote: > > > > On Mon, Sep 16, 2019 at 05:13:52PM -0400, Olga Kornievskaia wrote: > > > @@ -1956,6 +1964,45 @@ static void svcxdr_init_encode(struct svc_rqst *rqstp, > > > - rqstp->rq_auth_slack; > > > } > > > > > > +#ifdef CONFIG_NFSD_V4_2_INTER_SSC > > > +static __be32 > > > +check_if_stalefh_allowed(struct nfsd4_compoundargs *args) > > > +{ > > > + struct nfsd4_op *op, *current_op, *saved_op; > > > > current_op and saved_op need to be initialized to NULL here. > > > > > + struct nfsd4_copy *copy; > > > + struct nfsd4_putfh *putfh; > > > + int i; > > > + > > > + /* traverse all operation and if it's a COPY compound, mark the > > > + * source filehandle to skip verification > > > + */ > > > + for (i = 0; i < args->opcnt; i++) { > > > + op = &args->ops[i]; > > > + if (op->opnum == OP_PUTFH) > > > + current_op = op; > > > + else if (op->opnum == OP_SAVEFH) > > > + saved_op = current_op; > > > + else if (op->opnum == OP_RESTOREFH) > > > + current_op = saved_op; > > > + else if (op->opnum == OP_COPY) { > > > + copy = (struct nfsd4_copy *)&op->u; > > > + if (!saved_op) > > > + return nfserr_nofilehandle; > > > > Looks like this results in returning an empty compound result with just > > the bare result. I believe what we need to do is execute all the > > ops preceding the COPY normally, then return the nofilehandle error on > > the COPY. > > > > One approach might be > > > > if (!saved_op) { > > op->status = nfserr_nofilehandle; > > return; > > } > > > > and change check_if_stalefh_allowed to have no return value. > > Ok thanks. I'm just curious if op->status assignment is necessary or I > can just do a return. When nfsd4_copy() calls nfs4_verify_copy() it > would check > if (!cstate->save_fh.fh_dentry) > return nfserr_nofilehandle; Catching the error here might be a little safer if you think there's ever a risk of messing up the later logic? But, really, either should work. Since a real client is never going to hit any interesting cases here, it could be useful to have a pynfs test or two that sent some weird illegal compounds. --b. > > > > > --b. > > > > > + putfh = (struct nfsd4_putfh *)&saved_op->u; > > > + if (!copy->cp_intra) > > > + putfh->no_verify = true; > > > + } > > > + } > > > + return nfs_ok; > > > +} > > ... > > > @@ -2004,6 +2051,9 @@ static void svcxdr_init_encode(struct svc_rqst *rqstp, > > > resp->opcnt = 1; > > > goto encode_op; > > > } > > > + status = check_if_stalefh_allowed(args); > > > + if (status) > > > + goto out; > > > > > > > > trace_nfsd_compound(rqstp, args->opcnt); > > > while (!status && resp->opcnt < args->opcnt) {
diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig index 10cefb0..b7172a8 100644 --- a/fs/nfsd/Kconfig +++ b/fs/nfsd/Kconfig @@ -133,6 +133,16 @@ config NFSD_FLEXFILELAYOUT If unsure, say N. +config NFSD_V4_2_INTER_SSC + bool "NFSv4.2 inter server to server COPY" + depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2 + help + This option enables support for NFSv4.2 inter server to + server copy where the destination server calls the NFSv4.2 + client to read the data to copy from the source server. + + If unsure, say N. + config NFSD_V4_SECURITY_LABEL bool "Provide Security Label support for NFSv4 server" depends on NFSD_V4 && SECURITY diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 90d0b67..7574ba7 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -504,12 +504,20 @@ static __be32 nfsd4_open_omfg(struct svc_rqst *rqstp, struct nfsd4_compound_stat union nfsd4_op_u *u) { struct nfsd4_putfh *putfh = &u->putfh; + __be32 ret; fh_put(&cstate->current_fh); cstate->current_fh.fh_handle.fh_size = putfh->pf_fhlen; memcpy(&cstate->current_fh.fh_handle.fh_base, putfh->pf_fhval, putfh->pf_fhlen); - return fh_verify(rqstp, &cstate->current_fh, 0, NFSD_MAY_BYPASS_GSS); + ret = fh_verify(rqstp, &cstate->current_fh, 0, NFSD_MAY_BYPASS_GSS); +#ifdef CONFIG_NFSD_V4_2_INTER_SSC + if (ret == nfserr_stale && putfh->no_verify) { + SET_FH_FLAG(&cstate->current_fh, NFSD4_FH_FOREIGN); + ret = 0; + } +#endif + return ret; } static __be32 @@ -1956,6 +1964,45 @@ static void svcxdr_init_encode(struct svc_rqst *rqstp, - rqstp->rq_auth_slack; } +#ifdef CONFIG_NFSD_V4_2_INTER_SSC +static __be32 +check_if_stalefh_allowed(struct nfsd4_compoundargs *args) +{ + struct nfsd4_op *op, *current_op, *saved_op; + struct nfsd4_copy *copy; + struct nfsd4_putfh *putfh; + int i; + + /* traverse all operation and if it's a COPY compound, mark the + * source filehandle to skip verification + */ + for (i = 0; i < args->opcnt; i++) { + op = &args->ops[i]; + if (op->opnum == OP_PUTFH) + current_op = op; + else if (op->opnum == OP_SAVEFH) + saved_op = current_op; + else if (op->opnum == OP_RESTOREFH) + current_op = saved_op; + else if (op->opnum == OP_COPY) { + copy = (struct nfsd4_copy *)&op->u; + if (!saved_op) + return nfserr_nofilehandle; + putfh = (struct nfsd4_putfh *)&saved_op->u; + if (!copy->cp_intra) + putfh->no_verify = true; + } + } + return nfs_ok; +} +#else +static __be32 +check_if_stalefh_allowed(struct nfsd4_compoundargs *args) +{ + return nfs_ok; +} +#endif + /* * COMPOUND call. */ @@ -2004,6 +2051,9 @@ static void svcxdr_init_encode(struct svc_rqst *rqstp, resp->opcnt = 1; goto encode_op; } + status = check_if_stalefh_allowed(args); + if (status) + goto out; trace_nfsd_compound(rqstp, args->opcnt); while (!status && resp->opcnt < args->opcnt) { @@ -2019,13 +2069,14 @@ static void svcxdr_init_encode(struct svc_rqst *rqstp, op->status = nfsd4_open_omfg(rqstp, cstate, op); goto encode_op; } - - if (!current_fh->fh_dentry) { + if (!current_fh->fh_dentry && + !HAS_FH_FLAG(current_fh, NFSD4_FH_FOREIGN)) { if (!(op->opdesc->op_flags & ALLOWED_WITHOUT_FH)) { op->status = nfserr_nofilehandle; goto encode_op; } - } else if (current_fh->fh_export->ex_fslocs.migrated && + } else if (current_fh->fh_export && + current_fh->fh_export->ex_fslocs.migrated && !(op->opdesc->op_flags & ALLOWED_ON_ABSENT_FS)) { op->status = nfserr_moved; goto encode_op; diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h index 755e256..b9c7568 100644 --- a/fs/nfsd/nfsfh.h +++ b/fs/nfsd/nfsfh.h @@ -35,7 +35,7 @@ static inline ino_t u32_to_ino_t(__u32 uino) bool fh_locked; /* inode locked by us */ bool fh_want_write; /* remount protection taken */ - + int fh_flags; /* FH flags */ #ifdef CONFIG_NFSD_V3 bool fh_post_saved; /* post-op attrs saved */ bool fh_pre_saved; /* pre-op attrs saved */ @@ -56,6 +56,9 @@ static inline ino_t u32_to_ino_t(__u32 uino) #endif /* CONFIG_NFSD_V3 */ } svc_fh; +#define NFSD4_FH_FOREIGN (1<<0) +#define SET_FH_FLAG(c, f) ((c)->fh_flags |= (f)) +#define HAS_FH_FLAG(c, f) ((c)->fh_flags & (f)) enum nfsd_fsid { FSID_DEV = 0, diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h index 0b4fe07..b16f602 100644 --- a/fs/nfsd/xdr4.h +++ b/fs/nfsd/xdr4.h @@ -221,6 +221,7 @@ struct nfsd4_lookup { struct nfsd4_putfh { u32 pf_fhlen; /* request */ char *pf_fhval; /* request */ + bool no_verify; /* represents foreigh fh */ }; struct nfsd4_open {
The inter server to server COPY source server filehandle is a foreign filehandle as the COPY is sent to the destination server. Signed-off-by: Olga Kornievskaia <kolga@netapp.com> --- fs/nfsd/Kconfig | 10 +++++++++ fs/nfsd/nfs4proc.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++---- fs/nfsd/nfsfh.h | 5 ++++- fs/nfsd/xdr4.h | 1 + 4 files changed, 70 insertions(+), 5 deletions(-)