[RFC]: Break up a patch in two (rfc3448bis changes to feedback reception)

2007-12-17 Thread Arnaldo Carvalho de Melo
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)

2007-12-17 Thread Gerrit Renker
|   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