[PATCH] Allow Acknowledgment of all valid Response packets

[PATCH] Allow Acknowledgment of all valid Response packets

[Samuel Jero]

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

All,

In the course of examining the performance of DCCP over long delay links
for a Delay Tolerant Networking related research project, I believe I
have identified a bug in the DCCP code.

The bug is in the handling of state for listening sockets, particularly
the sequence numbers for REQUEST and RESPONSE packets. Currently, the
code only keeps track of one REQUEST sequence number (the Initial
Sequence Received--isr) and one RESPONSE packet (the Initial Sequence
Sent iss). See include/linux/dccp.h line 400 and net/dccp/ipv4.c line 639.

This becomes a problem when a sender sends multiple REQUESTS (presumably
because of the retransmission timeout). The receiver sends a RESPONSE
for every REQUEST, but only keeps state on the last one.
If the sender eventually ACKs a RESPONSE other than the most recent one
(say because the link has a several second RTT), the receiver will
consider such an ACK to be invalid because it only keeps state for the
most recent RESPONSE (and the such an ACK will acknowledge a sequence
number before the ISS). This results in the connection being reset.

I experienced this bug when I attempted to establish a DCCP connection
with an RTT greater than three seconds. However, I believe this also
violates the RFC 4340, specifically sections 7.1 and 8.5 step 11.
Further, this could cause significant problems if DCCP ever reduces it
first REQUEST timeout.

My fix is to only set the Initial Sequence (Sent/Received) once and to
keep additional state on Greatest Sequence (Sent/Received). That way the
ACKs of old RESPONSEs can be correctly identified and processed.

Samuel Jero
Internetworking Research Group
Ohio University

---

DCCP currently only keeps an Initial Sequence Sent for the RESPONSE
packets it sends and resets this each time it sends a new one. This
causes problems with ACKs for old RESPONSEs. Such ACKs will acknowledge
a packet that is now below the ISS and such connections will therefore
be reset.

This causes problems for connections where retransmitted REQUESTs are
not always due to packet loss (specifically long delay networks), and
violates RFC 4340 7.1

This patch keeps more state (a GSR and GSS) on REQUEST/RESPONSE packets
so that we can correctly identify and accept ACKs for old RESPONSEs.

---

