2014-09-22 19:20:12

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 0/2] Use the gssproxy damon for GSSAPI credentials (v3)

The gssproxy(8) daemon will now be used to manage the GSSAPI
credentials on the server.

The nfs-server systemd script will now start gssproxy and
the building/installing of rpc.svcgssd is now off by default.

Steve Dickson (2):
nfs-service: Added the starting of gssproxy
rpc.svcgssd: the build of rpc.svcgssd is off by default

configure.ac | 23 +++++++++++++++++++----
systemd/nfs-server.service | 5 +++--
utils/gssd/Makefile.am | 11 +++++++++--
3 files changed, 31 insertions(+), 8 deletions(-)

--
1.9.3



2014-09-23 00:19:31

by Simo Sorce

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

On Mon, 22 Sep 2014 18:57:10 -0400
Steve Dickson <[email protected]> wrote:

>
>
> On 09/22/2014 05:32 PM, Simo Sorce wrote:
> > On Mon, 22 Sep 2014 17:14:05 -0400
> > Steve Dickson <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >>>>>> ---
> >>>>>> 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
> >>>>> " 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.
> >
> > This guarantees ordering (to some degree) between rpc.gssd and
> > rpoc.svcgssd, but says nothing about gssproxy ...
> The question was how is the auth_rpcgss module being loaded. Since
> both rpc-svcgssd.service and rpc-gssd.service service have
> a After=var-lib-nfs-rpc_pipefs.mount and gssproxy is requiring
> them, that's how auth_rpcgss is being loaded.
>
> If you only in enable gssproxy (not nfs-server or nfs-client) the
> module still get loaded via gssproxy,service file
>
> >> So when gssproxy.service does it's "Before=nfs-secure.service
> >> nfs-secure-server.service" line everything is loaded before
> >> gssproxy start...
> >>
> >> 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..
> >
> > Maybe we should add "Before: gssproxy.service rpc-svcgssd.service"
> > to var-lib-nfs-rpc_pipefs.mount instead (and also drop any mention
> > of nfs services in gssproxy unit file so you have complete control
> > of the dependencies ?
> No.
> The loading of sunrpc and the mounting of the file system has nothing
> to do with starting up the gssd daemons.
>
> I would suggest gssproxy does to two things:
>
> 1) Add a Requires: nfs-utils to the spec file since you are requiring
> services from nfs-utils

No we are not requiring services from nfs-utils, nfs-utils is one of
our users, you got that reversed.

> 2) Add a After=var-lib-nfs-rpc_pipefs.mount to the gssproxy.service
> file since gssproxy could careless about either rpc.gssd or
> rpc.svcgssd daemons. All it is looking for is the sunrpc and
> auth_rpcgss kernel modules.

The correct thing is to add a Before: gssproxy.service to
var-lib-nfs-rpc_pipefs.mount IMO.

Simo.

--
Simo Sorce * Red Hat, Inc * New York

2014-09-23 02:55:28

by NeilBrown

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

On Mon, 22 Sep 2014 22:09:28 -0400 "J. Bruce Fields" <[email protected]>
wrote:

> On Tue, Sep 23, 2014 at 11:42:29AM +1000, NeilBrown wrote:
> > Surely gssproxy is only serving nfsd requests if both /run/gssproxy.pid
> > exists and /proc/net/rpc/use-gss-proxy exists.
> > If either of those files is missing, then rpc.svcgssd needs to run.
> > In one case, the gssproxy daemon isn't available for some reason. In the
> > other case the kernel cannot make use of it.
> >
> > Is that not correct?
> >
> > That is exactly the rule that I (tried to) encode in the service file with
> > these two conditions.
>
> Eh, I see your point, but the gssproxy.pid one still seems a little odd
> to me.
>
> I guess it's friendlier to people that don't have gss-proxy installed at
> all, or want to turn it off for some reason--but then they or their
> distro can fix up the unit files too.

Having to fix up unit files is something I would much rather avoid. I think
of them as code and just because they can be edited it doesn't mean they
should be.

I'm quite open to having rpc.svcgssd test to see if gssproxy is installed
rather than if it is running. In that case we would have a 'Want=' somewhere
in nfs-utils for gssproxy.service (which I previously said I didn't like but
I'm beginning to see the wisdom of).

But if gssproxy isn't installed, then I think rpc.svcgssd should run whether
use-gss-proxy is present or not.

>
> Otherwise if we've got gss-proxy and the kernel supports it then it
> should work, and if it's failing to come up in that case I'd kind of
> like to know why and get a bug report like "gssproxy failed to start" or
> "krb5 exports stopped working" rather than "krb5 exports are working in
> some subtly different way than they did last week."

This is quite a strong argument.

Thanks,
NeilBrown

>
> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Attachments:
signature.asc (828.00 B)

2014-09-23 01:42:39

by NeilBrown

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

On Mon, 22 Sep 2014 16:00:50 -0400 Simo Sorce <[email protected]> wrote:

> On Mon, 22 Sep 2014 15:53:46 -0400
> Steve Dickson <[email protected]> wrote:
>
> >
> >
> > On 09/22/2014 03:46 PM, Simo Sorce wrote:
> > > On Mon, 22 Sep 2014 15:40:57 -0400
> > > "J. Bruce Fields" <[email protected]> wrote:
> > >
> > >> On Mon, Sep 22, 2014 at 03:20:07PM -0400, Steve Dickson 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.
> > >>
> > >> That should read "when the kernel supports gssproxy", not "when the
> > >> gssproxy daemon is already running."
> > >
> > > Actually the language is currently correct but it is another bug,
> > > the systemd/rpc-svcgssd.service file still includes
> > > "ConditionPathExists=|!/run/gssproxy.pid"
> > > This line should be removed in this patch.
> >
> > I left that on purpose because isn't that ConditionPathExists
> > seeing if /run/gssproxy.pid exists and if it does
> > it means gssproxy is already running so rpc.svcgssd
> > should not start?
>
> No.
> First of all the fact gss-proxy is running does not mean it is
> serving nfsd necessarily, it may be running on an older kernel where it
> servers apache or some other process (remember gssproxy is not just
> for nfsd).
> Second you already have
> "ConditionPathExists=|!/proc/net/rpc/use-gss-proxy" which is the
> correct trigger to decide which of the two to use.
>

Surely gssproxy is only serving nfsd requests if both /run/gssproxy.pid
exists and /proc/net/rpc/use-gss-proxy exists.
If either of those files is missing, then rpc.svcgssd needs to run.
In one case, the gssproxy daemon isn't available for some reason. In the
other case the kernel cannot make use of it.

Is that not correct?

That is exactly the rule that I (tried to) encode in the service file with
these two conditions.

NeilBrown


Attachments:
signature.asc (828.00 B)

2014-09-23 15:52:21

by Steve Dickson

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

On 09/23/2014 11:16 AM, J. Bruce Fields wrote:
> On Tue, Sep 23, 2014 at 11:06:45AM -0400, Steve Dickson wrote:
>> >
>> >
>> > On 09/22/2014 09:55 PM, J. Bruce Fields wrote:
>>> > > On Mon, Sep 22, 2014 at 08:26:55PM -0400, Simo Sorce wrote:
>>>> > >> No, starting before those service in itself achieves nothing.\
>>>> > >> I think what may cause the module to load maybe the fact
>>>> > >> gssproxy.service includes:
>>>> > >> Requires=proc-fs-nfsd.mount
>>> > >
>>> > > I'd expect that to only load the nfsd module.
>>> > >
>>> > > Hm, I think nfsd actually has a dependency on auth_rpcgss. I wonder if
>>> > > that's correct. Maybe that's what's doing it.
>> > It is... In another thread I clearly showed that....
> The dependency of nfsd on auth_rpcgss looks to me like an implementation
> detail. The client module doesn't have the same dependency.
It does... auth_rpcgss and friends are loaded during the mount

> We definitely shouldn't depend on it, as the dependency could get removed
> some day.
Well if that dependency gets removed (which I can't see why it ever would)
we'll deal with the then...

Remember the scope of this patch set is to enable the server to use
the gssproxy, seamlessly... As thing stand today, I think we can
easily do that by modifying the server service file to "call"
gssproxy when its installed... Plus since both gssproxy
and the server have the 'Requires=proc-fs-nfsd.mount' in their
service files and Today we know that loads the needed modules
I thinking we are good go to...

If things change down the road, lets deal with it then...

>
>> > So I really don't think another unit file is necessary...
> I haven't seen an alternative yet.
I'm thinking adding yet another unit file to do something that
is already being done is just adding more complexity to an
already confusing situation...

So I guess my alternative is to do nothing. ;-)

steved.

2014-09-23 01:55:50

by J. Bruce Fields

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

On Mon, Sep 22, 2014 at 08:26:55PM -0400, Simo Sorce wrote:
> On Mon, 22 Sep 2014 19:58:05 -0400
> Steve Dickson <[email protected]> wrote:
>
> >
> >
> > On 09/22/2014 06:34 PM, J. Bruce Fields 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 <[email protected]> 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 <[email protected]>
> > >>>>>> ---
> > >>>>>> 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.
> > No, why should there be? There is After= line referring to rpc.gssd
> > in nfs-server.service
> >
> > grep After systemd/nfs-server.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=nfs-config.service
> >
> > So when the server starts,rpc.gssd will start and rpc.svcgssd will
> > start if gssproxy is not enable and there is a key tab.
>
> They can start in parallel, there is nothing in that line that makes
> one start before the other.
>
> If you are relying on this you are relying on luck.
>
> > >> 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.
> > Right, which will load the sunrpc modules.
>
> No, starting before those service in itself achieves nothing.\
> I think what may cause the module to load maybe the fact
> gssproxy.service includes:
> Requires=proc-fs-nfsd.mount

I'd expect that to only load the nfsd module.

Hm, I think nfsd actually has a dependency on auth_rpcgss. I wonder if
that's correct. Maybe that's what's doing it.

> But to be honest this was a hack to deal with broken nfs service files,
> gss-proxy should not require nfsd, the dependency should be the other
> way around, as gss-proxy can run on machines where there is no nfs
> service whatsoever, as it stand this is a bug in gssproxy.service and
> I'd like to fix it.

So, something like this? (Untested, no idea if I'm doing this right.)

--b.

diff --git a/systemd/auth-rpcgss-module.service b/systemd/auth-rpcgss-module.service
new file mode 100644
index 000000000000..252545b458fd
--- /dev/null
+++ b/systemd/auth-rpcgss-module.service
@@ -0,0 +1,15 @@
+# We want to start gss-proxy on kernels that support it and rpc.svcgssd
+# on those that don't. Those services check for support by checking
+# for existence of the path /proc/net/rpc/use-gss-proxy. Before they
+# can perform that check, they need this module loaded. (Unless
+# rpcsec_gss is built directly into the kernel, in which case this unit
+# will fail. But that's OK.)
+[Unit]
+Description=Kernel Module supporting RPCSEC_GSS
+Before=gssproxy.service rpc-svcgssd.service
+
+[Service]
+ExecStart=/sbin/modprobe -q auth_rpcgss
+
+[Install]
+WantedBy=gssproxy.service rpc-svcgssd.service

2014-09-22 21:32:43

by Simo Sorce

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

On Mon, 22 Sep 2014 17:14:05 -0400
Steve Dickson <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >>>> ---
> >>>> 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
> >>> " 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.

This guarantees ordering (to some degree) between rpc.gssd and
rpoc.svcgssd, but says nothing about gssproxy ...

> So when gssproxy.service does it's "Before=nfs-secure.service
> nfs-secure-server.service" line everything is loaded before gssproxy
> start...
>
> 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..

Maybe we should add "Before: gssproxy.service rpc-svcgssd.service"
to var-lib-nfs-rpc_pipefs.mount instead (and also drop any mention of
nfs services in gssproxy unit file so you have complete control of the
dependencies ?

> >
> >> 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...
> There were a number of "colorful" discussion on how things
> were so broken until systemd came along and saved humanity! ;-)

This doesn't help really, I see no reason why we could not have a pre
exec statement to modprobe rpc_authgss in a unit file (whether that is
var-lib-nfs-rpc_pipefs.mount or something else), to guarantee correct
ordering.

Simo.

--
Simo Sorce * Red Hat, Inc * New York

2014-09-23 00:27:03

by Simo Sorce

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

On Mon, 22 Sep 2014 19:58:05 -0400
Steve Dickson <[email protected]> wrote:

>
>
> On 09/22/2014 06:34 PM, J. Bruce Fields 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 <[email protected]> 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 <[email protected]>
> >>>>>> ---
> >>>>>> 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.
> No, why should there be? There is After= line referring to rpc.gssd
> in nfs-server.service
>
> grep After systemd/nfs-server.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=nfs-config.service
>
> So when the server starts,rpc.gssd will start and rpc.svcgssd will
> start if gssproxy is not enable and there is a key tab.

They can start in parallel, there is nothing in that line that makes
one start before the other.

If you are relying on this you are relying on luck.

> >> 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.
> Right, which will load the sunrpc modules.

No, starting before those service in itself achieves nothing.\
I think what may cause the module to load maybe the fact
gssproxy.service includes:
Requires=proc-fs-nfsd.mount

But to be honest this was a hack to deal with broken nfs service files,
gss-proxy should not require nfsd, the dependency should be the other
way around, as gss-proxy can run on machines where there is no nfs
service whatsoever, as it stand this is a bug in gssproxy.service and
I'd like to fix it.

> >> 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.
> On the client side the mount -o sec=krb5? loads auth_rpcgss module.

This happens eons after gss-proxy has been started.

> Maybe the loading of nfsd loads the module? But I don't think that
> module has to be loaded for any of the gss daemons (gssd, svcgssd or
> gssproxy) to start successfully...

Atm it *has to*.
If the module is not loaded then /proc/net/rpc/use-gss-proxy will not
exist and GSS-Proxy will not register with the kernel (it does so only
at startup). It also means rpc.svcgssd will be started when it shouldn't


> >>>> 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.
> Yes, this was the conversation I was referring to.. Thank you
> for digging it out..
>
> >
> > 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.
> Lets talk with the systemd people to see what they say...

Feel free to CC the,.

Simo.


--
Simo Sorce * Red Hat, Inc * New York

2014-09-22 19:20:13

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 1/2] nfs-service: Added the starting of gssproxy

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 <[email protected]>
---
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 gssproxy.service rpc-svcgssd.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
--
1.9.3


2014-09-23 02:11:12

by J. Bruce Fields

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

On Tue, Sep 23, 2014 at 12:08:04PM +1000, NeilBrown wrote:
> On Mon, 22 Sep 2014 21:55:49 -0400 "J. Bruce Fields" <[email protected]>
> wrote:
> > So, something like this? (Untested, no idea if I'm doing this right.)
> >
> > --b.
> >
> > diff --git a/systemd/auth-rpcgss-module.service b/systemd/auth-rpcgss-module.service
> > new file mode 100644
> > index 000000000000..252545b458fd
> > --- /dev/null
> > +++ b/systemd/auth-rpcgss-module.service
> > @@ -0,0 +1,15 @@
> > +# We want to start gss-proxy on kernels that support it and rpc.svcgssd
> > +# on those that don't. Those services check for support by checking
> > +# for existence of the path /proc/net/rpc/use-gss-proxy. Before they
> > +# can perform that check, they need this module loaded. (Unless
> > +# rpcsec_gss is built directly into the kernel, in which case this unit
> > +# will fail. But that's OK.)
> > +[Unit]
> > +Description=Kernel Module supporting RPCSEC_GSS
> > +Before=gssproxy.service rpc-svcgssd.service
> > +
> > +[Service]
> > +ExecStart=/sbin/modprobe -q auth_rpcgss
>
> I think you need
> Type=oneshot
>
> else systemd won't wait for the modprobe to complete before running the other
> services.

Whoops, yes, thought I'd done that but I guess not.

> > +
> > +[Install]
> > +WantedBy=gssproxy.service rpc-svcgssd.service
>
> I don't think you want an install section. That means the service has to be
> explicitly enabled, which is a pain.
> I think nfs-server.service should Want= this.
> I also think
>
> ConditionPathExists=/etc/krb5.keytab
>
> would be appropriate.

OK.

--b.

2014-09-22 19:26:06

by Simo Sorce

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

On Mon, 22 Sep 2014 15:20:07 -0400
Steve Dickson <[email protected]> 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 <[email protected]>
> ---
> 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 ?

Simo.

--
Simo Sorce * Red Hat, Inc * New York

2014-09-23 15:08:18

by Simo Sorce

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

On Tue, 23 Sep 2014 10:58:38 -0400
Steve Dickson <[email protected]> wrote:

>
>
> On 09/23/2014 08:52 AM, Simo Sorce wrote:
> > On Mon, 22 Sep 2014 21:19:18 -0400
> > Steve Dickson <[email protected]> wrote:
> >
> >>
> >>
> >> On 09/22/2014 08:19 PM, Simo Sorce wrote:
> >>> On Mon, 22 Sep 2014 18:57:10 -0400
> >>> Steve Dickson <[email protected]> wrote:
> >>>
> >>>>
> >>>>
> >>>> On 09/22/2014 05:32 PM, Simo Sorce wrote:
> >>>>> On Mon, 22 Sep 2014 17:14:05 -0400
> >>>>> Steve Dickson <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >>>>>>>>>> ---
> >>>>>>>>>> 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 " 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.
> >>>>>
> >>>>> This guarantees ordering (to some degree) between rpc.gssd and
> >>>>> rpoc.svcgssd, but says nothing about gssproxy ...
> >>>> The question was how is the auth_rpcgss module being loaded.
> >>>> Since both rpc-svcgssd.service and rpc-gssd.service service have
> >>>> a After=var-lib-nfs-rpc_pipefs.mount and gssproxy is requiring
> >>>> them, that's how auth_rpcgss is being loaded.
> >>>>
> >>>> If you only in enable gssproxy (not nfs-server or nfs-client)
> >>>> the module still get loaded via gssproxy,service file
> >>>>
> >>>>>> So when gssproxy.service does it's "Before=nfs-secure.service
> >>>>>> nfs-secure-server.service" line everything is loaded before
> >>>>>> gssproxy start...
> >>>>>>
> >>>>>> 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..
> >>>>>
> >>>>> Maybe we should add "Before: gssproxy.service
> >>>>> rpc-svcgssd.service" to var-lib-nfs-rpc_pipefs.mount instead
> >>>>> (and also drop any mention of nfs services in gssproxy unit
> >>>>> file so you have complete control of the dependencies ?
> >>>> No.
> >>>> The loading of sunrpc and the mounting of the file system has
> >>>> nothing to do with starting up the gssd daemons.
> >>>>
> >>>> I would suggest gssproxy does to two things:
> >>>>
> >>>> 1) Add a Requires: nfs-utils to the spec file since you are
> >>>> requiring services from nfs-utils
> >>>
> >>> No we are not requiring services from nfs-utils, nfs-utils is one
> >>> of our users, you got that reversed.
> >> So having "Before=nfs-secure.service nfs-secure-server.service" in
> >> gssproxy.service and not having nfs-utils installed will not cause
> >> the service fail during start up?
> >
> > It will not, but right now gssproxy.service also has:
> > Requires=proc-fs-nfsd.mount
> >
> > I want to drop this one.
> You don't want to drop this because it loads the needed kernel
> modules.

I want the unit file Bruce proposed to require this and to be run before
gssproxy.service
Once we have that unit I can drop mine which has always been just a
workaround.

Simo.

--
Simo Sorce * Red Hat, Inc * New York

2014-09-23 21:15:23

by Steve Dickson

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


On 09/23/2014 04:25 PM, J. Bruce Fields wrote:
>> I through this into my test world
> Thanks!
>
>> > and one side effect of this patch
>> > is both rpc.gssd and rpc.svcgssd daemons are *always* started when
>> > a key tab exists (/etc/krb5.keytab) and *all* the services (nfs-client,
>> > nfs-server, rpc-gssd, and rpc-svcgssd) are disabled, which is not
>> > good... Those daemons don't need to be started when both sides
>> > are disabled... But the auth_rpcgss is loaded! ;-)
> Weird. I can't see how this patch on its own would have any effect on
> that.
I agree... I was surprised myself... Unfortunately I'm sure how to
debug what/how/when systemd starts things... But I moving
the key tab out of the way stop the daemons from starting.
Moving it back in place caused the daemons to start again,
which didn't happen with the current f21 nfs-utils.

Maybe its a bug in some other unit file...

steved.

2014-09-23 19:29:09

by J. Bruce Fields

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

On Tue, Sep 23, 2014 at 08:52:45AM -0400, Simo Sorce wrote:
> It will not, but right now gssproxy.service also has:
> Requires=proc-fs-nfsd.mount
>
> I want to drop this one.

By the way, I think all you actually want is After=proc-fs-nfsd.mount.

You don't actually want to bail out if fs-nfsd.mount fails, in fact you
don't care if it's activated at all, all you care about is that *if*
it's already activated (because nfsd is), then you want it to start
before you.

And in fact we could instead replace that with a Before=gssproxy.service
in proc-fs-nfsd.mount if you preferred. I don't know how to decide
which place it goes, or if it matters.

--b.

>
> > >> 2) Add a After=var-lib-nfs-rpc_pipefs.mount to the
> > >> gssproxy.service file since gssproxy could careless about either
> > >> rpc.gssd or rpc.svcgssd daemons. All it is looking for is the
> > >> sunrpc and auth_rpcgss kernel modules.
> > >
> > > The correct thing is to add a Before: gssproxy.service to
> > > var-lib-nfs-rpc_pipefs.mount IMO.
> > Again what happen if gssproxy is not installed? Things will
> > still come up successfully?
>
> Yes, afaik After and Before are just ordering instruction and do not
> cause any failure if the units are not present at all, or fail to start.
>
> There are Want and Require directives for strong relationships.
>
> Simo.
>
> --
> Simo Sorce * Red Hat, Inc * New York

2014-09-23 16:05:40

by J. Bruce Fields

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

