On 4/2/07, Ian McDonald <ian.mcdonald@xxxxxxxxxxx> wrote:
On 4/2/07, Gerrit Renker <gerrit@xxxxxxxxxxxxxx> wrote:
> Quoting Ian McDonald:
> | > +/**
> | > + * Macro for exponentially weighted moving average
> | > + * @weight: Weight to be used as damping factor, in units of 1/10
> | > + * Beware that @val is evaluated multiple times.
> | > + */
> | > +#define TFRC_EWMA(val, newval, weight) \
> | > + val = val? ((weight) * val + (10 - (weight)) * (newval)) / 10 : (newval)
> | > +
> | Just wondering why you pass weight when you only use 9 anyway and it
> | just adds a step. Is it used in following patches with a different
> | weight?
> |
> In the following patches it is used one more time, which is for the `Preventing
> Oscillations' changeset (this is not in the next patch set but in the one after that);
> here again the weight 9/10 is used. I think it is useful to keep the `weight' argument,
> since RFC 3448 suggests to use a different weight for the RTT sample when the `Oscillation
> Prevention' e.g. is not deployed (a q close to zero).
> I think it is better to keep the weight argument, it allows changing the code later, while
> still providing the abstraction: for CCID 3 there are 4 uses of this macro alone.
>
OK.
Acked-by: Ian McDonald <ian.mcdonald@xxxxxxxxxxx>
I don't like macros that receive as a parameter something that will be
changed, its more readable to have it as a inline function and return
the new value, even if the resulting code uses more characters, so I'm
applying the attached patch and will take care of the fallout, that is
small from what I grepped at the_whole_lot.
- Arnaldo
diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c
index 9474331..428aa92 100644
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -183,7 +183,7 @@ static inline void ccid3_hc_tx_update_s(struct ccid3_hc_tx_sock *hctx, int len)
{
const u16 old_s = hctx->ccid3hctx_s;
- hctx->ccid3hctx_s = old_s == 0 ? len : (9 * old_s + len) / 10;
+ hctx->ccid3hctx_s = tfrc_ewma(hctx->ccid3hctx_s, len, 9);
if (hctx->ccid3hctx_s != old_s)
ccid3_update_send_interval(hctx);
@@ -474,8 +474,7 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)
*
* q is a constant, RFC 3448 recommends 0.9
*/
- hctx->ccid3hctx_rtt = hctx->ccid3hctx_rtt == 0 ? r_sample
- : (9 * hctx->ccid3hctx_rtt + r_sample) / 10;
+ hctx->ccid3hctx_rtt = tfrc_ewma(hctx->ccid3hctx_rtt, r_sample, 9);
/*
* Update allowed sending rate as per draft rfc3448bis, 4.2/4.3
@@ -724,8 +723,7 @@ static inline void ccid3_hc_rx_update_s(struct ccid3_hc_rx_sock *hcrx, int len)
if (unlikely(len == 0)) /* don't update on empty packets (e.g. ACKs) */
ccid3_pr_debug("Packet payload length is 0 - not updating\n");
else
- hcrx->ccid3hcrx_s = hcrx->ccid3hcrx_s == 0 ? len :
- (9 * hcrx->ccid3hcrx_s + len) / 10;
+ hcrx->ccid3hcrx_s = tfrc_ewma(hcrx->ccid3hcrx_s, len, 9);
}
static void ccid3_hc_rx_send_feedback(struct sock *sk)
diff --git a/net/dccp/ccids/lib/tfrc.h b/net/dccp/ccids/lib/tfrc.h
index faf5f7e..f26e9e9 100644
--- a/net/dccp/ccids/lib/tfrc.h
+++ b/net/dccp/ccids/lib/tfrc.h
@@ -37,6 +37,17 @@ static inline u32 scaled_div32(u64 a, u32 b)
return result;
}
+/**
+ * Exponentially weighted moving average
+ * @weight: Weight to be used as damping factor, in units of 1/10
+ */
+static inline u32 tfrc_ewma(const u32 val, const u32 newval, const u8 weight)
+{
+ if (val != 0)
+ return (weight * val + (10 - weight) * newval) / 10;
+ return newval;
+}
+
extern u32 tfrc_calc_x(u16 s, u32 R, u32 p);
extern u32 tfrc_calc_x_reverse_lookup(u32 fvalue);
