From patchwork Mon Aug 6 14:19:22 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Cress X-Patchwork-Id: 175369 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 6FB272C008C for ; Tue, 7 Aug 2012 00:19:27 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756442Ab2HFOTZ (ORCPT ); Mon, 6 Aug 2012 10:19:25 -0400 Received: from mail.us.kontron.com ([209.118.138.70]:59986 "EHLO mail.us.kontron.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756336Ab2HFOTY (ORCPT ); Mon, 6 Aug 2012 10:19:24 -0400 X-ASG-Debug-ID: 1344262763-03b74931867d7b00001-BZBGGp Received: from ka-exchange-1.kontronamerica.local ([10.100.2.39]) by mail.us.kontron.com with ESMTP id fKZiJF0oJYFftfZv; Mon, 06 Aug 2012 07:19:23 -0700 (PDT) X-Barracuda-Envelope-From: andy.cress@us.kontron.com X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Subject: RE: pch_gbe: dont-copy-payload (was [PATCH 1/4] ...) Date: Mon, 6 Aug 2012 07:19:22 -0700 X-ASG-Orig-Subj: RE: pch_gbe: dont-copy-payload (was [PATCH 1/4] ...) Message-ID: <40680C535D6FE6498883F1640FACD44D0122CF12@ka-exchange-1.kontronamerica.local> In-Reply-To: <40680C535D6FE6498883F1640FACD44D011BBE1D@ka-exchange-1.kontronamerica.local> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: pch_gbe: dont-copy-payload (was [PATCH 1/4] ...) Thread-Index: Ac1j7mxEpeQsJTklTFKSCgOlm4jg8AGsfkygAk8b5IA= References: <40680C535D6FE6498883F1640FACD44D011432D4@ka-exchange-1.kontronamerica.local> <1342508968.2626.148.camel@edumazet-glaptop> <1342510387.2626.174.camel@edumazet-glaptop> <40680C535D6FE6498883F1640FACD44D011BBE1D@ka-exchange-1.kontronamerica.local> From: "Andy Cress" To: "Eric Dumazet" Cc: X-Barracuda-Connect: UNKNOWN[10.100.2.39] X-Barracuda-Start-Time: 1344262763 X-Barracuda-URL: http://10.100.2.60:8000/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at kontron.com X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=5.0 tests=BSF_SC0_MISMATCH_TO X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.2.104857 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.00 BSF_SC0_MISMATCH_TO Envelope rcpt doesn't match header Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org I found out why the proposed dont-copy-payload patch didn't work with this pch_gbe NIC. This NIC PHY requires 64-byte-aligned DMA, and the transmit buffers won't be transferred if skb->data is not 64-byte-aligned. Apparently the data copy has a by-product of aligning the buffers. I tried using skb_reserve(skb,64) in pch_gbe_alloc_tx_buffers and pch-gbe_alloc_rx_buffers, but that didn't seem to resolve it. How can I make sure that the transmit data buffers are 64-byte-aligned? Andy -----Original Message----- From: Andy Cress Sent: Wednesday, July 25, 2012 4:11 PM Subject: RE: [PATCH 1/4] pch_gbe: Fix the checksum fill to the error location [...] I tried applying the dont-copy-payload patch you outlined below, and for some reason it won't send successfully with that change. I'm not sure why though. This approach seems sound, and should greatly help transmit performance, if I could figure out what else it needs. Andy -----Original Message----- From: Eric Dumazet Sent: Tuesday, July 17, 2012 3:33 AM To: Andy Cress Cc: netdev@vger.kernel.org; Zhong Hongbo Subject: Re: [PATCH 1/4] pch_gbe: Fix the checksum fill to the error location On Tue, 2012-07-17 at 09:09 +0200, Eric Dumazet wrote: [...] > 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] */ [...] 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))