[RFC,PATCH 5/14] knfsd: max_payload per transport

[RFC,PATCH 5/14] knfsd: max_payload per transport

[Greg Banks]

Make svc_max_payload() delegate to a new method in svc_sock_ops
instead of reaching into the socket (beause later the NFS/RDMA
transport will not even have a socket).

Signed-off-by: Greg Banks <gnb@melbourne.sgi.com>
Signed-off-by: Peter Leckie <pleckie@melbourne.sgi.com>
---

 include/linux/sunrpc/svcsock.h |    6 ++++++
 net/sunrpc/svc.c               |    5 ++---
 net/sunrpc/svcsock.c           |   16 ++++++++++++++--
 3 files changed, 22 insertions(+), 5 deletions(-)

Index: linux/include/linux/sunrpc/svcsock.h
===================================================================
--- linux.orig/include/linux/sunrpc/svcsock.h	2007-05-17 00:46:47.944827892 +1000
+++ linux/include/linux/sunrpc/svcsock.h	2007-05-17 01:28:41.131377156 +1000
@@ -37,6 +37,12 @@ struct svc_sock_ops {
 	 * Return 1 if sufficient space to write reply to network.
 	 */
 	int			(*sko_has_wspace)(struct svc_sock *);
+	/*
+	 * Return the largest payload (i.e. READ, WRITE or READDIR
+	 * data length not including NFS headers) supported by the
+	 * svc_sock.
+	 */
+	u32			(*sko_max_payload)(struct svc_sock *);
 };
 
 /*
Index: linux/net/sunrpc/svc.c
===================================================================
--- linux.orig/net/sunrpc/svc.c	2007-05-17 00:36:20.557193782 +1000
+++ linux/net/sunrpc/svc.c	2007-05-17 01:28:25.217367463 +1000
@@ -1020,10 +1020,9 @@ err_bad:
  */
 u32 svc_max_payload(const struct svc_rqst *rqstp)
 {
-	int max = RPCSVC_MAXPAYLOAD_TCP;
+	struct svc_sock *svsk = rqstp->rq_sock;
+	int max = svsk->sk_ops->sko_max_payload(svsk);
 
-	if (rqstp->rq_sock->sk_sock->type == SOCK_DGRAM)
-		max = RPCSVC_MAXPAYLOAD_UDP;
 	if (rqstp->rq_server->sv_max_payload < max)
 		max = rqstp->rq_server->sv_max_payload;
 	return max;
Index: linux/net/sunrpc/svcsock.c
===================================================================
--- linux.orig/net/sunrpc/svcsock.c	2007-05-17 00:49:50.820303639 +1000
+++ linux/net/sunrpc/svcsock.c	2007-05-17 01:28:25.221366963 +1000
@@ -910,6 +910,11 @@ svc_udp_has_wspace(struct svc_sock *svsk
 	return svc_sock_has_write_space(svsk, sock_wspace(svsk->sk_sk));
 }
 
+static u32 svc_udp_max_payload(struct svc_sock *svsk)
+{
+	return RPCSVC_MAXPAYLOAD_UDP;
+}
+
 static const struct svc_sock_ops svc_udp_ops = {
 	.sko_name = "udp",
 	.sko_recvfrom = svc_udp_recvfrom,
@@ -917,7 +922,8 @@ static const struct svc_sock_ops svc_udp
 	.sko_detach = svc_tcpip_detach,
 	.sko_free = svc_tcpip_free,
 	.sko_prepare_reply = svc_tcpip_prepare_reply,
-	.sko_has_wspace = svc_udp_has_wspace
+	.sko_has_wspace = svc_udp_has_wspace,
+	.sko_max_payload = svc_udp_max_payload
 };
 
 static void
@@ -1378,6 +1384,11 @@ svc_tcp_has_wspace(struct svc_sock *svsk
 	return svc_sock_has_write_space(svsk, sk_stream_wspace(svsk->sk_sk));
 }
 
+static u32 svc_tcp_max_payload(struct svc_sock *svsk)
+{
+	return RPCSVC_MAXPAYLOAD_TCP;
+}
+
 static const struct svc_sock_ops svc_tcp_ops = {
 	.sko_name = "tcp",
 	.sko_recvfrom = svc_tcp_recvfrom,
@@ -1385,7 +1396,8 @@ static const struct svc_sock_ops svc_tcp
 	.sko_detach = svc_tcpip_detach,
 	.sko_free = svc_tcpip_free,
 	.sko_prepare_reply = svc_tcp_prepare_reply,
-	.sko_has_wspace = svc_tcp_has_wspace
+	.sko_has_wspace = svc_tcp_has_wspace,
+	.sko_max_payload = svc_tcp_max_payload
 };
 
 static void
-- 
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 DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 5/14] knfsd: max_payload per transport

[Neil Brown]

