Opened 11 years ago

Last modified 8 years ago

#4785 new Bug

Bad TOS values for "lowcost" and "lowdelay" socket TOS option

Reported by: ddebin Owned by: jordan
Priority: High Milestone: None Set
Component: libtransmission Version: 2.50
Severity: Normal Keywords:
Cc:

Description

From the beginning, it seems socket TOS field values for "lowcost" and "lowdelay" options are swapped...

"lowcost"/"mincost" should return 0x02 and "lowdelay" should return 0x10.

Better is to use macros defined in <netinet/ip.h>, IPTOS_LOWCOST and IPTOS_LOWDELAY (besides IPTOS_THROUGHPUT and IPTOS_RELIABILITY).

"throughput" and "reliability" values seem OK.

Attachments (1)

tos.patch​ (794 bytes) - added by ddebin 11 years ago.
Bugfix

Download all attachments as: .zip

Change History (12)

comment:1 Changed 11 years ago by jordan

ddebin, do you have time to make a patch for this?

Changed 11 years ago by ddebin

Bugfix

comment:2 Changed 11 years ago by ddebin

I attached a patch to fix "lowcost" TOS value...

But, in fact the problem is more subtle than expected... Since 1998 and RFC2474, signification of TOS field in IPv4 header has changed and old classification (type of service) is deprecated and we must use DSCP instead. So that words like "throughput", "reliability", "lowdelay" are now meaningless. Only "lowcost" can be transposed in new classification (what I did in the patch).

My advice would to be to revert to old behavior and get rid of these human readable values "throughput", "reliability", "lowdelay", "lowcost" and keep this option as an integer. Hiding the signification of this option from newbie is good in my opinion, because for this option to be efficient, the whole network around the Transmission client must be properly configured.

comment:3 Changed 11 years ago by cfpp2p

My advice would to be to revert to old behavior and get rid of these human readable values "throughput", "reliability", "lowdelay", "lowcost" and keep this option as an integer.

I agree. For those wanting to participate in this discussion here is some pertinent information:

​Type-Of-Service (TOS)

from: https://trac.transmissionbt.com/wiki/EditConfigFiles

peer-socket-tos: String (default = "default") Set the ​Type-Of-Service (TOS) parameter for outgoing TCP packets. Possible values are "default", "lowcost", "throughput", "lowdelay" and "reliability". The value "lowcost" is recommended if you're using a smart router, and shouldn't harm in any case.

from: session.c 

static int
parse_tos( const char *str )
{
    char *p;
    int value;

    if( !evutil_ascii_strcasecmp( str, "" ) )
        return 0;
    if( !evutil_ascii_strcasecmp( str, "default" ) )
        return 0;

    if( !evutil_ascii_strcasecmp( str, "lowcost" ) )
        return 0x10;
    if( !evutil_ascii_strcasecmp( str, "mincost" ) )
        return 0x10;

    if( !evutil_ascii_strcasecmp( str, "throughput" ) )
        return 0x08;
    if( !evutil_ascii_strcasecmp( str, "reliability" ) )
        return 0x04;
    if( !evutil_ascii_strcasecmp( str, "lowdelay" ) )
        return 0x02;

    value = strtol( str, &p, 0 );
    if( !p || ( p == str ) )
        return 0;

    return value;
}

With an integer/octal/hex value an informed user could set the TOS parameter to whatever value that person determined was best for their particular network configuration.

comment:4 Changed 10 years ago by jch

Since 1998 and RFC2474, signification of TOS field in IPv4 header has changed and old classification (type of service) is deprecated and we must use DSCP instead. So that words like "throughput", "reliability", "lowdelay" are now meaningless.

In practice, that's not the case. DSCP is used inside ISP's networks (the DSCP bits are usually cleared at the edge of such networks). Home routers use the TOS interpretation.

Transmission is beyond the edge, so DSCP assignments will be ignored in that case.

My advice would to be to revert to old behavior and get rid of these human readable values "throughput", "reliability", "lowdelay", "lowcost" and keep this option as an integer.

