From 4300e8c7f64d95a80ffa7d98d98738f41546bc30 Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Fri, 26 Mar 2010 10:23:30 -0700 Subject: Revert "r8169: enable 64-bit DMA by default for PCI Express devices (v2)" This reverts commit 353176888386d9025062a12dcec08d49af10cf2c. People are reporting problems due to this change and there is no anticipation that the cause will be tracked down any time soon. We can try next time to selectively re-enable this based upon chip type, or have a black list of some sort. Signed-off-by: David S. Miller --- drivers/net/r8169.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) (limited to 'drivers/net/r8169.c') diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index 9d3ebf3e975e..b93fd23b9f0d 100644 --- a/drivers/net/r8169.c +++ b/drivers/net/r8169.c @@ -187,7 +187,7 @@ static DEFINE_PCI_DEVICE_TABLE(rtl8169_pci_tbl) = { MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl); static int rx_copybreak = 200; -static int use_dac = -1; +static int use_dac; static struct { u32 msg_enable; } debug = { -1 }; @@ -511,8 +511,7 @@ MODULE_DESCRIPTION("RealTek RTL-8169 Gigabit Ethernet driver"); module_param(rx_copybreak, int, 0); MODULE_PARM_DESC(rx_copybreak, "Copy breakpoint for copy-only-tiny-frames"); module_param(use_dac, int, 0); -MODULE_PARM_DESC(use_dac, "Enable PCI DAC. -1 defaults on for PCI Express only." -" Unsafe on 32 bit PCI slot."); +MODULE_PARM_DESC(use_dac, "Enable PCI DAC. Unsafe on 32 bit PCI slot."); module_param_named(debug, debug.msg_enable, int, 0); MODULE_PARM_DESC(debug, "Debug verbosity level (0=none, ..., 16=all)"); MODULE_LICENSE("GPL"); @@ -2974,7 +2973,6 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) void __iomem *ioaddr; unsigned int i; int rc; - int this_use_dac = use_dac; if (netif_msg_drv(&debug)) { printk(KERN_INFO "%s Gigabit Ethernet driver %s loaded\n", @@ -3040,17 +3038,8 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) tp->cp_cmd = PCIMulRW | RxChkSum; - tp->pcie_cap = pci_find_capability(pdev, PCI_CAP_ID_EXP); - if (!tp->pcie_cap) - netif_info(tp, probe, dev, "no PCI Express capability\n"); - - if (this_use_dac < 0) - this_use_dac = tp->pcie_cap != 0; - if ((sizeof(dma_addr_t) > 4) && - this_use_dac && - !pci_set_dma_mask(pdev, DMA_BIT_MASK(64))) { - netif_info(tp, probe, dev, "using 64-bit DMA\n"); + !pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) && use_dac) { tp->cp_cmd |= PCIDAC; dev->features |= NETIF_F_HIGHDMA; } else { @@ -3069,6 +3058,10 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) goto err_out_free_res_4; } + tp->pcie_cap = pci_find_capability(pdev, PCI_CAP_ID_EXP); + if (!tp->pcie_cap) + netif_info(tp, probe, dev, "no PCI Express capability\n"); + RTL_W16(IntrMask, 0x0000); /* Soft reset the chip. */ -- cgit v1.2.3 From 78f1cd02457252e1ffbc6caa44a17424a45286b8 Mon Sep 17 00:00:00 2001 From: Francois Romieu Date: Sat, 27 Mar 2010 19:35:46 -0700 Subject: r8169: fix broken register writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is quite similar to b39fe41f481d20c201012e4483e76c203802dda7 though said registers are not even documented as 64-bit registers - as opposed to the initial TxDescStartAddress ones - but as single bytes which must be combined into 32 bits at the MMIO read/write level before being merged into a 64 bit logical entity. Credits go to Ben Hutchings for the MAR registers (aka "multicast is broken for ages on ARM) and to Timo Teräs for the MAC registers. Signed-off-by: Francois Romieu Signed-off-by: David S. Miller --- drivers/net/r8169.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/net/r8169.c') diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index b93fd23b9f0d..7193afc00e47 100644 --- a/drivers/net/r8169.c +++ b/drivers/net/r8169.c @@ -2820,8 +2820,8 @@ static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr) spin_lock_irq(&tp->lock); RTL_W8(Cfg9346, Cfg9346_Unlock); - RTL_W32(MAC0, low); RTL_W32(MAC4, high); + RTL_W32(MAC0, low); RTL_W8(Cfg9346, Cfg9346_Lock); spin_unlock_irq(&tp->lock); @@ -4747,8 +4747,8 @@ static void rtl_set_rx_mode(struct net_device *dev) mc_filter[1] = swab32(data); } - RTL_W32(MAR0 + 0, mc_filter[0]); RTL_W32(MAR0 + 4, mc_filter[1]); + RTL_W32(MAR0 + 0, mc_filter[0]); RTL_W32(RxConfig, tmp); -- cgit v1.2.3 From c0cd884af045338476b8e69a61fceb3f34ff22f1 Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Mon, 29 Mar 2010 13:16:02 -0700 Subject: r8169: offical fix for CVE-2009-4537 (overlength frame DMAs) Official patch to fix the r8169 frame length check error. Based on this initial thread: http://marc.info/?l=linux-netdev&m=126202972828626&w=1 This is the official patch to fix the frame length problems in the r8169 driver. As noted in the previous thread, while this patch incurs a performance hit on the driver, its possible to improve performance dynamically by updating the mtu and rx_copybreak values at runtime to return performance to what it was for those NICS which are unaffected by the ideosyncracy (if there are any). Summary: A while back Eric submitted a patch for r8169 in which the proper allocated frame size was written to RXMaxSize to prevent the NIC from dmaing too much data. This was done in commit fdd7b4c3302c93f6833e338903ea77245eb510b4. A long time prior to that however, Francois posted 126fa4b9ca5d9d7cb7d46f779ad3bd3631ca387c, which expiclitly disabled the MaxSize setting due to the fact that the hardware behaved in odd ways when overlong frames were received on NIC's supported by this driver. This was mentioned in a security conference recently: http://events.ccc.de/congress/2009/Fahrplan//events/3596.en.html It seems that if we can't enable frame size filtering, then, as Eric correctly noticed, we can find ourselves DMA-ing too much data to a buffer, causing corruption. As a result is seems that we are forced to allocate a frame which is ready to handle a maximally sized receive. This obviously has performance issues with it, so to mitigate that issue, this patch does two things: 1) Raises the copybreak value to the frame allocation size, which should force appropriately sized packets to get allocated on rx, rather than a full new 16k buffer. 2) This patch only disables frame filtering initially (i.e., during the NIC open), changing the MTU results in ring buffer allocation of a size in relation to the new mtu (along with a warning indicating that this is dangerous). Because of item (2), individuals who can't cope with the performance hit (or can otherwise filter frames to prevent the bug), or who have hardware they are sure is unaffected by this issue, can manually lower the copybreak and reset the mtu such that performance is restored easily. Signed-off-by: Neil Horman Signed-off-by: David S. Miller --- drivers/net/r8169.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) (limited to 'drivers/net/r8169.c') diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index 7193afc00e47..96740051cdcc 100644 --- a/drivers/net/r8169.c +++ b/drivers/net/r8169.c @@ -186,7 +186,12 @@ static DEFINE_PCI_DEVICE_TABLE(rtl8169_pci_tbl) = { MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl); -static int rx_copybreak = 200; +/* + * we set our copybreak very high so that we don't have + * to allocate 16k frames all the time (see note in + * rtl8169_open() + */ +static int rx_copybreak = 16383; static int use_dac; static struct { u32 msg_enable; @@ -3217,9 +3222,13 @@ static void __devexit rtl8169_remove_one(struct pci_dev *pdev) } static void rtl8169_set_rxbufsize(struct rtl8169_private *tp, - struct net_device *dev) + unsigned int mtu) { - unsigned int max_frame = dev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN; + unsigned int max_frame = mtu + VLAN_ETH_HLEN + ETH_FCS_LEN; + + if (max_frame != 16383) + printk(KERN_WARNING "WARNING! Changing of MTU on this NIC" + "May lead to frame reception errors!\n"); tp->rx_buf_sz = (max_frame > RX_BUF_SIZE) ? max_frame : RX_BUF_SIZE; } @@ -3231,7 +3240,17 @@ static int rtl8169_open(struct net_device *dev) int retval = -ENOMEM; - rtl8169_set_rxbufsize(tp, dev); + /* + * Note that we use a magic value here, its wierd I know + * its done because, some subset of rtl8169 hardware suffers from + * a problem in which frames received that are longer than + * the size set in RxMaxSize register return garbage sizes + * when received. To avoid this we need to turn off filtering, + * which is done by setting a value of 16383 in the RxMaxSize register + * and allocating 16k frames to handle the largest possible rx value + * thats what the magic math below does. + */ + rtl8169_set_rxbufsize(tp, 16383 - VLAN_ETH_HLEN - ETH_FCS_LEN); /* * Rx and Tx desscriptors needs 256 bytes alignment. @@ -3884,7 +3903,7 @@ static int rtl8169_change_mtu(struct net_device *dev, int new_mtu) rtl8169_down(dev); - rtl8169_set_rxbufsize(tp, dev); + rtl8169_set_rxbufsize(tp, dev->mtu); ret = rtl8169_init_ring(dev); if (ret < 0) -- cgit v1.2.3 From 93f4d91d879acfcb0ba9c2725e3133fcff2dfd1e Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Thu, 1 Apr 2010 07:30:07 +0000 Subject: r8169: clean up my printk uglyness Fix formatting on r8169 printk Brandon Philips noted that I had a spacing issue in my printk for the last r8169 patch that made it quite ugly. Fix that up and add the PFX macro to it as well so it looks like the other r8169 printks Signed-off-by: Neil Horman Signed-off-by: David S. Miller --- drivers/net/r8169.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/net/r8169.c') diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index 96740051cdcc..dbb1f5a1824c 100644 --- a/drivers/net/r8169.c +++ b/drivers/net/r8169.c @@ -3227,8 +3227,8 @@ static void rtl8169_set_rxbufsize(struct rtl8169_private *tp, unsigned int max_frame = mtu + VLAN_ETH_HLEN + ETH_FCS_LEN; if (max_frame != 16383) - printk(KERN_WARNING "WARNING! Changing of MTU on this NIC" - "May lead to frame reception errors!\n"); + printk(KERN_WARNING PFX "WARNING! Changing of MTU on this " + "NIC may lead to frame reception errors!\n"); tp->rx_buf_sz = (max_frame > RX_BUF_SIZE) ? max_frame : RX_BUF_SIZE; } -- cgit v1.2.3