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 9b255602b8cfd7f3ceb0fd80671f232a0164e329..67808807941d34e6cc698ce0352711024b8b42e9 100644 (file)
--- a/src/FwdState.cc
+++ b/src/FwdState.cc
@@ -780,6 +780,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).
@@ -820,23 +835,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;
@@ -875,17 +878,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->url.host());
 
         dispatch();
         return;

diff --git a/src/FwdState.h b/src/FwdState.h
index ff47632f28957882a84b09d4b94037f4b0671116..63e497fb1c2b823a6faa11349ae197a3bf218d84 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 8b29e7b776769ed8ff0bf347b2b00e01b14517fd..f346e2f0a883fe25ac4f4bf2a9eb8dff497391d0 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;
 }