[PATCH 7/7] dccp ccid-2: Phase out the use of boolean Ack Vector sysctl

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: <davem@...>
Cc: <dccp@...>, <netdev@...>, Gerrit Renker <gerrit@...>
Date: Saturday, December 6, 2008 - 12:40 pm

This removes the use of the sysctl and the minisock variable for the Send Ack
Vector feature, as it now is handled fully dynamically via feature negotiation
(i.e. when CCID-2 is enabled, Ack Vectors are automatically enabled as per
 RFC 4341, 4.).

Using a sysctl in parallel to this implementation would open the door to
crashes, since much of the code relies on tests of the boolean minisock /
sysctl variable. Thus, this patch replaces all tests of type

	if (dccp_msk(sk)->dccpms_send_ack_vector)
		/* ... */
with
	if (dp->dccps_hc_rx_ackvec != NULL)
		/* ... */

The dccps_hc_rx_ackvec is allocated by the dccp_hdlr_ackvec() when feature
negotiation concluded that Ack Vectors are to be used on the half-connection.
Otherwise, it is NULL (due to dccp_init_sock/dccp_create_openreq_child),
so that the test is a valid one.

The activation handler for Ack Vectors is called as soon as the feature
negotiation has concluded at the
 * server when the Ack marking the transition RESPOND => OPEN arrives;
 * client after it has sent its ACK, marking the transition REQUEST => PARTOPEN.

Adding the sequence number of the Response packet to the Ack Vector has been
removed, since
 (a) connection establishment implies that the Response has been received;
 (b) the CCIDs only look at packets received in the (PART)OPEN state, i.e.
     this entry will always be ignored;
 (c) it can not be used for anything useful - to detect loss for instance, only
     packets received after the loss can serve as pseudo-dupacks.

There was a FIXME to change the error code when dccp_ackvec_add() fails.
I removed this after finding out that:
 * the check whether ackno < ISN is already made earlier,
 * this Response is likely the 1st packet with an Ackno that the client gets,
 * so when dccp_ackvec_add() fails, the reason is likely not a packet error.


Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Acked-by: Ian McDonald <ian.mcdonald@jandi.co.nz>
---
 Documentation/networking/dccp.txt |    3 ---
 include/linux/dccp.h              |    3 ---
 net/dccp/dccp.h                   |    3 +--
 net/dccp/diag.c                   |    2 +-
 net/dccp/input.c                  |   12 +++---------
 net/dccp/minisocks.c              |    1 -
 net/dccp/options.c                |    7 ++-----
 net/dccp/proto.c                  |    3 +--
 net/dccp/sysctl.c                 |    7 -------
 9 files changed, 8 insertions(+), 33 deletions(-)

--- a/Documentation/networking/dccp.txt
+++ b/Documentation/networking/dccp.txt
@@ -133,9 +133,6 @@ retries2
 	importance for retransmitted acknowledgments and feature negotiation,
 	data packets are never retransmitted. Analogue of tcp_retries2.
 
-send_ackvec = 1
-	Whether or not to send Ack Vector options (sec. 11.5).
-
 tx_ccid = 2
 	Default CCID for the sender-receiver half-connection. Depending on the
 	choice of CCID, the Send Ack Vector feature is enabled automatically.
--- a/include/linux/dccp.h
+++ b/include/linux/dccp.h
@@ -360,7 +360,6 @@ static inline unsigned int dccp_hdr_len(const struct sk_buff *skb)
 #define DCCPF_INITIAL_SEQUENCE_WINDOW		100
 #define DCCPF_INITIAL_ACK_RATIO			2
 #define DCCPF_INITIAL_CCID			DCCPC_CCID2
-#define DCCPF_INITIAL_SEND_ACK_VECTOR		1
 /* FIXME: for now we're default to 1 but it should really be 0 */
 #define DCCPF_INITIAL_SEND_NDP_COUNT		1
 
