Re: [patch] NE2000

Re: [patch] NE2000

[Jeff Garzik]

pavel rabel wrote:
> There are three drivers for n2k cards. One is MCA only, one is PCI only,
> and the then the third one (ne.c) is both ISA and PCI. I think the ISA
> driver should be ISA only, as is described in Documentation and in config
> help. So I removed PCI code from ne.c to have ISA only driver. It
> gets a bit smaller, although I am not sure whether more code can be
> removed.

This change sounds ok to me, if noone else objects.  (I added to the CC
a bit)  I saw that code, and was thinking about doing the same thing
myself.  ne2k-pci.c definitely has changes which are not included in
ne.c, and it seems silly to duplicate ne2000 PCI support.

Regards,

	Jeff


P.S.  Pavel, for the future, patches made with "diff -u" are preferred.

-- 
Jeff Garzik             | "Mind if I drive?"  -Sam
Building 1024           | "Not if you don't mind me clawing at the
MandrakeSoft            |  dash and shrieking like a cheerleader."
                        |                     -Max
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [patch] NE2000

[Alan Cox]

> This change sounds ok to me, if noone else objects.  (I added to the CC
> a bit)  I saw that code, and was thinking about doing the same thing
> myself.  ne2k-pci.c definitely has changes which are not included in
> ne.c, and it seems silly to duplicate ne2000 PCI support.

Unless there are any cards that need the bug workarounds in ne.c for use
on PCI then I see no problem. I've not heard of any. 

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

Re: [patch] NE2000

[Jorge Nerin]

Alan Cox wrote:
> 
> > This change sounds ok to me, if noone else objects.  (I added to the CC
> > a bit)  I saw that code, and was thinking about doing the same thing
> > myself.  ne2k-pci.c definitely has changes which are not included in
> > ne.c, and it seems silly to duplicate ne2000 PCI support.
> 
> Unless there are any cards that need the bug workarounds in ne.c for use
> on PCI then I see no problem. I've not heard of any.
> 

Ok, I reported it several times, but it gets ignored. I have a Realtek
8029 (ne2k-pci), and with both drivers ne and ne2k-pci I can easily get
it stuck by doing a ping -f to a host in the local net, and sometimes it
happens doing copies to/from nfs shared resources.

rmmod & insmod don't cure the problem, it seems that no interrupts are
delivered from the card, and there are no log messages, so a reboot is
needed to restore net access.

System is dual 2x200mmx 96Mb ide discs no interrupts shared, and as far
as I can remember all kernel from 2.2.x, 2.3.x up to 2.4.0-testx exhibit
this problem.

-- 
Jorge Nerin
<comandante@zaralinux.com>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

changed section attributes

[Petko Manolov]

	Hi there,

I noticed that when i changed to binutils 2.10.91 (Debian,woody)
i start to see messages like:

"Warning: Ignoring changed section attributes for .modinfo"

Chasing down the problem appeared that section modinfo is
declared for the first time as ".section .modinfo" without any
attributes. This is done by the including of linux/module.h.
The next declaration is ".section .modinfo,"a",@progbits".
And assembler moans on that line number.

Changing the declaration in linux/module.h to ".modinfo,"a""
fixed the problem, but i noticed that the author said that
"we want .modinfo to not be allocated"

I wonder why?

I already tried to allocate it (.modinfo,"a" in module.h) and
it seems to work.

Any ideas?



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

Re: changed section attributes

[Keith Owens]

On Tue, 31 Oct 2000 15:54:05 +0200, 
Petko Manolov <petkan@dce.bg> wrote:
>"Warning: Ignoring changed section attributes for .modinfo"
>
>Changing the declaration in linux/module.h to ".modinfo,"a""
>fixed the problem, but i noticed that the author said that
>"we want .modinfo to not be allocated"

Historically that was the only way of preventing the .modinfo section
from being included in modules when they were loaded into the kernel.
An alternative is to allow .modinfo to be allocated and have modutils
treat it as non-allocated.  This feature was added to modutils 2.3.19
on October 22 (bleeding edge toolchains for IA64 are "fun") so anybody
who is annoyed by the warning messages can apply this patch.

Index: 0-test10-pre7.1/include/linux/module.h
--- 0-test10-pre7.1/include/linux/module.h Tue, 31 Oct 2000 08:28:16 +1100 kaos (linux-2.4/W/33_module.h 1.1.2.1.2.1.2.1.2.1.1.1 644)
+++ 0-test10-pre7.1(w)/include/linux/module.h Wed, 01 Nov 2000 01:13:22 +1100 kaos (linux-2.4/W/33_module.h 1.1.2.1.2.1.2.1.2.1.1.1 644)
@@ -218,11 +218,6 @@
   MODULE_GENERIC_TABLE(type##_device,name)
 /* not put to .modinfo section to avoid section type conflicts */
 
-/* The attributes of a section are set the first time the section is
-   seen; we want .modinfo to not be allocated.  */
-
-__asm__(".section .modinfo\n\t.previous");
-
 /* Define the module variable, and usage macros.  */
 extern struct module __this_module;
 
Index: 0-test10-pre7.1/Documentation/Changes
--- 0-test10-pre7.1/Documentation/Changes Fri, 27 Oct 2000 22:11:48 +1100 kaos (linux-2.4/G/c/25_Changes 1.1.1.4.1.6 644)
+++ 0-test10-pre7.1(w)/Documentation/Changes Wed, 01 Nov 2000 01:13:03 +1100 kaos (linux-2.4/G/c/25_Changes 1.1.1.4.1.6 644)
@@ -52,7 +52,7 @@
 o  Gnu make               3.77                    # make --version
 o  binutils               2.9.1.0.25              # ld -v
 o  util-linux             2.10o                   # kbdrate -v
-o  modutils               2.3.18                  # insmod -V
+o  modutils               2.3.19                  # insmod -V
 o  e2fsprogs              1.19                    # tune2fs --version
 o  pcmcia-cs              3.1.21                  # cardmgr -V
 o  PPP                    2.4.0                   # pppd --version
@@ -284,7 +284,7 @@
 
 Modutils
 --------
-o  <ftp://ftp.kernel.org/pub/linux/utils/kernel/modutils/v2.3/modutils-2.3.18.tar.bz2>
+o  <ftp://ftp.kernel.org/pub/linux/utils/kernel/modutils/v2.3/modutils-2.3.19.tar.bz2>
 
 Mkinitrd
 --------

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

Re: changed section attributes

[Petko Manolov]

Keith Owens wrote:
> 
> >Changing the declaration in linux/module.h to ".modinfo,"a""
> >fixed the problem, but i noticed that the author said that
> >"we want .modinfo to not be allocated"
> 
> Historically that was the only way of preventing the .modinfo section
> from being included in modules when they were loaded into the kernel.
> An alternative is to allow .modinfo to be allocated and have modutils
> treat it as non-allocated.  This feature was added to modutils 2.3.19
> on October 22 (bleeding edge toolchains for IA64 are "fun") so anybody
> who is annoyed by the warning messages can apply this patch.

[snip]
 
> -/* The attributes of a section are set the first time the section is
> -   seen; we want .modinfo to not be allocated.  */
> -
> -__asm__(".section .modinfo\n\t.previous");
> -
>  /* Define the module variable, and usage macros.  */
>  extern struct module __this_module;


This is exactly what i did (excluding removing of the comment ;-)

I wonder why the compiler decides to add ".section
.modinfo,"a",@progbits"
May be this is the thing which should be fixed.


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

Re: changed section attributes

[Keith Owens]

On Tue, 31 Oct 2000 16:29:16 +0200, 
Petko Manolov <petkan@dce.bg> wrote:
>I wonder why the compiler decides to add ".section
>.modinfo,"a",@progbits"
>May be this is the thing which should be fixed.

That is just gcc speak for section .modinfo is marked as allocated,
type progbits.  Read the ELF standard if you want to know more.

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

Re: changed section attributes

[Petko Manolov]

Keith Owens wrote:
> 
> On Tue, 31 Oct 2000 16:29:16 +0200,
> Petko Manolov <petkan@dce.bg> wrote:
> >I wonder why the compiler decides to add ".section
> >.modinfo,"a",@progbits"
> >May be this is the thing which should be fixed.
> 
> That is just gcc speak for section .modinfo is marked as allocated,
> type progbits.  Read the ELF standard if you want to know more.


I already red the info as pages, but the description was too brief.

If this is default gcc behavior then it seems that changing to latest
modutils is the only option ;-)

I wonder if Linus will apply your patch.


best,
Petkan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [patch] NE2000

[Jorge Nerin]

Paul Gortmaker wrote:
> 
> Jorge Nerin wrote:
> 
> >
> > Ok, I reported it several times, but it gets ignored. I have a Realtek
> > 8029 (ne2k-pci), and with both drivers ne and ne2k-pci I can easily get
> > it stuck by doing a ping -f to a host in the local net, and sometimes it
> > happens doing copies to/from nfs shared resources.
> >
> > rmmod & insmod don't cure the problem, it seems that no interrupts are
> > delivered from the card, and there are no log messages, so a reboot is
> > needed to restore net access.
> >
> > System is dual 2x200mmx 96Mb ide discs no interrupts shared, and as far
> > as I can remember all kernel from 2.2.x, 2.3.x up to 2.4.0-testx exhibit
> > this problem.
> 
> Any messages from the driver whatsoever?   Does running a non-SMP
> kernel make the problem go away?
> 
> Paul.
> 

Well, I have tried it with 2.4.0-test10, both SMP and non-SMP, and the
result is a little confusing.

Under SMP a ping -s 50000 -f other_host takes down the network access
with no messages (ne2k-pci), and no possibility of being restored
without a reboot.

Under UP the same command works ok, but after a while the dots stop for
30sec, then ping prints an 'E' and the dots continue. strace revealed
this:

sendmsg(4, {msg_name(16)={sin_family=AF_INET, sin_port=htons(0),
sin_addr=inet_addr("192.168.1.20")}},
msg_iov(1)=[{"\10\0\305~|\23\231\0\v\317\2:\177\236\r\0\10\t\n\v\f\r"...,
50008}], msg_controllen=0, msg_flags=0}, 0x800) = 50008 <30.016855>

ping has been waiting for sendmsg to end for 30 seconds! I don't know if
this could be caused by filling the network buffers, and then waiting a
while while the nic sends it out. As the packet size increases (the -s
option) the stops are more frequent, there is still activity on the
network, but I don't know if they are my packets or the replies.

When ping is stopped it's stuck in sock_wait_for_wmem, and when it's
running it's (usually) in wait_for_packet.

So I think that it could be a little window near sock_wait_for_wmem that
could be SMP insecure wich is affecting me.

The code of sock_wait_for_wmem in 2.4.0-test10 is this:

static long sock_wait_for_wmem(struct sock * sk, long timeo)
{
	DECLARE_WAITQUEUE(wait, current);

	clear_bit(SOCK_ASYNC_NOSPACE, &sk->socket->flags);
	add_wait_queue(sk->sleep, &wait);
	for (;;) {
		if (signal_pending(current))
			break;
		set_bit(SOCK_NOSPACE, &sk->socket->flags);
		set_current_state(TASK_INTERRUPTIBLE);
		if (atomic_read(&sk->wmem_alloc) < sk->sndbuf)
			break;
		if (sk->shutdown & SEND_SHUTDOWN)
			break;
		if (sk->err)
			break;
		timeo = schedule_timeout(timeo);
	}
	__set_current_state(TASK_RUNNING);
	remove_wait_queue(sk->sleep, &wait);
	return timeo;
}

Does someone see something SMP insecure? Perhaps I'm totally wrong, this
could also be somewhere in the interrupt handling, don't know.

-- 
Jorge Nerin
<comandante@zaralinux.com>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [patch] NE2000

[Andrew Morton]

Jorge Nerin wrote:
> 
> ...
> So I think that it could be a little window near sock_wait_for_wmem that
> could be SMP insecure wich is affecting me.
> 
> The code of sock_wait_for_wmem in 2.4.0-test10 is this:
> 
> static long sock_wait_for_wmem(struct sock * sk, long timeo)
> {
>         DECLARE_WAITQUEUE(wait, current);
> 
>         clear_bit(SOCK_ASYNC_NOSPACE, &sk->socket->flags);
>         add_wait_queue(sk->sleep, &wait);
>         for (;;) {
>                 if (signal_pending(current))
>                         break;
>                 set_bit(SOCK_NOSPACE, &sk->socket->flags);
>                 set_current_state(TASK_INTERRUPTIBLE);
>                 if (atomic_read(&sk->wmem_alloc) < sk->sndbuf)
>                         break;
>                 if (sk->shutdown & SEND_SHUTDOWN)
>                         break;
>                 if (sk->err)
>                         break;
>                 timeo = schedule_timeout(timeo);
>         }
>         __set_current_state(TASK_RUNNING);
>         remove_wait_queue(sk->sleep, &wait);
>         return timeo;
> }
> 
> Does someone see something SMP insecure? Perhaps I'm totally wrong, this
> could also be somewhere in the interrupt handling, don't know.

No, that code is correct, provided (current->state == TASK_RUNNING)
on entry.  If it isn't, there's a race window which can cause
lost wakeups.   As a check you could add:

	if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0)
		BUG();

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

Re: [patch] NE2000

[Jorge Nerin]

Andrew Morton wrote:
> 
> Jorge Nerin wrote:
> >
> > ...
> > So I think that it could be a little window near sock_wait_for_wmem that
> > could be SMP insecure wich is affecting me.
> >
> > The code of sock_wait_for_wmem in 2.4.0-test10 is this:
> >
> > static long sock_wait_for_wmem(struct sock * sk, long timeo)
> > {
> >         DECLARE_WAITQUEUE(wait, current);
> >
> >         clear_bit(SOCK_ASYNC_NOSPACE, &sk->socket->flags);
> >         add_wait_queue(sk->sleep, &wait);
> >         for (;;) {
> >                 if (signal_pending(current))
> >                         break;
> >                 set_bit(SOCK_NOSPACE, &sk->socket->flags);
> >                 set_current_state(TASK_INTERRUPTIBLE);
> >                 if (atomic_read(&sk->wmem_alloc) < sk->sndbuf)
> >                         break;
> >                 if (sk->shutdown & SEND_SHUTDOWN)
> >                         break;
> >                 if (sk->err)
> >                         break;
> >                 timeo = schedule_timeout(timeo);
> >         }
> >         __set_current_state(TASK_RUNNING);
> >         remove_wait_queue(sk->sleep, &wait);
> >         return timeo;
> > }
> >
> > Does someone see something SMP insecure? Perhaps I'm totally wrong, this
> > could also be somewhere in the interrupt handling, don't know.
> 
> No, that code is correct, provided (current->state == TASK_RUNNING)
> on entry.  If it isn't, there's a race window which can cause
> lost wakeups.   As a check you could add:
> 
>         if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0)
>                 BUG();
> 
> to the start of this function.

OK, added, the function now looks like this:

static long sock_wait_for_wmem(struct sock * sk, long timeo)
{
        DECLARE_WAITQUEUE(wait, current);

        if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE))
== 0)
                BUG();

        clear_bit(SOCK_ASYNC_NOSPACE, &sk->socket->flags);
        add_wait_queue(sk->sleep, &wait);
        for (;;) {
                if (signal_pending(current))
                        break;
                set_bit(SOCK_NOSPACE, &sk->socket->flags);
                set_current_state(TASK_INTERRUPTIBLE);
                if (atomic_read(&sk->wmem_alloc) < sk->sndbuf)
                        break;
                if (sk->shutdown & SEND_SHUTDOWN)
                        break;
                if (sk->err)
                        break;
                timeo = schedule_timeout(timeo);
        }
        __set_current_state(TASK_RUNNING);
        remove_wait_queue(sk->sleep, &wait);
        return timeo;
}

I have to put it _after_ DECLARE_WAITQUEUE in order to compile, if I put
it before it says me that wait is undeclared :-?

Well recompile, reboot, and got caugth by BUG(), after some tests.

