Re: [RFC] [PATCH] rpcbind netid declared per-transport

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

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

Talpey, Thomas wrote:
> The following patch moves rpcbind v3 "netid" selection from the
> rpbcind client to the originating transport, where I believe it belongs.
> This also enables IPv6 rpcbind transport resolution by the client.
> 
> Patch base is 2.6.23-rc4 + Trond NFS_ALL.
> 
> Comments?
> 
> Tom.
> 
> -----
> 
> SUNRPC: export per-transport netid's
> 
> The rpcbind (v3+) netid is provided by each RPC client transport.
> This enables IPv6 rpcbind client support, and future extension.

More clear: "This fixes an omission with IPv6 rpcbind client support."

"Enables" suggests there's nothing more to do to make IPv6 work.

> Signed-off-by: Tom Talpey <tmt@netapp.com>
> 
> ---
> 
>  include/linux/sunrpc/clnt.h |    8 ++++++++
>  include/linux/sunrpc/xprt.h |    1 +
>  net/sunrpc/rpcb_clnt.c      |   24 +++++++++---------------
>  net/sunrpc/xprtsock.c       |   10 ++++++++++
>  4 files changed, 28 insertions(+), 15 deletions(-)
> 
> Index: linux-2.6.22/include/linux/sunrpc/xprt.h
> ===================================================================
> --- linux-2.6.22.orig/include/linux/sunrpc/xprt.h
> +++ linux-2.6.22/include/linux/sunrpc/xprt.h
> @@ -56,6 +56,7 @@ enum rpc_display_format_t {
>  	RPC_DISPLAY_HEX_ADDR,
>  	RPC_DISPLAY_HEX_PORT,
>  	RPC_DISPLAY_UNIVERSAL_ADDR,
> +	RPC_DISPLAY_NETID,
>  	RPC_DISPLAY_MAX,
>  };
>  
> Index: linux-2.6.22/net/sunrpc/xprtsock.c
> ===================================================================
> --- linux-2.6.22.orig/net/sunrpc/xprtsock.c
> +++ linux-2.6.22/net/sunrpc/xprtsock.c
> @@ -337,6 +337,11 @@ static void xs_format_ipv4_peer_addresse
>  				ntohs(addr->sin_port) & 0xff);
>  	}
>  	xprt->address_strings[RPC_DISPLAY_UNIVERSAL_ADDR] = buf;
> +
> +	if (xprt->prot == IPPROTO_UDP)
> +		xprt->address_strings[RPC_DISPLAY_NETID] = RPCB_NETID_UDP;
> +	else
> +		xprt->address_strings[RPC_DISPLAY_NETID] = RPCB_NETID_TCP;
>  }
>  
>  static void xs_format_ipv6_peer_addresses(struct rpc_xprt *xprt)
> @@ -398,6 +403,11 @@ static void xs_format_ipv6_peer_addresse
>  				ntohs(addr->sin6_port) & 0xff);
>  	}
>  	xprt->address_strings[RPC_DISPLAY_UNIVERSAL_ADDR] = buf;
> +
> +	if (xprt->prot == IPPROTO_UDP)
> +		xprt->address_strings[RPC_DISPLAY_NETID] = RPCB_NETID_UDP6;
> +	else
> +		xprt->address_strings[RPC_DISPLAY_NETID] = RPCB_NETID_TCP6;
>  }
>  
>  static void xs_free_peer_addresses(struct rpc_xprt *xprt)
> Index: linux-2.6.22/net/sunrpc/rpcb_clnt.c
> ===================================================================
> --- linux-2.6.22.orig/net/sunrpc/rpcb_clnt.c
> +++ linux-2.6.22/net/sunrpc/rpcb_clnt.c
> @@ -95,20 +95,9 @@ enum {
>  /*
>   * r_netid
>   *
> - * Quoting RFC 3530, section 2.2:
> - *
> - * For TCP over IPv4 the value of r_netid is the string "tcp".  For UDP
> - * over IPv4 the value of r_netid is the string "udp".
> - *
> - * ...
> - *
> - * For TCP over IPv6 the value of r_netid is the string "tcp6".  For UDP
> - * over IPv6 the value of r_netid is the string "udp6".
> + * Note that RFC 1833 does not put any size restrictions on the
> + * netid string, but all currently defined netid's fit in 4 bytes.
>   */
> -#define RPCB_NETID_UDP	"\165\144\160"		/* "udp" */
> -#define RPCB_NETID_TCP	"\164\143\160"		/* "tcp" */
> -#define RPCB_NETID_UDP6	"\165\144\160\066"	/* "udp6" */
> -#define RPCB_NETID_TCP6	"\164\143\160\066"	/* "tcp6" */
>  
>  #define RPCB_MAXNETIDLEN	(4u)

Actually I would keep RPCB_MAXNETIDLEN with the RPCB_NETID definitions 
in the header file.

> @@ -408,8 +397,13 @@ void rpcb_getport_async(struct rpc_task 
>  	map->r_prot = xprt->prot;
>  	map->r_port = 0;
>  	map->r_xprt = xprt_get(xprt);
> -	map->r_netid = (xprt->prot == IPPROTO_TCP) ? RPCB_NETID_TCP :
> -						   RPCB_NETID_UDP;
> +	map->r_netid = rpc_peeraddr2str(clnt, RPC_DISPLAY_NETID);
> +	if (strlen(map->r_netid) > RPCB_MAXNETIDLEN) {
> +		status = -ENAMETOOLONG;
> +		dprintk("RPC: %5u %s: netid %s too long (%d)\n",
> +			task->tk_pid, __FUNCTION__, map->r_netid, RPCB_MAXNETIDLEN);
> +		goto bailout;
> +	}
 >
>  	memcpy(&map->r_addr,
>  	       rpc_peeraddr2str(rpcb_clnt, RPC_DISPLAY_UNIVERSAL_ADDR),
>  	       sizeof(map->r_addr));
> Index: linux-2.6.22/include/linux/sunrpc/clnt.h
> ===================================================================
> --- linux-2.6.22.orig/include/linux/sunrpc/clnt.h
> +++ linux-2.6.22/include/linux/sunrpc/clnt.h
> @@ -92,6 +92,14 @@ struct rpc_procinfo {
>  	char *			p_name;		/* name of procedure */
>  };
>  
> +/*
> + * RFC1833/RFC3530 rpcbind (v3+) well-known netid's.
> + */
> +#define	RPCB_NETID_UDP	"\165\144\160"		/* "udp" */
> +#define	RPCB_NETID_TCP	"\164\143\160"		/* "tcp" */
> +#define	RPCB_NETID_UDP6	"\165\144\160\066"	/* "udp6" */
> +#define	RPCB_NETID_TCP6	"\164\143\160\066"	/* "tcp6" */
> +
>  #ifdef __KERNEL__
>  
>  struct rpc_create_args {

IMO this is the wrong header to use for these definitions.

clnt.h is for internal definitions that are specific to the Linux RPC 
client implementation.  msg_prot.h is for definitions that are specified 
by the relevant RFCs.  I think the NETID values fall in the latter 
category and thus belong in msg_prot.h

[-- 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: 315 bytes --]

-------------------------------------------------------------------------
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/

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

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