On Mon, 5 Dec 2011, Camiel Dobbelaar wrote:
> > Another might be to inhibit the processing of IMSG_HOST_STATUS only until
> > the configuration has been completed (that is after receiving
> > IMSG_CFG_DONE):
>
> I'm going to try this one. I'm not sure how bad it is to discard
> messages though.
I tried it, and it does not work correctly. Because the imsg is dropped
while the process is marked inactive, you get desynchronized and this code
for example still breaks:
if (host->check_cnt != st.check_cnt) {
log_debug("%s: host %d => %d", __func__,
host->conf.id, host->up);
fatalx("pfe_dispatch_hce: desynchronized");
}
Maybe that can be fixed, if we can assume that it's not bad to drop
some status messages once in a while.
I tried another approach below: only start the processes if _all_ of them
have loaded the config. This should fix the configuration race after
startup completely.
There's still a race while reloading though. Some processes might still
be active with an old config, while others may be busy purging their old
config before loading the new one. The right way would be to pauze all
the processes first. But I'd say that's a seperate problem. :-)
--
Cam
Index: hce.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/hce.c,v
retrieving revision 1.61
diff -u -p -r1.61 hce.c
--- hce.c 12 Nov 2011 19:36:17 -0000 1.61
+++ hce.c 2 Jan 2012 13:57:40 -0000
@@ -355,6 +355,8 @@ hce_dispatch_parent(int fd, struct privs
break;
case IMSG_CFG_DONE:
config_getcfg(env, imsg);
+ break;
+ case IMSG_CTL_START:
hce_setup_events();
break;
case IMSG_CTL_RESET:
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
retrieving revision 1.159
diff -u -p -r1.159 parse.y
--- parse.y 21 Sep 2011 18:45:40 -0000 1.159
+++ parse.y 2 Jan 2012 13:57:40 -0000
@@ -2280,9 +2280,6 @@ load_config(const char *filename, struct
errors++;
}
- if (TAILQ_EMPTY(conf->sc_relays))
- conf->sc_prefork_relay = 0;
-
/* Cleanup relay list to inherit */
while ((rlay = TAILQ_FIRST(&relays)) != NULL) {
TAILQ_REMOVE(&relays, rlay, rl_entry);
Index: pfe.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/pfe.c,v
retrieving revision 1.71
diff -u -p -r1.71 pfe.c
--- pfe.c 12 Nov 2011 19:36:17 -0000 1.71
+++ pfe.c 2 Jan 2012 13:57:40 -0000
@@ -203,6 +203,8 @@ pfe_dispatch_parent(int fd, struct privs
config_getcfg(env, imsg);
init_filter(env, imsg->fd);
init_tables(env);
+ break;
+ case IMSG_CTL_START:
pfe_setup_events();
pfe_sync();
break;
Index: relay.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.143
diff -u -p -r1.143 relay.c
--- relay.c 21 Sep 2011 18:45:40 -0000 1.143
+++ relay.c 2 Jan 2012 13:57:40 -0000
@@ -2577,6 +2577,8 @@ relay_dispatch_parent(int fd, struct pri
break;
case IMSG_CFG_DONE:
config_getcfg(env, imsg);
+ break;
+ case IMSG_CTL_START:
relay_launch();
break;
case IMSG_CTL_RESET:
Index: relayd.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.104
diff -u -p -r1.104 relayd.c
--- relayd.c 4 Sep 2011 20:26:58 -0000 1.104
+++ relayd.c 2 Jan 2012 13:57:40 -0000
@@ -49,6 +49,7 @@
__dead void usage(void);
int parent_configure(struct relayd *);
+void parent_configure_done(struct relayd *);
void parent_reload(struct relayd *, u_int, const char *);
void parent_sig_handler(int, short, void *);
void parent_shutdown(struct relayd *);
@@ -292,6 +293,9 @@ parent_configure(struct relayd *env)
TAILQ_FOREACH(rlay, env->sc_relays, rl_entry)
config_setrelay(env, rlay);
+ /* HCE, PFE and the preforked relays need to reload their config. */
+ env->sc_reload = 2 + env->sc_prefork_relay;
+
for (id = 0; id < PROC_MAX; id++) {
if (id == privsep_process)
continue;
@@ -308,7 +312,6 @@ parent_configure(struct relayd *env)
} else
s = -1;
- env->sc_reload++;
proc_compose_imsg(env->sc_ps, id, -1, IMSG_CFG_DONE, s,
&cf, sizeof(cf));
}
@@ -354,6 +357,28 @@ parent_reload(struct relayd *env, u_int
}
void
+parent_configure_done(struct relayd *env)
+{
+ int id;
+
+ if (env->sc_reload == 0) {
+ log_warnx("%s: configuration already finished", __func__);
+ return;
+ }
+
+ env->sc_reload--;
+ if (env->sc_reload == 0) {
+ for (id = 0; id < PROC_MAX; id++) {
+ if (id == privsep_process)
+ continue;
+
+ proc_compose_imsg(env->sc_ps, id, -1, IMSG_CTL_START,
+ -1, NULL, 0);
+ }
+ }
+}
+
+void
parent_shutdown(struct relayd *env)
{
config_purge(env, CONFIG_ALL);
@@ -408,8 +433,7 @@ parent_dispatch_pfe(int fd, struct privs
parent_shutdown(env);
break;
case IMSG_CFG_DONE:
- if (env->sc_reload)
- env->sc_reload--;
+ parent_configure_done(env);
break;
default:
return (-1);
@@ -437,8 +461,7 @@ parent_dispatch_hce(int fd, struct privs
(void)snmp_setsock(env, p->p_id);
break;
case IMSG_CFG_DONE:
- if (env->sc_reload)
- env->sc_reload--;
+ parent_configure_done(env);
break;
default:
return (-1);
@@ -476,8 +499,7 @@ parent_dispatch_relay(int fd, struct pri
IMSG_BINDANY, s, &bnd.bnd_id, sizeof(bnd.bnd_id));
break;
case IMSG_CFG_DONE:
- if (env->sc_reload)
- env->sc_reload--;
+ parent_configure_done(env);
break;
default:
return (-1);
Index: relayd.h
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
retrieving revision 1.151
diff -u -p -r1.151 relayd.h
--- relayd.h 4 Sep 2011 20:26:58 -0000 1.151
+++ relayd.h 2 Jan 2012 13:57:40 -0000
@@ -742,6 +742,7 @@ enum imsg_type {
IMSG_CTL_HOST_ENABLE,
IMSG_CTL_HOST_DISABLE,
IMSG_CTL_SHUTDOWN,
+ IMSG_CTL_START,
IMSG_CTL_RELOAD,
IMSG_CTL_RESET,
IMSG_CTL_POLL,
