Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

srp_daemon: Rework systemd integration #135

Merged
merged 7 commits into from May 28, 2017
Merged

Conversation

bvanassche
Copy link
Contributor

Split srp_daemon.service into srp_daemon.service and the srp_daemon_port@.service template. The former does not start any srp_daemon process but controls the latter. srp_daemon_port@.service instances are started automatically upon hot-add of an RDMA port to support hot-plugging.

Bart Van Assche added 4 commits May 24, 2017 09:57
Surround expansions of shell variables that can contain spaces with
double quotes. Use arrays instead of space-separated strings to
store lists to avoid that strings that contain whitespace get split
when iterating over a list.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
From https://liw.fi/manpages/, about the SYNOPSIS section:

Font usage is important here, and carries information. All the
parts that are in bold are things that the user is expected to
write verbatim. Italic indicates values the user is expected to
fill in. Normal font is used for syntax meta-characters: for
the brackets that indicate optionality, and the ellipsis that
indicates repetition.

Additionally, change the identifiers that refer to arguments to
lower case.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
This command-line option is used in a later patch to avoid having
to start a shell script from a udev rule.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
… alias

Add a udev rule that creates the /dev/infiniband/umad/${ibdev}:${port}.device
alias. This alias is useful for opensm and srp_daemon.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
@@ -1 +1,2 @@
usr/lib/*/libibumad*.so.*
lib/udev/rules.d/libibumad.rules
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bdrung @jarodwilson is having a udev .rule in a library package OK for packaging, multi-arch/etc ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It's fine. For multi-arch, you just have to ensure that all these files that share the same path have the same content (which is true in this case).

@bdrung
Copy link
Contributor

bdrung commented May 26, 2017

Please remove the file extension from the srp_daemon_start_all.sh script. The Debian policy says in https://www.debian.org/doc/debian-policy/ch-files.html#s-scripts:

When scripts are installed into a directory in the system PATH, the script name should not include an extension such as .sh or .pl that denotes the scripting language currently used to implement it.

@jgunthorpe
Copy link
Member

That script should probably be installed into /usr/lib or libexec since it is not intended to be called by the end user?

@bvanassche
Copy link
Contributor Author

The .sh suffix has been removed from srp_daemon_start_all.sh and that script has been moved to the libexec directory. Additionally, privileges that are neither used by the srp_daemon nor by the srp_daemon_port@ service have been dropped.

ProtectKernelModules=yes
ProtectSystem=full
RestrictRealtime=yes
SystemCallFilter=~@clock @cpu-emulation @debug @keyring @module @mount @obsolete @raw-io

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably not necessary since start_on_all_ports is not network facing. I would worry that someday the calls to systemctl will break .. Other stuff looks great

usr/sbin/ibsrpdm
usr/sbin/srp_daemon
libexec/srp_daemon/start_on_all_ports
usr/share/doc/rdma-core/ibsrpdm.md usr/share/doc/srptools/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The libexec dir is usr/lib/ on Debian - looks like this is why travis is failing?

Bart Van Assche added 2 commits May 26, 2017 11:09
A new template service is added, namely srp_daemon_port@.service.
This service controls /usr/sbin/srp_daemon for a single port and
replaces /usr/sbin/srp_daemon.sh. The srp_daemon.service is
modified such that it starts srp_daemon_port@.service instances
instead of /usr/sbin/srp_daemon.sh. 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.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Add a udev rule that instantiates the srp_daemon_port@.service
every time an RDMA port is hot-added.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
…@ services

Drop those privileges that are neither used by the srp_daemon nor
by the srp_daemon_port@ service.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
@bvanassche
Copy link
Contributor Author

I removed PrivateNetwork, ProtectControlGroups, ProtectSystem and SystemCallFilter from srp_daemon.service. The start_on_all_ports path had already been fixed.

@dledford dledford merged commit 139cac2 into linux-rdma:master May 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants