rx: compare RX_ACK_TYPE_ACK as a bit-field

The rx_ackPacket.acks array (the SACK table) consists of up to 255
octets.  Each octet stores either the value zero (RX_ACK_TYPE_NACK)
or one (RX_ACK_TYPE_ACK).  Effectively only bit-zero of each octet
is used.

The rx_ackPacket.acks array cannot be enlarged but one possible
method of encoding the ACK/NACK state for packets when the
window size is greater than 255 is to use bits 1-7 of each
octet.

This change alters the test for ACK vs NACK to be a bit comparison
instead of a equality comparison.  This change permits RX to be
compatible with any future use of bits 1-7.

No peer that treats the SACK table as bytes can ever send more
than 255 packets regardless of the advertised receive window.
Therefore, existing peers will never receive a SACK table with
more than 255 packets worth of bits.

Change-Id: I4dfcdc9ea18e060eeb257832297f557b7109e93a
Reviewed-on: https://gerrit.openafs.org/14465
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

diff --git a/src/rx/rx.c b/src/rx/rx.c
index dc56f17..33ce345 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -4440,7 +4440,8 @@ rxi_ReceiveAckPacket(struct rx_call *call, struct rx_packet *np,
            int offset;
 
            for (offset = 0; offset < nAcks && len < sizeof(msg); offset++)
-               msg[len++] = (ap->acks[offset] == RX_ACK_TYPE_NACK ? '-' : '*');
+               msg[len++] = ((ap->acks[offset] & RX_ACK_TYPE_ACK) != 0 ? '*'
+                                                                       : '-');
        }
        msg[len++]='\n';
        msg[len] = '\0';
@@ -4456,7 +4457,7 @@ rxi_ReceiveAckPacket(struct rx_call *call, struct rx_packet *np,
        if (nAcks) {
            int offset;
            for (offset = 0; offset < nAcks; offset++)
-               putc(ap->acks[offset] == RX_ACK_TYPE_NACK ? '-' : '*',
+               putc((ap->acks[offset] & RX_ACK_TYPE_ACK) != 0 ? '*' : '-',
                     rx_Log);
        }
        putc('\n', rx_Log);
@@ -4572,7 +4573,7 @@ rxi_ReceiveAckPacket(struct rx_call *call, struct rx_packet *np,
         * be downgraded when the server has discarded a packet it
         * soacked previously, or when an ack packet is received
         * out of sequence. */
-       if (ap->acks[tp->header.seq - first] == RX_ACK_TYPE_ACK) {
+       if ((ap->acks[tp->header.seq - first] & RX_ACK_TYPE_ACK) != 0) {
            if (!(tp->flags & RX_PKTFLAG_ACKED)) {
                newAckCount++;
                tp->flags |= RX_PKTFLAG_ACKED;
@@ -5821,7 +5822,8 @@ rxi_SendAck(struct rx_call *call,
            int offset;
 
            for (offset = 0; offset < ap->nAcks && len < sizeof(msg); offset++)
-               msg[len++] = (ap->acks[offset] == RX_ACK_TYPE_NACK ? '-' : '*');
+               msg[len++] = ((ap->acks[offset] & RX_ACK_TYPE_ACK) != 0 ? '*'
+                                                                       : '-');
        }
        msg[len++]='\n';
        msg[len] = '\0';
@@ -5834,7 +5836,7 @@ rxi_SendAck(struct rx_call *call,
                (unsigned int)p->header.seq, ntohl(ap->firstPacket));
        if (ap->nAcks) {
            for (offset = 0; offset < ap->nAcks; offset++)
-               putc(ap->acks[offset] == RX_ACK_TYPE_NACK ? '-' : '*',
+               putc((ap->acks[offset] & RX_ACK_TYPE_ACK) != 0 ? '*' : '-',
                     rx_Log);
        }
        putc('\n', rx_Log);

diff --git a/src/rx/rx.h b/src/rx/rx.h
index 3e1b77e..f1d00c5 100644 (file)
--- a/src/rx/rx.h
+++ b/src/rx/rx.h
@@ -416,9 +416,22 @@ struct rx_ackPacket {
     afs_uint32 previousPacket; /* The previous packet number received (obsolete?) */
     afs_uint32 serial;         /* Serial number of the packet which prompted the acknowledge */
     u_char reason;             /* Reason for the acknowledge of ackPacket, defined below */
-    u_char nAcks;              /* Number of acknowledgements */
-    u_char acks[RX_MAXACKS];   /* Up to RX_MAXACKS packet acknowledgements, defined below */
-    /* Packets <firstPacket are implicitly acknowledged and may be discarded by the sender.  Packets >= firstPacket+nAcks are implicitly NOT acknowledged.  No packets with sequence numbers >= firstPacket should be discarded by the sender (they may thrown out at any time by the receiver) */
+    u_char nAcks;              /* Number of acknowledgements (saturates at 255) */
+    u_char acks[RX_MAXACKS];   /* Packet acknowledgements, one bit per packet.
+                                * The first (up to) RX_MAXACKS packets'
+                                * acknowledgment state is indicated by bit-0
+                                * of the corresponding byte of acks[].  The
+                                * additional bits are reserved for future use. */
+    /*
+     * DATA packets whose sequence number is less than firstPacket are
+     * implicitly acknowledged and may be discarded by the sender.
+     * DATA packets whose sequence number is greater than or equal to
+     * (firstPacket + nAcks) are implicitly NOT acknowledged.
+     * No DATA packets with sequence numbers greater than or equal to
+     * firstPacket should be discarded by the sender (they may be thrown
+     * out by the receiver and listed as NOT acknowledged in a subsequent
+     * ACK packet.)
+     */
 };
 
 #define FIRSTACKOFFSET 4
@@ -436,9 +449,9 @@ struct rx_ackPacket {
                                         * be used to compute RTT */
 #define RX_ACK_MTU             -1       /* will be rewritten to ACK_PING */
 
-/* Packet acknowledgement type */
-#define        RX_ACK_TYPE_NACK        0       /* I Don't have this packet */
-#define        RX_ACK_TYPE_ACK         1       /* I have this packet, although I may discard it later */
+/* Packet acknowledgement type (for maximum window size 255) */
+#define RX_ACK_TYPE_NACK        0x0     /* I Don't have this packet */
+#define RX_ACK_TYPE_ACK         0x1     /* I have this packet, although I may discard it later */
 
 /* The packet size transmitted for an acknowledge is adjusted to reflect the actual size of the acks array.  This macro defines the size */
 #define rx_AckDataSize(nAcks) (3 + nAcks + offsetof(struct rx_ackPacket, acks[0]))