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) {

Reply via email to