On Thursday May 17, gnb@sgi.com wrote:
> 
> Make svc_max_payload() delegate to a new method in svc_sock_ops
> instead of reaching into the socket (beause later the NFS/RDMA
> transport will not even have a socket).
> 
> Signed-off-by: Greg Banks <gnb@melbourne.sgi.com>
> Signed-off-by: Peter Leckie <pleckie@melbourne.sgi.com>
..
>  
> +static u32 svc_tcp_max_payload(struct svc_sock *svsk)
> +{
> +	return RPCSVC_MAXPAYLOAD_TCP;
> +}
> +

Seeing these are implementation (or protocol) defined constants, do we
really need a function call?  How about a
	int max_payload;
in struct svc_sock_ops??  Or is it tacky to put an integer in a *_ops
structure? 

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 5/14] knfsd: max_payload per transport

[Tom Tucker]

On Thu, 2007-05-17 at 20:34 +1000, Neil Brown wrote:
> On Thursday May 17, gnb@sgi.com wrote:
> > 
> > Make svc_max_payload() delegate to a new method in svc_sock_ops
> > instead of reaching into the socket (beause later the NFS/RDMA
> > transport will not even have a socket).
> > 
> > Signed-off-by: Greg Banks <gnb@melbourne.sgi.com>
> > Signed-off-by: Peter Leckie <pleckie@melbourne.sgi.com>
> ..
> >  
> > +static u32 svc_tcp_max_payload(struct svc_sock *svsk)
> > +{
> > +	return RPCSVC_MAXPAYLOAD_TCP;
> > +}
> > +
> 
> Seeing these are implementation (or protocol) defined constants, do we
> really need a function call?  How about a
> 	int max_payload;
> in struct svc_sock_ops??  Or is it tacky to put an integer in a *_ops
> structure? 

I think if we called it a "svc_transport" structure instead of a
"svc_sock_ops" structure, it removes the tackiness. It's also more
accurate since "svc_sock_ops" implies these are socket ops which they
are not.




-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 5/14] knfsd: max_payload per transport

[Greg Banks]

On Thu, May 17, 2007 at 10:23:06AM -0500, Tom Tucker wrote:
> On Thu, 2007-05-17 at 20:34 +1000, Neil Brown wrote:
> > On Thursday May 17, gnb@sgi.com wrote:
> > > 
> > > Make svc_max_payload() delegate to a new method in svc_sock_ops
> > > instead of reaching into the socket (beause later the NFS/RDMA
> > > transport will not even have a socket).
> > > 
> > > Signed-off-by: Greg Banks <gnb@melbourne.sgi.com>
> > > Signed-off-by: Peter Leckie <pleckie@melbourne.sgi.com>
> > ..
> > >  
> > > +static u32 svc_tcp_max_payload(struct svc_sock *svsk)
> > > +{
> > > +	return RPCSVC_MAXPAYLOAD_TCP;
> > > +}
> > > +
> > 
> > Seeing these are implementation (or protocol) defined constants, do we
> > really need a function call?  How about a
> > 	int max_payload;
> > in struct svc_sock_ops??  Or is it tacky to put an integer in a *_ops
> > structure? 
> 
> I think if we called it a "svc_transport" structure instead of a
> "svc_sock_ops" structure, it removes the tackiness. It's also more
> accurate since "svc_sock_ops" implies these are socket ops which they
> are not.

I like this idea, so I renamed

svc_sock_ops -> svc_transport_type
sko_foo -> st_foo
sk_ops -> sk_type

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 DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 5/14] knfsd: max_payload per transport

[Greg Banks]

On Thu, May 17, 2007 at 08:34:10PM +1000, Neil Brown wrote:
> On Thursday May 17, gnb@sgi.com wrote:
> > 
> > Make svc_max_payload() delegate to a new method in svc_sock_ops
> > instead of reaching into the socket (beause later the NFS/RDMA
> > transport will not even have a socket).
> > 
> > Signed-off-by: Greg Banks <gnb@melbourne.sgi.com>
> > Signed-off-by: Peter Leckie <pleckie@melbourne.sgi.com>
> ..
> >  
> > +static u32 svc_tcp_max_payload(struct svc_sock *svsk)
> > +{
> > +	return RPCSVC_MAXPAYLOAD_TCP;
> > +}
> > +
> 
> Seeing these are implementation (or protocol) defined constants, do we
> really need a function call?  How about a
> 	int max_payload;
> in struct svc_sock_ops?? 

These ones are indeed constants, as is RDMA's.

> Or is it tacky to put an integer in a *_ops
> structure? 

No tackier than putting a string name like I did, or the things done
in tcp_congestion_ops or sock_request_ops.  It comes down to whether
you want to allow future protocols to have variable limits, and right
now that seems unnecesssary.

I'll change the field to an int.

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 DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs