rx: Add RX_CALL_ACKALL_SENT flag and rxi_SendAck processing

3cd3715e608b801b4848399e42cb47464e6e3cc3 modified rxi_ReceiveDataPacket
to send an ACKALL whenever RX_CALL_RECEIVE_DONE is set on the call.
This produced the potential for a race with ACKs that set the
firstPacket value to 'rnext' when the receive queue for the call
has yet to be emptied.  From the perspective of receiver the ACK
was already processed and does not require a response since the
previously received ACKALL acknowledged the delivery of all data
packets to the application.  When sending ACKs after ACKALL it is
therefore required that firstPacket be set to the sequence number
after the last unprocessed packet in the receive queue.

Thanks to Simon Wilkinson for his extensive assistance in identifying
the problem and the development of this patchset.

Reviewed-on: http://gerrit.openafs.org/4798
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit cd326b2f54c3397468807c32ce0834f73c9d5d1b)

Change-Id: Ic4fa021eb33462ddf759ce0d7a8845edf1ecbed5
Reviewed-on: http://gerrit.openafs.org/4914
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>

diff --git a/src/rx/rx.c b/src/rx/rx.c
index b482ad200a0430621a34ccc9993bca4f7aba6267..6128bd7cfa91a2fdb41473f4cb1e1a9b24789e03 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -4827,6 +4827,7 @@ rxi_AckAll(struct rxevent *event, struct rx_call *call, char *dummy)
     }
     rxi_SendSpecial(call, call->conn, (struct rx_packet *)0,
                    RX_PACKET_TYPE_ACKALL, NULL, 0, 0);
+    call->flags |= RX_CALL_ACKALL_SENT;
     if (event)
        MUTEX_EXIT(&call->lock);
 #else /* RX_ENABLE_LOCKS */
@@ -4834,6 +4835,7 @@ rxi_AckAll(struct rxevent *event, struct rx_call *call, char *dummy)
        call->delayedAckEvent = NULL;
     rxi_SendSpecial(call, call->conn, (struct rx_packet *)0,
                    RX_PACKET_TYPE_ACKALL, NULL, 0, 0);
+    call->flags |= RX_CALL_ACKALL_SENT;
 #endif /* RX_ENABLE_LOCKS */
 }
 
@@ -5419,7 +5421,21 @@ rxi_SendAck(struct rx_call *call,
     ap->serial = htonl(serial);
     ap->maxSkew = 0;           /* used to be peer->inPacketSkew */
 
-    ap->firstPacket = htonl(call->rnext);      /* First packet not yet forwarded to reader */
+    /*
+     * First packet not yet forwarded to reader. When ACKALL has been
+     * sent the peer has been told that all received packets will be
+     * delivered to the reader.  The value 'rnext' is used internally
+     * to refer to the next packet in the receive queue that must be
+     * delivered to the reader.  From the perspective of the peer it
+     * already has so report the last sequence number plus one if there
+     * are packets in the receive queue awaiting processing.
+     */
+    if ((call->flags & RX_CALL_ACKALL_SENT) &&
+        !queue_IsEmpty(&call->rq)) {
+        ap->firstPacket = htonl(queue_Last(&call->rq, rx_packet)->header.seq + 1);
+    } else
+        ap->firstPacket = htonl(call->rnext);
+
     ap->previousPacket = htonl(call->rprev);   /* Previous packet received */
 
     /* No fear of running out of ack packet here because there can only be at most

diff --git a/src/rx/rx.h b/src/rx/rx.h
index d37fcda2d6002a212e1398b6eba30618a3e558d0..e7dc9a932b6b3a3a8a852d27fa19e919ed95a903 100644 (file)
--- a/src/rx/rx.h
+++ b/src/rx/rx.h
@@ -643,6 +643,7 @@ struct rx_call {
 #define RX_CALL_NEED_START     0x10000 /* tells rxi_Start to start again */
 #define RX_CALL_PEER_BUSY      0x20000 /* the last packet we received on this call was a
                                          * BUSY packet; i.e. the channel for this call is busy */
+#define RX_CALL_ACKALL_SENT     0x40000 /* ACKALL has been sent on the call */
 
 
 /* The structure of the data portion of an acknowledge packet: An acknowledge