Re: [PATCH v3 04/11] SUNRPC: parametrize svc creation calls withportmapper flag
From: Stanislav Kinsbursky
Date: Tue Sep 20 2011 - 06:15:47 EST
19.09.2011 22:11, Jeff Layton ÐÐÑÐÑ:
On Mon, 19 Sep 2011 19:42:12 +0400
Stanislav Kinsbursky<skinsbursky@xxxxxxxxxxxxx> wrote:
19.09.2011 19:07, Jeff Layton ÐÐÑÐÑ:
On Mon, 19 Sep 2011 18:51:31 +0400
Stanislav Kinsbursky<skinsbursky@xxxxxxxxxxxxx> wrote:
19.09.2011 18:08, Jeff Layton ÐÐÑÐÑ:
On Tue, 13 Sep 2011 22:13:51 +0400
Stanislav Kinsbursky<skinsbursky@xxxxxxxxxxxxx> wrote:
This new flag ("setup_rpcbind) will be used to detect, that new service will
send portmapper register calls. For such services we will create rpcbind
clients and remove all stale portmap registrations.
Also, svc_rpcb_cleanup() will be set as sv_shutdown callback for such services
in case of this field wasn't initialized earlier. This will allow to destroy
rpcbind clients when no other users of them left.
Signed-off-by: Stanislav Kinsbursky<skinsbursky@xxxxxxxxxxxxx>
---
include/linux/sunrpc/svc.h | 2 ++
net/sunrpc/svc.c | 21 ++++++++++++++-------
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 223588a..528952a 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -402,11 +402,13 @@ struct svc_procedure {
* Function prototypes.
*/
struct svc_serv *svc_create(struct svc_program *, unsigned int,
+ int setup_rpcbind,
^^^
Instead of adding this parameter, why not
base this on the vs_hidden flag in the
svc_version? IOW, have a function that looks at
all the svc_versions for a particular
svc_program, and returns "true" if any of them
have vs_hidden unset? The mechanism you're
proposing here has the potential to be out of
sync with the vs_hidden flag.
Could you, please, clarify me this vs_hidden flag?
I understand, that it's used to avoid portmap registration.
But as I see, it's set only for nfs_callback_version1. But this svc_version is a
part of nfs4_callback_program with nfs_callback_version4, which is not hidden.
Does this flag is missed here? If not, how we can return "true" from your
proposed function if any of them have vs_hidden unset?
Also sockets for this program are created with SVC_SOCK_ANONYMOUS flag and we
will not register any of this program versions with portmapper.
Thus, from my pow, this vs_hidden flag affects only svc_unregister. And only
nfs_callback_version1. This looks really strange.
I.e. if we use this flag only for passing through this versions during
svc_(un)register, and we actually also want to pass through
nfs_callback_version4 as well (but just missed this vs_hidden flag for it), then
with current patch-set we can move this flag from (vs_hidden) svc_version to
svc_program and check it during svc_create instead of my home-brew
"setup_rpcbind" variable.
Agreed. The current situation is a mess, which is why I suggested a
cleanup and overhaul before you do this...
The vs_hidden flag is intended to show that a particular program
version should not be registered with (or unregistered from) the
portmapper. Unfortunately, nothing looks at vs_hidden during
registration time, only when unregistering (as you mention).
It's quite possible that several svc_versions declared in the kernel do
not have this set correctly. One thing that would be good is to audit
each of those.
We currently rely on SVC_SOCK_ANONYMOUS for registration, but that
wasn't its original intent. It's was just convenient to use it there
too.
SVC_SOCK_ANONYMOUS was (as best I can tell) originally intended for use
on temporary sockets that we establish on receive. So for
instance...when a client connects to nfsd, we need to create a new
socket for nfsd, but obviously we don't want to register that socket
with the portmapper (since nfsd should already be registered there).
SVC_SOCK_ANONYMOUS ensures that that socket is not registered.
The whole scheme could probably use a fundamental re-think. I'm not
sure I have a great idea to propose in lieu of it, but I think adding
yet another flag here is probably not the best way to go.
Ok, thank you, Jeff.
It looks like no mentions about portmapper are present in RFC's for NFS versions
4.* after a brief look.
This SVC_SOCK_ANONYMOUS is understandable and can't be removed with this
patch-set from my pow.
But now I strongly believe, that we can move this vs_hidden flag from
svc_version to svc_program structure and set it for both NFSv4.* programs.
Hope, someone else will confirm of refute this statement.
The problem is nfsd. In principle, there's no real reason we have to
register NFSv4 with the portmapper at all. One could envision a
setup where a v4-only server doesn't need to run rpcbind at all. Making
it per-program may hamstring you from doing that later.
It think it would be a good thing to keep it per-version, and it's
trivial to write a routine to do what I've described. svc_creates only
happen rarely.
Just walk the pg_vers array for the program and look at each vs_hidden
value. Return true when you hit one that has vs_hidden unset. Return
false if none do. Then use that return value to replace your new flag
in this patch.
Done. I've sent v4 patch-set.
--
Best regards,
Stanislav Kinsbursky
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/