lkml.org 
[lkml]   [2005]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: <REAL> iBurst (TM) compatible driver
It is good to see more hardware supported, but like many drivers
you are adding a lot of racy ugliness just for debug crap.
Did you actually read any of the other network device drivers first
or try to write code from scratch off some book?

In order of severity.

1. Your network device registration code is wrong.
Read Documentation/netdevices.txt
You need to use alloc_netdev() and make the device specific data
be allocated there, not bury netdevice in a kmalloc'd structure.

2. Transmit routine is wrong. Drop packets if out of memory.
You can only return NETDEV_TX_OKAY or NETDEV_TX_BUSY
You probably don't need to allocate memory anyway if you use
hard header space properly.

3. You need to flow control the transmit queue. When it gets full
use netdev_stop_queue() and call netdev_wake_queue when you
have more space.

4. WTF is the whole ib_net_tap file stuff, and why do you need it?
You can eliminate a lot of module races etc, by just pulling it.

5. Why bother with a /proc interface?
6. If you must then use seq_file.
7. If you must then do one device per file.
8. Then you can get rid of having a global array of device structures
which is hard to maintain properly.
9. If you don't support ioctl's just leave dev->ioctl as NULL
10. Error code's from call's like register_netdev() should propogate back
out.
11. ib_net_open, ib_net_close only called from user context don't need irqsave
use spin_lock_irq()
12. Coding style: don't use u_long it's a BSDism
13. Please have source code laid out as patch to kernel, not out of tree driver

--
Stephen Hemminger <shemminger@osdl.org>
OSDL http://developer.osdl.org/~shemminger
-
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/

\
 
 \ /
  Last update: 2005-10-28 00:33    [W:0.063 / U:3.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site