From greg@kroah.com Tue Dec 13 14:05:49 2005 Received: with ECARTIS (v1.0.0; list netdev); Tue, 13 Dec 2005 14:06:31 -0800 (PST) Received: from perch.kroah.org (mail.kroah.org [69.55.234.183]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBDM5jZ7031502 for ; Tue, 13 Dec 2005 14:05:49 -0800 Received: from [192.168.0.10] (c-24-22-115-24.hsd1.or.comcast.net [24.22.115.24]) (authenticated) by perch.kroah.org (8.11.6/8.11.6) with ESMTP id jBDM0CL31980; Tue, 13 Dec 2005 14:00:12 -0800 Received: from greg by echidna.kroah.org with local (masqmail 0.2.19) id 1EmIBe-4Mh-00; Tue, 13 Dec 2005 13:59:54 -0800 Date: Tue, 13 Dec 2005 13:59:54 -0800 From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org, stable@kernel.org Cc: Justin Forbes , Zwane Mwaikambo , "Theodore Ts'o" , Randy Dunlap , Dave Jones , Chuck Wolber , torvalds@osdl.org, akpm@osdl.org, alan@lxorguk.ukuu.org.uk, ryanh@us.ibm.com, davem@davemloft.net, netdev@oss.sgi.com Subject: [patch 2/2] [PATCH] br: fix race on bridge del if Message-ID: <20051213215954.GC16739@kroah.com> References: <20051213214109.971735000@press.kroah.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline; filename="br-fix-race-on-bridge-del-if.patch" In-Reply-To: <20051213215936.GA16739@kroah.com> User-Agent: Mutt/1.5.11 X-archive-position: 4157 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: gregkh@suse.de Precedence: bulk X-list: netdev Content-Length: 1234 Lines: 41 -stable review patch. If anyone has any objections, please let us know. ------------------ From: Stephen Hemminger This fixes the RCU race on bridge delete interface. Basically, the network device has to be detached from the bridge in the first step (pre-RCU), rather than later. At that point, no more bridge traffic will come in, and the other code will not think that network device is part of a bridge. This should also fix the XEN test problems. If there is another 2.6.13-stable, add it as well. Signed-off-by: Stephen Hemminger Signed-off-by: Chris Wright Signed-off-by: Greg Kroah-Hartman --- net/bridge/br_if.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- linux-2.6.13.4.orig/net/bridge/br_if.c +++ linux-2.6.13.4/net/bridge/br_if.c @@ -79,7 +79,6 @@ static void destroy_nbp(struct net_bridg { struct net_device *dev = p->dev; - dev->br_port = NULL; p->br = NULL; p->dev = NULL; dev_put(dev); @@ -100,6 +99,7 @@ static void del_nbp(struct net_bridge_po struct net_bridge *br = p->br; struct net_device *dev = p->dev; + dev->br_port = NULL; dev_set_promiscuity(dev, -1); spin_lock_bh(&br->lock); -- From ak@suse.de Thu Dec 15 16:32:48 2005 Received: with ECARTIS (v1.0.0; list netdev); Thu, 15 Dec 2005 16:32:50 -0800 (PST) Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBG0Wl8n017481 for ; Thu, 15 Dec 2005 16:32:48 -0800 Received: from Relay2.suse.de (mail2.suse.de [195.135.221.8]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 950131D936; Fri, 16 Dec 2005 01:29:01 +0100 (CET) Date: Fri, 16 Dec 2005 01:29:01 +0100 From: Andi Kleen To: Ayaz Abdulla Cc: jgarzik@pobox.com, netdev@oss.sgi.com Subject: Re: 2.6.15rc5-git4 Forcedeth unstable on Nforce4 - it's not TSO Message-ID: <20051216002901.GP23384@wotan.suse.de> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-archive-position: 4261 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: ak@suse.de Precedence: bulk X-list: netdev Content-Length: 164 Lines: 6 I spoke too early when I said earlier that #undef NETIF_F_TSO fixes it. Without it it seems to survive longer, but I still get occasional corrupted MACs. -Andi From manfred@colorfullife.com Thu Dec 22 14:09:36 2005 Received: with ECARTIS (v1.0.0; list netdev); Thu, 22 Dec 2005 14:09:38 -0800 (PST) Received: from dbl.q-ag.de ([213.172.117.3]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBMM9Q8n007982 for ; Thu, 22 Dec 2005 14:09:35 -0800 Received: from [127.0.0.2] (dbl [127.0.0.1]) by dbl.q-ag.de (8.13.3/8.13.3/Debian-6) with ESMTP id jBMMFsbk004982; Thu, 22 Dec 2005 23:15:55 +0100 Message-ID: <43AB232E.7050309@colorfullife.com> Date: Thu, 22 Dec 2005 23:05:34 +0100 From: Manfred Spraul User-Agent: Mozilla/5.0 (X11; U; Linux i686; fr-FR; rv:1.7.12) Gecko/20050923 Fedora/1.7.12-1.5.1 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Jeff Garzik CC: Netdev Subject: [PATCH] forcedeth: Fix incorrect parameters in pci_unmap_single calls Content-Type: multipart/mixed; boundary="------------020905000705080401070509" X-archive-position: 4598 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: manfred@colorfullife.com Precedence: bulk X-list: netdev Content-Length: 2108 Lines: 66 This is a multi-part message in MIME format. --------------020905000705080401070509 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Hi, The natsemi rx codepath calls pci_unmap_single with a different size than the initial pci_map_single call. The attached patch fixes that. Signed-Off-By: Manfred Spraul --------------020905000705080401070509 Content-Type: text/plain; name="patch-natsemi-rxunmapsize" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="patch-natsemi-rxunmapsize" --- 2.6/drivers/net/natsemi.c 2005-12-19 01:36:54.000000000 +0100 +++ x64/drivers/net/natsemi.c 2005-12-22 22:10:56.000000000 +0100 @@ -1920,6 +1920,7 @@ int entry = np->dirty_rx % RX_RING_SIZE; if (np->rx_skbuff[entry] == NULL) { unsigned int buflen = np->rx_buf_sz+NATSEMI_PADDING; + skb = dev_alloc_skb(buflen); np->rx_skbuff[entry] = skb; if (skb == NULL) @@ -2006,7 +2007,6 @@ static void drain_rx(struct net_device *dev) { struct netdev_private *np = netdev_priv(dev); - unsigned int buflen = np->rx_buf_sz; int i; /* Free all the skbuffs in the Rx queue. */ @@ -2014,6 +2014,8 @@ np->rx_ring[i].cmd_status = 0; np->rx_ring[i].addr = 0xBADF00D0; /* An invalid address. */ if (np->rx_skbuff[i]) { + unsigned int buflen = np->rx_buf_sz+NATSEMI_PADDING; + pci_unmap_single(np->pci_dev, np->rx_dma[i], buflen, PCI_DMA_FROMDEVICE); @@ -2225,7 +2227,6 @@ int entry = np->cur_rx % RX_RING_SIZE; int boguscnt = np->dirty_rx + RX_RING_SIZE - np->cur_rx; s32 desc_status = le32_to_cpu(np->rx_head_desc->cmd_status); - unsigned int buflen = np->rx_buf_sz; void __iomem * ioaddr = ns_ioaddr(dev); /* If the driver owns the next entry it's a new packet. Send it up. */ @@ -2267,6 +2268,8 @@ */ } else { struct sk_buff *skb; + unsigned int buflen = np->rx_buf_sz+NATSEMI_PADDING; + /* Omit CRC size. */ /* Check if the packet is long enough to accept * without copying to a minimally-sized skbuff. */ --------------020905000705080401070509-- From manfred@colorfullife.com Sat Dec 24 05:23:40 2005 Received: with ECARTIS (v1.0.0; list netdev); Sat, 24 Dec 2005 05:23:51 -0800 (PST) Received: from dbl.q-ag.de ([213.172.117.3]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBODNd8n019904 for ; Sat, 24 Dec 2005 05:23:40 -0800 Received: from [127.0.0.2] (dbl [127.0.0.1]) by dbl.q-ag.de (8.13.3/8.13.3/Debian-6) with ESMTP id jBODTlxC011490; Sat, 24 Dec 2005 14:29:52 +0100 Message-ID: <43AD4ADC.8050004@colorfullife.com> Date: Sat, 24 Dec 2005 14:19:24 +0100 From: Manfred Spraul User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; fr-FR; rv:1.7.12) Gecko/20050923 Fedora/1.7.12-1.5.1 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Linus Torvalds CC: Jeff Garzik , Ayaz Abdulla , Linux Kernel Mailing List , Netdev Subject: [PATCH] forcedeth: fix random memory scribbling bug Content-Type: multipart/mixed; boundary="------------050507070605060005030100" X-archive-position: 4717 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: manfred@colorfullife.com Precedence: bulk X-list: netdev Content-Length: 3119 Lines: 91 This is a multi-part message in MIME format. --------------050507070605060005030100 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Two critical bugs were found in forcedeth 0.47: - TSO doesn't work. - pci_map_single() for the rx buffers is called with size==0. This bug is critical, it causes random memory corruptions on systems with an iommu. Below is a minimal fix for both bugs, for inclusion into 2.6.15. TSO will be fixed properly in the next version. Tested on x86-64. Signed-Off-By: Manfred Spraul --------------050507070605060005030100 Content-Type: text/plain; name="patch-forcedeth-048-bugfixes" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="patch-forcedeth-048-bugfixes" --- 2.6/drivers/net/forcedeth.c 2005-12-19 01:36:54.000000000 +0100 +++ x64/drivers/net/forcedeth.c 2005-12-24 12:16:30.000000000 +0100 @@ -10,7 +10,7 @@ * trademarks of NVIDIA Corporation in the United States and other * countries. * - * Copyright (C) 2003,4 Manfred Spraul + * Copyright (C) 2003,4,5 Manfred Spraul * Copyright (C) 2004 Andrew de Quincey (wol support) * Copyright (C) 2004 Carl-Daniel Hailfinger (invalid MAC handling, insane * IRQ rate fixes, bigendian fixes, cleanups, verification) @@ -100,6 +100,7 @@ * 0.45: 18 Sep 2005: Remove nv_stop/start_rx from every link check * 0.46: 20 Oct 2005: Add irq optimization modes. * 0.47: 26 Oct 2005: Add phyaddr 0 in phy scan. + * 0.48: 24 Dec 2005: Disable TSO, bugfix for pci_map_single * * Known bugs: * We suspect that on some hardware no TX done interrupts are generated. @@ -111,7 +112,7 @@ * DEV_NEED_TIMERIRQ will not harm you on sane hardware, only generating a few * superfluous timer interrupts from the nic. */ -#define FORCEDETH_VERSION "0.47" +#define FORCEDETH_VERSION "0.48" #define DRV_NAME "forcedeth" #include @@ -871,8 +872,8 @@ } else { skb = np->rx_skbuff[nr]; } - np->rx_dma[nr] = pci_map_single(np->pci_dev, skb->data, skb->len, - PCI_DMA_FROMDEVICE); + np->rx_dma[nr] = pci_map_single(np->pci_dev, skb->data, + skb->end-skb->data, PCI_DMA_FROMDEVICE); if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) { np->rx_ring.orig[nr].PacketBuffer = cpu_to_le32(np->rx_dma[nr]); wmb(); @@ -999,7 +1000,7 @@ wmb(); if (np->rx_skbuff[i]) { pci_unmap_single(np->pci_dev, np->rx_dma[i], - np->rx_skbuff[i]->len, + np->rx_skbuff[i]->end-np->rx_skbuff[i]->data, PCI_DMA_FROMDEVICE); dev_kfree_skb(np->rx_skbuff[i]); np->rx_skbuff[i] = NULL; @@ -1334,7 +1335,7 @@ * the performance. */ pci_unmap_single(np->pci_dev, np->rx_dma[i], - np->rx_skbuff[i]->len, + np->rx_skbuff[i]->end-np->rx_skbuff[i]->data, PCI_DMA_FROMDEVICE); { @@ -2455,7 +2456,7 @@ np->txrxctl_bits |= NVREG_TXRXCTL_RXCHECK; dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG; #ifdef NETIF_F_TSO - dev->features |= NETIF_F_TSO; + /* disabled dev->features |= NETIF_F_TSO; */ #endif } --------------050507070605060005030100-- From jgarzik@pobox.com Sat Dec 24 07:13:36 2005 Received: with ECARTIS (v1.0.0; list netdev); Sat, 24 Dec 2005 07:13:43 -0800 (PST) Received: from mail.dvmed.net ([216.237.124.58]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBOFDZ8n010299 for ; Sat, 24 Dec 2005 07:13:36 -0800 Received: from cpe-069-134-188-146.nc.res.rr.com ([69.134.188.146] helo=[10.10.10.88]) by mail.dvmed.net with esmtpsa (Exim 4.52 #1 (Red Hat Linux)) id 1EqB1Z-0000Ep-SL; Sat, 24 Dec 2005 15:09:34 +0000 Message-ID: <43AD64AB.2070306@pobox.com> Date: Sat, 24 Dec 2005 10:09:31 -0500 From: Jeff Garzik User-Agent: Mozilla Thunderbird 1.0.7-1.1.fc4 (X11/20050929) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Manfred Spraul CC: Linus Torvalds , Ayaz Abdulla , Linux Kernel Mailing List , Netdev Subject: Re: [PATCH] forcedeth: fix random memory scribbling bug References: <43AD4ADC.8050004@colorfullife.com> In-Reply-To: <43AD4ADC.8050004@colorfullife.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-archive-position: 4725 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: jgarzik@pobox.com Precedence: bulk X-list: netdev Content-Length: 716 Lines: 24 Manfred Spraul wrote: > Two critical bugs were found in forcedeth 0.47: > - TSO doesn't work. > - pci_map_single() for the rx buffers is called with size==0. This bug > is critical, it causes random memory corruptions on systems with an iommu. > > Below is a minimal fix for both bugs, for inclusion into 2.6.15. > TSO will be fixed properly in the next version. > Tested on x86-64. > > Signed-Off-By: Manfred Spraul 1) Why does forcedeth require a non-standard calculation for each pci_map_single() call? 2) I have requested multiple times that you avoid MIME... 3) Why disable TSO completely? It sounds like it should default to off, then permit enabling via ethtool. Jeff From manfred@colorfullife.com Sat Dec 24 08:12:29 2005 Received: with ECARTIS (v1.0.0; list netdev); Sat, 24 Dec 2005 08:12:31 -0800 (PST) Received: from dbl.q-ag.de (dbl.q-ag.de [213.172.117.3]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBOGCR8n023324 for ; Sat, 24 Dec 2005 08:12:28 -0800 Received: from [127.0.0.2] (dbl [127.0.0.1]) by dbl.q-ag.de (8.13.3/8.13.3/Debian-6) with ESMTP id jBOGIYRY012041; Sat, 24 Dec 2005 17:18:34 +0100 Message-ID: <43AD726A.5010703@colorfullife.com> Date: Sat, 24 Dec 2005 17:08:10 +0100 From: Manfred Spraul User-Agent: Mozilla/5.0 (X11; U; Linux i686; fr-FR; rv:1.7.12) Gecko/20050923 Fedora/1.7.12-1.5.1 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Jeff Garzik CC: Linus Torvalds , Ayaz Abdulla , Linux Kernel Mailing List , Netdev Subject: Re: [PATCH] forcedeth: fix random memory scribbling bug References: <43AD4ADC.8050004@colorfullife.com> <43AD64AB.2070306@pobox.com> In-Reply-To: <43AD64AB.2070306@pobox.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-archive-position: 4729 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: manfred@colorfullife.com Precedence: bulk X-list: netdev Content-Length: 1584 Lines: 45 Jeff Garzik wrote: > Manfred Spraul wrote: > >> Two critical bugs were found in forcedeth 0.47: >> - TSO doesn't work. >> - pci_map_single() for the rx buffers is called with size==0. This >> bug is critical, it causes random memory corruptions on systems with >> an iommu. >> >> Below is a minimal fix for both bugs, for inclusion into 2.6.15. >> TSO will be fixed properly in the next version. >> Tested on x86-64. >> >> Signed-Off-By: Manfred Spraul > > > 1) Why does forcedeth require a non-standard calculation for each > pci_map_single() call? > - skb->len is the wrong thing (tm), since it's 0 until skb_put(). - I have not found a field that contains the actual size of the data area of an skb. - the results must be identical for map and unmap. - I could recalculate the size of the allocation from np->rx_buf_sz, but I don't like that. Right now it would work, but it's too subtile that changing rx_buf_sz while there are outstanding rx buffers results in a iommu memory leak. Therefore I decided to calculate the mapping size with "skb->end - skb->data": The size of the mapping for an skb is calculated by looking at fields in the skb, no knowledge about driver fields. > 2) I have requested multiple times that you avoid MIME... > It's the first time that you complain about Content-Transfer-Encoding: 7bit attachments. > 3) Why disable TSO completely? It sounds like it should default to > off, then permit enabling via ethtool. > The bugfix is in 0.49 - it's just a bit larger, I would consider it for 2.5.16. -- Manfred From torvalds@osdl.org Sat Dec 24 11:56:30 2005 Received: with ECARTIS (v1.0.0; list netdev); Sat, 24 Dec 2005 11:56:32 -0800 (PST) Received: from smtp.osdl.org (smtp.osdl.org [65.172.181.4]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBOJuT8n008398 for ; Sat, 24 Dec 2005 11:56:29 -0800 Received: from shell0.pdx.osdl.net (fw.osdl.org [65.172.181.6]) by smtp.osdl.org (8.12.8/8.12.8) with ESMTP id jBOJqQDZ024028 (version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=NO); Sat, 24 Dec 2005 11:52:26 -0800 Received: from localhost (shell0.pdx.osdl.net [10.9.0.31]) by shell0.pdx.osdl.net (8.13.1/8.11.6) with ESMTP id jBOJqP5w008977; Sat, 24 Dec 2005 11:52:25 -0800 Date: Sat, 24 Dec 2005 11:52:25 -0800 (PST) From: Linus Torvalds To: Manfred Spraul cc: Jeff Garzik , Ayaz Abdulla , Linux Kernel Mailing List , Netdev Subject: Re: [PATCH] forcedeth: fix random memory scribbling bug In-Reply-To: <43AD4ADC.8050004@colorfullife.com> Message-ID: References: <43AD4ADC.8050004@colorfullife.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-MIMEDefang-Filter: osdl$Revision: 1.129 $ X-Scanned-By: MIMEDefang 2.36 X-archive-position: 4741 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: torvalds@osdl.org Precedence: bulk X-list: netdev Content-Length: 934 Lines: 32 On Sat, 24 Dec 2005, Manfred Spraul wrote: > > Two critical bugs were found in forcedeth 0.47: > - TSO doesn't work. > - pci_map_single() for the rx buffers is called with size==0. This bug is > critical, it causes random memory corruptions on systems with an iommu. Good catch. Btw, should we perhaps disallow (or at least WARN_ON()) pci_map_single() with a length of zero? I think it's always likely a bug.. However, that "skb->end - skb->data" calculation is a bit strange. It correctly maps the whole skb, but wouldn't it make more sense to use the length we actually tell the card to use? In other words, wouldn't it be a whole lot more sensible and logical to use np->rx_buf_sz instead? That's the value we use for allocation and that's the size we tell the card we have. Of course, on the alloc path, it seems to add an additional "NV_RX_ALLOC_PAD" thing, so maybe the "end-data" thing makes sense. Linus From manfred@colorfullife.com Sat Dec 24 12:00:01 2005 Received: with ECARTIS (v1.0.0; list netdev); Sat, 24 Dec 2005 12:00:08 -0800 (PST) Received: from dbl.q-ag.de (dbl.q-ag.de [213.172.117.3]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBOJxx8n009258 for ; Sat, 24 Dec 2005 12:00:00 -0800 Received: from [127.0.0.2] (dbl [127.0.0.1]) by dbl.q-ag.de (8.13.3/8.13.3/Debian-6) with ESMTP id jBOK6P9f013468; Sat, 24 Dec 2005 21:06:25 +0100 Message-ID: <43ADA7D0.9010908@colorfullife.com> Date: Sat, 24 Dec 2005 20:56:00 +0100 From: Manfred Spraul User-Agent: Mozilla/5.0 (X11; U; Linux i686; fr-FR; rv:1.7.12) Gecko/20050923 Fedora/1.7.12-1.5.1 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Linus Torvalds CC: Jeff Garzik , Ayaz Abdulla , Linux Kernel Mailing List , Netdev Subject: Re: [PATCH] forcedeth: fix random memory scribbling bug References: <43AD4ADC.8050004@colorfullife.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-archive-position: 4742 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: manfred@colorfullife.com Precedence: bulk X-list: netdev Content-Length: 456 Lines: 15 Linus Torvalds wrote: >Of course, on the alloc path, it seems to add an additional >"NV_RX_ALLOC_PAD" thing, so maybe the "end-data" thing makes sense. > > > The problem is the pci_unmap_single() call that happens during nv_close() or the rx interrupt handler. I think it makes more sense to rely on fields in the individual skb instead of reading from np->rx_buf_sz. If np->rx_buf_sz changes inbetween, then we have a memory leak. -- Manfred From torvalds@osdl.org Sat Dec 24 12:01:50 2005 Received: with ECARTIS (v1.0.0; list netdev); Sat, 24 Dec 2005 12:01:52 -0800 (PST) Received: from smtp.osdl.org (smtp.osdl.org [65.172.181.4]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBOK1n8n009938 for ; Sat, 24 Dec 2005 12:01:49 -0800 Received: from shell0.pdx.osdl.net (fw.osdl.org [65.172.181.6]) by smtp.osdl.org (8.12.8/8.12.8) with ESMTP id jBOJvnDZ024227 (version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=NO); Sat, 24 Dec 2005 11:57:49 -0800 Received: from localhost (shell0.pdx.osdl.net [10.9.0.31]) by shell0.pdx.osdl.net (8.13.1/8.11.6) with ESMTP id jBOJvm0i009167; Sat, 24 Dec 2005 11:57:48 -0800 Date: Sat, 24 Dec 2005 11:57:48 -0800 (PST) From: Linus Torvalds To: Manfred Spraul cc: Jeff Garzik , Ayaz Abdulla , Linux Kernel Mailing List , Netdev Subject: Re: [PATCH] forcedeth: fix random memory scribbling bug In-Reply-To: <43AD726A.5010703@colorfullife.com> Message-ID: References: <43AD4ADC.8050004@colorfullife.com> <43AD64AB.2070306@pobox.com> <43AD726A.5010703@colorfullife.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-MIMEDefang-Filter: osdl$Revision: 1.129 $ X-Scanned-By: MIMEDefang 2.36 X-archive-position: 4743 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: torvalds@osdl.org Precedence: bulk X-list: netdev Content-Length: 739 Lines: 22 On Sat, 24 Dec 2005, Manfred Spraul wrote: > > > 2) I have requested multiple times that you avoid MIME... > > It's the first time that you complain about Content-Transfer-Encoding: 7bit > attachments. These proper text encodings are easy to _apply_, because the raw email is uncorrupted. However, attachments are still broken for a very fundamental reason: basically no email client will ever quote them on replies. Which means that if somebody has commentary about some specific part of the patch, the attachement is _totally_ the wrong thing to do. In other words, there's a reason I encourage people VERY STRONGLY to use in-line patches. If you have a broken mailer that corrupts whitespace, please just fix it. Linus From jgarzik@pobox.com Sat Dec 24 12:02:26 2005 Received: with ECARTIS (v1.0.0; list netdev); Sat, 24 Dec 2005 12:02:31 -0800 (PST) Received: from mail.dvmed.net (mail.dvmed.net [216.237.124.58]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBOK2Q8n010124 for ; Sat, 24 Dec 2005 12:02:26 -0800 Received: from cpe-069-134-188-146.nc.res.rr.com ([69.134.188.146] helo=[10.10.10.88]) by mail.dvmed.net with esmtpsa (Exim 4.52 #1 (Red Hat Linux)) id 1EqFXB-0000OL-Fl; Sat, 24 Dec 2005 19:58:30 +0000 Message-ID: <43ADA862.6010906@pobox.com> Date: Sat, 24 Dec 2005 14:58:26 -0500 From: Jeff Garzik User-Agent: Mozilla Thunderbird 1.0.7-1.1.fc4 (X11/20050929) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Linus Torvalds CC: Manfred Spraul , Ayaz Abdulla , Linux Kernel Mailing List , Netdev Subject: Re: [PATCH] forcedeth: fix random memory scribbling bug References: <43AD4ADC.8050004@colorfullife.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-archive-position: 4744 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: jgarzik@pobox.com Precedence: bulk X-list: netdev Content-Length: 726 Lines: 33 Linus Torvalds wrote: > However, that > > "skb->end - skb->data" > > calculation is a bit strange. It correctly maps the whole skb, but nod > wouldn't it make more sense to use the length we actually tell the card to > use? > > In other words, wouldn't it be a whole lot more sensible and logical to > use > > np->rx_buf_sz > > instead? That's the value we use for allocation and that's the size we > tell the card we have. That's the sort of thing I prefer. > Of course, on the alloc path, it seems to add an additional > "NV_RX_ALLOC_PAD" thing, so maybe the "end-data" thing makes sense. None of the other ethernet drivers do 'end - data', which is why I hesitated quite a bit on this change. Jeff From torvalds@osdl.org Sat Dec 24 12:45:40 2005 Received: with ECARTIS (v1.0.0; list netdev); Sat, 24 Dec 2005 12:45:43 -0800 (PST) Received: from smtp.osdl.org (smtp.osdl.org [65.172.181.4]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBOKjd8n023297 for ; Sat, 24 Dec 2005 12:45:40 -0800 Received: from shell0.pdx.osdl.net (fw.osdl.org [65.172.181.6]) by smtp.osdl.org (8.12.8/8.12.8) with ESMTP id jBOKfbDZ025626 (version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=NO); Sat, 24 Dec 2005 12:41:38 -0800 Received: from localhost (shell0.pdx.osdl.net [10.9.0.31]) by shell0.pdx.osdl.net (8.13.1/8.11.6) with ESMTP id jBOKfZRU010478; Sat, 24 Dec 2005 12:41:36 -0800 Date: Sat, 24 Dec 2005 12:41:35 -0800 (PST) From: Linus Torvalds To: Manfred Spraul cc: Jeff Garzik , Ayaz Abdulla , Linux Kernel Mailing List , Netdev Subject: Re: [PATCH] forcedeth: fix random memory scribbling bug In-Reply-To: <43ADA7D0.9010908@colorfullife.com> Message-ID: References: <43AD4ADC.8050004@colorfullife.com> <43ADA7D0.9010908@colorfullife.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-MIMEDefang-Filter: osdl$Revision: 1.129 $ X-Scanned-By: MIMEDefang 2.36 X-archive-position: 4745 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: torvalds@osdl.org Precedence: bulk X-list: netdev Content-Length: 531 Lines: 17 On Sat, 24 Dec 2005, Manfred Spraul wrote: > Linus Torvalds wrote: > > > Of course, on the alloc path, it seems to add an additional > > "NV_RX_ALLOC_PAD" thing, so maybe the "end-data" thing makes sense. > > The problem is the pci_unmap_single() call that happens during nv_close() or > the rx interrupt handler. I think it makes more sense to rely on fields in the > individual skb instead of reading from np->rx_buf_sz. If np->rx_buf_sz changes > inbetween, then we have a memory leak. Fair enough. Patch applied. Linus From jgarzik@pobox.com Sat Dec 24 13:09:58 2005 Received: with ECARTIS (v1.0.0; list netdev); Sat, 24 Dec 2005 13:10:00 -0800 (PST) Received: from mail.dvmed.net (mail.dvmed.net [216.237.124.58]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id jBOL9w8n028587 for ; Sat, 24 Dec 2005 13:09:58 -0800 Received: from cpe-069-134-188-146.nc.res.rr.com ([69.134.188.146] helo=[10.10.10.88]) by mail.dvmed.net with esmtpsa (Exim 4.52 #1 (Red Hat Linux)) id 1EqGab-0000Q6-KZ; Sat, 24 Dec 2005 21:06:07 +0000 Message-ID: <43ADB83A.4090005@pobox.com> Date: Sat, 24 Dec 2005 16:06:02 -0500 From: Jeff Garzik User-Agent: Mozilla Thunderbird 1.0.7-1.1.fc4 (X11/20050929) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Linus Torvalds CC: Manfred Spraul , Ayaz Abdulla , Linux Kernel Mailing List , Netdev Subject: Re: [PATCH] forcedeth: fix random memory scribbling bug References: <43AD4ADC.8050004@colorfullife.com> <43ADA7D0.9010908@colorfullife.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-archive-position: 4747 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: jgarzik@pobox.com Precedence: bulk X-list: netdev Content-Length: 918 Lines: 31 Linus Torvalds wrote: > > On Sat, 24 Dec 2005, Manfred Spraul wrote: > > >>Linus Torvalds wrote: >> >> >>>Of course, on the alloc path, it seems to add an additional >>>"NV_RX_ALLOC_PAD" thing, so maybe the "end-data" thing makes sense. >> >>The problem is the pci_unmap_single() call that happens during nv_close() or >>the rx interrupt handler. I think it makes more sense to rely on fields in the >>individual skb instead of reading from np->rx_buf_sz. If np->rx_buf_sz changes >>inbetween, then we have a memory leak. > > > Fair enough. Patch applied. Paranoia -- the situation above never occurs. It is coded as are other drivers: np->rx_buf_sz only changes in ->change_mtu(), which (a) is serialized against close and (b) always stops the engine and drains RX skbs before changing the size. So can we please remove the subtraction code now added to the hot path? If not now, for 2.6.16? Jeff