Skip to contentSkip to conversation

: Support inserting options during the 3-way handshake

PATCH In review Gerrit Renker 3 participants 5 messages Sep 25, 2007 – Sep 26, 2007 Networking

lore
Share & tools
lore
#0 lore raw

PATCH 8/8 : Support inserting options during the 3-way handshake

[DCCP]: Support inserting options during the 3-way handshake

This provides a separate routine to insert options during the initial handshake. The main purpose is to conduct feature negotiation, for the moment the only user is the timestamp echo needed for the (CCID3) handshake RTT sample.

Padding of options has been put into a small separate routine, to be shared among the two functions. This could also be used as a generic routine to finish inserting options.

Also removed an `XXX' comment since its content was obvious.

---
 net/dccp/dccp.h    |    1 +
 net/dccp/options.c |   40 ++++++++++++++++++++++++++++------------
 net/dccp/output.c  |    2 +-
 3 files changed, 30 insertions(+), 13 deletions(-)
net/dccp/output.c +1 -1
@@ -303,7 +303,7 @@ struct sk_buff *dccp_make_response(struc
 	DCCP_SKB_CB(skb)->dccpd_type = DCCP_PKT_RESPONSE;
 	DCCP_SKB_CB(skb)->dccpd_seq  = dreq->dreq_iss;
 
-	if (dccp_insert_options(sk, skb)) {
+	if (dccp_insert_options_rsk(dreq, skb)) {
 		kfree_skb(skb);
 		return NULL;
 	}
net/dccp/dccp.h +1 -0
@@ -422,6 +422,7 @@ static inline int dccp_ack_pending(const
 }
 
 extern int dccp_insert_options(struct sock *sk, struct sk_buff *skb);
+extern int dccp_insert_options_rsk(struct dccp_request_sock*, struct sk_buff*);
 extern int dccp_insert_option_elapsed_time(struct sock *sk,
 					    struct sk_buff *skb,
 					    u32 elapsed_time);
net/dccp/options.c +28 -12
@@ -160,7 +160,7 @@ int dccp_parse_options(struct sock *sk, 
 			if (len != 4)
 				goto out_invalid_option;
 
-			tse = dreq? &dreq->dreq_tstamp : dp->dccps_tstamp;
+			tse = dreq? &dreq->dreq_tstamp : &dp->dccps_tstamp;
 			/*
 			 * Keep the earliest received timestamp on the socket,
 			 * until echoing to the peer frees it. This policy is
@@ -525,6 +525,18 @@ static int dccp_insert_options_feat(stru
 	return 0;
 }
 
+/* The length of all options needs to be a multiple of 4 (5.8) */
+static void dccp_insert_option_padding(struct sk_buff *skb)
+{
+	int padding = DCCP_SKB_CB(skb)->dccpd_opt_len % 4;
+
+	if (padding != 0) {
+		padding = 4 - padding;
+		memset(skb_push(skb, padding), 0, padding);
+		DCCP_SKB_CB(skb)->dccpd_opt_len += padding;
+	}
+}
+
 int dccp_insert_options(struct sock *sk, struct sk_buff *skb)
 {
 	struct dccp_sock *dp = dccp_sk(sk);
@@ -568,18 +580,22 @@ int dccp_insert_options(struct sock *sk,
 	    dccp_insert_option_timestamp_echo(&dp->dccps_tstamp, skb))
 		return -1;
 
-	/* XXX: insert other options when appropriate */
+	dccp_insert_option_padding(skb);
+	return 0;
+}
 
-	if (DCCP_SKB_CB(skb)->dccpd_opt_len != 0) {
-		/* The length of all options has to be a multiple of 4 */
-		int padding = DCCP_SKB_CB(skb)->dccpd_opt_len % 4;
-
-		if (padding != 0) {
-			padding = 4 - padding;
-			memset(skb_push(skb, padding), 0, padding);
-			DCCP_SKB_CB(skb)->dccpd_opt_len += padding;
-		}
-	}
+/**
+ * dccp_insert_options_rsk  -  dccp_insert_options for request sockets
+ * This function is analogous to above - also use with dccp_reserve_hdr_space().
+ */
+int dccp_insert_options_rsk(struct dccp_request_sock *dreq, struct sk_buff *skb)
+{
+	DCCP_SKB_CB(skb)->dccpd_opt_len = 0;
+
+	if (dreq->dreq_tstamp != NULL &&
+	    dccp_insert_option_timestamp_echo(&dreq->dreq_tstamp, skb))
+		return -1;
 
+	dccp_insert_option_padding(skb);
 	return 0;
 }
#1 lore raw

Re: [PATCH 8/8]: Support inserting options during the 3-way

Em Tue, Sep 25, 2007 at 03:30:51PM +0100, Gerrit Renker escreveu:

Quoted 43 lines

[DCCP]: Support inserting options during the 3-way handshake

This provides a separate routine to insert options during the initial handshake. The main purpose is to conduct feature negotiation, for the moment the only user is the timestamp echo needed for the (CCID3) handshake RTT sample.

Padding of options has been put into a small separate routine, to be shared among the two functions. This could also be used as a generic routine to finish inserting options.

Also removed an `XXX' comment since its content was obvious.

