Messages in this thread | | | From | Eric Dumazet <> | Date | Mon, 30 Dec 2019 05:40:49 -0800 | Subject | Re: [PATCH] tcp: fix "old stuff" D-SACK causing SACK to be treated as D-SACK |
| |
On Mon, Dec 30, 2019 at 1:55 AM Pengcheng Yang <yangpc@wangsu.com> wrote: > > When we receive a D-SACK, where the sequence number satisfies: > undo_marker <= start_seq < end_seq <= prior_snd_una > we consider this is a valid D-SACK and tcp_is_sackblock_valid() > returns true, then this D-SACK is discarded as "old stuff", > but the variable first_sack_index is not marked as negative > in tcp_sacktag_write_queue(). > > If this D-SACK also carries a SACK that needs to be processed > (for example, the previous SACK segment was lost),
What do you mean by ' previous sack segment was lost' ?
this SACK > will be treated as a D-SACK in the following processing of > tcp_sacktag_write_queue(), which will eventually lead to > incorrect updates of undo_retrans and reordering. > > Fixes: fd6dad616d4f ("[TCP]: Earlier SACK block verification & simplify access to them") > Signed-off-by: Pengcheng Yang <yangpc@wangsu.com> > --- > net/ipv4/tcp_input.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > index 88b987c..0238b55 100644 > --- a/net/ipv4/tcp_input.c > +++ b/net/ipv4/tcp_input.c > @@ -1727,8 +1727,11 @@ static int tcp_sack_cache_ok(const struct tcp_sock *tp, const struct tcp_sack_bl > } > > /* Ignore very old stuff early */ > - if (!after(sp[used_sacks].end_seq, prior_snd_una)) > + if (!after(sp[used_sacks].end_seq, prior_snd_una)) { > + if (i == 0) > + first_sack_index = -1; > continue; > + } > > used_sacks++; > }
Hi Pengcheng Yang
This corner case deserves a packetdrill test so that we understand the issue, can you provide one ?
Thanks.
| |