Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Jonathan deBoer]

Jay Cliburn wrote:
> I've been working on this with Jay since his initial submission. 
> Thanks to everyone who has provided feedback on the resubmit. We're 
> currently quite short on actual testers, since the chip only seems to 
> be on Asus M2V motherboards at present. Please let me and Jay know if 
> you have one of these boards and would like to test and/or have 
> encountered bugs.
I purchased a Asus P5B-E today which also has this network card, and 
would be interested in testing driver changes.

Please email me directly, as I am not subscribed to the LKML yet.

Thanks.

-- 
Jonathan deBoer
email: jonathan<at>seltec<dot>ab<dot>ca

[PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Jay Cliburn]

Based upon feedback from Stephen Hemminger and Francois Romieu, please
consider this resubmitted patchset that provides support for the Attansic 
L1 gigabit ethernet adapter.  This patchset is built against 2.6.19-rc6.  
The original patchset was submitted 20060927.

The monolithic version of this patchset may be found at:
ftp://hogchain.net/pub/linux/m2v/attansic/kernel_driver/atl1-2.0.2/atl1-2.0.2-kernel2.6.19.rc6.patch.bz2

As a reminder, this driver a modified version of the Attansic reference 
driver for the L1 ethernet adapter.  Attansic has granted permission for 
its inclusion in the mainline kernel.

The patch contains:

 Kconfig             |   12 
 Makefile            |    1 
 atl1/Makefile       |   30 
 atl1/atl1.h         |  251 +++++
 atl1/atl1_ethtool.c |  530 ++++++++++
 atl1/atl1_hw.c      |  840 +++++++++++++++++
 atl1/atl1_hw.h      |  991 ++++++++++++++++++++
 atl1/atl1_main.c    | 2551 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 atl1/atl1_osdep.h   |   78 +
 atl1/atl1_param.c   |  203 ++++
 10 files changed, 5487 insertions(+)

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Alan]

Would be nice if it used atl_ not at_ so its less likely to cause
namespace clashes.

You have various macros for swaps that are pretty ugly - we have
cpu_to and le/be_to_cpu functions for most swapping cases and these are
generally optimised assembler (eg bswap on x86)

AT_DESC_USED/UNUSED would be better as inline functions but thats not a
serious concern.

Be careful with :1 bitfields when working with hardware - the compiler
has more than one choice about how to pack them.

The irq enable/disable use for locking on vlan appears unsafe. PCI
interrupt delivery is asynchronous which means you can get this happen


	card sends PCI interrupt
	We call irq_disable
	We take lock
	We poke bits
	We drop lock

	PCI interrupt arrives


This really does happen, typically its nasty to debug as well because you
usually only get it on PIII boards on the one in n-zillion times a
message collides and is retransmitted on the APIC bus.

skb->len is unsigned so <= 0 can be == 0. More importantly the subtraction
before the test will wrap and is completely unsafe (see at_xmit_frame)


Alan

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Chris Snook]

Alan wrote:
> Would be nice if it used atl_ not at_ so its less likely to cause
> namespace clashes.

Some of this code looked like Attansic may have meant to share it 
between drivers for atl1/atl2/atf1/atf2, but seeing as I can't find any 
code for those, I'll convert it all to atl1_ and let Attansic generalize 
the code if they ever decide they want to submit drivers.

> You have various macros for swaps that are pretty ugly - we have
> cpu_to and le/be_to_cpu functions for most swapping cases and these are
> generally optimised assembler (eg bswap on x86)
> 
> AT_DESC_USED/UNUSED would be better as inline functions but thats not a
> serious concern.
> 
> Be careful with :1 bitfields when working with hardware - the compiler
> has more than one choice about how to pack them.

Lacking a spec, I'm not entirely sure what the original intent was, so 
we're stuck with testing.  Is there a specific disambiguation technique 
you recommend?

> The irq enable/disable use for locking on vlan appears unsafe. PCI
> interrupt delivery is asynchronous which means you can get this happen
> 
> 
> 	card sends PCI interrupt
> 	We call irq_disable
> 	We take lock
> 	We poke bits
> 	We drop lock
> 
> 	PCI interrupt arrives
> 
> 
> This really does happen, typically its nasty to debug as well because you
> usually only get it on PIII boards on the one in n-zillion times a
> message collides and is retransmitted on the APIC bus.

Nice catch.  I admit the VLAN code is not so well audited or tested. 
Fortunately, the chip only seems to be on Asus M2V motherboards, at 
least for now, but I want to audit all of the locking code at some point.

> skb->len is unsigned so <= 0 can be == 0. More importantly the subtraction
> before the test will wrap and is completely unsafe (see at_xmit_frame)

Thanks!

	-- Chris

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Alan]

