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.
I have been running the large patch (which removes proc_id) in
production for a few months on multiple installations, and it all seems
fine.
Test case:
# cat relayd.conf
table <local> { 127.0.0.1 }
relay "test" {
listen on 0.0.0.0 port 2222
forward to <local> port 22 check tcp
}
# relayd -f relayd.conf
# for i in `echo "1 2 3 4 5 6 7 8 9 0"`; do nc 127.0.0.1 2222 >
/dev/null & done; sleep 3; pkill -9 nc
(wait a minute for statistics to be summarized)
# relayctl show relays
Id Type Name Avlblty Status
1 relay test active
total: 7 sessions
last: 0/60s 7/h 7/d sessions
average: 1/60s 0/h 0/d sessions
7 sessions hit the first relay fork. We should expect 10 sessions.
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.
(reported by Tom Knienieder: relayd UDP bug and patch ...)
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 --
...or... (the "large patch")
-- 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 --
