Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Nagendra Tomar <Nagendra.Tomar@microsoft.com>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>
Subject: Re: [PATCH 0/5] nfs: Add mount option for forcing RPC requests for
 one file over one connection
Thread-Topic: [PATCH 0/5] nfs: Add mount option for forcing RPC requests for
 one file over one connection
Thread-Index: Adcfp3hcmhRvn3IHQv25MQlIHJPMUAARGOoAAAMHR+AAAek9AA==
Date:   Tue, 23 Mar 2021 16:14:37 +0000
Message-ID: <5B030422-09B7-470D-9C7A-18C666F5817D@oracle.com>
References: <SG2P153MB0361E2D69F8AD8AFBF9E7CD89E649@SG2P153MB0361.APCP153.PROD.OUTLOOK.COM>
 <CD47CE3E-0C07-4019-94AF-5474D531B12D@oracle.com>
 <SG2P153MB0361BCD6D1F8F86E0648E3EB9E649@SG2P153MB0361.APCP153.PROD.OUTLOOK.COM>
In-Reply-To: <SG2P153MB0361BCD6D1F8F86E0648E3EB9E649@SG2P153MB0361.APCP153.PROD.OUTLOOK.COM>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <4D9704D763ED454BBFE796B2F1602DA1@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: c0fa8fce-f2d7-4313-1cbd-08d8ee16c181
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Mar 2021 16:14:37.5503
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: yjbCWG/NNxU2LFIPjpibVR3/1xNetgLHW5b0CRQmaGzEbYFqShsu8TejANjcnTfHjhC5dJ7KEHgIDmbXqKKvpw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4386
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9932 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 bulkscore=0 phishscore=0
 mlxlogscore=999 suspectscore=0 spamscore=0 malwarescore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000
 definitions=main-2103230118
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9932 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 lowpriorityscore=0 priorityscore=1501
 impostorscore=0 spamscore=0 mlxscore=0 suspectscore=0 mlxlogscore=999
 phishscore=0 bulkscore=0 adultscore=0 malwarescore=0 clxscore=1015
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000
 definitions=main-2103230118
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Mar 23, 2021, at 11:57 AM, Nagendra Tomar <Nagendra.Tomar@microsoft.co=
m> wrote:
>=20
>>> On Mar 23, 2021, at 1:46 AM, Nagendra Tomar
>> <Nagendra.Tomar@microsoft.com> wrote:
>>>=20
>>> From: Nagendra S Tomar <natomar@microsoft.com>
>>>=20
>>> 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.
>>>=20
>>> 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=3Droundrobin|hash" which can be
>>> used to select the nconnect multipath policy for a given mount and
>>> pass the selected policy to the RPC client.
>>=20
>> This sets off my "not another administrative knob that has
>> to be tested and maintained, and can be abused" allergy.
>>=20
>> Also, my "because connections are shared by mounts of the same
>> server, all those mounts will all adopt this behavior" rhinitis.
>=20
> Yes, it's fair to call this out, but ncpolicy behaves like the nconnect
> parameter in this regards.
>=20
>> And my "why add a new feature to a legacy NFS version" hives.
>>=20
>>=20
>> I agree that your scenario can and should be addressed somehow.
>> I'd really rather see this done with pNFS.
>>=20
>> Since you are proposing patches against the upstream NFS client,
>> I presume all your clients /can/ support NFSv4.1+. It's the NFS
>> servers that are stuck on NFSv3, correct?
>=20
> Yes.
>=20
>>=20
>> The flexfiles layout can handle an NFSv4.1 client and NFSv3 data
>> servers. In fact it was designed for exactly this kind of mix of
>> NFS versions.
>>=20
>> No client code change will be necessary -- there are a lot more
>> clients than servers. The MDS can be made to work smartly in
>> concert with the load balancer, over time; or it can adopt other
>> clever strategies.
>>=20
>> IMHO pNFS is the better long-term strategy here.
>=20
> The fundamental difference here is that the clustered NFSv3 server
> is available over a single virtual IP, so IIUC even if we were to use
> NFSv41 with flexfiles layout, all it can handover to the client is that s=
ingle
> (load-balanced) virtual IP and now when the clients do connect to the
> NFSv3 DS we still have the same issue. Am I understanding you right?
> Can you pls elaborate what you mean by "MDS can be made to work
> smartly in concert with the load balancer"?

I had thought there were multiple NFSv3 server targets in play.

If the load balancer is making them look like a single IP address,
then take it out of the equation: expose all the NFSv3 servers to
the clients and let the MDS direct operations to each data server.

AIUI this is the approach (without the use of NFSv3) taken by
NetApp next generation clusters.


>>> 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.
>>>=20
>>> Thoughts?
>>>=20
>>> Thanks,
>>> Tomar
>>>=20
>>> 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
>>>=20
>>> 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(-)
>>=20
>> --
>> Chuck Lever

--
Chuck Lever