netdev - Re: [PATCHv3 net-next 2/4] ipv4: Use binary search to choose tcp PMTU probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <54F42D8E.2050601@gmail.com>
Date:	Mon, 02 Mar 2015 17:29:50 +0800
From:	Fan Du <fengyuleidian0615@...il.com>
To:	John Heffner <johnwheffner@...il.com>
CC:	Fan Du <fan.du@...el.com>, edumazet@...gle.com,
	David Miller <davem@...emloft.net>,
	Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCHv3 net-next 2/4] ipv4: Use binary search to choose tcp
 PMTU probe_size

于 2015年03月01日 07:20, John Heffner 写道:
>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>> index a2a796c..c418829 100644
>> --- a/net/ipv4/tcp_output.c
>> +++ b/net/ipv4/tcp_output.c
>> @@ -1837,11 +1837,13 @@ static int tcp_mtu_probe(struct sock *sk)
>>          struct tcp_sock *tp = tcp_sk(sk);
>>          struct inet_connection_sock *icsk = inet_csk(sk);
>>          struct sk_buff *skb, *nskb, *next;
>> +       struct net *net = sock_net(sk);
>>          int len;
>>          int probe_size;
>>          int size_needed;
>>          int copy;
>>          int mss_now;
>> +       int interval;
>>
>>          /* Not currently probing/verifying,
>>           * not in recovery,
>> @@ -1854,11 +1856,17 @@ static int tcp_mtu_probe(struct sock *sk)
>>              tp->rx_opt.num_sacks || tp->rx_opt.dsack)
>>                  return -1;
>>
>> -       /* Very simple search strategy: just double the MSS. */
>> +       /* Use binary search for probe_size bewteen tcp_mss_base,
>> +        * and current mss_clamp. if (search_high - search_low)
>> +        * smaller than a threshold, backoff from probing.
>> +        */
>>          mss_now = tcp_current_mss(sk);
>> -       probe_size = 2 * tp->mss_cache;
>> +       probe_size = (icsk->icsk_mtup.search_high +
>> +                     icsk->icsk_mtup.search_low) >> 1;
>>          size_needed = probe_size + (tp->reordering + 1) * tp->mss_cache;
>> -       if (probe_size > tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_high)) {
>> +       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 */
>>                  return -1;
>>          }
>
> A couple things: the local variable probe_size here is TCP segment
> size, while search_low and search_high are IP datagram sizes.  Use
> tcp_mtu_to_mss to subtract headers.  Also, I think if you set
> sysctl_tcp_probe_threshold <= 0, this will keep probing indefinitely
> at search_low (not useful).  You probably want to test interval <
> max(1, sysctl_tcp_probe_threshold).
Thanks for the comments, will update in next version.
btw, I'm little confused about two points here:

1. Checking if there is enough user data available in write queue with size_needed,
    Is there any special consideration here involving (tp->reordering + 1) * tp->mss_cache ?
    Since the assembly always copies "probe_size" bytes data from the write queue.

    size_needed = probe_size + (tp->reordering + 1) * tp->mss_cache;

2. Traverse write queue to build probing packet with "probe_size" bytes segment,
    when will the final nskb len exceeding probe_size? which trigger the break in line:1971

1971                 if (len >= probe_size)
1972                         break;
1973         }
1974         tcp_init_tso_segs(sk, nskb, nskb->len);


>    -John
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html