Unless I got something wrong, using an integer is still supported, it's just not documented.

--jch

comment:5 Changed 10 years ago by pathetic_loser

Home routers use the TOS interpretation.

Very dependent on router's firmware to say the least. And getting "user friendly" names doing not what they're supposed to is the worst nightmare of anyone. Since then you have to grab source and see what all those "friendly" names mean. And this is not user-friendly at all.

comment:6 Changed 10 years ago by lucke

I had set up traffic shaping with shorewall, with tos-minimize-delay going to the class with the highest priority, as suggested in the examples in shorewall's docs. Now I've discovered that my traffic from transmission wasn't yielding, it was in fact going through that class with the highest priority. I found out that I had "peer-socket-tos" set to lowcost in transmission's settings, as suggested by transmission's docs. This bug makes the result of following shorewall's and transmission's docs counterproductive.

comment:7 Changed 10 years ago by x190

So which Transmission setting, if any, gave you the desired result?

comment:8 Changed 10 years ago by lucke

I've changed "peer-socket-tos" to "default" and transmission's traffic isn't put in the class with the highest priority anymore. Probably changing it to anything other than "lowcost" would have worked in my case, because shorewall in my (and docs') setup classifies only packets with tos-minimize-delay (lowdelay) and ignores other tos field's values.

Shouldn't the patch just replace 0x02 with 0x10 and vice versa? It leaves 0x02 alone and changes 0x10 to 0x20, which isn't quite a regular tos value (it's IPTOS_CLASS_CS1).

From netinet/ip.h: 

#define IPTOS_LOWDELAY          0x10
#define IPTOS_THROUGHPUT        0x08
#define IPTOS_RELIABILITY       0x04
#define IPTOS_LOWCOST           0x02
#define IPTOS_MINCOST           IPTOS_LOWCOST

From ​http://www.shorewall.net/traffic_shaping.htm:

tos-minimize-delay (16) tos-maximize-throughput (8) tos-maximize-reliability (4) tos-minimize-cost (2) tos-normal-service (0)

-edit-

transmission sets the value of the tos field only for tcp packets. Wouldn't it make sense to do that for udp as well?

Last edited 10 years ago by lucke (previous) (diff)

comment:9 Changed 9 years ago by agusdallalba

This bug still affects Transmission 2.84. I was configuring Transmission remotely and it severely affected ssh latency when it took the highest network priority without warning, so I think it's rather serious.

Moreover, now "lowdelay" doesn't do anything as seen with tcpdump: 

00:05:33.344042 IP (tos 0x0, ttl 64, id 1675, offset 0, flags [DF], proto TCP (6), length 52)
    192.168.0.101.51413 > 89.104.13.185.54625: Flags [.], cksum 0x2855 (incorrect -> 0x506a), ack 250883, win 2641, options [nop,nop,TS val 8927922 ecr 25077550], length 0

"peer-socket-tos": "0x02", is replaced with "peer-socket-tos": "lowdelay", on restart, so it doesn't do anything either, and while other settings work, they are not appled to µTP traffic; setting "peer-socket-tos": "lowcost", here's one TCP and one UDP packet: 

00:09:38.193345 IP (tos 0x10, ttl 64, id 8302, offset 0, flags [DF], proto TCP (6), length 52)
    192.168.0.101.51413 > 80.109.98.106.53452: Flags [.], cksum 0x740b (incorrect -> 0x5af0), ack 2049, win 1236, options [nop,nop,TS val 8989134 ecr 210248343], length 0
00:09:38.194801 IP (tos 0x0, ttl 64, id 54131, offset 0, flags [DF], proto UDP (17), length 1430)
    192.168.0.101.51413 > 204.244.209.40.62576: UDP, length 1402

In case anyone arrives here from Google, this is what I'm using to work around the bug for now. This filters outgoing packets from tcp and udp connections in port 51413 (that might not be owned by transmission's user if it was an incoming connection) and outgoing connections from transmission's user (that might not go through the same port): 

