[conntrack-utils PATCH r7285 00/11] conntrackd with libevent

[conntrack-utils PATCH r7285 00/11] conntrackd with libevent

[Max Kellermann]

Hi Pablo,

another patch series, some smaller fixes, and the big part is
conntrackd with libevent.  While looking for a nice solutions for the
"huge sorted list of alarms" scaling problem, I thought about storing
alarms in some sort of tree.  After a quick look at libevent, I saw
they did it exactly this way.  Let's benchmark libevent against the
existing alarm library.  I don't think it's worth it to continue work
on alarm.c, when well-tested code is already available in libevent.

I did not update configure.ac yet to check for libevent, I am waiting
for your benchmark results.

Max

---

Max Kellermann (11):
      remove the alarm library
      use libevent
      added handler callback to mcast_sock
      check if the received packet is large enough
      use size_t
      remove obsolete prototypes
      added missing ntohs()
      added alarm_pending()
      moved process function pointer to struct local_server
      added struct local_server
      -1 means error, not 0


 include/Makefile.am  |    2 -
 include/alarm.h      |   33 ---------
 include/conntrackd.h |    6 +-
 include/local.h      |   20 ++++-
 include/mcast.h      |   16 +++-
 include/network.h    |    8 +-
 src/Makefile.am      |    4 +
 src/alarm.c          |  191 --------------------------------------------------
 src/build.c          |    2 -
 src/cache_timer.c    |   39 +++++++---
 src/local.c          |   42 +++++++----
 src/main.c           |   12 +++
 src/mcast.c          |   75 ++++++++++++++++++--
 src/network.c        |   22 +++---
 src/run.c            |  177 ++++++++++++++++++----------------------------
 src/stats-mode.c     |    1 
 src/sync-alarm.c     |   58 ++++++++++-----
 src/sync-ftfw.c      |   27 +++++--
 src/sync-mode.c      |   60 ++++------------
 19 files changed, 329 insertions(+), 466 deletions(-)
 delete mode 100644 include/alarm.h
 delete mode 100644 src/alarm.c

[conntrack-utils PATCH r7285 01/11] -1 means error, not 0

[Max Kellermann]

---

 src/run.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/src/run.c b/src/run.c
index abe968f..bdf6b0b 100644
--- a/src/run.c
+++ b/src/run.c
@@ -116,7 +116,7 @@ init(void)
 
 	/* local UNIX socket */
 	STATE(local) = local_server_create(&CONFIG(local));