Signed-off-by: Samuel Jero <sj323707@ohio.edu>
diff --git a/include/linux/dccp.h b/include/linux/dccp.h
index c188f5e..9beb840 100644
--- a/include/linux/dccp.h
+++ b/include/linux/dccp.h
@@ -400,6 +400,8 @@ struct dccp_request_sock {
 	struct inet_request_sock dreq_inet_rsk;
 	__u64			 dreq_iss;
 	__u64			 dreq_isr;
+	__u64			 dreq_gss;
+	__u64			 dreq_gsr;
 	__be32			 dreq_service;
 	struct list_head	 dreq_featneg;
 	__u32			 dreq_timestamp_echo;
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 9440e98..084b504 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -296,7 +296,8 @@ static void dccp_v4_err(struct sk_buff *skb, u32 info)
 		 */
 		WARN_ON(req->sk);

-		if (seq != dccp_rsk(req)->dreq_iss) {
+		if (seq < dccp_rsk(req)->dreq_iss ||
+						seq > dccp_rsk(req)->dreq_gss) {
 			NET_INC_STATS_BH(net, LINUX_MIB_OUTOFWINDOWICMPS);
 			goto out;
 		}
@@ -639,7 +639,9 @@ int dccp_v4_conn_request(struct sock *sk, struct
sk_buff *skb)
 	 * dccp_create_openreq_child.
 	 */
 	dreq->dreq_isr	   = dcb->dccpd_seq;
+	dreq->dreq_gsr	   = dreq->dreq_isr;
 	dreq->dreq_iss	   = dccp_v4_init_sequence(skb);
+	dreq->dreq_gss     = dreq->dreq_iss;
 	dreq->dreq_service = service;

 	if (dccp_v4_send_response(sk, req, NULL))
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 4b9d691..483fc6f 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -192,7 +192,8 @@ static void dccp_v6_err(struct sk_buff *skb, struct
inet6_skb_parm *opt,
 		 */
 		WARN_ON(req->sk != NULL);

-		if (seq != dccp_rsk(req)->dreq_iss) {
+		if (seq < dccp_rsk(req)->dreq_iss ||
+						seq > dccp_rsk(req)->dreq_gss) {
 			NET_INC_STATS_BH(net, LINUX_MIB_OUTOFWINDOWICMPS);
 			goto out;
 		}
@@ -443,7 +443,9 @@ static int dccp_v6_conn_request(struct sock *sk,
struct sk_buff *skb)
 	 *   dccp_create_openreq_child.
 	 */
 	dreq->dreq_isr	   = dcb->dccpd_seq;
+	dreq->dreq_gsr     = dreq->dreq_isr;
 	dreq->dreq_iss	   = dccp_v6_init_sequence(skb);
+	dreq->dreq_gss     = dreq->dreq_iss;
 	dreq->dreq_service = service;

 	if (dccp_v6_send_response(sk, req, NULL))
diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
index 5a7f90b..ab831e8 100644
--- a/net/dccp/minisocks.c
+++ b/net/dccp/minisocks.c
@@ -127,9 +127,11 @@ struct sock *dccp_create_openreq_child(struct sock *sk,
 		 *    activation below, as these windows all depend on the local
 		 *    and remote Sequence Window feature values (7.5.2).
 		 */
-		newdp->dccps_gss = newdp->dccps_iss = dreq->dreq_iss;
+		newdp->dccps_iss = dreq->dreq_iss;
+		newdp->dccps_gss = dreq->dreq_gss;
 		newdp->dccps_gar = newdp->dccps_iss;
-		newdp->dccps_gsr = newdp->dccps_isr = dreq->dreq_isr;
+		newdp->dccps_isr = dreq->dreq_isr;
+		newdp->dccps_gsr = dreq->dreq_gsr;

 		/*
 		 * Activate features: initialise CCIDs, sequence windows etc.
@@ -164,9 +166,9 @@ struct sock *dccp_check_req(struct sock *sk, struct
sk_buff *skb,
 	/* Check for retransmitted REQUEST */
 	if (dccp_hdr(skb)->dccph_type == DCCP_PKT_REQUEST) {

-		if (after48(DCCP_SKB_CB(skb)->dccpd_seq, dreq->dreq_isr)) {
+		if (after48(DCCP_SKB_CB(skb)->dccpd_seq, dreq->dreq_gsr)) {
 			dccp_pr_debug("Retransmitted REQUEST\n");
-			dreq->dreq_isr = DCCP_SKB_CB(skb)->dccpd_seq;
+			dreq->dreq_gsr = DCCP_SKB_CB(skb)->dccpd_seq;
 			/*
 			 * Send another RESPONSE packet
 			 * To protect against Request floods, increment retrans
@@ -186,12 +188,14 @@ struct sock *dccp_check_req(struct sock *sk,
struct sk_buff *skb,
 		goto drop;

 	/* Invalid ACK */
-	if (DCCP_SKB_CB(skb)->dccpd_ack_seq != dreq->dreq_iss) {
+	if (DCCP_SKB_CB(skb)->dccpd_ack_seq < dreq->dreq_iss ||
+			DCCP_SKB_CB(skb)->dccpd_ack_seq > dreq->dreq_gss) {
 		dccp_pr_debug("Invalid ACK number: ack_seq=%llu, "
-			      "dreq_iss=%llu\n",
+			      "dreq_iss=%llu,dreq_gss=%llu\n",
 			      (unsigned long long)
 			      DCCP_SKB_CB(skb)->dccpd_ack_seq,
-			      (unsigned long long) dreq->dreq_iss);
+			      (unsigned long long) dreq->dreq_iss,
+			      (unsigned long long) dreq->dreq_gss);
 		goto drop;
 	}

diff --git a/net/dccp/output.c b/net/dccp/output.c
index ccb9844..c51c71e 100644
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -423,10 +423,10 @@ struct sk_buff *dccp_make_response(struct sock
*sk, struct dst_entry *dst,
 	skb_dst_set(skb, dst_clone(dst));

 	dreq = dccp_rsk(req);
-	if (inet_rsk(req)->acked)	/* increase ISS upon retransmission */
-		dccp_inc_seqno(&dreq->dreq_iss);
+	if (inet_rsk(req)->acked)	/* increase GSS upon retransmission */
+		dccp_inc_seqno(&dreq->dreq_gss);
 	DCCP_SKB_CB(skb)->dccpd_type = DCCP_PKT_RESPONSE;
-	DCCP_SKB_CB(skb)->dccpd_seq  = dreq->dreq_iss;
+	DCCP_SKB_CB(skb)->dccpd_seq  = dreq->dreq_gss;

 	/* Resolve feature dependencies resulting from choice of CCID */
 	if (dccp_feat_server_ccid_dependencies(dreq))
@@ -444,8 +444,8 @@ struct sk_buff *dccp_make_response(struct sock *sk,
struct dst_entry *dst,
 			   DCCP_SKB_CB(skb)->dccpd_opt_len) / 4;
 	dh->dccph_type	= DCCP_PKT_RESPONSE;
 	dh->dccph_x	= 1;
-	dccp_hdr_set_seq(dh, dreq->dreq_iss);
-	dccp_hdr_set_ack(dccp_hdr_ack_bits(skb), dreq->dreq_isr);
+	dccp_hdr_set_seq(dh, dreq->dreq_gss);
+	dccp_hdr_set_ack(dccp_hdr_ack_bits(skb), dreq->dreq_gsr);
 	dccp_hdr_response(skb)->dccph_resp_service = dreq->dreq_service;

 	dccp_csum_outgoing(skb);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH] Allow Acknowledgment of all valid Response packets

[Gerrit Renker]

Hi Samuel,

apologies for the late reply, there were some changes in the mail system that
lead to being offline last week.

Thank you for the patch. What I am asking myself is whether the problem is
 * specific to links with a long RTT or
 * can be used in general for all links?

I believe the patch is useful, but am not sure using it for the general case,
need to work through the implications. Also the patch did not apply due to 
line-wrap, I tried to fix this manually but there were several places.

The 'increase ISS on retransmission' issue was first discovered 2006 by Vladimir,
  http://www.mail-archive.com/dccp@vger.kernel.org/msg00710.html

Also, there are other things in DCCP that currently will not work well over long-delay links:
 * DCCP_TIMEOUT_INIT of 3 seconds (you mentioned this already in your email)
 * DCCP_SANE_RTT_MAX = 3 seconds  (only used by ccid3/4)

I wonder thus if it makes sense to introduce a compilation flag "DCCP_LONG_DELAY_LINKS"
which changes these parameters and which would conditionally enable your patch?

Gerrit

Re: [PATCH] Allow Acknowledgment of all valid Response packets

[Samuel Jero]

[-- Attachment #1.1: Type: text/plain, Size: 7449 bytes --]

> Thank you for the patch. What I am asking myself is whether the problem is
>  * specific to links with a long RTT or
>  * can be used in general for all links?

I would argue that this patch is important for all links. Without it, the DCCP
REQUEST/RESPONSE handling is broken. If multiple REQUESTS are sent and nothing
gets lost, the connection will be reset.

It is true that this should only happen when DCCP is operating over a link with
a very long RTT because of the default timeout value (which is much larger that
RFC 4340 recommends). However, it's the principle that I'm more concerned with.
DCCP aught to be able to handle this situation even if it never sees it in
normal operation.

Further, I would note that this could easily be turned into a DOS attack. All
that needs to be done is to duplicate the REQUEST packet, increment it's
sequence number, and make sure it gets to the receiver before the first Ack gets
there (within 1 RTT).

> I believe the patch is useful, but am not sure using it for the general case,
> need to work through the implications. Also the patch did not apply due to 
> line-wrap, I tried to fix this manually but there were several places.

Oops, I've attached the patch to this email, and tried to tell my mail client
not to wrap the lines below. Hopefully that fixes the issue.

> Also, there are other things in DCCP that currently will not work well over long-delay links:
>  * DCCP_TIMEOUT_INIT of 3 seconds (you mentioned this already in your email)
>  * DCCP_SANE_RTT_MAX = 3 seconds  (only used by ccid3/4)
> 
> I wonder thus if it makes sense to introduce a compilation flag "DCCP_LONG_DELAY_LINKS"
> which changes these parameters and which would conditionally enable your patch?

At the moment, I've simply changed DCCP_SANE_RTT_MAX when I compile for my
testing. It might be a good idea to introduce such a compilation flag, to adjust
that variable (and possibly DCCP_TIMEOUT_INIT).

Samuel Jero
Internetworking Research Group
Ohio University

---

Signed-off-by: Samuel Jero <sj323707@ohio.edu>
diff --git a/include/linux/dccp.h b/include/linux/dccp.h
index c188f5e..9beb840 100644
--- a/include/linux/dccp.h
+++ b/include/linux/dccp.h
@@ -400,6 +400,8 @@ struct dccp_request_sock {
 	struct inet_request_sock dreq_inet_rsk;
 	__u64			 dreq_iss;
 	__u64			 dreq_isr;
+	__u64			 dreq_gss;
+	__u64			 dreq_gsr;
 	__be32			 dreq_service;
 	struct list_head	 dreq_featneg;
 	__u32			 dreq_timestamp_echo;
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 9440e98..084b504 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -296,7 +296,8 @@ static void dccp_v4_err(struct sk_buff *skb, u32 info)
 		 */
 		WARN_ON(req->sk);
 
-		if (seq != dccp_rsk(req)->dreq_iss) {
+		if (seq < dccp_rsk(req)->dreq_iss ||
+						seq > dccp_rsk(req)->dreq_gss) {
 			NET_INC_STATS_BH(net, LINUX_MIB_OUTOFWINDOWICMPS);
 			goto out;
 		}
@@ -639,7 +639,9 @@ int dccp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
 	 * dccp_create_openreq_child.
 	 */
 	dreq->dreq_isr	   = dcb->dccpd_seq;
+	dreq->dreq_gsr	   = dreq->dreq_isr;
 	dreq->dreq_iss	   = dccp_v4_init_sequence(skb);
+	dreq->dreq_gss     = dreq->dreq_iss;
 	dreq->dreq_service = service;
 
 	if (dccp_v4_send_response(sk, req, NULL))
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 4b9d691..483fc6f 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -192,7 +192,8 @@ static void dccp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		 */
 		WARN_ON(req->sk != NULL);
 
-		if (seq != dccp_rsk(req)->dreq_iss) {
+		if (seq < dccp_rsk(req)->dreq_iss ||
+						seq > dccp_rsk(req)->dreq_gss) {
 			NET_INC_STATS_BH(net, LINUX_MIB_OUTOFWINDOWICMPS);
 			goto out;
 		}
@@ -443,7 +443,9 @@ static int dccp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
 	 *   dccp_create_openreq_child.
 	 */
 	dreq->dreq_isr	   = dcb->dccpd_seq;
+	dreq->dreq_gsr     = dreq->dreq_isr;
 	dreq->dreq_iss	   = dccp_v6_init_sequence(skb);
+	dreq->dreq_gss     = dreq->dreq_iss;
 	dreq->dreq_service = service;
 
 	if (dccp_v6_send_response(sk, req, NULL))
diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
index 5a7f90b..ab831e8 100644
--- a/net/dccp/minisocks.c
+++ b/net/dccp/minisocks.c
@@ -127,9 +127,11 @@ struct sock *dccp_create_openreq_child(struct sock *sk,
 		 *    activation below, as these windows all depend on the local
 		 *    and remote Sequence Window feature values (7.5.2).
 		 */
-		newdp->dccps_gss = newdp->dccps_iss = dreq->dreq_iss;
+		newdp->dccps_iss = dreq->dreq_iss;
+		newdp->dccps_gss = dreq->dreq_gss;
 		newdp->dccps_gar = newdp->dccps_iss;
-		newdp->dccps_gsr = newdp->dccps_isr = dreq->dreq_isr;
+		newdp->dccps_isr = dreq->dreq_isr;
+		newdp->dccps_gsr = dreq->dreq_gsr;
 
 		/*
 		 * Activate features: initialise CCIDs, sequence windows etc.
@@ -164,9 +166,9 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
 	/* Check for retransmitted REQUEST */
 	if (dccp_hdr(skb)->dccph_type == DCCP_PKT_REQUEST) {
 
-		if (after48(DCCP_SKB_CB(skb)->dccpd_seq, dreq->dreq_isr)) {
+		if (after48(DCCP_SKB_CB(skb)->dccpd_seq, dreq->dreq_gsr)) {
 			dccp_pr_debug("Retransmitted REQUEST\n");
-			dreq->dreq_isr = DCCP_SKB_CB(skb)->dccpd_seq;
+			dreq->dreq_gsr = DCCP_SKB_CB(skb)->dccpd_seq;
 			/*
 			 * Send another RESPONSE packet
 			 * To protect against Request floods, increment retrans
@@ -186,12 +188,14 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
 		goto drop;
 
 	/* Invalid ACK */
-	if (DCCP_SKB_CB(skb)->dccpd_ack_seq != dreq->dreq_iss) {
+	if (DCCP_SKB_CB(skb)->dccpd_ack_seq < dreq->dreq_iss ||
+			DCCP_SKB_CB(skb)->dccpd_ack_seq > dreq->dreq_gss) {
 		dccp_pr_debug("Invalid ACK number: ack_seq=%llu, "
-			      "dreq_iss=%llu\n",
+			      "dreq_iss=%llu,dreq_gss=%llu\n",
 			      (unsigned long long)
 			      DCCP_SKB_CB(skb)->dccpd_ack_seq,
-			      (unsigned long long) dreq->dreq_iss);
+			      (unsigned long long) dreq->dreq_iss,
+			      (unsigned long long) dreq->dreq_gss);
 		goto drop;
 	}
 
diff --git a/net/dccp/output.c b/net/dccp/output.c
index ccb9844..c51c71e 100644
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -423,10 +423,10 @@ struct sk_buff *dccp_make_response(struct sock *sk, struct dst_entry *dst,
 	skb_dst_set(skb, dst_clone(dst));
 
 	dreq = dccp_rsk(req);
-	if (inet_rsk(req)->acked)	/* increase ISS upon retransmission */
-		dccp_inc_seqno(&dreq->dreq_iss);
+	if (inet_rsk(req)->acked)	/* increase GSS upon retransmission */
+		dccp_inc_seqno(&dreq->dreq_gss);
 	DCCP_SKB_CB(skb)->dccpd_type = DCCP_PKT_RESPONSE;
-	DCCP_SKB_CB(skb)->dccpd_seq  = dreq->dreq_iss;
+	DCCP_SKB_CB(skb)->dccpd_seq  = dreq->dreq_gss;
 
 	/* Resolve feature dependencies resulting from choice of CCID */
 	if (dccp_feat_server_ccid_dependencies(dreq))
@@ -444,8 +444,8 @@ struct sk_buff *dccp_make_response(struct sock *sk, struct dst_entry *dst,
 			   DCCP_SKB_CB(skb)->dccpd_opt_len) / 4;
 	dh->dccph_type	= DCCP_PKT_RESPONSE;
 	dh->dccph_x	= 1;
-	dccp_hdr_set_seq(dh, dreq->dreq_iss);
-	dccp_hdr_set_ack(dccp_hdr_ack_bits(skb), dreq->dreq_isr);
+	dccp_hdr_set_seq(dh, dreq->dreq_gss);
+	dccp_hdr_set_ack(dccp_hdr_ack_bits(skb), dreq->dreq_gsr);
 	dccp_hdr_response(skb)->dccph_resp_service = dreq->dreq_service;
 
 	dccp_csum_outgoing(skb);

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Track_Request_Response_Seq_Nums.patch --]
[-- Type: text/x-patch; name="Track_Request_Response_Seq_Nums.patch", Size: 5404 bytes --]

Signed-off-by: Samuel Jero <sj323707@ohio.edu>
diff --git a/include/linux/dccp.h b/include/linux/dccp.h
index c188f5e..9beb840 100644
--- a/include/linux/dccp.h
+++ b/include/linux/dccp.h
@@ -400,6 +400,8 @@ struct dccp_request_sock {
 	struct inet_request_sock dreq_inet_rsk;
 	__u64			 dreq_iss;
 	__u64			 dreq_isr;
+	__u64			 dreq_gss;
+	__u64			 dreq_gsr;
 	__be32			 dreq_service;
 	struct list_head	 dreq_featneg;
 	__u32			 dreq_timestamp_echo;
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 9440e98..084b504 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -296,7 +296,8 @@ static void dccp_v4_err(struct sk_buff *skb, u32 info)
 		 */
 		WARN_ON(req->sk);
 
-		if (seq != dccp_rsk(req)->dreq_iss) {
+		if (seq < dccp_rsk(req)->dreq_iss ||
+						seq > dccp_rsk(req)->dreq_gss) {
 			NET_INC_STATS_BH(net, LINUX_MIB_OUTOFWINDOWICMPS);
 			goto out;
 		}
@@ -639,7 +639,9 @@ int dccp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
 	 * dccp_create_openreq_child.
 	 */
 	dreq->dreq_isr	   = dcb->dccpd_seq;
+	dreq->dreq_gsr	   = dreq->dreq_isr;
 	dreq->dreq_iss	   = dccp_v4_init_sequence(skb);
+	dreq->dreq_gss     = dreq->dreq_iss;
 	dreq->dreq_service = service;
 
 	if (dccp_v4_send_response(sk, req, NULL))
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 4b9d691..483fc6f 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -192,7 +192,8 @@ static void dccp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		 */
 		WARN_ON(req->sk != NULL);
 
-		if (seq != dccp_rsk(req)->dreq_iss) {
+		if (seq < dccp_rsk(req)->dreq_iss ||
+						seq > dccp_rsk(req)->dreq_gss) {
 			NET_INC_STATS_BH(net, LINUX_MIB_OUTOFWINDOWICMPS);
 			goto out;
 		}
@@ -443,7 +443,9 @@ static int dccp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
 	 *   dccp_create_openreq_child.
 	 */
 	dreq->dreq_isr	   = dcb->dccpd_seq;
+	dreq->dreq_gsr     = dreq->dreq_isr;
 	dreq->dreq_iss	   = dccp_v6_init_sequence(skb);
+	dreq->dreq_gss     = dreq->dreq_iss;
 	dreq->dreq_service = service;
 
 	if (dccp_v6_send_response(sk, req, NULL))
diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
index 5a7f90b..ab831e8 100644
--- a/net/dccp/minisocks.c
+++ b/net/dccp/minisocks.c
@@ -127,9 +127,11 @@ struct sock *dccp_create_openreq_child(struct sock *sk,
 		 *    activation below, as these windows all depend on the local
 		 *    and remote Sequence Window feature values (7.5.2).
 		 */
-		newdp->dccps_gss = newdp->dccps_iss = dreq->dreq_iss;
+		newdp->dccps_iss = dreq->dreq_iss;
+		newdp->dccps_gss = dreq->dreq_gss;
 		newdp->dccps_gar = newdp->dccps_iss;
-		newdp->dccps_gsr = newdp->dccps_isr = dreq->dreq_isr;
+		newdp->dccps_isr = dreq->dreq_isr;
+		newdp->dccps_gsr = dreq->dreq_gsr;
 
 		/*
 		 * Activate features: initialise CCIDs, sequence windows etc.
@@ -164,9 +166,9 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
 	/* Check for retransmitted REQUEST */
 	if (dccp_hdr(skb)->dccph_type == DCCP_PKT_REQUEST) {
 
-		if (after48(DCCP_SKB_CB(skb)->dccpd_seq, dreq->dreq_isr)) {
+		if (after48(DCCP_SKB_CB(skb)->dccpd_seq, dreq->dreq_gsr)) {
 			dccp_pr_debug("Retransmitted REQUEST\n");
-			dreq->dreq_isr = DCCP_SKB_CB(skb)->dccpd_seq;
+			dreq->dreq_gsr = DCCP_SKB_CB(skb)->dccpd_seq;
 			/*
 			 * Send another RESPONSE packet
 			 * To protect against Request floods, increment retrans
@@ -186,12 +188,14 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
 		goto drop;
 
 	/* Invalid ACK */
-	if (DCCP_SKB_CB(skb)->dccpd_ack_seq != dreq->dreq_iss) {
+	if (DCCP_SKB_CB(skb)->dccpd_ack_seq < dreq->dreq_iss ||
+			DCCP_SKB_CB(skb)->dccpd_ack_seq > dreq->dreq_gss) {
 		dccp_pr_debug("Invalid ACK number: ack_seq=%llu, "
-			      "dreq_iss=%llu\n",
+			      "dreq_iss=%llu,dreq_gss=%llu\n",
 			      (unsigned long long)
 			      DCCP_SKB_CB(skb)->dccpd_ack_seq,
-			      (unsigned long long) dreq->dreq_iss);
+			      (unsigned long long) dreq->dreq_iss,
+			      (unsigned long long) dreq->dreq_gss);
 		goto drop;
 	}
 
diff --git a/net/dccp/output.c b/net/dccp/output.c
index ccb9844..c51c71e 100644
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -423,10 +423,10 @@ struct sk_buff *dccp_make_response(struct sock *sk, struct dst_entry *dst,
 	skb_dst_set(skb, dst_clone(dst));
 
 	dreq = dccp_rsk(req);
-	if (inet_rsk(req)->acked)	/* increase ISS upon retransmission */
-		dccp_inc_seqno(&dreq->dreq_iss);
+	if (inet_rsk(req)->acked)	/* increase GSS upon retransmission */
+		dccp_inc_seqno(&dreq->dreq_gss);
 	DCCP_SKB_CB(skb)->dccpd_type = DCCP_PKT_RESPONSE;
-	DCCP_SKB_CB(skb)->dccpd_seq  = dreq->dreq_iss;
+	DCCP_SKB_CB(skb)->dccpd_seq  = dreq->dreq_gss;
 
 	/* Resolve feature dependencies resulting from choice of CCID */
 	if (dccp_feat_server_ccid_dependencies(dreq))
@@ -444,8 +444,8 @@ struct sk_buff *dccp_make_response(struct sock *sk, struct dst_entry *dst,
 			   DCCP_SKB_CB(skb)->dccpd_opt_len) / 4;
 	dh->dccph_type	= DCCP_PKT_RESPONSE;
 	dh->dccph_x	= 1;
-	dccp_hdr_set_seq(dh, dreq->dreq_iss);
-	dccp_hdr_set_ack(dccp_hdr_ack_bits(skb), dreq->dreq_isr);
+	dccp_hdr_set_seq(dh, dreq->dreq_gss);
+	dccp_hdr_set_ack(dccp_hdr_ack_bits(skb), dreq->dreq_gsr);
 	dccp_hdr_response(skb)->dccph_resp_service = dreq->dreq_service;
 
 	dccp_csum_outgoing(skb);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH] Allow Acknowledgment of all valid Response packets

[Gerrit Renker]

Hi Samuel,

I have had time to go over the patch. Thanks a lot for submitting it, the previous
implementation was indeed not correct. I made a few small updates, adding comments
and between48(), please have a look at 
    http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p‹cp_exp.git;a=commitdiff;h%26d0c75d6ef8b5bf4bb326155b03d9cc727610

Also tested the result by suppressing the Response so that Request got twice submitted (v4).
Connection was successfully established when the third Request came in.

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] Allow Acknowledgment of all valid Response packets

[Samuel Jero]

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

> I have had time to go over the patch. Thanks a lot for submitting it, the previous
> implementation was indeed not correct. I made a few small updates, adding comments
> and between48(), please have a look at 
>     http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p=dccp_exp.git;a=commitdiff;h=2526d0c75d6ef8b5bf4bb326155b03d9cc727610

Looks good. Good catch on the between48()'s.

Samuel Jero
Internetworking Research Group
Ohio University



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]