: Wrap computation of RFC3390-initial rate into separate function
PATCH 2/7 : Wrap computation of RFC3390-initial rate into separate function
[CCID 3]: Wrap computation of RFC3390-initial rate into separate function
The CCID 3 and TFRC specs (RFC 4342, RFC 3448, draft-3448bis) make frequent reference to the computation of the RFC-3390 initial sending rate:
1. Initial sending rate when RTT is known (RFC 4342, p. 6)
2. Response to Idle/Application-Limited periods (RFC 4342, 5.1)
This warrants putting the code into its own function, for later code reuse.
--- net/dccp/ccids/ccid3.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
net/dccp/ccids/ccid3.c +18 -6
@@ -81,6 +81,21 @@ static void ccid3_hc_tx_set_state(struct }/* +* Compute the initial sending rate X_init according to RFC 3390: +* w_init = min(4 * MSS, max(2 * MSS, 4380 bytes)) +* X_init = w_init / RTT +* For consistency with other parts of the code, X_init is scaled by 2^6. +*/ +static inline u64 rfc3390_initial_rate (struct sock *sk) +{ +const struct dccp_sock *dp =dccp_sk (sk); +__u32 w_init =min (4 * dp->dccps_mss_cache, +max (2 * dp->dccps_mss_cache,4380U )); + +return scaled_div (w_init <<6 ,ccid3_hc_tx_sk (sk)->ccid3hctx_rtt); +} + +/* * Recalculate t_ipi and delta (should be called whenver X changes) */ static inline void ccid3_update_send_interval (struct ccid3_hc_tx_sock *hctx)
@@ -471,19 +486,16 @@ static void ccid3_hc_tx_packet_recv(stru/* * Larger Initial Windows [RFC 4342, sec. 5] */ -__u32 w_init =min (4 * dp->dccps_mss_cache, -max (2 * dp->dccps_mss_cache,4380U )); hctx->ccid3hctx_rtt = r_sample; - hctx->ccid3hctx_x =scaled_div (w_init <<6 , r_sample); + hctx->ccid3hctx_x =rfc3390_initial_rate (sk); hctx->ccid3hctx_t_ld = now;ccid3_update_send_interval (hctx); -ccid3_pr_debug ("%s(%p), s=%u, MSS=%u, w_init=%u, " +ccid3_pr_debug ("%s(%p), s=%u, MSS=%u, " "R_sample=%dus, X=%llu\n" ,dccp_role (sk), sk, hctx->ccid3hctx_s, - dp->dccps_mss_cache, w_init, - (int )r_sample, + dp->dccps_mss_cache, (int )r_sample, hctx->ccid3hctx_x >>6 );ccid3_hc_tx_set_state (sk,TFRC_SSTATE_FBACK );
Quoted 2 lines · Gerrit Renker
[CCID 3]: Wrap computation of RFC3390-initial rate into separate function +static inline u64
rfc3390_initial_rate(struct sock *sk)
Apart from in header files which already exist you shouldn't be putting new inline functions in as per a number of discussions on lkml. The reason for this is that gcc now decides itself when to inline code and when not to.
Once this is fixed - Acked-by: Ian McDonald <[email protected]>
Signature · 3 lines
Web: http://wand.net.nz/~iam4 Blog: http://iansblog.jandi.co.nz WAND Network Research Group
Quoting Ian McDonald:
| On 24/01/07, Gerrit Renker <[email protected]> wrote: | > [CCID 3]: Wrap computation of RFC3390-initial rate into separate function | > +static inline u64 rfc3390_initial_rate(struct sock *sk) | | Apart from in header files which already exist you shouldn't be | putting new inline functions in as per a number of discussions on | lkml. The reason for this is that gcc now decides itself when to | inline code and when not to. |
I would fully agree with you if the function contained some more statements. But this one is really meagre, i.e. it would make no difference when placing it as statement.
I have read through some of the discussions on inlining and am referring to
http://lwn.net/Articles/166172/
There, Linus Torvalds and Andrew Morton suggest to decide on a case-by-case analysis. I believe it is good practice to tell the compiler the intention (and thus have started to use `const' for similar reasons).
Would you be ok with moving that function into ccid3.h instead?
Quoted 14 lines · Gerrit Renker
Quoting Ian McDonald:
| On 24/01/07, Gerrit Renker <[email protected]> wrote: | > [CCID 3]: Wrap computation of RFC3390-initial rate into separate function | > +static inline u64 rfc3390_initial_rate(struct sock *sk) | | Apart from in header files which already exist you shouldn't be | putting new inline functions in as per a number of discussions on | lkml. The reason for this is that gcc now decides itself when to | inline code and when not to. |I would fully agree with you if the function contained some more statements. But this one is really meagre, i.e. it would make no difference when placing it as statement.
Quoted 8 lines
I have read through some of the discussions on inlining and am referring to
http://lwn.net/Articles/166172/
There, Linus Torvalds and Andrew Morton suggest to decide on a case-by-case analysis. I believe it is good practice to tell the compiler the intention (and thus have started to use `const' for similar reasons).Would you be ok with moving that function into
ccid3.hinstead?
Don't worry about it. It's no big deal. Leave it as is.
Ian
Signature · 3 lines
Web: http://wand.net.nz/~iam4 Blog: http://iansblog.jandi.co.nz WAND Network Research Group