Skip to contentSkip to conversation

net-next-2.6 [PATCH 0/6] dccp: several long-tested updates from dccp test tree

PATCH New Gerrit Renker 2 participants 8 messages Jul 4, 2011 – Jul 5, 2011 1 message not in the archive

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): test_tree_patch_set_update_2011-07-04

Gerrit Renker

author
#1 lore raw

Dear Dave,

in the usual slow steps (i.e. only a few patches per week) I would like to continue submitting the DCCP test tree patches. Most of these have been in tested for several years, several fix bugs or open issues, some are needed by subsequent test tree patches.

This set contains several minor fixes and CWV for CCID-2.

Patch #1: fixes several issues in slow-path input processing. Patch #2: refactors common pattern (queueing + cloning) into the same function. Patch #3: straightens cosmetics of info message appearing in logs. Patch #4: puts frequently used CCID-2 inline function into header file. Patch #5: reuses existing function to test for data packets in CCID-2. Patch #6: implements congestion window validation (RFC 2861) in CCID-2.

This set is also available for today's net-next-2.6 at

	git://eden-feed.erg.abdn.ac.uk/net-next-2.6   ==>   subtree "dccp"
---
 net/dccp/ccid.c        |    4 +-
 net/dccp/ccids/ccid2.c |  109 ++++++++++++++++++++++++++++++++++++++----------
 net/dccp/ccids/ccid2.h |   25 +++++++++++-
 net/dccp/input.c       |   61 +++++++++++++--------------
 net/dccp/output.c      |   14 +++---
 5 files changed, 150 insertions(+), 63 deletions(-)
maintainer
#2 lore raw

From: Gerrit Renker <[email protected]>
Date: Mon, 4 Jul 2011 13:01:13 -0600

Quoted 9 lines

This set contains several minor fixes and CWV for CCID-2.

Patch #1: fixes several issues in slow-path input processing. Patch #2: refactors common pattern (queueing + cloning) into the same function. Patch #3: straightens cosmetics of info message appearing in logs. Patch #4: puts frequently used CCID-2 inline function into header file. Patch #5: reuses existing function to test for data packets in CCID-2. Patch #6: implements congestion window validation (RFC 2861) in CCID-2.

Quoted 3 lines

This set is also available for today's net-next-2.6 at

	git://eden-feed.erg.abdn.ac.uk/net-next-2.6   ==>   subtree "dccp"

Pulled, thanks Gerrit.

author
#3 lore raw

PATCH 1/6 dccp: Clean up slow-path input processing

In reply to a message not in the archive

This patch rearranges the order of statements of the slow-path input processing (i.e. any other state than OPEN), to resolve the following issues.

1. Dependencies: the order of statements now better matches RFC 4340, 8.5, i.e. step 7 is before step 9 (previously 9 was before 7), and parsing options in step 8 (which may consume resources) now comes after step 7.
2. Sequence number checks are omitted if in state LISTEN/REQUEST, due to the note underneath the table in RFC 4340, 7.5.3.
As a result, CCID processing is now indeed confined to OPEN/PARTOPEN states, i.e. congestion control is performed only on the flow of data packets. This avoids pathological cases of doing congestion control on those messages which set up and terminate the connection.
3. Packets are now passed on to Ack Vector / CCID processing only after
- step 7 (receive unexpected packets),
- step 9 (receive Reset),
- step 13 (receive CloseReq),
- step 14 (receive Close)
and only if the state is PARTOPEN. This simplifies CCID processing:
- in LISTEN/CLOSED the CCIDs are non-existent;
- in RESPOND/REQUEST the CCIDs have not yet been negotiated;
- in CLOSEREQ and active-CLOSING the node has already closed this socket;
- in passive-CLOSING the client is waiting for its Reset.
In the last case, RFC 4340, 8.3 leaves it open to ignore further incoming data, which is the approach taken here.

Signed-off-by Gerrit Renker <[email protected]>
---
 net/dccp/input.c |   61 ++++++++++++++++++++++++++---------------------------
 1 files changed, 30 insertions(+), 31 deletions(-)
net/dccp/input.c +30 -31
@@ -619,20 +619,31 @@ int dccp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
 		return 1;
 	}
 
