On Tue, 17 May 2022, Trond Myklebust wrote: > On Tue, 2022-05-17 at 11:05 +1000, NeilBrown wrote: > > On Tue, 17 May 2022, Trond Myklebust wrote: > > > On Tue, 2022-05-17 at 10:40 +1000, NeilBrown wrote: > > > > On Tue, 17 May 2022, Trond Myklebust wrote: > > > > > On Tue, 2022-05-17 at 10:05 +1000, NeilBrown wrote: > > > > > > > > > > > > Hi, > > > > > > any thoughts on these patches? > > > > > > > > > > > > > > > To me, this problem is simply not worth breaking hot path > > > > > functionality > > > > > for. If the credential changes on the server, but not on the > > > > > client > > > > > (so > > > > > that the cred cache comparison still matches), then why do we > > > > > care? > > > > > > > > > > IOW: I'm a NACK until convinced otherwise. > > > > > > > > In what way is the hot path broken? It only affect a permission > > > > test > > > > failure. Why is that considered "hot path"?? > > > > > > It is a permission test that is critical for caching path > > > resolution on > > > a per-user basis. > > > > > > > > > > > RFC 1813 says - for NFSv3 at least - > > > > > > > > The information returned by the server in response to an > > > > ACCESS call is not permanent. It was correct at the exact > > > > time that the server performed the checks, but not > > > > necessarily afterwards. The server can revoke access > > > > permission at any time. > > > > > > > > Clearly the server can allow allow access at any time. > > > > This seems to argue against caching - or at least against relying > > > > too > > > > heavily on the cache. > > > > > > > > RFC 8881 has the same text for NFSv4.1 - section 18.1.4 > > > > > > > > "why do we care?" Because the server has changed to grant access, > > > > but > > > > the client is ignoring the possibility of that change - so the > > > > user > > > > is > > > > prevented from doing work. > > > > > > The server enforces permissions in NFS. The client permissions > > > checks > > > are performed in order to gate access to data that is already in > > > cache. > > > > So if the permission check fails, then the client should ignore the > > cache and ask the server for the requested data, so that the server > > has > > a chance to enforce the permissions - whether denying or allowing the > > access. > > > > I completely agree with you, and that is effectively what my patch > > implements. > > > > No. I'm saying that no matter how many spec paragraphs you quote at me, > I'm not willing to introduce a timeout on a cache that is critical for > path resolution in order to address a corner case of a corner case. > > I'm saying that if you want this problem fixed, then you need to look > at a different solution that doesn't have side-effects for the > 99.99999% cases or more that I do care about. What, specifically, as the cases that you do care about? I assumed that the cases you care about are cases where the user *does* have access, and where this access is correctly cached, so that a permission(..., MAY_EXEC) call returns immediately with a positive answer. I care about these cases too, and I've ensured that the patch doesn't change the behaviour for these cases. What other cases - cases where permission() returns an error - do you care about? Thanks, NeilBrown
