Skip to contentSkip to conversation

: Shorten statement for updating p

PATCH In review Gerrit Renker 2 participants 4 messages Mar 21, 2007 – Apr 2, 2007 Networking

lore
Share & tools
lore
#0 lore raw

PATCH 10/25 : Shorten statement for updating p

[CCID 3]: Shorten statement for updating p

This shortens the statement for updating the loss event rate p when a feedback packet is received.

---
 net/dccp/ccids/ccid3.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
net/dccp/ccids/ccid3.c +2 -5
@@ -466,12 +466,9 @@ static void ccid3_hc_tx_packet_recv(stru
 	hctx->ccid3hctx_x_recv = opt_recv->ccid3or_receive_rate;
 	hctx->ccid3hctx_x_recv <<= 6;
 
-	/* Update loss event rate */
+	/* Update loss event rate (scaled by 1e6), cf. RFC 4342, 8.5 */
 	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 = 1000000 / pinv;
+	hctx->ccid3hctx_p = (pinv = ~0U || pinv = 0)? 0 : scaled_div(1, pinv);
 
 	/*
 	 * Calculate new round trip sample as per [RFC 3448, 4.3] by
Acked-by on v1 10/25
#1 lore raw
Quoted 15 lines · Gerrit Renker

[CCID 3]: Shorten statement for updating p

This shortens the statement for updating the loss event rate p when a feedback packet is received.

---
 net/dccp/ccids/ccid3.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
net/dccp/ccids/ccid3.c +0 -0
@@ -466,12 +466,9 @@ static void ccid3_hc_tx_packet_recv(stru
        hctx->ccid3hctx_x_recv = opt_recv->ccid3or_receive_rate;
        hctx->ccid3hctx_x_recv <<= 6;

net/dccp/ccids/ccid3.c static void ccid3_hc_tx_packet_recv(stru · line 472

Quoted 10 lines
net/dccp/ccids/ccid3.c +2 -5
 
-       /* Update loss event rate */
+       /* Update loss event rate (scaled by 1e6), cf. RFC 4342, 8.5 */
        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 = 1000000 / pinv;
+       hctx->ccid3hctx_p = (pinv = ~0U || pinv = 0)? 0 : scaled_div(1, pinv);
 

Agree with the scaled_div bit but don't like removing if/then and repalce withing ?: as harder to read and compiler can optimise just as well. Matter of personal choice though so up to Arnaldo really.

Signature · 3 lines
Web: http://wand.net.nz/~iam4
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group
#2 lore raw
In reply to Gerrit Renker
|  > +       /* Update loss event rate (scaled by 1e6), cf. RFC 4342, 8.5 */
|  >         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 = 1000000 / pinv;
|  > +       hctx->ccid3hctx_p = (pinv = ~0U || pinv = 0)? 0 : scaled_div(1, pinv);
|  >
|  
|  Agree with the scaled_div bit but don't like removing if/then and
|  repalce withing ?: as harder to read and compiler can optimise just as
|  well. Matter of personal choice though so up to Arnaldo really.
|  

I could take this patch out if you / Arnaldo think so, since it is overridden later by another patch which uses only one socket field instead of two (p depends on pinv, so having both is redundant).
Please let me be state this - I aim as best as possible to avoid overlap and overriding in later patches. But this is not always possible and - having just spent almost two days integrating away another patch set with my local changesets - can sometimes be a real nightmare. It is not always avoidable when trying to split larger changes into small and separate chunks.

#3 lore raw
In reply to Gerrit Renker
Quoted 13 lines · Gerrit Renker
|  > +       /* Update loss event rate (scaled by 1e6), cf. RFC 4342, 8.5 */
|  >         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 = 1000000 / pinv;
|  > +       hctx->ccid3hctx_p = (pinv = ~0U || pinv = 0)? 0 : scaled_div(1, pinv);
|  >
|
|  Agree with the scaled_div bit but don't like removing if/then and
|  repalce withing ?: as harder to read and compiler can optimise just as
|  well. Matter of personal choice though so up to Arnaldo really.
|
Quoted 3 lines

I could take this patch out if you / Arnaldo think so, since it is overridden later by another patch which uses only one socket field instead of two (p depends on pinv, so having both is redundant).

No leave it in then - it's just a matter of style, not logic.

Quoted 5 lines

Please let me be state this - I aim as best as possible to avoid overlap and overriding in later patches. But this is not always possible and - having just spent almost two days integrating away another patch set with my local changesets - can sometimes be a real nightmare. It is not always avoidable when trying to split larger changes into small and separate chunks.

Yes I understand your pain and hope I didn't cause too much of it...

Signature · 3 lines
Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group