-	if (!STATE(local)) {
+	if (STATE(local) == -1) {
 		dlog(LOG_ERR, "can't open unix socket!");
 		return -1;
 	}

[conntrack-utils PATCH r7285 02/11] added struct local_server

[Max Kellermann]

---

 include/conntrackd.h |    2 +-
 include/local.h      |   11 ++++++++---
 src/local.c          |   17 ++++++++++-------
 src/run.c            |   16 +++++++++-------
 4 files changed, 28 insertions(+), 18 deletions(-)


diff --git a/include/conntrackd.h b/include/conntrackd.h
index b223a17..47898e2 100644
--- a/include/conntrackd.h
+++ b/include/conntrackd.h
@@ -98,7 +98,7 @@ struct ct_general_state {
 	sigset_t 			block;
 	FILE 				*log;
 	FILE				*stats_log;
-	int 				local;
+	struct local_server		local;
 	struct ct_mode 			*mode;
 	struct ignore_pool		*ignore_pool;
 
diff --git a/include/local.h b/include/local.h
index be77d35..55bceba 100644
--- a/include/local.h
+++ b/include/local.h
@@ -11,10 +11,15 @@ struct local_conf {
 	char path[UNIX_PATH_MAX];
 };
 
+struct local_server {
+	int fd;
+	const char *path;
+};
+
 /* local server */
-int local_server_create(struct local_conf *conf);
-void local_server_destroy(int fd, const char *);
-int do_local_server_step(int fd, void *data, 
+int local_server_create(struct local_server *server, struct local_conf *conf);
+void local_server_destroy(struct local_server *server);
+int do_local_server_step(struct local_server *server, void *data, 
 			 void (*process)(int fd, void *data));
 
 /* local client */
diff --git a/src/local.c b/src/local.c
index f0aba1c..6067880 100644
--- a/src/local.c
+++ b/src/local.c
@@ -26,7 +26,7 @@
 #include <arpa/inet.h>
 #include <sys/un.h>
 
-int local_server_create(struct local_conf *conf)
+int local_server_create(struct local_server *server, struct local_conf *conf)
 {
 	int fd;
 	int len;
@@ -59,23 +59,26 @@ int local_server_create(struct local_conf *conf)
 		return -1;
 	}
 
-	return fd;
+	server->fd = fd;
+	server->path = conf->path;
+
+	return 0;
 }
 
-void local_server_destroy(int fd, const char *path)
+void local_server_destroy(struct local_server *server)
 {
-	unlink(path);
-	close(fd);
+	unlink(server->path);
+	close(server->fd);
 }
 
-int do_local_server_step(int fd, void *data, 
+int do_local_server_step(struct local_server *server, void *data, 
 			 void (*process)(int fd, void *data))
 {
 	int rfd;
 	struct sockaddr_un local;
 	socklen_t sin_size = sizeof(struct sockaddr_un);
 	
-	if ((rfd = accept(fd, (struct sockaddr *)&local, &sin_size)) == -1)
+	if ((rfd = accept(server->fd, (struct sockaddr *)&local, &sin_size)) == -1)
 		return -1;
 
 	process(rfd, data);
diff --git a/src/run.c b/src/run.c
index bdf6b0b..40dc2d4 100644
--- a/src/run.c
+++ b/src/run.c
@@ -40,7 +40,7 @@ void killer(int foo)
 	nfct_close(STATE(dump));
 
 	ignore_pool_destroy(STATE(ignore_pool));
-	local_server_destroy(STATE(local), CONFIG(local).path);
+	local_server_destroy(&STATE(local));
 	STATE(mode)->kill();
 	destroy_alarm_hash();
 	unlink(CONFIG(lockfile));
@@ -92,6 +92,8 @@ void local_handler(int fd, void *data)
 int
 init(void)
 {
+	int ret;
+
 	if (CONFIG(flags) & CTD_STATS_MODE)
 		STATE(mode) = &stats_mode;
 	else if (CONFIG(flags) & CTD_SYNC_MODE)
@@ -115,8 +117,8 @@ init(void)
 	}
 
 	/* local UNIX socket */
-	STATE(local) = local_server_create(&CONFIG(local));
-	if (STATE(local) == -1) {
+	ret = local_server_create(&STATE(local), &CONFIG(local));
+	if (ret == -1) {
 		dlog(LOG_ERR, "can't open unix socket!");
 		return -1;
 	}
@@ -165,10 +167,10 @@ static void __run(struct timeval *next_alarm)
 	fd_set readfds;
 
 	FD_ZERO(&readfds);
-	FD_SET(STATE(local), &readfds);
+	FD_SET(STATE(local).fd, &readfds);
 	FD_SET(nfct_fd(STATE(event)), &readfds);
 
-	max = MAX(STATE(local), nfct_fd(STATE(event)));
+	max = MAX(STATE(local).fd, nfct_fd(STATE(event)));
 
 	if (STATE(mode)->add_fds_to_set)
 		max = MAX(max, STATE(mode)->add_fds_to_set(&readfds));
@@ -187,8 +189,8 @@ static void __run(struct timeval *next_alarm)
 	sigprocmask(SIG_BLOCK, &STATE(block), NULL);		
 
 	/* order received via UNIX socket */
-	if (FD_ISSET(STATE(local), &readfds))
-		do_local_server_step(STATE(local), NULL, local_handler);
+	if (FD_ISSET(STATE(local).fd, &readfds))
+		do_local_server_step(&STATE(local), NULL, local_handler);
 
 	/* conntrack event has happened */
 	if (FD_ISSET(nfct_fd(STATE(event)), &readfds)) {

[conntrack-utils PATCH r7285 03/11] moved process function pointer to struct local_server

[Max Kellermann]

---

 include/local.h |   10 +++++++---
 src/local.c     |   11 +++++++----
 src/run.c       |    5 +++--
 3 files changed, 17 insertions(+), 9 deletions(-)


diff --git a/include/local.h b/include/local.h
index 55bceba..c9a57b8 100644
--- a/include/local.h
+++ b/include/local.h
@@ -14,13 +14,17 @@ struct local_conf {
 struct local_server {
 	int fd;
 	const char *path;
+
+	void *data;
+	void (*process)(int fd, void *data);
 };
 
 /* local server */
-int local_server_create(struct local_server *server, struct local_conf *conf);
+int local_server_create(struct local_server *server, struct local_conf *conf,
+			void *data,
+			void (*process)(int fd, void *data));
 void local_server_destroy(struct local_server *server);
-int do_local_server_step(struct local_server *server, void *data, 
-			 void (*process)(int fd, void *data));
+int do_local_server_step(struct local_server *server);
 
 /* local client */
 int local_client_create(struct local_conf *conf);
diff --git a/src/local.c b/src/local.c
index 6067880..7aab151 100644
--- a/src/local.c
+++ b/src/local.c
@@ -26,7 +26,9 @@
 #include <arpa/inet.h>
 #include <sys/un.h>
 
-int local_server_create(struct local_server *server, struct local_conf *conf)
+int local_server_create(struct local_server *server, struct local_conf *conf,
+			void *data,
+			void (*process)(int fd, void *data))
 {
 	int fd;
 	int len;
@@ -61,6 +63,8 @@ int local_server_create(struct local_server *server, struct local_conf *conf)
 
 	server->fd = fd;
 	server->path = conf->path;
+	server->data = data;
+	server->process = process;
 
 	return 0;
 }
@@ -71,8 +75,7 @@ void local_server_destroy(struct local_server *server)
 	close(server->fd);
 }
 
-int do_local_server_step(struct local_server *server, void *data, 
-			 void (*process)(int fd, void *data))
+int do_local_server_step(struct local_server *server)
 {
 	int rfd;
 	struct sockaddr_un local;
@@ -81,7 +84,7 @@ int do_local_server_step(struct local_server *server, void *data,
 	if ((rfd = accept(server->fd, (struct sockaddr *)&local, &sin_size)) == -1)
 		return -1;
 
-	process(rfd, data);
+	server->process(rfd, server->data);
 	close(rfd);
 
 	return 0;
diff --git a/src/run.c b/src/run.c
index 40dc2d4..fcba393 100644
--- a/src/run.c
+++ b/src/run.c
@@ -117,7 +117,8 @@ init(void)
 	}
 
 	/* local UNIX socket */
-	ret = local_server_create(&STATE(local), &CONFIG(local));
+	ret = local_server_create(&STATE(local), &CONFIG(local),
+				  NULL, local_handler);
 	if (ret == -1) {
 		dlog(LOG_ERR, "can't open unix socket!");
 		return -1;
@@ -190,7 +191,7 @@ static void __run(struct timeval *next_alarm)
 
 	/* order received via UNIX socket */
 	if (FD_ISSET(STATE(local).fd, &readfds))
-		do_local_server_step(&STATE(local), NULL, local_handler);
+		do_local_server_step(&STATE(local));
 
 	/* conntrack event has happened */
 	if (FD_ISSET(nfct_fd(STATE(event)), &readfds)) {

[conntrack-utils PATCH r7285 04/11] added alarm_pending()

[Max Kellermann]

---

 include/alarm.h   |    2 ++
 src/alarm.c       |   10 ++++++++++
 src/cache_timer.c |    5 ++++-
 3 files changed, 16 insertions(+), 1 deletions(-)


diff --git a/include/alarm.h b/include/alarm.h
index c4ea9d7..e791057 100644
--- a/include/alarm.h
+++ b/include/alarm.h
@@ -24,6 +24,8 @@ void add_alarm(struct alarm_list *alarm, unsigned long sc, unsigned long usc);
 
 void del_alarm(struct alarm_list *alarm);
 
+int alarm_pending(struct alarm_list *alarm, struct timeval *tv);
+
 struct timeval *
 get_next_alarm_run(struct timeval *next_alarm);
 
diff --git a/src/alarm.c b/src/alarm.c
index 7352ccb..7cbae10 100644
--- a/src/alarm.c
+++ b/src/alarm.c
@@ -70,6 +70,16 @@ void del_alarm(struct alarm_list *alarm)
 		list_del_init(&alarm->head);
 }
 
+int alarm_pending(struct alarm_list *alarm, struct timeval *tv)
+{
+	if (list_empty(&alarm->head))
+		return 0;
+
+	if (tv != NULL)
+		*tv = alarm->tv;
+	return 1;
+}
+
 static struct timeval *
 calculate_next_run(struct timeval *cand,
 		   struct timeval *tv, 
diff --git a/src/cache_timer.c b/src/cache_timer.c
index 86bb8fc..a7afbe2 100644
--- a/src/cache_timer.c
+++ b/src/cache_timer.c
@@ -60,8 +60,11 @@ static int timer_dump(struct us_conntrack *u, void *data, char *buf, int type)
 	if (type == NFCT_O_XML)
 		return 0;
 
+	if (!alarm_pending(alarm, &tmp))
+		return 0;
+
 	gettimeofday(&tv, NULL);
-	timersub(&tv, &alarm->tv, &tmp);
+	timersub(&tv, &tmp, &tmp);
 	return sprintf(buf, " [expires in %lds]", tmp.tv_sec);
 }
 

[conntrack-utils PATCH r7285 05/11] added missing ntohs()

[Max Kellermann]

---

 src/sync-mode.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/src/sync-mode.c b/src/sync-mode.c
index 4b2fad7..7db180e 100644
--- a/src/sync-mode.c
+++ b/src/sync-mode.c
@@ -113,8 +113,8 @@ static void mcast_handler(void)
 			return;
 		}
 		do_mcast_handler_step(net);
-		ptr += net->len;
-		remain -= net->len;
+		ptr += ntohs(net->len);
+		remain -= ntohs(net->len);
 	}
 }
 

[conntrack-utils PATCH r7285 06/11] remove obsolete prototypes

[Max Kellermann]

---

 include/network.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)


diff --git a/include/network.h b/include/network.h
index 6dfd79d..e3e9ce1 100644
--- a/include/network.h
+++ b/include/network.h
@@ -55,7 +55,6 @@ struct mcast_sock;
 void build_netmsg(struct nf_conntrack *ct, int query, struct nethdr *net);
 int prepare_send_netmsg(struct mcast_sock *m, void *data);
 int mcast_send_netmsg(struct mcast_sock *m, void *data);
-int mcast_recv_netmsg(struct mcast_sock *m, void *data, int len);
 int handle_netmsg(struct nethdr *net);
 int mcast_track_seq(uint32_t seq, uint32_t *exp_seq);
 

[conntrack-utils PATCH r7285 07/11] use size_t

[Max Kellermann]

---

 include/mcast.h   |    4 ++--
 include/network.h |    7 ++++---
 src/build.c       |    2 +-
 src/local.c       |    4 ++--
 src/mcast.c       |    8 ++++----
 src/network.c     |   22 +++++++++++-----------
 src/sync-alarm.c  |    2 +-
 src/sync-ftfw.c   |    4 ++--
 src/sync-mode.c   |   11 ++++++-----
 9 files changed, 33 insertions(+), 31 deletions(-)


diff --git a/include/mcast.h b/include/mcast.h
index e3cdb38..4e89c72 100644
--- a/include/mcast.h
+++ b/include/mcast.h
@@ -43,8 +43,8 @@ void mcast_server_destroy(struct mcast_sock *m);
 struct mcast_sock *mcast_client_create(struct mcast_conf *conf);
 void mcast_client_destroy(struct mcast_sock *m);
 
-int mcast_send(struct mcast_sock *m, void *data, int size);
-int mcast_recv(struct mcast_sock *m, void *data, int size);
+ssize_t mcast_send(struct mcast_sock *m, void *data, int size);
+ssize_t mcast_recv(struct mcast_sock *m, void *data, int size);
 
 struct mcast_stats *mcast_get_stats(struct mcast_sock *m);
 void mcast_dump_stats(int fd, struct mcast_sock *s, struct mcast_sock *r);
diff --git a/include/network.h b/include/network.h
index e3e9ce1..92a8490 100644
--- a/include/network.h
+++ b/include/network.h
@@ -2,6 +2,7 @@
 #define _NETWORK_H_
 
 #include <stdint.h>
+#include <sys/types.h>
 
 struct nf_conntrack;
 
@@ -53,7 +54,7 @@ struct us_conntrack;
 struct mcast_sock;
 
 void build_netmsg(struct nf_conntrack *ct, int query, struct nethdr *net);
-int prepare_send_netmsg(struct mcast_sock *m, void *data);
+size_t prepare_send_netmsg(struct mcast_sock *m, void *data);
 int mcast_send_netmsg(struct mcast_sock *m, void *data);
 int handle_netmsg(struct nethdr *net);
 int mcast_track_seq(uint32_t seq, uint32_t *exp_seq);
@@ -62,8 +63,8 @@ struct mcast_conf;
 
 int mcast_buffered_init(struct mcast_conf *conf);
 void mcast_buffered_destroy(void);
-int mcast_buffered_send_netmsg(struct mcast_sock *m, void *data, int len);
-int mcast_buffered_pending_netmsg(struct mcast_sock *m);
+int mcast_buffered_send_netmsg(struct mcast_sock *m, void *data, size_t len);
+ssize_t mcast_buffered_pending_netmsg(struct mcast_sock *m);
 
 #define IS_DATA(x)	((x->flags & ~NET_F_HELLO) == 0)
 #define IS_ACK(x)	(x->flags & NET_F_ACK)
diff --git a/src/build.c b/src/build.c
index c99990b..3de1c25 100644
--- a/src/build.c
+++ b/src/build.c
@@ -20,7 +20,7 @@
 #include <libnetfilter_conntrack/libnetfilter_conntrack.h>
 #include "network.h"
 
-static void addattr(struct netpld *pld, int attr, const void *data, int len)
+static void addattr(struct netpld *pld, int attr, const void *data, size_t len)
 {
 	struct netattr *nta;
 	int tlen = NTA_LENGTH(len);
diff --git a/src/local.c b/src/local.c
index 7aab151..f054c0a 100644
--- a/src/local.c
+++ b/src/local.c
@@ -31,7 +31,7 @@ int local_server_create(struct local_server *server, struct local_conf *conf,
 			void (*process)(int fd, void *data))
 {
 	int fd;
-	int len;
+	socklen_t len;
 	struct sockaddr_un local;
 
 	if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
@@ -92,7 +92,7 @@ int do_local_server_step(struct local_server *server)
 
 int local_client_create(struct local_conf *conf)
 {
-	int len;
+	socklen_t len;
 	struct sockaddr_un local;
 	int fd;
 
diff --git a/src/mcast.c b/src/mcast.c
index e977c0b..8307f26 100644
--- a/src/mcast.c
+++ b/src/mcast.c
@@ -238,9 +238,9 @@ void mcast_client_destroy(struct mcast_sock *m)
 	free(m);
 }
 
-int mcast_send(struct mcast_sock *m, void *data, int size)
+ssize_t mcast_send(struct mcast_sock *m, void *data, int size)
 {
-	int ret;
+	ssize_t ret;
 	
 	ret = sendto(m->fd, 
 		     data,
@@ -260,9 +260,9 @@ int mcast_send(struct mcast_sock *m, void *data, int size)
 	return ret;
 }
 
-int mcast_recv(struct mcast_sock *m, void *data, int size)
+ssize_t mcast_recv(struct mcast_sock *m, void *data, int size)
 {
-	int ret;
+	ssize_t ret;
 	socklen_t sin_size = sizeof(struct sockaddr_in);
 
         ret = recvfrom(m->fd,
diff --git a/src/network.c b/src/network.c
index da26545..92999a1 100644
--- a/src/network.c
+++ b/src/network.c
@@ -27,7 +27,7 @@
 
 static unsigned int seq_set, cur_seq;
 
-static int __do_send(struct mcast_sock *m, void *data, int len)
+static size_t __do_send(struct mcast_sock *m, void *data, size_t len)
 {
 	struct nethdr *net = data;
 
@@ -50,7 +50,7 @@ static int __do_send(struct mcast_sock *m, void *data, int len)
 	return mcast_send(m, net, len);
 }
 
-static int __do_prepare(struct mcast_sock *m, void *data, int len)
+static size_t __do_prepare(struct mcast_sock *m, void *data, size_t len)
 {
 	struct nethdr *net = data;
 
@@ -66,12 +66,12 @@ static int __do_prepare(struct mcast_sock *m, void *data, int len)
 	return len;
 }
 
-static int __prepare_ctl(struct mcast_sock *m, void *data)
+static size_t __prepare_ctl(struct mcast_sock *m, void *data)
 {
 	return __do_prepare(m, data, NETHDR_ACK_SIZ);
 }
 
-static int __prepare_data(struct mcast_sock *m, void *data)
+static size_t __prepare_data(struct mcast_sock *m, void *data)
 {
 	struct nethdr *net = (struct nethdr *) data;
 	struct netpld *pld = NETHDR_DATA(net);
@@ -79,7 +79,7 @@ static int __prepare_data(struct mcast_sock *m, void *data)
 	return __do_prepare(m, data, ntohs(pld->len) + NETPLD_SIZ + NETHDR_SIZ);
 }
 
-int prepare_send_netmsg(struct mcast_sock *m, void *data)
+size_t prepare_send_netmsg(struct mcast_sock *m, void *data)
 {
 	int ret = 0;
 	struct nethdr *net = (struct nethdr *) data;
@@ -92,8 +92,8 @@ int prepare_send_netmsg(struct mcast_sock *m, void *data)
 	return ret;
 }
 
-static int tx_buflenmax;
-static int tx_buflen = 0;
+static size_t tx_buflenmax;
+static size_t tx_buflen = 0;
 static char *tx_buf;
 
 #define HEADERSIZ 28 /* IP header (20 bytes) + UDP header 8 (bytes) */
@@ -121,7 +121,7 @@ void mcast_buffered_destroy(void)
 }
 
 /* return 0 if it is not sent, otherwise return 1 */
-int mcast_buffered_send_netmsg(struct mcast_sock *m, void *data, int len)
+int mcast_buffered_send_netmsg(struct mcast_sock *m, void *data, size_t len)
 {
 	int ret = 0;
 	struct nethdr *net = data;
@@ -140,9 +140,9 @@ retry:
 	return ret;
 }
 
-int mcast_buffered_pending_netmsg(struct mcast_sock *m)
+ssize_t mcast_buffered_pending_netmsg(struct mcast_sock *m)
 {
-	int ret;
+	ssize_t ret;
 
 	if (tx_buflen == 0)
 		return 0;
@@ -156,7 +156,7 @@ int mcast_buffered_pending_netmsg(struct mcast_sock *m)
 int mcast_send_netmsg(struct mcast_sock *m, void *data)
 {
 	int ret;
-	int len = prepare_send_netmsg(m, data);
+	size_t len = prepare_send_netmsg(m, data);
 
 	ret = mcast_buffered_send_netmsg(m, data, len);
 	mcast_buffered_pending_netmsg(m);
diff --git a/src/sync-alarm.c b/src/sync-alarm.c
index 66727e7..c7cecc8 100644
--- a/src/sync-alarm.c
+++ b/src/sync-alarm.c
@@ -29,7 +29,7 @@
 
 static void refresher(struct alarm_list *a, void *data)
 {
-	int len;
+	size_t len;
 	struct nethdr *net;
 	struct us_conntrack *u = data;
 
diff --git a/src/sync-ftfw.c b/src/sync-ftfw.c
index 1d12002..49c0b2c 100644
--- a/src/sync-ftfw.c
+++ b/src/sync-ftfw.c
@@ -285,7 +285,7 @@ static void ftfw_send(struct nethdr *net, struct us_conntrack *u)
 static int tx_queue_xmit(void *data1, const void *data2)
 {
 	struct nethdr *net = data1;
-	int len = prepare_send_netmsg(STATE_SYNC(mcast_client), net);
+	size_t len = prepare_send_netmsg(STATE_SYNC(mcast_client), net);
 
 	dp("tx_queue sq: %u fl:%u len:%u\n",
                ntohl(net->seq), ntohs(net->flags), ntohs(net->len));
@@ -307,7 +307,7 @@ static int tx_list_xmit(struct list_head *i, struct us_conntrack *u)
 {
 	int ret;
 	struct nethdr *net = BUILD_NETMSG(u->ct, NFCT_Q_UPDATE);
-	int len = prepare_send_netmsg(STATE_SYNC(mcast_client), net);
+	size_t len = prepare_send_netmsg(STATE_SYNC(mcast_client), net);
 
 	dp("tx_list sq: %u fl:%u len:%u\n",
                 ntohl(net->seq), ntohs(net->flags),
diff --git a/src/sync-mode.c b/src/sync-mode.c
index 7db180e..bf8ddef 100644
--- a/src/sync-mode.c
+++ b/src/sync-mode.c
@@ -88,14 +88,15 @@ retry:
 /* handler for multicast messages received */
 static void mcast_handler(void)
 {
-	int numbytes, remain;
+	ssize_t numbytes;
+	size_t remain;
 	char __net[65536], *ptr = __net; /* XXX: maximum MTU for IPv4 */
 
 	numbytes = mcast_recv(STATE_SYNC(mcast_server), __net, sizeof(__net));
 	if (numbytes <= 0)
 		return;
 
-	remain = numbytes;
+	remain = (size_t)numbytes;
 	while (remain > 0) {
 		struct nethdr *net = (struct nethdr *) ptr;
 
@@ -333,7 +334,7 @@ static void mcast_send_sync(struct us_conntrack *u,
 			    struct nf_conntrack *ct,
 			    int query)
 {
-	int len;
+	size_t len;
 	struct nethdr *net;
 
 	if (!state_helper_verdict(query, ct))
@@ -367,7 +368,7 @@ static int overrun_cb(enum nf_conntrack_msg_type type,
 
 	if (!cache_test(STATE_SYNC(internal), ct)) {
 		if ((u = cache_update_force(STATE_SYNC(internal), ct))) {
-			int len;
+			size_t len;
 
 			debug_ct(u->ct, "overrun resync");
 
@@ -391,7 +392,7 @@ static int overrun_purge_step(void *data1, void *data2)
 
 	ret = nfct_query(h, NFCT_Q_GET, u->ct);
 	if (ret == -1 && errno == ENOENT) {
-		int len;
+		size_t len;
 		struct nethdr *net = BUILD_NETMSG(u->ct, NFCT_Q_DESTROY);
 
 		debug_ct(u->ct, "overrun purge resync");

[conntrack-utils PATCH r7285 08/11] check if the received packet is large enough

[Max Kellermann]

---

 src/sync-mode.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)


diff --git a/src/sync-mode.c b/src/sync-mode.c
index bf8ddef..5f78bfb 100644
--- a/src/sync-mode.c
+++ b/src/sync-mode.c
@@ -100,6 +100,11 @@ static void mcast_handler(void)
 	while (remain > 0) {
 		struct nethdr *net = (struct nethdr *) ptr;
 
+		if (remain < sizeof(*net)) {
+			dlog(LOG_ERR, "packet too small");
+			break;
+		}
+
 		if (ntohs(net->len) > remain) {
 			dlog(LOG_ERR, "fragmented messages");
 			break;

[conntrack-utils PATCH r7285 09/11] added handler callback to mcast_sock

[Max Kellermann]

---

 include/mcast.h |   11 ++++++++++-
 src/mcast.c     |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 src/sync-mode.c |   48 +++++++++++-------------------------------------
 3 files changed, 70 insertions(+), 39 deletions(-)


diff --git a/include/mcast.h b/include/mcast.h
index 4e89c72..4d4124f 100644
--- a/include/mcast.h
+++ b/include/mcast.h
@@ -5,6 +5,8 @@
 #include <netinet/in.h>
 #include <net/if.h>
 
+struct nethdr;
+
 struct mcast_conf {
 	int ipproto;
 	int reuseaddr;
@@ -35,9 +37,14 @@ struct mcast_sock {
 		struct sockaddr_in6 ipv6;
 	} addr;
 	struct mcast_stats stats;
+
+	void *data;
+	int (*handler)(struct nethdr *net, void *data);
 };
 
-struct mcast_sock *mcast_server_create(struct mcast_conf *conf);
+struct mcast_sock *mcast_server_create(struct mcast_conf *conf,
+				       void *data,
+				       int (*handler)(struct nethdr *net, void *data));
 void mcast_server_destroy(struct mcast_sock *m);
 
 struct mcast_sock *mcast_client_create(struct mcast_conf *conf);
@@ -46,6 +53,8 @@ void mcast_client_destroy(struct mcast_sock *m);
 ssize_t mcast_send(struct mcast_sock *m, void *data, int size);
 ssize_t mcast_recv(struct mcast_sock *m, void *data, int size);
 
+int mcast_run(struct mcast_sock *m);
+
 struct mcast_stats *mcast_get_stats(struct mcast_sock *m);
 void mcast_dump_stats(int fd, struct mcast_sock *s, struct mcast_sock *r);
 
diff --git a/src/mcast.c b/src/mcast.c
index 8307f26..86de962 100644
--- a/src/mcast.c
+++ b/src/mcast.c
@@ -19,9 +19,12 @@
  */
 
 #include "mcast.h"
+#include "network.h"
+#include "log.h"
 #include "debug.h"
 
 #include <stdio.h>
+#include <syslog.h>
 #include <stdlib.h>
 #include <arpa/inet.h>
 #include <unistd.h>
@@ -29,7 +32,9 @@
 #include <sys/ioctl.h>
 #include <net/if.h>
 
-struct mcast_sock *mcast_server_create(struct mcast_conf *conf)
+struct mcast_sock *mcast_server_create(struct mcast_conf *conf,
+				       void *data,
+				       int (*handler)(struct nethdr *net, void *data))
 {
 	int yes = 1;
 	union {
@@ -122,6 +127,9 @@ struct mcast_sock *mcast_server_create(struct mcast_conf *conf)
 		break;
 	}
 
+	m->data = data;
+	m->handler = handler;
+
 	return m;
 }
 
@@ -283,6 +291,46 @@ ssize_t mcast_recv(struct mcast_sock *m, void *data, int size)
 	return ret;
 }
 
+int mcast_run(struct mcast_sock *m)
+{
+	ssize_t numbytes;
+	size_t remain;
+	int ret;
+	char __net[65536], *ptr = __net; /* XXX: maximum MTU for IPv4 */
+
+	numbytes = mcast_recv(m, __net, sizeof(__net));
+	if (numbytes <= 0)
+		return -1;
+
+	remain = (size_t)numbytes;
+	while (remain > 0) {
+		struct nethdr *net = (struct nethdr *) ptr;
+
+		if (remain < sizeof(*net)) {
+			dlog(LOG_ERR, "packet too small");
+			break;
+		}
+
+		if (ntohs(net->len) > remain) {
+			dlog(LOG_ERR, "fragmented messages");
+			break;
+		}
+
+		debug("recv sq: %u fl:%u len:%u (rem:%d)\n", 
+			ntohl(net->seq), ntohs(net->flags),
+			ntohs(net->len), remain);
+
+		ret = m->handler(net, m->data);
+		if (ret < 0)
+			return ret;
+
+		ptr += ntohs(net->len);
+		remain -= ntohs(net->len);
+	}
+
+	return 0;
+}
+
 struct mcast_stats *mcast_get_stats(struct mcast_sock *m)
 {
 	return &m->stats;
diff --git a/src/sync-mode.c b/src/sync-mode.c
index 5f78bfb..7811d3d 100644
--- a/src/sync-mode.c
+++ b/src/sync-mode.c
@@ -85,43 +85,16 @@ retry:
 	}
 }
 
-/* handler for multicast messages received */
-static void mcast_handler(void)
+static int
+mcast_handler(struct nethdr *net, void *data)
 {
-	ssize_t numbytes;
-	size_t remain;
-	char __net[65536], *ptr = __net; /* XXX: maximum MTU for IPv4 */
-
-	numbytes = mcast_recv(STATE_SYNC(mcast_server), __net, sizeof(__net));
-	if (numbytes <= 0)
-		return;
-
-	remain = (size_t)numbytes;
-	while (remain > 0) {
-		struct nethdr *net = (struct nethdr *) ptr;
-
-		if (remain < sizeof(*net)) {
-			dlog(LOG_ERR, "packet too small");
-			break;
-		}
-
-		if (ntohs(net->len) > remain) {
-			dlog(LOG_ERR, "fragmented messages");
-			break;
-		}
-
-		debug("recv sq: %u fl:%u len:%u (rem:%d)\n", 
-			ntohl(net->seq), ntohs(net->flags),
-			ntohs(net->len), remain);
-
-		if (handle_netmsg(net) == -1) {
-			STATE(malformed)++;
-			return;
-		}
-		do_mcast_handler_step(net);
-		ptr += ntohs(net->len);
-		remain -= ntohs(net->len);
+	if (handle_netmsg(net) == -1) {
+		STATE(malformed)++;
+		return -1;
 	}
+
+	do_mcast_handler_step(net);
+	return 0;
 }
 
 static int init_sync(void)
@@ -176,7 +149,8 @@ static int init_sync(void)
 	}
 
 	/* multicast server to receive events from the wire */
-	STATE_SYNC(mcast_server) = mcast_server_create(&CONFIG(mcast));
+	STATE_SYNC(mcast_server) = mcast_server_create(&CONFIG(mcast),
+						       NULL, mcast_handler);
 	if (STATE_SYNC(mcast_server) == NULL) {
 		dlog(LOG_ERR, "can't open multicast server!");
 		return -1;
@@ -214,7 +188,7 @@ static void run_sync(fd_set *readfds)
 {
 	/* multicast packet has been received */
 	if (FD_ISSET(STATE_SYNC(mcast_server->fd), readfds))
-		mcast_handler();
+		mcast_run(STATE_SYNC(mcast_server));
 
 	if (STATE_SYNC(sync)->run)
 		STATE_SYNC(sync)->run();

[conntrack-utils PATCH r7285 10/11] use libevent

[Max Kellermann]

---

 include/alarm.h      |   18 +----
 include/conntrackd.h |    4 -
 include/local.h      |    5 +
 include/mcast.h      |    5 +
 src/Makefile.am      |    2 -
 src/alarm.c          |  173 +++++---------------------------------------------
 src/local.c          |   16 +++--
 src/main.c           |   12 +++
 src/mcast.c          |   19 +++++
 src/run.c            |  167 ++++++++++++++++++------------------------------
 src/stats-mode.c     |    1 
 src/sync-mode.c      |   14 ----
 12 files changed, 135 insertions(+), 301 deletions(-)


diff --git a/include/alarm.h b/include/alarm.h
index e791057..5b59994 100644
--- a/include/alarm.h
+++ b/include/alarm.h
@@ -1,21 +1,15 @@
 #ifndef _TIMER_H_
 #define _TIMER_H_
 
-#include "linux_list.h"
-
-#include <sys/time.h>
+#include <sys/types.h>
+#include <event.h>
 
 struct alarm_list {
-	struct list_head	head;
-	struct timeval		tv;
+	struct event event;
 	void			*data;
 	void			(*function)(struct alarm_list *a, void *data);
 };
 
-int init_alarm_hash(void);
-
-void destroy_alarm_hash(void);
-
 void init_alarm(struct alarm_list *t,
 		void *data,
 		void (*fcn)(struct alarm_list *a, void *data));
@@ -26,10 +20,4 @@ void del_alarm(struct alarm_list *alarm);
 
 int alarm_pending(struct alarm_list *alarm, struct timeval *tv);
 
-struct timeval *
-get_next_alarm_run(struct timeval *next_alarm);
-
-struct timeval *
-do_alarm_run(struct timeval *next_alarm);
-
 #endif
diff --git a/include/conntrackd.h b/include/conntrackd.h
index 47898e2..beeea9e 100644
--- a/include/conntrackd.h
+++ b/include/conntrackd.h
@@ -95,7 +95,6 @@ struct ct_conf {
 #define STATE(x) st.x
 
 struct ct_general_state {
-	sigset_t 			block;
 	FILE 				*log;
 	FILE				*stats_log;
 	struct local_server		local;
@@ -151,8 +150,7 @@ extern struct ct_general_state st;
 
 struct ct_mode {
 	int (*init)(void);
-	int (*add_fds_to_set)(fd_set *readfds);
-	void (*run)(fd_set *readfds);
+	void (*run)(void);
 	int (*local)(int fd, int type, void *data);
 	void (*kill)(void);
 	void (*dump)(struct nf_conntrack *ct);
diff --git a/include/local.h b/include/local.h
index c9a57b8..65346f4 100644
--- a/include/local.h
+++ b/include/local.h
@@ -1,6 +1,9 @@
 #ifndef _LOCAL_SOCKET_H_
 #define _LOCAL_SOCKET_H_
 
+#include <sys/types.h>
+#include <event.h>
+
 #ifndef UNIX_PATH_MAX
 #define UNIX_PATH_MAX   108
 #endif
@@ -13,6 +16,7 @@ struct local_conf {
 
 struct local_server {
 	int fd;
+	struct event event;
 	const char *path;
 
 	void *data;
@@ -24,7 +28,6 @@ int local_server_create(struct local_server *server, struct local_conf *conf,
 			void *data,
 			void (*process)(int fd, void *data));
 void local_server_destroy(struct local_server *server);
-int do_local_server_step(struct local_server *server);
 
 /* local client */
 int local_client_create(struct local_conf *conf);
diff --git a/include/mcast.h b/include/mcast.h
index 4d4124f..cd0324b 100644
--- a/include/mcast.h
+++ b/include/mcast.h
@@ -4,6 +4,8 @@
 #include <stdint.h>
 #include <netinet/in.h>
 #include <net/if.h>
+#include <sys/types.h>
+#include <event.h>
 
 struct nethdr;
 
@@ -32,6 +34,7 @@ struct mcast_stats {
 
 struct mcast_sock {
 	int fd;
+	struct event event;
 	union {
 		struct sockaddr_in ipv4;
 		struct sockaddr_in6 ipv6;
@@ -53,8 +56,6 @@ void mcast_client_destroy(struct mcast_sock *m);
 ssize_t mcast_send(struct mcast_sock *m, void *data, int size);
 ssize_t mcast_recv(struct mcast_sock *m, void *data, int size);
 
-int mcast_run(struct mcast_sock *m);
-
 struct mcast_stats *mcast_get_stats(struct mcast_sock *m);
 void mcast_dump_stats(int fd, struct mcast_sock *s, struct mcast_sock *r);
 
diff --git a/src/Makefile.am b/src/Makefile.am
index 15628b7..d71cd55 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -25,6 +25,6 @@ conntrackd_SOURCES = alarm.c main.c run.c hash.c queue.c \
 # yacc and lex generate dirty code
 read_config_yy.o read_config_lex.o: AM_CFLAGS += -Wno-missing-prototypes -Wno-missing-declarations -Wno-implicit-function-declaration -Wno-nested-externs -Wno-undef -Wno-redundant-decls
 
-conntrackd_LDFLAGS = $(all_libraries) @LIBNETFILTER_CONNTRACK_LIBS@
+conntrackd_LDFLAGS = $(all_libraries) @LIBNETFILTER_CONNTRACK_LIBS@ -levent
 
 EXTRA_DIST = read_config_yy.h
diff --git a/src/alarm.c b/src/alarm.c
index 7cbae10..b0fd28f 100644
--- a/src/alarm.c
+++ b/src/alarm.c
@@ -17,185 +17,44 @@
  */
 
 #include "alarm.h"
-#include "jhash.h"
+
 #include <stdlib.h>
-#include <limits.h>
 
-#define ALARM_HASH_SIZE		2048
+static void
+alarm_event_callback(int fd, short event, void *ctx)
+{
+	struct alarm_list *alarm = ctx;
 
-static struct list_head *alarm_hash;
+	alarm->function(alarm, alarm->data);
+}
 
 void init_alarm(struct alarm_list *t,
 		void *data,
 		void (*fcn)(struct alarm_list *a, void *data))
 {
-	/* initialize the head to check whether a node is inserted */
-	INIT_LIST_HEAD(&t->head);
-	timerclear(&t->tv);
+	evtimer_set(&t->event, alarm_event_callback, t);
 	t->data = data;
 	t->function = fcn;
 }
 
-static void
-__add_alarm(struct alarm_list *alarm)
-{
-	struct alarm_list *t;
-	int i = jhash(alarm, sizeof(alarm), 0) % ALARM_HASH_SIZE;
-
-	list_for_each_entry(t, &alarm_hash[i], head) {
-		if (timercmp(&alarm->tv, &t->tv, <)) {
-			list_add_tail(&alarm->head, &t->head);
-			return;
-		}
-	}
-	list_add_tail(&alarm->head, &alarm_hash[i]);
-}
-
 void add_alarm(struct alarm_list *alarm, unsigned long sc, unsigned long usc)
 {
 	struct timeval tv;
 
-	del_alarm(alarm);
-	alarm->tv.tv_sec = sc;
-	alarm->tv.tv_usec = usc;
-	gettimeofday(&tv, NULL);
-	timeradd(&alarm->tv, &tv, &alarm->tv);
-	__add_alarm(alarm);
-}
-
-void del_alarm(struct alarm_list *alarm)
-{
-	/* don't remove a non-inserted node */
-	if (!list_empty(&alarm->head))
-		list_del_init(&alarm->head);
-}
-
-int alarm_pending(struct alarm_list *alarm, struct timeval *tv)
-{
-	if (list_empty(&alarm->head))
-		return 0;
-
-	if (tv != NULL)
-		*tv = alarm->tv;
-	return 1;
-}
-
-static struct timeval *
-calculate_next_run(struct timeval *cand,
-		   struct timeval *tv, 
-		   struct timeval *next_run)
-{
-	if (cand->tv_sec != LONG_MAX) {
-		if (timercmp(cand, tv, >))
-			timersub(cand, tv, next_run);
-		else {
-			/* loop again inmediately */
-			next_run->tv_sec = 0;
-			next_run->tv_usec = 0;
-		}
-		return next_run;
-	}
-	return NULL;
-}
-
-struct timeval *
-get_next_alarm_run(struct timeval *next_run)
-{
-	int i;
-	struct alarm_list *t;
-	struct timeval tv;
-	struct timeval cand = {
-		.tv_sec = LONG_MAX,
-		.tv_usec = LONG_MAX
-	};
-
-	gettimeofday(&tv, NULL);
-
-	for (i=0; i<ALARM_HASH_SIZE; i++) {
-		if (!list_empty(&alarm_hash[i])) {
-			t = list_entry(alarm_hash[i].next, 
-				       struct alarm_list,
-				       head);
-			if (timercmp(&t->tv, &cand, <)) {
-				cand.tv_sec = t->tv.tv_sec;
-				cand.tv_usec = t->tv.tv_usec;
-			}
-		}
-	}
-
-	return calculate_next_run(&cand, &tv, next_run);
-}
-
-static inline int 
-tv_compare(struct alarm_list *a, struct timeval *cur, struct timeval *cand)
-{
-	if (timercmp(&a->tv, cur, >)) {
-		/* select the next alarm candidate */
-		if (timercmp(&a->tv, cand, <)) {
-			cand->tv_sec = a->tv.tv_sec;
-			cand->tv_usec = a->tv.tv_usec;
-		}
-		return 1;
-	}
-	return 0;
-}
-
-struct timeval *
-do_alarm_run(struct timeval *next_run)
-{
-	int i;
-	struct alarm_list *t, *next, *prev;
-	struct timeval tv;
-	struct timeval cand = {
-		.tv_sec = LONG_MAX,
-		.tv_usec = LONG_MAX
-	};
-
-	gettimeofday(&tv, NULL);
-
-	for (i=0; i<ALARM_HASH_SIZE; i++) {
-		list_for_each_entry_safe(t, next, &alarm_hash[i], head) {
-			if (tv_compare(t, &tv, &cand))
-				break;
-
-			/* annotate previous alarm */
-			prev = list_entry(next->head.prev,
-					  struct alarm_list,
-					  head);
-
-			del_alarm(t);
-			t->function(t, t->data);
+	evtimer_del(&alarm->event);
 
-			/* Special case: One deleted node is inserted 
-			 * again in the same place */
-			if (next->head.prev == &prev->head) {
-				t = list_entry(next->head.prev,
-					       struct alarm_list,
-					       head);
-				if (tv_compare(t, &tv, &cand))
-					break;
-			}
-		}
-	}
+	tv.tv_sec = sc;
+	tv.tv_usec = usc;
 
-	return calculate_next_run(&cand, &tv, next_run);
+	evtimer_add(&alarm->event, &tv);
 }
 
-int init_alarm_hash(void)
+void del_alarm(struct alarm_list *alarm)
 {
-	int i;
-
-	alarm_hash = malloc(sizeof(struct list_head) * ALARM_HASH_SIZE);
-	if (alarm_hash == NULL)
-		return -1;
-
-	for (i=0; i<ALARM_HASH_SIZE; i++)
-		INIT_LIST_HEAD(&alarm_hash[i]);
-
-	return 0;
+	evtimer_del(&alarm->event);
 }
 
-void destroy_alarm_hash(void)
+int alarm_pending(struct alarm_list *alarm, struct timeval *tv)
 {
-	free(alarm_hash);
+	return evtimer_pending(&alarm->event, tv) != 0;
 }
diff --git a/src/local.c b/src/local.c
index f054c0a..7bec5ab 100644
--- a/src/local.c
+++ b/src/local.c
@@ -26,6 +26,9 @@
 #include <arpa/inet.h>
 #include <sys/un.h>
 
+static void
+local_server_event_callback(int fd, short event, void *ctx);
+
 int local_server_create(struct local_server *server, struct local_conf *conf,
 			void *data,
 			void (*process)(int fd, void *data))
@@ -66,28 +69,33 @@ int local_server_create(struct local_server *server, struct local_conf *conf,
 	server->data = data;
 	server->process = process;
 
+	event_set(&server->event, server->fd,
+		  EV_READ|EV_PERSIST, local_server_event_callback, server);
+	event_add(&server->event, NULL);
+
 	return 0;
 }
 
 void local_server_destroy(struct local_server *server)
 {
 	unlink(server->path);
+	event_del(&server->event);
 	close(server->fd);
 }
 
-int do_local_server_step(struct local_server *server)
+static void
+local_server_event_callback(int fd, short event, void *ctx)
 {
+	struct local_server *server = ctx;
 	int rfd;
 	struct sockaddr_un local;
 	socklen_t sin_size = sizeof(struct sockaddr_un);
 	
 	if ((rfd = accept(server->fd, (struct sockaddr *)&local, &sin_size)) == -1)
-		return -1;
+		return;
 
 	server->process(rfd, server->data);
 	close(rfd);
-
-	return 0;
 }
 
 int local_client_create(struct local_conf *conf)
diff --git a/src/main.c b/src/main.c
index 8221564..062a4cb 100644
--- a/src/main.c
+++ b/src/main.c
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <unistd.h>
+#include <event.h>
 
 struct ct_general_state st;
 union ct_state state;
@@ -82,6 +83,7 @@ int main(int argc, char *argv[])
 	int type = 0;
 	struct utsname u;
 	int version, major, minor;
+	struct event_base *event_base;
 
 	/* Check kernel version: it must be >= 2.6.18 */
 	if (uname(&u) == -1) {
@@ -224,6 +226,8 @@ int main(int argc, char *argv[])
 	 * initialization process
 	 */
 
+	event_base = event_init();
+
 	if (init() == -1) {
 		close_log();
 		fprintf(stderr, "ERROR: conntrackd cannot start, please "
@@ -257,6 +261,14 @@ int main(int argc, char *argv[])
 	/*
 	 * run main process
 	 */
+
 	run();
+
+	/*
+	 * cleanup
+	 */
+
+	event_base_free(event_base);
+
 	return 0;
 }
diff --git a/src/mcast.c b/src/mcast.c
index 86de962..eed8652 100644
--- a/src/mcast.c
+++ b/src/mcast.c
@@ -32,6 +32,16 @@
 #include <sys/ioctl.h>
 #include <net/if.h>
 
+static int mcast_run(struct mcast_sock *m);
+
+static void
+mcast_server_event_callback(int fd, short event, void *ctx)
+{
+	struct mcast_sock *m = ctx;
+
+	mcast_run(m);
+}
+
 struct mcast_sock *mcast_server_create(struct mcast_conf *conf,
 				       void *data,
 				       int (*handler)(struct nethdr *net, void *data))
@@ -130,11 +140,18 @@ struct mcast_sock *mcast_server_create(struct mcast_conf *conf,
 	m->data = data;
 	m->handler = handler;
 
+	event_set(&m->event, m->fd,
+		  EV_READ|EV_PERSIST, mcast_server_event_callback, m);
+	event_add(&m->event, NULL);
+
+	m->handler = handler;
+
 	return m;
 }
 
 void mcast_server_destroy(struct mcast_sock *m)
 {
+	event_del(&m->event);
 	close(m->fd);
 	free(m);
 }
@@ -291,7 +308,7 @@ ssize_t mcast_recv(struct mcast_sock *m, void *data, int size)
 	return ret;
 }
 
-int mcast_run(struct mcast_sock *m)
+static int mcast_run(struct mcast_sock *m)
 {
 	ssize_t numbytes;
 	size_t remain;
diff --git a/src/run.c b/src/run.c
index fcba393..9631481 100644
--- a/src/run.c
+++ b/src/run.c
@@ -30,33 +30,71 @@
 #include <unistd.h>
 #include <sys/wait.h>
 #include <string.h>
+#include <event.h>
 
 void killer(int foo)
 {
-	/* no signals while handling signals */
-	sigprocmask(SIG_BLOCK, &STATE(block), NULL);
-
 	nfct_close(STATE(event));
 	nfct_close(STATE(dump));
 
 	ignore_pool_destroy(STATE(ignore_pool));
 	local_server_destroy(&STATE(local));
 	STATE(mode)->kill();
-	destroy_alarm_hash();
 	unlink(CONFIG(lockfile));
 	dlog(LOG_NOTICE, "---- shutdown received ----");
 	close_log();
 
-	sigprocmask(SIG_UNBLOCK, &STATE(block), NULL);
-
 	exit(0);			
 }
 
-static void child(int foo)
+static void
+exit_event_callback(int fd, short event, void *ctx)
+{
+	killer(0);
+}
+
+static void
+child_event_callback(int fd, short event, void *ctx)
 {
 	while(wait(NULL) > 0);
 }
 
+static void
+nfct_event_callback(int fd, short event, void *ctx)
+{
+	int ret;
+
+	while ((ret = nfct_catch(STATE(event))) != -1);
+	if (ret == -1) {
+		switch(errno) {
+		case ENOBUFS:
+			/*
+			 * It seems that ctnetlink can't back off,
+			 * it's likely that we're losing events.
+			 * Solution: duplicate the socket buffer
+			 * size and resync with master conntrack table.
+			 */
+			nl_resize_socket_buffer(STATE(event));
+			/* XXX: schedule overrun call via alarm */
+			STATE(mode)->overrun();
+			break;
+		case ENOENT:
+			/*
+			 * We received a message from another
+			 * netfilter subsystem that we are not
+			 * interested in. Just ignore it.
+			 */
+			break;
+		case EAGAIN:
+			break;
+		default:
+			dlog(LOG_WARNING,
+			     "event catch says: %s", strerror(errno));
+			break;
+		}
+	}
+}
+
 void local_handler(int fd, void *data)
 {
 	int ret;
@@ -92,6 +130,8 @@ void local_handler(int fd, void *data)
 int
 init(void)
 {
+	static struct event nfct_event;
+	static struct event sigint_event, sigterm_event, sigchld_event;
 	int ret;
 
 	if (CONFIG(flags) & CTD_STATS_MODE)
@@ -105,11 +145,6 @@ init(void)
 		STATE(mode) = &stats_mode;
 	}
 
-	if (init_alarm_hash() == -1) {
-		dlog(LOG_ERR, "can't initialize alarm hash");
-		return -1;
-	}
-
 	/* Initialization */
 	if (STATE(mode)->init() == -1) {
 		dlog(LOG_ERR, "initialization failed");
@@ -131,6 +166,10 @@ init(void)
 		return -1;
 	}
 
+	event_set(&nfct_event, nfct_fd(STATE(event)),
+		  EV_READ|EV_PERSIST, nfct_event_callback, NULL);
+	event_add(&nfct_event, NULL);
+
 	if (nl_init_dump_handler() == -1) {
 		dlog(LOG_ERR, "can't open netlink handler: %s",
 		     strerror(errno));
@@ -139,113 +178,35 @@ init(void)
 	}
 
         /* Signals handling */
-	sigemptyset(&STATE(block));
-	sigaddset(&STATE(block), SIGTERM);
-	sigaddset(&STATE(block), SIGINT);
-	sigaddset(&STATE(block), SIGCHLD);
 
-	if (signal(SIGINT, killer) == SIG_ERR)
-		return -1;
+	event_set(&sigint_event, SIGTERM, EV_SIGNAL|EV_PERSIST,
+		  exit_event_callback, NULL);
+	event_add(&sigint_event, NULL);
 
-	if (signal(SIGTERM, killer) == SIG_ERR)
-		return -1;
+	event_set(&sigterm_event, SIGTERM, EV_SIGNAL|EV_PERSIST,
+		  exit_event_callback, NULL);
+	event_add(&sigterm_event, NULL);
 
 	/* ignore connection reset by peer */
 	if (signal(SIGPIPE, SIG_IGN) == SIG_ERR)
 		return -1;
 
-	if (signal(SIGCHLD, child) == SIG_ERR)
-		return -1;
+	event_set(&sigchld_event, SIGCHLD, EV_SIGNAL|EV_PERSIST,
+		  child_event_callback, NULL);
+	event_add(&sigchld_event, NULL);
 
 	dlog(LOG_NOTICE, "initialization completed");
 
 	return 0;
 }
 
-static void __run(struct timeval *next_alarm)
-{
-	int max, ret;
-	fd_set readfds;
-
-	FD_ZERO(&readfds);
-	FD_SET(STATE(local).fd, &readfds);
-	FD_SET(nfct_fd(STATE(event)), &readfds);
-
-	max = MAX(STATE(local).fd, nfct_fd(STATE(event)));
-
-	if (STATE(mode)->add_fds_to_set)
-		max = MAX(max, STATE(mode)->add_fds_to_set(&readfds));
-
-	ret = select(max+1, &readfds, NULL, NULL, next_alarm);
-	if (ret == -1) {
-		/* interrupted syscall, retry */
-		if (errno == EINTR)
-			return;
-
-		dlog(LOG_WARNING, "select failed: %s", strerror(errno));
-		return;
-	}
-
-	/* signals are racy */
-	sigprocmask(SIG_BLOCK, &STATE(block), NULL);		
-
-	/* order received via UNIX socket */
-	if (FD_ISSET(STATE(local).fd, &readfds))
-		do_local_server_step(&STATE(local));
-
-	/* conntrack event has happened */
-	if (FD_ISSET(nfct_fd(STATE(event)), &readfds)) {
-		while ((ret = nfct_catch(STATE(event))) != -1);
-		if (ret == -1) {
-			switch(errno) {
-			case ENOBUFS:
-                		/*
-		 		 * It seems that ctnetlink can't back off,
-				 * it's likely that we're losing events.
-				 * Solution: duplicate the socket buffer
-				 * size and resync with master conntrack table.
-				 */
-				nl_resize_socket_buffer(STATE(event));
-				/* XXX: schedule overrun call via alarm */
-				STATE(mode)->overrun();
-				break;
-			case ENOENT:
-				/*
-				 * We received a message from another
-				 * netfilter subsystem that we are not
-				 * interested in. Just ignore it.
-				 */
-				break;
-			case EAGAIN:
-				break;
-			default:
-				dlog(LOG_WARNING,
-				     "event catch says: %s", strerror(errno));
-				break;
-			}
-		}
-	}
-
-	if (STATE(mode)->run)
-		STATE(mode)->run(&readfds);
-
-	sigprocmask(SIG_UNBLOCK, &STATE(block), NULL);
-}
-
 void __attribute__((noreturn))
 run(void)
 {
-	struct timeval next_alarm; 
-	struct timeval *next = NULL;
-
 	while(1) {
-		sigprocmask(SIG_BLOCK, &STATE(block), NULL);
-		if (next != NULL && !timerisset(next))
-			next = do_alarm_run(&next_alarm);
-		else
-			next = get_next_alarm_run(&next_alarm);
-		sigprocmask(SIG_UNBLOCK, &STATE(block), NULL);
-
-		__run(next);
+		event_loop(EVLOOP_ONCE);
+
+		if (STATE(mode)->run)
+			STATE(mode)->run();
 	}
 }
diff --git a/src/stats-mode.c b/src/stats-mode.c
index 9e6089c..de9f146 100644
--- a/src/stats-mode.c
+++ b/src/stats-mode.c
@@ -182,7 +182,6 @@ static int event_destroy_stats(struct nf_conntrack *ct)
 
 struct ct_mode stats_mode = {
 	.init 			= init_stats,
-	.add_fds_to_set 	= NULL,
 	.run			= NULL,
 	.local			= local_handler_stats,
 	.kill			= kill_stats,
diff --git a/src/sync-mode.c b/src/sync-mode.c
index 7811d3d..60960da 100644
--- a/src/sync-mode.c
+++ b/src/sync-mode.c
@@ -177,19 +177,8 @@ static int init_sync(void)
 	return 0;
 }
 
-static int add_fds_to_set_sync(fd_set *readfds) 
+static void run_sync(void)
 {
-	FD_SET(STATE_SYNC(mcast_server->fd), readfds);
-
-	return STATE_SYNC(mcast_server->fd);
-}
-
-static void run_sync(fd_set *readfds)
-{
-	/* multicast packet has been received */
-	if (FD_ISSET(STATE_SYNC(mcast_server->fd), readfds))
-		mcast_run(STATE_SYNC(mcast_server));
-
 	if (STATE_SYNC(sync)->run)
 		STATE_SYNC(sync)->run();
 
@@ -475,7 +464,6 @@ static int event_destroy_sync(struct nf_conntrack *ct)
 
 struct ct_mode sync_mode = {
 	.init 			= init_sync,
-	.add_fds_to_set 	= add_fds_to_set_sync,
 	.run			= run_sync,
 	.local			= local_handler_sync,
 	.kill			= kill_sync,

[conntrack-utils PATCH r7285 11/11] remove the alarm library

[Max Kellermann]

---

 include/Makefile.am |    2 +-
 include/alarm.h     |   23 --------------------
 src/Makefile.am     |    2 +-
 src/alarm.c         |   60 ---------------------------------------------------
 src/cache_timer.c   |   36 ++++++++++++++++++++-----------
 src/run.c           |    1 -
 src/sync-alarm.c    |   56 ++++++++++++++++++++++++++++++++----------------
 src/sync-ftfw.c     |   23 ++++++++++++++------
 8 files changed, 81 insertions(+), 122 deletions(-)
 delete mode 100644 include/alarm.h
 delete mode 100644 src/alarm.c


diff --git a/include/Makefile.am b/include/Makefile.am
index e8e7f81..906ce20 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -1,5 +1,5 @@
 
-noinst_HEADERS = alarm.h jhash.h slist.h cache.h linux_list.h \
+noinst_HEADERS = jhash.h slist.h cache.h linux_list.h \
 		 sync.h conntrackd.h local.h us-conntrack.h \
 		 debug.h log.h hash.h mcast.h conntrack.h \
 		 state_helper.h network.h ignore.h queue.h \
diff --git a/include/alarm.h b/include/alarm.h
deleted file mode 100644
index 5b59994..0000000
--- a/include/alarm.h
+++ /dev/null
@@ -1,23 +0,0 @@
-#ifndef _TIMER_H_
-#define _TIMER_H_
-
-#include <sys/types.h>
-#include <event.h>
-
-struct alarm_list {
-	struct event event;
-	void			*data;
-	void			(*function)(struct alarm_list *a, void *data);
-};
-
-void init_alarm(struct alarm_list *t,
-		void *data,
-		void (*fcn)(struct alarm_list *a, void *data));
-
-void add_alarm(struct alarm_list *alarm, unsigned long sc, unsigned long usc);
-
-void del_alarm(struct alarm_list *alarm);
-
-int alarm_pending(struct alarm_list *alarm, struct timeval *tv);
-
-#endif
diff --git a/src/Makefile.am b/src/Makefile.am
index d71cd55..2264fd1 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -10,7 +10,7 @@ conntrack_SOURCES = conntrack.c
 conntrack_LDADD = ../extensions/libct_proto_tcp.la ../extensions/libct_proto_udp.la ../extensions/libct_proto_icmp.la
 conntrack_LDFLAGS = $(all_libraries) @LIBNETFILTER_CONNTRACK_LIBS@
 
-conntrackd_SOURCES = alarm.c main.c run.c hash.c queue.c \
+conntrackd_SOURCES = main.c run.c hash.c queue.c \
 		    local.c log.c mcast.c netlink.c \
 		    ignore_pool.c \
 		    cache.c cache_iterators.c \
diff --git a/src/alarm.c b/src/alarm.c
deleted file mode 100644
index b0fd28f..0000000
--- a/src/alarm.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * (C) 2006 by Pablo Neira Ayuso <pablo@netfilter.org>
- * 
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- */
-
-#include "alarm.h"
-
-#include <stdlib.h>
-
-static void
-alarm_event_callback(int fd, short event, void *ctx)
-{
-	struct alarm_list *alarm = ctx;
-
-	alarm->function(alarm, alarm->data);
-}
-
-void init_alarm(struct alarm_list *t,
-		void *data,
-		void (*fcn)(struct alarm_list *a, void *data))
-{
-	evtimer_set(&t->event, alarm_event_callback, t);
-	t->data = data;
-	t->function = fcn;
-}
-
-void add_alarm(struct alarm_list *alarm, unsigned long sc, unsigned long usc)
-{
-	struct timeval tv;
-
-	evtimer_del(&alarm->event);
-
-	tv.tv_sec = sc;
-	tv.tv_usec = usc;
-
-	evtimer_add(&alarm->event, &tv);
-}
-
-void del_alarm(struct alarm_list *alarm)
-{
-	evtimer_del(&alarm->event);
-}
-
-int alarm_pending(struct alarm_list *alarm, struct timeval *tv)
-{
-	return evtimer_pending(&alarm->event, tv) != 0;
-}
diff --git a/src/cache_timer.c b/src/cache_timer.c
index a7afbe2..f3efec4 100644
--- a/src/cache_timer.c
+++ b/src/cache_timer.c
@@ -19,12 +19,13 @@
 #include "cache.h"
 #include "conntrackd.h"
 #include "us-conntrack.h"
-#include "alarm.h"
 #include "debug.h"
 
 #include <stdio.h>
+#include <sys/types.h>
+#include <event.h>
 
-static void timeout(struct alarm_list *a, void *data)
+static void timeout(int fd, short event, void *data)
 {
 	struct us_conntrack *u = data;
 
@@ -34,33 +35,44 @@ static void timeout(struct alarm_list *a, void *data)
 
 static void timer_add(struct us_conntrack *u, void *data)
 {
-	struct alarm_list *alarm = data;
+	struct event *event = data;
+	struct timeval tv = {
+		.tv_sec = CONFIG(cache_timeout),
+		.tv_usec = 0,
+	};
 
-	init_alarm(alarm, u, timeout);
-	add_alarm(alarm, CONFIG(cache_timeout), 0);
+	evtimer_set(event, timeout, u);
+	evtimer_add(event, &tv);
 }
 
 static void timer_update(struct us_conntrack *u, void *data)
 {
-	struct alarm_list *alarm = data;
-	add_alarm(alarm, CONFIG(cache_timeout), 0);
+	struct event *event = data;
+	struct timeval tv = {
+		.tv_sec = CONFIG(cache_timeout),
+		.tv_usec = 0,
+	};
+
+	evtimer_del(event);
+	evtimer_add(event, &tv);
 }
 
 static void timer_destroy(struct us_conntrack *u, void *data)
 {
-	struct alarm_list *alarm = data;
-	del_alarm(alarm);
+	struct event *event = data;
+
+	evtimer_del(event);
 }
 
 static int timer_dump(struct us_conntrack *u, void *data, char *buf, int type)
 {
+	struct event *event = data;
 	struct timeval tv, tmp;
- 	struct alarm_list *alarm = data;
 
 	if (type == NFCT_O_XML)
 		return 0;
 
-	if (!alarm_pending(alarm, &tmp))
+	if (!evtimer_pending(event, &tmp))
 		return 0;
 
 	gettimeofday(&tv, NULL);
@@ -69,7 +81,7 @@ static int timer_dump(struct us_conntrack *u, void *data, char *buf, int type)
 }
 
 struct cache_feature timer_feature = {
-	.size		= sizeof(struct alarm_list),
+	.size		= sizeof(struct event),
 	.add		= timer_add,
 	.update		= timer_update,
 	.destroy	= timer_destroy,
diff --git a/src/run.c b/src/run.c
index 9631481..e762815 100644
--- a/src/run.c
+++ b/src/run.c
@@ -22,7 +22,6 @@
 #include "netlink.h"
 #include "ignore.h"
 #include "log.h"
-#include "alarm.h"
 
 #include <errno.h>
 #include <signal.h>
diff --git a/src/sync-alarm.c b/src/sync-alarm.c
index c7cecc8..9e318b0 100644
--- a/src/sync-alarm.c
+++ b/src/sync-alarm.c
@@ -20,24 +20,33 @@
 #include "sync.h"
 #include "network.h"
 #include "us-conntrack.h"
-#include "alarm.h"
 #include "cache.h"
 #include "debug.h"
 
 #include <stdlib.h>
 #include <string.h>
+#include <sys/types.h>
+#include <event.h>
 
-static void refresher(struct alarm_list *a, void *data)
+struct cache_alarm {
+	struct us_conntrack *u;
+	struct event event;
+};
+
+static void refresher(int fd, short event, void *data)
 {
+	struct cache_alarm *sa = data;
+	struct timeval tv;
+	struct us_conntrack *u = sa->u;
 	size_t len;
 	struct nethdr *net;
-	struct us_conntrack *u = data;
 
 	debug_ct(u->ct, "persistence update");
 
-	add_alarm(a, 
-		  random() % CONFIG(refresh) + 1,
-		  ((random() % 5 + 1)  * 200000) - 1);
+	tv.tv_sec = random() % CONFIG(refresh) + 1;
+	tv.tv_usec = ((random() % 5 + 1)  * 200000) - 1;
+
+	evtimer_add(&sa->event, &tv);
 
 	net = BUILD_NETMSG(u->ct, NFCT_Q_UPDATE);
 	len = prepare_send_netmsg(STATE_SYNC(mcast_client), net);
@@ -46,30 +55,41 @@ static void refresher(struct alarm_list *a, void *data)
 
 static void cache_alarm_add(struct us_conntrack *u, void *data)
 {
-	struct alarm_list *alarm = data;
+	struct cache_alarm *sa = data;
+	struct timeval tv;
+
+	sa->u = u;
+
+	tv.tv_sec = random() % CONFIG(refresh) + 1;
+	tv.tv_usec = ((random() % 5 + 1)  * 200000) - 1;
 
-	init_alarm(alarm, u, refresher);
-	add_alarm(alarm,
-		  random() % CONFIG(refresh) + 1,
-		  ((random() % 5 + 1)  * 200000) - 1);
+	evtimer_set(&sa->event, refresher, sa);
+	evtimer_add(&sa->event, &tv);
 }
 
 static void cache_alarm_update(struct us_conntrack *u, void *data)
 {
-	struct alarm_list *alarm = data;
-	add_alarm(alarm, 
-		  random() % CONFIG(refresh) + 1,
-		  ((random() % 5 + 1)  * 200000) - 1);
+	struct cache_alarm *sa = data;
+	struct timeval tv;
+
+	sa->u = u;
+
+	tv.tv_sec = random() % CONFIG(refresh) + 1;
+	tv.tv_usec = ((random() % 5 + 1)  * 200000) - 1;
+
+	evtimer_del(&sa->event);
+	evtimer_add(&sa->event, &tv);
 }
 
 static void cache_alarm_destroy(struct us_conntrack *u, void *data)
 {
-	struct alarm_list *alarm = data;
-	del_alarm(alarm);
+	struct cache_alarm *sa = data;
+
+	evtimer_del(&sa->event);
 }
 
 static struct cache_extra cache_alarm_extra = {
-	.size 		= sizeof(struct alarm_list),
+	.size 		= sizeof(struct cache_alarm),
 	.add		= cache_alarm_add,
 	.update		= cache_alarm_update,
 	.destroy	= cache_alarm_destroy
diff --git a/src/sync-ftfw.c b/src/sync-ftfw.c
index 49c0b2c..a4394ba 100644
--- a/src/sync-ftfw.c
+++ b/src/sync-ftfw.c
@@ -22,11 +22,12 @@
 #include "queue.h"
 #include "debug.h"
 #include "network.h"
-#include "alarm.h"
 #include "log.h"
 #include "cache.h"
 
 #include <string.h>
+#include <sys/types.h>
+#include <event.h>
 
 #if 0 
 #define dp printf
@@ -83,17 +84,27 @@ static void tx_queue_add_ctlmsg(uint32_t flags, uint32_t from, uint32_t to)
 	queue_add(tx_queue, &ack, NETHDR_ACK_SIZ);
 }
 
-static struct alarm_list alive_alarm;
+static struct event alive_event;
 
-static void do_alive_alarm(struct alarm_list *a, void *data)
+static void do_alive_alarm(int fd, short event, void *data)
 {
+	struct timeval tv = {
+		.tv_sec = 1,
+		.tv_usec = 0,
+	};
+
 	tx_queue_add_ctlmsg(NET_F_ALIVE, 0, 0);
 
-	add_alarm(&alive_alarm, 1, 0);
+	evtimer_add(&alive_event, &tv);
 }
 
 static int ftfw_init(void)
 {
+	struct timeval tv = {
+		.tv_sec = 1,
+		.tv_usec = 0,
+	};
+
 	tx_queue = queue_create(CONFIG(resend_queue_size));
 	if (tx_queue == NULL) {
 		dlog(LOG_ERR, "cannot create tx queue");
@@ -110,8 +121,8 @@ static int ftfw_init(void)
 	INIT_LIST_HEAD(&rs_list);
 
 	/* XXX: alive message expiration configurable */
-	init_alarm(&alive_alarm, NULL, do_alive_alarm);
-	add_alarm(&alive_alarm, 1, 0);
+	evtimer_set(&alive_event, do_alive_alarm, NULL);
+	evtimer_add(&alive_event, &tv);
 
 	return 0;
 }

Re: [conntrack-utils PATCH r7285 01/11] -1 means error, not 0

[Pablo Neira Ayuso]

Max Kellermann wrote:
>  src/run.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Applied. Thanks.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 02/11] added struct local_server

[Pablo Neira Ayuso]

Max Kellermann wrote:
> ---
> 
>  include/conntrackd.h |    2 +-
>  include/local.h      |   11 ++++++++---
>  src/local.c          |   17 ++++++++++-------
>  src/run.c            |   16 +++++++++-------
>  4 files changed, 28 insertions(+), 18 deletions(-)

Applied with some minor changes. Split one wraparound line (over 80
chars per column). Reserve space for the string in local_server struct.
I know, this duplicates the space used since now we have two array of
UNIX_PATH_MAX size in ct_conf and ct_state, however I prefer it to avoid
dependencies between structures.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 03/11] moved process function pointer to struct local_server

[Pablo Neira Ayuso]

Max Kellermann wrote:
>  include/local.h |   10 +++++++---
>  src/local.c     |   11 +++++++----
>  src/run.c       |    5 +++--
>  3 files changed, 17 insertions(+), 9 deletions(-)

This makes local.c less flexible since it sticks the local socket to a
given handler. With the current approach, you may call different
handlers in do_local_server_step with the same socket. I know, we aren't
using it in that way for now but perhaps in the near future. I have kept
this patch back.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 03/11] moved process function pointer to struct local_server

[Max Kellermann]

On 2008/01/23 11:45, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Max Kellermann wrote:
> >  include/local.h |   10 +++++++---
> >  src/local.c     |   11 +++++++----
> >  src/run.c       |    5 +++--
> >  3 files changed, 17 insertions(+), 9 deletions(-)
> 
> This makes local.c less flexible since it sticks the local socket to a
> given handler. With the current approach, you may call different
> handlers in do_local_server_step with the same socket. I know, we aren't
> using it in that way for now but perhaps in the near future. I have kept
> this patch back.

This is just the code which accepts connections from the unix socket.
I assumed that the daemon will not change its behaviour during its
lifetime, please explain your plan.  In the process function, you can
still choose what to do with the new client connection.  You could
also add a local_server_set_callback() or similar.

Max

Re: [conntrack-utils PATCH r7285 05/11] added missing ntohs()

[Pablo Neira Ayuso]

Max Kellermann wrote:
>  src/sync-mode.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

handle_msg() sanity checks and converts nethdr from network to host byte
order.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 06/11] remove obsolete prototypes

[Pablo Neira Ayuso]

Max Kellermann wrote:
> ---
> 
>  include/network.h |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)

Applied. Thanks. Probably we can get it back later to put most of
mcast_handler inside of it.

mcast_recv_netmsg(STATE_SYNC(mcast_server), net, sizeof(__net),
do_mcast_handler_step);

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 08/11] check if the received packet is large enough

[Pablo Neira Ayuso]

Max Kellermann wrote:
> ---
> 
>  src/sync-mode.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)

Applied. Thanks. I like this sort of packet sanity catch ups a lot.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 04/11] added alarm_pending()

[Pablo Neira Ayuso]

Max Kellermann wrote:
> +int alarm_pending(struct alarm_list *alarm, struct timeval *tv)
> +{
> +	if (list_empty(&alarm->head))
> +		return 0;
> +
> +	if (tv != NULL)
> +		*tv = alarm->tv;

This introduces an extra copy.

> diff --git a/src/cache_timer.c b/src/cache_timer.c
> index 86bb8fc..a7afbe2 100644
> --- a/src/cache_timer.c
> +++ b/src/cache_timer.c
> @@ -60,8 +60,11 @@ static int timer_dump(struct us_conntrack *u, void *data, char *buf, int type)
>  	if (type == NFCT_O_XML)
>  		return 0;
>  
> +	if (!alarm_pending(alarm, &tmp))
> +		return 0;

I cannot think of a situation where alarm_pending returns true at the
moment.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 04/11] added alarm_pending()

[Max Kellermann]

On 2008/01/23 12:50, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Max Kellermann wrote:
> > +int alarm_pending(struct alarm_list *alarm, struct timeval *tv)
> > +{
> > +	if (list_empty(&alarm->head))
> > +		return 0;
> > +
> > +	if (tv != NULL)
> > +		*tv = alarm->tv;
> 
> This introduces an extra copy.

I know.  This patch should make transition to libevent smooth.
libevent still has this copy in evtimer_pending(), but it is called
rarely, and libevent does not do a lookup in 2048 lists in every main
loop iteration.

> > +	if (!alarm_pending(alarm, &tmp))
> > +		return 0;
> 
> I cannot think of a situation where alarm_pending returns true at the
> moment.

You mean, where alarm_pending returns false?  The check is just here
for completeness, you could replace it with an assertion.

Max

Re: [conntrack-utils PATCH r7285 00/11] conntrackd with libevent

[Pablo Neira Ayuso]

Hi Max,

Max Kellermann wrote:
> another patch series, some smaller fixes, and the big part is
> conntrackd with libevent.  While looking for a nice solutions for the
> "huge sorted list of alarms" scaling problem, I thought about storing
> alarms in some sort of tree.  After a quick look at libevent, I saw
> they did it exactly this way.  Let's benchmark libevent against the
> existing alarm library.  I don't think it's worth it to continue work
> on alarm.c, when well-tested code is already available in libevent.

I agree here. Using a tree should scale up better than the current alarm
hash table.

BTW, I have noticed that next release of libevent has replaced RB-tree
by a heap. One of my concerns with introducing a new library dependency
is this sort of changes as well as supporting conntrackd with way many
different versions of libevent. And probably, having people reporting
problems or different behaviours of conntrackd that are not my fault.
Don't get me wrong I have enough with bugs :). Also, I still think that
libevent is way to much since basically we would only be using their
alarm implementation. The current alarm scheduler is barely 200 lines long.

My experience is that synchronization software is a bit hard because
bugs are somehow more difficult to detect (you need a good testbed and
QA testing requires a considerable amount of time). If we introduce more
source code lines, chances to introduce bugs increase, for that reason I
try to keep it as simple as possible.

In any case, I'm going to benchmark your patch and get back to you.

> I did not update configure.ac yet to check for libevent, I am waiting
> for your benchmark results.

Never mind on the autoconf crap. Thanks for the libevent patch.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 04/11] added alarm_pending()

[Pablo Neira Ayuso]

Max Kellermann wrote:
> On 2008/01/23 12:50, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> Max Kellermann wrote:
>>> +int alarm_pending(struct alarm_list *alarm, struct timeval *tv)
>>> +{
>>> +	if (list_empty(&alarm->head))
>>> +		return 0;
>>> +
>>> +	if (tv != NULL)
>>> +		*tv = alarm->tv;
>> This introduces an extra copy.
> 
> I know.  This patch should make transition to libevent smooth.
> libevent still has this copy in evtimer_pending() but it is called
> rarely

I have noticed it during the first loop review.

> and libevent does not do a lookup in 2048 lists in every main
> loop iteration.
>>> +	if (!alarm_pending(alarm, &tmp))
>>> +		return 0;
>> I cannot think of a situation where alarm_pending returns true at the
>> moment.
> 
> You mean, where alarm_pending returns false?  The check is just here
> for completeness, you could replace it with an assertion.

Indeed. I like this extra checking for completeness. Applied without the
tv parameter.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 00/11] conntrackd with libevent

[Max Kellermann]

On 2008/01/23 13:07, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> My experience is that synchronization software is a bit hard because
> bugs are somehow more difficult to detect (you need a good testbed
> and QA testing requires a considerable amount of time). If we
> introduce more source code lines, chances to introduce bugs
> increase, for that reason I try to keep it as simple as possible.

I understand, but think about how many critical bugs we fixed in this
so simple 200 lines alarm implementation.  libevent has proven quite
stable in the past, many daemons use it nowadays.  We not only use it
for alarms, but also for socket events (local_server, nfnetlink) and
for signal handlers.  There is the decision between well-tested but
complex external code, and on the other hand untested new code, which
is simple currently but might grow in the future.

By the way, the libevent migration is not completely finished with my
patch, run() should be replaced with event_dispatch(), but for this,
STATE(mode)->run() has to be rewritten to be event driven.  I believe
this rewrite could make conntrackd even better and cleaner (and
faster), even if we do not use libevent.

Max

Re: [conntrack-utils PATCH r7285 07/11] use size_t

[Pablo Neira Ayuso]

Max Kellermann wrote:
> ---
> 
>  include/mcast.h   |    4 ++--
>  include/network.h |    7 ++++---
>  src/build.c       |    2 +-
>  src/local.c       |    4 ++--
>  src/mcast.c       |    8 ++++----
>  src/network.c     |   22 +++++++++++-----------
>  src/sync-alarm.c  |    2 +-
>  src/sync-ftfw.c   |    4 ++--
>  src/sync-mode.c   |   11 ++++++-----
>  9 files changed, 33 insertions(+), 31 deletions(-)

Applied with a minor change.

> --- a/src/sync-mode.c
> +++ b/src/sync-mode.c
> @@ -88,14 +88,15 @@ retry:
>  /* handler for multicast messages received */
>  static void mcast_handler(void)
>  {
> -	int numbytes, remain;
> +	ssize_t numbytes;
> +	size_t remain;
>  	char __net[65536], *ptr = __net; /* XXX: maximum MTU for IPv4 */
>  
>  	numbytes = mcast_recv(STATE_SYNC(mcast_server), __net, sizeof(__net));
>  	if (numbytes <= 0)
>  		return;
>  
> -	remain = numbytes;
> +	remain = (size_t)numbytes;
>  	while (remain > 0) {
>  		struct nethdr *net = (struct nethdr *) ptr;

I have kept remain signed since a malformed header can result in an
overflow.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 07/11] use size_t

[Max Kellermann]

On 2008/01/23 13:29, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> I have kept remain signed since a malformed header can result in an
> overflow.

How?  And how will making it signed help?

Max

Re: [conntrack-utils PATCH r7285 00/11] conntrackd with libevent

[Pablo Neira Ayuso]

Max Kellermann wrote:
> On 2008/01/23 13:07, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> My experience is that synchronization software is a bit hard because
>> bugs are somehow more difficult to detect (you need a good testbed
>> and QA testing requires a considerable amount of time). If we
>> introduce more source code lines, chances to introduce bugs
>> increase, for that reason I try to keep it as simple as possible.
> 
> I understand, but think about how many critical bugs we fixed in this
> so simple 200 lines alarm implementation.  libevent has proven quite
> stable in the past, many daemons use it nowadays.  We not only use it
> for alarms, but also for socket events (local_server, nfnetlink) and
> for signal handlers.  There is the decision between well-tested but
> complex external code, and on the other hand untested new code, which
> is simple currently but might grow in the future.

You can blame me for those critical bugs during the development, I
should take my time before comitting untested code. We don't use that
many descriptors so conntrackd cannot exploit the main benefit of
libevent IMO.

BTW, there's an implementation of the RB-trees in the kernel under lib/
that seems to be quite stable and which has several clients. We may use
it to implement the alarm scheduler. Moreover it is GPLv2 or any later.
Although I have not investigate it further yet.

> By the way, the libevent migration is not completely finished with my
> patch, run() should be replaced with event_dispatch(), but for this,
> STATE(mode)->run() has to be rewritten to be event driven.  I believe
> this rewrite could make conntrackd even better and cleaner (and
> faster), even if we do not use libevent.

I would apply such rewrite on the fly if it improves the code and
doesn't depend on libevent for now.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 07/11] use size_t

[Pablo Neira Ayuso]

Max Kellermann wrote:
> On 2008/01/23 13:29, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> I have kept remain signed since a malformed header can result in an
>> overflow.
> 
> How?  And how will making it signed help?

Indeed. This doesn't make any sense. I thought that if net->len may be
bigger than remain, thus remain becomes negative and we stop looping.
But this condition is never true since we have already checked that
net->len <= remain in the beginning of the loop. I have changed remain
to size_t.

-- 
"Los honestos son inadaptados sociales" -- Les Luthies

Re: [conntrack-utils PATCH r7285 00/11] conntrackd with libevent

[Pablo Neira Ayuso]

Pablo Neira Ayuso wrote:
> BTW, there's an implementation of the RB-trees in the kernel under lib/
> that seems to be quite stable and which has several clients. We may use
> it to implement the alarm scheduler. Moreover it is GPLv2 or any later.
> Although I have not investigate it further yet.

JFYI: I have uploaded to my netfilter place a rbtree-based alarm
implementation but I haven't commit it to SVN since I want to give it
some extra spins.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [conntrack-utils PATCH r7285 00/11] conntrackd with libevent

[Pablo Neira Ayuso]

Pablo Neira Ayuso wrote:
> Pablo Neira Ayuso wrote:
>> BTW, there's an implementation of the RB-trees in the kernel under lib/
>> that seems to be quite stable and which has several clients. We may use
>> it to implement the alarm scheduler. Moreover it is GPLv2 or any later.
>> Although I have not investigate it further yet.
> 
> JFYI: I have uploaded to my netfilter place a rbtree-based alarm
> implementation but I haven't commit it to SVN since I want to give it
> some extra spins.

http://people.netfilter.org/pablo/

Maybe you don't know where it is.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers