Fix tcp-highwater stats updating

After the network manager rewrite, tcp-higwater stats was only being
updated when a valid DNS query was received over tcp.

It turns out tcp-quota is updated right after a tcp connection is
accepted, before any data is read, so in the event that some client
connect but don't send a valid query, it wouldn't be taken into
account to update tcp-highwater stats, that is wrong.

This commit fix tcp-highwater to update its stats whenever a tcp connection
is established, independent of what happens after (timeout/invalid
request, etc).

lib/isc/include/isc/netmgr.h

@@ -264,10 +264,10 @@ isc_nm_tcp_stoplistening(isc_nmsocket_t *sock);
 
 isc_result_t
 isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface,
-		     isc_nm_recv_cb_t cb, void *arg,
+		    isc_nm_recv_cb_t cb, void *cbarg,
-		     size_t extrahandlesize, int backlog,
+		    isc_nm_cb_t accept_cb, void *accept_cbarg,
-		     isc_quota_t *quota,
+		    size_t extrahandlesize, int backlog, isc_quota_t *quota,
-		     isc_nmsocket_t **sockp);
+		    isc_nmsocket_t **sockp);
 /*%<
  * Start listening for DNS messages over the TCP interface 'iface', using
  * net manager 'mgr'.
@@ -281,6 +281,9 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface,
  * When a complete DNS message is received on the socket, 'cb' will be
  * called with 'cbarg' as its argument.
  *
+ * When a new TCP connection is accepted, 'accept_cb' will be called
+ * with 'accept_cbarg' as its argument.
+ *
  * When handles are allocated for the socket, 'extrasize' additional bytes
  * will be allocated along with the handle for an associated object
  * (typically ns_client).

lib/isc/netmgr/netmgr-int.h

@@ -477,6 +477,9 @@ struct isc_nmsocket {
 
 	isc__nm_readcb_t	rcb;
 	void			*rcbarg;
+
+	isc__nm_cb_t		accept_cb;
+	void 			*accept_cbarg;
 };
 
 bool

lib/isc/netmgr/tcpdns.c

@@ -96,7 +96,7 @@ dnstcp_readtimeout(uv_timer_t *timer) {
 }
 
 /*
- * Accept callback for TCP-DNS connection
+ * Accept callback for TCP-DNS connection.
  */
 static void
 dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
@@ -111,10 +111,14 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 		return;
 	}
 
+	if (dnslistensock->accept_cb.accept != NULL) {
+		dnslistensock->accept_cb.accept(handle, ISC_R_SUCCESS,
+						dnslistensock->accept_cbarg);
+	}
+
 	/* We need to create a 'wrapper' dnssocket for this connection */
 	dnssock = isc_mem_get(handle->sock->mgr->mctx, sizeof(*dnssock));
-	isc__nmsocket_init(dnssock, handle->sock->mgr,
+	isc__nmsocket_init(dnssock, handle->sock->mgr, isc_nm_tcpdnssocket);
-			   isc_nm_tcpdnssocket);
 
 	/* We need to copy read callbacks from outer socket */
 	dnssock->rcb.recv = dnslistensock->rcb.recv;
