On 3/22/22 2:50 PM, J. Bruce Fields wrote:
On Thu, Mar 17, 2022 at 12:43:43AM -0700, Dai Ngo wrote:
Update find_clp_in_name_tree to check and expire courtesy client.
Update find_confirmed_client_by_name to discard the courtesy
client by setting CLIENT_EXPIRED.
Update nfsd4_setclientid to expire client with CLIENT_EXPIRED
state to prevent multiple confirmed clients with the same name
on the conf_id_hashtbl list.
This should be nn->conf_name_tree instead of conf_id_hashtbl list.
I could use a little more "why" here.
I'll give it another read, but right now I'm just not understanding how
this is meant to work.
Here is the problem description when running pynfs/nfs4.0 CID1 (need to
run CID5 first to create ton of clients) which requires the change in
nfsd4_setclientid to get the test to pass:
1st setclientid(name=clientA,verf=X)
. find confirmed courtesy client with same name clientA (clientid:1) and
mark it DESTROY and leave it on the confirmed list.
. assign new clientA with clientid:2
. put clientid:2 on unconfirmed list
. return clientid:2
1st setclientid_confirm(clientid:2)
. does not find any confirmed for clientid:2
. find unconfirmed clientid:2
. does not find any confirmed clientA (find_confirmed_client_by_name)
since courtesy clientA was marked DESTROY
. put clientid:2: to conf_name_tree list
PROBLEM: new clientA (clientid:2) and courtesy clientA (clientid:1)
are both now on the conf_name_tree list.
clientid:2 does OPEN -> stidX
2nd setclientid(name=clientA, verf=Y)
. does not find any confirmed clientA: skip courtesy clientA since it
was marked DESTROY and does not find confirmed client:2 due to the
PROBLEM noted above; duplicate entry in binary tree.
. create new clientid:3 and put it on unconfirmed list
. return clientid:3
2nd setclientid_confirm(clientid:3)
. does not find any confirmed clientid:3
. find unconfirmed clientid:3
. does not find any confirmed clientA: skip courtesy clientA since it
was marked DESTROY and does not find confirmed client:2 due to the
PROBLEM noted above; duplicate entry in binary tree.
. put clientid:3 on conf_name_tree list
clientid:3 does CLOSE(stidX) -> NFS4_OK since clientid:2 still valid <ERROR!>
-Dai
--b.
Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx>
---
fs/nfsd/nfs4state.c | 27 ++++++++++++++++++++++++---
fs/nfsd/state.h | 22 ++++++++++++++++++++++
2 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c6b5e05c9c34..dc0e60bf694c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2834,8 +2834,11 @@ find_clp_in_name_tree(struct xdr_netobj *name, struct rb_root *root)
node = node->rb_left;
else if (cmp < 0)
node = node->rb_right;
- else
+ else {
+ if (nfsd4_courtesy_clnt_expired(clp))
+ return NULL;
return clp;
+ }
}
return NULL;
}
@@ -2914,8 +2917,15 @@ static bool clp_used_exchangeid(struct nfs4_client *clp)
static struct nfs4_client *
find_confirmed_client_by_name(struct xdr_netobj *name, struct nfsd_net *nn)
{
+ struct nfs4_client *clp;
+
lockdep_assert_held(&nn->client_lock);
- return find_clp_in_name_tree(name, &nn->conf_name_tree);
+ clp = find_clp_in_name_tree(name, &nn->conf_name_tree);
+ if (clp && clp->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
+ nfsd4_discard_courtesy_clnt(clp);
+ clp = NULL;
+ }
+ return clp;
}
static struct nfs4_client *
@@ -4032,12 +4042,19 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfs4_client *unconf = NULL;
__be32 status;
struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+ struct nfs4_client *cclient = NULL;
new = create_client(clname, rqstp, &clverifier);
if (new == NULL)
return nfserr_jukebox;
spin_lock(&nn->client_lock);
- conf = find_confirmed_client_by_name(&clname, nn);
+ /* find confirmed client by name */
+ conf = find_clp_in_name_tree(&clname, &nn->conf_name_tree);
+ if (conf && conf->cl_cs_client_state == NFSD4_CLIENT_RECONNECTED) {
+ cclient = conf;
+ conf = NULL;
+ }
+
if (conf && client_has_state(conf)) {
status = nfserr_clid_inuse;
if (clp_used_exchangeid(conf))
@@ -4068,7 +4085,11 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
new = NULL;
status = nfs_ok;
out:
+ if (cclient)
+ unhash_client_locked(cclient);
spin_unlock(&nn->client_lock);
+ if (cclient)
+ expire_client(cclient);
if (new)
free_client(new);
if (unconf) {
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index d156ae3ab46c..14b2c158ccca 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -735,6 +735,7 @@ extern void nfsd4_client_record_remove(struct nfs4_client *clp);
extern int nfsd4_client_record_check(struct nfs4_client *clp);
extern void nfsd4_record_grace_done(struct nfsd_net *nn);
+/* courteous server */
static inline bool
nfsd4_expire_courtesy_clnt(struct nfs4_client *clp)
{
@@ -749,4 +750,25 @@ nfsd4_expire_courtesy_clnt(struct nfs4_client *clp)
return rc;
}
+static inline void
+nfsd4_discard_courtesy_clnt(struct nfs4_client *clp)
+{
+ spin_lock(&clp->cl_cs_lock);
+ clp->cl_cs_client_state = NFSD4_CLIENT_EXPIRED;
+ spin_unlock(&clp->cl_cs_lock);
+}
+
+static inline bool
+nfsd4_courtesy_clnt_expired(struct nfs4_client *clp)
+{
+ bool rc = false;
+
+ spin_lock(&clp->cl_cs_lock);
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_EXPIRED)
+ rc = true;
+ if (clp->cl_cs_client_state == NFSD4_CLIENT_COURTESY)
+ clp->cl_cs_client_state = NFSD4_CLIENT_RECONNECTED;
+ spin_unlock(&clp->cl_cs_lock);
+ return rc;
+}
#endif /* NFSD4_STATE_H */
--
2.9.5
