From: "J. Bruce Fields" Subject: Re: [PATCH v2 23/47] nfsd41: create_session operation Date: Tue, 31 Mar 2009 19:38:51 -0400 Message-ID: <20090331233851.GB23198@fieldses.org> References: <49CDDFC2.4070402@panasas.com> <1238229177-10984-1-git-send-email-bhalevy@panasas.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-nfs@vger.kernel.org, pnfs@linux-nfs.org To: Benny Halevy Return-path: Received: from mail.fieldses.org ([141.211.133.115]:58649 "EHLO pickle.fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1765048AbZCaXiy (ORCPT ); Tue, 31 Mar 2009 19:38:54 -0400 In-Reply-To: <1238229177-10984-1-git-send-email-bhalevy@panasas.com> Sender: linux-nfs-owner@vger.kernel.org List-ID: On Sat, Mar 28, 2009 at 11:32:57AM +0300, Benny Halevy wrote: > From: Andy Adamson > > 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. The above two paragraphs just summarize the code step-by-step; that doesn't belong in the commit message. > > 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. I think this might all be clearer if we started with a #define for the maximum cached response size, and then calculated NFSD_PAGES_PER_SLOT from that. Also, factor in the +1 (or +3, or whatever it should be) into the PAGES_PER_SLOT so we don't have to remember to do that everywhere. > 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 > Signed-off-by: Dean Hildebrand > [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 > Signed-off-by: Benny Halevy > [use sessionid_lock spin lock] > [nfsd41: use bool inuse for slot state] > Signed-off-by: Benny Halevy > --- > 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); Don't use the sv_lock for this. (I greatly appreciate any help with the locking, but the difficult part isn't adding lock calls, but developing and explaining clear rules about what locks are taken where, and why.) > + > + if (np <= 0) { It's too late--we've already increased sv_drc_pages_used. This check needs to happen earlier. > + status = nfserr_resource; > + fchan->maxreqs = 0; > + } else > + fchan->maxreqs = np / NFSD_PAGES_PER_SLOT; Check to make sure that's not zero? > + > + 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*/ Space after "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; Just return. > + > + /* 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; Let's just allocate the session and slot table as one chunk of size (sizeof(struct nfsd4_session)+ numslots * sizeof(struct nfsd4_slot). > + > + for (i = 0; i < new->se_fnumslots; i++) > + new->se_slots[i].sl_session = new; And let's get rid of sl_session. > + > + 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); > + > + new->se_flags = cses->flags; > + kref_init(&new->se_ref); > + INIT_LIST_HEAD(&new->se_hash); > + INIT_LIST_HEAD(&new->se_perclnt); These INIT_LIST_HEAD's are redundant given the immediately following list_add's. > + 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)) { > + 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; > + } > + > + 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 >