Milter-greylist

Hi,

>>>>> On Thu, 13 Oct 2005 10:12:33 +0300
>>>>> Fredrik Nyberg DC <nba@...> said:

nba> Matt Kettler wrote:
> Based on that you're talking about 50 million entries. If you've got a 5-day
> timeout that's only 10 million attempts per day. For a site with 8,000 users
> that doesn't seem unreasonable.

nba> We have around 1 million entries in our greylist database. What really 
nba> bugs me is that the current 2.3 GB of VmData that milter-greylist is 
nba> using does not show up in physical memory or swap... it will be 
nba> interresting to see what happens when the process hits 3 GB, since this 
nba> is the amount of memory + swap that one of the servers has.

I found that a pending entry has a redundant p_addr field which holds
string form of an IP address.  Since a pending entry has a p_sa field,
I think it is elective.  Actually, an autowhite entry doesn't have
such field.
The following patch should reduce about 20 bytes per each pending
entry.
This patch also fixes one minor problem.  It shouldn't happen, but the
pending_get() may return NULL on error.  The pending_check() doesn't
check it, and simply passes it to peer_create().

Index: pending.c
diff -u -p pending.c.orig pending.c
--- pending.c.orig	Thu May 12 01:25:43 2005
+++ pending.c	Fri Oct 14 21:54:49 2005
@@ -103,11 +103,15 @@ pending_get(sa, salen, from, rcpt, date)
 	char *rcpt;
 	time_t date;
 {
-	struct pending *pending;
+	struct pending *pending = NULL;
 	struct timeval tv;
 	int delay = conf.c_delay;
 	char addr[IPADDRSTRLEN];
 
+	/* make sure having only valid IP address */
+	if (!iptostring(sa, salen, addr, sizeof(addr)))
+		goto out;
+
 	if ((pending = malloc(sizeof(*pending))) == NULL)
 		goto out;
 
@@ -127,15 +131,7 @@ pending_get(sa, salen, from, rcpt, date)
 	}
 	memcpy(pending->p_sa, sa, salen);
 	pending->p_salen = salen;