On Tue, Sep 23, 2014 at 11:52:12AM -0400, Steve Dickson wrote:
> On 09/23/2014 11:16 AM, J. Bruce Fields wrote:
> > On Tue, Sep 23, 2014 at 11:06:45AM -0400, Steve Dickson wrote:
> >> >
> >> >
> >> > On 09/22/2014 09:55 PM, J. Bruce Fields wrote:
> >>> > > On Mon, Sep 22, 2014 at 08:26:55PM -0400, Simo Sorce wrote:
> >>>> > >> No, starting before those service in itself achieves nothing.\
> >>>> > >> I think what may cause the module to load maybe the fact
> >>>> > >> gssproxy.service includes:
> >>>> > >> Requires=proc-fs-nfsd.mount
> >>> > >
> >>> > > I'd expect that to only load the nfsd module.
> >>> > >
> >>> > > Hm, I think nfsd actually has a dependency on auth_rpcgss. I wonder if
> >>> > > that's correct. Maybe that's what's doing it.
> >> > It is... In another thread I clearly showed that....
> > The dependency of nfsd on auth_rpcgss looks to me like an implementation
> > detail. The client module doesn't have the same dependency.
> It does... auth_rpcgss and friends are loaded during the mount

They get loaded for other reasons.

The nfs module does not have any static dependency on the auth_rpcgss
module that I can see.

> > We definitely shouldn't depend on it, as the dependency could get removed
> > some day.
> Well if that dependency gets removed (which I can't see why it ever would)
> we'll deal with the then...

No, please don't depend on this implementation detail.

--b.

2014-09-24 15:07:28

by Steve Dickson

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

On 09/23/2014 05:15 PM, Steve Dickson wrote:
>
> On 09/23/2014 04:25 PM, J. Bruce Fields wrote:
>>> I through this into my test world
>> Thanks!
>>
>>>> and one side effect of this patch
>>>> is both rpc.gssd and rpc.svcgssd daemons are *always* started when
>>>> a key tab exists (/etc/krb5.keytab) and *all* the services (nfs-client,
>>>> nfs-server, rpc-gssd, and rpc-svcgssd) are disabled, which is not
>>>> good... Those daemons don't need to be started when both sides
>>>> are disabled... But the auth_rpcgss is loaded! ;-)
>> Weird. I can't see how this patch on its own would have any effect on
>> that.
It turns out I must have had the nfs-client.target enabled...

I just realize 'systemctl disable nfs-client' does not fail,
but it does not do anything either. :-( I would think
it should fail with some type of "unit not found", but it
does not...

'systemctl disable nfs-client.target' was the command I
wanted to disable the client, so your patch works...

Question, Why is rpc.svcgssd/gssproxy when only the
nfs-client is enabled??

steved.

2014-09-24 15:30:54

by Steve Dickson

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



On 09/24/2014 11:23 AM, J. Bruce Fields wrote:
> On Wed, Sep 24, 2014 at 11:07:16AM -0400, Steve Dickson wrote:
>> On 09/23/2014 05:15 PM, Steve Dickson wrote:
>>>
>>> On 09/23/2014 04:25 PM, J. Bruce Fields wrote:
>>>>> I through this into my test world
>>>> Thanks!
>>>>
>>>>>> and one side effect of this patch
>>>>>> is both rpc.gssd and rpc.svcgssd daemons are *always* started when
>>>>>> a key tab exists (/etc/krb5.keytab) and *all* the services (nfs-client,
>>>>>> nfs-server, rpc-gssd, and rpc-svcgssd) are disabled, which is not
>>>>>> good... Those daemons don't need to be started when both sides
>>>>>> are disabled... But the auth_rpcgss is loaded! ;-)
>>>> Weird. I can't see how this patch on its own would have any effect on
>>>> that.
>> It turns out I must have had the nfs-client.target enabled...
>>
>> I just realize 'systemctl disable nfs-client' does not fail,
>> but it does not do anything either. :-( I would think
>> it should fail with some type of "unit not found", but it
>> does not...
>
> Huh, yes, that seems kind of unhelpful.
>
> # systemctl disable dighdfdij
> # echo $?
> 0

yup... that's what bit me in the original testing...

The wonderful world of systemd!!! :-P

Your patch has been committed...

steved


2014-09-22 19:43:11

by Steve Dickson

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



On 09/22/2014 03:26 PM, Simo Sorce wrote:
> On Mon, 22 Sep 2014 15:20:07 -0400
> Steve Dickson <[email protected]> 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 <[email protected]>
>> ---
>> 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...

Plus, from my understanding... loading module from a service
file is a big no no! People were having problems with
way back when...

steved.


2014-09-22 20:00:58

by Simo Sorce

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

On Mon, 22 Sep 2014 15:53:46 -0400
Steve Dickson <[email protected]> wrote:

>
>
> On 09/22/2014 03:46 PM, Simo Sorce wrote:
> > On Mon, 22 Sep 2014 15:40:57 -0400
> > "J. Bruce Fields" <[email protected]> wrote:
> >
> >> On Mon, Sep 22, 2014 at 03:20:07PM -0400, Steve Dickson 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.
> >>
> >> That should read "when the kernel supports gssproxy", not "when the
> >> gssproxy daemon is already running."
> >
> > Actually the language is currently correct but it is another bug,
> > the systemd/rpc-svcgssd.service file still includes
> > "ConditionPathExists=|!/run/gssproxy.pid"
> > This line should be removed in this patch.
>
> I left that on purpose because isn't that ConditionPathExists
> seeing if /run/gssproxy.pid exists and if it does
> it means gssproxy is already running so rpc.svcgssd
> should not start?

No.
First of all the fact gss-proxy is running does not mean it is
serving nfsd necessarily, it may be running on an older kernel where it
servers apache or some other process (remember gssproxy is not just
for nfsd).
Second you already have
"ConditionPathExists=|!/proc/net/rpc/use-gss-proxy" which is the
correct trigger to decide which of the two to use.

Simo.

--
Simo Sorce * Red Hat, Inc * New York

2014-09-24 15:23:37

by J. Bruce Fields

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

On Wed, Sep 24, 2014 at 11:07:16AM -0400, Steve Dickson wrote:
> On 09/23/2014 05:15 PM, Steve Dickson wrote:
> >
> > On 09/23/2014 04:25 PM, J. Bruce Fields wrote:
> >>> I through this into my test world
> >> Thanks!
> >>
> >>>> and one side effect of this patch
> >>>> is both rpc.gssd and rpc.svcgssd daemons are *always* started when
> >>>> a key tab exists (/etc/krb5.keytab) and *all* the services (nfs-client,
> >>>> nfs-server, rpc-gssd, and rpc-svcgssd) are disabled, which is not
> >>>> good... Those daemons don't need to be started when both sides
> >>>> are disabled... But the auth_rpcgss is loaded! ;-)
> >> Weird. I can't see how this patch on its own would have any effect on
> >> that.
> It turns out I must have had the nfs-client.target enabled...
>
> I just realize 'systemctl disable nfs-client' does not fail,
> but it does not do anything either. :-( I would think
> it should fail with some type of "unit not found", but it
> does not...

Huh, yes, that seems kind of unhelpful.

# systemctl disable dighdfdij
# echo $?
0

--b.

2014-09-23 20:17:36

by Steve Dickson

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

On 09/23/2014 03:23 PM, J. Bruce Fields wrote:
> commit 3fab32b4bf96
> Author: J. Bruce Fields <[email protected]>
> Date: Mon Sep 22 21:43:59 2014 -0400
>
> systemd: manually insert auth_rpcgss module.
>
> We need to insert the auth_rpcgss module before starting rpc.svcgssd or
> gss-proxy, for two reasons:
>
> - gss-proxy needs access to the /proc/net/rpc/use-gss-proxy file
> to set up communication with knfsd.
> - the unit files need to able to test for the existance of the
> same path in order to decide whether the kernel supports
> gss-proxy or not.
>
> Currently we're using dependencies on proc-fs-nfsd.mount for this, but
> that works only because of the nfsd kernel module references some
> symbols in auth_rpcgss, which is an odd implementation detail we're
> likely to fix some day.
>
> Signed-off-by: J. Bruce Fields <[email protected]>
>
> diff --git a/systemd/auth-rpcgss-module.service b/systemd/auth-rpcgss-module.service
> new file mode 100644
> index 000000000000..3fc2f4ac924f
> --- /dev/null
> +++ b/systemd/auth-rpcgss-module.service
> @@ -0,0 +1,14 @@
> +# We want to start gss-proxy on kernels that support it and rpc.svcgssd
> +# on those that don't. Those services check for support by checking
> +# for existence of the path /proc/net/rpc/use-gss-proxy. Before they
> +# can perform that check, they need this module loaded. (Unless
> +# rpcsec_gss support is built directly into the kernel, in which case this
> +# unit will fail. But that's OK.)
> +[Unit]
> +Description=Kernel Module supporting RPCSEC_GSS
> +Before=gssproxy.service rpc-svcgssd.service
> +ConditionPathExists=/etc/krb5.keytab
> +
> +[Service]
> +Type=oneshot
> +ExecStart=/sbin/modprobe -q auth_rpcgss
> diff --git a/systemd/nfs-client.target b/systemd/nfs-client.target
> index f3c09e76a0f7..474f5e9ad74b 100644
> --- a/systemd/nfs-client.target
> +++ b/systemd/nfs-client.target
> @@ -5,7 +5,7 @@ Wants=remote-fs-pre.target
>
> # Note: we don't "Wants=rpc-statd.service" as "mount.nfs" will arrange to
> # start that on demand if needed.
> -Wants=rpc-gssd.service rpc-svcgssd.service
> +Wants=rpc-gssd.service rpc-svcgssd.service auth-rpcgss-module.service
> Wants=nfs-blkmap.service rpc-statd-notify.service
> Before=rpc-gssd.service rpc-svcgssd.service nfs-blkmap.service
>
> diff --git a/systemd/nfs-server.service b/systemd/nfs-server.service
> index 2fa7387e1cb9..fd213a3995de 100644
> --- a/systemd/nfs-server.service
> +++ b/systemd/nfs-server.service
> @@ -3,6 +3,7 @@ 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=auth-rpcgss-module.service
> Wants=rpc-statd-notify.service
>
> After= network.target proc-fs-nfsd.mount rpcbind.target nfs-mountd.service
>
I through this into my test world and one side effect of this patch
is both rpc.gssd and rpc.svcgssd daemons are *always* started when
a key tab exists (/etc/krb5.keytab) and *all* the services (nfs-client,
nfs-server, rpc-gssd, and rpc-svcgssd) are disabled, which is not
good... Those daemons don't need to be started when both sides
are disabled... But the auth_rpcgss is loaded! ;-)

steved.

2014-09-23 02:01:25

by NeilBrown

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

On Mon, 22 Sep 2014 18:34:23 -0400 "J. Bruce Fields" <[email protected]>
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 <[email protected]> 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 <[email protected]>
> > >>>> ---
> > >>>> 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


Attachments:
signature.asc (828.00 B)

2014-09-23 20:25:16

by J. Bruce Fields

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

