* [DCCP] [Patch 0/7]: Bug fixes and format updates [not found] <dccp_bug_fixes_and_format_updates> @ 2008-05-27 8:32 ` Gerrit Renker 2008-05-27 8:32 ` [PATCH 1/7] [DCCP]: Fix to handle short sequence numbers packet correctly Gerrit Renker 2008-05-28 6:12 ` [DCCP] [Patch 0/7]: Bug fixes and format updates Gerrit Renker 0 siblings, 2 replies; 21+ messages in thread From: Gerrit Renker @ 2008-05-27 8:32 UTC (permalink / raw) To: davem; +Cc: dccp, netdev Hi David, please consider pulling from git://eden-feed.erg.abdn.ac.uk/net-next-2.6.git Best regards, Gerrit ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/7] [DCCP]: Fix to handle short sequence numbers packet correctly 2008-05-27 8:32 ` [DCCP] [Patch 0/7]: Bug fixes and format updates Gerrit Renker @ 2008-05-27 8:32 ` Gerrit Renker 2008-05-27 8:32 ` [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes Gerrit Renker 2008-05-27 13:22 ` [PATCH 1/7] [DCCP]: Fix to handle short sequence numbers packet correctly David Miller 2008-05-28 6:12 ` [DCCP] [Patch 0/7]: Bug fixes and format updates Gerrit Renker 1 sibling, 2 replies; 21+ messages in thread From: Gerrit Renker @ 2008-05-27 8:32 UTC (permalink / raw) To: davem; +Cc: dccp, netdev, Wei Yongjun From: Wei Yongjun <yjwei@cn.fujitsu.com> RFC4340 said: 8.5. Pseudocode ... If P.type is not Data, Ack, or DataAck and P.X == 0 (the packet has short sequence numbers), drop packet and return But DCCP has some mistake to handle short sequence numbers packet, now it drop packet only if P.type is Data, Ack, or DataAck and P.X == 0. This patch fixed this problem. Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> Acked-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- net/dccp/ipv4.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) --- a/net/dccp/ipv4.c +++ b/net/dccp/ipv4.c @@ -739,8 +739,8 @@ int dccp_invalid_packet(struct sk_buff *skb) * If P.type is not Data, Ack, or DataAck and P.X == 0 (the packet * has short sequence numbers), drop packet and return */ - if (dh->dccph_type >= DCCP_PKT_DATA && - dh->dccph_type <= DCCP_PKT_DATAACK && dh->dccph_x == 0) { + if ((dh->dccph_type < DCCP_PKT_DATA || + dh->dccph_type > DCCP_PKT_DATAACK) && dh->dccph_x == 0) { DCCP_WARN("P.type (%s) not Data || [Data]Ack, while P.X == 0\n", dccp_packet_name(dh->dccph_type)); return 1; -- 1.5.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes 2008-05-27 8:32 ` [PATCH 1/7] [DCCP]: Fix to handle short sequence numbers packet correctly Gerrit Renker @ 2008-05-27 8:32 ` Gerrit Renker 2008-05-27 8:32 ` [PATCH 3/7] [CCID-3]: Remove ccid3hc{tx,rx}_ prefixes Gerrit Renker 2008-05-27 13:25 ` [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes David Miller 2008-05-27 13:22 ` [PATCH 1/7] [DCCP]: Fix to handle short sequence numbers packet correctly David Miller 1 sibling, 2 replies; 21+ messages in thread From: Gerrit Renker @ 2008-05-27 8:32 UTC (permalink / raw) To: davem; +Cc: dccp, netdev, Gerrit Renker This patch fixes two problems caused by the ubiquitous long "hctx->ccid2htx_" and "hcrx->ccid2hcrx_" prefixes: * code becomes hard to read; * multiple-line statements are almost inevitable even for simple expressions; The prefixes are not really necessary (compare with "struct tcp_sock"). There had been previous discussion of this on dccp@vger, but so far this was not followed up (most people agreed that the prefixes are too long). Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> Signed-off-by: Leandro Melo de Sales <leandroal@gmail.com> --- net/dccp/ccids/ccid2.c | 274 ++++++++++++++++++++++++------------------------ net/dccp/ccids/ccid2.h | 48 ++++---- 2 files changed, 159 insertions(+), 163 deletions(-) --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -39,16 +39,16 @@ static void ccid2_hc_tx_check_sanity(const struct ccid2_hc_tx_sock *hctx) { int len = 0; int pipe = 0; - struct ccid2_seq *seqp = hctx->ccid2hctx_seqh; + struct ccid2_seq *seqp = hctx->seqh; /* there is data in the chain */ - if (seqp != hctx->ccid2hctx_seqt) { + if (seqp != hctx->seqt) { seqp = seqp->ccid2s_prev; len++; if (!seqp->ccid2s_acked) pipe++; - while (seqp != hctx->ccid2hctx_seqt) { + while (seqp != hctx->seqt) { struct ccid2_seq *prev = seqp->ccid2s_prev; len++; @@ -65,16 +65,16 @@ static void ccid2_hc_tx_check_sanity(const struct ccid2_hc_tx_sock *hctx) } } - BUG_ON(pipe != hctx->ccid2hctx_pipe); + BUG_ON(pipe != hctx->pipe); ccid2_pr_debug("len of chain=%d\n", len); do { seqp = seqp->ccid2s_prev; len++; - } while (seqp != hctx->ccid2hctx_seqh); + } while (seqp != hctx->seqh); ccid2_pr_debug("total len=%d\n", len); - BUG_ON(len != hctx->ccid2hctx_seqbufc * CCID2_SEQBUF_LEN); + BUG_ON(len != hctx->seqbufc * CCID2_SEQBUF_LEN); } #else #define ccid2_pr_debug(format, a...) @@ -87,8 +87,7 @@ static int ccid2_hc_tx_alloc_seq(struct ccid2_hc_tx_sock *hctx) int i; /* check if we have space to preserve the pointer to the buffer */ - if (hctx->ccid2hctx_seqbufc >= (sizeof(hctx->ccid2hctx_seqbuf) / - sizeof(struct ccid2_seq*))) + if (hctx->seqbufc >= sizeof(hctx->seqbuf) / sizeof(struct ccid2_seq *)) return -ENOMEM; /* allocate buffer and initialize linked list */ @@ -104,20 +103,20 @@ static int ccid2_hc_tx_alloc_seq(struct ccid2_hc_tx_sock *hctx) seqp->ccid2s_prev = &seqp[CCID2_SEQBUF_LEN - 1]; /* This is the first allocation. Initiate the head and tail. */ - if (hctx->ccid2hctx_seqbufc == 0) - hctx->ccid2hctx_seqh = hctx->ccid2hctx_seqt = seqp; + if (hctx->seqbufc == 0) + hctx->seqh = hctx->seqt = seqp; else { /* link the existing list with the one we just created */ - hctx->ccid2hctx_seqh->ccid2s_next = seqp; - seqp->ccid2s_prev = hctx->ccid2hctx_seqh; + hctx->seqh->ccid2s_next = seqp; + seqp->ccid2s_prev = hctx->seqh; - hctx->ccid2hctx_seqt->ccid2s_prev = &seqp[CCID2_SEQBUF_LEN - 1]; - seqp[CCID2_SEQBUF_LEN - 1].ccid2s_next = hctx->ccid2hctx_seqt; + hctx->seqt->ccid2s_prev = &seqp[CCID2_SEQBUF_LEN - 1]; + seqp[CCID2_SEQBUF_LEN - 1].ccid2s_next = hctx->seqt; } /* store the original pointer to the buffer so we can free it */ - hctx->ccid2hctx_seqbuf[hctx->ccid2hctx_seqbufc] = seqp; - hctx->ccid2hctx_seqbufc++; + hctx->seqbuf[hctx->seqbufc] = seqp; + hctx->seqbufc++; return 0; } @@ -126,7 +125,7 @@ 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) + if (hctx->pipe < hctx->cwnd) return 0; return 1; /* XXX CCID should dequeue when ready instead of polling */ @@ -135,7 +134,7 @@ 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); + u32 max_ratio = DIV_ROUND_UP(ccid2_hc_tx_sk(sk)->cwnd, 2); /* * Ensure that Ack Ratio does not exceed ceil(cwnd/2), which is (2) from @@ -160,7 +159,7 @@ static void ccid2_change_l_ack_ratio(struct sock *sk, u32 val) static void ccid2_change_srtt(struct ccid2_hc_tx_sock *hctx, long val) { ccid2_pr_debug("change SRTT to %ld\n", val); - hctx->ccid2hctx_srtt = val; + hctx->srtt = val; } static void ccid2_start_rto_timer(struct sock *sk); @@ -173,8 +172,7 @@ static void ccid2_hc_tx_rto_expire(unsigned long data) bh_lock_sock(sk); if (sock_owned_by_user(sk)) { - sk_reset_timer(sk, &hctx->ccid2hctx_rtotimer, - jiffies + HZ / 5); + sk_reset_timer(sk, &hctx->rtotimer, jiffies + HZ / 5); goto out; } @@ -183,28 +181,28 @@ static void ccid2_hc_tx_rto_expire(unsigned long data) ccid2_hc_tx_check_sanity(hctx); /* back-off timer */ - hctx->ccid2hctx_rto <<= 1; + hctx->rto <<= 1; - s = hctx->ccid2hctx_rto / HZ; + s = hctx->rto / HZ; if (s > 60) - hctx->ccid2hctx_rto = 60 * HZ; + hctx->rto = 60 * HZ; ccid2_start_rto_timer(sk); /* adjust pipe, cwnd etc */ - hctx->ccid2hctx_ssthresh = hctx->ccid2hctx_cwnd / 2; - if (hctx->ccid2hctx_ssthresh < 2) - hctx->ccid2hctx_ssthresh = 2; - hctx->ccid2hctx_cwnd = 1; - hctx->ccid2hctx_pipe = 0; + hctx->ssthresh = hctx->cwnd / 2; + if (hctx->ssthresh < 2) + hctx->ssthresh = 2; + hctx->cwnd = 1; + hctx->pipe = 0; /* clear state about stuff we sent */ - hctx->ccid2hctx_seqt = hctx->ccid2hctx_seqh; - hctx->ccid2hctx_packets_acked = 0; + hctx->seqt = hctx->seqh; + hctx->packets_acked = 0; /* clear ack ratio state. */ - hctx->ccid2hctx_rpseq = 0; - hctx->ccid2hctx_rpdupack = -1; + hctx->rpseq = 0; + hctx->rpdupack = -1; ccid2_change_l_ack_ratio(sk, 1); ccid2_hc_tx_check_sanity(hctx); out: @@ -216,11 +214,11 @@ static void ccid2_start_rto_timer(struct sock *sk) { struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); - ccid2_pr_debug("setting RTO timeout=%ld\n", hctx->ccid2hctx_rto); + ccid2_pr_debug("setting RTO timeout=%ld\n", hctx->rto); - BUG_ON(timer_pending(&hctx->ccid2hctx_rtotimer)); - sk_reset_timer(sk, &hctx->ccid2hctx_rtotimer, - jiffies + hctx->ccid2hctx_rto); + BUG_ON(timer_pending(&hctx->rtotimer)); + sk_reset_timer(sk, &hctx->rtotimer, + jiffies + hctx->rto); } static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) @@ -229,27 +227,26 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); struct ccid2_seq *next; - hctx->ccid2hctx_pipe++; + hctx->pipe++; - hctx->ccid2hctx_seqh->ccid2s_seq = dp->dccps_gss; - hctx->ccid2hctx_seqh->ccid2s_acked = 0; - hctx->ccid2hctx_seqh->ccid2s_sent = jiffies; + hctx->seqh->ccid2s_seq = dp->dccps_gss; + hctx->seqh->ccid2s_acked = 0; + hctx->seqh->ccid2s_sent = jiffies; - next = hctx->ccid2hctx_seqh->ccid2s_next; + next = hctx->seqh->ccid2s_next; /* check if we need to alloc more space */ - if (next == hctx->ccid2hctx_seqt) { + if (next == hctx->seqt) { if (ccid2_hc_tx_alloc_seq(hctx)) { DCCP_CRIT("packet history - out of memory!"); /* FIXME: find a more graceful way to bail out */ return; } - next = hctx->ccid2hctx_seqh->ccid2s_next; - BUG_ON(next == hctx->ccid2hctx_seqt); + next = hctx->seqh->ccid2s_next; + BUG_ON(next == hctx->seqt); } - hctx->ccid2hctx_seqh = next; + hctx->seqh = next; - ccid2_pr_debug("cwnd=%d pipe=%d\n", hctx->ccid2hctx_cwnd, - hctx->ccid2hctx_pipe); + ccid2_pr_debug("cwnd=%d pipe=%d\n", hctx->cwnd, hctx->pipe); /* * FIXME: The code below is broken and the variables have been removed @@ -272,12 +269,12 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) */ #if 0 /* Ack Ratio. Need to maintain a concept of how many windows we sent */ - hctx->ccid2hctx_arsent++; + hctx->arsent++; /* We had an ack loss in this window... */ - if (hctx->ccid2hctx_ackloss) { - if (hctx->ccid2hctx_arsent >= hctx->ccid2hctx_cwnd) { - hctx->ccid2hctx_arsent = 0; - hctx->ccid2hctx_ackloss = 0; + if (hctx->ackloss) { + if (hctx->arsent >= hctx->cwnd) { + hctx->arsent = 0; + hctx->ackloss = 0; } } else { /* No acks lost up to now... */ @@ -287,28 +284,28 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) int denom = dp->dccps_l_ack_ratio * dp->dccps_l_ack_ratio - dp->dccps_l_ack_ratio; - denom = hctx->ccid2hctx_cwnd * hctx->ccid2hctx_cwnd / denom; + denom = hctx->cwnd * hctx->cwnd / denom; - if (hctx->ccid2hctx_arsent >= denom) { + if (hctx->arsent >= denom) { ccid2_change_l_ack_ratio(sk, dp->dccps_l_ack_ratio - 1); - hctx->ccid2hctx_arsent = 0; + hctx->arsent = 0; } } else { /* we can't increase ack ratio further [1] */ - hctx->ccid2hctx_arsent = 0; /* or maybe set it to cwnd*/ + hctx->arsent = 0; /* or maybe set it to cwnd*/ } } #endif /* setup RTO timer */ - if (!timer_pending(&hctx->ccid2hctx_rtotimer)) + if (!timer_pending(&hctx->rtotimer)) ccid2_start_rto_timer(sk); #ifdef CONFIG_IP_DCCP_CCID2_DEBUG do { - struct ccid2_seq *seqp = hctx->ccid2hctx_seqt; + struct ccid2_seq *seqp = hctx->seqt; - while (seqp != hctx->ccid2hctx_seqh) { + while (seqp != hctx->seqh) { ccid2_pr_debug("out seq=%llu acked=%d time=%lu\n", (unsigned long long)seqp->ccid2s_seq, seqp->ccid2s_acked, seqp->ccid2s_sent); @@ -386,7 +383,7 @@ static void ccid2_hc_tx_kill_rto_timer(struct sock *sk) { struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); - sk_stop_timer(sk, &hctx->ccid2hctx_rtotimer); + sk_stop_timer(sk, &hctx->rtotimer); ccid2_pr_debug("deleted RTO timer\n"); } @@ -396,73 +393,73 @@ static inline void ccid2_new_ack(struct sock *sk, { struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); - if (hctx->ccid2hctx_cwnd < hctx->ccid2hctx_ssthresh) { - if (*maxincr > 0 && ++hctx->ccid2hctx_packets_acked == 2) { - hctx->ccid2hctx_cwnd += 1; - *maxincr -= 1; - hctx->ccid2hctx_packets_acked = 0; + if (hctx->cwnd < hctx->ssthresh) { + if (*maxincr > 0 && ++hctx->packets_acked == 2) { + hctx->cwnd += 1; + *maxincr -= 1; + hctx->packets_acked = 0; } - } else if (++hctx->ccid2hctx_packets_acked >= hctx->ccid2hctx_cwnd) { - hctx->ccid2hctx_cwnd += 1; - hctx->ccid2hctx_packets_acked = 0; + } else if (++hctx->packets_acked >= hctx->cwnd) { + hctx->cwnd += 1; + hctx->packets_acked = 0; } /* update RTO */ - if (hctx->ccid2hctx_srtt == -1 || - time_after(jiffies, hctx->ccid2hctx_lastrtt + hctx->ccid2hctx_srtt)) { + if (hctx->srtt == -1 || + time_after(jiffies, hctx->lastrtt + hctx->srtt)) { unsigned long r = (long)jiffies - (long)seqp->ccid2s_sent; int s; /* first measurement */ - if (hctx->ccid2hctx_srtt == -1) { + if (hctx->srtt == -1) { ccid2_pr_debug("R: %lu Time=%lu seq=%llu\n", r, jiffies, (unsigned long long)seqp->ccid2s_seq); ccid2_change_srtt(hctx, r); - hctx->ccid2hctx_rttvar = r >> 1; + hctx->rttvar = r >> 1; } else { /* RTTVAR */ - long tmp = hctx->ccid2hctx_srtt - r; + long tmp = hctx->srtt - r; long srtt; if (tmp < 0) tmp *= -1; tmp >>= 2; - hctx->ccid2hctx_rttvar *= 3; - hctx->ccid2hctx_rttvar >>= 2; - hctx->ccid2hctx_rttvar += tmp; + hctx->rttvar *= 3; + hctx->rttvar >>= 2; + hctx->rttvar += tmp; /* SRTT */ - srtt = hctx->ccid2hctx_srtt; + srtt = hctx->srtt; srtt *= 7; srtt >>= 3; tmp = r >> 3; srtt += tmp; ccid2_change_srtt(hctx, srtt); } - s = hctx->ccid2hctx_rttvar << 2; + s = hctx->rttvar << 2; /* clock granularity is 1 when based on jiffies */ if (!s) s = 1; - hctx->ccid2hctx_rto = hctx->ccid2hctx_srtt + s; + hctx->rto = hctx->srtt + s; /* must be at least a second */ - s = hctx->ccid2hctx_rto / HZ; + s = hctx->rto / HZ; /* DCCP doesn't require this [but I like it cuz my code sux] */ #if 1 if (s < 1) - hctx->ccid2hctx_rto = HZ; + hctx->rto = HZ; #endif /* max 60 seconds */ if (s > 60) - hctx->ccid2hctx_rto = HZ * 60; + hctx->rto = HZ * 60; - hctx->ccid2hctx_lastrtt = jiffies; + hctx->lastrtt = jiffies; 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->srtt, hctx->rttvar, + hctx->rto, HZ, r); } /* we got a new ack, so re-start RTO timer */ @@ -474,12 +471,12 @@ static void ccid2_hc_tx_dec_pipe(struct sock *sk) { struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk); - if (hctx->ccid2hctx_pipe == 0) + if (hctx->pipe == 0) DCCP_BUG("pipe == 0"); else - hctx->ccid2hctx_pipe--; + hctx->pipe--; - if (hctx->ccid2hctx_pipe == 0) + if (hctx->pipe == 0) ccid2_hc_tx_kill_rto_timer(sk); } @@ -487,19 +484,19 @@ 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)) { + if (time_before(seqp->ccid2s_sent, hctx->last_cong)) { ccid2_pr_debug("Multiple losses in an RTT---treating as one\n"); return; } - hctx->ccid2hctx_last_cong = jiffies; + hctx->last_cong = jiffies; - hctx->ccid2hctx_cwnd = hctx->ccid2hctx_cwnd / 2 ? : 1U; - hctx->ccid2hctx_ssthresh = max(hctx->ccid2hctx_cwnd, 2U); + hctx->cwnd = hctx->cwnd / 2 ? : 1U; + hctx->ssthresh = max(hctx->cwnd, 2U); /* 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); + if (dccp_sk(sk)->dccps_l_ack_ratio > hctx->cwnd) + ccid2_change_l_ack_ratio(sk, hctx->cwnd); } static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) @@ -523,21 +520,21 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) * -sorbo. */ /* need to bootstrap */ - if (hctx->ccid2hctx_rpdupack == -1) { - hctx->ccid2hctx_rpdupack = 0; - hctx->ccid2hctx_rpseq = seqno; + if (hctx->rpdupack == -1) { + hctx->rpdupack = 0; + hctx->rpseq = seqno; } else { /* check if packet is consecutive */ - if (dccp_delta_seqno(hctx->ccid2hctx_rpseq, seqno) == 1) - hctx->ccid2hctx_rpseq = seqno; + if (dccp_delta_seqno(hctx->rpseq, seqno) == 1) + hctx->rpseq = seqno; /* it's a later packet */ - else if (after48(seqno, hctx->ccid2hctx_rpseq)) { - hctx->ccid2hctx_rpdupack++; + else if (after48(seqno, hctx->rpseq)) { + hctx->rpdupack++; /* check if we got enough dupacks */ - if (hctx->ccid2hctx_rpdupack >= NUMDUPACK) { - hctx->ccid2hctx_rpdupack = -1; /* XXX lame */ - hctx->ccid2hctx_rpseq = 0; + if (hctx->rpdupack >= NUMDUPACK) { + hctx->rpdupack = -1; /* XXX lame */ + hctx->rpseq = 0; ccid2_change_l_ack_ratio(sk, 2 * dp->dccps_l_ack_ratio); } @@ -546,7 +543,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) /* check forward path congestion */ /* still didn't send out new data packets */ - if (hctx->ccid2hctx_seqh == hctx->ccid2hctx_seqt) + if (hctx->seqh == hctx->seqt) return; switch (DCCP_SKB_CB(skb)->dccpd_type) { @@ -558,14 +555,14 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) } ackno = DCCP_SKB_CB(skb)->dccpd_ack_seq; - if (after48(ackno, hctx->ccid2hctx_high_ack)) - hctx->ccid2hctx_high_ack = ackno; + if (after48(ackno, hctx->high_ack)) + hctx->high_ack = ackno; - seqp = hctx->ccid2hctx_seqt; + seqp = hctx->seqt; while (before48(seqp->ccid2s_seq, ackno)) { seqp = seqp->ccid2s_next; - if (seqp == hctx->ccid2hctx_seqh) { - seqp = hctx->ccid2hctx_seqh->ccid2s_prev; + if (seqp == hctx->seqh) { + seqp = hctx->seqh->ccid2s_prev; break; } } @@ -575,7 +572,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) * packets per acknowledgement. Rounding up avoids that cwnd is not * advanced when Ack Ratio is 1 and gives a slight edge otherwise. */ - if (hctx->ccid2hctx_cwnd < hctx->ccid2hctx_ssthresh) + if (hctx->cwnd < hctx->ssthresh) maxincr = DIV_ROUND_UP(dp->dccps_l_ack_ratio, 2); /* go through all ack vectors */ @@ -594,7 +591,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) * seqnos. */ while (after48(seqp->ccid2s_seq, ackno)) { - if (seqp == hctx->ccid2hctx_seqt) { + if (seqp == hctx->seqt) { done = 1; break; } @@ -626,7 +623,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) (unsigned long long)seqp->ccid2s_seq); ccid2_hc_tx_dec_pipe(sk); } - if (seqp == hctx->ccid2hctx_seqt) { + if (seqp == hctx->seqt) { done = 1; break; } @@ -645,11 +642,11 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) /* The state about what is acked should be correct now * Check for NUMDUPACK */ - seqp = hctx->ccid2hctx_seqt; - while (before48(seqp->ccid2s_seq, hctx->ccid2hctx_high_ack)) { + seqp = hctx->seqt; + while (before48(seqp->ccid2s_seq, hctx->high_ack)) { seqp = seqp->ccid2s_next; - if (seqp == hctx->ccid2hctx_seqh) { - seqp = hctx->ccid2hctx_seqh->ccid2s_prev; + if (seqp == hctx->seqh) { + seqp = hctx->seqh->ccid2s_prev; break; } } @@ -660,7 +657,7 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) if (done == NUMDUPACK) break; } - if (seqp == hctx->ccid2hctx_seqt) + if (seqp == hctx->seqt) break; seqp = seqp->ccid2s_prev; } @@ -683,20 +680,20 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) ccid2_congestion_event(sk, seqp); ccid2_hc_tx_dec_pipe(sk); } - if (seqp == hctx->ccid2hctx_seqt) + if (seqp == hctx->seqt) break; seqp = seqp->ccid2s_prev; } - hctx->ccid2hctx_seqt = last_acked; + hctx->seqt = last_acked; } /* trim acked packets in tail */ - while (hctx->ccid2hctx_seqt != hctx->ccid2hctx_seqh) { - if (!hctx->ccid2hctx_seqt->ccid2s_acked) + while (hctx->seqt != hctx->seqh) { + if (!hctx->seqt->ccid2s_acked) break; - hctx->ccid2hctx_seqt = hctx->ccid2hctx_seqt->ccid2s_next; + hctx->seqt = hctx->seqt->ccid2s_next; } ccid2_hc_tx_check_sanity(hctx); @@ -709,17 +706,17 @@ 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 = ~0U; + hctx->ssthresh = ~0U; /* * 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 = clamp(4380U / dp->dccps_mss_cache, 2U, 4U); + hctx->cwnd = clamp(4380U / dp->dccps_mss_cache, 2U, 4U); /* Make sure that Ack Ratio is enabled and within bounds. */ - max_ratio = DIV_ROUND_UP(hctx->ccid2hctx_cwnd, 2); + max_ratio = DIV_ROUND_UP(hctx->cwnd, 2); if (dp->dccps_l_ack_ratio == 0 || dp->dccps_l_ack_ratio > max_ratio) dp->dccps_l_ack_ratio = max_ratio; @@ -727,13 +724,12 @@ static int ccid2_hc_tx_init(struct ccid *ccid, struct sock *sk) if (ccid2_hc_tx_alloc_seq(hctx)) return -ENOMEM; - hctx->ccid2hctx_rto = 3 * HZ; + hctx->rto = 3 * HZ; ccid2_change_srtt(hctx, -1); - hctx->ccid2hctx_rttvar = -1; - hctx->ccid2hctx_rpdupack = -1; - hctx->ccid2hctx_last_cong = jiffies; - setup_timer(&hctx->ccid2hctx_rtotimer, ccid2_hc_tx_rto_expire, - (unsigned long)sk); + hctx->rttvar = -1; + hctx->rpdupack = -1; + hctx->last_cong = jiffies; + setup_timer(&hctx->rtotimer, ccid2_hc_tx_rto_expire, (unsigned long)sk); ccid2_hc_tx_check_sanity(hctx); return 0; @@ -746,9 +742,9 @@ static void ccid2_hc_tx_exit(struct sock *sk) ccid2_hc_tx_kill_rto_timer(sk); - for (i = 0; i < hctx->ccid2hctx_seqbufc; i++) - kfree(hctx->ccid2hctx_seqbuf[i]); - hctx->ccid2hctx_seqbufc = 0; + for (i = 0; i < hctx->seqbufc; i++) + kfree(hctx->seqbuf[i]); + hctx->seqbufc = 0; } static void ccid2_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) @@ -759,10 +755,10 @@ static void ccid2_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) switch (DCCP_SKB_CB(skb)->dccpd_type) { case DCCP_PKT_DATA: case DCCP_PKT_DATAACK: - hcrx->ccid2hcrx_data++; - if (hcrx->ccid2hcrx_data >= dp->dccps_r_ack_ratio) { + hcrx->data++; + if (hcrx->data >= dp->dccps_r_ack_ratio) { dccp_send_ack(sk); - hcrx->ccid2hcrx_data = 0; + hcrx->data = 0; } break; } --- a/net/dccp/ccids/ccid2.h +++ b/net/dccp/ccids/ccid2.h @@ -42,34 +42,34 @@ struct ccid2_seq { /** struct ccid2_hc_tx_sock - CCID2 TX half connection * - * @ccid2hctx_{cwnd,ssthresh,pipe}: as per RFC 4341, section 5 - * @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 -*/ + * @{cwnd,ssthresh,pipe}: as per RFC 4341, section 5 + * @packets_acked: Ack counter for deriving cwnd growth (RFC 3465) + * @lastrtt: time RTT was last measured + * @rpseq: last consecutive seqno + * @rpdupack: dupacks since rpseq + */ struct ccid2_hc_tx_sock { - u32 ccid2hctx_cwnd; - u32 ccid2hctx_ssthresh; - u32 ccid2hctx_pipe; - u32 ccid2hctx_packets_acked; - struct ccid2_seq *ccid2hctx_seqbuf[CCID2_SEQBUF_MAX]; - int ccid2hctx_seqbufc; - struct ccid2_seq *ccid2hctx_seqh; - struct ccid2_seq *ccid2hctx_seqt; - long ccid2hctx_rto; - long ccid2hctx_srtt; - long ccid2hctx_rttvar; - unsigned long ccid2hctx_lastrtt; - struct timer_list ccid2hctx_rtotimer; - u64 ccid2hctx_rpseq; - int ccid2hctx_rpdupack; - unsigned long ccid2hctx_last_cong; - u64 ccid2hctx_high_ack; + u32 cwnd; + u32 ssthresh; + u32 pipe; + u32 packets_acked; + struct ccid2_seq *seqbuf[CCID2_SEQBUF_MAX]; + int seqbufc; + struct ccid2_seq *seqh; + struct ccid2_seq *seqt; + long rto; + long srtt; + long rttvar; + unsigned long lastrtt; + struct timer_list rtotimer; + u64 rpseq; + int rpdupack; + unsigned long last_cong; + u64 high_ack; }; struct ccid2_hc_rx_sock { - int ccid2hcrx_data; + int data; }; static inline struct ccid2_hc_tx_sock *ccid2_hc_tx_sk(const struct sock *sk) -- 1.5.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/7] [CCID-3]: Remove ccid3hc{tx,rx}_ prefixes 2008-05-27 8:32 ` [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes Gerrit Renker @ 2008-05-27 8:32 ` Gerrit Renker 2008-05-27 8:32 ` [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug Gerrit Renker 2008-05-27 13:26 ` [PATCH 3/7] [CCID-3]: Remove ccid3hc{tx,rx}_ prefixes David Miller 2008-05-27 13:25 ` [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes David Miller 1 sibling, 2 replies; 21+ messages in thread From: Gerrit Renker @ 2008-05-27 8:32 UTC (permalink / raw) To: davem; +Cc: dccp, netdev, Gerrit Renker This patch does the same for CCID-3 as the previous patch for CCID-2: s#ccid3hctx_##g; s#ccid3hcrx_##g; plus manual editing to retain consistency. Please note: expanded the fields of the `struct tfrc_tx_info' in the hc_tx_sock, since using short #define identifiers is not a good idea. The only place where this embedded struct was used is ccid3_hc_tx_getsockopt(). Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> --- net/dccp/ccids/ccid3.c | 341 ++++++++++++++++++++++------------------------- net/dccp/ccids/ccid3.h | 119 +++++++++--------- net/dccp/probe.c | 6 +- 3 files changed, 221 insertions(+), 245 deletions(-) --- a/net/dccp/ccids/ccid3.c +++ b/net/dccp/ccids/ccid3.c @@ -67,13 +67,13 @@ static void ccid3_hc_tx_set_state(struct sock *sk, enum ccid3_hc_tx_states state) { struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk); - enum ccid3_hc_tx_states oldstate = hctx->ccid3hctx_state; + enum ccid3_hc_tx_states oldstate = hctx->state; ccid3_pr_debug("%s(%p) %-8.8s -> %s\n", dccp_role(sk), sk, ccid3_tx_state_name(oldstate), ccid3_tx_state_name(state)); WARN_ON(state == oldstate); - hctx->ccid3hctx_state = state; + hctx->state = state; } /* @@ -88,10 +88,9 @@ static void ccid3_hc_tx_set_state(struct sock *sk, static inline u64 rfc3390_initial_rate(struct sock *sk) { const struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk); - const __u32 w_init = clamp_t(__u32, 4380U, - 2 * hctx->ccid3hctx_s, 4 * hctx->ccid3hctx_s); + const __u32 w_init = clamp_t(__u32, 4380U, 2 * hctx->s, 4 * hctx->s); - return scaled_div(w_init << 6, hctx->ccid3hctx_rtt); + return scaled_div(w_init << 6, hctx->rtt); } /* @@ -100,24 +99,20 @@ static inline u64 rfc3390_initial_rate(struct sock *sk) static void ccid3_update_send_interval(struct ccid3_hc_tx_sock *hctx) { /* Calculate new t_ipi = s / X_inst (X_inst is in 64 * bytes/second) */ - hctx->ccid3hctx_t_ipi = scaled_div32(((u64)hctx->ccid3hctx_s) << 6, - hctx->ccid3hctx_x); + hctx->t_ipi = scaled_div32(((u64)hctx->s) << 6, hctx->x); /* Calculate new delta by delta = min(t_ipi / 2, t_gran / 2) */ - hctx->ccid3hctx_delta = min_t(u32, hctx->ccid3hctx_t_ipi / 2, - TFRC_OPSYS_HALF_TIME_GRAN); - - ccid3_pr_debug("t_ipi=%u, delta=%u, s=%u, X=%u\n", - hctx->ccid3hctx_t_ipi, hctx->ccid3hctx_delta, - hctx->ccid3hctx_s, (unsigned)(hctx->ccid3hctx_x >> 6)); + hctx->delta = min_t(u32, hctx->t_ipi / 2, TFRC_OPSYS_HALF_TIME_GRAN); + ccid3_pr_debug("t_ipi=%u, delta=%u, s=%u, X=%u\n", hctx->t_ipi, + hctx->delta, hctx->s, (unsigned)(hctx->x >> 6)); } static u32 ccid3_hc_tx_idle_rtt(struct ccid3_hc_tx_sock *hctx, ktime_t now) { - u32 delta = ktime_us_delta(now, hctx->ccid3hctx_t_last_win_count); + u32 delta = ktime_us_delta(now, hctx->t_last_win_count); - return delta / hctx->ccid3hctx_rtt; + return delta / hctx->rtt; } /** @@ -133,8 +128,8 @@ static u32 ccid3_hc_tx_idle_rtt(struct ccid3_hc_tx_sock *hctx, ktime_t now) static void ccid3_hc_tx_update_x(struct sock *sk, ktime_t *stamp) { struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk); - __u64 min_rate = 2 * hctx->ccid3hctx_x_recv; - const __u64 old_x = hctx->ccid3hctx_x; + u64 min_rate = 2 * hctx->x_recv; + const u64 old_x = hctx->x; ktime_t now = stamp ? *stamp : ktime_get_real(); /* @@ -145,33 +140,26 @@ static void ccid3_hc_tx_update_x(struct sock *sk, ktime_t *stamp) */ if (ccid3_hc_tx_idle_rtt(hctx, now) >= 2) { min_rate = rfc3390_initial_rate(sk); - min_rate = max(min_rate, 2 * hctx->ccid3hctx_x_recv); + min_rate = max(min_rate, 2 * hctx->x_recv); } - if (hctx->ccid3hctx_p > 0) { + if (hctx->p > 0) { - hctx->ccid3hctx_x = min(((__u64)hctx->ccid3hctx_x_calc) << 6, - min_rate); - hctx->ccid3hctx_x = max(hctx->ccid3hctx_x, - (((__u64)hctx->ccid3hctx_s) << 6) / - TFRC_T_MBI); + hctx->x = min(((u64)hctx->x_calc) << 6, min_rate); + hctx->x = max(hctx->x, (((u64)hctx->s) << 6) / TFRC_T_MBI); - } else if (ktime_us_delta(now, hctx->ccid3hctx_t_ld) - - (s64)hctx->ccid3hctx_rtt >= 0) { + } else if (ktime_us_delta(now, hctx->t_ld) - (s64)hctx->rtt >= 0) { - hctx->ccid3hctx_x = - max(min(2 * hctx->ccid3hctx_x, min_rate), - scaled_div(((__u64)hctx->ccid3hctx_s) << 6, - hctx->ccid3hctx_rtt)); - hctx->ccid3hctx_t_ld = now; + hctx->x = max(min(2 * hctx->x, min_rate), + scaled_div(((u64)hctx->s) << 6, hctx->rtt)); + hctx->t_ld = now; } - if (hctx->ccid3hctx_x != old_x) { + if (hctx->x != old_x) { ccid3_pr_debug("X_prev=%u, X_now=%u, X_calc=%u, " "X_recv=%u\n", (unsigned)(old_x >> 6), - (unsigned)(hctx->ccid3hctx_x >> 6), - hctx->ccid3hctx_x_calc, - (unsigned)(hctx->ccid3hctx_x_recv >> 6)); + (unsigned)(hctx->x >> 6), hctx->x_calc, + (unsigned)(hctx->x_recv >> 6)); ccid3_update_send_interval(hctx); } @@ -183,11 +171,11 @@ static void ccid3_hc_tx_update_x(struct sock *sk, ktime_t *stamp) */ static inline void ccid3_hc_tx_update_s(struct ccid3_hc_tx_sock *hctx, int len) { - const u16 old_s = hctx->ccid3hctx_s; + const u16 old_s = hctx->s; - hctx->ccid3hctx_s = tfrc_ewma(hctx->ccid3hctx_s, len, 9); + hctx->s = tfrc_ewma(hctx->s, len, 9); - if (hctx->ccid3hctx_s != old_s) + if (hctx->s != old_s) ccid3_update_send_interval(hctx); } @@ -200,16 +188,16 @@ static inline void ccid3_hc_tx_update_win_count(struct ccid3_hc_tx_sock *hctx, { u32 quarter_rtts; - if (unlikely(hctx->ccid3hctx_rtt < 4)) /* avoid divide-by-zero */ + if (unlikely(hctx->rtt < 4)) /* avoid divide-by-zero */ return; - quarter_rtts = ktime_us_delta(now, hctx->ccid3hctx_t_last_win_count); - quarter_rtts /= hctx->ccid3hctx_rtt / 4; + quarter_rtts = ktime_us_delta(now, hctx->t_last_win_count); + quarter_rtts /= hctx->rtt / 4; if (quarter_rtts > 0) { - hctx->ccid3hctx_t_last_win_count = now; - hctx->ccid3hctx_last_win_count += min_t(u32, quarter_rtts, 5); - hctx->ccid3hctx_last_win_count &= 0xF; /* mod 16 */ + hctx->t_last_win_count = now; + hctx->last_win_count += min_t(u32, quarter_rtts, 5); + hctx->last_win_count &= 0xF; /* mod 16 */ } } @@ -227,23 +215,21 @@ static void ccid3_hc_tx_no_feedback_timer(unsigned long data) } ccid3_pr_debug("%s(%p, state=%s) - entry \n", dccp_role(sk), sk, - ccid3_tx_state_name(hctx->ccid3hctx_state)); + ccid3_tx_state_name(hctx->state)); - if (hctx->ccid3hctx_state == TFRC_SSTATE_FBACK) + if (hctx->state == TFRC_SSTATE_FBACK) ccid3_hc_tx_set_state(sk, TFRC_SSTATE_NO_FBACK); - else if (hctx->ccid3hctx_state != TFRC_SSTATE_NO_FBACK) + else if (hctx->state != TFRC_SSTATE_NO_FBACK) goto out; /* * Determine new allowed sending rate X as per draft rfc3448bis-00, 4.4 + * RTO is 0 if and only if no feedback has been received yet. */ - if (hctx->ccid3hctx_t_rto == 0 || /* no feedback received yet */ - hctx->ccid3hctx_p == 0) { + if (hctx->t_rto == 0 || hctx->p == 0) { /* halve send rate directly */ - hctx->ccid3hctx_x = max(hctx->ccid3hctx_x / 2, - (((__u64)hctx->ccid3hctx_s) << 6) / - TFRC_T_MBI); + hctx->x = max(hctx->x / 2, (((u64)hctx->s) << 6) / TFRC_T_MBI); ccid3_update_send_interval(hctx); } else { /* @@ -256,33 +242,32 @@ static void ccid3_hc_tx_no_feedback_timer(unsigned long data) * * Note that X_recv is scaled by 2^6 while X_calc is not */ - BUG_ON(hctx->ccid3hctx_p && !hctx->ccid3hctx_x_calc); + BUG_ON(hctx->p && !hctx->x_calc); - if (hctx->ccid3hctx_x_calc > (hctx->ccid3hctx_x_recv >> 5)) - hctx->ccid3hctx_x_recv = - max(hctx->ccid3hctx_x_recv / 2, - (((__u64)hctx->ccid3hctx_s) << 6) / - (2 * TFRC_T_MBI)); + if (hctx->x_calc > (hctx->x_recv >> 5)) + hctx->x_recv = + max(hctx->x_recv / 2, + (((__u64)hctx->s) << 6) / (2 * TFRC_T_MBI)); else { - hctx->ccid3hctx_x_recv = hctx->ccid3hctx_x_calc; - hctx->ccid3hctx_x_recv <<= 4; + hctx->x_recv = hctx->x_calc; + hctx->x_recv <<= 4; } ccid3_hc_tx_update_x(sk, NULL); } ccid3_pr_debug("Reduced X to %llu/64 bytes/sec\n", - (unsigned long long)hctx->ccid3hctx_x); + (unsigned long long)hctx->x); /* * Set new timeout for the nofeedback timer. * See comments in packet_recv() regarding the value of t_RTO. */ - if (unlikely(hctx->ccid3hctx_t_rto == 0)) /* no feedback yet */ + if (unlikely(hctx->t_rto == 0)) /* no feedback received yet */ t_nfb = TFRC_INITIAL_TIMEOUT; else - t_nfb = max(hctx->ccid3hctx_t_rto, 2 * hctx->ccid3hctx_t_ipi); + t_nfb = max(hctx->t_rto, 2 * hctx->t_ipi); restart_timer: - sk_reset_timer(sk, &hctx->ccid3hctx_no_feedback_timer, + sk_reset_timer(sk, &hctx->no_feedback_timer, jiffies + usecs_to_jiffies(t_nfb)); out: bh_unlock_sock(sk); @@ -310,18 +295,17 @@ static int ccid3_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) if (unlikely(skb->len == 0)) return -EBADMSG; - switch (hctx->ccid3hctx_state) { + switch (hctx->state) { case TFRC_SSTATE_NO_SENT: - sk_reset_timer(sk, &hctx->ccid3hctx_no_feedback_timer, - (jiffies + + sk_reset_timer(sk, &hctx->no_feedback_timer, (jiffies + usecs_to_jiffies(TFRC_INITIAL_TIMEOUT))); - hctx->ccid3hctx_last_win_count = 0; - hctx->ccid3hctx_t_last_win_count = now; + hctx->last_win_count = 0; + hctx->t_last_win_count = now; /* Set t_0 for initial packet */ - hctx->ccid3hctx_t_nom = now; + hctx->t_nom = now; - hctx->ccid3hctx_s = skb->len; + hctx->s = skb->len; /* * Use initial RTT sample when available: recommended by erratum @@ -330,13 +314,13 @@ static int ccid3_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) */ if (dp->dccps_syn_rtt) { ccid3_pr_debug("SYN RTT = %uus\n", dp->dccps_syn_rtt); - hctx->ccid3hctx_rtt = dp->dccps_syn_rtt; - hctx->ccid3hctx_x = rfc3390_initial_rate(sk); - hctx->ccid3hctx_t_ld = now; + hctx->rtt = dp->dccps_syn_rtt; + hctx->x = rfc3390_initial_rate(sk); + hctx->t_ld = now; } else { /* Sender does not have RTT sample: X_pps = 1 pkt/sec */ - hctx->ccid3hctx_x = hctx->ccid3hctx_s; - hctx->ccid3hctx_x <<= 6; + hctx->x = hctx->s; + hctx->x <<= 6; } ccid3_update_send_interval(hctx); @@ -344,7 +328,7 @@ static int ccid3_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) break; case TFRC_SSTATE_NO_FBACK: case TFRC_SSTATE_FBACK: - delay = ktime_us_delta(hctx->ccid3hctx_t_nom, now); + delay = ktime_us_delta(hctx->t_nom, now); ccid3_pr_debug("delay=%ld\n", (long)delay); /* * Scheduling of packet transmissions [RFC 3448, 4.6] @@ -354,7 +338,7 @@ static int ccid3_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) * else * // send the packet in (t_nom - t_now) milliseconds. */ - if (delay - (s64)hctx->ccid3hctx_delta >= 1000) + if (delay - (s64)hctx->delta >= 1000) return (u32)delay / 1000L; ccid3_hc_tx_update_win_count(hctx, now); @@ -366,11 +350,10 @@ static int ccid3_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) /* prepare to send now (add options etc.) */ dp->dccps_hc_tx_insert_options = 1; - DCCP_SKB_CB(skb)->dccpd_ccval = hctx->ccid3hctx_last_win_count; + DCCP_SKB_CB(skb)->dccpd_ccval = hctx->last_win_count; /* set the nominal send time for the next following packet */ - hctx->ccid3hctx_t_nom = ktime_add_us(hctx->ccid3hctx_t_nom, - hctx->ccid3hctx_t_ipi); + hctx->t_nom = ktime_add_us(hctx->t_nom, hctx->t_ipi); return 0; } @@ -381,14 +364,14 @@ static void ccid3_hc_tx_packet_sent(struct sock *sk, int more, ccid3_hc_tx_update_s(hctx, len); - if (tfrc_tx_hist_add(&hctx->ccid3hctx_hist, dccp_sk(sk)->dccps_gss)) + if (tfrc_tx_hist_add(&hctx->hist, dccp_sk(sk)->dccps_gss)) DCCP_CRIT("packet history - out of memory!"); } static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) { struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk); - struct ccid3_options_received *opt_recv; + struct ccid3_options_received *opt_recv = &hctx->options_received; ktime_t now; unsigned long t_nfb; u32 pinv, r_sample; @@ -398,15 +381,14 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) DCCP_SKB_CB(skb)->dccpd_type == DCCP_PKT_DATAACK)) return; /* ... and only in the established state */ - if (hctx->ccid3hctx_state != TFRC_SSTATE_FBACK && - hctx->ccid3hctx_state != TFRC_SSTATE_NO_FBACK) + if (hctx->state != TFRC_SSTATE_FBACK && + hctx->state != TFRC_SSTATE_NO_FBACK) return; - opt_recv = &hctx->ccid3hctx_options_received; now = ktime_get_real(); /* Estimate RTT from history if ACK number is valid */ - r_sample = tfrc_tx_hist_rtt(hctx->ccid3hctx_hist, + r_sample = tfrc_tx_hist_rtt(hctx->hist, DCCP_SKB_CB(skb)->dccpd_ack_seq, now); if (r_sample == 0) { DCCP_WARN("%s(%p): %s with bogus ACK-%llu\n", dccp_role(sk), sk, @@ -416,37 +398,37 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) } /* Update receive rate in units of 64 * bytes/second */ - hctx->ccid3hctx_x_recv = opt_recv->ccid3or_receive_rate; - hctx->ccid3hctx_x_recv <<= 6; + hctx->x_recv = opt_recv->ccid3or_receive_rate; + hctx->x_recv <<= 6; /* Update loss event rate (which is scaled by 1e6) */ pinv = opt_recv->ccid3or_loss_event_rate; if (pinv == ~0U || pinv == 0) /* see RFC 4342, 8.5 */ - hctx->ccid3hctx_p = 0; + hctx->p = 0; else /* can not exceed 100% */ - hctx->ccid3hctx_p = scaled_div(1, pinv); + hctx->p = scaled_div(1, pinv); /* * Validate new RTT sample and update moving average */ r_sample = dccp_sample_rtt(sk, r_sample); - hctx->ccid3hctx_rtt = tfrc_ewma(hctx->ccid3hctx_rtt, r_sample, 9); + hctx->rtt = tfrc_ewma(hctx->rtt, r_sample, 9); /* * Update allowed sending rate X as per draft rfc3448bis-00, 4.2/3 */ - if (hctx->ccid3hctx_state == TFRC_SSTATE_NO_FBACK) { + if (hctx->state == TFRC_SSTATE_NO_FBACK) { ccid3_hc_tx_set_state(sk, TFRC_SSTATE_FBACK); - if (hctx->ccid3hctx_t_rto == 0) { + if (hctx->t_rto == 0) { /* * Initial feedback packet: Larger Initial Windows (4.2) */ - hctx->ccid3hctx_x = rfc3390_initial_rate(sk); - hctx->ccid3hctx_t_ld = now; + hctx->x = rfc3390_initial_rate(sk); + hctx->t_ld = now; ccid3_update_send_interval(hctx); goto done_computing_x; - } else if (hctx->ccid3hctx_p == 0) { + } else if (hctx->p == 0) { /* * First feedback after nofeedback timer expiry (4.3) */ @@ -455,25 +437,20 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) } /* Update sending rate (step 4 of [RFC 3448, 4.3]) */ - if (hctx->ccid3hctx_p > 0) - hctx->ccid3hctx_x_calc = - tfrc_calc_x(hctx->ccid3hctx_s, - hctx->ccid3hctx_rtt, - hctx->ccid3hctx_p); + if (hctx->p > 0) + hctx->x_calc = tfrc_calc_x(hctx->s, hctx->rtt, hctx->p); ccid3_hc_tx_update_x(sk, &now); done_computing_x: ccid3_pr_debug("%s(%p), RTT=%uus (sample=%uus), s=%u, " "p=%u, X_calc=%u, X_recv=%u, X=%u\n", - dccp_role(sk), - sk, hctx->ccid3hctx_rtt, r_sample, - hctx->ccid3hctx_s, hctx->ccid3hctx_p, - hctx->ccid3hctx_x_calc, - (unsigned)(hctx->ccid3hctx_x_recv >> 6), - (unsigned)(hctx->ccid3hctx_x >> 6)); + dccp_role(sk), sk, hctx->rtt, r_sample, + hctx->s, hctx->p, hctx->x_calc, + (unsigned)(hctx->x_recv >> 6), + (unsigned)(hctx->x >> 6)); /* unschedule no feedback timer */ - sk_stop_timer(sk, &hctx->ccid3hctx_no_feedback_timer); + sk_stop_timer(sk, &hctx->no_feedback_timer); /* * As we have calculated new ipi, delta, t_nom it is possible @@ -487,21 +464,19 @@ done_computing_x: * This can help avoid triggering the nofeedback timer too * often ('spinning') on LANs with small RTTs. */ - hctx->ccid3hctx_t_rto = max_t(u32, 4 * hctx->ccid3hctx_rtt, - (CONFIG_IP_DCCP_CCID3_RTO * - (USEC_PER_SEC / 1000))); + hctx->t_rto = max_t(u32, 4 * hctx->rtt, (CONFIG_IP_DCCP_CCID3_RTO * + (USEC_PER_SEC / 1000))); /* * Schedule no feedback timer to expire in * max(t_RTO, 2 * s/X) = max(t_RTO, 2 * t_ipi) */ - t_nfb = max(hctx->ccid3hctx_t_rto, 2 * hctx->ccid3hctx_t_ipi); + t_nfb = max(hctx->t_rto, 2 * hctx->t_ipi); ccid3_pr_debug("%s(%p), Scheduled no feedback timer to " "expire in %lu jiffies (%luus)\n", - dccp_role(sk), - sk, usecs_to_jiffies(t_nfb), t_nfb); + dccp_role(sk), sk, usecs_to_jiffies(t_nfb), t_nfb); - sk_reset_timer(sk, &hctx->ccid3hctx_no_feedback_timer, + sk_reset_timer(sk, &hctx->no_feedback_timer, jiffies + usecs_to_jiffies(t_nfb)); } @@ -512,11 +487,9 @@ static int ccid3_hc_tx_parse_options(struct sock *sk, unsigned char option, int rc = 0; const struct dccp_sock *dp = dccp_sk(sk); struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk); - struct ccid3_options_received *opt_recv; + struct ccid3_options_received *opt_recv = &hctx->options_received; __be32 opt_val; - opt_recv = &hctx->ccid3hctx_options_received; - if (opt_recv->ccid3or_seqno != dp->dccps_gsr) { opt_recv->ccid3or_seqno = dp->dccps_gsr; opt_recv->ccid3or_loss_event_rate = ~0; @@ -571,11 +544,10 @@ static int ccid3_hc_tx_init(struct ccid *ccid, struct sock *sk) { struct ccid3_hc_tx_sock *hctx = ccid_priv(ccid); - hctx->ccid3hctx_state = TFRC_SSTATE_NO_SENT; - hctx->ccid3hctx_hist = NULL; - setup_timer(&hctx->ccid3hctx_no_feedback_timer, - ccid3_hc_tx_no_feedback_timer, (unsigned long)sk); - + hctx->state = TFRC_SSTATE_NO_SENT; + hctx->hist = NULL; + setup_timer(&hctx->no_feedback_timer, + ccid3_hc_tx_no_feedback_timer, (unsigned long)sk); return 0; } @@ -584,9 +556,9 @@ static void ccid3_hc_tx_exit(struct sock *sk) struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk); ccid3_hc_tx_set_state(sk, TFRC_SSTATE_TERM); - sk_stop_timer(sk, &hctx->ccid3hctx_no_feedback_timer); + sk_stop_timer(sk, &hctx->no_feedback_timer); - tfrc_tx_hist_purge(&hctx->ccid3hctx_hist); + tfrc_tx_hist_purge(&hctx->hist); } static void ccid3_hc_tx_get_info(struct sock *sk, struct tcp_info *info) @@ -598,14 +570,15 @@ static void ccid3_hc_tx_get_info(struct sock *sk, struct tcp_info *info) return; hctx = ccid3_hc_tx_sk(sk); - info->tcpi_rto = hctx->ccid3hctx_t_rto; - info->tcpi_rtt = hctx->ccid3hctx_rtt; + info->tcpi_rto = hctx->t_rto; + info->tcpi_rtt = hctx->rtt; } static int ccid3_hc_tx_getsockopt(struct sock *sk, const int optname, int len, u32 __user *optval, int __user *optlen) { const struct ccid3_hc_tx_sock *hctx; + struct tfrc_tx_info tfrc; const void *val; /* Listen socks doesn't have a private CCID block */ @@ -615,10 +588,17 @@ static int ccid3_hc_tx_getsockopt(struct sock *sk, const int optname, int len, hctx = ccid3_hc_tx_sk(sk); switch (optname) { case DCCP_SOCKOPT_CCID_TX_INFO: - if (len < sizeof(hctx->ccid3hctx_tfrc)) + if (len < sizeof(tfrc)) return -EINVAL; - len = sizeof(hctx->ccid3hctx_tfrc); - val = &hctx->ccid3hctx_tfrc; + tfrc.tfrctx_x = hctx->x; + tfrc.tfrctx_x_recv = hctx->x_recv; + tfrc.tfrctx_x_calc = hctx->x_calc; + tfrc.tfrctx_rtt = hctx->rtt; + tfrc.tfrctx_p = hctx->p; + tfrc.tfrctx_rto = hctx->t_rto; + tfrc.tfrctx_ipi = hctx->t_ipi; + len = sizeof(tfrc); + val = &tfrc; break; default: return -ENOPROTOOPT; @@ -659,13 +639,13 @@ static void ccid3_hc_rx_set_state(struct sock *sk, enum ccid3_hc_rx_states state) { struct ccid3_hc_rx_sock *hcrx = ccid3_hc_rx_sk(sk); - enum ccid3_hc_rx_states oldstate = hcrx->ccid3hcrx_state; + enum ccid3_hc_rx_states oldstate = hcrx->state; ccid3_pr_debug("%s(%p) %-8.8s -> %s\n", dccp_role(sk), sk, ccid3_rx_state_name(oldstate), ccid3_rx_state_name(state)); WARN_ON(state == oldstate); - hcrx->ccid3hcrx_state = state; + hcrx->state = state; } static void ccid3_hc_rx_send_feedback(struct sock *sk, @@ -677,15 +657,15 @@ static void ccid3_hc_rx_send_feedback(struct sock *sk, ktime_t now; s64 delta = 0; - if (unlikely(hcrx->ccid3hcrx_state == TFRC_RSTATE_TERM)) + if (unlikely(hcrx->state == TFRC_RSTATE_TERM)) return; now = ktime_get_real(); switch (fbtype) { case CCID3_FBACK_INITIAL: - hcrx->ccid3hcrx_x_recv = 0; - hcrx->ccid3hcrx_pinv = ~0U; /* see RFC 4342, 8.5 */ + hcrx->x_recv = 0; + hcrx->p_inverse = ~0U; /* see RFC 4342, 8.5 */ break; case CCID3_FBACK_PARAM_CHANGE: /* @@ -698,27 +678,26 @@ static void ccid3_hc_rx_send_feedback(struct sock *sk, * the number of bytes since last feedback. * This is a safe fallback, since X is bounded above by X_calc. */ - if (hcrx->ccid3hcrx_x_recv > 0) + if (hcrx->x_recv > 0) break; /* fall through */ case CCID3_FBACK_PERIODIC: - delta = ktime_us_delta(now, hcrx->ccid3hcrx_tstamp_last_feedback); + delta = ktime_us_delta(now, hcrx->tstamp_last_feedback); if (delta <= 0) DCCP_BUG("delta (%ld) <= 0", (long)delta); else - hcrx->ccid3hcrx_x_recv = - scaled_div32(hcrx->ccid3hcrx_bytes_recv, delta); + hcrx->x_recv = scaled_div32(hcrx->bytes_recv, delta); break; default: return; } - ccid3_pr_debug("Interval %ldusec, X_recv=%u, 1/p=%u\n", (long)delta, - hcrx->ccid3hcrx_x_recv, hcrx->ccid3hcrx_pinv); + ccid3_pr_debug("Interval %ldusec, X_recv=%u, 1/p=%u\n", + (long)delta, hcrx->x_recv, hcrx->p_inverse); - hcrx->ccid3hcrx_tstamp_last_feedback = now; - hcrx->ccid3hcrx_last_counter = dccp_hdr(skb)->dccph_ccval; - hcrx->ccid3hcrx_bytes_recv = 0; + hcrx->tstamp_last_feedback = now; + hcrx->last_counter = dccp_hdr(skb)->dccph_ccval; + hcrx->bytes_recv = 0; dp->dccps_hc_rx_insert_options = 1; dccp_send_ack(sk); @@ -737,8 +716,8 @@ static int ccid3_hc_rx_insert_options(struct sock *sk, struct sk_buff *skb) if (dccp_packet_without_ack(skb)) return 0; - x_recv = htonl(hcrx->ccid3hcrx_x_recv); - pinv = htonl(hcrx->ccid3hcrx_pinv); + x_recv = htonl(hcrx->x_recv); + pinv = htonl(hcrx->p_inverse); if (dccp_insert_option(sk, skb, TFRC_OPT_LOSS_EVENT_RATE, &pinv, sizeof(pinv)) || @@ -764,22 +743,23 @@ static u32 ccid3_first_li(struct sock *sk) u32 x_recv, p, delta; u64 fval; - if (hcrx->ccid3hcrx_rtt == 0) { + if (hcrx->rtt == 0) { DCCP_WARN("No RTT estimate available, using fallback RTT\n"); - hcrx->ccid3hcrx_rtt = DCCP_FALLBACK_RTT; + hcrx->rtt = DCCP_FALLBACK_RTT; } - delta = ktime_to_us(net_timedelta(hcrx->ccid3hcrx_tstamp_last_feedback)); - x_recv = scaled_div32(hcrx->ccid3hcrx_bytes_recv, delta); + delta = ktime_to_us(net_timedelta(hcrx->tstamp_last_feedback)); + x_recv = scaled_div32(hcrx->bytes_recv, delta); if (x_recv == 0) { /* would also trigger divide-by-zero */ DCCP_WARN("X_recv==0\n"); - if ((x_recv = hcrx->ccid3hcrx_x_recv) == 0) { + if (hcrx->x_recv == 0) { DCCP_BUG("stored value of X_recv is zero"); return ~0U; } + x_recv = hcrx->x_recv; } - fval = scaled_div(hcrx->ccid3hcrx_s, hcrx->ccid3hcrx_rtt); + fval = scaled_div(hcrx->s, hcrx->rtt); fval = scaled_div32(fval, x_recv); p = tfrc_calc_x_reverse_lookup(fval); @@ -796,14 +776,14 @@ static void ccid3_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) const u32 ndp = dccp_sk(sk)->dccps_options_received.dccpor_ndp; const bool is_data_packet = dccp_data_packet(skb); - if (unlikely(hcrx->ccid3hcrx_state == TFRC_RSTATE_NO_DATA)) { + if (unlikely(hcrx->state == TFRC_RSTATE_NO_DATA)) { if (is_data_packet) { const u32 payload = skb->len - dccp_hdr(skb)->dccph_doff * 4; do_feedback = CCID3_FBACK_INITIAL; ccid3_hc_rx_set_state(sk, TFRC_RSTATE_DATA); - hcrx->ccid3hcrx_s = payload; + hcrx->s = payload; /* - * Not necessary to update ccid3hcrx_bytes_recv here, + * Not necessary to update bytes_recv here, * since X_recv = 0 for the first feedback packet (cf. * RFC 3448, 6.3) -- gerrit */ @@ -811,7 +791,7 @@ static void ccid3_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) goto update_records; } - if (tfrc_rx_hist_duplicate(&hcrx->ccid3hcrx_hist, skb)) + if (tfrc_rx_hist_duplicate(&hcrx->hist, skb)) return; /* done receiving */ if (is_data_packet) { @@ -819,22 +799,21 @@ static void ccid3_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) /* * Update moving-average of s and the sum of received payload bytes */ - hcrx->ccid3hcrx_s = tfrc_ewma(hcrx->ccid3hcrx_s, payload, 9); - hcrx->ccid3hcrx_bytes_recv += payload; + hcrx->s = tfrc_ewma(hcrx->s, payload, 9); + hcrx->bytes_recv += payload; } /* * Handle pending losses and otherwise check for new loss */ - if (tfrc_rx_hist_loss_pending(&hcrx->ccid3hcrx_hist) && - tfrc_rx_handle_loss(&hcrx->ccid3hcrx_hist, - &hcrx->ccid3hcrx_li_hist, - skb, ndp, ccid3_first_li, sk) ) { + if (tfrc_rx_hist_loss_pending(&hcrx->hist) && + tfrc_rx_handle_loss(&hcrx->hist, &hcrx->li_hist, + skb, ndp, ccid3_first_li, sk)) { do_feedback = CCID3_FBACK_PARAM_CHANGE; goto done_receiving; } - if (tfrc_rx_hist_new_loss_indicated(&hcrx->ccid3hcrx_hist, skb, ndp)) + if (tfrc_rx_hist_new_loss_indicated(&hcrx->hist, skb, ndp)) goto update_records; /* @@ -843,17 +822,17 @@ static void ccid3_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) if (unlikely(!is_data_packet)) goto update_records; - if (!tfrc_lh_is_initialised(&hcrx->ccid3hcrx_li_hist)) { - const u32 sample = tfrc_rx_hist_sample_rtt(&hcrx->ccid3hcrx_hist, skb); + if (!tfrc_lh_is_initialised(&hcrx->li_hist)) { + const u32 sample = tfrc_rx_hist_sample_rtt(&hcrx->hist, skb); /* * Empty loss history: no loss so far, hence p stays 0. * Sample RTT values, since an RTT estimate is required for the * computation of p when the first loss occurs; RFC 3448, 6.3.1. */ if (sample != 0) - hcrx->ccid3hcrx_rtt = tfrc_ewma(hcrx->ccid3hcrx_rtt, sample, 9); + hcrx->rtt = tfrc_ewma(hcrx->rtt, sample, 9); - } else if (tfrc_lh_update_i_mean(&hcrx->ccid3hcrx_li_hist, skb)) { + } else if (tfrc_lh_update_i_mean(&hcrx->li_hist, skb)) { /* * Step (3) of [RFC 3448, 6.1]: Recompute I_mean and, if I_mean * has decreased (resp. p has increased), send feedback now. @@ -864,11 +843,11 @@ static void ccid3_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) /* * Check if the periodic once-per-RTT feedback is due; RFC 4342, 10.3 */ - if (SUB16(dccp_hdr(skb)->dccph_ccval, hcrx->ccid3hcrx_last_counter) > 3) + if (SUB16(dccp_hdr(skb)->dccph_ccval, hcrx->last_counter) > 3) do_feedback = CCID3_FBACK_PERIODIC; update_records: - tfrc_rx_hist_add_packet(&hcrx->ccid3hcrx_hist, skb, ndp); + tfrc_rx_hist_add_packet(&hcrx->hist, skb, ndp); done_receiving: if (do_feedback) @@ -879,9 +858,9 @@ static int ccid3_hc_rx_init(struct ccid *ccid, struct sock *sk) { struct ccid3_hc_rx_sock *hcrx = ccid_priv(ccid); - hcrx->ccid3hcrx_state = TFRC_RSTATE_NO_DATA; - tfrc_lh_init(&hcrx->ccid3hcrx_li_hist); - return tfrc_rx_hist_alloc(&hcrx->ccid3hcrx_hist); + hcrx->state = TFRC_RSTATE_NO_DATA; + tfrc_lh_init(&hcrx->li_hist); + return tfrc_rx_hist_alloc(&hcrx->hist); } static void ccid3_hc_rx_exit(struct sock *sk) @@ -890,8 +869,8 @@ static void ccid3_hc_rx_exit(struct sock *sk) ccid3_hc_rx_set_state(sk, TFRC_RSTATE_TERM); - tfrc_rx_hist_purge(&hcrx->ccid3hcrx_hist); - tfrc_lh_cleanup(&hcrx->ccid3hcrx_li_hist); + tfrc_rx_hist_purge(&hcrx->hist); + tfrc_lh_cleanup(&hcrx->li_hist); } static void ccid3_hc_rx_get_info(struct sock *sk, struct tcp_info *info) @@ -903,9 +882,9 @@ static void ccid3_hc_rx_get_info(struct sock *sk, struct tcp_info *info) return; hcrx = ccid3_hc_rx_sk(sk); - info->tcpi_ca_state = hcrx->ccid3hcrx_state; + info->tcpi_ca_state = hcrx->state; info->tcpi_options |= TCPI_OPT_TIMESTAMPS; - info->tcpi_rcv_rtt = hcrx->ccid3hcrx_rtt; + info->tcpi_rcv_rtt = hcrx->rtt; } static int ccid3_hc_rx_getsockopt(struct sock *sk, const int optname, int len, @@ -924,10 +903,10 @@ static int ccid3_hc_rx_getsockopt(struct sock *sk, const int optname, int len, case DCCP_SOCKOPT_CCID_RX_INFO: if (len < sizeof(rx_info)) return -EINVAL; - rx_info.tfrcrx_x_recv = hcrx->ccid3hcrx_x_recv; - rx_info.tfrcrx_rtt = hcrx->ccid3hcrx_rtt; - rx_info.tfrcrx_p = hcrx->ccid3hcrx_pinv == 0 ? ~0U : - scaled_div(1, hcrx->ccid3hcrx_pinv); + rx_info.tfrcrx_x_recv = hcrx->x_recv; + rx_info.tfrcrx_rtt = hcrx->rtt; + rx_info.tfrcrx_p = hcrx->p_inverse == 0 ? ~0U : + scaled_div(1, hcrx->p_inverse); len = sizeof(rx_info); val = &rx_info; break; --- a/net/dccp/ccids/ccid3.h +++ b/net/dccp/ccids/ccid3.h @@ -77,44 +77,43 @@ enum ccid3_hc_tx_states { /** struct ccid3_hc_tx_sock - CCID3 sender half-connection socket * - * @ccid3hctx_x - Current sending rate in 64 * bytes per second - * @ccid3hctx_x_recv - Receive rate in 64 * bytes per second - * @ccid3hctx_x_calc - Calculated rate in bytes per second - * @ccid3hctx_rtt - Estimate of current round trip time in usecs - * @ccid3hctx_p - Current loss event rate (0-1) scaled by 1000000 - * @ccid3hctx_s - Packet size in bytes - * @ccid3hctx_t_rto - Nofeedback Timer setting in usecs - * @ccid3hctx_t_ipi - Interpacket (send) interval (RFC 3448, 4.6) in usecs - * @ccid3hctx_state - Sender state, one of %ccid3_hc_tx_states - * @ccid3hctx_last_win_count - Last window counter sent - * @ccid3hctx_t_last_win_count - Timestamp of earliest packet - * with last_win_count value sent - * @ccid3hctx_no_feedback_timer - Handle to no feedback timer - * @ccid3hctx_t_ld - Time last doubled during slow start - * @ccid3hctx_t_nom - Nominal send time of next packet - * @ccid3hctx_delta - Send timer delta (RFC 3448, 4.6) in usecs - * @ccid3hctx_hist - Packet history - * @ccid3hctx_options_received - Parsed set of retrieved options + * @x - Current sending rate in 64 * bytes per second + * @x_recv - Receive rate in 64 * bytes per second + * @x_calc - Calculated rate in bytes per second + * @rtt - Estimate of current round trip time in usecs + * @p - Current loss event rate (0-1) scaled by 1000000 + * @s - Packet size in bytes + * @t_rto - Nofeedback Timer setting in usecs + * @t_ipi - Interpacket (send) interval (RFC 3448, 4.6) in usecs + * @state - Sender state, one of %ccid3_hc_tx_states + * @last_win_count - Last window counter sent + * @t_last_win_count - Timestamp of earliest packet with + * last_win_count value sent + * @no_feedback_timer - Handle to no feedback timer + * @t_ld - Time last doubled during slow start + * @t_nom - Nominal send time of next packet + * @delta - Send timer delta (RFC 3448, 4.6) in usecs + * @hist - Packet history + * @options_received - Parsed set of retrieved options */ struct ccid3_hc_tx_sock { - struct tfrc_tx_info ccid3hctx_tfrc; -#define ccid3hctx_x ccid3hctx_tfrc.tfrctx_x -#define ccid3hctx_x_recv ccid3hctx_tfrc.tfrctx_x_recv -#define ccid3hctx_x_calc ccid3hctx_tfrc.tfrctx_x_calc -#define ccid3hctx_rtt ccid3hctx_tfrc.tfrctx_rtt -#define ccid3hctx_p ccid3hctx_tfrc.tfrctx_p -#define ccid3hctx_t_rto ccid3hctx_tfrc.tfrctx_rto -#define ccid3hctx_t_ipi ccid3hctx_tfrc.tfrctx_ipi - u16 ccid3hctx_s; - enum ccid3_hc_tx_states ccid3hctx_state:8; - u8 ccid3hctx_last_win_count; - ktime_t ccid3hctx_t_last_win_count; - struct timer_list ccid3hctx_no_feedback_timer; - ktime_t ccid3hctx_t_ld; - ktime_t ccid3hctx_t_nom; - u32 ccid3hctx_delta; - struct tfrc_tx_hist_entry *ccid3hctx_hist; - struct ccid3_options_received ccid3hctx_options_received; + u64 x; + u64 x_recv; + u32 x_calc; + u32 rtt; + u32 p; + u32 t_rto; + u32 t_ipi; + u16 s; + enum ccid3_hc_tx_states state:8; + u8 last_win_count; + ktime_t t_last_win_count; + struct timer_list no_feedback_timer; + ktime_t t_ld; + ktime_t t_nom; + u32 delta; + struct tfrc_tx_hist_entry *hist; + struct ccid3_options_received options_received; }; static inline struct ccid3_hc_tx_sock *ccid3_hc_tx_sk(const struct sock *sk) @@ -133,32 +132,32 @@ enum ccid3_hc_rx_states { /** struct ccid3_hc_rx_sock - CCID3 receiver half-connection socket * - * @ccid3hcrx_x_recv - Receiver estimate of send rate (RFC 3448 4.3) - * @ccid3hcrx_rtt - Receiver estimate of rtt (non-standard) - * @ccid3hcrx_p - Current loss event rate (RFC 3448 5.4) - * @ccid3hcrx_last_counter - Tracks window counter (RFC 4342, 8.1) - * @ccid3hcrx_state - Receiver state, one of %ccid3_hc_rx_states - * @ccid3hcrx_bytes_recv - Total sum of DCCP payload bytes - * @ccid3hcrx_x_recv - Receiver estimate of send rate (RFC 3448, sec. 4.3) - * @ccid3hcrx_rtt - Receiver estimate of RTT - * @ccid3hcrx_tstamp_last_feedback - Time at which last feedback was sent - * @ccid3hcrx_tstamp_last_ack - Time at which last feedback was sent - * @ccid3hcrx_hist - Packet history (loss detection + RTT sampling) - * @ccid3hcrx_li_hist - Loss Interval database - * @ccid3hcrx_s - Received packet size in bytes - * @ccid3hcrx_pinv - Inverse of Loss Event Rate (RFC 4342, sec. 8.5) + * @x_recv - Receiver estimate of send rate (RFC 3448 4.3) + * @rtt - Receiver estimate of rtt (non-standard) + * @p - Current loss event rate (RFC 3448 5.4) + * @last_counter - Tracks window counter (RFC 4342, 8.1) + * @state - Receiver state, one of %ccid3_hc_rx_states + * @bytes_recv - Total sum of DCCP payload bytes + * @x_recv - Receiver estimate of send rate (RFC 3448, sec. 4.3) + * @rtt - Receiver estimate of RTT + * @tstamp_last_feedback - Time at which last feedback was sent + * @tstamp_last_ack - Time at which last feedback was sent + * @hist - Packet history (loss detection + RTT sampling) + * @li_hist - Loss Interval database + * @s - Received packet size in bytes + * @p_inverse - Inverse of Loss Event Rate (RFC 4342, sec. 8.5) */ struct ccid3_hc_rx_sock { - u8 ccid3hcrx_last_counter:4; - enum ccid3_hc_rx_states ccid3hcrx_state:8; - u32 ccid3hcrx_bytes_recv; - u32 ccid3hcrx_x_recv; - u32 ccid3hcrx_rtt; - ktime_t ccid3hcrx_tstamp_last_feedback; - struct tfrc_rx_hist ccid3hcrx_hist; - struct tfrc_loss_hist ccid3hcrx_li_hist; - u16 ccid3hcrx_s; -#define ccid3hcrx_pinv ccid3hcrx_li_hist.i_mean + u8 last_counter:4; + enum ccid3_hc_rx_states state:8; + u32 bytes_recv; + u32 x_recv; + u32 rtt; + ktime_t tstamp_last_feedback; + struct tfrc_rx_hist hist; + struct tfrc_loss_hist li_hist; + u16 s; +#define p_inverse li_hist.i_mean }; static inline struct ccid3_hc_rx_sock *ccid3_hc_rx_sk(const struct sock *sk) --- a/net/dccp/probe.c +++ b/net/dccp/probe.c @@ -90,10 +90,8 @@ static int jdccp_sendmsg(struct kiocb *iocb, struct sock *sk, "%llu %llu %d\n", NIPQUAD(inet->saddr), ntohs(inet->sport), NIPQUAD(inet->daddr), ntohs(inet->dport), size, - hctx->ccid3hctx_s, hctx->ccid3hctx_rtt, - hctx->ccid3hctx_p, hctx->ccid3hctx_x_calc, - hctx->ccid3hctx_x_recv >> 6, - hctx->ccid3hctx_x >> 6, hctx->ccid3hctx_t_ipi); + hctx->s, hctx->rtt, hctx->p, hctx->x_calc, + hctx->x_recv >> 6, hctx->x >> 6, hctx->t_ipi); else printl("%d.%d.%d.%d:%u %d.%d.%d.%d:%u %d\n", NIPQUAD(inet->saddr), ntohs(inet->sport), -- 1.5.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug 2008-05-27 8:32 ` [PATCH 3/7] [CCID-3]: Remove ccid3hc{tx,rx}_ prefixes Gerrit Renker @ 2008-05-27 8:32 ` Gerrit Renker 2008-05-27 8:32 ` [PATCH 5/7] [CCID-3]: Remove ugly RTT-sampling history lookup Gerrit Renker 2008-05-27 13:35 ` [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug David Miller 2008-05-27 13:26 ` [PATCH 3/7] [CCID-3]: Remove ccid3hc{tx,rx}_ prefixes David Miller 1 sibling, 2 replies; 21+ messages in thread From: Gerrit Renker @ 2008-05-27 8:32 UTC (permalink / raw) To: davem; +Cc: dccp, netdev, Gerrit Renker The identification of this bug is thanks to Cheng Wei and Tomasz Grobelny. To avoid divide-by-zero, the implementation previously ignored RTTs smaller than 4 microseconds when performing integer division RTT/4. When the RTT reached a value less than 4 microseconds (as observed on loopback), this prevented the Window Counter CCVal value from advancing. As a result, the receiver stopped sending feedback. This in turn caused non-ending expiries of the nofeedback timer at the sender, so that the sending rate was progressively reduced until reaching the minimum of one packet per 64 seconds. The patch fixes this bug by handling integer division more intelligently. Due to consistent use of dccp_sample_rtt(), divide-by-zero-RTT is avoided. Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> --- net/dccp/ccids/ccid3.c | 13 ++++--------- 1 files changed, 4 insertions(+), 9 deletions(-) --- a/net/dccp/ccids/ccid3.c +++ b/net/dccp/ccids/ccid3.c @@ -181,22 +181,17 @@ static inline void ccid3_hc_tx_update_s(struct ccid3_hc_tx_sock *hctx, int len) /* * Update Window Counter using the algorithm from [RFC 4342, 8.1]. - * The algorithm is not applicable if RTT < 4 microseconds. + * As elsewhere, RTT > 0 is assumed by using dccp_sample_rtt(). */ static inline void ccid3_hc_tx_update_win_count(struct ccid3_hc_tx_sock *hctx, ktime_t now) { - u32 quarter_rtts; - - if (unlikely(hctx->rtt < 4)) /* avoid divide-by-zero */ - return; - - quarter_rtts = ktime_us_delta(now, hctx->t_last_win_count); - quarter_rtts /= hctx->rtt / 4; + u32 delta = ktime_us_delta(now, hctx->t_last_win_count), + quarter_rtts = (4 * delta) / hctx->rtt; if (quarter_rtts > 0) { hctx->t_last_win_count = now; - hctx->last_win_count += min_t(u32, quarter_rtts, 5); + hctx->last_win_count += min(quarter_rtts, 5U); hctx->last_win_count &= 0xF; /* mod 16 */ } } -- 1.5.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/7] [CCID-3]: Remove ugly RTT-sampling history lookup 2008-05-27 8:32 ` [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug Gerrit Renker @ 2008-05-27 8:32 ` Gerrit Renker 2008-05-27 8:32 ` [PATCH 6/7] [CCID-2/3]: Fix sparse warnings Gerrit Renker 2008-05-27 13:36 ` [PATCH 5/7] [CCID-3]: Remove ugly RTT-sampling history lookup David Miller 2008-05-27 13:35 ` [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug David Miller 1 sibling, 2 replies; 21+ messages in thread From: Gerrit Renker @ 2008-05-27 8:32 UTC (permalink / raw) To: davem; +Cc: dccp, netdev, Gerrit Renker This removes the RTT-sampling function tfrc_tx_hist_rtt(), since 1. it suffered from complex passing of return values (the return value both indicated successful lookup while the value doubled as RTT sample); 2. when for some odd reason the sample value equalled 0, this triggered a bug warning about "bogus Ack", due to the ambiguity of the return value; 3. on a passive host which has not sent anything the TX history is empty and thus will lead to unwanted "bogus Ack" warnings such as ccid3_hc_tx_packet_recv: server(e7b7d518): DATAACK with bogus ACK-28197148 ccid3_hc_tx_packet_recv: server(e7b7d518): DATAACK with bogus ACK-26641606. The fix is to replace the implicit encoding by performing the steps manually. Furthermore, the "bogus Ack" warning has been removed, since it can actually be triggered due to several reasons (network reordering, old packet, (3) above), hence it is not very useful. Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> --- net/dccp/ccids/ccid3.c | 34 ++++++++++++++++------------- net/dccp/ccids/lib/packet_history.c | 40 ----------------------------------- net/dccp/ccids/lib/packet_history.h | 22 ++++++++++++++++-- 3 files changed, 38 insertions(+), 58 deletions(-) --- a/net/dccp/ccids/ccid3.c +++ b/net/dccp/ccids/ccid3.c @@ -367,6 +367,7 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) { struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk); struct ccid3_options_received *opt_recv = &hctx->options_received; + struct tfrc_tx_hist_entry *acked; ktime_t now; unsigned long t_nfb; u32 pinv, r_sample; @@ -380,17 +381,24 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) hctx->state != TFRC_SSTATE_NO_FBACK) return; - now = ktime_get_real(); - - /* Estimate RTT from history if ACK number is valid */ - r_sample = tfrc_tx_hist_rtt(hctx->hist, - DCCP_SKB_CB(skb)->dccpd_ack_seq, now); - if (r_sample == 0) { - DCCP_WARN("%s(%p): %s with bogus ACK-%llu\n", dccp_role(sk), sk, - dccp_packet_name(DCCP_SKB_CB(skb)->dccpd_type), - (unsigned long long)DCCP_SKB_CB(skb)->dccpd_ack_seq); + /* + * Locate the acknowledged packet in the TX history. + * + * Returning "entry not found" here can for instance happen when + * - the host has not sent out anything (e.g. a passive server), + * - the Ack is outdated (packet with higher Ack number was received), + * - it is a bogus Ack (for a packet not sent on this connection). + */ + acked = tfrc_tx_hist_find_entry(hctx->hist, dccp_hdr_ack_seq(skb)); + if (acked == NULL) return; - } + /* For the sake of RTT sampling, ignore/remove all older entries */ + tfrc_tx_hist_purge(&acked->next); + + /* Update the moving average for the RTT estimate (RFC 3448, 4.3) */ + now = ktime_get_real(); + r_sample = dccp_sample_rtt(sk, ktime_us_delta(now, acked->stamp)); + hctx->rtt = tfrc_ewma(hctx->rtt, r_sample, 9); /* Update receive rate in units of 64 * bytes/second */ hctx->x_recv = opt_recv->ccid3or_receive_rate; @@ -402,11 +410,7 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) hctx->p = 0; else /* can not exceed 100% */ hctx->p = scaled_div(1, pinv); - /* - * Validate new RTT sample and update moving average - */ - r_sample = dccp_sample_rtt(sk, r_sample); - hctx->rtt = tfrc_ewma(hctx->rtt, r_sample, 9); + /* * Update allowed sending rate X as per draft rfc3448bis-00, 4.2/3 */ --- a/net/dccp/ccids/lib/packet_history.c +++ b/net/dccp/ccids/lib/packet_history.c @@ -40,18 +40,6 @@ #include "packet_history.h" #include "../../dccp.h" -/** - * tfrc_tx_hist_entry - Simple singly-linked TX history list - * @next: next oldest entry (LIFO order) - * @seqno: sequence number of this entry - * @stamp: send time of packet with sequence number @seqno - */ -struct tfrc_tx_hist_entry { - struct tfrc_tx_hist_entry *next; - u64 seqno; - ktime_t stamp; -}; - /* * Transmitter History Routines */ @@ -73,15 +61,6 @@ void tfrc_tx_packet_history_exit(void) } } -static struct tfrc_tx_hist_entry * - tfrc_tx_hist_find_entry(struct tfrc_tx_hist_entry *head, u64 seqno) -{ - while (head != NULL && head->seqno != seqno) - head = head->next; - - return head; -} - int tfrc_tx_hist_add(struct tfrc_tx_hist_entry **headp, u64 seqno) { struct tfrc_tx_hist_entry *entry = kmem_cache_alloc(tfrc_tx_hist_slab, gfp_any()); @@ -111,25 +90,6 @@ void tfrc_tx_hist_purge(struct tfrc_tx_hist_entry **headp) } EXPORT_SYMBOL_GPL(tfrc_tx_hist_purge); -u32 tfrc_tx_hist_rtt(struct tfrc_tx_hist_entry *head, const u64 seqno, - const ktime_t now) -{ - u32 rtt = 0; - struct tfrc_tx_hist_entry *packet = tfrc_tx_hist_find_entry(head, seqno); - - if (packet != NULL) { - rtt = ktime_us_delta(now, packet->stamp); - /* - * Garbage-collect older (irrelevant) entries: - */ - tfrc_tx_hist_purge(&packet->next); - } - - return rtt; -} -EXPORT_SYMBOL_GPL(tfrc_tx_hist_rtt); - - /* * Receiver History Routines */ --- a/net/dccp/ccids/lib/packet_history.h +++ b/net/dccp/ccids/lib/packet_history.h @@ -40,12 +40,28 @@ #include <linux/slab.h> #include "tfrc.h" -struct tfrc_tx_hist_entry; +/** + * tfrc_tx_hist_entry - Simple singly-linked TX history list + * @next: next oldest entry (LIFO order) + * @seqno: sequence number of this entry + * @stamp: send time of packet with sequence number @seqno + */ +struct tfrc_tx_hist_entry { + struct tfrc_tx_hist_entry *next; + u64 seqno; + ktime_t stamp; +}; + +static inline struct tfrc_tx_hist_entry * + tfrc_tx_hist_find_entry(struct tfrc_tx_hist_entry *head, u64 seqno) +{ + while (head != NULL && head->seqno != seqno) + head = head->next; + return head; +} extern int tfrc_tx_hist_add(struct tfrc_tx_hist_entry **headp, u64 seqno); extern void tfrc_tx_hist_purge(struct tfrc_tx_hist_entry **headp); -extern u32 tfrc_tx_hist_rtt(struct tfrc_tx_hist_entry *head, - const u64 seqno, const ktime_t now); /* Subtraction a-b modulo-16, respects circular wrap-around */ #define SUB16(a, b) (((a) + 16 - (b)) & 0xF) -- 1.5.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 6/7] [CCID-2/3]: Fix sparse warnings 2008-05-27 8:32 ` [PATCH 5/7] [CCID-3]: Remove ugly RTT-sampling history lookup Gerrit Renker @ 2008-05-27 8:32 ` Gerrit Renker 2008-05-27 8:32 ` [PATCH 7/7] [TCP][DCCP]: Consolidate duplicate code which does RFC3390 conversion Gerrit Renker 2008-05-27 13:37 ` [PATCH 6/7] [CCID-2/3]: Fix sparse warnings David Miller 2008-05-27 13:36 ` [PATCH 5/7] [CCID-3]: Remove ugly RTT-sampling history lookup David Miller 1 sibling, 2 replies; 21+ messages in thread From: Gerrit Renker @ 2008-05-27 8:32 UTC (permalink / raw) To: davem; +Cc: dccp, netdev, Gerrit Renker This patch fixes the following two sparse warnings: 1. Due to a nested min(max()) expression, sparse gave the following warnings: net/dccp/ccids/ccid3.c:91:21: warning: symbol '__x' shadows an earlier one net/dccp/ccids/ccid3.c:91:21: warning: symbol '__y' shadows an earlier one (Didn't use clamp() here since this part is liketly to be split later, when implementing the changes from draft rfc3448bis.) 2. Declaration of function prototypes was in .c instead of .h file. This resulted in "should it be static" warnings. Corrected by moving the declarations to the tfrc.h file instead of the tfrc.c file. Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> --- net/dccp/ccids/ccid3.c | 3 ++- net/dccp/ccids/lib/tfrc.c | 8 -------- net/dccp/ccids/lib/tfrc.h | 11 +++++++++-- 3 files changed, 11 insertions(+), 11 deletions(-) --- a/net/dccp/ccids/ccid3.c +++ b/net/dccp/ccids/ccid3.c @@ -150,7 +150,8 @@ static void ccid3_hc_tx_update_x(struct sock *sk, ktime_t *stamp) } else if (ktime_us_delta(now, hctx->t_ld) - (s64)hctx->rtt >= 0) { - hctx->x = max(min(2 * hctx->x, min_rate), + hctx->x = min(2 * hctx->x, min_rate); + hctx->x = max(hctx->x, scaled_div(((u64)hctx->s) << 6, hctx->rtt)); hctx->t_ld = now; } --- a/net/dccp/ccids/lib/tfrc.c +++ b/net/dccp/ccids/lib/tfrc.c @@ -14,14 +14,6 @@ module_param(tfrc_debug, bool, 0444); MODULE_PARM_DESC(tfrc_debug, "Enable debug messages"); #endif -extern int tfrc_tx_packet_history_init(void); -extern void tfrc_tx_packet_history_exit(void); -extern int tfrc_rx_packet_history_init(void); -extern void tfrc_rx_packet_history_exit(void); - -extern int tfrc_li_init(void); -extern void tfrc_li_exit(void); - static int __init tfrc_module_init(void) { int rc = tfrc_li_init(); --- a/net/dccp/ccids/lib/tfrc.h +++ b/net/dccp/ccids/lib/tfrc.h @@ -58,7 +58,14 @@ static inline u32 tfrc_ewma(const u32 avg, const u32 newval, const u8 weight) return avg ? (weight * avg + (10 - weight) * newval) / 10 : newval; } -extern u32 tfrc_calc_x(u16 s, u32 R, u32 p); -extern u32 tfrc_calc_x_reverse_lookup(u32 fvalue); +extern u32 tfrc_calc_x(u16 s, u32 R, u32 p); +extern u32 tfrc_calc_x_reverse_lookup(u32 fvalue); +extern int tfrc_tx_packet_history_init(void); +extern void tfrc_tx_packet_history_exit(void); +extern int tfrc_rx_packet_history_init(void); +extern void tfrc_rx_packet_history_exit(void); + +extern int tfrc_li_init(void); +extern void tfrc_li_exit(void); #endif /* _TFRC_H_ */ -- 1.5.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 7/7] [TCP][DCCP]: Consolidate duplicate code which does RFC3390 conversion 2008-05-27 8:32 ` [PATCH 6/7] [CCID-2/3]: Fix sparse warnings Gerrit Renker @ 2008-05-27 8:32 ` Gerrit Renker 2008-05-27 13:37 ` David Miller 2008-05-27 13:37 ` [PATCH 6/7] [CCID-2/3]: Fix sparse warnings David Miller 1 sibling, 1 reply; 21+ messages in thread From: Gerrit Renker @ 2008-05-27 8:32 UTC (permalink / raw) To: davem; +Cc: dccp, netdev, Gerrit Renker This patch consolidates the code common to TCP and CCID-2: * TCP uses RFC 3390 in a packet-oriented manner (tcp_input.c) and * CCID-2 uses RFC 3390 in packet-oriented manner (RFC 4341). Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> --- include/net/tcp.h | 15 +++++++++++++++ net/dccp/ccids/ccid2.c | 8 ++------ net/ipv4/tcp_input.c | 17 ++--------------- 3 files changed, 19 insertions(+), 21 deletions(-) --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -783,6 +783,21 @@ static inline __u32 tcp_current_ssthresh(const struct sock *sk) /* Use define here intentionally to get WARN_ON location shown at the caller */ #define tcp_verify_left_out(tp) WARN_ON(tcp_left_out(tp) > tp->packets_out) +/* + * Convert RFC3390 larger initial windows into an equivalent number of packets. + * + * John Heffner states: + * + * The RFC specifies a window of no more than 4380 bytes + * unless 2*MSS > 4380. Reading the pseudocode in the RFC + * is a bit misleading because they use a clamp at 4380 bytes + * rather than a multiplier in the relevant range. + */ +static inline u32 rfc3390_bytes_to_packets(const u32 bytes) +{ + return bytes < 1095 ? 4 : (bytes > 1460 ? 2 : 3); +} + extern void tcp_enter_cwr(struct sock *sk, const int set_ssthresh); extern __u32 tcp_init_cwnd(struct tcp_sock *tp, struct dst_entry *dst); --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -708,12 +708,8 @@ static int ccid2_hc_tx_init(struct ccid *ccid, struct sock *sk) /* RFC 4341, 5: initialise ssthresh to arbitrarily high (max) value */ hctx->ssthresh = ~0U; - /* - * 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->cwnd = clamp(4380U / dp->dccps_mss_cache, 2U, 4U); + /* Use larger initial windows (RFC 3390, rfc2581bis) */ + hctx->cwnd = rfc3390_bytes_to_packets(dp->dccps_mss_cache); /* Make sure that Ack Ratio is enabled and within bounds. */ max_ratio = DIV_ROUND_UP(hctx->cwnd, 2); --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -808,25 +808,12 @@ void tcp_update_metrics(struct sock *sk) } } -/* Numbers are taken from RFC3390. - * - * John Heffner states: - * - * The RFC specifies a window of no more than 4380 bytes - * unless 2*MSS > 4380. Reading the pseudocode in the RFC - * is a bit misleading because they use a clamp at 4380 bytes - * rather than use a multiplier in the relevant range. - */ __u32 tcp_init_cwnd(struct tcp_sock *tp, struct dst_entry *dst) { __u32 cwnd = (dst ? dst_metric(dst, RTAX_INITCWND) : 0); - if (!cwnd) { - if (tp->mss_cache > 1460) - cwnd = 2; - else - cwnd = (tp->mss_cache > 1095) ? 3 : 4; - } + if (!cwnd) + cwnd = rfc3390_bytes_to_packets(tp->mss_cache); return min_t(__u32, cwnd, tp->snd_cwnd_clamp); } -- 1.5.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 7/7] [TCP][DCCP]: Consolidate duplicate code which does RFC3390 conversion 2008-05-27 8:32 ` [PATCH 7/7] [TCP][DCCP]: Consolidate duplicate code which does RFC3390 conversion Gerrit Renker @ 2008-05-27 13:37 ` David Miller 2008-05-28 6:24 ` [Patch v2 1/1] [tcp/dccp]: " Gerrit Renker 0 siblings, 1 reply; 21+ messages in thread From: David Miller @ 2008-05-27 13:37 UTC (permalink / raw) To: gerrit; +Cc: dccp, netdev From: Gerrit Renker <gerrit@erg.abdn.ac.uk> Date: Tue, 27 May 2008 09:32:47 +0100 > This patch consolidates the code common to TCP and CCID-2: > > * TCP uses RFC 3390 in a packet-oriented manner (tcp_input.c) and > * CCID-2 uses RFC 3390 in packet-oriented manner (RFC 4341). > > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> Depends upon hctx prefix changes which will not be applied, please repsin. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch v2 1/1] [tcp/dccp]: Consolidate duplicate code which does RFC3390 conversion 2008-05-27 13:37 ` David Miller @ 2008-05-28 6:24 ` Gerrit Renker 2008-05-31 13:49 ` [Patch v3 " Gerrit Renker 0 siblings, 1 reply; 21+ messages in thread From: Gerrit Renker @ 2008-05-28 6:24 UTC (permalink / raw) To: David Miller; +Cc: dccp, netdev Please find below recoded patch as requested. >>>>>>>>>>>>>>>>>>>>> Patch v2 <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< tcp/dccp: Consolidate duplicate code which does RFC3390 conversion This patch consolidates the code common to TCP and CCID-2: * TCP uses RFC 3390 in a packet-oriented manner (tcp_input.c) and * CCID-2 uses RFC 3390 in packet-oriented manner (RFC 4341). Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> --- include/net/tcp.h | 15 +++++++++++++++ net/dccp/ccids/ccid2.c | 8 ++------ net/ipv4/tcp_input.c | 17 ++--------------- 3 files changed, 19 insertions(+), 21 deletions(-) --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -808,25 +808,12 @@ void tcp_update_metrics(struct sock *sk) } } -/* Numbers are taken from RFC3390. - * - * John Heffner states: - * - * The RFC specifies a window of no more than 4380 bytes - * unless 2*MSS > 4380. Reading the pseudocode in the RFC - * is a bit misleading because they use a clamp at 4380 bytes - * rather than use a multiplier in the relevant range. - */ __u32 tcp_init_cwnd(struct tcp_sock *tp, struct dst_entry *dst) { __u32 cwnd = (dst ? dst_metric(dst, RTAX_INITCWND) : 0); - if (!cwnd) { - if (tp->mss_cache > 1460) - cwnd = 2; - else - cwnd = (tp->mss_cache > 1095) ? 3 : 4; - } + if (!cwnd) + cwnd = rfc3390_bytes_to_packets(tp->mss_cache); return min_t(__u32, cwnd, tp->snd_cwnd_clamp); } --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -783,6 +783,21 @@ static inline __u32 tcp_current_ssthresh /* Use define here intentionally to get WARN_ON location shown at the caller */ #define tcp_verify_left_out(tp) WARN_ON(tcp_left_out(tp) > tp->packets_out) +/* + * Convert RFC3390 larger initial windows into an equivalent number of packets. + * + * John Heffner states: + * + * The RFC specifies a window of no more than 4380 bytes + * unless 2*MSS > 4380. Reading the pseudocode in the RFC + * is a bit misleading because they use a clamp at 4380 bytes + * rather than a multiplier in the relevant range. + */ +static inline u32 rfc3390_bytes_to_packets(const u32 bytes) +{ + return bytes < 1095 ? 4 : (bytes > 1460 ? 2 : 3); +} + extern void tcp_enter_cwr(struct sock *sk, const int set_ssthresh); extern __u32 tcp_init_cwnd(struct tcp_sock *tp, struct dst_entry *dst); --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -711,12 +711,8 @@ static int ccid2_hc_tx_init(struct ccid /* RFC 4341, 5: initialise ssthresh to arbitrarily high (max) value */ hctx->ccid2hctx_ssthresh = ~0U; - /* - * 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 = clamp(4380U / dp->dccps_mss_cache, 2U, 4U); + /* Use larger initial windows (RFC 3390, rfc2581bis) */ + hctx->ccid2hctx_cwnd = rfc3390_bytes_to_packets(dp->dccps_mss_cache); /* Make sure that Ack Ratio is enabled and within bounds. */ max_ratio = DIV_ROUND_UP(hctx->ccid2hctx_cwnd, 2); ^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch v3 1/1] [tcp/dccp]: Consolidate duplicate code which does RFC3390 conversion 2008-05-28 6:24 ` [Patch v2 1/1] [tcp/dccp]: " Gerrit Renker @ 2008-05-31 13:49 ` Gerrit Renker 0 siblings, 0 replies; 21+ messages in thread From: Gerrit Renker @ 2008-05-31 13:49 UTC (permalink / raw) To: David Miller, dccp, netdev Please disregard previous version of this patch, which had an error: when `bytes' == 1095, the sender can still send 4 full windows (4*1095 = 4380). The inter-diff to the previous version is: - return bytes < 1095 ? 4 : (bytes > 1460 ? 2 : 3); + return bytes <= 1095 ? 4 : (bytes > 1460 ? 2 : 3); -----------------------------> Patch v2 <------------------------------------------ tcp/dccp: Consolidate duplicate code which does RFC3390 conversion This patch consolidates the code common to TCP and CCID-2: * TCP uses RFC 3390 in a packet-oriented manner (tcp_input.c) and * CCID-2 uses RFC 3390 in packet-oriented manner (RFC 4341). Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> --- include/net/tcp.h | 15 +++++++++++++++ net/dccp/ccids/ccid2.c | 8 ++------ net/ipv4/tcp_input.c | 17 ++--------------- 3 files changed, 19 insertions(+), 21 deletions(-) --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -783,6 +783,21 @@ static inline __u32 tcp_current_ssthresh /* Use define here intentionally to get WARN_ON location shown at the caller */ #define tcp_verify_left_out(tp) WARN_ON(tcp_left_out(tp) > tp->packets_out) +/* + * Convert RFC3390 larger initial windows into an equivalent number of packets. + * + * John Heffner states: + * + * The RFC specifies a window of no more than 4380 bytes + * unless 2*MSS > 4380. Reading the pseudocode in the RFC + * is a bit misleading because they use a clamp at 4380 bytes + * rather than a multiplier in the relevant range. + */ +static inline u32 rfc3390_bytes_to_packets(const u32 bytes) +{ + return bytes <= 1095 ? 4 : (bytes > 1460 ? 2 : 3); +} + extern void tcp_enter_cwr(struct sock *sk, const int set_ssthresh); extern __u32 tcp_init_cwnd(struct tcp_sock *tp, struct dst_entry *dst); --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -808,25 +808,12 @@ void tcp_update_metrics(struct sock *sk) } } -/* Numbers are taken from RFC3390. - * - * John Heffner states: - * - * The RFC specifies a window of no more than 4380 bytes - * unless 2*MSS > 4380. Reading the pseudocode in the RFC - * is a bit misleading because they use a clamp at 4380 bytes - * rather than use a multiplier in the relevant range. - */ __u32 tcp_init_cwnd(struct tcp_sock *tp, struct dst_entry *dst) { __u32 cwnd = (dst ? dst_metric(dst, RTAX_INITCWND) : 0); - if (!cwnd) { - if (tp->mss_cache > 1460) - cwnd = 2; - else - cwnd = (tp->mss_cache > 1095) ? 3 : 4; - } + if (!cwnd) + cwnd = rfc3390_bytes_to_packets(tp->mss_cache); return min_t(__u32, cwnd, tp->snd_cwnd_clamp); } --- a/net/dccp/ccids/ccid2.c +++ b/net/dccp/ccids/ccid2.c @@ -711,12 +711,8 @@ static int ccid2_hc_tx_init(struct ccid /* RFC 4341, 5: initialise ssthresh to arbitrarily high (max) value */ hctx->ccid2hctx_ssthresh = ~0U; - /* - * 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 = clamp(4380U / dp->dccps_mss_cache, 2U, 4U); + /* Use larger initial windows (RFC 3390, rfc2581bis) */ + hctx->ccid2hctx_cwnd = rfc3390_bytes_to_packets(dp->dccps_mss_cache); /* Make sure that Ack Ratio is enabled and within bounds. */ max_ratio = DIV_ROUND_UP(hctx->ccid2hctx_cwnd, 2); ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 6/7] [CCID-2/3]: Fix sparse warnings 2008-05-27 8:32 ` [PATCH 6/7] [CCID-2/3]: Fix sparse warnings Gerrit Renker 2008-05-27 8:32 ` [PATCH 7/7] [TCP][DCCP]: Consolidate duplicate code which does RFC3390 conversion Gerrit Renker @ 2008-05-27 13:37 ` David Miller 1 sibling, 0 replies; 21+ messages in thread From: David Miller @ 2008-05-27 13:37 UTC (permalink / raw) To: gerrit; +Cc: dccp, netdev From: Gerrit Renker <gerrit@erg.abdn.ac.uk> Date: Tue, 27 May 2008 09:32:46 +0100 > This patch fixes the following two sparse warnings: > > 1. Due to a nested min(max()) expression, sparse gave the following warnings: > > net/dccp/ccids/ccid3.c:91:21: warning: symbol '__x' shadows an earlier one > net/dccp/ccids/ccid3.c:91:21: warning: symbol '__y' shadows an earlier one > > (Didn't use clamp() here since this part is liketly to be split later, when > implementing the changes from draft rfc3448bis.) > > 2. Declaration of function prototypes was in .c instead of .h file. > This resulted in "should it be static" warnings. Corrected by moving the > declarations to the tfrc.h file instead of the tfrc.c file. > > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> Conflicts with hctx prefix changes, please respin. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/7] [CCID-3]: Remove ugly RTT-sampling history lookup 2008-05-27 8:32 ` [PATCH 5/7] [CCID-3]: Remove ugly RTT-sampling history lookup Gerrit Renker 2008-05-27 8:32 ` [PATCH 6/7] [CCID-2/3]: Fix sparse warnings Gerrit Renker @ 2008-05-27 13:36 ` David Miller 1 sibling, 0 replies; 21+ messages in thread From: David Miller @ 2008-05-27 13:36 UTC (permalink / raw) To: gerrit; +Cc: dccp, netdev From: Gerrit Renker <gerrit@erg.abdn.ac.uk> Date: Tue, 27 May 2008 09:32:45 +0100 > This removes the RTT-sampling function tfrc_tx_hist_rtt(), since > > 1. it suffered from complex passing of return values (the return value both > indicated successful lookup while the value doubled as RTT sample); > > 2. when for some odd reason the sample value equalled 0, this triggered a bug > warning about "bogus Ack", due to the ambiguity of the return value; > > 3. on a passive host which has not sent anything the TX history is empty and > thus will lead to unwanted "bogus Ack" warnings such as > ccid3_hc_tx_packet_recv: server(e7b7d518): DATAACK with bogus ACK-28197148 > ccid3_hc_tx_packet_recv: server(e7b7d518): DATAACK with bogus ACK-26641606. > > The fix is to replace the implicit encoding by performing the steps manually. > > Furthermore, the "bogus Ack" warning has been removed, since it can actually be > triggered due to several reasons (network reordering, old packet, (3) above), > hence it is not very useful. > > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> This mixes bug fixes with cleanups, and also conflicts because I'm not applying the hctx prefix changes. Look, if you want to make the member names shorter, shorten the post- prefix part of the name, instead of getting rid of the useful prefix part. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug 2008-05-27 8:32 ` [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug Gerrit Renker 2008-05-27 8:32 ` [PATCH 5/7] [CCID-3]: Remove ugly RTT-sampling history lookup Gerrit Renker @ 2008-05-27 13:35 ` David Miller 2008-05-27 14:16 ` Gerrit Renker 1 sibling, 1 reply; 21+ messages in thread From: David Miller @ 2008-05-27 13:35 UTC (permalink / raw) To: gerrit; +Cc: dccp, netdev From: Gerrit Renker <gerrit@erg.abdn.ac.uk> Date: Tue, 27 May 2008 09:32:44 +0100 > The identification of this bug is thanks to Cheng Wei and Tomasz Grobelny. > > To avoid divide-by-zero, the implementation previously ignored RTTs smaller > than 4 microseconds when performing integer division RTT/4. > > When the RTT reached a value less than 4 microseconds (as observed on loopback), > this prevented the Window Counter CCVal value from advancing. As a result, the > receiver stopped sending feedback. This in turn caused non-ending expiries of > the nofeedback timer at the sender, so that the sending rate was progressively > reduced until reaching the minimum of one packet per 64 seconds. > > The patch fixes this bug by handling integer division more intelligently. Due to > consistent use of dccp_sample_rtt(), divide-by-zero-RTT is avoided. > > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> Because you're mixing coding style cleanups, both of which I've just NACK'd, with real bug fixes, you're making a lot of work for me. Bug fixes go into net-2.6 and anything else goes into net-next-2.6 I've backported this patch into net-2.6, therefore without all the hctx prefix changes. dccp ccid-3: Fix "t_ipi explosion" bug The identification of this bug is thanks to Cheng Wei and Tomasz Grobelny. To avoid divide-by-zero, the implementation previously ignored RTTs smaller than 4 microseconds when performing integer division RTT/4. When the RTT reached a value less than 4 microseconds (as observed on loopback), this prevented the Window Counter CCVal value from advancing. As a result, the receiver stopped sending feedback. This in turn caused non-ending expiries of the nofeedback timer at the sender, so that the sending rate was progressively reduced until reaching the minimum of one packet per 64 seconds. The patch fixes this bug by handling integer division more intelligently. Due to consistent use of dccp_sample_rtt(), divide-by-zero-RTT is avoided. Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> Signed-off-by: David S. Miller <davem@davemloft.net> --- net/dccp/ccids/ccid3.c | 13 ++++--------- 1 files changed, 4 insertions(+), 9 deletions(-) diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c index cd61dea..f813077 100644 --- a/net/dccp/ccids/ccid3.c +++ b/net/dccp/ccids/ccid3.c @@ -193,22 +193,17 @@ static inline void ccid3_hc_tx_update_s(struct ccid3_hc_tx_sock *hctx, int len) /* * Update Window Counter using the algorithm from [RFC 4342, 8.1]. - * The algorithm is not applicable if RTT < 4 microseconds. + * As elsewhere, RTT > 0 is assumed by using dccp_sample_rtt(). */ static inline void ccid3_hc_tx_update_win_count(struct ccid3_hc_tx_sock *hctx, ktime_t now) { - u32 quarter_rtts; - - if (unlikely(hctx->ccid3hctx_rtt < 4)) /* avoid divide-by-zero */ - return; - - quarter_rtts = ktime_us_delta(now, hctx->ccid3hctx_t_last_win_count); - quarter_rtts /= hctx->ccid3hctx_rtt / 4; + u32 delta = ktime_us_delta(now, hctx->ccid3hctx_t_last_win_count), + quarter_rtts = (4 * delta) / hctx->ccid3hctx_rtt; if (quarter_rtts > 0) { hctx->ccid3hctx_t_last_win_count = now; - hctx->ccid3hctx_last_win_count += min_t(u32, quarter_rtts, 5); + hctx->ccid3hctx_last_win_count += min(quarter_rtts, 5U); hctx->ccid3hctx_last_win_count &= 0xF; /* mod 16 */ } } -- 1.5.5.1.308.g1fbb5 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug 2008-05-27 13:35 ` [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug David Miller @ 2008-05-27 14:16 ` Gerrit Renker 0 siblings, 0 replies; 21+ messages in thread From: Gerrit Renker @ 2008-05-27 14:16 UTC (permalink / raw) To: David Miller; +Cc: dccp, netdev | | Because you're mixing coding style cleanups, both of which I've | just NACK'd, with real bug fixes, you're making a lot of work for | me. | I am sorry, but I was under the impression that you were busy already and thus that the fixes would need to wait until net-next-2.6. This was in part my fault and would have been avoidable, since I had actually created both trees in parallel with the same set of patches: git://eden-feed.erg.abdn.ac.uk/net-2.6.git git://eden-feed.erg.abdn.ac.uk/net-next-2.6.git I'd suggest to stop for now, I will resubmit the whole lot which inevitably will need to be recoded. Gerrit ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/7] [CCID-3]: Remove ccid3hc{tx,rx}_ prefixes 2008-05-27 8:32 ` [PATCH 3/7] [CCID-3]: Remove ccid3hc{tx,rx}_ prefixes Gerrit Renker 2008-05-27 8:32 ` [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug Gerrit Renker @ 2008-05-27 13:26 ` David Miller 1 sibling, 0 replies; 21+ messages in thread From: David Miller @ 2008-05-27 13:26 UTC (permalink / raw) To: gerrit; +Cc: dccp, netdev From: Gerrit Renker <gerrit@erg.abdn.ac.uk> Date: Tue, 27 May 2008 09:32:43 +0100 > This patch does the same for CCID-3 as the previous patch for CCID-2: > > s#ccid3hctx_##g; > s#ccid3hcrx_##g; > > plus manual editing to retain consistency. > > Please note: expanded the fields of the `struct tfrc_tx_info' in the hc_tx_sock, > since using short #define identifiers is not a good idea. The only place where > this embedded struct was used is ccid3_hc_tx_getsockopt(). > > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> My same comments for CCID-2 apply here, don't do this, we use the member name prefixes to make the code easier to search precisely. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes 2008-05-27 8:32 ` [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes Gerrit Renker 2008-05-27 8:32 ` [PATCH 3/7] [CCID-3]: Remove ccid3hc{tx,rx}_ prefixes Gerrit Renker @ 2008-05-27 13:25 ` David Miller 2008-05-27 14:07 ` Gerrit Renker 1 sibling, 1 reply; 21+ messages in thread From: David Miller @ 2008-05-27 13:25 UTC (permalink / raw) To: gerrit; +Cc: dccp, netdev From: Gerrit Renker <gerrit@erg.abdn.ac.uk> Date: Tue, 27 May 2008 09:32:42 +0100 > This patch fixes two problems caused by the ubiquitous long "hctx->ccid2htx_" > and "hcrx->ccid2hcrx_" prefixes: > * code becomes hard to read; > * multiple-line statements are almost inevitable even for simple expressions; > The prefixes are not really necessary (compare with "struct tcp_sock"). > > There had been previous discussion of this on dccp@vger, but so far this was > not followed up (most people agreed that the prefixes are too long). > > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> > Signed-off-by: Leandro Melo de Sales <leandroal@gmail.com> You're the dccp maintainer, but... The whole reason we use these member prefixes is so that the tree is actually greppable. tcp_sock is the way it is because we never enforced this useful policy back when it was originally created. If you use short member names, it becomes eventually impossible to grep for "->foo" and have it not match a ton of crap you are completely not interested in. Using a prefix creates a struct member namespace, and thus fixes that problem. Use editor macros if you don't want to type so much :-) ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes 2008-05-27 13:25 ` [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes David Miller @ 2008-05-27 14:07 ` Gerrit Renker 2008-05-27 19:31 ` David Miller 0 siblings, 1 reply; 21+ messages in thread From: Gerrit Renker @ 2008-05-27 14:07 UTC (permalink / raw) To: David Miller; +Cc: dccp, netdev | The whole reason we use these member prefixes is so that the | tree is actually greppable. tcp_sock is the way it is because | we never enforced this useful policy back when it was originally | created. | | If you use short member names, it becomes eventually impossible | to grep for "->foo" and have it not match a ton of crap you | are completely not interested in. Using a prefix creates a | struct member namespace, and thus fixes that problem. | Ok, but this creates two problems at another end: 1/ as it is, the code is very hard to read, since 80% of expressions are taken up by the repetitive `hc{r,t}x->ccid{2-4}_hc_{r,t}x_'; 2/ there are many expressions which involve computations involving two and more field members. This almost inevitably creates unwieldy multiline expressions. These structs affect a subset of DCCP only, and are always preceded either by "hctx->..." or "hcrx->...". This is a consistent convention. With regard to being greppable, * cscope was found to work with the short identifiers, * have not tested ctags, here it may indeed be that there are multiple matches, * most of the source code is confined to net/dccp (the only exception is include/linux/{dccp,tfrc}.h). These two patches didn't come out of a personal whim: over the past 6 months this was discussed several times on dccp@vger, each time people confirmed that the identifiers were too long. There was clear agreement in this regard. If you still find the above unacceptable with regard to the policy, is there an alternative/compromise which would satisfy both goals? Gerrit ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes 2008-05-27 14:07 ` Gerrit Renker @ 2008-05-27 19:31 ` David Miller 0 siblings, 0 replies; 21+ messages in thread From: David Miller @ 2008-05-27 19:31 UTC (permalink / raw) To: gerrit; +Cc: dccp, netdev From: Gerrit Renker <gerrit@erg.abdn.ac.uk> Date: Tue, 27 May 2008 15:07:44 +0100 > If you still find the above unacceptable with regard to the policy, > is there an alternative/compromise which would satisfy both goals? I listed on in another response to one of these patches. You can also make the prefix shorter. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/7] [DCCP]: Fix to handle short sequence numbers packet correctly 2008-05-27 8:32 ` [PATCH 1/7] [DCCP]: Fix to handle short sequence numbers packet correctly Gerrit Renker 2008-05-27 8:32 ` [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes Gerrit Renker @ 2008-05-27 13:22 ` David Miller 1 sibling, 0 replies; 21+ messages in thread From: David Miller @ 2008-05-27 13:22 UTC (permalink / raw) To: gerrit; +Cc: dccp, netdev, yjwei From: Gerrit Renker <gerrit@erg.abdn.ac.uk> Date: Tue, 27 May 2008 09:32:41 +0100 > From: Wei Yongjun <yjwei@cn.fujitsu.com> > > RFC4340 said: > 8.5. Pseudocode > ... > If P.type is not Data, Ack, or DataAck and P.X == 0 (the packet > has short sequence numbers), drop packet and return > > But DCCP has some mistake to handle short sequence numbers packet, now > it drop packet only if P.type is Data, Ack, or DataAck and P.X == 0. > > This patch fixed this problem. > > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> > Acked-by: Gerrit Renker <gerrit@erg.abdn.ac.uk> > Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> As it is a bug fix I've applied this to net-2.6, thanks. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [DCCP] [Patch 0/7]: Bug fixes and format updates 2008-05-27 8:32 ` [DCCP] [Patch 0/7]: Bug fixes and format updates Gerrit Renker 2008-05-27 8:32 ` [PATCH 1/7] [DCCP]: Fix to handle short sequence numbers packet correctly Gerrit Renker @ 2008-05-28 6:12 ` Gerrit Renker 1 sibling, 0 replies; 21+ messages in thread From: Gerrit Renker @ 2008-05-28 6:12 UTC (permalink / raw) To: davem; +Cc: dccp, netdev Not so happy with yesterday's submission. I wished I could have spared you the trouble; I would like to improve this for future submissions: As a rule, I will not ask you to pull patches that haven't had any previous mailing list review. The patches submitted yesterday had been posted to the list earlier, and had been in the test tree for a while; but perhaps that was too long ago or was not clear. I will try to make clearer in the future where review is needed. There are several ways for people to contribute. There is a web interface to browse the changesets: http://eden-feed.erg.abdn.ac.uk and code under review is available in several formats: - tree: git://eden-feed.erg.abdn.ac.uk/dccp_exp (subtree dccp) - tarball: http://eden-feed.erg.abdn.ac.uk/latest-dccp-test-tree.tar.bz2 - snapshots: http://www.erg.abdn.ac.uk/users/gerrit/dccp/testing_dccp/test-tree/ Mixing format changes and bug fixes ... yes, also my error, will in future create separate changesets. For the moment, I have recoded the patch(es) that you have asked me to and have isolated bug fixes from the tree for a separate submission. I hope we can keep this constructive, Gerrit ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2008-05-31 13:49 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <dccp_bug_fixes_and_format_updates>
2008-05-27 8:32 ` [DCCP] [Patch 0/7]: Bug fixes and format updates Gerrit Renker
2008-05-27 8:32 ` [PATCH 1/7] [DCCP]: Fix to handle short sequence numbers packet correctly Gerrit Renker
2008-05-27 8:32 ` [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes Gerrit Renker
2008-05-27 8:32 ` [PATCH 3/7] [CCID-3]: Remove ccid3hc{tx,rx}_ prefixes Gerrit Renker
2008-05-27 8:32 ` [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug Gerrit Renker
2008-05-27 8:32 ` [PATCH 5/7] [CCID-3]: Remove ugly RTT-sampling history lookup Gerrit Renker
2008-05-27 8:32 ` [PATCH 6/7] [CCID-2/3]: Fix sparse warnings Gerrit Renker
2008-05-27 8:32 ` [PATCH 7/7] [TCP][DCCP]: Consolidate duplicate code which does RFC3390 conversion Gerrit Renker
2008-05-27 13:37 ` David Miller
2008-05-28 6:24 ` [Patch v2 1/1] [tcp/dccp]: " Gerrit Renker
2008-05-31 13:49 ` [Patch v3 " Gerrit Renker
2008-05-27 13:37 ` [PATCH 6/7] [CCID-2/3]: Fix sparse warnings David Miller
2008-05-27 13:36 ` [PATCH 5/7] [CCID-3]: Remove ugly RTT-sampling history lookup David Miller
2008-05-27 13:35 ` [PATCH 4/7] [CCID-3]: Fix "t_ipi explosion" bug David Miller
2008-05-27 14:16 ` Gerrit Renker
2008-05-27 13:26 ` [PATCH 3/7] [CCID-3]: Remove ccid3hc{tx,rx}_ prefixes David Miller
2008-05-27 13:25 ` [PATCH 2/7] [CCID-2]: Remove ccid2hc{tx,rx}_ prefixes David Miller
2008-05-27 14:07 ` Gerrit Renker
2008-05-27 19:31 ` David Miller
2008-05-27 13:22 ` [PATCH 1/7] [DCCP]: Fix to handle short sequence numbers packet correctly David Miller
2008-05-28 6:12 ` [DCCP] [Patch 0/7]: Bug fixes and format updates Gerrit Renker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).