-	if (!iptostring(sa, salen, addr, sizeof(addr)) ||
-	    (pending->p_addr = strdup(addr)) == NULL) {
-		free(pending->p_sa);
-		free(pending);
-		pending = NULL;
-		goto out;
-	}
 	if ((pending->p_from = strdup(from)) == NULL) {
-		free(pending->p_addr);
 		free(pending->p_sa);
 		free(pending);
 		pending = NULL;
@@ -143,7 +139,6 @@ pending_get(sa, salen, from, rcpt, date)
 	}
 	if ((pending->p_rcpt = strdup(rcpt)) == NULL) {
 		free(pending->p_from);
-		free(pending->p_addr);
 		free(pending->p_sa);
 		free(pending);
 		pending = NULL;
@@ -160,7 +155,7 @@ pending_get(sa, salen, from, rcpt, date)
 
 	if (conf.c_debug) {
 		syslog(LOG_DEBUG, "created: %s from %s to %s delayed for %lds",
-		    pending->p_addr, pending->p_from, pending->p_rcpt, 
+		    addr, pending->p_from, pending->p_rcpt,
 		    pending->p_tv.tv_sec - tv.tv_sec);
 	}
 out:
@@ -171,9 +166,13 @@ void
 pending_put(pending) /* pending list should be write-locked */
 	struct pending *pending;
 {
+	char addr[IPADDRSTRLEN];
+
 	if (conf.c_debug) {
+		iptostring(pending->p_sa, pending->p_salen, addr,
+		    sizeof(addr));
 		syslog(LOG_DEBUG, "removed: %s from %s to %s",
-		    pending->p_addr, pending->p_from, pending->p_rcpt);
+		    addr, pending->p_from, pending->p_rcpt);
 	}
 
 	TAILQ_REMOVE(&pending_head, pending, p_list);	
@@ -198,8 +197,6 @@ pending_del(sa, salen, from, rcpt, time)
 	struct timeval tv;
 
 	gettimeofday(&tv, NULL);
-	if (!iptostring(sa, salen, addr, sizeof(addr)))
-		return;
 
 	PENDING_WRLOCK;	/* XXX take it as read and upgrade it */
 	for (pending = TAILQ_FIRST(&pending_head); pending; pending = next) {
@@ -208,7 +205,7 @@ pending_del(sa, salen, from, rcpt, time)
 		/*
 		 * Look for our entry.
 		 */
-		if ((strncmp(addr, pending->p_addr, sizeof(addr)) == 0) &&
+		if (ip_equal(sa, pending->p_sa) &&
 		    (strcmp(from, pending->p_from) == 0) &&
 		    (strcmp(rcpt, pending->p_rcpt) == 0) &&
 		    (pending->p_tv.tv_sec == time)) {
@@ -221,10 +218,11 @@ pending_del(sa, salen, from, rcpt, time)
 		 */
 		if (tv.tv_sec - pending->p_tv.tv_sec > conf.c_timeout) {
 			if (conf.c_debug) {
+				iptostring(pending->p_sa, pending->p_salen,
+				    addr, sizeof(addr));
 				syslog(LOG_DEBUG,
 				    "del: %s from %s to %s timed out",
-				    pending->p_addr, pending->p_from,
-				    pending->p_rcpt);
+				    addr, pending->p_from, pending->p_rcpt);
 			}
 
 			pending_put(pending);
@@ -273,10 +271,11 @@ pending_check(sa, salen, from, rcpt, rem
 		 */
 		if (now - accepted > conf.c_timeout) {
 			if (conf.c_debug) {
+				iptostring(pending->p_sa, pending->p_salen,
+				    addr, sizeof(addr));
 				syslog(LOG_DEBUG,
 				    "check: %s from %s to %s timed out",
-				    pending->p_addr, pending->p_from,
-				    pending->p_rcpt);
+				    addr, pending->p_from, pending->p_rcpt);
 			}
 
 			pending_put(pending);
@@ -319,8 +318,8 @@ pending_check(sa, salen, from, rcpt, rem
 	 * It was not found. Create it and propagagte it to peers.
 	 * Error handling is useless here, we will tempfail anyway
 	 */
-	pending = pending_get(sa, salen, from, rcpt, (time_t)0);
-	peer_create(pending);
+	if ((pending = pending_get(sa, salen, from, rcpt, (time_t)0)) != NULL)
+		peer_create(pending);
 	rest = delay;
 	dirty = 1;
 
@@ -349,6 +348,7 @@ pending_textdump(stream)
 	struct pending *pending;
 	int done = 0;
 	char textdate[DATELEN + 1];
+	char textaddr[IPADDRSTRLEN];
 	struct tm tm;
 
 	fprintf(stream, "\n\n#\n# greylisted tuples\n#\n");
@@ -360,10 +360,12 @@ pending_textdump(stream)
 		localtime_r((time_t *)&pending->p_tv.tv_sec, &tm);
 		strftime(textdate, DATELEN, "%Y-%m-%d %T", &tm);
 
-		fprintf(stream, "%s\t%s\t%s\t%ld # %s\n", 
-		    pending->p_addr, pending->p_from, 
-		    pending->p_rcpt, (long)pending->p_tv.tv_sec, textdate);
-		
+		iptostring(pending->p_sa, pending->p_salen, textaddr,
+		    sizeof(textaddr));
+		fprintf(stream, "%s\t%s\t%s\t%ld # %s\n",
+		    textaddr, pending->p_from, pending->p_rcpt,
+		    (long)pending->p_tv.tv_sec, textdate);
+
 		done++;
 	}
 	PENDING_UNLOCK;
@@ -393,7 +395,6 @@ pending_free(pending)
 	}
 	UNLOCK(refcnt_lock);
 	free(pending->p_sa);
-	free(pending->p_addr);
 	free(pending->p_from);
 	free(pending->p_rcpt);
 	free(pending);
Index: pending.h
diff -u pending.h.orig pending.h
--- pending.h.orig	Tue Sep 14 03:41:55 2004
+++ pending.h	Fri Oct 14 19:16:17 2005
@@ -62,7 +62,6 @@
 TAILQ_HEAD(pendinglist, pending);
 
 struct pending {
-	char *p_addr;
 	struct sockaddr *p_sa;
 	socklen_t p_salen;
 	char *p_from;
Index: sync.c
diff -u -p sync.c.orig sync.c
--- sync.c.orig	Sun Jan 23 02:23:17 2005
+++ sync.c	Fri Oct 14 21:49:56 2005
@@ -236,6 +236,7 @@ sync_send(peer, type, pending)	/* peer l
 	char *replystr;
 	int replycode;
 	char line[LINELEN + 1];
+	char addr[IPADDRSTRLEN];
 	char *cookie = NULL;
 
 	if ((peer->p_stream == NULL) && (peer_connect(peer) != 0))
@@ -246,9 +247,10 @@ sync_send(peer, type, pending)	/* peer l
 	else
 		fprintf(peer->p_stream, "del ");
 
+	iptostring(pending->p_sa, pending->p_salen, addr, sizeof(addr));
 	fprintf(peer->p_stream, "addr %s from %s rcpt %s date %ld\r\n", 
-	    pending->p_addr, pending->p_from, 
-	    pending->p_rcpt, (long)pending->p_tv.tv_sec);
+	    addr, pending->p_from, pending->p_rcpt,
+	    (long)pending->p_tv.tv_sec);
 	fflush(peer->p_stream);
 
 	/* 


Sincerely,

--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
ume@...  ume@{,jp.}FreeBSD.org
http://www.imasy.org/~ume/