Skip to contentSkip to conversation

ibmveth bug fixes 2/4

PATCH In review Santiago Leon 2 participants 3 messages Aug 10, 2004 Network drivers

lore
Share & tools
lore

Santiago Leon

author
#0 lore raw

PATCH 1/1 ibmveth bug fixes 2/4

Andrew,

This patch fixes a race condition that would panic the kernel when replenishing a buffer pool. Please apply.

Signed-off-by Santiago Leon <[email protected]>
Signature · 3 lines
Santiago A. Leon
Power Linux Development
IBM Linux Technology Center

===== drivers/net/ibmveth.c 1.14 vs edited =====

drivers/net/ibmveth.c +2 -1
@@ -219,6 +219,7 @@
 
 		dma_addr = vio_map_single(adapter->vdev, skb->data, pool->buff_size, DMA_FROM_DEVICE);
 
+               pool->free_map[free_index] = 0xffff;
 		pool->dma_addr[index] = dma_addr;
 		pool->skbuff[index] = skb;
 

@@ -233,6 +234,7 @@
 		lpar_rc = h_add_logical_lan_buffer(adapter->vdev->unit_address, desc.desc);
 		    
 		if(lpar_rc != H_Success) {
+                       pool->free_map[free_index] = index;
 			pool->skbuff[index] = NULL;
 			pool->consumer_index--;
 			vio_unmap_single(adapter->vdev, pool->dma_addr[index], pool->buff_size, DMA_FROM_DEVICE);

@@ -240,7 +242,6 @@
 			adapter->replenish_add_buff_failure++;
 			break;
 		} else {
-			pool->free_map[free_index] = 0xffff;
 			buffers_added++;
 			adapter->replenish_add_buff_success++;
 		}
#1 lore raw
Quoted 2 lines · Santiago Leon

This patch fixes a race condition that would panic the kernel when replenishing a buffer pool. Please apply.

How about something like this that doesn't add more magic numbers?

I'm not so sure about the type, though. Is that (u16) cast OK?

-- Dave

drivers/net/ibmveth.h renamed from drivers/net/ibmveth.h.orig +2 -0
@@ -77,6 +77,8 @@
 #define IbmVethPool1DftCnt  256
 #define IbmVethPool2DftCnt  256
 
+#define IBM_VETH_INVALID_MAP ((u16)0xffff)
+
 struct ibmveth_buff_pool {
     u32 size;
     u32 index;
drivers/net/ibmveth.c renamed from drivers/net/ibmveth.c.orig +3 -2
@@ -213,11 +213,12 @@ static void ibmveth_replenish_buffer_poo
 		free_index = pool->consumer_index++ % pool->size;
 		index = pool->free_map[free_index];
 	
-		ibmveth_assert(index != 0xffff);
+		ibmveth_assert(index != IBM_VETH_INVALID_MAP);
 		ibmveth_assert(pool->skbuff[index] == NULL);
 
 		dma_addr = vio_map_single(adapter->vdev, skb->data, pool->buff_size, DMA_FROM_DEVICE);
 
+		pool->free_map[free_index] = IBM_VETH_INVALID_MAP;
 		pool->dma_addr[index] = dma_addr;
 		pool->skbuff[index] = skb;
 

@@ -232,6 +233,7 @@ static void ibmveth_replenish_buffer_poo
 		lpar_rc = h_add_logical_lan_buffer(adapter->vdev->unit_address, desc.desc);
 		    
 		if(lpar_rc != H_Success) {
+			pool->free_map[free_index] = IBM_VETH_INVALID_MAP;
 			pool->skbuff[index] = NULL;
 			pool->consumer_index--;
 			vio_unmap_single(adapter->vdev, pool->dma_addr[index], pool->buff_size, DMA_FROM_DEVICE);

@@ -239,7 +241,6 @@ static void ibmveth_replenish_buffer_poo
 			adapter->replenish_add_buff_failure++;
 			break;
 		} else {
-			pool->free_map[free_index] = 0xffff;
 			buffers_added++;
 			adapter->replenish_add_buff_success++;
 		}
author
#2 lore raw
Quoted 3 lines

How about something like this that doesn't add more magic numbers?

I'm not so sure about the type, though. Is that (u16) cast OK?

Yep... Thanks Dave... no magic numbers is good... and the (u16) cast is ok...

Andrew... please apply Dave's patch...

Signature · 3 lines
Santiago A. Leon
Power Linux Development
IBM Linux Technology Center