Re: [PATCH v7 14/20] nfsd: implement server support for NFS_LOCALIO_PROGRAM

Chuck Lever
2024-06-24T18:46:01Z

On Mon, Jun 24, 2024 at 12:27:35PM -0400, Mike Snitzer wrote:
> LOCALIOPROC_GETUUID encodes the server's uuid_t in terms of the fixed
> UUID_SIZE (16). The fixed size opaque encode and decode XDR methods
> are used instead of the less efficient variable sized methods.
>
> Aside from a bit of code in nfssvc.c, all the knowledge of the LOCALIO
> RPC protocol is in fs/nfsd/localio.c which implements just a single
> version (1) that is used independently of what NFS version is used.
>
> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> [neilb: factored out and simplified single localio protocol]
> Co-developed-by: NeilBrown <neilb@suse.de>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/localio.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/nfssvc.c | 29 ++++++++++++++++++-
> 2 files changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
> index f6df66b1d523..aaa5293eb352 100644
> --- a/fs/nfsd/localio.c
> +++ b/fs/nfsd/localio.c
> @@ -11,12 +11,15 @@
> #include <linux/sunrpc/svcauth_gss.h>
> #include <linux/sunrpc/clnt.h>
> #include <linux/nfs.h>
> +#include <linux/nfs_fs.h>
> +#include <linux/nfs_xdr.h>
> #include <linux/string.h>
>
> #include "nfsd.h"
> #include "vfs.h"
> #include "netns.h"
> #include "filecache.h"
> +#include "cache.h"
>
> #define NFSDDBG_FACILITY NFSDDBG_FH
>
> @@ -249,3 +252,74 @@ EXPORT_SYMBOL_GPL(nfsd_open_local_fh);
>
> /* Compile time type checking, not used by anything */
> static nfs_to_nfsd_open_t __maybe_unused nfsd_open_local_fh_typecheck = nfsd_open_local_fh;
> +
> +/*
> + * GETUUID XDR encode functions
> + */
> +
> +static __be32 nfsd_proc_null(struct svc_rqst *rqstp)
> +{
> + return rpc_success;
> +}

We already have an nfsd_proc_null() (for NFSv2). Let's use

localio_proc_null()

instead.

> +struct nfsd_getuuidres {
> + uuid_t uuid;
> +};
> +
> +static __be32 nfsd_proc_getuuid(struct svc_rqst *rqstp)

And here, please use

localio_proc_getuuid()

instead.

> +{
> + struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> + struct nfsd_getuuidres *resp = rqstp->rq_resp;
> +
> + uuid_copy(&resp->uuid, &nn->nfsd_uuid.uuid);
> +
> + return rpc_success;
> +}
> +
> +static bool nfslocalio_encode_getuuidres(struct svc_rqst *rqstp,
> + struct xdr_stream *xdr)

Please use

localio_encode_getuuidres()

instead.

