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).

diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h
index 157961609fb90815ea5388b5bc648f194b5024a8..d3000036c4a3455cc50a80fd6636994de07097a0 100644 (file)
--- a/lib/isc/include/isc/netmgr.h
+++ b/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,
-                    size_t extrahandlesize, int backlog,
-                    isc_quota_t *quota,
-                    isc_nmsocket_t **sockp);
+                   isc_nm_recv_cb_t cb, void *cbarg,
+                   isc_nm_cb_t accept_cb, void *accept_cbarg,
+                   size_t extrahandlesize, int backlog, isc_quota_t *quota,
+                   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).

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index daae5fd880b8f14f7daaa0fd524dec5697d0ad5e..235fbd2288fb7ead2f0e3b057c7acdf23c087c4e 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/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

diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index 5048200acaff670378dc2907acc6d7245629da4c..a6cf921490a86a1f51624fc1040cf1fb70ba7a9e 100644 (file)
--- a/lib/isc/netmgr/tcpdns.c
+++ b/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_nm_tcpdnssocket);
+       isc__nmsocket_init(dnssock, handle->sock->mgr, 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_quota_t *quota,
+                   isc_nm_cb_t accept_cb, void *accept_cbarg,
+                   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 */

diff --git a/lib/ns/client.c b/lib/ns/client.c
index cf8467316f13080e097e6afdf1afa4b2a246fdb9..c178c043c6012c9d1e9a43bb04305cf81d53c9dc 100644 (file)
--- a/lib/ns/client.c
+++ b/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;

diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h
index c808064665ca806719b2e745f0b58e884b9bb17c..d1e4f28f335456d2a1c72ff92626e4b48e5b4f55 100644 (file)
--- a/lib/ns/include/ns/client.h
+++ b/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);
 

diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c
index 1e9011bf11defcbbf3204310b81239dd5c1e8e86..60f916de48ead606d990ae55c7b30b254c0d681a 100644 (file)
--- a/lib/ns/interfacemgr.c
+++ b/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
-        * the TCP quota when starting, as it ensures that at least one
-        * client will be created for every interface it is listening to.
+        * We call this now to update the tcp-highwater statistic:
+        * this is necessary because we are adding to the TCP quota just
+        * by listening.
         */
-       tcpquota = isc_quota_getused(&ifp->mgr->sctx->tcpquota);
-       ns_stats_update_if_greater(ifp->mgr->sctx->nsstats,
-                                  ns_statscounter_tcphighwater,
-                                  tcpquota);
+       ns__client_tcpconn(NULL, ISC_R_SUCCESS, ifp->mgr->sctx);
 
 #if 0
 #ifndef ISC_ALLOW_MAPPED