[pnfs] [PATCH] update exchange id reply to draft 9

[J. Bruce Fields]

On Wed, Mar 07, 2007 at 11:18:39PM -0800, Marc Eshel wrote:
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 5c4363f..8c35829 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
...
> +	/* implementation id */
> +	READ_BUF(4);
> +	READ32(res->impl_id.domain_len);
> +	READ_BUF(res->impl_id.domain_len);
> +	if (res->impl_id.domain_len < NFS4_NAME_LIMIT)
> +		COPYMEM(res->impl_id.domain, res->impl_id.domain_len);
> +	else {
> +		COPYMEM(res->impl_id.domain, NFS4_NAME_LIMIT);
> +		p += XDR_QUADLEN(res->impl_id.domain_len - NFS4_NAME_LIMIT);
> +		res->impl_id.domain_len = NFS4_NAME_LIMIT;
> +	}
> +	READ_BUF(4);
> +	READ32(res->impl_id.name_len);
> +	READ_BUF(res->impl_id.name_len);
> +	if (res->impl_id.name_len < NFS4_NAME_LIMIT)
> +		COPYMEM(res->impl_id.name, res->impl_id.name_len);
> +	else {
> +		COPYMEM(res->impl_id.name, NFS4_NAME_LIMIT);
> +		p += XDR_QUADLEN(res->impl_id.name_len - NFS4_NAME_LIMIT);
> +		res->impl_id.name_len = NFS4_NAME_LIMIT;
> +	}
> +	READ_BUF(12);
> +	READ64(res->impl_id.date.seconds);
> +	READ32(res->impl_id.date.nseconds);
>  
>  	return 0;
>  }

So I think we can just replace this whole thing by something like

	READ_BUF(4);
	READ32(len);
	READ_BUF(len); /* skip implementation id domain */
	READ_BUF(4);
	READ32(len);
	READ_BUF(len); /* skip implement id name */
	READ_BUF(12); /* skip implementation id timestamp */

and then remove the impl_id field.

> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 6e3c73a..4261b8c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -665,7 +665,7 @@ check_name(struct xdr_netobj name) {
>  
>  	if (name.len == 0) 
>  		return 0;
> -	if (name.len > NFS4_OPAQUE_LIMIT) {
> +	if (name.len > NFS4_NAME_LIMIT) {
>  		printk("NFSD: check_name: name too long(%d)!\n", name.len);
>  		return 0;
>  	}

This chunk isn't right; check_name() is used to check lengths of various
client-provided strings, which I think we must accept up to the full
NFS4_OPAQUE_LIMIT.

> @@ -2736,6 +2744,17 @@ nfsd4_encode_exchange_id(struct nfsd4_co
>                  WRITE32(server_scope_sz);
>                  WRITEMEM(server_scope, server_scope_sz);
>                  ADJUST_ARGS();
> +
> +		/* Implementation id */
> +		RESERVE_SPACE(4+domain_sz+4+name_sz+8+4);
> +		WRITE32(domain_sz);
> +		WRITEMEM(domain, domain_sz);
> +		WRITE32(name_sz);
> +		WRITEMEM(name, name_sz);
> +		WRITE64(clid->impl_id.date.seconds);
> +		WRITE32(clid->impl_id.date.nseconds);
> +
> +                ADJUST_ARGS();
>          }

Similarly, I think all this has to do is just write 5*4 zero bytes.

>  struct server_scope {
>  	uint32_t			server_scope_sz;
> -	char 				server_scope[NFS4_OPAQUE_LIMIT];
> +	char 				server_scope[NFS4_NAME_LIMIT];
>  };

This is a server-generated string, right?  So it should allow the full
NFS4_OPAQUE_LIMIT on the client side.

Note if we're including these buffers inline like this, that may mean we
want different structs on the client and server.  And that might allow
some other simplifications since the client and server may use them in
quite different ways.

--b.