Re: [PATCH v7 13/20] nfs: implement client support for NFS_LOCALIO_PROGRAM

NeilBrown
2024-06-25T23:21:16Z

On Tue, 25 Jun 2024, Mike Snitzer wrote:
> LOCALIOPROC_GETUUID allows a client to discover the server's uuid.
>
> nfs_local_probe() will retrieve server's uuid via LOCALIO protocol and
> verify the server with that uuid it is known to be local. This ensures
> client and server 1: support localio 2: are local to each other.
>
> All the knowledge of the LOCALIO RPC protocol is in fs/nfs/localio.c
> which implements just a single version (1) that is used independently
> of what NFS version is used.
>
> Get nfsd_open_local_fh and store it in rpc_client during client
> creation, put the symbol during nfs_local_disable -- which is also
> called during client destruction.
>
> 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/nfs/client.c | 6 +-
> fs/nfs/localio.c | 159 +++++++++++++++++++++++++++++++++++++++++---
> include/linux/nfs.h | 7 ++
> 3 files changed, 161 insertions(+), 11 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 1300c388f971..6faa9fdc444d 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -434,8 +434,10 @@ struct nfs_client *nfs_get_client(const struct nfs_client_initdata *cl_init)
> list_add_tail(&new->cl_share_link,
> &nn->nfs_client_list);
> spin_unlock(&nn->nfs_client_lock);
> - nfs_local_probe(new);
> - return rpc_ops->init_client(new, cl_init);
> + new = rpc_ops->init_client(new, cl_init);
> + if (!IS_ERR(new))
> + nfs_local_probe(new);
> + return new;

I would fold this back into the earlier patch that introduced
nfs_local_probe(). It makes this patch ugly.
But I won't insist.

NeilBrown

