[PATCH - test-tree] DCCP: Increment sequence number on retransmitted

[PATCH - test-tree] DCCP: Increment sequence number on retransmitted

[Wei Yongjun]

When retransmit the first REQUEST, the sequence number does not be 
increased. This is because before retransmit the first REQUEST packet, 
the icsk->icsk_retransmits is 0, so dccp_transmit_skb() will fetch the 
dp->dccps_iss as the retransmit sequence number.

This patch fix the problem.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>

--- a/net/dccp/output.c	2008-05-29 22:27:55.000000000 -0400
+++ b/net/dccp/output.c	2008-05-30 19:26:43.000000000 -0400
@@ -69,9 +69,6 @@ static int dccp_transmit_skb(struct sock
 
 		case DCCP_PKT_REQUEST:
 			set_ack = 0;
-			/* Use ISS on the first (non-retransmitted) Request. */
-			if (icsk->icsk_retransmits = 0)
-				dcb->dccpd_seq = dp->dccps_iss;
 			/* fall through */
 
 		case DCCP_PKT_SYNC:
@@ -526,6 +523,8 @@ int dccp_connect(struct sock *sk)
 
 	/* Initialise GAR as per 8.5; AWL/AWH are set in dccp_transmit_skb() */
 	dp->dccps_gar = dp->dccps_iss;
+	/* Initialise GSS to ISS - 1, will be increased in dccp_transmit_skb() */
+	dp->dccps_gss = SUB48(dp->dccps_iss, 1);
 
 	skb = alloc_skb(sk->sk_prot->max_header, sk->sk_allocation);
 	if (unlikely(skb = NULL))

Re: [PATCH - test-tree] DCCP: Increment sequence number on

[Gerrit Renker]

> When retransmit the first REQUEST, the sequence number does not be  
> increased. This is because before retransmit the first REQUEST packet,  
> the icsk->icsk_retransmits is 0, so dccp_transmit_skb() will fetch the  
> dp->dccps_iss as the retransmit sequence number.
>
> This patch fix the problem.
>
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
>
> --- a/net/dccp/output.c	2008-05-29 22:27:55.000000000 -0400
> +++ b/net/dccp/output.c	2008-05-30 19:26:43.000000000 -0400
> @@ -69,9 +69,6 @@ static int dccp_transmit_skb(struct sock
>
> 		case DCCP_PKT_REQUEST:
> 			set_ack = 0;
> -			/* Use ISS on the first (non-retransmitted) Request. */
> -			if (icsk->icsk_retransmits = 0)
> -				dcb->dccpd_seq = dp->dccps_iss;
> 			/* fall through */
>
> 		case DCCP_PKT_SYNC:
> @@ -526,6 +523,8 @@ int dccp_connect(struct sock *sk)
>
> 	/* Initialise GAR as per 8.5; AWL/AWH are set in dccp_transmit_skb() */
> 	dp->dccps_gar = dp->dccps_iss;
> +	/* Initialise GSS to ISS - 1, will be increased in dccp_transmit_skb() */
> +	dp->dccps_gss = SUB48(dp->dccps_iss, 1);
>
> 	skb = alloc_skb(sk->sk_prot->max_header, sk->sk_allocation);
> 	if (unlikely(skb = NULL))
>
Ack - excellent spotting. 

The problem is that icsk->retransmits is increased only after calling
dccp_transmit_skb() via dccp_retransmit_skb(), hence the sequence number is
increased only in the second retransmit.

I will put this into the test tree later on today, I am thinking about
finding a different solution around the fact that GSS is always
increased at the top of dccp_transmit_skb().

Thanks a lot for investigating this.

Gerrit

Re: [PATCH - test-tree] DCCP: Increment sequence number on

[Arnaldo Carvalho de Melo]

Em Wed, Jun 04, 2008 at 03:37:19PM +0800, Wei Yongjun escreveu:
> When retransmit the first REQUEST, the sequence number does not be  
> increased. This is because before retransmit the first REQUEST packet,  
> the icsk->icsk_retransmits is 0, so dccp_transmit_skb() will fetch the  
> dp->dccps_iss as the retransmit sequence number.
>
> This patch fix the problem.

Looks like a hack, ugh. Proper fix must be not bumping it on
dccp_transmit_skb when the packet is a REQUEST, no?

- Arnaldo

> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
>
> --- a/net/dccp/output.c	2008-05-29 22:27:55.000000000 -0400
> +++ b/net/dccp/output.c	2008-05-30 19:26:43.000000000 -0400
> @@ -69,9 +69,6 @@ static int dccp_transmit_skb(struct sock
>
> 		case DCCP_PKT_REQUEST:
> 			set_ack = 0;
> -			/* Use ISS on the first (non-retransmitted) Request. */
> -			if (icsk->icsk_retransmits = 0)
> -				dcb->dccpd_seq = dp->dccps_iss;
> 			/* fall through */
>
> 		case DCCP_PKT_SYNC:
> @@ -526,6 +523,8 @@ int dccp_connect(struct sock *sk)
>
> 	/* Initialise GAR as per 8.5; AWL/AWH are set in dccp_transmit_skb() */
> 	dp->dccps_gar = dp->dccps_iss;
> +	/* Initialise GSS to ISS - 1, will be increased in dccp_transmit_skb() */
> +	dp->dccps_gss = SUB48(dp->dccps_iss, 1);
>
> 	skb = alloc_skb(sk->sk_prot->max_header, sk->sk_allocation);
> 	if (unlikely(skb = NULL))

Re: [PATCH - test-tree] DCCP: Increment sequence number on

[Gerrit Renker]

Quoting Arnaldo:
| Em Wed, Jun 04, 2008 at 03:37:19PM +0800, Wei Yongjun escreveu:
| > When retransmit the first REQUEST, the sequence number does not be  
| > increased. This is because before retransmit the first REQUEST packet,  
| > the icsk->icsk_retransmits is 0, so dccp_transmit_skb() will fetch the  
| > dp->dccps_iss as the retransmit sequence number.
| >
| > This patch fix the problem.
| 
| Looks like a hack, ugh. Proper fix must be not bumping it on
| dccp_transmit_skb when the packet is a REQUEST, no?
| 
Yes, correct, it is a hack. I was working on that until one hour ago
and then decided to put it aside. The fix is in the test tree for now
but is not meant to remain there.

The problem is that, for the three retransmittable packet types in DCCP
(Close, CloseReq, Request), there is no obvious way of distinguishing
the original packet from the cloned packet.

Here is the sketch of the solution I was working on, I'd be glad for input:
 * dccp_entail(), skb_clone(), dccp_transmit_skb() and setting of the icsk
   retransmit timer always occur together;
 * as soon as skb_clone() is called, skb->cloned is true;
 * so the idea is to 
   - use dccp_transmit_skb() to transmit the un-cloned skb;
   - sk->send_head = skb_clone(skb, gfp_any())
   - need some more careful analysis, but I think the set_owner_w() can
     also be removed, needs checking against the WARN_ON in
     dccp_transmit_skb().

Or maybe there is an easier way - the objective is to have a clear
indicator whether the skb is a retransmitted one or the first one
(since a similar problem arises when sending Close(Req) packets).

Suggestions welcome.

Gerrit

Re: [PATCH - test-tree] DCCP: Increment sequence number on

[Arnaldo Carvalho de Melo]

Em Wed, Jun 04, 2008 at 02:38:49PM +0100, Gerrit Renker escreveu:
> Quoting Arnaldo:
> | Em Wed, Jun 04, 2008 at 03:37:19PM +0800, Wei Yongjun escreveu:
> | > When retransmit the first REQUEST, the sequence number does not be  
> | > increased. This is because before retransmit the first REQUEST packet,  
> | > the icsk->icsk_retransmits is 0, so dccp_transmit_skb() will fetch the  
> | > dp->dccps_iss as the retransmit sequence number.
> | >
> | > This patch fix the problem.
> | 
> | Looks like a hack, ugh. Proper fix must be not bumping it on
> | dccp_transmit_skb when the packet is a REQUEST, no?
> | 
> Yes, correct, it is a hack. I was working on that until one hour ago
> and then decided to put it aside. The fix is in the test tree for now
> but is not meant to remain there.
> 
> The problem is that, for the three retransmittable packet types in DCCP
> (Close, CloseReq, Request), there is no obvious way of distinguishing
> the original packet from the cloned packet.

But why do you have to? If it is a REQUEST, don't bump the GSS... wait,
I actually took the time and went to re-read the RFC and I think the
changelog Wei wrote is misleading, as:

-------------------- 8< ------------------------------------------------
   A client in the REQUEST state SHOULD use an exponential-backoff timer
   to send new DCCP-Request packets if no response is received.  The
   first retransmission should occur after approximately one second,
   backing off to not less than one packet every 64 seconds; or the



Kohler, et al.              Standards Track                    [Page 58]
^L
RFC 4340      Datagram Congestion Control Protocol (DCCP)     March 2006


   endpoint can use whatever retransmission strategy is followed for
   retransmitting TCP SYNs.  Each new DCCP-Request MUST increment the
   Sequence Number by one and MUST contain the same Service Code and
   application data as the original DCCP-Request.
-------------------- 8< ------------------------------------------------

So the fix would be for not bumping GSS on the first packet? I guess so,
the flow is:

To create a socket:

 dccp_v4_connect(): sets iss
   dccp_connect()
     dccp_connect_init(): iss = gss
     dccp_transmit_skb: gss += 1

So the problem is that if this is the first packet being sent on a
connection we should not do the gss += 1...

> Here is the sketch of the solution I was working on, I'd be glad for input:
>  * dccp_entail(), skb_clone(), dccp_transmit_skb() and setting of the icsk
>    retransmit timer always occur together;
>  * as soon as skb_clone() is called, skb->cloned is true;
>  * so the idea is to 
>    - use dccp_transmit_skb() to transmit the un-cloned skb;
>    - sk->send_head = skb_clone(skb, gfp_any())
>    - need some more careful analysis, but I think the set_owner_w() can
>      also be removed, needs checking against the WARN_ON in
>      dccp_transmit_skb().
> 
> Or maybe there is an easier way - the objective is to have a clear
> indicator whether the skb is a retransmitted one or the first one
> (since a similar problem arises when sending Close(Req) packets).

...since I think I got to the same page as you guys... :-) Lets continue
from this point:

I think we have to use set_owner_w, its safer, we better get the
connection stuck due to using all its socket send buffer space if the
packet is never freed because of some stack bug than to get the whole
machine with a problem, even if after a long time, if we leak these
packets.

Perhaps we can use a flag in skb->cb that we would set only on the first
packet? Nah, I think we should have a __dccp_transmit_skb, that... code,
lets show it as a patch, see at the end of this message.

WRT Close(Req) I think it is right as it is, i.e. we should bump GSS, if
not we'll send the packet with the same sequence number as the last
packet sent, i.e. the problem is just on the very first packet.

Or am I missing something?

- Arnaldo

diff --git a/net/dccp/output.c b/net/dccp/output.c
index 1f8a9b6..3110df1 100644
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -39,7 +39,7 @@ static void dccp_skb_entail(struct sock *sk, struct sk_buff *skb)
  * IP so it can do the same plus pass the packet off to the
  * device.
  */
-static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
+static int __dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
 {
 	if (likely(skb != NULL)) {
 		const struct inet_sock *inet = inet_sk(sk);
@@ -54,8 +54,6 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
 		int err, set_ack = 1;
 		u64 ackno = dp->dccps_gsr;
 
-		dccp_inc_seqno(&dp->dccps_gss);
-
 		switch (dcb->dccpd_type) {
 		case DCCP_PKT_DATA:
 			set_ack = 0;
@@ -132,6 +130,12 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
 	return -ENOBUFS;
 }
 
+static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
+{
+	dccp_inc_seqno(&dccp_sk(sk)->dccps_gss);
+	__dccp_transmit_skb(sk, skb);
+}
+
 /**
  * dccp_determine_ccmps  -  Find out about CCID-specfic packet-size limits
  * We only consider the HC-sender CCID for setting the CCMPS (RFC 4340, 14.),
@@ -472,7 +476,7 @@ int dccp_connect(struct sock *sk)
 	DCCP_SKB_CB(skb)->dccpd_type = DCCP_PKT_REQUEST;
 
 	dccp_skb_entail(sk, skb);
-	dccp_transmit_skb(sk, skb_clone(skb, GFP_KERNEL));
+	__dccp_transmit_skb(sk, skb_clone(skb, GFP_KERNEL));
 	DCCP_INC_STATS(DCCP_MIB_ACTIVEOPENS);
 
 	/* Timer for repeating the REQUEST until an answer. */

Re: [PATCH - test-tree] DCCP: Increment sequence number on

[Gerrit Renker]

| But why do you have to? If it is a REQUEST, don't bump the GSS... wait,
| I actually took the time and went to re-read the RFC and I think the
| changelog Wei wrote is misleading, as:
| 
| -------------------- 8< ------------------------------------------------
|    A client in the REQUEST state SHOULD use an exponential-backoff timer
|    to send new DCCP-Request packets if no response is received.  The
|    first retransmission should occur after approximately one second,
|    backing off to not less than one packet every 64 seconds; or the
|    endpoint can use whatever retransmission strategy is followed for
|    retransmitting TCP SYNs.  Each new DCCP-Request MUST increment the
|    Sequence Number by one and MUST contain the same Service Code and
|    application data as the original DCCP-Request.
| -------------------- 8< ------------------------------------------------
| 
| So the fix would be for not bumping GSS on the first packet? I guess so,
| the flow is:
| 
| To create a socket:
| 
|  dccp_v4_connect(): sets iss
|    dccp_connect()
|      dccp_connect_init(): iss = gss
|      dccp_transmit_skb: gss += 1
| 
| So the problem is that if this is the first packet being sent on a
| connection we should not do the gss += 1...
| 
Yes exactly that is the problem. And I find it ugly, too, to first set
ISS, and then set GSS=ISS+1.


| > Here is the sketch of the solution I was working on, I'd be glad for input:
| >  * dccp_entail(), skb_clone(), dccp_transmit_skb() and setting of the icsk
| >    retransmit timer always occur together;
| >  * as soon as skb_clone() is called, skb->cloned is true;
| >  * so the idea is to 
| >    - use dccp_transmit_skb() to transmit the un-cloned skb;
| >    - sk->send_head = skb_clone(skb, gfp_any())
| >    - need some more careful analysis, but I think the set_owner_w() can
| >      also be removed, needs checking against the WARN_ON in
| >      dccp_transmit_skb().
| > 
| > Or maybe there is an easier way - the objective is to have a clear
| > indicator whether the skb is a retransmitted one or the first one
| > (since a similar problem arises when sending Close(Req) packets).
| 
| ...since I think I got to the same page as you guys... :-) Lets continue
| from this point:
| 
| I think we have to use set_owner_w, its safer, we better get the
| connection stuck due to using all its socket send buffer space if the
| packet is never freed because of some stack bug than to get the whole
| machine with a problem, even if after a long time, if we leak these
| packets.
| 
| Perhaps we can use a flag in skb->cb that we would set only on the first
| packet? Nah, I think we should have a __dccp_transmit_skb, that... code,
| lets show it as a patch, see at the end of this message.
| 
I had a further look, it is actually possible. Your point about
set_owner_w is good - I've had to reboot three times today because of
non-freed references.

What I found so far is that skb_cloned(skb) in dccp_retransmit_skb is
never true, since we put the original skb onto the sk->send_head, so
dccp_retransmit_skb() always calls skb_clone. That is why
dccp_transmit_skb() never complains that skb->sk is already set.

What I was hoping for was a rewrite of the dccp_retransmit_skb(), so
that the original skb is transmitted first and the clone put onto the
send-head. But doesn't work since queue_xmit frees the original skb, so
was stuck there.

| WRT Close(Req) I think it is right as it is, i.e. we should bump GSS, if
| not we'll send the packet with the same sequence number as the last
| packet sent, i.e. the problem is just on the very first packet.
| 
| Or am I missing something?
Correct - this applies only to the initial packet, so we can safely
increment GSS on entering dccp_transmit_skb().

I will look at your code at home, we will be shutting down here
everything in 15 minutes.

Thanks a lot for the input,
Gerrit

Re: [PATCH - test-tree] DCCP: Increment sequence number on

[Arnaldo Carvalho de Melo]

Em Wed, Jun 04, 2008 at 04:55:40PM +0100, Gerrit Renker escreveu:
> | But why do you have to? If it is a REQUEST, don't bump the GSS... wait,
> | I actually took the time and went to re-read the RFC and I think the
> | changelog Wei wrote is misleading, as:
> | 
> | -------------------- 8< ------------------------------------------------
> |    A client in the REQUEST state SHOULD use an exponential-backoff timer
> |    to send new DCCP-Request packets if no response is received.  The
> |    first retransmission should occur after approximately one second,
> |    backing off to not less than one packet every 64 seconds; or the
> |    endpoint can use whatever retransmission strategy is followed for
> |    retransmitting TCP SYNs.  Each new DCCP-Request MUST increment the
> |    Sequence Number by one and MUST contain the same Service Code and
> |    application data as the original DCCP-Request.
> | -------------------- 8< ------------------------------------------------
> | 
> | So the fix would be for not bumping GSS on the first packet? I guess so,
> | the flow is:
> | 
> | To create a socket:
> | 
> |  dccp_v4_connect(): sets iss
> |    dccp_connect()
> |      dccp_connect_init(): iss = gss
> |      dccp_transmit_skb: gss += 1
> | 
> | So the problem is that if this is the first packet being sent on a
> | connection we should not do the gss += 1...
> | 
> Yes exactly that is the problem. And I find it ugly, too, to first set
> ISS, and then set GSS=ISS+1.
> 
> 
> | > Here is the sketch of the solution I was working on, I'd be glad for input:
> | >  * dccp_entail(), skb_clone(), dccp_transmit_skb() and setting of the icsk
> | >    retransmit timer always occur together;
> | >  * as soon as skb_clone() is called, skb->cloned is true;
> | >  * so the idea is to 
> | >    - use dccp_transmit_skb() to transmit the un-cloned skb;
> | >    - sk->send_head = skb_clone(skb, gfp_any())
> | >    - need some more careful analysis, but I think the set_owner_w() can
> | >      also be removed, needs checking against the WARN_ON in
> | >      dccp_transmit_skb().
> | > 
> | > Or maybe there is an easier way - the objective is to have a clear
> | > indicator whether the skb is a retransmitted one or the first one
> | > (since a similar problem arises when sending Close(Req) packets).
> | 
> | ...since I think I got to the same page as you guys... :-) Lets continue
> | from this point:
> | 
> | I think we have to use set_owner_w, its safer, we better get the
> | connection stuck due to using all its socket send buffer space if the
> | packet is never freed because of some stack bug than to get the whole
> | machine with a problem, even if after a long time, if we leak these
> | packets.
> | 
> | Perhaps we can use a flag in skb->cb that we would set only on the first
> | packet? Nah, I think we should have a __dccp_transmit_skb, that... code,
> | lets show it as a patch, see at the end of this message.
> | 
> I had a further look, it is actually possible. Your point about
> set_owner_w is good - I've had to reboot three times today because of
> non-freed references.
> 
> What I found so far is that skb_cloned(skb) in dccp_retransmit_skb is
> never true, since we put the original skb onto the sk->send_head, so
> dccp_retransmit_skb() always calls skb_clone. That is why
> dccp_transmit_skb() never complains that skb->sk is already set.
> 
> What I was hoping for was a rewrite of the dccp_retransmit_skb(), so
> that the original skb is transmitted first and the clone put onto the
> send-head. But doesn't work since queue_xmit frees the original skb, so
> was stuck there.

One thing to always have in mind is that we should, as long as it
remains sane, to try to have the same code structure as TCP, so that
people that are used to the TCP code can read the DCCP more easily and
to identify areas that are are common.

So when trying to change things like this check if TCP has a similar
function and if the change would make sense there too.
 
> | WRT Close(Req) I think it is right as it is, i.e. we should bump GSS, if
> | not we'll send the packet with the same sequence number as the last
> | packet sent, i.e. the problem is just on the very first packet.
> | 
> | Or am I missing something?
> Correct - this applies only to the initial packet, so we can safely
> increment GSS on entering dccp_transmit_skb().
> 
> I will look at your code at home, we will be shutting down here
> everything in 15 minutes.

OK.

- Arnaldo

Re: [PATCH - test-tree] DCCP: Increment sequence number on

[Gerrit Renker]

| One thing to always have in mind is that we should, as long as it
| remains sane, to try to have the same code structure as TCP, so that
| people that are used to the TCP code can read the DCCP more easily and
| to identify areas that are are common.
| 
| So when trying to change things like this check if TCP has a similar
| function and if the change would make sense there too.
|  
More than happy to do that since the TCP code is so very good. 

I was wondering if it would be possible to convert part of CCID-2 to directly call
TCP functions? 

There is a lot of overlap, and much in the CCID-2 specification is directly from TCP.

So far there are at least three cases where CCID-2 has benefited from TCP:

 * the computation of the RTO (with the clamping to 200ms), this is already in the test tree
   http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p‹cp_exp.git;a=commit;h#62d489189bf73b47ff778f7f5eca1daaea5f7b

 * Congestion Window Validation (CWV, RFC 2861)
   This is still in my development tree, as I have not had time to write up an experimental
   evaluation. I have had good experiences on a wireless link with it (where the Acks arrive
   in a bundle since the communication is not full duplex); cwnd growth was really well-behaved.

 * have submitted one patch for RFC 3390 conversion where CCID-2/TCP could use the same function:
   http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p‹cp_exp.git;a=commitdiff;h"b6d875e14b70e3959641512c15cccef4f90692

With regard to the DCCP retransmissions, will reply in other thread.
Gerrit   
--
To unsubscribe from this list: send the line "unsubscribe dccp" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH - test-tree] DCCP: Increment sequence number on

[Gerrit Renker]

I have thought over the approach  consisting of

 * creating __dccp_transmit_skb() 
   - as the old dccp_transmit_skb(), but without dccp_inc_seqno(&GSS)
   - called directly only from dccp_connect(), so that correctly
     ISS is not incremented before sending the first Request packet.

 * creating a new dccp_transmit_skb()
   - which is called from all other code paths except from dccp_connect()
   - thus including the retransmissions of Requests via dccp_retransmit_skb()
   - which increments GSS only when it is already greater than or equal to ISS;
   - and calls __dccp_transmit_skb() for the rest.

The approach is correct and works.

I am having a second problem here which the following patch in the test tree tried to solve, 
http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p‹cp_exp.git;a=commitdiff;h\x1a9c8cc01caf3da8e65d82b5d47d377bafbd51bd
	 
The problem is that AWL=GSS was only set in dccp_connect(), so we need to call
dccp_update_gss() in addition to dccp_inc_seqno().

It gets a bit more complicated, since
 * the option-insertion code  may rely on the current value of GSS (Ack Vectors are an example);
 * the option-insertion code can fail (e.g. insufficient space left for options on skb);
 * so first incrementing GSS and then aborting dccp_transmit_skb() would cause a sequence hole.

The approach taken in the above patch-URL is to
 * assign the incremented value of GSS to the temporary `seq' field in dccp_skb_cb;
 * overwrite this `seq' field with ISS on the first Request;
 * in all instances, update GSS for real only if option-insertion worked;
 * that call to dccp_update_gss() also sets AWL, fixing the second problem.

The reason that this fix is at a later stage in the test tree is that updating AWL in turn depends
on the value of the local Sequence Window feature (RFC 4340, 7.5.1), which in turn depends on the
feature-negotiation code; this part is in
http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p‹cp_exp.git;a=commitdiff;hº62a9e2fa53f20bf80f5691d757950fb86c6f30

The second step/bullet-point is, as pointed out by Wei Yongjun, still not working correctly
with regard to incrementing sequence numbers on retransmissions.

I have an alternative suggestion for handling the retransmissions, am
currently testing this, and will submit patch subsequently.

Gerrit
--
To unsubscribe from this list: send the line "unsubscribe dccp" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[dccp] [RFC/RFT] [Patch 0/2]: Test-tree update to handle

[Gerrit Renker]

Here are two patches for the test tree. As with all RFC/RFT patches, these are
posted with the main objective of getting review/comments, and are then put into
the test tree for ... testing.

These patches evolved thanks to input provided by Wei Yongjun regarding an
unresolved problem with retransmissions, fixed by the following two updates:

Patch #1: allows to distinguish retransmitted from original packets,
          using a slight reordering of existing code.
          This is the main fix for the earlier problem of not incrementing
	  the sequence number properly on retransmitted Request packets.

	  The patch introduces a generic condition which can also be tested
	  by other code (e.g. for the retransmission of Close/CloseReq).

Patch #2: puts two strongly related statements, dccp_entail() and
          skb_clone() both in dccp_entail(), since they are not 
	  independent. Patch can be a matter of taste and may be omitted.


The patches are not yet committed - if there are no major objections, they will
be added to the test tree tomoroow, on 

	git://eden-feed.erg.abdn.ac.uk/dccp_exp (subtree dccp)

with snapshots on
	http://eden-feed.erg.abdn.ac.uk/latest-dccp-test-tree.tar.bz2
	http://eden-feed.erg.abdn.ac.uk/latest-dccp-test-tree.diff.gz

[dccp] [RFC/RFT] [Patch 0/2]: Test-tree update to handle retransmissions correctly

[Gerrit Renker]

Here are two patches for the test tree. As with all RFC/RFT patches, these are
posted with the main objective of getting review/comments, and are then put into
the test tree for ... testing.

These patches evolved thanks to input provided by Wei Yongjun regarding an
unresolved problem with retransmissions, fixed by the following two updates:

Patch #1: allows to distinguish retransmitted from original packets,
          using a slight reordering of existing code.
          This is the main fix for the earlier problem of not incrementing
	  the sequence number properly on retransmitted Request packets.

	  The patch introduces a generic condition which can also be tested
	  by other code (e.g. for the retransmission of Close/CloseReq).

Patch #2: puts two strongly related statements, dccp_entail() and
          skb_clone() both in dccp_entail(), since they are not 
	  independent. Patch can be a matter of taste and may be omitted.


The patches are not yet committed - if there are no major objections, they will
be added to the test tree tomoroow, on 

	git://eden-feed.erg.abdn.ac.uk/dccp_exp (subtree dccp)

with snapshots on
	http://eden-feed.erg.abdn.ac.uk/latest-dccp-test-tree.tar.bz2
	http://eden-feed.erg.abdn.ac.uk/latest-dccp-test-tree.diff.gz

[dccp] [RFC/RFT] [Patch 1/2]: Allow to distinguish original and

[Gerrit Renker]

dccp: Allow to distinguish original and retransmitted packets

[dccp] [RFC/RFT] [Patch 1/2]: Allow to distinguish original and retransmitted packets

[Gerrit Renker]

dccp: Allow to distinguish original and retransmitted packets

Re: [dccp] [RFC/RFT] [Patch 1/2]: Allow to distinguish original and retransmitted packets

[Gerrit Renker]

dccp: Allow to distinguish original and retransmitted packets

This patch allows the sending code to distinguish original/initial packets
from retransmitted ones.

This is in particular needed for the retransmission of Request packets, since
 * the first packet uses ISS (generated in net/dccp/ipv?.c) and sets GSS=ISS;
 * all retransmitted packets use GSS' = GSS+1, so that the n-th retransmitted
   packet has sequence number ISS+n (mod 48).

To add this support, the patch reorganises existing code in such a manner that
 * icsk_retransmits == 0 for original packet and
 * icsk_retransmits = n > 0 for n-th retransmitted packet
at the time dccp_transmit_skb() is called via dccp_retransmit_skb().
 
The patch further exploits that, since sk_send_head always contains the original
skb (enqueued/tailed by dccp_entail()), skb_cloned() never evaluated to true.

Lastly, removed the `skb' argument from dccp_retransmit_skb(), since sk_send_head
is used for all retransmissions (the exception is client-Acks in PARTOPEN state,
but these are not put onto the sk_send_head). Updated the documentation also.

Many thanks to Arnaldo Carvalho de Melo and Wei Yongjun for pointing out
unresolved problems during development and for providing helpful input.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/dccp.h   |    2 +-
 net/dccp/output.c |   20 ++++++++++++++++----
 net/dccp/timer.c  |   20 ++++----------------
 3 files changed, 21 insertions(+), 21 deletions(-)

--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -207,7 +207,7 @@ static inline void dccp_csum_outgoing(st
 
 extern void dccp_v4_send_check(struct sock *sk, int len, struct sk_buff *skb);
 
-extern int  dccp_retransmit_skb(struct sock *sk, struct sk_buff *skb);
+extern int  dccp_retransmit_skb(struct sock *sk);
 
 extern void dccp_send_ack(struct sock *sk);
 extern void dccp_reqsk_send_ack(struct sk_buff *sk, struct request_sock *rsk);
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -290,14 +290,26 @@ void dccp_write_xmit(struct sock *sk, in
 	}
 }
 
-int dccp_retransmit_skb(struct sock *sk, struct sk_buff *skb)
+/**
+ * dccp_retransmit_skb  -  Retransmit Request, Close, or CloseReq packets
+ * There are only four retransmittable packet types in DCCP:
+ * - Request  in client-REQUEST  state (sec. 8.1.1),
+ * - CloseReq in server-CLOSEREQ state (sec. 8.3),
+ * - Close    in   node-CLOSING  state (sec. 8.3),
+ * - Acks in client-PARTOPEN state (sec. 8.1.5, handled by dccp_delack_timer()).
+ * This function expects sk->sk_send_head to contain the original skb.
+ */
+int dccp_retransmit_skb(struct sock *sk)
 {
+	BUG_TRAP(sk->sk_send_head != NULL);
+
 	if (inet_csk(sk)->icsk_af_ops->rebuild_header(sk) != 0)
 		return -EHOSTUNREACH; /* Routing failure or similar. */
 
-	return dccp_transmit_skb(sk, (skb_cloned(skb) ?
-				      pskb_copy(skb, GFP_ATOMIC):
-				      skb_clone(skb, GFP_ATOMIC)));
+	/* this count is used to distinguish original and retransmitted skb */
+	inet_csk(sk)->icsk_retransmits++;
+
+	return dccp_transmit_skb(sk, skb_clone(sk->sk_send_head, GFP_ATOMIC));
 }
 
 struct sk_buff *dccp_make_response(struct sock *sk, struct dst_entry *dst,
--- a/net/dccp/timer.c
+++ b/net/dccp/timer.c
@@ -88,21 +88,11 @@ static void dccp_retransmit_timer(struct
 	struct inet_connection_sock *icsk = inet_csk(sk);
 
 	/*
-	 * sk->sk_send_head has to have one skb with
-	 * DCCP_SKB_CB(skb)->dccpd_type set to one of the retransmittable DCCP
-	 * packet types. The only packets eligible for retransmission are:
-	 *	-- Requests in client-REQUEST  state (sec. 8.1.1)
-	 *	-- Acks     in client-PARTOPEN state (sec. 8.1.5)
-	 *	-- CloseReq in server-CLOSEREQ state (sec. 8.3)
-	 *	-- Close    in   node-CLOSING  state (sec. 8.3)                */
-	BUG_TRAP(sk->sk_send_head != NULL);
-
-	/*
 	 * More than than 4MSL (8 minutes) has passed, a RESET(aborted) was
 	 * sent, no need to retransmit, this sock is dead.
 	 */
 	if (dccp_write_timeout(sk))
-		goto out;
+		return;
 
 	/*
 	 * We want to know the number of packets retransmitted, not the
@@ -111,29 +101,27 @@ static void dccp_retransmit_timer(struct
 	if (icsk->icsk_retransmits == 0)
 		DCCP_INC_STATS_BH(DCCP_MIB_TIMEOUTS);
 
-	if (dccp_retransmit_skb(sk, sk->sk_send_head) < 0) {
+	if (dccp_retransmit_skb(sk) != 0) {
 		/*
 		 * Retransmission failed because of local congestion,
 		 * do not backoff.
 		 */
-		if (icsk->icsk_retransmits == 0)
+		if (--icsk->icsk_retransmits == 0)
 			icsk->icsk_retransmits = 1;
 		inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
 					  min(icsk->icsk_rto,
 					      TCP_RESOURCE_PROBE_INTERVAL),
 					  DCCP_RTO_MAX);
-		goto out;
+		return;
 	}
 
 	icsk->icsk_backoff++;
-	icsk->icsk_retransmits++;
 
 	icsk->icsk_rto = min(icsk->icsk_rto << 1, DCCP_RTO_MAX);
 	inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto,
 				  DCCP_RTO_MAX);
 	if (icsk->icsk_retransmits > sysctl_dccp_retries1)
 		__sk_dst_reset(sk);
-out:;
 }
 
 static void dccp_write_timer(unsigned long data)

[dccp] [RFC/RFT] [Patch 2/2]: Combine the functionality of enqeueing and cloning

[Gerrit Renker]

dccp: Combine the functionality of enqeueing and cloning

Realising that there is a pattern in that

 * first dccp_entail() is called to enqueue a new skb;
 * then skb_clone() is called to transmit a clone of that skb,

this patch integrates these two interrelated steps into dccp_entail.

Note: the return value of skb_clone is not checked. It may be an idea to add a
      warning if this occurs. In both instances, however, a timer is set for
      retransmission, so that cloning is re-tried via dccp_retransmit_skb().

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/output.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -26,11 +26,13 @@ static inline void dccp_event_ack_sent(s
 	inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
 }
 
-static void dccp_skb_entail(struct sock *sk, struct sk_buff *skb)
+/* enqueue @skb on sk_send_head for retransmission, return clone to send now */
+static struct sk_buff *dccp_skb_entail(struct sock *sk, struct sk_buff *skb)
 {
 	skb_set_owner_w(skb, sk);
 	WARN_ON(sk->sk_send_head);
 	sk->sk_send_head = skb;
+	return skb_clone(sk->sk_send_head, gfp_any());
 }
 
 /*
@@ -536,8 +538,7 @@ int dccp_connect(struct sock *sk)
 
 	DCCP_SKB_CB(skb)->dccpd_type = DCCP_PKT_REQUEST;
 
-	dccp_skb_entail(sk, skb);
-	dccp_transmit_skb(sk, skb_clone(skb, GFP_KERNEL));
+	dccp_transmit_skb(sk, dccp_skb_entail(sk, skb));
 	DCCP_INC_STATS(DCCP_MIB_ACTIVEOPENS);
 
 	/* Timer for repeating the REQUEST until an answer. */
@@ -662,8 +663,7 @@ void dccp_send_close(struct sock *sk, co
 		DCCP_SKB_CB(skb)->dccpd_type = DCCP_PKT_CLOSE;
 
 	if (active) {
-		dccp_skb_entail(sk, skb);
-		dccp_transmit_skb(sk, skb_clone(skb, prio));
+		skb = dccp_skb_entail(sk, skb);
 		/*
 		 * Retransmission timer for active-close: RFC 4340, 8.3 requires
 		 * to retransmit the Close/CloseReq until the CLOSING/CLOSEREQ
@@ -676,6 +676,6 @@ void dccp_send_close(struct sock *sk, co
 		 */
 		inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
 					  DCCP_TIMEOUT_INIT, DCCP_RTO_MAX);
-	} else
-		dccp_transmit_skb(sk, skb);
+	}
+	dccp_transmit_skb(sk, skb);
 }

[dccp] [RFC/RFT] [Patch 1/2]: Allow to distinguish original and

[Gerrit Renker]

dccp: Allow to distinguish original and retransmitted packets

This patch allows the sending code to distinguish original/initial packets
from retransmitted ones.

This is in particular needed for the retransmission of Request packets, since
 * the first packet uses ISS (generated in net/dccp/ipv?.c) and sets GSS=ISS;
 * all retransmitted packets use GSS' = GSS+1, so that the n-th retransmitted
   packet has sequence number ISS+n (mod 48).

To add this support, the patch reorganises existing code in such a manner that
 * icsk_retransmits = 0 for original packet and
 * icsk_retransmits = n > 0 for n-th retransmitted packet
at the time dccp_transmit_skb() is called via dccp_retransmit_skb().
 
The patch further exploits that, since sk_send_head always contains the original
skb (enqueued/tailed by dccp_entail()), skb_cloned() never evaluated to true.

Lastly, removed the `skb' argument from dccp_retransmit_skb(), since sk_send_head
is used for all retransmissions (the exception is client-Acks in PARTOPEN state,
but these are not put onto the sk_send_head). Updated the documentation also.

Many thanks to Arnaldo Carvalho de Melo and Wei Yongjun for pointing out
unresolved problems during development and for providing helpful input.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/dccp.h   |    2 +-
 net/dccp/output.c |   20 ++++++++++++++++----
 net/dccp/timer.c  |   20 ++++----------------
 3 files changed, 21 insertions(+), 21 deletions(-)

--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -207,7 +207,7 @@ static inline void dccp_csum_outgoing(st
 
 extern void dccp_v4_send_check(struct sock *sk, int len, struct sk_buff *skb);
 
-extern int  dccp_retransmit_skb(struct sock *sk, struct sk_buff *skb);
+extern int  dccp_retransmit_skb(struct sock *sk);
 
 extern void dccp_send_ack(struct sock *sk);
 extern void dccp_reqsk_send_ack(struct sk_buff *sk, struct request_sock *rsk);
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -290,14 +290,26 @@ void dccp_write_xmit(struct sock *sk, in
 	}
 }
 
-int dccp_retransmit_skb(struct sock *sk, struct sk_buff *skb)
+/**
+ * dccp_retransmit_skb  -  Retransmit Request, Close, or CloseReq packets
+ * There are only four retransmittable packet types in DCCP:
+ * - Request  in client-REQUEST  state (sec. 8.1.1),
+ * - CloseReq in server-CLOSEREQ state (sec. 8.3),
+ * - Close    in   node-CLOSING  state (sec. 8.3),
+ * - Acks in client-PARTOPEN state (sec. 8.1.5, handled by dccp_delack_timer()).
+ * This function expects sk->sk_send_head to contain the original skb.
+ */
+int dccp_retransmit_skb(struct sock *sk)
 {
+	BUG_TRAP(sk->sk_send_head != NULL);
+
 	if (inet_csk(sk)->icsk_af_ops->rebuild_header(sk) != 0)
 		return -EHOSTUNREACH; /* Routing failure or similar. */
 
-	return dccp_transmit_skb(sk, (skb_cloned(skb) ?
-				      pskb_copy(skb, GFP_ATOMIC):
-				      skb_clone(skb, GFP_ATOMIC)));
+	/* this count is used to distinguish original and retransmitted skb */
+	inet_csk(sk)->icsk_retransmits++;
+
+	return dccp_transmit_skb(sk, skb_clone(sk->sk_send_head, GFP_ATOMIC));
 }
 
 struct sk_buff *dccp_make_response(struct sock *sk, struct dst_entry *dst,
--- a/net/dccp/timer.c
+++ b/net/dccp/timer.c
@@ -88,21 +88,11 @@ static void dccp_retransmit_timer(struct
 	struct inet_connection_sock *icsk = inet_csk(sk);
 
 	/*
-	 * sk->sk_send_head has to have one skb with
-	 * DCCP_SKB_CB(skb)->dccpd_type set to one of the retransmittable DCCP
-	 * packet types. The only packets eligible for retransmission are:
-	 *	-- Requests in client-REQUEST  state (sec. 8.1.1)
-	 *	-- Acks     in client-PARTOPEN state (sec. 8.1.5)
-	 *	-- CloseReq in server-CLOSEREQ state (sec. 8.3)
-	 *	-- Close    in   node-CLOSING  state (sec. 8.3)                */
-	BUG_TRAP(sk->sk_send_head != NULL);
-
-	/*
 	 * More than than 4MSL (8 minutes) has passed, a RESET(aborted) was
 	 * sent, no need to retransmit, this sock is dead.
 	 */
 	if (dccp_write_timeout(sk))
-		goto out;
+		return;
 
 	/*
 	 * We want to know the number of packets retransmitted, not the
@@ -111,29 +101,27 @@ static void dccp_retransmit_timer(struct
 	if (icsk->icsk_retransmits = 0)
 		DCCP_INC_STATS_BH(DCCP_MIB_TIMEOUTS);
 
-	if (dccp_retransmit_skb(sk, sk->sk_send_head) < 0) {
+	if (dccp_retransmit_skb(sk) != 0) {
 		/*
 		 * Retransmission failed because of local congestion,
 		 * do not backoff.
 		 */
-		if (icsk->icsk_retransmits = 0)
+		if (--icsk->icsk_retransmits = 0)
 			icsk->icsk_retransmits = 1;
 		inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
 					  min(icsk->icsk_rto,
 					      TCP_RESOURCE_PROBE_INTERVAL),
 					  DCCP_RTO_MAX);
-		goto out;
+		return;
 	}
 
 	icsk->icsk_backoff++;
-	icsk->icsk_retransmits++;
 
 	icsk->icsk_rto = min(icsk->icsk_rto << 1, DCCP_RTO_MAX);
 	inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto,
 				  DCCP_RTO_MAX);
 	if (icsk->icsk_retransmits > sysctl_dccp_retries1)
 		__sk_dst_reset(sk);
