Linux NFS development
 help / color / mirror / Atom feed
* [RFC] [PATCH] rpcbind netid declared per-transport
@ 2007-08-31 13:35 Talpey, Thomas
  2007-08-31 14:08 ` Chuck Lever
  2007-08-31 14:32 ` Chuck Lever
  0 siblings, 2 replies; 12+ messages in thread
From: Talpey, Thomas @ 2007-08-31 13:35 UTC (permalink / raw)
  To: nfs

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.

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)
 
@@ -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 {

-------------------------------------------------------------------------
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  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] [PATCH] rpcbind netid declared per-transport
  2007-08-31 13:35 [RFC] [PATCH] rpcbind netid declared per-transport Talpey, Thomas
@ 2007-08-31 14:08 ` Chuck Lever
  2007-08-31 14:26   ` Trond Myklebust
  2007-08-31 14:32 ` Chuck Lever
  1 sibling, 1 reply; 12+ messages in thread
From: Chuck Lever @ 2007-08-31 14:08 UTC (permalink / raw)
  To: Talpey, Thomas; +Cc: nfs

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] [PATCH] rpcbind netid declared per-transport
  2007-08-31 14:08 ` Chuck Lever
@ 2007-08-31 14:26   ` Trond Myklebust
  2007-08-31 14:30     ` Chuck Lever
  2007-08-31 14:33     ` Chuck Lever
  0 siblings, 2 replies; 12+ messages in thread
From: Trond Myklebust @ 2007-08-31 14:26 UTC (permalink / raw)
  To: chuck.lever; +Cc: nfs, Talpey, Thomas

On Fri, 2007-08-31 at 10:08 -0400, Chuck Lever wrote:

> >  
> > +/*
> > + * 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" */

BTW: Any reason why we are using escaped octal instead of plain ascii
here?

Trond


-------------------------------------------------------------------------
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  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] [PATCH] rpcbind netid declared per-transport
  2007-08-31 14:26   ` Trond Myklebust
@ 2007-08-31 14:30     ` Chuck Lever
  2007-08-31 15:13       ` Trond Myklebust
  2007-08-31 14:33     ` Chuck Lever
  1 sibling, 1 reply; 12+ messages in thread
From: Chuck Lever @ 2007-08-31 14:30 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: nfs, Talpey, Thomas

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

Trond Myklebust wrote:
> On Fri, 2007-08-31 at 10:08 -0400, Chuck Lever wrote:
> 
>>>  
>>> +/*
>>> + * 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" */
> 
> BTW: Any reason why we are using escaped octal instead of plain ascii
> here?

Is there a guarantee that these C strings will be US-ASCII on *every* 
platform in every locale on which Linux kernels are built?  Z-series, 
for example?

If yes, then we can use a normal string.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] [PATCH] rpcbind netid declared per-transport
  2007-08-31 13:35 [RFC] [PATCH] rpcbind netid declared per-transport Talpey, Thomas
  2007-08-31 14:08 ` Chuck Lever
@ 2007-08-31 14:32 ` Chuck Lever
  1 sibling, 0 replies; 12+ messages in thread
From: Chuck Lever @ 2007-08-31 14:32 UTC (permalink / raw)
  To: Talpey, Thomas; +Cc: nfs

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

Talpey, Thomas wrote:
> 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 {

Also I would rename these -- "RPCB_" is used just for identifiers inside 
of rpcb_clnt.c.  Once you move these to a header, they should share the 
same prefix as the other global RPC defines.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] [PATCH] rpcbind netid declared per-transport
  2007-08-31 14:26   ` Trond Myklebust
  2007-08-31 14:30     ` Chuck Lever
@ 2007-08-31 14:33     ` Chuck Lever
  1 sibling, 0 replies; 12+ messages in thread
From: Chuck Lever @ 2007-08-31 14:33 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Linux NFS mailing list

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

Trond Myklebust wrote:
> On Fri, 2007-08-31 at 10:08 -0400, Chuck Lever wrote:
> 
>>>  
>>> +/*
>>> + * 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" */
> 
> BTW: Any reason why we are using escaped octal instead of plain ascii
> here?

In other words, I'm not smart enough to do this any other way.  :-/

