Received-SPF: pass (google.com: domain of linux-nfs+bounces-3002-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 25 Apr 2024 09:26:48 -0400
From: Chuck Lever <chuck.lever@oracle.com>
To: NeilBrown <neilb@suse.de>
Cc: Dai Ngo <dai.ngo@oracle.com>, 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>
Subject: Re: [PATCH] nfsd: don't fail OP_SETCLIENTID when there are lots of
 clients.
Message-ID: <ZipaGCI3B0PX7mdm@tissot.1015granger.net>
References: <171382882695.7600.8486072164212452863@noble.neil.brown.name>
 <62B41B1D-0A9C-44B5-8EC3-962AC862EFB7@oracle.com>
 <171400371581.7600.11354407820942719081@noble.neil.brown.name>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <171400371581.7600.11354407820942719081@noble.neil.brown.name>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?d2ZTcEFSbC9MMmpwZjFla1o4YnlPbE9RZm14Wk9CdlAxVW9HUmJrVW9zZjA2?=
 =?utf-8?B?R0lJS25SeTN0L09lNTdhWUhPaHp5MUZRclR0RFlIdW81OUhKSnBlUHFmUmRt?=
 =?utf-8?B?UXRXMTdlODU2QThFWWFWcysrekdMK08yWTZ6UVNnNGJnTlh5ME1naE5HNnNx?=
 =?utf-8?B?bGlvN0Z5ZFVlLzNENlorUHZIaUdxOXBCbE95MmJ0RzM5Tnp4S3kxdXhVanlQ?=
 =?utf-8?B?bXVlOXovckN6YlNuaSsySnFHUlpwelU2bFN2ZFR2Nk52U2ZyVExzS1RRZ0dn?=
 =?utf-8?B?WHVpQ0o5aEZlQVhmczEwaGdBM1RqZ1daeGRlbnM3WXNyTGhESDBUaDVwRWVo?=
 =?utf-8?B?QTNTdXRYMjJmOUtTZDg1TXdDWHZnTEp1OFpCRk53NjFSY1Fid2psZlNTZVp0?=
 =?utf-8?B?cEpBV3lEdWZKQ3BkeitmSTJWWlJYTnlGby9VWFlLOXVRc0x3TFlXNW5GNDg1?=
 =?utf-8?B?RDRhMVlNZ2E4QlVlcGJDKzZtbWNNdnRKMmRKbWYveEMvYjVhL21SczhyTExj?=
 =?utf-8?B?ejkyNXFaNXpqdTlWSHU2OGdZSm1oaWhSMDkzQnlPMW1YZFp0Y01rdEpjUkNs?=
 =?utf-8?B?K1VBaDJ5SkI2WGxDL08wdmtvbjVBY0t0c0p4UUlnd0w3Vm9sUFM4ZnpoZStS?=
 =?utf-8?B?ZnpvZlBGRkRqbjY2b1lzYmtEY29sUzlRMXpXeENWeVZKVHVDME50QWFnREFL?=
 =?utf-8?B?WDVpc0c3WjN0NEkwemdRektsSXFBSXViL0VRZ0trS0tEN3BDcG8veEMycGZp?=
 =?utf-8?B?aVY1OXM3RU9WUXNWREZjR2NkMWVnUDRrMUFxS0NMNTF2N2RqNDh6N1NQblQr?=
 =?utf-8?B?MTRHYkxDUUxxaDNiKzVUSVQ1YVlEelNsQXAwMzdWdEg2TVhxQWNud0doUjVW?=
 =?utf-8?B?R29pZm5oeUVJT2M2OENvYW5qSXpIOWtiVGZ3L1BDU2xvdjF3cU5ocjVyRFRz?=
 =?utf-8?B?OXlLSDRYdkliQ3hQSnY3WXpLR0tXOGplS2p1d0VDeU1nVHQvdXlEOGszakh4?=
 =?utf-8?B?amcvQ0JKd1lkUzl1VXoxNGZTS1JKY1JNMmlJUjRvYWg1MUtqd1BRajVSbCtI?=
 =?utf-8?B?TndHRE9SakxRYkkvemdKdVFTSnRzR3JQRjA1SnlKTU9lY0REQWpEVUJJVHhs?=
 =?utf-8?B?YjNyZU5IbmpaU3RPbFRxZUx0bDhuaW0zSGkvc2tNTUYrUUJjL3AwOUFkK1hl?=
 =?utf-8?B?ZnNJZWlGa1d3K3YyajhwNWVadHlSQ04waUFaZU1BeVREci85NFg2ZGlDbUdj?=
 =?utf-8?B?bHJndTBIdkxIdHZ5cUQ3UGJqMnFFY1dTWTloY0RWN3hpRE5TeHBvM1hoV3I4?=
 =?utf-8?B?dEpkektxRmd2VHpmVVVXRjlKR2hMZlJnRTBybnV4eDhQYkVic2w0UXdheTdM?=
 =?utf-8?B?czROUkRYa3VPNk9sVFU3ejlidkJoeHRBTmQ2Z3dzSkZDWjlrN2lNaXUvZy9B?=
 =?utf-8?B?aDBKWml6Z2RVMlNkN3VGVE81YnE5VGdZa0RNbndqSUhBUHA3SWJtWjNTdDVh?=
 =?utf-8?B?VDVuVENuQzlMaGkvRThpOHVoSzNDUFF6Y1VSdllzNm9YTjRpU2psMGQ4K2NS?=
 =?utf-8?B?ckhuT2J3b3c0ZGJqemQ5TjdWb3NGOGhFbXlNWmtsUzFldTVidU5pZHRhS0xQ?=
 =?utf-8?B?QlQxajZycjE4TkN1STJjc25jMVQ3eEptZnltOHQwSk5UL1hNa0wwYThMaEJq?=
 =?utf-8?B?aitlNmFPZy92dHZ2cWM2My9SMnhDVGhBRUlEc1BUaWNXN043aWJjdlNoa1RQ?=
 =?utf-8?B?RnZSNWFDSCtjVnZIeDk3aTJWNFdqcUpBVUJRR2FuQlkxWG1tOWduQ0tMMURC?=
 =?utf-8?B?bUV3RGhqVi9EWDYzR2wrekpsaElRWWlGaXZKaStFL1U2Q2Z4WDlSUEpXM3J5?=
 =?utf-8?B?eXh2MWlQN0c2Rm1vUEhTTkJrdnl2WVRJVXZyYXRJL29vNk5WeGxJM2d1REor?=
 =?utf-8?B?Z1d4SUs0WFNzdUk4ZDQ0M00vSE4zdjU5L2lUVU9hV2trTnVra1lPZFdMdVZw?=
 =?utf-8?B?SVVpRG9JaCt5Nk1pdEFDQzUrNmQ3azMwZDYvNm1xOHpVSFF1WTNNOEkxSVY5?=
 =?utf-8?B?VWZMcjduK2grMk5lQXJxL0tqejgrOVRrdTdhV01GTHhxYjBEdmZaVDJOaVB0?=
 =?utf-8?B?MUk1QjdmaUhjOTlDK3BWenpVZDBuK3czeWhPU0hXdXljaWtROE5GVlZ2YWo2?=
 =?utf-8?B?NXc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	qTgPRY71at6eexzrC2jEmoM9GkoQNtgbHuK/Y70xkeNidrmXVE6wC14Q3Yug7oWwAaEkWPFnVUzQBDJSOVnbDXbnph/kAFLE/BSlr+4FRPxIWley5yFyq/6xmKfpHqOVFYCh9klYc+PvYlHf/GlQ8vdxh/OmeFbIaD3dv+kkXNmarUTnpbFjCqZLTYzWZRS7J+vVJ7IW1ads1C2x9pbm8l31ag8dPA8PnW9ou2aLmlty5L+72tjkrI2d+eFd/232tGqb9oFyEtgz0N8M05K/98R0qJv+MAbfjg5Zom7GLYfwmi/0JqPDhSzZvlh2g+VlhbCWTVT7mLzorUP3LwFqqX76njGYtQLbW1zh0nfF8QXjVjrDWre6z+xxt/p+Qp0UTmxi2Z3OXyjSRDX40M5VravpuexePlrOxJb47d82Sw9Hz6Yy9qzoDYDeYZY+q8ZqjNFLLWaQmz5Rh1CWqAvLjyNqZzGPV/fbp7bkUbiLDGev3OEHekbE5x/82GbRdKFDjXSxUbD9ES4xe92wrNs/IP99nxv4wi+O7Q6Z8P+RlC85BjtGIZoAg6mhIKUh2R/Ud1cV/SG7vB6wL87TJK9gAJxNbs5iF01lkXI3/7xO5OI=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9a5e99e0-b4e3-4ec1-6861-08dc652b5da5
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Apr 2024 13:26:51.0308
 (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: hunOt+LX3Bcf+y1G9Z+Ivmk/CEvxMnXZwfERZlrsCeOfOcyPJNfc0/DK/kkusiWO+4QwgXyk1tQ3PrHZcMO/lw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB5730
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-25_13,2024-04-25_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxlogscore=999
 mlxscore=0 phishscore=0 spamscore=0 adultscore=0 bulkscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2404010000 definitions=main-2404250097
X-Proofpoint-GUID: ouGf35BckZra6tFPDRogkAqCOXjE9Lox
X-Proofpoint-ORIG-GUID: ouGf35BckZra6tFPDRogkAqCOXjE9Lox

On Thu, Apr 25, 2024 at 10:08:35AM +1000, NeilBrown wrote:
> On Tue, 23 Apr 2024, 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.
> 
> I doubt that the set of active clients is so dynamic that it is worth
> waiting in case some client goes away soon.  If we hit the limit then we
> probably already have more clients than we can reasonably handle and it
> is time to indicate failure.
> 
> > 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.
> 
> Yes, server side warnings would be a good idea.
> 
> > 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 didn't realise that.  Lots of code in nfs4xdr.c returns
> nfserr_resource.  For v4.1 it appears to get translated to
> nfserr_rep_too_big_too_cache or nfserr_rep_too_big, which might not
> always make sense.

Yes. It's confusing, but that's how NFSv4.1 support was grafted
into NFSD's XDR layer.


> > I recognize that you are tweaking only SETCLIENTID here,
> > but I think behavior should be consistent for all minor
> > versions of NFSv4.
> 
> I really want to change EXCHANGEID too.

IIRC, CREATE_SESSION can also fail based on the number of clients
and the server's physical memory configuration, so it needs some
attention as well.


> Maybe we should use NFS4ERR_SERVERFAULT.  It seems to be a
> catch-all for "some fatal error". The server has failed to
> allocate required resources.

We need to be aware of how clients might respond to whichever status
codes are chosen.

SETCLIENTID and SETCLIENTID_CONFIRM are permitted to return
NFS4ERR_RESOURCE, and these are implemented separately from their
NFSv4.1 equivalents. So perhaps they can return something saner
than SERVERFAULT.


-- 
Chuck Lever