Re: [PATCH 1/4] pch_gbe: Fix the checksum fill to the error location
From: Eric Dumazet <hidden>
Date: 2012-07-17 07:33:12
Subsystem:
networking drivers, the rest · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
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))