-	if (sk->sk_state != DCCP_REQUESTING && sk->sk_state != DCCP_RESPOND) {
-		if (dccp_check_seqno(sk, skb))
-			goto discard;
-
-		/*
-		 * Step 8: Process options and mark acknowledgeable
-		 */
-		if (dccp_parse_options(sk, NULL, skb))
-			return 1;
+	/* Step 6: Check sequence numbers (omitted in LISTEN/REQUEST state) */
+	if (sk->sk_state != DCCP_REQUESTING && dccp_check_seqno(sk, skb))
+		goto discard;
 
-		dccp_handle_ackvec_processing(sk, skb);
-		dccp_deliver_input_to_ccids(sk, skb);
+	/*
+	 *   Step 7: Check for unexpected packet types
+	 *      If (S.is_server and P.type = Response)
+	 *	    or (S.is_client and P.type = Request)
+	 *	    or (S.state = RESPOND and P.type = Data),
+	 *	  Send Sync packet acknowledging P.seqno
+	 *	  Drop packet and return
+	 */
+	if ((dp->dccps_role != DCCP_ROLE_CLIENT &&
+	     dh->dccph_type = DCCP_PKT_RESPONSE) ||
+	    (dp->dccps_role = DCCP_ROLE_CLIENT &&
+	     dh->dccph_type = DCCP_PKT_REQUEST) ||
+	    (sk->sk_state = DCCP_RESPOND && dh->dccph_type = DCCP_PKT_DATA)) {
+		dccp_send_sync(sk, dcb->dccpd_seq, DCCP_PKT_SYNC);
+		goto discard;
 	}
 
+	/*  Step 8: Process options */
+	if (dccp_parse_options(sk, NULL, skb))
+		return 1;
+
 	/*
 	 *  Step 9: Process Reset
 	 *	If P.type = Reset,

@@ -640,31 +651,15 @@ int dccp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
 	 *		S.state := TIMEWAIT
 	 *		Set TIMEWAIT timer
 	 *		Drop packet and return
-	*/
+	 */
 	if (dh->dccph_type = DCCP_PKT_RESET) {
 		dccp_rcv_reset(sk, skb);
 		return 0;
-		/*
-		 *   Step 7: Check for unexpected packet types
-		 *      If (S.is_server and P.type = Response)
-		 *	    or (S.is_client and P.type = Request)
-		 *	    or (S.state = RESPOND and P.type = Data),
-		 *	  Send Sync packet acknowledging P.seqno
-		 *	  Drop packet and return
-		 */
-	} else if ((dp->dccps_role != DCCP_ROLE_CLIENT &&
-		    dh->dccph_type = DCCP_PKT_RESPONSE) ||
-		    (dp->dccps_role = DCCP_ROLE_CLIENT &&
-		     dh->dccph_type = DCCP_PKT_REQUEST) ||
-		    (sk->sk_state = DCCP_RESPOND &&
-		     dh->dccph_type = DCCP_PKT_DATA)) {
-		dccp_send_sync(sk, dcb->dccpd_seq, DCCP_PKT_SYNC);
-		goto discard;
-	} else if (dh->dccph_type = DCCP_PKT_CLOSEREQ) {
+	} else if (dh->dccph_type = DCCP_PKT_CLOSEREQ) {	/* Step 13 */
 		if (dccp_rcv_closereq(sk, skb))
 			return 0;
 		goto discard;
-	} else if (dh->dccph_type = DCCP_PKT_CLOSE) {
+	} else if (dh->dccph_type = DCCP_PKT_CLOSE) {		/* Step 14 */
 		if (dccp_rcv_close(sk, skb))
 			return 0;
 		goto discard;

@@ -679,8 +674,12 @@ int dccp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
 		__kfree_skb(skb);
 		return 0;
 
-	case DCCP_RESPOND:
 	case DCCP_PARTOPEN:
+		/* Step 8: if using Ack Vectors, mark packet acknowledgeable */
+		dccp_handle_ackvec_processing(sk, skb);
+		dccp_deliver_input_to_ccids(sk, skb);
+		/* fall through */
+	case DCCP_RESPOND:
 		queued = dccp_rcv_respond_partopen_state_process(sk, skb,
 								 dh, len);
 		break;
author
#4 lore raw

PATCH 2/6 dccp: combine the functionality of enqeueing and cloning

In reply to a message not in the archive

Realising the following call pattern,
* first dccp_entail() is called to enqueue a new skb and
* then skb_clone() is called to transmit a clone of that skb,
this patch integrates both into the same function.

Signed-off-by Gerrit Renker <[email protected]>
---
 net/dccp/output.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
net/dccp/output.c +7 -7
@@ -27,11 +27,13 @@ static inline void dccp_event_ack_sent(struct sock *sk)
 	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());
 }
 
 /*

@@ -552,8 +554,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. */

@@ -678,8 +679,7 @@ void dccp_send_close(struct sock *sk, const int active)
 		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

@@ -692,6 +692,6 @@ void dccp_send_close(struct sock *sk, const int active)
 		 */
 		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);
 }
author
#5 lore raw

PATCH 3/6 dccp: cosmetics of info message

In reply to a message not in the archive

Change the CCID (de)activation message to start with the protocol name, as 'CCID' is already in there.

Signed-off-by Gerrit Renker <[email protected]>
---
 net/dccp/ccid.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
net/dccp/ccid.c +2 -2
@@ -118,7 +118,7 @@ static int ccid_activate(struct ccid_operations *ccid_ops)
 	if (ccid_ops->ccid_hc_tx_slab = NULL)
 		goto out_free_rx_slab;
 
-	pr_info("CCID: Activated CCID %d (%s)\n",
+	pr_info("DCCP: Activated CCID %d (%s)\n",
 		ccid_ops->ccid_id, ccid_ops->ccid_name);
 	err = 0;
 out:

@@ -136,7 +136,7 @@ static void ccid_deactivate(struct ccid_operations *ccid_ops)
 	ccid_kmem_cache_destroy(ccid_ops->ccid_hc_rx_slab);
 	ccid_ops->ccid_hc_rx_slab = NULL;
 
-	pr_info("CCID: Deactivated CCID %d (%s)\n",
+	pr_info("DCCP: Deactivated CCID %d (%s)\n",
 		ccid_ops->ccid_id, ccid_ops->ccid_name);
 }
author
#6 lore raw

PATCH 4/6 dccp ccid-2: move rfc 3390 function into header file

In reply to a message not in the archive

This moves CCID-2's initial window function into the header file, since several parts throughout the CCID-2 code need to call it (CCID-2 still uses RFC 3390).

Signed-off-by Gerrit Renker <[email protected]>
Acked-by Leandro Melo de Sales <[email protected]>
---
 net/dccp/ccids/ccid2.c |    9 ---------
 net/dccp/ccids/ccid2.h |    9 +++++++++
 2 files changed, 9 insertions(+), 9 deletions(-)
net/dccp/ccids/ccid2.c +0 -9
@@ -583,15 +583,6 @@ done:
 	dccp_ackvec_parsed_cleanup(&hc->tx_av_chunks);
 }
 
-/*
- * Convert RFC 3390 larger initial window into an equivalent number of packets.
- * This is based on the numbers specified in RFC 5681, 3.1.
- */
-static inline u32 rfc3390_bytes_to_packets(const u32 smss)
-{
-	return smss <= 1095 ? 4 : (smss > 2190 ? 2 : 3);
-}
-
 static int ccid2_hc_tx_init(struct ccid *ccid, struct sock *sk)
 {
 	struct ccid2_hc_tx_sock *hc = ccid_priv(ccid);
net/dccp/ccids/ccid2.h +9 -0
@@ -88,6 +88,15 @@ static inline bool ccid2_cwnd_network_limited(struct ccid2_hc_tx_sock *hc)
 	return hc->tx_pipe >= hc->tx_cwnd;
 }
 
+/*
+ * Convert RFC 3390 larger initial window into an equivalent number of packets.
+ * This is based on the numbers specified in RFC 5681, 3.1.
+ */
+static inline u32 rfc3390_bytes_to_packets(const u32 smss)
+{
+	return smss <= 1095 ? 4 : (smss > 2190 ? 2 : 3);
+}
+
 struct ccid2_hc_rx_sock {
 	int	rx_data;
 };
author
#7 lore raw

PATCH 5/6 dccp ccid-2: Use existing function to test for data packets

In reply to a message not in the archive

This replaces a switch statement with a test, using the equivalent function dccp_data_packet(skb). It also doubles the range of the field rx_num_data_pkts' by changing the type from int' to u32', avoiding signed/unsigned comparison with the u16 field dccps_r_ack_ratio'.

Signed-off-by Gerrit Renker <[email protected]>
---
 net/dccp/ccids/ccid2.c |   16 ++++++----------
 net/dccp/ccids/ccid2.h |    6 +++++-
 2 files changed, 11 insertions(+), 11 deletions(-)
