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

[William A. (Andy) Adamson]

i agree that we should ignore the implementation id.

-->Andy

On 3/8/07, Marc Eshel <eshel at almaden.ibm.com> wrote:
>
> 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.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://linux-nfs.org/pipermail/pnfs/attachments/20070308/884c40a8/attachment.htm