Re: [PATCH 0/5] nfs: Add mount option for forcing RPC requests for one file over one connection

[Tom Talpey <tom@talpey.com>]

All the patches in this series have the same subject/title. They
really should have more context so they stand alone and can be
reviewed separately.

High level question below.

On 3/23/2021 1:46 AM, Nagendra Tomar wrote:
> From: Nagendra S Tomar <natomar@microsoft.com>
> 
> If a clustered NFS server is behind an L4 loadbalancer the default
> nconnect roundrobin policy may cause RPC requests to a file to be
> sent to different cluster nodes. This is because the source port
> would be different for all the nconnect connections.
> While this should functionally work (since the cluster will usually
> have a consistent view irrespective of which node is serving the
> request), it may not be desirable from performance pov. As an
> example we have an NFSv3 frontend to our Object store, where every
> NFSv3 file is an object. Now if writes to the same file are sent
> roundrobin to different cluster nodes, the writes become very
> inefficient due to the consistency requirement for object update
> being done from different nodes.
> Similarly each node may maintain some kind of cache to serve the file
> data/metadata requests faster and even in that case it helps to have
> a xprt affinity for a file/dir.
> In general we have seen such scheme to scale very well.
> 
> This patch introduces a new rpc_xprt_iter_ops for using an additional
> u32 (filehandle hash) to affine RPCs to the same file to one xprt.
> It adds a new mount option "ncpolicy=roundrobin|hash" which can be
> used to select the nconnect multipath policy for a given mount and
> pass the selected policy to the RPC client.

What's the reason for exposing these as a mount option, with multiple
values? What makes one value better than the other, and why is there
not a default?

Tom.

> It adds a new rpc_procinfo member p_fhhash, which can be supplied
> by the specific RPC programs to return a u32 hash of the file/dir the
> RPC is targetting, and lastly it provides p_fhhash implementation
> for various NFS v3/v4/v41/v42 RPCs to generate the hash correctly.
> 
> Thoughts?
> 
> Thanks,
> Tomar
> 
> Nagendra S Tomar (5):
>    SUNRPC: Add a new multipath xprt policy for xprt selection based
>      on target filehandle hash
>    SUNRPC/NFSv3/NFSv4: Introduce "enum ncpolicy" to represent the nconnect
>      policy and pass it down from mount option to rpc layer
>    SUNRPC/NFSv4: Rename RPC_TASK_NO_ROUND_ROBIN -> RPC_TASK_USE_MAIN_XPRT
>    NFSv3: Add hash computation methods for NFSv3 RPCs
>    NFSv4: Add hash computation methods for NFSv4/NFSv42 RPCs
> 
>   fs/nfs/client.c                      |   3 +
>   fs/nfs/fs_context.c                  |  26 ++
>   fs/nfs/internal.h                    |   2 +
>   fs/nfs/nfs3client.c                  |   4 +-
>   fs/nfs/nfs3xdr.c                     | 154 +++++++++++
>   fs/nfs/nfs42xdr.c                    | 112 ++++++++
>   fs/nfs/nfs4client.c                  |  14 +-
>   fs/nfs/nfs4proc.c                    |  18 +-
>   fs/nfs/nfs4xdr.c                     | 516 ++++++++++++++++++++++++++++++-----
>   fs/nfs/super.c                       |   7 +-
>   include/linux/nfs_fs_sb.h            |   1 +
>   include/linux/sunrpc/clnt.h          |  15 +
>   include/linux/sunrpc/sched.h         |   2 +-
>   include/linux/sunrpc/xprtmultipath.h |   9 +-
>   include/trace/events/sunrpc.h        |   4 +-
>   net/sunrpc/clnt.c                    |  38 ++-
>   net/sunrpc/xprtmultipath.c           |  91 +++++-
>   17 files changed, 913 insertions(+), 103 deletions(-)
>