git: 1435a9b293e2 - main - ctld: Defer initialization of NVMeoF associations

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Thu, 09 Oct 2025 19:28:57 UTC
The branch main has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=1435a9b293e21f8fca1f654420c5075ea7434e8f

commit 1435a9b293e21f8fca1f654420c5075ea7434e8f
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2025-10-09 19:24:27 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2025-10-09 19:24:27 +0000

    ctld: Defer initialization of NVMeoF associations
    
    Wait until all of the configuration has been parsed before creating
    associations for NVMe portals.  This ensures that any options
    specified in a transport group are honored when creating associations.
    
    To enable this, add a new virtual method portal::prepare invoked when
    applying a configuration prior to opening a socket (or reusing an
    existing socket) for a portal.
    
    Sponsored by:   Chelsio Communications
    Differential Revision:  https://reviews.freebsd.org/D52844
---
 usr.sbin/ctld/ctld.cc |  5 ++++
 usr.sbin/ctld/ctld.hh |  1 +
 usr.sbin/ctld/nvmf.cc | 77 +++++++++++++++++++++++----------------------------
 usr.sbin/ctld/nvmf.hh | 16 ++++-------
 4 files changed, 46 insertions(+), 53 deletions(-)

diff --git a/usr.sbin/ctld/ctld.cc b/usr.sbin/ctld/ctld.cc
index 10c12f25068e..331c029e282e 100644
--- a/usr.sbin/ctld/ctld.cc
+++ b/usr.sbin/ctld/ctld.cc
@@ -814,6 +814,11 @@ portal_group::open_sockets(struct conf &oldconf)
 	}
 
 	for (portal_up &portal : pg_portals) {
+		if (!portal->prepare()) {
+			cumulated_error++;
+			continue;
+		}
+
 		/*
 		 * Try to find already open portal and reuse the
 		 * listening socket.  We don't care about what portal
diff --git a/usr.sbin/ctld/ctld.hh b/usr.sbin/ctld/ctld.hh
index cc88e6eb590e..3bf18f6a32c0 100644
--- a/usr.sbin/ctld/ctld.hh
+++ b/usr.sbin/ctld/ctld.hh
@@ -151,6 +151,7 @@ struct portal {
 		p_protocol(protocol) {}
 	virtual ~portal() = default;
 
+	virtual bool prepare() { return true; }
 	bool reuse_socket(portal &oldp);
 	bool init_socket();
 	virtual bool init_socket_options(int s __unused) { return true; }
diff --git a/usr.sbin/ctld/nvmf.cc b/usr.sbin/ctld/nvmf.cc
index d1240bfa4f6c..eb116903f5c1 100644
--- a/usr.sbin/ctld/nvmf.cc
+++ b/usr.sbin/ctld/nvmf.cc
@@ -34,11 +34,8 @@
 
 struct nvmf_io_portal final : public nvmf_portal {
 	nvmf_io_portal(struct portal_group *pg, const char *listen,
-	    portal_protocol protocol, freebsd::addrinfo_up ai,
-	    const struct nvmf_association_params &aparams,
-	    nvmf_association_up na) :
-		nvmf_portal(pg, listen, protocol, std::move(ai), aparams,
-		    std::move(na)) {}
+	    portal_protocol protocol, freebsd::addrinfo_up ai) :
+		nvmf_portal(pg, listen, protocol, std::move(ai)) {}
 
 	void handle_connection(freebsd::fd_up fd, const char *host,
 	    const struct sockaddr *client_sa) override;
@@ -63,8 +60,6 @@ struct nvmf_transport_group final : public portal_group {
 	    override;
 
 private:
-	struct nvmf_association_params init_aparams(portal_protocol protocol);
-
 	static uint16_t last_port_id;
 };
 
@@ -143,48 +138,55 @@ parse_number(const nvlist_t *nvl, const char *key, uint64_t def, uint64_t minv,
 	return def;
 }
 
-struct nvmf_association_params
-nvmf_transport_group::init_aparams(portal_protocol protocol)
+bool
+nvmf_portal::prepare()
 {
-	struct nvmf_association_params params;
-	memset(&params, 0, sizeof(params));
+	memset(&p_aparams, 0, sizeof(p_aparams));
 
 	/* Options shared between discovery and I/O associations. */
-	const nvlist_t *nvl = pg_options.get();
-	params.tcp.header_digests = parse_bool(nvl, "HDGST", false);
-	params.tcp.data_digests = parse_bool(nvl, "DDGST", false);
-	uint64_t value = parse_number(nvl, "MAXH2CDATA", DEFAULT_MAXH2CDATA,
-	    4096, UINT32_MAX);
+	freebsd::nvlist_up nvl = portal_group()->options();
+	p_aparams.tcp.header_digests = parse_bool(nvl.get(), "HDGST", false);
+	p_aparams.tcp.data_digests = parse_bool(nvl.get(), "DDGST", false);
+	uint64_t value = parse_number(nvl.get(), "MAXH2CDATA",
+	    DEFAULT_MAXH2CDATA, 4096, UINT32_MAX);
 	if (value % 4 != 0) {
 		log_warnx("Invalid value \"%ju\" for option MAXH2CDATA",
 		    (uintmax_t)value);
 		value = DEFAULT_MAXH2CDATA;
 	}
-	params.tcp.maxh2cdata = value;
+	p_aparams.tcp.maxh2cdata = value;
 
-	switch (protocol) {
+	switch (protocol()) {
 	case portal_protocol::NVME_TCP:
-		params.sq_flow_control = parse_bool(nvl, "SQFC", false);
-		params.dynamic_controller_model = true;
-		params.max_admin_qsize = parse_number(nvl, "max_admin_qsize",
-		    NVME_MAX_ADMIN_ENTRIES, NVME_MIN_ADMIN_ENTRIES,
-		    NVME_MAX_ADMIN_ENTRIES);
-		params.max_io_qsize = parse_number(nvl, "max_io_qsize",
+		p_aparams.sq_flow_control = parse_bool(nvl.get(), "SQFC",
+		    false);
+		p_aparams.dynamic_controller_model = true;
+		p_aparams.max_admin_qsize = parse_number(nvl.get(),
+		    "max_admin_qsize", NVME_MAX_ADMIN_ENTRIES,
+		    NVME_MIN_ADMIN_ENTRIES, NVME_MAX_ADMIN_ENTRIES);
+		p_aparams.max_io_qsize = parse_number(nvl.get(), "max_io_qsize",
 		    NVME_MAX_IO_ENTRIES, NVME_MIN_IO_ENTRIES,
 		    NVME_MAX_IO_ENTRIES);
-		params.tcp.pda = 0;
+		p_aparams.tcp.pda = 0;
 		break;
 	case portal_protocol::NVME_DISCOVERY_TCP:
-		params.sq_flow_control = false;
-		params.dynamic_controller_model = true;
-		params.max_admin_qsize = NVME_MAX_ADMIN_ENTRIES;
-		params.tcp.pda = 0;
+		p_aparams.sq_flow_control = false;
+		p_aparams.dynamic_controller_model = true;
+		p_aparams.max_admin_qsize = NVME_MAX_ADMIN_ENTRIES;
+		p_aparams.tcp.pda = 0;
 		break;
 	default:
 		__assert_unreachable();
 	}
 
-	return params;
+	p_association.reset(nvmf_allocate_association(NVMF_TRTYPE_TCP, true,
+	    &p_aparams));
+	if (!p_association) {
+		log_warn("Failed to create NVMe controller association");
+		return false;
+	}
+
+	return true;
 }
 
 portal_group_up
@@ -209,15 +211,12 @@ bool
 nvmf_transport_group::add_portal(const char *value, portal_protocol protocol)
 {
 	freebsd::addrinfo_up ai;
-	enum nvmf_trtype trtype;
 
 	switch (protocol) {
 	case portal_protocol::NVME_TCP:
-		trtype = NVMF_TRTYPE_TCP;
 		ai = parse_addr_port(value, "4420");
 		break;
 	case portal_protocol::NVME_DISCOVERY_TCP:
-		trtype = NVMF_TRTYPE_TCP;
 		ai = parse_addr_port(value, "8009");
 		break;
 	default:
@@ -230,14 +229,6 @@ nvmf_transport_group::add_portal(const char *value, portal_protocol protocol)
 		return false;
 	}
 
-	struct nvmf_association_params aparams = init_aparams(protocol);
-	nvmf_association_up association(nvmf_allocate_association(trtype, true,
-	    &aparams));
-	if (!association) {
-		log_warn("Failed to create NVMe controller association");
-		return false;
-	}
-
 	/*
 	 * XXX: getaddrinfo(3) may return multiple addresses; we should turn
 	 *	those into multiple portals.
@@ -246,10 +237,10 @@ nvmf_transport_group::add_portal(const char *value, portal_protocol protocol)
 	portal_up portal;
 	if (protocol == portal_protocol::NVME_DISCOVERY_TCP) {
 		portal = std::make_unique<nvmf_discovery_portal>(this, value,
-		    protocol, std::move(ai), aparams, std::move(association));
+		    protocol, std::move(ai));
 	} else {
 		portal = std::make_unique<nvmf_io_portal>(this, value,
-		    protocol, std::move(ai), aparams, std::move(association));
+		    protocol, std::move(ai));
 		need_tcp_transport = true;
 	}
 
diff --git a/usr.sbin/ctld/nvmf.hh b/usr.sbin/ctld/nvmf.hh
index 0b4f8d45adfd..6f34a2858ef9 100644
--- a/usr.sbin/ctld/nvmf.hh
+++ b/usr.sbin/ctld/nvmf.hh
@@ -38,13 +38,12 @@ using nvmf_qpair_up = std::unique_ptr<nvmf_qpair, nvmf_qpair_deleter>;
 
 struct nvmf_portal : public portal {
 	nvmf_portal(struct portal_group *pg, const char *listen,
-	    portal_protocol protocol, freebsd::addrinfo_up ai,
-	    const struct nvmf_association_params &aparams,
-	    nvmf_association_up na) :
-		portal(pg, listen, protocol, std::move(ai)),
-		p_aparams(aparams), p_association(std::move(na)) {}
+	    portal_protocol protocol, freebsd::addrinfo_up ai) :
+		portal(pg, listen, protocol, std::move(ai)) {}
 	virtual ~nvmf_portal() override = default;
 
+	virtual bool prepare() override;
+
 	const struct nvmf_association_params *aparams() const
 	{ return &p_aparams; }
 
@@ -58,11 +57,8 @@ private:
 
 struct nvmf_discovery_portal final : public nvmf_portal {
 	nvmf_discovery_portal(struct portal_group *pg, const char *listen,
-	    portal_protocol protocol, freebsd::addrinfo_up ai,
-	    const struct nvmf_association_params &aparams,
-	    nvmf_association_up na) :
-		nvmf_portal(pg, listen, protocol, std::move(ai), aparams,
-		    std::move(na)) {}
+	    portal_protocol protocol, freebsd::addrinfo_up ai) :
+		nvmf_portal(pg, listen, protocol, std::move(ai)) {}
 
 	void handle_connection(freebsd::fd_up fd, const char *host,
 	    const struct sockaddr *client_sa) override;