-out:;
 }
 
 static void dccp_write_timer(unsigned long data)

Re: [dccp] [RFC/RFT] [Patch 1/2]: Allow to distinguish original

[Arnaldo Carvalho de Melo]

Em Thu, Jun 05, 2008 at 03:45:29PM +0100, Gerrit Renker escreveu:
> dccp: Allow to distinguish original and retransmitted packets

Why not do it the way I suggested? I.e. using
__dccp_transmit_skb/dccp_transmit_skb? Less changes, same effect, or am
I missing something?

- Arnaldo

Re: [dccp] [RFC/RFT] [Patch 1/2]: Allow to distinguish original and retransmitted packets

[Arnaldo Carvalho de Melo]

Em Thu, Jun 05, 2008 at 03:45:29PM +0100, Gerrit Renker escreveu:
> dccp: Allow to distinguish original and retransmitted packets

Why not do it the way I suggested? I.e. using
__dccp_transmit_skb/dccp_transmit_skb? Less changes, same effect, or am
I missing something?

- Arnaldo

Re: [dccp] [RFC/RFT] [Patch 1/2]: Allow to distinguish original

[Gerrit Renker]

Quoting Arnaldo:
| Em Thu, Jun 05, 2008 at 03:45:29PM +0100, Gerrit Renker escreveu:
| > dccp: Allow to distinguish original and retransmitted packets
| 
| Why not do it the way I suggested? I.e. using
| __dccp_transmit_skb/dccp_transmit_skb? Less changes, same effect, or am
| I missing something?
| 
I had sat down and tried to integrate your solution and had replied in a
separate thread what the difficulties were in doing this. Yes, your
solution solves the problem of not incrementing GSS when sending the
initial Request packet via dccp_connect.

