Page MenuHomeFreeBSD

Set DSCP bits in ip_carp
ClosedPublic

Authored by darkfiberiru_gmail.com on Feb 27 2018, 6:10 PM.

Details

Summary

Update carp to set DSCP value CS7(Network Traffic) in the flowlabel field of packets by default. Currently carp only sets TOS_LOWDELAY in IPv4 which was deprecated in 1998. This also implements sysctl that can revert carp back to it's old behavior if desired.

This will allow implementation of QOS on modern network devices to make sure carp packets aren't dropped during interface contention.

Test Plan

Load Module.

Setup carp address for both ipv4 and ipv6 and use tcpdump -evv to look at tos values on those packets
Test setting entries and make sure that it is reflected in the captured output
Test setting values outside of 0 to 63 range to make sure they are denied.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fixed default setting of new sysctl in man page.

This revision is now accepted and ready to land.Feb 27 2018, 11:00 PM

Would it make sense to set the DSCP value to the value configured in 'net.inet.carp.dscp' rather than a hardcoded value?

In D14536#305526, @kristof wrote:

Would it make sense to set the DSCP value to the value configured in 'net.inet.carp.dscp' rather than a hardcoded value?

I went back and forth on this and finally came to the conclusion that CS7 made the most sense as this type of traffic directly fits the category of network control traffic.

If we allow it to be set in what way do you thing we should set it (integer or string). Also do you still think cs7 is the correct default in that case or does something else make more sense to you.

I have no specific views on what the value should be. The remark mostly comes from the desire to avoid having the kernel enforce policy. It's only a small extra step here for more flexibility.
I think I'd make net.inet.carp.dscp be an integer, with a default value of (the value of) IPTOS_LOWDELAY.

In D14536#305909, @kristof wrote:

I have no specific views on what the value should be. The remark mostly comes from the desire to avoid having the kernel enforce policy. It's only a small extra step here for more flexibility.
I think I'd make net.inet.carp.dscp be an integer, with a default value of (the value of) IPTOS_LOWDELAY.

Sorry I thought I had already replied to this. I have issues with keeping that as the default value as it has been deprecated since 1998. As such it's not really a sane default and not compatible with much modern routing gear. The setting I choose comes directly our of the RFC for this type of traffic.

If we do move it to be an integer the dscp field is only 6 bits long so I will need to figure out what we do there for decimal value because from sysadmin point of view cs7 is 56 while the equivalent tos value bit shifted two places to be 224

Sorry I thought I had already replied to this. I have issues with keeping that as the default value as it has been deprecated since 1998. As such it's not really a sane default and not compatible with much modern routing gear. The setting I choose comes directly our of the RFC for this type of traffic.

Well, as I said, I have no views on what the value should be. I defaulted to keeping the old value, but if you've got a good reason to change it that's fine by me.

If we do move it to be an integer the dscp field is only 6 bits long so I will need to figure out what we do there for decimal value because from sysadmin point of view cs7 is 56 while the equivalent tos value bit shifted two places to be 224

I'd suggest letting sysadmins configure the value they expect (so 56 for cs7) and shifting it when it's filled out in the packet.

Update sysctl to be based on decimal dscp value instead of toggle

This revision now requires review to proceed.Jun 8 2018, 7:46 PM

I'd prefer the sysctl to reject values that are out of range.
What do you think about this: https://people.freebsd.org/~kp/patches/D14536.patch ?

sys/netinet/ip_carp.c
994 ↗(On Diff #43474)

(Trivial style remark) This line is too long.

darkfiberiru_gmail.com edited the test plan for this revision. (Show Details)

Updated with help from kristof to validate sysctl on change.

Thanks for the help kristof. I couldn't find the documentation to bound input of sysctl's but I definitely agree it's better to validate it on entry.

Sorry for the delay on the reply. Wanted to make sure I had fully tested this before using your updated diff.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 1 2018, 8:37 AM
Closed by commit rS335837: carp: Set DSCP value CS7 (authored by kp). · Explain Why
This revision was automatically updated to reflect the committed changes.