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. -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html