If there's a nice C construct that gives us guaranteed US-ASCII strings 
no matter what the locale then I'd love to learn it.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] [PATCH] rpcbind netid declared per-transport
  2007-08-31 14:30     ` Chuck Lever
@ 2007-08-31 15:13       ` Trond Myklebust
  2007-08-31 15:38         ` Chuck Lever
  0 siblings, 1 reply; 12+ messages in thread
From: Trond Myklebust @ 2007-08-31 15:13 UTC (permalink / raw)
  To: chuck.lever; +Cc: nfs, Talpey, Thomas

On Fri, 2007-08-31 at 10:30 -0400, Chuck Lever wrote:
> Trond Myklebust wrote:
> > On Fri, 2007-08-31 at 10:08 -0400, Chuck Lever wrote:
> > 
> >>>  
> >>> +/*
> >>> + * 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" */
> > 
> > BTW: Any reason why we are using escaped octal instead of plain ascii
> > here?
> 
> Is there a guarantee that these C strings will be US-ASCII on *every* 
> platform in every locale on which Linux kernels are built?  Z-series, 
> for example?
> 
> If yes, then we can use a normal string.

I don't understand your reasoning. Are you expecting someone to be
converting our source code into EBCDIC?

The basic character set of C is US-ASCII. Even if you are cross
compiling from a EBCDIC computer, the source code will be in US-ASCII.

You might have somebody convert all the strings in the kernel into
EBCDIC, just for fun, but that will make the strings unreadable on the
computer running the resulting kernel ('cos Linux uses ASCII) and would
likely break all sorts of kernel-userspace interfaces besides breaking
this little snippet in the rpcbind code.

Trond


-------------------------------------------------------------------------
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  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] [PATCH] rpcbind netid declared per-transport
  2007-08-31 15:13       ` Trond Myklebust
@ 2007-08-31 15:38         ` Chuck Lever
  2007-08-31 16:08           ` Talpey, Thomas
  0 siblings, 1 reply; 12+ messages in thread
From: Chuck Lever @ 2007-08-31 15:38 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: nfs, Talpey, Thomas

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

Trond Myklebust wrote:
> On Fri, 2007-08-31 at 10:30 -0400, Chuck Lever wrote:
>> Trond Myklebust wrote:
>>> On Fri, 2007-08-31 at 10:08 -0400, Chuck Lever wrote:
>>>
>>>>>  
>>>>> +/*
>>>>> + * 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" */
>>> BTW: Any reason why we are using escaped octal instead of plain ascii
>>> here?
>> Is there a guarantee that these C strings will be US-ASCII on *every* 
>> platform in every locale on which Linux kernels are built?  Z-series, 
>> for example?
>>
>> If yes, then we can use a normal string.