> > Be careful with :1 bitfields when working with hardware - the compiler
> > has more than one choice about how to pack them.
> 
> Lacking a spec, I'm not entirely sure what the original intent was, so 
> we're stuck with testing.  Is there a specific disambiguation technique 
> you recommend?

Assuming the driver works on x86 then you know how the bits are laid out.
You may find later you need to put in ifdefs for bitfield endian-ness as
we do in things like include/linux/ip.h

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Chris Snook]

Jay Cliburn wrote:
> Based upon feedback from Stephen Hemminger and Francois Romieu, please
> consider this resubmitted patchset that provides support for the Attansic 
> L1 gigabit ethernet adapter.  This patchset is built against 2.6.19-rc6.  
> The original patchset was submitted 20060927.
> 
> The monolithic version of this patchset may be found at:
> ftp://hogchain.net/pub/linux/m2v/attansic/kernel_driver/atl1-2.0.2/atl1-2.0.2-kernel2.6.19.rc6.patch.bz2
> 
> As a reminder, this driver a modified version of the Attansic reference 
> driver for the L1 ethernet adapter.  Attansic has granted permission for 
> its inclusion in the mainline kernel.
> 
> The patch contains:
> 
>  Kconfig             |   12 
>  Makefile            |    1 
>  atl1/Makefile       |   30 
>  atl1/atl1.h         |  251 +++++
>  atl1/atl1_ethtool.c |  530 ++++++++++
>  atl1/atl1_hw.c      |  840 +++++++++++++++++
>  atl1/atl1_hw.h      |  991 ++++++++++++++++++++
>  atl1/atl1_main.c    | 2551 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  atl1/atl1_osdep.h   |   78 +
>  atl1/atl1_param.c   |  203 ++++
>  10 files changed, 5487 insertions(+)
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

I've been working on this with Jay since his initial submission.  Thanks 
to everyone who has provided feedback on the resubmit.  We're currently 
quite short on actual testers, since the chip only seems to be on Asus 
M2V motherboards at present.  Please let me and Jay know if you have one 
of these boards and would like to test and/or have encountered bugs.

	-- Chris

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Luca Tettamanti]

Chris Snook <csnook@redhat.com> ha scritto:
> Jay Cliburn wrote:
>> Based upon feedback from Stephen Hemminger and Francois Romieu, please
>> consider this resubmitted patchset that provides support for the Attansic 
>> L1 gigabit ethernet adapter.  This patchset is built against 2.6.19-rc6.  
>> The original patchset was submitted 20060927.
[...]
> 
> I've been working on this with Jay since his initial submission.  Thanks 
> to everyone who has provided feedback on the resubmit.  We're currently 
> quite short on actual testers, since the chip only seems to be on Asus 
> M2V motherboards at present.  Please let me and Jay know if you have one 
> of these boards and would like to test and/or have encountered bugs.

Asus P5B-E also has L1 chip. I'll get the board in a few days and I'll
test whatever patch you throw at me ;)

Luca
-- 
Not an editor command: Wq

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Chris Ott]

Luca Tettamanti <kronos.it <at> gmail.com> writes:
> 
> Chris Snook <csnook <at> redhat.com> ha scritto:
> > 
> > We're currently 
> > quite short on actual testers, since the chip only seems to be on Asus 
> > M2V motherboards at present.  Please let me and Jay know if you have one 
> > of these boards and would like to test and/or have encountered bugs.
> 
> Asus P5B-E also has L1 chip. I'll get the board in a few days and I'll
> test whatever patch you throw at me ;)

ASUS P5L-VM 1394 has it, too. I would also be willing to test.

Chris

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[russell_i_brown]

Chris Ott wrote:
> 
> 
>> Chris Snook <csnook <at> redhat.com> ha scritto:
>> > 
>> > We're currently 
>> > quite short on actual testers, since the chip only seems to be on Asus 
>> > M2V motherboards at present.  Please let me and Jay know if you have
>> one 
>> > of these boards and would like to test and/or have encountered bugs.
> 
> Asus V3-P5945G also uses the Attansic L1 Gigabit Ethernet Adapter. I've
> tried the atl1 driver
> supplied by ASUS with the m/b (v0.1.40.8) but it's horribly slow (50KB/s
> on a 100Mb LAN!).
> 
> I'd also therefore be willing to test.
> 
> What would you like us to do Chris?
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
View this message in context: http://www.nabble.com/-PATCH-0-4--atl1%3A-Revised-Attansic-L1-ethernet-driver-tf2665327.html#a7511768
Sent from the linux-kernel mailing list archive at Nabble.com.

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Luca Tettamanti]

Hello,

