> On Jun 19, 2023, at 4:19 PM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote: > > On Mon, 2023-06-19 at 19:54 +0000, Chuck Lever III 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. >> >> 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. >> > > No. I don't give a rats arse what the spec says. The spec is what we've got. If there's a problem with it somebody should document it. I'm citing spec here because other server implementations might be in the same boat as NFSD. > I'm telling you why the client is doing what it does. Yes, we understand why it's doing that. We read the patch description for e144cbcc251f. We agree that close-to-open is a desirable thing. But I'm not seeing how the updated change attribute or size is helpful in the write delegation case: a write delegation is supposed to prevent changes to the file that the client is not aware of. Can you elaborate why a client that holds a write delegation would ask for the file size when returning that delegation? > You either deal > with it, or you don't, but we're not changing the client after 20 years > of this behaviour. More like 10 years. e144cbcc251f showed up in 2012. Neither Solaris nor NetApp return a delegation in this case. They simply don't bother, there's no looking ahead or any other such behavior. We've already round-tripped with the nfsv4 working group, that behavior is not compliant with RFC 7530 Section 16.7.5. That's why this has worked "for 20 years". Linux depends on non-compliant or undocumented server behavior for this trick to work. If CB_GETATTR avoids this conundrum, then we can re-prioritize implementation of CB_GETATTR for NFSD. Seems like CB_GETATTR would have a similar loop, though: the conflicting GETATTR would trigger a CB_GETATTR and the server would then return what the client already has. -- Chuck Lever
