Add DSCP support (alternate/duplicate of #4266)

[jgottula]

What is the purpose of this change?

Enables the user to manually specify a DSCP (TOS) value for the IP packets sent by rclone. This makes it far easier to (de)prioritize rclone upload traffic appropriately via QoS systems.

Previously, doing this sort of thing potentially involved much more hassle.

In my case (a Google Drive user), I was using FireQoS on my firewall/router box to do packet prioritization. And the only good way I could come up with to somewhat-reliably identify which packets on the LAN were from background bulk rclone upload traffic was to write a Bash-and-Python contraption that would do DNS queries of www.googleapis.com every 5 minutes; store the IP addresses in a rudimentary database along with each address's date-and-time-last-seen; and then also every 5 minutes, update the iptables rules on the machine via FireQoS by doing a really horrible scripting thing where I'd basically just dump like ~100 individual match rules in there for each one of the IP addresses in my database that were known to probably be associated with Google Drive. (Yes, I really did that. Yes, I was ashamed of what I had done. But also, yes, it did essentially work. 😝)

With the ability to just set the DSCP value on the packets directly via rclone.conf, I can now use a much simpler (and more exact) matching mechanism on the router box that merely reads the DSCP value and prioritizes accordingly. (With a single iptables match rule, rather than a hundred or so.)

And I believe some hardware also will just automatically Do The Right Thing based on the DSCP values they see. (I think all of my networking hardware is too cheap for that level of fanciness though.)

Was the change discussed in an issue or in the forum before?

#755, #4266.

Why on earth am I submitting a pull request for this now, when a much better one was already submitted just two weeks ago?

Yeah, funny story. So, I originally worked on this patch back around December 2018 (judging by file datestamps). But I'm very much a C++ person, with effectively zero experience with Go at all; and after trying a few things with the available libraries, I couldn't quite get it all to work properly, so I shelved it and eventually ended up writing the previously-described DNS-based monstrosity instead.

Then, today, I decided I'd come back to this dumb thing and make it work once and for all. And I actually was able to get a patch working this time, albeit a somewhat rough one; but it definitely does work, on Linux, for IPv4. (The patch hasn't been tested on anything else.)

Next I set about searching the issues and PR's on this repository so I could find the right issue numbers to reference in preparation to submit a cleaned-up PR... and that's when I discovered that @Max-Sum had already submitted a much nicer PR (#4266) for this exact feature just two weeks ago. 🤦 D'oh! Should have checked that before, not after.

In any case, I figured it would be worthwhile to throw the code I have up here anyway, just in case there's anything to be gained from looking at a slightly different approach to the same problem. For what it's worth, I think #4266 is what you should go forward with; my patch is a bit of a "hey it actually works!" situation, and some aspects of it aren't ideal (e.g. relying on the setsockopt syscall, which isn't particularly existent on Windows). But again, maybe something in here will be useful to incorporate over there.

I'm not sure if the fact that I used the Dialer.Control callback interface in my implementation is a good thing or not. Maybe it's better because the setting will be applied slightly earlier in the connection process? But then I don't know how easy it'd be to use the higher-level SetTOS/SetTrafficClass functions from within there. So I dunno.

One thing I think both #4266 and my own PR could have maybe done better, is to allow specifying either a raw DSCP value or one of the pre-defined RFC 4594 style named values (CS0, AF11, EF, etc). Personally I'd probably leave out the RFC 1349 style named values (low-delay, throughput, reliability, etc).

(It inevitably gets hairy, because over the decades there have been approximately a thousand different, conflicting, mutually-exclusive RFC's and de-facto "standards" for that one stupid damned byte in the IP header. Using just the 6-bit DSCP portion of the TOS byte and using the RFC 4594 style values seems to have emerged as the most agreed-upon approach these days, so I'd probably just stick to that.)

Checklist

• I have read the contribution guidelines.
• I have added tests for all changes in this PR if appropriate.
• I have added documentation for the changes if appropriate.
• All commit messages are in house style.
• I'm done, this Pull Request is ready for review :-)

(Yeah, I didn't exactly get around to all the make-it-spiffy type stuff, for reasons described above. 😜)

[jgottula]

In any case, I'm super happy that DSCP support is likely to make its way into mainline builds of rclone pretty soon now. 🙂

[jgottula]

Huh. Apparently, depending on the exact version of Go (I think?), the syscall.setsockoptint function will either take an fd directly as an int, or will require casting to a syscall.Handle instead.

On my machine, Go grumbled about syscall.Handle not existing and wanting an int instead, so I did that. But now, here, the CI is grumbling about me using an int rather than a syscall.Handle.

Again, I’m not much of a Go programmer. But do they really make old-code-breaking changes like that to their standard libraries all the time? I think I ran into at least one other issue like this one while updating my old Dec 2018 code to compile now.

[jgottula]

Huh. Apparently, adding a flag in this manner doesn't allow it to take effect from the config file at all; only via command-line parameter (--dscp 0x38) or environment variable (RCLONE_DSCP=0x38). When I add a dscp = 0x38 line to any of my remotes' sections in rclone.conf, that value just simply never gets to the proper location where it could take effect. No error or anything, just ignored.

I guess only backend-specific flags are even read at all from the config file; via fs.Get, as registered by fs.ConfigMap, it seems? I've actually yet to figure out how/where the ConfigInfo structure passed to e.g. the fshttp functions is even filled in. I suppose it does appear that ConfigInfo only has the global (non-backend-specific) flag values in it, so it would make sense that there's a separate mechanism going on there. (Maybe another configmap.Map somewhere else that has a different getter setup or something? I'll have to dig around for this...)

---

Okay, so, actually, having searched through the GitHub issues a bit now, it does appear that it is indeed currently intended behavior (albeit not ultimately-ideally-desired behavior) that global flags can't be set in rclone.conf.

Adding a DSCP option further increases the number of global flags that people are going to wish they could set in the config file. Being able to set DSCP per-remote, and/or to set a default value for all remotes (but in the config file, rather than being forced to tack on env vars or command line flags for every rclone invocation), is definitely something I would like to be able to do.

#3706 is recent and contains some ideas for improving the overall config system.

#2697 explicitly proposes implementing support for global flags in the config file in a relatively elegant way based on "profiles"; which, as I understand it, would basically be named config file sections each containing a set of global flag values (and potentially backend flag default values as well). Sounds pretty good, but unfortunately it looks like it's been "coming soon" for a couple of years now. Maybe I could do something to get us at least partially there... 🤔

[ncw]

@jgottula

Thanks for doing the work on this and I'm sorry that it "crossed in the post" with the other pull request.

Now that #4266 has had your feedback are you happy for me to close this?

Thanks also for your notes on the config system - yes these have been coming soon for a while! If you fancied helping with those that would be really useful! I think the profile system is probably the best idea though being able to add global flags into the backend config would be very useful too

[ivandeex]

@jgottula ping
May I close this PR?

[jgottula]

Fine with me.

[ivandeex]

Closing in favor of #4266
@jgottula Thanks for your contribution to both PRs.