On Tue, Sep 23, 2014 at 04:17:31PM -0400, Steve Dickson wrote:
> On 09/23/2014 03:23 PM, J. Bruce Fields wrote:
> > commit 3fab32b4bf96
> > Author: J. Bruce Fields <[email protected]>
> > Date: Mon Sep 22 21:43:59 2014 -0400
> >
> > systemd: manually insert auth_rpcgss module.
> >
> > We need to insert the auth_rpcgss module before starting rpc.svcgssd or
> > gss-proxy, for two reasons:
> >
> > - gss-proxy needs access to the /proc/net/rpc/use-gss-proxy file
> > to set up communication with knfsd.
> > - the unit files need to able to test for the existance of the
> > same path in order to decide whether the kernel supports
> > gss-proxy or not.
> >
> > Currently we're using dependencies on proc-fs-nfsd.mount for this, but
> > that works only because of the nfsd kernel module references some
> > symbols in auth_rpcgss, which is an odd implementation detail we're
> > likely to fix some day.
> >
> > Signed-off-by: J. Bruce Fields <[email protected]>
> >
> > diff --git a/systemd/auth-rpcgss-module.service b/systemd/auth-rpcgss-module.service
> > new file mode 100644
> > index 000000000000..3fc2f4ac924f
> > --- /dev/null
> > +++ b/systemd/auth-rpcgss-module.service
> > @@ -0,0 +1,14 @@
> > +# We want to start gss-proxy on kernels that support it and rpc.svcgssd
> > +# on those that don't. Those services check for support by checking
> > +# for existence of the path /proc/net/rpc/use-gss-proxy. Before they
> > +# can perform that check, they need this module loaded. (Unless
> > +# rpcsec_gss support is built directly into the kernel, in which case this
> > +# unit will fail. But that's OK.)
> > +[Unit]
> > +Description=Kernel Module supporting RPCSEC_GSS
> > +Before=gssproxy.service rpc-svcgssd.service
> > +ConditionPathExists=/etc/krb5.keytab
> > +
> > +[Service]
> > +Type=oneshot
> > +ExecStart=/sbin/modprobe -q auth_rpcgss
> > diff --git a/systemd/nfs-client.target b/systemd/nfs-client.target
> > index f3c09e76a0f7..474f5e9ad74b 100644
> > --- a/systemd/nfs-client.target
> > +++ b/systemd/nfs-client.target
> > @@ -5,7 +5,7 @@ Wants=remote-fs-pre.target
> >
> > # Note: we don't "Wants=rpc-statd.service" as "mount.nfs" will arrange to
> > # start that on demand if needed.
> > -Wants=rpc-gssd.service rpc-svcgssd.service
> > +Wants=rpc-gssd.service rpc-svcgssd.service auth-rpcgss-module.service
> > Wants=nfs-blkmap.service rpc-statd-notify.service
> > Before=rpc-gssd.service rpc-svcgssd.service nfs-blkmap.service
> >
> > diff --git a/systemd/nfs-server.service b/systemd/nfs-server.service
> > index 2fa7387e1cb9..fd213a3995de 100644
> > --- a/systemd/nfs-server.service
> > +++ b/systemd/nfs-server.service
> > @@ -3,6 +3,7 @@ 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=auth-rpcgss-module.service
> > Wants=rpc-statd-notify.service
> >
> > After= network.target proc-fs-nfsd.mount rpcbind.target nfs-mountd.service
> >
> I through this into my test world

Thanks!

> and one side effect of this patch
> is both rpc.gssd and rpc.svcgssd daemons are *always* started when
> a key tab exists (/etc/krb5.keytab) and *all* the services (nfs-client,
> nfs-server, rpc-gssd, and rpc-svcgssd) are disabled, which is not
> good... Those daemons don't need to be started when both sides
> are disabled... But the auth_rpcgss is loaded! ;-)

Weird. I can't see how this patch on its own would have any effect on
that.

--b.

2014-09-23 15:20:03

by J. Bruce Fields

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

On Tue, Sep 23, 2014 at 08:48:54AM -0400, Simo Sorce wrote:
> On Tue, 23 Sep 2014 12:08:04 +1000
> NeilBrown <[email protected]> wrote:
> > I don't think you want an install section. That means the service
> > has to be explicitly enabled, which is a pain.
> > I think nfs-server.service should Want= this.
> > I also think
> >
> > ConditionPathExists=/etc/krb5.keytab
> >
> > would be appropriate.
>
> If GSS-Proxy is in use the administrator may choose to use a keytab in
> a different location, so I am not entirely sure we should depend
> on /etc/krb5.keytab, however it is also ok to decide that if the admin
> wants to use a different place that they create a custom unit file.
> Up to you.

Note we're already using the same line in rpc-gssd.service and
rpc-svcgssd.service.

Can you suggest a better "does this host have krb5 configured?" test?

I think false positives are OK, but not false negatives.

(So, if we run those daemons unnecessarily it may annoy some people, but
if we fail to run them when they're needed then things really don't
work.)

--b.

2014-09-22 19:20:14

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 2/2] rpc.svcgssd: the build of rpc.svcgssd is off by default

Due to the fact the gssproxy is not managing the
GASSAPI credentials, on the server, there is
no need to build/install rpc.svcgssd.

To re-enable the build use the --enable-svcgss
configure flag

Signed-off-by: Steve Dickson <[email protected]>
---
configure.ac | 23 +++++++++++++++++++----
utils/gssd/Makefile.am | 11 +++++++++--
2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index bc48373..6767190 100644
--- a/configure.ac
+++ b/configure.ac
@@ -90,21 +90,36 @@ AC_ARG_ENABLE(nfsv41,

AC_ARG_ENABLE(gss,
[AC_HELP_STRING([--enable-gss],
- [enable support for rpcsec_gss @<:@default=yes@:>@])],
+ [enable client support for rpcsec_gss @<:@default=yes@:>@])],
enable_gss=$enableval,
enable_gss=yes)
if test "$enable_gss" = yes; then
GSSD=gssd
- SVCGSSD=svcgssd
else
enable_gss=
GSSD=
- SVCGSSD=
fi
AC_SUBST(GSSD)
- AC_SUBST(SVCGSSD)
AC_SUBST(enable_gss)
AM_CONDITIONAL(CONFIG_GSS, [test "$enable_gss" = "yes"])
+
+AC_ARG_ENABLE(svcgss,
+ [AC_HELP_STRING([--enable-svcgss],
+ [enable server support for rpcsec_gss @<:@default=no@:>@])],
+ enable_svcgss=$enableval,
+ enable_svcgss=no)
+ if test "$enable_gss" = yes; then
+ if "enable_svcgss" = yes; then
+ SVCGSSD=svcgssd
+ fi
+ else
+ enable_svcgss=
+ SVCGSSD=
+ fi
+ AC_SUBST(SVCGSSD)
+ AC_SUBST(enable_svcgss)
+ AM_CONDITIONAL(CONFIG_SVCGSS, [test "$enable_svcgss" = "yes"])
+
AC_ARG_ENABLE(kprefix,
[AC_HELP_STRING([--enable-kprefix], [install progs as rpc.knfsd etc])],
test "$enableval" = "yes" && kprefix=k,
diff --git a/utils/gssd/Makefile.am b/utils/gssd/Makefile.am
index af59791..9835117 100644
--- a/utils/gssd/Makefile.am
+++ b/utils/gssd/Makefile.am
@@ -1,10 +1,17 @@
## Process this file with automake to produce Makefile.in

-man8_MANS = gssd.man svcgssd.man
+man8_MANS = gssd.man
+if CONFIG_SVCGSS
+man8_MANS += svcgssd.man
+endif

RPCPREFIX = rpc.
KPREFIX = @kprefix@
-sbin_PREFIXED = gssd svcgssd
+sbin_PREFIXED = gssd
+if CONFIG_SVCGSS
+sbin_PREFIXED += svcgssd
+endif
+
sbin_PROGRAMS = $(sbin_PREFIXED)

EXTRA_DIST = \
--
1.9.3


2014-09-23 12:52:54

by Simo Sorce

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

On Mon, 22 Sep 2014 21:19:18 -0400
Steve Dickson <[email protected]> wrote:

>
>
> On 09/22/2014 08:19 PM, Simo Sorce wrote:
> > On Mon, 22 Sep 2014 18:57:10 -0400
> > Steve Dickson <[email protected]> wrote:
> >
> >>
> >>
> >> On 09/22/2014 05:32 PM, Simo Sorce wrote:
> >>> On Mon, 22 Sep 2014 17:14:05 -0400
> >>> Steve Dickson <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >>>>>>>> ---
> >>>>>>>> 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 " 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.
> >>>
> >>> This guarantees ordering (to some degree) between rpc.gssd and
> >>> rpoc.svcgssd, but says nothing about gssproxy ...
> >> The question was how is the auth_rpcgss module being loaded. Since
> >> both rpc-svcgssd.service and rpc-gssd.service service have
> >> a After=var-lib-nfs-rpc_pipefs.mount and gssproxy is requiring
> >> them, that's how auth_rpcgss is being loaded.
> >>
> >> If you only in enable gssproxy (not nfs-server or nfs-client) the
> >> module still get loaded via gssproxy,service file
> >>
> >>>> So when gssproxy.service does it's "Before=nfs-secure.service
> >>>> nfs-secure-server.service" line everything is loaded before
> >>>> gssproxy start...
> >>>>
> >>>> 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..
> >>>
> >>> Maybe we should add "Before: gssproxy.service rpc-svcgssd.service"
> >>> to var-lib-nfs-rpc_pipefs.mount instead (and also drop any mention
> >>> of nfs services in gssproxy unit file so you have complete control
> >>> of the dependencies ?
> >> No.
> >> The loading of sunrpc and the mounting of the file system has
> >> nothing to do with starting up the gssd daemons.
> >>
> >> I would suggest gssproxy does to two things:
> >>
> >> 1) Add a Requires: nfs-utils to the spec file since you are
> >> requiring services from nfs-utils
> >
> > No we are not requiring services from nfs-utils, nfs-utils is one of
> > our users, you got that reversed.
> So having "Before=nfs-secure.service nfs-secure-server.service" in
> gssproxy.service and not having nfs-utils installed will not cause the
> service fail during start up?

It will not, but right now gssproxy.service also has:
Requires=proc-fs-nfsd.mount

I want to drop this one.

> >> 2) Add a After=var-lib-nfs-rpc_pipefs.mount to the
> >> gssproxy.service file since gssproxy could careless about either
> >> rpc.gssd or rpc.svcgssd daemons. All it is looking for is the
> >> sunrpc and auth_rpcgss kernel modules.
> >
> > The correct thing is to add a Before: gssproxy.service to
> > var-lib-nfs-rpc_pipefs.mount IMO.
> Again what happen if gssproxy is not installed? Things will
> still come up successfully?

Yes, afaik After and Before are just ordering instruction and do not
cause any failure if the units are not present at all, or fail to start.

There are Want and Require directives for strong relationships.

Simo.

--
Simo Sorce * Red Hat, Inc * New York

2014-09-23 12:45:43

by Simo Sorce

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

On Tue, 23 Sep 2014 11:42:29 +1000
NeilBrown <[email protected]> wrote:

