On Wed, Jul 15, 2015 at 04:47:48PM -0400, J. Bruce Fields wrote:
> On Mon, Jul 13, 2015 at 05:29:41PM +0800, Kinglong Mee wrote:
> > Commit 294ac32e99 "nfsd: protect clid and verifier generation with client_lock"
> > have moved gen_confirm() to gen_clid().
>
> This means the statement in that earlier commit is wrong:
>
>
> With this, there's no need to keep two counters as they'd always
> be in sync anyway, so just use the clientid_counter for both.
>
> Looks to me like this may need a separate counter to eliminate the
> possibibility of returning the same confirm twice for a one clientid?
(but frankly I can never completely review changes to the
setclientid/setclientid_confirm behavior without rereading RFC 7530
16.33.5 every time, which is a slog. Might help to contrive a pynfs
test derived from that text which tests for this particular behavior.)
>
> --b.
>
> >
> > After it, setclientid will return a bad reply with all zero confirms
> > after copy_clid().
> >
> > Signed-off-by: Kinglong Mee <kinglongmee@xxxxxxxxx>
> > ---
> > fs/nfsd/nfs4state.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index e0a4556..b1f84fc 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -3042,10 +3042,11 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > unconf = find_unconfirmed_client_by_name(&clname, nn);
> > if (unconf)
> > unhash_client_locked(unconf);
> > - if (conf && same_verf(&conf->cl_verifier, &clverifier))
> > + if (conf && same_verf(&conf->cl_verifier, &clverifier)) {
> > /* case 1: probable callback update */
> > copy_clid(new, conf);
> > - else /* case 4 (new client) or cases 2, 3 (client reboot): */
> > + gen_confirm(new, nn);
> > + } else /* case 4 (new client) or cases 2, 3 (client reboot): */
> > gen_clid(new, nn);
> > new->cl_minorversion = 0;
> > gen_callback(new, setclid, rqstp);
> > --
> > 2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
