[PATCH] Add rdma service for kernel boot support

[PATCH] Add rdma service for kernel boot support

[Benjamin Drung]

Currently upstream does not provide a rdma or openibd service. Only the
RedHat package ships a rdma service and rdma modules configuration
files. The ibacm and srp_daemon services depend on the openibd service.

Make RedHat's rdma service available to all distros by cherry-picking
the basic files for the rdma service to run and trim them down to the
minimum. Do not pick workarounds or other quirk that might not needed
any more. Then replace the openibd service dependency by the rdma
service.

Signed-off-by: Benjamin Drung <benjamin.drung-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
---
 CMakeLists.txt                                     |   1 +
 debian/rdma-core.install                           |   4 +
 ibacm/CMakeLists.txt                               |   1 -
 ibacm/ibacm.init.in                                |   4 +-
 kernel-boot/CMakeLists.txt                         |  22 ++++
 kernel-boot/rdma-init-kernel.in                    | 143 +++++++++++++++++++++
 kernel-boot/rdma.conf                              |  23 ++++
 redhat/rdma.service => kernel-boot/rdma.service.in |   6 +-
 {redhat => kernel-boot}/rdma.udev-rules            |   6 +-
 redhat/rdma-core.spec                              |   2 -
 redhat/rdma.conf                                   |  25 ----
 srp_daemon/CMakeLists.txt                          |   2 -
 srp_daemon/srpd.in                                 |   4 +-
 13 files changed, 203 insertions(+), 40 deletions(-)
 create mode 100644 kernel-boot/CMakeLists.txt
 create mode 100644 kernel-boot/rdma-init-kernel.in
 create mode 100644 kernel-boot/rdma.conf
 rename redhat/rdma.service => kernel-boot/rdma.service.in (59%)
 rename {redhat => kernel-boot}/rdma.udev-rules (100%)
 delete mode 100644 redhat/rdma.conf

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 16196205..a03d8da3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -398,6 +398,7 @@ configure_file("${BUILDLIB}/config.h.in" "${BUILD_INCLUDE}/config.h" ESCAPE_QUOT
 add_subdirectory(ccan)
 add_subdirectory(util)
 add_subdirectory(Documentation)
+add_subdirectory(kernel-boot)
 # Libraries
 add_subdirectory(libibumad)
 add_subdirectory(libibumad/man)
diff --git a/debian/rdma-core.install b/debian/rdma-core.install
index 371aaadc..d303a780 100644
--- a/debian/rdma-core.install
+++ b/debian/rdma-core.install
@@ -1,5 +1,9 @@
+etc/rdma/rdma.conf
 lib/systemd/system/rdma-ndd.service
+lib/systemd/system/rdma.service
+lib/udev/rules.d/98-rdma.rules
 lib/udev/rules.d/rdma-ndd.rules
+usr/lib/rdma-init-kernel
 usr/sbin/rdma-ndd
 usr/share/doc/rdma-core/MAINTAINERS
 usr/share/doc/rdma-core/README.md
diff --git a/ibacm/CMakeLists.txt b/ibacm/CMakeLists.txt
index 7eba294b..448d2e14 100644
--- a/ibacm/CMakeLists.txt
+++ b/ibacm/CMakeLists.txt
@@ -58,7 +58,6 @@ rdma_man_pages(
   )
 
 # FIXME: update the .init.in
-set(rdmascript "openibd")
 set(prefix "${CMAKE_INSTALL_PREFIX}")
 rdma_subst_install(FILES "ibacm.init.in"
   DESTINATION "${CMAKE_INSTALL_INITDDIR}"
diff --git a/ibacm/ibacm.init.in b/ibacm/ibacm.init.in
index bd7b70a1..73a333b2 100644
--- a/ibacm/ibacm.init.in
+++ b/ibacm/ibacm.init.in
@@ -10,8 +10,8 @@
 # Provides:       ibacm
 # Default-Start: 2 3 4 5
 # Default-Stop: 0 1 6
-# Required-Start: @rdmascript@ $network $remote_fs
-# Required-Stop: @rdmascript@ $network $remote_fs
+# Required-Start: rdma $network $remote_fs
+# Required-Stop: rdma $network $remote_fs
 # Should-Start:
 # Should-Stop:
 # Short-Description: Starts and stops the InfiniBand ACM service
diff --git a/kernel-boot/CMakeLists.txt b/kernel-boot/CMakeLists.txt
new file mode 100644
index 00000000..f674d2a6
--- /dev/null
+++ b/kernel-boot/CMakeLists.txt
@@ -0,0 +1,22 @@
+# Copyright (c) 2017 Benjamin Drung <benjamin.drung-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
+# Licensed under BSD (MIT variant) or GPLv2. See COPYING.
+
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+
+install(FILES rdma.conf DESTINATION "${CMAKE_INSTALL_SYSCONFDIR}/rdma/"
+  PERMISSIONS OWNER_WRITE OWNER_READ GROUP_READ WORLD_READ)
+
+rdma_subst_install(FILES "rdma-init-kernel.in"
+  DESTINATION "${CMAKE_INSTALL_FULL_LIBEXECDIR}"
+  RENAME "rdma-init-kernel"
+  PERMISSIONS OWNER_WRITE OWNER_READ GROUP_READ WORLD_READ OWNER_EXECUTE GROUP_EXECUTE WORLD_EXECUTE)
+
+install(FILES "rdma.udev-rules"
+  DESTINATION "${CMAKE_INSTALL_UDEV_RULESDIR}"
+  RENAME "98-rdma.rules"
+  PERMISSIONS OWNER_WRITE OWNER_READ GROUP_READ WORLD_READ)
+
+rdma_subst_install(FILES "rdma.service.in"
+  DESTINATION "${CMAKE_INSTALL_SYSTEMD_SERVICEDIR}"
+  RENAME "rdma.service"
+  PERMISSIONS OWNER_WRITE OWNER_READ GROUP_READ WORLD_READ)
diff --git a/kernel-boot/rdma-init-kernel.in b/kernel-boot/rdma-init-kernel.in
new file mode 100644
index 00000000..9d1da12b
--- /dev/null
+++ b/kernel-boot/rdma-init-kernel.in
@@ -0,0 +1,143 @@
+#!/bin/bash
+#
+# Bring up the kernel RDMA stack
+#
+# This is usually run automatically by systemd after a hardware activation
+# event in udev has triggered a start of the rdma.service unit
+#
+
+set -eu
+shopt -s nullglob
+
+CONFIG=@CMAKE_INSTALL_SYSCONFDIR@/rdma/rdma.conf
+
+LOAD_ULP_MODULES=""
+LOAD_CORE_USER_MODULES="ib_umad ib_uverbs ib_ucm rdma_ucm"
+LOAD_CORE_CM_MODULES="iw_cm ib_cm rdma_cm"
+LOAD_CORE_MODULES="ib_core"
+
+if [ -f $CONFIG ]; then
+    . $CONFIG
+
+    if [ "${RDS_LOAD-}" = "yes" ]; then
+        IPOIB_LOAD=yes
+    fi
+
+    if [ "${IPOIB_LOAD-}" = "yes" ]; then
+        LOAD_ULP_MODULES="$LOAD_ULP_MODULES ib_ipoib"
+    fi
+
+    if [ "${RDS_LOAD-}" = "yes" ] && [ -f "/lib/modules/$(uname -r)/kernel/net/rds/rds.ko" ]; then
+        LOAD_ULP_MODULES="$LOAD_ULP_MODULES rds"
+        if [ -f "/lib/modules/$(uname -r)/kernel/net/rds/rds_tcp.ko" ]; then
+            LOAD_ULP_MODULES="$LOAD_ULP_MODULES rds_tcp"
+        fi
+        if [ -f "/lib/modules/$(uname -r)/kernel/net/rds/rds_rdma.ko" ]; then
+            LOAD_ULP_MODULES="$LOAD_ULP_MODULES rds_rdma"
+        fi
+    fi
+
+    if [ "${SRP_LOAD-}" = "yes" ]; then
+        LOAD_ULP_MODULES="$LOAD_ULP_MODULES ib_srp"
+    fi
+
+    if [ "${SRPT_LOAD-}" = "yes" ]; then
+        LOAD_ULP_MODULES="$LOAD_ULP_MODULES ib_srpt"
+    fi
+
+    if [ "${ISER_LOAD-}" = "yes" ]; then
+        LOAD_ULP_MODULES="$LOAD_ULP_MODULES ib_iser"
+    fi
+
+    if [ "${ISERT_LOAD-}" = "yes" ]; then
+        LOAD_ULP_MODULES="$LOAD_ULP_MODULES ib_isert"
+    fi
+
+    if [ "${XPRTRDMA_LOAD-}" = "yes" ]; then
+        LOAD_ULP_MODULES="$LOAD_ULP_MODULES xprtrdma"
+    fi
+
+    if [ "${SVCRDMA_LOAD-}" = "yes" ]; then
+        LOAD_ULP_MODULES="$LOAD_ULP_MODULES svcrdma"
+    fi
+else
+    LOAD_ULP_MODULES="ib_ipoib"
+fi
+
+# If module $1 is loaded return - 0 else - 1
+is_loaded()
+{
+    /sbin/lsmod | grep -w "$1" > /dev/null 2>&1
+}
+
+load_modules()
+{
+    local RC=0
+
+    for module in "$@"; do
+        if ! /sbin/modinfo "$module" > /dev/null 2>&1; then
+            # do not attempt to load modules which do not exist
+            continue
+        fi
+        if ! is_loaded "$module"; then
+            if ! /sbin/modprobe "$module"; then
+                RC=$(( RC + 1 ))
+                echo
+                echo "Failed to load module $module"
+            fi
+        fi
+    done
+    return $RC
+}
+
+load_hardware_modules()
+{
+    local -i RC=0
+
+    # We match both class NETWORK and class INFINIBAND devices since our
+    # iWARP hardware is listed under class NETWORK.  The side effect of
+    # this is that we might cause a non-iWARP network driver to be loaded.
+    udevadm trigger --subsystem-match=pci --attr-nomatch=driver --attr-match=class=0x020000 --attr-match=class=0x0c0600
+    udevadm settle
+    if [ -r /proc/device-tree ]; then
+        if ls /proc/device-tree/*lhca* >/dev/null 2>&1; then
+            if ! is_loaded ib_ehca; then
+                load_modules ib_ehca || RC=$(( RC + $? ))
+            fi
+        fi
+    fi
+    if is_loaded mlx4_core -a ! is_loaded mlx4_ib; then
+        load_modules mlx4_ib || RC=$(( RC + $? ))
+    fi
+    if is_loaded mlx4_core -a ! is_loaded mlx4_en; then
+        load_modules mlx4_en || RC=$(( RC + $? ))
+    fi
+    if is_loaded mlx5_core -a ! is_loaded mlx5_ib; then
+        load_modules mlx5_ib || RC=$(( RC + $? ))
+    fi
+    if is_loaded cxgb3 -a ! is_loaded iw_cxgb3; then
+        load_modules iw_cxgb3 || RC=$(( RC + $? ))
+    fi
+    if is_loaded cxgb4 -a ! is_loaded iw_cxgb4; then
+        load_modules iw_cxgb4 || RC=$(( RC + $? ))
+    fi
+    if is_loaded be2net -a ! is_loaded ocrdma; then
+        load_modules ocrdma || RC=$(( RC + $? ))
+    fi
+    if is_loaded enic -a ! is_loaded usnic_verbs; then
+        load_modules usnic_verbs || RC=$(( RC + $? ))
+    fi
+    if is_loaded i40e -a ! is_loaded i40iw; then
+        load_modules i40iw || RC=$(( RC + $? ))
+    fi
+    return $RC
+}
+
+RC=0
+load_hardware_modules || RC=$(( RC + $? ))
+load_modules $LOAD_CORE_MODULES || RC=$(( RC + $? ))
+load_modules $LOAD_CORE_CM_MODULES || RC=$(( RC + $? ))
+load_modules $LOAD_CORE_USER_MODULES || RC=$(( RC + $? ))
+load_modules $LOAD_ULP_MODULES || RC=$(( RC + $? ))
+
+exit $RC
diff --git a/kernel-boot/rdma.conf b/kernel-boot/rdma.conf
new file mode 100644
index 00000000..ad4dc737
--- /dev/null
+++ b/kernel-boot/rdma.conf
@@ -0,0 +1,23 @@
+# Load IP over InfiniBand (IPoIB) network driver module
+IPOIB_LOAD=yes
+
+# Load SCSI Remote Protocol (SRP) initiator support module
+SRP_LOAD=yes
+
+# Load SCSI Remote Protocol target (SRPT) support module
+SRPT_LOAD=yes
+
+# Load iSCSI Extensions for RDMA (iSER) initiator support module
+ISER_LOAD=yes
+
+# Load iSCSI Extensions for RDMA target (iSERT) support module
+ISERT_LOAD=yes
+
+# Load Reliable Datagram Service (RDS) network protocol
+RDS_LOAD=no
+
+# Load NFS over RDMA (NFSoRDMA) client transport module
+XPRTRDMA_LOAD=yes
+
+# Load NFS over RDMA (NFSoRDMA) server transport module
+SVCRDMA_LOAD=no
diff --git a/redhat/rdma.service b/kernel-boot/rdma.service.in
similarity index 59%
rename from redhat/rdma.service
rename to kernel-boot/rdma.service.in
index 514ef58b..14e2f8f4 100644
--- a/redhat/rdma.service
+++ b/kernel-boot/rdma.service.in
@@ -1,15 +1,15 @@
 [Unit]
 Description=Initialize the iWARP/InfiniBand/RDMA stack in the kernel
-Documentation=file:/etc/rdma/rdma.conf
+Documentation=file:@CMAKE_INSTALL_SYSCONFDIR@/rdma/rdma.conf
 RefuseManualStop=true
 DefaultDependencies=false
 Conflicts=emergency.target emergency.service
-Before=network.target remote-fs-pre.target
+Before=network-pre.target remote-fs-pre.target
 
 [Service]
 Type=oneshot
 RemainAfterExit=yes
-ExecStart=/usr/libexec/rdma-init-kernel
+ExecStart=@CMAKE_INSTALL_FULL_LIBEXECDIR@/rdma-init-kernel
 
 [Install]
 WantedBy=sysinit.target
diff --git a/redhat/rdma.udev-rules b/kernel-boot/rdma.udev-rules
similarity index 100%
rename from redhat/rdma.udev-rules
rename to kernel-boot/rdma.udev-rules
index 7f44fe30..31d45b41 100644
--- a/redhat/rdma.udev-rules
+++ b/kernel-boot/rdma.udev-rules
@@ -4,9 +4,9 @@
 # enable the IB stack, so do so unilaterally) and on load of any of that
 # hardware, we trigger the rdma.service load in systemd
 
+SUBSYSTEM=="module", KERNEL=="be2net", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
 SUBSYSTEM=="module", KERNEL=="cxgb*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
+SUBSYSTEM=="module", KERNEL=="enic", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
 SUBSYSTEM=="module", KERNEL=="ib_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
-SUBSYSTEM=="module", KERNEL=="mlx*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
 SUBSYSTEM=="module", KERNEL=="iw_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
-SUBSYSTEM=="module", KERNEL=="be2net", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
-SUBSYSTEM=="module", KERNEL=="enic", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
+SUBSYSTEM=="module", KERNEL=="mlx*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
diff --git a/redhat/rdma-core.spec b/redhat/rdma-core.spec
index 5cebc251..94c5f95b 100644
--- a/redhat/rdma-core.spec
+++ b/redhat/rdma-core.spec
@@ -250,7 +250,6 @@ mkdir -p %{buildroot}%{_libexecdir}
 mkdir -p %{buildroot}%{_udevrulesdir}
 mkdir -p %{buildroot}%{dracutlibdir}/modules.d/05rdma
 mkdir -p %{buildroot}%{sysmodprobedir}
-install -D -m0644 redhat/rdma.conf %{buildroot}/%{_sysconfdir}/rdma/rdma.conf
 install -D -m0644 redhat/rdma.sriov-vfs %{buildroot}/%{_sysconfdir}/rdma/sriov-vfs
 install -D -m0644 redhat/rdma.mlx4.conf %{buildroot}/%{_sysconfdir}/rdma/mlx4.conf
 install -D -m0755 redhat/rdma.ifup-ib %{buildroot}/%{_sysconfdir}/sysconfig/network-scripts/ifup-ib
@@ -259,7 +258,6 @@ install -D -m0644 redhat/rdma.service %{buildroot}%{_unitdir}/rdma.service
 install -D -m0644 redhat/rdma.udev-ipoib-naming.rules %{buildroot}%{_sysconfdir}/udev/rules.d/70-persistent-ipoib.rules
 install -D -m0644 redhat/rdma.mlx4.user.modprobe %{buildroot}%{_sysconfdir}/modprobe.d/mlx4.conf
 install -D -m0755 redhat/rdma.modules-setup.sh %{buildroot}%{dracutlibdir}/modules.d/05rdma/module-setup.sh
-install -D -m0644 redhat/rdma.udev-rules %{buildroot}%{_udevrulesdir}/98-rdma.rules
 install -D -m0644 redhat/rdma.mlx4.sys.modprobe %{buildroot}%{sysmodprobedir}/libmlx4.conf
 install -D -m0644 redhat/rdma.cxgb3.sys.modprobe %{buildroot}%{sysmodprobedir}/cxgb3.conf
 install -D -m0644 redhat/rdma.cxgb4.sys.modprobe %{buildroot}%{sysmodprobedir}/cxgb4.conf
diff --git a/redhat/rdma.conf b/redhat/rdma.conf
deleted file mode 100644
index 94465649..00000000
--- a/redhat/rdma.conf
+++ /dev/null
@@ -1,25 +0,0 @@
-# Load IPoIB
-IPOIB_LOAD=yes
-# Load SRP (SCSI Remote Protocol initiator support) module
-SRP_LOAD=yes
-# Load SRPT (SCSI Remote Protocol target support) module
-SRPT_LOAD=yes
-# Load iSER (iSCSI over RDMA initiator support) module
-ISER_LOAD=yes
-# Load iSERT (iSCSI over RDMA target support) module
-ISERT_LOAD=yes
-# Load RDS (Reliable Datagram Service) network protocol
-RDS_LOAD=no
-# Load NFSoRDMA client transport module
-XPRTRDMA_LOAD=yes
-# Load NFSoRDMA server transport module
-SVCRDMA_LOAD=no
-# Load Tech Preview device driver modules
-TECH_PREVIEW_LOAD=no
-# Should we modify the system mtrr registers?  We may need to do this if you
-# get messages from the ib_ipath driver saying that it couldn't enable
-# write combining for the PIO buffs on the card.
-#
-# Note: recent kernels should do this for us, but in case they don't, we'll
-# leave this option
-FIXUP_MTRR_REGS=no
diff --git a/srp_daemon/CMakeLists.txt b/srp_daemon/CMakeLists.txt
index 62e91bb0..ee569ac1 100644
--- a/srp_daemon/CMakeLists.txt
+++ b/srp_daemon/CMakeLists.txt
@@ -45,8 +45,6 @@ install(FILES srp_daemon.conf DESTINATION "${CMAKE_INSTALL_SYSCONFDIR}")
 
 install(FILES srp_daemon.rules DESTINATION "${CMAKE_INSTALL_UDEV_RULESDIR}")
 
-# FIXME: The ib init.d file should really be included in rdma-core as well.
-set(RDMA_SERVICE "openibd" CACHE STRING "init.d file service name to order srpd after")
 # NOTE: These defaults are for CentOS, packagers should override.
 set(SRP_DEFAULT_START "2 3 4 5" CACHE STRING "Default-Start service data for srpd")
 set(SRP_DEFAULT_STOP "0 1 6" CACHE STRING "Default-Stop service data for srpd")
diff --git a/srp_daemon/srpd.in b/srp_daemon/srpd.in
index 7e2316f6..4aa68483 100755
--- a/srp_daemon/srpd.in
+++ b/srp_daemon/srpd.in
@@ -9,8 +9,8 @@
 #
 ### BEGIN INIT INFO
 # Provides:       srpd
-# Required-Start: $syslog @RDMA_SERVICE@
-# Required-Stop: $syslog @RDMA_SERVICE@
+# Required-Start: $syslog rdma
+# Required-Stop: $syslog rdma
 # Default-Start: @SRP_DEFAULT_START@
 # Default-Stop: @SRP_DEFAULT_STOP@
 # Should-Start:
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Bart Van Assche]

On Thu, 2017-07-13 at 19:20 +0200, Benjamin Drung wrote:
> Currently upstream does not provide a rdma or openibd service. Only the
> RedHat package ships a rdma service and rdma modules configuration
> files. The ibacm and srp_daemon services depend on the openibd service.
> 
> Make RedHat's rdma service available to all distros by cherry-picking
> the basic files for the rdma service to run and trim them down to the
> minimum. Do not pick workarounds or other quirk that might not needed
> any more. Then replace the openibd service dependency by the rdma
> service.

Hello Benjamin,

Shouldn't the rdma / openibd service be split into multiple services -
one for IPoIB, one for the SRP initiator driver, one for the SRP target
driver, one for the iSER initiator driver, one for the iSER target driver,
one for RDS and one for NFSoRDMA? I think that would allow us to get rid
of the rdma.conf configuration file and also that this would allow systemd
to load those services concurrently that do not depend on each other.

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 05:48:22PM +0000, Bart Van Assche wrote:
> On Thu, 2017-07-13 at 19:20 +0200, Benjamin Drung wrote:
> > Currently upstream does not provide a rdma or openibd service. Only the
> > RedHat package ships a rdma service and rdma modules configuration
> > files. The ibacm and srp_daemon services depend on the openibd service.
> > 
> > Make RedHat's rdma service available to all distros by cherry-picking
> > the basic files for the rdma service to run and trim them down to the
> > minimum. Do not pick workarounds or other quirk that might not needed
> > any more. Then replace the openibd service dependency by the rdma
> > service.
> 
> Hello Benjamin,
> 
> Shouldn't the rdma / openibd service be split into multiple services -
> one for IPoIB, one for the SRP initiator driver, one for the SRP target
> driver, one for the iSER initiator driver, one for the iSER target driver,
> one for RDS and one for NFSoRDMA? I think that would allow us to get rid
> of the rdma.conf configuration file and also that this would allow systemd
> to load those services concurrently that do not depend on each other.

I broadly agree.

For instance, Bart's work has already completely fixed srp to load
properly and race free. The missing piece is to have it also autoload
the required srp kernel module.

Benjamin, I have a different, simpler suggestion for some of the other
bits, let me post an RFC

Here is an untested attempt to move module loading into srp_daemon -
what do you think Bart?

Jason

>From 28c1e19719ed850bc8f328d187c536c97eef7fd7 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date: Thu, 13 Jul 2017 12:10:07 -0600
Subject: [PATCH] srp: Autoload the SRP kernel module if required

Doing this before starting the daemon ensures the daemon will work.
Since the daemon is using sysfs files there is no easy way to have the
kernel auto load it.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 srp_daemon/srp_daemon.service.in       |  2 +-
 srp_daemon/srp_daemon_port@.service.in |  1 +
 srp_daemon/srp_kernel_module.service   | 12 ++++++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)
 create mode 100644 srp_daemon/srp_kernel_module.service

diff --git a/srp_daemon/srp_daemon.service.in b/srp_daemon/srp_daemon.service.in
index 33dddd5cb46fef..cca1fce9c99283 100644
--- a/srp_daemon/srp_daemon.service.in
+++ b/srp_daemon/srp_daemon.service.in
@@ -1,6 +1,6 @@
 [Unit]
 Description=Daemon that discovers and logs in to SRP target systems
-Documentation=man:srp_daemon file:/etc/rdma/rdma.conf file:/etc/srp_daemon.conf
+Documentation=man:srp_daemon file:/etc/srp_daemon.conf
 DefaultDependencies=false
 Conflicts=emergency.target emergency.service
 Before=remote-fs-pre.target
diff --git a/srp_daemon/srp_daemon_port@.service.in b/srp_daemon/srp_daemon_port@.service.in
index 0ec966f912aec8..3c9c824fd243aa 100644
--- a/srp_daemon/srp_daemon_port@.service.in
+++ b/srp_daemon/srp_daemon_port@.service.in
@@ -3,6 +3,7 @@ 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=srp_kernel_module.service
 After=srp_daemon.service dev-infiniband-umad-%i.device network.target
 BindsTo=srp_daemon.service dev-infiniband-umad-%i.device
 Before=remote-fs-pre.target
diff --git a/srp_daemon/srp_kernel_module.service b/srp_daemon/srp_kernel_module.service
new file mode 100644
index 00000000000000..b779031578dae1
--- /dev/null
+++ b/srp_daemon/srp_kernel_module.service
@@ -0,0 +1,12 @@
+[Unit]
+Description=Load the SRP daemon kernel module
+Documentation=man:srp_daemon
+After=systemd-modules-load.service
+ConditionCapability=CAP_SYS_MODULE
+ConditionPathIsDirectory=!/sys/class/infiniband_srp/
+
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecPre=bash -c 'mkdir -p /run/modules-load.d && echo ib_srp > /run/modules-load.d/rdma_core_srp_modules.conf'
+ExecStart=/lib/systemd/systemd-modules-load
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Bart Van Assche]

On Thu, 2017-07-13 at 12:20 -0600, Jason Gunthorpe wrote:
> Here is an untested attempt to move module loading into srp_daemon -
> what do you think Bart?

Hello Jason,

Thanks for the patch. To me this looks like a nice simplification compared
to the current approach. Do you want me to test this patch?

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 06:33:07PM +0000, Bart Van Assche wrote:
> On Thu, 2017-07-13 at 12:20 -0600, Jason Gunthorpe wrote:
> > Here is an untested attempt to move module loading into srp_daemon -
> > what do you think Bart?
> 
> Hello Jason,
> 
> Thanks for the patch. To me this looks like a nice simplification compared
> to the current approach. Do you want me to test this patch?

I did some more steps..

Benjamin/Bart let me know what you think, I'll send the patches to the
list if you like the idea.

https://github.com/jgunthorpe/rdma-plumbing/tree/systemd

This replaces Benjamins rdma.conf based approach.

The basic approach is to directly use systemd to load the modules and
determine what modules to load by using udev and explicit Requires in
the units.

Eg, the extra modules for mlx4 are loaded by triggering this udev rule:

SUBSYSTEM=="module", KERNEL=="mlx4_core", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma-load-modules@mlx4"

Which causes the module loader to load the contents of the modules
list in /etc/rdma/modules/mlx4.conf

If the user does not want a specific to load then they can comment out
the module line in the /etc/rdma/modules files or use the usual module
black list scheme.

The interesting thing about this is how the boot ordering works, since
rdma-load-modules@.service is a 'before' sysinit.target it will run
before normal system startup tasks if the unit is scheduled to run
early enough. I expect that boot time module loads triggered by udev
will be early enough..

This makes it much closer to normal module loading, instead of being
so strange and should eliminate much of the need for explicit
rdma.service dependencies. The places that need something special,
like srp, can directly depend on their specific module loader:

Requires=rdma-load-modules@srp_daemon

Again, if srp_daemon is scheduled to start during system boot this
should result in the modules being loaded together with the rest of
the modules before sysinit.target.

The one nit that I don't have an easy solution for is starting modules
depending on the device technology, eg not starting srp or ipoib on
!IB devices. The issue is that I can't think of an easy way to detect
the device technology from the udev rule, at least not without a
helper script or additional kernel sysfs..

I havne't been able to test this yet, help would be appreciated.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Bart Van Assche]

On Thu, 2017-07-13 at 16:15 -0600, Jason Gunthorpe wrote:
> The one nit that I don't have an easy solution for is starting modules
> depending on the device technology, eg not starting srp or ipoib on
> !IB devices. The issue is that I can't think of an easy way to detect
> the device technology from the udev rule, at least not without a
> helper script or additional kernel sysfs..

Are users expected to modify the kernel-boot/modules/*.conf files? I
expect that only a minority of the RDMA users will want to load the ib_srpt
and ib_isert kernel modules. 

> I havne't been able to test this yet, help would be appreciated.

I will try to free up some time to test the SRP-related changes.

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 10:36:49PM +0000, Bart Van Assche wrote:
> On Thu, 2017-07-13 at 16:15 -0600, Jason Gunthorpe wrote:
> > The one nit that I don't have an easy solution for is starting modules
> > depending on the device technology, eg not starting srp or ipoib on
> > !IB devices. The issue is that I can't think of an easy way to detect
> > the device technology from the udev rule, at least not without a
> > helper script or additional kernel sysfs..
> 
> Are users expected to modify the kernel-boot/modules/*.conf files? I
> expect that only a minority of the RDMA users will want to load the ib_srpt
> and ib_isert kernel modules. 

I'd say in the same way they would be expected to modify the RedHat
/etc/rdma/rdma.conf file. I copied the defaults from that file, which
was to autoload target drivers.

I agree with you, they probably don't make sense to be defaulted on.
Ideally their respective supporting packages or kernel would ensure
the modules autoload if someone is working with RDMA... I know nothing
about the target infrastructure - can you see a way to do that?

Thanks,
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Bart Van Assche]

On Thu, 2017-07-13 at 16:41 -0600, Jason Gunthorpe wrote:
> On Thu, Jul 13, 2017 at 10:36:49PM +0000, Bart Van Assche wrote:
> > On Thu, 2017-07-13 at 16:15 -0600, Jason Gunthorpe wrote:
> > > The one nit that I don't have an easy solution for is starting modules
> > > depending on the device technology, eg not starting srp or ipoib on
> > > !IB devices. The issue is that I can't think of an easy way to detect
> > > the device technology from the udev rule, at least not without a
> > > helper script or additional kernel sysfs..
> > 
> > Are users expected to modify the kernel-boot/modules/*.conf files? I
> > expect that only a minority of the RDMA users will want to load the ib_srpt
> > and ib_isert kernel modules. 
> 
> I'd say in the same way they would be expected to modify the RedHat
> /etc/rdma/rdma.conf file. I copied the defaults from that file, which
> was to autoload target drivers.
> 
> I agree with you, they probably don't make sense to be defaulted on.
> Ideally their respective supporting packages or kernel would ensure
> the modules autoload if someone is working with RDMA... I know nothing
> about the target infrastructure - can you see a way to do that?

Hello Jason,

Users who want to enable RDMA target functionality probably will know the
names of the kernel modules that provide such functionality. On Debian
systems the names of these kernel modules can e.g. be added to /etc/modules.
However, I'm not sure all distro's have an equivalent of the /etc/modules
file.

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 10:45:14PM +0000, Bart Van Assche wrote:

> Users who want to enable RDMA target functionality probably will know the
> names of the kernel modules that provide such functionality. On Debian
> systems the names of these kernel modules can e.g. be added to /etc/modules.
> However, I'm not sure all distro's have an equivalent of the /etc/modules
> file.

systemd supports /etc/modules-load.d/ as a built in, some distros will
symlink ../modules.conf to that directory, but these days dropping a
file in /etc/modules-load.d/ is pretty safe.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Benjamin Drung]

Hi Jason,

Am Donnerstag, den 13.07.2017, 16:15 -0600 schrieb Jason Gunthorpe:
> On Thu, Jul 13, 2017 at 06:33:07PM +0000, Bart Van Assche wrote:
> > On Thu, 2017-07-13 at 12:20 -0600, Jason Gunthorpe wrote:
> > > Here is an untested attempt to move module loading into
> > > srp_daemon -
> > > what do you think Bart?
> > 
> > Hello Jason,
> > 
> > Thanks for the patch. To me this looks like a nice simplification
> > compared
> > to the current approach. Do you want me to test this patch?
> 
> I did some more steps..
> 
> Benjamin/Bart let me know what you think, I'll send the patches to
> the
> list if you like the idea.
> 
> https://github.com/jgunthorpe/rdma-plumbing/tree/systemd
> 
> This replaces Benjamins rdma.conf based approach.
> 
> The basic approach is to directly use systemd to load the modules and
> determine what modules to load by using udev and explicit Requires in
> the units.

I like your approach very much. It's cleaner and simpler. So let's
ditch my approach and use yours.

I tested your branch on a Debian 8 (jessie) system with a mlx4 card and
found several points to fix/improve/discuss:

1. The rdma-load-modules@ service needs to set DefaultDependencies=no.
Otherwise it will end up in a dependency loop:

Found ordering cycle on rdma-load-modules-A9MMwDI1uB1TDjBF/Jpztg@public.gmane.org/start
Found dependency on basic.target/start
Found dependency on paths.target/start
Found dependency on acpid.path/start
Found dependency on sysinit.target/start
Found dependency on rdma-load-modules-A9MMwDI1uB1TDjBF/Jpztg@public.gmane.org/start
Breaking ordering cycle by deleting job paths.target/start
Found ordering cycle on rdma-load-modules-A9MMwDI1uB1TDjBF/Jpztg@public.gmane.org/start
Found dependency on paths.target/start
Found dependency on acpid.path/start
Found dependency on sysinit.target/start
Found dependency on rdma-load-modules-A9MMwDI1uB1TDjBF/Jpztg@public.gmane.org/start
Starting Load RDMA modules from /etc/rdma/modules/mlx4.conf...

2. You can just specify "etc/rdma/modules" in debian/rdma-core.install
instead of listing each .conf file individually.

3. Default conf files for opa.conf and roce.conf are missing.

4. Should rdma-load-modules@ *not* fail if the corresponding .conf is
missing?

5. How to handle build-in modules correctly? Our kernel has the i40e
module built in (CONFIG_I40E=y) and rdma-load-modules-QqjR1VyAyFhTDjBF/Jpztg@public.gmane.org will
be started, but the system does not have a i40e card and thus I don't
want to have the module started. 

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
Web: https://www.profitbricks.com

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506B.
Geschäftsführer: Achim Weiss.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Benjamin Drung]

Am Freitag, den 14.07.2017, 12:08 +0200 schrieb Benjamin Drung:
> Hi Jason,
> 
> Am Donnerstag, den 13.07.2017, 16:15 -0600 schrieb Jason Gunthorpe:
> > On Thu, Jul 13, 2017 at 06:33:07PM +0000, Bart Van Assche wrote:
> > > On Thu, 2017-07-13 at 12:20 -0600, Jason Gunthorpe wrote:
> > > > Here is an untested attempt to move module loading into
> > > > srp_daemon -
> > > > what do you think Bart?
> > > 
> > > Hello Jason,
> > > 
> > > Thanks for the patch. To me this looks like a nice simplification
> > > compared
> > > to the current approach. Do you want me to test this patch?
> > 
> > I did some more steps..
> > 
> > Benjamin/Bart let me know what you think, I'll send the patches to
> > the
> > list if you like the idea.
> > 
> > https://github.com/jgunthorpe/rdma-plumbing/tree/systemd
> > 
> > This replaces Benjamins rdma.conf based approach.
> > 
> > The basic approach is to directly use systemd to load the modules
> > and
> > determine what modules to load by using udev and explicit Requires
> > in
> > the units.
> 
> I like your approach very much. It's cleaner and simpler. So let's
> ditch my approach and use yours.
> 
> I tested your branch on a Debian 8 (jessie) system with a mlx4 card
> and
> found several points to fix/improve/discuss:
> 
> 1. The rdma-load-modules@ service needs to set
> DefaultDependencies=no.
> Otherwise it will end up in a dependency loop:
> 
> Found ordering cycle on rdma-load-modules-A9MMwDI1uB1TDjBF/Jpztg@public.gmane.org/start
> Found dependency on basic.target/start
> Found dependency on paths.target/start
> Found dependency on acpid.path/start
> Found dependency on sysinit.target/start
> Found dependency on rdma-load-modules-A9MMwDI1uB1TDjBF/Jpztg@public.gmane.org/start
> Breaking ordering cycle by deleting job paths.target/start
> Found ordering cycle on rdma-load-modules-A9MMwDI1uB1TDjBF/Jpztg@public.gmane.org/start
> Found dependency on paths.target/start
> Found dependency on acpid.path/start
> Found dependency on sysinit.target/start
> Found dependency on rdma-load-modules-A9MMwDI1uB1TDjBF/Jpztg@public.gmane.org/start
> Starting Load RDMA modules from /etc/rdma/modules/mlx4.conf...
> 
> 2. You can just specify "etc/rdma/modules" in debian/rdma-
> core.install
> instead of listing each .conf file individually.
> 
> 3. Default conf files for opa.conf and roce.conf are missing.
> 
> 4. Should rdma-load-modules@ *not* fail if the corresponding .conf is
> missing?
> 
> 5. How to handle build-in modules correctly? Our kernel has the i40e
> module built in (CONFIG_I40E=y) and rdma-load-modules-QqjR1VyAyFhTDjBF/Jpztg@public.gmane.org
> will
> be started, but the system does not have a i40e card and thus I don't
> want to have the module started. 

Forgot one point:

6. The ipoib module (loaded by rdma-load-modules@infiniband) needs to
loaded before the networking.service is running. The networking.service
brings up the network devices on Debian. It runs "ifup -a" which reads
/etc/network/interfaces which we use to configure our ipoib devices.
Here is the networking.service definition:

bdrung@server:~$ systemctl cat networking.service
# /run/systemd/generator.late/networking.service
# Automatically generated by systemd-sysv-generator

[Unit]
SourcePath=/etc/init.d/networking
Description=LSB: Raise network interfaces.
DefaultDependencies=no
Before=sysinit.target shutdown.target
After=mountkernfs.service local-fs.target urandom.service
Conflicts=shutdown.target

[Service]
Type=forking
Restart=no
TimeoutSec=0
IgnoreSIGPIPE=no
KillMode=process
GuessMainPID=no
RemainAfterExit=yes
SysVStartPriority=13
ExecStart=/etc/init.d/networking start
ExecStop=/etc/init.d/networking stop
ExecReload=/etc/init.d/networking reload

# /run/systemd/generator/networking.service.d/50-insserv.conf-$network.conf
# Automatically generated by systemd-insserv-generator

[Unit]
Wants=network.target
Before=network.target

# /lib/systemd/system/networking.service.d/network-pre.conf
[Unit]
After=network-pre.target systemd-sysctl.service systemd-modules-load.service

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
Web: https://www.profitbricks.com

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506B.
Geschäftsführer: Achim Weiss.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Jason Gunthorpe]

On Fri, Jul 14, 2017 at 12:08:41PM +0200, Benjamin Drung wrote:
> I tested your branch on a Debian 8 (jessie) system with a mlx4 card and
> found several points to fix/improve/discuss:

Thanks!

> 1. The rdma-load-modules@ service needs to set DefaultDependencies=no.
> Otherwise it will end up in a dependency loop:

Makes sense
> 2. You can just specify "etc/rdma/modules" in debian/rdma-core.install
> instead of listing each .conf file individually.

The srp_daemon.conf is in the srp package, so I don't think I can do
that?

> 3. Default conf files for opa.conf and roce.conf are missing.

Right.. Not sure if we should split them like this or not, depends if
we can detect the card type at runtime.

> 4. Should rdma-load-modules@ *not* fail if the corresponding .conf is
> missing?

Does it fail now? Failing seems like the right thing to do for a
missing conf file.

> 5. How to handle build-in modules correctly? Our kernel has the i40e
> module built in (CONFIG_I40E=y) and rdma-load-modules-QqjR1VyAyFhTDjBF/Jpztg@public.gmane.org will
> be started, but the system does not have a i40e card and thus I don't
> want to have the module started. 

Fixing this would require more fancy udev wonkery - I copied RH's
tested approach which triggers on driver presence, not on driver
binding.

My udev is not great, but something like this:

DRIVER=="mlx4_core", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma-load-modules@mlx4"

Might work better? I think that triggers on driver bind? Could you try
to switch your mlx and i40e drivers in that way?

> 6. The ipoib module (loaded by rdma-load-modules@infiniband) needs to
> loaded before the networking.service is running. The networking.service
> brings up the network devices on Debian. It runs "ifup -a" which reads

Hum. That LSB networking.service sure is an ugly hack, it doesn't
support hotplug so it has this:

  After=network-pre.target systemd-sysctl.service systemd-modules-load.service

To 'try' and run after some amount of hot plugging is done. IMHO this
is done wrong, it should start after sysinit.target but before
network-online.target or something... 

The only solution to this kind of problem is to add more ordering,
Debian should include a patch to rdma-load-modules@ to put it before
their unique networking.service..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Benjamin Drung]

Am Freitag, den 14.07.2017, 09:55 -0600 schrieb Jason Gunthorpe:
> 2. You can just specify "etc/rdma/modules" in debian/rdma-
> > core.install
> > instead of listing each .conf file individually.
> 
> The srp_daemon.conf is in the srp package, so I don't think I can do
> that?

Yes. You are right. So it needs to be listed individually.

> > 4. Should rdma-load-modules@ *not* fail if the corresponding .conf
> > is
> > missing?
> 
> Does it fail now? Failing seems like the right thing to do for a
> missing conf file.

Yes. Currently it fails in this situation.

> > 5. How to handle build-in modules correctly? Our kernel has the
> > i40e
> > module built in (CONFIG_I40E=y) and rdma-load-modules-QqjR1VyAyFhTDjBF/Jpztg@public.gmane.org
> > will
> > be started, but the system does not have a i40e card and thus I
> > don't
> > want to have the module started. 
> 
> Fixing this would require more fancy udev wonkery - I copied RH's
> tested approach which triggers on driver presence, not on driver
> binding.
> 
> My udev is not great, but something like this:
> 
> DRIVER=="mlx4_core", ACTION=="add", TAG+="systemd",
> ENV{SYSTEMD_WANTS}="rdma-load-modules@mlx4"
> 
> Might work better? I think that triggers on driver bind? Could you
> try
> to switch your mlx and i40e drivers in that way?

I tried it. It works as expected. rdma-load-modules@mlx4 is loaded and 
rdma-load-modules-QqjR1VyAyFhTDjBF/Jpztg@public.gmane.org is not loaded. Not tested if the udev
trigger will also work for used built-in modules.

> > 6. The ipoib module (loaded by rdma-load-modules@infiniband) needs
> > to
> > loaded before the networking.service is running. The
> > networking.service
> > brings up the network devices on Debian. It runs "ifup -a" which
> > reads
> 
> Hum. That LSB networking.service sure is an ugly hack, it doesn't
> support hotplug so it has this:
> 
>   After=network-pre.target systemd-sysctl.service systemd-modules-
> load.service
> 
> To 'try' and run after some amount of hot plugging is done. IMHO this
> is done wrong, it should start after sysinit.target but before
> network-online.target or something... 
> 
> The only solution to this kind of problem is to add more ordering,
> Debian should include a patch to rdma-load-modules@ to put it before
> their unique networking.service..

Or patch rdma-load-modules@ to put it before network-pre.target

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
Web: https://www.profitbricks.com

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506B.
Geschäftsführer: Achim Weiss.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Jason Gunthorpe]

On Fri, Jul 14, 2017 at 06:23:13PM +0200, Benjamin Drung wrote:

> > My udev is not great, but something like this:
> > 
> > DRIVER=="mlx4_core", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma-load-modules@mlx4"
> > 
> > Might work better? I think that triggers on driver bind? Could you
> > try to switch your mlx and i40e drivers in that way?
> 
> I tried it. It works as expected. rdma-load-modules@mlx4 is loaded and 
> rdma-load-modules-QqjR1VyAyFhTDjBF/Jpztg@public.gmane.org is not loaded. Not tested if the udev
> trigger will also work for used built-in modules.

Okay, I will change it to be like that and we can see what people
think.

> > The only solution to this kind of problem is to add more ordering,
> > Debian should include a patch to rdma-load-modules@ to put it before
> > their unique networking.service..
> 
> Or patch rdma-load-modules@ to put it before network-pre.target

I think that would be OK.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Benjamin Drung]

Am Freitag, den 14.07.2017, 10:40 -0600 schrieb Jason Gunthorpe:
> On Fri, Jul 14, 2017 at 06:23:13PM +0200, Benjamin Drung wrote:
> 
> > > My udev is not great, but something like this:
> > > 
> > > DRIVER=="mlx4_core", ACTION=="add", TAG+="systemd",
> > > ENV{SYSTEMD_WANTS}="rdma-load-modules@mlx4"
> > > 
> > > Might work better? I think that triggers on driver bind? Could
> > > you
> > > try to switch your mlx and i40e drivers in that way?
> > 
> > I tried it. It works as expected. rdma-load-modules@mlx4 is loaded
> > and 
> > rdma-load-modules-QqjR1VyAyFhTDjBF/Jpztg@public.gmane.org is not loaded. Not tested if the
> > udev
> > trigger will also work for used built-in modules.
> 
> Okay, I will change it to be like that and we can see what people
> think.

Any updates?

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
Web: https://www.profitbricks.com

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506B.
Geschäftsführer: Achim Weiss.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add rdma service for kernel boot support

[Jason Gunthorpe]

On Thu, Jul 20, 2017 at 11:04:10AM +0200, Benjamin Drung wrote:
> Am Freitag, den 14.07.2017, 10:40 -0600 schrieb Jason Gunthorpe:
> > On Fri, Jul 14, 2017 at 06:23:13PM +0200, Benjamin Drung wrote:
> > 
> > > > My udev is not great, but something like this:
> > > > 
> > > > DRIVER=="mlx4_core", ACTION=="add", TAG+="systemd",
> > > > ENV{SYSTEMD_WANTS}="rdma-load-modules@mlx4"
> > > > 
> > > > Might work better? I think that triggers on driver bind? Could
> > > > you
> > > > try to switch your mlx and i40e drivers in that way?
> > > 
> > > I tried it. It works as expected.?rdma-load-modules@mlx4 is loaded
> > > and?
> > > rdma-load-modules-QqjR1VyAyFhTDjBF/Jpztg@public.gmane.org is not loaded. Not tested if the
> > > udev
> > > trigger will also work for used built-in modules.
> > 
> > Okay, I will change it to be like that and we can see what people
> > think.
> 
> Any updates?

I've been doing some more testing here and it does not work entirely
properly yet.

The change to 'DRIVER==' causes things to fail, the kernel only
generates uevents when kobjects are created, and binding a driver to a
PCI device does not create a kobject. This means these lines:

DRIVER=="mlx4_core", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma-load-modules@mlx4"

Do not work reliably. Depending on the module like before would still
seem to be the best option.

Also, the ordering before sysinit.target does not work reliably, udev
loads modules asynchronously and systemd does not block on udev unless
systemd-udevd-settle is depended on.

I'm not sure how Debian's legacy networking.service manages to work at
all, perhaps it is broken and it is just unlikely to be hit because of
how quickly most ethernet drivers get loaded by udev.

As far as I can tell, it needs to depend on systemd-udevd-settle to
guarentee that all the cold-plug network drivers have been loaded by
udev before starting..

.. all of these comments would seem to apply equally to the
rdma.service approach, so I think this is still an improvement.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html