Hi,

There is a bug in relayd's handling of statistics collected from each "prefork" (0,1,2,3,4,..). Once received over IMSG, statistics should be saved in a rl_stats[RELAY_MAXPROC + 1] array per relay (rlay->rl_stats). However all ends up in rlay->rl_stats[0] overwriting other children's statistics in a holy mess.

So, fixing this bug, and you should start to see up to five times more statistics than before. :)

This is due to the unset variable proc_id which is used as if it were incremented for each forked child. This proc_id should be the same as p->p_instance and env->sc_ps->ps_instance. So I have two suggested fixes.

1. Use the big patch below, it removes the proc_id variable and imho. cleans up the current struct relayd* env confusion. 2. Use the small quick-fix patch below, it sets up proc_id properly. However env is NULL dereferenced for sc_timeout in relay_udp.c, thus crashing in a simple "dns forward" test. If the big patch is applied, env is always set in relay.c and relay_udp.c, so we should let this patch decide if another bug report is needed.

Regards
Erik

Two patched attached below.

-- cut --
Index: relay.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.144
diff -u -r1.144 relay.c
--- relay.c    21 Jan 2012 13:40:48 -0000    1.144
+++ relay.c    7 Mar 2012 10:24:57 -0000
@@ -332,6 +332,9 @@

     if (config_init(ps->ps_env) == -1)
         fatal("failed to initialize configuration");
+
+    /* Set to current prefork id */
+    proc_id = p->p_instance;

     /* We use a custom shutdown callback */
     p->p_shutdown = relay_shutdown;
-- cut --
Index: pfe.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/pfe.c,v
retrieving revision 1.72
diff -u -r1.72 pfe.c
--- pfe.c    21 Jan 2012 13:40:48 -0000    1.72
+++ pfe.c    7 Mar 2012 00:30:19 -0000
@@ -46,7 +46,7 @@
 int     pfe_dispatch_hce(int, struct privsep_proc *, struct imsg *);
 int     pfe_dispatch_relay(int, struct privsep_proc *, struct imsg *);

-static struct relayd        *env = NULL;
+struct relayd *env = NULL;

 static struct privsep_proc procs[] = {
     { "parent",    PROC_PARENT,    pfe_dispatch_parent },
Index: relay.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.144
diff -u -r1.144 relay.c
--- relay.c    21 Jan 2012 13:40:48 -0000    1.144
+++ relay.c    7 Mar 2012 00:30:20 -0000
@@ -135,8 +135,7 @@
 volatile sig_atomic_t relay_sessions;
 objid_t relay_conid;

-static struct relayd        *env = NULL;
-int                 proc_id;
+extern struct relayd *env;

 static struct privsep_proc procs[] = {
     { "parent",    PROC_PARENT,    relay_dispatch_parent },
@@ -305,7 +304,7 @@

     switch (rlay->rl_proto->type) {
     case RELAY_PROTO_DNS:
-        relay_udp_privinit(env, rlay);
+        relay_udp_privinit(rlay);
         break;
     case RELAY_PROTO_TCP:
     case RELAY_PROTO_HTTP:
@@ -367,7 +366,7 @@
         bzero(&crs, sizeof(crs));
         resethour = resetday = 0;

-        cur = &rlay->rl_stats[proc_id];
+        cur = &rlay->rl_stats[env->sc_ps->ps_instance];
         cur->cnt += cur->last;
         cur->tick++;
         cur->avg = (cur->last + cur->avg) / 2;
@@ -390,7 +389,7 @@
             cur->last_day = 0;

         crs.id = rlay->rl_conf.id;
-        crs.proc = proc_id;
+        crs.proc = env->sc_ps->ps_instance;
         proc_compose_imsg(env->sc_ps, PROC_PFE, -1, IMSG_STATISTICS, -1,
&crs, sizeof(crs));

@@ -2009,7 +2008,7 @@
     SPLAY_INSERT(session_tree, &rlay->rl_sessions, con);

     /* Increment the per-relay session counter */
-    rlay->rl_stats[proc_id].last++;
+    rlay->rl_stats[env->sc_ps->ps_instance].last++;

     /* Pre-allocate output buffer */
     con->se_out.output = evbuffer_new();
@@ -2050,7 +2049,7 @@
         bzero(cnl, sizeof(*cnl));
         cnl->in = -1;
         cnl->id = con->se_id;
-        cnl->proc = proc_id;
+        cnl->proc = env->sc_ps->ps_instance;
         cnl->proto = IPPROTO_TCP;

         bcopy(&con->se_in.ss, &cnl->src, sizeof(cnl->src));
@@ -2236,7 +2235,7 @@

     bzero(&bnd, sizeof(bnd));
     bnd.bnd_id = con->se_id;
-    bnd.bnd_proc = proc_id;
+    bnd.bnd_proc = env->sc_ps->ps_instance;
     bnd.bnd_port = port;
     bnd.bnd_proto = proto;
     bcopy(&con->se_in.ss, &bnd.bnd_ss, sizeof(bnd.bnd_ss));
@@ -2486,7 +2485,7 @@
             fatalx("relay_dispatch_pfe: invalid table id");

         DPRINTF("%s: [%d] state %d for "
-            "host %u %s", __func__, proc_id, st.up,
+            "host %u %s", __func__, env->sc_ps->ps_instance, st.up,
             host->conf.id, host->conf.name);

         if ((st.up == HOST_UNKNOWN && host->up == HOST_DOWN) ||
Index: relay_udp.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relay_udp.c,v
retrieving revision 1.24
diff -u -r1.24 relay_udp.c
--- relay_udp.c    9 May 2011 12:08:47 -0000    1.24
+++ relay_udp.c    7 Mar 2012 00:30:20 -0000
@@ -49,10 +49,9 @@

 extern volatile sig_atomic_t relay_sessions;
 extern objid_t relay_conid;
-extern int proc_id;
 extern int debug;

-struct relayd *env = NULL;
+extern struct relayd *env;
 struct shuffle relay_shuffle;

 int         relay_udp_socket(struct sockaddr_storage *, in_port_t,
@@ -70,11 +69,8 @@
 int         relay_dns_cmp(struct rsession *, struct rsession *);

 void
-relay_udp_privinit(struct relayd *x_env, struct relay *rlay)
+relay_udp_privinit(struct relay *rlay)
 {
-    if (env == NULL)
-        env = x_env;
-
     if (rlay->rl_conf.flags & F_SSL)
         fatalx("ssl over udp is not supported");
     rlay->rl_conf.flags |= F_UDP;
@@ -279,7 +275,7 @@
     SPLAY_INSERT(session_tree, &rlay->rl_sessions, con);

     /* Increment the per-relay session counter */
-    rlay->rl_stats[proc_id].last++;
+    rlay->rl_stats[env->sc_ps->ps_instance].last++;

     /* Pre-allocate output buffer */
     con->se_out.output = evbuffer_new();
@@ -316,7 +312,7 @@
         bzero(cnl, sizeof(*cnl));
         cnl->in = -1;
         cnl->id = con->se_id;
-        cnl->proc = proc_id;
+        cnl->proc = env->sc_ps->ps_instance;
         cnl->proto = IPPROTO_UDP;
         bcopy(&con->se_in.ss, &cnl->src, sizeof(cnl->src));
         bcopy(&rlay->rl_conf.ss, &cnl->dst, sizeof(cnl->dst));
Index: relayd.h
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
retrieving revision 1.152
diff -u -r1.152 relayd.h
--- relayd.h    21 Jan 2012 13:40:48 -0000    1.152
+++ relayd.h    7 Mar 2012 00:30:20 -0000
@@ -957,7 +957,7 @@
 SPLAY_PROTOTYPE(session_tree, rsession, se_nodes, relay_session_cmp);

 /* relay_udp.c */
-void     relay_udp_privinit(struct relayd *, struct relay *);
+void     relay_udp_privinit(struct relay *);
 void     relay_udp_init(struct relay *);
 int     relay_udp_bind(struct sockaddr_storage *, in_port_t,
         struct protocol *);
-- cut --

Reply via email to