> }
>
> spin_unlock(&nn->nfs_client_lock);
> diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
> index 418b8d76692b..e4f860a51170 100644
> --- a/fs/nfs/localio.c
> +++ b/fs/nfs/localio.c
> @@ -15,6 +15,7 @@
> #include <linux/sunrpc/addr.h>
> #include <linux/inetdevice.h>
> #include <net/addrconf.h>
> +#include <linux/nfslocalio.h>
> #include <linux/module.h>
> #include <linux/bvec.h>
>
> @@ -123,13 +124,72 @@ nfs4errno(int errno)
> static bool localio_enabled __read_mostly = true;
> module_param(localio_enabled, bool, 0644);
>
> +static inline bool nfs_client_is_local(const struct nfs_client *clp)
> +{
> + return !!test_bit(NFS_CS_LOCAL_IO, &clp->cl_flags);
> +}
> +
> bool nfs_server_is_local(const struct nfs_client *clp)
> {
> - return test_bit(NFS_CS_LOCAL_IO, &clp->cl_flags) != 0 &&
> - localio_enabled;
> + return nfs_client_is_local(clp) && localio_enabled;
> }
> EXPORT_SYMBOL_GPL(nfs_server_is_local);
>
> +/*
> + * GETUUID XDR functions
> + */
> +
> +static void localio_xdr_enc_getuuidargs(struct rpc_rqst *req,
> + struct xdr_stream *xdr,
> + const void *data)
> +{
> + /* void function */
> +}
> +
> +static int localio_xdr_dec_getuuidres(struct rpc_rqst *req,
> + struct xdr_stream *xdr,
> + void *result)
> +{
> + u8 *uuid = result;
> +
> + return decode_opaque_fixed(xdr, uuid, UUID_SIZE);
> +}
> +
> +static const struct rpc_procinfo nfs_localio_procedures[] = {
> + [LOCALIOPROC_GETUUID] = {
> + .p_proc = LOCALIOPROC_GETUUID,
> + .p_encode = localio_xdr_enc_getuuidargs,
> + .p_decode = localio_xdr_dec_getuuidres,
> + .p_arglen = 0,
> + .p_replen = XDR_QUADLEN(UUID_SIZE),
> + .p_statidx = LOCALIOPROC_GETUUID,
> + .p_name = "GETUUID",
> + },
> +};
> +
> +static unsigned int nfs_localio_counts[ARRAY_SIZE(nfs_localio_procedures)];
> +const struct rpc_version nfslocalio_version1 = {
> + .number = 1,
> + .nrprocs = ARRAY_SIZE(nfs_localio_procedures),
> + .procs = nfs_localio_procedures,
> + .counts = nfs_localio_counts,
> +};
> +
> +static const struct rpc_version *nfslocalio_version[] = {
> + [1] = &nfslocalio_version1,
> +};
> +
> +extern const struct rpc_program nfslocalio_program;
> +static struct rpc_stat nfslocalio_rpcstat = { &nfslocalio_program };
> +
> +const struct rpc_program nfslocalio_program = {
> + .name = "nfslocalio",
> + .number = NFS_LOCALIO_PROGRAM,
> + .nrvers = ARRAY_SIZE(nfslocalio_version),
> + .version = nfslocalio_version,
> + .stats = &nfslocalio_rpcstat,
> +};
> +
> /*
> * nfs_local_enable - attempt to enable local i/o for an nfs_client
> */
> @@ -149,20 +209,100 @@ void nfs_local_disable(struct nfs_client *clp)
> {
> if (test_and_clear_bit(NFS_CS_LOCAL_IO, &clp->cl_flags)) {
> trace_nfs_local_disable(clp);
> + put_nfsd_open_local_fh();
> + clp->nfsd_open_local_fh = NULL;
> + if (!IS_ERR(clp->cl_rpcclient_localio)) {
> + rpc_shutdown_client(clp->cl_rpcclient_localio);
> + clp->cl_rpcclient_localio = ERR_PTR(-EINVAL);
> + }
> clp->cl_nfssvc_net = NULL;
> }
> }
>
> /*
> - * nfs_local_probe - probe local i/o support for an nfs_client
> + * nfs_init_localioclient - Initialise an NFS localio client connection
> */
> -void
> -nfs_local_probe(struct nfs_client *clp)
> +static void nfs_init_localioclient(struct nfs_client *clp)
> {
> - bool enable = false;
> + if (unlikely(!IS_ERR(clp->cl_rpcclient_localio)))
> + goto out;
> + clp->cl_rpcclient_localio = rpc_bind_new_program(clp->cl_rpcclient,
> + &nfslocalio_program, 1);
> + if (IS_ERR(clp->cl_rpcclient_localio))
> + goto out;
> + /* No errors! Assume that localio is supported */
> + clp->nfsd_open_local_fh = get_nfsd_open_local_fh();
> + if (!clp->nfsd_open_local_fh) {
> + rpc_shutdown_client(clp->cl_rpcclient_localio);
> + clp->cl_rpcclient_localio = ERR_PTR(-EINVAL);
> + }
> +out:
> + dprintk_rcu("%s: server (%s) %s NFS LOCALIO, nfsd_open_local_fh is %s.\n",
> + __func__, rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_ADDR),
> + (IS_ERR(clp->cl_rpcclient_localio) ? "does not support" : "supports"),
> + (clp->nfsd_open_local_fh ? "set" : "not set"));
> +}
>
> - if (enable)
> - nfs_local_enable(clp);
> +static bool nfs_local_server_getuuid(struct nfs_client *clp, uuid_t *nfsd_uuid)
> +{
> + u8 uuid[UUID_SIZE];
> + struct rpc_message msg = {
> + .rpc_resp = &uuid,
> + };
> + int status;
> +
> + nfs_init_localioclient(clp);
> + if (IS_ERR(clp->cl_rpcclient_localio))
> + return false;
> +
> + dprintk("%s: NFS issuing getuuid\n", __func__);
> + msg.rpc_proc = &nfs_localio_procedures[LOCALIOPROC_GETUUID];
> + status = rpc_call_sync(clp->cl_rpcclient_localio, &msg, 0);
> + dprintk("%s: NFS reply getuuid: status=%d uuid=%pU\n",
> + __func__, status, uuid);
> + if (status)
> + return false;
> +
> + import_uuid(nfsd_uuid, uuid);
> +
> + return true;
> +}
> +
> +/*
> + * nfs_local_probe - probe local i/o support for an nfs_server and nfs_client
> + * - called after alloc_client and init_client (so cl_rpcclient exists)
> + * - this function is idempotent, it can be called for old or new clients
> + */
> +void nfs_local_probe(struct nfs_client *clp)
> +{
> + uuid_t uuid;
> + struct net *net = NULL;
> +
> + if (!localio_enabled || clp->cl_rpcclient->cl_vers == 2)
> + goto unsupported;
> +
> + if (nfs_client_is_local(clp)) {
> + /* If already enabled, disable and re-enable */
> + nfs_local_disable(clp);
> + }
> +
> + /*
> + * Retrieve server's uuid via LOCALIO protocol and verify the
> + * server with that uuid is known to be local. This ensures
> + * client and server 1: support localio 2: are local to each other
> + * by verifying client's nfsd, with specified uuid, is local.
> + */
> + if (!nfs_local_server_getuuid(clp, &uuid) ||
> + !nfsd_uuid_is_local(&uuid, &net))
> + goto unsupported;
> +
> + dprintk("%s: detected local server.\n", __func__);
> + nfs_local_enable(clp, net);
> + return;
> +
> +unsupported:
> + /* localio not supported */
> + nfs_local_disable(clp);
> }
> EXPORT_SYMBOL_GPL(nfs_local_probe);
>
> @@ -189,7 +329,8 @@ nfs_local_open_fh(struct nfs_client *clp, const struct cred *cred,
> trace_nfs_local_open_fh(fh, mode, status);
> switch (status) {
> case -ENXIO:
> - nfs_local_disable(clp);
> + /* Revalidate localio, will disable if unsupported */
> + nfs_local_probe(clp);
> fallthrough;
> case -ETIMEDOUT:
> status = -EAGAIN;
> diff --git a/include/linux/nfs.h b/include/linux/nfs.h
> index 64ed672a0b34..036f6b0ed94d 100644
> --- a/include/linux/nfs.h
> +++ b/include/linux/nfs.h
> @@ -15,6 +15,13 @@
> #include <linux/crc32.h>
> #include <uapi/linux/nfs.h>
>
> +/* The localio program is entirely private to Linux and is
> + * NOT part of the uapi.
> + */
> +#define NFS_LOCALIO_PROGRAM 400122
> +#define LOCALIOPROC_NULL 0
> +#define LOCALIOPROC_GETUUID 1
> +
> /*
> * This is the kernel NFS client file handle representation
> */
> --
> 2.44.0
>
>