> +{
> + struct nfsd_getuuidres *resp = rqstp->rq_resp;
> + u8 uuid[UUID_SIZE];
> +
> + export_uuid(uuid, &resp->uuid);
> + encode_opaque_fixed(xdr, uuid, UUID_SIZE);
> + dprintk("%s: uuid=%pU\n", __func__, uuid);
> +
> + return true;
> +}
> +
> +static const struct svc_procedure nfsd_localio_procedures[2] = {

I think I prefer:

localio_procedures1

> + [LOCALIOPROC_NULL] = {
> + .pc_func = nfsd_proc_null,
> + .pc_decode = nfssvc_decode_voidarg,
> + .pc_encode = nfssvc_encode_voidres,
> + .pc_argsize = sizeof(struct nfsd_voidargs),
> + .pc_ressize = sizeof(struct nfsd_voidres),
> + .pc_cachetype = RC_NOCACHE,
> + .pc_xdrressize = 0,
> + .pc_name = "NULL",
> + },
> + [LOCALIOPROC_GETUUID] = {
> + .pc_func = nfsd_proc_getuuid,
> + .pc_decode = nfssvc_decode_voidarg,
> + .pc_encode = nfslocalio_encode_getuuidres,
> + .pc_argsize = sizeof(struct nfsd_voidargs),
> + .pc_ressize = sizeof(struct nfsd_getuuidres),
> + .pc_cachetype = RC_NOCACHE,
> + .pc_xdrressize = XDR_QUADLEN(UUID_SIZE),
> + .pc_name = "GETUUID",
> + },
> +};
> +
> +static DEFINE_PER_CPU_ALIGNED(unsigned long,
> + nfsd_localio_count[ARRAY_SIZE(nfsd_localio_procedures)]);
> +const struct svc_version nfsd_localio_version1 = {

I'd prefer:

localiosvc_version1

> + .vs_vers = 1,
> + .vs_nproc = 2,
> + .vs_proc = nfsd_localio_procedures,
> + .vs_dispatch = nfsd_dispatch,
> + .vs_count = nfsd_localio_count,
> + .vs_xdrsize = XDR_QUADLEN(UUID_SIZE),
> + .vs_hidden = true,
> +};
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index a477d2c5088a..bc69a2c90077 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -81,6 +81,26 @@ DEFINE_SPINLOCK(nfsd_drc_lock);
> unsigned long nfsd_drc_max_mem;
> unsigned long nfsd_drc_mem_used;
>
> +#if IS_ENABLED(CONFIG_NFSD_LOCALIO)
> +extern const struct svc_version nfsd_localio_version1;
> +static const struct svc_version *nfsd_localio_version[] = {

Instead:

localio_versions[]

> + [1] = &nfsd_localio_version1,
> +};
> +
> +#define NFSD_LOCALIO_NRVERS ARRAY_SIZE(nfsd_localio_version)
> +
> +static struct svc_program nfsd_localio_program = {
> + .pg_prog = NFS_LOCALIO_PROGRAM,
> + .pg_nvers = NFSD_LOCALIO_NRVERS,
> + .pg_vers = nfsd_localio_version,
> + .pg_name = "nfslocalio",
> + .pg_class = "nfsd",
> + .pg_authenticate = &svc_set_client,
> + .pg_init_request = svc_generic_init_request,
> + .pg_rpcbind_set = svc_generic_rpcbind_set,
> +};
> +#endif /* CONFIG_NFSD_LOCALIO */
> +
> #if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
> static const struct svc_version *nfsd_acl_version[] = {
> # if defined(CONFIG_NFSD_V2_ACL)
> @@ -95,6 +115,9 @@ static const struct svc_version *nfsd_acl_version[] = {
> #define NFSD_ACL_NRVERS ARRAY_SIZE(nfsd_acl_version)
>
> static struct svc_program nfsd_acl_program = {
> +#if IS_ENABLED(CONFIG_NFSD_LOCALIO)
> + .pg_next = &nfsd_localio_program,
> +#endif /* CONFIG_NFSD_LOCALIO */
> .pg_prog = NFS_ACL_PROGRAM,
> .pg_nvers = NFSD_ACL_NRVERS,
> .pg_vers = nfsd_acl_version,
> @@ -123,6 +146,10 @@ static const struct svc_version *nfsd_version[] = {
> struct svc_program nfsd_program = {
> #if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
> .pg_next = &nfsd_acl_program,
> +#else
> +#if IS_ENABLED(CONFIG_NFSD_LOCALIO)
> + .pg_next = &nfsd_localio_program,
> +#endif /* CONFIG_NFSD_LOCALIO */
> #endif
> .pg_prog = NFS_PROGRAM, /* program number */
> .pg_nvers = NFSD_NRVERS, /* nr of entries in nfsd_version */
> @@ -975,7 +1002,7 @@ nfsd(void *vrqstp)
> }
>
> /**
> - * nfsd_dispatch - Process an NFS or NFSACL Request
> + * nfsd_dispatch - Process an NFS or NFSACL or NFSLOCALIO Request

Nit: "nfsd_dispatch - Process an NFS, NFSACL, or LOCALIO request"

> * @rqstp: incoming request
> *
> * This RPC dispatcher integrates the NFS server's duplicate reply cache.
> --
> 2.44.0
>

--
Chuck Lever