Set DSCP bits in ip_carp

[darkfiberiru_gmail.com]

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.

[darkfiberiru_gmail.com]

Fixed default setting of new sysctl in man page.

[kp]

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

[darkfiberiru_gmail.com]

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.

[kp]

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.

[darkfiberiru_gmail.com]

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

[kp]

In D14536#316763, @darkfiberiru_gmail.com wrote:

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.

[darkfiberiru_gmail.com]

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

[kp]

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

(Trivial style remark) This line is too long.

[darkfiberiru_gmail.com]

Updated with help from kristof to validate sysctl on change.

[darkfiberiru_gmail.com]

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.