netdev - Re: [PATCH net-next] tcp: refine tcp_prune_ofo

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] [day] [month] [year] [list]

Message-ID: <1471543545.29842.83.camel@edumazet-glaptop3.roam.corp.google.com>
Date:   Thu, 18 Aug 2016 11:05:45 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Yuchung Cheng <ycheng@...gle.com>
Cc:     David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Soheil Hassas Yeganeh <soheil@...gle.com>,
        Neal Cardwell <ncardwell@...gle.com>,
        "C. Stephen Gun" <csg@...gle.com>, Van Jacobson <vanj@...gle.com>
Subject: Re: [PATCH net-next] tcp: refine tcp_prune_ofo_queue() to not drop
 all packets

On Thu, 2016-08-18 at 10:55 -0700, Yuchung Cheng wrote:

> > +       /* Reset SACK state.  A conforming SACK implementation will
> > +        * do the same at a timeout based retransmit.  When a connection
> > +        * is in a sad state like this, we care only about integrity
> > +        * of the connection not performance.
> > +        */
> > +       if (tp->rx_opt.sack_ok)
> > +               tcp_sack_reset(&tp->rx_opt);
> I am curious what if we don't reset. It seems SACK will continue to
> function properly (at least for Linux sender). But this of course
> belongs  to a different patch / discussion.
> 

Agreed, we might try to be smarter here and update sack block if
possible.

BTW, here is the packetdrill test I used to test this patch :

# cat sack-prune.pkt

0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [2048], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0

+0 < S 0:0(0) win 32792 <mss 1000,sackOK,nop,nop,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 0>
+0.100 < . 1:1(0) ack 1 win 1024
+0 accept(3, ..., ...) = 4

+0.01 < . 2:3(1) ack 1 win 1024
+0    > . 1:1(0) ack 1 <nop,nop, sack 2:3>

+0.01 < . 4:5(1) ack 1 win 1024
+0    > . 1:1(0) ack 1 <nop,nop, sack 4:5 2:3>

+0.01 < . 6:7(1) ack 1 win 1024
+0    > . 1:1(0) ack 1 <nop,nop, sack 6:7 4:5 2:3>

+0.01 < . 8:9(1) ack 1 win 1024
+0    > . 1:1(0) ack 1 <nop,nop, sack 8:9 6:7 4:5 2:3>

+0.01 < . 10:11(1) ack 1 win 1024
// Alas, we had to clean OFO queue, last skb (sequence 8:9) was dropped.
+0    > . 1:1(0) ack 1 <nop,nop, sack 10:11>

+0.01 < . 1:2(1) ack 1 win 1024
// Make sure kernel did not drop 2:3 sequence
+0    > . 1:1(0) ack 3 <nop,nop, sack 10:11>

+0 read(4, ..., 2) = 2

+0.01 < . 3:4(1) ack 1 win 1024
+0    > . 1:1(0) ack 5 <nop,nop, sack 10:11>

+0.01 < . 5:6(1) ack 1 win 1024
+0    > . 1:1(0) ack 7 <nop,nop, sack 10:11>

+0 read(4, ..., 4) = 4

+0.01 < . 7:8(1) ack 1 win 1024
+0    > . 1:1(0) ack 8 <nop,nop, sack 10:11>

+0.01 < . 8:9(1) ack 1 win 1024
+0    > . 1:1(0) ack 9 <nop,nop, sack 10:11>

+0.01 < . 9:10(1) ack 1 win 1024
+0    > . 1:1(0) ack 11