Luca Tettamanti <kronos.it@gmail.com> ha scritto:
> Chris Snook <csnook@redhat.com> ha scritto:
>> 
>> I've been working on this with Jay since his initial submission.  Thanks 
>> to everyone who has provided feedback on the resubmit.  We're currently 
>> quite short on actual testers, since the chip only seems to be on Asus 
>> M2V motherboards at present.  Please let me and Jay know if you have one 
>> of these boards and would like to test and/or have encountered bugs.
> 
> Asus P5B-E also has L1 chip. I'll get the board in a few days and I'll
> test whatever patch you throw at me ;)

Got the board, done some basic testing: so far so good :)

The controller also supports MSI and (at least with my chipset - G965)
it works fine:

218:      80649          0   PCI-MSI-edge      eth1

which is nice, otherwise it ends up sharing the IRQ with SATA and USB.

I also have a small patch:

On probe failure and on removal the device should be disabled:

Signed-Off-By: Luca Tettamanti <kronos.it@gmail.com> 

---
 drivers/net/atl1/atl1_main.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/atl1/atl1_main.c b/drivers/net/atl1/atl1_main.c
index 167e28c..4883111 100644
--- a/drivers/net/atl1/atl1_main.c
+++ b/drivers/net/atl1/atl1_main.c
@@ -2224,14 +2224,14 @@ static int __devinit at_probe(struct pci
 			printk(KERN_DEBUG 
 				"%s: no usable DMA configuration, aborting\n",
  				at_driver_name);
-			return err;
+			goto err_dma;
 		}
 		pci_using_64 = false;
 	}
 	/* Mark all PCI regions associated with PCI device 
 	 * pdev as being reserved by owner at_driver_name */
 	if ((err = pci_request_regions(pdev, at_driver_name)))
-		return err;
+		goto err_request_regions;
 	/* Enables bus-mastering on the device and calls 
 	 * pcibios_set_master to do the needed arch specific settings */
 	pci_set_master(pdev);
@@ -2384,6 +2384,9 @@ static int __devinit at_probe(struct pci
 	free_netdev(netdev);
       err_alloc_etherdev:
 	pci_release_regions(pdev);
+      err_request_regions:
+      err_dma:
+	pci_disable_device(pdev);
 	return err;
 }
 
@@ -2410,6 +2413,7 @@ static void __devexit at_remove(struct p
 	iounmap(adapter->hw.hw_addr);
 	pci_release_regions(pdev);
 	free_netdev(netdev);
+	pci_disable_device(pdev);
 }
 
 static int at_suspend(struct pci_dev *pdev, pm_message_t state)


Luca
-- 
Not an editor command: Wq

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Jay Cliburn]

Luca Tettamanti wrote:

> Got the board, done some basic testing: so far so good :)
> 
> The controller also supports MSI and (at least with my chipset - G965)
> it works fine:
> 
> 218:      80649          0   PCI-MSI-edge      eth1
> 
> which is nice, otherwise it ends up sharing the IRQ with SATA and USB.
> 
> I also have a small patch:

Thanks for the patch.  We'll add it for the next version.

FYI, TSO performance is _really_ bad; your tx speed will drop dramatically with 
TSO on (and it's on by default).  I haven't yet been able to find the problem. 
If you want to improve tx performance, turn off TSO with ethtool.

Jay

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Vasco]

I've got the p5B-E board to with the onboard attansic l1.

I did the the atl1-2.0.2 patch against the 2.6.19.rc6 kernel.

I can confirm the card is working but performance is *really* bad, 200Kb/s
over Gbit LAN.
I tried copying a ~50MB file over SSH and it didn't complete because of a
connection stall.

I also tried turninig off TSO as adviced, but i got the message "operation
not supported". 

Im willing to do more testing, just tell me what I can do.

Greetings
Vasco


Jay Cliburn wrote:
> 
> Luca Tettamanti wrote:
> 
>> Got the board, done some basic testing: so far so good :)
>> 
>> The controller also supports MSI and (at least with my chipset - G965)
>> it works fine:
>> 
>> 218:      80649          0   PCI-MSI-edge      eth1
>> 
>> which is nice, otherwise it ends up sharing the IRQ with SATA and USB.
>> 
>> I also have a small patch:
> 
> Thanks for the patch.  We'll add it for the next version.
> 
> FYI, TSO performance is _really_ bad; your tx speed will drop dramatically
> with 
> TSO on (and it's on by default).  I haven't yet been able to find the
> problem. 
> If you want to improve tx performance, turn off TSO with ethtool.
> 
> Jay
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

-- 
View this message in context: http://www.nabble.com/-PATCH-0-4--atl1%3A-Revised-Attansic-L1-ethernet-driver-tf2665327.html#a7768447
Sent from the linux-kernel mailing list archive at Nabble.com.

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Vasco]

I've got the p5B-E board to with the onboard attansic l1.

