r8169.c

r8169.c

[Dorn Hetzel]

I've been working on building a new system using an Abit AA8 Duramax
motherboard, which includes a realtek 8169/8110 gigE controller as 
well as Intel ICH6R AHCI SATA interface.

The r8169.c version 1.2 driver in 2.6.9 had issues hanging the ethernet
on the AA8, so I downloaded version 2.2 from the realtek.com.tw
website, which fixed that issue.

2.6.9, however, was not making happy noises with the ICH6R/AHCI SATA
controller, and after reviewing changelogs and finding encouraging
notes, I gave 2.6.10-rc2 a try and I am happy to report that it
seems to work very well with the AHCI SATA drives.

2.6.10-rc2 still uses the 1.2 8169 driver, same as 2.6.9...

However, the r8169 2.2 driver would not build with 2.6.10-rc2,
due to use of pci_dma_sync_single.

I've made a V2.3 of the 8169 driver which uses 
pci_dma_sync_single_for_cpu instead, and it seems to work fine.

I was hoping to submit a patch to move the new driver into
some appropriate release, but the 2.2/2.3 driver is so far
changed from 1.2 that the diff -u is about the size of the
original and new file combined :(

Which brings me to my question (my apologies for the roundabout path)...

With such a huge diff, should I send a diff, or the whole new file, or
do something else entirely?

Thanks in advance for any advice!

Dorn Hetzel

Re: r8169.c

[Francois Romieu]

Dorn Hetzel <kernel@dorn.hetzel.org> :
[Abit AA8 experience]
> 2.6.9, however, was not making happy noises with the ICH6R/AHCI SATA
> controller, and after reviewing changelogs and finding encouraging
> notes, I gave 2.6.10-rc2 a try and I am happy to report that it
> seems to work very well with the AHCI SATA drives.
> 
> 2.6.10-rc2 still uses the 1.2 8169 driver, same as 2.6.9...
> 
> However, the r8169 2.2 driver would not build with 2.6.10-rc2,
> due to use of pci_dma_sync_single.
> 
> I've made a V2.3 of the 8169 driver which uses 
> pci_dma_sync_single_for_cpu instead, and it seems to work fine.
> 
> I was hoping to submit a patch to move the new driver into
> some appropriate release, but the 2.2/2.3 driver is so far
> changed from 1.2 that the diff -u is about the size of the
> original and new file combined :(
[...]
> With such a huge diff, should I send a diff, or the whole new file, or
> do something else entirely?

Do something else:
- take a look at the changes for the 8169 driver in Jeff Garzik's -netdev
  patchkit (included in -mm). It may be interesting to know how it behaves;
- less +/8169 MAINTAINERS;
- provide a more elaborate description of the issue with your computer
  (+ gcc version, lspci -vx, dmesg at boot, lsmod, /proc/interrupts, ifconfig);
- realize that the so called version number in 2.6.9 has no meaning.

Last time I looked at Realtek's driver (linux-8169(220).zip), it still
contained bugs which had been fixed in mainline (though it merges some
part of it) and I did not find anything which should do a difference
from a correctness POV. Intermediate versions of Realtek's code are
not available and the datasheet has disappeared from their website.
With due respect for Realtek's work (serious, really) it does not make
my life fun _at all_ (and I guess that "my" is also accurate for anyone
who tries to work with the 8169 driver on the long run).

Btw merging a 20 megaton patch is not the way network drivers changes
are submitted. People expect a serie of small changes whose effects
are clearly explained (see http://linux.yyz.us/patch-format.html for
the suggested format).

Imho your issue is not completely specific to the 8169 hardware. With
a wet finger in the wind, I'd suspect something related to timing or irq
(duration of locking or such).

Please Cc: netdev@oss.sgi.com on further messages. Cc: jgarzik@pobox.com
for network devices patches is also suggested.

--
Ueimor

Re: r8169.c

[Dorn Hetzel]

On Fri, Nov 19, 2004 at 09:12:03PM +0100, Francois Romieu wrote:
> Dorn Hetzel <kernel@dorn.hetzel.org> :
> [Abit AA8 experience]
> 
> Do something else:
> - take a look at the changes for the 8169 driver in Jeff Garzik's -netdev
>   patchkit (included in -mm). It may be interesting to know how it behaves;

Thank you very much for the pointers.  I have been using Linux for
ages (well, since 1993 anyway), but this is my first attempt at fixing
something in the kernel.  (first time I've needed to :) )

I'm off to look for and try the -mm patchkit after I write this?
Will it apply to 2.6.10-rc2?  (I had to get to rc2 to get my SATA controller
 to work) :)

> - less +/8169 MAINTAINERS;
> - provide a more elaborate description of the issue with your computer
>   (+ gcc version, lspci -vx, dmesg at boot, lsmod, /proc/interrupts, ifconfig);

with regard to the "original" version shipping with 2.6.10-rc2
(which seems to be identical to the version in 2.6.9)...

gcc version = 2.95.4  (happy to update that if you think it will help)

see   http://www.hetzel.org/8169/orig/lspci.txt   for the lspci -vx
see   http://www.hetzel.org/8169/orig/dmesg.txt   for the dmesg

lsmod returns empty because though I have module support enabled, I'm
building everything I'm using in directly...

see   http://www.hetzel.org/8169/orig/interrupts.txt   for the /proc/interrupts
see   http://www.hetzel.org/8169/orig/ifconfig.txt   for the ifconfig -a

"NETDEV WATCHDOG: eth0: transmit timed out" is one of the observed
errors.  "RX: no buffer available" (or similar, from memory) is another.

with regard to version "2.3" (my hacked version)...

gcc version = 2.95.4  (happy to update that if you think it will help)

see   http://www.hetzel.org/8169/v23/lspci.txt   for the lspci -vx
see   http://www.hetzel.org/8169/v23/dmesg.txt   for the dmesg

lsmod returns empty because though I have module support enabled, I'm
building everything I'm using in directly...

see   http://www.hetzel.org/8169/v23/interrupts.txt   for the /proc/interrupts
see   http://www.hetzel.org/8169/v23/ifconfig.txt   for the ifconfig -a


> - realize that the so called version number in 2.6.9 has no meaning.
>
By this, do you mean that the comment of version number in the r8169.c
of 2.6.9 is no longer related to the version numbers at Realtek?
 
> Last time I looked at Realtek's driver (linux-8169(220).zip), it still
> contained bugs which had been fixed in mainline (though it merges some
> part of it) and I did not find anything which should do a difference
> from a correctness POV. Intermediate versions of Realtek's code are
> not available and the datasheet has disappeared from their website.
> With due respect for Realtek's work (serious, really) it does not make
> my life fun _at all_ (and I guess that "my" is also accurate for anyone
> who tries to work with the 8169 driver on the long run).
>
Yeah, I noticed just one version and no history, but to the good, it
does work for me, at least so far, after the minor patch :)
 
> Btw merging a 20 megaton patch is not the way network drivers changes
> are submitted. People expect a serie of small changes whose effects
> are clearly explained (see http://linux.yyz.us/patch-format.html for
> the suggested format).
>
I was pretty sure the Godzilla patch was not the answer, I just couldn't
figure out what the answer was...  I had fixed *my* problem, for now,
but it would be nice to share it if it's worth sharing...
 
> Imho your issue is not completely specific to the 8169 hardware. With
> a wet finger in the wind, I'd suspect something related to timing or irq
> (duration of locking or such).
> 
> Please Cc: netdev@oss.sgi.com on further messages. Cc: jgarzik@pobox.com
> for network devices patches is also suggested.

Done.

-Dorn

Re: r8169.c

[Francois Romieu]

Dorn Hetzel <kernel@dorn.hetzel.org> :
[...]
> Thank you very much for the pointers.  I have been using Linux for
> ages (well, since 1993 anyway), but this is my first attempt at fixing
> something in the kernel.  (first time I've needed to :) )
> 
> I'm off to look for and try the -mm patchkit after I write this?
> Will it apply to 2.6.10-rc2?  (I had to get to rc2 to get my SATA controller
>  to work) :)

You have two options (or more) on top of 2.6.10-rc2:
- ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.10-rc2/2.6.10-rc2-mm2/2.6.10-rc2-mm2.bz2
- http://www.kernel.org/pub/linux/kernel/people/jgarzik/patchkits/2.6/2.6.10-rc2-netdev1.patch.bz2

Once you have applied one of the patch above, the patch below will improve
your "transmit timed out" (please apply in order and enable NAPI):
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-250.patch
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-255.patch

If things perform better you may want to use bigger frames and apply as
well r8169-260.patch and r8169-265.patch.
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-260.patch
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-265.patch


[...]
> with regard to the "original" version shipping with 2.6.10-rc2
> (which seems to be identical to the version in 2.6.9)...

Yes.
I hope the current set of changes in -mm/-netdev will be merged in
early post-2.6.10.

> gcc version = 2.95.4  (happy to update that if you think it will help)

You will be welcome to upgrade.

[...]
> > - realize that the so called version number in 2.6.9 has no meaning.
> >
> By this, do you mean that the comment of version number in the r8169.c
> of 2.6.9 is no longer related to the version numbers at Realtek?

Exactly.

One year ago, Realtek's driver was split and merged in the kernel. That
and the contributions of many people took several months to achieve a
(imho) decently stable driver. At the same time, no change appeared on
Realtek's side. Still some months later Realtek issues a new driver with
a rev number bump (2.2). Its code suggests that it includes a partial
merge from some of the changes made to the in-kernel driver as well as
some internal experiments (hooks for missing code).

The drivers are now quite different. A change of version number is 
included in the -mm/-netdev driver to protect the innocent but it seems
I'll have to push weirder numbers. :o)

[...]
> Yeah, I noticed just one version and no history, but to the good, it
> does work for me, at least so far, after the minor patch :)

1 - netif_stop_queue() race between the Tx xmit and the Tx IRQ handler.
2 - "entry" can overflow in rtl8169_tx_interrupt() -> read of random
    status and early free of Tx buffer.

When the in-kernel version had similar code, it translated into "r8169
hangs under load".

--
Ueimor

Re: r8169.c

[Dorn Hetzel]

On Sat, Nov 20, 2004 at 01:29:46AM +0100, Francois Romieu wrote:
> Dorn Hetzel <kernel@dorn.hetzel.org> :
> 
> You have two options (or more) on top of 2.6.10-rc2:
> - ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.10-rc2/2.6.10-rc2-mm2/2.6.10-rc2-mm2.bz2
> - http://www.kernel.org/pub/linux/kernel/people/jgarzik/patchkits/2.6/2.6.10-rc2-netdev1.patch.bz2
>
I have gotten as far as rc2-mm2, which was a fairly complete failure.  After
just a couple of pings on the interface, the whole system started to freeze
up fairly hard.  Please see   http://www.hetzel.org/8169/rc2-mm2/   for a
set of information on the system state this time around.  The last two
lines in messages.txt are:

illyria kernel: NETDEV WATCHDOG: eth0: transmit timed out
illyria kernel: eth0: interrupt 0001 taken in poll

Then things go south pretty hard and fast...

I'll try the other patches on top of rc2-mm2 tonight and see if that turns
out any better :)

> Once you have applied one of the patch above, the patch below will improve
> your "transmit timed out" (please apply in order and enable NAPI):
> http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-250.patch
> http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-255.patch
> 
> If things perform better you may want to use bigger frames and apply as
> well r8169-260.patch and r8169-265.patch.
> http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-260.patch
> http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-265.patch
>

Thanks again for all your help!

-Dorn
 

Re: r8169.c

[Dorn Hetzel]

> On Sat, Nov 20, 2004 at 01:29:46AM +0100, Francois Romieu wrote:
>
> > Once you have applied one of the patch above, the patch below will improve
> > your "transmit timed out" (please apply in order and enable NAPI):
> > http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-250.patch
> > http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-255.patch
> > 
> > If things perform better you may want to use bigger frames and apply as
> > well r8169-260.patch and r8169-265.patch.
> > http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-260.patch
> > http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-265.patch
> >
>
Stacked on these 4 patches and things seem much better :)

This is on an Abit AA8 Duramax motherboard.

-Dorn
 

Re: r8169.c

[Francois Romieu]

Dorn Hetzel <kernel@dorn.hetzel.org> :
[...]
[...]
[...]
[...]
[...]
[...]
[...]
[...]
[...]
> Stacked on these 4 patches and things seem much better :)

Did you change the compiler as well ?

If yes, it would be nice to know if the system performs correctly when built with
the previous compiler (feel free to answer #1 only if you are busy :o) ).

--
Ueimor

Re: r8169.c

[Dorn Hetzel]

On Tue, Nov 23, 2004 at 08:47:40PM +0100, Francois Romieu wrote:
> Dorn Hetzel <kernel@dorn.hetzel.org> :
> [...]
> > Stacked on these 4 patches and things seem much better :)
> 
> Did you change the compiler as well ?
> 
> If yes, it would be nice to know if the system performs correctly when built with
> the previous compiler (feel free to answer #1 only if you are busy :o) ).
>

Yes, I moved to gcc 3.4.3.

I'm out of town for the Thanksgiving holiday, but I will revert the compiler
and test again as soon as I get home (early next week).

Do you know when the 250-265 patches will be merged and into which train?

Best Regards,

-Dorn
 
> --
> Ueimor
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: r8169.c

[Dorn Hetzel]

On Tue, Nov 23, 2004 at 08:47:40PM +0100, Francois Romieu wrote:
> Dorn Hetzel <kernel@dorn.hetzel.org> :

> > Stacked on these 4 patches and things seem much better :)
> 
> Did you change the compiler as well ?
> 
> If yes, it would be nice to know if the system performs correctly when built with
> the previous compiler (feel free to answer #1 only if you are busy :o) ).
>

I went ahead and remotely rebuilt using gcc 2.95.4 and upon reboot it
worked long enough to ssh in and then it failed.  So it sounds like the
version of gcc DOES make a difference :)

I will report further details when I get home and can access the box
again ;)

-Dorn
 
> --
> Ueimor
> -

Re: r8169.c

[Francois Romieu]

Dorn Hetzel <kernel@dorn.hetzel.org> :
[...]
> I went ahead and remotely rebuilt using gcc 2.95.4 and upon reboot it
> worked long enough to ssh in and then it failed.  So it sounds like the
> version of gcc DOES make a difference :)

Ok, I'll have to audit the driver for the typical inline assembler +
arithmetic ops which 2.95.x dislikes.

--
Ueimor

Re: r8169.c

[Dorn Hetzel]

It did *build* well enough not to blow up the kernel build with 2.95.4,
it just failed in use...

On Thu, Nov 25, 2004 at 09:54:11PM +0100, Francois Romieu wrote:
> Dorn Hetzel <kernel@dorn.hetzel.org> :
> [...]
> > I went ahead and remotely rebuilt using gcc 2.95.4 and upon reboot it
> > worked long enough to ssh in and then it failed.  So it sounds like the
> > version of gcc DOES make a difference :)
> 
> Ok, I'll have to audit the driver for the typical inline assembler +
> arithmetic ops which 2.95.x dislikes.
> 
> --
> Ueimor

Re: r8169.c

[Dorn Hetzel]

On Mon, Nov 22, 2004 at 01:13:07PM -0500, Dorn Hetzel wrote:
> On Sat, Nov 20, 2004 at 01:29:46AM +0100, Francois Romieu wrote:
> 
> > Once you have applied one of the patch above, the patch below will improve
> > your "transmit timed out" (please apply in order and enable NAPI):
> > http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-250.patch
> > http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-255.patch
> > 
> > If things perform better you may want to use bigger frames and apply as
> > well r8169-260.patch and r8169-265.patch.
> > http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-260.patch
> > http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.10-rc2-mm1/r8169-265.patch
> >

I was wondering if the above 4 patches have made it into one of the
rc? releases, or at least a rc?-mm?  ?

Regards,

-Dorn

Re: r8169.c

[Francois Romieu]

Dorn Hetzel <kernel@dorn.hetzel.org> :
[...]
> I was wondering if the above 4 patches have made it into one of the
> rc? releases, or at least a rc?-mm?  ?

They need to apply on top of -netdev which is included in -mm.
I'll send the patch for inclusion in -mm so there is no need for
Jeff to hurry.

--
Ueimor