Don't increment network error stats on UV_EOF

When networking statistics was added to the netmgr (in commit
5234a8e00a6ae1df738020f27544594ccb8d5215), two lines were added that
increment the 'STATID_RECVFAIL' statistic: One if 'uv_read_start'
fails and one at the end of the 'read_cb'.  The latter happens
if 'nread < 0'.

According to the libuv documentation, I/O read callbacks (such as for
files and sockets) are passed a parameter 'nread'. If 'nread' is less
than 0, there was an error and 'UV_EOF' is the end of file error, which
you may want to handle differently.

In other words, we should not treat EOF as a RECVFAIL error.

(cherry picked from commit 6c5ff9421875a1fcdfb8f03ac01afe292075d8d2)

diff --git a/CHANGES b/CHANGES
index 2f875887b8c22d66d8b101dc2a35904d22ee94d0..d4a806b0fdded3fede03f54a76681b901cc5f4d2 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+5517.  [bug]           Handle 'UV_EOF' differently and don't contribute it to
+                       the RECVFAIL statistic count. [GL #2208]
+
 5516.  [func]          The default EDNS buffer size has been changed from 4096
                        to 1232. [GL #2183]
 

diff --git a/bin/tests/system/statistics/tests.sh b/bin/tests/system/statistics/tests.sh
index 0301fb49a888bab87622747d3e70c84937f55302..66e11b17dfb66b630382ea87266b77db8dd25111 100644 (file)
--- a/bin/tests/system/statistics/tests.sh
+++ b/bin/tests/system/statistics/tests.sh
@@ -162,6 +162,8 @@ if $FEATURETEST --have-libxml2 && [ -x "${CURL}" ] && [ -x "${XSLTPROC}" ]  ; th
     ${CURL} http://10.53.0.3:${EXTRAPORT1}/xml/v3 > curl.out.${n}.xml 2>/dev/null || ret=1
     ${CURL} http://10.53.0.3:${EXTRAPORT1}/bind9.xsl > curl.out.${n}.xsl 2>/dev/null || ret=1
     ${XSLTPROC} curl.out.${n}.xsl - < curl.out.${n}.xml > xsltproc.out.${n} 2>/dev/null || ret=1
+    cp curl.out.${n}.xml stats.xml.out || ret=1
+
     #
     # grep for expected sections.
     #
@@ -206,6 +208,30 @@ if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 n=`expr $n + 1`
 
+ret=0
+echo_i "checking bind9.xml socket statistics ($n)"
+if $FEATURETEST --have-libxml2 && [ -x "${CURL}" ] && [ -x "${XSLTPROC}" ]  ; then
+    # Socket statistics (expect no errors)
+    grep "<counter name=\"TCP4AcceptFail\">0</counter>" stats.xml.out >/dev/null || ret=1
+    grep "<counter name=\"TCP4BindFail\">0</counter>" stats.xml.out >/dev/null || ret=1
+    grep "<counter name=\"TCP4ConnFail\">0</counter>" stats.xml.out >/dev/null || ret=1
+    grep "<counter name=\"TCP4OpenFail\">0</counter>" stats.xml.out >/dev/null || ret=1
+    grep "<counter name=\"TCP4RecvErr\">0</counter>" stats.xml.out >/dev/null || ret=1
+    grep "<counter name=\"TCP4SendErr\">0</counter>" stats.xml.out >/dev/null || ret=1
+
+    grep "<counter name=\"TCP6AcceptFail\">0</counter>" stats.xml.out >/dev/null || ret=1
+    grep "<counter name=\"TCP6BindFail\">0</counter>" stats.xml.out >/dev/null || ret=1
+    grep "<counter name=\"TCP6ConnFail\">0</counter>" stats.xml.out >/dev/null || ret=1
+    grep "<counter name=\"TCP6OpenFail\">0</counter>" stats.xml.out >/dev/null || ret=1
+    grep "<counter name=\"TCP6RecvErr\">0</counter>" stats.xml.out >/dev/null || ret=1
+    grep "<counter name=\"TCP6SendErr\">0</counter>" stats.xml.out >/dev/null || ret=1
+else
+    echo_i "skipping test as libxml2 and/or curl and/or xsltproc was not found"
+fi
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+n=`expr $n + 1`
+
 ret=0
 echo_i "checking priming queries are counted ($n)"
 grep "1 priming queries" ns3/named.stats

diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst
index 462dc0f422f31d34bd329a85a9fcca4cf23aae44..5471c38509cbf62e898752440178cda563ee1b4e 100644 (file)
--- a/doc/notes/notes-current.rst
+++ b/doc/notes/notes-current.rst
@@ -67,3 +67,6 @@ Bug Fixes
 - `named` would start continous rollovers for policies that algorithms
   Ed25519 or Ed448 due to a mismatch in created key size and expected key size.
   [GL #2171]
+
+- Handle `UV_EOF` differently such that it is not treated as a `TCP4RecvErr` or
+  `TCP6RecvErr`. [GL #2208]

diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 9c1471217cfeb334e07252d6523aef1cc4f04458..3cd7bcd468416cadb9d9f1eb221254bb10cb614c 100644 (file)
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -803,8 +803,10 @@ read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
         * This might happen if the inner socket is closing.  It means that
         * it's detached, so the socket will be closed.
         */
-       if (cb != NULL) {
+       if (nread != UV_EOF) {
                isc__nm_incstats(sock->mgr, sock->statsindex[STATID_RECVFAIL]);
+       }
+       if (cb != NULL) {
                isc__nmsocket_clearcb(sock);
                cb(sock->statichandle, ISC_R_EOF, NULL, cbarg);
        }