From: Stanislav Kinsbursky <hidden> Date: 2011-09-09 12:08:34
v2:
1) rebased to v3.1-rc4 (against branch
git://git.linux-nfs.org/projects/trondmy/nfs-2.6.git)
2) Patch 3/5 (SUNRPC: make RPC service dependable on rpcbind clients creation)
fixed: added rpcb_put_local() call in case of svc_serv creation failure.
This patch is required for further RPC layer virtualization, because rpcbind
clients have to be per network namespace.
To achive this, we have to untie network namespace from rpcbind clients sockets.
The idea of this patch set is to make rpcbind clients non-static. I.e. rpcbind
clients will be created during first RPC service creation, and destroyed when
last RPC service is stopped.
With this patch set rpcbind clients can be virtualized easely.
The following series consists of:
---
Stanislav Kinsbursky (5):
SUNRPC: introduce helpers for reference counted rpcbind clients
SUNRPC: use rpcbind reference counting helpers
SUNRPC: make RPC service dependable on rpcbind clients creation
SUNRPC: remove rpcbind clients creation during service registering
SUNRPC: remove rpcbind clients destruction on module cleanup
include/linux/sunrpc/clnt.h | 2 +
net/sunrpc/rpcb_clnt.c | 86 ++++++++++++++++++++++++++++---------------
net/sunrpc/sunrpc_syms.c | 3 --
net/sunrpc/svc.c | 13 ++++++-
4 files changed, 69 insertions(+), 35 deletions(-)
--
Signature
From: Stanislav Kinsbursky <hidden> Date: 2011-09-09 12:08:42
This helpers will be used for dynamical creation and destruction of rpcbind
clients.
Variable rpcb_users is actually a counter of lauched RPC services. If rpcbind
clients has been created already, then we just increase rpcb_users.
Signed-off-by: Stanislav Kinsbursky <redacted>
---
net/sunrpc/rpcb_clnt.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 51 insertions(+), 0 deletions(-)
From: Stanislav Kinsbursky <hidden> Date: 2011-09-09 12:08:51
All is simple: we just increase users counter if rpcbind clients has been
created already. Otherwise we create them and set users counter to 1.
Signed-off-by: Stanislav Kinsbursky <redacted>
---
net/sunrpc/rpcb_clnt.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)
From: Stanislav Kinsbursky <hidden> Date: 2011-09-09 12:09:10
We don't need this code since rpcbind clients are creating during RPC service
creation.
Signed-off-by: Stanislav Kinsbursky <redacted>
---
net/sunrpc/rpcb_clnt.c | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)
@@ -429,11 +429,6 @@ int rpcb_register(u32 prog, u32 vers, int prot, unsigned short port)structrpc_messagemsg={.rpc_argp=&map,};-interror;--error=rpcb_create_local();-if(error)-returnerror;dprintk("RPC: %sregistering (%u, %u, %d, %u) with local ""rpcbind\n",(port?"":"un"),
From: Stanislav Kinsbursky <hidden> Date: 2011-09-09 12:09:16
Rpcbind clients destruction during SUNRPC module removing is obsolete since now
those clients are destroying during last RPC service shutdown.
Signed-off-by: Stanislav Kinsbursky <redacted>
---
net/sunrpc/rpcb_clnt.c | 12 ------------
net/sunrpc/sunrpc_syms.c | 3 ---
2 files changed, 0 insertions(+), 15 deletions(-)
I don't get it -- what's the advantage of creating rpcbind clients in
__svc_create vs. the old way of creating them just before we plan to
use them?
With this scheme, won't we end up creating rpcbind sockets even when we
don't need them? For instance, if I create a callback socket for NFSv4
then I don't really need to talk to rpcbind. With this patch I'll still
get the rpcbind clients created though.
It would seem to me to make more sense to create the rpcbind clients
somewhere closer to svc_setup_socket, and only if SVC_SOCK_ANONYMOUS is
not set.
--
Jeff Layton [off-list ref]
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
I don't get it -- what's the advantage of creating rpcbind clients in
__svc_create vs. the old way of creating them just before we plan to
use them?
The main problem here is not in creation, but in destroying those clients.
Now rpcbind clients are created during rpcb_register(). I.e. once per every family, program version and so on.
But can be unregistered for all protocol families by one call. So it's impossible to put reference counting for those clients in the place, where they are created now.
With this scheme, won't we end up creating rpcbind sockets even when we
don't need them? For instance, if I create a callback socket for NFSv4
then I don't really need to talk to rpcbind. With this patch I'll still
get the rpcbind clients created though.
Yep, you right. This is not a real problem from my pow.
But I'll think about how to avoid this creation for nfs callbacks.
It would seem to me to make more sense to create the rpcbind clients
somewhere closer to svc_setup_socket, and only if SVC_SOCK_ANONYMOUS is
not set.
Probably. Will try to find better place.
--
Best regards,
Stanislav Kinsbursky
I don't get it -- what's the advantage of creating rpcbind clients in
__svc_create vs. the old way of creating them just before we plan to
use them?
The main problem here is not in creation, but in destroying those clients.
Now rpcbind clients are created during rpcb_register(). I.e. once per every family, program version and so on.
But can be unregistered for all protocol families by one call. So it's impossible to put reference counting for those clients in the place, where they are created now.
Could we perhaps set up a 'struct pernet_operations' to create a
destructor for them?
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org
www.netapp.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
I don't get it -- what's the advantage of creating rpcbind clients in
__svc_create vs. the old way of creating them just before we plan to
use them?
The main problem here is not in creation, but in destroying those clients.
Now rpcbind clients are created during rpcb_register(). I.e. once per every family, program version and so on.
But can be unregistered for all protocol families by one call. So it's impossible to put reference counting for those clients in the place, where they are created now.
Could we perhaps set up a 'struct pernet_operations' to create a
destructor for them?
An even easier idea might be to just not take a reference to the
rpcbind client for svc_programs that have vs_hidden set on every
version.
--
Jeff Layton [off-list ref]
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
I don't get it -- what's the advantage of creating rpcbind clients in
__svc_create vs. the old way of creating them just before we plan to
use them?
The main problem here is not in creation, but in destroying those clients.
Now rpcbind clients are created during rpcb_register(). I.e. once per every family, program version and so on.
But can be unregistered for all protocol families by one call. So it's impossible to put reference counting for those clients in the place, where they are created now.
Could we perhaps set up a 'struct pernet_operations' to create a
destructor for them?
An even easier idea might be to just not take a reference to the
rpcbind client for svc_programs that have vs_hidden set on every
version.
Isn't the problem that Stanislav is trying to solve that we need to be
able to register and unregister RPC services to the correct rpcbind
server, depending on which net namespace we are in?
My understanding is that the current code will register everything to
whatever rpcbind server is running in the init net namespace because
that's what rpcb_create_local() uses.
My suggestion is to use a struct pernet_operations to detect when a net
namespace is being created or destroyed, so that the rpcbind client code
knows when to create or destroy a connection to the server that is
running in that namespace.
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org
www.netapp.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Pavel Emelyanov <hidden> Date: 2011-09-09 20:58:21
Isn't the problem that Stanislav is trying to solve that we need to be
able to register and unregister RPC services to the correct rpcbind
server, depending on which net namespace we are in?
My understanding is that the current code will register everything to
whatever rpcbind server is running in the init net namespace because
that's what rpcb_create_local() uses.
My suggestion is to use a struct pernet_operations to detect when a net
namespace is being created or destroyed, so that the rpcbind client code
knows when to create or destroy a connection to the server that is
running in that namespace.
The problem is that the client has a reference on transport, the transport gets
a socket and the socket holds the net namesace and thus we will not receive the
net->exit event with this approach.
Another option is to break the client's socket reference and kill the rpcb client
on netns stop (i.e. as you propose), but in that case we have another problem -
how to destroy _other_ clients (not rpc bind ones) when the net namespace stops.
If you prefer either way just let us know, we'll fix the patches.
I don't get it -- what's the advantage of creating rpcbind clients in
__svc_create vs. the old way of creating them just before we plan to
use them?
The main problem here is not in creation, but in destroying those clients.
Now rpcbind clients are created during rpcb_register(). I.e. once per every family, program version and so on.
But can be unregistered for all protocol families by one call. So it's impossible to put reference counting for those clients in the place, where they are created now.
Could we perhaps set up a 'struct pernet_operations' to create a
destructor for them?
An even easier idea might be to just not take a reference to the
rpcbind client for svc_programs that have vs_hidden set on every
version.
Isn't the problem that Stanislav is trying to solve that we need to be
able to register and unregister RPC services to the correct rpcbind
server, depending on which net namespace we are in?
Yes, it is.
I'm going to make rpcbind clients per net namespace.
My understanding is that the current code will register everything to
whatever rpcbind server is running in the init net namespace because
that's what rpcb_create_local() uses.
My suggestion is to use a struct pernet_operations to detect when a net
namespace is being created or destroyed, so that the rpcbind client code
knows when to create or destroy a connection to the server that is
running in that namespace.
But as Pavel mentioned already, we can't use netns destructor for rpcbind clients because they holds netns reference.
That's why we have to untie netns from rpbind clients first.
Another solution is to not increment netns ref counter for rpcbind sockets as, again, Pavel already mentioned.
But first approach looks clearer from my pow. That's why I'm trying to make rcpbind client's self-destructible.After achieving this we can just make this rpcbind clients per netns and then we can virtualize lockd.
I've tried to find some better place for creating rpcbind clients (instead of __svc_create()). But this place looks like the best one for current solution.
About avoiding of creation of rpcbind clients for nfs 4.* callbacks.
Probably, we can implement init-fini calls for svc_program structure (rpcb_create_local() and rpcb_put_local() will be used in our case) and then inherit them for svc_serv. This will allow us to call this hooks only if they defined and thus avoid rpcbind clients creation for nfs callbacks.
What all of you think about this hook's idea?
I don't get it -- what's the advantage of creating rpcbind clients in
__svc_create vs. the old way of creating them just before we plan to
use them?
The main problem here is not in creation, but in destroying those clients.
Now rpcbind clients are created during rpcb_register(). I.e. once per every family, program version and so on.
But can be unregistered for all protocol families by one call. So it's impossible to put reference counting for those clients in the place, where they are created now.
Could we perhaps set up a 'struct pernet_operations' to create a
destructor for them?
An even easier idea might be to just not take a reference to the
rpcbind client for svc_programs that have vs_hidden set on every
version.
Isn't the problem that Stanislav is trying to solve that we need to be
able to register and unregister RPC services to the correct rpcbind
server, depending on which net namespace we are in?
Yes, it is.
I'm going to make rpcbind clients per net namespace.
quoted
My understanding is that the current code will register everything to
whatever rpcbind server is running in the init net namespace because
that's what rpcb_create_local() uses.
My suggestion is to use a struct pernet_operations to detect when a net
namespace is being created or destroyed, so that the rpcbind client code
knows when to create or destroy a connection to the server that is
running in that namespace.
But as Pavel mentioned already, we can't use netns destructor for rpcbind
clients because they holds netns reference.
That's why we have to untie netns from rpbind clients first.
Another solution is to not increment netns ref counter for rpcbind sockets as,
again, Pavel already mentioned.
But first approach looks clearer from my pow. That's why I'm trying to make
rcpbind client's self-destructible.After achieving this we can just make this
rpcbind clients per netns and then we can virtualize lockd.
I've tried to find some better place for creating rpcbind clients (instead of
__svc_create()). But this place looks like the best one for current solution.
About avoiding of creation of rpcbind clients for nfs 4.* callbacks.
Probably, we can implement init-fini calls for svc_program structure
(rpcb_create_local() and rpcb_put_local() will be used in our case) and then
inherit them for svc_serv. This will allow us to call this hooks only if they
defined and thus avoid rpcbind clients creation for nfs callbacks.
What all of you think about this hook's idea?
Since we already have sv_shutdown callback, we can use it to put rpcbind clients.
Creation of rpcbind clients can be performed in lockd_up() and nfsd_create_serv() before calling svc_create(_pooled)().
I don't get it -- what's the advantage of creating rpcbind clients in
__svc_create vs. the old way of creating them just before we plan to
use them?
The main problem here is not in creation, but in destroying those clients.
Now rpcbind clients are created during rpcb_register(). I.e. once per every family, program version and so on.
But can be unregistered for all protocol families by one call. So it's impossible to put reference counting for those clients in the place, where they are created now.
Could we perhaps set up a 'struct pernet_operations' to create a
destructor for them?
An even easier idea might be to just not take a reference to the
rpcbind client for svc_programs that have vs_hidden set on every
version.
Isn't the problem that Stanislav is trying to solve that we need to be
able to register and unregister RPC services to the correct rpcbind
server, depending on which net namespace we are in?
My understanding is that the current code will register everything to
whatever rpcbind server is running in the init net namespace because
that's what rpcb_create_local() uses.
My assumption in reading this set (maybe wrong) was that this was a
preliminary set for now that just plops in function calls in the places
that do this sort of thing now. I figured that eventually he'd convert
rpcb_create_local, et. al. to do the same thing but within the correct
namespace for the calling task.
My suggestion is to use a struct pernet_operations to detect when a net
namespace is being created or destroyed, so that the rpcbind client code
knows when to create or destroy a connection to the server that is
running in that namespace.
I'm not sure that solves anything. I'd also ass|u|me that something
here (the xprt?) will hold a reference to its netns. If we try to do
this with pernet ops, then I think we'd end up with a chicken-and-egg
problem...
I think the simplest solution would be to basically call these
functions closer to where the rpcbind calls happen today, and just
don't do them when the svc_program has vs_hidden set or if the xprt is
being created with SVC_SOCK_ANONYMOUS set.
In fact, a rethink of the vs_hidden/SVC_SOCK_ANONYMOUS scheme would
probably be a good preliminary patch here. There's a lot of overlap
between those two flags, and somehow consolidating them would probably
be a good thing.
--
Jeff Layton [off-list ref]
From: Stanislav Kinsbursky <hidden> Date: 2011-09-13 13:40:13
13.09.2011 16:51, Jeff Layton пишет:
My assumption in reading this set (maybe wrong) was that this was a
preliminary set for now that just plops in function calls in the places
that do this sort of thing now. I figured that eventually he'd convert
rpcb_create_local, et. al. to do the same thing but within the correct
namespace for the calling task.
You have a right assumption. This is exactly what I'm going to do next.
I think the simplest solution would be to basically call these
functions closer to where the rpcbind calls happen today, and just
don't do them when the svc_program has vs_hidden set or if the xprt is
being created with SVC_SOCK_ANONYMOUS set.
This solution is not the simplest one since we call svc_register() for every svc socket if it's not anonymous. But svc_unregister() is called only once for all inet families and protocols.
Also I've noticed, that we call svc_unregister in __svc_create(). I.e. we call it for nfs callbacks as well (in spite of that we don't need this). Thus, for now, nfs callbacks service creation depends on rpcbind clients presence.
So, for my pow, we need something like startup() callback, passed to svc_create(_pooled)() to clean up this mess.
This callback will be defined only for lockd and nfsd and will create rpcbind clients and remove any stale portmap registrations.
--
Best regards,
Stanislav Kinsbursky
From: Jeff Layton <hidden> Date: 2011-09-13 13:48:53
On Tue, 13 Sep 2011 17:39:59 +0400
Stanislav Kinsbursky [off-list ref] wrote:
13.09.2011 16:51, Jeff Layton пишет:
quoted
My assumption in reading this set (maybe wrong) was that this was a
preliminary set for now that just plops in function calls in the places
that do this sort of thing now. I figured that eventually he'd convert
rpcb_create_local, et. al. to do the same thing but within the correct
namespace for the calling task.
You have a right assumption. This is exactly what I'm going to do next.
quoted
I think the simplest solution would be to basically call these
functions closer to where the rpcbind calls happen today, and just
don't do them when the svc_program has vs_hidden set or if the xprt is
being created with SVC_SOCK_ANONYMOUS set.
This solution is not the simplest one since we call svc_register() for every svc
socket if it's not anonymous. But svc_unregister() is called only once for all
inet families and protocols.
Ahh ok, good point.
Also I've noticed, that we call svc_unregister in __svc_create(). I.e. we call
it for nfs callbacks as well (in spite of that we don't need this). Thus, for
now, nfs callbacks service creation depends on rpcbind clients presence.
Yeah, that's just to remove the any existing registration before we set
up the new one. In the case of a "hidden" service that can probably be
skipped if it makes things easier.
So, for my pow, we need something like startup() callback, passed to
svc_create(_pooled)() to clean up this mess.
This callback will be defined only for lockd and nfsd and will create rpcbind
clients and remove any stale portmap registrations.
That sounds like a reasonable scheme. I'll wait to see the patches.
--
Jeff Layton [off-list ref]
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html