From e9a429e9ba3a57e5f9e7c152c6d21ff6fd024d6e Mon Sep 17 00:00:00 2001 From: John Meneghini Date: Mon, 17 Jul 2023 17:18:18 -0400 Subject: [PATCH] nvmf-autoconnect: restart service to avoid dropping AEN Includes: fabrics: only look for matching ctrl on same host Resolves: #2223436 Signed-off-by: John Meneghini --- ...-restart-service-to-avoid-dropping-A.patch | 56 +++++++ ...-look-for-matching-ctrl-on-same-host.patch | 156 ++++++++++++++++++ nvme-cli.spec | 9 +- 3 files changed, 220 insertions(+), 1 deletion(-) create mode 100644 0006-nvmf-autoconnect-restart-service-to-avoid-dropping-A.patch create mode 100644 0007-fabrics-only-look-for-matching-ctrl-on-same-host.patch diff --git a/0006-nvmf-autoconnect-restart-service-to-avoid-dropping-A.patch b/0006-nvmf-autoconnect-restart-service-to-avoid-dropping-A.patch new file mode 100644 index 0000000..4415179 --- /dev/null +++ b/0006-nvmf-autoconnect-restart-service-to-avoid-dropping-A.patch @@ -0,0 +1,56 @@ +From 7723f827e8165652a4a18844114004d78ac411df Mon Sep 17 00:00:00 2001 +From: Caleb Sander +Date: Fri, 12 May 2023 13:41:06 -0600 +Subject: [PATCH] nvmf-autoconnect: restart service to avoid dropping AEN +Content-type: text/plain + +If another discovery log page change AEN is received while the +autoconnect service is already running for a previous AEN, +the autoconnect service is not being relaunched. +For example, if some of the entries in the log page are not reachable, +attempting to connect to them may take a long time before timing out, +so AENs received while waiting for the connections have no effect. +The AEN is effectively dropped: we ignore the new discovery log entries +and worse, the controller won't send more AENs because it's waiting +for the log page to be fetched in response to the previous AEN. + +Use systemctl restart instead of systemctl start +to ensure nvme connect-all is run in response to each AEN. +If a previous instance of the service is already running, +it will be interrupted and the new log page will be fetched. + +Fixes: 391d3b12248a ("nvme-cli: nvmf auto-connect scripts") +Signed-off-by: Caleb Sander +--- + nvmf-autoconnect/udev-rules/70-nvmf-autoconnect.rules.in | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +diff --git a/nvmf-autoconnect/udev-rules/70-nvmf-autoconnect.rules.in b/nvmf-autoconnect/udev-rules/70-nvmf-autoconnect.rules.in +index e751ee09..3eca6dfe 100644 +--- a/nvmf-autoconnect/udev-rules/70-nvmf-autoconnect.rules.in ++++ b/nvmf-autoconnect/udev-rules/70-nvmf-autoconnect.rules.in +@@ -15,18 +15,18 @@ ENV{NVME_HOST_IFACE}=="", ENV{NVME_HOST_IFACE}="none" + ACTION=="change", SUBSYSTEM=="nvme", ENV{NVME_AEN}=="0x70f002",\ + ENV{NVME_TRTYPE}=="*", ENV{NVME_TRADDR}=="*", \ + ENV{NVME_TRSVCID}=="*", ENV{NVME_HOST_TRADDR}=="*", ENV{NVME_HOST_IFACE}=="*", \ +- RUN+="@SYSTEMCTL@ --no-block start nvmf-connect@--device=$kernel\t--transport=$env{NVME_TRTYPE}\t--traddr=$env{NVME_TRADDR}\t--trsvcid=$env{NVME_TRSVCID}\t--host-traddr=$env{NVME_HOST_TRADDR}\t--host-iface=$env{NVME_HOST_IFACE}.service" ++ RUN+="@SYSTEMCTL@ --no-block restart nvmf-connect@--device=$kernel\t--transport=$env{NVME_TRTYPE}\t--traddr=$env{NVME_TRADDR}\t--trsvcid=$env{NVME_TRSVCID}\t--host-traddr=$env{NVME_HOST_TRADDR}\t--host-iface=$env{NVME_HOST_IFACE}.service" + + # nvme-fc transport generated events (old-style for compatibility) + ACTION=="change", SUBSYSTEM=="fc", ENV{FC_EVENT}=="nvmediscovery", \ + ENV{NVMEFC_HOST_TRADDR}=="*", ENV{NVMEFC_TRADDR}=="*", \ +- RUN+="@SYSTEMCTL@ --no-block start nvmf-connect@--device=none\t--transport=fc\t--traddr=$env{NVMEFC_TRADDR}\t--trsvcid=none\t--host-traddr=$env{NVMEFC_HOST_TRADDR}.service" ++ RUN+="@SYSTEMCTL@ --no-block restart nvmf-connect@--device=none\t--transport=fc\t--traddr=$env{NVMEFC_TRADDR}\t--trsvcid=none\t--host-traddr=$env{NVMEFC_HOST_TRADDR}.service" + + # A discovery controller just (re)connected, re-read the discovery log change to + # check if there were any changes since it was last connected. + ACTION=="change", SUBSYSTEM=="nvme", ENV{NVME_EVENT}=="rediscover", ATTR{cntrltype}=="discovery", \ + ENV{NVME_TRTYPE}=="*", ENV{NVME_TRADDR}=="*", \ + ENV{NVME_TRSVCID}=="*", ENV{NVME_HOST_TRADDR}=="*", ENV{NVME_HOST_IFACE}=="*", \ +- RUN+="@SYSTEMCTL@ --no-block start nvmf-connect@--device=$kernel\t--transport=$env{NVME_TRTYPE}\t--traddr=$env{NVME_TRADDR}\t--trsvcid=$env{NVME_TRSVCID}\t--host-traddr=$env{NVME_HOST_TRADDR}\t--host-iface=$env{NVME_HOST_IFACE}.service" ++ RUN+="@SYSTEMCTL@ --no-block restart nvmf-connect@--device=$kernel\t--transport=$env{NVME_TRTYPE}\t--traddr=$env{NVME_TRADDR}\t--trsvcid=$env{NVME_TRSVCID}\t--host-traddr=$env{NVME_HOST_TRADDR}\t--host-iface=$env{NVME_HOST_IFACE}.service" + + LABEL="autoconnect_end" +-- +2.39.3 + diff --git a/0007-fabrics-only-look-for-matching-ctrl-on-same-host.patch b/0007-fabrics-only-look-for-matching-ctrl-on-same-host.patch new file mode 100644 index 0000000..aec9a16 --- /dev/null +++ b/0007-fabrics-only-look-for-matching-ctrl-on-same-host.patch @@ -0,0 +1,156 @@ +From 9cf31d24f8b3911b870cb217c6715637459a376a Mon Sep 17 00:00:00 2001 +From: Caleb Sander +Date: Wed, 28 Jun 2023 14:35:24 -0600 +Subject: [PATCH] fabrics: only look for matching ctrl on same host +Content-type: text/plain + +With the lookup_ctrl() check to prevent duplicate connections, +identical connections from different hosts are no longer possible. +Fix lookup_ctrl() to only look for matching controllers +on the same host that is being connected. +This restores the ability for multiple hosts on the same machine +to connect to the same transport addresses on the same subsystem. + +Fixes: 07d6b91 ("fabrics: Do not attempt to reconnect to already +connected ctrls") +Signed-off-by: Caleb Sander +--- + fabrics.c | 40 ++++++++++++++++++++++++++-------------- + fabrics.h | 2 +- + nbft.c | 4 ++-- + 3 files changed, 29 insertions(+), 17 deletions(-) + +diff --git a/fabrics.c b/fabrics.c +index ac240cad..14a91cc6 100644 +--- a/fabrics.c ++++ b/fabrics.c +@@ -149,6 +149,8 @@ static bool is_persistent_discovery_ctrl(nvme_host_t h, nvme_ctrl_t c) + return false; + } + ++typedef bool (*ctrl_match_fn_t)(nvme_ctrl_t, struct tr_config *); ++ + static bool disc_ctrl_config_match(nvme_ctrl_t c, struct tr_config *trcfg) + { + if (nvme_ctrl_is_discovery_ctrl(c) && +@@ -175,34 +177,45 @@ static bool ctrl_config_match(nvme_ctrl_t c, struct tr_config *trcfg) + return false; + } + +-static nvme_ctrl_t __lookup_ctrl(nvme_root_t r, struct tr_config *trcfg, +- bool (*filter)(nvme_ctrl_t, struct tr_config *)) ++static nvme_ctrl_t __lookup_host_ctrl(nvme_host_t h, struct tr_config *trcfg, ++ ctrl_match_fn_t filter) + { +- nvme_host_t h; + nvme_subsystem_t s; + nvme_ctrl_t c; + +- nvme_for_each_host(r, h) { +- nvme_for_each_subsystem(h, s) { +- nvme_subsystem_for_each_ctrl(s, c) { +- if (!(filter(c, trcfg))) +- continue; ++ nvme_for_each_subsystem(h, s) { ++ nvme_subsystem_for_each_ctrl(s, c) { ++ if (filter(c, trcfg)) + return c; +- } + } + } + + return NULL; + } + ++static nvme_ctrl_t __lookup_ctrl(nvme_root_t r, struct tr_config *trcfg, ++ ctrl_match_fn_t filter) ++{ ++ nvme_host_t h; ++ nvme_ctrl_t c; ++ ++ nvme_for_each_host(r, h) { ++ c = __lookup_host_ctrl(h, trcfg, filter); ++ if (c) ++ return c; ++ } ++ ++ return NULL; ++} ++ + static nvme_ctrl_t lookup_discovery_ctrl(nvme_root_t r, struct tr_config *trcfg) + { + return __lookup_ctrl(r, trcfg, disc_ctrl_config_match); + } + +-nvme_ctrl_t lookup_ctrl(nvme_root_t r, struct tr_config *trcfg) ++nvme_ctrl_t lookup_ctrl(nvme_host_t h, struct tr_config *trcfg) + { +- return __lookup_ctrl(r, trcfg, ctrl_config_match); ++ return __lookup_host_ctrl(h, trcfg, ctrl_config_match); + } + + static int set_discovery_kato(struct nvme_fabrics_config *cfg) +@@ -317,7 +330,6 @@ static int __discover(nvme_ctrl_t c, struct nvme_fabrics_config *defcfg, + struct nvmf_discovery_log *log = NULL; + nvme_subsystem_t s = nvme_ctrl_get_subsystem(c); + nvme_host_t h = nvme_subsystem_get_host(s); +- nvme_root_t r = nvme_host_get_root(h); + uint64_t numrec; + + struct nvme_get_discovery_args args = { +@@ -362,7 +374,7 @@ static int __discover(nvme_ctrl_t c, struct nvme_fabrics_config *defcfg, + }; + + /* Already connected ? */ +- cl = lookup_ctrl(r, &trcfg); ++ cl = lookup_ctrl(h, &trcfg); + if (cl && nvme_ctrl_get_name(cl)) + continue; + +@@ -989,7 +1001,7 @@ int nvmf_connect(const char *desc, int argc, char **argv) + .trsvcid = trsvcid, + }; + +- c = lookup_ctrl(r, &trcfg); ++ c = lookup_ctrl(h, &trcfg); + if (c && nvme_ctrl_get_name(c)) { + fprintf(stderr, "already connected\n"); + errno = EALREADY; +diff --git a/fabrics.h b/fabrics.h +index 02cebf5d..c16df604 100644 +--- a/fabrics.h ++++ b/fabrics.h +@@ -11,7 +11,7 @@ struct tr_config { + const char *trsvcid; + }; + +-extern nvme_ctrl_t lookup_ctrl(nvme_root_t r, struct tr_config *trcfg); ++extern nvme_ctrl_t lookup_ctrl(nvme_host_t h, struct tr_config *trcfg); + extern int nvmf_discover(const char *desc, int argc, char **argv, bool connect); + extern int nvmf_connect(const char *desc, int argc, char **argv); + extern int nvmf_disconnect(const char *desc, int argc, char **argv); +diff --git a/nbft.c b/nbft.c +index c73413f5..e46e9d3d 100644 +--- a/nbft.c ++++ b/nbft.c +@@ -144,7 +144,7 @@ int discover_from_nbft(nvme_root_t r, char *hostnqn_arg, char *hostid_arg, + }; + + /* Already connected ? */ +- cl = lookup_ctrl(r, &trcfg); ++ cl = lookup_ctrl(h, &trcfg); + if (cl && nvme_ctrl_get_name(cl)) + continue; + +@@ -170,7 +170,7 @@ int discover_from_nbft(nvme_root_t r, char *hostnqn_arg, char *hostid_arg, + nvme_free_ctrl(c); + + trcfg.host_traddr = NULL; +- cl = lookup_ctrl(r, &trcfg); ++ cl = lookup_ctrl(h, &trcfg); + if (cl && nvme_ctrl_get_name(cl)) + continue; + +-- +2.39.3 + diff --git a/nvme-cli.spec b/nvme-cli.spec index 11d6724..223806c 100644 --- a/nvme-cli.spec +++ b/nvme-cli.spec @@ -3,7 +3,7 @@ Name: nvme-cli Version: 2.4 -Release: 7%{?dist} +Release: 8%{?dist} Summary: NVMe management command line interface License: GPLv2+ @@ -15,6 +15,8 @@ Patch1: 0002-nbft-added-NBFT-v1.0-table-support.patch Patch2: 0003-nbft-add-the-nbft-show-plugin.patch Patch3: 0004-Revert-nvme-Masks-SSTAT-in-sanize-log-output.patch Patch4: 0005-util-Fix-suffix_si_parse-to-parse-no-decimal-point-b.patch +Patch5: 0006-nvmf-autoconnect-restart-service-to-avoid-dropping-A.patch +Patch6: 0007-fabrics-only-look-for-matching-ctrl-on-same-host.patch BuildRequires: meson >= 0.50.0 BuildRequires: gcc gcc-c++ @@ -41,6 +43,8 @@ nvme-cli provides NVM-Express user space tooling for Linux. %patch2 -p1 %patch3 -p1 %patch4 -p1 +%patch5 -p1 +%patch6 -p1 %build %meson -Dudevrulesdir=%{_udevrulesdir} -Dsystemddir=%{_unitdir} -Ddocs=all -Ddocs-build=true -Dhtmldir=%{_pkgdocdir} @@ -99,6 +103,9 @@ if [ $1 -eq 1 ] || [ $1 -eq 2 ]; then fi %changelog +* Mon Jul 17 2023 John Meneghini - 2.4-8 +- Fix BZ#2223436 + * Wed Jun 14 2023 Maurizio Lombardi - 2.4-7 - Fix BZ#2210656