[RFC]: Break up a patch in two (rfc3448bis changes to feedback reception)
Hi Gerrit,
Please take a look at the two attached patches, they were made
from your patch [CCID3]: Implement rfc3448bis changes to feedback reception,
that has this changeset comment:
--
[CCID 3]: Implement rfc3448bis changes to feedback reception
This implements the algorithm to update the allowed sending rate X upon
receiving feedback packets, as described in draft rfc3448bis, 4.2/4.3.
The patch further removes two irrelevant states in TX feedback handling:
* the NO_SENT state is only triggered in bidirectional mode,
costing unnecessary processing.
* the TERM (terminating) state is irrelevant.
--
The second part further removes, was made a separate patch
that I have applied before the rfc3448bis changes to feedback reception.
Doing it this way eases understanding the change as we don't mixup
identantion changes made due to removing the switch statement.
The end result should be equivalent, but please take a look and
let me know if the algorithm was changed in any way.
Thanks a lot,
- Arnaldo
From 9165240abd3d4e8280647b9372dc3f223a802347 Mon Sep 17 00:00:00 2001
From: Gerrit Renker [EMAIL PROTECTED]
Date: Mon, 17 Dec 2007 10:25:06 -0200
Subject: [PATCH 2/3] [CCID3]: Remove two irrelevant states in TX feedback
handling
* the NO_SENT state is only triggered in bidirectional mode,
costing unnecessary processing.
* the TERM (terminating) state is irrelevant.
Signed-off-by: Gerrit Renker [EMAIL PROTECTED]
Signed-off-by: Ian McDonald [EMAIL PROTECTED]
Signed-off-by: Arnaldo Carvalho de Melo [EMAIL PROTECTED]
---
net/dccp/ccids/ccid3.c | 173 +++-
1 files changed, 84 insertions(+), 89 deletions(-)
diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c
index 90e0454..1b1cb74 100644
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -402,112 +402,107 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk,
struct sk_buff *skb)
if (!(DCCP_SKB_CB(skb)-dccpd_type == DCCP_PKT_ACK ||
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)
+ return;
opt_recv = hctx-ccid3hctx_options_received;
+ now = ktime_get_real();
- switch (hctx-ccid3hctx_state) {
- case TFRC_SSTATE_NO_FBACK:
- case TFRC_SSTATE_FBACK:
- now = ktime_get_real();
-
- /* estimate RTT from history if ACK number is valid */
- r_sample = tfrc_tx_hist_rtt(hctx-ccid3hctx_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);
- return;
- }
+ /* Estimate RTT from history if ACK number is valid */
+ r_sample = tfrc_tx_hist_rtt(hctx-ccid3hctx_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);
+ return;
+ }
+
+ /* Update receive rate in units of 64 * bytes/second */
+ hctx-ccid3hctx_x_recv = opt_recv-ccid3or_receive_rate;
+ hctx-ccid3hctx_x_recv = 6;
- /* Update receive rate in units of 64 * bytes/second */
- hctx-ccid3hctx_x_recv = opt_recv-ccid3or_receive_rate;
- hctx-ccid3hctx_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;
+ else /* can not exceed 100% */
+ hctx-ccid3hctx_p = 100 / 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);
- /* Update loss event rate */
- pinv = opt_recv-ccid3or_loss_event_rate;
- if (pinv == ~0U || pinv == 0) /* see RFC 4342, 8.5 */
- hctx-ccid3hctx_p = 0;
- else /* can not exceed 100% */
- hctx-ccid3hctx_p = 100 / pinv;
+ if (hctx-ccid3hctx_state == TFRC_SSTATE_NO_FBACK) {
/*
-
Re: [RFC]: Break up a patch in two (rfc3448bis changes to feedback reception)
| The end result should be equivalent, but please take a look and That is a good catch - this patch was a pain to keep updated exactly due to the many indentation levels. I had a quick look, the patch looks ok. Just a small suggestion - since the RTT lookup code in tx_packet_recv() is new, would it make sense to group it with the RTT validation, as e.g. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHES 0/5]: DCCP patches for 2.6.25
Hi David, Please consider pulling from: master.kernel.org:/pub/scm/linux/kernel/git/acme/net-2.6.25 Best Regards, - Arnaldo net/dccp/ccids/ccid3.c | 252 +- net/dccp/ccids/ccid3.h |8 - net/dccp/dccp.h|6 - 3 files changed, 130 insertions(+), 136 deletions(-) - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] [CCID3]: Use a function to update p_inv, and p is never used
From: Gerrit Renker [EMAIL PROTECTED]
This patch
1) concentrates previously scattered computation of p_inv into one function;
2) removes the `p' element of the CCID3 RX sock (it is redundant);
3) makes the tfrc_rx_info structure standalone, only used on demand.
Signed-off-by: Gerrit Renker [EMAIL PROTECTED]
Signed-off-by: Ian McDonald [EMAIL PROTECTED]
Signed-off-by: Arnaldo Carvalho de Melo [EMAIL PROTECTED]
---
net/dccp/ccids/ccid3.c | 11 ---
net/dccp/ccids/ccid3.h |8
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c
index 876c747..90e0454 100644
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -917,6 +917,7 @@ static int ccid3_hc_rx_getsockopt(struct sock *sk, const
int optname, int len,
u32 __user *optval, int __user *optlen)
{
const struct ccid3_hc_rx_sock *hcrx;
+ struct tfrc_rx_info rx_info;
const void *val;
/* Listen socks doesn't have a private CCID block */
@@ -926,10 +927,14 @@ static int ccid3_hc_rx_getsockopt(struct sock *sk, const
int optname, int len,
hcrx = ccid3_hc_rx_sk(sk);
switch (optname) {
case DCCP_SOCKOPT_CCID_RX_INFO:
- if (len sizeof(hcrx-ccid3hcrx_tfrc))
+ if (len sizeof(rx_info))
return -EINVAL;
- len = sizeof(hcrx-ccid3hcrx_tfrc);
- val = hcrx-ccid3hcrx_tfrc;
+ 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);
+ len = sizeof(rx_info);
+ val = rx_info;
break;
default:
return -ENOPROTOOPT;
diff --git a/net/dccp/ccids/ccid3.h b/net/dccp/ccids/ccid3.h
index e9f6ff4..49ca32b 100644
--- a/net/dccp/ccids/ccid3.h
+++ b/net/dccp/ccids/ccid3.h
@@ -139,6 +139,8 @@ enum ccid3_hc_rx_states {
* @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)
@@ -147,13 +149,11 @@ enum ccid3_hc_rx_states {
* @ccid3hcrx_pinv - Inverse of Loss Event Rate (RFC 4342, sec. 8.5)
*/
struct ccid3_hc_rx_sock {
- struct tfrc_rx_info ccid3hcrx_tfrc;
-#define ccid3hcrx_x_recv ccid3hcrx_tfrc.tfrcrx_x_recv
-#define ccid3hcrx_rtt ccid3hcrx_tfrc.tfrcrx_rtt
-#define ccid3hcrx_pccid3hcrx_tfrc.tfrcrx_p
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;
--
1.5.3.6
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] [DCCP]: Remove unused inline function
From: Gerrit Renker [EMAIL PROTECTED]
The function follows48(), which is a special-case of dccp_delta_seqno(),
is nowhere used in the DCCP code, thus removed by this patch.
Signed-off-by: Gerrit Renker [EMAIL PROTECTED]
Signed-off-by: Ian McDonald [EMAIL PROTECTED]
Signed-off-by: Arnaldo Carvalho de Melo [EMAIL PROTECTED]
---
net/dccp/dccp.h |6 --
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h
index b138e20..ebe59d9 100644
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -153,12 +153,6 @@ static inline u64 max48(const u64 seq1, const u64 seq2)
return after48(seq1, seq2) ? seq1 : seq2;
}
-/* is seq1 next seqno after seq2 */
-static inline int follows48(const u64 seq1, const u64 seq2)
-{
- return dccp_delta_seqno(seq2, seq1) == 1;
-}
-
enum {
DCCP_MIB_NUM = 0,
DCCP_MIB_ACTIVEOPENS, /* ActiveOpens */
--
1.5.3.6
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] [CCID3]: Nofeedback timer according to rfc3448bis
From: Gerrit Renker [EMAIL PROTECTED]
This implements the changes to the nofeedback timer handling suggested
in draft rfc3448bis00, section 4.4. In particular, these changes mean:
* better handling of the lossless case (p == 0)
* the timestamp for computing t_ld becomes obsolete
* much more recent document (RFC 3448 is almost 5 years old)
* concepts in rfc3448bis arose from a real, working implementation
(cf. sec. 12)
Signed-off-by: Gerrit Renker [EMAIL PROTECTED]
Signed-off-by: Ian McDonald [EMAIL PROTECTED]
Signed-off-by: Arnaldo Carvalho de Melo [EMAIL PROTECTED]
---
net/dccp/ccids/ccid3.c | 63 ++--
1 files changed, 29 insertions(+), 34 deletions(-)
diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c
index 7c8e9ad..00b5f11 100644
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -131,12 +131,11 @@ 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;
- ktime_t now = stamp? *stamp : ktime_get_real();
+ ktime_t now = stamp ? *stamp : ktime_get_real();
/*
* Handle IDLE periods: do not reduce below RFC3390 initial sending rate
@@ -230,27 +229,27 @@ 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));
- switch (hctx-ccid3hctx_state) {
- case TFRC_SSTATE_NO_FBACK:
- /* RFC 3448, 4.4: Halve send rate directly */
+ if (hctx-ccid3hctx_state == TFRC_SSTATE_FBACK)
+ ccid3_hc_tx_set_state(sk, TFRC_SSTATE_NO_FBACK);
+ else if (hctx-ccid3hctx_state != TFRC_SSTATE_NO_FBACK)
+ goto out;
+
+ /*
+* Determine new allowed sending rate X as per draft rfc3448bis-00, 4.4
+*/
+ if (hctx-ccid3hctx_t_rto == 0 || /* no feedback received yet */
+ hctx-ccid3hctx_p == 0) {
+
+ /* halve send rate directly */
hctx-ccid3hctx_x = max(hctx-ccid3hctx_x / 2,
(((__u64)hctx-ccid3hctx_s) 6) /
TFRC_T_MBI);
-
- ccid3_pr_debug(%s(%p, state=%s), updated tx rate to %u
- bytes/s\n, dccp_role(sk), sk,
- ccid3_tx_state_name(hctx-ccid3hctx_state),
- (unsigned)(hctx-ccid3hctx_x 6));
- /* The value of R is still undefined and so we can not recompute
-* the timout value. Keep initial value as per [RFC 4342, 5]. */
- t_nfb = TFRC_INITIAL_TIMEOUT;
ccid3_update_send_interval(hctx);
- break;
- case TFRC_SSTATE_FBACK:
+ } else {
/*
-* Modify the cached value of X_recv [RFC 3448, 4.4]
+* Modify the cached value of X_recv
*
-* If (p == 0 || X_calc 2 * X_recv)
+* If (X_calc 2 * X_recv)
*X_recv = max(X_recv / 2, s / (2 * t_mbi));
* Else
*X_recv = X_calc / 4;
@@ -259,32 +258,28 @@ static void ccid3_hc_tx_no_feedback_timer(unsigned long
data)
*/
BUG_ON(hctx-ccid3hctx_p !hctx-ccid3hctx_x_calc);
- if (hctx-ccid3hctx_p == 0 ||
- (hctx-ccid3hctx_x_calc (hctx-ccid3hctx_x_recv 5))) {
-
+ 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));
- } else {
+ else {
hctx-ccid3hctx_x_recv = hctx-ccid3hctx_x_calc;
hctx-ccid3hctx_x_recv = 4;
}
- /* Now recalculate X [RFC 3448, 4.3, step (4)] */
ccid3_hc_tx_update_x(sk, NULL);
- /*
-* Schedule no feedback timer to expire in
-* max(t_RTO, 2 * s/X) = max(t_RTO, 2 * t_ipi)
-* See comments in packet_recv() regarding the value of t_RTO.
-*/
- t_nfb = max(hctx-ccid3hctx_t_rto, 2 * hctx-ccid3hctx_t_ipi);
- break;
- case TFRC_SSTATE_NO_SENT:
- DCCP_BUG(%s(%p) - Illegal state NO_SENT, dccp_role(sk), sk);
- /* fall through */
- case TFRC_SSTATE_TERM:
- goto out;
}
+ ccid3_pr_debug(Reduced X to %llu/64
[PATCH] net/dccp/: Spelling fixes
Signed-off-by: Joe Perches [EMAIL PROTECTED]
---
net/dccp/ackvec.h |2 +-
net/dccp/ccids/ccid3.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/dccp/ackvec.h b/net/dccp/ackvec.h
index 9ef0737..9671ecd 100644
--- a/net/dccp/ackvec.h
+++ b/net/dccp/ackvec.h
@@ -71,7 +71,7 @@ struct dccp_ackvec {
* @dccpavr_ack_ackno - sequence number being acknowledged
* @dccpavr_ack_ptr - pointer into dccpav_buf where this record starts
* @dccpavr_ack_nonce - dccpav_ack_nonce at the time this record was sent
- * @dccpavr_sent_len - lenght of the record in dccpav_buf
+ * @dccpavr_sent_len - length of the record in dccpav_buf
*/
struct dccp_ackvec_record {
struct list_head dccpavr_node;
diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c
index 19b3358..d133416 100644
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -239,7 +239,7 @@ static void ccid3_hc_tx_no_feedback_timer(unsigned long
data)
ccid3_tx_state_name(hctx-ccid3hctx_state),
(unsigned)(hctx-ccid3hctx_x 6));
/* The value of R is still undefined and so we can not recompute
-* the timout value. Keep initial value as per [RFC 4342, 5]. */
+* the timeout value. Keep initial value as per [RFC 4342, 5].
*/
t_nfb = TFRC_INITIAL_TIMEOUT;
ccid3_update_send_interval(hctx);
break;
--
1.5.3.7.949.g2221a6
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHES 0/5]: DCCP patches for 2.6.25
From: Arnaldo Carvalho de Melo [EMAIL PROTECTED] Date: Mon, 17 Dec 2007 13:00:14 -0200 Please consider pulling from: master.kernel.org:/pub/scm/linux/kernel/git/acme/net-2.6.25 Pulled and pushed out, thanks! - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