> You might have somebody convert all the strings in the kernel into
> EBCDIC, just for fun, but that will make the strings unreadable on the
> computer running the resulting kernel ('cos Linux uses ASCII) and would
> likely break all sorts of kernel-userspace interfaces besides breaking
> this little snippet in the rpcbind code.

Well, according to Harbison & Steele, the C source and eventual 
execution character set can be different.  I'm not as certain as you are 
of the guarantee that the Linux kernel environment is US-ASCII on every 
platform from compile to execution.  I've actually written C system 
programs in OpenEdition on MVS/390.

However, if you are comfortable using just a plain old C string (and 
maybe keeping documentation of the US-ASCII requirement in a comment), 
have Tom replace the octal strings with C character strings in his patch.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] [PATCH] rpcbind netid declared per-transport
  2007-08-31 15:38         ` Chuck Lever
@ 2007-08-31 16:08           ` Talpey, Thomas
  2007-08-31 16:27             ` Chuck Lever
  0 siblings, 1 reply; 12+ messages in thread
From: Talpey, Thomas @ 2007-08-31 16:08 UTC (permalink / raw)
  To: chuck.lever; +Cc: nfs

I'll change the strings to whatever representation folks want. All I did
was move 'em!

Repies to earlier comments:

"fixes an omission": sounds good.

"RPCB_" prefix: They're (with this patch) part of the rpcbind api. The prefix
seems logical to me... when you say "the same as the other RPC..." do you
mean they should be "RPC_NETID_..."?

RPCB_MAXNETIDLEN: Sure, I'll move it. It is, after all, a limit on the string
passed as the netid, i.e. part of the API. But let me ask this. It really isn't
necessary, the code in rpcbind can easily accommodate longer strings. Is
it worth coding the extra generality? The reason I didn't is because (as the
comment mentions) there aren't any longer strings defined by any RFC.

clnt.h vs msg_prot.h: After discovering the rpcbind calls defined in clnt.h,
and also seeing rpc goop in there, it felt more correct than msg_prot.h,
which is pretty much about rpc marshaling and error codes. All the other
rpcbind stuff is in clnt.h, and most all clnt's need to call rpcbind to find
their port. That's my rationale.

Tom.


At 11:38 AM 8/31/2007, Chuck Lever wrote:
>Trond Myklebust wrote:
>> On Fri, 2007-08-31 at 10:30 -0400, Chuck Lever wrote:
>>> Trond Myklebust wrote:
>>>> On Fri, 2007-08-31 at 10:08 -0400, Chuck Lever wrote:
>>>>
>>>>>>  
>>>>>> +/*
>>>>>> + * 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" */
>>>> BTW: Any reason why we are using escaped octal instead of plain ascii
>>>> here?
>>> Is there a guarantee that these C strings will be US-ASCII on *every* 
>>> platform in every locale on which Linux kernels are built?  Z-series, 
>>> for example?
>>>
>>> If yes, then we can use a normal string.
>
>> You might have somebody convert all the strings in the kernel into
>> EBCDIC, just for fun, but that will make the strings unreadable on the
>> computer running the resulting kernel ('cos Linux uses ASCII) and would
>> likely break all sorts of kernel-userspace interfaces besides breaking
>> this little snippet in the rpcbind code.
>
>Well, according to Harbison & Steele, the C source and eventual 
>execution character set can be different.  I'm not as certain as you are 
>of the guarantee that the Linux kernel environment is US-ASCII on every 
>platform from compile to execution.  I've actually written C system 
>programs in OpenEdition on MVS/390.
>
>However, if you are comfortable using just a plain old C string (and 
>maybe keeping documentation of the US-ASCII requirement in a comment), 
>have Tom replace the octal strings with C character strings in his patch.
>
>

-------------------------------------------------------------------------
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  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] [PATCH] rpcbind netid declared per-transport
  2007-08-31 16:08           ` Talpey, Thomas
@ 2007-08-31 16:27             ` Chuck Lever
  2007-08-31 16:41               ` Talpey, Thomas
  0 siblings, 1 reply; 12+ messages in thread
From: Chuck Lever @ 2007-08-31 16:27 UTC (permalink / raw)
  To: Talpey, Thomas; +Cc: nfs

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

Talpey, Thomas wrote:
> "RPCB_" prefix: They're (with this patch) part of the rpcbind api. The prefix
> seems logical to me... 

Well, that's a fair argument, since the idea of netid is defined in RFC 
1833, which is about rpcbind specifically.  However, the globally scoped 
RPC-related identifiers are usually named RPC_... and you are now making 
use of these things in the transport layer as well as in rpcbind.

It's a nit, I guess.

> when you say "the same as the other RPC..." do you
> mean they should be "RPC_NETID_..."?

Yes.

> RPCB_MAXNETIDLEN: Sure, I'll move it. It is, after all, a limit on the string
> passed as the netid, i.e. part of the API. But let me ask this. It really isn't
> necessary, the code in rpcbind can easily accommodate longer strings. Is
> it worth coding the extra generality? The reason I didn't is because (as the
> comment mentions) there aren't any longer strings defined by any RFC.

We need to know the maximum size to compute the largest possible size of 
the RPC buffer needed for the request.  See the definition of 
RPCB_netid_sz.  Those are all added together to get the largest possible 
request size, and that's used to compute the largest possible buffer size.

Using a macro documents what the heck that number is.  "4?  4 what?"  I 
was asked at the time this was submitted to use macros instead of 
open-coded integers, and I tend to agree that looks better.