net/dccp/ccids/ccid2.c +6 -10
@@ -627,18 +627,14 @@ static void ccid2_hc_tx_exit(struct sock *sk)
 
 static void ccid2_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb)
 {
-	const struct dccp_sock *dp = dccp_sk(sk);
 	struct ccid2_hc_rx_sock *hc = ccid2_hc_rx_sk(sk);
 
-	switch (DCCP_SKB_CB(skb)->dccpd_type) {
-	case DCCP_PKT_DATA:
-	case DCCP_PKT_DATAACK:
-		hc->rx_data++;
-		if (hc->rx_data >= dp->dccps_r_ack_ratio) {
-			dccp_send_ack(sk);
-			hc->rx_data = 0;
-		}
-		break;
+	if (!dccp_data_packet(skb))
+		return;
+
+	if (++hc->rx_num_data_pkts >= dccp_sk(sk)->dccps_r_ack_ratio) {
+		dccp_send_ack(sk);
+		hc->rx_num_data_pkts = 0;
 	}
 }
net/dccp/ccids/ccid2.h +5 -1
@@ -97,8 +97,12 @@ static inline u32 rfc3390_bytes_to_packets(const u32 smss)
 	return smss <= 1095 ? 4 : (smss > 2190 ? 2 : 3);
 }
 
+/**
+ * struct ccid2_hc_rx_sock  -  Receiving end of CCID-2 half-connection
+ * @rx_num_data_pkts: number of data packets received since last feedback
+ */
 struct ccid2_hc_rx_sock {
-	int	rx_data;
+	u32	rx_num_data_pkts;
 };
 
 static inline struct ccid2_hc_tx_sock *ccid2_hc_tx_sk(const struct sock *sk)
author
#8 lore raw

PATCH 6/6 dccp ccid-2: Perform congestion-window validation

In reply to a message not in the archive

CCID-2's cwnd increases like TCP during slow-start, which has implications for
* the local Sequence Window value (should be > cwnd),
* the Ack Ratio value.
Hence an exponential growth, if it does not reflect the actual network conditions, can quickly lead to instability.

This patch adds congestion-window validation (RFC2861) to CCID-2:
* cwnd is constrained if the sender is application limited;
* cwnd is reduced after a long idle period, as suggested in the '90 paper by Van Jacobson, in RFC 2581 (sec. 4.1);
* cwnd is never reduced below the RFC 3390 initial window.

As marked in the comments, the code is actually almost a direct copy of the TCP congestion-window-validation algorithms. By continuing this work, it may in future be possible to use the TCP code (not possible at the moment).

The mechanism can be turned off using a module parameter. Sampling of the currently-used window (moving-maximum) is however done constantly; this is used to determine the expected window, which can be exploited to regulate DCCP's Sequence Window value.

This patch also sets slow-start-after-idle (RFC 4341, 5.1), i.e. it behaves like TCP when net.ipv4.tcp_slow_start_after_idle = 1.

Signed-off-by Gerrit Renker <[email protected]>
---
 net/dccp/ccids/ccid2.h |   10 ++++++
 net/dccp/ccids/ccid2.c |   84 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 91 insertions(+), 3 deletions(-)
net/dccp/ccids/ccid2.h +10 -0
@@ -53,6 +53,10 @@ struct ccid2_seq {
  * @tx_rttvar:		     moving average/maximum of @mdev_max
  * @tx_rto:		     RTO value deriving from SRTT and RTTVAR (RFC 2988)
  * @tx_rtt_seq:		     to decay RTTVAR at most once per flight
+ * @tx_cwnd_used:	     actually used cwnd, W_used of RFC 2861
+ * @tx_expected_wnd:	     moving average of @tx_cwnd_used
+ * @tx_cwnd_stamp:	     to track idle periods in CWV
+ * @tx_lsndtime:	     last time (in jiffies) a data packet was sent
  * @tx_rpseq:		     last consecutive seqno
  * @tx_rpdupack:	     dupacks since rpseq
  * @tx_av_chunks:	     list of Ack Vectors received on current skb

@@ -76,6 +80,12 @@ struct ccid2_hc_tx_sock {
 	u64			tx_rtt_seq:48;
 	struct timer_list	tx_rtotimer;
 
+	/* Congestion Window validation (optional, RFC 2861) */
+	u32			tx_cwnd_used,
+				tx_expected_wnd,
+				tx_cwnd_stamp,
+				tx_lsndtime;
+
 	u64			tx_rpseq;
 	int			tx_rpdupack;
 	u32			tx_last_cong;
net/dccp/ccids/ccid2.c +81 -3

111 lines not on this page · raw message