Re: [PATCH 1/2] nfs-service: Added the starting of gssproxy

NeilBrown
2014-09-23T02:01:25Z

[-- Attachment #1: Type: text/plain, Size: 5658 bytes --]

On Mon, 22 Sep 2014 18:34:23 -0400 "J. Bruce Fields" <bfields@fieldses.org>
wrote:

> On Mon, Sep 22, 2014 at 05:14:05PM -0400, Steve Dickson wrote:
> >
> >
> > On 09/22/2014 04:44 PM, J. Bruce Fields wrote:
> > > On Mon, Sep 22, 2014 at 03:43:09PM -0400, Steve Dickson wrote:
> > >>
> > >>
> > >> On 09/22/2014 03:26 PM, Simo Sorce wrote:
> > >>> On Mon, 22 Sep 2014 15:20:07 -0400
> > >>> Steve Dickson <steved@redhat.com> wrote:
> > >>>
> > >>>> Added the gssproxy.service to both the Wants= and
> > >>>> Atfers= lines, before the rpc-svcgssd.service. There
> > >>>> are ConditionPathExists= lines in the rpc-svcgssd.service
> > >>>> unit which will stop the rpc.svcgssd daemon from
> > >>>> starting when the gssproxy daemon is already running.
> > >>>>
> > >>>> Signed-off-by: Steve Dickson <steved@redhat.com>
> > >>>> ---
> > >>>> systemd/nfs-server.service | 5 +++--
> > >>>> 1 file changed, 3 insertions(+), 2 deletions(-)
> > >>>>
> > >>>> diff --git a/systemd/nfs-server.service b/systemd/nfs-server.service
> > >>>> index 2fa7387..c740fa2 100644
> > >>>> --- a/systemd/nfs-server.service
> > >>>> +++ b/systemd/nfs-server.service
> > >>>> @@ -2,12 +2,13 @@
> > >>>> Description=NFS server and services
> > >>>> Requires= network.target proc-fs-nfsd.mount rpcbind.target
> > >>>> Requires= nfs-mountd.service
> > >>>> -Wants=rpc-statd.service nfs-idmapd.service rpc-gssd.service
> > >>>> rpc-svcgssd.service +Wants=rpc-statd.service nfs-idmapd.service
> > >>>> +Wants=rpc-gssd.service
> > >>>> Wants=rpc-statd-notify.service
> > >>>>
> > >>>> After= network.target proc-fs-nfsd.mount rpcbind.target
> > >>>> nfs-mountd.service After= nfs-idmapd.service rpc-statd.service
> > >>>> -After= rpc-gssd.service rpc-svcgssd.service
> > >>>> +After= rpc-gssd.service gssproxy.service rpc-svcgssd.service
> > >>>> Before= rpc-statd-notify.service
> > >>>>
> > >>>> Wants=nfs-config.service
> > >>>
> > >>> I think you really need to insure that the modules are loaded before
> > >>> any of the server services are started, perhaps adding a unit file that
> > >>> exec's modprobe and has "Before: gssproxy.service rpc-svcgssd.service"
> > >>> in it ?
> > >> I really don't think its needed... From my testing it appears
> > >> gssproxy is always being started and rpc.svcgssd is not...
> > >
> > > Huh. Well rpc-svcgssd.service has var-lib-nfs-rpc_pipefs.mount as both
> > > "Requires=" and "After=", so rpc-svcgssd.service will never run
> > > without first running var-lib-nfs-rpc_pipefs.mount, which will load
> > > sunrpc. But I don't see where auth_rpcgss is getting loaded. And I
> > > don't see what ensures anything happening before gssproxy runs.
> > It happens during the mount on the client and when the server
> > is started.
> >
> > >
> > > We want to make sure your testing's not just getting lucky on the
> > > startup order.
> > The reason it working is because rpc.gssd is being started on the server
> > these days for callbacks and the After= line in rpc-svcgssd.service is being
> > executed before the ConditionPathExists which cause rpc.svcgssd not to start.
>
> nfs-utils$ grep After systemd/rpc-svcgssd.service
> After=var-lib-nfs-rpc_pipefs.mount
> After=gssproxy.service
> After=nfs-config.service
>
> There doesn't seem to be an After= line referring to rpc.gssd.
>
> > So when gssproxy.service does it's "Before=nfs-secure.service nfs-secure-server.service"
> > line everything is loaded before gssproxy start...
>
> That line only makes gss-proxy start before those other things.
>
> > I'm think gssproxy.service just needs to the put the Wants and After=
> > var-lib-nfs-rpc_pipefs.mount lines, instead of that Before line..
>
> That would make sure sunrpc's loaded, but not auth_rpcgss.
>
> > >> Plus, from my understanding... loading module from a service
> > >> file is a big no no! People were having problems with
> > >> way back when...
> > >
> > > Any pointers? Google's not finding me anything.
> > Search the the Fedora bz's when systemd first came out...
>
> All I can find is:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=699040#c16
>
> Btw afaik modules should be loaded via autoloading based on bus
> information, or via /etc/modules-load.d/*.conf. and unloading a
> module from the kernel should not be done except for debugging
> purposes so loading all these modules is it really necessary?
>
> Which I agree with--modules should normally load on demand when we need
> them, and we should have an explanation for exceptions.
>
> But here we have a pretty reasonable explanation (we need to know
> on startup whether a certain module has a certain feature, and we have
> to modprobe to do that). I don't see any blanket prohibition against
> loading modules.
>
> OK, and in 702707 there's a request for support of the monolithic kernel
> case, but that's easy, we just allow the modprobe to fail in that case.
>

I certainly think it is absolutely fine for the service files for the gss
daemons to modprobe auth_rpcgss. It would be really nice if the loading of
the module would automatically trigger the starting of the daemons, or would
signal the daemons to start talking to the module. But that is probably
unnecessary complexity.

I would probably have a separate service file which did the modprobe.
It would declare itself to be Before= all of the gss daemons,
and it would be conditional on /etc/krb5.conf existing.
nfs-server would Want it, so it would be started when needed at just the
right time...

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]