Commit 6fdd34d43bff8be9bb925b49d87a0ee144d2ab07

Authored by Gerrit Renker
Committed by David S. Miller
1 parent 4098dce5be

dccp ccid-2: Phase out the use of boolean Ack Vector sysctl

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>
Signed-off-by: David S. Miller <davem@davemloft.net>

Showing 9 changed files with 8 additions and 33 deletions Side-by-side Diff

Documentation/networking/dccp.txt
... ... @@ -133,9 +133,6 @@
133 133 importance for retransmitted acknowledgments and feature negotiation,
134 134 data packets are never retransmitted. Analogue of tcp_retries2.
135 135  
136   -send_ackvec = 1
137   - Whether or not to send Ack Vector options (sec. 11.5).
138   -
139 136 tx_ccid = 2
140 137 Default CCID for the sender-receiver half-connection. Depending on the
141 138 choice of CCID, the Send Ack Vector feature is enabled automatically.
include/linux/dccp.h
... ... @@ -360,7 +360,6 @@
360 360 #define DCCPF_INITIAL_SEQUENCE_WINDOW 100
361 361 #define DCCPF_INITIAL_ACK_RATIO 2
362 362 #define DCCPF_INITIAL_CCID DCCPC_CCID2
363   -#define DCCPF_INITIAL_SEND_ACK_VECTOR 1
364 363 /* FIXME: for now we're default to 1 but it should really be 0 */
365 364 #define DCCPF_INITIAL_SEND_NDP_COUNT 1
366 365  
367 366  
... ... @@ -370,13 +369,11 @@
370 369 * Will be used to pass the state from dccp_request_sock to dccp_sock.
371 370 *
372 371 * @dccpms_sequence_window - Sequence Window Feature (section 7.5.2)
373   - * @dccpms_send_ack_vector - Send Ack Vector Feature (section 11.5)
374 372 * @dccpms_pending - List of features being negotiated
375 373 * @dccpms_conf -
376 374 */
377 375 struct dccp_minisock {
378 376 __u64 dccpms_sequence_window;
379   - __u8 dccpms_send_ack_vector;
380 377 struct list_head dccpms_pending;
381 378 struct list_head dccpms_conf;
382 379 };
... ... @@ -98,7 +98,6 @@
98 98 extern int sysctl_dccp_feat_sequence_window;
99 99 extern int sysctl_dccp_feat_rx_ccid;
100 100 extern int sysctl_dccp_feat_tx_ccid;
101   -extern int sysctl_dccp_feat_send_ack_vector;
102 101 extern int sysctl_dccp_tx_qlen;
103 102 extern int sysctl_dccp_sync_ratelimit;
104 103  
... ... @@ -434,7 +433,7 @@
434 433 const struct dccp_sock *dp = dccp_sk(sk);
435 434 return dp->dccps_timestamp_echo != 0 ||
436 435 #ifdef CONFIG_IP_DCCP_ACKVEC
437   - (dccp_msk(sk)->dccpms_send_ack_vector &&
  436 + (dp->dccps_hc_rx_ackvec != NULL &&
438 437 dccp_ackvec_pending(dp->dccps_hc_rx_ackvec)) ||
439 438 #endif
440 439 inet_csk_ack_scheduled(sk);
... ... @@ -29,7 +29,7 @@
29 29 info->tcpi_backoff = icsk->icsk_backoff;
30 30 info->tcpi_pmtu = icsk->icsk_pmtu_cookie;
31 31  
32   - if (dccp_msk(sk)->dccpms_send_ack_vector)
  32 + if (dp->dccps_hc_rx_ackvec != NULL)
33 33 info->tcpi_options |= TCPI_OPT_SACK;
34 34  
35 35 ccid_hc_rx_get_info(dp->dccps_hc_rx_ccid, sk, info);
... ... @@ -163,7 +163,7 @@
163 163 {
164 164 struct dccp_sock *dp = dccp_sk(sk);
165 165  
166   - if (dccp_msk(sk)->dccpms_send_ack_vector)
  166 + if (dp->dccps_hc_rx_ackvec != NULL)
167 167 dccp_ackvec_check_rcv_ackno(dp->dccps_hc_rx_ackvec, sk,
168 168 DCCP_SKB_CB(skb)->dccpd_ack_seq);
169 169 }
... ... @@ -375,7 +375,7 @@
375 375 if (DCCP_SKB_CB(skb)->dccpd_ack_seq != DCCP_PKT_WITHOUT_ACK_SEQ)
376 376 dccp_event_ack_recv(sk, skb);
377 377  
378   - if (dccp_msk(sk)->dccpms_send_ack_vector &&
  378 + if (dp->dccps_hc_rx_ackvec != NULL &&
379 379 dccp_ackvec_add(dp->dccps_hc_rx_ackvec, sk,
380 380 DCCP_SKB_CB(skb)->dccpd_seq,
381 381 DCCP_ACKVEC_STATE_RECEIVED))
... ... @@ -434,12 +434,6 @@
434 434 dp->dccps_syn_rtt = dccp_sample_rtt(sk, 10 * (tstamp -
435 435 dp->dccps_options_received.dccpor_timestamp_echo));
436 436  
437   - if (dccp_msk(sk)->dccpms_send_ack_vector &&
438   - dccp_ackvec_add(dp->dccps_hc_rx_ackvec, sk,
439   - DCCP_SKB_CB(skb)->dccpd_seq,
440   - DCCP_ACKVEC_STATE_RECEIVED))
441   - goto out_invalid_packet; /* FIXME: change error code */
442   -
443 437 /* Stop the REQUEST timer */
444 438 inet_csk_clear_xmit_timer(sk, ICSK_TIME_RETRANS);
445 439 WARN_ON(sk->sk_send_head == NULL);
... ... @@ -637,7 +631,7 @@
637 631 if (dcb->dccpd_ack_seq != DCCP_PKT_WITHOUT_ACK_SEQ)
638 632 dccp_event_ack_recv(sk, skb);
639 633  
640   - if (dccp_msk(sk)->dccpms_send_ack_vector &&
  634 + if (dp->dccps_hc_rx_ackvec != NULL &&
641 635 dccp_ackvec_add(dp->dccps_hc_rx_ackvec, sk,
642 636 DCCP_SKB_CB(skb)->dccpd_seq,
643 637 DCCP_ACKVEC_STATE_RECEIVED))
net/dccp/minisocks.c
... ... @@ -45,7 +45,6 @@
45 45 void dccp_minisock_init(struct dccp_minisock *dmsk)
46 46 {
47 47 dmsk->dccpms_sequence_window = sysctl_dccp_feat_sequence_window;
48   - dmsk->dccpms_send_ack_vector = sysctl_dccp_feat_send_ack_vector;
49 48 }
50 49  
51 50 void dccp_time_wait(struct sock *sk, int state, int timeo)
... ... @@ -26,7 +26,6 @@
26 26 int sysctl_dccp_feat_sequence_window = DCCPF_INITIAL_SEQUENCE_WINDOW;
27 27 int sysctl_dccp_feat_rx_ccid = DCCPF_INITIAL_CCID;
28 28 int sysctl_dccp_feat_tx_ccid = DCCPF_INITIAL_CCID;
29   -int sysctl_dccp_feat_send_ack_vector = DCCPF_INITIAL_SEND_ACK_VECTOR;
30 29  
31 30 u64 dccp_decode_value_var(const u8 *bf, const u8 len)
32 31 {
... ... @@ -145,8 +144,7 @@
145 144 case DCCPO_ACK_VECTOR_1:
146 145 if (dccp_packet_without_ack(skb)) /* RFC 4340, 11.4 */
147 146 break;
148   -
149   - if (dccp_msk(sk)->dccpms_send_ack_vector &&
  147 + if (dp->dccps_hc_rx_ackvec != NULL &&
150 148 dccp_ackvec_parse(sk, skb, &ackno, opt, value, len))
151 149 goto out_invalid_option;
152 150 break;
... ... @@ -526,7 +524,6 @@
526 524 int dccp_insert_options(struct sock *sk, struct sk_buff *skb)
527 525 {
528 526 struct dccp_sock *dp = dccp_sk(sk);
529   - struct dccp_minisock *dmsk = dccp_msk(sk);
530 527  
531 528 DCCP_SKB_CB(skb)->dccpd_opt_len = 0;
532 529  
... ... @@ -547,7 +544,7 @@
547 544 if (dccp_insert_option_timestamp(sk, skb))
548 545 return -1;
549 546  
550   - } else if (dmsk->dccpms_send_ack_vector &&
  547 + } else if (dp->dccps_hc_rx_ackvec != NULL &&
551 548 dccp_ackvec_pending(dp->dccps_hc_rx_ackvec) &&
552 549 dccp_insert_option_ackvec(sk, skb)) {
553 550 return -1;
... ... @@ -201,7 +201,6 @@
201 201 void dccp_destroy_sock(struct sock *sk)
202 202 {
203 203 struct dccp_sock *dp = dccp_sk(sk);
204   - struct dccp_minisock *dmsk = dccp_msk(sk);
205 204  
206 205 /*
207 206 * DCCP doesn't use sk_write_queue, just sk_send_head
... ... @@ -219,7 +218,7 @@
219 218 kfree(dp->dccps_service_list);
220 219 dp->dccps_service_list = NULL;
221 220  
222   - if (dmsk->dccpms_send_ack_vector) {
  221 + if (dp->dccps_hc_rx_ackvec != NULL) {
223 222 dccp_ackvec_free(dp->dccps_hc_rx_ackvec);
224 223 dp->dccps_hc_rx_ackvec = NULL;
225 224 }
... ... @@ -41,13 +41,6 @@
41 41 .proc_handler = proc_dointvec,
42 42 },
43 43 {
44   - .procname = "send_ackvec",
45   - .data = &sysctl_dccp_feat_send_ack_vector,
46   - .maxlen = sizeof(sysctl_dccp_feat_send_ack_vector),
47   - .mode = 0644,
48   - .proc_handler = proc_dointvec,
49   - },
50   - {
51 44 .procname = "request_retries",
52 45 .data = &sysctl_dccp_request_retries,
53 46 .maxlen = sizeof(sysctl_dccp_request_retries),