From 643a632a38d3a9b0b9d4819abb37e227b5946c47 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Tue, 12 Aug 2008 19:39:59 +0000 Subject: [PATCH] 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() --- src/rx/rx_globals.h | 21 ++++++++++++++------- src/rx/rx_packet.c | 13 +++++++++---- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/rx/rx_globals.h b/src/rx/rx_globals.h index c054071b4..5f28495b9 100644 --- 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 7f083de39..398ca066d 100644 --- 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++) { -- 2.39.5