ctld: Convert struct port to a hierarchy of C++ classes

The existing C struct port was used to describe three types of ports:
iSCSI ports associated with a portal_group, ioctl ports, and
"physical" ports associated with a kernel device.  This change chooses
to split these out into separate sub-classes of an abstract port base
class.  Virtual methods are used in a few places such as sending the
class-specific CTL ioctls for creating and removing CTL kernel ports.

For ownership purposes, a struct conf instance "owns" each port via a
std::unique_ptr<> in a std::unordered_map<> indexed by name.  Other
objects such as targets and portal_groups can also contain collections
of ports (targets hold a std::list of pointers, portal groups hold a
std::unordered_map<> indexed by target names).  One
not-so-straightforward case is that if a new port fails to register,
it is removed from the configuration.  In that case, these other
references also have to be removed explicitly.

Sponsored by:	Chelsio Communications
Pull Request:	https://github.com/freebsd/freebsd-src/pull/1794

usr.sbin/ctld/conf.cc

@@ -571,7 +571,6 @@ target_add_portal_group(const char *pg_name, const char *ag_name)
 {
 	struct portal_group *pg;
 	auth_group_sp ag;
-	struct port *p;
 
 	pg = portal_group_find(conf, pg_name);
 	if (pg == NULL) {
@@ -589,13 +588,11 @@ target_add_portal_group(const char *pg_name, const char *ag_name)
 		}
 	}
 
-	p = port_new(conf, target, pg);
-	if (p == NULL) {
+	if (!port_new(conf, target, pg, std::move(ag))) {
 		log_warnx("can't link portal-group \"%s\" to target \"%s\"",
 		    pg_name, target->t_name);
 		return (false);
 	}
-	p->p_auth_group = std::move(ag);
 	return (true);
 }
 

usr.sbin/ctld/ctld.cc

@@ -103,7 +103,6 @@ conf_new(void)
 	conf = new struct conf();
 	TAILQ_INIT(&conf->conf_luns);
 	TAILQ_INIT(&conf->conf_targets);
-	TAILQ_INIT(&conf->conf_ports);
 	TAILQ_INIT(&conf->conf_portal_groups);
 	TAILQ_INIT(&conf->conf_isns);
 
@@ -134,7 +133,6 @@ conf_delete(struct conf *conf)
 		portal_group_delete(pg);
 	TAILQ_FOREACH_SAFE(is, &conf->conf_isns, i_next, istmp)
 		isns_delete(is);
-	assert(TAILQ_EMPTY(&conf->conf_ports));
 	free(conf->conf_pidfile_path);
 	delete conf;
 }
@@ -455,7 +453,6 @@ portal_group_new(struct conf *conf, const char *name)
 	pg = new portal_group();
 	pg->pg_name = checked_strdup(name);
 	pg->pg_options = nvlist_create(0);
-	TAILQ_INIT(&pg->pg_ports);
 	pg->pg_conf = conf;
 	pg->pg_tag = 0;		/* Assigned later in conf_apply(). */
 	pg->pg_dscp = -1;
@@ -468,10 +465,6 @@ portal_group_new(struct conf *conf, const char *name)
 void
 portal_group_delete(struct portal_group *pg)
 {
-	struct port *port, *tport;
-
-	TAILQ_FOREACH_SAFE(port, &pg->pg_ports, p_pgs, tport)
-		port_delete(port);
 	TAILQ_REMOVE(&pg->pg_conf->conf_portal_groups, pg, pg_next);
 
 	nvlist_destroy(pg->pg_options);
@@ -625,7 +618,6 @@ isns_do_register(struct isns *isns, int s, const char *hostname)
 	struct conf *conf = isns->i_conf;
 	struct target *target;
 	struct portal_group *pg;
-	struct port *port;
 	uint32_t error;
 
 	isns_req req(ISNS_FUNC_DEVATTRREG, ISNS_FLAG_CLIENT);
@@ -647,8 +639,9 @@ isns_do_register(struct isns *isns, int s, const char *hostname)
 		req.add_32(33, 1); /* 1 -- Target*/
 		if (target->t_alias != NULL)
 			req.add_str(34, target->t_alias);
-		TAILQ_FOREACH(port, &target->t_ports, p_ts) {
-			if ((pg = port->p_portal_group) == NULL)
+		for (const port *port : target->t_ports) {
+			pg = port->portal_group();
+			if (pg == nullptr)
 				continue;
 			req.add_32(51, pg->pg_tag);
 			for (const portal_up &portal : pg->pg_portals) {
@@ -801,12 +794,6 @@ isns_deregister(struct isns *isns)
 	set_timeout(0, false);
 }
 
-pport::~pport()
-{
-	if (pp_port != nullptr)
-		port_delete(pp_port);
-}
-
 bool
 kports::add_port(const char *name, uint32_t ctl_port)
 {
@@ -834,151 +821,129 @@ kports::find_port(std::string_view name)
 	return (&it->second);
 }
 
-struct port *
-port_new(struct conf *conf, struct target *target, struct portal_group *pg)
+port::port(struct target *target) :
+	p_target(target)
 {
-	struct port *port;
-	char *name;
-	int ret;
-
-	ret = asprintf(&name, "%s-%s", pg->pg_name, target->t_name);
-	if (ret <= 0)
-		log_err(1, "asprintf");
-	if (port_find(conf, name) != NULL) {
-		log_warnx("duplicate port \"%s\"", name);
-		free(name);
-		return (NULL);
-	}
-	port = new struct port();
-	port->p_conf = conf;
-	port->p_name = name;
-	TAILQ_INSERT_TAIL(&conf->conf_ports, port, p_next);
-	TAILQ_INSERT_TAIL(&target->t_ports, port, p_ts);
-	port->p_target = target;
-	TAILQ_INSERT_TAIL(&pg->pg_ports, port, p_pgs);
-	port->p_portal_group = pg;
-	return (port);
+	target->t_ports.push_back(this);
 }
 
-struct port *
+void
+port::clear_references()
+{
+	p_target->t_ports.remove(this);
+}
+
+portal_group_port::portal_group_port(struct target *target,
+    struct portal_group *pg, auth_group_sp ag) :
+	port(target), p_auth_group(ag), p_portal_group(pg)
+{
+	pg->pg_ports.emplace(target->t_name, this);
+}
+
+portal_group_port::portal_group_port(struct target *target,
+    struct portal_group *pg, uint32_t ctl_port) :
+	port(target), p_portal_group(pg)
+{
+	p_ctl_port = ctl_port;
+	pg->pg_ports.emplace(target->t_name, this);
+}
+
+bool
+portal_group_port::is_dummy() const
+{
+	if (p_portal_group->pg_foreign)
+		return (true);
+	if (p_portal_group->pg_portals.empty())
+		return (true);
+	return (false);
+}
+
+void
+portal_group_port::clear_references()
+{
+	auto it = p_portal_group->pg_ports.find(p_target->t_name);
+	p_portal_group->pg_ports.erase(it);
+	port::clear_references();
+}
+
+bool
+port_new(struct conf *conf, struct target *target, struct portal_group *pg,
+    auth_group_sp ag)
+{
+	std::string name = freebsd::stringf("%s-%s", pg->pg_name,
+	    target->t_name);
+	const auto &pair = conf->conf_ports.try_emplace(name,
+	    std::make_unique<portal_group_port>(target, pg, ag));
+	if (!pair.second) {
+		log_warnx("duplicate port \"%s\"", name.c_str());
+		return (false);
+	}
+
+	return (true);
+}
+
+bool
+port_new(struct conf *conf, struct target *target, struct portal_group *pg,
+    uint32_t ctl_port)
+{
+	std::string name = freebsd::stringf("%s-%s", pg->pg_name,
+	    target->t_name);
+	const auto &pair = conf->conf_ports.try_emplace(name,
+	    std::make_unique<portal_group_port>(target, pg, ctl_port));
+	if (!pair.second) {
+		log_warnx("duplicate port \"%s\"", name.c_str());
+		return (false);
+	}
+
+	return (true);
+}
+
+static bool
+port_new_pp(struct conf *conf, struct target *target, struct pport *pp)
+{
+	std::string name = freebsd::stringf("%s-%s", pp->name(),
+	    target->t_name);
+	const auto &pair = conf->conf_ports.try_emplace(name,
+	    std::make_unique<kernel_port>(target, pp));
+	if (!pair.second) {
+		log_warnx("duplicate port \"%s\"", name.c_str());
+		return (false);
+	}
+
+	pp->link();
+	return (true);
+}
+
+static bool
 port_new_ioctl(struct conf *conf, struct kports &kports, struct target *target,
     int pp, int vp)
 {
 	struct pport *pport;
-	struct port *port;
-	char *pname;
-	char *name;
-	int ret;
 
-	ret = asprintf(&pname, "ioctl/%d/%d", pp, vp);
-	if (ret <= 0) {
-		log_err(1, "asprintf");
-		return (NULL);
-	}
+	std::string pname = freebsd::stringf("ioctl/%d/%d", pp, vp);
 
 	pport = kports.find_port(pname);
-	if (pport != NULL) {
-		free(pname);
+	if (pport != NULL)
 		return (port_new_pp(conf, target, pport));
+
+	std::string name = pname + "-" + target->t_name;
+	const auto &pair = conf->conf_ports.try_emplace(name,
+	    std::make_unique<ioctl_port>(target, pp, vp));
+	if (!pair.second) {
+		log_warnx("duplicate port \"%s\"", name.c_str());
+		return (false);
 	}
 
-	ret = asprintf(&name, "%s-%s", pname, target->t_name);
-	free(pname);
-
-	if (ret <= 0)
-		log_err(1, "asprintf");
-	if (port_find(conf, name) != NULL) {
-		log_warnx("duplicate port \"%s\"", name);
-		free(name);
-		return (NULL);
-	}
-	port = new struct port();
-	port->p_conf = conf;
-	port->p_name = name;
-	port->p_ioctl_port = true;
-	port->p_ioctl_pp = pp;
-	port->p_ioctl_vp = vp;
-	TAILQ_INSERT_TAIL(&conf->conf_ports, port, p_next);
-	TAILQ_INSERT_TAIL(&target->t_ports, port, p_ts);
-	port->p_target = target;
-	return (port);
-}
-
-struct port *
-port_new_pp(struct conf *conf, struct target *target, struct pport *pp)
-{
-	struct port *port;
-	char *name;
-	int ret;
-
-	ret = asprintf(&name, "%s-%s", pp->name(), target->t_name);
-	if (ret <= 0)
-		log_err(1, "asprintf");
-	if (port_find(conf, name) != NULL) {
-		log_warnx("duplicate port \"%s\"", name);
-		free(name);
-		return (NULL);
-	}
-	port = new struct port();
-	port->p_conf = conf;
-	port->p_name = name;
-	TAILQ_INSERT_TAIL(&conf->conf_ports, port, p_next);
-	TAILQ_INSERT_TAIL(&target->t_ports, port, p_ts);
-	port->p_target = target;
-	pp->link(port);
-	return (port);
-}
-
-struct port *
-port_find(const struct conf *conf, const char *name)
-{
-	struct port *port;
-
-	TAILQ_FOREACH(port, &conf->conf_ports, p_next) {
-		if (strcasecmp(port->p_name, name) == 0)
-			return (port);
-	}
-
-	return (NULL);
+	return (true);
 }
 
 struct port *
 port_find_in_pg(const struct portal_group *pg, const char *target)
 {
-	struct port *port;
-
-	TAILQ_FOREACH(port, &pg->pg_ports, p_pgs) {
-		if (strcasecmp(port->p_target->t_name, target) == 0)
-			return (port);
-	}
-
-	return (NULL);
-}
-
-void
-port_delete(struct port *port)
-{
-
-	if (port->p_portal_group)
-		TAILQ_REMOVE(&port->p_portal_group->pg_ports, port, p_pgs);
-	if (port->p_target)
-		TAILQ_REMOVE(&port->p_target->t_ports, port, p_ts);
-	TAILQ_REMOVE(&port->p_conf->conf_ports, port, p_next);
-	free(port->p_name);
-	free(port);
-}
-
-bool
-port_is_dummy(struct port *port)
-{
-
-	if (port->p_portal_group) {
-		if (port->p_portal_group->pg_foreign)
-			return (true);
-		if (port->p_portal_group->pg_portals.empty())
-			return (true);
-	}
-	return (false);
+	auto it = pg->pg_ports.find(target);
+	if (it == pg->pg_ports.end())
+		return (nullptr);
+	return (it->second);
 }
 
 struct target *
@@ -1006,7 +971,6 @@ target_new(struct conf *conf, const char *name)
 		targ->t_name[i] = tolower(targ->t_name[i]);
 
 	targ->t_conf = conf;
-	TAILQ_INIT(&targ->t_ports);
 	TAILQ_INSERT_TAIL(&conf->conf_targets, targ, t_next);
 
 	return (targ);
@@ -1015,10 +979,6 @@ target_new(struct conf *conf, const char *name)
 void
 target_delete(struct target *targ)
 {
-	struct port *port, *tport;
-
-	TAILQ_FOREACH_SAFE(port, &targ->t_ports, p_ts, tport)
-		port_delete(port);
 	TAILQ_REMOVE(&targ->t_conf->conf_targets, targ, t_next);
 
 	free(targ->t_pport);
@@ -1261,10 +1221,10 @@ conf_verify(struct conf *conf)
 			    "default");
 			assert(targ->t_auth_group != NULL);
 		}
-		if (TAILQ_EMPTY(&targ->t_ports)) {
+		if (targ->t_ports.empty()) {
 			pg = portal_group_find(conf, "default");
 			assert(pg != NULL);
-			port_new(conf, targ, pg);
+			port_new(conf, targ, pg, nullptr);
 		}
 		found = false;
 		for (i = 0; i < MAX_LUNS; i++) {
@@ -1293,14 +1253,14 @@ conf_verify(struct conf *conf)
 			pg->pg_discovery_filter = PG_FILTER_NONE;
 
 		if (pg->pg_redirection != NULL) {
-			if (!TAILQ_EMPTY(&pg->pg_ports)) {
+			if (!pg->pg_ports.empty()) {
 				log_debugx("portal-group \"%s\" assigned "
 				    "to target, but configured "
 				    "for redirection",
 				    pg->pg_name);
 			}
 			pg->pg_unassigned = false;
-		} else if (!TAILQ_EMPTY(&pg->pg_ports)) {
+		} else if (!pg->pg_ports.empty()) {
 			pg->pg_unassigned = false;
 		} else {
 			if (strcmp(pg->pg_name, "default") != 0)
@@ -1453,7 +1413,6 @@ conf_apply(struct conf *oldconf, struct conf *newconf)
 {
 	struct lun *oldlun, *newlun, *tmplun;
 	struct portal_group *oldpg, *newpg;
-	struct port *oldport, *newport, *tmpport;
 	struct isns *oldns, *newns;
 	int changed, cumulated_error = 0, error;
 
@@ -1517,17 +1476,19 @@ conf_apply(struct conf *oldconf, struct conf *newconf)
 	 * First, remove any ports present in the old configuration
 	 * and missing in the new one.
 	 */
-	TAILQ_FOREACH_SAFE(oldport, &oldconf->conf_ports, p_next, tmpport) {
-		if (port_is_dummy(oldport))
+	for (const auto &kv : oldconf->conf_ports) {
+		const std::string &name = kv.first;
+		port *oldport = kv.second.get();
+
+		if (oldport->is_dummy())
 			continue;
-		newport = port_find(newconf, oldport->p_name);
-		if (newport != NULL && !port_is_dummy(newport))
+		const auto it = newconf->conf_ports.find(name);
+		if (it != newconf->conf_ports.end() &&
+		    !it->second->is_dummy())
 			continue;
-		log_debugx("removing port \"%s\"", oldport->p_name);
-		error = kernel_port_remove(oldport);
-		if (error != 0) {
-			log_warnx("failed to remove port %s",
-			    oldport->p_name);
+		log_debugx("removing port \"%s\"", name.c_str());
+		if (!oldport->kernel_remove()) {
+			log_warnx("failed to remove port %s", name.c_str());
 			/*
 			 * XXX: Uncomment after fixing the root cause.
 			 *
@@ -1640,30 +1601,46 @@ conf_apply(struct conf *oldconf, struct conf *newconf)
 	/*
 	 * Now add new ports or modify existing ones.
 	 */
-	TAILQ_FOREACH_SAFE(newport, &newconf->conf_ports, p_next, tmpport) {
-		if (port_is_dummy(newport))
-			continue;
-		oldport = port_find(oldconf, newport->p_name);
+	for (auto it = newconf->conf_ports.begin();
+	     it != newconf->conf_ports.end(); ) {
+		const std::string &name = it->first;
+		port *newport = it->second.get();
 
-		if (oldport == NULL || port_is_dummy(oldport)) {
-			log_debugx("adding port \"%s\"", newport->p_name);
-			error = kernel_port_add(newport);
-		} else {
-			log_debugx("updating port \"%s\"", newport->p_name);
-			newport->p_ctl_port = oldport->p_ctl_port;
-			error = kernel_port_update(newport, oldport);
+		if (newport->is_dummy()) {
+			it++;
+			continue;
 		}
-		if (error != 0) {
-			log_warnx("failed to %s port %s",
-			    (oldport == NULL) ? "add" : "update",
-			    newport->p_name);
-			if (oldport == NULL || port_is_dummy(oldport))
-				port_delete(newport);
-			/*
-			 * XXX: Uncomment after fixing the root cause.
-			 *
-			 * cumulated_error++;
-			 */
+		const auto oldit = oldconf->conf_ports.find(name);
+		if (oldit == oldconf->conf_ports.end() ||
+		    oldit->second->is_dummy()) {
+			log_debugx("adding port \"%s\"", name.c_str());
+			if (!newport->kernel_add()) {
+				log_warnx("failed to add port %s",
+				    name.c_str());
+
+				/*
+				 * XXX: Uncomment after fixing the
+				 * root cause.
+				 *
+				 * cumulated_error++;
+				 */
+
+				/*
+				 * conf "owns" the port, but other
+				 * objects contain pointers to this
+				 * port that must be removed before
+				 * deleting the port.
+				 */
+				newport->clear_references();
+				it = newconf->conf_ports.erase(it);
+			} else
+				it++;
+		} else {
+			log_debugx("updating port \"%s\"", name.c_str());
+			if (!newport->kernel_update(oldit->second.get()))
+				log_warnx("failed to update port %s",
+				    name.c_str());
+			it++;
 		}
 	}
 
@@ -2134,7 +2111,6 @@ new_pports_from_conf(struct conf *conf, struct kports &kports)
 {
 	struct target *targ;
 	struct pport *pp;
-	struct port *tp;
 	int ret, i_pp, i_vp;
 
 	TAILQ_FOREACH(targ, &conf->conf_targets, t_next) {
@@ -2143,8 +2119,7 @@ new_pports_from_conf(struct conf *conf, struct kports &kports)
 
 		ret = sscanf(targ->t_pport, "ioctl/%d/%d", &i_pp, &i_vp);
 		if (ret > 0) {
-			tp = port_new_ioctl(conf, kports, targ, i_pp, i_vp);
-			if (tp == NULL) {
+			if (!port_new_ioctl(conf, kports, targ, i_pp, i_vp)) {
 				log_warnx("can't create new ioctl port "
 				    "for target \"%s\"", targ->t_name);
 				return (false);
@@ -2165,8 +2140,7 @@ new_pports_from_conf(struct conf *conf, struct kports &kports)
 			    targ->t_pport, targ->t_name);
 			return (false);
 		}
-		tp = port_new_pp(conf, targ, pp);
-		if (tp == NULL) {
+		if (!port_new_pp(conf, targ, pp)) {
 			log_warnx("can't link port \"%s\" to target \"%s\"",
 			    targ->t_pport, targ->t_name);
 			return (false);

usr.sbin/ctld/ctld.hh

@@ -57,6 +57,8 @@
 #define	MAX_LUNS			1024
 #define	SOCKBUF_SIZE			1048576
 
+struct port;
+
 struct auth {
 	auth(std::string_view secret) : a_secret(secret) {}
 	auth(std::string_view secret, std::string_view mutual_user,
@@ -168,7 +170,7 @@ struct portal_group {
 	bool				pg_foreign = false;
 	bool				pg_unassigned = false;
 	std::list<portal_up>	        pg_portals;
-	TAILQ_HEAD(, port)		pg_ports;
+	std::unordered_map<std::string, port *> pg_ports;
 	char				*pg_offload = nullptr;
 	char				*pg_redirection = nullptr;
 	int				pg_dscp;
@@ -178,22 +180,79 @@ struct portal_group {
 };
 
 struct port {
-	TAILQ_ENTRY(port)		p_next;
-	TAILQ_ENTRY(port)		p_pgs;
-	TAILQ_ENTRY(port)		p_ts;
-	struct conf			*p_conf;
-	char				*p_name;
-	auth_group_sp			p_auth_group;
-	struct portal_group		*p_portal_group = nullptr;
-	struct pport			*p_pport = nullptr;
+	port(struct target *target);
+	virtual ~port() = default;
+
+	struct target *target() const { return p_target; }
+	virtual struct auth_group *auth_group() const { return nullptr; }
+	virtual struct portal_group *portal_group() const { return nullptr; }
+
+	virtual bool is_dummy() const { return true; }
+
+	virtual void clear_references();
+
+	bool kernel_add();
+	bool kernel_update(const port *oport);
+	bool kernel_remove();
+
+	virtual bool kernel_create_port() = 0;
+	virtual bool kernel_remove_port() = 0;
+
+protected:
 	struct target			*p_target;
 
-	bool				p_ioctl_port = false;
-	int				p_ioctl_pp = 0;
-	int				p_ioctl_vp = 0;
 	uint32_t			p_ctl_port = 0;
 };
 
+struct portal_group_port final : public port {
+	portal_group_port(struct target *target, struct portal_group *pg,
+	    auth_group_sp ag);
+	portal_group_port(struct target *target, struct portal_group *pg,
+	    uint32_t ctl_port);
+	~portal_group_port() override = default;
+
+	struct auth_group *auth_group() const override
+	{ return p_auth_group.get(); }
+	struct portal_group *portal_group() const override
+	{ return p_portal_group; }
+
+	bool is_dummy() const override;
+
+	void clear_references() override;
+
+	bool kernel_create_port() override;
+	bool kernel_remove_port() override;
+
+private:
+	auth_group_sp			p_auth_group;
+	struct portal_group		*p_portal_group;
+};
+
+struct ioctl_port final : public port {
+	ioctl_port(struct target *target, int pp, int vp) :
+		port(target), p_ioctl_pp(pp), p_ioctl_vp(vp) {}
+	~ioctl_port() override = default;
+
+	bool kernel_create_port() override;
+	bool kernel_remove_port() override;
+
+private:
+	int				p_ioctl_pp;
+	int				p_ioctl_vp;
+};
+
+struct kernel_port final : public port {
+	kernel_port(struct target *target, struct pport *pp) :
+		port(target), p_pport(pp) {}
+	~kernel_port() override = default;
+
+	bool kernel_create_port() override;
+	bool kernel_remove_port() override;
+
+private:
+	struct pport			*p_pport;
+};
+
 struct lun {
 	TAILQ_ENTRY(lun)		l_next;
 	struct conf			*l_conf;
@@ -216,7 +275,7 @@ struct target {
 	struct conf			*t_conf;
 	struct lun			*t_luns[MAX_LUNS] = {};
 	auth_group_sp			t_auth_group;
-	TAILQ_HEAD(, port)		t_ports;
+	std::list<port *>		t_ports;
 	char				*t_name;
 	char				*t_alias;
 	char				*t_redirection;
@@ -237,7 +296,7 @@ struct conf {
 	TAILQ_HEAD(, lun)		conf_luns;
 	TAILQ_HEAD(, target)		conf_targets;
 	std::unordered_map<std::string, auth_group_sp> conf_auth_groups;
-	TAILQ_HEAD(, port)		conf_ports;
+	std::unordered_map<std::string, std::unique_ptr<port>> conf_ports;
 	TAILQ_HEAD(, portal_group)	conf_portal_groups;
 	TAILQ_HEAD(, isns)		conf_isns;
 	int				conf_isns_period;
@@ -264,19 +323,17 @@ private:
 struct pport {
 	pport(std::string_view name, uint32_t ctl_port) : pp_name(name),
 	    pp_ctl_port(ctl_port) {}
-	~pport();
 
 	const char *name() const { return pp_name.c_str(); }
 	uint32_t ctl_port() const { return pp_ctl_port; }
 
-	bool linked() const { return pp_port != nullptr; }
-	void link(struct port *port) { pp_port = port; }
+	bool linked() const { return pp_linked; }
+	void link() { pp_linked = true; }
 
 private:
-	struct port			*pp_port;
 	std::string			pp_name;
-
 	uint32_t			pp_ctl_port;
+	bool				pp_linked;
 };
 
 struct kports {
@@ -341,18 +398,12 @@ void			isns_register(struct isns *isns, struct isns *oldisns);
 void			isns_check(struct isns *isns);
 void			isns_deregister(struct isns *isns);
 
-struct port		*port_new(struct conf *conf, struct target *target,
-			    struct portal_group *pg);
-struct port		*port_new_ioctl(struct conf *conf,
-			    struct kports &kports, struct target *target,
-			    int pp, int vp);
-struct port		*port_new_pp(struct conf *conf, struct target *target,
-			    struct pport *pp);
-struct port		*port_find(const struct conf *conf, const char *name);
+bool			port_new(struct conf *conf, struct target *target,
+			    struct portal_group *pg, auth_group_sp ag);
+bool			port_new(struct conf *conf, struct target *target,
+			    struct portal_group *pg, uint32_t ctl_port);
 struct port		*port_find_in_pg(const struct portal_group *pg,
 			    const char *target);
-void			port_delete(struct port *port);
-bool			port_is_dummy(struct port *port);
 
 struct target		*target_new(struct conf *conf, const char *name);
 void			target_delete(struct target *target);
@@ -372,9 +423,6 @@ int			kernel_lun_add(struct lun *lun);
 int			kernel_lun_modify(struct lun *lun);
 int			kernel_lun_remove(struct lun *lun);
 void			kernel_handoff(struct ctld_connection *conn);
-int			kernel_port_add(struct port *port);
-int			kernel_port_update(struct port *port, struct port *old);
-int			kernel_port_remove(struct port *port);
 void			kernel_capsicate(void);
 
 #ifdef ICL_KERNEL_PROXY

usr.sbin/ctld/discovery.cc

@@ -101,17 +101,17 @@ logout_new_response(struct pdu *request)
 static void
 discovery_add_target(struct keys *response_keys, const struct target *targ)
 {
-	struct port *port;
 	char *buf;
 	char hbuf[NI_MAXHOST], sbuf[NI_MAXSERV];
 	const struct addrinfo *ai;
 	int ret;
 
 	keys_add(response_keys, "TargetName", targ->t_name);
-	TAILQ_FOREACH(port, &targ->t_ports, p_ts) {
-	    if (port->p_portal_group == NULL)
+	for (const port *port : targ->t_ports) {
+	    const struct portal_group *pg = port->portal_group();
+	    if (pg == nullptr)
 		continue;
-	    for (portal_up &portal : port->p_portal_group->pg_portals) {
+	    for (const portal_up &portal : pg->pg_portals) {
 		ai = portal->ai();
 		ret = getnameinfo(ai->ai_addr, ai->ai_addrlen,
 		    hbuf, sizeof(hbuf), sbuf, sizeof(sbuf),
@@ -125,13 +125,13 @@ discovery_add_target(struct keys *response_keys, const struct target *targ)
 			if (strcmp(hbuf, "0.0.0.0") == 0)
 				continue;
 			ret = asprintf(&buf, "%s:%s,%d", hbuf, sbuf,
-			    port->p_portal_group->pg_tag);
+			    pg->pg_tag);
 			break;
 		case AF_INET6:
 			if (strcmp(hbuf, "::") == 0)
 				continue;
 			ret = asprintf(&buf, "[%s]:%s,%d", hbuf, sbuf,
-			    port->p_portal_group->pg_tag);
+			    pg->pg_tag);
 			break;
 		default:
 			continue;
@@ -154,8 +154,8 @@ discovery_target_filtered_out(const struct ctld_connection *conn,
 	const struct auth *auth;
 	int error;
 
-	targ = port->p_target;
-	ag = port->p_auth_group.get();
+	targ = port->target();
+	ag = port->auth_group();
 	if (ag == nullptr)
 		ag = targ->t_auth_group.get();
 	pg = conn->conn_portal->portal_group();
@@ -228,12 +228,13 @@ discovery(struct ctld_connection *conn)
 	response_keys = keys_new();
 
 	if (strcmp(send_targets, "All") == 0) {
-		TAILQ_FOREACH(port, &pg->pg_ports, p_pgs) {
+		for (const auto &kv : pg->pg_ports) {
+			port = kv.second;
 			if (discovery_target_filtered_out(conn, port)) {
 				/* Ignore this target. */
 				continue;
 			}
-			discovery_add_target(response_keys, port->p_target);
+			discovery_add_target(response_keys, port->target());
 		}
 	} else {
 		port = port_find_in_pg(pg, send_targets);
@@ -244,7 +245,8 @@ discovery(struct ctld_connection *conn)
 			if (discovery_target_filtered_out(conn, port)) {
 				/* Ignore this target. */
 			} else {
-				discovery_add_target(response_keys, port->p_target);
+				discovery_add_target(response_keys,
+				    port->target());
 			}
 		}
 	}

usr.sbin/ctld/kernel.cc

@@ -401,7 +401,6 @@ conf_new_from_kernel(struct kports &kports)
 	struct conf *conf = NULL;
 	struct target *targ;
 	struct portal_group *pg;
-	struct port *cp;
 	struct lun *cl;
 	struct ctl_lun_list list;
 	struct cctl_devlist_data devlist;
@@ -575,12 +574,10 @@ retry_port:
 			}
 		}
 		pg->pg_tag = port->cfiscsi_portal_group_tag;
-		cp = port_new(conf, targ, pg);
-		if (cp == NULL) {
+		if (!port_new(conf, targ, pg, port->port_id)) {
 			log_warnx("port_new failed");
 			continue;
 		}
-		cp->p_ctl_port = port->port_id;
 	}
 	while ((port = STAILQ_FIRST(&devlist.port_list))) {
 		STAILQ_REMOVE_HEAD(&devlist.port_list, links);
@@ -895,100 +892,118 @@ kernel_handoff(struct ctld_connection *conn)
 	}
 }
 
-int
-kernel_port_add(struct port *port)
+static bool
+ctl_create_port(const char *driver, const nvlist_t *nvl, uint32_t *ctl_port)
 {
-	struct ctl_port_entry entry;
 	struct ctl_req req;
-	struct ctl_lun_map lm;
-	struct target *targ = port->p_target;
-	struct portal_group *pg = port->p_portal_group;
 	char result_buf[NVLIST_BUFSIZE];
-	int error, i;
+	int error;
 
-	/* Create iSCSI port. */
-	if (port->p_portal_group || port->p_ioctl_port) {
-		bzero(&req, sizeof(req));
-		req.reqtype = CTL_REQ_CREATE;
+	bzero(&req, sizeof(req));
+	req.reqtype = CTL_REQ_CREATE;
 
-		if (port->p_portal_group) {
-			strlcpy(req.driver, "iscsi", sizeof(req.driver));
-			req.args_nvl = nvlist_clone(pg->pg_options);
-			nvlist_add_string(req.args_nvl, "cfiscsi_target",
-			    targ->t_name);
-			nvlist_add_string(req.args_nvl,
-			    "ctld_portal_group_name", pg->pg_name);
-			nvlist_add_stringf(req.args_nvl,
-			    "cfiscsi_portal_group_tag", "%u", pg->pg_tag);
-
-			if (targ->t_alias) {
-				nvlist_add_string(req.args_nvl,
-				    "cfiscsi_target_alias", targ->t_alias);
-			}
-		}
-
-		if (port->p_ioctl_port) {
-			strlcpy(req.driver, "ioctl", sizeof(req.driver));
-			req.args_nvl = nvlist_create(0);
-			nvlist_add_stringf(req.args_nvl, "pp", "%d",
-			    port->p_ioctl_pp);
-			nvlist_add_stringf(req.args_nvl, "vp", "%d",
-			    port->p_ioctl_vp);
-		}
-
-		req.args = nvlist_pack(req.args_nvl, &req.args_len);
-		if (req.args == NULL) {
-			nvlist_destroy(req.args_nvl);
-			log_warn("error packing nvlist");
-			return (1);
-		}
-
-		req.result = result_buf;
-		req.result_len = sizeof(result_buf);
-		error = ioctl(ctl_fd, CTL_PORT_REQ, &req);
-		free(req.args);
-		nvlist_destroy(req.args_nvl);
-
-		if (error != 0) {
-			log_warn("error issuing CTL_PORT_REQ ioctl");
-			return (1);
-		}
-		if (req.status == CTL_LUN_ERROR) {
-			log_warnx("error returned from port creation request: %s",
-			    req.error_str);
-			return (1);
-		}
-		if (req.status != CTL_LUN_OK) {
-			log_warnx("unknown port creation request status %d",
-			    req.status);
-			return (1);
-		}
-
-		req.result_nvl = nvlist_unpack(result_buf, req.result_len, 0);
-		if (req.result_nvl == NULL) {
-			log_warnx("error unpacking result nvlist");
-			return (1);
-		}
-
-		port->p_ctl_port = nvlist_get_number(req.result_nvl, "port_id");
-		nvlist_destroy(req.result_nvl);
-	} else if (port->p_pport) {
-		port->p_ctl_port = port->p_pport->ctl_port();
-
-		if (strncmp(targ->t_name, "naa.", 4) == 0 &&
-		    strlen(targ->t_name) == 20) {
-			bzero(&entry, sizeof(entry));
-			entry.port_type = CTL_PORT_NONE;
-			entry.targ_port = port->p_ctl_port;
-			entry.flags |= CTL_PORT_WWNN_VALID;
-			entry.wwnn = strtoull(targ->t_name + 4, NULL, 16);
-			if (ioctl(ctl_fd, CTL_SET_PORT_WWNS, &entry) == -1)
-				log_warn("CTL_SET_PORT_WWNS ioctl failed");
-		}
+	strlcpy(req.driver, driver, sizeof(req.driver));
+	req.args = nvlist_pack(nvl, &req.args_len);
+	if (req.args == NULL) {
+		log_warn("error packing nvlist");
+		return (false);
 	}
 
+	req.result = result_buf;
+	req.result_len = sizeof(result_buf);
+	error = ioctl(ctl_fd, CTL_PORT_REQ, &req);
+	free(req.args);
+
+	if (error != 0) {
+		log_warn("error issuing CTL_PORT_REQ ioctl");
+		return (false);
+	}
+	if (req.status == CTL_LUN_ERROR) {
+		log_warnx("error returned from port creation request: %s",
+		    req.error_str);
+		return (false);
+	}
+	if (req.status != CTL_LUN_OK) {
+		log_warnx("unknown port creation request status %d",
+		    req.status);
+		return (false);
+	}
+
+	freebsd::nvlist_up result_nvl(nvlist_unpack(result_buf, req.result_len,
+	    0));
+	if (result_nvl == NULL) {
+		log_warnx("error unpacking result nvlist");
+		return (false);
+	}
+
+	*ctl_port = nvlist_get_number(result_nvl.get(), "port_id");
+	return (true);
+}
+
+bool
+portal_group_port::kernel_create_port()
+{
+	struct portal_group *pg = p_portal_group;
+	struct target *targ = p_target;
+
+	freebsd::nvlist_up nvl(nvlist_clone(pg->pg_options));
+	nvlist_add_string(nvl.get(), "cfiscsi_target", targ->t_name);
+	nvlist_add_string(nvl.get(), "ctld_portal_group_name", pg->pg_name);
+	nvlist_add_stringf(nvl.get(), "cfiscsi_portal_group_tag", "%u",
+	    pg->pg_tag);
+
+	if (targ->t_alias) {
+		nvlist_add_string(nvl.get(), "cfiscsi_target_alias",
+		    targ->t_alias);
+	}
+
+	return (ctl_create_port("iscsi", nvl.get(), &p_ctl_port));
+}
+
+bool
+ioctl_port::kernel_create_port()
+{
+	freebsd::nvlist_up nvl(nvlist_create(0));
+	nvlist_add_stringf(nvl.get(), "pp", "%d", p_ioctl_pp);
+	nvlist_add_stringf(nvl.get(), "vp", "%d", p_ioctl_vp);
+
+	return (ctl_create_port("ioctl", nvl.get(), &p_ctl_port));
+}
+
+bool
+kernel_port::kernel_create_port()
+{
+	struct ctl_port_entry entry;
+	struct target *targ = p_target;
+
+	p_ctl_port = p_pport->ctl_port();
+
+	if (strncmp(targ->t_name, "naa.", 4) == 0 &&
+	    strlen(targ->t_name) == 20) {
+		bzero(&entry, sizeof(entry));
+		entry.port_type = CTL_PORT_NONE;
+		entry.targ_port = p_ctl_port;
+		entry.flags |= CTL_PORT_WWNN_VALID;
+		entry.wwnn = strtoull(targ->t_name + 4, NULL, 16);
+		if (ioctl(ctl_fd, CTL_SET_PORT_WWNS, &entry) == -1)
+			log_warn("CTL_SET_PORT_WWNS ioctl failed");
+	}
+	return (true);
+}
+
+bool
+port::kernel_add()
+{
+	struct ctl_port_entry entry;
+	struct ctl_lun_map lm;
+	struct target *targ = p_target;
+	int error, i;
+
+	if (!kernel_create_port())
+		return (false);
+
 	/* Explicitly enable mapping to block any access except allowed. */
-	lm.port = port->p_ctl_port;
+	lm.port = p_ctl_port;
 	lm.plun = UINT32_MAX;
 	lm.lun = 0;
 	error = ioctl(ctl_fd, CTL_LUN_MAP, &lm);
@@ -999,7 +1014,7 @@ kernel_port_add(struct port *port)
 	for (i = 0; i < MAX_LUNS; i++) {
 		if (targ->t_luns[i] == NULL)
 			continue;
-		lm.port = port->p_ctl_port;
+		lm.port = p_ctl_port;
 		lm.plun = i;
 		lm.lun = targ->t_luns[i]->l_ctl_lun;
 		error = ioctl(ctl_fd, CTL_LUN_MAP, &lm);
@@ -1009,28 +1024,30 @@ kernel_port_add(struct port *port)
 
 	/* Enable port */
 	bzero(&entry, sizeof(entry));
-	entry.targ_port = port->p_ctl_port;
+	entry.targ_port = p_ctl_port;
 	error = ioctl(ctl_fd, CTL_ENABLE_PORT, &entry);
 	if (error != 0) {
 		log_warn("CTL_ENABLE_PORT ioctl failed");
-		return (-1);
+		return (false);
 	}
 
-	return (0);
+	return (true);
 }
 
-int
-kernel_port_update(struct port *port, struct port *oport)
+bool
+port::kernel_update(const struct port *oport)
 {
 	struct ctl_lun_map lm;
-	struct target *targ = port->p_target;
+	struct target *targ = p_target;
 	struct target *otarg = oport->p_target;
 	int error, i;
 	uint32_t olun;
 
+	p_ctl_port = oport->p_ctl_port;
+
 	/* Map configured LUNs and unmap others */
 	for (i = 0; i < MAX_LUNS; i++) {
-		lm.port = port->p_ctl_port;
+		lm.port = p_ctl_port;
 		lm.plun = i;
 		if (targ->t_luns[i] == NULL)
 			lm.lun = UINT32_MAX;
@@ -1046,83 +1063,94 @@ kernel_port_update(struct port *port, struct port *oport)
 		if (error != 0)
 			log_warn("CTL_LUN_MAP ioctl failed");
 	}
-	return (0);
+	return (true);
 }
 
-int
-kernel_port_remove(struct port *port)
+bool
+ctl_remove_port(const char *driver, nvlist_t *nvl)
+{
+	struct ctl_req req;
+	int error;
+
+	strlcpy(req.driver, driver, sizeof(req.driver));
+	req.reqtype = CTL_REQ_REMOVE;
+	req.args = nvlist_pack(nvl, &req.args_len);
+	if (req.args == NULL) {
+		log_warn("error packing nvlist");
+		return (false);
+	}
+
+	error = ioctl(ctl_fd, CTL_PORT_REQ, &req);
+	free(req.args);
+
+	if (error != 0) {
+		log_warn("error issuing CTL_PORT_REQ ioctl");
+		return (false);
+	}
+	if (req.status == CTL_LUN_ERROR) {
+		log_warnx("error returned from port removal request: %s",
+		    req.error_str);
+		return (false);
+	}
+	if (req.status != CTL_LUN_OK) {
+		log_warnx("unknown port removal request status %d", req.status);
+		return (false);
+	}
+	return (true);
+}
+
+bool
+portal_group_port::kernel_remove_port()
+{
+	freebsd::nvlist_up nvl(nvlist_create(0));
+	nvlist_add_string(nvl.get(), "cfiscsi_target", p_target->t_name);
+	nvlist_add_stringf(nvl.get(), "cfiscsi_portal_group_tag", "%u",
+	    p_portal_group->pg_tag);
+
+	return (ctl_remove_port("iscsi", nvl.get()));
+}
+
+bool
+ioctl_port::kernel_remove_port()
+{
+	freebsd::nvlist_up nvl(nvlist_create(0));
+	nvlist_add_stringf(nvl.get(), "port_id", "%d", p_ctl_port);
+
+	return (ctl_remove_port("ioctl", nvl.get()));
+}
+
+bool
+kernel_port::kernel_remove_port()
+{
+	struct ctl_lun_map lm;
+	int error;
+
+	/* Disable LUN mapping. */
+	lm.port = p_ctl_port;
+	lm.plun = UINT32_MAX;
+	lm.lun = UINT32_MAX;
+	error = ioctl(ctl_fd, CTL_LUN_MAP, &lm);
+	if (error != 0)
+		log_warn("CTL_LUN_MAP ioctl failed");
+	return (true);
+}
+
+bool
+port::kernel_remove()
 {
 	struct ctl_port_entry entry;
-	struct ctl_lun_map lm;
-	struct ctl_req req;
-	struct target *targ = port->p_target;
-	struct portal_group *pg = port->p_portal_group;
 	int error;
 
 	/* Disable port */
 	bzero(&entry, sizeof(entry));
-	entry.targ_port = port->p_ctl_port;
+	entry.targ_port = p_ctl_port;
 	error = ioctl(ctl_fd, CTL_DISABLE_PORT, &entry);
 	if (error != 0) {
 		log_warn("CTL_DISABLE_PORT ioctl failed");
-		return (-1);
+		return (false);
 	}
 
-	/* Remove iSCSI or ioctl port. */
-	if (port->p_portal_group || port->p_ioctl_port) {
-		bzero(&req, sizeof(req));
-		strlcpy(req.driver, port->p_ioctl_port ? "ioctl" : "iscsi",
-		    sizeof(req.driver));
-		req.reqtype = CTL_REQ_REMOVE;
-		req.args_nvl = nvlist_create(0);
-		if (req.args_nvl == NULL)
-			log_err(1, "nvlist_create");
-
-		if (port->p_ioctl_port)
-			nvlist_add_stringf(req.args_nvl, "port_id", "%d",
-			    port->p_ctl_port);
-		else {
-			nvlist_add_string(req.args_nvl, "cfiscsi_target",
-			    targ->t_name);
-			nvlist_add_stringf(req.args_nvl,
-			    "cfiscsi_portal_group_tag", "%u", pg->pg_tag);
-		}
-
-		req.args = nvlist_pack(req.args_nvl, &req.args_len);
-		if (req.args == NULL) {
-			nvlist_destroy(req.args_nvl);
-			log_warn("error packing nvlist");
-			return (1);
-		}
-
-		error = ioctl(ctl_fd, CTL_PORT_REQ, &req);
-		free(req.args);
-		nvlist_destroy(req.args_nvl);
-
-		if (error != 0) {
-			log_warn("error issuing CTL_PORT_REQ ioctl");
-			return (1);
-		}
-		if (req.status == CTL_LUN_ERROR) {
-			log_warnx("error returned from port removal request: %s",
-			    req.error_str);
-			return (1);
-		}
-		if (req.status != CTL_LUN_OK) {
-			log_warnx("unknown port removal request status %d",
-			    req.status);
-			return (1);
-		}
-	} else {
-		/* Disable LUN mapping. */
-		lm.port = port->p_ctl_port;
-		lm.plun = UINT32_MAX;
-		lm.lun = UINT32_MAX;
-		error = ioctl(ctl_fd, CTL_LUN_MAP, &lm);
-		if (error != 0)
-			log_warn("CTL_LUN_MAP ioctl failed");
-	}
-	return (0);
+	return (kernel_remove_port());
 }
 
 #ifdef ICL_KERNEL_PROXY

usr.sbin/ctld/login.cc

@@ -981,17 +981,17 @@ login(struct ctld_connection *conn)
 			log_errx(1, "requested target \"%s\" not found",
 			    target_name);
 		}
-		conn->conn_target = conn->conn_port->p_target;
+		conn->conn_target = conn->conn_port->target();
 	}
 
 	/*
 	 * At this point we know what kind of authentication we need.
 	 */
 	if (conn->conn_session_type == CONN_SESSION_TYPE_NORMAL) {
-		ag = conn->conn_port->p_auth_group.get();
+		ag = conn->conn_port->auth_group();
 		if (ag == nullptr)
 			ag = conn->conn_target->t_auth_group.get();
-		if (conn->conn_port->p_auth_group == nullptr &&
+		if (conn->conn_port->auth_group() == nullptr &&
 		    conn->conn_target->t_private_auth) {
 			log_debugx("initiator requests to connect "
 			    "to target \"%s\"", conn->conn_target->t_name);