The branch main has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=1ee9931d6e6f0f9fc4151efcac225d742de9e6fa
commit 1ee9931d6e6f0f9fc4151efcac225d742de9e6fa 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 isns_req to a C++ class Use a std::vector<> of chars to hold the iSNS packet. Convert the various isns_req_* functions to be class methods instead. Sponsored by: Chelsio Communications Pull Request: https://github.com/freebsd/freebsd-src/pull/1794 --- usr.sbin/ctld/ctld.cc | 113 +++++++++++++------------------- usr.sbin/ctld/isns.cc | 176 +++++++++++++++++++------------------------------- usr.sbin/ctld/isns.hh | 36 ++++++----- 3 files changed, 130 insertions(+), 195 deletions(-) diff --git a/usr.sbin/ctld/ctld.cc b/usr.sbin/ctld/ctld.cc index bf700d5b4051..2c385baa8c5a 100644 --- a/usr.sbin/ctld/ctld.cc +++ b/usr.sbin/ctld/ctld.cc @@ -637,7 +637,7 @@ isns_do_connect(struct isns *isns) return(s); } -static int +static void isns_do_register(struct isns *isns, int s, const char *hostname) { struct conf *conf = isns->i_conf; @@ -645,122 +645,100 @@ isns_do_register(struct isns *isns, int s, const char *hostname) struct portal *portal; struct portal_group *pg; struct port *port; - struct isns_req *req; - int res = 0; uint32_t error; - req = isns_req_create(ISNS_FUNC_DEVATTRREG, ISNS_FLAG_CLIENT); - isns_req_add_str(req, 32, TAILQ_FIRST(&conf->conf_targets)->t_name); - isns_req_add_delim(req); - isns_req_add_str(req, 1, hostname); - isns_req_add_32(req, 2, 2); /* 2 -- iSCSI */ - isns_req_add_32(req, 6, conf->conf_isns_period); + isns_req req(ISNS_FUNC_DEVATTRREG, ISNS_FLAG_CLIENT); + req.add_str(32, TAILQ_FIRST(&conf->conf_targets)->t_name); + req.add_delim(); + req.add_str(1, hostname); + req.add_32(2, 2); /* 2 -- iSCSI */ + req.add_32(6, conf->conf_isns_period); TAILQ_FOREACH(pg, &conf->conf_portal_groups, pg_next) { if (pg->pg_unassigned) continue; TAILQ_FOREACH(portal, &pg->pg_portals, p_next) { - isns_req_add_addr(req, 16, portal->p_ai); - isns_req_add_port(req, 17, portal->p_ai); + req.add_addr(16, portal->p_ai); + req.add_port(17, portal->p_ai); } } TAILQ_FOREACH(target, &conf->conf_targets, t_next) { - isns_req_add_str(req, 32, target->t_name); - isns_req_add_32(req, 33, 1); /* 1 -- Target*/ + req.add_str(32, target->t_name); + req.add_32(33, 1); /* 1 -- Target*/ if (target->t_alias != NULL) - isns_req_add_str(req, 34, target->t_alias); + req.add_str(34, target->t_alias); TAILQ_FOREACH(port, &target->t_ports, p_ts) { if ((pg = port->p_portal_group) == NULL) continue; - isns_req_add_32(req, 51, pg->pg_tag); + req.add_32(51, pg->pg_tag); TAILQ_FOREACH(portal, &pg->pg_portals, p_next) { - isns_req_add_addr(req, 49, portal->p_ai); - isns_req_add_port(req, 50, portal->p_ai); + req.add_addr(49, portal->p_ai); + req.add_port(50, portal->p_ai); } } } - res = isns_req_send(s, req); - if (res < 0) { + if (!req.send(s)) { log_warn("send(2) failed for %s", isns->i_addr); - goto quit; + return; } - res = isns_req_receive(s, req); - if (res < 0) { + if (!req.receive(s)) { log_warn("receive(2) failed for %s", isns->i_addr); - goto quit; + return; } - error = isns_req_get_status(req); + error = req.get_status(); if (error != 0) { log_warnx("iSNS register error %d for %s", error, isns->i_addr); - res = -1; } -quit: - isns_req_free(req); - return (res); } -static int +static bool isns_do_check(struct isns *isns, int s, const char *hostname) { struct conf *conf = isns->i_conf; - struct isns_req *req; - int res = 0; uint32_t error; - req = isns_req_create(ISNS_FUNC_DEVATTRQRY, ISNS_FLAG_CLIENT); - isns_req_add_str(req, 32, TAILQ_FIRST(&conf->conf_targets)->t_name); - isns_req_add_str(req, 1, hostname); - isns_req_add_delim(req); - isns_req_add(req, 2, 0, NULL); - res = isns_req_send(s, req); - if (res < 0) { + isns_req req(ISNS_FUNC_DEVATTRQRY, ISNS_FLAG_CLIENT); + req.add_str(32, TAILQ_FIRST(&conf->conf_targets)->t_name); + req.add_str(1, hostname); + req.add_delim(); + req.add(2, 0, NULL); + if (!req.send(s)) { log_warn("send(2) failed for %s", isns->i_addr); - goto quit; + return (false); } - res = isns_req_receive(s, req); - if (res < 0) { + if (!req.receive(s)) { log_warn("receive(2) failed for %s", isns->i_addr); - goto quit; + return (false); } - error = isns_req_get_status(req); + error = req.get_status(); if (error != 0) { log_warnx("iSNS check error %d for %s", error, isns->i_addr); - res = -1; + return (false); } -quit: - isns_req_free(req); - return (res); + return (true); } -static int +static void isns_do_deregister(struct isns *isns, int s, const char *hostname) { struct conf *conf = isns->i_conf; - struct isns_req *req; - int res = 0; uint32_t error; - req = isns_req_create(ISNS_FUNC_DEVDEREG, ISNS_FLAG_CLIENT); - isns_req_add_str(req, 32, TAILQ_FIRST(&conf->conf_targets)->t_name); - isns_req_add_delim(req); - isns_req_add_str(req, 1, hostname); - res = isns_req_send(s, req); - if (res < 0) { + isns_req req(ISNS_FUNC_DEVDEREG, ISNS_FLAG_CLIENT); + req.add_str(32, TAILQ_FIRST(&conf->conf_targets)->t_name); + req.add_delim(); + req.add_str(1, hostname); + if (!req.send(s)) { log_warn("send(2) failed for %s", isns->i_addr); - goto quit; + return; } - res = isns_req_receive(s, req); - if (res < 0) { + if (!req.receive(s)) { log_warn("receive(2) failed for %s", isns->i_addr); - goto quit; + return; } - error = isns_req_get_status(req); + error = req.get_status(); if (error != 0) { log_warnx("iSNS deregister error %d for %s", error, isns->i_addr); - res = -1; } -quit: - isns_req_free(req); - return (res); } void @@ -795,7 +773,7 @@ void isns_check(struct isns *isns) { struct conf *conf = isns->i_conf; - int error, s, res; + int error, s; char hostname[256]; if (TAILQ_EMPTY(&conf->conf_targets) || @@ -811,8 +789,7 @@ isns_check(struct isns *isns) if (error != 0) log_err(1, "gethostname"); - res = isns_do_check(isns, s, hostname); - if (res < 0) { + if (!isns_do_check(isns, s, hostname)) { isns_do_deregister(isns, s, hostname); isns_do_register(isns, s, hostname); } diff --git a/usr.sbin/ctld/isns.cc b/usr.sbin/ctld/isns.cc index 2f1e0094c0bf..9e27999d2bf9 100644 --- a/usr.sbin/ctld/isns.cc +++ b/usr.sbin/ctld/isns.cc @@ -27,7 +27,7 @@ * */ -#include <sys/types.h> +#include <sys/param.h> #include <sys/time.h> #include <sys/socket.h> #include <sys/wait.h> @@ -43,129 +43,86 @@ #include "ctld.hh" #include "isns.hh" -struct isns_req * -isns_req_alloc(void) +isns_req::isns_req(uint16_t func, uint16_t flags) { - struct isns_req *req; + struct isns_hdr hdr; - req = reinterpret_cast<struct isns_req *>(calloc(1, sizeof(struct isns_req))); - if (req == NULL) { - log_err(1, "calloc"); - return (NULL); - } - req->ir_buflen = sizeof(struct isns_hdr); - req->ir_usedlen = 0; - req->ir_buf = reinterpret_cast<uint8_t *>(calloc(1, req->ir_buflen)); - if (req->ir_buf == NULL) { - free(req); - log_err(1, "calloc"); - return (NULL); - } - return (req); -} - -struct isns_req * -isns_req_create(uint16_t func, uint16_t flags) -{ - struct isns_req *req; - struct isns_hdr *hdr; - - req = isns_req_alloc(); - req->ir_usedlen = sizeof(struct isns_hdr); - hdr = (struct isns_hdr *)req->ir_buf; - be16enc(hdr->ih_version, ISNS_VERSION); - be16enc(hdr->ih_function, func); - be16enc(hdr->ih_flags, flags); - return (req); + be16enc(&hdr.ih_version, ISNS_VERSION); + be16enc(&hdr.ih_function, func); + be16enc(&hdr.ih_flags, flags); + append(&hdr, sizeof(hdr)); } void -isns_req_free(struct isns_req *req) +isns_req::getspace(uint32_t len) { - - free(req->ir_buf); - free(req); + ir_buf.reserve(ir_buf.size() + len); } -static int -isns_req_getspace(struct isns_req *req, uint32_t len) +void +isns_req::append(const void *buf, size_t len) { - void *newbuf; - int newlen; - - if (req->ir_usedlen + len <= req->ir_buflen) - return (0); - newlen = 1 << flsl(req->ir_usedlen + len); - newbuf = realloc(req->ir_buf, newlen); - if (newbuf == NULL) { - log_err(1, "realloc"); - return (1); - } - req->ir_buf = reinterpret_cast<uint8_t *>(newbuf); - req->ir_buflen = newlen; - return (0); + const char *cp = reinterpret_cast<const char *>(buf); + ir_buf.insert(ir_buf.end(), cp, cp + len); } void -isns_req_add(struct isns_req *req, uint32_t tag, uint32_t len, - const void *value) +isns_req::add(uint32_t tag, uint32_t len, const void *value) { - struct isns_tlv *tlv; + struct isns_tlv tlv; uint32_t vlen; - vlen = len + ((len & 3) ? (4 - (len & 3)) : 0); - isns_req_getspace(req, sizeof(*tlv) + vlen); - tlv = (struct isns_tlv *)&req->ir_buf[req->ir_usedlen]; - be32enc(tlv->it_tag, tag); - be32enc(tlv->it_length, vlen); - memcpy(tlv->it_value, value, len); + vlen = roundup2(len, 4); + getspace(sizeof(tlv) + vlen); + be32enc(&tlv.it_tag, tag); + be32enc(&tlv.it_length, vlen); + append(&tlv, sizeof(tlv)); + append(value, len); if (vlen != len) - memset(&tlv->it_value[len], 0, vlen - len); - req->ir_usedlen += sizeof(*tlv) + vlen; + ir_buf.insert(ir_buf.end(), vlen - len, 0); } void -isns_req_add_delim(struct isns_req *req) +isns_req::add_delim() { - - isns_req_add(req, 0, 0, NULL); + add(0, 0, nullptr); } void -isns_req_add_str(struct isns_req *req, uint32_t tag, const char *value) +isns_req::add_str(uint32_t tag, const char *value) { - isns_req_add(req, tag, strlen(value) + 1, value); + add(tag, strlen(value) + 1, value); } void -isns_req_add_32(struct isns_req *req, uint32_t tag, uint32_t value) +isns_req::add_32(uint32_t tag, uint32_t value) { uint32_t beval; be32enc(&beval, value); - isns_req_add(req, tag, sizeof(value), &beval); + add(tag, sizeof(value), &beval); } void -isns_req_add_addr(struct isns_req *req, uint32_t tag, struct addrinfo *ai) +isns_req::add_addr(uint32_t tag, const struct addrinfo *ai) { - struct sockaddr_in *in4; - struct sockaddr_in6 *in6; + const struct sockaddr_in *in4; + const struct sockaddr_in6 *in6; uint8_t buf[16]; switch (ai->ai_addr->sa_family) { case AF_INET: - in4 = (struct sockaddr_in *)(void *)ai->ai_addr; + in4 = (const struct sockaddr_in *)ai->ai_addr; memset(buf, 0, 10); buf[10] = 0xff; buf[11] = 0xff; memcpy(&buf[12], &in4->sin_addr, sizeof(in4->sin_addr)); - isns_req_add(req, tag, sizeof(buf), buf); + add(tag, sizeof(buf), buf); break; case AF_INET6: - in6 = (struct sockaddr_in6 *)(void *)ai->ai_addr; - isns_req_add(req, tag, sizeof(in6->sin6_addr), &in6->sin6_addr); + in6 = (const struct sockaddr_in6 *)ai->ai_addr; + add(tag, sizeof(in6->sin6_addr), &in6->sin6_addr); break; default: log_errx(1, "Unsupported address family %d", @@ -174,22 +131,22 @@ isns_req_add_addr(struct isns_req *req, uint32_t tag, struct addrinfo *ai) } void -isns_req_add_port(struct isns_req *req, uint32_t tag, struct addrinfo *ai) +isns_req::add_port(uint32_t tag, const struct addrinfo *ai) { - struct sockaddr_in *in4; - struct sockaddr_in6 *in6; + const struct sockaddr_in *in4; + const struct sockaddr_in6 *in6; uint32_t buf; switch (ai->ai_addr->sa_family) { case AF_INET: - in4 = (struct sockaddr_in *)(void *)ai->ai_addr; + in4 = (const struct sockaddr_in *)ai->ai_addr; be32enc(&buf, ntohs(in4->sin_port)); - isns_req_add(req, tag, sizeof(buf), &buf); + add(tag, sizeof(buf), &buf); break; case AF_INET6: - in6 = (struct sockaddr_in6 *)(void *)ai->ai_addr; + in6 = (const struct sockaddr_in6 *)ai->ai_addr; be32enc(&buf, ntohs(in6->sin6_port)); - isns_req_add(req, tag, sizeof(buf), &buf); + add(tag, sizeof(buf), &buf); break; default: log_errx(1, "Unsupported address family %d", @@ -197,55 +154,54 @@ isns_req_add_port(struct isns_req *req, uint32_t tag, struct addrinfo *ai) } } -int -isns_req_send(int s, struct isns_req *req) +bool +isns_req::send(int s) { struct isns_hdr *hdr; int res; - hdr = (struct isns_hdr *)req->ir_buf; - be16enc(hdr->ih_length, req->ir_usedlen - sizeof(*hdr)); + hdr = (struct isns_hdr *)ir_buf.data(); + be16enc(hdr->ih_length, ir_buf.size() - sizeof(*hdr)); be16enc(hdr->ih_flags, be16dec(hdr->ih_flags) | ISNS_FLAG_LAST | ISNS_FLAG_FIRST); be16enc(hdr->ih_transaction, 0); be16enc(hdr->ih_sequence, 0); - res = write(s, req->ir_buf, req->ir_usedlen); - return ((res < 0) ? -1 : 0); + res = write(s, ir_buf.data(), ir_buf.size()); + return (res > 0 && (size_t)res == ir_buf.size()); } -int -isns_req_receive(int s, struct isns_req *req) +bool +isns_req::receive(int s) { struct isns_hdr *hdr; ssize_t res, len; - req->ir_usedlen = 0; - isns_req_getspace(req, sizeof(*hdr)); - res = read(s, req->ir_buf, sizeof(*hdr)); - if (res < (ssize_t)sizeof(*hdr)) - return (-1); - req->ir_usedlen = sizeof(*hdr); - hdr = (struct isns_hdr *)req->ir_buf; + ir_buf.resize(sizeof(*hdr)); + res = read(s, ir_buf.data(), sizeof(*hdr)); + if (res < (ssize_t)sizeof(*hdr)) { + ir_buf.clear(); + return (false); + } + hdr = (struct isns_hdr *)ir_buf.data(); if (be16dec(hdr->ih_version) != ISNS_VERSION) - return (-1); + return (false); if ((be16dec(hdr->ih_flags) & (ISNS_FLAG_LAST | ISNS_FLAG_FIRST)) != (ISNS_FLAG_LAST | ISNS_FLAG_FIRST)) - return (-1); + return (false); len = be16dec(hdr->ih_length); - isns_req_getspace(req, len); - res = read(s, &req->ir_buf[req->ir_usedlen], len); + ir_buf.resize(sizeof(*hdr) + len); + res = read(s, ir_buf.data() + sizeof(*hdr), len); if (res < len) - return (-1); - req->ir_usedlen += len; - return (0); + return (false); + return (res == len); } uint32_t -isns_req_get_status(struct isns_req *req) +isns_req::get_status() { - if (req->ir_usedlen < sizeof(struct isns_hdr) + 4) + if (ir_buf.size() < sizeof(struct isns_hdr) + 4) return (-1); - return (be32dec(&req->ir_buf[sizeof(struct isns_hdr)])); + return (be32dec(&ir_buf[sizeof(struct isns_hdr)])); } diff --git a/usr.sbin/ctld/isns.hh b/usr.sbin/ctld/isns.hh index 259a98f8b6be..79a288f7d133 100644 --- a/usr.sbin/ctld/isns.hh +++ b/usr.sbin/ctld/isns.hh @@ -27,6 +27,8 @@ #ifndef __ISNS_HH__ #define __ISNS_HH__ +#include <vector> + #define ISNS_VERSION 0x0001 #define ISNS_FUNC_DEVATTRREG 0x0001 @@ -68,23 +70,23 @@ struct isns_tlv { }; struct isns_req { - u_int ir_buflen; - u_int ir_usedlen; - uint8_t *ir_buf; -}; + isns_req() {} + isns_req(uint16_t func, uint16_t flags); -struct isns_req * isns_req_alloc(void); -struct isns_req * isns_req_create(uint16_t func, uint16_t flags); -void isns_req_free(struct isns_req *req); -void isns_req_add(struct isns_req *req, uint32_t tag, uint32_t len, - const void *value); -void isns_req_add_delim(struct isns_req *req); -void isns_req_add_str(struct isns_req *req, uint32_t tag, const char *value); -void isns_req_add_32(struct isns_req *req, uint32_t tag, uint32_t value); -void isns_req_add_addr(struct isns_req *req, uint32_t tag, struct addrinfo *ai); -void isns_req_add_port(struct isns_req *req, uint32_t tag, struct addrinfo *ai); -int isns_req_send(int s, struct isns_req *req); -int isns_req_receive(int s, struct isns_req *req); -uint32_t isns_req_get_status(struct isns_req *req); + void add(uint32_t tag, uint32_t len, const void *value); + void add_delim(); + void add_str(uint32_t tag, const char *value); + void add_32(uint32_t tag, uint32_t value); + void add_addr(uint32_t tag, const struct addrinfo *ai); + void add_port(uint32_t tag, const struct addrinfo *ai); + bool send(int s); + bool receive(int s); + uint32_t get_status(); +private: + void getspace(uint32_t len); + void append(const void *buf, size_t len); + + std::vector<char> ir_buf; +}; #endif /* __ISNS_HH__ */