tcp: Improve mtu probe preconditions
Subsystems (4)
v2 cover tcp: Improve mtu probe preconditions
According to RFC4821 Section 7.4 "Protocols MAY delay sending non-probes in order to accumulate enough data" but in practice linux only sends probes when a lot of data accumulates on the send side.
Another improvement is to rely on TCP RACK performing timely loss detection with fewer outstanding packets. If this is enabled the size required for a probe can be shrunk.
Successive successful mtu probes will result in reducing the cwnd since it's measured in packets and we send bigger packets. The cwnd value can get stuck below 11 on low-latency links and this prevents further probing. The cwnd logic in tcp_mtu_probe can be reworked to be based on the the number of packets that we actually need to send instead of arbitrary constants.
It is difficult to improve this behavior without introducing unreasonable delays or even stalls. Looking at the current behavior of tcp_mtu_probe it already waits in some scenarios: when there is not enough room inside cwnd or when there is a gap of unacklowledged data between snd_una and snd_nxt. It appears that it is safe to wait if packets_in_flight() != 0.
---
Previous RFC: https://lore.kernel.org/netdev/[email protected]/
This series seems to be "correct" this time, I would appreciate any feedback. It possible my understanding of when it is safe to return 0 from tcp_mtu_probe is incorrect. It's possible that even current code would interact poorly with delayed acks in some circumstances?
The tcp_xmit_size_goal changes were dropped. It's still possible to see strange interactions between tcp_push_one and mtu probing: If the receiver window is small (60k) the sender will do a "push_one" when half a window is accumulated (30k) and that would prevent mtu probing even if the sender is writing more than enough data in a single syscall.
Leonard Crestez (3):
tcp: Use smaller mtu probes if RACK is enabled
tcp: Adjust congestion window handling for mtu probe
tcp: Wait for sufficient data in tcp_mtu_probe
Documentation/networking/ip-sysctl.rst | 10 ++++ include/net/netns/ipv4.h | 2 + net/ipv4/sysctl_net_ipv4.c | 14 ++++++ net/ipv4/tcp_ipv4.c | 2 + net/ipv4/tcp_output.c | 70 +++++++++++++++++++++----- 5 files changed, 86 insertions(+), 12 deletions(-)
base-commit: e4e92ee78702b13ad55118d8b66f06e1aef62586
Signature · 1 line
2.25.1
RFC v2 1/3 tcp: Use smaller mtu probes if RACK is enabled
RACK allows detecting a loss in rtt + min_rtt / 4 based on just one extra packet. If enabled use this instead of relying of fast retransmit.
--- Documentation/networking/ip-sysctl.rst | 5 +++++ include/net/netns/ipv4.h | 1 + net/ipv4/sysctl_net_ipv4.c | 7 +++++++ net/ipv4/tcp_ipv4.c | 1 + net/ipv4/tcp_output.c | 26 +++++++++++++++++++++++++- 5 files changed, 39 insertions(+), 1 deletion(-)
Documentation/networking/ip-sysctl.rst +5 -0
@@ -349,10 +349,15 @@ tcp_mtu_probe_floor - INTEGER If MTU probing is enabled this caps the minimum MSS used for search_low for the connection. Default : 48 +tcp_mtu_probe_rack - BOOLEAN + Try to use shorter probes if RACK is also enabled + + Default: 1 + tcp_min_snd_mss - INTEGER TCP SYN and SYNACK messages usually advertise an ADVMSS option, as described in RFC 1122 and RFC 6691. If this ADVMSS option is smaller than tcp_min_snd_mss,
include/net/netns/ipv4.h +1 -0
@@ -112,10 +112,11 @@ struct netns_ipv4 {#ifdef CONFIG_NET_L3_MASTER_DEV u8 sysctl_tcp_l3mdev_accept;#endif u8 sysctl_tcp_mtu_probing;int sysctl_tcp_mtu_probe_floor; +int sysctl_tcp_mtu_probe_rack;int sysctl_tcp_base_mss;int sysctl_tcp_min_snd_mss;int sysctl_tcp_probe_threshold;u32 sysctl_tcp_probe_interval;
net/ipv4/sysctl_net_ipv4.c +7 -0
@@ -847,10 +847,17 @@ static struct ctl_table ipv4_net_table[] = { .mode = 0644, .proc_handler = proc_dointvec_minmax, .extra1 = &tcp_min_snd_mss_min, .extra2 = &tcp_min_snd_mss_max, }, + { + .procname = "tcp_mtu_probe_rack", + .data = &init_net.ipv4.sysctl_tcp_mtu_probe_rack, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "tcp_probe_threshold", .data = &init_net.ipv4.sysctl_tcp_probe_threshold, .maxlen = sizeof(int), .mode = 0644,
net/ipv4/tcp_ipv4.c +1 -0
@@ -2892,10 +2892,11 @@ static int __net_init tcp_sk_init(struct net *net) net->ipv4.sysctl_tcp_base_mss =TCP_BASE_MSS ; net->ipv4.sysctl_tcp_min_snd_mss =TCP_MIN_SND_MSS ; net->ipv4.sysctl_tcp_probe_threshold =TCP_PROBE_THRESHOLD ; net->ipv4.sysctl_tcp_probe_interval =TCP_PROBE_INTERVAL ; net->ipv4.sysctl_tcp_mtu_probe_floor =TCP_MIN_SND_MSS ; + net->ipv4.sysctl_tcp_mtu_probe_rack =1 ; net->ipv4.sysctl_tcp_keepalive_time =TCP_KEEPALIVE_TIME ; net->ipv4.sysctl_tcp_keepalive_probes =TCP_KEEPALIVE_PROBES ; net->ipv4.sysctl_tcp_keepalive_intvl =TCP_KEEPALIVE_INTVL ;
net/ipv4/tcp_output.c +25 -1
@@ -2311,10 +2311,19 @@ static bool tcp_can_coalesce_send_queue_head(struct sock *sk, int len) }return true; } +/* Check if rack is supported for current connection */ +static int tcp_mtu_probe_is_rack (const struct sock *sk) +{ +struct net *net =sock_net (sk); + +return (net->ipv4.sysctl_tcp_recovery &TCP_RACK_LOSS_DETECTION && + net->ipv4.sysctl_tcp_mtu_probe_rack); +} +/* Create a new MTU probe if we are ready. * MTU probe is regularly attempting to increase the path MTU by * deliberately sending larger packets. This discovers routing * changes resulting in larger path MTUs. *
@@ -2351,11 +2360,26 @@ static int tcp_mtu_probe(struct sock *sk)* smaller than a threshold, backoff from probing. */ mss_now =tcp_current_mss (sk); probe_size =tcp_mtu_to_mss (sk, (icsk->icsk_mtup.search_high + icsk->icsk_mtup.search_low) >>1 ); - size_needed = probe_size + (tp->reordering +1 ) * tp->mss_cache; +/* Probing the MTU requires one packet which is larger that current MSS as well +* as enough following mtu-sized packets to ensure that a probe loss can be +* detected without a full Retransmit Time Out. +*/ +if (tcp_mtu_probe_is_rack (sk)) { +/* RACK allows recovering in min_rtt / 4 based on just one extra packet +* Use two to account for unrelated losses +*/ + size_needed = probe_size +2 * tp->mss_cache; + }else { +/* Without RACK send enough extra packets to trigger fast retransmit +* This is dynamic DupThresh + 1 +*/ + size_needed = probe_size + (tp->reordering +1 ) * tp->mss_cache; + } + interval = icsk->icsk_mtup.search_high - icsk->icsk_mtup.search_low;/* When misfortune happens, we are reprobing actively, * and then reprobe timer has expired. We stick with current * probing process by not resetting search range to its orignal. */
Signature · 1 line
2.25.1
Neal Cardwell
maintainerRe: [RFCv2 1/3] tcp: Use smaller mtu probes if RACK is enabled
Quoted 3 lines · Leonard Crestez
RACK allows detecting a loss in rtt +
min_rtt/ 4 based on just one extra packet. If enabled use this instead of relying of fast retransmit.
IMHO it would be worth adding some more text to motivate the change, to justify the added complexity and risk from the change. The substance of the change seems to be decreasing the requirement for PMTU probing from needing roughly 5 packets worth of data to needing roughly 3 packets worth of data. It's not clear to me as a reader of this patch by itself that there are lots of applications that very often only have 3-4 packets worth of data to send and yet can benefit greatly from PMTU discovery.
Quoted 15 lines
Suggested-by Neal Cardwell <[email protected]>Signed-off-by Leonard Crestez <[email protected]>--- Documentation/networking/ip-sysctl.rst | 5 +++++ include/net/netns/ipv4.h | 1 + net/ipv4/sysctl_net_ipv4.c | 7 +++++++ net/ipv4/tcp_ipv4.c | 1 + net/ipv4/tcp_output.c | 26 +++++++++++++++++++++++++- 5 files changed, 39 insertions(+), 1 deletion(-)Documentation/networking/ip-sysctl.rst +0
-0
Documentation/networking/ip-sysctl.rst tcp_mtu_probe_floor - INTEGER · line 357
Quoted 9 lines
Documentation/networking/ip-sysctl.rst +4
-0@@ -349,10 +349,15 @@ tcp_mtu_probe_floor - INTEGER If MTU probing is enabled this caps the minimum MSS used for search_low for the connection. Default : 48 +tcp_mtu_probe_rack - BOOLEAN + Try to use shorter probes if RACK is also enabled + + Default: 1
I would vote to not have a sysctl for this. If we think it's a good idea to allow MTU probing with a smaller amount of data if RACK is enabled (which seems true to me), then this is a low-risk enough change that we should just change the behavior.
Quoted 67 lines
Documentation/networking/ip-sysctl.rst +0
-0tcp_min_snd_mss - INTEGER TCP SYN and SYNACK messages usually advertise an ADVMSS option, as described in RFC 1122 and RFC 6691. If this ADVMSS option is smaller than tcp_min_snd_mss,include/net/netns/ipv4.h +1
-0@@ -112,10 +112,11 @@ struct netns_ipv4 {#ifdef CONFIG_NET_L3_MASTER_DEV u8 sysctl_tcp_l3mdev_accept;#endif u8 sysctl_tcp_mtu_probing;int sysctl_tcp_mtu_probe_floor; +int sysctl_tcp_mtu_probe_rack;int sysctl_tcp_base_mss;int sysctl_tcp_min_snd_mss;int sysctl_tcp_probe_threshold;u32 sysctl_tcp_probe_interval;net/ipv4/sysctl_net_ipv4.c +7
-0@@ -847,10 +847,17 @@ static struct ctl_table ipv4_net_table[] = { .mode = 0644, .proc_handler = proc_dointvec_minmax, .extra1 = &tcp_min_snd_mss_min, .extra2 = &tcp_min_snd_mss_max, }, + { + .procname = "tcp_mtu_probe_rack", + .data = &init_net.ipv4.sysctl_tcp_mtu_probe_rack, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "tcp_probe_threshold", .data = &init_net.ipv4.sysctl_tcp_probe_threshold, .maxlen = sizeof(int), .mode = 0644,net/ipv4/tcp_ipv4.c +1
-0@@ -2892,10 +2892,11 @@ static int __net_init tcp_sk_init(struct net *net) net->ipv4.sysctl_tcp_base_mss =TCP_BASE_MSS ; net->ipv4.sysctl_tcp_min_snd_mss =TCP_MIN_SND_MSS ; net->ipv4.sysctl_tcp_probe_threshold =TCP_PROBE_THRESHOLD ; net->ipv4.sysctl_tcp_probe_interval =TCP_PROBE_INTERVAL ; net->ipv4.sysctl_tcp_mtu_probe_floor =TCP_MIN_SND_MSS ; + net->ipv4.sysctl_tcp_mtu_probe_rack =1 ; net->ipv4.sysctl_tcp_keepalive_time =TCP_KEEPALIVE_TIME ; net->ipv4.sysctl_tcp_keepalive_probes =TCP_KEEPALIVE_PROBES ; net->ipv4.sysctl_tcp_keepalive_intvl =TCP_KEEPALIVE_INTVL ;net/ipv4/tcp_output.c +0
-0@@ -2311,10 +2311,19 @@ static bool tcp_can_coalesce_send_queue_head(struct sock *sk, int len) }return true;
net/ipv4/tcp_output.c static bool tcp_can_coalesce_send_queue_head(struct sock *sk, int len) · line 2323
Quoted 10 lines
net/ipv4/tcp_output.c +8
-0} +/* Check if rack is supported for current connection */ +static int tcp_mtu_probe_is_rack (const struct sock *sk) +{ +struct net *net =sock_net (sk); + +return (net->ipv4.sysctl_tcp_recovery &TCP_RACK_LOSS_DETECTION && + net->ipv4.sysctl_tcp_mtu_probe_rack); +}
You may want to use the existing helper, tcp_is_rack(), by moving it to include/net/tcp.h
thanks,
neal
Re: [RFCv2 1/3] tcp: Use smaller mtu probes if RACK is enabled
Quoted 35 lines · Neal Cardwell
Quoted 3 lines · Leonard Crestez
RACK allows detecting a loss in rtt +
min_rtt/ 4 based on just one extra packet. If enabled use this instead of relying of fast retransmit.IMHO it would be worth adding some more text to motivate the change, to justify the added complexity and risk from the change. The substance of the change seems to be decreasing the requirement for PMTU probing from needing roughly 5 packets worth of data to needing roughly 3 packets worth of data. It's not clear to me as a reader of this patch by itself that there are lots of applications that very often only have 3-4 packets worth of data to send and yet can benefit greatly from PMTU discovery.
Quoted 20 lines
Suggested-by Neal Cardwell <[email protected]>Signed-off-by Leonard Crestez <[email protected]>--- Documentation/networking/ip-sysctl.rst | 5 +++++ include/net/netns/ipv4.h | 1 + net/ipv4/sysctl_net_ipv4.c | 7 +++++++ net/ipv4/tcp_ipv4.c | 1 + net/ipv4/tcp_output.c | 26 +++++++++++++++++++++++++- 5 files changed, 39 insertions(+), 1 deletion(-)Documentation/networking/ip-sysctl.rst +0
-0@@ -349,10 +349,15 @@ tcp_mtu_probe_floor - INTEGER If MTU probing is enabled this caps the minimum MSS used for search_low for the connection. Default : 48
Quoted 9 lines
Documentation/networking/ip-sysctl.rst tcp_mtu_probe_floor - INTEGER · line 357
Quoted 4 lines
Documentation/networking/ip-sysctl.rst +4
-0+tcp_mtu_probe_rack - BOOLEAN + Try to use shorter probes if RACK is also enabled + + Default: 1I would vote to not have a sysctl for this. If we think it's a good idea to allow MTU probing with a smaller amount of data if RACK is enabled (which seems true to me), then this is a low-risk enough change that we should just change the behavior.
+1 to not have another sysctl
Quoted 84 lines
Quoted 68 lines
Documentation/networking/ip-sysctl.rst +0
-0tcp_min_snd_mss - INTEGER TCP SYN and SYNACK messages usually advertise an ADVMSS option, as described in RFC 1122 and RFC 6691. If this ADVMSS option is smaller than tcp_min_snd_mss,include/net/netns/ipv4.h +1
-0@@ -112,10 +112,11 @@ struct netns_ipv4 {#ifdef CONFIG_NET_L3_MASTER_DEV u8 sysctl_tcp_l3mdev_accept;#endif u8 sysctl_tcp_mtu_probing;int sysctl_tcp_mtu_probe_floor; +int sysctl_tcp_mtu_probe_rack;int sysctl_tcp_base_mss;int sysctl_tcp_min_snd_mss;int sysctl_tcp_probe_threshold;u32 sysctl_tcp_probe_interval;net/ipv4/sysctl_net_ipv4.c +7
-0@@ -847,10 +847,17 @@ static struct ctl_table ipv4_net_table[] = { .mode = 0644, .proc_handler = proc_dointvec_minmax, .extra1 = &tcp_min_snd_mss_min, .extra2 = &tcp_min_snd_mss_max, }, + { + .procname = "tcp_mtu_probe_rack", + .data = &init_net.ipv4.sysctl_tcp_mtu_probe_rack, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "tcp_probe_threshold", .data = &init_net.ipv4.sysctl_tcp_probe_threshold, .maxlen = sizeof(int), .mode = 0644,net/ipv4/tcp_ipv4.c +1
-0@@ -2892,10 +2892,11 @@ static int __net_init tcp_sk_init(struct net *net) net->ipv4.sysctl_tcp_base_mss =TCP_BASE_MSS ; net->ipv4.sysctl_tcp_min_snd_mss =TCP_MIN_SND_MSS ; net->ipv4.sysctl_tcp_probe_threshold =TCP_PROBE_THRESHOLD ; net->ipv4.sysctl_tcp_probe_interval =TCP_PROBE_INTERVAL ; net->ipv4.sysctl_tcp_mtu_probe_floor =TCP_MIN_SND_MSS ; + net->ipv4.sysctl_tcp_mtu_probe_rack =1 ; net->ipv4.sysctl_tcp_keepalive_time =TCP_KEEPALIVE_TIME ; net->ipv4.sysctl_tcp_keepalive_probes =TCP_KEEPALIVE_PROBES ; net->ipv4.sysctl_tcp_keepalive_intvl =TCP_KEEPALIVE_INTVL ;net/ipv4/tcp_output.c +0
-0@@ -2311,10 +2311,19 @@ static bool tcp_can_coalesce_send_queue_head(struct sock *sk, int len) }return true;net/ipv4/tcp_output.c static bool tcp_can_coalesce_send_queue_head(struct sock *sk, int len) · line 2323
Quoted 10 lines
net/ipv4/tcp_output.c +8
-0} +/* Check if rack is supported for current connection */ +static int tcp_mtu_probe_is_rack (const struct sock *sk) +{ +struct net *net =sock_net (sk); + +return (net->ipv4.sysctl_tcp_recovery &TCP_RACK_LOSS_DETECTION && + net->ipv4.sysctl_tcp_mtu_probe_rack); +}You may want to use the existing helper,
tcp_is_rack(), by moving it toinclude/net/tcp.hthanks,
neal
Re: [RFCv2 1/3] tcp: Use smaller mtu probes if RACK is enabled
In reply to Neal CardwellQuoted 113 lines · Neal Cardwell
Quoted 3 lines · Leonard Crestez
RACK allows detecting a loss in rtt +
min_rtt/ 4 based on just one extra packet. If enabled use this instead of relying of fast retransmit.IMHO it would be worth adding some more text to motivate the change, to justify the added complexity and risk from the change. The substance of the change seems to be decreasing the requirement for PMTU probing from needing roughly 5 packets worth of data to needing roughly 3 packets worth of data. It's not clear to me as a reader of this patch by itself that there are lots of applications that very often only have 3-4 packets worth of data to send and yet can benefit greatly from PMTU discovery.
Quoted 15 lines
Suggested-by Neal Cardwell <[email protected]>Signed-off-by Leonard Crestez <[email protected]>--- Documentation/networking/ip-sysctl.rst | 5 +++++ include/net/netns/ipv4.h | 1 + net/ipv4/sysctl_net_ipv4.c | 7 +++++++ net/ipv4/tcp_ipv4.c | 1 + net/ipv4/tcp_output.c | 26 +++++++++++++++++++++++++- 5 files changed, 39 insertions(+), 1 deletion(-)Documentation/networking/ip-sysctl.rst +0
-0Documentation/networking/ip-sysctl.rst tcp_mtu_probe_floor - INTEGER · line 357
Quoted 9 lines
Documentation/networking/ip-sysctl.rst +4
-0@@ -349,10 +349,15 @@ tcp_mtu_probe_floor - INTEGER If MTU probing is enabled this caps the minimum MSS used for search_low for the connection. Default : 48 +tcp_mtu_probe_rack - BOOLEAN + Try to use shorter probes if RACK is also enabled + + Default: 1I would vote to not have a sysctl for this. If we think it's a good idea to allow MTU probing with a smaller amount of data if RACK is enabled (which seems true to me), then this is a low-risk enough change that we should just change the behavior.
Quoted 69 lines
Documentation/networking/ip-sysctl.rst +0
-0tcp_min_snd_mss - INTEGER TCP SYN and SYNACK messages usually advertise an ADVMSS option, as described in RFC 1122 and RFC 6691. If this ADVMSS option is smaller than tcp_min_snd_mss,include/net/netns/ipv4.h +1
-0@@ -112,10 +112,11 @@ struct netns_ipv4 {#ifdef CONFIG_NET_L3_MASTER_DEV u8 sysctl_tcp_l3mdev_accept;#endif u8 sysctl_tcp_mtu_probing;int sysctl_tcp_mtu_probe_floor; +int sysctl_tcp_mtu_probe_rack;int sysctl_tcp_base_mss;int sysctl_tcp_min_snd_mss;int sysctl_tcp_probe_threshold;u32 sysctl_tcp_probe_interval;net/ipv4/sysctl_net_ipv4.c +7
-0@@ -847,10 +847,17 @@ static struct ctl_table ipv4_net_table[] = { .mode = 0644, .proc_handler = proc_dointvec_minmax, .extra1 = &tcp_min_snd_mss_min, .extra2 = &tcp_min_snd_mss_max, }, + { + .procname = "tcp_mtu_probe_rack", + .data = &init_net.ipv4.sysctl_tcp_mtu_probe_rack, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "tcp_probe_threshold", .data = &init_net.ipv4.sysctl_tcp_probe_threshold, .maxlen = sizeof(int), .mode = 0644,net/ipv4/tcp_ipv4.c +1
-0@@ -2892,10 +2892,11 @@ static int __net_init tcp_sk_init(struct net *net) net->ipv4.sysctl_tcp_base_mss =TCP_BASE_MSS ; net->ipv4.sysctl_tcp_min_snd_mss =TCP_MIN_SND_MSS ; net->ipv4.sysctl_tcp_probe_threshold =TCP_PROBE_THRESHOLD ; net->ipv4.sysctl_tcp_probe_interval =TCP_PROBE_INTERVAL ; net->ipv4.sysctl_tcp_mtu_probe_floor =TCP_MIN_SND_MSS ; + net->ipv4.sysctl_tcp_mtu_probe_rack =1 ; net->ipv4.sysctl_tcp_keepalive_time =TCP_KEEPALIVE_TIME ; net->ipv4.sysctl_tcp_keepalive_probes =TCP_KEEPALIVE_PROBES ; net->ipv4.sysctl_tcp_keepalive_intvl =TCP_KEEPALIVE_INTVL ;net/ipv4/tcp_output.c +0
-0@@ -2311,10 +2311,19 @@ static bool tcp_can_coalesce_send_queue_head(struct sock *sk, int len) }return true; }
Quoted 11 lines
net/ipv4/tcp_output.c static bool tcp_can_coalesce_send_queue_head(struct sock *sk, int len) · line 2323
Quoted 8 lines
net/ipv4/tcp_output.c +8
-0+/* Check if rack is supported for current connection */ +static int tcp_mtu_probe_is_rack (const struct sock *sk) +{ +struct net *net =sock_net (sk); + +return (net->ipv4.sysctl_tcp_recovery &TCP_RACK_LOSS_DETECTION && + net->ipv4.sysctl_tcp_mtu_probe_rack); +}You may want to use the existing helper,
tcp_is_rack(), by moving it toinclude/net/tcp.h
OK, for this and other comments.
Initially I though that maybe a more elaborate check is required but it seems to be only up to the sender to keep individual timeouts.
--
Regards,
Leonard
RFC v2 2/3 tcp: Adjust congestion window handling for mtu probe
In reply to Leonard CrestezOn very fast links after successive succesful MTU probes the cwnd (measured in packets) shrinks and does not grow again because tcp_is_cwnd_limited returns false unless at least half the window is being used. If snd_cwnd falls below 11 then no more probes are sent despite the link being otherwise idle.
When preparing an mtu probe linux checks for snd_cwnd >= 11 and for 2 more packets to fit alongside what is currently in flight. The reasoning behind these constants is unclear. Replace this with checks based on the required probe size:
* Skip probing if congestion window is too small to ever fit a probe.
* Wait for the congestion window to drain if too many packets are already in flight.
This is very similar to snd_wnd logic except packets are counted instead of bytes. This patch also adds more documentation regarding how "return 0" works in tcp_mtu_probe because I found it difficult to understand.
This patch allows mtu probing at smaller cwnd values and does not contradict any standard. Since "0" is only returned if packets are in flight no stalls should happen expect when many acks are lost.
Removing the snd_cwnd >= 11 check also allows probing to happen for bursty traffic where the cwnd is reset to 10 after a few hundred ms of idling.
It does not completely solve the problem of very small cwnds on fast links.
--- net/ipv4/tcp_output.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-)
net/ipv4/tcp_output.c +21 -9
@@ -2328,32 +2328,35 @@ static int tcp_mtu_probe_is_rack(const struct sock *sk)* changes resulting in larger path MTUs. * * Returns 0 if we should wait to probe (no cwnd available), * 1 if a probe was sent, * -1 otherwise +* +* Caller won't queue future write attempts if this returns 0. Zero is only +* returned if acks are pending from packets in flight which will trigger +* tcp_write_xmit again later. */ static int tcp_mtu_probe (struct sock *sk) {struct inet_connection_sock *icsk =inet_csk (sk);struct tcp_sock *tp =tcp_sk (sk);struct sk_buff *skb, *nskb, *next;struct net *net =sock_net (sk);int probe_size;int size_needed; +int packets_needed;int copy, len;int mss_now;int interval;/* Not currently probing/verifying, * not in recovery, -* have enough cwnd, and * not SACKing (the variable headers throw things off) */ if (likely (!icsk->icsk_mtup.enabled || icsk->icsk_mtup.probe_size ||inet_csk (sk)->icsk_ca_state != TCP_CA_Open || - tp->snd_cwnd <11 || tp->rx_opt.num_sacks || tp->rx_opt.dsack))return -1 ;/* Use binary search for probe_size between tcp_mss_base, * and current mss_clamp. if (search_high - search_low)
@@ -2375,10 +2378,11 @@ static int tcp_mtu_probe(struct sock *sk)/* Without RACK send enough extra packets to trigger fast retransmit * This is dynamic DupThresh + 1 */ size_needed = probe_size + (tp->reordering +1 ) * tp->mss_cache; } + packets_needed =DIV_ROUND_UP (size_needed, tp->mss_cache); interval = icsk->icsk_mtup.search_high - icsk->icsk_mtup.search_low;/* When misfortune happens, we are reprobing actively, * and then reprobe timer has expired. We stick with current * probing process by not resetting search range to its orignal.
@@ -2394,22 +2398,30 @@ static int tcp_mtu_probe(struct sock *sk)/* Have enough data in the send queue to probe? */ if (tp->write_seq - tp->snd_nxt < size_needed)return -1 ; +/* Can probe fit inside congestion window? */ +if (packets_needed > tp->snd_cwnd) +return -1 ; + +/* Can probe fit inside receiver window? If not then skip probing. +* The receiver might increase the window as data is processed but +* don't assume that. +* If some data is inflight (between snd_una and snd_nxt) we wait for it to +* clear below. +*/ if (tp->snd_wnd < size_needed)return -1 ; + +/* Do we need for more acks to clear the receive window? */ if (after (tp->snd_nxt + size_needed,tcp_wnd_end (tp)))return 0 ; -/* Do we need to wait to drain cwnd? With none in flight, don't stall */ -if (tcp_packets_in_flight (tp) +2 > tp->snd_cwnd) { -if (!tcp_packets_in_flight (tp)) -return -1 ; -else -return 0 ; - } +/* Do we need the congestion window to clear? */ +if (tcp_packets_in_flight (tp) + packets_needed > tp->snd_cwnd) +return 0 ;if (!tcp_can_coalesce_send_queue_head (sk, probe_size))return -1 ;/* We're allowed to probe. Build it now. */
Signature · 1 line
2.25.1
RFC v2 3/3 tcp: Wait for sufficient data in tcp_mtu_probe
In reply to Leonard CrestezAccording to RFC4821 Section 7.4 "Protocols MAY delay sending non-probes in order to accumulate enough data" but linux almost never does that.
Implement this by returning 0 from tcp_mtu_probe if not enough data is queued locally but some packets are still in flight. This makes mtu probing more likely to happen for applications that do small writes.
Only doing this if packets are in flight should ensure that writing will be attempted again later. This is similar to how tcp_mtu_probe already returns zero if the probe doesn't fit inside the receiver window or the congestion window.
Control this with a sysctl because this implies a latency tradeoff but only up to one RTT.
--- Documentation/networking/ip-sysctl.rst | 5 +++++ include/net/netns/ipv4.h | 1 + net/ipv4/sysctl_net_ipv4.c | 7 +++++++ net/ipv4/tcp_ipv4.c | 1 + net/ipv4/tcp_output.c | 18 ++++++++++++++---- 5 files changed, 28 insertions(+), 4 deletions(-)
Documentation/networking/ip-sysctl.rst +5 -0
@@ -349,10 +349,15 @@ tcp_mtu_probe_floor - INTEGER If MTU probing is enabled this caps the minimum MSS used for search_low for the connection. Default : 48 +tcp_mtu_probe_waitdata - BOOLEAN + Wait for enough data for an mtu probe to accumulate on the sender. + + Default: 1 + tcp_mtu_probe_rack - BOOLEAN Try to use shorter probes if RACK is also enabled Default: 1
include/net/netns/ipv4.h +1 -0
@@ -112,10 +112,11 @@ struct netns_ipv4 {#ifdef CONFIG_NET_L3_MASTER_DEV u8 sysctl_tcp_l3mdev_accept;#endif u8 sysctl_tcp_mtu_probing;int sysctl_tcp_mtu_probe_floor; +int sysctl_tcp_mtu_probe_waitdata;int sysctl_tcp_mtu_probe_rack;int sysctl_tcp_base_mss;int sysctl_tcp_min_snd_mss;int sysctl_tcp_probe_threshold;u32 sysctl_tcp_probe_interval;
net/ipv4/sysctl_net_ipv4.c +7 -0
@@ -847,10 +847,17 @@ static struct ctl_table ipv4_net_table[] = { .mode = 0644, .proc_handler = proc_dointvec_minmax, .extra1 = &tcp_min_snd_mss_min, .extra2 = &tcp_min_snd_mss_max, }, + { + .procname = "tcp_mtu_probe_waitdata", + .data = &init_net.ipv4.sysctl_tcp_mtu_probe_waitdata, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { .procname = "tcp_mtu_probe_rack", .data = &init_net.ipv4.sysctl_tcp_mtu_probe_rack, .maxlen = sizeof(int), .mode = 0644,
net/ipv4/tcp_ipv4.c +1 -0
@@ -2892,10 +2892,11 @@ static int __net_init tcp_sk_init(struct net *net) net->ipv4.sysctl_tcp_base_mss =TCP_BASE_MSS ; net->ipv4.sysctl_tcp_min_snd_mss =TCP_MIN_SND_MSS ; net->ipv4.sysctl_tcp_probe_threshold =TCP_PROBE_THRESHOLD ; net->ipv4.sysctl_tcp_probe_interval =TCP_PROBE_INTERVAL ; net->ipv4.sysctl_tcp_mtu_probe_floor =TCP_MIN_SND_MSS ; + net->ipv4.sysctl_tcp_mtu_probe_waitdata =1 ; net->ipv4.sysctl_tcp_mtu_probe_rack =1 ; net->ipv4.sysctl_tcp_keepalive_time =TCP_KEEPALIVE_TIME ; net->ipv4.sysctl_tcp_keepalive_probes =TCP_KEEPALIVE_PROBES ; net->ipv4.sysctl_tcp_keepalive_intvl =TCP_KEEPALIVE_INTVL ;
net/ipv4/tcp_output.c +14 -4
@@ -2394,14 +2394,10 @@ static int tcp_mtu_probe(struct sock *sk)*/ tcp_mtu_check_reprobe (sk);return -1 ; } -/* Have enough data in the send queue to probe? */ -if (tp->write_seq - tp->snd_nxt < size_needed) -return -1 ; -/* Can probe fit inside congestion window? */ if (packets_needed > tp->snd_cwnd)return -1 ;/* Can probe fit inside receiver window? If not then skip probing.
@@ -2411,10 +2407,24 @@ static int tcp_mtu_probe(struct sock *sk)* clear below. */ if (tp->snd_wnd < size_needed)return -1 ; +/* Have enough data in the send queue to probe? */ +if (tp->write_seq - tp->snd_nxt < size_needed) { +/* If packets are already in flight it's safe to wait for more data to +* accumulate on the sender because writing will be triggered as ACKs +* arrive. +* If no packets are in flight returning zero can stall. +*/ +if (net->ipv4.sysctl_tcp_mtu_probe_waitdata && +tcp_packets_in_flight (tp)) +return 0 ; +else +return -1 ; + } +/* Do we need for more acks to clear the receive window? */ if (after (tp->snd_nxt + size_needed,tcp_wnd_end (tp)))return 0 ;/* Do we need the congestion window to clear? */
Signature · 1 line
2.25.1
Re: [RFCv2 3/3] tcp: Wait for sufficient data in tcp_mtu_probe
Quoted 50 lines · Leonard Crestez
According to RFC4821 Section 7.4 "Protocols MAY delay sending non-probes in order to accumulate enough data" but linux almost never does that.
Implement this by returning 0 from
tcp_mtu_probeif not enough data is queued locally but some packets are still in flight. This makes mtu probing more likely to happen for applications that do small writes.Only doing this if packets are in flight should ensure that writing will be attempted again later. This is similar to how
tcp_mtu_probealready returns zero if the probe doesn't fit inside the receiver window or the congestion window.Control this with a sysctl because this implies a latency tradeoff but only up to one RTT.
Signed-off-by Leonard Crestez <[email protected]>--- Documentation/networking/ip-sysctl.rst | 5 +++++ include/net/netns/ipv4.h | 1 + net/ipv4/sysctl_net_ipv4.c | 7 +++++++ net/ipv4/tcp_ipv4.c | 1 + net/ipv4/tcp_output.c | 18 ++++++++++++++---- 5 files changed, 28 insertions(+), 4 deletions(-)Documentation/networking/ip-sysctl.rst +5
-0@@ -349,10 +349,15 @@ tcp_mtu_probe_floor - INTEGER If MTU probing is enabled this caps the minimum MSS used for search_low for the connection. Default : 48 +tcp_mtu_probe_waitdata - BOOLEAN + Wait for enough data for an mtu probe to accumulate on the sender. + + Default: 1 + tcp_mtu_probe_rack - BOOLEAN Try to use shorter probes if RACK is also enabled Default: 1include/net/netns/ipv4.h +0
-0
include/net/netns/ipv4.h struct netns_ipv4 { · line 117
Quoted 6 lines
include/net/netns/ipv4.h +1
-0@@ -112,10 +112,11 @@ struct netns_ipv4 {#ifdef CONFIG_NET_L3_MASTER_DEV u8 sysctl_tcp_l3mdev_accept;#endif u8 sysctl_tcp_mtu_probing;int sysctl_tcp_mtu_probe_floor; +int sysctl_tcp_mtu_probe_waitdata;
If this is a boolean, you should use u8, and place this field to avoid adding a hole.
Quoted 11 lines
include/net/netns/ipv4.h +0
-0int sysctl_tcp_mtu_probe_rack;int sysctl_tcp_base_mss;int sysctl_tcp_min_snd_mss;int sysctl_tcp_probe_threshold;u32 sysctl_tcp_probe_interval;net/ipv4/sysctl_net_ipv4.c +0
-0@@ -847,10 +847,17 @@ static struct ctl_table ipv4_net_table[] = { .mode = 0644,
net/ipv4/sysctl_net_ipv4.c static struct ctl_table ipv4_net_table[] = { · line 857
Quoted 10 lines
net/ipv4/sysctl_net_ipv4.c +6
-0.proc_handler = proc_dointvec_minmax, .extra1 = &tcp_min_snd_mss_min, .extra2 = &tcp_min_snd_mss_max, }, + { + .procname = "tcp_mtu_probe_waitdata", + .data = &init_net.ipv4.sysctl_tcp_mtu_probe_waitdata, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec,
If this is a boolean, please use proc_dou8vec_minmax, and SYSCTL_ZERO/SYSCTL_ONE
Quoted 45 lines
net/ipv4/sysctl_net_ipv4.c +1
-0+ }, { .procname = "tcp_mtu_probe_rack", .data = &init_net.ipv4.sysctl_tcp_mtu_probe_rack, .maxlen = sizeof(int), .mode = 0644,net/ipv4/tcp_ipv4.c +1
-0@@ -2892,10 +2892,11 @@ static int __net_init tcp_sk_init(struct net *net) net->ipv4.sysctl_tcp_base_mss =TCP_BASE_MSS ; net->ipv4.sysctl_tcp_min_snd_mss =TCP_MIN_SND_MSS ; net->ipv4.sysctl_tcp_probe_threshold =TCP_PROBE_THRESHOLD ; net->ipv4.sysctl_tcp_probe_interval =TCP_PROBE_INTERVAL ; net->ipv4.sysctl_tcp_mtu_probe_floor =TCP_MIN_SND_MSS ; + net->ipv4.sysctl_tcp_mtu_probe_waitdata =1 ; net->ipv4.sysctl_tcp_mtu_probe_rack =1 ; net->ipv4.sysctl_tcp_keepalive_time =TCP_KEEPALIVE_TIME ; net->ipv4.sysctl_tcp_keepalive_probes =TCP_KEEPALIVE_PROBES ; net->ipv4.sysctl_tcp_keepalive_intvl =TCP_KEEPALIVE_INTVL ;net/ipv4/tcp_output.c +0
-4@@ -2394,14 +2394,10 @@ static int tcp_mtu_probe(struct sock *sk)*/ tcp_mtu_check_reprobe (sk);return -1 ; } -/* Have enough data in the send queue to probe? */ -if (tp->write_seq - tp->snd_nxt < size_needed) -return -1 ; -/* Can probe fit inside congestion window? */ if (packets_needed > tp->snd_cwnd)return -1 ;/* Can probe fit inside receiver window? If not then skip probing. @@ -2411,10 +2407,24 @@ static int tcp_mtu_probe(struct sock *sk)* clear below. */ if (tp->snd_wnd < size_needed)
net/ipv4/tcp_output.c static int tcp_mtu_probe(struct sock *sk) · line 2419
Quoted 10 lines
net/ipv4/tcp_output.c +8
-0return -1 ; +/* Have enough data in the send queue to probe? */ +if (tp->write_seq - tp->snd_nxt < size_needed) { +/* If packets are already in flight it's safe to wait for more data to +* accumulate on the sender because writing will be triggered as ACKs +* arrive. +* If no packets are in flight returning zero can stall. +*/ + if (net->ipv4.sysctl_tcp_mtu_probe_waitdata &&
I have serious doubts about RPC traffic.
Adding one RTT latency is going to make this unlikely to be used.
Quoted 14 lines
net/ipv4/tcp_output.c +6
-1+ tcp_packets_in_flight(tp)) + return 0; + else + return -1; + } +/* Do we need for more acks to clear the receive window? */ if (after (tp->snd_nxt + size_needed,tcp_wnd_end (tp)))return 0 ;/* Do we need the congestion window to clear? */ --2.25.1
Re: [RFCv2 3/3] tcp: Wait for sufficient data in tcp_mtu_probe
Quoted 132 lines · Eric Dumazet
Quoted 50 lines · Leonard Crestez
According to RFC4821 Section 7.4 "Protocols MAY delay sending non-probes in order to accumulate enough data" but linux almost never does that.
Implement this by returning 0 from
tcp_mtu_probeif not enough data is queued locally but some packets are still in flight. This makes mtu probing more likely to happen for applications that do small writes.Only doing this if packets are in flight should ensure that writing will be attempted again later. This is similar to how
tcp_mtu_probealready returns zero if the probe doesn't fit inside the receiver window or the congestion window.Control this with a sysctl because this implies a latency tradeoff but only up to one RTT.
Signed-off-by Leonard Crestez <[email protected]>--- Documentation/networking/ip-sysctl.rst | 5 +++++ include/net/netns/ipv4.h | 1 + net/ipv4/sysctl_net_ipv4.c | 7 +++++++ net/ipv4/tcp_ipv4.c | 1 + net/ipv4/tcp_output.c | 18 ++++++++++++++---- 5 files changed, 28 insertions(+), 4 deletions(-)Documentation/networking/ip-sysctl.rst +5
-0@@ -349,10 +349,15 @@ tcp_mtu_probe_floor - INTEGER If MTU probing is enabled this caps the minimum MSS used for search_low for the connection. Default : 48 +tcp_mtu_probe_waitdata - BOOLEAN + Wait for enough data for an mtu probe to accumulate on the sender. + + Default: 1 + tcp_mtu_probe_rack - BOOLEAN Try to use shorter probes if RACK is also enabled Default: 1include/net/netns/ipv4.h +0
-0include/net/netns/ipv4.h struct netns_ipv4 { · line 117
Quoted 6 lines
include/net/netns/ipv4.h +1
-0@@ -112,10 +112,11 @@ struct netns_ipv4 {#ifdef CONFIG_NET_L3_MASTER_DEV u8 sysctl_tcp_l3mdev_accept;#endif u8 sysctl_tcp_mtu_probing;int sysctl_tcp_mtu_probe_floor; +int sysctl_tcp_mtu_probe_waitdata;If this is a boolean, you should use u8, and place this field to avoid adding a hole.
Quoted 11 lines
include/net/netns/ipv4.h +0
-0int sysctl_tcp_mtu_probe_rack;int sysctl_tcp_base_mss;int sysctl_tcp_min_snd_mss;int sysctl_tcp_probe_threshold;u32 sysctl_tcp_probe_interval;net/ipv4/sysctl_net_ipv4.c +0
-0@@ -847,10 +847,17 @@ static struct ctl_table ipv4_net_table[] = { .mode = 0644,net/ipv4/sysctl_net_ipv4.c static struct ctl_table ipv4_net_table[] = { · line 857
Quoted 10 lines
net/ipv4/sysctl_net_ipv4.c +6
-0.proc_handler = proc_dointvec_minmax, .extra1 = &tcp_min_snd_mss_min, .extra2 = &tcp_min_snd_mss_max, }, + { + .procname = "tcp_mtu_probe_waitdata", + .data = &init_net.ipv4.sysctl_tcp_mtu_probe_waitdata, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec,If this is a boolean, please use
proc_dou8vec_minmax, and SYSCTL_ZERO/SYSCTL_ONEQuoted 47 lines
net/ipv4/sysctl_net_ipv4.c +1
-0+ }, { .procname = "tcp_mtu_probe_rack", .data = &init_net.ipv4.sysctl_tcp_mtu_probe_rack, .maxlen = sizeof(int), .mode = 0644,net/ipv4/tcp_ipv4.c +1
-0@@ -2892,10 +2892,11 @@ static int __net_init tcp_sk_init(struct net *net) net->ipv4.sysctl_tcp_base_mss =TCP_BASE_MSS ; net->ipv4.sysctl_tcp_min_snd_mss =TCP_MIN_SND_MSS ; net->ipv4.sysctl_tcp_probe_threshold =TCP_PROBE_THRESHOLD ; net->ipv4.sysctl_tcp_probe_interval =TCP_PROBE_INTERVAL ; net->ipv4.sysctl_tcp_mtu_probe_floor =TCP_MIN_SND_MSS ; + net->ipv4.sysctl_tcp_mtu_probe_waitdata =1 ; net->ipv4.sysctl_tcp_mtu_probe_rack =1 ; net->ipv4.sysctl_tcp_keepalive_time =TCP_KEEPALIVE_TIME ; net->ipv4.sysctl_tcp_keepalive_probes =TCP_KEEPALIVE_PROBES ; net->ipv4.sysctl_tcp_keepalive_intvl =TCP_KEEPALIVE_INTVL ;net/ipv4/tcp_output.c +0
-4@@ -2394,14 +2394,10 @@ static int tcp_mtu_probe(struct sock *sk)*/ tcp_mtu_check_reprobe (sk);return -1 ; } -/* Have enough data in the send queue to probe? */ -if (tp->write_seq - tp->snd_nxt < size_needed) -return -1 ; -/* Can probe fit inside congestion window? */ if (packets_needed > tp->snd_cwnd)return -1 ;/* Can probe fit inside receiver window? If not then skip probing. @@ -2411,10 +2407,24 @@ static int tcp_mtu_probe(struct sock *sk)* clear below. */ if (tp->snd_wnd < size_needed)return -1 ;
Quoted 18 lines
net/ipv4/tcp_output.c static int tcp_mtu_probe(struct sock *sk) · line 2419
Quoted 8 lines
net/ipv4/tcp_output.c +8
-0+/* Have enough data in the send queue to probe? */ +if (tp->write_seq - tp->snd_nxt < size_needed) { +/* If packets are already in flight it's safe to wait for more data to +* accumulate on the sender because writing will be triggered as ACKs +* arrive. +* If no packets are in flight returning zero can stall. +*/ + if (net->ipv4.sysctl_tcp_mtu_probe_waitdata &&I have serious doubts about RPC traffic.
Adding one RTT latency is going to make this unlikely to be used.Quoted 6 lines
net/ipv4/tcp_output.c +6
-0+ tcp_packets_in_flight(tp)) + return 0; + else + return -1; + } +
This could be measured with tcp_rr mode in netperf, right? I've been using iperf which is more focused on bandwidth. My expectation would be that the "maximum" latency would increase but not the average since MTU probing is a rare occurrence.
Another way to implement waiting would be to check sk_wmem_alloc > skb->size like autocork does and this would promise zero latency. But I'm not sure how to do that correctly. If it's up to tcp_mtu_probe to ensure that traffic does not stall then could it set the TSQ_THROTTLED flag?
--
Regards,
Leonard