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:     Trond Myklebust <trondmy@hammerspace.com>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Anna Schumaker <anna.schumaker@netapp.com>,
        Olga Kornievskaia <olga.kornievskaia@gmail.com>
Subject: Re: [PATCH v2 2/3] NFSv4 introduce max_connect mount options
Thread-Topic: [PATCH v2 2/3] NFSv4 introduce max_connect mount options
Thread-Index: AQHXXXoN9xSEM+vT7E6aMlyo/90m26sNPoWAgAABLICAAAY6AIAABLWAgAAG8IA=
Date:   Thu, 10 Jun 2021 14:38:22 +0000
Message-ID: <D78ECF52-E70B-46C5-A5E7-BB0BE6ADB059@oracle.com>
References: <20210609215319.5518-1-olga.kornievskaia@gmail.com>
 <20210609215319.5518-3-olga.kornievskaia@gmail.com>
 <6C64456A-931F-4CAD-A559-412A12F0F741@oracle.com>
 <6bca80de292b5aa36734e7d942d0e9f53430903b.camel@hammerspace.com>
 <83D9342E-FDAF-4B2C-A518-20BF0A9AD073@oracle.com>
 <3658c226b43fb190de38c00e5199ccf35ccc4369.camel@hammerspace.com>
In-Reply-To: <3658c226b43fb190de38c00e5199ccf35ccc4369.camel@hammerspace.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?/Pi6UvJy3U0TDL7h3oyHIANQ+fczvtuNcnYGgKQwYIbUr+FM2kOUnm/MrrO8?=
 =?us-ascii?Q?0vqPlgqMdM0W44cuMzzdBjWabJQ0xw6rLIdk/dG5XSXepF5D47gLTYTCbh/V?=
 =?us-ascii?Q?uw59/jlDOK28iAPKu8BRAq0muwARbGhRWQSxRyXATnFvZLRGBgHYEP97Dj6A?=
 =?us-ascii?Q?hFLbu6djeiZp5m26p8A7JFAwHqpmEXzRTx7vBnJbsdDz5t9sFPaWRLyumST9?=
 =?us-ascii?Q?xzOT0cXdyivC0CZogMPrBwAWlRPCCZNMb4PkUGlRiXZjGKWbxrRhxWjrU/70?=
 =?us-ascii?Q?CC/IpDn5tiubor/3cYeZGzXpQAumtaQHi5wTs7b8OdDgfyzKJNPs3qqFDVJt?=
 =?us-ascii?Q?q4RVDBYiG5g09Wox8V/R3Kl3o6O39PDbba7n4WMrQp1PNUB5dCk+SsXa1+PJ?=
 =?us-ascii?Q?vDzY6yjQBBxvml9P/nj+j6ORfzhKDQQH/WS3qvXSqWx+mvbhmp6Va/k1fNth?=
 =?us-ascii?Q?haA72Iso+YB9XqOZ+L7ZDTaKGRbu2aTE6E7/949fnENFhBribnABfHKyy4HV?=
 =?us-ascii?Q?OM5zY1PZ5JlpxaX3xs2U45v2AOF8dkyW1xZQEDY7XbjHeAZrlUg1s/W/qU+T?=
 =?us-ascii?Q?7k70oNOAvnh8QbD3SSRIhhBT3Iq3+CcIAXsKWi6qrAywZtkqw4W/Av3Pbqu2?=
 =?us-ascii?Q?xg8L5sqWjpLhcq0bGg7hveHtjdjcSRnejsPK+lqkkRlgLaG2f6EMCoj23mio?=
 =?us-ascii?Q?Z+xPtqVifcWgft0cpxZIU8YVfOePF7UarBXb6M5ZosGZzINlrBiawJnc0emJ?=
 =?us-ascii?Q?Pw5uDUZIux2ID912xJeaJ8VVdGGanKT6b1pdibbzaNN0hBcgybqQqszCS5Fa?=
 =?us-ascii?Q?cgvN/DFAHOcOQTHUvv65E6RTrMj/IMoG9X37isTyClwqVz9+KFsIdIjf9GaD?=
 =?us-ascii?Q?icrBsE636ik8efjnOiHEsfI0XQP0PnsYcZcfFPXxKrNZBE0mg3nrgPVQ+Ssm?=
 =?us-ascii?Q?vnqQpLfJpgNMKJt/bLMi/ftqRgfrciQDTANpMW+jI7MrqACxAQmnCTV7kc/5?=
 =?us-ascii?Q?71ds6VhjejW8mQaN37dTOnY06SvcEfASS8hYeuuKP3S9baVB0blGjso8AcZO?=
 =?us-ascii?Q?hKmPZ9EjifovCItfAficOXIuSooqX/6D50jQYU6VkdJ3wLOp5hdm41HmWe9y?=
 =?us-ascii?Q?L24izsZjFfKzDaIJ1nN0k4mNS5MonLf36bxI4m1Y3C/X43j8xSQl2DoYE16r?=
 =?us-ascii?Q?rdKZcYegIRGAIE5yrkTxTC7GyUiuKBHkWUKHOFozOK50hHcD6l0g5WPTPWem?=
 =?us-ascii?Q?cGI5dLsG5taPpWG452ldhWctecvLa9LB99Ayhia9qHcbqiwSXFJ9TDy5Q7rw?=
 =?us-ascii?Q?ohFjIPXQYAh555snFeMA16jp?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-ID: <0E967D795CF09045BBAA63B1FD04CEC9@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4673.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 057cab08-3ba7-4eb9-a8ce-08d92c1d65d2
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Jun 2021 14:38:22.2449
 (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: LZEk16dA/T9pdjPdBSqsVp6ATfBimutwh0GpSdvLGlgOdBzwDDEYUa5gM1cJ8V/VRTTDDgH9/p+bi/iPJ/1Yaw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR1001MB2141
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10011 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 mlxscore=0
 spamscore=0 adultscore=0 mlxlogscore=999 suspectscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104190000
 definitions=main-2106100094
X-Proofpoint-ORIG-GUID: z_-0Z5TYCZxMu6KlIvR9uMVQw23NjSjd
X-Proofpoint-GUID: z_-0Z5TYCZxMu6KlIvR9uMVQw23NjSjd
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jun 10, 2021, at 10:13 AM, Trond Myklebust <trondmy@hammerspace.com> w=
rote:
>=20
> On Thu, 2021-06-10 at 13:56 +0000, Chuck Lever III wrote:
>>=20
>>=20
>>> On Jun 10, 2021, at 9:34 AM, Trond Myklebust <
>>> trondmy@hammerspace.com> wrote:
>>>=20
>>> On Thu, 2021-06-10 at 13:30 +0000, Chuck Lever III wrote:
>>>>=20
>>>>=20
>>>>> On Jun 9, 2021, at 5:53 PM, Olga Kornievskaia <=20
>>>>> olga.kornievskaia@gmail.com> wrote:
>>>>>=20
>>>>> From: Olga Kornievskaia <kolga@netapp.com>
>>>>>=20
>>>>> This option will control up to how many xprts can the client
>>>>> establish to the server. This patch parses the value and sets
>>>>> up structures that keep track of max_connect.
>>>>>=20
>>>>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>>>>> ---
>>>>> fs/nfs/client.c           |  1 +
>>>>> fs/nfs/fs_context.c       |  8 ++++++++
>>>>> fs/nfs/internal.h         |  2 ++
>>>>> fs/nfs/nfs4client.c       | 12 ++++++++++--
>>>>> fs/nfs/super.c            |  2 ++
>>>>> include/linux/nfs_fs_sb.h |  1 +
>>>>> 6 files changed, 24 insertions(+), 2 deletions(-)
>>>>>=20
>>>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>>>>> index 330f65727c45..486dec59972b 100644
>>>>> --- a/fs/nfs/client.c
>>>>> +++ b/fs/nfs/client.c
>>>>> @@ -179,6 +179,7 @@ struct nfs_client *nfs_alloc_client(const
>>>>> struct nfs_client_initdata *cl_init)
>>>>>=20
>>>>>         clp->cl_proto =3D cl_init->proto;
>>>>>         clp->cl_nconnect =3D cl_init->nconnect;
>>>>> +       clp->cl_max_connect =3D cl_init->max_connect ? cl_init-
>>>>>> max_connect : 1;
>>>>=20
>>>> So, 1 is the default setting, meaning the "add another transport"
>>>> facility is disabled by default. Would it be less surprising for
>>>> an admin to allow some extra connections by default?
>>>>=20
>>>>=20
>>>>>         clp->cl_net =3D get_net(cl_init->net);
>>>>>=20
>>>>>         clp->cl_principal =3D "*";
>>>>> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
>>>>> index d95c9a39bc70..cfbff7098f8e 100644
>>>>> --- a/fs/nfs/fs_context.c
>>>>> +++ b/fs/nfs/fs_context.c
>>>>> @@ -29,6 +29,7 @@
>>>>> #endif
>>>>>=20
>>>>> #define NFS_MAX_CONNECTIONS 16
>>>>> +#define NFS_MAX_TRANSPORTS 128
>>>>=20
>>>> This maximum seems excessive... again, there are diminishing
>>>> returns to adding more connections to the same server. what's
>>>> wrong with re-using NFS_MAX_CONNECTIONS for the maximum?
>>>>=20
>>>> As always, I'm a little queasy about adding yet another mount
>>>> option. Are there real use cases where a whole-client setting
>>>> (like a sysfs attribute) would be inadequate? Is there a way
>>>> the client could figure out a reasonable maximum without a
>>>> human intervention, say, by counting the number of NICs on
>>>> the system?
>>>=20
>>> Oh, hell no! We're not tying anything to the number of NICs...
>>=20
>> That's a bit of an over-reaction. :-) A little more explanation
>> would be welcome. I mean, don't you expect someone to ask "How
>> do I pick a good value?" and someone might reasonably answer
>> "Well, start with the number of NICs on your client times 3" or
>> something like that.
>>=20
>> IMO we're about to add another admin setting without understanding
>> how it will be used, how to select a good maximum value, or even
>> whether this maximum needs to be adjustable. In a previous e-mail
>> Olga has already demonstrated that it will be difficult to explain
>> how to use this setting with nconnect=3D.
>>=20
>> Thus I would favor a (moderate) soldered-in maximum to start with,
>> and then as real world use cases arise, consider adding a tuning
>> mechanism based on actual requirements.
>=20
> It's not an overreaction.

The "Oh, hell no!" was an overreaction. But thank you for providing
the additional explanation, that helped me understand your position.
I agree that the number of local NICs is frequently unrelated to
the topology of the whole network.


> It's insane to think that counting NICs gives
> you any notion whatsoever about the network topology and connectivity
> between the client and server. It doesn't even tell you how many of
> those NICs might potentially be available to your application.
>=20
> We're not doing any automation based on that kind of layering
> violation.

Fair enough.


--
Chuck Lever