Re: patch to implement RFC3517 in linux 2.4.22

Re: patch to implement RFC3517 in linux 2.4.22

[kuznet]

Hello!

> The new code marks packets in sack holes as lost as soon as three or more packets with higher sequence numbers are sacked and these will then be immediately retransmitted - this should mean that retransmission of lost packets generally happens earlier and recovery is faster.  

What those chunks of code which you killed with IsReno() do to your
opinion?

Alexey

PS Please, try to format mails more sanely. This one is close to unreadable.

Alexey

Re: patch to implement RFC3517 in linux 2.4.22

[kuznet]

Hello!

> Alexey, comments?

I do not understand the patch. I suppose it _is_ current behaviour,
the corresponding places are "commented out" with IsReno() predicate.

Shortly, I cannot figure out even what the patch is expected to change.

Alexey

Re: patch to implement RFC3517 in linux 2.4.22

[David S. Miller]

On Thu, 9 Oct 2003 07:27:15 -0000
"Douglas Leith" <doug@eee.strath.ac.uk> wrote:

> Comments appreciated.

1) Why do you only update the scorebord in the reno case?  All the
   retransmission modes need it to be updated.

2) You really need to fix the super-long lines, the code becomes
   unreadable after your changes.

Alexey, comments?

patch to implement RFC3517 in linux 2.4.22

[Douglas Leith]

