The branch main has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=4b1aac931465f39c5c26bfa1d5539a428d340f20
commit 4b1aac931465f39c5c26bfa1d5539a428d340f20 Author: John Baldwin <j...@freebsd.org> AuthorDate: 2025-08-04 19:38:07 +0000 Commit: John Baldwin <j...@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) {