tcp: improve sending of TTL/hoplimit and DSCP

[tuexen]

Summary

This patch improves the sending of TCP segments with user specified TTL/hoplimit or DSCP. For IPv6, the code within the IPv6 code is always used.

Test Plan

Use the setsockopt() tests, which have been improved in https://github.com/freebsd-net/tcp-testsuite/commit/6f0aeb0ffdad8044b00ded5b8ca26a892268e150.

[rscheff]

sys/netinet/tcp_stacks/rack.c

18837

shouldn't this be symetrical to the ip6_output above? At least in the base stack, inp->inp_options seem to be used

sys/netinet/tcp_subr.c

2214

Detto?

[tuexen]

sys/netinet/tcp_stacks/rack.c

18837

This is related to IPv4 options. I don't think the RACK stack support this right now. If needed, we can fix that separately. Any particular IPv4 options you have in mind?

sys/netinet/tcp_subr.c

2214

I think this just means that the base stack handles IPv4 options inconsistently. If this is a problem, we can fix that separately.

[rscheff]

No particular ipv4 option; guess it shows that IPv4 options are not really relevant (but with ipv6, that aspect did change). Agree - if problems arise with ipv4 lets make a separate diff.

[cc]

sys/netinet/tcp_subr.c

2053–2060

Better use "if (inp != NULL)" for readability.

2054

Looks IPTOS_ECN_CE can't be included in inp->inp_ip_tos, so we don't need this line. If I am right, we can simplify the code as below:

if (inp != NULL) {
        ip->ip_tos = inp->inp_ip_tos;
        ip->ip_ttl = inp->inp_ip_ttl;
} else {
        ip->ip_tos = ect;
        ip->ip_ttl = V_ip_defttl;
}

[tuexen]

Address most of cc's comments.

[tuexen]

sys/netinet/tcp_subr.c

2053–2060

Done.

2054

Right now you are correct, that ect is zero here, but this might change once ECN++ is implemented. So let us keep the code prepared for this.

[cc]

sys/netinet/tcp_subr.c

2054

Looks IPTOS_ECN_CE can't be included in inp->inp_ip_tos, so we don't need this line. If I am right, we can simplify the code as below:

if (inp != NULL) {
        ip->ip_tos = inp->inp_ip_tos;
        ip->ip_ttl = inp->inp_ip_ttl;
} else {
        ip->ip_tos = ect;
        ip->ip_ttl = V_ip_defttl;
}

Withdraw my above comment. Looks ECN and DSCP are separated. Based on Richard's commit in 3b0ee680507a and my read, I didn't find a place where inp->inp_ip_tos can contain any ECN bits. So if just be cautious, the code can be:

if (inp != NULL) {
        KASSERT((inp->inp_ip_tos & IPTOS_ECN_MASK) == 0, ("inp_ip_tos contains ECN bits"));
        ip->ip_tos = inp->inp_ip_tos | ect;
        ip->ip_ttl = inp->inp_ip_ttl;
} else {
        ip->ip_tos = ect;
        ip->ip_ttl = V_ip_defttl;
}

[tuexen]

Be consistent.

[tuexen]

sys/netinet/tcp_subr.c

2054

Lets do this in a consistent way. Other places also clear the ENC bits before applying the new ones.

If we want to remove this and replace it by an KASSERT, then do this separately and consistently in all places. This would catch error in main code paths and not only in one special path.

[cc]

sys/netinet/tcp_subr.c

2054

Sounds good to me. Would you take this path in a next change? Or I can take this work for such checks in main code path.