But there are two other problems which it didn't solve
 (a) when GSS is incremented in (__)dccp_transmit_skb() and the
     option-insertion code fails for some reason, there is a hole in the 
     sequence space. I don't know whether this is a big problem, but to
     me it seems cleaner to first assign the incremented value of GSS to
     a temporary variable and update GSS only if option-insertion succeeds;
 (b) in the mainline code updating AWL=GSS is missing, so we additionally
     need to call dccp_update_gss to update AWL. This is a bug which is
     fixed in the test tree. In RFC 4340, 7.5.1 it is defined for DCCP A as
          AWL := max(GSS + 1 - W', ISS).
     where W' is the local value of the Sequence Window. The problem in the
     mainline code is that there is no distinction between local and remote
     Sequence Window, i.e. there is only one variable and no negotiation.
     For this particular problem it  does not need feature negotiation
     since it is the local value and since Sequence Window is a non-negotiable
     feature (RFC 4340, 6.4), i.e. the remote peer has to accept this value
     via Change L(Sequence Window).
     But irrespective of feature negotiation, we need to call dccp_update_gss
     in dccp_transmit_skb, so that the Ack window is kept up-to-date with
     regard to GSS.

So, facing these two problems, I was stuck with how to integrate your
solution with the above and couldn't figure out how to fit that in.

That is because the control flow in dccp_transmit_skb, to solve the above, is
 
	/* assign incremented value to dccp_skb_cb */
	dcb->dccpd_seq = ADD48(dp->dccps_gss, 1);

	switch (dcb->dccpd_type) {
		case DCCP_PKT_REQUEST:
                        set_ack = 0;
                        /* Use ISS on the first (non-retransmitted) Request. */
                        if (icsk->icsk_retransmits = 0)
                                dcb->dccpd_seq = dp->dccps_iss;
			break;
	// ...
	}

	/* if this fails, GSS is not updated */
	if (dccp_insert_options(sk, skb)) {
		kfree_skb(skb);
		return -EPROTO;
	}

	// ...
	dccp_update_gss(sk, dcb->dccpd_seq);	/* this updates GSS, AWH, AWL */
	dccp_hdr_set_seq(dh, dp->dccps_gss);

	// ...
	// transmit the skb via queue_xmit

I have tested my code to work correctly with up to net.dccp.default.retries1=5,
it will also work correctly with higher values.

Gerrit

Re: [dccp] [RFC/RFT] [Patch 1/2]: Allow to distinguish original and retransmitted packets

[Gerrit Renker]

Quoting Arnaldo:
| Em Thu, Jun 05, 2008 at 03:45:29PM +0100, Gerrit Renker escreveu:
| > dccp: Allow to distinguish original and retransmitted packets
| 
| Why not do it the way I suggested? I.e. using
| __dccp_transmit_skb/dccp_transmit_skb? Less changes, same effect, or am
| I missing something?
| 
I had sat down and tried to integrate your solution and had replied in a
separate thread what the difficulties were in doing this. Yes, your
solution solves the problem of not incrementing GSS when sending the
initial Request packet via dccp_connect.

But there are two other problems which it didn't solve
 (a) when GSS is incremented in (__)dccp_transmit_skb() and the
     option-insertion code fails for some reason, there is a hole in the 
     sequence space. I don't know whether this is a big problem, but to
     me it seems cleaner to first assign the incremented value of GSS to
     a temporary variable and update GSS only if option-insertion succeeds;
 (b) in the mainline code updating AWL=GSS is missing, so we additionally
     need to call dccp_update_gss to update AWL. This is a bug which is
     fixed in the test tree. In RFC 4340, 7.5.1 it is defined for DCCP A as
          AWL := max(GSS + 1 - W', ISS).
     where W' is the local value of the Sequence Window. The problem in the
     mainline code is that there is no distinction between local and remote
     Sequence Window, i.e. there is only one variable and no negotiation.
     For this particular problem it  does not need feature negotiation
     since it is the local value and since Sequence Window is a non-negotiable
     feature (RFC 4340, 6.4), i.e. the remote peer has to accept this value
     via Change L(Sequence Window).
     But irrespective of feature negotiation, we need to call dccp_update_gss
     in dccp_transmit_skb, so that the Ack window is kept up-to-date with
     regard to GSS.

So, facing these two problems, I was stuck with how to integrate your
solution with the above and couldn't figure out how to fit that in.

That is because the control flow in dccp_transmit_skb, to solve the above, is
 
	/* assign incremented value to dccp_skb_cb */
	dcb->dccpd_seq = ADD48(dp->dccps_gss, 1);

	switch (dcb->dccpd_type) {
		case DCCP_PKT_REQUEST:
                        set_ack = 0;
                        /* Use ISS on the first (non-retransmitted) Request. */
                        if (icsk->icsk_retransmits == 0)
                                dcb->dccpd_seq = dp->dccps_iss;
			break;
	// ...
	}

	/* if this fails, GSS is not updated */
	if (dccp_insert_options(sk, skb)) {
		kfree_skb(skb);
		return -EPROTO;
	}

	// ...
	dccp_update_gss(sk, dcb->dccpd_seq);	/* this updates GSS, AWH, AWL */
	dccp_hdr_set_seq(dh, dp->dccps_gss);

	// ...
	// transmit the skb via queue_xmit

I have tested my code to work correctly with up to net.dccp.default.retries1=5,
it will also work correctly with higher values.

Gerrit