> On Mon, 22 Sep 2014 16:00:50 -0400 Simo Sorce <[email protected]> wrote:
>
> > On Mon, 22 Sep 2014 15:53:46 -0400
> > Steve Dickson <[email protected]> wrote:
> >
> > >
> > >
> > > On 09/22/2014 03:46 PM, Simo Sorce wrote:
> > > > On Mon, 22 Sep 2014 15:40:57 -0400
> > > > "J. Bruce Fields" <[email protected]> wrote:
> > > >
> > > >> On Mon, Sep 22, 2014 at 03:20:07PM -0400, Steve Dickson 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.
> > > >>
> > > >> That should read "when the kernel supports gssproxy", not
> > > >> "when the gssproxy daemon is already running."
> > > >
> > > > Actually the language is currently correct but it is another
> > > > bug, the systemd/rpc-svcgssd.service file still includes
> > > > "ConditionPathExists=|!/run/gssproxy.pid"
> > > > This line should be removed in this patch.
> > >
> > > I left that on purpose because isn't that ConditionPathExists
> > > seeing if /run/gssproxy.pid exists and if it does
> > > it means gssproxy is already running so rpc.svcgssd
> > > should not start?
> >
> > No.
> > First of all the fact gss-proxy is running does not mean it is
> > serving nfsd necessarily, it may be running on an older kernel
> > where it servers apache or some other process (remember gssproxy is
> > not just for nfsd).
> > Second you already have
> > "ConditionPathExists=|!/proc/net/rpc/use-gss-proxy" which is the
> > correct trigger to decide which of the two to use.
> >
>
> Surely gssproxy is only serving nfsd requests if
> both /run/gssproxy.pid exists and /proc/net/rpc/use-gss-proxy exists.
> If either of those files is missing, then rpc.svcgssd needs to run.
> In one case, the gssproxy daemon isn't available for some reason. In
> the other case the kernel cannot make use of it.
>
> Is that not correct?

At the moment it is not, as there is no ordering between the 2
starting /run/gssproxy.pid may simply not be available "yet", also if
it is available it doesn't mean rpc.svcgssd should not start.
GSS-Proxy may be running but /proc/net/rpc/use-gss-proxy may not be
present.
So in general gssproxy.pid is not an indication of whether rpc.svcgssd
should start or not.

> That is exactly the rule that I (tried to) encode in the service file
> with these two conditions.

Then you also need to add After: gssproxy.service to
rpc-svcgssd.service so you give it a chance to start and create the pid
file. But again the mere existence of the pid file is not enough if the
proc file is not there.

Simo.

--
Simo Sorce * Red Hat, Inc * New York

2014-09-22 19:46:53

by Simo Sorce

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

On Mon, 22 Sep 2014 15:40:57 -0400
"J. Bruce Fields" <[email protected]> wrote:

> On Mon, Sep 22, 2014 at 03:20:07PM -0400, Steve Dickson 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.
>
> That should read "when the kernel supports gssproxy", not "when the
> gssproxy daemon is already running."

Actually the language is currently correct but it is another bug, the
systemd/rpc-svcgssd.service file still includes
"ConditionPathExists=|!/run/gssproxy.pid"
This line should be removed in this patch.

Simo.

> --b.
>
> >
> > Signed-off-by: Steve Dickson <[email protected]>
> > ---
> > 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 gssproxy.service rpc-svcgssd.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
> > --
> > 1.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-nfs" in the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Simo Sorce * Red Hat, Inc * New York

2014-09-23 12:46:39

by Simo Sorce

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

On Mon, 22 Sep 2014 21:55:49 -0400
"J. Bruce Fields" <[email protected]> wrote:

> On Mon, Sep 22, 2014 at 08:26:55PM -0400, Simo Sorce wrote:
> > On Mon, 22 Sep 2014 19:58:05 -0400
> > Steve Dickson <[email protected]> wrote:
> >
> > >
> > >
> > > On 09/22/2014 06:34 PM, J. Bruce Fields 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 <[email protected]> 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 <[email protected]>
> > > >>>>>> ---
> > > >>>>>> 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.
> > > No, why should there be? There is After= line referring to
> > > rpc.gssd in nfs-server.service
> > >
> > > grep After systemd/nfs-server.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=nfs-config.service
> > >
> > > So when the server starts,rpc.gssd will start and rpc.svcgssd will
> > > start if gssproxy is not enable and there is a key tab.
> >
> > They can start in parallel, there is nothing in that line that makes
> > one start before the other.
> >
> > If you are relying on this you are relying on luck.
> >
> > > >> 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.
> > > Right, which will load the sunrpc modules.
> >
> > No, starting before those service in itself achieves nothing.\
> > I think what may cause the module to load maybe the fact
> > gssproxy.service includes:
> > Requires=proc-fs-nfsd.mount
>
> I'd expect that to only load the nfsd module.
>
> Hm, I think nfsd actually has a dependency on auth_rpcgss. I wonder
> if that's correct. Maybe that's what's doing it.
>
> > But to be honest this was a hack to deal with broken nfs service
> > files, gss-proxy should not require nfsd, the dependency should be
> > the other way around, as gss-proxy can run on machines where there
> > is no nfs service whatsoever, as it stand this is a bug in
> > gssproxy.service and I'd like to fix it.
>
> So, something like this? (Untested, no idea if I'm doing this right.)
>
> --b.
>
> diff --git a/systemd/auth-rpcgss-module.service
> b/systemd/auth-rpcgss-module.service new file mode 100644
> index 000000000000..252545b458fd
> --- /dev/null
> +++ b/systemd/auth-rpcgss-module.service
> @@ -0,0 +1,15 @@
> +# We want to start gss-proxy on kernels that support it and
> rpc.svcgssd +# on those that don't. Those services check for support
> by checking +# for existence of the
> path /proc/net/rpc/use-gss-proxy. Before they +# can perform that
> check, they need this module loaded. (Unless +# rpcsec_gss is built
> directly into the kernel, in which case this unit +# will fail. But
> that's OK.) +[Unit]
> +Description=Kernel Module supporting RPCSEC_GSS
> +Before=gssproxy.service rpc-svcgssd.service
> +
> +[Service]
> +ExecStart=/sbin/modprobe -q auth_rpcgss
> +
> +[Install]
> +WantedBy=gssproxy.service rpc-svcgssd.service

Not sure about the last WantedBy, but this looks like what I had in
mind.

Simo.

--
Simo Sorce * Red Hat, Inc * New York

2014-09-22 20:44:03

by J. Bruce Fields

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

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 <[email protected]> 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 <[email protected]>
> >> ---
> >> 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.

We want to make sure your testing's not just getting lucky on the
startup order.

> 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.

--b.

2014-09-23 02:09:31

by J. Bruce Fields

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

On Tue, Sep 23, 2014 at 11:42:29AM +1000, NeilBrown wrote:
> Surely gssproxy is only serving nfsd requests if both /run/gssproxy.pid
> exists and /proc/net/rpc/use-gss-proxy exists.
> If either of those files is missing, then rpc.svcgssd needs to run.
> In one case, the gssproxy daemon isn't available for some reason. In the
> other case the kernel cannot make use of it.
>
> Is that not correct?
>
> That is exactly the rule that I (tried to) encode in the service file with
> these two conditions.

Eh, I see your point, but the gssproxy.pid one still seems a little odd
to me.

I guess it's friendlier to people that don't have gss-proxy installed at
all, or want to turn it off for some reason--but then they or their
distro can fix up the unit files too.

Otherwise if we've got gss-proxy and the kernel supports it then it
should work, and if it's failing to come up in that case I'd kind of
like to know why and get a bug report like "gssproxy failed to start" or
"krb5 exports stopped working" rather than "krb5 exports are working in
some subtly different way than they did last week."

--b.

2014-09-22 21:14:09

by Steve Dickson

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



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 <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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.

So when gssproxy.service does it's "Before=nfs-secure.service nfs-secure-server.service"
line everything is loaded before gssproxy start...

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..

>
>> 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...
There were a number of "colorful" discussion on how things
were so broken until systemd came along and saved humanity! ;-)

steved.

2014-09-23 19:40:41

by Simo Sorce

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

On Tue, 23 Sep 2014 15:29:09 -0400
"J. Bruce Fields" <[email protected]> wrote:

> On Tue, Sep 23, 2014 at 08:52:45AM -0400, Simo Sorce wrote:
> > It will not, but right now gssproxy.service also has:
> > Requires=proc-fs-nfsd.mount
> >
> > I want to drop this one.
>
> By the way, I think all you actually want is After=proc-fs-nfsd.mount.
>
> You don't actually want to bail out if fs-nfsd.mount fails, in fact
> you don't care if it's activated at all, all you care about is that
> *if* it's already activated (because nfsd is), then you want it to
> start before you.
>
> And in fact we could instead replace that with a
> Before=gssproxy.service in proc-fs-nfsd.mount if you preferred. I
> don't know how to decide which place it goes, or if it matters.

Given we are already messing with service files I wouldn't object to
moving the depenecy chain creation in nfs-util by adding a
Before=gssproxy.service directive in proc-fs-nfsd.mount and dropping
the Reuire from gssproxy.service

Simo.

--
Simo Sorce * Red Hat, Inc * New York

2014-09-23 19:51:49

by J. Bruce Fields

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

On Tue, Sep 23, 2014 at 03:40:36PM -0400, Simo Sorce wrote:
> On Tue, 23 Sep 2014 15:29:09 -0400
> "J. Bruce Fields" <[email protected]> wrote:
>
> > On Tue, Sep 23, 2014 at 08:52:45AM -0400, Simo Sorce wrote:
> > > It will not, but right now gssproxy.service also has:
> > > Requires=proc-fs-nfsd.mount
> > >
> > > I want to drop this one.
> >
> > By the way, I think all you actually want is After=proc-fs-nfsd.mount.
> >
> > You don't actually want to bail out if fs-nfsd.mount fails, in fact
> > you don't care if it's activated at all, all you care about is that
> > *if* it's already activated (because nfsd is), then you want it to
> > start before you.
> >
> > And in fact we could instead replace that with a
> > Before=gssproxy.service in proc-fs-nfsd.mount if you preferred. I
> > don't know how to decide which place it goes, or if it matters.
>
> Given we are already messing with service files I wouldn't object to
> moving the depenecy chain creation in nfs-util by adding a
> Before=gssproxy.service directive in proc-fs-nfsd.mount and dropping
> the Reuire from gssproxy.service

Wella ctually we should add the auth-rpcgss unit (which includes the
necessary Before). But you might want to s/Requires=/After=/ meanwhile
as the current thing seems possibly unreliable (since there's no
ordering dependency).

--b.

2014-09-22 22:57:13

by Steve Dickson

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



On 09/22/2014 05:32 PM, Simo Sorce wrote:
> On Mon, 22 Sep 2014 17:14:05 -0400
> Steve Dickson <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>>>> ---
>>>>>> 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
>>>>> " 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.
>
> This guarantees ordering (to some degree) between rpc.gssd and
> rpoc.svcgssd, but says nothing about gssproxy ...
The question was how is the auth_rpcgss module being loaded. Since
both rpc-svcgssd.service and rpc-gssd.service service have
a After=var-lib-nfs-rpc_pipefs.mount and gssproxy is requiring
them, that's how auth_rpcgss is being loaded.

If you only in enable gssproxy (not nfs-server or nfs-client) the
module still get loaded via gssproxy,service file

>> So when gssproxy.service does it's "Before=nfs-secure.service
>> nfs-secure-server.service" line everything is loaded before gssproxy
>> start...
>>
>> 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..
>
> Maybe we should add "Before: gssproxy.service rpc-svcgssd.service"
> to var-lib-nfs-rpc_pipefs.mount instead (and also drop any mention of
> nfs services in gssproxy unit file so you have complete control of the
> dependencies ?
No.
The loading of sunrpc and the mounting of the file system has nothing to
do with starting up the gssd daemons.

I would suggest gssproxy does to two things:

1) Add a Requires: nfs-utils to the spec file since you are requiring
services from nfs-utils

2) Add a After=var-lib-nfs-rpc_pipefs.mount to the gssproxy.service
file since gssproxy could careless about either rpc.gssd or rpc.svcgssd
daemons. All it is looking for is the sunrpc and auth_rpcgss kernel
modules.

