Re: [PATCH v2 23/47] nfsd41: create_session operation

["J. Bruce Fields" <bfields@fieldses.org>]

On Sat, Mar 28, 2009 at 11:32:57AM +0300, Benny Halevy wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> Implement the create_session operation confoming to
> http://tools.ietf.org/html/draft-ietf-nfsv4-minorversion1-26
> 
> Look up the client id (generated by the server on exchange_id,
> given by the client on create_session).
> If neither a confirmed or unconfirmed client is found
> then the client id is stale
> If a confirmed cilent is found (i.e. we already received
> create_session for it) then compare the sequence id
> to determine if it's a replay or possibly a mis-ordered rpc.
> If the seqid is in order, update the confirmed client seqid
> and procedd with updating the session parameters.
> 
> If an unconfirmed client_id is found then verify the creds
> and seqid.  If both match move the client id to confirmed state
> and proceed with processing the create_session.
> 
> Currently, we do not support persistent sessions, and RDMA.
> 
> alloc_init_session generates a new sessionid and creates
> a session structure.
> 
> NFSD_PAGES_PER_SLOT is used for the max response cached calculation, and for
> the counting of DRC pages using the hard limits set in struct srv_serv.
> 
> A note on NFSD_PAGES_PER_SLOT:
> 
> Other patches in this series allow for NFSD_PAGES_PER_SLOT + 1 pages to be
> cached in a DRC slot when the response size is less than NFSD_PAGES_PER_SLOT *
> PAGE_SIZE but xdr_buf pages are used. e.g. a READDIR operation will encode a
> small amount of data in the xdr_buf head, and then the READDIR in the xdr_buf
> pages.  So, the hard limit calculation use of pages by a session is
> underestimated by the number of cached operations using the xdr_buf pages.
> 
> Yet another patch caches no pages for the solo sequence operation, or any
> compound where cache_this is False.  So the hard limit calculation use of
> pages by a session is overestimated by the number of these operations in the
> cache.
> 
> TODO: improve resource pre-allocation and negotiate session
> parameters accordingly.  Respect and possibly adjust
> backchannel attributes.
> 
> Signed-off-by: Marc Eshel <eshel@almaden.ibm.com>
> Signed-off-by: Dean Hildebrand <dhildeb@us.ibm.com>
> [nfsd41: remove headerpadsz from channel attributes]
> Our client and server only support a headerpadsz of 0.
> [nfsd41: use DRC limits in fore channel init]
> [nfsd41: do not change CREATE_SESSION back channel attrs]
> Signed-off-by: Andy Adamson <andros@netapp.com>
> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
> [use sessionid_lock spin lock]
> [nfsd41: use bool inuse for slot state]
> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
> ---
>  fs/nfsd/nfs4state.c        |  197 +++++++++++++++++++++++++++++++++++++++++++-
>  fs/nfsd/nfs4xdr.c          |  147 ++++++++++++++++++++++++++++++++-
>  include/linux/nfsd/state.h |    7 ++
>  include/linux/nfsd/xdr4.h  |   21 +++++-
>  4 files changed, 368 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 37865c9..e4e2c19 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -68,6 +68,9 @@ static u32 current_delegid = 1;
>  static u32 nfs4_init;
>  static stateid_t zerostateid;             /* bits all 0 */
>  static stateid_t onestateid;              /* bits all 1 */
> +#ifdef CONFIG_NFSD_V4_1
> +static u64 current_sessionid = 1;
> +#endif /* CONFIG_NFSD_V4_1 */
>  
>  #define ZERO_STATEID(stateid) (!memcmp((stateid), &zerostateid, sizeof(stateid_t)))
>  #define ONE_STATEID(stateid)  (!memcmp((stateid), &onestateid, sizeof(stateid_t)))
> @@ -402,6 +405,138 @@ dump_sessionid(const char *fn, struct nfs4_sessionid *sessionid)
>  	dprintk("%s: %u:%u:%u:%u\n", fn, ptr[0], ptr[1], ptr[2], ptr[3]);
>  }
>  
> +static void
> +gen_sessionid(struct nfsd4_session *ses)
> +{
> +	struct nfs4_client *clp = ses->se_client;
> +	struct nfsd4_sessionid *sid;
> +
> +	sid = (struct nfsd4_sessionid *)ses->se_sessionid.data;
> +	sid->clientid = clp->cl_clientid;
> +	sid->sequence = current_sessionid++;
> +	sid->reserved = 0;
> +}
> +
> +/*
> + * Give the client the number of slots it requests bound by
> + * NFSD_MAX_SLOTS_PER_SESSION and by sv_drc_max_pages.
> + *
> + * If we run out of pages (sv_drc_pages_used == sv_drc_max_pages) we
> + * should (up to a point) re-negotiate active sessions and reduce their
> + * slot usage to make rooom for new connections. For now we just fail the
> + * create session.
> + */
> +static int set_forechannel_maxreqs(struct nfsd4_channel_attrs *fchan)
> +{
> +	int status = 0, np = fchan->maxreqs * NFSD_PAGES_PER_SLOT;
> +
> +	spin_lock(&nfsd_serv->sv_lock);
> +	if (np + nfsd_serv->sv_drc_pages_used > nfsd_serv->sv_drc_max_pages)
> +		np = nfsd_serv->sv_drc_max_pages - nfsd_serv->sv_drc_pages_used;
> +	nfsd_serv->sv_drc_pages_used += np;
> +	spin_unlock(&nfsd_serv->sv_lock);
> +
> +	if (np <= 0) {
> +		status = nfserr_resource;
> +		fchan->maxreqs = 0;
> +	} else
> +		fchan->maxreqs = np / NFSD_PAGES_PER_SLOT;
> +
> +	return status;
> +}
> +
> +/*
> + * fchan holds the client values on input, and the server values on output
> + */
> +static int init_forechannel_attrs(struct svc_rqst *rqstp,
> +				    struct nfsd4_session *session,
> +				    struct nfsd4_channel_attrs *fchan)
> +{
> +	int status = 0;
> +	__u32   maxcount = svc_max_payload(rqstp);
> +
> +	/* headerpadsz set to zero in encode routine*/
> +
> +	/* Use the client's max request and max response size if possible */
> +	if (fchan->maxreq_sz > maxcount)
> +		fchan->maxreq_sz = maxcount;
> +	session->se_fmaxreq_sz = fchan->maxreq_sz;
> +
> +	if (fchan->maxresp_sz > maxcount)
> +		fchan->maxresp_sz = maxcount;
> +	session->se_fmaxresp_sz = fchan->maxresp_sz;
> +
> +	/* Set the max response cached size our default which is
> +	 * a multiple of PAGE_SIZE and small */
> +	session->se_fmaxresp_cached = NFSD_PAGES_PER_SLOT * PAGE_SIZE;
> +	fchan->maxresp_cached = session->se_fmaxresp_cached;
> +
> +	/* Use the client's maxops if possible */
> +	if (fchan->maxops > NFSD_MAX_OPS_PER_COMPOUND)
> +		fchan->maxops = NFSD_MAX_OPS_PER_COMPOUND;
> +	session->se_fmaxops = fchan->maxops;
> +
> +	/* try to use the client requested number of slots */
> +	if (fchan->maxreqs > NFSD_MAX_SLOTS_PER_SESSION)
> +		fchan->maxreqs = NFSD_MAX_SLOTS_PER_SESSION;
> +
> +	/* FIXME: Error means no more DRC pages so the server should
> +	 * recover pages from existing sessions. For now fail session
> +	 * creation.
> +	 */
> +	status = set_forechannel_maxreqs(fchan);
> +
> +	session->se_fnumslots = fchan->maxreqs;
> +	return status;
> +}
> +
> +static int
> +alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp,
> +		   struct nfsd4_create_session *cses)
> +{
> +	struct nfsd4_session *new;
> +	int idx, status = nfserr_resource, slotsize, i;
> +
> +	new = kzalloc(sizeof(*new), GFP_KERNEL);
> +	if (!new)
> +		goto out;
> +
> +	/* FIXME: For now, we just accept the client back channel attributes. */
> +	status = init_forechannel_attrs(rqstp, new, &cses->fore_channel);
> +	if (status)
> +		goto out_free;
> +
> +	slotsize = new->se_fnumslots * sizeof(struct nfsd4_slot);
> +	new->se_slots = kzalloc(slotsize, GFP_KERNEL);
> +	if (!new->se_slots)
> +		goto out_free;
> +
> +	for (i = 0; i < new->se_fnumslots; i++)
> +		new->se_slots[i].sl_session = new;
> +
> +	new->se_client = clp;
> +	gen_sessionid(new);
> +	idx = hash_sessionid(&new->se_sessionid);
> +	memcpy(clp->cl_sessionid.data, new->se_sessionid.data,
> +	       NFS4_MAX_SESSIONID_LEN);

The sessionid should be part of the new session, not the client, right?

> +
> +	new->se_flags = cses->flags;
> +	kref_init(&new->se_ref);
> +	INIT_LIST_HEAD(&new->se_hash);
> +	INIT_LIST_HEAD(&new->se_perclnt);
> +	spin_lock(&sessionid_lock);
> +	list_add(&new->se_hash, &sessionid_hashtbl[idx]);
> +	list_add(&new->se_perclnt, &clp->cl_sessions);
> +	spin_unlock(&sessionid_lock);
> +
> +	status = nfs_ok;
> +out:
> +	return status;
> +out_free:
> +	kfree(new);
> +	goto out;
> +}
> +
>  /* caller must hold sessionid_lock */
>  static struct nfsd4_session *
>  find_in_sessionid_hashtbl(struct nfs4_sessionid *sessionid)
> @@ -1186,7 +1321,67 @@ nfsd4_create_session(struct svc_rqst *rqstp,
>  		     struct nfsd4_compound_state *cstate,
>  		     struct nfsd4_create_session *cr_ses)
>  {
> -	return -1;	/* stub */
> +	u32 ip_addr = svc_addr_in(rqstp)->sin_addr.s_addr;
> +	struct nfs4_client *conf, *unconf;
> +	int status = 0;
> +
> +	nfs4_lock_state();
> +	unconf = find_unconfirmed_client(&cr_ses->clientid);
> +	conf = find_confirmed_client(&cr_ses->clientid);
> +
> +	if (conf) {
> +		status = nfs_ok;
> +		if (conf->cl_seqid == cr_ses->seqid) {
> +			dprintk("Got a create_session replay! seqid= %d\n",
> +				conf->cl_seqid);
> +			goto out_replay;
> +		} else if (cr_ses->seqid != conf->cl_seqid + 1) {
> +			status = nfserr_seq_misordered;
> +			dprintk("Sequence misordered!\n");
> +			dprintk("Expected seqid= %d but got seqid= %d\n",
> +				conf->cl_seqid, cr_ses->seqid);
> +			goto out;
> +		}
> +		conf->cl_seqid++;
> +	} else if (unconf) {
> +		if (!same_creds(&unconf->cl_cred, &rqstp->rq_cred) ||
> +		    (ip_addr != unconf->cl_addr)) {

Again, I don't believe the nfsv4/4.1 specs require the client to keep to
a single ip address, so we shouldn't be doing ip address checks.

--b.

> +			status = nfserr_clid_inuse;
> +			goto out;
> +		}
> +
> +		if (unconf->cl_seqid != cr_ses->seqid) {
> +			status = nfserr_seq_misordered;
> +			goto out;
> +		}
> +
> +		move_to_confirmed(unconf);
> +
> +		/*
> +		 * We do not support RDMA or persistent sessions
> +		 */
> +		cr_ses->flags &= ~SESSION4_PERSIST;
> +		cr_ses->flags &= ~SESSION4_RDMA;
> +
> +		conf = unconf;
> +	} else {
> +		status = nfserr_stale_clientid;
> +		goto out;

Is this the right error?

--b.

> +	}
> +
> +	status = alloc_init_session(rqstp, conf, cr_ses);
> +	if (status)
> +		goto out;
> +
> +out_replay:
> +	memcpy(cr_ses->sessionid.data, conf->cl_sessionid.data,
> +	       NFS4_MAX_SESSIONID_LEN);
> +	cr_ses->seqid = conf->cl_seqid;
> +
> +out:
> +	nfs4_unlock_state();
> +	dprintk("%s returns %d\n", __func__, ntohl(status));
> +	return status;
>  }
>  
>  __be32
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 57afb33..60db854 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1100,7 +1100,108 @@ static __be32
>  nfsd4_decode_create_session(struct nfsd4_compoundargs *argp,
>  			    struct nfsd4_create_session *sess)
>  {
> -	return nfserr_opnotsupp;	/* stub */
> +	DECODE_HEAD;
> +
> +	u32 dummy;
> +	char *machine_name;
> +	int i;
> +	int nr_secflavs;
> +
> +	READ_BUF(16);
> +	COPYMEM(&sess->clientid, 8);
> +	READ32(sess->seqid);
> +	READ32(sess->flags);
> +
> +	/* Fore channel attrs */
> +	READ_BUF(28);
> +	READ32(dummy); /* headerpadsz is always 0 */
> +	READ32(sess->fore_channel.maxreq_sz);
> +	READ32(sess->fore_channel.maxresp_sz);
> +	READ32(sess->fore_channel.maxresp_cached);
> +	READ32(sess->fore_channel.maxops);
> +	READ32(sess->fore_channel.maxreqs);
> +	READ32(sess->fore_channel.nr_rdma_attrs);
> +	if (sess->fore_channel.nr_rdma_attrs == 1) {
> +		READ_BUF(4);
> +		READ32(sess->fore_channel.rdma_attrs);
> +	} else if (sess->fore_channel.nr_rdma_attrs > 1) {
> +		dprintk("Too many fore channel attr bitmaps!\n");
> +		goto xdr_error;
> +	}
> +
> +	/* Back channel attrs */
> +	READ_BUF(28);
> +	READ32(dummy); /* headerpadsz is always 0 */
> +	READ32(sess->back_channel.maxreq_sz);
> +	READ32(sess->back_channel.maxresp_sz);
> +	READ32(sess->back_channel.maxresp_cached);
> +	READ32(sess->back_channel.maxops);
> +	READ32(sess->back_channel.maxreqs);
> +	READ32(sess->back_channel.nr_rdma_attrs);
> +	if (sess->back_channel.nr_rdma_attrs == 1) {
> +		READ_BUF(4);
> +		READ32(sess->back_channel.rdma_attrs);
> +	} else if (sess->back_channel.nr_rdma_attrs > 1) {
> +		dprintk("Too many back channel attr bitmaps!\n");
> +		goto xdr_error;
> +	}
> +
> +	READ_BUF(8);
> +	READ32(sess->callback_prog);
> +
> +	/* callback_sec_params4 */
> +	READ32(nr_secflavs);
> +	for (i = 0; i < nr_secflavs; ++i) {
> +		READ_BUF(4);
> +		READ32(dummy);
> +		switch (dummy) {
> +		case RPC_AUTH_NULL:
> +			/* Nothing to read */
> +			break;
> +		case RPC_AUTH_UNIX:
> +			READ_BUF(8);
> +			/* stamp */
> +			READ32(dummy);
> +
> +			/* machine name */
> +			READ32(dummy);
> +			READ_BUF(dummy);
> +			SAVEMEM(machine_name, dummy);
> +
> +			/* uid, gid */
> +			READ_BUF(8);
> +			READ32(sess->uid);
> +			READ32(sess->gid);
> +
> +			/* more gids */
> +			READ_BUF(4);
> +			READ32(dummy);
> +			READ_BUF(dummy * 4);
> +			for (i = 0; i < dummy; ++i)
> +				READ32(dummy);
> +			break;
> +		case RPC_AUTH_GSS:
> +			dprintk("RPC_AUTH_GSS callback secflavor "
> +				"not supported!\n");
> +			READ_BUF(8);
> +			/* gcbp_service */
> +			READ32(dummy);
> +			/* gcbp_handle_from_server */
> +			READ32(dummy);
> +			READ_BUF(dummy);
> +			p += XDR_QUADLEN(dummy);
> +			/* gcbp_handle_from_client */
> +			READ_BUF(4);
> +			READ32(dummy);
> +			READ_BUF(dummy);
> +			p += XDR_QUADLEN(dummy);
> +			break;
> +		default:
> +			dprintk("Illegal callback secflavor\n");
> +			return nfserr_inval;
> +		}
> +	}
> +	DECODE_TAIL;
>  }
>  
>  static __be32
> @@ -2829,7 +2930,49 @@ static __be32
>  nfsd4_encode_create_session(struct nfsd4_compoundres *resp, int nfserr,
>  			    struct nfsd4_create_session *sess)
>  {
> -	/* stub */
> +	ENCODE_HEAD;
> +
> +	if (nfserr)
> +		goto out;
> +
> +	RESERVE_SPACE(24);
> +	WRITEMEM(sess->sessionid.data, NFS4_MAX_SESSIONID_LEN);
> +	WRITE32(sess->seqid);
> +	WRITE32(sess->flags);
> +	ADJUST_ARGS();
> +
> +	RESERVE_SPACE(28);
> +	WRITE32(0); /* headerpadsz */
> +	WRITE32(sess->fore_channel.maxreq_sz);
> +	WRITE32(sess->fore_channel.maxresp_sz);
> +	WRITE32(sess->fore_channel.maxresp_cached);
> +	WRITE32(sess->fore_channel.maxops);
> +	WRITE32(sess->fore_channel.maxreqs);
> +	WRITE32(sess->fore_channel.nr_rdma_attrs);
> +	ADJUST_ARGS();
> +
> +	if (sess->fore_channel.nr_rdma_attrs) {
> +		RESERVE_SPACE(4);
> +		WRITE32(sess->fore_channel.rdma_attrs);
> +		ADJUST_ARGS();
> +	}
> +
> +	RESERVE_SPACE(28);
> +	WRITE32(0); /* headerpadsz */
> +	WRITE32(sess->back_channel.maxreq_sz);
> +	WRITE32(sess->back_channel.maxresp_sz);
> +	WRITE32(sess->back_channel.maxresp_cached);
> +	WRITE32(sess->back_channel.maxops);
> +	WRITE32(sess->back_channel.maxreqs);
> +	WRITE32(sess->back_channel.nr_rdma_attrs);
> +	ADJUST_ARGS();
> +
> +	if (sess->back_channel.nr_rdma_attrs) {
> +		RESERVE_SPACE(4);
> +		WRITE32(sess->back_channel.rdma_attrs);
> +		ADJUST_ARGS();
> +	}
> +out:
>  	return nfserr;
>  }
>  
> diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
> index 8ca6a82..98d7b1c 100644
> --- a/include/linux/nfsd/state.h
> +++ b/include/linux/nfsd/state.h
> @@ -99,8 +99,12 @@ struct nfs4_callback {
>  	struct rpc_clnt *       cb_client;
>  };
>  
> +/* Maximum number of slots per session. 128 is useful for long haul TCP */
> +#define NFSD_MAX_SLOTS_PER_SESSION	128
>  /* Maximum number of pages per slot cache entry */
>  #define NFSD_PAGES_PER_SLOT	1
> +/* Maximum number of operations per session compound */
> +#define NFSD_MAX_OPS_PER_COMPOUND	16
>  
>  struct nfsd4_cache_entry {
>  	__be32		ce_status;
> @@ -188,6 +192,9 @@ struct nfs4_client {
>  	struct list_head	cl_sessions;
>  	u32			cl_seqid;       /* seqid for create_session */
>  	u32			cl_exchange_flags;
> +	struct nfs4_sessionid	cl_sessionid;
> +
> +	struct svc_xprt		*cl_cb_xprt;	/* 4.1 callback transport */
>  #endif /* CONFIG_NFSD_V4_1 */
>  };
>  
> diff --git a/include/linux/nfsd/xdr4.h b/include/linux/nfsd/xdr4.h
> index 5c0d376..c7bf0a1 100644
> --- a/include/linux/nfsd/xdr4.h
> +++ b/include/linux/nfsd/xdr4.h
> @@ -360,8 +360,27 @@ struct nfsd4_exchange_id {
>  	int		spa_how;
>  };
>  
> +struct nfsd4_channel_attrs {
> +	u32		headerpadsz;
> +	u32		maxreq_sz;
> +	u32		maxresp_sz;
> +	u32		maxresp_cached;
> +	u32		maxops;
> +	u32		maxreqs;
> +	u32		nr_rdma_attrs;
> +	u32		rdma_attrs;
> +};
> +
>  struct nfsd4_create_session {
> -	int	foo;	/* stub */
> +	clientid_t		clientid;
> +	struct nfs4_sessionid	sessionid;
> +	u32			seqid;
> +	u32			flags;
> +	struct nfsd4_channel_attrs fore_channel;
> +	struct nfsd4_channel_attrs back_channel;
> +	u32			callback_prog;
> +	u32			uid;
> +	u32			gid;
>  };
>  
>  struct nfsd4_sequence {
> -- 
> 1.6.2.1
>