[root@quartz ~]# ping -f -s 15000 pp_head
PING pp_head.pp.redvip.net (192.168.1.20) from 192.168.1.3 :
15000(15028) bytes of data.
..............invalid operand: 0000
CPU:    0
EIP:    0010:[<c0195b30>]
EFLAGS: 00010296
eax: 0000001a   ebx: c2604000   ecx: c021e2e8   edx: c0266440
esi: c26eeba0   edi: c26eeba0   ebp: 7fffffff   esp: c2605c88
ds: 0018   es: 0018   ss: 0018
Process ping (pid: 2202, stackpage=c2605000)
Stack: c02047a5 c02049eb 000002d2 7fffffff c26eeba0 c2604000 00000000
c26c4024
       01234567 c2604000 00000000 00000000 01234567 c2604000 00000000
00000000
       c0195c99 c26eeba0 7fffffff c26c4024 c264d0c0 c26c4010 000005dc
00000000
Call Trace: [<c02047a5>] [<c02049eb>] [<c0195c99>] [<c01aa692>]
[<c01aaa1e>] [<c01c0180>] [<c01c04de>]
       [<c01c0180>] [<c01c6f48>] [<c01c6f86>] [<c0192fad>] [<c01c6f48>]
[<c01941b8>] [<c0193ee6>] [<c0165c9a>]
       [<c016e602>] [<c01945fc>] [<c0109477>]
Code: 0f 0b 83 c4 0c 8b 87 50 03 00 00 f0 0f ba 70 04 00 8d 4c 24
Violación de segmento
[root@quartz ~]#


Nov  6 12:00:07 quartz kernel: kernel BUG at sock.c:722!
Nov  6 12:00:07 quartz kernel: invalid operand: 0000
Nov  6 12:00:07 quartz kernel: CPU:    0
Nov  6 12:00:07 quartz kernel: EIP:    0010:[sock_wait_for_wmem+104/244]
Nov  6 12:00:07 quartz kernel: EFLAGS: 00010296
Nov  6 12:00:07 quartz kernel: eax: 0000001a   ebx: c2604000   ecx:
c021e2e8   edx: c0266440
Nov  6 12:00:07 quartz kernel: esi: c26eeba0   edi: c26eeba0   ebp:
7fffffff   esp: c2605c88
Nov  6 12:00:07 quartz kernel: ds: 0018   es: 0018   ss: 0018
Nov  6 12:00:07 quartz kernel: Process ping (pid: 2202,
stackpage=c2605000)
Nov  6 12:00:07 quartz kernel: Stack: c02047a5 c02049eb 000002d2
7fffffff c26eeba0 c2604000 00000000 c26c4024
Nov  6 12:00:07 quartz kernel:        01234567 c2604000 00000000
00000000 01234567 c2604000 00000000 00000000
Nov  6 12:00:07 quartz kernel:        c0195c99 c26eeba0 7fffffff
c26c4024 c264d0c0 c26c4010 000005dc 00000000
Nov  6 12:00:07 quartz kernel: Call Trace: [vga_con+2501/10176]
[vga_con+3083/10176] [sock_alloc_send_skb+221/300]
[ip_build_xmit_slow+378/1208] [ip_build_xmit+78/816] [raw_getfrag+0/36]
[raw_sendmsg+642/752]
Nov  6 12:00:07 quartz kernel:        [raw_getfrag+0/36]
[inet_sendmsg+0/68] [inet_sendmsg+62/68] [sock_sendmsg+129/164]
[inet_sendmsg+0/68] [sys_sendmsg+380/464] [sys_recvfrom+238/256]
[set_cursor+110/132]
Nov  6 12:00:07 quartz kernel:        [write_chan+462/488]
[sys_socketcall+460/484] [system_call+55/64]
Nov  6 12:00:07 quartz kernel: Code: 0f 0b 83 c4 0c 8b 87 50 03 00 00 f0
0f ba 70 04 00 8d 4c 24

Nov  6 12:06:11 quartz kernel: kernel BUG at sock.c:722!
Nov  6 12:06:11 quartz kernel: invalid operand: 0000
Nov  6 12:06:11 quartz kernel: CPU:    1
Nov  6 12:06:11 quartz kernel: EIP:    0010:[sock_wait_for_wmem+104/244]
Nov  6 12:06:11 quartz kernel: EFLAGS: 00010296
Nov  6 12:06:11 quartz kernel: eax: 0000001a   ebx: c1f54000   ecx:
c021e2e8   edx: c0266440
Nov  6 12:06:11 quartz kernel: esi: c3cd6100   edi: c3cd6100   ebp:
7fffffff   esp: c1f55c88
Nov  6 12:06:11 quartz kernel: ds: 0018   es: 0018   ss: 0018
Nov  6 12:06:11 quartz kernel: Process ping (pid: 2993,
stackpage=c1f55000)
Nov  6 12:06:11 quartz kernel: Stack: c02047a5 c02049eb 000002d2
7fffffff c3cd6100 c1f54000 00000000 c2648824
Nov  6 12:06:11 quartz kernel:        01234567 c1f54000 00000000
00000000 01234567 c1f54000 00000000 00000000
Nov  6 12:06:11 quartz kernel:        c0195c99 c3cd6100 7fffffff
c2648824 c260c2c0 c2648810 000005dc 00000000
Nov  6 12:06:11 quartz kernel: Call Trace: [vga_con+2501/10176]
[vga_con+3083/10176] [sock_alloc_send_skb+221/300]
[ip_build_xmit_slow+378/1208] [ip_build_xmit+78/816] [raw_getfrag+0/36]
[raw_sendmsg+642/752]
Nov  6 12:06:11 quartz kernel:        [raw_getfrag+0/36]
[inet_sendmsg+0/68] [inet_sendmsg+62/68] [sock_sendmsg+129/164]
[inet_sendmsg+0/68] [sys_sendmsg+380/464] [sys_recvfrom+238/256]
[set_cursor+110/132]
Nov  6 12:06:11 quartz kernel:        [write_chan+462/488]
[sys_socketcall+460/484] [system_call+55/64]
Nov  6 12:06:11 quartz kernel: Code: 0f 0b 83 c4 0c 8b 87 50 03 00 00 f0
0f ba 70 04 00 8d 4c 24
Nov  6 12:06:11 quartz kernel: NET: 6 messages suppressed.
Nov  6 12:06:11 quartz kernel: NAT: 0 dropping untracked packet c26984a0
1 192.168.1.20 -> 192.168.1.3
Nov  6 12:06:11 quartz kernel: NAT: 0 dropping untracked packet c2828a80
1 192.168.1.20 -> 192.168.1.3
Nov  6 12:06:11 quartz kernel: NAT: 0 dropping untracked packet c265d560
1 192.168.1.20 -> 192.168.1.3
Nov  6 12:06:11 quartz kernel: NAT: 0 dropping untracked packet c2828a80
1 192.168.1.20 -> 192.168.1.3
Nov  6 12:06:11 quartz kernel: NAT: 0 dropping untracked packet c53bc820
1 192.168.1.20 -> 192.168.1.3
Nov  6 12:06:11 quartz kernel: NAT: 0 dropping untracked packet c264d780
1 192.168.1.20 -> 192.168.1.3
Nov  6 12:06:41 quartz kernel: kernel BUG at sock.c:722!
Nov  6 12:06:41 quartz kernel: invalid operand: 0000
Nov  6 12:06:41 quartz kernel: CPU:    1
Nov  6 12:06:41 quartz kernel: EIP:    0010:[sock_wait_for_wmem+104/244]
Nov  6 12:06:41 quartz kernel: EFLAGS: 00010296
Nov  6 12:06:41 quartz kernel: eax: 0000001a   ebx: c1f54000   ecx:
c021e2e8   edx: c0266440
Nov  6 12:06:41 quartz kernel: esi: c3cd6800   edi: c3cd6800   ebp:
7fffffff   esp: c1f55c88
Nov  6 12:06:41 quartz kernel: ds: 0018   es: 0018   ss: 0018
Nov  6 12:06:41 quartz kernel: Process ping (pid: 2994,
stackpage=c1f55000)
Nov  6 12:06:41 quartz kernel: Stack: c02047a5 c02049eb 000002d2
7fffffff c3cd6800 c1f54000 00000000 c2644824
Nov  6 12:06:41 quartz kernel:        01234567 c1f54000 00000000
00000000 01234567 c1f54000 00000000 00000000
Nov  6 12:06:41 quartz kernel:        c0195c99 c3cd6800 7fffffff
c2644824 c5b8be60 c2644810 000005dc 00000000
Nov  6 12:06:41 quartz kernel: Call Trace: [vga_con+2501/10176]
[vga_con+3083/10176] [sock_alloc_send_skb+221/300]
[ip_build_xmit_slow+378/1208] [ip_build_xmit+78/816] [raw_getfrag+0/36]
[raw_sendmsg+642/752]
Nov  6 12:06:41 quartz kernel:        [raw_getfrag+0/36]
[inet_sendmsg+0/68] [inet_sendmsg+62/68] [sock_sendmsg+129/164]
[inet_sendmsg+0/68] [sys_sendmsg+380/464] [sys_recvfrom+238/256]
[kfree_skbmem+40/140]
Nov  6 12:06:41 quartz kernel:        [__kfree_skb+369/376]
[nfsd:__insmod_nfsd_O/lib/modules/2.4.0-test10-ne2k/kernel/fs/nfs+-289558/96]
[nfsd:__insmod_nfsd_O/lib/modules/2.4.0-test10-ne2k/kernel/fs/nfs+-288860/96]
[qdisc_restart+108/376] [net_tx_action+194/300] [sys_socketcall+460/484]
[system_call+55/64]
Nov  6 12:06:41 quartz kernel: Code: 0f 0b 83 c4 0c 8b 87 50 03 00 00 f0
0f ba 70 04 00 8d 4c 24

Sorry, I can't pass it througth ksymoops because it doesn't work for me
in later kernels (RH 6.9.5) it says Fatal Error (re_compile) - Invalid
range end, and I have recompiled it. So I have to give you the results
of sysklogd.

As a side note I have to say that after those BUG the net is still
working, and I have those rules added in init scripts:
        modprobe -k ip_tables
        modprobe -k iptable_nat
        insmod -k ipt_MASQUERADE 
        iptables -t nat -A POSTROUTING -o ppp+ -j MASQUERADE
        iptables -A FORWARD -i ppp+ -m state --state RELATED,ESTABLISHED
-j ACCEPT
        iptables -A FORWARD -o ppp+ -j ACCEPT
        echo 0 >/proc/sys/net/ipv4/tcp_ecn
Under heavy packet load in these tests I see that NAT messages about
dropped packets.

More tests as requested.

-- 
Jorge Nerin
<comandante@zaralinux.com>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [patch] NE2000

[kuznet]

Hello!

>         if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE))
> == 0)
>                 BUG();

The Puzzle... 8) It is truly impossible.

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

Re: [patch] NE2000

[kuznet]

Hello!

> No, that code is correct, provided (current->state == TASK_RUNNING)
> on entry.  If it isn't, there's a race window which can cause
> lost wakeups.   As a check you could add:
> 
> 	if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0)
> 		BUG();

Though it really cannot happen and really happens, as we have seen... 8)

In any case, Andrew, where is the race, when we enter in sleeping state?
Wakeup is not lost, it is just not required when we are not going
to schedule and force task to running state.

I still do not see how it is possible that task runs in sleeping state.
Apparently, set_current_state is forgotten somewhere. Do you see, where? 8)

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

Re: [patch] NE2000

[Andrew Morton]

kuznet@ms2.inr.ac.ru wrote:
> 
> Hello!
> 
> > No, that code is correct, provided (current->state == TASK_RUNNING)
> > on entry.  If it isn't, there's a race window which can cause
> > lost wakeups.   As a check you could add:
> >
> >       if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0)
> >               BUG();
> 
> Though it really cannot happen and really happens, as we have seen... 8)
> 
> In any case, Andrew, where is the race, when we enter in sleeping state?
> Wakeup is not lost, it is just not required when we are not going
> to schedule and force task to running state.

	set_current_state(TASK_INTERRUPTIBLE);
	add_wait_queue(...);
	/* window here */
	set_current_state(TASK_INTERRUPTIBLE);
	schedule();

If there's a wakeup by another CPU (or this CPU in an interrupt) in
that window, current->state will get switched to TASK_RUNNING.

Then it's immediately overwritten and we go to sleep.  Lost wakeup.

> I still do not see how it is possible that task runs in sleeping state.
> Apparently, set_current_state is forgotten somewhere. Do you see, where? 8)

Nope.  Is Jorge running SMP?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [patch] NE2000

[kuznet]

Hello!

> > In any case, Andrew, where is the race, when we enter in sleeping state?
> > Wakeup is not lost, it is just not required when we are not going
> > to schedule and force task to running state.
> 
> 	set_current_state(TASK_INTERRUPTIBLE);
> 	add_wait_queue(...);
> 	/* window here */
> 	set_current_state(TASK_INTERRUPTIBLE);
> 	schedule();
> 
> If there's a wakeup by another CPU (or this CPU in an interrupt) in
> that window, current->state will get switched to TASK_RUNNING.
> 
> Then it's immediately overwritten and we go to sleep.  Lost wakeup.

Look into code yet. It looks sort of different. Again:

> > Wakeup is not lost, it is just not required when we are not going
> > to schedule and force task to running state.

So that it is right not depening on anything.

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

Re: [patch] NE2000

[Andrew Morton]

kuznet@ms2.inr.ac.ru wrote:
> 
> Hello!
> 
> > No, that code is correct, provided (current->state == TASK_RUNNING)
> > on entry.  If it isn't, there's a race window which can cause
> > lost wakeups.   As a check you could add:
> >
> >       if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0)
> >               BUG();
> 
> Though it really cannot happen and really happens, as we have seen... 8)
> 
> In any case, Andrew, where is the race, when we enter in sleeping state?
> Wakeup is not lost, it is just not required when we are not going
> to schedule and force task to running state.
> 
> I still do not see how it is possible that task runs in sleeping state.
> Apparently, set_current_state is forgotten somewhere. Do you see, where? 8)
> 

OK, there are a few areas which look fishy.

Calling __lock_sock when we're getting ready to wait
on a different waitqueue looks like a rather risky area.
We have a single task which is on two waitqueues.

Consider the case of tcp_data_wait():

	add_wait_queue(sk->sleep)
	set_current_state(TASK_INTERRUPTIBLE);
	release_sock(sk);
	if (...)	/* Suppose this evaluates to false */
		schedule_timeout();
	lock_sock();

	__lock_sock()
	{
		add_wait_queue_exclusive(sk->lock.wq);
		/* Window 1: What does a wake_up(sk->sleep) do here? */
		current->state = TASK_EXCLUSIVE | TASK_UNINTERRUPTIBLE;
		/* Window 2: Bad things happen here */
		schedule();

If someone does a wakeup(sk->sleep) in Window 2 in
__lock_sock() the wakeup code will think that the
task is sleeping on sk->sleep in state
TASK_EXCLUSIVE|TASK_UNINTERRUPTIBLE,
when in fact it is not.  So a wakeup which _should_ have gone to
a different exclusive task actually goes to this one. This is
fantastically hard to hit because of the direction of the
waitqueue scan.

If the wakeup on sk->sleep happens during Window 1
it will be completely lost, but that's OK because
this task is not yet TASK_EXCLUSIVE (providing the
write ordering behaves as we want?)

If a wakeup on sk->lock.wq happens during Window 1
it will be completely lost.

wait_for_connect() and wait_for_tcp_memory() play similar
games with lock_sock() whereby they can appear to be on
two waitqueues at the same time. And again, because
lock_sock() uses TASK_EXCLUSIVE a wake_up on sk->sleep
could choose this task instead of a TASK_EXCLUSIVE task
which is _really_ sleeping on sk->sleep.

Now, this may not be a problem in practise, and in fact the
above may not be bugs because I missed something. But I suggest you
have a think about it.  My brain is starting to hurt.

But none of these explain Jorge's problem.  How he got to where
he did in !TASK_RUNNING.  Plus the possible lock_sock problems
just look too damn hard to hit to explain Jorge's repeatability.

It may be useful to put a Pentium hardware watchpoint onto
current->state.  Does kdb support those?

Can sock_fasync() be called when we're on a waitqueue, not in
state TASK_RUNNING and prior to schedule()?

inet_wait_for_connect() is OK.
wait_for_tcp_connect() is OK.
tcp_close() is OK.

Also, are you sure that all occurrences of 

	current->state = <whatever>;

are still safe on weakly ordered CPUs? (Not that this
would explain Jorge's problem).

hmm..  khttpd tries to do wake-one, but
interruptible_sleep_on_timeout() confounds it.
Bummer.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [patch] NE2000

[kuznet]

Hello!

[ Dave, please, look! I will strain brains this night too.
  Indeed, this sounds dubious. ]

No, Andrew, this is surely not related to either of puzzles even if it
is really buggy place. ping does not use either tcp or socket lock. 8)

> Can sock_fasync() be called when we're on a waitqueue, not in
> state TASK_RUNNING and prior to schedule()?

No top level syscalls can be called in either state but running.
That funny BUG() proves the opposite, but this must not happen in any case.
At least, I still cannot reproduce this here. 8)

Alexey



> OK, there are a few areas which look fishy.
> 
> Calling __lock_sock when we're getting ready to wait
> on a different waitqueue looks like a rather risky area.
> We have a single task which is on two waitqueues.
> 
> Consider the case of tcp_data_wait():
> 
> 	add_wait_queue(sk->sleep)
> 	set_current_state(TASK_INTERRUPTIBLE);
> 	release_sock(sk);
> 	if (...)	/* Suppose this evaluates to false */
> 		schedule_timeout();
> 	lock_sock();
> 
> 	__lock_sock()
> 	{
> 		add_wait_queue_exclusive(sk->lock.wq);
> 		/* Window 1: What does a wake_up(sk->sleep) do here? */
> 		current->state = TASK_EXCLUSIVE | TASK_UNINTERRUPTIBLE;
> 		/* Window 2: Bad things happen here */
> 		schedule();
> 
> If someone does a wakeup(sk->sleep) in Window 2 in
> __lock_sock() the wakeup code will think that the
> task is sleeping on sk->sleep in state
> TASK_EXCLUSIVE|TASK_UNINTERRUPTIBLE,
> when in fact it is not.  So a wakeup which _should_ have gone to
> a different exclusive task actually goes to this one. This is
> fantastically hard to hit because of the direction of the
> waitqueue scan.
> 
> If the wakeup on sk->sleep happens during Window 1
> it will be completely lost, but that's OK because
> this task is not yet TASK_EXCLUSIVE (providing the
> write ordering behaves as we want?)
> 
> If a wakeup on sk->lock.wq happens during Window 1
> it will be completely lost.
> 
> wait_for_connect() and wait_for_tcp_memory() play similar
> games with lock_sock() whereby they can appear to be on
> two waitqueues at the same time. And again, because
> lock_sock() uses TASK_EXCLUSIVE a wake_up on sk->sleep
> could choose this task instead of a TASK_EXCLUSIVE task
> which is _really_ sleeping on sk->sleep.
> 
> Now, this may not be a problem in practise, and in fact the
> above may not be bugs because I missed something. But I suggest you
> have a think about it.  My brain is starting to hurt.
> 
> But none of these explain Jorge's problem.  How he got to where
> he did in !TASK_RUNNING.  Plus the possible lock_sock problems
> just look too damn hard to hit to explain Jorge's repeatability.
> 
> It may be useful to put a Pentium hardware watchpoint onto
> current->state.  Does kdb support those?
> 
> Can sock_fasync() be called when we're on a waitqueue, not in
> state TASK_RUNNING and prior to schedule()?
> 
> inet_wait_for_connect() is OK.
> wait_for_tcp_connect() is OK.
> tcp_close() is OK.
> 
> Also, are you sure that all occurrences of 
> 
> 	current->state = <whatever>;
> 
> are still safe on weakly ordered CPUs? (Not that this
> would explain Jorge's problem).
> 
> hmm..  khttpd tries to do wake-one, but
> interruptible_sleep_on_timeout() confounds it.
> Bummer.
> 

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

Re: [patch] NE2000

[David S. Miller]

   From: kuznet@ms2.inr.ac.ru
   Date: Wed, 8 Nov 2000 23:31:28 +0300 (MSK)

   [ Dave, please, look! I will strain brains this night too.
     Indeed, this sounds dubious. ]

It is true disaster to be on multiple wait queues at once.
There are no doubts.

   No, Andrew, this is surely not related to either of puzzles even if it
   is really buggy place. ping does not use either tcp or socket lock. 8)

(BTW, this BUG() case sounds like memory corruption, not logic bug in
 the code.  BUTTT there was hard error in test9, but fixed in test10,
 about wakeups.  It would set task running state back to TASK_RUNNING
 outside of runqueue lock, then add task to runqueue with lock held.
 I assume test10 was tried already though.)

Yes, these multiple wait-queue cases must be repaired.  BTW, look
at fs/pipe.c:pipe_wait(), whoever wrote this understood, even though
second wait queue hides behind semaphore :-)))

Consider next the case of being on some wait queue, and touching user
space, taking fault and sleeping on disk I/O or low memory.  This
issue could have very far reaching consequences.

I will think about this some more.

Later,
David S. Miller
davem@redhat.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [patch] NE2000

[David S. Miller]

   From: kuznet@ms2.inr.ac.ru
   Date: Wed, 8 Nov 2000 23:31:28 +0300 (MSK)

   [ Dave, please, look! I will strain brains this night too.
     Indeed, this sounds dubious. ]

Alexey!  Even someone understood all this already, look
to include/net/sock.h SOCK_SLEEP_{PRE,POST} macros :-)

I will compose a patch to fix all this.

Later,
David S. Miller
davem@redhat.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [patch] NE2000

[David S. Miller]

   Date: Thu, 09 Nov 2000 21:53:42 +1100
   From: Andrew Morton <andrewm@uow.edu.au>

   "David S. Miller" wrote:
   > I will compose a patch to fix all this.

   I've quickly been through just about all of the kernel wrt
   waitqueues.

My analysis was in error, BEWARE!

Being on multiple wait queues at once is just fine.  I verified this
with Linus tonight.

The problem case is in mixing TASK_EXCLUSIVE and non-TASK_EXCLUSIVE
sleeps, that is what can actually cause problems.

Everything else is fine.  Anyways, the (untested) patch below should
cure the lock_sock() cases.

--- ./net/ipv4/af_inet.c.~1~	Tue Oct 24 14:26:18 2000
+++ ./net/ipv4/af_inet.c	Wed Nov  8 17:28:47 2000
@@ -543,24 +543,27 @@
 {
 	DECLARE_WAITQUEUE(wait, current);
 
-	__set_current_state(TASK_INTERRUPTIBLE);
-	add_wait_queue(sk->sleep, &wait);
-
 	/* Basic assumption: if someone sets sk->err, he _must_
 	 * change state of the socket from TCP_SYN_*.
 	 * Connect() does not allow to get error notifications
 	 * without closing the socket.
 	 */
 	while ((1<<sk->state)&(TCPF_SYN_SENT|TCPF_SYN_RECV)) {
+		__set_current_state(TASK_INTERRUPTIBLE);
+		add_wait_queue(sk->sleep, &wait);
+
 		release_sock(sk);
-		timeo = schedule_timeout(timeo);
+
+		if ((1<<sk->state)&(TCPF_SYN_SENT|TCPF_SYN_RECV))
+			timeo = schedule_timeout(timeo);
+
+		__set_current_state(TASK_RUNNING);
+		remove_wait_queue(sk->sleep, &wait);
+
 		lock_sock(sk);
 		if (signal_pending(current) || !timeo)
 			break;
-		set_current_state(TASK_INTERRUPTIBLE);
 	}
-	__set_current_state(TASK_RUNNING);
-	remove_wait_queue(sk->sleep, &wait);
 	return timeo;
 }
 
--- ./net/ipv4/tcp.c.~1~	Fri Oct  6 15:45:41 2000
+++ ./net/ipv4/tcp.c	Wed Nov  8 17:35:31 2000
@@ -826,10 +826,12 @@
 
 		release_sock(sk);
 		*timeo_p = schedule_timeout(*timeo_p);
-		lock_sock(sk);
 
 		__set_task_state(tsk, TASK_RUNNING);
 		remove_wait_queue(sk->sleep, &wait);
+
+		lock_sock(sk);
+
 		sk->tp_pinfo.af_tcp.write_pending--;
 	}
 	return 0;
@@ -854,24 +856,31 @@
 
 	clear_bit(SOCK_ASYNC_NOSPACE, &sk->socket->flags);
 
-	add_wait_queue(sk->sleep, &wait);
 	for (;;) {
 		set_bit(SOCK_NOSPACE, &sk->socket->flags);
 
+		add_wait_queue(sk->sleep, &wait);
 		set_current_state(TASK_INTERRUPTIBLE);
 
-		if (signal_pending(current))
-			break;
-		if (tcp_memory_free(sk) && !vm_wait)
-			break;
-		if (sk->shutdown & SEND_SHUTDOWN)
-			break;
-		if (sk->err)
+		if (signal_pending(current) ||
+		    (tcp_memory_free(sk) && !vm_wait) ||
+		    (sk->shutdown & SEND_SHUTDOWN) ||
+		    sk->err) {
+			current->state = TASK_RUNNING;
+			remove_wait_queue(sk->sleep, &wait);
 			break;
+		}
+
 		release_sock(sk);
+
 		if (!tcp_memory_free(sk) || vm_wait)
 			current_timeo = schedule_timeout(current_timeo);
+
+		current->state = TASK_RUNNING;
+		remove_wait_queue(sk->sleep, &wait);
+
 		lock_sock(sk);
+
 		if (vm_wait) {
 			if (timeo != MAX_SCHEDULE_TIMEOUT &&
 			    (timeo -= vm_wait-current_timeo) < 0)
@@ -881,8 +890,6 @@
 			timeo = current_timeo;
 		}
 	}
-	current->state = TASK_RUNNING;
-	remove_wait_queue(sk->sleep, &wait);
 	return timeo;
 }
 
@@ -1266,7 +1273,6 @@
 	DECLARE_WAITQUEUE(wait, current);
 
 	add_wait_queue(sk->sleep, &wait);
-
 	__set_current_state(TASK_INTERRUPTIBLE);
 
 	set_bit(SOCK_ASYNC_WAITDATA, &sk->socket->flags);
@@ -1275,11 +1281,12 @@
 	if (skb_queue_empty(&sk->receive_queue))
 		timeo = schedule_timeout(timeo);
 
+	remove_wait_queue(sk->sleep, &wait);
+	__set_current_state(TASK_RUNNING);
+
 	lock_sock(sk);
 	clear_bit(SOCK_ASYNC_WAITDATA, &sk->socket->flags);
 
-	remove_wait_queue(sk->sleep, &wait);
-	__set_current_state(TASK_RUNNING);
 	return timeo;
 }
 
@@ -1826,19 +1833,23 @@
 		struct task_struct *tsk = current;
 		DECLARE_WAITQUEUE(wait, current);
 
-		add_wait_queue(sk->sleep, &wait);
-
 		do {
+			add_wait_queue(sk->sleep, &wait);
 			set_current_state(TASK_INTERRUPTIBLE);
-			if (!closing(sk))
+			if (!closing(sk)) {
+				tsk->state = TASK_RUNNING;
+				remove_wait_queue(sk->sleep, &wait);
 				break;
+			}
 			release_sock(sk);
+
 			timeout = schedule_timeout(timeout);
+
+			tsk->state = TASK_RUNNING;
+			remove_wait_queue(sk->sleep, &wait);
+
 			lock_sock(sk);
 		} while (!signal_pending(tsk) && timeout);
-
-		tsk->state = TASK_RUNNING;
-		remove_wait_queue(sk->sleep, &wait);
 	}
 
 adjudge_to_death:
@@ -2009,12 +2020,17 @@
 	 * our exclusiveness temporarily when we get woken up without
 	 * having to remove and re-insert us on the wait queue.
 	 */
-	add_wait_queue_exclusive(sk->sleep, &wait);
 	for (;;) {
+		add_wait_queue_exclusive(sk->sleep, &wait);
 		current->state = TASK_EXCLUSIVE | TASK_INTERRUPTIBLE;
+
 		release_sock(sk);
 		if (sk->tp_pinfo.af_tcp.accept_queue == NULL)
 			timeo = schedule_timeout(timeo);
+
+		current->state = TASK_RUNNING;
+		remove_wait_queue(sk->sleep, &wait);
+
 		lock_sock(sk);
 		err = 0;
 		if (sk->tp_pinfo.af_tcp.accept_queue)
@@ -2029,8 +2045,6 @@
 		if (!timeo)
 			break;
 	}
-	current->state = TASK_RUNNING;
-	remove_wait_queue(sk->sleep, &wait);
 	return err;
 }
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [patch] NE2000

[Tom Leete]

"David S. Miller" wrote:
> 
>    Date: Thu, 09 Nov 2000 21:53:42 +1100
>    From: Andrew Morton <andrewm@uow.edu.au>
> 
>    "David S. Miller" wrote:
>    > I will compose a patch to fix all this.
> 
>    I've quickly been through just about all of the kernel wrt
>    waitqueues.
> 
> My analysis was in error, BEWARE!
> 
> Being on multiple wait queues at once is just fine.  I verified this
> with Linus tonight.
> 
> The problem case is in mixing TASK_EXCLUSIVE and non-TASK_EXCLUSIVE
> sleeps, that is what can actually cause problems.
> 
> Everything else is fine.  Anyways, the (untested) patch below should
> cure the lock_sock() cases.
> 
> --- ./net/ipv4/af_inet.c.~1~    Tue Oct 24 14:26:18 2000
> +++ ./net/ipv4/af_inet.c        Wed Nov  8 17:28:47 2000
[...]
> --- ./net/ipv4/tcp.c.~1~        Fri Oct  6 15:45:41 2000
> +++ ./net/ipv4/tcp.c    Wed Nov  8 17:35:31 2000

This touches the places where I saw hangs, so I'm testing.
Too soon to have statistics, but with this patch I have
observed no more failures to wake (what I referred  to as
"soft hangs").

I have seen a total I/O lockup, but no info escapes to
indicate its source. No NMI wakeup available, maybe I should
rig a pushbutton.

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

Re: [patch] NE2000

[kuznet]

Hello!

> Alexey!  Even someone understood all this already, look
> to include/net/sock.h SOCK_SLEEP_{PRE,POST} macros :-)
> 
> I will compose a patch to fix all this.

O! But who was this wiseman? 8)

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

Re: [patch] NE2000

[Steve Whitehouse]

Hi,

I have to own up and say that it was me :-) you'll see that DECnet is the
only protocol to use these macros at the moment. I'm sure though that I
only copied what IPv4 was doing at the time, along with the hints I had
from yourself and Dave,

Steve.

> 
> Hello!
> 
> > Alexey!  Even someone understood all this already, look
> > to include/net/sock.h SOCK_SLEEP_{PRE,POST} macros :-)
> > 
> > I will compose a patch to fix all this.
> 
> O! But who was this wiseman? 8)
> 
> Alexey
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> Please read the FAQ at http://www.tux.org/lkml/
> 

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

Re: ping -f kills ne2k (was:[patch] NE2000)

[Paul Gortmaker]

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

> 
> Well, I have tried it with 2.4.0-test10, both SMP and non-SMP, and the
> result is a little confusing.
> 
> Under SMP a ping -s 50000 -f other_host takes down the network access
> with no messages (ne2k-pci), and no possibility of being restored
> without a reboot.
> 
> Under UP the same command works ok, but after a while the dots stop for
> 30sec, then ping prints an 'E' and the dots continue. strace revealed
> this:

Another suggestion - if you have your heart set on using ping
as your network stress tool, you may want to try using multiple
instances of MTU sized pings versus  a single "ping -s 50000".
In this way you aren't involving any IP frag code and its associated
bean counting - giving us one less factor to consider.

Oh, and since you get a silent failure, maybe you would be interested
in testing this patch I was (originally) saving for 2.5.x. -- It adds
watchdog transmit timeout functionality to 8390.c (which is used by
the ne2k-pci driver).  Last time I updated it was a couple of months
ago, but nothing has changed since then.

Paul.

[-- Attachment #2: 2400-t5-8390-diff0 --]
[-- Type: text/plain, Size: 5623 bytes --]

--- linux~/drivers/net/8390.c	Fri Jul  7 03:45:36 2000
+++ linux/drivers/net/8390.c	Sat Jul 22 04:56:51 2000
@@ -38,6 +38,7 @@
   Paul Gortmaker	: add kmod support for auto-loading of the 8390
 			  module by all drivers that require it.
   Alan Cox		: Spinlocking work, added 'BUG_83C690'
+  Paul Gortmaker	: Separate out Tx timeout code from Tx path.
 
   Sources:
   The National Semiconductor LAN Databook, and the 3Com 3c503 databook.
@@ -105,6 +106,7 @@
 /* Index to functions. */
 static void ei_tx_intr(struct net_device *dev);
 static void ei_tx_err(struct net_device *dev);
+static void ei_tx_timeout(struct net_device *dev);
 static void ei_receive(struct net_device *dev);
 static void ei_rx_overrun(struct net_device *dev);
 
@@ -161,6 +163,13 @@
 		printk(KERN_EMERG "%s: ei_open passed a non-existent device!\n", dev->name);
 		return -ENXIO;
 	}
+	
+	/* The card I/O part of the driver (e.g. 3c503) can hook a Tx timeout
+	    wrapper that does e.g. media check & then calls ei_tx_timeout. */
+	if (dev->tx_timeout == NULL)
+		 dev->tx_timeout = ei_tx_timeout;
+	if (dev->watchdog_timeo <= 0)
+		 dev->watchdog_timeo = TX_TIMEOUT;
     
 	/*
 	 *	Grab the page lock so we own the register set, then call
@@ -200,89 +209,66 @@
 }
 
 /**
- * ei_start_xmit - begin packet transmission
- * @skb: packet to be sent
- * @dev: network device to which packet is sent
+ * ei_tx_timeout - handle transmit time out condition
+ * @dev: network device which has apparently fallen asleep
  *
- * Sends a packet to an 8390 network device.
+ * Called by kernel when device never acknowledges a transmit has
+ * completed (or failed) - i.e. never posted a Tx related interrupt.
  */
- 
-static int ei_start_xmit(struct sk_buff *skb, struct net_device *dev)
+
+void ei_tx_timeout(struct net_device *dev)
 {
 	long e8390_base = dev->base_addr;
 	struct ei_device *ei_local = (struct ei_device *) dev->priv;
-	int length, send_length, output_page;
+	int txsr, isr, tickssofar = jiffies - dev->trans_start;
 	unsigned long flags;
 
-	/*
-	 *  If it has been too long since the last Tx, we assume the
-	 *  board has died and kick it.
-	 */
- 
-	if (netif_queue_stopped(dev)) {
-		/* Do timeouts, just like the 8003 driver. */
-		int txsr;
-		int isr;
-		int tickssofar = jiffies - dev->trans_start;
-
-		/*
-		 *	Need the page lock. Now see what went wrong. This bit is
-		 *	fast.
-		 */
-		 		
-		spin_lock_irqsave(&ei_local->page_lock, flags);
-		txsr = inb(e8390_base+EN0_TSR);
-		if (tickssofar < TX_TIMEOUT ||	(tickssofar < (TX_TIMEOUT+5) && ! (txsr & ENTSR_PTX))) 
-		{
-			spin_unlock_irqrestore(&ei_local->page_lock, flags);
-			return 1;
-		}
-
-		ei_local->stat.tx_errors++;
-		isr = inb(e8390_base+EN0_ISR);
-		if (!netif_running(dev)) {
-			spin_unlock_irqrestore(&ei_local->page_lock, flags);
-			printk(KERN_WARNING "%s: xmit on stopped card\n", dev->name);
-			return 1;
-		}
-
-		/*
-		 * Note that if the Tx posted a TX_ERR interrupt, then the
-		 * error will have been handled from the interrupt handler
-		 * and not here. Error statistics are handled there as well.
-		 */
+	ei_local->stat.tx_errors++;
 
-		printk(KERN_DEBUG "%s: Tx timed out, %s TSR=%#2x, ISR=%#2x, t=%d.\n",
-			dev->name, (txsr & ENTSR_ABT) ? "excess collisions." :
-			(isr) ? "lost interrupt?" : "cable problem?", txsr, isr, tickssofar);
+	spin_lock_irqsave(&ei_local->page_lock, flags);
+	txsr = inb(e8390_base+EN0_TSR);
+	isr = inb(e8390_base+EN0_ISR);
+	spin_unlock_irqrestore(&ei_local->page_lock, flags);
 
-		if (!isr && !ei_local->stat.tx_packets) 
-		{
-			/* The 8390 probably hasn't gotten on the cable yet. */
-			ei_local->interface_num ^= 1;   /* Try a different xcvr.  */
-		}
+	printk(KERN_DEBUG "%s: Tx timed out, %s TSR=%#2x, ISR=%#2x, t=%d.\n",
+		dev->name, (txsr & ENTSR_ABT) ? "excess collisions." :
+		(isr) ? "lost interrupt?" : "cable problem?", txsr, isr, tickssofar);
 
-		/*
-		 *	Play shuffle the locks, a reset on some chips takes a few
-		 *	mS. We very rarely hit this point.
-		 */
-		 
-		spin_unlock_irqrestore(&ei_local->page_lock, flags);
+	if (!isr && !ei_local->stat.tx_packets) 
+	{
+		/* The 8390 probably hasn't gotten on the cable yet. */
+		ei_local->interface_num ^= 1;   /* Try a different xcvr.  */
+	}
 
-		/* Ugly but a reset can be slow, yet must be protected */
+	/* Ugly but a reset can be slow, yet must be protected */
 		
-		disable_irq_nosync(dev->irq);
-		spin_lock(&ei_local->page_lock);
+	disable_irq_nosync(dev->irq);
+	spin_lock(&ei_local->page_lock);
 		
-		/* Try to restart the card.  Perhaps the user has fixed something. */
-		ei_reset_8390(dev);
-		NS8390_init(dev, 1);
+	/* Try to restart the card.  Perhaps the user has fixed something. */
+	ei_reset_8390(dev);
+	NS8390_init(dev, 1);
 		
-		spin_unlock(&ei_local->page_lock);
-		enable_irq(dev->irq);
-		dev->trans_start = jiffies;
-	}
+	spin_unlock(&ei_local->page_lock);
+	enable_irq(dev->irq);
+	netif_wake_queue(dev);
+}
     
+/**
+ * ei_start_xmit - begin packet transmission
+ * @skb: packet to be sent
+ * @dev: network device to which packet is sent
+ *
+ * Sends a packet to an 8390 network device.
+ */
+ 
+static int ei_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	long e8390_base = dev->base_addr;
+	struct ei_device *ei_local = (struct ei_device *) dev->priv;
+	int length, send_length, output_page;
+	unsigned long flags;
+
 	length = skb->len;
 
 	/* Mask interrupts from the ethercard. 
@@ -1147,6 +1133,7 @@
 EXPORT_SYMBOL(ei_open);
 EXPORT_SYMBOL(ei_close);
 EXPORT_SYMBOL(ei_interrupt);
+EXPORT_SYMBOL(ei_tx_timeout);
 EXPORT_SYMBOL(ethdev_init);
 EXPORT_SYMBOL(NS8390_init);
 

Re: ping -f kills ne2k (was:[patch] NE2000)

[Jorge Nerin]

Paul Gortmaker wrote:
> 
> >
> > Well, I have tried it with 2.4.0-test10, both SMP and non-SMP, and the
> > result is a little confusing.
> >
> > Under SMP a ping -s 50000 -f other_host takes down the network access
> > with no messages (ne2k-pci), and no possibility of being restored
> > without a reboot.
> >
> > Under UP the same command works ok, but after a while the dots stop for
> > 30sec, then ping prints an 'E' and the dots continue. strace revealed
> > this:
> 
> Another suggestion - if you have your heart set on using ping
> as your network stress tool, you may want to try using multiple
> instances of MTU sized pings versus  a single "ping -s 50000".
> In this way you aren't involving any IP frag code and its associated
> bean counting - giving us one less factor to consider.
> 
> Oh, and since you get a silent failure, maybe you would be interested
> in testing this patch I was (originally) saving for 2.5.x. -- It adds
> watchdog transmit timeout functionality to 8390.c (which is used by
> the ne2k-pci driver).  Last time I updated it was a couple of months
> ago, but nothing has changed since then.
> 
> Paul.
> 

Tested with ping -f -s 1400 (1400 in order not to reach 1500)
It took about half an hour and more than one million packets, but I
finally took the net down, with 12 concurrent pings.

To eliminate factors I have deleted all the NAT rules wich gave messages
about dropped packets, and unloaded all the iptables modules.

I have to make the test without the test check in sock_wait_for_wmem:
        if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE))
== 0)
                BUG();

Because as I said in a previous msg it gave me BUG()s very early in the
tests, and I still had network access.

If someone has a better sugestion as a nic stress tool I can try it, but
now I only have two ways of reaching this limits, ping -f of big
packets, and sometimes (only 4 or 5) during a copy of a large file over
NFS, but it's not a easy way to cause this.

-- 
Jorge Nerin
<comandante@zaralinux.com>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: ping -f kills ne2k (was:[patch] NE2000)

[Jorge Nerin]

Jorge Nerin wrote:
> 
> Paul Gortmaker wrote:
> >
> > >
> > > Well, I have tried it with 2.4.0-test10, both SMP and non-SMP, and the
> > > result is a little confusing.
> > >
> > > Under SMP a ping -s 50000 -f other_host takes down the network access
> > > with no messages (ne2k-pci), and no possibility of being restored
> > > without a reboot.
> > >
> > > Under UP the same command works ok, but after a while the dots stop for
> > > 30sec, then ping prints an 'E' and the dots continue. strace revealed
> > > this:
> >
> > Another suggestion - if you have your heart set on using ping
> > as your network stress tool, you may want to try using multiple
> > instances of MTU sized pings versus  a single "ping -s 50000".
> > In this way you aren't involving any IP frag code and its associated
> > bean counting - giving us one less factor to consider.
> >
> > Oh, and since you get a silent failure, maybe you would be interested
> > in testing this patch I was (originally) saving for 2.5.x. -- It adds
> > watchdog transmit timeout functionality to 8390.c (which is used by
> > the ne2k-pci driver).  Last time I updated it was a couple of months
> > ago, but nothing has changed since then.
> >
> > Paul.
> >
> 
> Tested with ping -f -s 1400 (1400 in order not to reach 1500)
> It took about half an hour and more than one million packets, but I
> finally took the net down, with 12 concurrent pings.
> 
> To eliminate factors I have deleted all the NAT rules wich gave messages
> about dropped packets, and unloaded all the iptables modules.
> 
> I have to make the test without the test check in sock_wait_for_wmem:
>         if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE))
> == 0)
>                 BUG();
> 
> Because as I said in a previous msg it gave me BUG()s very early in the
> tests, and I still had network access.
> 
> If someone has a better sugestion as a nic stress tool I can try it, but
> now I only have two ways of reaching this limits, ping -f of big
> packets, and sometimes (only 4 or 5) during a copy of a large file over
> NFS, but it's not a easy way to cause this.
> 
> --
> Jorge Nerin
> <comandante@zaralinux.com>

Well, now it's kernel 2.4.0-test11-pre1 + 8390nmi, and the same
conditions, about 8 pings concurrent, and this time it took only 202k
packets to take the ne2k-pci down, but this time the watchdog says:

Nov  9 16:00:52 quartz kernel: NETDEV WATCHDOG: eth0: transmit timed out
Nov  9 16:00:52 quartz kernel: eth0: Tx timed out, lost interrupt?
TSR=0x3, ISR=0x3, t=1792.
Nov  9 16:00:54 quartz kernel: NETDEV WATCHDOG: eth0: transmit timed out
Nov  9 16:00:54 quartz kernel: eth0: Tx timed out, lost interrupt?
TSR=0x3, ISR=0x3, t=117.
Nov  9 16:00:56 quartz kernel: NETDEV WATCHDOG: eth0: transmit timed out
Nov  9 16:00:56 quartz kernel: eth0: Tx timed out, lost interrupt?
TSR=0x3, ISR=0x3, t=117.
Nov  9 16:00:58 quartz kernel: NETDEV WATCHDOG: eth0: transmit timed out
Nov  9 16:00:58 quartz kernel: eth0: Tx timed out, lost interrupt?
TSR=0x3, ISR=0x3, t=117.
Nov  9 16:01:00 quartz kernel: NETDEV WATCHDOG: eth0: transmit timed out
Nov  9 16:01:00 quartz kernel: eth0: Tx timed out, lost interrupt?
TSR=0x3, ISR=0x3, t=117.
Nov  9 16:01:02 quartz kernel: NETDEV WATCHDOG: eth0: transmit timed out
Nov  9 16:01:02 quartz kernel: eth0: Tx timed out, lost interrupt?
TSR=0x3, ISR=0x3, t=117.

And never comes alive again.

-- 
Jorge Nerin
<comandante@zaralinux.com>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [patch] NE2000

[Paul Gortmaker]

Jeff Garzik wrote:
> 
> pavel rabel wrote:
> > help. So I removed PCI code from ne.c to have ISA only driver. It
> 
> This change sounds ok to me, if noone else objects.  (I added to the CC
> a bit)  I saw that code, and was thinking about doing the same thing
> myself.  ne2k-pci.c definitely has changes which are not included in
> ne.c, and it seems silly to duplicate ne2000 PCI support.

Actually if you look at the archives (ID 39A3A608.1F984C60@yahoo.com)
you will see that I've stated this will be done for 2.5 a couple of
months ago.  (Which is also why PCI support in ne.c hasn't tracked that
of ne2k-pci.c - I want to avoid encouraging new PCI users of ne.c)

There is no urgency in trying to squeeze a patch like this in the back
door of a 2.4.0 release.  For example, there are people out there now
who are using the ne.c driver to run both ISA and PCI cards in the same 
box without having to use 2 different drivers.  We can wait until 2.5.0
to break their .config file.

[ I've several other 8390 related patches I've been sitting on - trying
to not contribute to the delay of 2.4.0 unless explicitly asked, such
as the 8390.h get_module_symbol deletion.  Other 8390 patches I have 
are a separated Tx timeout for 8390.c, kill off dev->rmem_start/end 
and use ioremap() where required, and replace old check/request_region()
with code that makes use of the newer resource structures. ]

Good to know people are still keeping an eye out for dead code though...

Thanks,
Paul.


_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com

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

Re: [patch] NE2000

[pavel rabel]

On Mon, 30 Oct 2000, Paul Gortmaker wrote:

> There is no urgency in trying to squeeze a patch like this in the back
> door of a 2.4.0 release.  For example, there are people out there now
> who are using the ne.c driver to run both ISA and PCI cards in the same 
> box without having to use 2 different drivers.  We can wait until 2.5.0
> to break their .config file.

I am not quite sure how it will work when you try to use both ne.c and
ne2k-pci drivers in the same box. Which driver will be used for PCI card?
Maybe people with both cards are forced to use inferior driver for PCI
card.

Pavel Rabel


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

Re: [patch] NE2000

[Jeff Garzik]

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

Paul Gortmaker wrote:
> There is no urgency in trying to squeeze a patch like this in the back
> door of a 2.4.0 release.  For example, there are people out there now
> who are using the ne.c driver to run both ISA and PCI cards in the same
> box without having to use 2 different drivers.  We can wait until 2.5.0
> to break their .config file.

IMNSHO this is a bug, though...

Do a diff of the key 8390 interface routines in ne.c, and ne2k-pci.c. 
Ignoring the inb_p and outb_p differences, there are distinct advantages
to using ne2k-pci.c on with an NE2000 PCI board.

Since ne2k-pci.c supports all boards ne.c does, and includes some fixes
that ne.c does not, it seems like removing the PCI support in ne.c is a
bug fix change.

It looks like ne2k-pci.c does need a HZ scaling fixing from ne.c
though...

	Jeff



-- 
Jeff Garzik             | "Mind if I drive?"  -Sam
Building 1024           | "Not if you don't mind me clawing at the
MandrakeSoft            |  dash and shrieking like a cheerleader."
                        |                     -Max

[-- Attachment #2: ne2000.diff --]
[-- Type: text/plain, Size: 10910 bytes --]

--- /g/g/tmp/1	Mon Oct 30 14:22:41 2000
+++ /g/g/tmp/2	Mon Oct 30 14:22:58 2000
@@ -1,60 +1,61 @@
 /* Hard reset the card.  This used to pause for the same period that a
    8390 reset command required, but that shouldn't be necessary. */
-
-static void ne_reset_8390(struct net_device *dev)
+static void
+ne2k_pci_reset_8390(struct net_device *dev)
 {
 	unsigned long reset_start_time = jiffies;
 
-	if (ei_debug > 1)
-		printk(KERN_DEBUG "resetting the 8390 t=%ld...", jiffies);
+	if (debug > 1) printk("%s: Resetting the 8390 t=%ld...",
+						  dev->name, jiffies);
 
-	/* DON'T change these to inb_p/outb_p or reset will fail on clones. */
 	outb(inb(NE_BASE + NE_RESET), NE_BASE + NE_RESET);
 
 	ei_status.txing = 0;
 	ei_status.dmaing = 0;
 
 	/* This check _should_not_ be necessary, omit eventually. */
-	while ((inb_p(NE_BASE+EN0_ISR) & ENISR_RESET) == 0)
-		if (jiffies - reset_start_time > 2*HZ/100) {
-			printk(KERN_WARNING "%s: ne_reset_8390() did not complete.\n", dev->name);
+	while ((inb(NE_BASE+EN0_ISR) & ENISR_RESET) == 0)
+		if (jiffies - reset_start_time > 2) {
+			printk("%s: ne2k_pci_reset_8390() did not complete.\n", dev->name);
 			break;
 		}
-	outb_p(ENISR_RESET, NE_BASE + EN0_ISR);	/* Ack intr. */
+	outb(ENISR_RESET, NE_BASE + EN0_ISR);	/* Ack intr. */
 }
 
 /* Grab the 8390 specific header. Similar to the block_input routine, but
    we don't need to be concerned with ring wrap as the header will be at
    the start of a page, so we optimize accordingly. */
 
-static void ne_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr, int ring_page)
+static void
+ne2k_pci_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr, int ring_page)
 {
-	int nic_base = dev->base_addr;
 
-	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
+	long nic_base = dev->base_addr;
 
-	if (ei_status.dmaing) 
-	{
-		printk(KERN_EMERG "%s: DMAing conflict in ne_get_8390_hdr "
+	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
+	if (ei_status.dmaing) {
+		printk("%s: DMAing conflict in ne2k_pci_get_8390_hdr "
 			"[DMAstat:%d][irqlock:%d].\n",
 			dev->name, ei_status.dmaing, ei_status.irqlock);
 		return;
 	}
 
 	ei_status.dmaing |= 0x01;
-	outb_p(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base+ NE_CMD);
-	outb_p(sizeof(struct e8390_pkt_hdr), nic_base + EN0_RCNTLO);
-	outb_p(0, nic_base + EN0_RCNTHI);
-	outb_p(0, nic_base + EN0_RSARLO);		/* On page boundary */
-	outb_p(ring_page, nic_base + EN0_RSARHI);
-	outb_p(E8390_RREAD+E8390_START, nic_base + NE_CMD);
+	outb(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base+ NE_CMD);
+	outb(sizeof(struct e8390_pkt_hdr), nic_base + EN0_RCNTLO);
+	outb(0, nic_base + EN0_RCNTHI);
+	outb(0, nic_base + EN0_RSARLO);		/* On page boundary */
+	outb(ring_page, nic_base + EN0_RSARHI);
+	outb(E8390_RREAD+E8390_START, nic_base + NE_CMD);
 
-	if (ei_status.word16)
+	if (ei_status.ne2k_flags & ONLY_16BIT_IO) {
 		insw(NE_BASE + NE_DATAPORT, hdr, sizeof(struct e8390_pkt_hdr)>>1);
-	else
-		insb(NE_BASE + NE_DATAPORT, hdr, sizeof(struct e8390_pkt_hdr));
+	} else {
+		*(u32*)hdr = le32_to_cpu(inl(NE_BASE + NE_DATAPORT));
+		le16_to_cpus(&hdr->count);
+	}
 
-	outb_p(ENISR_RDC, nic_base + EN0_ISR);	/* Ack intr. */
+	outb(ENISR_RDC, nic_base + EN0_ISR);	/* Ack intr. */
 	ei_status.dmaing &= ~0x01;
 }
 
@@ -63,172 +64,116 @@
    The NEx000 doesn't share the on-board packet memory -- you have to put
    the packet out through the "remote DMA" dataport using outb. */
 
-static void ne_block_input(struct net_device *dev, int count, struct sk_buff *skb, int ring_offset)
+static void
+ne2k_pci_block_input(struct net_device *dev, int count, struct sk_buff *skb, int ring_offset)
 {
-#ifdef NE_SANITY_CHECK
-	int xfer_count = count;
-#endif
-	int nic_base = dev->base_addr;
+	long nic_base = dev->base_addr;
 	char *buf = skb->data;
 
 	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
-	if (ei_status.dmaing) 
-	{
-		printk(KERN_EMERG "%s: DMAing conflict in ne_block_input "
+	if (ei_status.dmaing) {
+		printk("%s: DMAing conflict in ne2k_pci_block_input "
 			"[DMAstat:%d][irqlock:%d].\n",
 			dev->name, ei_status.dmaing, ei_status.irqlock);
 		return;
 	}
 	ei_status.dmaing |= 0x01;
-	outb_p(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base+ NE_CMD);
-	outb_p(count & 0xff, nic_base + EN0_RCNTLO);
-	outb_p(count >> 8, nic_base + EN0_RCNTHI);
-	outb_p(ring_offset & 0xff, nic_base + EN0_RSARLO);
-	outb_p(ring_offset >> 8, nic_base + EN0_RSARHI);
-	outb_p(E8390_RREAD+E8390_START, nic_base + NE_CMD);
-	if (ei_status.word16) 
-	{
+	if (ei_status.ne2k_flags & ONLY_32BIT_IO)
+		count = (count + 3) & 0xFFFC;
+	outb(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base+ NE_CMD);
+	outb(count & 0xff, nic_base + EN0_RCNTLO);
+	outb(count >> 8, nic_base + EN0_RCNTHI);
+	outb(ring_offset & 0xff, nic_base + EN0_RSARLO);
+	outb(ring_offset >> 8, nic_base + EN0_RSARHI);
+	outb(E8390_RREAD+E8390_START, nic_base + NE_CMD);
+
+	if (ei_status.ne2k_flags & ONLY_16BIT_IO) {
 		insw(NE_BASE + NE_DATAPORT,buf,count>>1);
-		if (count & 0x01) 
-		{
+		if (count & 0x01) {
 			buf[count-1] = inb(NE_BASE + NE_DATAPORT);
-#ifdef NE_SANITY_CHECK
-			xfer_count++;
-#endif
 		}
 	} else {
-		insb(NE_BASE + NE_DATAPORT, buf, count);
+		insl(NE_BASE + NE_DATAPORT, buf, count>>2);
+		if (count & 3) {
+			buf += count & ~3;
+			if (count & 2)
+				*((u16*)buf)++ = le16_to_cpu(inw(NE_BASE + NE_DATAPORT));
+			if (count & 1)
+				*buf = inb(NE_BASE + NE_DATAPORT);
 	}
-
-#ifdef NE_SANITY_CHECK
-	/* This was for the ALPHA version only, but enough people have
-	   been encountering problems so it is still here.  If you see
-	   this message you either 1) have a slightly incompatible clone
-	   or 2) have noise/speed problems with your bus. */
-
-	if (ei_debug > 1) 
-	{
-		/* DMA termination address check... */
-		int addr, tries = 20;
-		do {
-			/* DON'T check for 'inb_p(EN0_ISR) & ENISR_RDC' here
-			   -- it's broken for Rx on some cards! */
-			int high = inb_p(nic_base + EN0_RSARHI);
-			int low = inb_p(nic_base + EN0_RSARLO);
-			addr = (high << 8) + low;
-			if (((ring_offset + xfer_count) & 0xff) == low)
-				break;
-		} while (--tries > 0);
-	 	if (tries <= 0)
-			printk(KERN_WARNING "%s: RX transfer address mismatch,"
-				"%#4.4x (expected) vs. %#4.4x (actual).\n",
-				dev->name, ring_offset + xfer_count, addr);
 	}
-#endif
-	outb_p(ENISR_RDC, nic_base + EN0_ISR);	/* Ack intr. */
+
+	outb(ENISR_RDC, nic_base + EN0_ISR);	/* Ack intr. */
 	ei_status.dmaing &= ~0x01;
 }
 
-static void ne_block_output(struct net_device *dev, int count,
+static void
+ne2k_pci_block_output(struct net_device *dev, int count,
 		const unsigned char *buf, const int start_page)
 {
-	int nic_base = NE_BASE;
+	long nic_base = NE_BASE;
 	unsigned long dma_start;
-#ifdef NE_SANITY_CHECK
-	int retries = 0;
-#endif
 
-	/* Round the count up for word writes.  Do we need to do this?
-	   What effect will an odd byte count have on the 8390?
-	   I should check someday. */
-	   
-	if (ei_status.word16 && (count & 0x01))
+	/* On little-endian it's always safe to round the count up for
+	   word writes. */
+	if (ei_status.ne2k_flags & ONLY_32BIT_IO)
+		count = (count + 3) & 0xFFFC;
+	else
+		if (count & 0x01)
 		count++;
 
 	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
-	if (ei_status.dmaing) 
-	{
-		printk(KERN_EMERG "%s: DMAing conflict in ne_block_output."
+	if (ei_status.dmaing) {
+		printk("%s: DMAing conflict in ne2k_pci_block_output."
 			"[DMAstat:%d][irqlock:%d]\n",
 			dev->name, ei_status.dmaing, ei_status.irqlock);
 		return;
 	}
 	ei_status.dmaing |= 0x01;
 	/* We should already be in page 0, but to be safe... */
-	outb_p(E8390_PAGE0+E8390_START+E8390_NODMA, nic_base + NE_CMD);
-
-#ifdef NE_SANITY_CHECK
-retry:
-#endif
+	outb(E8390_PAGE0+E8390_START+E8390_NODMA, nic_base + NE_CMD);
 
 #ifdef NE8390_RW_BUGFIX
 	/* Handle the read-before-write bug the same way as the
 	   Crynwr packet driver -- the NatSemi method doesn't work.
 	   Actually this doesn't always work either, but if you have
 	   problems with your NEx000 this is better than nothing! */
-	   
-	outb_p(0x42, nic_base + EN0_RCNTLO);
-	outb_p(0x00,   nic_base + EN0_RCNTHI);
-	outb_p(0x42, nic_base + EN0_RSARLO);
-	outb_p(0x00, nic_base + EN0_RSARHI);
-	outb_p(E8390_RREAD+E8390_START, nic_base + NE_CMD);
-	/* Make certain that the dummy read has occurred. */
-	udelay(6);
+	outb(0x42, nic_base + EN0_RCNTLO);
+	outb(0x00, nic_base + EN0_RCNTHI);
+	outb(0x42, nic_base + EN0_RSARLO);
+	outb(0x00, nic_base + EN0_RSARHI);
+	outb(E8390_RREAD+E8390_START, nic_base + NE_CMD);
 #endif
-
-	outb_p(ENISR_RDC, nic_base + EN0_ISR);
+	outb(ENISR_RDC, nic_base + EN0_ISR);
 
 	/* Now the normal output. */
-	outb_p(count & 0xff, nic_base + EN0_RCNTLO);
-	outb_p(count >> 8,   nic_base + EN0_RCNTHI);
-	outb_p(0x00, nic_base + EN0_RSARLO);
-	outb_p(start_page, nic_base + EN0_RSARHI);
-
-	outb_p(E8390_RWRITE+E8390_START, nic_base + NE_CMD);
-	if (ei_status.word16) {
+	outb(count & 0xff, nic_base + EN0_RCNTLO);
+	outb(count >> 8,   nic_base + EN0_RCNTHI);
+	outb(0x00, nic_base + EN0_RSARLO);
+	outb(start_page, nic_base + EN0_RSARHI);
+	outb(E8390_RWRITE+E8390_START, nic_base + NE_CMD);
+	if (ei_status.ne2k_flags & ONLY_16BIT_IO) {
 		outsw(NE_BASE + NE_DATAPORT, buf, count>>1);
 	} else {
-		outsb(NE_BASE + NE_DATAPORT, buf, count);
+		outsl(NE_BASE + NE_DATAPORT, buf, count>>2);
+		if (count & 3) {
+			buf += count & ~3;
+			if (count & 2)
+				outw(cpu_to_le16(*((u16*)buf)++), NE_BASE + NE_DATAPORT);
+		}
 	}
 
 	dma_start = jiffies;
 
-#ifdef NE_SANITY_CHECK
-	/* This was for the ALPHA version only, but enough people have
-	   been encountering problems so it is still here. */
-	
-	if (ei_debug > 1) 
-	{
-		/* DMA termination address check... */
-		int addr, tries = 20;
-		do {
-			int high = inb_p(nic_base + EN0_RSARHI);
-			int low = inb_p(nic_base + EN0_RSARLO);
-			addr = (high << 8) + low;
-			if ((start_page << 8) + count == addr)
-				break;
-		} while (--tries > 0);
-
-		if (tries <= 0) 
-		{
-			printk(KERN_WARNING "%s: Tx packet transfer address mismatch,"
-				"%#4.4x (expected) vs. %#4.4x (actual).\n",
-				dev->name, (start_page << 8) + count, addr);
-			if (retries++ == 0)
-				goto retry;
-		}
-	}
-#endif
-
-	while ((inb_p(nic_base + EN0_ISR) & ENISR_RDC) == 0)
-		if (jiffies - dma_start > 2*HZ/100) {		/* 20ms */
-			printk(KERN_WARNING "%s: timeout waiting for Tx RDC.\n", dev->name);
-			ne_reset_8390(dev);
+	while ((inb(nic_base + EN0_ISR) & ENISR_RDC) == 0)
+		if (jiffies - dma_start > 2) { 			/* Avoid clock roll-over. */
+			printk("%s: timeout waiting for Tx RDC.\n", dev->name);
+			ne2k_pci_reset_8390(dev);
 			NS8390_init(dev,1);
 			break;
 		}
 
-	outb_p(ENISR_RDC, nic_base + EN0_ISR);	/* Ack intr. */
+	outb(ENISR_RDC, nic_base + EN0_ISR);	/* Ack intr. */
 	ei_status.dmaing &= ~0x01;
 	return;
 }

Re: [patch] NE2000

[Paul Gortmaker]

Jeff Garzik wrote:
> 
> Paul Gortmaker wrote:
> > There is no urgency in trying to squeeze a patch like this in the back
> > door of a 2.4.0 release.  For example, there are people out there now
> > who are using the ne.c driver to run both ISA and PCI cards in the same
> > box without having to use 2 different drivers.  We can wait until 2.5.0
> > to break their .config file.
> 
> IMNSHO this is a bug, though...

Maybe I wasn't 100% clear - these are people who have intentionally
chosen to do so. (There is probably a slight icache benefit in sharing
the same driver like this, FWIW....)

> Since ne2k-pci.c supports all boards ne.c does, and includes some fixes
> that ne.c does not, it seems like removing the PCI support in ne.c is a
> bug fix change.

If you want to roll it into the merge (and can get it past Linus) then
please feel free to do so - I'll be glad to cross it off my list sooner
as opposed to later.

In addition to removing all remains of PCI support from ne.c this patch
also has three trivial changes that were also in my queue for ne.c.

	1) Two new ID signatures added to the bad_clone_list

	2) int base_addr promoted to unsigned int base_addr in ne_probe,
	   as required for SuperH CPU systems that have ne2000 cards.

	3) ISA PnP card info printk'd *before* the actual probe, and
	   not after (just in case the probe silently hangs or whatever).

Paul.

--- 2400-t10/linux-g/drivers/net/ne.c~	Tue Jul 11 02:29:10 2000
+++ 2400-t10/linux-g/drivers/net/ne.c	Wed Nov  1 00:02:17 2000
@@ -29,6 +29,7 @@
     occur after memory is allocated for dev->priv. Deallocated memory
     last in cleanup_modue()
     Richard Guenther    : Added support for ISAPnP cards
+    Paul Gortmaker	: Discontinued PCI support - use ne2k-pci.c instead.
     
 */
 
@@ -43,7 +44,6 @@
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/errno.h>
-#include <linux/pci.h>
 #include <linux/isapnp.h>
 #include <linux/init.h>
 #include <linux/delay.h>
@@ -75,23 +75,6 @@
 };
 #endif
 
-#ifdef CONFIG_PCI
-/* Ack! People are making PCI ne2000 clones! Oh the horror, the horror... */
-static struct { unsigned short vendor, dev_id; char *name; }
-pci_clone_list[] __initdata = {
-	{PCI_VENDOR_ID_REALTEK,		PCI_DEVICE_ID_REALTEK_8029,		"Realtek 8029" },
-	{PCI_VENDOR_ID_WINBOND2,	PCI_DEVICE_ID_WINBOND2_89C940,		"Winbond 89C940" },
-	{PCI_VENDOR_ID_COMPEX,		PCI_DEVICE_ID_COMPEX_RL2000,		"Compex ReadyLink 2000" },
-	{PCI_VENDOR_ID_KTI,		PCI_DEVICE_ID_KTI_ET32P2,		"KTI ET32P2" },
-	{PCI_VENDOR_ID_NETVIN,		PCI_DEVICE_ID_NETVIN_NV5000SC,		"NetVin NV5000" },
-	{PCI_VENDOR_ID_VIA,		PCI_DEVICE_ID_VIA_82C926,		"VIA 82C926 Amazon" },
-	{PCI_VENDOR_ID_SURECOM,		PCI_DEVICE_ID_SURECOM_NE34,		"SureCom NE34"},
-	{0,}
-};
-
-static int probe_pci = 1;
-#endif
-
 static struct { unsigned short vendor, function; char *name; }
 isapnp_clone_list[] __initdata = {
 	{ISAPNP_VENDOR('E','D','I'), ISAPNP_FUNCTION(0x0216),		"NN NE2000" },
@@ -114,7 +97,9 @@
     {"ET-100","ET-200", {0x00, 0x45, 0x54}}, /* YANG and YA clone */
     {"COMPEX","COMPEX16",{0x00,0x80,0x48}}, /* Broken ISA Compex cards */
     {"E-LAN100", "E-LAN200", {0x00, 0x00, 0x5d}}, /* Broken ne1000 clones */
-    {"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */    
+    {"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */
+    {"REALTEK", "RTL8019", {0x00, 0x00, 0xe8}}, /* no-name with Realtek chip */
+    {"LCS-8834", "LCS-8836", {0x04, 0x04, 0x37}}, /* ShinyNet (SET) */
     {0,}
 };
 #endif
@@ -132,15 +117,9 @@
 #define NESM_START_PG	0x40	/* First page of TX buffer */
 #define NESM_STOP_PG	0x80	/* Last page +1 of RX ring */
 
-/* Non-zero only if the current card is a PCI with BIOS-set IRQ. */
-static unsigned int pci_irq_line = 0;
-
 int ne_probe(struct net_device *dev);
 static int ne_probe1(struct net_device *dev, int ioaddr);
 static int ne_probe_isapnp(struct net_device *dev);
-#ifdef CONFIG_PCI
-static int ne_probe_pci(struct net_device *dev);
-#endif
 
 static int ne_open(struct net_device *dev);
 static int ne_close(struct net_device *dev);
@@ -180,16 +159,9 @@
 {"ne", ne_probe1, NE_IO_EXTENT, netcard_portlist};
 #else
 
-/*
- * Note that at boot, this probe only picks up one card at a time, even for
- * multiple PCI ne2k cards. Use "ether=0,0,eth1" if you have a second PCI
- * ne2k card.  This keeps things consistent regardless of the bus type of
- * the card.
- */
-
 int __init ne_probe(struct net_device *dev)
 {
-	int base_addr = dev ? dev->base_addr : 0;
+	unsigned int base_addr = dev ? dev->base_addr : 0;
 
 	/* First check any supplied i/o locations. User knows best. <cough> */
 	if (base_addr > 0x1ff)	/* Check a single specified location. */
@@ -197,12 +169,6 @@
 	else if (base_addr != 0)	/* Don't probe at all. */
 		return -ENXIO;
 
-#ifdef CONFIG_PCI
-	/* Then look for any installed PCI clones */
-	if (probe_pci && pci_present() && (ne_probe_pci(dev) == 0))
-		return 0;
-#endif
-
 	/* Then look for any installed ISAPnP clones */
 	if (isapnp_present() && (ne_probe_isapnp(dev) == 0))
 		return 0;
@@ -222,43 +188,6 @@
 }
 #endif
 
-#ifdef CONFIG_PCI
-static int __init ne_probe_pci(struct net_device *dev)
-{
-	int i;
-
-	for (i = 0; pci_clone_list[i].vendor != 0; i++) {
-		struct pci_dev *pdev = NULL;
-		unsigned int pci_ioaddr = 0;
-
-		while ((pdev = pci_find_device(pci_clone_list[i].vendor, pci_clone_list[i].dev_id, pdev))) {
-			if (pci_enable_device(pdev))
-				continue;
-			pci_ioaddr = pci_resource_start (pdev, 0);
-			/* Avoid already found cards from previous calls */
-			if (check_region(pci_ioaddr, NE_IO_EXTENT))
-				continue;
-			pci_irq_line = pdev->irq;
-			if (pci_irq_line) break;	/* Found it */
-		}
-		if (!pdev)
-			continue;
-		printk(KERN_INFO "ne.c: PCI BIOS reports %s at i/o %#x, irq %d.\n",
-				pci_clone_list[i].name,
-				pci_ioaddr, pci_irq_line);
-		printk("*\n* Use of the PCI-NE2000 driver with this card is recommended!\n*\n");
-		if (ne_probe1(dev, pci_ioaddr) != 0) {	/* Shouldn't happen. */
-			printk(KERN_ERR "ne.c: Probe of PCI card at %#x failed.\n", pci_ioaddr);
-			pci_irq_line = 0;
-			return -ENXIO;
-		}
-		pci_irq_line = 0;
-		return 0;
-	}
-	return -ENODEV;
-}
-#endif  /* CONFIG_PCI */
-
 static int __init ne_probe_isapnp(struct net_device *dev)
 {
 	int i;
@@ -275,14 +204,18 @@
 				continue;
 			if (idev->activate(idev))
 				continue;
-			pci_irq_line = idev->irq_resource[0].start;
 			/* if no irq, search for next */
-			if (!pci_irq_line)
+			if (idev->irq_resource[0].start == 0)
 				continue;
 			/* found it */
-			if (ne_probe1(dev, idev->resource[0].start) != 0) {	/* Shouldn't happen. */
-				printk(KERN_ERR "ne.c: Probe of ISAPnP card at %#lx failed.\n",
-				       idev->resource[0].start);
+			dev->base_addr = idev->resource[0].start;
+			dev->irq = idev->irq_resource[0].start;
+			printk(KERN_INFO "ne.c: ISAPnP reports %s at i/o %#lx, irq %d.\n",
+				isapnp_clone_list[i].name,
+				
+				dev->base_addr, dev->irq);
+			if (ne_probe1(dev, dev->base_addr) != 0) {	/* Shouldn't happen. */
+				printk(KERN_ERR "ne.c: Probe of ISAPnP card at %#lx failed.\n", dev->base_addr);
 				return -ENXIO;
 			}
 			ei_status.priv = (unsigned long)idev;
@@ -290,9 +223,6 @@
 		}
 		if (!idev)
 			continue;
-		printk(KERN_INFO "ne.c: ISAPnP reports %s at i/o %#lx, irq %d.\n",
-				isapnp_clone_list[i].name,
-				dev->base_addr, dev->irq);
 		return 0;
 	}
 
@@ -403,20 +333,10 @@
 			wordlength = 1;
 	}
 
-	/* At this point, wordlength *only* tells us if the SA_prom is doubled
-	   up or not because some broken PCI cards don't respect the byte-wide
-	   request in program_seq above, and hence don't have doubled up values.
-	   These broken cards would otherwise be detected as an ne1000.  */
-
-	if (wordlength == 2)
-		for (i = 0; i < 16; i++)
-			SA_prom[i] = SA_prom[i+i];
-
-	if (pci_irq_line || ioaddr >= 0x400)
-		wordlength = 2;		/* Catch broken PCI cards mentioned above. */
-
 	if (wordlength == 2) 
 	{
+		for (i = 0; i < 16; i++)
+			SA_prom[i] = SA_prom[i+i];
 		/* We must set the 8390 for word mode. */
 		outb_p(0x49, ioaddr + EN0_DCFG);
 		start_page = NESM_START_PG;
@@ -473,9 +393,6 @@
 #endif
 	}
 
-	if (pci_irq_line)
-		dev->irq = pci_irq_line;
-
 	if (dev->irq < 2) 
 	{
 		autoirq_setup(0);
@@ -509,8 +426,7 @@
 	   share and the board will usually be enabled. */
 
 	{
-		int irqval = request_irq(dev->irq, ei_interrupt,
-				 pci_irq_line ? SA_SHIRQ : 0, name, dev);
+		int irqval = request_irq(dev->irq, ei_interrupt, 0, name, dev);
 		if (irqval) {
 			printk (" unable to get IRQ %d (irqval=%d).\n", dev->irq, irqval);
 			kfree(dev->priv);
@@ -823,10 +739,6 @@
 MODULE_PARM(irq, "1-" __MODULE_STRING(MAX_NE_CARDS) "i");
 MODULE_PARM(bad, "1-" __MODULE_STRING(MAX_NE_CARDS) "i");
 
-#ifdef CONFIG_PCI
-MODULE_PARM(probe_pci, "i");
-#endif
-
 /* This is set up so that no ISA autoprobe takes place. We can't guarantee
 that the ne2k probe is the last 8390 based probe to take place (as it
 is at boot) and so the probe will get confused by any other 8390 cards.
@@ -855,7 +767,7 @@
 		if (io[this_dev] != 0)
 			printk(KERN_WARNING "ne.c: No NE*000 card found at i/o = %#x\n", io[this_dev]);
 		else
-			printk(KERN_NOTICE "ne.c: No PCI cards found. Use \"io=0xNNN\" value(s) for ISA cards.\n");
+			printk(KERN_NOTICE "ne.c: You must supply \"io=0xNNN\" value(s) for ISA cards.\n");
 		unload_8390_module();
 		return -ENXIO;
 	}



_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com

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

Re: [patch] NE2000

[Donald Becker]

On Wed, 1 Nov 2000, Paul Gortmaker wrote:

> Jeff Garzik wrote:
> > Paul Gortmaker wrote:
> > > There is no urgency in trying to squeeze a patch like this in the back
> > > door of a 2.4.0 release.  For example, there are people out there now
> > > who are using the ne.c driver to run both ISA and PCI cards in the same
> > > box without having to use 2 different drivers.  We can wait until 2.5.0
> > > to break their .config file.
> > 
> > IMNSHO this is a bug, though...
..
> If you want to roll it into the merge (and can get it past Linus) then
> please feel free to do so - I'll be glad to cross it off my list sooner
> as opposed to later.

If the ne* drivers are going to be updated, you might want to add in the
full-duplex support of the latest ne2k-pci.c driver at
    ftp://www.scyld.com/pub/network/ne2k-pci.c

Donald Becker				becker@scyld.com
Scyld Computing Corporation		http://www.scyld.com
410 Severn Ave. Suite 210		Second Generation Beowulf Clusters
Annapolis MD 21403			410-990-9993

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

Re: [patch] NE2000

[Jeff Garzik]

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

Paul,

Ok, here's what I have.  Included are your changes, as well as

drivers/net/ne.c:
* use probe_irq_on/off instead of autoirq_xxx (autoirq is going away)
* request_region first thing in ne_probe1, before any hardware
interaction takes place.  Eliminates any potential resource races.  Also
eliminates a call to check_region.
* Trim trailing whitespace

drivers/net/ne2k-pci.c:
* Merge Becker version 1.02 ne2k-pci.c, which adds forced full duplex
support, and also several cosmetic changes where merely serve to bring
the kernel's ne2k-pci.c closer to Becker's version (ie. makes the diff
smaller).
* Just call BUG() if we don't have a net_device in ne2k_pci_remove_one
* Correct pci_module_init return code handling

As an aside, for 2.5.x, would it be possible to merge all the common
ne2000 code (block_input/output, etc.) from the various ne2k drivers? 
As it stands there exists ne.c, ne2.c, ne2k-pci.c, and pcnet_cs.c, all
of which do pretty much the same thing at their core.  [and AFAICS, all
but pcnet_cs might easily call a common ne2k library]

Regards,

	Jeff


-- 
Jeff Garzik             | "Mind if I drive?"  -Sam
Building 1024           | "Not if you don't mind me clawing at the
MandrakeSoft            |  dash and shrieking like a cheerleader."
                        |                     -Max

[-- Attachment #2: ne.patch --]
[-- Type: text/plain, Size: 27653 bytes --]

Index: drivers/net/ne.c
===================================================================
RCS file: /cvsroot/gkernel/linux_2_4/drivers/net/ne.c,v
retrieving revision 1.1.1.4
diff -u -r1.1.1.4 ne.c
--- drivers/net/ne.c	2000/10/31 21:21:49	1.1.1.4
+++ drivers/net/ne.c	2000/11/01 13:24:36
@@ -29,21 +29,22 @@
     occur after memory is allocated for dev->priv. Deallocated memory
     last in cleanup_modue()
     Richard Guenther    : Added support for ISAPnP cards
-    
+    Paul Gortmaker	: Discontinued PCI support - use ne2k-pci.c instead.
+
 */
 
 /* Routines for the NatSemi-based designs (NE[12]000). */
 
-static const char *version =
-    "ne.c:v1.10 9/23/94 Donald Becker (becker@cesdis.gsfc.nasa.gov)\n";
+static const char version1[] =
+"ne.c:v1.10 9/23/94 Donald Becker (becker@scyld.com)\n";
+static const char version2[] =
+"Last modified Nov 1, 2000 by Paul Gortmaker\n";
 
 
 #include <linux/module.h>
-#include <linux/config.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/errno.h>
-#include <linux/pci.h>
 #include <linux/isapnp.h>
 #include <linux/init.h>
 #include <linux/delay.h>
@@ -70,28 +71,11 @@
 
 /* A zero-terminated list of I/O addresses to be probed at boot. */
 #ifndef MODULE
-static unsigned int netcard_portlist[] __initdata = { 
+static unsigned int netcard_portlist[] __initdata = {
 	0x300, 0x280, 0x320, 0x340, 0x360, 0x380, 0
 };
 #endif
 
-#ifdef CONFIG_PCI
-/* Ack! People are making PCI ne2000 clones! Oh the horror, the horror... */
-static struct { unsigned short vendor, dev_id; char *name; }
-pci_clone_list[] __initdata = {
-	{PCI_VENDOR_ID_REALTEK,		PCI_DEVICE_ID_REALTEK_8029,		"Realtek 8029" },
-	{PCI_VENDOR_ID_WINBOND2,	PCI_DEVICE_ID_WINBOND2_89C940,		"Winbond 89C940" },
-	{PCI_VENDOR_ID_COMPEX,		PCI_DEVICE_ID_COMPEX_RL2000,		"Compex ReadyLink 2000" },
-	{PCI_VENDOR_ID_KTI,		PCI_DEVICE_ID_KTI_ET32P2,		"KTI ET32P2" },
-	{PCI_VENDOR_ID_NETVIN,		PCI_DEVICE_ID_NETVIN_NV5000SC,		"NetVin NV5000" },
-	{PCI_VENDOR_ID_VIA,		PCI_DEVICE_ID_VIA_82C926,		"VIA 82C926 Amazon" },
-	{PCI_VENDOR_ID_SURECOM,		PCI_DEVICE_ID_SURECOM_NE34,		"SureCom NE34"},
-	{0,}
-};
-
-static int probe_pci = 1;
-#endif
-
 static struct { unsigned short vendor, function; char *name; }
 isapnp_clone_list[] __initdata = {
 	{ISAPNP_VENDOR('E','D','I'), ISAPNP_FUNCTION(0x0216),		"NN NE2000" },
@@ -114,7 +98,9 @@
     {"ET-100","ET-200", {0x00, 0x45, 0x54}}, /* YANG and YA clone */
     {"COMPEX","COMPEX16",{0x00,0x80,0x48}}, /* Broken ISA Compex cards */
     {"E-LAN100", "E-LAN200", {0x00, 0x00, 0x5d}}, /* Broken ne1000 clones */
-    {"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */    
+    {"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */
+    {"REALTEK", "RTL8019", {0x00, 0x00, 0xe8}}, /* no-name with Realtek chip */
+    {"LCS-8834", "LCS-8836", {0x04, 0x04, 0x37}}, /* ShinyNet (SET) */
     {0,}
 };
 #endif
@@ -132,15 +118,9 @@
 #define NESM_START_PG	0x40	/* First page of TX buffer */
 #define NESM_STOP_PG	0x80	/* Last page +1 of RX ring */
 
-/* Non-zero only if the current card is a PCI with BIOS-set IRQ. */
-static unsigned int pci_irq_line = 0;
-
 int ne_probe(struct net_device *dev);
 static int ne_probe1(struct net_device *dev, int ioaddr);
 static int ne_probe_isapnp(struct net_device *dev);
-#ifdef CONFIG_PCI
-static int ne_probe_pci(struct net_device *dev);
-#endif
 
 static int ne_open(struct net_device *dev);
 static int ne_close(struct net_device *dev);
@@ -175,16 +155,9 @@
 	E2010	 starts at 0x100 and ends at 0x4000.
 	E2010-x starts at 0x100 and ends at 0xffff.  */
 
-/*
- * Note that at boot, this probe only picks up one card at a time, even for
- * multiple PCI ne2k cards. Use "ether=0,0,eth1" if you have a second PCI
- * ne2k card.  This keeps things consistent regardless of the bus type of
- * the card.
- */
-
 int __init ne_probe(struct net_device *dev)
 {
-	int base_addr = dev ? dev->base_addr : 0;
+	unsigned int base_addr = dev ? dev->base_addr : 0;
 
 	/* First check any supplied i/o locations. User knows best. <cough> */
 	if (base_addr > 0x1ff)	/* Check a single specified location. */
@@ -192,12 +165,6 @@
 	else if (base_addr != 0)	/* Don't probe at all. */
 		return -ENXIO;
 
-#ifdef CONFIG_PCI
-	/* Then look for any installed PCI clones */
-	if (probe_pci && pci_present() && (ne_probe_pci(dev) == 0))
-		return 0;
-#endif
-
 	/* Then look for any installed ISAPnP clones */
 	if (isapnp_present() && (ne_probe_isapnp(dev) == 0))
 		return 0;
@@ -206,8 +173,6 @@
 	/* Last resort. The semi-risky ISA auto-probe. */
 	for (base_addr = 0; netcard_portlist[base_addr] != 0; base_addr++) {
 		int ioaddr = netcard_portlist[base_addr];
-		if (check_region(ioaddr, NE_IO_EXTENT))
-			continue;
 		if (ne_probe1(dev, ioaddr) == 0)
 			return 0;
 	}
@@ -216,47 +181,10 @@
 	return -ENODEV;
 }
 
-#ifdef CONFIG_PCI
-static int __init ne_probe_pci(struct net_device *dev)
-{
-	int i;
-
-	for (i = 0; pci_clone_list[i].vendor != 0; i++) {
-		struct pci_dev *pdev = NULL;
-		unsigned int pci_ioaddr = 0;
-
-		while ((pdev = pci_find_device(pci_clone_list[i].vendor, pci_clone_list[i].dev_id, pdev))) {
-			if (pci_enable_device(pdev))
-				continue;
-			pci_ioaddr = pci_resource_start (pdev, 0);
-			/* Avoid already found cards from previous calls */
-			if (check_region(pci_ioaddr, NE_IO_EXTENT))
-				continue;
-			pci_irq_line = pdev->irq;
-			if (pci_irq_line) break;	/* Found it */
-		}
-		if (!pdev)
-			continue;
-		printk(KERN_INFO "ne.c: PCI BIOS reports %s at i/o %#x, irq %d.\n",
-				pci_clone_list[i].name,
-				pci_ioaddr, pci_irq_line);
-		printk("*\n* Use of the PCI-NE2000 driver with this card is recommended!\n*\n");
-		if (ne_probe1(dev, pci_ioaddr) != 0) {	/* Shouldn't happen. */
-			printk(KERN_ERR "ne.c: Probe of PCI card at %#x failed.\n", pci_ioaddr);
-			pci_irq_line = 0;
-			return -ENXIO;
-		}
-		pci_irq_line = 0;
-		return 0;
-	}
-	return -ENODEV;
-}
-#endif  /* CONFIG_PCI */
-
 static int __init ne_probe_isapnp(struct net_device *dev)
 {
 	int i;
-	
+
 	for (i = 0; isapnp_clone_list[i].vendor != 0; i++) {
 		struct pci_dev *idev = NULL;
 
@@ -269,14 +197,18 @@
 				continue;
 			if (idev->activate(idev))
 				continue;
-			pci_irq_line = idev->irq_resource[0].start;
 			/* if no irq, search for next */
-			if (!pci_irq_line)
+			if (idev->irq_resource[0].start == 0)
 				continue;
 			/* found it */
-			if (ne_probe1(dev, idev->resource[0].start) != 0) {	/* Shouldn't happen. */
-				printk(KERN_ERR "ne.c: Probe of ISAPnP card at %#lx failed.\n",
-				       idev->resource[0].start);
+			dev->base_addr = idev->resource[0].start;
+			dev->irq = idev->irq_resource[0].start;
+			printk(KERN_INFO "ne.c: ISAPnP reports %s at i/o %#lx, irq %d.\n",
+				isapnp_clone_list[i].name,
+
+				dev->base_addr, dev->irq);
+			if (ne_probe1(dev, dev->base_addr) != 0) {	/* Shouldn't happen. */
+				printk(KERN_ERR "ne.c: Probe of ISAPnP card at %#lx failed.\n", dev->base_addr);
 				return -ENXIO;
 			}
 			ei_status.priv = (unsigned long)idev;
@@ -284,9 +216,6 @@
 		}
 		if (!idev)
 			continue;
-		printk(KERN_INFO "ne.c: ISAPnP reports %s at i/o %#lx, irq %d.\n",
-				isapnp_clone_list[i].name,
-				dev->base_addr, dev->irq);
 		return 0;
 	}
 
@@ -301,11 +230,17 @@
 	const char *name = NULL;
 	int start_page, stop_page;
 	int neX000, ctron, copam, bad_card;
-	int reg0 = inb_p(ioaddr);
+	int reg0, ret;
 	static unsigned version_printed = 0;
+
+	if (!request_region(ioaddr, NE_IO_EXTENT, dev->name))
+		return -EBUSY;
 
-	if (reg0 == 0xFF)
-		return -ENODEV;
+	reg0 = inb_p(ioaddr);
+	if (reg0 == 0xFF) {
+		ret = -ENODEV;
+		goto err_out;
+	}
 
 	/* Do a preliminary verification that we have a 8390. */
 	{
@@ -318,12 +253,13 @@
 		if (inb_p(ioaddr + EN0_COUNTER0) != 0) {
 			outb_p(reg0, ioaddr);
 			outb_p(regd, ioaddr + 0x0d);	/* Restore the old values. */
-			return -ENODEV;
+			ret = -ENODEV;
+			goto err_out;
 		}
 	}
 
 	if (ei_debug  &&  version_printed++ == 0)
-		printk(version);
+		printk(KERN_INFO "%s" KERN_INFO "%s", version1, version2);
 
 	printk(KERN_INFO "NE*000 ethercard probe at %#3x:", ioaddr);
 
@@ -350,10 +286,11 @@
 				break;
 			} else {
 				printk(" not found (no reset ack).\n");
-				return -ENODEV;
+				ret = -ENODEV;
+				goto err_out;
 			}
 		}
-	
+
 		outb_p(0xff, ioaddr + EN0_ISR);		/* Ack all intr. */
 	}
 
@@ -362,7 +299,7 @@
 	   We can't reliably read the SAPROM address without this.
 	   (I learned the hard way!). */
 	{
-		struct {unsigned char value, offset; } program_seq[] = 
+		struct {unsigned char value, offset; } program_seq[] =
 		{
 			{E8390_NODMA+E8390_PAGE0+E8390_STOP, E8390_CMD}, /* Select page 0*/
 			{0x48,	EN0_DCFG},	/* Set byte-wide (0x48) access. */
@@ -390,20 +327,10 @@
 			wordlength = 1;
 	}
 
-	/* At this point, wordlength *only* tells us if the SA_prom is doubled
-	   up or not because some broken PCI cards don't respect the byte-wide
-	   request in program_seq above, and hence don't have doubled up values.
-	   These broken cards would otherwise be detected as an ne1000.  */
-
 	if (wordlength == 2)
+	{
 		for (i = 0; i < 16; i++)
 			SA_prom[i] = SA_prom[i+i];
-
-	if (pci_irq_line || ioaddr >= 0x400)
-		wordlength = 2;		/* Catch broken PCI cards mentioned above. */
-
-	if (wordlength == 2) 
-	{
 		/* We must set the 8390 for word mode. */
 		outb_p(0x49, ioaddr + EN0_DCFG);
 		start_page = NESM_START_PG;
@@ -416,13 +343,12 @@
 	neX000 = (SA_prom[14] == 0x57  &&  SA_prom[15] == 0x57);
 	ctron =  (SA_prom[0] == 0x00 && SA_prom[1] == 0x00 && SA_prom[2] == 0x1d);
 	copam =  (SA_prom[14] == 0x49 && SA_prom[15] == 0x00);
-	
 
 	/* Set up the rest of the parameters. */
 	if (neX000 || bad_card || copam) {
 		name = (wordlength == 2) ? "NE2000" : "NE1000";
 	}
-	else if (ctron) 
+	else if (ctron)
 	{
 		name = (wordlength == 2) ? "Ctron-8" : "Ctron-16";
 		start_page = 0x01;
@@ -433,13 +359,13 @@
 #ifdef SUPPORT_NE_BAD_CLONES
 		/* Ack!  Well, there might be a *bad* NE*000 clone there.
 		   Check for total bogus addresses. */
-		for (i = 0; bad_clone_list[i].name8; i++) 
+		for (i = 0; bad_clone_list[i].name8; i++)
 		{
 			if (SA_prom[0] == bad_clone_list[i].SAprefix[0] &&
 				SA_prom[1] == bad_clone_list[i].SAprefix[1] &&
-				SA_prom[2] == bad_clone_list[i].SAprefix[2]) 
+				SA_prom[2] == bad_clone_list[i].SAprefix[2])
 			{
-				if (wordlength == 2) 
+				if (wordlength == 2)
 				{
 					name = bad_clone_list[i].name16;
 				} else {
@@ -448,31 +374,30 @@
 				break;
 			}
 		}
-		if (bad_clone_list[i].name8 == NULL) 
+		if (bad_clone_list[i].name8 == NULL)
 		{
 			printk(" not found (invalid signature %2.2x %2.2x).\n",
 				SA_prom[14], SA_prom[15]);
-			return -ENXIO;
+			ret = -ENXIO;
+			goto err_out;
 		}
 #else
 		printk(" not found.\n");
-		return -ENXIO;
+		ret = -ENXIO;
+		goto err_out;
 #endif
 	}
-
-	if (pci_irq_line)
-		dev->irq = pci_irq_line;
 
-	if (dev->irq < 2) 
+	if (dev->irq < 2)
 	{
-		autoirq_setup(0);
+		unsigned long cookie = probe_irq_on();
 		outb_p(0x50, ioaddr + EN0_IMR);	/* Enable one interrupt. */
 		outb_p(0x00, ioaddr + EN0_RCNTLO);
 		outb_p(0x00, ioaddr + EN0_RCNTHI);
 		outb_p(E8390_RREAD+E8390_START, ioaddr); /* Trigger it... */
 		mdelay(10);		/* wait 10ms for interrupt to propagate */
 		outb_p(0x00, ioaddr + EN0_IMR); 		/* Mask it again. */
-		dev->irq = autoirq_report(0);
+		dev->irq = probe_irq_off(cookie);
 		if (ei_debug > 2)
 			printk(" autoirq is %d\n", dev->irq);
 	} else if (dev->irq == 2)
@@ -482,31 +407,27 @@
 
 	if (! dev->irq) {
 		printk(" failed to detect IRQ line.\n");
-		return -EAGAIN;
+		ret = -EAGAIN;
+		goto err_out;
 	}
 
 	/* Allocate dev->priv and fill in 8390 specific dev fields. */
-	if (ethdev_init(dev)) 
+	if (ethdev_init(dev))
 	{
         	printk (" unable to get memory for dev->priv.\n");
-        	return -ENOMEM;
+        	ret = -ENOMEM;
+		goto err_out;
 	}
-   
+
 	/* Snarf the interrupt now.  There's no point in waiting since we cannot
 	   share and the board will usually be enabled. */
-
-	{
-		int irqval = request_irq(dev->irq, ei_interrupt,
-				 pci_irq_line ? SA_SHIRQ : 0, name, dev);
-		if (irqval) {
-			printk (" unable to get IRQ %d (irqval=%d).\n", dev->irq, irqval);
-			kfree(dev->priv);
-			dev->priv = NULL;
-			return -EAGAIN;
-		}
+	ret = request_irq(dev->irq, ei_interrupt, 0, name, dev);
+	if (ret) {
+		printk (" unable to get IRQ %d (errno=%d).\n", dev->irq, ret);
+		goto err_out_kfree;
 	}
+
 	dev->base_addr = ioaddr;
-	request_region(ioaddr, NE_IO_EXTENT, name);
 
 	for(i = 0; i < ETHER_ADDR_LEN; i++) {
 		printk(" %2.2x", SA_prom[i]);
@@ -536,6 +457,13 @@
 	dev->stop = &ne_close;
 	NS8390_init(dev, 0);
 	return 0;
+
+err_out_kfree:
+	kfree(dev->priv);
+	dev->priv = NULL;
+err_out:
+	release_region(ioaddr, NE_IO_EXTENT);
+	return ret;
 }
 
 static int ne_open(struct net_device *dev)
@@ -589,7 +517,7 @@
 
 	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
 
-	if (ei_status.dmaing) 
+	if (ei_status.dmaing)
 	{
 		printk(KERN_EMERG "%s: DMAing conflict in ne_get_8390_hdr "
 			"[DMAstat:%d][irqlock:%d].\n",
@@ -628,7 +556,7 @@
 	char *buf = skb->data;
 
 	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
-	if (ei_status.dmaing) 
+	if (ei_status.dmaing)
 	{
 		printk(KERN_EMERG "%s: DMAing conflict in ne_block_input "
 			"[DMAstat:%d][irqlock:%d].\n",
@@ -642,10 +570,10 @@
 	outb_p(ring_offset & 0xff, nic_base + EN0_RSARLO);
 	outb_p(ring_offset >> 8, nic_base + EN0_RSARHI);
 	outb_p(E8390_RREAD+E8390_START, nic_base + NE_CMD);
-	if (ei_status.word16) 
+	if (ei_status.word16)
 	{
 		insw(NE_BASE + NE_DATAPORT,buf,count>>1);
-		if (count & 0x01) 
+		if (count & 0x01)
 		{
 			buf[count-1] = inb(NE_BASE + NE_DATAPORT);
 #ifdef NE_SANITY_CHECK
@@ -662,7 +590,7 @@
 	   this message you either 1) have a slightly incompatible clone
 	   or 2) have noise/speed problems with your bus. */
 
-	if (ei_debug > 1) 
+	if (ei_debug > 1)
 	{
 		/* DMA termination address check... */
 		int addr, tries = 20;
@@ -697,12 +625,12 @@
 	/* Round the count up for word writes.  Do we need to do this?
 	   What effect will an odd byte count have on the 8390?
 	   I should check someday. */
-	   
+
 	if (ei_status.word16 && (count & 0x01))
 		count++;
 
 	/* This *shouldn't* happen. If it does, it's the last thing you'll see */
-	if (ei_status.dmaing) 
+	if (ei_status.dmaing)
 	{
 		printk(KERN_EMERG "%s: DMAing conflict in ne_block_output."
 			"[DMAstat:%d][irqlock:%d]\n",
@@ -722,7 +650,7 @@
 	   Crynwr packet driver -- the NatSemi method doesn't work.
 	   Actually this doesn't always work either, but if you have
 	   problems with your NEx000 this is better than nothing! */
-	   
+
 	outb_p(0x42, nic_base + EN0_RCNTLO);
 	outb_p(0x00,   nic_base + EN0_RCNTHI);
 	outb_p(0x42, nic_base + EN0_RSARLO);
@@ -752,8 +680,8 @@
 #ifdef NE_SANITY_CHECK
 	/* This was for the ALPHA version only, but enough people have
 	   been encountering problems so it is still here. */
-	
-	if (ei_debug > 1) 
+
+	if (ei_debug > 1)
 	{
 		/* DMA termination address check... */
 		int addr, tries = 20;
@@ -765,7 +693,7 @@
 				break;
 		} while (--tries > 0);
 
-		if (tries <= 0) 
+		if (tries <= 0)
 		{
 			printk(KERN_WARNING "%s: Tx packet transfer address mismatch,"
 				"%#4.4x (expected) vs. %#4.4x (actual).\n",
@@ -801,10 +729,6 @@
 MODULE_PARM(irq, "1-" __MODULE_STRING(MAX_NE_CARDS) "i");
 MODULE_PARM(bad, "1-" __MODULE_STRING(MAX_NE_CARDS) "i");
 
-#ifdef CONFIG_PCI
-MODULE_PARM(probe_pci, "i");
-#endif
-
 /* This is set up so that no ISA autoprobe takes place. We can't guarantee
 that the ne2k probe is the last 8390 based probe to take place (as it
 is at boot) and so the probe will get confused by any other 8390 cards.
@@ -833,7 +757,7 @@
 		if (io[this_dev] != 0)
 			printk(KERN_WARNING "ne.c: No NE*000 card found at i/o = %#x\n", io[this_dev]);
 		else
-			printk(KERN_NOTICE "ne.c: No PCI cards found. Use \"io=0xNNN\" value(s) for ISA cards.\n");
+			printk(KERN_NOTICE "ne.c: You must supply \"io=0xNNN\" value(s) for ISA cards.\n");
 		unload_8390_module();
 		return -ENXIO;
 	}
Index: drivers/net/ne2k-pci.c
===================================================================
RCS file: /cvsroot/gkernel/linux_2_4/drivers/net/ne2k-pci.c,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 ne2k-pci.c
--- drivers/net/ne2k-pci.c	2000/10/22 21:55:54	1.1.1.3
+++ drivers/net/ne2k-pci.c	2000/11/01 13:24:36
@@ -2,30 +2,49 @@
 /*
 	A Linux device driver for PCI NE2000 clones.
 
-	Authorship and other copyrights:
-	1992-1998 by Donald Becker, NE2000 core and various modifications.
+	Authors and other copyright holders:
+	1992-2000 by Donald Becker, NE2000 core and various modifications.
 	1995-1998 by Paul Gortmaker, core modifications and PCI support.
-
 	Copyright 1993 assigned to the United States Government as represented
 	by the Director, National Security Agency.
-
-	This software may be used and distributed according to the terms
-	of the GNU Public License, incorporated herein by reference.
 
-	The author may be reached as becker@CESDIS.gsfc.nasa.gov, or C/O
-	Center of Excellence in Space Data and Information Sciences
-	Code 930.5, Goddard Space Flight Center, Greenbelt MD 20771
+	This software may be used and distributed according to the terms of
+	the GNU General Public License (GPL), incorporated herein by reference.
+	Drivers based on or derived from this code fall under the GPL and must
+	retain the authorship, copyright and license notice.  This file is not
+	a complete program and may only be used when the entire operating
+	system is licensed under the GPL.
+
+	The author may be reached as becker@scyld.com, or C/O
+	Scyld Computing Corporation
+	410 Severn Ave., Suite 210
+	Annapolis MD 21403
 
-	People are making PCI ne2000 clones! Oh the horror, the horror...
-
 	Issues remaining:
-	No full-duplex support.
+	People are making PCI ne2000 clones! Oh the horror, the horror...
+	Limited full-duplex support.
 */
 
-/* Our copyright info must remain in the binary. */
-static const char *version =
-"ne2k-pci.c:vpre-1.00e 5/27/99 D. Becker/P. Gortmaker http://cesdis.gsfc.nasa.gov/linux/drivers/ne2k-pci.html\n";
+/* These identify the driver base version and may not be removed. */
+static const char version1[] =
+"ne2k-pci.c:v1.02 10/19/2000 D. Becker/P. Gortmaker\n";
+static const char version2[] =
+"  http://www.scyld.com/network/ne2k-pci.html\n";
+
+/* The user-configurable values.
+   These may be modified when a driver module is loaded.*/
+
+static int debug = 1;			/* 1 normal messages, 0 quiet .. 7 verbose. */
+
+#define MAX_UNITS 8				/* More are supported, limit only on options */
+/* Used to pass the full-duplex flag, etc. */
+static int full_duplex[MAX_UNITS];
+static int options[MAX_UNITS];
 
+/* Force a non std. amount of memory.  Units are 256 byte pages. */
+/* #define PACKETBUF_MEMSIZE	0x40 */
+
+
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
@@ -48,8 +67,11 @@
 #define outsl outsl_ns
 #endif
 
-/* Set statically or when loading the driver module. */
-static int debug = 1;
+MODULE_AUTHOR("Donald Becker / Paul Gortmaker");
+MODULE_DESCRIPTION("PCI NE2000 clone driver");
+MODULE_PARM(debug, "i");
+MODULE_PARM(options, "1-" __MODULE_STRING(MAX_UNITS) "i");
+MODULE_PARM(full_duplex, "1-" __MODULE_STRING(MAX_UNITS) "i");
 
 /* Some defines that people can play with if so inclined. */
 
@@ -59,19 +81,16 @@
 /* Do we implement the read before write bugfix ? */
 /* #define NE_RW_BUGFIX */
 
-/* Do we have a non std. amount of memory? (in units of 256 byte pages) */
-/* #define PACKETBUF_MEMSIZE	0x40 */
-
-#define ne2k_flags reg0			/* Rename an existing field to store flags! */
-
-/* Only the low 8 bits are usable for non-init-time flags! */
+/* Flags.  We rename an existing ei_status field to store flags! */
+/* Thus only the low 8 bits are usable for non-init-time flags. */
+#define ne2k_flags reg0
 enum {
-	HOLTEK_FDX=1, 		/* Full duplex -> set 0x80 at offset 0x20. */
-	ONLY_16BIT_IO=2, ONLY_32BIT_IO=4,	/* Chip can do only 16/32-bit xfers. */
+	ONLY_16BIT_IO=8, ONLY_32BIT_IO=4,	/* Chip can do only 16/32-bit xfers. */
+	FORCE_FDX=0x20,						/* User override. */
+	REALTEK_FDX=0x40, HOLTEK_FDX=0x80,
 	STOP_PG_0x60=0x100,
 };
 
-
 enum ne2k_pci_chipsets {
 	CH_RealTek_RTL_8029 = 0,
 	CH_Winbond_89C940,
@@ -90,7 +109,7 @@
 	char *name;
 	int flags;
 } pci_clone_list[] __devinitdata = {
-	{"RealTek RTL-8029", 0},
+	{"RealTek RTL-8029", REALTEK_FDX},
 	{"Winbond 89C940", 0},
 	{"Compex RL2000", 0},
 	{"KTI ET32P2", 0},
@@ -145,7 +164,8 @@
 
 \f
 
-/* No room in the standard 8390 structure for extra info we need. */
+/* There is no room in the standard 8390 structure for extra info we need,
+   so we build a meta/outer-wrapper structure.. */
 struct ne2k_pci_card {
 	struct net_device *dev;
 	struct pci_dev *pci_dev;
@@ -171,33 +191,35 @@
 				     const struct pci_device_id *ent)
 {
 	struct net_device *dev;
-	int i, irq, reg0, start_page, stop_page;
+	int i;
 	unsigned char SA_prom[32];
-	int chip_idx = ent->driver_data;
-	static unsigned version_printed = 0;
+	int start_page, stop_page;
+	int irq, reg0, chip_idx = ent->driver_data;
+	static unsigned int fnd_cnt;
 	long ioaddr;
-	
-	if (version_printed++ == 0)
-		printk(KERN_INFO "%s", version);
+	int flags = pci_clone_list[chip_idx].flags;
+
+	if (fnd_cnt++ == 0)
+		printk(KERN_INFO "%s" KERN_INFO "%s", version1, version2);
 
 	ioaddr = pci_resource_start (pdev, 0);
 	irq = pdev->irq;
-	
+
 	if (!ioaddr || ((pci_resource_flags (pdev, 0) & IORESOURCE_IO) == 0)) {
 		printk (KERN_ERR "ne2k-pci: no I/O resource at PCI BAR #0\n");
 		return -ENODEV;
 	}
-	
+
 	i = pci_enable_device (pdev);
 	if (i)
 		return i;
-	
+
 	if (request_region (ioaddr, NE_IO_EXTENT, "ne2k-pci") == NULL) {
 		printk (KERN_ERR "ne2k-pci: I/O resource 0x%x @ 0x%lx busy\n",
 			NE_IO_EXTENT, ioaddr);
 		return -EBUSY;
 	}
-	
+
 	reg0 = inb(ioaddr);
 	if (reg0 == 0xFF)
 		goto err_out_free_res;
@@ -222,7 +244,7 @@
 		printk (KERN_ERR "ne2k-pci: cannot allocate ethernet device\n");
 		goto err_out_free_res;
 	}
-	
+
 	/* Reset card. Who knows what dain-bramaged state it was left in. */
 	{
 		unsigned long reset_start_time = jiffies;
@@ -238,7 +260,7 @@
 				printk("ne2k-pci: Card failure (no reset ack).\n");
 				goto err_out_free_netdev;
 			}
-		
+
 		outb(0xff, ioaddr + EN0_ISR);		/* Ack all intr. */
 	}
 
@@ -269,7 +291,7 @@
 
 	/* Note: all PCI cards have at least 16 bit access, so we don't have
 	   to check for 8 bit cards.  Most cards permit 32 bit access. */
-	if (pci_clone_list[chip_idx].flags & ONLY_32BIT_IO) {
+	if (flags & ONLY_32BIT_IO) {
 		for (i = 0; i < 4 ; i++)
 			((u32 *)SA_prom)[i] = le32_to_cpu(inl(ioaddr + NE_DATAPORT));
 	} else
@@ -280,8 +302,7 @@
 	outb(0x49, ioaddr + EN0_DCFG);
 	start_page = NESM_START_PG;
 
-	stop_page =
-		pci_clone_list[chip_idx].flags&STOP_PG_0x60 ? 0x60 : NESM_STOP_PG;
+	stop_page = flags & STOP_PG_0x60 ? 0x60 : NESM_STOP_PG;
 
 	/* Set up the rest of the parameters. */
 	dev->irq = irq;
@@ -305,7 +326,11 @@
 	ei_status.tx_start_page = start_page;
 	ei_status.stop_page = stop_page;
 	ei_status.word16 = 1;
-	ei_status.ne2k_flags = pci_clone_list[chip_idx].flags;
+	ei_status.ne2k_flags = flags;
+	if (fnd_cnt < MAX_UNITS) {
+		if (full_duplex[fnd_cnt] > 0  ||  (options[fnd_cnt] & FORCE_FDX))
+			ei_status.ne2k_flags |= FORCE_FDX;
+	}
 
 	ei_status.rx_start_page = start_page + TX_PAGES;
 #ifdef PACKETBUF_MEMSIZE
@@ -331,20 +356,27 @@
 
 }
 
-static int
-ne2k_pci_open(struct net_device *dev)
+static int ne2k_pci_open(struct net_device *dev)
 {
 	MOD_INC_USE_COUNT;
 	if (request_irq(dev->irq, ei_interrupt, SA_SHIRQ, dev->name, dev)) {
 		MOD_DEC_USE_COUNT;
 		return -EAGAIN;
 	}
+	/* Set full duplex for the chips that we know about. */
+	if (ei_status.ne2k_flags & FORCE_FDX) {
+		long ioaddr = dev->base_addr;
+		if (ei_status.ne2k_flags & REALTEK_FDX) {
+			outb(0xC0 + E8390_NODMA, ioaddr + NE_CMD); /* Page 3 */
+			outb(inb(ioaddr + 0x20) | 0x80, ioaddr + 0x20);
+		} else if (ei_status.ne2k_flags & HOLTEK_FDX)
+			outb(inb(ioaddr + 0x20) | 0x80, ioaddr + 0x20);
+	}
 	ei_open(dev);
 	return 0;
 }
 
-static int
-ne2k_pci_close(struct net_device *dev)
+static int ne2k_pci_close(struct net_device *dev)
 {
 	ei_close(dev);
 	free_irq(dev->irq, dev);
@@ -354,8 +386,7 @@
 
 /* Hard reset the card.  This used to pause for the same period that a
    8390 reset command required, but that shouldn't be necessary. */
-static void
-ne2k_pci_reset_8390(struct net_device *dev)
+static void ne2k_pci_reset_8390(struct net_device *dev)
 {
 	unsigned long reset_start_time = jiffies;
 
@@ -380,8 +411,7 @@
    we don't need to be concerned with ring wrap as the header will be at
    the start of a page, so we optimize accordingly. */
 
-static void
-ne2k_pci_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr, int ring_page)
+static void ne2k_pci_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr, int ring_page)
 {
 
 	long nic_base = dev->base_addr;
@@ -418,8 +448,8 @@
    The NEx000 doesn't share the on-board packet memory -- you have to put
    the packet out through the "remote DMA" dataport using outb. */
 
-static void
-ne2k_pci_block_input(struct net_device *dev, int count, struct sk_buff *skb, int ring_offset)
+static void ne2k_pci_block_input(struct net_device *dev, int count,
+				 struct sk_buff *skb, int ring_offset)
 {
 	long nic_base = dev->base_addr;
 	char *buf = skb->data;
@@ -461,9 +491,8 @@
 	ei_status.dmaing &= ~0x01;
 }
 
-static void
-ne2k_pci_block_output(struct net_device *dev, int count,
-		const unsigned char *buf, const int start_page)
+static void ne2k_pci_block_output(struct net_device *dev, int count,
+				  const unsigned char *buf, const int start_page)
 {
 	long nic_base = NE_BASE;
 	unsigned long dma_start;
@@ -520,8 +549,8 @@
 	dma_start = jiffies;
 
 	while ((inb(nic_base + EN0_ISR) & ENISR_RDC) == 0)
-		if (jiffies - dma_start > 2) { 			/* Avoid clock roll-over. */
-			printk("%s: timeout waiting for Tx RDC.\n", dev->name);
+		if (jiffies - dma_start > 2) {			/* Avoid clock roll-over. */
+			printk(KERN_WARNING "%s: timeout waiting for Tx RDC.\n", dev->name);
 			ne2k_pci_reset_8390(dev);
 			NS8390_init(dev,1);
 			break;
@@ -536,12 +565,10 @@
 static void __devexit ne2k_pci_remove_one (struct pci_dev *pdev)
 {
 	struct net_device *dev = pci_get_drvdata(pdev);
-	
-	if (!dev) {
-		printk (KERN_ERR "bug! ne2k_pci_remove_one called w/o net_device\n");
-		return;
-	}
-	
+
+	if (!dev)
+		BUG();
+
 	unregister_netdev(dev);
 	release_region(dev->base_addr, NE_IO_EXTENT);
 	kfree(dev);
@@ -560,18 +587,12 @@
 static int __init ne2k_pci_init(void)
 {
 	int rc;
-	
+
 	if (load_8390_module("ne2k-pci.c"))
 		return -ENOSYS;
-	
-	rc = pci_module_init (&ne2k_driver);
-	
-	/* XXX should this test CONFIG_HOTPLUG like pci_module_init? */
 
-	/* YYY No. If we're returning non-zero, we're being unloaded
-	 * 	   immediately. dwmw2 
-	 */
-	if (rc <= 0)
+	rc = pci_module_init (&ne2k_driver);
+	if (rc)
 		unload_8390_module();
 
 	return rc;
@@ -586,16 +607,3 @@
 
 module_init(ne2k_pci_init);
 module_exit(ne2k_pci_cleanup);
-
-\f
-/*
- * Local variables:
- *  compile-command: "gcc -DMODVERSIONS  -DMODULE -D__KERNEL__ -Wall -Wstrict-prototypes -O6 -fomit-frame-pointer -I/usr/src/linux/drivers/net/ -c ne2k-pci.c"
- *  alt-compile-command: "gcc -DMODULE -D__KERNEL__ -Wall -Wstrict-prototypes -O6 -fomit-frame-pointer -I/usr/src/linux/drivers/net/ -c ne2k-pci.c"
- *  c-indent-level: 4
- *  c-basic-offset: 4
- *  tab-width: 4
- *  version-control: t
- *  kept-new-versions: 5
- * End:
- */