------------------------------------------------------------
revno: 13941
revision-id: squid3@treenet.co.nz-20151028034756-yianujhxd7ptyihi
parent: squid3@treenet.co.nz-20151027224824-c5bn5e1ja1g854tl
author: Alex Rousskov <rousskov@measurement-factory.com>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Tue 2015-10-27 20:47:56 -0700
message:
  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.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20151028034756-yianujhxd7ptyihi
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: 8eec85106bd453a307dfe4e80f4f01f8e09161ab
# timestamp: 2015-10-28 03:51:02 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20151027224824-\
#   c5bn5e1ja1g854tl
# 
# Begin patch
=== modified file 'src/FwdState.cc'
--- src/FwdState.cc	2015-07-14 17:51:21 +0000
+++ src/FwdState.cc	2015-10-28 03:47:56 +0000
@@ -772,6 +772,21 @@
         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 @@
             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 @@
 
         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;

=== modified file 'src/FwdState.h'
--- src/FwdState.h	2015-01-13 09:13:49 +0000
+++ src/FwdState.h	2015-10-28 03:47:56 +0000
@@ -120,6 +120,8 @@
     /// 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;

=== modified file 'src/comm/TcpAcceptor.cc'
--- src/comm/TcpAcceptor.cc	2015-07-16 07:00:12 +0000
+++ src/comm/TcpAcceptor.cc	2015-10-28 03:47:56 +0000
@@ -181,13 +181,11 @@
     // 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
 

=== modified file 'src/ip/Qos.cci'
--- src/ip/Qos.cci	2015-01-13 09:13:49 +0000
+++ src/ip/Qos.cci	2015-10-28 03:47:56 +0000
@@ -19,6 +19,8 @@
     //     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 @@
 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 @@
 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 @@
 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;
 }