rx-buffer-allocation-20080812

LICENSE MIT

Prevent rxi_MorePacketsNoLock() from dereferencing a NULL pointer if
the requested allocation size cannot be satsified.  In that case back
off the number of packets until osi_Alloc() succeeds or panic if no
packets can be allocated at all.

In AllocPacketBufs() do not transfer more than rx_TSFPQGlobSize packets.

Modify RX_TS_FPQ_GTOL2() macro to protect against transfering more packets
that are actually free.

Modify RX_TS_FPQ_COMPUTE_LIMITS() to enforce a rx_TSFPQGlobSize maximum
value of 64 packets to prevent ever increasing allocation sizes within
AllocPacketBufs()

diff --git a/src/rx/rx_globals.h b/src/rx/rx_globals.h
index c054071..5f28495 100644 (file)
--- a/src/rx/rx_globals.h
+++ b/src/rx/rx_globals.h
@@ -245,17 +245,22 @@ EXT void rxi_FlushLocalPacketsTSFPQ(void); /* flush all thread-local packets to
 #define RX_TS_FPQ_FLUSH_GLOBAL 1
 #define RX_TS_FPQ_PULL_GLOBAL 1
 #define RX_TS_FPQ_ALLOW_OVERCOMMIT 1
-/* compute the localmax and globsize values from rx_TSFPQMaxProcs and rx_nPackets.
-   arbitarily set local max so that all threads consume 90% of packets, if all local queues are full.
-   arbitarily set transfer glob size to 20% of max local packet queue length.
-   also set minimum values of 15 and 3. */
+/*
+ * compute the localmax and globsize values from rx_TSFPQMaxProcs and rx_nPackets.
+ * arbitarily set local max so that all threads consume 90% of packets, if all local queues are full.
+ * arbitarily set transfer glob size to 20% of max local packet queue length.
+ * also set minimum values of 15 and 3.  Given the algorithms, the number of buffers allocated
+ * by each call to AllocPacketBufs() will increase indefinitely without a cap on the transfer
+ * glob size.  A cap of 64 is selected because that will produce an allocation of greater than
+ * three times that amount which is greater than half of ncalls * maxReceiveWindow. 
+ */
 #define RX_TS_FPQ_COMPUTE_LIMITS \
     do { \
         register int newmax, newglob; \
         newmax = (rx_nPackets * 9) / (10 * rx_TSFPQMaxProcs); \
         newmax = (newmax >= 15) ? newmax : 15; \
         newglob = newmax / 5; \
-        newglob = (newglob >= 3) ? newglob : 3; \
+        newglob = (newglob >= 3) ? (newglob < 64 ? newglob : 64) : 3; \
         rx_TSFPQLocalMax = newmax; \
         rx_TSFPQGlobSize = newglob; \
     } while(0)
@@ -325,10 +330,12 @@ EXT void rxi_FlushLocalPacketsTSFPQ(void); /* flush all thread-local packets to
 /* same as above, except user has direct control over number to transfer */
 #define RX_TS_FPQ_GTOL2(rx_ts_info_p,num_transfer) \
     do { \
-        register int i; \
+        register int i, tsize; \
         register struct rx_packet * p; \
+        tsize = (num_transfer); \
+        if (tsize > rx_nFreePackets) tsize = rx_nFreePackets; \
         for (i=0,p=queue_First(&rx_freePacketQueue, rx_packet); \
-             i < (num_transfer); i++,p=queue_Next(p, rx_packet)); \
+             i < tsize; i++,p=queue_Next(p, rx_packet)); \
         queue_SplitBeforeAppend(&rx_freePacketQueue,&((rx_ts_info_p)->_FPQ),p); \
         (rx_ts_info_p)->_FPQ.len += i; \
         rx_nFreePackets -= i; \

diff --git a/src/rx/rx_packet.c b/src/rx/rx_packet.c
index 7f083de..398ca06 100644 (file)
--- a/src/rx/rx_packet.c
+++ b/src/rx/rx_packet.c
@@ -285,7 +285,7 @@ AllocPacketBufs(int class, int num_pkts, struct rx_queue * q)
            /* alloc enough for us, plus a few globs for other threads */
            alloc = transfer + (3 * rx_TSFPQGlobSize) - rx_nFreePackets;
            rxi_MorePacketsNoLock(MAX(alloc, rx_initSendWindow));
-           transfer += rx_TSFPQGlobSize;
+           transfer = rx_TSFPQGlobSize;
        }
 
        RX_TS_FPQ_GTOL2(rx_ts_info, transfer);
@@ -654,9 +654,14 @@ rxi_MorePacketsNoLock(int apackets)
      * to hold maximal amounts of data */
     apackets += (apackets / 4)
        * ((rx_maxJumboRecvSize - RX_FIRSTBUFFERSIZE) / RX_CBUFFERSIZE);
-    getme = apackets * sizeof(struct rx_packet);
-    p = rx_mallocedP = (struct rx_packet *)osi_Alloc(getme);
-
+    do {
+        getme = apackets * sizeof(struct rx_packet);
+        p = rx_mallocedP = (struct rx_packet *)osi_Alloc(getme);
+       if (p == NULL) {
+            apackets -= apackets / 4;
+            osi_Assert(apackets > 0);
+        }
+    } while(p == NULL);
     memset((char *)p, 0, getme);
 
     for (e = p + apackets; p < e; p++) {