# iptables -A POSTROUTING -t mangle -p tcp --sport 51413 -j TOS --set-tos Minimize-Cost
# iptables -A POSTROUTING -t mangle -p udp --sport 51413 -j TOS --set-tos Minimize-Cost
# iptables -A POSTROUTING -t mangle -m owner --uid-owner debian-transmission -j TOS --set-tos Minimize-Cost

comment:10 Changed 8 years ago by dreck

There may be Low cost > Minimize cost (old: IPTOS_LOWCOST (0x10), new IPTOS_MINCOST (0x02)) 

--- libtransmission/session.c
+++ libtransmission/session.c
@@ -277,16 +277,16 @@ parse_tos (const char *str)
     return 0;
 
   if (!evutil_ascii_strcasecmp (str, "lowcost"))
-    return 0x10;
+    return IPTOS_PREC_PRIORITY;
   if (!evutil_ascii_strcasecmp (str, "mincost"))
-    return 0x10;
+    return IPTOS_MINCOST;
 
   if (!evutil_ascii_strcasecmp (str, "throughput"))
-    return 0x08;
+    return IPTOS_THROUGHPUT;
   if (!evutil_ascii_strcasecmp (str, "reliability"))
-    return 0x04;
+    return IPTOS_RELIABILITY;
   if (!evutil_ascii_strcasecmp (str, "lowdelay"))
-    return 0x02;
+    return IPTOS_LOWDELAY;
 
   value = strtol (str, &p, 0);
   if (!p || (p == str))
