Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 10 Oct 2016 09:56:24 -0400
From:      Randall Stewart <rrs@netflix.com>
To:        hiren panchasara <hiren@strugglingcoder.info>
Cc:        FreeBSD Transports <transport@FreeBSD.org>, lstewart@FreeBSD.org
Subject:   Re: Exiting from loss recovery
Message-ID:  <1BC53E8D-08B9-4C4C-8E7B-6346CA83F66E@netflix.com>
In-Reply-To: <20161007002211.GO50669@strugglingcoder.info>
References:  <20161007002211.GO50669@strugglingcoder.info>

next in thread | previous in thread | raw e-mail | index | archive | help
Hiren:

I have a bit of experience now with this code.

If you exit recovery where Lawerence has marked, then when you go
down a few lines and credit the ack to the cwnd it can be a very
large =E2=80=9Cstretch=E2=80=9D ack.. making cwnd spike up incorrectly =
by standard
new-reno terms=E2=80=A6=20

If you do move exit recover to there, you probably need to have a limit
on how big the cwnd can move=E2=80=A6 i.e. some sort of segment limit.

R

> On Oct 6, 2016, at 8:22 PM, hiren panchasara =
<hiren@strugglingcoder.info> wrote:
>=20
> In tcp_do_segment():
>=20
>                /*
>                 * If the congestion window was inflated to account
>                 * for the other side's cached packets, retract it.
>                 */
>                if (IN_FASTRECOVERY(tp->t_flags)) {
>                        if (SEQ_LT(th->th_ack, tp->snd_recover)) {
>                                if (tp->t_flags & TF_SACK_PERMIT)
>                                        tcp_sack_partialack(tp, th);=20
>                                else
>                                        tcp_newreno_partial_ack(tp, =
th);=20
>                        } else=20
>                                cc_post_recovery(tp, th);=20
>                }
>=20
> Here, if we get an ack that marks recovery from loss i.e. >=3D
> snd_recovery, we call cc_post_recovery() which in-turn calls CC =
specific
> post_recovery routine. But we don't reset TF_FASTRECOVERY |
> TF_CONGRECOVERY flags by calling EXIT_RECOVERY()=20
>=20
> Later in the code we do this check again in 'process_ACK:'
>=20
>                /* XXXLAS: Can this be moved up into cc_post_recovery? =
*/
>                if (IN_RECOVERY(tp->t_flags) &&
>                    SEQ_GEQ(th->th_ack, tp->snd_recover)) {
>                        EXIT_RECOVERY(tp->t_flags);
>                }
>=20
> And as it can be seen, Lawrence marked it as something that could
> possibly be done here and at the end of cc_post_recovery().=20
>=20
> So, should we do it? i.e call EXIT_RECOVERY() at the end of
> cc_post_recovery() and remove the block from 'process_ACK' section? or
> there is something subtle I am not seeing?
>=20
> Cheers,
> Hiren

--------
Randall Stewart
rrs@netflix.com
803-317-4952








Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1BC53E8D-08B9-4C4C-8E7B-6346CA83F66E>