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

[Marc Eshel]

OK, if we all agree that we want to completely ignore implementation id 
that I will redo the patch. Bruc, READ_BUF doesn't move p forward maybe 
there some other macro to do it but for now I will use "p += 
XDR_QUADLEN(len)". I see that much of the new code was not tabs and I 
change only new line, should we add tabs any time we see it missing even 
for unrelated fix ?
Marc. 
 
"J. Bruce Fields" <bfields at fieldses.org> wrote on 03/08/2007 07:50:51 AM:

> 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.