Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#857 closed Enhancement (fixed)

DSCP support for Transmission

Reported by: RonF Owned by: charles
Priority: Normal Milestone: 1.20
Component: Transmission Version: 1.11
Severity: Normal Keywords:
Cc:

Description

For a while now, I've been interested in getting support in Transmission for setting the DSCP bits on peer connections so I could more easily do traffic shaping on my router. I noticed recently that the Transmission source was available in svn, and so I spend a few hours adding this feature to the latest version.

So far, I've added this support in the CLI, GTk, and English MacOS UIs. I haven't hooked it up to the IPC framework or added it to the Windows client, but it should be relatively straightforward to do that if anyone is interested. I'm attaching my changes to the bug. Feel free to include these changes in the official Transmission release if you find them worthy. Let me know if you have any questions about them.

Attachments (2)

transmission_dscp.patch (46.1 KB) - added by RonF 14 years ago.
Patch to add DSCP support to Transmission
transmission_dscp2.patch (4.4 KB) - added by RonF 14 years ago.

Download all attachments as: .zip

Change History (14)

Changed 14 years ago by RonF

Patch to add DSCP support to Transmission

comment:1 Changed 14 years ago by charles

  • Milestone changed from None Set to 1.20
  • Owner set to charles
  • Status changed from new to assigned

comment:2 Changed 14 years ago by charles

  • Milestone changed from 1.20 to None Set

In general I like the idea of making it easier to shape Transmission's traffic on a local network so that your browsing can take higher precedence, but I have a couple of questions about this patch:

(1) by setting the IP_TOS to 0x20 (IPTOS_PREC_PRIORITY), doesn't that elevate the BT peer traffic's precedence above traffic on other applications that didn't set its IP_TOS and is therefore set at 0X00 (IPTOS_PREC_ROUTINE)?

(2) This patch calls setsockopt(IP_TOS) for incoming sockets too. Is this intentional? The patch keeps referring to "outgoing dscp".

comment:3 Changed 14 years ago by charles

Regarding (1) above: http://lists.ibiblio.org/pipermail/bittorrent/2004-December/000392.html brings up this same point because Bram's client was using 32, and the post suggested that was an error and that 0x08 (IPTOS_THROUGHPUT) would be the better choice. In the latest source release of Bram's client, 0x08 *is* being used (via peer_socket_tos in defaultargs.py). IMO that's what we should be using as well.

comment:4 Changed 14 years ago by charles

  • Milestone changed from None Set to 1.20

r5588 is part one of this commit, covering libT and the gtk+ client.

RonF: I'm very grateful for your patch, because it's a nice feature and because you took the time to make it fit with the flow of libtransmission. Thanks!

However I really don't like the idea of being able to set this in the GUI to an arbitrary int value, because (1) 99% (or more) of users will have no idea what it means. (2) some subset of those 99% of users will change the number to who-knows-what just to see what happens. hilarity ensues. (3) those who *do* know what the feature's for will have to read an RFC or a system header file to find the right integer value. And (4) the "change-the-peers'-tos-on-the-fly" subfeature is far more LOC than it's worth.

So r5588 is a minimalist subset of your patch. The TOS value is set once on startup, and can be overridden in the gtk+ client via editing the prefs.ini file. This way determined hackers can have configurability, but average users won't get tripped up by it.

comment:5 Changed 14 years ago by charles

Hi Charles...

Sorry to bother you this way, but I can't seem to log in to the Transmission Trac site any more to add comments to the entry I filed there. It won't accept my password, and I don't see an option to reset it.

Regarding the comments/questions you had, here are some answers:

  • The definitions in the UNIX include files of IP TOS values are out of date with the latest use of these bits. It used to be that 3 bits were used to specify precedence (Routine, Priority, etc.), three other bits set specific traffic characteristics (Low Delay, High Throughput, and High Reliability), and two other bits were initially reserved and later used for TCP Explicit Congestion Notification. With that definition, what you're saying about using IPTOS_THROUGHPUT instead of IPTOS_PREC_PRIORITY makes sense. However, these bits are now defined differently -- see RFC 2474 and the article at http://www.cisco.com/en/US/tech/tk543/tk757/technologies_tech_note09186a00800949f2.shtml for more info. The default I picked corresponds to CS1 there, which has been unofficially defined as a "Scavenger" or "Bulk Handling" class with less precedence than best-effort. One reference about that is RFC 3662 (though it only suggests CS1 when a class selector is used, and does not mandate it). There are a number of other references which suggest CS1 as well for this if you check around, though.
  • On the point of setting it on incoming sockets, my understanding was that peer sockets were used to send data in both directions once they were open. The "outgoing DSCP" refers to the fact that outgoing DSCP packets are marked with these bits. This is true regardless of which peer initially opened the connection, though. If you only set it on outgoing connections, all the data packets sent out to peers who opened a connection to you would not be properly marked. I debated about also setting it on the listening sockets to get the SYN packets marked correctly, but decided that was too many LOC for the tiny amount of traffic it would affect.
  • On the point of whether to provide a GUI for this or not, my initial thought was to provide some kind of drop-down menu to give guidance about what values to set. However, if you read the specs, it is made very clear that every local network is free to make the values 0-63 in any way they like and the predefined classes are just suggestions. Also, I wanted to get an initial change to you quickly, as I was about to head out on vacation. I think it's fine if this starts off as a hidden setting that open applies at startup (and that is in fact the first thing I coded when I was writing it), but I would like to provide nicer access to it eventually if we can figure out a good way to give people guidance about how to set it.
  • I only did the update on-the-fly code because I had the GUI for it. While it did touch a lot of files, it actually wasn't that many LOC. The iterator support for finding all the sockets made it pretty easy. I'm happy to accept whatever you think is best on this point, though -- even if we add it to the GUI, we could always mark the setting as taking effect on the next restart.

comment:6 Changed 14 years ago by charles

  • Resolution set to fixed
  • Status changed from assigned to closed

Changed 14 years ago by RonF

comment:7 Changed 14 years ago by RonF

  • Resolution fixed deleted
  • Status changed from closed to reopened

Since I'm on the Mac, I need to make one additional change to hook up the defaults support to let me change the initial TOS value using the transmission preferences plist file. Also, I rewrote my changes to the transmission CLI to allow it to be set there using a command line option, and changed the default to 32 in line with my comments above. Would it be possible to get these changes added to what you've already checked in? A new patch is attached.

comment:8 Changed 14 years ago by charles

  • Resolution set to fixed
  • Status changed from reopened to closed

Changing the default value, as in the latest patch, seems a little bleeding-edge to me. Some cursory Googling indicates that, in addition to the mainline client, several ftp clients also use the default value we're using.

comment:9 Changed 14 years ago by mezz

How about add an advanced tab option? It can be always add a warning about poke in advanced option.

comment:10 Changed 14 years ago by livings124

There is no need for this option in 99.9% of cases. Most users will just end up making things worse if the option's available. mezz: do you have a reason for wanting this feature?

comment:11 Changed 14 years ago by mezz

I don't even need DSCP support. I am just being an open mind and make a suggest, that's all. :-)

comment:12 Changed 14 years ago by livings124

See, most people have no need for it, but with it being there people will use it and create problems. ;)

Note: See TracTickets for help on using tickets.