netdev - Re: [PATCH] RFC2988bis + taking RTT sample from 3WHS for the passive open side

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: <BANLkTimo_fbezdX2evKt0pAg0N+DSJQeSR6NoHLus=Z40WLkHg@mail.gmail.com>
Date:	Wed, 8 Jun 2011 15:26:40 -0700
From:	Jerry Chu <hkchu@...gle.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	David Miller <davem@...emloft.net>,
	Hagen Paul Pfeifer <hagen@...u.net>, tsunanet@...il.com,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH] RFC2988bis + taking RTT sample from 3WHS for the passive
 open side

Eric,

It just occurred to me now that initRTO is being reduced, both TCP_SYN_RETRIES
and TCP_SYNACK_RETRIES should be bumped up a bit, to e.g., 7 (?) to meet the
3 minutes R2 requirement per RFC1122.

If you agree, I will submit another patch.

Thanks,

Jerry

On Wed, Jun 8, 2011 at 10:04 AM, Jerry Chu <hkchu@...gle.com> wrote:
> +netdev (add missing cc)
>
> On Wed, Jun 8, 2011 at 9:30 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
>>
>> Le mardi 07 juin 2011 à 22:00 -0700, H.K. Jerry Chu a écrit :
>> > From: Jerry Chu <hkchu@...gle.com>
>> >
>> > This patch lowers the default initRTO from 3secs to 1sec per
>> > RFC2988bis. It falls back to 3secs if the SYN or SYN-ACK packet
>> > has been retransmitted, AND the TCP timestamp option is not on.
>> >
>> > It also adds support to take RTT sample during 3WHS on the passive
>> > open side, just like its active open counterpart, and uses it, if
>> > valid, to seed the initRTO for the data transmission phase.
>> >
>> > The patch also resets ssthresh to its initial default at the
>> > beginning of the data transmission phase, and reduces cwnd to 1 if
>> > there has been MORE THAN ONE retransmission during 3WHS per RFC5681.
>> >
>> > Signed-off-by: H.K. Jerry Chu <hkchu@...gle.com>
>> > ---
>>
>> ...
>>
>> > @@ -1854,7 +1859,7 @@ static int tcp_v4_init_sock(struct sock *sk)
>> >        * algorithms that we must have the following bandaid to talk
>> >        * efficiently to them.  -DaveM
>> >        */
>> > -     tp->snd_cwnd = 2;
>> > +     tp->snd_cwnd = TCP_INIT_CWND;
>>
>> Hmm, are you sure this belongs to this patch ?
>>
>> >
>> >       /* See draft-stevens-tcpca-spec-01 for discussion of the
>> >        * initialization of these values.
>> > diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
>> > index 80b1f80..d2fe4e0 100644
>> > --- a/net/ipv4/tcp_minisocks.c
>> > +++ b/net/ipv4/tcp_minisocks.c
>> > @@ -486,7 +486,7 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
>> >                * algorithms that we must have the following bandaid to talk
>> >                * efficiently to them.  -DaveM
>> >                */
>> > -             newtp->snd_cwnd = 2;
>> > +             newtp->snd_cwnd = TCP_INIT_CWND;
>>
>> same here ?
>
> It's part of the cwnd initialization stuff, which I included some fix
> per RFC5681 (in
> tcp_init_metrics()). The field is currently not being used anyway
> until data-in-SYN is
> supported. I fixed it here just because TCP_INIT_CWND is the right value.
>
>>
>> >               newtp->snd_cwnd_cnt = 0;
>> >               newtp->bytes_acked = 0;
>> >
>> > @@ -720,6 +720,10 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
>> >               NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPDEFERACCEPTDROP);
>> >               return NULL;
>> >       }
>> > +     if (tmp_opt.saw_tstamp && tmp_opt.rcv_tsecr)
>> > +             tcp_rsk(req)->snt_synack = tmp_opt.rcv_tsecr;
>> > +     else if (req->retrans) /* don't take RTT sample if retrans && ~TS */
>> > +             tcp_rsk(req)->snt_synack = 0;
>>
>> Could you clarify why this is done in this order ?
>
> What else is more appropriate? It needs to decide if we've got a valid
> RTT sample.
> If TS is on and rcv_tsec != 0 then we've got a valid RTT sample. Otherwise it
> does not take sample if there has been retrans per Karn's algorithm.
>
> Thanks,
>
> Jerry
>
>>
>> Thanks !
>>
>>
>
--
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