2013-11-13 16:21:16

by Christoph Hellwig

[permalink] [raw]
Subject: sharing protocol defintions between client and server?

>From a lot of the recent work it seems like there's basically no
sharing of protocol definitions between the Linux NFS client and
server, which seems fairly annoying to me. Is there a good historic
reason for this and did we ever attempt to change it? Maybe even
use some kernel-specific rpcgen variant to generate them directly
from the spec?


2013-11-14 19:35:34

by J. Bruce Fields

[permalink] [raw]
Subject: Re: sharing protocol defintions between client and server?

On Thu, Nov 14, 2013 at 08:40:46AM -0800, Christoph Hellwig wrote:
> On Thu, Nov 14, 2013 at 10:05:46AM -0500, J. Bruce Fields wrote:
> > On Wed, Nov 13, 2013 at 08:21:16AM -0800, Christoph Hellwig wrote:
> > > >From a lot of the recent work it seems like there's basically no
> > > sharing of protocol definitions between the Linux NFS client and
> > > server, which seems fairly annoying to me.
> >
> > What are you thinking of exactly?
> >
> > I suspect that there could be more sharing.
>
> To make it easy I'll just quote from the SEEK patches.
>
> client:
>
> +struct nfs42_seek_args {
> + struct nfs4_sequence_args seq_args;
> +
> + struct nfs_fh *sa_fh;
> + nfs4_stateid *sa_stateid;
> + u64 sa_offset;
> + u32 sa_what;
> +};
> +
> +struct nfs42_seek_res {
> + struct nfs4_sequence_res seq_res;
> + unsigned int status;
> +
> + u32 sr_eof;
> + u32 sr_whence;
> + u64 sr_offset;
> + u64 sr_length;
> + u32 sr_allocated;
> +};
> +#endif
>
> server:
>
> +struct nfsd4_seek {
> + /* request */
> + stateid_t seek_stateid;
> + loff_t seek_offset;
> + u32 seek_whence;
> +
> + /* response */
> + u64 seek_pos;
> + u32 seek_eof;
> + u64 seek_length;
> + u32 seek_allocated;
> +};
>
> note that a lot of server operations also seem to have separate
> args and result substrutures. In general I'd love to have one
> structure for the actual on-the wire operation in a header, and then
> client and server could build in-memory versions around them.
>
> Of course just generating those from the XDR would be even better.

Sometimes a lot of those fields are unused. And we can currently do
things like read/write directly from/to the data structures these are
eventually going to end up in.

So in theory the existing code might be more efficient.

In practice I don't know if it is. And it'd be nice to be able to
autogenerate huge swaths of boring code.

--b.

> Maybe I'll play around with doing a krpcgen that we can initially
> just use for producing the structures, for which it should be
> pretty clear benefit. If we're lucky we might be to also move
> some marshalling/unmarshalling over to it later.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-11-14 15:05:52

by J. Bruce Fields

[permalink] [raw]
Subject: Re: sharing protocol defintions between client and server?

On Wed, Nov 13, 2013 at 08:21:16AM -0800, Christoph Hellwig wrote:
> >From a lot of the recent work it seems like there's basically no
> sharing of protocol definitions between the Linux NFS client and
> server, which seems fairly annoying to me.

What are you thinking of exactly?

I suspect that there could be more sharing.

> Is there a good historic
> reason for this and did we ever attempt to change it? Maybe even
> use some kernel-specific rpcgen variant to generate them directly
> from the spec?

I don't know if anyone's looked into it.

There's a lot of hand-written xdr code (the encoders/decoders
themselves, the maxsz constants in fs/nfs/nfs*xdr.c, the _rsize junk in
fs/nfsd/nfs4proc.c, ...). It seems error-prone to me though I can't
recall if we've had a lot of bugs there in practice.

--b.

2013-11-14 16:40:46

by Christoph Hellwig

[permalink] [raw]
Subject: Re: sharing protocol defintions between client and server?

On Thu, Nov 14, 2013 at 10:05:46AM -0500, J. Bruce Fields wrote:
> On Wed, Nov 13, 2013 at 08:21:16AM -0800, Christoph Hellwig wrote:
> > >From a lot of the recent work it seems like there's basically no
> > sharing of protocol definitions between the Linux NFS client and
> > server, which seems fairly annoying to me.
>
> What are you thinking of exactly?
>
> I suspect that there could be more sharing.

To make it easy I'll just quote from the SEEK patches.

client:

+struct nfs42_seek_args {
+ struct nfs4_sequence_args seq_args;
+
+ struct nfs_fh *sa_fh;
+ nfs4_stateid *sa_stateid;
+ u64 sa_offset;
+ u32 sa_what;
+};
+
+struct nfs42_seek_res {
+ struct nfs4_sequence_res seq_res;
+ unsigned int status;
+
+ u32 sr_eof;
+ u32 sr_whence;
+ u64 sr_offset;
+ u64 sr_length;
+ u32 sr_allocated;
+};
+#endif

server:

+struct nfsd4_seek {
+ /* request */
+ stateid_t seek_stateid;
+ loff_t seek_offset;
+ u32 seek_whence;
+
+ /* response */
+ u64 seek_pos;
+ u32 seek_eof;
+ u64 seek_length;
+ u32 seek_allocated;
+};

note that a lot of server operations also seem to have separate
args and result substrutures. In general I'd love to have one
structure for the actual on-the wire operation in a header, and then
client and server could build in-memory versions around them.

Of course just generating those from the XDR would be even better.

Maybe I'll play around with doing a krpcgen that we can initially
just use for producing the structures, for which it should be
pretty clear benefit. If we're lucky we might be to also move
some marshalling/unmarshalling over to it later.