I did the the atl1-2.0.2 patch against the 2.6.19.rc6 kernel.

I can confirm the card is working but performance is *really* bad, 200Kb/s
over Gbit LAN.
I tried copying a ~50MB file over SSH and it didn't complete because of a
connection stall.

I also tried turninig off TSO as adviced, but i got the message "operation
not supported". 

Im willing to do more testing, just tell me what I can do.

Greetings
Vasco


Jay Cliburn wrote:
> 
> Luca Tettamanti wrote:
> 
>> Got the board, done some basic testing: so far so good :)
>> 
>> The controller also supports MSI and (at least with my chipset - G965)
>> it works fine:
>> 
>> 218:      80649          0   PCI-MSI-edge      eth1
>> 
>> which is nice, otherwise it ends up sharing the IRQ with SATA and USB.
>> 
>> I also have a small patch:
> 
> Thanks for the patch.  We'll add it for the next version.
> 
> FYI, TSO performance is _really_ bad; your tx speed will drop dramatically
> with 
> TSO on (and it's on by default).  I haven't yet been able to find the
> problem. 
> If you want to improve tx performance, turn off TSO with ethtool.
> 
> Jay
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

-- 
View this message in context: http://www.nabble.com/-PATCH-0-4--atl1%3A-Revised-Attansic-L1-ethernet-driver-tf2665327.html#a7768448
Sent from the linux-kernel mailing list archive at Nabble.com.

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Luca Tettamanti]

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


Vasco <vvisser@science.uva.nl> ha scritto:
> 
> 
> I've got the p5B-E board to with the onboard attansic l1.
> 
> I did the the atl1-2.0.2 patch against the 2.6.19.rc6 kernel.

Hi,
there has been another iteration of the driver.

> I can confirm the card is working but performance is *really* bad, 200Kb/s
> over Gbit LAN.
> I tried copying a ~50MB file over SSH and it didn't complete because of a
> connection stall.
> 
> I also tried turninig off TSO as adviced, but i got the message "operation
> not supported". 

Hum, did you tried:

ethtool -K tso off eth0

Anyway the new version of the driver keeps TSO off by default, so it
should be fine.
I'm attaching what I'm using here (2.0.3 + my MSI patch).

Luca
-- 
Se alla sera, dopo una strepitosa vittoria, guardandoti allo
specchio dovessi notare un secondo paio di palle, che il tuo 
cuore non si riempia d'orgoglio, perche` vuol dire che ti 
stanno inculando -- Saggio Cinese

[-- Attachment #2: atl1.diff.bz2 --]
[-- Type: application/octet-stream, Size: 35063 bytes --]

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Vasco Visser]

Hi,

I've tested the new driver for a couple of days, mostly by running
XDMCP session over the Gbit LAN. Its all working good.

I didn't experience any crashes or slowdowns at all. The driver is
definitly usefull in it current state.

Performance is good, tops at ~40MB/s.

BTW: what is this MSI patch?

gr,
Vasco

Luca Tettamanti wrote:
> Vasco <vvisser@science.uva.nl> ha scritto:
>   
>> I've got the p5B-E board to with the onboard attansic l1.
>>
>> I did the the atl1-2.0.2 patch against the 2.6.19.rc6 kernel.
>>     
>
> Hi,
> there has been another iteration of the driver.
>
>   
>> I can confirm the card is working but performance is *really* bad, 200Kb/s
>> over Gbit LAN.
>> I tried copying a ~50MB file over SSH and it didn't complete because of a
>> connection stall.
>>
>> I also tried turninig off TSO as adviced, but i got the message "operation
>> not supported". 
>>     
>
> Hum, did you tried:
>
> ethtool -K tso off eth0
>
> Anyway the new version of the driver keeps TSO off by default, so it
> should be fine.
> I'm attaching what I'm using here (2.0.3 + my MSI patch).
>
> Luca
>   

Re: [PATCH 0/4] atl1: Revised Attansic L1 ethernet driver

[Luca Tettamanti]

(I'm CC-ing the other developer)

Il Thu, Dec 14, 2006 at 03:44:43AM +0000, Vasco Visser ha scritto: 
> Hi,
> 
> I've tested the new driver for a couple of days, mostly by running
> XDMCP session over the Gbit LAN. Its all working good.
> 
> I didn't experience any crashes or slowdowns at all. The driver is
> definitly usefull in it current state.

Good to hear.
Jay, I gave him driver version 2.0.3.

> 
> Performance is good, tops at ~40MB/s.
> 
> BTW: what is this MSI patch?

It enables PCI MSI, i.e. a method for delivering interrupts as regular
PCI transactions.
It's disabled by default since it may cause troubles on non-Intel
chipsets.

Luca
-- 
The trouble with computers is that they do what you tell them, 
not what you want.
D. Cohen