All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trondmy@hammerspace.com>
To: "jlayton@kernel.org" <jlayton@kernel.org>,
	"neilb@suse.de" <neilb@suse.de>
Cc: "anna@kernel.org" <anna@kernel.org>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"bcodding@redhat.com" <bcodding@redhat.com>
Subject: Re: [PATCH 0/2] NFS: limit use of ACCESS cache for negative responses
Date: Sat, 3 Sep 2022 15:49:43 +0000	[thread overview]
Message-ID: <1cfb40103f3f539973587f4565af79d5dc439096.camel@hammerspace.com> (raw)
In-Reply-To: <166219906850.28768.1525969921769808093@noble.neil.brown.name>

On Sat, 2022-09-03 at 19:57 +1000, NeilBrown wrote:
> The whole point of the NFS ACCESS request is that the client cannot
> know
> how to interpret any access controls that the server might have in
> place.  Who knows, they might even be time based access controls
> (games can only be played on the weekend).  Ok, that is a bit
> extreme,
> but the principle remains.
> 
> *You* might "know" that there are two pieces (in this specific
> example)
> and that one doesn't directly relate to the inodes, but the NFS
> client
> doesn't know that.  The NFS client only knows that it can request
> ACCESS
> information for a filehandle in the context of a credential.  The
> Linux
> NFS client then caches that against the inode (so it is all somehow
> related to the inode).
> 
> The spec doesn't give the client permission to cached these details
> AT
> ALL.  Caching positive results makes perfect sense from a performance
> perspective and is easily defensible.  Caching negative results ... 
> not
> so much.

Of course the spec allows the client to cache permissions in both NFSv3
and NFSv4. Otherwise there would be no way we could cache objects at
all. Enabling the client to gate access to cached data such as
directory contents, and cached data without troubling the server is the
whole point of the ACCESS operation. Without that ability, we would
have to ask the server every time a user tried to 'cd' into a
directory, called open(), called getdents(), called read(), called
write(), ... Without that ability, NFSv4 delegations would be
pointless, because we'd have to go and ask the server for all these
operations.
I know the spec is littered with mealy mouthed "The server can revoke
access permission at any time" statements in various spots, but that
looks more like something to placate a review by the IBM legal team
(yes, it happened) rather than a basic assumption that was engineered
into the protocol. The protocol simply cannot work as described in
RFC5661 and RFC7530 without the assumption that you can cache.

The other point is that access cache changes due to AD/LDAP/NIS group
membership updates happen once in a blue moon, and when they do, they
affect _ALL_ cached values for that user, and not just one at a time.
When those things do happen, the user normally expects to have to log
out and then log back in so that the new group memberships are
reflected in that user's cred (as per POSIX prescribed behaviour). That
will cause the NFS client to send new ACCESS calls to the server,
because the new cred will not be found in the existing access cache.
The only case where this assumption fails, is if the server is
implementing the '--manage-gids' behaviour, and caches its
representation of the user's creds past the time when the user logged
out and logged in again.
So this is literally a 1 in a billion chance problem that you're asking
us to fix. Optimising for that case by adding in a timeout that affects
all cached access value is just not in the cards.

I've already sent out a patch that forces realignment of the NFS client
behaviour with that of POSIX by requiring the client to recheck cached
access values for an existing credential after a new login, but does
not add any further impediments to the access cache. Why is that
approach not sufficient?

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



  reply	other threads:[~2022-09-03 15:49 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-28  1:37 [PATCH 0/2] NFS: limit use of ACCESS cache for negative responses NeilBrown
2022-04-28  1:37 ` [PATCH 1/2] NFS: change nfs_access_get_cached() to nfs_access_check_cached() NeilBrown
2022-04-28  1:37 ` [PATCH 2/2] NFS: limit use of ACCESS cache for negative responses NeilBrown
2022-05-17  0:05 ` [PATCH 0/2] " NeilBrown
2022-05-17  0:20   ` Trond Myklebust
2022-05-17  0:40     ` NeilBrown
2022-05-17  0:55       ` Trond Myklebust
2022-05-17  1:05         ` NeilBrown
2022-05-17  1:14           ` Trond Myklebust
2022-05-17  1:22             ` NeilBrown
2022-05-17  1:36               ` Trond Myklebust
2022-08-26 14:59                 ` Benjamin Coddington
2022-08-26 15:44                   ` Trond Myklebust
2022-08-26 16:43                     ` Benjamin Coddington
2022-08-26 16:56                       ` Trond Myklebust
2022-08-26 18:27                         ` Benjamin Coddington
2022-08-27  0:52                           ` Trond Myklebust
2022-09-19 19:09                             ` Benjamin Coddington
2022-09-19 22:38                               ` NeilBrown
2022-09-20  1:18                                 ` Trond Myklebust
2022-08-26 23:39                     ` NeilBrown
2022-08-27  3:38                       ` Trond Myklebust
2022-08-28 23:32                         ` NeilBrown
2022-08-29 14:07                           ` Jeff Layton
2022-09-03  9:57                             ` NeilBrown
2022-09-03 15:49                               ` Trond Myklebust [this message]
2022-09-04 23:28                                 ` NeilBrown
2022-09-04 23:40                                   ` Trond Myklebust
2022-09-05  0:09                                     ` NeilBrown
2022-09-05  0:49                                       ` Trond Myklebust

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1cfb40103f3f539973587f4565af79d5dc439096.camel@hammerspace.com \
    --to=trondmy@hammerspace.com \
    --cc=anna@kernel.org \
    --cc=bcodding@redhat.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.