2007-10-09 22:57:44

by Tom Tucker

[permalink] [raw]
Subject: [RFC,PATCH] svc: Add xprt header len to svc_deferred_req


A transport may have a header that precedes the RPC message. Save the
size of this header in the svc_deferred_req so that when the RPC
is revisited, the deferred recv function doesn't need to re-parse the
header to determine it's length.

In addition, a misleading comment related to the transport header
for TCP/UDP has been rewritten.

Signed-off-by: Tom Tucker <[email protected]>
---

include/linux/sunrpc/svc.h | 1 +
net/sunrpc/svc_xprt.c | 1 +
net/sunrpc/svcsock.c | 2 +-
3 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index ea07e3d..f2ada2a 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -323,6 +323,7 @@ struct svc_deferred_req {
size_t addrlen;
union svc_addr_u daddr; /* where reply must come from */
struct cache_deferred_req handle;
+ int xprt_hlen;
int argslen;
__be32 args[0];
};
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 080ecf5..b580d4c 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -884,6 +884,7 @@ static struct cache_deferred_req *svc_de
dr->addrlen = rqstp->rq_addrlen;
dr->daddr = rqstp->rq_daddr;
dr->argslen = (rqstp->rq_arg.len + rqstp->rq_xprt_hlen) >> 2;
+ dr->xprt_hlen = rqstp->rq_xprt_hlen;

/* back up head to the start of the buffer and copy */
skip = (rqstp->rq_arg.len + rqstp->rq_xprt_hlen) -
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index ec834a7..7838b27 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -393,7 +393,7 @@ svc_recvfrom(struct svc_rqst *rqstp, str
};
int len;

- /* TCP/UDP have no transport header */
+ /* UDP doesn't have a xprt header and TCP doesn't need to save it */
rqstp->rq_xprt_hlen = 0;

len = kernel_recvmsg(svsk->sk_sock, &msg, iov, nr, buflen,

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2007-10-10 02:31:20

by Greg Banks

[permalink] [raw]
Subject: Re: [RFC,PATCH] svc: Add xprt header len to svc_deferred_req

On Tue, Oct 09, 2007 at 05:57:40PM -0500, Tom Tucker wrote:
>
> A transport may have a header that precedes the RPC message. Save the
> size of this header in the svc_deferred_req so that when the RPC
> is revisited, the deferred recv function doesn't need to re-parse the
> header to determine it's length.
>
> In addition, a misleading comment related to the transport header
> for TCP/UDP has been rewritten.
>
> Signed-off-by: Tom Tucker <[email protected]>
> ---
>
> include/linux/sunrpc/svc.h | 1 +
> net/sunrpc/svc_xprt.c | 1 +
> net/sunrpc/svcsock.c | 2 +-
> 3 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index ea07e3d..f2ada2a 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -323,6 +323,7 @@ struct svc_deferred_req {
> size_t addrlen;
> union svc_addr_u daddr; /* where reply must come from */
> struct cache_deferred_req handle;
> + int xprt_hlen;
> int argslen;
> __be32 args[0];
> };
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 080ecf5..b580d4c 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -884,6 +884,7 @@ static struct cache_deferred_req *svc_de
> dr->addrlen = rqstp->rq_addrlen;
> dr->daddr = rqstp->rq_daddr;
> dr->argslen = (rqstp->rq_arg.len + rqstp->rq_xprt_hlen) >> 2;
> + dr->xprt_hlen = rqstp->rq_xprt_hlen;
>
> /* back up head to the start of the buffer and copy */
> skip = (rqstp->rq_arg.len + rqstp->rq_xprt_hlen) -

Are you planning on restoring rqstp->rq_xprt_hlen from
dr->xprt_hlen in svc_deferred_recv() ?

Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
Apparently, I'm Bedevere. Which MPHG character are you?
I don't speak for SGI.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-10-10 02:35:36

by Tom Tucker

[permalink] [raw]
Subject: Re: [RFC,PATCH] svc: Add xprt header len to svc_deferred_req

On Wed, 2007-10-10 at 12:39 +1000, Greg Banks wrote:
> On Tue, Oct 09, 2007 at 05:57:40PM -0500, Tom Tucker wrote:
> >
> > A transport may have a header that precedes the RPC message. Save the
> > size of this header in the svc_deferred_req so that when the RPC
> > is revisited, the deferred recv function doesn't need to re-parse the
> > header to determine it's length.
> >
> > In addition, a misleading comment related to the transport header
> > for TCP/UDP has been rewritten.
> >
> > Signed-off-by: Tom Tucker <[email protected]>
> > ---
> >
> > include/linux/sunrpc/svc.h | 1 +
> > net/sunrpc/svc_xprt.c | 1 +
> > net/sunrpc/svcsock.c | 2 +-
> > 3 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> > index ea07e3d..f2ada2a 100644
> > --- a/include/linux/sunrpc/svc.h
> > +++ b/include/linux/sunrpc/svc.h
> > @@ -323,6 +323,7 @@ struct svc_deferred_req {
> > size_t addrlen;
> > union svc_addr_u daddr; /* where reply must come from */
> > struct cache_deferred_req handle;
> > + int xprt_hlen;
> > int argslen;
> > __be32 args[0];
> > };
> > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> > index 080ecf5..b580d4c 100644
> > --- a/net/sunrpc/svc_xprt.c
> > +++ b/net/sunrpc/svc_xprt.c
> > @@ -884,6 +884,7 @@ static struct cache_deferred_req *svc_de
> > dr->addrlen = rqstp->rq_addrlen;
> > dr->daddr = rqstp->rq_daddr;
> > dr->argslen = (rqstp->rq_arg.len + rqstp->rq_xprt_hlen) >> 2;
> > + dr->xprt_hlen = rqstp->rq_xprt_hlen;
> >
> > /* back up head to the start of the buffer and copy */
> > skip = (rqstp->rq_arg.len + rqstp->rq_xprt_hlen) -
>
> Are you planning on restoring rqstp->rq_xprt_hlen from
> dr->xprt_hlen in svc_deferred_recv() ?

Well that's a really good question. I'm assuming RPC are never deferred
more than one. That seems naive looking at the code.

>
> Greg.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-10-10 02:38:29

by Greg Banks

[permalink] [raw]
Subject: Re: [RFC,PATCH] svc: Add xprt header len to svc_deferred_req

On Tue, Oct 09, 2007 at 09:34:21PM -0500, Tom Tucker wrote:
> On Wed, 2007-10-10 at 12:39 +1000, Greg Banks wrote:
> > On Tue, Oct 09, 2007 at 05:57:40PM -0500, Tom Tucker wrote:
> > >
> > Are you planning on restoring rqstp->rq_xprt_hlen from
> > dr->xprt_hlen in svc_deferred_recv() ?
>
> Well that's a really good question. I'm assuming RPC are never deferred
> more than one. That seems naive looking at the code.

Also, multiple other RPCs can be handled by the nfsd thread while
waiting for deferred RPCs, each of which can change rq_xprt_hlen
to different values.

Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
Apparently, I'm Bedevere. Which MPHG character are you?
I don't speak for SGI.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs