git: 4b1aac931465 - main - ctld: Convert struct pport and struct kports to C++ classes
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 04 Aug 2025 19:46:55 UTC
The branch main has been updated by jhb:
URL: https://cgit.FreeBSD.org/src/commit/?id=4b1aac931465f39c5c26bfa1d5539a428d340f20
commit 4b1aac931465f39c5c26bfa1d5539a428d340f20
Author: John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2025-08-04 19:38:07 +0000
Commit: John Baldwin <jhb@FreeBSD.org>
CommitDate: 2025-08-04 19:38:07 +0000
ctld: Convert struct pport and struct kports to C++ classes
- Use an unordered_map<> indexed by std::string to replace the TAILQ
of pport objects in struct kports since pport objects are looked up
name. Use a few wrapper methods around the unordered_map<> to
simplify consumers.
- Don't store a list of port pointers in pport. Only a single port is
ever associated (previously the code failed with an error if the
TAILQ wasn't empty when adding a port), so just store a pointer to a
single port and replace the empty TAILQ test with checking if the
pointer is null.
- Use std::string for the pport name.
- Add accessors (and a setter) for members of pport so that all the
fields can be private.
Sponsored by: Chelsio Communications
Pull Request: https://github.com/freebsd/freebsd-src/pull/1794
---
usr.sbin/ctld/ctld.cc | 80 +++++++++++++++++++------------------------------
usr.sbin/ctld/ctld.hh | 47 ++++++++++++++++-------------
usr.sbin/ctld/kernel.cc | 13 ++++----
3 files changed, 61 insertions(+), 79 deletions(-)
diff --git a/usr.sbin/ctld/ctld.cc b/usr.sbin/ctld/ctld.cc
index bb35d5ad68de..07592a07c019 100644
--- a/usr.sbin/ctld/ctld.cc
+++ b/usr.sbin/ctld/ctld.cc
@@ -801,53 +801,37 @@ isns_deregister(struct isns *isns)
set_timeout(0, false);
}
-struct pport *
-pport_new(struct kports *kports, const char *name, uint32_t ctl_port)
+pport::~pport()
{
- struct pport *pp;
-
- pp = reinterpret_cast<struct pport *>(calloc(1, sizeof(*pp)));
- if (pp == NULL)
- log_err(1, "calloc");
- pp->pp_kports = kports;
- pp->pp_name = checked_strdup(name);
- pp->pp_ctl_port = ctl_port;
- TAILQ_INIT(&pp->pp_ports);
- TAILQ_INSERT_TAIL(&kports->pports, pp, pp_next);
- return (pp);
+ if (pp_port != nullptr)
+ port_delete(pp_port);
}
-struct pport *
-pport_find(const struct kports *kports, const char *name)
+bool
+kports::add_port(const char *name, uint32_t ctl_port)
{
- struct pport *pp;
-
- TAILQ_FOREACH(pp, &kports->pports, pp_next) {
- if (strcasecmp(pp->pp_name, name) == 0)
- return (pp);
+ const auto &pair = pports.try_emplace(name, name, ctl_port);
+ if (!pair.second) {
+ log_warnx("duplicate kernel port \"%s\" (%u)", name, ctl_port);
+ return (false);
}
- return (NULL);
+
+ return (true);
}
-struct pport *
-pport_copy(struct pport *pp, struct kports *kports)
+bool
+kports::has_port(std::string_view name)
{
- struct pport *ppnew;
-
- ppnew = pport_new(kports, pp->pp_name, pp->pp_ctl_port);
- return (ppnew);
+ return (pports.count(std::string(name)) > 0);
}
-void
-pport_delete(struct pport *pp)
+struct pport *
+kports::find_port(std::string_view name)
{
- struct port *port, *tport;
-
- TAILQ_FOREACH_SAFE(port, &pp->pp_ports, p_ts, tport)
- port_delete(port);
- TAILQ_REMOVE(&pp->pp_kports->pports, pp, pp_next);
- free(pp->pp_name);
- free(pp);
+ auto it = pports.find(std::string(name));
+ if (it == pports.end())
+ return (nullptr);
+ return (&it->second);
}
struct port *
@@ -877,7 +861,7 @@ 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,
+port_new_ioctl(struct conf *conf, struct kports &kports, struct target *target,
int pp, int vp)
{
struct pport *pport;
@@ -892,7 +876,7 @@ port_new_ioctl(struct conf *conf, struct kports *kports, struct target *target,
return (NULL);
}
- pport = pport_find(kports, pname);
+ pport = kports.find_port(pname);
if (pport != NULL) {
free(pname);
return (port_new_pp(conf, target, pport));
@@ -927,7 +911,7 @@ port_new_pp(struct conf *conf, struct target *target, struct pport *pp)
char *name;
int ret;
- ret = asprintf(&name, "%s-%s", pp->pp_name, target->t_name);
+ ret = asprintf(&name, "%s-%s", pp->name(), target->t_name);
if (ret <= 0)
log_err(1, "asprintf");
if (port_find(conf, name) != NULL) {
@@ -941,8 +925,7 @@ port_new_pp(struct conf *conf, struct target *target, struct pport *pp)
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(&pp->pp_ports, port, p_pps);
- port->p_pport = pp;
+ pp->link(port);
return (port);
}
@@ -978,8 +961,6 @@ port_delete(struct port *port)
if (port->p_portal_group)
TAILQ_REMOVE(&port->p_portal_group->pg_ports, port, p_pgs);
- if (port->p_pport)
- TAILQ_REMOVE(&port->p_pport->pp_ports, port, p_pps);
if (port->p_target)
TAILQ_REMOVE(&port->p_target->t_ports, port, p_ts);
TAILQ_REMOVE(&port->p_conf->conf_ports, port, p_next);
@@ -2149,7 +2130,7 @@ conf_new_from_file(const char *path, bool ucl)
* with the config file. If necessary, create them.
*/
static bool
-new_pports_from_conf(struct conf *conf, struct kports *kports)
+new_pports_from_conf(struct conf *conf, struct kports &kports)
{
struct target *targ;
struct pport *pp;
@@ -2172,13 +2153,13 @@ new_pports_from_conf(struct conf *conf, struct kports *kports)
continue;
}
- pp = pport_find(kports, targ->t_pport);
+ pp = kports.find_port(targ->t_pport);
if (pp == NULL) {
log_warnx("unknown port \"%s\" for target \"%s\"",
targ->t_pport, targ->t_name);
return (false);
}
- if (!TAILQ_EMPTY(&pp->pp_ports)) {
+ if (pp->linked()) {
log_warnx("can't link port \"%s\" to target \"%s\", "
"port already linked to some target",
targ->t_pport, targ->t_name);
@@ -2241,7 +2222,6 @@ main(int argc, char **argv)
log_init(debug);
kernel_init();
- TAILQ_INIT(&kports.pports);
newconf = conf_new_from_file(config_path, use_ucl);
if (newconf == NULL)
@@ -2264,14 +2244,14 @@ main(int argc, char **argv)
register_signals();
- oldconf = conf_new_from_kernel(&kports);
+ oldconf = conf_new_from_kernel(kports);
if (debug > 0) {
oldconf->conf_debug = debug;
newconf->conf_debug = debug;
}
- if (!new_pports_from_conf(newconf, &kports))
+ if (!new_pports_from_conf(newconf, kports))
log_errx(1, "Error associating physical ports; exiting");
if (daemonize) {
@@ -2313,7 +2293,7 @@ main(int argc, char **argv)
if (tmpconf == NULL) {
log_warnx("configuration error, "
"continuing with old configuration");
- } else if (!new_pports_from_conf(tmpconf, &kports)) {
+ } else if (!new_pports_from_conf(tmpconf, kports)) {
log_warnx("Error associating physical ports, "
"continuing with old configuration");
conf_delete(tmpconf);
diff --git a/usr.sbin/ctld/ctld.hh b/usr.sbin/ctld/ctld.hh
index ea7f41f707b9..ee50acf1f3e8 100644
--- a/usr.sbin/ctld/ctld.hh
+++ b/usr.sbin/ctld/ctld.hh
@@ -177,20 +177,9 @@ struct portal_group {
uint16_t pg_tag;
};
-/* Ports created by the kernel. Perhaps the "p" means "physical" ? */
-struct pport {
- TAILQ_ENTRY(pport) pp_next;
- TAILQ_HEAD(, port) pp_ports;
- struct kports *pp_kports;
- char *pp_name;
-
- uint32_t pp_ctl_port;
-};
-
struct port {
TAILQ_ENTRY(port) p_next;
TAILQ_ENTRY(port) p_pgs;
- TAILQ_ENTRY(port) p_pps;
TAILQ_ENTRY(port) p_ts;
struct conf *p_conf;
char *p_name;
@@ -272,8 +261,31 @@ private:
};
/* Physical ports exposed by the kernel */
+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; }
+
+private:
+ struct port *pp_port;
+ std::string pp_name;
+
+ uint32_t pp_ctl_port;
+};
+
struct kports {
- TAILQ_HEAD(, pport) pports;
+ bool add_port(const char *name, uint32_t ctl_port);
+ bool has_port(std::string_view name);
+ struct pport *find_port(std::string_view name);
+
+private:
+ std::unordered_map<std::string, struct pport> pports;
};
#define CONN_SESSION_TYPE_NONE 0
@@ -305,7 +317,7 @@ bool parse_conf(const char *path);
bool uclparse_conf(const char *path);
struct conf *conf_new(void);
-struct conf *conf_new_from_kernel(struct kports *kports);
+struct conf *conf_new_from_kernel(struct kports &kports);
void conf_delete(struct conf *conf);
void conf_finish(void);
void conf_start(struct conf *new_conf);
@@ -329,17 +341,10 @@ void isns_register(struct isns *isns, struct isns *oldisns);
void isns_check(struct isns *isns);
void isns_deregister(struct isns *isns);
-struct pport *pport_new(struct kports *kports, const char *name,
- uint32_t ctl_port);
-struct pport *pport_find(const struct kports *kports,
- const char *name);
-struct pport *pport_copy(struct pport *pp, struct kports *kports);
-void pport_delete(struct pport *pport);
-
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,
+ struct kports &kports, struct target *target,
int pp, int vp);
struct port *port_new_pp(struct conf *conf, struct target *target,
struct pport *pp);
diff --git a/usr.sbin/ctld/kernel.cc b/usr.sbin/ctld/kernel.cc
index 5a9a9f3c991f..eb3bfa1dc760 100644
--- a/usr.sbin/ctld/kernel.cc
+++ b/usr.sbin/ctld/kernel.cc
@@ -396,12 +396,11 @@ cctl_char_handler(void *user_data, const XML_Char *str, int len)
}
struct conf *
-conf_new_from_kernel(struct kports *kports)
+conf_new_from_kernel(struct kports &kports)
{
struct conf *conf = NULL;
struct target *targ;
struct portal_group *pg;
- struct pport *pp;
struct port *cp;
struct lun *cl;
struct ctl_lun_list list;
@@ -542,11 +541,9 @@ retry_port:
if (port->cfiscsi_target == NULL) {
log_debugx("CTL port %u \"%s\" wasn't managed by ctld; ",
port->port_id, name);
- pp = pport_find(kports, name);
- if (pp == NULL) {
- pp = pport_new(kports, name, port->port_id);
- if (pp == NULL) {
- log_warnx("pport_new failed");
+ if (!kports.has_port(name)) {
+ if (!kports.add_port(name, port->port_id)) {
+ log_warnx("kports::add_port failed");
continue;
}
}
@@ -976,7 +973,7 @@ kernel_port_add(struct port *port)
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->pp_ctl_port;
+ port->p_ctl_port = port->p_pport->ctl_port();
if (strncmp(targ->t_name, "naa.", 4) == 0 &&
strlen(targ->t_name) == 20) {