[ofa-general] [PATCH] connected mode leaks pages on receive & reuses old skb

[Ralph Campbell]

This patch fixes a bug in connected mode where the newly allocated skb
is not put on the SRQ ring and the unused pages from the old skb are not
transferred to the new one.

This patch was created in Roland's for-2.6.24 branch but
probably applies to 2.6.23 too.

Here is the basic order of events in the original code:

init:
create SRQ with ipoib_recvq_size entries
fill SRQ ring with max sized SKBs by calling ipoib_cm_alloc_rx_skb()
post all SKBs w/ num_sge = IPOIB_CM_RX_SG

receive:
allocate new SKB based on msg size, fill mapping 0..frags by doing DMA map
DMA unmap mapping[0..frags] for old SKB
NOTE that [frags..IPOIB_CM_RX_SG-1] are *not* unmapped (which is OK).
replace srq_ring[id].mapping[0..frags] with new mapping from new SKB
(data is still in old SKB's frag pointers)
NOTE that [frags..IPOIB_CM_RX_SG-1] are *not* copied to newskb and
thus leaked. Also, newskb's nr_frags is set based on the message size
rather than the full size.
NOTE that srq_ring[id].skb is *not* updated to point to newskb and
thus will be used again after being passed to the Linux network stack.

Also, use ib_dma_unmap_page() for addresses mapped by ib_dma_map_page().

Signed-off-by: Ralph Campbell <ralph.campbell at qlogic.com>

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
index 08b4676..a3434f2 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -78,7 +78,7 @@ static void ipoib_cm_dma_unmap_rx(struct ipoib_dev_priv *priv, int frags,
 	ib_dma_unmap_single(priv->ca, mapping[0], IPOIB_CM_HEAD_SIZE, DMA_FROM_DEVICE);
 
 	for (i = 0; i < frags; ++i)
-		ib_dma_unmap_single(priv->ca, mapping[i + 1], PAGE_SIZE, DMA_FROM_DEVICE);
+		ib_dma_unmap_page(priv->ca, mapping[i + 1], PAGE_SIZE, DMA_FROM_DEVICE);
 }
 
 static int ipoib_cm_post_receive(struct net_device *dev, int id)
@@ -149,7 +149,7 @@ partial_error:
 	ib_dma_unmap_single(priv->ca, mapping[0], IPOIB_CM_HEAD_SIZE, DMA_FROM_DEVICE);
 
 	for (; i > 0; --i)
-		ib_dma_unmap_single(priv->ca, mapping[i], PAGE_SIZE, DMA_FROM_DEVICE);
+		ib_dma_unmap_page(priv->ca, mapping[i], PAGE_SIZE, DMA_FROM_DEVICE);
 
 	dev_kfree_skb_any(skb);
 	return NULL;
@@ -365,7 +365,11 @@ static int ipoib_cm_rx_handler(struct ib_cm_id *cm_id,
 		return 0;
 	}
 }
-/* Adjust length of skb with fragments to match received data */
+
+/*
+ * Adjust the length of skb to match the received data and
+ * move the unused frag pages in skb to the (empty) frag slots in toskb.
+ */
 static void skb_put_frags(struct sk_buff *skb, unsigned int hdr_space,
 			  unsigned int length, struct sk_buff *toskb)
 {
@@ -378,23 +382,26 @@ static void skb_put_frags(struct sk_buff *skb, unsigned int hdr_space,
 	skb->len += size;
 	length -= size;
 
-	num_frags = skb_shinfo(skb)->nr_frags;
+	num_frags = skb_shinfo(toskb)->nr_frags;
 	for (i = 0; i < num_frags; i++) {
 		skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
 
-		if (length == 0) {
-			/* don't need this page */
-			skb_fill_page_desc(toskb, i, frag->page, 0, PAGE_SIZE);
-			--skb_shinfo(skb)->nr_frags;
-		} else {
-			size = min(length, (unsigned) PAGE_SIZE);
-
-			frag->size = size;
-			skb->data_len += size;
-			skb->truesize += size;
-			skb->len += size;
-			length -= size;
-		}
+		size = min(length, (unsigned) PAGE_SIZE);
+		WARN_ON(size == 0);
+		frag->size = size;
+		skb->data_len += size;
+		skb->truesize += size;
+		skb->len += size;
+		length -= size;
+	}
+	WARN_ON(length != 0);
+	skb_shinfo(skb)->nr_frags = num_frags;
+
+	/* move unused pages from skb to toskb */
+	for (; i < IPOIB_CM_RX_SG - 1; i++) {
+		skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+
+		skb_fill_page_desc(toskb, i, frag->page, 0, PAGE_SIZE);
 	}
 }
 
@@ -463,6 +470,7 @@ void ipoib_cm_handle_rx_wc(struct net_device *dev, struct ib_wc *wc)
 
 	ipoib_cm_dma_unmap_rx(priv, frags, priv->cm.srq_ring[wr_id].mapping);
 	memcpy(priv->cm.srq_ring[wr_id].mapping, mapping, (frags + 1) * sizeof *mapping);
+	priv->cm.srq_ring[wr_id].skb = newskb;
 
 	ipoib_dbg_data(priv, "received %d bytes, SLID 0x%04x\n",
 		       wc->byte_len, wc->slid);