Connection stats, including %<lp, missing for persistent connections.

The code reusing a pconn was missing a hier.note() call, resulting in 0
values logged for %<lp (local port number of the last server or peer
connection) and probably other missing stats.

Also refactored poorly copied statistics collection code to remove
duplication and always update to-server connection stats when the actual
connection becomes available.

Positive side effect: Upon setsockopt(2) failures, the tos and nfmark
fields of a pinned connection were set to the desired (but not actually
applied) values, while persistent connection fields were left intact
(and, hence, stale). Both fields are now reset to zero on failures, for
both types of connections.

diff --git a/src/FwdState.cc b/src/FwdState.cc
index d0c1b090d33092651f00184cee576b60aa3c6010..1236a22c47c9cbff97be7c7081b92a2a8c8b3f9b 100644 (file)
--- a/src/FwdState.cc
+++ b/src/FwdState.cc
@@ -772,6 +772,21 @@ FwdState::timeLeft() const
         return (time_t)ctimeout;
 }
 
+/// called when serverConn is set to an _open_ to-peer connection
+void
+FwdState::syncWithServerConn(const char *host)
+{
+    if (Ip::Qos::TheConfig.isAclTosActive())
+        Ip::Qos::setSockTos(serverConn, GetTosToServer(request));
+
+#if SO_MARK
+    if (Ip::Qos::TheConfig.isAclNfmarkActive())
+        Ip::Qos::setSockNfmark(serverConn, GetNfmarkToServer(request));
+#endif
+
+    request->hier.note(serverConn, host);
+}
+
 /**
  * Called after forwarding path selection (via peer select) has taken place
  * and whenever forwarding needs to attempt a new connection (routing failover).
@@ -812,23 +827,11 @@ FwdState::connectStart()
             flags.connected_okay = true;
             ++n_tries;
             request->flags.pinned = true;
-            request->hier.note(serverConn, pinned_connection->pinning.host);
             if (pinned_connection->pinnedAuth())
                 request->flags.auth = true;
             comm_add_close_handler(serverConn->fd, fwdServerClosedWrapper, this);
 
-            /* Update server side TOS and Netfilter mark on the connection. */
-            if (Ip::Qos::TheConfig.isAclTosActive()) {
-                debugs(17, 3, HERE << "setting tos for pinned connection to " << (int)serverConn->tos );
-                serverConn->tos = GetTosToServer(request);
-                Ip::Qos::setSockTos(serverConn, serverConn->tos);
-            }
-#if SO_MARK
-            if (Ip::Qos::TheConfig.isAclNfmarkActive()) {
-                serverConn->nfmark = GetNfmarkToServer(request);
-                Ip::Qos::setSockNfmark(serverConn, serverConn->nfmark);
-            }
-#endif
+            syncWithServerConn(pinned_connection->pinning.host);
 
             // the server may close the pinned connection before this request
             pconnRace = racePossible;
@@ -867,17 +870,7 @@ FwdState::connectStart()
 
         comm_add_close_handler(serverConnection()->fd, fwdServerClosedWrapper, this);
 
-        /* Update server side TOS and Netfilter mark on the connection. */
-        if (Ip::Qos::TheConfig.isAclTosActive()) {
-            const tos_t tos = GetTosToServer(request);
-            Ip::Qos::setSockTos(temp, tos);
-        }
-#if SO_MARK
-        if (Ip::Qos::TheConfig.isAclNfmarkActive()) {
-            const nfmark_t nfmark = GetNfmarkToServer(request);
-            Ip::Qos::setSockNfmark(temp, nfmark);
-        }
-#endif
+        syncWithServerConn(request->GetHost());
 
         dispatch();
         return;

diff --git a/src/FwdState.h b/src/FwdState.h
index 56dad304a04bd7543cac67c6e1b51bdf209c5124..b19c44feab3083a3341049979d61cc5d4bd157dc 100644 (file)
--- a/src/FwdState.h
+++ b/src/FwdState.h
@@ -120,6 +120,8 @@ private:
     /// stops monitoring server connection for closure and updates pconn stats
     void closeServerConnection(const char *reason);
 
+    void syncWithServerConn(const char *host);
+
 public:
     StoreEntry *entry;
     HttpRequest *request;

diff --git a/src/comm/TcpAcceptor.cc b/src/comm/TcpAcceptor.cc
index 4c98b7b7f19027e0bf40c71266aefc4ae4280c34..44511a19db5ef9c137f430dc0bd7b60488a05649 100644 (file)
--- a/src/comm/TcpAcceptor.cc
+++ b/src/comm/TcpAcceptor.cc
@@ -181,13 +181,11 @@ Comm::TcpAcceptor::setListen()
     // Set TOS if needed.
     // To correctly implement TOS values on listening sockets, probably requires
     // more work to inherit TOS values to created connection objects.
-    if (conn->tos &&
-            Ip::Qos::setSockTos(conn->fd, conn->tos, conn->remote.isIPv4() ? AF_INET : AF_INET6) < 0)
-        conn->tos = 0;
+    if (conn->tos)
+        Ip::Qos::setSockTos(conn, conn->tos)
 #if SO_MARK
-    if (conn->nfmark &&
-            Ip::Qos::setSockNfmark(conn->fd, conn->nfmark) < 0)
-        conn->nfmark = 0;
+        if (conn->nfmark)
+            Ip::Qos::setSockNfmark(conn, conn->nfmark);
 #endif
 #endif
 

diff --git a/src/ip/Qos.cci b/src/ip/Qos.cci
index f2e2ab923c264e1e5371c82b4ebe3e0d18a692f6..b24650cef26d74da261103f6aab5630d290e118c 100644 (file)
--- a/src/ip/Qos.cci
+++ b/src/ip/Qos.cci
@@ -19,6 +19,8 @@ Ip::Qos::setSockTos(const int fd, tos_t tos, int type)
     //     so we convert to a int before setting.
     int bTos = tos;
 
+    debugs(50, 3, "for FD " << fd << " to " << bTos);
+
     if (type == AF_INET) {
 #if defined(IP_TOS)
         const int x = setsockopt(fd, IPPROTO_IP, IP_TOS, &bTos, sizeof(bTos));
@@ -48,9 +50,7 @@ int
 Ip::Qos::setSockTos(const Comm::ConnectionPointer &conn, tos_t tos)
 {
     const int x = Ip::Qos::setSockTos(conn->fd, tos, conn->remote.isIPv4() ? AF_INET : AF_INET6);
-    if (x >= 0)
-        conn->tos = tos;
-
+    conn->tos = (x >= 0) ? tos : 0;
     return x;
 }
 
@@ -58,6 +58,7 @@ int
 Ip::Qos::setSockNfmark(const int fd, nfmark_t mark)
 {
 #if SO_MARK && USE_LIBCAP
+    debugs(50, 3, "for FD " << fd << " to " << mark);
     const int x = setsockopt(fd, SOL_SOCKET, SO_MARK, &mark, sizeof(nfmark_t));
     if (x < 0)
         debugs(50, 2, "setSockNfmark: setsockopt(SO_MARK) on " << fd << ": " << xstrerror());
@@ -75,8 +76,7 @@ int
 Ip::Qos::setSockNfmark(const Comm::ConnectionPointer &conn, nfmark_t mark)
 {
     const int x = Ip::Qos::setSockNfmark(conn->fd, mark);
-    if (x >= 0)
-        conn->nfmark = mark;
+    conn->nfmark = (x >= 0) ? mark : 0;
     return x;
 }