Re: drivers/net/chelsio/sge.c: two array overflows
Thanks Pete, This is correct, the array should contain 3 elements. The bug was we were accessing a 4th element ([3]) which did not exist. We should be modifying the last element ([2]) instead. -Scott Hans-Peter Jansen wrote: [from the nitpick department..] Hi Jeff, hi Scott, Adrian wrote: The Coverity checker spotted the following two array overflows in drivers/net/chelsio/sge.c (in both cases, the arrays contain 3 elements): Am Freitag, 17. März 2006 01:21 schrieb Jeff Garzik: Scott Bardone wrote: Adrian, This is a bug. The array should contain 2 elements. Attached is a patch which fixes it. Thanks. Signed-off-by: Scott Bardone [EMAIL PROTECTED] applied. please avoid attachments and use a proper patch description in the future. I had to hand-edit and hand-apply your patch. where you wrote in kernel tree commit 347a444e687b5f8cf0f6485704db1c6024d3: This is a bug. The array should contain 2 elements. Here is the fix. If I'm not completely off the track, you both committed a description off by one error: since the patch doesn't change the array size, it's presumely¹ still 3 elements, where index 2 references the last one. Here's hopefully a better patch description: Fixed off by one thinko in stats accounting, spotted by Coverity checker, notified by Adrian The Cleanman Bunk. SCR, Pete ¹) otherwise, it's still off by one.. - 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
Re: drivers/net/chelsio/sge.c: two array overflows
I had meant to say the array contains 3 elements but should only access index 2. Sorry for the confusion. -Scott Ingo Oeser wrote: Hi Scott, You wrote: This is a bug. The array should contain 2 elements. Sure? It either contains 3 elements or your patch is not fixing the issue. Please comment on the list which one is wrong :-) Remember that array indexes start with 0 in C: int ary[3]; ary[0] = 1; ary[1] = 2; ary[2] = 3; Regards Ingo Oeser - 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
Re: drivers/net/chelsio/sge.c: two array overflows
Adrian,
This is a bug. The array should contain 2 elements.
Attached is a patch which fixes it.
Thanks.
Signed-off-by: Scott Bardone [EMAIL PROTECTED]
Adrian Bunk wrote:
The Coverity checker spotted the following two array overflows in
drivers/net/chelsio/sge.c (in both cases, the arrays contain 3 elements):
-- snip --
...
static void restart_tx_queues(struct sge *sge)
{
...
sge-stats.cmdQ_restarted[3]++;
...
static int t1_sge_tx(struct sk_buff *skb, struct adapter *adapter,
unsigned int qid, struct net_device *dev)
{
...
sge-stats.cmdQ_full[3]++;
...
-- snip --
cu
Adrian
--- sge.c 2006-02-17 14:23:45.0 -0800
+++ sge.fix.c 2006-03-13 10:51:24.0 -0800
@@ -1021,7 +1021,7 @@
if (test_and_clear_bit(nd-if_port,
sge-stopped_tx_queues)
netif_running(nd)) {
-sge-stats.cmdQ_restarted[3]++;
+sge-stats.cmdQ_restarted[2]++;
netif_wake_queue(nd);
}
}
@@ -1350,7 +1350,7 @@
if (unlikely(credits count)) {
netif_stop_queue(dev);
set_bit(dev-if_port, sge-stopped_tx_queues);
- sge-stats.cmdQ_full[3]++;
+ sge-stats.cmdQ_full[2]++;
spin_unlock(q-lock);
if (!netif_queue_stopped(dev))
CH_ERR(%s: Tx ring full while queue awake!\n,
@@ -1358,7 +1358,7 @@
return NETDEV_TX_BUSY;
}
if (unlikely(credits - count q-stop_thres)) {
- sge-stats.cmdQ_full[3]++;
+ sge-stats.cmdQ_full[2]++;
netif_stop_queue(dev);
set_bit(dev-if_port, sge-stopped_tx_queues);
}
Re: [RFC: 2.6 patch] chelsio/espi.c:tricn_init(): remove dead code
This patch is correct, these two variables are unused in this driver. Thanks for
catching this!
Signed-off-by: Scott Bardone [EMAIL PROTECTED]
Adrian Bunk wrote:
The Coverity checker spotted these two unused variables.
Please check whether this patch is correct or whether they should be
used.
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
drivers/net/chelsio/espi.c | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
--- linux-2.6.16-rc5-mm3-full/drivers/net/chelsio/espi.c.old2006-03-09
23:19:54.0 +0100
+++ linux-2.6.16-rc5-mm3-full/drivers/net/chelsio/espi.c2006-03-09
23:20:35.0 +0100
@@ -87,15 +87,9 @@
static int tricn_init(adapter_t *adapter)
{
int i = 0;
- int sme = 1;
int stat= 0;
int timeout = 0;
int is_ready= 0;
- int dynamic_deskew = 0;
-
- if (dynamic_deskew)
- sme = 0;
-
/* 1 */
timeout=1000;
@@ -113,11 +107,9 @@
}
/* 2 */
- if (sme) {
- tricn_write(adapter, 0, 0, 0, TRICN_CNFG, 0x81);
- tricn_write(adapter, 0, 1, 0, TRICN_CNFG, 0x81);
- tricn_write(adapter, 0, 2, 0, TRICN_CNFG, 0x81);
- }
+ tricn_write(adapter, 0, 0, 0, TRICN_CNFG, 0x81);
+ tricn_write(adapter, 0, 1, 0, TRICN_CNFG, 0x81);
+ tricn_write(adapter, 0, 2, 0, TRICN_CNFG, 0x81);
for (i=1; i= 8; i++) tricn_write(adapter, 0, 0, i, TRICN_CNFG, 0xf1);
for (i=1; i= 2; i++) tricn_write(adapter, 0, 1, i, TRICN_CNFG, 0xf1);
for (i=1; i= 3; i++) tricn_write(adapter, 0, 2, i, TRICN_CNFG, 0xe1);
-
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
[PATCH] TCP Offload (TOE) - Chelsio
OPEN TOE submission from Chelsio Communications.
The following items have been addressed:
- cleaned up indentation.
- cleaned up comments.
- cleaned up c-styles.
- using EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL
- removed 2.4 compatibility.
- created TCP_OFFLOAD config option.
- moved #defines to appropriate files.
- removed obfuscating macros.
- included necessary definitions instead of struct.
- made IS_OFFLOADED an inline function instead of macro.
The following items are currently being worked on:
- use sysfs instead of procfs.
- addressing the use of semaphores in 'register_tom'.
- use RCU, need to look at this.
- use inline function instead of TOEDEV macro, requires some work.
Comments:
- static was removed from functions '__tcp_inherit_port' '__tcp_v4_hash'
because these are called outside of tcp_ipv4.c from the TOM driver.
Signed-off-by: Scott Bardone [EMAIL PROTECTED]
diff -Naur linux-2.6.13-rc6-git3/include/linux/netdevice.h
linux-2.6.13-rc6-git3.patched/include/linux/netdevice.h
--- linux-2.6.13-rc6-git3/include/linux/netdevice.h 2005-08-07
11:18:56.0 -0700
+++ linux-2.6.13-rc6-git3.patched/include/linux/netdevice.h 2005-08-11
21:28:36.0 -0700
@@ -408,6 +408,9 @@
#define NETIF_F_VLAN_CHALLENGED1024/* Device cannot handle VLAN
packets */
#define NETIF_F_TSO2048/* Can offload TCP/IP segmentation */
#define NETIF_F_LLTX 4096/* LockLess TX */
+#ifdef CONFIG_TCP_OFFLOAD
+#define NETIF_F_TCPIP_OFFLOAD 65536 /* Can offload TCP/IP */
+#endif
/* Called after device is detached from network. */
void(*uninit)(struct net_device *dev);
diff -Naur linux-2.6.13-rc6-git3/include/linux/tcp_diag.h
linux-2.6.13-rc6-git3.patched/include/linux/tcp_diag.h
--- linux-2.6.13-rc6-git3/include/linux/tcp_diag.h 2005-08-07
11:18:56.0 -0700
+++ linux-2.6.13-rc6-git3.patched/include/linux/tcp_diag.h 2005-08-11
21:28:36.0 -0700
@@ -4,6 +4,11 @@
/* Just some random number */
#define TCPDIAG_GETSOCK 18
+/* TOE API */
+#ifdef CONFIG_TCP_OFFLOAD
+#define TCPDIAG_OFFLOAD 5
+#endif
+
/* Socket identity */
struct tcpdiag_sockid
{
diff -Naur linux-2.6.13-rc6-git3/include/linux/tcp.h
linux-2.6.13-rc6-git3.patched/include/linux/tcp.h
--- linux-2.6.13-rc6-git3/include/linux/tcp.h 2005-08-07 11:18:56.0
-0700
+++ linux-2.6.13-rc6-git3.patched/include/linux/tcp.h 2005-08-11
21:28:36.0 -0700
@@ -235,6 +235,10 @@
return (struct tcp_request_sock *)req;
}
+#ifdef CONFIG_TCP_OFFLOAD
+struct toe_funcs;
+#endif
+
struct tcp_sock {
/* inet_sock has to be the first member of tcp_sock */
struct inet_sockinet;
@@ -342,6 +346,10 @@
struct tcp_func *af_specific; /* Operations which are
AF_INET{4,6} specific */
+#ifdef CONFIG_TCP_OFFLOAD
+ struct toe_funcs*toe_specific; /* Operations overriden by TOEs
*/
+#endif
+
__u32 rcv_wnd;/* Current receiver window */
__u32 rcv_wup;/* rcv_nxt on last window update sent */
__u32 write_seq; /* Tail(+1) of data held in tcp send buffer */
diff -Naur linux-2.6.13-rc6-git3/include/linux/toedev.h
linux-2.6.13-rc6-git3.patched/include/linux/toedev.h
--- linux-2.6.13-rc6-git3/include/linux/toedev.h1969-12-31
16:00:00.0 -0800
+++ linux-2.6.13-rc6-git3.patched/include/linux/toedev.h2005-08-11
22:37:03.94780 -0700
@@ -0,0 +1,126 @@
+/*
+ * *
+ * File: *
+ * toedev.h *
+ * *
+ * Description: *
+ * TOE device definitions. *
+ * *
+ * This program is free software; you can redistribute it and/or modify *
+ * it under the terms of the GNU General Public License, version 2, as *
+ * published by the Free Software Foundation.*
+ * *
+ * You should have received a copy of the GNU General Public License along *
+ * with this program; if not, write to the Free Software Foundation, Inc., *
+ * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *
+ * *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR IMPLIED*
+ * WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF *
+ * MERCHANTABILITY AND FITNESS
[ANNOUNCE] Chelsio 10Gb TOE (TCP Offload Engine)
Chelsio Communications would like to announce the availability of its TCP offload (TOE) support for Linux under the GPL. This is code developed by us over the past couple of years and has been in production for over a year. The code, architecture description, and some papers comparing TOE performance to other technologies are available from https://service.chelsio.com/open_toe/index.html. We are aware that TOEs are viewed with much skepticism in the Linux community but we believe that a lot of the concerns often brought up have to do with implementation details of particular products rather than with the technology as a whole. Chelsio is proposing a solution that we feel allows TOEs to coexist alongside the regular stack's TCP without breaking networking features, and allows the combined network stack to offer superior TCP performance. The code we are releasing today has been used with an older version of Linux to set the current Internet2 land speed record and has demonstrated improved performance with a variety of applications and benchmarks. As another example of performance benefits, while today's NICs cannot handle 10G receive with regular frames, a TOE can comfortably do so with much of the CPU left for application processing. The proposed design is intended to accommodate products from multiple vendors and roughly has the following components: - a vendor neutral cut-down analog of core/dev.c that provides registration and activation facilities for TOE devices and some basic data path functionality (mostly to deal with sniffers). This component does not introduce any new soft irqs, instead TOE devices use regular facilities, such as NAPI, to service incoming traffic; - some changes to existing TCP code and some additions to provide offloading. Changes to existing code are a few dozen lines and are usually either notification of TOEs when the SW stack processes certain events, e.g., ARP, or they allow TOEs to perform some socket operations differently from the SW stack (usually this is done by changing sk_prot, but some of the differing operations aren't covered by that and so need to be done through other changes); - the offloading support specific to each TOE is provided by two drivers, one that deals with HW and one that interfaces with the SW TCP/IP and sockets layer (these are separate conceptually, they may be one driver implementation-wise) More details of the proposed scheme and of the working of the various operations can be found in the architecture document at the above URL. We are including a patch containing the TCP changes below (against 2.6.12), and the rest of the vendor-neutral pieces will follow in subsequent emails. We are not posting the drivers on the list due to their size (the TOE driver though is an extension of Chelsio's NIC driver presently in Jeff's tree). All the code is available at the above URL. (We'd like to point out that the released code is our current production codebase that accommodates both 2.4 and 2.6 kernels. We are aware that we'll need to strip the compatibility stuff and plan to do so.) Thanks for your attention and we are looking forward to your comments. Chelsio Communications. - 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
