Received-SPF: pass (google.com: domain of linux-nfs+bounces-2966-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <86dff5d9-7053-4b4b-a1f5-ebcb3380e3d0@oracle.com>
Date: Tue, 23 Apr 2024 11:02:27 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] nfsd: don't fail OP_SETCLIENTID when there are lots of
 clients.
To: Chuck Lever III <chuck.lever@oracle.com>, Neil Brown <neilb@suse.de>
Cc: Jeff Layton <jlayton@kernel.org>, Olga Kornievskaia <kolga@netapp.com>,
        Tom Talpey <tom@talpey.com>, Petr Vorel <pvorel@suse.cz>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
References: <171382882695.7600.8486072164212452863@noble.neil.brown.name>
 <62B41B1D-0A9C-44B5-8EC3-962AC862EFB7@oracle.com>
Content-Language: en-US
From: Dai Ngo <dai.ngo@oracle.com>
In-Reply-To: <62B41B1D-0A9C-44B5-8EC3-962AC862EFB7@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?TFQ1T2FlUmN3UWc5OGF2aXNmS3BybUZwaU9GN2hMMlN5N3BDZ1g4cXhsZ3R2?=
 =?utf-8?B?cEFoSnFLQ25qbXhVV2RrVnA1eXJwWmhtKzJkSklyb3JQZjFiNzZvVXcwRkZT?=
 =?utf-8?B?R3dsR1BPVm02am83RjZQemwweW1DZEFDZDFLSnQzVHZUVE9YMUJmTVlQNWd0?=
 =?utf-8?B?dVFQdzd1MFltWXBrWUtkNUUyUnhIMExQT3ZMWENvY0RYMEtEZVR3WFpSR0dG?=
 =?utf-8?B?TDNHVmdyc09WZnlVZTR1czg1NmRad0xGVFN3bVBHMU9UdldIUmFGV3cyVU9Z?=
 =?utf-8?B?TjZwQVhNbGhzbWIyREVucFhtaDBXdHQxeVo5SVpRckxHZ1FBR3U0enZxZC9z?=
 =?utf-8?B?THpGTkt5eFJFQUtiTFQxQklhWGt6N1QxUStIQ2ZlQmR0UGY0MnBFV3YxTnd5?=
 =?utf-8?B?Y29zZ0NTTGViMVk5Sy9pZGMvRFdTUjBMdkRsSXFoeE80M3ZsY3ozM1hpMWZ2?=
 =?utf-8?B?bGNBT3pKVGZnS1R5L3ZDU20yN2Npd2hDcG0rcmlkQzFuNlMwclNDZ0pXbURW?=
 =?utf-8?B?NVB1NTRTcEpGOWJrSXRoNGVJNm1IRy90cWpucFBqS3NsU1RkWUdGbS9LM21o?=
 =?utf-8?B?UlhHMzR4blpYV2pYeEZ4dFhnQXp6bnRFSm5rQ1Q0WHVOMkVKL2dXV3dENDU1?=
 =?utf-8?B?NUVUMnZGdTJNVjcwdmdZUm9aM2d5TW1xOE1zWGJ3dUJ2TFhMTzBMVzFxNW5u?=
 =?utf-8?B?VkFFR0FyMGQxMFNzMk5QenZDb3ZpRXd0b0NsUlBPNTBBaFNEQWt5bXR5bkdM?=
 =?utf-8?B?ZG4zaU1CWjFMWGsvdHdMMDVXbjdiZmg0T3ZjbjFHc1IwMVVDb1hiTFN3UjNW?=
 =?utf-8?B?ZHdNWC9yZUc0YVp4ZXRySXlVWEw2ZnNQRmVRV2ZOb3ZvenQxREhBRE91N2V2?=
 =?utf-8?B?WnVlZTMwWi9JS1J1UUNyQlZwcHJ6eWd1eFk3Q29xb0c0UFl0aXJPS3dTb2FV?=
 =?utf-8?B?QWxRanEzS2FjM2dTbC94ZG5LRURvZlRVZ0JEUkdRM1AxRjZ4dlp1bXBiSUNI?=
 =?utf-8?B?dGF0OXJtN3pNR3R6S250OTBLSm9IT3JiYW1EejZYVVFveU5VaGhHYnVZZG1I?=
 =?utf-8?B?aURyNy83emFGR0tKWEU2VmFwVmNtNEhvZ1NXTjg1b0pXcTBVMHN1R0xRVUU1?=
 =?utf-8?B?cFBwbVY1V2hzU2NOZ0w1M3pGdWdTbHpkRnFHNnNVTmwvb3o5dHBVKyszQnZv?=
 =?utf-8?B?NjNGekNHNzV3VCthVUVTV3QySUx1OFhLWkJCVk9FenpuaCtFaEUxcnYrQkh6?=
 =?utf-8?B?K0djeGNYK250d2ovU3UzNXpXSE1Sb3FKdzNrbHA1eTQxZ3RUUXVuSnd5SDQ3?=
 =?utf-8?B?bW9HWG00V0tXVVZiQ2EvNi9tQ1lrNEdIQ20yTXdscGhkTFNaZXEwZmhzUHZR?=
 =?utf-8?B?WDBqNE92a3ZWUm5pNGtHL2ZqL3cvdTYzb21TOWNCbFVTSVhzakdVREc0Tmli?=
 =?utf-8?B?aEdFOWtSR083RTA5Z1lFOHhJd0F2aitPQ3pKd0FDazVWeHVvQ1lHQ29ON2p1?=
 =?utf-8?B?RmxPc0tWREVHdFhMNWNNcGo0VzhOcjZjeHB1dGltMGZxUEFRaU1DYm1ZZjc0?=
 =?utf-8?B?WTJFMzNmN3NoQmtYdGUyUEJHZmorWWZlY0NHZzdNM2hTRm5uUnBFTTFvOWhG?=
 =?utf-8?B?QVozTWdaR2NsbXYrcm8rVThwMUtQS2s3QkN0ZExCU3FCT0pUMHN4eGY3b1Fh?=
 =?utf-8?B?Q1I2eTBabytRQytCY1Q0MnFsV1RtQy9nRGhNRlQyQThsYzBWWkd2U2JtWS9z?=
 =?utf-8?B?YS80VkpRWlVoSlRKdStsZzYwQUVyV1d1TzY3RUh6T3hyQzdBNFVuVnAvVVlM?=
 =?utf-8?B?UllkSWVheDVKYnQyZ2h5VkVtVlhndTFkQW0reEhJejFxSVdMOHVJZ0U3Ym9o?=
 =?utf-8?B?Vitkd3llenVwNFZTRWJBZ2ljT2hONmhWWjZLTG9scGpzMSt5bER1cElUL1Jo?=
 =?utf-8?B?T2dWQmEyVVhCUWhOVURlMlBrRVBwZ1N6ZzlQVE00a1R5R3ZmaXF4bEx1d3hm?=
 =?utf-8?B?RVZ1bHI2UUNDa01DM1B6RXUxU3Vzb0NwNDBaWEZFRWlHRiszaHFkMVJGSW1S?=
 =?utf-8?B?Z1FsaGcrdTBQK2NQWEd1MnAxOU00M3pmcFQwNUxGenNPRHMwQXNmdjgvVGJY?=
 =?utf-8?Q?e1xPpI+cwAQxNz9awf16oSN+I?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	Gt1NQ6JgcL03uTcu7G+nSxc1UY1a/+6pkzUkFqBiLZFAGjBf3IBxJj5sc6UhX4Sy0RRMCP18vBp31KYSDqddo/lIIUHeZWTJfVI++soGJc+/Qz1wyIXS9TMG9mh/tXsiPXPxyUPzedp+NKboI37h4dEklGSG1db9JJ/MD4luZ2QvHy+OxgFUpTMTtWzqLYImlKWTWgdDV5Itg1GYnh2KugCBLLEje+sh9GA0hl+y+pNTTvbZY+48RQh5z6T0oZekbveggaOs19pHuWN2wW3YZIRI0H1cUVOL/r56GRXB6e55fdQkjFCQpVRKX03ho4JCtzNsHMOYQsERJ+7O59ZiMoOtLoGGUSYBKd5tflwrNodKeNsTVLhColPdh35kGuEHdxDdpEDYN/u0j2vruNENeJ7uUDKWSgh26hQYoQdTqSs/ZpdWc2xwLjb07yoS9HqVPTEHkupXjRfjMb76bqYPwprcFnwyejnxlGNj3uOvZU+DV2xAJgTFlu8BMFRKf99hk0v+ISbG/l3a58Lq5V2bR/FeO14dYjSDN0zzdW3C0wH0QQsiorri6s6R1dSsjVrIOjWqoX0Fd+KkFPPB2rZ8OWxL+D7TEe+h+SheGi5IlTU=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 17c9d99d-72ca-40a1-3a16-08dc63bf8aca
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4257.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Apr 2024 18:02:29.9538
 (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: cVNV2YTzXrTir+sW7wUhUai4YhYHHSruCqv3p+QhNwqNasx6Wj2bHEdYGIAExU+A1ijYvonBB6qlV1yr+epMqQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR10MB4917
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-04-23_15,2024-04-23_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxlogscore=999
 bulkscore=0 mlxscore=0 suspectscore=0 adultscore=0 malwarescore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2404010000 definitions=main-2404230041
X-Proofpoint-GUID: t5uAsX-yPWVriZOFJkee8xVKmkI6OG-1
X-Proofpoint-ORIG-GUID: t5uAsX-yPWVriZOFJkee8xVKmkI6OG-1


On 4/23/24 6:15 AM, Chuck Lever III wrote:
>> On Apr 22, 2024, at 7:34 PM, NeilBrown <neilb@suse.de> wrote:
>>
>> ﻿On Mon, 22 Apr 2024, Chuck Lever wrote:
>>>> On Mon, Apr 22, 2024 at 12:09:19PM +1000, NeilBrown wrote:
>>>> The calculation of how many clients the nfs server can manage is only an
>>>> heuristic.  Triggering the laundromat to clean up old clients when we
>>>> have more than the heuristic limit is valid, but refusing to create new
>>>> clients is not.  Client creation should only fail if there really isn't
>>>> enough memory available.
>>>>
>>>> This is not known to have caused a problem is production use, but
>>>> testing of lots of clients reports an error and it is not clear that
>>>> this error is justified.
>>> It is justified, see 4271c2c08875 ("NFSD: limit the number of v4
>>> clients to 1024 per 1GB of system memory"). In cases like these,
>>> the recourse is to add more memory to the test system.
>> Does each client really need 1MB?
>> Obviously we don't want all memory to be used by client state....
>>
>>> However, that commit claims that the client is told to retry; I
>>> don't expect client creation to fail outright. Can you describe the
>>> failure mode you see?
>> The failure mode is repeated client retries that never succeed.  I think
>> an outright failure would be preferable - it would be more clear than
>> memory must be added.
>>
>> The server has N active clients and M courtesy clients.
>> Triggering reclaim when N+M exceeds a limit and M>0 makes sense.
>> A hard failure (NFS4ERR_RESOURCE) when N exceeds a limit makes sense.
>> A soft failure (NFS4ERR_DELAY) while reclaim is running makes sense.
>>
>> I don't think a retry while N exceeds the limit makes sense.
> It’s not optimal, I agree.
>
> NFSD has to limit the total number of active and courtesy
> clients, because otherwise it would be subject to an easy
> (d)DoS attack, which Dai demonstrated to me before I
> accepted his patch. A malicious actor or broken clients
> can continue to create leases on the server until it stops
> responding.
>
> I think failing outright would accomplish the mitigation
> as well as delaying does, but delaying once or twice
> gives some slack that allows a mount attempt to succeed
> eventually even when the server temporarily exceeds the
> maximum client count.
>
> Also IMO there could be a rate-limited pr_warn on the
> server that fires to indicate when a low-memory situation
> has been reached.
>
> The problem with NFS4ERR_RESOURCE, however, is that
> NFSv4.1 and newer do not have that status code. All
> versions of NFS have DELAY/JUKEBOX.
>
> I recognize that you are tweaking only SETCLIENTID here,
> but I think behavior should be consistent for all minor
> versions of NFSv4.
>
>
>> Do we have a count of active vs courtesy clients?
> Dai can correct me if I’m wrong, but I believe NFSD
> maintains a count of both.

NFSD maintains both counts for active clients, nfs4_client_count,
and courtesy clients, nfsd_courtesy_clients. However the 'real'
active client count is 'nfs4_client_count - nfsd_courtesy_clients).
   

>
> But only the active leases really matter, becase
> courtesy clients can be dropped as memory becomes tight.

Yes, when the NFSD shrinker is activated it calls courtesy_client_reaper
to remove courtesy clients.

-Dai

> Dropping an active lease would be somewhat more
> catastrophic.
>
>
> —
> Chuck Lever