From: Frank van Maarseveen <frankvm@frankvm.com>
To: Trond Myklebust <trond.myklebust@fys.uio.no>
Cc: Linux NFS mailing list <nfs@lists.sourceforge.net>
Subject: Re: [PATCH 2/3] SUNRPC client: add interface for binding to a local address
Date: Fri, 6 Jul 2007 16:50:26 +0200 [thread overview]
Message-ID: <20070706145026.GA18242@janus> (raw)
In-Reply-To: <1183729576.6463.29.camel@heimdal.trondhjem.org>
On Fri, Jul 06, 2007 at 09:46:16AM -0400, Trond Myklebust wrote:
> On Thu, 2007-07-05 at 17:26 +0200, Frank van Maarseveen wrote:
> > In addition to binding to a local privileged port the NFS client should
> > allow binding to a specific local address. This is used by the server
> > for callbacks. The patch adds the necessary interface.
> >
> > Signed-off-by: Frank van Maarseveen <frankvm@frankvm.com>
> > ---
> >
> > diff -urp a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> > --- a/include/linux/sunrpc/clnt.h 2007-04-27 18:05:33.000000000 +0200
> > +++ b/include/linux/sunrpc/clnt.h 2007-07-05 15:33:11.000000000 +0200
> > @@ -97,6 +97,7 @@ struct rpc_create_args {
> > int protocol;
> > struct sockaddr *address;
> > size_t addrsize;
> > + struct sockaddr *saddress;
> > struct rpc_timeout *timeout;
> > char *servername;
> > struct rpc_program *program;
> > diff -urp a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
> > --- a/include/linux/sunrpc/xprt.h 2007-02-27 22:55:35.000000000 +0100
> > +++ b/include/linux/sunrpc/xprt.h 2007-07-05 15:33:11.000000000 +0200
> > @@ -201,7 +201,8 @@ void xprt_set_timeout(struct rpc_timeo
> > /*
> > * Generic internal transport functions
> > */
> > -struct rpc_xprt * xprt_create_transport(int proto, struct sockaddr *addr, size_t size, struct rpc_timeout *toparms);
> > +struct rpc_xprt * xprt_create_transport(int proto, struct sockaddr *addr, size_t size,
> > + struct sockaddr *saddr, struct rpc_timeout *toparms);
> > void xprt_connect(struct rpc_task *task);
> > void xprt_reserve(struct rpc_task *task);
> > int xprt_reserve_xprt(struct rpc_task *task);
> > @@ -239,8 +240,10 @@ void xprt_disconnect(struct rpc_xprt *
> > /*
> > * Socket transport setup operations
> > */
> > -struct rpc_xprt * xs_setup_udp(struct sockaddr *addr, size_t addrlen, struct rpc_timeout *to);
> > -struct rpc_xprt * xs_setup_tcp(struct sockaddr *addr, size_t addrlen, struct rpc_timeout *to);
> > +struct rpc_xprt * xs_setup_udp(struct sockaddr *addr, size_t addrlen,
> > + struct sockaddr *saddr, struct rpc_timeout *to);
> > +struct rpc_xprt * xs_setup_tcp(struct sockaddr *addr, size_t addrlen,
> > + struct sockaddr *saddr, struct rpc_timeout *to);
> >
> > /*
> > * Reserved bit positions in xprt->state
> > diff -urp a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> > --- a/net/sunrpc/clnt.c 2007-04-27 18:05:39.000000000 +0200
> > +++ b/net/sunrpc/clnt.c 2007-07-05 15:33:11.000000000 +0200
> > @@ -209,7 +209,8 @@ struct rpc_clnt *rpc_create(struct rpc_c
> > struct rpc_clnt *clnt;
> >
> > xprt = xprt_create_transport(args->protocol, args->address,
> > - args->addrsize, args->timeout);
> > + args->addrsize, args->saddress,
> > + args->timeout);
> > if (IS_ERR(xprt))
> > return (struct rpc_clnt *)xprt;
> >
> > diff -urp a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> > --- a/net/sunrpc/xprt.c 2007-04-27 18:05:39.000000000 +0200
> > +++ b/net/sunrpc/xprt.c 2007-07-05 15:33:11.000000000 +0200
> > @@ -893,17 +893,19 @@ void xprt_set_timeout(struct rpc_timeout
> > * @to: timeout parameters
> > *
> > */
> > -struct rpc_xprt *xprt_create_transport(int proto, struct sockaddr *ap, size_t size, struct rpc_timeout *to)
> > +struct rpc_xprt *xprt_create_transport(int proto, struct sockaddr *ap,
> > + size_t size, struct sockaddr *saddr,
> > + struct rpc_timeout *to)
>
> Urgh... It might be time to fold all these arguments into a "struct
> rpc_xprtsock_create" instead of copying them on the stack.
Can be done before or after this patch. Or combined, just tell me.
>
> > {
> > struct rpc_xprt *xprt;
> > struct rpc_rqst *req;
> >
> > switch (proto) {
> > case IPPROTO_UDP:
> > - xprt = xs_setup_udp(ap, size, to);
> > + xprt = xs_setup_udp(ap, size, saddr, to);
> > break;
> > case IPPROTO_TCP:
> > - xprt = xs_setup_tcp(ap, size, to);
> > + xprt = xs_setup_tcp(ap, size, saddr, to);
> > break;
> > default:
> > printk(KERN_ERR "RPC: unrecognized transport protocol: %d\n",
> > diff -urp a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> > --- a/net/sunrpc/xprtsock.c 2007-04-27 18:05:39.000000000 +0200
> > +++ b/net/sunrpc/xprtsock.c 2007-07-05 15:39:29.000000000 +0200
> > @@ -235,6 +235,7 @@ struct sock_xprt {
> > * Connection of transports
> > */
> > struct delayed_work connect_worker;
> > + struct sockaddr_storage saddr;
> > unsigned short port;
> >
> > /*
> > @@ -1146,31 +1147,33 @@ static void xs_set_port(struct rpc_xprt
> > sap->sin_port = htons(port);
> > }
> >
> > -static int xs_bindresvport(struct sock_xprt *transport, struct socket *sock)
> > +static int xs_bind(struct sock_xprt *transport, struct socket *sock)
> > {
> > struct sockaddr_in myaddr = {
> > .sin_family = AF_INET,
> > };
> > + struct sockaddr_in *sa;
> > int err;
> > unsigned short port = transport->port;
>
> Instead of forcing two tests of transport->xprt.resvport per loop, why
> not just set port to 0 in the case of !transport->xprt.resvport?
ok, that would be better.
>
> >
> > + sa = (struct sockaddr_in *)&transport->saddr;
> > + myaddr.sin_addr = sa->sin_addr;
> > do {
> > - myaddr.sin_port = htons(port);
> > + if (transport->xprt.resvport)
> > + myaddr.sin_port = htons(port);
> > err = kernel_bind(sock, (struct sockaddr *) &myaddr,
> > sizeof(myaddr));
> > - if (err == 0) {
> > + if (err == 0 || !transport->xprt.resvport) {
> > transport->port = port;
> > - dprintk("RPC: xs_bindresvport bound to port %u\n",
> > - port);
> > - return 0;
> > + break;
> > }
> > if (port <= xprt_min_resvport)
> > port = xprt_max_resvport;
> > else
> > port--;
> > } while (err == -EADDRINUSE && port != transport->port);
> > -
> > - dprintk("RPC: can't bind to reserved port (%d).\n", -err);
> > + dprintk("RPC: xs_bind "NIPQUAD_FMT":%u: %d\n",
> > + NIPQUAD(myaddr.sin_addr), port, err);
>
> Hmm... I though you said this was IPv6 clean :-)
Only the server socket handling code. The other files are not IPv6 clean
to begin with (I have some ideas for this).
>
> > return err;
> > }
> >
> > @@ -1229,7 +1232,7 @@ static void xs_udp_connect_worker(struct
> > }
> > xs_reclassify_socket(sock);
> >
> > - if (xprt->resvport && xs_bindresvport(transport, sock) < 0) {
> > + if (xs_bind(transport, sock)) {
> > sock_release(sock);
> > goto out;
> > }
> > @@ -1316,7 +1319,7 @@ static void xs_tcp_connect_worker(struct
> > }
> > xs_reclassify_socket(sock);
> >
> > - if (xprt->resvport && xs_bindresvport(transport, sock) < 0) {
> > + if (xs_bind(transport, sock)) {
> > sock_release(sock);
> > goto out;
> > }
> > @@ -1505,7 +1508,8 @@ static struct rpc_xprt_ops xs_tcp_ops =
> > .print_stats = xs_tcp_print_stats,
> > };
> >
> > -static struct rpc_xprt *xs_setup_xprt(struct sockaddr *addr, size_t addrlen, unsigned int slot_table_size)
> > +static struct rpc_xprt *xs_setup_xprt(struct sockaddr *addr, size_t addrlen,
> > + struct sockaddr *saddr, unsigned int slot_table_size)
> > {
> > struct rpc_xprt *xprt;
> > struct sock_xprt *new;
> > @@ -1534,6 +1538,8 @@ static struct rpc_xprt *xs_setup_xprt(st
> >
> > memcpy(&xprt->addr, addr, addrlen);
> > xprt->addrlen = addrlen;
> > + if (saddr)
> > + memcpy(&new->saddr, saddr, addrlen);
>
> Is it safe to assume that the source and destination sockaddr structures
> are always the same size?
Well, yes unless there are plans for AF_UNIX support. Look into socket
getname(), e.g. inet_getname() and inet6_getname(): both addresses of
a connection must belong to the same address family.
>
> > new->port = xs_get_random_port();
> >
> > return xprt;
> > @@ -1546,12 +1552,13 @@ static struct rpc_xprt *xs_setup_xprt(st
> > * @to: timeout parameters
> > *
> > */
> > -struct rpc_xprt *xs_setup_udp(struct sockaddr *addr, size_t addrlen, struct rpc_timeout *to)
> > +struct rpc_xprt *xs_setup_udp(struct sockaddr *addr, size_t addrlen,
> > + struct sockaddr *saddr, struct rpc_timeout *to)
> > {
> > struct rpc_xprt *xprt;
> > struct sock_xprt *transport;
> >
> > - xprt = xs_setup_xprt(addr, addrlen, xprt_udp_slot_table_entries);
> > + xprt = xs_setup_xprt(addr, addrlen, saddr, xprt_udp_slot_table_entries);
> > if (IS_ERR(xprt))
> > return xprt;
> > transport = container_of(xprt, struct sock_xprt, xprt);
> > @@ -1591,12 +1598,13 @@ struct rpc_xprt *xs_setup_udp(struct soc
> > * @to: timeout parameters
> > *
> > */
> > -struct rpc_xprt *xs_setup_tcp(struct sockaddr *addr, size_t addrlen, struct rpc_timeout *to)
> > +struct rpc_xprt *xs_setup_tcp(struct sockaddr *addr, size_t addrlen,
> > + struct sockaddr *saddr, struct rpc_timeout *to)
> > {
> > struct rpc_xprt *xprt;
> > struct sock_xprt *transport;
> >
> > - xprt = xs_setup_xprt(addr, addrlen, xprt_tcp_slot_table_entries);
> > + xprt = xs_setup_xprt(addr, addrlen, saddr, xprt_tcp_slot_table_entries);
> > if (IS_ERR(xprt))
> > return xprt;
> > transport = container_of(xprt, struct sock_xprt, xprt);
--
Frank
-------------------------------------------------------------------------
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
next prev parent reply other threads:[~2007-07-06 14:50 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-07-05 15:26 [PATCH 2/3] SUNRPC client: add interface for binding to a local address Frank van Maarseveen
2007-07-06 13:46 ` Trond Myklebust
2007-07-06 14:50 ` Frank van Maarseveen [this message]
2007-07-06 15:05 ` Trond Myklebust
2007-07-06 15:40 ` Chuck Lever
2007-07-06 16:31 ` Frank van Maarseveen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20070706145026.GA18242@janus \
--to=frankvm@frankvm.com \
--cc=nfs@lists.sourceforge.net \
--cc=trond.myklebust@fys.uio.no \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox