From b24b8a247ff65c01b252025926fe564209fae4fc Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Wed, 23 Jan 2008 21:20:07 -0800 Subject: [NET]: Convert init_timer into setup_timer Many-many code in the kernel initialized the timer->function and timer->data together with calling init_timer(timer). There is already a helper for this. Use it for networking code. The patch is HUGE, but makes the code 130 lines shorter (98 insertions(+), 228 deletions(-)). Signed-off-by: Pavel Emelyanov Acked-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index d694656b880..c9c465e8628 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -760,10 +760,8 @@ static int ccid2_hc_tx_init(struct ccid *ccid, struct sock *sk) hctx->ccid2hctx_rttvar = -1; hctx->ccid2hctx_rpdupack = -1; hctx->ccid2hctx_last_cong = jiffies; - - hctx->ccid2hctx_rtotimer.function = &ccid2_hc_tx_rto_expire; - hctx->ccid2hctx_rtotimer.data = (unsigned long)sk; - init_timer(&hctx->ccid2hctx_rtotimer); + setup_timer(&hctx->ccid2hctx_rtotimer, ccid2_hc_tx_rto_expire, + (unsigned long)sk); ccid2_hc_tx_check_sanity(hctx); return 0; -- cgit v1.2.3-70-g09d2 From 3de5489f47febe0333b142e0eb6389b9924b2634 Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 20:37:48 -0200 Subject: [CCID2]: Bug in reading Ack Vectors In CCID2 the receiver-history is sorted in ascending order of sequence number, but the processing of received Ack Vectors requires the list traversal in the opposite direction. The current code has a bug in this regard: the list traversal is upwards. As a consequence, only Ack Vectors with a run length of 1 will pass, in all other Ack Vectors the remaining (acked) sequence numbers are missed, and may later falsely be identified as lost. Note: This bug is only visible when Ack Ratio > 1, since otherwise the run lengths of Ack Vectors are 0. Signed-off-by: Gerrit Renker Signed-off-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index c9c465e8628..7873dc78b6b 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -666,7 +666,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) done = 1; break; } - seqp = seqp->ccid2s_next; + seqp = seqp->ccid2s_prev; } if (done) break; -- cgit v1.2.3-70-g09d2 From cfbbeabc8864902c4af1c0cadf0972b352930a26 Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 20:43:59 -0200 Subject: [CCID2]: Fix sequence number arithmetic/comparisons This replaces use of normal subtraction with modulo-48 subtraction. Signed-off-by: Gerrit Renker Acked-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index 7873dc78b6b..55522182f2f 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -24,9 +24,6 @@ /* * This implementation should follow RFC 4341 - * - * BUGS: - * - sequence number wrapping */ #include "../ccid.h" @@ -619,9 +616,8 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) /* go through this ack vector */ while (veclen--) { const u8 rl = *vector & DCCP_ACKVEC_LEN_MASK; - u64 ackno_end_rl; + u64 ackno_end_rl = SUB48(ackno, rl); - dccp_set_seqno(&ackno_end_rl, ackno - rl); ccid2_pr_debug("ackvec start:%llu end:%llu\n", (unsigned long long)ackno, (unsigned long long)ackno_end_rl); @@ -671,8 +667,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) if (done) break; - - dccp_set_seqno(&ackno, ackno_end_rl - 1); + ackno = SUB48(ackno_end_rl, 1); vector++; } if (done) -- cgit v1.2.3-70-g09d2 From df054e1d00fdafa2e2920319df326ddb3f0d0413 Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 21:32:53 -0200 Subject: [CCID2]: Don't assign negative values to Ack Ratio Since it makes not sense to assign negative values to Ack Ratio, this patch disallows this possibility. As a consequence, a Bug test for negative Ack Ratio values becomes obsolete. Furthermore, a check against overflow (as Ack Ratio may not exceed 2 bytes, due to RFC 4340, 11.3) has been added. Signed-off-by: Gerrit Renker Acked-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index 55522182f2f..f18235e8ce8 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -140,7 +140,7 @@ static int ccid2_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) return 1; /* XXX CCID should dequeue when ready instead of polling */ } -static void ccid2_change_l_ack_ratio(struct sock *sk, int val) +static void ccid2_change_l_ack_ratio(struct sock *sk, u32 val) { struct dccp_sock *dp = dccp_sk(sk); /* @@ -159,9 +159,10 @@ static void ccid2_change_l_ack_ratio(struct sock *sk, int val) if (val > max) val = max; } + if (val > 0xFFFF) /* RFC 4340, 11.3 */ + val = 0xFFFF; - ccid2_pr_debug("changing local ack ratio to %d\n", val); - WARN_ON(val <= 0); + ccid2_pr_debug("changing local ack ratio to %u\n", val); dp->dccps_l_ack_ratio = val; } @@ -572,7 +573,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) hctx->ccid2hctx_rpdupack = -1; /* XXX lame */ hctx->ccid2hctx_rpseq = 0; - ccid2_change_l_ack_ratio(sk, dp->dccps_l_ack_ratio << 1); + ccid2_change_l_ack_ratio(sk, 2 * dp->dccps_l_ack_ratio); } } } -- cgit v1.2.3-70-g09d2 From d50ad163e6db2dcc365b8d02b30350220f86df04 Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 21:40:24 -0200 Subject: [CCID2]: Deadlock and spurious timeouts when Ack Ratio > cwnd This patch removes a bug in the current code. I agree with Andrea's comment that there is a problem here but the way it is treated does not fix it. The problem is that whenever Ack Ratio > cwnd, starvation/deadlock occurs: * the receiver will not send an Ack until (Ack Ratio - cwnd) data packets have arrived; * the sender will not send any data packet before the receipt of an Ack advances the send window. The only way that the connection then progresses was via RTO timeout. In one extreme case (bulk transfer), it was observed that this happened for every single packet; i.e. hundreds of packets, each a RTO timeout of 1..3 seconds apart: a transfer which normally would take a fraction of a second thus grew to several minutes. The solution taken by this approach is to observe the relation "Ack Ratio <= cwnd" by using the constraint (1) from RFC 4341, 6.1.2; i.e. set Ack Ratio = ceil(cwnd / 2) and update it whenever either Ack Ratio or cwnd change. This ensures that the deadlock problem can not arise. Signed-off-by: Gerrit Renker Acked-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index f18235e8ce8..ef19fb83429 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -143,32 +143,30 @@ static int ccid2_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) static void ccid2_change_l_ack_ratio(struct sock *sk, u32 val) { struct dccp_sock *dp = dccp_sk(sk); + u32 max_ratio = DIV_ROUND_UP(ccid2_hc_tx_sk(sk)->ccid2hctx_cwnd, 2); + /* - * XXX I don't really agree with val != 2. If cwnd is 1, ack ratio - * should be 1... it shouldn't be allowed to become 2. - * -sorbo. + * Ensure that Ack Ratio does not exceed ceil(cwnd/2), which is (2) from + * RFC 4341, 6.1.2. We ignore the statement that Ack Ratio 2 is always + * acceptable since this causes starvation/deadlock whenever cwnd < 2. + * The same problem arises when Ack Ratio is 0 (ie. Ack Ratio disabled). */ - if (val != 2) { - const struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); - int max = hctx->ccid2hctx_cwnd / 2; - - /* round up */ - if (hctx->ccid2hctx_cwnd & 1) - max++; - - if (val > max) - val = max; + if (val == 0 || val > max_ratio) { + DCCP_WARN("Limiting Ack Ratio (%u) to %u\n", val, max_ratio); + val = max_ratio; } if (val > 0xFFFF) /* RFC 4340, 11.3 */ val = 0xFFFF; + if (val == dp->dccps_l_ack_ratio) + return; + ccid2_pr_debug("changing local ack ratio to %u\n", val); dp->dccps_l_ack_ratio = val; } static void ccid2_change_cwnd(struct ccid2_hc_tx_sock *hctx, u32 val) { - /* XXX do we need to change ack ratio? */ hctx->ccid2hctx_cwnd = val? : 1; ccid2_pr_debug("changed cwnd to %u\n", hctx->ccid2hctx_cwnd); } @@ -519,9 +517,10 @@ static void ccid2_hc_tx_dec_pipe(struct sock *sk) ccid2_hc_tx_kill_rto_timer(sk); } -static void ccid2_congestion_event(struct ccid2_hc_tx_sock *hctx, - struct ccid2_seq *seqp) +static void ccid2_congestion_event(struct sock *sk, struct ccid2_seq *seqp) { + struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); + if (time_before(seqp->ccid2s_sent, hctx->ccid2hctx_last_cong)) { ccid2_pr_debug("Multiple losses in an RTT---treating as one\n"); return; @@ -533,6 +532,10 @@ static void ccid2_congestion_event(struct ccid2_hc_tx_sock *hctx, hctx->ccid2hctx_ssthresh = hctx->ccid2hctx_cwnd; if (hctx->ccid2hctx_ssthresh < 2) hctx->ccid2hctx_ssthresh = 2; + + /* Avoid spurious timeouts resulting from Ack Ratio > cwnd */ + if (dccp_sk(sk)->dccps_l_ack_ratio > hctx->ccid2hctx_cwnd) + ccid2_change_l_ack_ratio(sk, hctx->ccid2hctx_cwnd); } static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) @@ -648,7 +651,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) !seqp->ccid2s_acked) { if (state == DCCP_ACKVEC_STATE_ECN_MARKED) { - ccid2_congestion_event(hctx, + ccid2_congestion_event(sk, seqp); } else ccid2_new_ack(sk, seqp, @@ -713,7 +716,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) * order to detect multiple congestion events in * one ack vector. */ - ccid2_congestion_event(hctx, seqp); + ccid2_congestion_event(sk, seqp); ccid2_hc_tx_dec_pipe(sk); } if (seqp == hctx->ccid2hctx_seqt) -- cgit v1.2.3-70-g09d2 From b00d2bbc45a287c9a72374582ce42205f3412419 Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 21:44:30 -0200 Subject: [CCID2]: Larger initial windows also for CCID2 RFC 4341, sec. 5 states that "The cwnd parameter is initialized to at most four packets for new connections, following the rules from [RFC3390]", which is implemented by this patch. Signed-off-by: Gerrit Renker Acked-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index ef19fb83429..9c5b6c73f7c 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -741,15 +741,25 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) static int ccid2_hc_tx_init(struct ccid *ccid, struct sock *sk) { struct ccid2_hc_tx_sock *hctx = ccid_priv(ccid); + struct dccp_sock *dp = dccp_sk(sk); + u32 max_ratio; - ccid2_change_cwnd(hctx, 1); - /* Initialize ssthresh to infinity. This means that we will exit the - * initial slow-start after the first packet loss. This is what we - * want. - */ + /* RFC 4341, 5: initialise ssthresh to arbitrarily high (max) value */ hctx->ccid2hctx_ssthresh = ~0; hctx->ccid2hctx_numdupack = 3; + /* + * RFC 4341, 5: "The cwnd parameter is initialized to at most four + * packets for new connections, following the rules from [RFC3390]". + * We need to convert the bytes of RFC3390 into the packets of RFC 4341. + */ + hctx->ccid2hctx_cwnd = min(4U, max(2U, 4380U / dp->dccps_mss_cache)); + + /* Make sure that Ack Ratio is enabled and within bounds. */ + max_ratio = DIV_ROUND_UP(hctx->ccid2hctx_cwnd, 2); + if (dp->dccps_l_ack_ratio == 0 || dp->dccps_l_ack_ratio > max_ratio) + dp->dccps_l_ack_ratio = max_ratio; + /* XXX init ~ to window size... */ if (ccid2_hc_tx_alloc_seq(hctx)) return -ENOMEM; -- cgit v1.2.3-70-g09d2 From 900bfed4718126e6c32244903b6f43e0990d04ad Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 21:58:33 -0200 Subject: [CCID2]: Disable broken Ack Ratio adaptation algorithm This comments out a problematic section comprising a half-finished algorithm: - The variable `ccid2hctx_ackloss' is never initialised to a value different from 0 and hence in fact is a read-only constant. - The `arsent' variable counts packets other than Acks (it is incremented for every packet), and there is no test for Ack Loss. - The concept of counting Acks as such leads to a complex calculation, and the calculation at the moment is inconsistent with this concept. The problem is that the number of Acks - rather than the number of windows - is counted, which leads to a complex (cubic/quadratic) expression - this is not even implemented. In its current state, the commented-out algorithm interfers with normal processing by changing Ack Ratio incorrectly, and at the wrong times. A new algorithm is necessary, which will not necessarily use the same variables as used by the unfinished one; hence the old variables have been removed. Signed-off-by: Gerrit Renker Acked-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 23 +++++++++++++++++++++-- net/dccp/ccids/ccid2.h | 4 ---- 2 files changed, 21 insertions(+), 6 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index 9c5b6c73f7c..a901202ada6 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -224,8 +224,6 @@ static void ccid2_hc_tx_rto_expire(unsigned long data) hctx->ccid2hctx_sent = 0; /* clear ack ratio state. */ - hctx->ccid2hctx_arsent = 0; - hctx->ccid2hctx_ackloss = 0; hctx->ccid2hctx_rpseq = 0; hctx->ccid2hctx_rpdupack = -1; ccid2_change_l_ack_ratio(sk, 1); @@ -289,6 +287,26 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) hctx->ccid2hctx_sent++; + /* + * FIXME: The code below is broken and the variables have been removed + * from the socket struct. The `ackloss' variable was always set to 0, + * and with arsent there are several problems: + * (i) it doesn't just count the number of Acks, but all sent packets; + * (ii) it is expressed in # of packets, not # of windows, so the + * comparison below uses the wrong formula: Appendix A of RFC 4341 + * comes up with the number K = cwnd / (R^2 - R) of consecutive windows + * of data with no lost or marked Ack packets. If arsent were the # of + * consecutive Acks received without loss, then Ack Ratio needs to be + * decreased by 1 when + * arsent >= K * cwnd / R = cwnd^2 / (R^3 - R^2) + * where cwnd / R is the number of Acks received per window of data + * (cf. RFC 4341, App. A). The problems are that + * - arsent counts other packets as well; + * - the comparison uses a formula different from RFC 4341; + * - computing a cubic/quadratic equation each time is too complicated. + * Hence a different algorithm is needed. + */ +#if 0 /* Ack Ratio. Need to maintain a concept of how many windows we sent */ hctx->ccid2hctx_arsent++; /* We had an ack loss in this window... */ @@ -316,6 +334,7 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) hctx->ccid2hctx_arsent = 0; /* or maybe set it to cwnd*/ } } +#endif /* setup RTO timer */ if (!timer_pending(&hctx->ccid2hctx_rtotimer)) diff --git a/net/dccp/ccids/ccid2.h b/net/dccp/ccids/ccid2.h index d9daa534c9b..ed95f8f0cc4 100644 --- a/net/dccp/ccids/ccid2.h +++ b/net/dccp/ccids/ccid2.h @@ -44,8 +44,6 @@ struct ccid2_seq { * @ccid2hctx_acks - ACKS recv in AI phase * @ccid2hctx_sent - packets sent in this window * @ccid2hctx_lastrtt -time RTT was last measured - * @ccid2hctx_arsent - packets sent [ack ratio] - * @ccid2hctx_ackloss - ack was lost in this win * @ccid2hctx_rpseq - last consecutive seqno * @ccid2hctx_rpdupack - dupacks since rpseq */ @@ -66,8 +64,6 @@ struct ccid2_hc_tx_sock { int ccid2hctx_sent; unsigned long ccid2hctx_lastrtt; struct timer_list ccid2hctx_rtotimer; - unsigned long ccid2hctx_arsent; - int ccid2hctx_ackloss; u64 ccid2hctx_rpseq; int ccid2hctx_rpdupack; int ccid2hctx_sendwait; -- cgit v1.2.3-70-g09d2 From 7792cd8885954eb7ac38e781a7a9faae5a80a3d8 Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 22:01:56 -0200 Subject: [CCID2]: Remove unused variable This removes a variable `ccid2hctx_sent' which is incremented but never referenced/read (i.e., dead code). Signed-off-by: Gerrit Renker Signed-off-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 4 ---- net/dccp/ccids/ccid2.h | 2 -- 2 files changed, 6 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index a901202ada6..9fa0eb5f691 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -221,7 +221,6 @@ static void ccid2_hc_tx_rto_expire(unsigned long data) hctx->ccid2hctx_seqt = hctx->ccid2hctx_seqh; hctx->ccid2hctx_ssacks = 0; hctx->ccid2hctx_acks = 0; - hctx->ccid2hctx_sent = 0; /* clear ack ratio state. */ hctx->ccid2hctx_rpseq = 0; @@ -285,8 +284,6 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) ccid2_pr_debug("cwnd=%d pipe=%d\n", hctx->ccid2hctx_cwnd, hctx->ccid2hctx_pipe); - hctx->ccid2hctx_sent++; - /* * FIXME: The code below is broken and the variables have been removed * from the socket struct. The `ackloss' variable was always set to 0, @@ -517,7 +514,6 @@ static inline void ccid2_new_ack(struct sock *sk, ccid2_pr_debug("srtt: %ld rttvar: %ld rto: %ld (HZ=%d) R=%lu\n", hctx->ccid2hctx_srtt, hctx->ccid2hctx_rttvar, hctx->ccid2hctx_rto, HZ, r); - hctx->ccid2hctx_sent = 0; } /* we got a new ack, so re-start RTO timer */ diff --git a/net/dccp/ccids/ccid2.h b/net/dccp/ccids/ccid2.h index ed95f8f0cc4..6d0b4e32b16 100644 --- a/net/dccp/ccids/ccid2.h +++ b/net/dccp/ccids/ccid2.h @@ -42,7 +42,6 @@ struct ccid2_seq { * * @ccid2hctx_ssacks - ACKs recv in slow start * @ccid2hctx_acks - ACKS recv in AI phase - * @ccid2hctx_sent - packets sent in this window * @ccid2hctx_lastrtt -time RTT was last measured * @ccid2hctx_rpseq - last consecutive seqno * @ccid2hctx_rpdupack - dupacks since rpseq @@ -61,7 +60,6 @@ struct ccid2_hc_tx_sock { long ccid2hctx_rto; long ccid2hctx_srtt; long ccid2hctx_rttvar; - int ccid2hctx_sent; unsigned long ccid2hctx_lastrtt; struct timer_list ccid2hctx_rtotimer; u64 ccid2hctx_rpseq; -- cgit v1.2.3-70-g09d2 From 63df18ad7fb91c65dafc89d3cf94a58a486ad416 Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 22:04:35 -0200 Subject: [CCID2]: Replace read-only variable with constant This replaces the field member `numdupack', which was used as a read-only constant in the code, with a #define. Signed-off-by: Gerrit Renker Signed-off-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 8 +++----- net/dccp/ccids/ccid2.h | 3 ++- 2 files changed, 5 insertions(+), 6 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index 9fa0eb5f691..a3c42cd00b0 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -586,8 +586,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) hctx->ccid2hctx_rpdupack++; /* check if we got enough dupacks */ - if (hctx->ccid2hctx_rpdupack >= - hctx->ccid2hctx_numdupack) { + if (hctx->ccid2hctx_rpdupack >= NUMDUPACK) { hctx->ccid2hctx_rpdupack = -1; /* XXX lame */ hctx->ccid2hctx_rpseq = 0; @@ -708,7 +707,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) while (1) { if (seqp->ccid2s_acked) { done++; - if (done == hctx->ccid2hctx_numdupack) + if (done == NUMDUPACK) break; } if (seqp == hctx->ccid2hctx_seqt) @@ -719,7 +718,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) /* If there are at least 3 acknowledgements, anything unacknowledged * below the last sequence number is considered lost */ - if (done == hctx->ccid2hctx_numdupack) { + if (done == NUMDUPACK) { struct ccid2_seq *last_acked = seqp; /* check for lost packets */ @@ -761,7 +760,6 @@ static int ccid2_hc_tx_init(struct ccid *ccid, struct sock *sk) /* RFC 4341, 5: initialise ssthresh to arbitrarily high (max) value */ hctx->ccid2hctx_ssthresh = ~0; - hctx->ccid2hctx_numdupack = 3; /* * RFC 4341, 5: "The cwnd parameter is initialized to at most four diff --git a/net/dccp/ccids/ccid2.h b/net/dccp/ccids/ccid2.h index 6d0b4e32b16..443f08a667a 100644 --- a/net/dccp/ccids/ccid2.h +++ b/net/dccp/ccids/ccid2.h @@ -24,6 +24,8 @@ #include #include #include "../ccid.h" +/* NUMDUPACK parameter from RFC 4341, p. 6 */ +#define NUMDUPACK 3 struct sock; @@ -52,7 +54,6 @@ struct ccid2_hc_tx_sock { int ccid2hctx_acks; unsigned int ccid2hctx_ssthresh; int ccid2hctx_pipe; - int ccid2hctx_numdupack; struct ccid2_seq *ccid2hctx_seqbuf[CCID2_SEQBUF_MAX]; int ccid2hctx_seqbufc; struct ccid2_seq *ccid2hctx_seqh; -- cgit v1.2.3-70-g09d2 From 3deeadd74bbf916b502d307222833ffcf68db557 Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 22:05:51 -0200 Subject: [CCID2]: Replace cwnd assignment-function with assignment The current function ccid2_change_cwnd in effect makes only an assignment, as the test whether cwnd has reached 0 is only required when cwnd is halved. This patch simplifies the code by replacing the function with the assignment it performs. Furthermore, since ssthresh derives from cwnd and appears in many assignments and comparisons, the type of ssthresh has also been changed to match that of cwnd. Signed-off-by: Gerrit Renker Signed-off-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 22 +++++++--------------- net/dccp/ccids/ccid2.h | 2 +- 2 files changed, 8 insertions(+), 16 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index a3c42cd00b0..747fa1c4e42 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -165,12 +165,6 @@ static void ccid2_change_l_ack_ratio(struct sock *sk, u32 val) dp->dccps_l_ack_ratio = val; } -static void ccid2_change_cwnd(struct ccid2_hc_tx_sock *hctx, u32 val) -{ - hctx->ccid2hctx_cwnd = val? : 1; - ccid2_pr_debug("changed cwnd to %u\n", hctx->ccid2hctx_cwnd); -} - static void ccid2_change_srtt(struct ccid2_hc_tx_sock *hctx, long val) { ccid2_pr_debug("change SRTT to %ld\n", val); @@ -212,10 +206,10 @@ static void ccid2_hc_tx_rto_expire(unsigned long data) /* adjust pipe, cwnd etc */ ccid2_change_pipe(hctx, 0); - hctx->ccid2hctx_ssthresh = hctx->ccid2hctx_cwnd >> 1; + hctx->ccid2hctx_ssthresh = hctx->ccid2hctx_cwnd / 2; if (hctx->ccid2hctx_ssthresh < 2) hctx->ccid2hctx_ssthresh = 2; - ccid2_change_cwnd(hctx, 1); + hctx->ccid2hctx_cwnd = 1; /* clear state about stuff we sent */ hctx->ccid2hctx_seqt = hctx->ccid2hctx_seqh; @@ -440,7 +434,7 @@ static inline void ccid2_new_ack(struct sock *sk, /* increase every 2 acks */ hctx->ccid2hctx_ssacks++; if (hctx->ccid2hctx_ssacks == 2) { - ccid2_change_cwnd(hctx, hctx->ccid2hctx_cwnd+1); + hctx->ccid2hctx_cwnd++; hctx->ccid2hctx_ssacks = 0; *maxincr = *maxincr - 1; } @@ -453,7 +447,7 @@ static inline void ccid2_new_ack(struct sock *sk, hctx->ccid2hctx_acks++; if (hctx->ccid2hctx_acks >= hctx->ccid2hctx_cwnd) { - ccid2_change_cwnd(hctx, hctx->ccid2hctx_cwnd + 1); + hctx->ccid2hctx_cwnd++; hctx->ccid2hctx_acks = 0; } } @@ -543,10 +537,8 @@ static void ccid2_congestion_event(struct sock *sk, struct ccid2_seq *seqp) hctx->ccid2hctx_last_cong = jiffies; - ccid2_change_cwnd(hctx, hctx->ccid2hctx_cwnd >> 1); - hctx->ccid2hctx_ssthresh = hctx->ccid2hctx_cwnd; - if (hctx->ccid2hctx_ssthresh < 2) - hctx->ccid2hctx_ssthresh = 2; + hctx->ccid2hctx_cwnd = hctx->ccid2hctx_cwnd / 2 ? : 1U; + hctx->ccid2hctx_ssthresh = max(hctx->ccid2hctx_cwnd, 2U); /* Avoid spurious timeouts resulting from Ack Ratio > cwnd */ if (dccp_sk(sk)->dccps_l_ack_ratio > hctx->ccid2hctx_cwnd) @@ -759,7 +751,7 @@ static int ccid2_hc_tx_init(struct ccid *ccid, struct sock *sk) u32 max_ratio; /* RFC 4341, 5: initialise ssthresh to arbitrarily high (max) value */ - hctx->ccid2hctx_ssthresh = ~0; + hctx->ccid2hctx_ssthresh = ~0U; /* * RFC 4341, 5: "The cwnd parameter is initialized to at most four diff --git a/net/dccp/ccids/ccid2.h b/net/dccp/ccids/ccid2.h index 443f08a667a..b72e9556a15 100644 --- a/net/dccp/ccids/ccid2.h +++ b/net/dccp/ccids/ccid2.h @@ -50,9 +50,9 @@ struct ccid2_seq { */ struct ccid2_hc_tx_sock { u32 ccid2hctx_cwnd; + u32 ccid2hctx_ssthresh; int ccid2hctx_ssacks; int ccid2hctx_acks; - unsigned int ccid2hctx_ssthresh; int ccid2hctx_pipe; struct ccid2_seq *ccid2hctx_seqbuf[CCID2_SEQBUF_MAX]; int ccid2hctx_seqbufc; -- cgit v1.2.3-70-g09d2 From 95b21d7e9d099f1cffca08e40f292d6658a88b3c Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 22:06:52 -0200 Subject: [CCID2]: Replace pipe assignment-function with assignment The function ccid2_change_pipe only does an assignment. This patch simplifies the code by replacing the function with the assignment it performs. Furthermore, the type of pipe is promoted from `signed' to unsigned (increasing the range). As a result, a BUG_ON test for negative values now becomes obsolete (for safety not removed, but replaced with a less annoying `DCCP_BUG'). Signed-off-by: Gerrit Renker Signed-off-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 16 ++++++---------- net/dccp/ccids/ccid2.h | 3 ++- 2 files changed, 8 insertions(+), 11 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index 747fa1c4e42..237007bb55a 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -171,11 +171,6 @@ static void ccid2_change_srtt(struct ccid2_hc_tx_sock *hctx, long val) hctx->ccid2hctx_srtt = val; } -static void ccid2_change_pipe(struct ccid2_hc_tx_sock *hctx, long val) -{ - hctx->ccid2hctx_pipe = val; -} - static void ccid2_start_rto_timer(struct sock *sk); static void ccid2_hc_tx_rto_expire(unsigned long data) @@ -205,11 +200,11 @@ static void ccid2_hc_tx_rto_expire(unsigned long data) ccid2_start_rto_timer(sk); /* adjust pipe, cwnd etc */ - ccid2_change_pipe(hctx, 0); hctx->ccid2hctx_ssthresh = hctx->ccid2hctx_cwnd / 2; if (hctx->ccid2hctx_ssthresh < 2) hctx->ccid2hctx_ssthresh = 2; hctx->ccid2hctx_cwnd = 1; + hctx->ccid2hctx_pipe = 0; /* clear state about stuff we sent */ hctx->ccid2hctx_seqt = hctx->ccid2hctx_seqh; @@ -248,8 +243,7 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) BUG_ON(!hctx->ccid2hctx_sendwait); hctx->ccid2hctx_sendwait = 0; - ccid2_change_pipe(hctx, hctx->ccid2hctx_pipe + 1); - BUG_ON(hctx->ccid2hctx_pipe < 0); + hctx->ccid2hctx_pipe++; /* There is an issue. What if another packet is sent between * packet_send() and packet_sent(). Then the sequence number would be @@ -519,8 +513,10 @@ static void ccid2_hc_tx_dec_pipe(struct sock *sk) { struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); - ccid2_change_pipe(hctx, hctx->ccid2hctx_pipe-1); - BUG_ON(hctx->ccid2hctx_pipe < 0); + if (hctx->ccid2hctx_pipe == 0) + DCCP_BUG("pipe == 0"); + else + hctx->ccid2hctx_pipe--; if (hctx->ccid2hctx_pipe == 0) ccid2_hc_tx_kill_rto_timer(sk); diff --git a/net/dccp/ccids/ccid2.h b/net/dccp/ccids/ccid2.h index b72e9556a15..bc659f00cb9 100644 --- a/net/dccp/ccids/ccid2.h +++ b/net/dccp/ccids/ccid2.h @@ -42,6 +42,7 @@ struct ccid2_seq { /** struct ccid2_hc_tx_sock - CCID2 TX half connection * + * @ccid2hctx_{cwnd,ssthresh,pipe}: as per RFC 4341, section 5 * @ccid2hctx_ssacks - ACKs recv in slow start * @ccid2hctx_acks - ACKS recv in AI phase * @ccid2hctx_lastrtt -time RTT was last measured @@ -51,9 +52,9 @@ struct ccid2_seq { struct ccid2_hc_tx_sock { u32 ccid2hctx_cwnd; u32 ccid2hctx_ssthresh; + u32 ccid2hctx_pipe; int ccid2hctx_ssacks; int ccid2hctx_acks; - int ccid2hctx_pipe; struct ccid2_seq *ccid2hctx_seqbuf[CCID2_SEQBUF_MAX]; int ccid2hctx_seqbufc; struct ccid2_seq *ccid2hctx_seqh; -- cgit v1.2.3-70-g09d2 From da98e0b5d4c1f88b7c9e63e8918783cd4905be2b Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 22:08:27 -0200 Subject: [CCID2]: Redundant debugging output This reduces the amount of redundant debugging messages: * pipe/cwnd are printed in both tx_send_packet() and tx_packet_sent(). Both functions are called immediately after one another, so one occurrence is sufficient. * Since tx_packet_sent() prints pipe/cwnd already, the second printk for pipe is redundant. * In tx_packet_sent() the check_sanity function is called twice (at the begin and at the end). Signed-off-by: Gerrit Renker Acked-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index 237007bb55a..2d1b7e30a73 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -126,9 +126,6 @@ static int ccid2_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) { struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); - ccid2_pr_debug("pipe=%d cwnd=%d\n", hctx->ccid2hctx_pipe, - hctx->ccid2hctx_cwnd); - if (hctx->ccid2hctx_pipe < hctx->ccid2hctx_cwnd) { /* OK we can send... make sure previous packet was sent off */ if (!hctx->ccid2hctx_sendwait) { @@ -239,8 +236,6 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) struct ccid2_seq *next; u64 seq; - ccid2_hc_tx_check_sanity(hctx); - BUG_ON(!hctx->ccid2hctx_sendwait); hctx->ccid2hctx_sendwait = 0; hctx->ccid2hctx_pipe++; @@ -326,7 +321,6 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) ccid2_start_rto_timer(sk); #ifdef CONFIG_IP_DCCP_CCID2_DEBUG - ccid2_pr_debug("pipe=%d\n", hctx->ccid2hctx_pipe); ccid2_pr_debug("Sent: seq=%llu\n", (unsigned long long)seq); do { struct ccid2_seq *seqp = hctx->ccid2hctx_seqt; -- cgit v1.2.3-70-g09d2 From 83399361c30f2ffae20ee348ba9ada9a856d499a Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 22:09:35 -0200 Subject: [CCID2]: Remove redundant synchronisation variable This removes the synchronisation variable `ccid2hctx_sendwait', which is set to 1 when the CCID2 sender may send a new packet, and which is set to 0 otherwise The variable is redundant, since it is only used in combination with the hc_tx_send_packet/ hc_tx_packet_sent function pair. Both functions are called under socket lock, so the following happens when the CCID2 may send a new packet: * it sets sendwait = 1 in tx_send_packet and returns 0; * the subsequent call to tx_packet_sent clears the sendwait flag; * since tx_send_packet returns 0 if and only if sendwait == 1, the BUG_ON condition in tx_packet_sent is never satisfied, since that function is never called when tx_send_packet returns a value different from 0 (cf. dccp_write_xmit); * the call to tx_packet_sent clears the flag so that the condition "!sendwait" is true the next time tx_packet_sent is called. In other words, it is sufficient to just return 0 / not-0 to synchronise tx_send_packet and tx_packet_sent -- which is what the patch does. Signed-off-by: Gerrit Renker Acked-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 11 ++--------- net/dccp/ccids/ccid2.h | 1 - 2 files changed, 2 insertions(+), 10 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index 2d1b7e30a73..4f6c35261b6 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -126,13 +126,8 @@ static int ccid2_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) { struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); - if (hctx->ccid2hctx_pipe < hctx->ccid2hctx_cwnd) { - /* OK we can send... make sure previous packet was sent off */ - if (!hctx->ccid2hctx_sendwait) { - hctx->ccid2hctx_sendwait = 1; - return 0; - } - } + if (hctx->ccid2hctx_pipe < hctx->ccid2hctx_cwnd) + return 0; return 1; /* XXX CCID should dequeue when ready instead of polling */ } @@ -236,8 +231,6 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) struct ccid2_seq *next; u64 seq; - BUG_ON(!hctx->ccid2hctx_sendwait); - hctx->ccid2hctx_sendwait = 0; hctx->ccid2hctx_pipe++; /* There is an issue. What if another packet is sent between diff --git a/net/dccp/ccids/ccid2.h b/net/dccp/ccids/ccid2.h index bc659f00cb9..2671f8ebe29 100644 --- a/net/dccp/ccids/ccid2.h +++ b/net/dccp/ccids/ccid2.h @@ -66,7 +66,6 @@ struct ccid2_hc_tx_sock { struct timer_list ccid2hctx_rtotimer; u64 ccid2hctx_rpseq; int ccid2hctx_rpdupack; - int ccid2hctx_sendwait; unsigned long ccid2hctx_last_cong; u64 ccid2hctx_high_ack; }; -- cgit v1.2.3-70-g09d2 From a302002516a094015e5d004b8d939a8a34559c82 Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 22:10:29 -0200 Subject: [CCID2]: Remove redundant ack-counting variable The code used two different variables to count Acks, one of them redundant. This patch reduces the number of Ack counters to one. The type of the Ack counter has also been changed to u32 (twice the range of int); and the variable has been renamed into `packets_acked' - for consistency with RFC 3465 (and similarly named variables are used by TCP and SCTP). Lastly, a slightly less aggressive `maxincr' increment is used (for even Ack Ratios, maxincr was Ack Ratio/2 + 1 instead of Ack Ratio/2). Signed-off-by: Gerrit Renker Acked-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 46 +++++++++++++++------------------------------- net/dccp/ccids/ccid2.h | 6 ++---- 2 files changed, 17 insertions(+), 35 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index 4f6c35261b6..02d126d9a71 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -199,9 +199,8 @@ static void ccid2_hc_tx_rto_expire(unsigned long data) hctx->ccid2hctx_pipe = 0; /* clear state about stuff we sent */ - hctx->ccid2hctx_seqt = hctx->ccid2hctx_seqh; - hctx->ccid2hctx_ssacks = 0; - hctx->ccid2hctx_acks = 0; + hctx->ccid2hctx_seqt = hctx->ccid2hctx_seqh; + hctx->ccid2hctx_packets_acked = 0; /* clear ack ratio state. */ hctx->ccid2hctx_rpseq = 0; @@ -406,31 +405,15 @@ static inline void ccid2_new_ack(struct sock *sk, { struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); - /* slow start */ if (hctx->ccid2hctx_cwnd < hctx->ccid2hctx_ssthresh) { - hctx->ccid2hctx_acks = 0; - - /* We can increase cwnd at most maxincr [ack_ratio/2] */ - if (*maxincr) { - /* increase every 2 acks */ - hctx->ccid2hctx_ssacks++; - if (hctx->ccid2hctx_ssacks == 2) { - hctx->ccid2hctx_cwnd++; - hctx->ccid2hctx_ssacks = 0; - *maxincr = *maxincr - 1; - } - } else { - /* increased cwnd enough for this single ack */ - hctx->ccid2hctx_ssacks = 0; - } - } else { - hctx->ccid2hctx_ssacks = 0; - hctx->ccid2hctx_acks++; - - if (hctx->ccid2hctx_acks >= hctx->ccid2hctx_cwnd) { - hctx->ccid2hctx_cwnd++; - hctx->ccid2hctx_acks = 0; + if (*maxincr > 0 && ++hctx->ccid2hctx_packets_acked == 2) { + hctx->ccid2hctx_cwnd += 1; + *maxincr -= 1; + hctx->ccid2hctx_packets_acked = 0; } + } else if (++hctx->ccid2hctx_packets_acked >= hctx->ccid2hctx_cwnd) { + hctx->ccid2hctx_cwnd += 1; + hctx->ccid2hctx_packets_acked = 0; } /* update RTO */ @@ -596,12 +579,13 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) } } - /* If in slow-start, cwnd can increase at most Ack Ratio / 2 packets for - * this single ack. I round up. - * -sorbo. + /* + * In slow-start, cwnd can increase up to a maximum of Ack Ratio/2 + * packets per acknowledgement. Rounding up avoids that cwnd is not + * advanced when Ack Ratio is 1 and gives a slight edge otherwise. */ - maxincr = dp->dccps_l_ack_ratio >> 1; - maxincr++; + if (hctx->ccid2hctx_cwnd < hctx->ccid2hctx_ssthresh) + maxincr = DIV_ROUND_UP(dp->dccps_l_ack_ratio, 2); /* go through all ack vectors */ while ((offset = ccid2_ackvector(sk, skb, offset, diff --git a/net/dccp/ccids/ccid2.h b/net/dccp/ccids/ccid2.h index 2671f8ebe29..2c94ca02901 100644 --- a/net/dccp/ccids/ccid2.h +++ b/net/dccp/ccids/ccid2.h @@ -43,8 +43,7 @@ struct ccid2_seq { /** struct ccid2_hc_tx_sock - CCID2 TX half connection * * @ccid2hctx_{cwnd,ssthresh,pipe}: as per RFC 4341, section 5 - * @ccid2hctx_ssacks - ACKs recv in slow start - * @ccid2hctx_acks - ACKS recv in AI phase + * @ccid2hctx_packets_acked - Ack counter for deriving cwnd growth (RFC 3465) * @ccid2hctx_lastrtt -time RTT was last measured * @ccid2hctx_rpseq - last consecutive seqno * @ccid2hctx_rpdupack - dupacks since rpseq @@ -53,8 +52,7 @@ struct ccid2_hc_tx_sock { u32 ccid2hctx_cwnd; u32 ccid2hctx_ssthresh; u32 ccid2hctx_pipe; - int ccid2hctx_ssacks; - int ccid2hctx_acks; + u32 ccid2hctx_packets_acked; struct ccid2_seq *ccid2hctx_seqbuf[CCID2_SEQBUF_MAX]; int ccid2hctx_seqbufc; struct ccid2_seq *ccid2hctx_seqh; -- cgit v1.2.3-70-g09d2 From dcfbc7e97a2e3a0d73a2e41e1bddb988dcca701e Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Sat, 24 Nov 2007 22:12:06 -0200 Subject: [CCID2]: Remove misleading comment This removes a comment which identifies an `issue' with dccp_write_xmit() where there is none. The comment assumes it is possible that a packet is sent between the calls to ccid_hc_tx_send_packet(), dccp_transmit_skb(), ccid_hc_tx_packet_sent() (in the above order) in dccp_write_xmit(). I think that this is impossible, since dccp_write_xmit() is always called under lock: * when called as dccp_write_xmit(sk, 1) from dccp_send_close(), the socket is locked (see code comment above dccp_send_close()); * when called as dccp_write_xmit(sk, 0) from dccp_send_msg(), it is after lock_sock() has been called; * when called as dccp_write_xmit(sk, 0) from dccp_write_xmit_timer(), bh_lock_sock() has been called and the if/else statement has made sure that sk_lock.owner is not set; * there are no other places where dccp_write_xmit() is called. Furthermore, the debug statement for printing the sequence number of the packet just sent has been removed, since the entire list is being printed anyway and so the entry of that number appears last. Signed-off-by: Gerrit Renker Acked-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccids/ccid2.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index 02d126d9a71..48eeb4494e8 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -228,18 +228,10 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) struct dccp_sock *dp = dccp_sk(sk); struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); struct ccid2_seq *next; - u64 seq; hctx->ccid2hctx_pipe++; - /* There is an issue. What if another packet is sent between - * packet_send() and packet_sent(). Then the sequence number would be - * wrong. - * -sorbo. - */ - seq = dp->dccps_gss; - - hctx->ccid2hctx_seqh->ccid2s_seq = seq; + hctx->ccid2hctx_seqh->ccid2s_seq = dp->dccps_gss; hctx->ccid2hctx_seqh->ccid2s_acked = 0; hctx->ccid2hctx_seqh->ccid2s_sent = jiffies; @@ -313,7 +305,6 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) ccid2_start_rto_timer(sk); #ifdef CONFIG_IP_DCCP_CCID2_DEBUG - ccid2_pr_debug("Sent: seq=%llu\n", (unsigned long long)seq); do { struct ccid2_seq *seqp = hctx->ccid2hctx_seqt; -- cgit v1.2.3-70-g09d2 From 84a97b0af8c29aa5a47cc5271968a9c6004fb91e Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Thu, 13 Dec 2007 23:33:25 -0200 Subject: [CCID]: More informative registration The patch makes the registration messages of CCID 2/3 a bit more informative: instead of repeating the CCID number as currently done, "CCID: Registered CCID 2 (ccid2)" or "CCID: Registered CCID 3 (ccid3)", the descriptive names of the CCID's (from RFCs) are now used: "CCID: Registered CCID 2 (TCP-like)" and "CCID: Registered CCID 3 (TCP-Friendly Rate Control)". To allow spaces in the name, the slab name string has been changed to refer to the numeric CCID identifier, using the same format as before. Signed-off-by: Gerrit Renker Signed-off-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- net/dccp/ccid.c | 8 ++++---- net/dccp/ccids/ccid2.c | 2 +- net/dccp/ccids/ccid3.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) (limited to 'net/dccp/ccids/ccid2.c') diff --git a/net/dccp/ccid.c b/net/dccp/ccid.c index c45088b5e6f..4809753d12a 100644 --- a/net/dccp/ccid.c +++ b/net/dccp/ccid.c @@ -92,15 +92,15 @@ int ccid_register(struct ccid_operations *ccid_ops) ccid_ops->ccid_hc_rx_slab = ccid_kmem_cache_create(ccid_ops->ccid_hc_rx_obj_size, - "%s_hc_rx_sock", - ccid_ops->ccid_name); + "ccid%u_hc_rx_sock", + ccid_ops->ccid_id); if (ccid_ops->ccid_hc_rx_slab == NULL) goto out; ccid_ops->ccid_hc_tx_slab = ccid_kmem_cache_create(ccid_ops->ccid_hc_tx_obj_size, - "%s_hc_tx_sock", - ccid_ops->ccid_name); + "ccid%u_hc_tx_sock", + ccid_ops->ccid_id); if (ccid_ops->ccid_hc_tx_slab == NULL) goto out_free_rx_slab; diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c index 48eeb4494e8..b5b52ebb269 100644 --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -770,7 +770,7 @@ static void ccid2_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) static struct ccid_operations ccid2 = { .ccid_id = DCCPC_CCID2, - .ccid_name = "ccid2", + .ccid_name = "TCP-like", .ccid_owner = THIS_MODULE, .ccid_hc_tx_obj_size = sizeof(struct ccid2_hc_tx_sock), .ccid_hc_tx_init = ccid2_hc_tx_init, diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c index 8f112d18d45..cd9b9ffe2ec 100644 --- a/net/dccp/ccids/ccid3.c +++ b/net/dccp/ccids/ccid3.c @@ -943,7 +943,7 @@ static int ccid3_hc_rx_getsockopt(struct sock *sk, const int optname, int len, static struct ccid_operations ccid3 = { .ccid_id = DCCPC_CCID3, - .ccid_name = "ccid3", + .ccid_name = "TCP-Friendly Rate Control", .ccid_owner = THIS_MODULE, .ccid_hc_tx_obj_size = sizeof(struct ccid3_hc_tx_sock), .ccid_hc_tx_init = ccid3_hc_tx_init, -- cgit v1.2.3-70-g09d2