On Mon, Jul 01, 2024 at 12:53:16PM +1000, NeilBrown wrote:
> This is a step towards having an interface like fh_verify() which
> doesn't require a struct svc_rqst *, but instead takes the specific
> parts of that which are actually needed.
>
> This first step allows the 'struct nfsd_net *' to be passed in
> separately. __fh_verify() does not use SVC_NET(), nor does its
> callers.
It might be a little cleaner to first update rqst_exp_find() as a
zeroeth step, but no big deal.
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> fs/nfsd/export.c | 12 ++++++++----
> fs/nfsd/export.h | 4 +++-
> fs/nfsd/nfs4proc.c | 2 +-
> fs/nfsd/nfsfh.c | 20 ++++++++++++++------
> 4 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 50b3135d07ac..a35f06b610d0 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -1165,11 +1165,15 @@ rqst_exp_get_by_name(struct svc_rqst *rqstp, struct path *path)
> }
>
> struct svc_export *
> -rqst_exp_find(struct svc_rqst *rqstp, int fsid_type, u32 *fsidv)
> +rqst_exp_find(struct svc_rqst *rqstp, struct nfsd_net *nn,
Extra blank after "*rqstp,"
I agree with Jeff, the new modern rqst_exp_find() deserves a kdoc
comment.
> + int fsid_type, u32 *fsidv)
> {
> struct svc_export *gssexp, *exp = ERR_PTR(-ENOENT);
> - struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> - struct cache_detail *cd = nn->svc_export_cache;
> + struct cache_detail *cd;
> +
> + if (!nn)
> + nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
If it's this easy to fabricate a network namespace, IMO the API
would be more straightforward if all callers passed a valid @nn.
In fact, by the end of the series, the combinations of which
parameters are allowed to be NULL and what each NULL means is a bit
brittle... Perhaps you can make that simpler in the next version of
this series?
> + cd = nn->svc_export_cache;
>
> if (rqstp->rq_client == NULL)
> goto gss;
> @@ -1220,7 +1224,7 @@ struct svc_export *rqst_find_fsidzero_export(struct svc_rqst *rqstp)
>
> mk_fsid(FSID_NUM, fsidv, 0, 0, 0, NULL);
>
> - return rqst_exp_find(rqstp, FSID_NUM, fsidv);
> + return rqst_exp_find(rqstp, NULL, FSID_NUM, fsidv);
Right, so here, I don't think there's a problem manufacturing a
proper nn to pass to rqstp_exp_find().
> }
>
> /*
> diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h
> index ca9dc230ae3d..1a54d388d58d 100644
> --- a/fs/nfsd/export.h
> +++ b/fs/nfsd/export.h
> @@ -127,6 +127,8 @@ static inline struct svc_export *exp_get(struct svc_export *exp)
> cache_get(&exp->h);
> return exp;
> }
> -struct svc_export * rqst_exp_find(struct svc_rqst *, int, u32 *);
> +struct nfsd_net;
> +struct svc_export * rqst_exp_find(struct svc_rqst *, struct nfsd_net *,
> + int, u32 *);
>
> #endif /* NFSD_EXPORT_H */
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 2e39cf2e502a..30335cdf9e6c 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -2231,7 +2231,7 @@ nfsd4_getdeviceinfo(struct svc_rqst *rqstp,
> return nfserr_noent;
> }
>
> - exp = rqst_exp_find(rqstp, map->fsid_type, map->fsid);
> + exp = rqst_exp_find(rqstp, NULL, map->fsid_type, map->fsid);
Ditto.
> if (IS_ERR(exp)) {
> dprintk("%s: could not find device id\n", __func__);
> return nfserr_noent;
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index 0b75305fb5f5..e27ed27054ab 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -151,7 +151,8 @@ static inline __be32 check_pseudo_root(struct svc_rqst *rqstp,
> * dentry. On success, the results are used to set fh_export and
> * fh_dentry.
> */
> -static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp)
> +static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct nfsd_net *nn,
> + struct svc_fh *fhp)
> {
> struct knfsd_fh *fh = &fhp->fh_handle;
> struct fid *fid = NULL;
> @@ -195,7 +196,7 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp)
> data_left -= len;
> if (data_left < 0)
> return error;
> - exp = rqst_exp_find(rqstp, fh->fh_fsid_type, fh->fh_fsid);
> + exp = rqst_exp_find(rqstp, nn, fh->fh_fsid_type, fh->fh_fsid);
> fid = (struct fid *)(fh->fh_fsid + len);
>
> error = nfserr_stale;
> @@ -324,16 +325,16 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp)
> * @access is formed from the NFSD_MAY_* constants defined in
> * fs/nfsd/vfs.h.
> */
> -__be32
> -fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> +static __be32
> +__fh_verify(struct svc_rqst *rqstp, struct nfsd_net *nn,
> + struct svc_fh *fhp, umode_t type, int access)
> {
> - struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> struct svc_export *exp = NULL;
> struct dentry *dentry;
> __be32 error;
>
> if (!fhp->fh_dentry) {
> - error = nfsd_set_fh_dentry(rqstp, fhp);
> + error = nfsd_set_fh_dentry(rqstp, nn, fhp);
> if (error)
> goto out;
> }
> @@ -400,6 +401,13 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> return error;
> }
Can this patch copy the kdoc comment from above? And of course the
kdoc comment for __fh_verify() will need to be updated
appropriately.
> +__be32
> +fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> +{
> + return __fh_verify(rqstp, net_generic(SVC_NET(rqstp), nfsd_net_id),
> + fhp, type, access);
> +}
> +
Extra blank line added.
> /*
> * Compose a file handle for an NFS reply.
> --
> 2.44.0
>
--
Chuck Lever
