Great to see TOE support getting reviewed.
Some general stuff:
* Use linux indentation style (Documentation/Codingstyle)
"Tabs are 8 characters, and thus indentations are also 8
characters." You are using 4 spaces.
* Don't use // for comments
* All patches must have Signed-off-by: line. This is a legal
requirement see Documentation/SubmittingPatches.
* Why use name for binding, it is slow to lookup and can change.
Either use if_index or pointer. If you used driver model/sysfs
you wouldn't need to.
* Please make the toe devices part of sysfs.
Probably simplest to make toedev a kobject that is a child
of the netdevice.
* Can't you use sysfs instead of /proc? (see above)
* Please change all EXPORT_SYMBOL to EXPORT_SYMBOL_GPL to explicitly
impede binary drivers.
* Please don't include 2.4 compatiablity in the 2.6 version. This
is new code.
* Consider making TOE a config option, and stubbing as appropriate
many platforms may not want it.
diff -Naur linux-2.6.13-rc3/include/linux/toedev.h linux-2.6.13-
rc3.patched/include/linux/toedev.h --- linux-2.6.13-rc3/include/linux/
toedev.h 1969-12-31 16:00:00.000000000 -0800 +++ linux-2.6.13-
rc3.patched/include/linux/toedev.h 2005-07-26 12:23:53.796497048
-0700 @@ -0,0 +1,96 @@
...
+
+#ifndef _TOEDEV_H_
+#define _TOEDEV_H_
+
+#include <linux/list.h>
+#include <asm/atomic.h>
+
+#define TOENAMSIZ 16
See above comments about name
+/* belongs in linux/netdevice.h */
+#define NETIF_F_TCPIP_OFFLOAD (1 << 16)
If it belongs there, then put it there!
+/* Get the toedev associated with a net_device */
+#define TOEDEV(netdev) ((struct toedev *)(netdev)->ec_ptr)
Use inline instead to get type checking.
+/* TOE type ids */
+enum {
+ TOE_ID_CHELSIO_T1 = 1,
+ TOE_ID_CHELSIO_T1C,
+ TOE_ID_CHELSIO_T3,
+};
+
+struct toe_id {
+ unsigned int id;
+ unsigned long data;
+};
+
+#define END_OF_TOE_ID_TABLE { 0, 0UL }
Get rid of this macro. that's just obfuscation.
+
+struct net_device;
+struct neighbour;
+struct tom_info;
+struct proc_dir_entry;
+struct sock;
+struct sk_buff;
Include the necessary definitions.
+struct toedev {
+ char name[TOENAMSIZ]; /* TOE device name */
+ struct list_head toe_list; /* for list linking */
+ int toe_index; /* unique TOE device index */
+ unsigned int ttid; /* TOE type id */
+ unsigned long flags; /* device flags */
+ unsigned int mtu; /* max size of TX offloaded
data */
+ unsigned int nconn; /* max # of offloaded
connections */
+ struct net_device *lldev; /* LL device associated with TOE
messages */ ...
diff -Naur linux-2.6.13-rc3/include/net/offload.h linux-2.6.13-
rc3.patched/include/net/offload.h --- linux-2.6.13-rc3/include/net/
offload.h 1969-12-31 16:00:00.000000000 -0800 +++ linux-2.6.13-
rc3.patched/include/net/offload.h 2005-07-26 12:23:53.796497048
-0700 @@ -0,0 +1,47 @@ +
+/* Returns true if sk is an offloaded IPv4 TCP socket. */
+#define IS_OFFLOADED(sk) (((sk)->sk_family == AF_INET && (sk)-
>sk_prot != &tcp_prot))
inline?
+struct toedev;
+struct sk_buff;
+struct sock;
More redundant fwd declarations.
+/* Per-skb backlog handler. Run when a socket's backlog is processed.
*/ +struct blog_skb_cb {
+ void (*backlog_rcv)(struct sock *sk, struct sk_buff *skb);
+ struct toedev *dev;
+};
+
+#define BLOG_SKB_CB(skb) ((struct blog_skb_cb *)(skb)->cb)
+
+/* belongs in linux/tcp_diag.h */
+#define TCPDIAG_OFFLOAD 5
+
Please patch tcp_diag.h then.
diff -Naur linux-2.6.13-rc3/net/core/toedev.c linux-2.6.13-rc3.patched/
net/core/toedev.c --- linux-2.6.13-rc3/net/core/toedev.c
1969-12-31 16:00:00.000000000 -0800 +++ linux-2.6.13-rc3.patched/net/
core/toedev.c 2005-07-26 12:23:53.799496592 -0700 @@ -0,0 +1,432
@@ +
+/* 2.4 compatibility */
+#ifndef subsys_initcall
+#define subsys_initcall(fn) module_init(fn)
+
+static int boot_phase = 1;
+#else
+#define boot_phase 0
+#endif
No, just do 2.6
+
+/*
+ * Returns the entry in the TOE id table 'table' that has a given id,
or NULL
+ * if the id is not found.
+ */
+static const struct toe_id *id_find(unsigned int id,
+ const struct toe_id *table)
+{
+ const struct toe_id *p;
+
+ for (p = table; p->id; ++p)
+ if (p->id == id) return p;
please split line
Could you use RCU?
+
+/*
+ * Register a TCP Offload Module (TOM).
+ */
+int register_tom(struct tom_info *t)
+{
+ down(&toedev_db_lock);
+ list_add(&t->list_node, &tom_list);
+ up(&toedev_db_lock);
+ return 0;
+}
Using a semaphore seems more than you need here.
Given the code path you could either assume RT
netlink semaphore or just use a spinlock.
...
diff -Naur linux-2.6.13-rc3/net/ipv4/tcp_ipv4.c linux-2.6.13-
rc3.patched/net/ipv4/tcp_ipv4.c --- linux-2.6.13-rc3/net/ipv4/
tcp_ipv4.c 2005-07-26 10:16:10.000000000 -0700 +++ linux-2.6.13-
rc3.patched/net/ipv4/tcp_ipv4.c 2005-07-26 12:23:53.803495984
-0700 /* Caller must disable local BH processing. */
-static __inline__ void __tcp_inherit_port(struct sock *sk, struct sock
*child) +/* static */ __inline__ void __tcp_inherit_port(struct sock
*sk, struct sock *child) {
struct tcp_bind_hashbucket *head =
&tcp_bhash[tcp_bhashfn(inet_sk(child)-
>num)]; @@ -351,7 +352,7 @@
}
}
-static __inline__ void __tcp_v4_hash(struct sock *sk, const int
listen_possible) +/* static */ __inline__ void __tcp_v4_hash(struct
sock *sk, const int listen_possible) {
I don't understatnd why you made these no longer static?
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html