[pnfs] [PATCH 1/5] nfs41: exchange_id operation
Andy Adamson
andros at netapp.com
Mon Jul 28 10:48:59 EDT 2008
On Sun, 2008-07-27 at 19:27 +0300, Benny Halevy wrote:
> On Wed, 16 Jul 2008 12:29:58 +0300 Benny Halevy <bhalevy at panasas.com> wrote:
> > Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> >
> > nfs41: sessions exchange_id AUTH_SYS root cred
> >
> > nfs41: WORKAROUND: turn off state renewal
> >
> > The NFSv4.1 sched_state_renewal function nfs41_proc_async_sequence() fails
> > to set any fields in the struct nfs41_sequence_res which means that
> > nfs41_sequence_done() has a garbage slot (slot = res->sr_slot), and causes a
> > seg fault.
> >
> > Futhermore, the credential for state renewal is obtained by
> > nfs4_get_renew_cred() which looks for an open state credential. This will fail
> > for exchange_id, create_session, etc...
> >
> > Fortunately, the session state recovery code works!
> >
> > Turn off NFSv4.1 state renewal until fixed.
> >
> > Signed-off-by: Andy Adamson<andros at umich.edu>
> > Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> >
> > Unlike NFSv4.0, NFSv4.1 requires machine credentials. RPC_AUTH_GSS machine
> > credentials will be passed into the kernel at mount time to be available for
> > the exchange_id operation.
> >
> > RPC_AUTH_UNIX root mounts can use the UNIX root credential. Store the root
> > credential in the nfs_client struct.
> >
> > Without a credential, NFSv4.1 state renewal fails.
> >
> > Signed-off-by: Andy Adamson<andros at umich.edu>
> > Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> >
> > nfs41: return negative error on EXCHANGE_ID
> >
> > Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> >
> > nfs41: fix cl_ex_cred
> >
> > Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> >
> > nfs41: establish clientid via exchange id only if cred != NULL
> >
> > From 2.6.26 reclaimer() uses machine cred for setting up the client id
> > therefore it is never expected to be NULL.
> >
> > Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> >
> > nfs41: fix exchange_id op
> >
> > unexport rpcauth_lookupcred
> > make nfs4_get_machine_cred static again
> > split off get_state_renewal_cred into a different patch
> >
> > Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> >
> > nfs41: don't set session state in exchange id
> >
> > Since the clientid has expired, all compounds using sessions
> > associated with the stale clientid are trying to use bad sessions and
> > are in some stage of reset.
> >
> > Signed-off-by: Andy Adamson<andros at netapp.com>
> > Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> > ---
> > fs/nfs/nfs4proc.c | 57 ++++++++++++++++++++++++-
> > fs/nfs/nfs4xdr.c | 102 ++++++++++++++++++++++++++++++++++++++++----
> > include/linux/nfs_fs_sb.h | 7 +++
> > include/linux/nfs_xdr.h | 41 ++++++++++++++++++
> > 4 files changed, 196 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index a513705..e4adba8 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -48,6 +48,7 @@
> > #include <linux/smp_lock.h>
> > #include <linux/namei.h>
> > #include <linux/mount.h>
> > +#include <linux/module.h>
> >
> > #include "nfs4_fs.h"
> > #include "delegation.h"
> > @@ -4235,9 +4236,63 @@ int nfs4_proc_fs_locations(struct inode *dir, const struct qstr *name,
> > }
> >
> > #ifdef CONFIG_NFS_V4_1
> > +/*
> > + * nfs4_proc_exchange_id()
> > + *
> > + * Since the clientid has expired, all compounds using sessions
> > + * associated with the stale clientid will be returning
> > + * NFS4ERR_BADSESSION in the sequence operation, and will therefore
> > + * be in some phase of session reset.
> > + */
> > static int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
> > {
> > - return -1; /* stub */
> > + nfs4_verifier verifier;
> > + struct nfs41_exchange_id_args args = {
> > + .flags = clp->cl_exchange_flags,
> > + };
> > + struct nfs41_exchange_id_res res = {
> > + .client = clp,
> > + };
> > + int status;
> > + int loop = 0;
> > + struct rpc_message msg = {
> > + .rpc_proc = nfs4_proc(clp, NFSPROC4_CLNT_EXCHANGE_ID),
> > + .rpc_argp = &args,
> > + .rpc_resp = &res,
> > + .rpc_cred = cred,
> > + };
> > + u32 *p;
>
> This should be __be32 *p (see nfs4_proc_setclientid)
>
> > +
> > + dprintk("--> %s\n", __func__);
> > + BUG_ON(clp == NULL);
> > + p = (u32 *)verifier.data;
> > + *p++ = htonl((u32)clp->cl_boot_time.tv_sec);
> > + *p = htonl((u32)clp->cl_boot_time.tv_nsec);
> > + args.verifier = &verifier;
> > +
> > + while (1) {
> > + args.id_len = scnprintf(args.id, sizeof(args.id),
> > + "%s/%u.%u.%u.%u %s %u",
> > + clp->cl_ipaddr,
> > + NIPQUAD(clp->cl_addr.__data),
>
> Using rpc_peeraddr2str seems more appropriate here
> so this wouldn't be IPv4 specific.
>
> > + "AUTH_SYS", clp->cl_id_uniquifier);
>
> what do we need the string AUTH_SYS for?
>
> > +
> > + status = rpc_call_sync(clp->cl_rpcclient, &msg, 0);
> > +
> > + if (status != NFS4ERR_CLID_INUSE)
> > + break;
> > +
> > + if (signalled())
> > + break;
> > +
> > + if (loop++ & 1)
> > + ssleep(clp->cl_lease_time + 1);
>
> Why retry after sleeping would help NFS4ERR_CLID_INUSE?
> Is this an historic hack that can be undone by now?
Waiting a lease period allows the server to throw away a clientid
previously established by this client. Of course, it doesn't help when
the conflicting clientid was established by another client.
>
> > + else if (++clp->cl_id_uniquifier == 0)
> > + break;
> > + }
> > +
> > + dprintk("<-- %s status= %d\n", __func__, status);
> > + return status;
> > }
> >
> > /* Initialize a slot table */
> > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> > index abe46e8..8863f1a 100644
> > --- a/fs/nfs/nfs4xdr.c
> > +++ b/fs/nfs/nfs4xdr.c
> > @@ -243,9 +243,17 @@ static int nfs4_stat_to_errno(int);
> >
> > #if defined(CONFIG_NFS_V4_1)
> > #define encode_exchange_id_maxsz (op_encode_hdr_maxsz + \
> > - 0 /* stub */)
> > + 4 /*server->ip_addr*/ + \
> > + 1 /*netid*/ + \
> > + 3 /*cred name*/ + \
> > + 1 /*id_uniquifier*/ + \
>
> eh?
>
> > + (NFS4_VERIFIER_SIZE >> 2) + \
> > + 1 /*flags*/ + \
>
> 1 /* zero length state_protect4_a */ + \
>
> > + 1 /*zero implemetation id array*/)
>
> also, add spaces in comments.
>
> > #define decode_exchange_id_maxsz (op_decode_hdr_maxsz + \
> > - 0 /* stub */)
> > + 2 + 1 + 1 + 2 + 1 + \
>
> let's see:
> 2 /* eir_clientid */
> 1 /* eir_sequenceid */
> 1 /* eir_flags */
> 1 /* state_protect4_r.spr_how */
> 0 /* SP4_NONE (for now) */
> 2 /* eir_server_owner.so_minor_id */
>
> > + (NFS4_OPAQUE_LIMIT >> 2) + 1 + \
>
> I guess that's for eir_server_owner.so_major_id
>
> > + (NFS4_OPAQUE_LIMIT >> 2) + 1)
>
> And that's for eir_server_scope
>
> missing:
>
> 1 /* eir_server_impl_id array length */ +
>
> 0 /* contents of eir_server_impl_id are ignored */
>
> > #define encode_sequence_maxsz (op_encode_hdr_maxsz + \
> > 0 /* stub */)
> > #define decode_sequence_maxsz (op_decode_hdr_maxsz + \
> > @@ -655,9 +663,11 @@ static int nfs4_stat_to_errno(int);
> > #define NFS41_dec_fs_locations_sz (NFS40_dec_fs_locations_sz + \
> > decode_sequence_maxsz)
> > #define NFS41_enc_exchange_id_sz \
> > - 0 /* stub */
> > + (compound_encode_hdr_maxsz + \
> > + encode_exchange_id_maxsz)
> > #define NFS41_dec_exchange_id_sz \
> > - 0 /* stub */
> > + (compound_decode_hdr_maxsz + \
> > + decode_exchange_id_maxsz)
> > #define NFS41_enc_create_session_sz \
> > 0 /* stub */
> > #define NFS41_dec_create_session_sz \
> > @@ -1529,9 +1539,23 @@ static int encode_delegreturn(struct xdr_stream *xdr, const nfs4_stateid *statei
> > #if defined(CONFIG_NFS_V4_1)
> > /* NFSv4.1 operations */
> > static int encode_exchange_id(struct xdr_stream *xdr,
> > - void *dummy)
> > + struct nfs41_exchange_id_args *args)
> > {
> > - return -1; /* stub */
> > +
> > + uint32_t *p;
> > +
> > + RESERVE_SPACE(4 + sizeof(args->verifier->data));
> > + WRITE32(OP_EXCHANGE_ID);
> > + WRITEMEM(args->verifier->data, sizeof(args->verifier->data));
> > +
> > + encode_string(xdr, args->id_len, args->id);
> > +
> > + RESERVE_SPACE(12);
> > + WRITE32(args->flags);
> > + WRITE32(0); /* zero length state_protect4_a */
> > + WRITE32(0); /* zero length implementation id array */
>
> replace spaces with tab
>
> > +
> > + return 0;
> > }
> >
> > static int encode_create_session(struct xdr_stream *xdr,
> > @@ -3054,7 +3078,17 @@ static int nfs41_xdr_enc_fs_locations(struct rpc_rqst *req, __be32 *p,
> > static int nfs41_xdr_enc_exchange_id(struct rpc_rqst *req, uint32_t *p,
> > void *args)
> > {
> > - return -1; /* stub */
> > + struct xdr_stream xdr;
> > + struct compound_hdr hdr = {
> > + .nops = 1,
> > + };
> > +
> > + xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> > + encode_compound_hdr(&xdr, &hdr, 1);
> > +
> > + encode_exchange_id(&xdr, args);
> > +
> > + return 0;
> > }
> >
> > /*
> > @@ -4740,9 +4774,49 @@ static int decode_delegreturn(struct xdr_stream *xdr)
> >
> > #if defined(CONFIG_NFS_V4_1)
> > static int decode_exchange_id(struct xdr_stream *xdr,
> > - void *dummy)
> > + struct nfs41_exchange_id_res *res)
> > {
> > - return -1; /* stub */
> > + uint32_t *p;
> > + int status, dummy;
> > + struct nfs_client *clp = res->client;
> > +
> > + status = decode_op_hdr(xdr, OP_EXCHANGE_ID);
> > + if (status)
> > + return status;
> > +
> > + READ_BUF(8);
> > + READ64(clp->cl_clientid);
> > + READ_BUF(12);
> > + READ32(clp->cl_seqid);
> > + READ32(clp->cl_exchange_flags);
> > +
> > + /* We ask for SP4_NONE */
> > + READ32(dummy);
> > + if (dummy != SP4_NONE)
> > + return -EIO;
> > +
> > + /* minor_id */
> > + READ_BUF(8);
> > + READ64(res->server_owner.minor_id);
> > +
> > + /* Major id */
> > + READ_BUF(4);
> > + READ32(res->server_owner.major_id_sz);
> > + READ_BUF(res->server_owner.major_id_sz);
> > + COPYMEM(res->server_owner.major_id, res->server_owner.major_id_sz);
> > +
> > + /* server_scope */
> > + READ_BUF(4);
> > + READ32(res->server_scope.server_scope_sz);
> > + READ_BUF(res->server_scope.server_scope_sz);
> > + COPYMEM(res->server_scope.server_scope,
> > + res->server_scope.server_scope_sz);
> > + /* Throw away Implementation id array */
> > + READ_BUF(4);
> > + READ32(dummy);
> > + p += XDR_QUADLEN(dummy);
>
> This is a bug. it works since we apparently only
> received dummy == 0 so far.
> Need to loop on dummy, as it is the array count
> (FWIW, it must be either 0 or 1 but we don't really care)
> then decode and discard the the impl_id contents properly.
>
> > +
> > + return 0;
> > }
> >
> > static int decode_create_session(struct xdr_stream *xdr,
> > @@ -6365,7 +6439,15 @@ static int nfs40_xdr_dec_setclientid(struct rpc_rqst *req, __be32 *p,
> > static int nfs41_xdr_dec_exchange_id(struct rpc_rqst *rqstp, uint32_t *p,
> > void *res)
> > {
> > - return -1; /* stub */
> > + struct xdr_stream xdr;
> > + struct compound_hdr hdr;
> > + int status;
> > +
> > + xdr_init_decode(&xdr, &rqstp->rq_rcv_buf, p);
> > + status = decode_compound_hdr(&xdr, &hdr);
> > + if (!status)
> > + status = decode_exchange_id(&xdr, res);
> > + return status;
> > }
> >
> > /*
> > diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> > index 7505e9b..1c320ad 100644
> > --- a/include/linux/nfs_fs_sb.h
> > +++ b/include/linux/nfs_fs_sb.h
> > @@ -76,6 +76,13 @@ struct nfs_client {
> > unsigned char cl_id_uniquifier;
> > u32 cl_minorversion;
> > #endif /* CONFIG_NFS_V4 */
> > +
> > +#ifdef CONFIG_NFS_V4_1
> > + /* The sequence id to use for the next CREATE_SESSION */
> > + u32 cl_seqid;
> > + /* The flags used for obtaining the clientid during EXCHANGE_ID */
> > + u32 cl_exchange_flags;
> > +#endif /* CONFIG_NFS_V4_1 */
> > };
> >
> > #ifdef CONFIG_NFS_V4
> > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > index 82c970e..7efd666 100644
> > --- a/include/linux/nfs_xdr.h
> > +++ b/include/linux/nfs_xdr.h
> > @@ -965,6 +965,47 @@ struct nfs4_fs_locations_res {
> >
> > #endif /* CONFIG_NFS_V4 */
> >
> > +#ifdef CONFIG_NFS_V4_1
> > +struct nfstime4 {
> > + u64 seconds;
> > + u32 nseconds;
> > +};
> > +
> > +struct nfs_impl_id4 {
> > + u32 domain_len;
> > + char *domain;
> > + u32 name_len;
> > + char *name;
> > + struct nfstime4 date;
> > +};
> > +
> > +struct nfs41_exchange_id_args {
> > + nfs4_verifier *verifier;
> > + unsigned int id_len;
> > + char id[48];
>
> make room for IPv6 addresses and put a comment
> explaining the string length.
>
> > + u32 flags;
> > +};
> > +
> > +struct server_owner {
> > + uint64_t minor_id;
> > + uint32_t major_id_sz;
> > + char major_id[NFS4_OPAQUE_LIMIT];
> > +};
> > +
> > +struct server_scope {
> > + uint32_t server_scope_sz;
> > + char server_scope[NFS4_OPAQUE_LIMIT];
> > +};
> > +
> > +struct nfs41_exchange_id_res {
> > + struct nfs_client *client;
> > + u32 flags;
> > + struct server_owner server_owner;
> > + struct server_scope server_scope;
> > + struct nfs_impl_id4 impl_id;
>
> impl_id not used. comment it out.
>
> > +};
> > +#endif /* CONFIG_NFS_V4_1 */
> > +
> > struct nfs_page;
> >
> > #define NFS_PAGEVEC_SIZE (8U)
More information about the pNFS
mailing list