@@ -302,10 +302,10 @@ format_tos (int value)
   switch (value)
     {
       case 0: return "default";
-      case 0x10: return "lowcost";
-      case 0x08: return "throughput";
-      case 0x04: return "reliability";
-      case 0x02: return "lowdelay";
+      case IPTOS_PREC_PRIORITY: return "lowcost";
+      case IPTOS_THROUGHPUT: return "throughput";
+      case IPTOS_RELIABILITY: return "reliability";
+      case IPTOS_LOWDELAY: return "lowdelay";
       default:
         tr_snprintf (buf, 8, "%d", value);
         return buf;

Mac OS X 10.11 (15A204h) /usr/include/netinet/ip.h 

/*
 * Definitions for IP type of service (ip_tos)
 */
#define	IPTOS_LOWDELAY		0x10
#define	IPTOS_THROUGHPUT	0x08
#define	IPTOS_RELIABILITY	0x04
#define	IPTOS_MINCOST		0x02
#if 1
/* ECN RFC3168 obsoletes RFC2481, and these will be deprecated soon. */
#define	IPTOS_CE		0x01
#define	IPTOS_ECT		0x02
#endif

/*
 * Definitions for IP precedence (also in ip_tos) (hopefully unused)
 */
#define	IPTOS_PREC_NETCONTROL		0xe0
#define	IPTOS_PREC_INTERNETCONTROL	0xc0
#define	IPTOS_PREC_CRITIC_ECP		0xa0
#define	IPTOS_PREC_FLASHOVERRIDE	0x80
#define	IPTOS_PREC_FLASH		0x60
#define	IPTOS_PREC_IMMEDIATE		0x40
#define	IPTOS_PREC_PRIORITY		0x20
#define	IPTOS_PREC_ROUTINE		0x00

/*
 * ECN (Explicit Congestion Notification) codepoints in RFC3168
 * mapped to the lower 2 bits of the TOS field.
 */
#define	IPTOS_ECN_NOTECT	0x00	/* not-ECT */
#define	IPTOS_ECN_ECT1		0x01	/* ECN-capable transport (1) */
#define	IPTOS_ECN_ECT0		0x02	/* ECN-capable transport (0) */
#define	IPTOS_ECN_CE		0x03	/* congestion experienced */
#define	IPTOS_ECN_MASK		0x03	/* ECN field mask */

comment:11 Changed 8 years ago by dreck

Definitions for IP type of service (ip_tos) [deprecated; use DSCP and CS definitions above instead.]

/usr/include/netinet/ip.h ubuntu 14.04 / 3.19 

/*
 * Definitions for Explicit Congestion Notification (ECN)
 *
 * Taken from RFC-3168, Section 5.
 */

#define	IPTOS_ECN_MASK		0x03
#define	IPTOS_ECN(x)		((x) & IPTOS_ECN_MASK)
#define	IPTOS_ECN_NOT_ECT	0x00
#define	IPTOS_ECN_ECT1		0x01
#define	IPTOS_ECN_ECT0		0x02
#define	IPTOS_ECN_CE		0x03

/*
 * Definitions for IP differentiated services code points (DSCP)
 *
 * Taken from RFC-2597, Section 6 and RFC-2598, Section 2.3.
 */

#define	IPTOS_DSCP_MASK		0xfc
#define	IPTOS_DSCP(x)		((x) & IPTOS_DSCP_MASK)
#define	IPTOS_DSCP_AF11		0x28
#define	IPTOS_DSCP_AF12		0x30
#define	IPTOS_DSCP_AF13		0x38
#define	IPTOS_DSCP_AF21		0x48
#define	IPTOS_DSCP_AF22		0x50
#define	IPTOS_DSCP_AF23		0x58
#define	IPTOS_DSCP_AF31		0x68
#define	IPTOS_DSCP_AF32		0x70
#define	IPTOS_DSCP_AF33		0x78
#define	IPTOS_DSCP_AF41		0x88
#define	IPTOS_DSCP_AF42		0x90
#define	IPTOS_DSCP_AF43		0x98
#define	IPTOS_DSCP_EF		0xb8

/*
 * In RFC 2474, Section 4.2.2.1, the Class Selector Codepoints subsume
 * the old ToS Precedence values.
 */

#define	IPTOS_CLASS_MASK		0xe0
#define	IPTOS_CLASS(class)		((class) & IPTOS_CLASS_MASK)
#define	IPTOS_CLASS_CS0			0x00
#define	IPTOS_CLASS_CS1			0x20
#define	IPTOS_CLASS_CS2			0x40
#define	IPTOS_CLASS_CS3			0x60
#define	IPTOS_CLASS_CS4			0x80
#define	IPTOS_CLASS_CS5			0xa0
#define	IPTOS_CLASS_CS6			0xc0
#define	IPTOS_CLASS_CS7			0xe0

#define	IPTOS_CLASS_DEFAULT		IPTOS_CLASS_CS0

/*
 * Definitions for IP type of service (ip_tos) [deprecated; use DSCP
 * and CS definitions above instead.]
 */
#define	IPTOS_TOS_MASK		0x1E
#define	IPTOS_TOS(tos)		((tos) & IPTOS_TOS_MASK)
#define	IPTOS_LOWDELAY		0x10
#define	IPTOS_THROUGHPUT	0x08
#define	IPTOS_RELIABILITY	0x04
#define	IPTOS_LOWCOST		0x02
#define	IPTOS_MINCOST		IPTOS_LOWCOST

/*
 * Definitions for IP precedence (also in ip_tos) [also deprecated.]
 */
#define	IPTOS_PREC_MASK			IPTOS_CLASS_MASK
#define	IPTOS_PREC(tos)			IPTOS_CLASS(tos)
#define	IPTOS_PREC_NETCONTROL		IPTOS_CLASS_CS7
#define	IPTOS_PREC_INTERNETCONTROL	IPTOS_CLASS_CS6
#define	IPTOS_PREC_CRITIC_ECP		IPTOS_CLASS_CS5
#define	IPTOS_PREC_FLASHOVERRIDE	IPTOS_CLASS_CS4
#define	IPTOS_PREC_FLASH		IPTOS_CLASS_CS3
#define	IPTOS_PREC_IMMEDIATE		IPTOS_CLASS_CS2
#define	IPTOS_PREC_PRIORITY		IPTOS_CLASS_CS1
#define	IPTOS_PREC_ROUTINE		IPTOS_CLASS_CS0
Note: See TracTickets for help on using tickets.