> clnt.h vs msg_prot.h: After discovering the rpcbind calls defined in clnt.h,
> and also seeing rpc goop in there, it felt more correct than msg_prot.h,
> which is pretty much about rpc marshaling and error codes. All the other
> rpcbind stuff is in clnt.h, and most all clnt's need to call rpcbind to find
> their port. That's my rationale.

The rpcbind function calls are defined in clnt.h because they are part 
of the RPC client API, specific to Linux.

Netids are standardized on-the-wire marshaled values, so they definitely 
belong in msg_prot.h, which as you say, contains marshaling and error 
code definitions.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] [PATCH] rpcbind netid declared per-transport
  2007-08-31 16:27             ` Chuck Lever
@ 2007-08-31 16:41               ` Talpey, Thomas
  2007-08-31 16:59                 ` Chuck Lever
  0 siblings, 1 reply; 12+ messages in thread
From: Talpey, Thomas @ 2007-08-31 16:41 UTC (permalink / raw)
  To: chuck.lever; +Cc: nfs

At 12:27 PM 8/31/2007, Chuck Lever wrote:
>Talpey, Thomas wrote:
>> "RPCB_" prefix: They're (with this patch) part of the rpcbind api. The prefix
>...
>It's a nit, I guess.

How about RPCBIND_NETID_foo? It's more obvious, and keeps the RPC...

>> RPCB_MAXNETIDLEN: Sure, I'll move it. It is, after all, a limit on the string
>>...
>
>We need to know the maximum size to compute the largest possible size of 
>the RPC buffer needed for the request.  See the definition of 
>RPCB_netid_sz.  Those are all added together to get the largest possible 
>request size, and that's used to compute the largest possible buffer size.

Yes, I know, but it could be larger and more dynamically calculated.
I'll move it, but leave it as-is then, there is no need to change it.
I'll also move the MAXADDRLEN, since that has the same meaning
for the UNIVERSAL_ADDR argument.


>Netids are standardized on-the-wire marshaled values, so they definitely 
>belong in msg_prot.h, which as you say, contains marshaling and error 
>code definitions.

XDR marshaling, a much lower level than rpcbind. But, I'll move the netid's.
It's a fine spot for them.

Stand by for updated patch.

Tom.

-------------------------------------------------------------------------
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  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] [PATCH] rpcbind netid declared per-transport
  2007-08-31 16:41               ` Talpey, Thomas
@ 2007-08-31 16:59                 ` Chuck Lever
  0 siblings, 0 replies; 12+ messages in thread
From: Chuck Lever @ 2007-08-31 16:59 UTC (permalink / raw)
  To: Talpey, Thomas; +Cc: nfs

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

Talpey, Thomas wrote:
> At 12:27 PM 8/31/2007, Chuck Lever wrote:
>> Talpey, Thomas wrote:
>>> "RPCB_" prefix: They're (with this patch) part of the rpcbind api. The prefix
>> ...
>> It's a nit, I guess.
> 
> How about RPCBIND_NETID_foo? It's more obvious, and keeps the RPC...

OK by me.

>>> RPCB_MAXNETIDLEN: Sure, I'll move it. It is, after all, a limit on the string
>>> ...
>> We need to know the maximum size to compute the largest possible size of 
>> the RPC buffer needed for the request.  See the definition of 
>> RPCB_netid_sz.  Those are all added together to get the largest possible 
>> request size, and that's used to compute the largest possible buffer size.
> 
> Yes, I know, but it could be larger and more dynamically calculated.

I'm not sure what you mean, then.  The buffer size maximums are 
statically computed at kernel compile time.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2007-08-31 17:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-31 13:35 [RFC] [PATCH] rpcbind netid declared per-transport Talpey, Thomas
2007-08-31 14:08 ` Chuck Lever
2007-08-31 14:26   ` Trond Myklebust
2007-08-31 14:30     ` Chuck Lever
2007-08-31 15:13       ` Trond Myklebust
2007-08-31 15:38         ` Chuck Lever
2007-08-31 16:08           ` Talpey, Thomas
2007-08-31 16:27             ` Chuck Lever
2007-08-31 16:41               ` Talpey, Thomas
2007-08-31 16:59                 ` Chuck Lever
2007-08-31 14:33     ` Chuck Lever
2007-08-31 14:32 ` Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox