r8169 MTU greater than 1500 causes corruption for other controllers

r8169 MTU greater than 1500 causes corruption for other controllers

[Timothy J Fontaine]

[-- Attachment #1: Type: text/plain, Size: 2560 bytes --]

Setting the MTU to greater than 1500 causes the host system to run out
of SW-IOMMU space for the transfer. This results in corruption for other
controllers like my AAR-2410SA or IDE controller.

Host system:
 ASUS P5M2 w/ Intel Core 2 Duo 6320
 Linux version 2.6.24-19-server (buildd@king) (gcc version 4.2.3 (Ubuntu 4.2.3-2ubuntu7)) #1 SMP Fri Jul 11 21:50:43 UTC 2008
 r8169 chipset pci card

Connected via Cross-Over cable to:
 SuperMicro 6014Hi
 Onboard Intel 82546GB e1000 chipset
 Running latest VMware ESXi or Vanilla 2.6.26

Initially I presumed the issue was with my aacraid because of the
trace[1], so I built a vanilla 2.6.26.2 and hit the same bug. I have
another system running 2.6.20 and the same card working so I tried
2.6.22 and had no problem. However, 2.6.23 failed. I sifted through the
ChangeLog for 2.6.23 and narrowed the field to some potential problem
commits regarding aacraid. I then proceeded to do a git bisect and found
commit b449655ff52ff8a29c66c5fc3fc03617e61182ee[2] which had nothing to
do with aacraid. Instead, the commit deals with large packets so I reset
the MTU to 1500 and retried on 2.6.24 and found that there was no
corruption, I then proceeded to try with various different size MTU only
to find that an MTU greater than 1500 triggers the corruption.

After initially encountering the bug with VMware ESXi I booted into a
vanilla 2.6.26 live cd and mounted an nfs share from the host with the
r8169 card and used dd_rescue to generate large files and transfers on
the host to expose the bug.

[1] http://tophat.atxconsulting.com/~tjfontaine/r8169_bug_report/oops_trace.txt
[2] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=b449655ff52ff8a29c66c5fc3fc03617e61182ee

Below you'll find a list of files referenced from REPORTING-BUGS that
are meant to be helpful, including the lspci and module information,
they can all be reached from the toplevel directory index

http://tophat.atxconsulting.com/~tjfontaine/r8169_bug_report/

http://tophat.atxconsulting.com/~tjfontaine/r8169_bug_report/cpuinfo
http://tophat.atxconsulting.com/~tjfontaine/r8169_bug_report/iomem
http://tophat.atxconsulting.com/~tjfontaine/r8169_bug_report/ioports
http://tophat.atxconsulting.com/~tjfontaine/r8169_bug_report/lspci
http://tophat.atxconsulting.com/~tjfontaine/r8169_bug_report/modules
http://tophat.atxconsulting.com/~tjfontaine/r8169_bug_report/oops_trace.txt
http://tophat.atxconsulting.com/~tjfontaine/r8169_bug_report/scsi
http://tophat.atxconsulting.com/~tjfontaine/r8169_bug_report/ver_linux

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: r8169 MTU greater than 1500 causes corruption for other controllers

[Ilpo Järvinen]

On Wed, 20 Aug 2008, Timothy J Fontaine wrote:

> Setting the MTU to greater than 1500 causes the host system to run out
> of SW-IOMMU space for the transfer. This results in corruption for other
> controllers like my AAR-2410SA or IDE controller.
> 
> Host system:
>  ASUS P5M2 w/ Intel Core 2 Duo 6320
>  Linux version 2.6.24-19-server (buildd@king) (gcc version 4.2.3 (Ubuntu 4.2.3-2ubuntu7)) #1 SMP Fri Jul 11 21:50:43 UTC 2008
>  r8169 chipset pci card
> 
> Connected via Cross-Over cable to:
>  SuperMicro 6014Hi
>  Onboard Intel 82546GB e1000 chipset
>  Running latest VMware ESXi or Vanilla 2.6.26
> 
> Initially I presumed the issue was with my aacraid because of the
> trace[1], so I built a vanilla 2.6.26.2 and hit the same bug. I have
> another system running 2.6.20 and the same card working so I tried
> 2.6.22 and had no problem. However, 2.6.23 failed. I sifted through the
> ChangeLog for 2.6.23 and narrowed the field to some potential problem
> commits regarding aacraid. I then proceeded to do a git bisect and found
> commit b449655ff52ff8a29c66c5fc3fc03617e61182ee[2] which had nothing to
> do with aacraid. Instead, the commit deals with large packets so I reset
> the MTU to 1500 and retried on 2.6.24 and found that there was no
> corruption, I then proceeded to try with various different size MTU only
> to find that an MTU greater than 1500 triggers the corruption.

Hmm, there was no explanation why the tp->rx_buf_sz to pkt_size change
was made in that commit (I have no idea whether those sizes have to match 
to other places that use tp->rx_buf_sz)... Below is a patch to revert 
that part of the change fully though it could be enough to change just 
pci_unmap_single arg (but that would probably need also 
rtl8169_mark_to_asic arg to be changed to match pkt_size).

> Below you'll find a list of files referenced from REPORTING-BUGS that
> are meant to be helpful, including the lspci and module information,
> they can all be reached from the toplevel directory index

Thanks for the detailed report btw... :-)

-- 
 i.

----
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index a3e3895..44961ef 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -2720,7 +2720,7 @@ static inline bool rtl8169_try_rx_copy(struct sk_buff **sk_buff,
 	if (!skb)
 		goto out;
 
-	pci_dma_sync_single_for_cpu(tp->pci_dev, addr, pkt_size,
+	pci_dma_sync_single_for_cpu(tp->pci_dev, addr, tp->rx_buf_sz,
 				    PCI_DMA_FROMDEVICE);
 	skb_reserve(skb, NET_IP_ALIGN);
 	skb_copy_from_linear_data(*sk_buff, skb->data, pkt_size);
@@ -2789,10 +2789,10 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
 
 			if (rtl8169_try_rx_copy(&skb, tp, pkt_size, addr)) {
 				pci_dma_sync_single_for_device(pdev, addr,
-					pkt_size, PCI_DMA_FROMDEVICE);
+					tp->rx_buf_sz, PCI_DMA_FROMDEVICE);
 				rtl8169_mark_to_asic(desc, tp->rx_buf_sz);
 			} else {
-				pci_unmap_single(pdev, addr, pkt_size,
+				pci_unmap_single(pdev, addr, tp->rx_buf_sz,
 						 PCI_DMA_FROMDEVICE);
 				tp->Rx_skbuff[entry] = NULL;
 			}

Re: r8169 MTU greater than 1500 causes corruption for other controllers

[Timothy J Fontaine]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

On Wed, Aug 20, 2008 at 10:21:11PM +0300, =?ISO-8859-1?Q?Ilpo_J=E4rvinen_ wrote:
> Hmm, there was no explanation why the tp->rx_buf_sz to pkt_size change
> was made in that commit (I have no idea whether those sizes have to match 
> to other places that use tp->rx_buf_sz)... Below is a patch to revert 
> that part of the change fully though it could be enough to change just 
> pci_unmap_single arg (but that would probably need also 
> rtl8169_mark_to_asic arg to be changed to match pkt_size).
> 

Excellent, with the patch applied transfers over NFS seem to be going
smoothly.

> > Below you'll find a list of files referenced from REPORTING-BUGS that
> > are meant to be helpful, including the lspci and module information,
> > they can all be reached from the toplevel directory index
> 
> Thanks for the detailed report btw... :-)

Thanks :) I was more than just a little intimidated by my first post to
LKML, and I wanted to make sure the first reply to my report wasn't that I
didn't have enough info, or that I presented it in the wrong way, or
hadn't done enough testing with it. I was also more than prepared to
wait weeks for someone to reply, so thanks for the quick return! :D

tjfontaine

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: r8169 MTU greater than 1500 causes corruption for other controllers

[Francois Romieu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 892 bytes --]

Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> :
[...]
> Hmm, there was no explanation why the tp->rx_buf_sz to pkt_size change
> was made in that commit (I have no idea whether those sizes have to match 
> to other places that use tp->rx_buf_sz)...

Because there is no need to sync data that has not been used ?

Timothy, could you check if the patch below does the job too ? It should
fix at least one pci_map / pci_unmap unbalance.

@@ -2789,10 +2789,10 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
 
 			if (rtl8169_try_rx_copy(&skb, tp, pkt_size, addr)) {
 				pci_dma_sync_single_for_device(pdev, addr,
 					pkt_size, PCI_DMA_FROMDEVICE);
 				rtl8169_mark_to_asic(desc, tp->rx_buf_sz);
 			} else {
-				pci_unmap_single(pdev, addr, pkt_size,
+				pci_unmap_single(pdev, addr, tp->rx_buf_sz,
 						 PCI_DMA_FROMDEVICE);
 				tp->Rx_skbuff[entry] = NULL;
 			}

-- 
Ueimor

Re: r8169 MTU greater than 1500 causes corruption for other controllers

[Timothy J Fontaine]

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

On Wed, Aug 20, 2008 at 11:43:50PM +0200, Francois Romieu wrote:
> Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> :
> [...]
> > Hmm, there was no explanation why the tp->rx_buf_sz to pkt_size change
> > was made in that commit (I have no idea whether those sizes have to match 
> > to other places that use tp->rx_buf_sz)...
> 
> Because there is no need to sync data that has not been used ?
> 
> Timothy, could you check if the patch below does the job too ? It should
> fix at least one pci_map / pci_unmap unbalance.
> 
> @@ -2789,10 +2789,10 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
>  
>  			if (rtl8169_try_rx_copy(&skb, tp, pkt_size, addr)) {
>  				pci_dma_sync_single_for_device(pdev, addr,
>  					pkt_size, PCI_DMA_FROMDEVICE);
>  				rtl8169_mark_to_asic(desc, tp->rx_buf_sz);
>  			} else {
> -				pci_unmap_single(pdev, addr, pkt_size,
> +				pci_unmap_single(pdev, addr, tp->rx_buf_sz,
>  						 PCI_DMA_FROMDEVICE);
>  				tp->Rx_skbuff[entry] = NULL;
>  			}
> 
> -- 
> Ueimor

Yup, this patch also works.

tjfontaine

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]