Re: [PATCH 0/2] NFS: limit use of ACCESS cache for negative responses

[Trond Myklebust <trondmy@hammerspace.com>]

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.

> 
> Thanks,
> NeilBrown
> 
> 
> On Thu, 28 Apr 2022, NeilBrown wrote:
> > Since Commit 57b691819ee2 ("NFS: Cache access checks more
> > aggressively")
> > (Linux 4.8) NFS has cached the results of ACCESS indefinitely while
> > the
> > inode isn't changing.
> > 
> > This is often a good choice, but doesn't take into account the
> > possibility that changes out side of the inode can change effective
> > permissions.
> > 
> > Depending on configuration, some servers can map the user provided
> > in
> > the RPC credential to a group list at time of request.  If the
> > group
> > list for a user is changed, the result of ACCESS can change.
> > 
> > This is particularly a problem when extra permissions are given on
> > the
> > server.  The client may make decisions based on outdated ACCESS
> > results
> > and not even try operations which would in fact succeed.
> > 
> > These two patches change the ACCESS cache so that when the cache
> > grants
> > an access, that is trusted indefinitely just as it currently does.
> > However when the cache denies an access, that is only trusted if
> > the
> > cached data is less than acmin seconds old.  Otherwise a new ACCESS
> > request is made.
> > 
> > This allows additions to group membership to become effective with
> > only a modest delay.
> > 
> > The second patch contains even more explanatory detail.
> > 
> > Thanks,
> > NeilBrown
> > 
> > ---
> > 
> > NeilBrown (2):
> >       NFS: change nfs_access_get_cached() to
> > nfs_access_check_cached()
> >       NFS: limit use of ACCESS cache for negative responses
> > 
> > 
> >  fs/nfs/dir.c           | 80 +++++++++++++++++++++++++-------------
> > ----
> >  fs/nfs/nfs4proc.c      | 25 ++++++-------
> >  include/linux/nfs_fs.h |  5 +--
> >  3 files changed, 61 insertions(+), 49 deletions(-)
> > 
> > --
> > Signature
> > 
> > 

-- 
Trond Myklebust
CTO, Hammerspace Inc
4984 El Camino Real, Suite 208
Los Altos, CA 94022
​
www.hammer.space