2007-08-31 13:37:12

by Talpey, Thomas

[permalink] [raw]
Subject: [RFC] [PATCH] rpcbind netid declared per-transport

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 <[email protected]>

---

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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2007-08-31 14:27:39

by Trond Myklebust

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

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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-08-31 16:08:56

by Talpey, Thomas

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

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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-08-31 15:32:43

by Trond Myklebust

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

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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-08-31 16:41:53

by Talpey, Thomas

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

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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs