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.
Details
- Reviewers
rrs rscheff cc - Group Reviewers
transport - Commits
- rGd66540e829ec: tcp: improve sending of TTL/hoplimit and DSCP
Use the setsockopt() tests, which have been improved in https://github.com/freebsd-net/tcp-testsuite/commit/6f0aeb0ffdad8044b00ded5b8ca26a892268e150.
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
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. |
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.
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; } |
sys/netinet/tcp_subr.c | ||
---|---|---|
2054 |
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; } |
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. |
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. |