rx: Fix resend accounting

rxi_Start flagged itself as 'resending' whenever it flushed the
transmit queue due to a resend event. However, it would flush the
entire transmit queue at this point, rather than only transmitting
packets that require a resend. When running with large window sizes
this results an a large number of packets erroneously being marked
as resent.

Instead, let SendXmitList decide whether a packet is being
retransmitted by using the presence of a serial number. This takes
advantage of the fact that a retransmitted packet must be the only
entry in a packet list - we just flag the packet list, instead of
having to maintain counters for each individual packet.

Change-Id: Id8e87b8efa87f5dfec2e51b7983a9fd6b1a5c4c5
Reviewed-on: http://gerrit.openafs.org/3135
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Derrick Brashear <shadow@dementia.org>
Reviewed-by: Derrick Brashear <shadow@dementia.org>

diff --git a/src/rx/rx.c b/src/rx/rx.c
index 312d746..27c1fda 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -5377,13 +5377,14 @@ rxi_SendAck(struct rx_call *call,
 struct xmitlist {
    struct rx_packet **list;
    int len;
+   int resending;
 };
 
 /* Send all of the packets in the list in single datagram */
 static void
 rxi_SendList(struct rx_call *call, struct xmitlist *xmit,
             int istack, int moreFlag, struct clock *now,
-            struct clock *retryTime, int resending)
+            struct clock *retryTime)
 {
     int i;
     int requestAck = 0;
@@ -5393,12 +5394,12 @@ rxi_SendList(struct rx_call *call, struct xmitlist *xmit,
 
     MUTEX_ENTER(&peer->peer_lock);
     peer->nSent += xmit->len;
-    if (resending)
+    if (xmit->resending)
        peer->reSends += xmit->len;
     MUTEX_EXIT(&peer->peer_lock);
 
     if (rx_stats_active) {
-        if (resending)
+        if (xmit->resending)
             rx_atomic_add(&rx_stats.dataPacketsReSent, xmit->len);
         else
             rx_atomic_add(&rx_stats.dataPacketsSent, xmit->len);
@@ -5487,7 +5488,7 @@ rxi_SendList(struct rx_call *call, struct xmitlist *xmit,
      * idle connections) */
     conn->lastSendTime = call->lastSendTime = clock_Sec();
     /* Let a set of retransmits trigger an idle timeout */
-    if (!resending)
+    if (!xmit->resending)
        call->lastSendData = call->lastSendTime;
 }
 
@@ -5502,12 +5503,11 @@ rxi_SendList(struct rx_call *call, struct xmitlist *xmit,
 
 static void
 rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len,
-                int istack, struct clock *now, struct clock *retryTime,
-                int resending)
+                int istack, struct clock *now, struct clock *retryTime)
 {
     int i;
     struct xmitlist working;
-    struct xmitlist last = {NULL, 0};
+    struct xmitlist last;
 
     struct rx_peer *peer = call->conn->peer;
     int morePackets = 0;
@@ -5515,6 +5515,7 @@ rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len,
     memset(&last, 0, sizeof(struct xmitlist));
     working.list = &list[0];
     working.len = 0;
+    working.resending = 0;
 
     for (i = 0; i < len; i++) {
        /* Does the current packet force us to flush the current list? */
@@ -5526,7 +5527,7 @@ rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len,
             * set into the 'last' one, and resets the working set */
 
            if (last.len > 0) {
-               rxi_SendList(call, &last, istack, 1, now, retryTime, resending);
+               rxi_SendList(call, &last, istack, 1, now, retryTime);
                /* If the call enters an error state stop sending, or if
                 * we entered congestion recovery mode, stop sending */
                if (call->error || (call->flags & RX_CALL_FAST_RECOVER_WAIT))
@@ -5534,12 +5535,17 @@ rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len,
            }
            last = working;
            working.len = 0;
+           working.resending = 0;
            working.list = &list[i];
        }
        /* Add the current packet to the list if it hasn't been acked.
         * Otherwise adjust the list pointer to skip the current packet.  */
        if (!(list[i]->flags & RX_PKTFLAG_ACKED)) {
            working.len++;
+
+           if (list[i]->header.serial)
+               working.resending = 1;
+
            /* Do we need to flush the list? */
            if (working.len >= (int)peer->maxDgramPackets
                || working.len >= (int)call->nDgramPackets 
@@ -5547,8 +5553,7 @@ rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len,
                || list[i]->header.serial
                || list[i]->length != RX_JUMBOBUFFERSIZE) {
                if (last.len > 0) {
-                   rxi_SendList(call, &last, istack, 1, now,
-                                retryTime, resending);
+                   rxi_SendList(call, &last, istack, 1, now, retryTime);
                    /* If the call enters an error state stop sending, or if
                     * we entered congestion recovery mode, stop sending */
                    if (call->error
@@ -5557,6 +5562,7 @@ rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len,
                }
                last = working;
                working.len = 0;
+               working.resending = 0;
                working.list = &list[i + 1];
            }
        } else {
@@ -5581,19 +5587,17 @@ rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len,
            morePackets = 1;
        }
        if (last.len > 0) {
-           rxi_SendList(call, &last, istack, morePackets, now,
-                        retryTime, resending);
+           rxi_SendList(call, &last, istack, morePackets, now, retryTime);
            /* If the call enters an error state stop sending, or if
             * we entered congestion recovery mode, stop sending */
            if (call->error || (call->flags & RX_CALL_FAST_RECOVER_WAIT))
                return;
        }
        if (morePackets) {
-           rxi_SendList(call, &working, istack, 0, now, retryTime,
-                        resending);
+           rxi_SendList(call, &working, istack, 0, now, retryTime);
        }
     } else if (last.len > 0) {
-       rxi_SendList(call, &last, istack, 0, now, retryTime, resending);
+       rxi_SendList(call, &last, istack, 0, now, retryTime);
        /* Packets which are in 'working' are not sent by this call */
     }
 }
@@ -5630,7 +5634,6 @@ rxi_Start(struct rxevent *event,
     int haveEvent;
     int nXmitPackets;
     int maxXmitPackets;
-    int resending = 0;
 
     /* If rxi_Start is being called as a result of a resend event,
      * then make sure that the event pointer is removed from the call
@@ -5641,7 +5644,6 @@ rxi_Start(struct rxevent *event,
        CALL_RELE(call, RX_CALL_REFCOUNT_RESEND);
         MUTEX_EXIT(&rx_refcnt_mutex);
        call->resendEvent = NULL;
-       resending = 1;
        if (queue_IsEmpty(&call->tq)) {
            /* Nothing to do */
            return;
@@ -5796,7 +5798,7 @@ rxi_Start(struct rxevent *event,
                        if (nXmitPackets == maxXmitPackets) {
                            rxi_SendXmitList(call, call->xmitList,
                                             nXmitPackets, istack, &now, 
-                                            &retryTime, resending);
+                                            &retryTime);
                            goto restart;
                        }
                         dpf(("call %d xmit packet %"AFS_PTR_FMT" now %u.%06u retryTime %u.%06u nextRetry %u.%06u\n",
@@ -5812,7 +5814,7 @@ rxi_Start(struct rxevent *event,
                 * ready to send. Now we loop to send the packets */
                if (nXmitPackets > 0) {
                    rxi_SendXmitList(call, call->xmitList, nXmitPackets,
-                                    istack, &now, &retryTime, resending);
+                                    istack, &now, &retryTime);
                }
 
 #ifdef AFS_GLOBAL_RXLOCK_KERNEL