Re: [PATCHv2 net-next 4/4] ipv4: Create probe timer for tcp PMTU as per RFC4821
flat view
From: Eric Dumazet <hidden>
Date: 2015-02-26 04:19:25
On Thu, 2015-02-26 at 11:49 +0800, Fan Du wrote:
As per RFC4821 7.3. Selecting Probe Size, a probe timer should
be armed once probing has converged. Once this timer expired,
probing again to take advantage of any path PMTU change. The
recommended probing interval is 10 minutes per RFC1981. Probing
interval could be sysctled by sysctl_tcp_probe_interval.
Eric suggested to implement pseudo timer based on 32bits jiffies
tcp_time_stamp instead of using classic timer for such rare event.
Signed-off-by: Fan Du <redacted>
---
v2:
- Create a pseudo timer based on 32bits jiffies tcp_time_stamp
to control when to reprobing as suggested by Eric....
quoted hunk ↗ jump to hunk
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index c418829..0e9ee6a 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c@@ -1354,6 +1354,8 @@ void tcp_mtup_init(struct sock *sk) icsk->icsk_af_ops->net_header_len; icsk->icsk_mtup.search_low = tcp_mss_to_mtu(sk, net->ipv4.sysctl_tcp_base_mss); icsk->icsk_mtup.probe_size = 0; + if (icsk->icsk_mtup.enabled)
I guess the test can be removed. Assignment of probe_timestamp can be done regardless of the flag.
quoted hunk ↗ jump to hunk
+ icsk->icsk_mtup.probe_timestamp = tcp_time_stamp; } EXPORT_SYMBOL(tcp_mtup_init);@@ -1823,6 +1825,35 @@ send_now: return false; } +static inline void tcp_mtu_check_reprobe(struct sock *sk) +{ + struct inet_connection_sock *icsk = inet_csk(sk); + struct net *net = sock_net(sk); + struct tcp_sock *tp = tcp_sk(sk); +
extra newline
quoted hunk ↗ jump to hunk
+ u32 delta; + u32 now = tcp_time_stamp; + u32 interval = net->ipv4.sysctl_tcp_probe_interval;
Please try to keep variables sorted by lengths, it helps readability : u32 interval = net->ipv4.sysctl_tcp_probe_interval; struct inet_connection_sock *icsk = inet_csk(sk); struct tcp_sock *tp = tcp_sk(sk); struct net *net = sock_net(sk); s32 delta;
+ + if (likely(now > icsk->icsk_mtup.probe_timestamp))
Wrapping issue. if ((s32)(now - icsk->icsk_mtup.probe_timestamp)) > 0) ...
quoted hunk ↗ jump to hunk
+ delta = now - icsk->icsk_mtup.probe_timestamp; + else + delta = now + (U32_MAX - icsk->icsk_mtup.probe_timestamp);
oh well.. delta = tcp_time_stamp - icsk->icsk_mtup.probe_timestamp; is fine (if delta is declared as s32). Adding U32_MAX makes no sense. No need for @now var.
+ if (unlikely(jiffies_to_msecs(delta) >= interval * MSEC_PER_SC)) {
if (unlikely(delta >= interval * HZ)) {
quoted hunk ↗ jump to hunk
+ int mss = tcp_current_mss(sk); + + /* Update current search range */ + icsk->icsk_mtup.search_high = tp->rx_opt.mss_clamp + + sizeof(struct tcphdr) + + icsk->icsk_af_ops->net_header_len; + icsk->icsk_mtup.search_low = tcp_mss_to_mtu(sk, mss); + icsk->icsk_mtup.probe_size = 0; + + /* Update probe time stamp */ + icsk->icsk_mtup.probe_timestamp = now;
icsk->icsk_mtup.probe_timestamp = tcp_time_stamp;
quoted hunk ↗ jump to hunk
+ } +} + /* 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@@ -1866,8 +1897,11 @@ static int tcp_mtu_probe(struct sock *sk) size_needed = probe_size + (tp->reordering + 1) * tp->mss_cache; interval = icsk->icsk_mtup.search_high - icsk->icsk_mtup.search_low; if (probe_size > tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_high) || - interval < net->ipv4.sysctl_tcp_probe_threshold) { - /* TODO: set timer for probe_converge_event */ + interval < net->ipv4.sysctl_tcp_probe_threshold) { + /* Check whether enough time has elaplased for + * another round of probing. + */ + tcp_mtu_check_reprobe(sk); return -1; }