Re: [PATCH v15 14/26] nfs_common: add NFS LOCALIO auxiliary protocol enablement

[Anna Schumaker <anna@kernel.org>]

On Sat, Aug 31, 2024 at 6:38 PM Mike Snitzer <snitzer@kernel.org> wrote:
>
> fs/nfs_common/nfslocalio.c provides interfaces that enable an NFS
> client to generate a nonce (single-use UUID) and associated
> short-lived nfs_uuid_t struct, register it with nfs_common for
> subsequent lookup and verification by the NFS server and if matched
> the NFS server populates members in the nfs_uuid_t struct.
>
> nfs_common's nfs_uuids list is the basis for localio enablement, as
> such it has members that point to nfsd memory for direct use by the
> client (e.g. 'net' is the server's network namespace, through it the
> client can access nn->nfsd_serv).
>
> This commit also provides the base nfs_uuid_t interfaces to allow
> proper net namespace refcounting for the LOCALIO use case.
>
> CONFIG_NFS_LOCALIO controls the nfs_common, NFS server and NFS client
> enablement for LOCALIO. If both NFS_FS=m and NFSD=m then
> NFS_COMMON_LOCALIO_SUPPORT=m and nfs_localio.ko is built (and provides
> nfs_common's LOCALIO support).
>
>   # lsmod | grep nfs_localio
>   nfs_localio            12288  2 nfsd,nfs
>   sunrpc                745472  35 nfs_localio,nfsd,auth_rpcgss,lockd,nfsv3,nfs
>
> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> Co-developed-by: NeilBrown <neilb@suse.de>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  fs/Kconfig                 |  23 ++++++++
>  fs/nfs_common/Makefile     |   3 +
>  fs/nfs_common/nfslocalio.c | 116 +++++++++++++++++++++++++++++++++++++
>  include/linux/nfslocalio.h |  36 ++++++++++++
>  4 files changed, 178 insertions(+)
>  create mode 100644 fs/nfs_common/nfslocalio.c
>  create mode 100644 include/linux/nfslocalio.h
>
> diff --git a/fs/Kconfig b/fs/Kconfig
> index a46b0cbc4d8f..24d4e4b419d1 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -382,6 +382,29 @@ config NFS_COMMON
>         depends on NFSD || NFS_FS || LOCKD
>         default y
>
> +config NFS_COMMON_LOCALIO_SUPPORT
> +       tristate
> +       default n
> +       default y if NFSD=y || NFS_FS=y
> +       default m if NFSD=m && NFS_FS=m
> +       select SUNRPC
> +
> +config NFS_LOCALIO
> +       bool "NFS client and server support for LOCALIO auxiliary protocol"
> +       depends on NFSD && NFS_FS
> +       select NFS_COMMON_LOCALIO_SUPPORT
> +       default n
> +       help
> +         Some NFS servers support an auxiliary NFS LOCALIO protocol
> +         that is not an official part of the NFS protocol.
> +
> +         This option enables support for the LOCALIO protocol in the
> +         kernel's NFS server and client. Enable this to permit local
> +         NFS clients to bypass the network when issuing reads and
> +         writes to the local NFS server.
> +
> +         If unsure, say N.
> +

I'm wondering if it would make sense to create a fs/nfs_common/Kconfig
file at some point (not as part of this patchset!) to hold this group
of nfs_common options and to tidy up this section of the fs/Kconfig
file.

Thoughts?
Anna

>  config NFS_V4_2_SSC_HELPER
>         bool
>         default y if NFS_V4_2
> diff --git a/fs/nfs_common/Makefile b/fs/nfs_common/Makefile
> index e58b01bb8dda..a5e54809701e 100644
> --- a/fs/nfs_common/Makefile
> +++ b/fs/nfs_common/Makefile
> @@ -6,6 +6,9 @@
>  obj-$(CONFIG_NFS_ACL_SUPPORT) += nfs_acl.o
>  nfs_acl-objs := nfsacl.o
>
> +obj-$(CONFIG_NFS_COMMON_LOCALIO_SUPPORT) += nfs_localio.o
> +nfs_localio-objs := nfslocalio.o
> +
>  obj-$(CONFIG_GRACE_PERIOD) += grace.o
>  obj-$(CONFIG_NFS_V4_2_SSC_HELPER) += nfs_ssc.o
>
> diff --git a/fs/nfs_common/nfslocalio.c b/fs/nfs_common/nfslocalio.c
> new file mode 100644
> index 000000000000..22b0ddf225ca
> --- /dev/null
> +++ b/fs/nfs_common/nfslocalio.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024 Mike Snitzer <snitzer@hammerspace.com>
> + * Copyright (C) 2024 NeilBrown <neilb@suse.de>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/rculist.h>
> +#include <linux/nfslocalio.h>
> +#include <net/netns/generic.h>
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("NFS localio protocol bypass support");
> +
> +static DEFINE_SPINLOCK(nfs_uuid_lock);
> +
> +/*
> + * Global list of nfs_uuid_t instances
> + * that is protected by nfs_uuid_lock.
> + */
> +LIST_HEAD(nfs_uuids);
> +
> +void nfs_uuid_begin(nfs_uuid_t *nfs_uuid)
> +{
> +       nfs_uuid->net = NULL;
> +       nfs_uuid->dom = NULL;
> +       uuid_gen(&nfs_uuid->uuid);
> +
> +       spin_lock(&nfs_uuid_lock);
> +       list_add_tail_rcu(&nfs_uuid->list, &nfs_uuids);
> +       spin_unlock(&nfs_uuid_lock);
> +}
> +EXPORT_SYMBOL_GPL(nfs_uuid_begin);
> +
> +void nfs_uuid_end(nfs_uuid_t *nfs_uuid)
> +{
> +       if (nfs_uuid->net == NULL) {
> +               spin_lock(&nfs_uuid_lock);
> +               list_del_init(&nfs_uuid->list);
> +               spin_unlock(&nfs_uuid_lock);
> +       }
> +}
> +EXPORT_SYMBOL_GPL(nfs_uuid_end);
> +
> +static nfs_uuid_t * nfs_uuid_lookup_locked(const uuid_t *uuid)
> +{
> +       nfs_uuid_t *nfs_uuid;
> +
> +       list_for_each_entry(nfs_uuid, &nfs_uuids, list)
> +               if (uuid_equal(&nfs_uuid->uuid, uuid))
> +                       return nfs_uuid;
> +
> +       return NULL;
> +}
> +
> +struct module *nfsd_mod;
> +
> +void nfs_uuid_is_local(const uuid_t *uuid, struct list_head *list,
> +                      struct net *net, struct auth_domain *dom,
> +                      struct module *mod)
> +{
> +       nfs_uuid_t *nfs_uuid;
> +
> +       spin_lock(&nfs_uuid_lock);
> +       nfs_uuid = nfs_uuid_lookup_locked(uuid);
> +       if (nfs_uuid) {
> +               kref_get(&dom->ref);
> +               nfs_uuid->dom = dom;
> +               /*
> +                * We don't hold a ref on the net, but instead put
> +                * ourselves on a list so the net pointer can be
> +                * invalidated.
> +                */
> +               list_move(&nfs_uuid->list, list);
> +               nfs_uuid->net = net;
> +
> +               __module_get(mod);
> +               nfsd_mod = mod;
> +       }
> +       spin_unlock(&nfs_uuid_lock);
> +}
> +EXPORT_SYMBOL_GPL(nfs_uuid_is_local);
> +
> +static void nfs_uuid_put_locked(nfs_uuid_t *nfs_uuid)
> +{
> +       if (nfs_uuid->net) {
> +               module_put(nfsd_mod);
> +               nfs_uuid->net = NULL;
> +       }
> +       if (nfs_uuid->dom) {
> +               auth_domain_put(nfs_uuid->dom);
> +               nfs_uuid->dom = NULL;
> +       }
> +       list_del_init(&nfs_uuid->list);
> +}
> +
> +void nfs_uuid_invalidate_clients(struct list_head *list)
> +{
> +       nfs_uuid_t *nfs_uuid, *tmp;
> +
> +       spin_lock(&nfs_uuid_lock);
> +       list_for_each_entry_safe(nfs_uuid, tmp, list, list)
> +               nfs_uuid_put_locked(nfs_uuid);
> +       spin_unlock(&nfs_uuid_lock);
> +}
> +EXPORT_SYMBOL_GPL(nfs_uuid_invalidate_clients);
> +
> +void nfs_uuid_invalidate_one_client(nfs_uuid_t *nfs_uuid)
> +{
> +       if (nfs_uuid->net) {
> +               spin_lock(&nfs_uuid_lock);
> +               nfs_uuid_put_locked(nfs_uuid);
> +               spin_unlock(&nfs_uuid_lock);
> +       }
> +}
> +EXPORT_SYMBOL_GPL(nfs_uuid_invalidate_one_client);
> diff --git a/include/linux/nfslocalio.h b/include/linux/nfslocalio.h
> new file mode 100644
> index 000000000000..4165ff8390c1
> --- /dev/null
> +++ b/include/linux/nfslocalio.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2024 Mike Snitzer <snitzer@hammerspace.com>
> + * Copyright (C) 2024 NeilBrown <neilb@suse.de>
> + */
> +#ifndef __LINUX_NFSLOCALIO_H
> +#define __LINUX_NFSLOCALIO_H
> +
> +#include <linux/module.h>
> +#include <linux/list.h>
> +#include <linux/uuid.h>
> +#include <linux/sunrpc/svcauth.h>
> +#include <linux/nfs.h>
> +#include <net/net_namespace.h>
> +
> +/*
> + * Useful to allow a client to negotiate if localio
> + * possible with its server.
> + *
> + * See Documentation/filesystems/nfs/localio.rst for more detail.
> + */
> +typedef struct {
> +       uuid_t uuid;
> +       struct list_head list;
> +       struct net *net; /* nfsd's network namespace */
> +       struct auth_domain *dom; /* auth_domain for localio */
> +} nfs_uuid_t;
> +
> +void nfs_uuid_begin(nfs_uuid_t *);
> +void nfs_uuid_end(nfs_uuid_t *);
> +void nfs_uuid_is_local(const uuid_t *, struct list_head *,
> +                      struct net *, struct auth_domain *, struct module *);
> +void nfs_uuid_invalidate_clients(struct list_head *list);
> +void nfs_uuid_invalidate_one_client(nfs_uuid_t *nfs_uuid);
> +
> +#endif  /* __LINUX_NFSLOCALIO_H */
> --
> 2.44.0
>