Revert "nvmf-autoconnect: restart service to avoid dropping AEN"

This reverts commit e9a429e9ba.

JIRA: https://issues.redhat.com/browse/RHEL-1492

Signed-off-by: John Meneghini <jmeneghi@redhat.com>
This commit is contained in:
John Meneghini 2023-08-21 09:25:09 -04:00
parent 5e7490b949
commit 0c0d56ad49
3 changed files with 4 additions and 217 deletions

View File

@ -1,56 +0,0 @@
From 7723f827e8165652a4a18844114004d78ac411df Mon Sep 17 00:00:00 2001
From: Caleb Sander <csander@purestorage.com>
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 <csander@purestorage.com>
---
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

View File

@ -1,156 +0,0 @@
From 9cf31d24f8b3911b870cb217c6715637459a376a Mon Sep 17 00:00:00 2001
From: Caleb Sander <csander@purestorage.com>
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 <csander@purestorage.com>
---
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

View File

@ -3,7 +3,7 @@
Name: nvme-cli
Version: 2.4
Release: 9%{?dist}
Release: 10%{?dist}
Summary: NVMe management command line interface
License: GPLv2+
@ -15,8 +15,6 @@ 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++
@ -43,8 +41,6 @@ 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}
@ -100,6 +96,9 @@ if [ $1 -eq 1 ] || [ $1 -eq 2 ]; then
fi
%changelog
* Mon Aug 21 2023 John Meneghini <jmeneghi@redhat.com> - 2.4-10
- JIRA: https://issues.redhat.com/browse/RHEL-1492
* Thu Aug 10 2023 John Meneghini <jmeneghi@redhat.com> - 2.4-9
- JIRA: https://issues.redhat.com/browse/RHEL-1147