steved.

>
>>>
>>>> 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...
>> There were a number of "colorful" discussion on how things
>> were so broken until systemd came along and saved humanity! ;-)
>
> This doesn't help really, I see no reason why we could not have a pre
> exec statement to modprobe rpc_authgss in a unit file (whether that is
> var-lib-nfs-rpc_pipefs.mount or something else), to guarantee correct
> ordering.
>
> Simo.
>

2014-09-24 15:15:04

by J. Bruce Fields

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

On Wed, Sep 24, 2014 at 11:07:16AM -0400, Steve Dickson wrote:
> On 09/23/2014 05:15 PM, Steve Dickson wrote:
> >
> > On 09/23/2014 04:25 PM, J. Bruce Fields wrote:
> >>> I through this into my test world
> >> Thanks!
> >>
> >>>> and one side effect of this patch
> >>>> is both rpc.gssd and rpc.svcgssd daemons are *always* started when
> >>>> a key tab exists (/etc/krb5.keytab) and *all* the services (nfs-client,
> >>>> nfs-server, rpc-gssd, and rpc-svcgssd) are disabled, which is not
> >>>> good... Those daemons don't need to be started when both sides
> >>>> are disabled... But the auth_rpcgss is loaded! ;-)
> >> Weird. I can't see how this patch on its own would have any effect on
> >> that.
> It turns out I must have had the nfs-client.target enabled...
>
> I just realize 'systemctl disable nfs-client' does not fail,
> but it does not do anything either. :-( I would think
> it should fail with some type of "unit not found", but it
> does not...
>
> 'systemctl disable nfs-client.target' was the command I
> wanted to disable the client, so your patch works...
>
> Question, Why is rpc.svcgssd/gssproxy when only the
> nfs-client is enabled??

It handles NFSv4.0/krb5 callbacks.

(It's not needed for NFSv4.1+, and even in the 4.0 case the only
consequence is that you'll lose delegations on krb5 mounts. So maybe
we'll be able to remove that dependency, one of these decades....)

--b.

2014-09-23 15:06:52

by Steve Dickson

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



On 09/22/2014 09:55 PM, J. Bruce Fields wrote:
> On Mon, Sep 22, 2014 at 08:26:55PM -0400, Simo Sorce wrote:
>> On Mon, 22 Sep 2014 19:58:05 -0400
>> Steve Dickson <[email protected]> wrote:
>>
>>>
>>>
>>> On 09/22/2014 06:34 PM, J. Bruce Fields 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 <[email protected]> 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 <[email protected]>
>>>>>>>>> ---
>>>>>>>>> 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.
>>> No, why should there be? There is After= line referring to rpc.gssd
>>> in nfs-server.service
>>>
>>> grep After systemd/nfs-server.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=nfs-config.service
>>>
>>> So when the server starts,rpc.gssd will start and rpc.svcgssd will
>>> start if gssproxy is not enable and there is a key tab.
>>
>> They can start in parallel, there is nothing in that line that makes
>> one start before the other.
>>
>> If you are relying on this you are relying on luck.
>>
>>>>> 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.
>>> Right, which will load the sunrpc modules.
>>
>> No, starting before those service in itself achieves nothing.\
>> I think what may cause the module to load maybe the fact
>> gssproxy.service includes:
>> Requires=proc-fs-nfsd.mount
>
> I'd expect that to only load the nfsd module.
>
> Hm, I think nfsd actually has a dependency on auth_rpcgss. I wonder if
> that's correct. Maybe that's what's doing it.
It is... In another thread I clearly showed that.... So I really
don't think another unit file is necessary...

steved.

>
>> But to be honest this was a hack to deal with broken nfs service files,
>> gss-proxy should not require nfsd, the dependency should be the other
>> way around, as gss-proxy can run on machines where there is no nfs
>> service whatsoever, as it stand this is a bug in gssproxy.service and
>> I'd like to fix it.
>
> So, something like this? (Untested, no idea if I'm doing this right.)
>
> --b.
>
> diff --git a/systemd/auth-rpcgss-module.service b/systemd/auth-rpcgss-module.service
> new file mode 100644
> index 000000000000..252545b458fd
> --- /dev/null
> +++ b/systemd/auth-rpcgss-module.service
> @@ -0,0 +1,15 @@
> +# We want to start gss-proxy on kernels that support it and rpc.svcgssd
> +# on those that don't. Those services check for support by checking
> +# for existence of the path /proc/net/rpc/use-gss-proxy. Before they
> +# can perform that check, they need this module loaded. (Unless
> +# rpcsec_gss is built directly into the kernel, in which case this unit
> +# will fail. But that's OK.)
> +[Unit]
> +Description=Kernel Module supporting RPCSEC_GSS
> +Before=gssproxy.service rpc-svcgssd.service
> +
> +[Service]
> +ExecStart=/sbin/modprobe -q auth_rpcgss
> +
> +[Install]
> +WantedBy=gssproxy.service rpc-svcgssd.service
>

2014-09-23 02:08:14

by NeilBrown

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

On Mon, 22 Sep 2014 21:55:49 -0400 "J. Bruce Fields" <[email protected]>
wrote:

> On Mon, Sep 22, 2014 at 08:26:55PM -0400, Simo Sorce wrote:
> > On Mon, 22 Sep 2014 19:58:05 -0400
> > Steve Dickson <[email protected]> wrote:
> >
> > >
> > >
> > > On 09/22/2014 06:34 PM, J. Bruce Fields 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 <[email protected]> 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 <[email protected]>
> > > >>>>>> ---
> > > >>>>>> 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.
> > > No, why should there be? There is After= line referring to rpc.gssd
> > > in nfs-server.service
> > >
> > > grep After systemd/nfs-server.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=nfs-config.service
> > >
> > > So when the server starts,rpc.gssd will start and rpc.svcgssd will
> > > start if gssproxy is not enable and there is a key tab.
> >
> > They can start in parallel, there is nothing in that line that makes
> > one start before the other.
> >
> > If you are relying on this you are relying on luck.
> >
> > > >> 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.
> > > Right, which will load the sunrpc modules.
> >
> > No, starting before those service in itself achieves nothing.\
> > I think what may cause the module to load maybe the fact
> > gssproxy.service includes:
> > Requires=proc-fs-nfsd.mount
>
> I'd expect that to only load the nfsd module.
>
> Hm, I think nfsd actually has a dependency on auth_rpcgss. I wonder if
> that's correct. Maybe that's what's doing it.
>
> > But to be honest this was a hack to deal with broken nfs service files,
> > gss-proxy should not require nfsd, the dependency should be the other
> > way around, as gss-proxy can run on machines where there is no nfs
> > service whatsoever, as it stand this is a bug in gssproxy.service and
> > I'd like to fix it.
>
> So, something like this? (Untested, no idea if I'm doing this right.)
>
> --b.
>
> diff --git a/systemd/auth-rpcgss-module.service b/systemd/auth-rpcgss-module.service
> new file mode 100644
> index 000000000000..252545b458fd
> --- /dev/null
> +++ b/systemd/auth-rpcgss-module.service
> @@ -0,0 +1,15 @@
> +# We want to start gss-proxy on kernels that support it and rpc.svcgssd
> +# on those that don't. Those services check for support by checking
> +# for existence of the path /proc/net/rpc/use-gss-proxy. Before they
> +# can perform that check, they need this module loaded. (Unless
> +# rpcsec_gss is built directly into the kernel, in which case this unit
> +# will fail. But that's OK.)
> +[Unit]
> +Description=Kernel Module supporting RPCSEC_GSS
> +Before=gssproxy.service rpc-svcgssd.service
> +
> +[Service]
> +ExecStart=/sbin/modprobe -q auth_rpcgss

I think you need
Type=oneshot

else systemd won't wait for the modprobe to complete before running the other
services.

> +
> +[Install]
> +WantedBy=gssproxy.service rpc-svcgssd.service

I don't think you want an install section. That means the service has to be
explicitly enabled, which is a pain.
I think nfs-server.service should Want= this.
I also think

ConditionPathExists=/etc/krb5.keytab

would be appropriate.

Thanks,
NeilBrown


Attachments:
signature.asc (828.00 B)

2014-09-23 12:49:01

by Simo Sorce

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

On Tue, 23 Sep 2014 12:08:04 +1000
NeilBrown <[email protected]> wrote:

> On Mon, 22 Sep 2014 21:55:49 -0400 "J. Bruce Fields"
> <[email protected]> wrote:
>
> > On Mon, Sep 22, 2014 at 08:26:55PM -0400, Simo Sorce wrote:
> > > On Mon, 22 Sep 2014 19:58:05 -0400
> > > Steve Dickson <[email protected]> wrote:
> > >
> > > >
> > > >
> > > > On 09/22/2014 06:34 PM, J. Bruce Fields 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 <[email protected]> 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 <[email protected]>
> > > > >>>>>> ---
> > > > >>>>>> 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.
> > > > No, why should there be? There is After= line referring to
> > > > rpc.gssd in nfs-server.service
> > > >
> > > > grep After systemd/nfs-server.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=nfs-config.service
> > > >
> > > > So when the server starts,rpc.gssd will start and rpc.svcgssd
> > > > will start if gssproxy is not enable and there is a key tab.
> > >
> > > They can start in parallel, there is nothing in that line that
> > > makes one start before the other.
> > >
> > > If you are relying on this you are relying on luck.
> > >
> > > > >> 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.
> > > > Right, which will load the sunrpc modules.
> > >
> > > No, starting before those service in itself achieves nothing.\
> > > I think what may cause the module to load maybe the fact
> > > gssproxy.service includes:
> > > Requires=proc-fs-nfsd.mount
> >
> > I'd expect that to only load the nfsd module.
> >
> > Hm, I think nfsd actually has a dependency on auth_rpcgss. I
> > wonder if that's correct. Maybe that's what's doing it.
> >
> > > But to be honest this was a hack to deal with broken nfs service
> > > files, gss-proxy should not require nfsd, the dependency should
> > > be the other way around, as gss-proxy can run on machines where
> > > there is no nfs service whatsoever, as it stand this is a bug in
> > > gssproxy.service and I'd like to fix it.
> >
> > So, something like this? (Untested, no idea if I'm doing this
> > right.)
> >
> > --b.
> >
> > diff --git a/systemd/auth-rpcgss-module.service
> > b/systemd/auth-rpcgss-module.service new file mode 100644
> > index 000000000000..252545b458fd
> > --- /dev/null
> > +++ b/systemd/auth-rpcgss-module.service
> > @@ -0,0 +1,15 @@
> > +# We want to start gss-proxy on kernels that support it and
> > rpc.svcgssd +# on those that don't. Those services check for
> > support by checking +# for existence of the
> > path /proc/net/rpc/use-gss-proxy. Before they +# can perform that
> > check, they need this module loaded. (Unless +# rpcsec_gss is
> > built directly into the kernel, in which case this unit +# will
> > fail. But that's OK.) +[Unit]
> > +Description=Kernel Module supporting RPCSEC_GSS
> > +Before=gssproxy.service rpc-svcgssd.service
> > +
> > +[Service]
> > +ExecStart=/sbin/modprobe -q auth_rpcgss
>
> I think you need
> Type=oneshot
>
> else systemd won't wait for the modprobe to complete before running
> the other services.
>
> > +
> > +[Install]
> > +WantedBy=gssproxy.service rpc-svcgssd.service
>
> I don't think you want an install section. That means the service
> has to be explicitly enabled, which is a pain.
> I think nfs-server.service should Want= this.
> I also think
>
> ConditionPathExists=/etc/krb5.keytab
>
> would be appropriate.

If GSS-Proxy is in use the administrator may choose to use a keytab in
a different location, so I am not entirely sure we should depend
on /etc/krb5.keytab, however it is also ok to decide that if the admin
wants to use a different place that they create a custom unit file.
Up to you.

Simo.


--
Simo Sorce * Red Hat, Inc * New York

2014-09-22 19:53:53

by Steve Dickson

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



On 09/22/2014 03:46 PM, Simo Sorce wrote:
> On Mon, 22 Sep 2014 15:40:57 -0400
> "J. Bruce Fields" <[email protected]> wrote:
>
>> On Mon, Sep 22, 2014 at 03:20:07PM -0400, Steve Dickson 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.
>>
>> That should read "when the kernel supports gssproxy", not "when the
>> gssproxy daemon is already running."
>
> Actually the language is currently correct but it is another bug, the
> systemd/rpc-svcgssd.service file still includes
> "ConditionPathExists=|!/run/gssproxy.pid"
> This line should be removed in this patch.

I left that on purpose because isn't that ConditionPathExists
seeing if /run/gssproxy.pid exists and if it does
it means gssproxy is already running so rpc.svcgssd
should not start?

steved.


2014-09-22 23:58:12

by Steve Dickson

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



On 09/22/2014 06:34 PM, J. Bruce Fields 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 <[email protected]> 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 <[email protected]>
>>>>>> ---
>>>>>> 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.
No, why should there be? There is After= line referring to rpc.gssd
in nfs-server.service

grep After systemd/nfs-server.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=nfs-config.service

So when the server starts,rpc.gssd will start and rpc.svcgssd will start
if gssproxy is not enable and there is a key tab.

>
>> 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.
Right, which will load the sunrpc modules.

>
>> 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.
On the client side the mount -o sec=krb5? loads auth_rpcgss module.
Maybe the loading of nfsd loads the module? But I don't think that
module has to be loaded for any of the gss daemons (gssd, svcgssd or
gssproxy) to start successfully...


>
>>>> 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.
Yes, this was the conversation I was referring to.. Thank you
for digging it out..

>
> 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.
Lets talk with the systemd people to see what they say...

steved.

>
> 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.
>
> --b.
>

2014-09-23 16:01:01

by Simo Sorce

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

On Tue, 23 Sep 2014 11:20:00 -0400
"J. Bruce Fields" <[email protected]> wrote:

> On Tue, Sep 23, 2014 at 08:48:54AM -0400, Simo Sorce wrote:
> > On Tue, 23 Sep 2014 12:08:04 +1000
> > NeilBrown <[email protected]> wrote:
> > > I don't think you want an install section. That means the service
> > > has to be explicitly enabled, which is a pain.
> > > I think nfs-server.service should Want= this.
> > > I also think
> > >
> > > ConditionPathExists=/etc/krb5.keytab
> > >
> > > would be appropriate.
> >
> > If GSS-Proxy is in use the administrator may choose to use a keytab
> > in a different location, so I am not entirely sure we should depend
> > on /etc/krb5.keytab, however it is also ok to decide that if the
> > admin wants to use a different place that they create a custom unit
> > file. Up to you.
>
> Note we're already using the same line in rpc-gssd.service and
> rpc-svcgssd.service.
>
> Can you suggest a better "does this host have krb5 configured?" test?
>
> I think false positives are OK, but not false negatives.
>
> (So, if we run those daemons unnecessarily it may annoy some people,
> but if we fail to run them when they're needed then things really
> don't work.)

I would simply not test for presence of a keytab if it were my call.

If the admin decided to start nfs-secure I assume he already got the
proper key material, ie I am not so sure that double-checking the admin
in the unit files is right for gssproxy, because gssproxy has
directives that allow the admin to put the keytab elsewhere.

Simo.

--
Simo Sorce * Red Hat, Inc * New York

2014-09-23 16:12:17

by J. Bruce Fields

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

On Tue, Sep 23, 2014 at 12:00:54PM -0400, Simo Sorce wrote:
> On Tue, 23 Sep 2014 11:20:00 -0400
> "J. Bruce Fields" <[email protected]> wrote:
>
> > On Tue, Sep 23, 2014 at 08:48:54AM -0400, Simo Sorce wrote:
> > > On Tue, 23 Sep 2014 12:08:04 +1000
> > > NeilBrown <[email protected]> wrote:
> > > > I don't think you want an install section. That means the service
> > > > has to be explicitly enabled, which is a pain.
> > > > I think nfs-server.service should Want= this.
> > > > I also think
> > > >
> > > > ConditionPathExists=/etc/krb5.keytab
> > > >
> > > > would be appropriate.
> > >
> > > If GSS-Proxy is in use the administrator may choose to use a keytab
> > > in a different location, so I am not entirely sure we should depend
> > > on /etc/krb5.keytab, however it is also ok to decide that if the
> > > admin wants to use a different place that they create a custom unit
> > > file. Up to you.
> >
> > Note we're already using the same line in rpc-gssd.service and
> > rpc-svcgssd.service.
> >
> > Can you suggest a better "does this host have krb5 configured?" test?
> >
> > I think false positives are OK, but not false negatives.
> >
> > (So, if we run those daemons unnecessarily it may annoy some people,
> > but if we fail to run them when they're needed then things really
> > don't work.)
>
> I would simply not test for presence of a keytab if it were my call.
>
> If the admin decided to start nfs-secure I assume he already got the
> proper key material, ie I am not so sure that double-checking the admin
> in the unit files is right for gssproxy, because gssproxy has
> directives that allow the admin to put the keytab elsewhere.

I believe nfs-secure is being removed (there's none under
nfs-utils/systemd).

We'd rather not require unnecessary configuration steps. Configuring
NFS and krb5 should be enough.

So the point is to start the daemons automatically.

--b.

2014-09-22 19:40:57

by J. Bruce Fields

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

On Mon, Sep 22, 2014 at 03:20:07PM -0400, Steve Dickson 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.

That should read "when the kernel supports gssproxy", not "when the
gssproxy daemon is already running."

--b.

>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> 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 gssproxy.service rpc-svcgssd.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
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-09-23 19:23:14

by J. Bruce Fields

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

On Mon, Sep 22, 2014 at 10:11:10PM -0400, J. Bruce Fields wrote:
> On Tue, Sep 23, 2014 at 12:08:04PM +1000, NeilBrown wrote:
> > I think you need
> > Type=oneshot
> >
> > else systemd won't wait for the modprobe to complete before running the other
> > services.
...
> > I don't think you want an install section. That means the service has to be
> > explicitly enabled, which is a pain.
> > I think nfs-server.service should Want= this.

And nfs-client.target?

> > I also think
> >
> > ConditionPathExists=/etc/krb5.keytab
> >
> > would be appropriate.

Still untested:

--b.

commit 3fab32b4bf96
Author: J. Bruce Fields <[email protected]>
Date: Mon Sep 22 21:43:59 2014 -0400

systemd: manually insert auth_rpcgss module.

We need to insert the auth_rpcgss module before starting rpc.svcgssd or
gss-proxy, for two reasons:

- gss-proxy needs access to the /proc/net/rpc/use-gss-proxy file
to set up communication with knfsd.
- the unit files need to able to test for the existance of the
same path in order to decide whether the kernel supports
gss-proxy or not.

Currently we're using dependencies on proc-fs-nfsd.mount for this, but
that works only because of the nfsd kernel module references some
symbols in auth_rpcgss, which is an odd implementation detail we're
likely to fix some day.

Signed-off-by: J. Bruce Fields <[email protected]>

diff --git a/systemd/auth-rpcgss-module.service b/systemd/auth-rpcgss-module.service
new file mode 100644
index 000000000000..3fc2f4ac924f
--- /dev/null
+++ b/systemd/auth-rpcgss-module.service
@@ -0,0 +1,14 @@
+# We want to start gss-proxy on kernels that support it and rpc.svcgssd
+# on those that don't. Those services check for support by checking
+# for existence of the path /proc/net/rpc/use-gss-proxy. Before they
+# can perform that check, they need this module loaded. (Unless
+# rpcsec_gss support is built directly into the kernel, in which case this
+# unit will fail. But that's OK.)
+[Unit]
+Description=Kernel Module supporting RPCSEC_GSS
+Before=gssproxy.service rpc-svcgssd.service
+ConditionPathExists=/etc/krb5.keytab
+
+[Service]
+Type=oneshot
+ExecStart=/sbin/modprobe -q auth_rpcgss
diff --git a/systemd/nfs-client.target b/systemd/nfs-client.target
index f3c09e76a0f7..474f5e9ad74b 100644
--- a/systemd/nfs-client.target
+++ b/systemd/nfs-client.target
@@ -5,7 +5,7 @@ Wants=remote-fs-pre.target

# Note: we don't "Wants=rpc-statd.service" as "mount.nfs" will arrange to
# start that on demand if needed.
-Wants=rpc-gssd.service rpc-svcgssd.service
+Wants=rpc-gssd.service rpc-svcgssd.service auth-rpcgss-module.service
Wants=nfs-blkmap.service rpc-statd-notify.service
Before=rpc-gssd.service rpc-svcgssd.service nfs-blkmap.service

diff --git a/systemd/nfs-server.service b/systemd/nfs-server.service
index 2fa7387e1cb9..fd213a3995de 100644
--- a/systemd/nfs-server.service
+++ b/systemd/nfs-server.service
@@ -3,6 +3,7 @@ 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=auth-rpcgss-module.service
Wants=rpc-statd-notify.service

After= network.target proc-fs-nfsd.mount rpcbind.target nfs-mountd.service

2014-09-23 14:58:45

by Steve Dickson

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



On 09/23/2014 08:52 AM, Simo Sorce wrote:
> On Mon, 22 Sep 2014 21:19:18 -0400
> Steve Dickson <[email protected]> wrote:
>
>>
>>
>> On 09/22/2014 08:19 PM, Simo Sorce wrote:
>>> On Mon, 22 Sep 2014 18:57:10 -0400
>>> Steve Dickson <[email protected]> wrote:
>>>
>>>>
>>>>
>>>> On 09/22/2014 05:32 PM, Simo Sorce wrote:
>>>>> On Mon, 22 Sep 2014 17:14:05 -0400
>>>>> Steve Dickson <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>>>>>>>> ---
>>>>>>>>>> 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 " 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.
>>>>>
>>>>> This guarantees ordering (to some degree) between rpc.gssd and
>>>>> rpoc.svcgssd, but says nothing about gssproxy ...
>>>> The question was how is the auth_rpcgss module being loaded. Since
>>>> both rpc-svcgssd.service and rpc-gssd.service service have
>>>> a After=var-lib-nfs-rpc_pipefs.mount and gssproxy is requiring
>>>> them, that's how auth_rpcgss is being loaded.
>>>>
>>>> If you only in enable gssproxy (not nfs-server or nfs-client) the
>>>> module still get loaded via gssproxy,service file
>>>>
>>>>>> So when gssproxy.service does it's "Before=nfs-secure.service
>>>>>> nfs-secure-server.service" line everything is loaded before
>>>>>> gssproxy start...
>>>>>>
>>>>>> 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..
>>>>>
>>>>> Maybe we should add "Before: gssproxy.service rpc-svcgssd.service"
>>>>> to var-lib-nfs-rpc_pipefs.mount instead (and also drop any mention
>>>>> of nfs services in gssproxy unit file so you have complete control
>>>>> of the dependencies ?
>>>> No.
>>>> The loading of sunrpc and the mounting of the file system has
>>>> nothing to do with starting up the gssd daemons.
>>>>
>>>> I would suggest gssproxy does to two things:
>>>>
>>>> 1) Add a Requires: nfs-utils to the spec file since you are
>>>> requiring services from nfs-utils
>>>
>>> No we are not requiring services from nfs-utils, nfs-utils is one of
>>> our users, you got that reversed.
>> So having "Before=nfs-secure.service nfs-secure-server.service" in
>> gssproxy.service and not having nfs-utils installed will not cause the
>> service fail during start up?
>
> It will not, but right now gssproxy.service also has:
> Requires=proc-fs-nfsd.mount
>
> I want to drop this one.
You don't want to drop this because it loads the needed kernel modules.

>
>>>> 2) Add a After=var-lib-nfs-rpc_pipefs.mount to the
>>>> gssproxy.service file since gssproxy could careless about either
>>>> rpc.gssd or rpc.svcgssd daemons. All it is looking for is the
>>>> sunrpc and auth_rpcgss kernel modules.
>>>
>>> The correct thing is to add a Before: gssproxy.service to
>>> var-lib-nfs-rpc_pipefs.mount IMO.
>> Again what happen if gssproxy is not installed? Things will
>> still come up successfully?
>
> Yes, afaik After and Before are just ordering instruction and do not
> cause any failure if the units are not present at all, or fail to start.
Ok... thanks!

steved.
>
> There are Want and Require directives for strong relationships.
>
> Simo.
>

2014-09-23 01:19:22

by Steve Dickson

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



On 09/22/2014 08:19 PM, Simo Sorce wrote:
> On Mon, 22 Sep 2014 18:57:10 -0400
> Steve Dickson <[email protected]> wrote:
>
>>
>>
>> On 09/22/2014 05:32 PM, Simo Sorce wrote:
>>> On Mon, 22 Sep 2014 17:14:05 -0400
>>> Steve Dickson <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>>>>>> ---
>>>>>>>> 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
>>>>>>> " 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.
>>>
>>> This guarantees ordering (to some degree) between rpc.gssd and
>>> rpoc.svcgssd, but says nothing about gssproxy ...
>> The question was how is the auth_rpcgss module being loaded. Since
>> both rpc-svcgssd.service and rpc-gssd.service service have
>> a After=var-lib-nfs-rpc_pipefs.mount and gssproxy is requiring
>> them, that's how auth_rpcgss is being loaded.
>>
>> If you only in enable gssproxy (not nfs-server or nfs-client) the
>> module still get loaded via gssproxy,service file
>>
>>>> So when gssproxy.service does it's "Before=nfs-secure.service
>>>> nfs-secure-server.service" line everything is loaded before
>>>> gssproxy start...
>>>>
>>>> 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..
>>>
>>> Maybe we should add "Before: gssproxy.service rpc-svcgssd.service"
>>> to var-lib-nfs-rpc_pipefs.mount instead (and also drop any mention
>>> of nfs services in gssproxy unit file so you have complete control
>>> of the dependencies ?
>> No.
>> The loading of sunrpc and the mounting of the file system has nothing
>> to do with starting up the gssd daemons.
>>
>> I would suggest gssproxy does to two things:
>>
>> 1) Add a Requires: nfs-utils to the spec file since you are requiring
>> services from nfs-utils
>
> No we are not requiring services from nfs-utils, nfs-utils is one of
> our users, you got that reversed.
So having "Before=nfs-secure.service nfs-secure-server.service" in
gssproxy.service and not having nfs-utils installed will not cause the
service fail during start up?

>
>> 2) Add a After=var-lib-nfs-rpc_pipefs.mount to the gssproxy.service
>> file since gssproxy could careless about either rpc.gssd or
>> rpc.svcgssd daemons. All it is looking for is the sunrpc and
>> auth_rpcgss kernel modules.
>
> The correct thing is to add a Before: gssproxy.service to
> var-lib-nfs-rpc_pipefs.mount IMO.
Again what happen if gssproxy is not installed? Things will
still come up successfully?

steved

>
> Simo.
>

2014-09-23 16:58:04

by Simo Sorce

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

On Tue, 23 Sep 2014 12:12:14 -0400
"J. Bruce Fields" <[email protected]> wrote:

> On Tue, Sep 23, 2014 at 12:00:54PM -0400, Simo Sorce wrote:
> > On Tue, 23 Sep 2014 11:20:00 -0400
> > "J. Bruce Fields" <[email protected]> wrote:
> >
> > > On Tue, Sep 23, 2014 at 08:48:54AM -0400, Simo Sorce wrote:
> > > > On Tue, 23 Sep 2014 12:08:04 +1000
> > > > NeilBrown <[email protected]> wrote:
> > > > > I don't think you want an install section. That means the
> > > > > service has to be explicitly enabled, which is a pain.
> > > > > I think nfs-server.service should Want= this.
> > > > > I also think
> > > > >
> > > > > ConditionPathExists=/etc/krb5.keytab
> > > > >
> > > > > would be appropriate.
> > > >
> > > > If GSS-Proxy is in use the administrator may choose to use a
> > > > keytab in a different location, so I am not entirely sure we
> > > > should depend on /etc/krb5.keytab, however it is also ok to
> > > > decide that if the admin wants to use a different place that
> > > > they create a custom unit file. Up to you.
> > >
> > > Note we're already using the same line in rpc-gssd.service and
> > > rpc-svcgssd.service.
> > >
> > > Can you suggest a better "does this host have krb5 configured?"
> > > test?
> > >
> > > I think false positives are OK, but not false negatives.
> > >
> > > (So, if we run those daemons unnecessarily it may annoy some
> > > people, but if we fail to run them when they're needed then
> > > things really don't work.)
> >
> > I would simply not test for presence of a keytab if it were my call.
> >
> > If the admin decided to start nfs-secure I assume he already got the
> > proper key material, ie I am not so sure that double-checking the
> > admin in the unit files is right for gssproxy, because gssproxy has
> > directives that allow the admin to put the keytab elsewhere.
>
> I believe nfs-secure is being removed (there's none under
> nfs-utils/systemd).
>
> We'd rather not require unnecessary configuration steps. Configuring
> NFS and krb5 should be enough.
>
> So the point is to start the daemons automatically.

I see, I can live with that for now.

Simo.


--
Simo Sorce * Red Hat, Inc * New York

2014-09-22 22:34:24

by J. Bruce Fields

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

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 <[email protected]> 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 <[email protected]>
> >>>> ---
> >>>> 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.

--b.

2014-09-23 15:16:22

by J. Bruce Fields

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

On Tue, Sep 23, 2014 at 11:06:45AM -0400, Steve Dickson wrote:
>
>
> On 09/22/2014 09:55 PM, J. Bruce Fields wrote:
> > On Mon, Sep 22, 2014 at 08:26:55PM -0400, Simo Sorce wrote:
> >> No, starting before those service in itself achieves nothing.\
> >> I think what may cause the module to load maybe the fact
> >> gssproxy.service includes:
> >> Requires=proc-fs-nfsd.mount
> >
> > I'd expect that to only load the nfsd module.
> >
> > Hm, I think nfsd actually has a dependency on auth_rpcgss. I wonder if
> > that's correct. Maybe that's what's doing it.
> It is... In another thread I clearly showed that....

The dependency of nfsd on auth_rpcgss looks to me like an implementation
detail. The client module doesn't have the same dependency. We
definitely shouldn't depend on it, as the dependency could get removed
some day.

> So I really don't think another unit file is necessary...

I haven't seen an alternative yet.

--b.

>
> steved.
>
> >
> >> But to be honest this was a hack to deal with broken nfs service files,
> >> gss-proxy should not require nfsd, the dependency should be the other
> >> way around, as gss-proxy can run on machines where there is no nfs
> >> service whatsoever, as it stand this is a bug in gssproxy.service and
> >> I'd like to fix it.
> >
> > So, something like this? (Untested, no idea if I'm doing this right.)
> >
> > --b.
> >
> > diff --git a/systemd/auth-rpcgss-module.service b/systemd/auth-rpcgss-module.service
> > new file mode 100644
> > index 000000000000..252545b458fd
> > --- /dev/null
> > +++ b/systemd/auth-rpcgss-module.service
> > @@ -0,0 +1,15 @@
> > +# We want to start gss-proxy on kernels that support it and rpc.svcgssd
> > +# on those that don't. Those services check for support by checking
> > +# for existence of the path /proc/net/rpc/use-gss-proxy. Before they
> > +# can perform that check, they need this module loaded. (Unless
> > +# rpcsec_gss is built directly into the kernel, in which case this unit
> > +# will fail. But that's OK.)
> > +[Unit]
> > +Description=Kernel Module supporting RPCSEC_GSS
> > +Before=gssproxy.service rpc-svcgssd.service
> > +
> > +[Service]
> > +ExecStart=/sbin/modprobe -q auth_rpcgss
> > +
> > +[Install]
> > +WantedBy=gssproxy.service rpc-svcgssd.service
> >

2014-09-22 20:02:35

by Steve Dickson

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



On 09/22/2014 04:00 PM, Simo Sorce wrote:
> On Mon, 22 Sep 2014 15:53:46 -0400
> Steve Dickson <[email protected]> wrote:
>
>>
>>
>> On 09/22/2014 03:46 PM, Simo Sorce wrote:
>>> On Mon, 22 Sep 2014 15:40:57 -0400
>>> "J. Bruce Fields" <[email protected]> wrote:
>>>
>>>> On Mon, Sep 22, 2014 at 03:20:07PM -0400, Steve Dickson 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.
>>>>
>>>> That should read "when the kernel supports gssproxy", not "when the
>>>> gssproxy daemon is already running."
>>>
>>> Actually the language is currently correct but it is another bug,
>>> the systemd/rpc-svcgssd.service file still includes
>>> "ConditionPathExists=|!/run/gssproxy.pid"
>>> This line should be removed in this patch.
>>
>> I left that on purpose because isn't that ConditionPathExists
>> seeing if /run/gssproxy.pid exists and if it does
>> it means gssproxy is already running so rpc.svcgssd
>> should not start?
>
> No.
> First of all the fact gss-proxy is running does not mean it is
> serving nfsd necessarily, it may be running on an older kernel where it
> servers apache or some other process (remember gssproxy is not just
> for nfsd).
> Second you already have
> "ConditionPathExists=|!/proc/net/rpc/use-gss-proxy" which is the
> correct trigger to decide which of the two to use.
Fair enough... that makes sense...

steved.

>
> Simo.
>