Re: [PATCH v13 00/19] nfs/nfsd: add support for localio

[Mike Snitzer <snitzer@kernel.org>]

On Sun, Aug 25, 2024 at 11:46:10AM -0400, Chuck Lever wrote:
> On Fri, Aug 23, 2024 at 02:13:58PM -0400, Mike Snitzer wrote:
> > These latest changes are available in my git tree here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=nfs-localio-for-next
> > 
> > Changes since v12:
> > - Rebased to rearrange code to avoid "churn" that Jeff Layton felt
> >   distracting (biggest improvement came from folding switch from
> >   struct file to nfsd_file changes in from the start of the series)
> > - Updated relevant patch headers accordingly.
> > - Updated localio.rst to provide more performance data.
> > - Dropped v12's buggy "nfsd: fix nfsfh tracepoints to properly handle
> >   NULL rqstp" patch -- fixing localio to support fh_verify tracepoints
> >   will need more think-time and discussion, but they aren't of
> >   critical importance so fixing them doesn't need to hold things up.
> > 
> > Please also see v12's patch header for v12's extensive changes:
> > https://marc.info/?l=linux-nfs&m=172409139907981&w=2
> > 
> > Thanks,
> > Mike
> > 
> > Mike Snitzer (9):
> >   nfs_common: factor out nfs_errtbl and nfs_stat_to_errno
> >   nfs_common: factor out nfs4_errtbl and nfs4_stat_to_errno
> >   nfs: factor out {encode,decode}_opaque_fixed to nfs_xdr.h
> >   SUNRPC: remove call_allocate() BUG_ONs
> >   nfs_common: add NFS LOCALIO auxiliary protocol enablement
> >   nfsd: implement server support for NFS_LOCALIO_PROGRAM
> >   nfs: pass struct nfsd_file to nfs_init_pgio and nfs_init_commit
> >   nfs: implement client support for NFS_LOCALIO_PROGRAM
> >   nfs: add Documentation/filesystems/nfs/localio.rst
> > 
> > NeilBrown (3):
> >   nfsd: factor out __fh_verify to allow NULL rqstp to be passed
> >   nfsd: add nfsd_file_acquire_local()
> >   SUNRPC: replace program list with program array
> > 
> > Trond Myklebust (4):
> >   nfs: enable localio for non-pNFS IO
> >   pnfs/flexfiles: enable localio support
> >   nfs/localio: use dedicated workqueues for filesystem read and write
> >   nfs: add FAQ section to Documentation/filesystems/nfs/localio.rst
> > 
> > Weston Andros Adamson (3):
> >   SUNRPC: add rpcauth_map_clnt_to_svc_cred_local
> >   nfsd: add localio support
> >   nfs: add localio support
> > 
> >  Documentation/filesystems/nfs/localio.rst | 276 ++++++++
> >  fs/Kconfig                                |   3 +
> >  fs/nfs/Kconfig                            |  15 +
> >  fs/nfs/Makefile                           |   1 +
> >  fs/nfs/client.c                           |  15 +-
> >  fs/nfs/filelayout/filelayout.c            |   6 +-
> >  fs/nfs/flexfilelayout/flexfilelayout.c    | 142 +++-
> >  fs/nfs/flexfilelayout/flexfilelayout.h    |   2 +
> >  fs/nfs/flexfilelayout/flexfilelayoutdev.c |   6 +
> >  fs/nfs/inode.c                            |  57 +-
> >  fs/nfs/internal.h                         |  61 +-
> >  fs/nfs/localio.c                          | 784 ++++++++++++++++++++++
> >  fs/nfs/nfs2xdr.c                          |  70 +-
> >  fs/nfs/nfs3xdr.c                          | 108 +--
> >  fs/nfs/nfs4xdr.c                          |  84 +--
> >  fs/nfs/nfstrace.h                         |  61 ++
> >  fs/nfs/pagelist.c                         |  16 +-
> >  fs/nfs/pnfs_nfs.c                         |   2 +-
> >  fs/nfs/write.c                            |  12 +-
> >  fs/nfs_common/Makefile                    |   5 +
> >  fs/nfs_common/common.c                    | 134 ++++
> >  fs/nfs_common/nfslocalio.c                | 194 ++++++
> >  fs/nfsd/Kconfig                           |  15 +
> >  fs/nfsd/Makefile                          |   1 +
> >  fs/nfsd/export.c                          |   8 +-
> >  fs/nfsd/filecache.c                       |  90 ++-
> >  fs/nfsd/filecache.h                       |   5 +
> >  fs/nfsd/localio.c                         | 183 +++++
> >  fs/nfsd/netns.h                           |   8 +-
> >  fs/nfsd/nfsctl.c                          |   2 +-
> >  fs/nfsd/nfsd.h                            |   6 +-
> >  fs/nfsd/nfsfh.c                           | 122 ++--
> >  fs/nfsd/nfsfh.h                           |   5 +
> >  fs/nfsd/nfssvc.c                          | 100 ++-
> >  fs/nfsd/trace.h                           |   3 +-
> >  fs/nfsd/vfs.h                             |  10 +
> >  include/linux/nfs.h                       |   9 +
> >  include/linux/nfs_common.h                |  17 +
> >  include/linux/nfs_fs_sb.h                 |  10 +
> >  include/linux/nfs_xdr.h                   |  20 +-
> >  include/linux/nfslocalio.h                |  56 ++
> >  include/linux/sunrpc/auth.h               |   4 +
> >  include/linux/sunrpc/svc.h                |   7 +-
> >  net/sunrpc/auth.c                         |  22 +
> >  net/sunrpc/clnt.c                         |   6 -
> >  net/sunrpc/svc.c                          |  68 +-
> >  net/sunrpc/svc_xprt.c                     |   2 +-
> >  net/sunrpc/svcauth_unix.c                 |   3 +-
> >  48 files changed, 2434 insertions(+), 402 deletions(-)
> >  create mode 100644 Documentation/filesystems/nfs/localio.rst
> >  create mode 100644 fs/nfs/localio.c
> >  create mode 100644 fs/nfs_common/common.c
> >  create mode 100644 fs/nfs_common/nfslocalio.c
> >  create mode 100644 fs/nfsd/localio.c
> >  create mode 100644 include/linux/nfs_common.h
> >  create mode 100644 include/linux/nfslocalio.h
> > 
> > -- 
> > 2.44.0
> > 
> 
> I've looked over the NFSD changes in this patch set. Some of them
> look ready to go (I've Acked those), others still need a little TLC.
> 
> For me, the big picture questions that remain are:
> 
> * How do we ensure that export options that modify RPC user
>   translation (eg, all_squash) continue to work as expected on
>   LOCALIO mounts?
> 
> * How do we ensure that a "stolen" FH can't be exploited by an NFS
>   client running in a local container that does not have access to
>   an export (server should return NFSERR_STALE)?
> 
> * Has LOCALIO been tested with two or more NFS servers running on
>   the local system? One assumes they would be in separate
>   containers.
> 
> The scenario I'm thinking of is two tenants, Pepsi and Coke, running
> in containers on the same physical host. They might want their own
> separate NFS servers, or they might share one server with
> appropriate access controls on the exports. Both of these scenarios
> need to ensure that Pepsi and Coke cannot open each other's files
> via LOCALIO.

Each server instance is isolated in their respective nfsd_net.  And
localio was made to build on that isolation.  So all creds used, etc
are specific to each nfsd instance.

I haven't verified these scenarios "just work" as expected but localio
preserves all the same checking and upcalls of standard NFS uses.

Mike