Re: [PATCH 1/4] pch_gbe: Fix the checksum fill to the error location

[Eric Dumazet <eric.dumazet@gmail.com>]

On Tue, 2012-07-17 at 09:09 +0200, Eric Dumazet wrote:

> Hmm... I fail to understand why you care about NIC doing checksums,
> while pch_gbe_tx_queue() make a _copy_ of each outgoing
> packets.
> 
> There _must_ be a way to avoid most of these copies (ie not touching
> payload), only mess with the header to insert these 2 nul bytes ?
> 
> /* [Header:14][payload] ---> [Header:14][paddong:2][payload]    */
> 
> So at device setup : dev->needed_headroom = 2;
> 
> and in xmit,
> 
> 	if (skb_headroom(skb) < 2) {
> 		struct sk_buff *skb_new;
> 
> 		skb_new = skb_realloc_headroom(skb, 2);
> 		if (!skb_new) { handle error }
> 		consume_skb(skb);
> 		skb = skb_new;
> 	}
> 	ptr = skb_push(skb, 2);
> 	memmove(ptr, ptr + 2, ETH_HLEN);
> 	ptr[ETH_HLEN] = 0;
> 	ptr[ETH_HLEN + 1] = 0;
> 
> 

Something like the following (untested) patch


 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |   55 +++++-----
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index b100656..2d3d982 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -1163,7 +1163,7 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 	struct pch_gbe_hw *hw = &adapter->hw;
 	struct pch_gbe_tx_desc *tx_desc;
 	struct pch_gbe_buffer *buffer_info;
-	struct sk_buff *tmp_skb;
+	char *ptr;
 	unsigned int frame_ctrl;
 	unsigned int ring_num;
 
@@ -1221,18 +1221,27 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 
 
 	buffer_info = &tx_ring->buffer_info[ring_num];
-	tmp_skb = buffer_info->skb;
+	if (skb_headroom(skb) < 2) {
+		struct sk_buff *skb_new;
+
+		skb_new = skb_realloc_headroom(skb, 2);
+		if (!skb_new) {
+			tx_ring->next_to_use = ring_num;
+			dev_kfree_skb_any(skb);
+			return;
+		}
+		consume_skb(skb);
+		skb = skb_new;
+	}
 
 	/* [Header:14][payload] ---> [Header:14][paddong:2][payload]    */
-	memcpy(tmp_skb->data, skb->data, ETH_HLEN);
-	tmp_skb->data[ETH_HLEN] = 0x00;
-	tmp_skb->data[ETH_HLEN + 1] = 0x00;
-	tmp_skb->len = skb->len;
-	memcpy(&tmp_skb->data[ETH_HLEN + 2], &skb->data[ETH_HLEN],
-	       (skb->len - ETH_HLEN));
+	ptr = skb_push(skb, 2);
+	memmove(ptr, ptr + 2, ETH_HLEN);
+	ptr[ETH_HLEN] = 0x00;
+	ptr[ETH_HLEN + 1] = 0x00;
 	/*-- Set Buffer information --*/
-	buffer_info->length = tmp_skb->len;
-	buffer_info->dma = dma_map_single(&adapter->pdev->dev, tmp_skb->data,
+	buffer_info->length = skb->len;
+	buffer_info->dma = dma_map_single(&adapter->pdev->dev, skb->data,
 					  buffer_info->length,
 					  DMA_TO_DEVICE);
 	if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma)) {
@@ -1240,18 +1249,20 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 		buffer_info->dma = 0;
 		buffer_info->time_stamp = 0;
 		tx_ring->next_to_use = ring_num;
+		dev_kfree_skb_any(skb);
 		return;
 	}
 	buffer_info->mapped = true;
 	buffer_info->time_stamp = jiffies;
+	buffer_info->skb = skb;
 
 	/*-- Set Tx descriptor --*/
 	tx_desc = PCH_GBE_TX_DESC(*tx_ring, ring_num);
-	tx_desc->buffer_addr = (buffer_info->dma);
-	tx_desc->length = (tmp_skb->len);
-	tx_desc->tx_words_eob = ((tmp_skb->len + 3));
+	tx_desc->buffer_addr = buffer_info->dma;
+	tx_desc->length = skb->len;
+	tx_desc->tx_words_eob = skb->len + 3;
 	tx_desc->tx_frame_ctrl = (frame_ctrl);
-	tx_desc->gbec_status = (DSC_INIT16);
+	tx_desc->gbec_status = DSC_INIT16;
 
 	if (unlikely(++ring_num == tx_ring->count))
 		ring_num = 0;
@@ -1265,7 +1276,6 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 	pch_tx_timestamp(adapter, skb);
 #endif
 
-	dev_kfree_skb_any(skb);
 }
 
 /**
@@ -1543,19 +1553,12 @@ static void pch_gbe_alloc_tx_buffers(struct pch_gbe_adapter *adapter,
 					struct pch_gbe_tx_ring *tx_ring)
 {
 	struct pch_gbe_buffer *buffer_info;
-	struct sk_buff *skb;
 	unsigned int i;
-	unsigned int bufsz;
 	struct pch_gbe_tx_desc *tx_desc;
 
-	bufsz =
-	    adapter->hw.mac.max_frame_size + PCH_GBE_DMA_ALIGN + NET_IP_ALIGN;
-
 	for (i = 0; i < tx_ring->count; i++) {
 		buffer_info = &tx_ring->buffer_info[i];
-		skb = netdev_alloc_skb(adapter->netdev, bufsz);
-		skb_reserve(skb, PCH_GBE_DMA_ALIGN);
-		buffer_info->skb = skb;
+		buffer_info->skb = NULL;
 		tx_desc = PCH_GBE_TX_DESC(*tx_ring, i);
 		tx_desc->gbec_status = (DSC_INIT16);
 	}
@@ -1622,9 +1625,9 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 					 buffer_info->length, DMA_TO_DEVICE);
 			buffer_info->mapped = false;
 		}
-		if (buffer_info->skb) {
-			pr_debug("trim buffer_info->skb : %d\n", i);
-			skb_trim(buffer_info->skb, 0);
+		if (skb) {
+			dev_kfree_skb_any(skb);
+			buffer_info->skb = NULL;
 		}
 		tx_desc->gbec_status = DSC_INIT16;
 		if (unlikely(++i == tx_ring->count))