> On Jun 19, 2023, at 6:49 PM, Olga Kornievskaia <aglo@xxxxxxxxx> wrote: > > On Mon, Jun 19, 2023 at 4:02 PM Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote: >> >> >> >>> On Jun 19, 2023, at 3:19 PM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote: >>> >>> Hi Dai, >>> >>> On Mon, 2023-06-19 at 10:02 -0700, dai.ngo@xxxxxxxxxx wrote: >>>> Hi Trond, >>>> >>>> I'm testing the NFS server with write delegation support and the >>>> Linux client >>>> using NFSv4.0 and run into a situation that needs your advise. >>>> >>>> In this scenario, the NFS server grants the write delegation to the >>>> client. >>>> Later when the client returns delegation it sends the compound PUTFH, >>>> GETATTR >>>> and DELERETURN. >>>> >>>> When the NFS server services the GETATTR, it detects that there is a >>>> write >>>> delegation on this file but it can not detect that this GETATTR >>>> request was >>>> sent from the same client that owns the write delegation (due to the >>>> nature >>>> of NFSv4.0 compound). As the result, the server sends CB_RECALL to >>>> recall >>>> the delegation and replies NFS4ERR_DELAY to the GETATTR request. >>>> >>>> When the client receives the NFS4ERR_DELAY it retries with the same >>>> compound >>>> PUTFH, GETATTR, DELERETURN and server again replies the >>>> NFS4ERR_DELAY. This >>>> process repeats until the recall times out and the delegation is >>>> revoked by >>>> the server. >>>> >>>> I noticed that the current order of GETATTR and DELEGRETURN was done >>>> by >>>> commit e144cbcc251f. Then later on, commit 8ac2b42238f5 was added to >>>> drop >>>> the GETATTR if the request was rejected with EACCES. >>>> >>>> Do you have any advise on where, on server or client, this issue >>>> should >>>> be addressed? >>> >>> This wants to be addressed in the server. The client has a very good >>> reason for wanting to retrieve the attributes before returning the >>> delegation here: it needs to update the change attribute while it is >>> still holding the delegation in order to ensure close-to-open cache >>> consistency. >>> >>> Since you do have a stateid in the DELEGRETURN, it should be possible >>> to determine that this is indeed the client that holds the delegation. >> >> I think it needs to be made clear in a specification that this is >> the intended and conventional server implementation needed for such >> a COMPOUND. >> >> RFC 7530 Section 14.2 says: >> >>> The server will process the COMPOUND procedure by evaluating each of >>> the operations within the COMPOUND procedure in order. >> >> 2nd paragraph of RFC 7530 Section 15.2.4 says: >> >>> The COMPOUND procedure is used to combine individual operations into >>> a single RPC request. The server interprets each of the operations >>> in turn. If an operation is executed by the server and the status of >>> that operation is NFS4_OK, then the next operation in the COMPOUND >>> procedure is executed. The server continues this process until there >>> are no more operations to be executed or one of the operations has a >>> status value other than NFS4_OK. >> >> Obviously in this case the client has sent a well-formed COMPOUND, >> but it's not one the server can execute given the ordering >> constraint spelled out above. >> >> Can you refer us to a part of any RFC that says it's appropriate >> to look ahead at subsequent operations in an NFSv4.0 COMPOUND to >> obtain a state or client ID? Otherwise the Linux client will have >> the same problem with any server implementation that handles >> GETATTR conflicts as described in RFC 7530 Section 16.7.5. > > I'm not sure if this is totally irrelevant here but for the NFSv4.2 > COPY nfsd does look ahead into the following operations in order to > determine if it should allow one of the filehandle that it can't > resolve (ie because it's a source server filehandle) which it would > normally return ERR_STALE if it were any other compound. But of course > one can argue that the spec specifically says that server needs to > look ahead. Exactly my point: IMO the client can't rely on the server behaving that way unless the spec indicates the server must do that. That's interoperability 101. >> Based on this language I don't believe NFSv4.0 clients can rely on >> server implementations to look ahead for client ID information. In >> my view the client ought to provide a client ID by placing a RENEW >> before the GETATTR. Even in that case, the server implementation >> might not be aware that it needs to save the client ID from the >> RENEW operation. >> >> >> -- >> Chuck Lever -- Chuck Lever