[-- Attachment #1: Type: text/plain, Size: 4064 bytes --]

Attached is a short patch against version 2.4.22 to implement the recent 
rfc3517 for SACK in TCP (see ftp://ftp.rfc-editor.org/in-notes/rfc3517.txt).  
This modifies how packets which fail to be sacked are marked as lost.  At 
present, any packets falling into sack holes are marked lost after the first 
successful retransmit.  The new code marks packets in sack holes as lost as 
soon as tp->reordering or more packets with higher sequence number are 
sacked.  This is consistent with how reordering is handled elsewhere and 
should lead to faster retransmission and recovery when multiple drops occur.

Comments appreciated.

Regards,

Doug Leith

www.hamilton.ie


diff -u -wbrP --exclude=config.save linux-2.4.22/net/ipv4/tcp_input.c linux-
2.4.22-retrans/net/ipv4/tcp_input.c
--- linux-2.4.22/net/ipv4/tcp_input.c   2003-06-13 15:51:39.000000000 +0100
+++ linux-2.4.22-retrans/net/ipv4/tcp_input.c   2003-10-09 07:59:48.000000000 
+0100
@@ -61,6 +61,7 @@
  *             Panu Kuhlberg:          Experimental audit of TCP (re)
transmission
  *                                     engine. Lots of bugs are found.
  *             Pasi Sarolahti:         F-RTO for dealing with spurious RTOs
+ *             Doug Leith:             Early retransmission of packets 
falling into sack holes.
  */

 #include <linux/config.h>
@@ -757,6 +758,10 @@
  *    for retransmitted and already SACKed segment -> reordering..
  * Both of these heuristics are not used in Loss state, when we cannot
  * account for retransmits accurately.
+ *
+ * Retransmission of packets
+ * -------------------------
+ * Extension of Hoe's retransmit to allow early retransmission of packets 
which fall into sack holes
  */
 static int
 tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 
prior_snd_una)
@@ -770,6 +775,7 @@
        u32 lost_retrans = 0;
        int flag = 0;
        int i;
+        u32 prior_sacked_out=tp->sacked_out;

        if (!tp->sacked_out)
                tp->fackets_out = 0;
@@ -781,6 +787,7 @@
                __u32 end_seq = ntohl(sp->end_seq);
                int fack_count = 0;
                int dup_sack = 0;
+                u32 sack_count=0;

                /* Check for D-SACK. */
                if (i == 0) {
@@ -828,6 +835,8 @@
                                break;

                        fack_count++;
+                       if (sacked&TCPCB_RETRANS)
+                          sack_count++;

                        in_sack = !after(start_seq, TCP_SKB_CB(skb)->seq) &&
                                !before(end_seq, TCP_SKB_CB(skb)->end_seq);
@@ -860,8 +869,14 @@
                            (!lost_retrans || after(end_seq, lost_retrans)))
                                lost_retrans = end_seq;

-                       if (!in_sack)
+                       if (!in_sack) {
+                           /* force early retransmit ? */
+                           if (!(sacked&TCPCB_TAGBITS) && (prior_sacked_out 
> sack_count + tp->reordering)) {
+                               TCP_SKB_CB(skb)->sacked |= TCPCB_LOST;
+                               tp->lost_out++;
+                           }
                                continue;
+                       }

                        if (!(sacked&TCPCB_SACKED_ACKED)) {
                                if (sacked & TCPCB_SACKED_RETRANS) {
@@ -1599,7 +1614,7 @@
        if ((flag&FLAG_DATA_LOST) &&
            before(tp->snd_una, tp->high_seq) &&
            tp->ca_state != TCP_CA_Open &&
-           tp->fackets_out > tp->reordering) {
+           tp->fackets_out > tp->reordering && IsReno(tp) ) {
                tcp_mark_head_lost(sk, tp, tp->fackets_out-tp->reordering, tp-
>high_seq);
                NET_INC_STATS_BH(TCPLoss);
        }
@@ -1714,7 +1729,7 @@
                tp->ca_state = TCP_CA_Recovery;
        }

-       if (is_dupack || tcp_head_timedout(sk, tp))
+       if ( (is_dupack || tcp_head_timedout(sk, tp)) && IsReno(tp) )
                tcp_update_scoreboard(sk, tp);
        tcp_cwnd_down(tp);
        tcp_xmit_retransmit_queue(sk);



[-- Attachment #2: rfc3517.patch --]
[-- Type: text/plain, Size: 2765 bytes --]

diff -u -wbrP --exclude=config.save linux-2.4.22/net/ipv4/tcp_input.c linux-2.4.22-retrans/net/ipv4/tcp_input.c
--- linux-2.4.22/net/ipv4/tcp_input.c	2003-06-13 15:51:39.000000000 +0100
+++ linux-2.4.22-retrans/net/ipv4/tcp_input.c	2003-10-09 07:59:48.000000000 +0100
@@ -61,6 +61,7 @@
  *		Panu Kuhlberg:		Experimental audit of TCP (re)transmission
  *					engine. Lots of bugs are found.
  *		Pasi Sarolahti:		F-RTO for dealing with spurious RTOs
+ *		Doug Leith:		Early retransmission of packets falling into sack holes.
  */
 
 #include <linux/config.h>
@@ -757,6 +758,10 @@
  *    for retransmitted and already SACKed segment -> reordering..
  * Both of these heuristics are not used in Loss state, when we cannot
  * account for retransmits accurately.
+ *
+ * Retransmission of packets
+ * -------------------------
+ * Extension of Hoe's retransmit to allow early retransmission of packets which fall into sack holes
  */
 static int
 tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_una)
@@ -770,6 +775,7 @@
 	u32 lost_retrans = 0;
 	int flag = 0;
 	int i;
+        u32 prior_sacked_out=tp->sacked_out;
 
 	if (!tp->sacked_out)
 		tp->fackets_out = 0;
@@ -781,6 +787,7 @@
 		__u32 end_seq = ntohl(sp->end_seq);
 		int fack_count = 0;
 		int dup_sack = 0;
+                u32 sack_count=0;
 
 		/* Check for D-SACK. */
 		if (i == 0) {
@@ -828,6 +835,8 @@
 				break;
 
 			fack_count++;
+			if (sacked&TCPCB_RETRANS) 
+			   sack_count++;
 
 			in_sack = !after(start_seq, TCP_SKB_CB(skb)->seq) &&
 				!before(end_seq, TCP_SKB_CB(skb)->end_seq);
@@ -860,8 +869,14 @@
 			    (!lost_retrans || after(end_seq, lost_retrans)))
 				lost_retrans = end_seq;
 
-			if (!in_sack)
+			if (!in_sack) {
+                           /* force early retransmit ? */
+                           if (!(sacked&TCPCB_TAGBITS) && (prior_sacked_out > sack_count + tp->reordering)) {
+                               TCP_SKB_CB(skb)->sacked |= TCPCB_LOST; 
+                               tp->lost_out++;
+                           }
 				continue;
+ 			}
 
 			if (!(sacked&TCPCB_SACKED_ACKED)) {
 				if (sacked & TCPCB_SACKED_RETRANS) {
@@ -1599,7 +1614,7 @@
 	if ((flag&FLAG_DATA_LOST) &&
 	    before(tp->snd_una, tp->high_seq) &&
 	    tp->ca_state != TCP_CA_Open &&
-	    tp->fackets_out > tp->reordering) {
+	    tp->fackets_out > tp->reordering && IsReno(tp) ) {
 		tcp_mark_head_lost(sk, tp, tp->fackets_out-tp->reordering, tp->high_seq);
 		NET_INC_STATS_BH(TCPLoss);
 	}
@@ -1714,7 +1729,7 @@
 		tp->ca_state = TCP_CA_Recovery;
 	}
 
-	if (is_dupack || tcp_head_timedout(sk, tp))
+	if ( (is_dupack || tcp_head_timedout(sk, tp)) && IsReno(tp) )
 		tcp_update_scoreboard(sk, tp);
 	tcp_cwnd_down(tp);
 	tcp_xmit_retransmit_queue(sk);