@@ -278,8 +282,8 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_region_t *region, void *arg) {
 isc_result_t
 isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface,
 		    isc_nm_recv_cb_t cb, void *cbarg,
-		    size_t extrahandlesize, int backlog,
+		    isc_nm_cb_t accept_cb, void *accept_cbarg,
-		    isc_quota_t *quota,
+		    size_t extrahandlesize, int backlog, isc_quota_t *quota,
 		    isc_nmsocket_t **sockp)
 {
 	/* A 'wrapper' socket object with outer set to true TCP socket */
@@ -293,6 +297,8 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface,
 	dnslistensock->iface = iface;
 	dnslistensock->rcb.recv = cb;
 	dnslistensock->rcbarg = cbarg;
+	dnslistensock->accept_cb.accept = accept_cb;
+	dnslistensock->accept_cbarg = accept_cbarg;
 	dnslistensock->extrahandlesize = extrahandlesize;
 
 	/* We set dnslistensock->outer to a true listening socket */

lib/ns/client.c

@@ -1657,11 +1657,6 @@ ns__client_request(isc_nmhandle_t *handle, isc_region_t *region, void *arg) {
 	}
 	if (isc_nmhandle_is_stream(handle)) {
 		client->attributes |= NS_CLIENTATTR_TCP;
-		unsigned int curr_tcpquota =
-			isc_quota_getused(&client->sctx->tcpquota);
-		ns_stats_update_if_greater(client->sctx->nsstats,
-					   ns_statscounter_tcphighwater,
-					   curr_tcpquota);
 	}
 
 	INSIST(client->recursionquota == NULL);
@@ -2185,6 +2180,20 @@ ns__client_request(isc_nmhandle_t *handle, isc_region_t *region, void *arg) {
 	isc_task_unpause(client->task);
 }
 
+void
+ns__client_tcpconn(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
+	ns_server_t *sctx = (ns_server_t *) arg;
+	unsigned int tcpquota;
+
+	UNUSED(handle);
+	UNUSED(result);
+
+	tcpquota = isc_quota_getused(&sctx->tcpquota);
+	ns_stats_update_if_greater(sctx->nsstats,
+				   ns_statscounter_tcphighwater,
+				   tcpquota);
+}
+
 static void
 get_clientmctx(ns_clientmgr_t *manager, isc_mem_t **mctxp) {
 	isc_mem_t *clientmctx;

lib/ns/include/ns/client.h

@@ -487,7 +487,7 @@ isc_result_t
 ns_client_addopt(ns_client_t *client, dns_message_t *message,
 		 dns_rdataset_t **opt);
 
-/*
+/*%<
  * Get a client object from the inactive queue, or create one, as needed.
  * (Not intended for use outside this module and associated tests.)
  */
@@ -495,11 +495,19 @@ ns_client_addopt(ns_client_t *client, dns_message_t *message,
 void
 ns__client_request(isc_nmhandle_t *handle, isc_region_t *region, void *arg);
 
-/*
+/*%<
  * Handle client requests.
  * (Not intended for use outside this module and associated tests.)
  */
 
+ void
+ ns__client_tcpconn(isc_nmhandle_t *handle, isc_result_t result, void *arg);
+
+ /*%<
+  * Called every time a TCP connection is establish.  This is used for
+  * updating TCP statistics.
+  */
+
 dns_rdataset_t *
 ns_client_newrdataset(ns_client_t *client);
 

lib/ns/interfacemgr.c

@@ -459,12 +459,12 @@ ns_interface_listenudp(ns_interface_t *ifp) {
 
 static isc_result_t
 ns_interface_listentcp(ns_interface_t *ifp) {
-	unsigned int tcpquota;
 	isc_result_t result;
 
 	result = isc_nm_listentcpdns(ifp->mgr->nm,
 				     (isc_nmiface_t *) &ifp->addr,
 				     ns__client_request, ifp,
+				     ns__client_tcpconn, ifp->mgr->sctx,
 				     sizeof(ns_client_t),
 				     ifp->mgr->backlog,
 				     &ifp->mgr->sctx->tcpquota,
@@ -476,14 +476,11 @@ ns_interface_listentcp(ns_interface_t *ifp) {
 	}
 
 	/*
-	 * We update tcp-highwater stats here, since named itself adds to
+	 * We call this now to update the tcp-highwater statistic:
-	 * the TCP quota when starting, as it ensures that at least one
+	 * this is necessary because we are adding to the TCP quota just
-	 * client will be created for every interface it is listening to.
+	 * by listening.
 	 */
-	tcpquota = isc_quota_getused(&ifp->mgr->sctx->tcpquota);
+	ns__client_tcpconn(NULL, ISC_R_SUCCESS, ifp->mgr->sctx);
-	ns_stats_update_if_greater(ifp->mgr->sctx->nsstats,
-				   ns_statscounter_tcphighwater,
-				   tcpquota);
 
 #if 0
 #ifndef ISC_ALLOW_MAPPED