The branch main has been updated by asomers:
URL:
https://cgit.FreeBSD.org/src/commit/?id=969876fcee57ea1cb1c7b4d2ee757793cbfbe353
commit 969876fcee57ea1cb1c7b4d2ee757793cbfbe353
Author: Alan Somers <asom...@freebsd.org>
AuthorDate: 2024-06-10 23:48:49 +0000
Commit: Alan Somers <asom...@freebsd.org>
CommitDate: 2024-08-07 14:36:52 +0000
ctld: parse config file independently of getting kernel info
Separate the parsing of the config file from the reading of kernel port
information. This has three benefits:
* Separation of concerns makes future changes easier.
* Allows the config file to be read earlier, which is necessary for
fixing PR 271460.
* Reduces total line count, by eliminating duplication between parse.y
(for traditional config file) and uclparse.c (for UCL config file).
MFC after: 2 weeks
Sponsored by: Axcient
Reviewed by: mav
Pull Request: https://github.com/freebsd/freebsd-src/pull/1287
---
usr.sbin/ctld/ctld.c | 99 +++++++++++++++++++++++++++++++++++++-----------
usr.sbin/ctld/ctld.h | 25 ++++++++----
usr.sbin/ctld/kernel.c | 6 +--
usr.sbin/ctld/parse.y | 37 +-----------------
usr.sbin/ctld/uclparse.c | 37 ++----------------
5 files changed, 100 insertions(+), 104 deletions(-)
diff --git a/usr.sbin/ctld/ctld.c b/usr.sbin/ctld/ctld.c
index bf2791040125..805648f0465f 100644
--- a/usr.sbin/ctld/ctld.c
+++ b/usr.sbin/ctld/ctld.c
@@ -98,7 +98,6 @@ conf_new(void)
TAILQ_INIT(&conf->conf_auth_groups);
TAILQ_INIT(&conf->conf_ports);
TAILQ_INIT(&conf->conf_portal_groups);
- TAILQ_INIT(&conf->conf_pports);
TAILQ_INIT(&conf->conf_isns);
conf->conf_isns_period = 900;
@@ -117,7 +116,6 @@ conf_delete(struct conf *conf)
struct target *targ, *tmp;
struct auth_group *ag, *cagtmp;
struct portal_group *pg, *cpgtmp;
- struct pport *pp, *pptmp;
struct isns *is, *istmp;
assert(conf->conf_pidfh == NULL);
@@ -130,8 +128,6 @@ conf_delete(struct conf *conf)
auth_group_delete(ag);
TAILQ_FOREACH_SAFE(pg, &conf->conf_portal_groups, pg_next, cpgtmp)
portal_group_delete(pg);
- TAILQ_FOREACH_SAFE(pp, &conf->conf_pports, pp_next, pptmp)
- pport_delete(pp);
TAILQ_FOREACH_SAFE(is, &conf->conf_isns, i_next, istmp)
isns_delete(is);
assert(TAILQ_EMPTY(&conf->conf_ports));
@@ -1177,27 +1173,27 @@ valid_iscsi_name(const char *name)
}
struct pport *
-pport_new(struct conf *conf, const char *name, uint32_t ctl_port)
+pport_new(struct kports *kports, const char *name, uint32_t ctl_port)
{
struct pport *pp;
pp = calloc(1, sizeof(*pp));
if (pp == NULL)
log_err(1, "calloc");
- pp->pp_conf = conf;
+ pp->pp_kports = kports;
pp->pp_name = checked_strdup(name);
pp->pp_ctl_port = ctl_port;
TAILQ_INIT(&pp->pp_ports);
- TAILQ_INSERT_TAIL(&conf->conf_pports, pp, pp_next);
+ TAILQ_INSERT_TAIL(&kports->pports, pp, pp_next);
return (pp);
}
struct pport *
-pport_find(const struct conf *conf, const char *name)
+pport_find(const struct kports *kports, const char *name)
{
struct pport *pp;
- TAILQ_FOREACH(pp, &conf->conf_pports, pp_next) {
+ TAILQ_FOREACH(pp, &kports->pports, pp_next) {
if (strcasecmp(pp->pp_name, name) == 0)
return (pp);
}
@@ -1205,11 +1201,11 @@ pport_find(const struct conf *conf, const char *name)
}
struct pport *
-pport_copy(struct pport *pp, struct conf *conf)
+pport_copy(struct pport *pp, struct kports *kports)
{
struct pport *ppnew;
- ppnew = pport_new(conf, pp->pp_name, pp->pp_ctl_port);
+ ppnew = pport_new(kports, pp->pp_name, pp->pp_ctl_port);
return (ppnew);
}
@@ -1220,7 +1216,7 @@ pport_delete(struct pport *pp)
TAILQ_FOREACH_SAFE(port, &pp->pp_ports, p_ts, tport)
port_delete(port);
- TAILQ_REMOVE(&pp->pp_conf->conf_pports, pp, pp_next);
+ TAILQ_REMOVE(&pp->pp_kports->pports, pp, pp_next);
free(pp->pp_name);
free(pp);
}
@@ -1255,7 +1251,8 @@ port_new(struct conf *conf, struct target *target, struct
portal_group *pg)
}
struct port *
-port_new_ioctl(struct conf *conf, struct target *target, int pp, int vp)
+port_new_ioctl(struct conf *conf, struct kports *kports, struct target *target,
+ int pp, int vp)
{
struct pport *pport;
struct port *port;
@@ -1269,7 +1266,7 @@ port_new_ioctl(struct conf *conf, struct target *target,
int pp, int vp)
return (NULL);
}
- pport = pport_find(conf, pname);
+ pport = pport_find(kports, pname);
if (pport != NULL) {
free(pname);
return (port_new_pp(conf, target, pport));
@@ -1424,6 +1421,7 @@ target_delete(struct target *targ)
port_delete(port);
TAILQ_REMOVE(&targ->t_conf->conf_targets, targ, t_next);
+ free(targ->t_pport);
free(targ->t_name);
free(targ->t_redirection);
free(targ);
@@ -2686,21 +2684,17 @@ check_perms(const char *path)
}
static struct conf *
-conf_new_from_file(const char *path, struct conf *oldconf, bool ucl)
+conf_new_from_file(const char *path, bool ucl)
{
struct conf *conf;
struct auth_group *ag;
struct portal_group *pg;
- struct pport *pp;
int error;
log_debugx("obtaining configuration from %s", path);
conf = conf_new();
- TAILQ_FOREACH(pp, &oldconf->conf_pports, pp_next)
- pport_copy(pp, conf);
-
ag = auth_group_new(conf, "default");
assert(ag != NULL);
@@ -2755,9 +2749,60 @@ conf_new_from_file(const char *path, struct conf
*oldconf, bool ucl)
return (conf);
}
+/*
+ * If the config file specifies physical ports for any target, associate them
+ * with the config file. If necessary, create them.
+ */
+static int
+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) {
+ if (!targ->t_pport)
+ continue;
+
+ 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) {
+ log_warnx("can't create new ioctl port "
+ "for target \"%s\"", targ->t_name);
+ return (1);
+ }
+
+ continue;
+ }
+
+ pp = pport_find(kports, targ->t_pport);
+ if (pp == NULL) {
+ log_warnx("unknown port \"%s\" for target \"%s\"",
+ targ->t_pport, targ->t_name);
+ return (1);
+ }
+ if (!TAILQ_EMPTY(&pp->pp_ports)) {
+ log_warnx("can't link port \"%s\" to target \"%s\", "
+ "port already linked to some target",
+ targ->t_pport, targ->t_name);
+ return (1);
+ }
+ tp = port_new_pp(conf, targ, pp);
+ if (tp == NULL) {
+ log_warnx("can't link port \"%s\" to target \"%s\"",
+ targ->t_pport, targ->t_name);
+ return (1);
+ }
+ }
+ return (0);
+}
+
int
main(int argc, char **argv)
{
+ struct kports kports;
struct conf *oldconf, *newconf, *tmpconf;
struct isns *newns;
const char *config_path = DEFAULT_CONFIG_PATH;
@@ -2800,8 +2845,9 @@ main(int argc, char **argv)
log_init(debug);
kernel_init();
- oldconf = conf_new_from_kernel();
- newconf = conf_new_from_file(config_path, oldconf, use_ucl);
+ TAILQ_INIT(&kports.pports);
+ oldconf = conf_new_from_kernel(&kports);
+ newconf = conf_new_from_file(config_path, use_ucl);
if (newconf == NULL)
log_errx(1, "configuration error; exiting");
@@ -2814,6 +2860,9 @@ main(int argc, char **argv)
newconf->conf_debug = debug;
}
+ if (new_pports_from_conf(newconf, &kports))
+ log_errx(1, "Error associating physical ports; exiting");
+
error = conf_apply(oldconf, newconf);
if (error != 0)
log_errx(1, "failed to apply configuration; exiting");
@@ -2841,17 +2890,21 @@ main(int argc, char **argv)
if (sighup_received) {
sighup_received = false;
log_debugx("received SIGHUP, reloading configuration");
- tmpconf = conf_new_from_file(config_path, newconf,
- use_ucl);
+ tmpconf = conf_new_from_file(config_path, use_ucl);
if (tmpconf == NULL) {
log_warnx("configuration error, "
"continuing with old configuration");
+ } else if (new_pports_from_conf(tmpconf, &kports)) {
+ log_warnx("Error associating physical ports, "
+ "continuing with old configuration");
+ conf_delete(tmpconf);
} else {
if (debug > 0)
tmpconf->conf_debug = debug;
oldconf = newconf;
newconf = tmpconf;
+
error = conf_apply(oldconf, newconf);
if (error != 0)
log_warnx("failed to reload "
diff --git a/usr.sbin/ctld/ctld.h b/usr.sbin/ctld/ctld.h
index bcc3c1956dc4..e1bab1a8e3b8 100644
--- a/usr.sbin/ctld/ctld.h
+++ b/usr.sbin/ctld/ctld.h
@@ -131,10 +131,11 @@ 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 conf *pp_conf;
+ struct kports *pp_kports;
char *pp_name;
uint32_t pp_ctl_port;
@@ -190,6 +191,8 @@ struct target {
char *t_name;
char *t_alias;
char *t_redirection;
+ /* Name of this target's physical port, if any, i.e. "isp0" */
+ char *t_pport;
};
struct isns {
@@ -206,7 +209,6 @@ struct conf {
TAILQ_HEAD(, auth_group) conf_auth_groups;
TAILQ_HEAD(, port) conf_ports;
TAILQ_HEAD(, portal_group) conf_portal_groups;
- TAILQ_HEAD(, pport) conf_pports;
TAILQ_HEAD(, isns) conf_isns;
int conf_isns_period;
int conf_isns_timeout;
@@ -224,6 +226,11 @@ struct conf {
bool conf_kernel_port_on;
};
+/* Physical ports exposed by the kernel */
+struct kports {
+ TAILQ_HEAD(, pport) pports;
+};
+
#define CONN_SESSION_TYPE_NONE 0
#define CONN_SESSION_TYPE_DISCOVERY 1
#define CONN_SESSION_TYPE_NORMAL 2
@@ -247,11 +254,11 @@ struct ctld_connection {
struct chap *conn_chap;
};
-int parse_conf(struct conf *conf, const char *path);
+int parse_conf(struct conf *newconf, const char *path);
int uclparse_conf(struct conf *conf, const char *path);
struct conf *conf_new(void);
-struct conf *conf_new_from_kernel(void);
+struct conf *conf_new_from_kernel(struct kports *kports);
void conf_delete(struct conf *conf);
int conf_verify(struct conf *conf);
@@ -305,15 +312,17 @@ 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 conf *conf, const char *name,
+struct pport *pport_new(struct kports *kports, const char *name,
uint32_t ctl_port);
-struct pport *pport_find(const struct conf *conf, const char *name);
-struct pport *pport_copy(struct pport *pport, struct conf *conf);
+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 target
*target,
+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);
diff --git a/usr.sbin/ctld/kernel.c b/usr.sbin/ctld/kernel.c
index ae455e7815f7..eed9f13d42fa 100644
--- a/usr.sbin/ctld/kernel.c
+++ b/usr.sbin/ctld/kernel.c
@@ -414,7 +414,7 @@ cctl_char_handler(void *user_data, const XML_Char *str, int
len)
}
struct conf *
-conf_new_from_kernel(void)
+conf_new_from_kernel(struct kports *kports)
{
struct conf *conf = NULL;
struct target *targ;
@@ -559,13 +559,13 @@ 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(conf, name);
+ pp = pport_find(kports, name);
if (pp == NULL) {
#if 0
log_debugx("found new kernel port %u \"%s\"",
port->port_id, name);
#endif
- pp = pport_new(conf, name, port->port_id);
+ pp = pport_new(kports, name, port->port_id);
if (pp == NULL) {
log_warnx("pport_new failed");
continue;
diff --git a/usr.sbin/ctld/parse.y b/usr.sbin/ctld/parse.y
index 8909df2a8345..d8274b623d3a 100644
--- a/usr.sbin/ctld/parse.y
+++ b/usr.sbin/ctld/parse.y
@@ -832,42 +832,7 @@ target_portal_group: PORTAL_GROUP STR STR
target_port: PORT STR
{
- struct pport *pp;
- struct port *tp;
- int ret, i_pp, i_vp = 0;
-
- ret = sscanf($2, "ioctl/%d/%d", &i_pp, &i_vp);
- if (ret > 0) {
- tp = port_new_ioctl(conf, target, i_pp, i_vp);
- if (tp == NULL) {
- log_warnx("can't create new ioctl port for "
- "target \"%s\"", target->t_name);
- free($2);
- return (1);
- }
- } else {
- pp = pport_find(conf, $2);
- if (pp == NULL) {
- log_warnx("unknown port \"%s\" for target
\"%s\"",
- $2, target->t_name);
- free($2);
- return (1);
- }
- if (!TAILQ_EMPTY(&pp->pp_ports)) {
- log_warnx("can't link port \"%s\" to target
\"%s\", "
- "port already linked to some target",
- $2, target->t_name);
- free($2);
- return (1);
- }
- tp = port_new_pp(conf, target, pp);
- if (tp == NULL) {
- log_warnx("can't link port \"%s\" to target
\"%s\"",
- $2, target->t_name);
- free($2);
- return (1);
- }
- }
+ target->t_pport = strdup($2);
free($2);
}
diff --git a/usr.sbin/ctld/uclparse.c b/usr.sbin/ctld/uclparse.c
index 8bd1ca88d166..e9e42bdf953e 100644
--- a/usr.sbin/ctld/uclparse.c
+++ b/usr.sbin/ctld/uclparse.c
@@ -853,41 +853,10 @@ uclparse_target(const char *name, const ucl_object_t *top)
}
if (!strcmp(key, "port")) {
- struct pport *pp;
- struct port *tp;
- const char *value = ucl_object_tostring(obj);
- int ret, i_pp, i_vp = 0;
-
- ret = sscanf(value, "ioctl/%d/%d", &i_pp, &i_vp);
- if (ret > 0) {
- tp = port_new_ioctl(conf, target, i_pp, i_vp);
- if (tp == NULL) {
- log_warnx("can't create new ioctl port "
- "for target \"%s\"",
target->t_name);
- return (1);
- }
+ const char *value;
- continue;
- }
-
- pp = pport_find(conf, value);
- if (pp == NULL) {
- log_warnx("unknown port \"%s\" for target
\"%s\"",
- value, target->t_name);
- return (1);
- }
- if (!TAILQ_EMPTY(&pp->pp_ports)) {
- log_warnx("can't link port \"%s\" to target
\"%s\", "
- "port already linked to some target",
- value, target->t_name);
- return (1);
- }
- tp = port_new_pp(conf, target, pp);
- if (tp == NULL) {
- log_warnx("can't link port \"%s\" to target
\"%s\"",
- value, target->t_name);
- return (1);
- }
+ value = ucl_object_tostring(obj);
+ target->t_pport = strdup(value);
}
if (!strcmp(key, "redirect")) {