@@ -370,13 +369,11 @@ static inline unsigned int dccp_hdr_len(const struct sk_buff *skb)
   * Will be used to pass the state from dccp_request_sock to dccp_sock.
   *
   * @dccpms_sequence_window - Sequence Window Feature (section 7.5.2)
-  * @dccpms_send_ack_vector - Send Ack Vector Feature (section 11.5)
   * @dccpms_pending - List of features being negotiated
   * @dccpms_conf -
   */
 struct dccp_minisock {
 	__u64			dccpms_sequence_window;
-	__u8			dccpms_send_ack_vector;
 	struct list_head	dccpms_pending;
 	struct list_head	dccpms_conf;
 };
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -98,7 +98,6 @@ extern int  sysctl_dccp_retries2;
 extern int  sysctl_dccp_feat_sequence_window;
 extern int  sysctl_dccp_feat_rx_ccid;
 extern int  sysctl_dccp_feat_tx_ccid;
-extern int  sysctl_dccp_feat_send_ack_vector;
 extern int  sysctl_dccp_tx_qlen;
 extern int  sysctl_dccp_sync_ratelimit;
 
@@ -434,7 +433,7 @@ static inline int dccp_ack_pending(const struct sock *sk)
 	const struct dccp_sock *dp = dccp_sk(sk);
 	return dp->dccps_timestamp_echo != 0 ||
 #ifdef CONFIG_IP_DCCP_ACKVEC
-	       (dccp_msk(sk)->dccpms_send_ack_vector &&
+	       (dp->dccps_hc_rx_ackvec != NULL &&
 		dccp_ackvec_pending(dp->dccps_hc_rx_ackvec)) ||
 #endif
 	       inet_csk_ack_scheduled(sk);
--- a/net/dccp/diag.c
+++ b/net/dccp/diag.c
@@ -29,7 +29,7 @@ static void dccp_get_info(struct sock *sk, struct tcp_info *info)
 	info->tcpi_backoff	= icsk->icsk_backoff;
 	info->tcpi_pmtu		= icsk->icsk_pmtu_cookie;
 
-	if (dccp_msk(sk)->dccpms_send_ack_vector)
+	if (dp->dccps_hc_rx_ackvec != NULL)
 		info->tcpi_options |= TCPI_OPT_SACK;
 
 	ccid_hc_rx_get_info(dp->dccps_hc_rx_ccid, sk, info);
--- a/net/dccp/input.c
+++ b/net/dccp/input.c
@@ -163,7 +163,7 @@ static void dccp_event_ack_recv(struct sock *sk, struct sk_buff *skb)
 {
 	struct dccp_sock *dp = dccp_sk(sk);
 
-	if (dccp_msk(sk)->dccpms_send_ack_vector)
+	if (dp->dccps_hc_rx_ackvec != NULL)
 		dccp_ackvec_check_rcv_ackno(dp->dccps_hc_rx_ackvec, sk,
 					    DCCP_SKB_CB(skb)->dccpd_ack_seq);
 }
@@ -375,7 +375,7 @@ int dccp_rcv_established(struct sock *sk, struct sk_buff *skb,
 	if (DCCP_SKB_CB(skb)->dccpd_ack_seq != DCCP_PKT_WITHOUT_ACK_SEQ)
 		dccp_event_ack_recv(sk, skb);
 
-	if (dccp_msk(sk)->dccpms_send_ack_vector &&
+	if (dp->dccps_hc_rx_ackvec != NULL &&
 	    dccp_ackvec_add(dp->dccps_hc_rx_ackvec, sk,
 			    DCCP_SKB_CB(skb)->dccpd_seq,
 			    DCCP_ACKVEC_STATE_RECEIVED))
@@ -434,12 +434,6 @@ static int dccp_rcv_request_sent_state_process(struct sock *sk,
 			dp->dccps_syn_rtt = dccp_sample_rtt(sk, 10 * (tstamp -
 			    dp->dccps_options_received.dccpor_timestamp_echo));
 
-		if (dccp_msk(sk)->dccpms_send_ack_vector &&
-		    dccp_ackvec_add(dp->dccps_hc_rx_ackvec, sk,
-				    DCCP_SKB_CB(skb)->dccpd_seq,
-				    DCCP_ACKVEC_STATE_RECEIVED))
-			goto out_invalid_packet; /* FIXME: change error code */
-
 		/* Stop the REQUEST timer */
 		inet_csk_clear_xmit_timer(sk, ICSK_TIME_RETRANS);
 		WARN_ON(sk->sk_send_head == NULL);
@@ -637,7 +631,7 @@ int dccp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
 		if (dcb->dccpd_ack_seq != DCCP_PKT_WITHOUT_ACK_SEQ)
 			dccp_event_ack_recv(sk, skb);
 
-		if (dccp_msk(sk)->dccpms_send_ack_vector &&
+		if (dp->dccps_hc_rx_ackvec != NULL &&
 		    dccp_ackvec_add(dp->dccps_hc_rx_ackvec, sk,
 				    DCCP_SKB_CB(skb)->dccpd_seq,
 				    DCCP_ACKVEC_STATE_RECEIVED))
--- a/net/dccp/minisocks.c
+++ b/net/dccp/minisocks.c
@@ -45,7 +45,6 @@ EXPORT_SYMBOL_GPL(dccp_death_row);
 void dccp_minisock_init(struct dccp_minisock *dmsk)
 {
 	dmsk->dccpms_sequence_window = sysctl_dccp_feat_sequence_window;
-	dmsk->dccpms_send_ack_vector = sysctl_dccp_feat_send_ack_vector;
 }
 
 void dccp_time_wait(struct sock *sk, int state, int timeo)
--- a/net/dccp/options.c
+++ b/net/dccp/options.c
@@ -26,7 +26,6 @@
 int sysctl_dccp_feat_sequence_window = DCCPF_INITIAL_SEQUENCE_WINDOW;
 int sysctl_dccp_feat_rx_ccid	      = DCCPF_INITIAL_CCID;
 int sysctl_dccp_feat_tx_ccid	      = DCCPF_INITIAL_CCID;
-int sysctl_dccp_feat_send_ack_vector = DCCPF_INITIAL_SEND_ACK_VECTOR;
 
 u64 dccp_decode_value_var(const u8 *bf, const u8 len)
 {
@@ -145,8 +144,7 @@ int dccp_parse_options(struct sock *sk, struct dccp_request_sock *dreq,
 		case DCCPO_ACK_VECTOR_1:
 			if (dccp_packet_without_ack(skb))   /* RFC 4340, 11.4 */
 				break;
-
-			if (dccp_msk(sk)->dccpms_send_ack_vector &&
+			if (dp->dccps_hc_rx_ackvec != NULL &&
 			    dccp_ackvec_parse(sk, skb, &ackno, opt, value, len))
 				goto out_invalid_option;
 			break;
@@ -526,7 +524,6 @@ static void dccp_insert_option_padding(struct sk_buff *skb)
 int dccp_insert_options(struct sock *sk, struct sk_buff *skb)
 {
 	struct dccp_sock *dp = dccp_sk(sk);
-	struct dccp_minisock *dmsk = dccp_msk(sk);
 
 	DCCP_SKB_CB(skb)->dccpd_opt_len = 0;
 
@@ -547,7 +544,7 @@ int dccp_insert_options(struct sock *sk, struct sk_buff *skb)
 			if (dccp_insert_option_timestamp(sk, skb))
 				return -1;
 
-		} else if (dmsk->dccpms_send_ack_vector	&&
+		} else if (dp->dccps_hc_rx_ackvec != NULL &&
 			   dccp_ackvec_pending(dp->dccps_hc_rx_ackvec) &&
 			   dccp_insert_option_ackvec(sk, skb)) {
 				return -1;
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -207,7 +207,6 @@ EXPORT_SYMBOL_GPL(dccp_init_sock);
 void dccp_destroy_sock(struct sock *sk)
 {
 	struct dccp_sock *dp = dccp_sk(sk);
-	struct dccp_minisock *dmsk = dccp_msk(sk);
 
 	/*
 	 * DCCP doesn't use sk_write_queue, just sk_send_head
@@ -225,7 +224,7 @@ void dccp_destroy_sock(struct sock *sk)
 	kfree(dp->dccps_service_list);
 	dp->dccps_service_list = NULL;
 
-	if (dmsk->dccpms_send_ack_vector) {
+	if (dp->dccps_hc_rx_ackvec != NULL) {
 		dccp_ackvec_free(dp->dccps_hc_rx_ackvec);
 		dp->dccps_hc_rx_ackvec = NULL;
 	}
--- a/net/dccp/sysctl.c
+++ b/net/dccp/sysctl.c
@@ -41,13 +41,6 @@ static struct ctl_table dccp_default_table[] = {
 		.proc_handler	= proc_dointvec,
 	},
 	{
-		.procname	= "send_ackvec",
-		.data		= &sysctl_dccp_feat_send_ack_vector,
-		.maxlen		= sizeof(sysctl_dccp_feat_send_ack_vector),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
-	},
-	{
 		.procname	= "request_retries",
 		.data		= &sysctl_dccp_request_retries,
 		.maxlen		= sizeof(sysctl_dccp_request_retries),
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: [PATCH 03/37] dccp: List management for new feature nego..., Arnaldo Carvalho de Melo, (Thu Aug 28, 3:43 pm)
Re: [PATCH 0/37] --- Summary of revision changes so far, Gerrit Renker, (Sat Aug 30, 1:25 pm)
Re: net-next-2.6 [pull-request] [PATCH 0/37] dccp: Revised s..., Arnaldo Carvalho de Melo, (Tue Sep 2, 9:50 am)
Re: [PATCH 1/5] dccp: Basic data structure for feature negot..., Arnaldo Carvalho de Melo, (Mon Sep 22, 10:10 am)
Re: [PATCH 2/5] dccp: Implement lookup table for feature-neg..., Arnaldo Carvalho de Melo, (Mon Sep 22, 10:21 am)
Re: v2 [PATCH 2/5] dccp: Implement lookup table for feature-..., Arnaldo Carvalho de Melo, (Wed Sep 24, 10:01 am)
Re: v2 [PATCH 1/5] dccp: Basic data structure for feature ne..., Arnaldo Carvalho de Melo, (Wed Sep 24, 9:59 am)
[PATCH 7/7] dccp ccid-2: Phase out the use of boolean Ack Ve..., Gerrit Renker, (Sat Dec 6, 12:40 pm)
Re: [PATCH 2/5] dccp: Auto-load (when supported) CCID plugin..., Arnaldo Carvalho de Melo, (Mon Dec 15, 9:48 am)
Re: [PATCH 2/5] dccp: Auto-load (when supported) CCID plugin..., Michał Mirosław, (Sat Dec 13, 9:55 am)
Re: [PATCH 2/5] dccp: Auto-load (when supported) CCID plugin..., Michał Mirosław, (Sun Dec 14, 10:50 am)
Re: [PATCH 2/5] dccp: Auto-load (when supported) CCID plugin..., Michał Mirosław, (Tue Dec 16, 7:31 am)
Re: [PATCH 2/5] dccp: Auto-load (when supported) CCID plugin..., Arnaldo Carvalho de Melo, (Tue Dec 16, 7:19 am)
Re: [PATCH 2/5] dccp: Auto-load (when supported) CCID plugin..., Arnaldo Carvalho de Melo, (Tue Dec 16, 6:25 pm)
Re: [PATCH 2/5] dccp: Auto-load (when supported) CCID plugin..., Arnaldo Carvalho de Melo, (Wed Dec 17, 9:13 am)
Re: [PATCH 2/5] dccp: Auto-load (when supported) CCID plugin..., Arnaldo Carvalho de Melo, (Thu Dec 18, 10:01 am)
Re: [PATCH 4/5] dccp: Initialisation and type-checking of fe..., Arnaldo Carvalho de Melo, (Mon Dec 15, 10:15 am)
[PATCH 3/6] dccp: Preference list reconciliation, Gerrit Renker, (Sun Nov 30, 9:22 am)
[PATCH 5/6] dccp: Processing Confirm options, Gerrit Renker, (Sun Nov 30, 9:22 am)
[PATCH 6/6] dccp: Feature activation handlers, Gerrit Renker, (Sun Nov 30, 9:22 am)
[PATCH 2/5] dccp: API to query the current TX/RX CCID, Gerrit Renker, (Sat Nov 22, 6:30 am)
[PATCH 4/5] dccp: Support for Mandatory options, Gerrit Renker, (Sat Nov 22, 6:30 am)
Re: [PATCH 4/5] dccp: Support for Mandatory options, David Miller, (Sun Nov 23, 8:09 pm)
[PATCH 1/5] dccp: Mechanism to resolve CCID dependencies, Gerrit Renker, (Sat Nov 15, 8:11 am)
[PATCH 2/5] dccp: Deprecate old setsockopt framework, Gerrit Renker, (Sat Nov 15, 8:11 am)
Re: [PATCH 2/5] dccp: Deprecate old setsockopt framework, Gerrit Renker, (Mon Nov 17, 11:31 am)
[PATCH 4/5] dccp: Deprecate Ack Ratio sysctl, Gerrit Renker, (Sat Nov 15, 8:11 am)
Re: [PATCH 4/5] dccp: Deprecate Ack Ratio sysctl, David Miller, (Mon Nov 17, 2:56 am)
[PATCH 5/5] dccp: Tidy up setsockopt calls, Gerrit Renker, (Sat Nov 15, 8:11 am)
Re: [PATCH 5/5] dccp: Tidy up setsockopt calls, David Miller, (Mon Nov 17, 2:57 am)
[PATCH 3/4] dccp: Query supported CCIDs, Gerrit Renker, (Thu Nov 6, 1:40 am)
v2 [PATCH 3/4] dccp: Query supported CCIDs, Gerrit Renker, (Wed Nov 12, 2:37 am)
Re: v2 [PATCH 3/4] dccp: Query supported CCIDs, David Miller, (Wed Nov 12, 4:49 am)
Re: [PATCH 3/4] dccp: Query supported CCIDs, David Miller, (Mon Nov 10, 5:16 pm)
Re: [PATCH 2/5] dccp: Implement lookup table for feature-neg..., Arnaldo Carvalho de Melo, (Mon Sep 22, 1:00 pm)
Re: [PATCH 2/5] dccp: Implement lookup table for feature-neg..., Arnaldo Carvalho de Melo, (Wed Sep 24, 9:58 am)
Re: [PATCH 2/5] dccp: Implement lookup table for feature-neg..., Arnaldo Carvalho de Melo, (Mon Sep 22, 12:49 pm)
Re: What to do with DCCP, David Miller, (Thu Sep 11, 1:53 am)
Re: What to do with DCCP, Gerrit Renker, (Fri Sep 12, 1:16 am)
Re: net-next-2.6 [pull-request] [PATCH 0/37] dccp: Revised s..., Arnaldo Carvalho de Melo, (Thu Sep 11, 10:02 am)
Re: [PATCH 04/37] dccp: Per-socket initialisation of feature..., Arnaldo Carvalho de Melo, (Thu Aug 28, 3:53 pm)
Re: [PATCH 06/37] dccp: Limit feature negotiation to connect..., Arnaldo Carvalho de Melo, (Thu Aug 28, 4:50 pm)
Re: [PATCH 07/37] dccp: Registration routines for changing f..., Arnaldo Carvalho de Melo, (Thu Aug 28, 4:54 pm)
[PATCH 08/37] dccp: Query supported CCIDs, Gerrit Renker, (Thu Aug 28, 1:44 pm)
Re: [PATCH 08/37] dccp: Query supported CCIDs, Arnaldo Carvalho de Melo, (Thu Aug 28, 5:00 pm)
Re: [PATCH 08/37] dccp: Query supported CCIDs, Gerrit Renker, (Sat Aug 30, 9:52 am)
Re: [PATCH 08/37] dccp: Query supported CCIDs, Gerrit Renker, (Fri Aug 29, 2:17 am)
Re: [PATCH 09/37] dccp: Resolve dependencies of features on ..., Arnaldo Carvalho de Melo, (Thu Aug 28, 5:07 pm)
Re: [PATCH 09/37] dccp: Resolve dependencies of features on ..., Arnaldo Carvalho de Melo, (Wed Sep 3, 8:59 pm)
[PATCH 11/37] dccp: Deprecate old setsockopt framework, Gerrit Renker, (Thu Aug 28, 1:44 pm)
Re: [PATCH 12/37] dccp: Feature negotiation for minimum-chec..., Arnaldo Carvalho de Melo, (Thu Aug 28, 5:25 pm)
[PATCH 13/37] dccp: Deprecate Ack Ratio sysctl, Gerrit Renker, (Thu Aug 28, 1:44 pm)
Re: [PATCH 13/37] dccp: Deprecate Ack Ratio sysctl, Arnaldo Carvalho de Melo, (Thu Aug 28, 5:26 pm)
[PATCH 14/37] dccp: Tidy up setsockopt calls, Gerrit Renker, (Thu Aug 28, 1:44 pm)
Re: [PATCH 14/37] dccp: Tidy up setsockopt calls, Eugene Teo, (Fri Aug 29, 5:25 am)
Re: [PATCH 14/37] dccp: Tidy up setsockopt calls, Gerrit Renker, (Sat Aug 30, 9:52 am)
Re: [PATCH 14/37] dccp: Tidy up setsockopt calls, Arnaldo Carvalho de Melo, (Thu Aug 28, 5:35 pm)
Re: [PATCH 14/37] dccp: Tidy up setsockopt calls, Gerrit Renker, (Fri Aug 29, 2:57 am)
Re: [PATCH 15/37] dccp: Set per-connection CCIDs via socket ..., Arnaldo Carvalho de Melo, (Thu Aug 28, 5:45 pm)
[PATCH 16/37] dccp: API to query the current TX/RX CCID, Gerrit Renker, (Thu Aug 28, 1:44 pm)
Re: [PATCH 16/37] dccp: API to query the current TX/RX CCID, Arnaldo Carvalho de Melo, (Thu Aug 28, 5:47 pm)
Re: [PATCH 17/37] dccp: Increase the scope of variable-lengt..., Arnaldo Carvalho de Melo, (Thu Aug 28, 5:48 pm)
[PATCH 18/37] dccp: Support for Mandatory options, Gerrit Renker, (Thu Aug 28, 1:44 pm)
Re: [PATCH 18/37] dccp: Support for Mandatory options, Arnaldo Carvalho de Melo, (Thu Aug 28, 5:50 pm)
[PATCH 22/37] dccp: Preference list reconciliation, Gerrit Renker, (Thu Aug 28, 1:44 pm)
[PATCH 24/37] dccp: Processing Confirm options, Gerrit Renker, (Thu Aug 28, 1:44 pm)
[PATCH 25/37] dccp: Feature activation handlers, Gerrit Renker, (Thu Aug 28, 1:45 pm)
Re: [PATCH 25/37] dccp: Feature activation handlers, Wei Yongjun, (Tue Sep 2, 2:34 am)
Re: [PATCH 25/37] dccp: Feature activation handlers, Gerrit Renker, (Wed Sep 3, 12:38 am)
Re: [PATCH 25/37] dccp: Feature activation handlers, Wei Yongjun, (Wed Sep 3, 1:42 am)
Re: [PATCH 25/37] dccp: Feature activation handlers, Gerrit Renker, (Thu Sep 4, 1:12 am)