Re: [PATCH 12/19] NFS/SUNRPC: support transport protocol naming

[Chuck Lever <chuck.lever@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 5505 bytes --]

Just a general comment.

I don't see how this will make it easier to add NFS client support for 
new RPC transports.  I thought we recently decided to pass down a 
string, and let the transports themselves register a netid that matches it?

For example, these changes are required to support RDMA -- you can't 
just ship a new RDMA SUNRPC transport module and expect it to work 
without this ULP change.  Likewise, if we add SCTP sometime in the 
future, will we want to make this same kind of modification to the NFS 
client before the SCTP SUNRCP transport module works correctly?

Talpey, Thomas wrote:
> NFS/SUNRPC: support transport protocol naming
> 
> To prepare for including non-sockets-based RPC transports, select
> RPC transports by an identifier (to be used in following patches).
> 
> Signed-off-by: Tom Talpey <tmt@netapp.com>
> 
> ---
> 
>  include/linux/sunrpc/xprt.h     |    5 ++---
>  include/linux/sunrpc/xprtsock.h |   11 +++++++++++
>  net/sunrpc/clnt.c               |    2 +-
>  net/sunrpc/xprt.c               |    8 +++-----
>  net/sunrpc/xprtsock.c           |    6 ++----
>  5 files changed, 19 insertions(+), 13 deletions(-)
> 
> Index: kernel/include/linux/sunrpc/xprt.h
> ===================================================================
> --- kernel.orig/include/linux/sunrpc/xprt.h
> +++ kernel/include/linux/sunrpc/xprt.h
> @@ -187,7 +187,7 @@ struct rpc_xprt {
>  };
>  
>  struct xprt_create {
> -	int			proto;		/* IPPROTO_UDP or IPPROTO_TCP */
> +	int			ident;		/* XPRT_TRANSPORT identifier */
>  	struct sockaddr *	srcaddr;	/* optional local address */
>  	struct sockaddr *	dstaddr;	/* remote peer address */
>  	size_t			addrlen;
> @@ -196,8 +196,7 @@ struct xprt_create {
>  
>  struct xprt_class {
>  	struct list_head	list;
> -	unsigned short		family;
> -	int			protocol;
> +	int			ident;		/* XPRT_TRANSPORT identifier */
>  	struct rpc_xprt *	(*setup)(struct xprt_create *);
>  	struct module		*owner;
>  	char			name[32];
> Index: kernel/include/linux/sunrpc/xprtsock.h
> ===================================================================
> --- kernel.orig/include/linux/sunrpc/xprtsock.h
> +++ kernel/include/linux/sunrpc/xprtsock.h
> @@ -19,6 +19,17 @@ int		init_socket_xprt(void);
>  void		cleanup_socket_xprt(void);
>  
>  /*
> + * RPC transport identifiers for UDP, TCP
> + *
> + * To preserve compatibility with the historical use of raw IP protocol
> + * id's for transport selection, these are specified with the previous
> + * values. No such restriction exists for new transports, except that
> + * they may not collide with these values (17 and 6, respectively).
> + */
> +#define XPRT_TRANSPORT_UDP	IPPROTO_UDP
> +#define XPRT_TRANSPORT_TCP	IPPROTO_TCP
> +
> +/*
>   * RPC slot table sizes for UDP, TCP transports
>   */
>  extern unsigned int xprt_udp_slot_table_entries;
> Index: kernel/net/sunrpc/xprt.c
> ===================================================================
> --- kernel.orig/net/sunrpc/xprt.c
> +++ kernel/net/sunrpc/xprt.c
> @@ -104,7 +104,7 @@ int xprt_register_transport(struct xprt_
>  	spin_lock(&xprt_list_lock);
>  	list_for_each_entry(t, &xprt_list, list) {
>  		/* don't register the same transport class twice */
> -		if (t == transport)
> +		if (t->ident == transport->ident)
>  			goto out;
>  	}
>  
> @@ -987,15 +987,13 @@ struct rpc_xprt *xprt_create_transport(s
>  
>  	spin_lock(&xprt_list_lock);
>  	list_for_each_entry(t, &xprt_list, list) {
> -		if ((t->family == args->dstaddr->sa_family) &&
> -		    (t->protocol == args->proto)) {
> +		if (t->ident == args->ident) {
>  			spin_unlock(&xprt_list_lock);
>  			goto found;
>  		}
>  	}
>  	spin_unlock(&xprt_list_lock);
> -	printk(KERN_ERR "RPC: transport (%u/%d) not supported\n",
> -			args->dstaddr->sa_family, args->proto);
> +	printk(KERN_ERR "RPC: transport (%d) not supported\n", args->ident);
>  	return ERR_PTR(-EIO);
>  
>  found:
> Index: kernel/net/sunrpc/xprtsock.c
> ===================================================================
> --- kernel.orig/net/sunrpc/xprtsock.c
> +++ kernel/net/sunrpc/xprtsock.c
> @@ -1955,8 +1955,7 @@ static struct xprt_class	xs_udp_transpor
>  	.list		= LIST_HEAD_INIT(xs_udp_transport.list),
>  	.name		= "udp",
>  	.owner		= THIS_MODULE,
> -	.family		= AF_INET,
> -	.protocol	= IPPROTO_UDP,
> +	.ident		= IPPROTO_UDP,
>  	.setup		= xs_setup_udp,
>  };
>  
> @@ -1964,8 +1963,7 @@ static struct xprt_class	xs_tcp_transpor
>  	.list		= LIST_HEAD_INIT(xs_tcp_transport.list),
>  	.name		= "tcp",
>  	.owner		= THIS_MODULE,
> -	.family		= AF_INET,
> -	.protocol	= IPPROTO_TCP,
> +	.ident		= IPPROTO_TCP,
>  	.setup		= xs_setup_tcp,
>  };
>  
> Index: kernel/net/sunrpc/clnt.c
> ===================================================================
> --- kernel.orig/net/sunrpc/clnt.c
> +++ kernel/net/sunrpc/clnt.c
> @@ -235,7 +235,7 @@ struct rpc_clnt *rpc_create(struct rpc_c
>  	struct rpc_xprt *xprt;
>  	struct rpc_clnt *clnt;
>  	struct xprt_create xprtargs = {
> -		.proto = args->protocol,
> +		.ident = args->protocol,
>  		.srcaddr = args->saddress,
>  		.dstaddr = args->address,
>  		.addrlen = args->addrsize,
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2005.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> NFS maillist  -  NFS@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nfs

[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 290 bytes --]

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
url:http://oss.oracle.com/~cel
version:2.1
end:vcard


[-- Attachment #3: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[-- Attachment #4: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs