| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <40680C535D6FE6498883F1640FACD44D011BBE1D@ka-exchange-1.kontronamerica.local>
Date: Wed, 25 Jul 2012 13:10:51 -0700
From: "Andy Cress" <andy.cress@...kontron.com>
To: "Eric Dumazet" <eric.dumazet@...il.com>
Cc: <netdev@...r.kernel.org>,
"Zhong Hongbo" <hongbo.zhong@...driver.com>
Subject: RE: [PATCH 1/4] pch_gbe: Fix the checksum fill to the error location
Eric,
For the checksum patch, that needs to be tabled until Hongbo has a chance to evaluate/revise it, but that isn't vital to the goal of avoiding transmit timeouts, so the other 3 patches should still go forward. I'll submit a revised patch set with just those 3.
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 [mailto:eric.dumazet@...il.com]
Sent: Tuesday, July 17, 2012 3:33 AM
To: Andy Cress
Cc: netdev@...r.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:
> 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))
Powered by blists - more mailing lists