------------------------------------------------------------ revno: 13505 revision-id: chtsanti@users.sourceforge.net-20140721145527-0ijfbpi3j6s3okvx parent: squid3@treenet.co.nz-20140721053957-gtw2je6tkooslo27 committer: Christos Tsantilas branch nick: trunk timestamp: Mon 2014-07-21 17:55:27 +0300 message: Fix tcp outgoing tos bugs The tcp_outgoing_tos is buggy in trunk: - The ToS is never set for packets of the first request of a TCP connection. - The ToS is never set for HTTPS traffic no matter whether requests are bumped or not. - The ToS value is not set for ftp data connections This patch solve the above problems: - It moves the codes which sets the TOS value for a new connection from the the comm_openex to a higher-level code, where the connection protocol (IPv4 or IPv6) is known. - Add code to set TOS value for ftp data connections. - Add a check on parsing code to warn users if the configured ToS value has the ECN bits set, and adjust the value to a correct one. Notes Currently squid support only passive ftp data connections. If squid in the future supports active ftp connections, then some work required to TcpAcceptor class to allow setting ToS values for connections established on squid listening sockets. This is a Measurement Factory project ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: chtsanti@users.sourceforge.net-20140721145527-\ # 0ijfbpi3j6s3okvx # target_branch: http://bzr.squid-cache.org/bzr/squid3/trunk/ # testament_sha1: 48ec1e8826d013399e42b46af0481b133037291b # timestamp: 2014-07-21 15:53:50 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/trunk/ # base_revision_id: squid3@treenet.co.nz-20140721053957-\ # gtw2je6tkooslo27 # # Begin patch === modified file 'src/FwdState.cc' --- src/FwdState.cc 2014-07-10 14:47:53 +0000 +++ src/FwdState.cc 2014-07-21 14:55:27 +0000 @@ -825,6 +825,20 @@ 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 + // the server may close the pinned connection before this request pconnRace = racePossible; dispatch(); === modified file 'src/cache_cf.cc' --- src/cache_cf.cc 2014-07-14 12:58:03 +0000 +++ src/cache_cf.cc 2014-07-21 14:55:27 +0000 @@ -1463,6 +1463,12 @@ return; } + const unsigned int chTos = tos & 0xFC; + if (chTos != tos) { + debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: Tos value '" << tos << "' adjusted to '" << chTos << "'"); + tos = chTos; + } + CBDATA_INIT_TYPE_FREECB(acl_tos, freed_acl_tos); l = cbdataAlloc(acl_tos); === modified file 'src/cf.data.pre' --- src/cf.data.pre 2014-07-17 01:48:19 +0000 +++ src/cf.data.pre 2014-07-21 14:55:27 +0000 @@ -1893,6 +1893,8 @@ Processing proceeds in the order specified, and stops at first fully matching line. + + Only fast ACLs are supported. DOC_END NAME: clientside_tos @@ -1935,6 +1937,8 @@ acl good_service_net src 10.0.1.0/24 tcp_outgoing_mark 0x00 normal_service_net tcp_outgoing_mark 0x20 good_service_net + + Only fast ACLs are supported. DOC_END NAME: clientside_mark === modified file 'src/comm.cc' --- src/comm.cc 2014-06-09 13:18:48 +0000 +++ src/comm.cc 2014-07-21 14:55:27 +0000 @@ -82,7 +82,7 @@ */ static IOCB commHalfClosedReader; -static void comm_init_opened(const Comm::ConnectionPointer &conn, tos_t tos, nfmark_t nfmark, const char *note, struct addrinfo *AI); +static void comm_init_opened(const Comm::ConnectionPointer &conn, const char *note, struct addrinfo *AI); static int comm_apply_flags(int new_socket, Ip::Address &addr, int flags, struct addrinfo *AI); #if USE_DELAY_POOLS @@ -245,7 +245,7 @@ int flags, const char *note) { - return comm_openex(sock_type, proto, addr, flags, 0, 0, note); + return comm_openex(sock_type, proto, addr, flags, note); } void @@ -258,7 +258,7 @@ conn->flags |= COMM_DOBIND; /* attempt native enabled port. */ - conn->fd = comm_openex(sock_type, proto, conn->local, conn->flags, 0, 0, note); + conn->fd = comm_openex(sock_type, proto, conn->local, conn->flags, note); } int @@ -274,7 +274,7 @@ flags |= COMM_DOBIND; /* attempt native enabled port. */ - sock = comm_openex(sock_type, proto, addr, flags, 0, 0, note); + sock = comm_openex(sock_type, proto, addr, flags, note); return sock; } @@ -349,8 +349,6 @@ int proto, Ip::Address &addr, int flags, - tos_t tos, - nfmark_t nfmark, const char *note) { int new_socket; @@ -408,14 +406,6 @@ debugs(50, 3, "comm_openex: Opened socket " << conn << " : family=" << AI->ai_family << ", type=" << AI->ai_socktype << ", protocol=" << AI->ai_protocol ); - /* set TOS if needed */ - if (tos) - Ip::Qos::setSockTos(conn, tos); - - /* set netfilter mark if needed */ - if (nfmark) - Ip::Qos::setSockNfmark(conn, nfmark); - if ( Ip::EnableIpv6&IPV6_SPECIAL_SPLITSTACK && addr.isIPv6() ) comm_set_v6only(conn->fd, 1); @@ -424,7 +414,7 @@ if ( Ip::EnableIpv6&IPV6_SPECIAL_V4MAPPING && addr.isIPv6() ) comm_set_v6only(conn->fd, 0); - comm_init_opened(conn, tos, nfmark, note, AI); + comm_init_opened(conn, note, AI); new_socket = comm_apply_flags(conn->fd, addr, flags, AI); Ip::Address::FreeAddrInfo(AI); @@ -439,8 +429,6 @@ /// update FD tables after a local or remote (IPC) comm_openex(); void comm_init_opened(const Comm::ConnectionPointer &conn, - tos_t tos, - nfmark_t nfmark, const char *note, struct addrinfo *AI) { @@ -458,9 +446,6 @@ fde *F = &fd_table[conn->fd]; F->local_addr = conn->local; - F->tosToServer = tos; - - F->nfmarkToServer = nfmark; F->sock_family = AI->ai_family; } @@ -537,7 +522,7 @@ assert(Comm::IsConnOpen(conn)); assert(AI); - comm_init_opened(conn, 0, 0, note, AI); + comm_init_opened(conn, note, AI); if (!(conn->flags & COMM_NOCLOEXEC)) fd_table[conn->fd].flags.close_on_exec = true; === modified file 'src/comm.h' --- src/comm.h 2014-06-05 14:57:58 +0000 +++ src/comm.h 2014-07-21 14:55:27 +0000 @@ -51,7 +51,7 @@ int comm_open_listener(int sock_type, int proto, Ip::Address &addr, int flags, const char *note); void comm_open_listener(int sock_type, int proto, Comm::ConnectionPointer &conn, const char *note); -int comm_openex(int, int, Ip::Address &, int, tos_t tos, nfmark_t nfmark, const char *); +int comm_openex(int, int, Ip::Address &, int, const char *); unsigned short comm_local_port(int fd); int comm_udp_sendto(int sock, const Ip::Address &to, const void *buf, int buflen); === modified file 'src/comm/ConnOpener.cc' --- src/comm/ConnOpener.cc 2014-07-10 14:47:53 +0000 +++ src/comm/ConnOpener.cc 2014-07-21 14:55:27 +0000 @@ -14,6 +14,7 @@ #include "icmp/net_db.h" #include "ip/tools.h" #include "ipcache.h" +#include "ip/QosConfig.h" #include "SquidConfig.h" #include "SquidTime.h" @@ -254,12 +255,25 @@ if (callback_ == NULL || callback_->canceled()) return false; - temporaryFd_ = comm_openex(SOCK_STREAM, IPPROTO_TCP, conn_->local, conn_->flags, conn_->tos, conn_->nfmark, host_); + temporaryFd_ = comm_openex(SOCK_STREAM, IPPROTO_TCP, conn_->local, conn_->flags, host_); if (temporaryFd_ < 0) { sendAnswer(Comm::ERR_CONNECT, 0, "Comm::ConnOpener::createFd"); return false; } + // Set TOS if needed. + if (conn_->tos && + Ip::Qos::setSockTos(temporaryFd_, conn_->tos, conn_->remote.isIPv4() ? AF_INET : AF_INET6) < 0) + conn_->tos = 0; +#if SO_MARK + if (conn_->nfmark && + Ip::Qos::setSockNfmark(temporaryFd_, conn_->nfmark) < 0) + conn_->nfmark = 0; +#endif + + fd_table[conn_->fd].tosToServer = conn_->tos; + fd_table[conn_->fd].nfmarkToServer = conn_->nfmark; + typedef CommCbMemFunT abortDialer; calls_.earlyAbort_ = JobCallback(5, 4, abortDialer, this, Comm::ConnOpener::earlyAbort); comm_add_close_handler(temporaryFd_, calls_.earlyAbort_); === modified file 'src/comm/TcpAcceptor.cc' --- src/comm/TcpAcceptor.cc 2014-07-14 09:48:47 +0000 +++ src/comm/TcpAcceptor.cc 2014-07-21 14:55:27 +0000 @@ -47,6 +47,7 @@ #include "fde.h" #include "globals.h" #include "ip/Intercept.h" +#include "ip/QosConfig.h" #include "MasterXaction.h" #include "profiler/Profiler.h" #include "SquidConfig.h" @@ -199,6 +200,21 @@ #endif } +#if 0 + // Untested code. + // 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 SO_MARK + if (conn->nfmark && + Ip::Qos::setSockNfmark(conn->fd, conn->nfmark) < 0) + conn->nfmark = 0; +#endif +#endif + typedef CommCbMemFunT Dialer; closer_ = JobCallback(5, 4, Dialer, this, Comm::TcpAcceptor::handleClosure); comm_add_close_handler(conn->fd, closer_); === modified file 'src/ftp.cc' --- src/ftp.cc 2014-06-05 14:57:58 +0000 +++ src/ftp.cc 2014-07-21 14:55:27 +0000 @@ -643,6 +643,9 @@ debugs(9, 3, HERE << "Unconnected data socket created on " << conn); } + conn->tos = ctrl.conn->tos; + conn->nfmark = ctrl.conn->nfmark; + assert(Comm::IsConnOpen(conn)); AsyncJob::Start(new Comm::TcpAcceptor(conn, note, sub)); @@ -2469,6 +2472,8 @@ conn->local.port(0); conn->remote = ftpState->ctrl.conn->remote; conn->remote.port(port); + conn->tos = ftpState->ctrl.conn->tos; + conn->nfmark = ftpState->ctrl.conn->nfmark; debugs(9, 3, HERE << "connecting to " << conn->remote); === modified file 'src/ip/Qos.cci' --- src/ip/Qos.cci 2013-10-29 10:51:07 +0000 +++ src/ip/Qos.cci 2014-07-21 14:55:27 +0000 @@ -3,7 +3,7 @@ #include "Debug.h" int -Ip::Qos::setSockTos(const Comm::ConnectionPointer &conn, tos_t tos) +Ip::Qos::setSockTos(const int fd, tos_t tos, int type) { // Bug 3731: FreeBSD produces 'invalid option' // unless we pass it a 32-bit variable storing 8-bits of data. @@ -11,26 +11,21 @@ // so we convert to a int before setting. int bTos = tos; - if (conn->remote.isIPv4()) { + if (type == AF_INET) { #if defined(IP_TOS) - int x = setsockopt(conn->fd, IPPROTO_IP, IP_TOS, &bTos, sizeof(bTos)); + const int x = setsockopt(fd, IPPROTO_IP, IP_TOS, &bTos, sizeof(bTos)); if (x < 0) - debugs(50, 2, "Ip::Qos::setSockTos: setsockopt(IP_TOS) on " << conn << ": " << xstrerror()); - else - conn->tos = tos; + debugs(50, 2, "Ip::Qos::setSockTos: setsockopt(IP_TOS) on " << fd << ": " << xstrerror()); return x; #else debugs(50, DBG_IMPORTANT, "WARNING: setsockopt(IP_TOS) not supported on this platform"); return -1; #endif - - } else { // if (conn->remote.isIPv6()) { + } else { // type == AF_INET6 #if defined(IPV6_TCLASS) - int x = setsockopt(conn->fd, IPPROTO_IPV6, IPV6_TCLASS, &bTos, sizeof(bTos)); + const int x = setsockopt(fd, IPPROTO_IPV6, IPV6_TCLASS, &bTos, sizeof(bTos)); if (x < 0) - debugs(50, 2, "Ip::Qos::setSockTos: setsockopt(IPV6_TCLASS) on " << conn << ": " << xstrerror()); - else - conn->tos = tos; + debugs(50, 2, "Ip::Qos::setSockTos: setsockopt(IPV6_TCLASS) on " << fd << ": " << xstrerror()); return x; #else debugs(50, DBG_IMPORTANT, "WARNING: setsockopt(IPV6_TCLASS) not supported on this platform"); @@ -42,14 +37,22 @@ } int -Ip::Qos::setSockNfmark(const Comm::ConnectionPointer &conn, nfmark_t mark) +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; + + return x; +} + +int +Ip::Qos::setSockNfmark(const int fd, nfmark_t mark) { #if SO_MARK && USE_LIBCAP - int x = setsockopt(conn->fd, SOL_SOCKET, SO_MARK, &mark, sizeof(nfmark_t)); + const int x = setsockopt(fd, SOL_SOCKET, SO_MARK, &mark, sizeof(nfmark_t)); if (x < 0) - debugs(50, 2, "setSockNfmark: setsockopt(SO_MARK) on " << conn << ": " << xstrerror()); - else - conn->nfmark = mark; + debugs(50, 2, "setSockNfmark: setsockopt(SO_MARK) on " << fd << ": " << xstrerror()); return x; #elif USE_LIBCAP debugs(50, DBG_IMPORTANT, "WARNING: setsockopt(SO_MARK) not supported on this platform"); @@ -60,6 +63,15 @@ #endif } +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; + return x; +} + bool Ip::Qos::Config::isHitTosActive() const { === modified file 'src/ip/QosConfig.h' --- src/ip/QosConfig.h 2014-02-21 10:46:19 +0000 +++ src/ip/QosConfig.h 2014-07-21 14:55:27 +0000 @@ -123,6 +123,14 @@ _SQUID_INLINE_ int setSockTos(const Comm::ConnectionPointer &conn, tos_t tos); /** +* The low level variant of setSockTos function to set TOS value of packets. +* Avoid if you can use the Connection-based setSockTos(). +* @param fd Descriptor of socket to set the TOS for +* @param type The socket family, AF_INET or AF_INET6 +*/ +_SQUID_INLINE_ int setSockTos(const int fd, tos_t tos, int type); + +/** * Function to set the netfilter mark value of packets. Sets the value on the * socket which then gets copied to the packets. Called from Ip::Qos::doNfmarkLocalMiss * @param conn Descriptor of socket to set the mark for @@ -130,6 +138,14 @@ _SQUID_INLINE_ int setSockNfmark(const Comm::ConnectionPointer &conn, nfmark_t mark); /** +* The low level variant of setSockNfmark function to set the netfilter mark +* value of packets. +* Avoid if you can use the Connection-based setSockNfmark(). +* @param fd Descriptor of socket to set the mark for +*/ +_SQUID_INLINE_ int setSockNfmark(const int fd, nfmark_t mark); + +/** * QOS configuration class. Contains all the parameters for QOS functions as well * as functions to check whether either TOS or MARK QOS is enabled. */