---
 net/dccp/dccp.h    |    1 +
 net/dccp/options.c |   40 ++++++++++++++++++++++++++++------------
 net/dccp/output.c  |    2 +-
 3 files changed, 30 insertions(+), 13 deletions(-)
net/dccp/output.c +1 -1
@@ -303,7 +303,7 @@ struct sk_buff *dccp_make_response(struc
 	DCCP_SKB_CB(skb)->dccpd_type = DCCP_PKT_RESPONSE;
 	DCCP_SKB_CB(skb)->dccpd_seq  = dreq->dreq_iss;
 
-	if (dccp_insert_options(sk, skb)) {
+	if (dccp_insert_options_rsk(dreq, skb)) {
 		kfree_skb(skb);
 		return NULL;
 	}
net/dccp/dccp.h +1 -0
@@ -422,6 +422,7 @@ static inline int dccp_ack_pending(const
 }
 
 extern int dccp_insert_options(struct sock *sk, struct sk_buff *skb);
+extern int dccp_insert_options_rsk(struct dccp_request_sock*, struct sk_buff*);
 extern int dccp_insert_option_elapsed_time(struct sock *sk,
 					    struct sk_buff *skb,
 					    u32 elapsed_time);

net/dccp/options.c int dccp_parse_options(struct sock *sk, · line 163

Quoted 5 lines
net/dccp/options.c +1 -1
@@ -160,7 +160,7 @@ int dccp_parse_options(struct sock *sk, 
 			if (len != 4)
 				goto out_invalid_option;
 
-			tse = dreq? &dreq->dreq_tstamp : dp->dccps_tstamp;
+			tse = dreq? &dreq->dreq_tstamp : &dp->dccps_tstamp;

And here you fix it in an unrelated patch, tsc, tsc :-)

I'll try to fix up these issues since I think its already late in Scotland :-)

Quoted 60 lines
net/dccp/options.c +27 -12
 			/*
 			 * Keep the earliest received timestamp on the socket,
 			 * until echoing to the peer frees it. This policy is
@@ -525,6 +525,18 @@ static int dccp_insert_options_feat(stru
 	return 0;
 }
 
+/* The length of all options needs to be a multiple of 4 (5.8) */
+static void dccp_insert_option_padding(struct sk_buff *skb)
+{
+	int padding = DCCP_SKB_CB(skb)->dccpd_opt_len % 4;
+
+	if (padding != 0) {
+		padding = 4 - padding;
+		memset(skb_push(skb, padding), 0, padding);
+		DCCP_SKB_CB(skb)->dccpd_opt_len += padding;
+	}
+}
+
 int dccp_insert_options(struct sock *sk, struct sk_buff *skb)
 {
 	struct dccp_sock *dp = dccp_sk(sk);
@@ -568,18 +580,22 @@ int dccp_insert_options(struct sock *sk,
 	    dccp_insert_option_timestamp_echo(&dp->dccps_tstamp, skb))
 		return -1;
 
-	/* XXX: insert other options when appropriate */
+	dccp_insert_option_padding(skb);
+	return 0;
+}
 
-	if (DCCP_SKB_CB(skb)->dccpd_opt_len != 0) {
-		/* The length of all options has to be a multiple of 4 */
-		int padding = DCCP_SKB_CB(skb)->dccpd_opt_len % 4;
-
-		if (padding != 0) {
-			padding = 4 - padding;
-			memset(skb_push(skb, padding), 0, padding);
-			DCCP_SKB_CB(skb)->dccpd_opt_len += padding;
-		}
-	}
+/**
+ * dccp_insert_options_rsk  -  dccp_insert_options for request sockets
+ * This function is analogous to above - also use with dccp_reserve_hdr_space().
+ */
+int dccp_insert_options_rsk(struct dccp_request_sock *dreq, struct sk_buff *skb)
+{
+	DCCP_SKB_CB(skb)->dccpd_opt_len = 0;
+
+	if (dreq->dreq_tstamp != NULL &&
+	    dccp_insert_option_timestamp_echo(&dreq->dreq_tstamp, skb))
+		return -1;
 
+	dccp_insert_option_padding(skb);
 	return 0;
 }
-

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

#2 lore raw
In reply to Gerrit Renker
Quoted 6 lines · Gerrit Renker

[DCCP]: Support inserting options during the 3-way handshake

This provides a separate routine to insert options during the initial handshake. The main purpose is to conduct feature negotiation, for the moment the only user is the timestamp echo needed for the (CCID3) handshake RTT sample.

Quoted 7 lines

Padding of options has been put into a small separate routine, to be shared among the two functions. This could also be used as a generic routine to finish inserting options.

Also removed an `XXX' comment since its content was obvious.

Once the tse pointer issue is shifted - I noticed this in this patch, but not original. I must only be half as smart as Arnaldo ;-)

Signature · 3 lines
Web1: http://wand.net.nz/~iam4/
Web2: http://www.jandi.co.nz
Blog: http://iansblog.jandi.co.nz
#3 lore raw
In reply to Gerrit Renker

Quoting Arnaldo Carvalho de Melo:

|  > --- a/net/dccp/options.c
|  > +++ b/net/dccp/options.c
|  > @@ -160,7 +160,7 @@ int dccp_parse_options(struct sock *sk, 
|  >                       if (len != 4)
|  >                               goto out_invalid_option;
|  >  
|  > -                     tse = dreq? &dreq->dreq_tstamp : dp->dccps_tstamp;
|  > +                     tse = dreq? &dreq->dreq_tstamp : &dp->dccps_tstamp;
|  
|  And here you fix it in an unrelated patch, tsc, tsc :-)

These kind of errors don't happen on purpose, if I submitted everything as one big patch no one would notice. But with one problem split into several tens of patches it easily happens that one hunk is fixed while quilt is in a different patch - and then it gets assigned the wrong patch. I missed this hunk so thank you two for spotting this - it is fixed now.

#4 lore raw

Re: [PATCH 8/8]: Support inserting options during the 3-way

In reply to Gerrit Renker

Em Wed, Sep 26, 2007 at 10:04:22AM +0100, Gerrit Renker escreveu:

Quoted 12 lines

Quoting Arnaldo Carvalho de Melo:

|  > --- a/net/dccp/options.c
|  > +++ b/net/dccp/options.c
|  > @@ -160,7 +160,7 @@ int dccp_parse_options(struct sock *sk, 
|  >                       if (len != 4)
|  >                               goto out_invalid_option;
|  >  
|  > -                     tse = dreq? &dreq->dreq_tstamp : dp->dccps_tstamp;
|  > +                     tse = dreq? &dreq->dreq_tstamp : &dp->dccps_tstamp;
|  
|  And here you fix it in an unrelated patch, tsc, tsc :-)
Quoted 4 lines

These kind of errors don't happen on purpose, if I submitted everything as one big patch no one would notice. But with one problem split into several tens of patches it easily happens that one hunk is fixed while quilt is in a different patch - and then it gets assigned the wrong patch. I missed this hunk so thank you two for spotting this - it is fixed now.

Thanks!

Dave pulled the first 6 patches I merged from you and has rebased 2.6.24, I'll follow suit, apply the ones I already have here and continue the review/test process.

- Arnaldo