Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ip-dscp option to specify the DSCP tag for outgoing packets #200

Merged
merged 2 commits into from Mar 24, 2020

Conversation

yarikk
Copy link
Contributor

@yarikk yarikk commented Mar 23, 2020

Summary

DSCP, the Differentiated Services codepoint, is a field in the IP header packets used for QoS. It replaces the obsolete TOS field in IPv4 and the Traffic Class field in IPv6, per RFC 3260.

Let's add ip-dscp server option to specify an integer value to be passed to setsockopt(…, IP_TOS, …) for every internet-family socket created, effectively enabling the QoS tagging of all outgoing packets.

Testing

Parsing of the new option confirmed with unbound-checkconf on testdata/04-checkconf.tdir/good.all and testdata/04-checkconf.tdir/bad.dscp.

A back-port to 1.9.6 was baked on production systems; the fact the outgoing traffic gets tagged is confirmed with tcpdump and other network monitoring tools:

11:09:31.574171 IP6 (class 0x8c, hlim 127, next-header UDP (17) payload length: 97) …
11:09:32.004307 IP (tos 0x8c, ttl 96, id 24361, offset 0, flags [none], proto UDP (17), length 79) …

cc @ralphdolmans @chantra

Copy link
Member

@wcawijngaards wcawijngaards left a comment

Changes look good, I'll make some modifications after merge. What is it useful for to set the DSCP tag on DNS traffic?

@wcawijngaards wcawijngaards merged commit a96a7a6 into NLnetLabs:master Mar 24, 2020
1 check failed
wcawijngaards added a commit that referenced this pull request Mar 24, 2020
- Merge PR #200 from yarikk: add ip-dscp option to specify the DSCP
  tag for outgoing packets.
@wcawijngaards
Copy link
Member

wcawijngaards commented Mar 24, 2020

Did you really mean with the case fallthrought to set both IPv6 and IPv4 TOS on IPv6 sockets? I now fixed it to set IPv6 for IPv6 sockets and IPv4 for IPv4 sockets. But the initial code has a case statement fallthrough which makes IPv4 TOS for IPv6 sockets in addition to the IPv6 socket TCLASS.

@wcawijngaards
Copy link
Member

wcawijngaards commented Mar 24, 2020

Thanks for the contribution! Merged it and some code alterations. Please let me know about the fallthrough so we can be sure to set the right socket options. I am also curious what it is used for.

@yarikk
Copy link
Contributor Author

yarikk commented Mar 24, 2020

@wcawijngaards Thanks for accepting and the follow-up fixes!

It is useable in QoS settings to differentiate the control-plane traffic like DNS and ensure is not pushed off the queues by the regular data traffic.

As for the fall though, thanks for the catch: it went under the radar. Let me verify if it works the same way with the breaks in place.

@yarikk
Copy link
Contributor Author

yarikk commented Mar 24, 2020

@wcawijngaards Back-ported the fall-through fix – confirmed to work as intended. Thanks!

@yarikk yarikk deleted the ipdiffserv branch Mar 24, 2020
@wcawijngaards
Copy link
Member

wcawijngaards commented Mar 25, 2020

Cool that you tested it, that is very nice. Another bug removed by code review :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants