On Mon, 2017-05-15 at 17:25 -0600, Jason Gunthorpe wrote:
> On Mon, May 15, 2017 at 03:47:33PM -0700, Bart Van Assche wrote:
> > srp_daemon.service is modified such that instead of starting
> > /usr/sbin/srp_daemon.sh that it does not start any process. A new
> > template service is added, namely srp_daemon_port@.service. This
> > service replaces srp_daemon.sh and controls /usr/sbin/srp_daemon
> > for a single port. A udev rule is added that instantiates the
> > srp_daemon_port@.service every time an RDMA port is hot-added.
> > Since the per-port service depends on the srp_daemon service,
> > starting or stopping the srp_daemon service affects all per-port
> > services.
>
> This looks pretty good to me ... You are happy with how the user experience works?
Yes, except for one aspect, namely that starting srp_daemon does not start
the per-port services. I have addressed that as follows in srp_daemon.service:
ExecStart=.../srp_daemon_start_all.sh
where srp_daemon_start_all.sh is the following script:
#!/bin/sh
for p in /sys/class/infiniband/*/ports/*; do
[ -e "$p" ] || continue
p=${p#/sys/class/infiniband/}
nohup /usr/bin/systemctl start "srp_daemon_port@${p/\/ports\//:}" </dev/null >&/dev/null &
done
> A few minor thoughts
>
> > +++ b/srp_daemon/90-srp-daemon.rules
> > @@ -0,0 +1 @@
> > +ACTION=="add", SUBSYSTEM=="infiniband_mad", PROGRAM:="/usr/bin/systemctl show srp_daemon -p UnitFileState", RESULT=="UnitFileState=enabled", TAG+="systemd", ENV{SYSTEMD_WANTS}+="srp_daemon_port@$attr{ibdev}:$attr{port}.service"
>
> I think we should have a main rdma udev rules file that always creates
> .device targets inside systemd for umad, unconditionally. This should
> be useful for other things like opensm/etc.
>
> So we'd have a line like:
>
> ACTION=="add", SUBSYSTEM=="infiniband_mad", TAG+="systemd", ENV{SYSTEMD_ALIAS}="/dev/infiniband/umad/$attr{ibdev}:$attr{port}"
>
> So we get (unescaped)
>
> /dev/infiniband/umad0.device
> /dev/infiniband/umad/mlx4_0:0.device
>
> Then the srp-daemon.rule would just be the wants:
>
> ACTION=="add", SUBSYSTEM=="infiniband_mad", PROGRAM:="/usr/bin/systemctl show srp_daemon -p UnitFileState", RESULT=="UnitFileState=enabled", ENV{SYSTEMD_WANTS}+="srp_daemon_port@$attr{ibdev}:$attr{port}.service"
>
> > index 00000000..666400ab
> > +++ b/srp_daemon/srp_daemon_port@.service
> > @@ -0,0 +1,18 @@
> > +[Unit]
> > +Description=SRP daemon that monitors port %i
> > +Documentation=man:srp_daemon file:/etc/rdma/rdma.conf file:/etc/srp_daemon.conf
> > +DefaultDependencies=false
> > +Conflicts=emergency.target emergency.service
> > +Requires=rdma.service
> > +Wants=opensm.service
> > +After=rdma.service opensm.service srp_daemon.service
> > +After=network.target
> > +Before=remote-fs-pre.target
> > +BindsTo=srp_daemon.service
>
> Using the new .device, we can add a requires:
>
> Requires: -dev-infiniband-umad0-%i.device
>
> Now, systemd will never start the port service until udev says that
> the umad is available for that port. It will actually wait until udev
> says that umad is present if something requires it.
All of this has been addressed in the following updated pull request:
https://github.com/linux-rdma/rdma-core/pull/135
> I also wonder if the systemd activation protocol should be used by
> srp_daemon, this should defer mount until srp_daemon has done the
> right stuff, but that might be no good if the 'right stuff' requires a
> link up?
>
> However, if the admin is using the above .mount unit Requires then
> they would want to wait until the link is up and the SM has been
> contacted before attempting the first mount? systemd obnoxiously does
> not have mount retries so everything should be perfect before mount is
> done.
I think that systemd mount units should have a dependency on /dev/disk/by-id
or /dev/disk/by-uuid instead of srp_daemon. Even after an srp_daemon_port@
service has been started it can take some time
before login occurs. Using a
dependency on /dev/disk/by-*id avoids that the mount service gets started
before the path it needs is available.
> Does
> systemctl mask srp_daemon_port@mlx4_0:1
>
> Work to inhibit the daemon on a specific port? If yes that should be
> mentioned in here I think. Otherwise, we probably still need a way to
> do that.
Information about masking has been added to the srp_daemon_port@ man page.
> The final bits would be to see if any security sandboxing options are
> appropriate for the .unit file, since srp_daemon is network
> facing. Drop caps, ro filesystems, etc. It should also dump root.
Sorry but since srp_daemon writes into /sys/class/infiniband_srp/*/add_target
I don't think it's possible to drop root privileges for the srp_daemon_port@
service.
Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
