On Tue, 2022-05-17 at 11:22 +1000, NeilBrown wrote: > 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? The general case, where the group membership is not changing on the server without also changing on the client. Whether it is positive or negative lookups. I care about the ability of the client to manage its cache without gratuitous invalidations. This isn't even something that is specific to NFS. This is how local filesystems operate too. So until you have a different solution that doesn't impact the client's ability to cache permissions, then the answer is going to be "no" to these patches. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
