From: Witold Kręcicki <wpk@isc.org>
Date: Fri, 28 Feb 2020 10:57:51 +0000 (+0100)
Subject: Proper accounting of active TCP connections
X-Git-Tag: v9.17.1~76^2~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fc9e2276ca4d8425bef8aa7b3a6cc16d15b8820c;p=thirdparty%2Fbind9.git

Proper accounting of active TCP connections
---

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index e233208d6f4..48181885700 100644
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -369,7 +369,16 @@ struct isc_nmsocket {
 	 */
 	isc_quota_t *quota;
 	isc_quota_t *pquota;
-	bool overquota;
+
+	/*%
+	 * How many connections we have not accepted due to quota?
+	 * When we close a connection we need to accept a new one.
+	 */
+	int overquota;
+	/*%
+	 * How many active connections we have?
+	 */
+	int conns;
 
 	/*%
 	 * Socket statistics
diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 81726b82a8c..0e04dcb8af3 100644
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -718,6 +718,9 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree) {
 		for (int i = 0; i < sock->nchildren; i++) {
 			if (!atomic_load(&sock->children[i].destroying)) {
 				nmsocket_cleanup(&sock->children[i], false);
+				if (sock->statsindex != NULL) {
+					isc__nm_decstats(sock->mgr, sock->statsindex[STATID_ACTIVE]);
+				}
 			}
 		}
 
@@ -729,6 +732,9 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree) {
 		sock->children = NULL;
 		sock->nchildren = 0;
 	}
+	if (sock->statsindex != NULL) {
+			isc__nm_decstats(sock->mgr, sock->statsindex[STATID_ACTIVE]);
+	}
 
 	if (sock->tcphandle != NULL) {
 		isc_nmhandle_unref(sock->tcphandle);
@@ -843,8 +849,6 @@ isc__nmsocket_prep_destroy(isc_nmsocket_t *sock) {
 	if (sock->children != NULL) {
 		for (int i = 0; i < sock->nchildren; i++) {
 			atomic_store(&sock->children[i].active, false);
-			isc__nm_decstats(sock->mgr,
-					 sock->statsindex[STATID_ACTIVE]);
 		}
 	}
 
diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 6e6cee5a409..60ca097de0f 100644
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -650,9 +650,6 @@ read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
 	}
 
 	isc__nm_free_uvbuf(sock, buf);
-	if (sock->quota) {
-		isc_quota_detach(&sock->quota);
-	}
 
 	/*
 	 * This might happen if the inner socket is closing.  It means that
@@ -680,6 +677,7 @@ accept_connection(isc_nmsocket_t *ssock) {
 	struct sockaddr_storage ss;
 	isc_sockaddr_t local;
 	int r;
+	bool overquota = false;
 
 	REQUIRE(VALID_NMSOCK(ssock));
 	REQUIRE(ssock->tid == isc_nm_tid());
@@ -693,10 +691,26 @@ accept_connection(isc_nmsocket_t *ssock) {
 
 	if (ssock->pquota != NULL) {
 		result = isc_quota_attach(ssock->pquota, &quota);
+		/*
+		 * We share the quota between sockets - we need to  have at
+		 * least one active connection here to restart listening
+		 * when the quota is available again (in tcp_close_direct).
+		 */
 		if (result != ISC_R_SUCCESS) {
-			isc__nm_incstats(ssock->mgr,
-					 ssock->statsindex[STATID_ACCEPTFAIL]);
-			return (result);
+			ssock->overquota++;
+			overquota = true;
+			if (ssock->conns > 0) {
+				isc__nm_incstats(
+					ssock->mgr,
+					ssock->statsindex[STATID_ACCEPTFAIL]);
+				return (result);
+			}
+			/*
+			 * We share the quota between sockets - we need to  have
+			 * at least one active connection here to restart
+			 * listening when the quota is available again (in
+			 * tcp_close_direct).
+			 */
 		}
 	}
 
@@ -743,6 +757,7 @@ accept_connection(isc_nmsocket_t *ssock) {
 	}
 
 	isc_nmsocket_attach(ssock, &csock->server);
+	ssock->conns++;
 
 	handle = isc__nmhandle_get(csock, NULL, &local);
 
@@ -761,6 +776,9 @@ error:
 	if (csock->quota != NULL) {
 		isc_quota_detach(&csock->quota);
 	}
+	if (overquota) {
+		ssock->overquota--;
+	}
 	/* We need to detach it properly to make sure uv_close is called. */
 	isc_nmsocket_detach(&csock);
 	return (result);
@@ -775,9 +793,6 @@ tcp_connection_cb(uv_stream_t *server, int status) {
 
 	result = accept_connection(ssock);
 	if (result != ISC_R_SUCCESS) {
-		if (result == ISC_R_QUOTA || result == ISC_R_SOFTQUOTA) {
-			ssock->overquota = true;
-		}
 		isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL,
 			      ISC_LOGMODULE_NETMGR, ISC_LOG_ERROR,
 			      "TCP connection failed: %s",
@@ -910,17 +925,22 @@ tcp_close_direct(isc_nmsocket_t *sock) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_nm_tid());
 	REQUIRE(sock->type == isc_nm_tcpsocket);
+	isc_nmsocket_t *ssock = sock->server;
 
 	if (sock->quota != NULL) {
-		isc_nmsocket_t *ssock = sock->server;
-
 		isc_quota_detach(&sock->quota);
-
-		if (ssock->overquota) {
+	}
+	if (ssock != NULL) {
+		ssock->conns--;
+		while (ssock->conns == 0 && ssock->overquota > 0) {
+			ssock->overquota--;
 			isc_result_t result = accept_connection(ssock);
-			if (result != ISC_R_QUOTA && result != ISC_R_SOFTQUOTA)
-			{
-				ssock->overquota = false;
+			if (result != ISC_R_SUCCESS) {
+				isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL,
+					      ISC_LOGMODULE_NETMGR,
+					      ISC_LOG_ERROR,
+					      "TCP connection failed: %s",
+					      isc_result_totext(result));
 			}
 		}
 	}
diff --git a/lib/isc/netmgr/uverr2result.c b/lib/isc/netmgr/uverr2result.c
index cf95eba88a4..82fa975b8f1 100644
--- a/lib/isc/netmgr/uverr2result.c
+++ b/lib/isc/netmgr/uverr2result.c
@@ -37,6 +37,8 @@ isc___nm_uverr2result(int uverr, bool dolog, const char *file,
 		return (ISC_R_INVALIDFILE);
 	case UV_ENOENT:
 		return (ISC_R_FILENOTFOUND);
+	case UV_EAGAIN:
+		return (ISC_R_NOCONN);
 	case UV_EACCES:
 	case UV_EPERM:
 		return (ISC_R_NOPERM);