netdev - Re: [PATCH 1/2] add driver for enc28j60 ethernet chip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 20 Dec 2007 12:47:16 +0100
From:	Claudio Lanconelli <lanconelli.claudio@...ar.com>
To:	Jeff Garzik <jgarzik@...ox.com>
CC:	netdev@...r.kernel.org,
	Stephen Hemminger <shemminger@...ux-foundation.org>
Subject: Re: [PATCH 1/2] add driver for enc28j60 ethernet chip

Here a second version of the patch with applied all changes Jeff and 
Stephen suggest me.
All but NAPI, see comments below at this regard.
Furthermore I fixed also a bug that caused a crash after ifconfig down 
and up cycle and changed the mutex_lock policy.

Jeff Garzik wrote:
>
> comments:
>
> * overall:  a clean driver that looks mostly acceptable, good work
Thanks
> * Why do interrupt work in a kernel thread?  Your comment says you 
> cannot, but does not explain.
The enc28j60 is a SPI to Ethernet adapter, so we cannot access register 
with simple in() out() instructions, but we need to use the SPI 
subsystem. The spi_sync() basic operation to read/write a register is a 
blocking operation, so can't be done in interrupt context.
Since every basic operation like read interrupt flag register call 
spi_sync() we need the work queue for almost everything.

>
> * should use NAPI
>
For the reason I just explained I don't think NAPI is a viable way for 
enc28j60.
Furthermore enc28j60 is a 10Mb only device and probably don't suffer to 
interrupt overload.
In my test there are a lot more interrupts from SPI than from enc28j60 
and the enc28j60 interrupts are less than RX+TX packets. See the dump below:

[root@...eotron /tmp]# ifconfig
eth0      Link encap:Ethernet  HWaddr 7E:C4:3B:C7:EA:F3
          inet addr:192.168.143.197  Bcast:192.168.143.255  
Mask:255.255.255.0
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:4972 errors:0 dropped:0 overruns:0 frame:0
          TX packets:2832 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:2009875 (1.9 MiB)  TX bytes:131922 (128.8 KiB)
          Interrupt:18

[root@...eotron /tmp]# cat /proc/interrupts
           CPU0
 18:       4316    s3c-ext0  enc28j60
 30:   11591615         s3c  S3C2410 Timer Tick
 32:          0         s3c  s162-lcd
 33:          0         s3c  s3c-mci
 37:         21         s3c  s3c-mci
 38:    2707866         s3c  s3c2410-spi
 42:          0         s3c  ohci_hcd:usb1
 43:         28         s3c  s3c2440-i2c
 45:          1         s3c  s3c2410-spi
 62:          1     s3c-ext  s3c-mci
 73:        342   s3c-uart1  s3c2440-uart
 74:       1225   s3c-uart1  s3c2440-uart
 79:          0     s3c-adc  s3c24xx_action
 80:          0     s3c-adc  s3c24xx_action
Err:          0

I think also USB-ethernet adapter have the same problem, and neither use 
NAPI

Cheers,
Claudio Lanconelli


View attachment "enc28j60.patch" of type "text/x-patch" (56017 bytes)