Netdev List
 help / color / mirror / Atom feed
* [PATCH 03/11] isdn/gigaset: ignore irrelevant device responses
From: Tilman Schmidt @ 2010-07-06  0:18 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20100705-patch-gigaset-00.tilman@imap.cc>

Downgrade the Gigaset driver's reaction to unknown AT responses from
the device from warning to debug level, and remove the handling of
some device responses which aren't relevant for the driver's
operation.

Impact: cleanup
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/ev-layer.c |   67 ++++++++------------------------------
 drivers/isdn/gigaset/gigaset.h  |    5 +--
 2 files changed, 16 insertions(+), 56 deletions(-)

diff --git a/drivers/isdn/gigaset/ev-layer.c b/drivers/isdn/gigaset/ev-layer.c
index 0d5984a..657757b 100644
--- a/drivers/isdn/gigaset/ev-layer.c
+++ b/drivers/isdn/gigaset/ev-layer.c
@@ -35,53 +35,40 @@
 #define RT_RING		2
 #define RT_NUMBER	3
 #define RT_STRING	4
-#define RT_HEX		5
 #define RT_ZCAU		6
 
 /* Possible ASCII responses */
 #define RSP_OK		0
-#define RSP_BUSY	1
-#define RSP_CONNECT	2
+#define RSP_ERROR	1
 #define RSP_ZGCI	3
 #define RSP_RING	4
-#define RSP_ZAOC	5
-#define RSP_ZCSTR	6
-#define RSP_ZCFGT	7
-#define RSP_ZCFG	8
-#define RSP_ZCCR	9
-#define RSP_EMPTY	10
-#define RSP_ZLOG	11
-#define RSP_ZCAU	12
-#define RSP_ZMWI	13
-#define RSP_ZABINFO	14
-#define RSP_ZSMLSTCHG	15
+#define RSP_ZVLS	5
+#define RSP_ZCAU	6
+
+/* responses with values to store in at_state */
+/* - numeric */
 #define RSP_VAR		100
 #define RSP_ZSAU	(RSP_VAR + VAR_ZSAU)
 #define RSP_ZDLE	(RSP_VAR + VAR_ZDLE)
-#define RSP_ZVLS	(RSP_VAR + VAR_ZVLS)
 #define RSP_ZCTP	(RSP_VAR + VAR_ZCTP)
+/* - string */
 #define RSP_STR		(RSP_VAR + VAR_NUM)
 #define RSP_NMBR	(RSP_STR + STR_NMBR)
 #define RSP_ZCPN	(RSP_STR + STR_ZCPN)
 #define RSP_ZCON	(RSP_STR + STR_ZCON)
 #define RSP_ZBC		(RSP_STR + STR_ZBC)
 #define RSP_ZHLC	(RSP_STR + STR_ZHLC)
-#define RSP_ERROR	-1	/* ERROR              */
+
 #define RSP_WRONG_CID	-2	/* unknown cid in cmd */
-#define RSP_UNKNOWN	-4	/* unknown response   */
-#define RSP_FAIL	-5	/* internal error     */
 #define RSP_INVAL	-6	/* invalid response   */
+#define RSP_NODEV	-9	/* device not connected */
 
 #define RSP_NONE	-19
 #define RSP_STRING	-20
 #define RSP_NULL	-21
-#define RSP_RETRYFAIL	-22
-#define RSP_RETRY	-23
-#define RSP_SKIP	-24
 #define RSP_INIT	-27
 #define RSP_ANY		-26
 #define RSP_LAST	-28
-#define RSP_NODEV	-9
 
 /* actions for process_response */
 #define ACT_NOTHING		0
@@ -259,13 +246,6 @@ struct reply_t gigaset_tab_nocid[] =
 
 /* misc. */
 {RSP_ERROR,	 -1,  -1, -1,			 -1, -1, {ACT_ERROR} },
-{RSP_ZCFGT,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-{RSP_ZCFG,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-{RSP_ZLOG,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-{RSP_ZMWI,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-{RSP_ZABINFO,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-{RSP_ZSMLSTCHG,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-
 {RSP_ZCAU,	 -1,  -1, -1,			 -1, -1, {ACT_ZCAU} },
 {RSP_NONE,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
 {RSP_ANY,	 -1,  -1, -1,			 -1, -1, {ACT_WARN} },
@@ -361,10 +341,6 @@ struct reply_t gigaset_tab_cid[] =
 
 /* misc. */
 {RSP_ZCON,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-{RSP_ZCCR,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-{RSP_ZAOC,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-{RSP_ZCSTR,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-
 {RSP_ZCAU,	 -1,  -1, -1,			 -1, -1, {ACT_ZCAU} },
 {RSP_NONE,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
 {RSP_ANY,	 -1,  -1, -1,			 -1, -1, {ACT_WARN} },
@@ -387,20 +363,11 @@ static const struct resp_type_t {
 	{"ZVLS",	RSP_ZVLS,	RT_NUMBER},
 	{"ZCTP",	RSP_ZCTP,	RT_NUMBER},
 	{"ZDLE",	RSP_ZDLE,	RT_NUMBER},
-	{"ZCFGT",	RSP_ZCFGT,	RT_NUMBER},
-	{"ZCCR",	RSP_ZCCR,	RT_NUMBER},
-	{"ZMWI",	RSP_ZMWI,	RT_NUMBER},
 	{"ZHLC",	RSP_ZHLC,	RT_STRING},
 	{"ZBC",		RSP_ZBC,	RT_STRING},
 	{"NMBR",	RSP_NMBR,	RT_STRING},
 	{"ZCPN",	RSP_ZCPN,	RT_STRING},
 	{"ZCON",	RSP_ZCON,	RT_STRING},
-	{"ZAOC",	RSP_ZAOC,	RT_STRING},
-	{"ZCSTR",	RSP_ZCSTR,	RT_STRING},
-	{"ZCFG",	RSP_ZCFG,	RT_HEX},
-	{"ZLOG",	RSP_ZLOG,	RT_NOTHING},
-	{"ZABINFO",	RSP_ZABINFO,	RT_NOTHING},
-	{"ZSMLSTCHG",	RSP_ZSMLSTCHG,	RT_NOTHING},
 	{NULL,		0,		0}
 };
 
@@ -588,10 +555,10 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 					break;
 
 			if (!rt->response) {
-				event->type = RSP_UNKNOWN;
-				dev_warn(cs->dev,
-					 "unknown modem response: %s\n",
-					 argv[curarg]);
+				event->type = RSP_NONE;
+				gig_dbg(DEBUG_EVENT,
+					"unknown modem response: '%s'\n",
+					argv[curarg]);
 				break;
 			}
 
@@ -655,14 +622,8 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 				curarg = params - 1;
 			break;
 		case RT_NUMBER:
-		case RT_HEX:
 			if (curarg < params) {
-				if (param_type == RT_HEX)
-					event->parameter =
-						isdn_gethex(argv[curarg]);
-				else
-					event->parameter =
-						isdn_getnum(argv[curarg]);
+				event->parameter = isdn_getnum(argv[curarg]);
 				++curarg;
 			} else
 				event->parameter = -1;
diff --git a/drivers/isdn/gigaset/gigaset.h b/drivers/isdn/gigaset/gigaset.h
index 1d0999b..add3d0c 100644
--- a/drivers/isdn/gigaset/gigaset.h
+++ b/drivers/isdn/gigaset/gigaset.h
@@ -193,9 +193,8 @@ void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
 /* variables in struct at_state_t */
 #define VAR_ZSAU	0
 #define VAR_ZDLE	1
-#define VAR_ZVLS	2
-#define VAR_ZCTP	3
-#define VAR_NUM		4
+#define VAR_ZCTP	2
+#define VAR_NUM		3
 
 #define STR_NMBR	0
 #define STR_ZCPN	1
-- 
1.6.5.3.298.g39add

^ permalink raw reply related

* [PATCH 02/11] isdn/gigaset: avoid copying AT commands twice
From: Tilman Schmidt @ 2010-07-06  0:18 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20100705-patch-gigaset-00.tilman@imap.cc>

Change the Gigaset driver's internal write_cmd interface to accept a
cmdbuf structure instead of a string. This avoids copying formatted
AT commands a second time.

Impact: optimization
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/bas-gigaset.c |   51 +++++-------------------
 drivers/isdn/gigaset/ev-layer.c    |   75 ++++++++++++++++-------------------
 drivers/isdn/gigaset/gigaset.h     |    4 +-
 drivers/isdn/gigaset/interface.c   |   36 ++++++++++++++---
 drivers/isdn/gigaset/ser-gigaset.c |   27 ++----------
 drivers/isdn/gigaset/usb-gigaset.c |   26 ++----------
 6 files changed, 85 insertions(+), 134 deletions(-)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index 47a5ffe..2dab531 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -1913,65 +1913,41 @@ static int start_cbsend(struct cardstate *cs)
  * USB transmission is started if necessary.
  * parameters:
  *	cs		controller state structure
- *	buf		command string to send
- *	len		number of bytes to send (max. IF_WRITEBUF)
- *	wake_tasklet	tasklet to run when transmission is completed
- *			(NULL if none)
+ *	cb		command buffer structure
  * return value:
  *	number of bytes queued on success
  *	error code < 0 on error
  */
-static int gigaset_write_cmd(struct cardstate *cs,
-			     const unsigned char *buf, int len,
-			     struct tasklet_struct *wake_tasklet)
+static int gigaset_write_cmd(struct cardstate *cs, struct cmdbuf_t *cb)
 {
-	struct cmdbuf_t *cb;
 	unsigned long flags;
 	int rc;
 
 	gigaset_dbg_buffer(cs->mstate != MS_LOCKED ?
 			     DEBUG_TRANSCMD : DEBUG_LOCKCMD,
-			   "CMD Transmit", len, buf);
-
-	if (len <= 0) {
-		/* nothing to do */
-		rc = 0;
-		goto notqueued;
-	}
+			   "CMD Transmit", cb->len, cb->buf);
 
 	/* translate "+++" escape sequence sent as a single separate command
 	 * into "close AT channel" command for error recovery
 	 * The next command will reopen the AT channel automatically.
 	 */
-	if (len == 3 && !memcmp(buf, "+++", 3)) {
+	if (cb->len == 3 && !memcmp(cb->buf, "+++", 3)) {
+		kfree(cb);
 		rc = req_submit(cs->bcs, HD_CLOSE_ATCHANNEL, 0, BAS_TIMEOUT);
-		goto notqueued;
-	}
-
-	if (len > IF_WRITEBUF)
-		len = IF_WRITEBUF;
-	cb = kmalloc(sizeof(struct cmdbuf_t) + len, GFP_ATOMIC);
-	if (!cb) {
-		dev_err(cs->dev, "%s: out of memory\n", __func__);
-		rc = -ENOMEM;
-		goto notqueued;
+		if (cb->wake_tasklet)
+			tasklet_schedule(cb->wake_tasklet);
+		return rc < 0 ? rc : cb->len;
 	}
 
-	memcpy(cb->buf, buf, len);
-	cb->len = len;
-	cb->offset = 0;
-	cb->next = NULL;
-	cb->wake_tasklet = wake_tasklet;
-
 	spin_lock_irqsave(&cs->cmdlock, flags);
 	cb->prev = cs->lastcmdbuf;
 	if (cs->lastcmdbuf)
 		cs->lastcmdbuf->next = cb;
 	else {
 		cs->cmdbuf = cb;
-		cs->curlen = len;
+		cs->curlen = cb->len;
 	}
-	cs->cmdbytes += len;
+	cs->cmdbytes += cb->len;
 	cs->lastcmdbuf = cb;
 	spin_unlock_irqrestore(&cs->cmdlock, flags);
 
@@ -1988,12 +1964,7 @@ static int gigaset_write_cmd(struct cardstate *cs,
 	}
 	rc = start_cbsend(cs);
 	spin_unlock_irqrestore(&cs->lock, flags);
-	return rc < 0 ? rc : len;
-
-notqueued:			/* request handled without queuing */
-	if (wake_tasklet)
-		tasklet_schedule(wake_tasklet);
-	return rc;
+	return rc < 0 ? rc : cb->len;
 }
 
 /* gigaset_write_room
diff --git a/drivers/isdn/gigaset/ev-layer.c b/drivers/isdn/gigaset/ev-layer.c
index ceaef9a..0d5984a 100644
--- a/drivers/isdn/gigaset/ev-layer.c
+++ b/drivers/isdn/gigaset/ev-layer.c
@@ -797,48 +797,27 @@ static void schedule_init(struct cardstate *cs, int state)
 static void send_command(struct cardstate *cs, const char *cmd, int cid,
 			 int dle, gfp_t kmallocflags)
 {
-	size_t cmdlen, buflen;
-	char *cmdpos, *cmdbuf, *cmdtail;
+	struct cmdbuf_t *cb;
+	size_t buflen;
 
-	cmdlen = strlen(cmd);
-	buflen = 11 + cmdlen;
-	if (unlikely(buflen <= cmdlen)) {
-		dev_err(cs->dev, "integer overflow in buflen\n");
+	buflen = strlen(cmd) + 12; /* DLE ( A T 1 2 3 4 5 <cmd> DLE ) \0 */
+	cb = kmalloc(sizeof(struct cmdbuf_t) + buflen, kmallocflags);
+	if (!cb) {
+		dev_err(cs->dev, "%s: out of memory\n", __func__);
 		return;
 	}
-
-	cmdbuf = kmalloc(buflen, kmallocflags);
-	if (unlikely(!cmdbuf)) {
-		dev_err(cs->dev, "out of memory\n");
-		return;
-	}
-
-	cmdpos = cmdbuf + 9;
-	cmdtail = cmdpos + cmdlen;
-	memcpy(cmdpos, cmd, cmdlen);
-
-	if (cid > 0 && cid <= 65535) {
-		do {
-			*--cmdpos = '0' + cid % 10;
-			cid /= 10;
-			++cmdlen;
-		} while (cid);
-	}
-
-	cmdlen += 2;
-	*--cmdpos = 'T';
-	*--cmdpos = 'A';
-
-	if (dle) {
-		cmdlen += 4;
-		*--cmdpos = '(';
-		*--cmdpos = 0x10;
-		*cmdtail++ = 0x10;
-		*cmdtail++ = ')';
-	}
-
-	cs->ops->write_cmd(cs, cmdpos, cmdlen, NULL);
-	kfree(cmdbuf);
+	if (cid > 0 && cid <= 65535)
+		cb->len = snprintf(cb->buf, buflen,
+				  dle ? "\020(AT%d%s\020)" : "AT%d%s",
+				  cid, cmd);
+	else
+		cb->len = snprintf(cb->buf, buflen,
+				  dle ? "\020(AT%s\020)" : "AT%s",
+				  cmd);
+	cb->offset = 0;
+	cb->next = NULL;
+	cb->wake_tasklet = NULL;
+	cs->ops->write_cmd(cs, cb);
 }
 
 static struct at_state_t *at_state_from_cid(struct cardstate *cs, int cid)
@@ -1240,8 +1219,22 @@ static void do_action(int action, struct cardstate *cs,
 		break;
 	case ACT_HUPMODEM:
 		/* send "+++" (hangup in unimodem mode) */
-		if (cs->connected)
-			cs->ops->write_cmd(cs, "+++", 3, NULL);
+		if (cs->connected) {
+			struct cmdbuf_t *cb;
+
+			cb = kmalloc(sizeof(struct cmdbuf_t) + 3, GFP_ATOMIC);
+			if (!cb) {
+				dev_err(cs->dev, "%s: out of memory\n",
+					__func__);
+				return;
+			}
+			memcpy(cb->buf, "+++", 3);
+			cb->len = 3;
+			cb->offset = 0;
+			cb->next = NULL;
+			cb->wake_tasklet = NULL;
+			cs->ops->write_cmd(cs, cb);
+		}
 		break;
 	case ACT_RING:
 		/* get fresh AT state structure for new CID */
diff --git a/drivers/isdn/gigaset/gigaset.h b/drivers/isdn/gigaset/gigaset.h
index 8738b08..904c947 100644
--- a/drivers/isdn/gigaset/gigaset.h
+++ b/drivers/isdn/gigaset/gigaset.h
@@ -574,9 +574,7 @@ struct bas_bc_state {
 struct gigaset_ops {
 	/* Called from ev-layer.c/interface.c for sending AT commands to the
 	   device */
-	int (*write_cmd)(struct cardstate *cs,
-			 const unsigned char *buf, int len,
-			 struct tasklet_struct *wake_tasklet);
+	int (*write_cmd)(struct cardstate *cs, struct cmdbuf_t *cb);
 
 	/* Called from interface.c for additional device control */
 	int (*write_room)(struct cardstate *cs);
diff --git a/drivers/isdn/gigaset/interface.c b/drivers/isdn/gigaset/interface.c
index c9f28dd..f45d686 100644
--- a/drivers/isdn/gigaset/interface.c
+++ b/drivers/isdn/gigaset/interface.c
@@ -339,7 +339,8 @@ static int if_tiocmset(struct tty_struct *tty, struct file *file,
 static int if_write(struct tty_struct *tty, const unsigned char *buf, int count)
 {
 	struct cardstate *cs;
-	int retval = -ENODEV;
+	struct cmdbuf_t *cb;
+	int retval;
 
 	cs = (struct cardstate *) tty->driver_data;
 	if (!cs) {
@@ -355,18 +356,39 @@ static int if_write(struct tty_struct *tty, const unsigned char *buf, int count)
 	if (!cs->connected) {
 		gig_dbg(DEBUG_IF, "not connected");
 		retval = -ENODEV;
-	} else if (!cs->open_count)
+		goto done;
+	}
+	if (!cs->open_count) {
 		dev_warn(cs->dev, "%s: device not opened\n", __func__);
-	else if (cs->mstate != MS_LOCKED) {
+		retval = -ENODEV;
+		goto done;
+	}
+	if (cs->mstate != MS_LOCKED) {
 		dev_warn(cs->dev, "can't write to unlocked device\n");
 		retval = -EBUSY;
-	} else {
-		retval = cs->ops->write_cmd(cs, buf, count,
-					    &cs->if_wake_tasklet);
+		goto done;
+	}
+	if (count <= 0) {
+		/* nothing to do */
+		retval = 0;
+		goto done;
 	}
 
-	mutex_unlock(&cs->mutex);
+	cb = kmalloc(sizeof(struct cmdbuf_t) + count, GFP_KERNEL);
+	if (!cb) {
+		dev_err(cs->dev, "%s: out of memory\n", __func__);
+		retval = -ENOMEM;
+		goto done;
+	}
 
+	memcpy(cb->buf, buf, count);
+	cb->len = count;
+	cb->offset = 0;
+	cb->next = NULL;
+	cb->wake_tasklet = &cs->if_wake_tasklet;
+	retval = cs->ops->write_cmd(cs, cb);
+done:
+	mutex_unlock(&cs->mutex);
 	return retval;
 }
 
diff --git a/drivers/isdn/gigaset/ser-gigaset.c b/drivers/isdn/gigaset/ser-gigaset.c
index e96c058..d151dcb 100644
--- a/drivers/isdn/gigaset/ser-gigaset.c
+++ b/drivers/isdn/gigaset/ser-gigaset.c
@@ -241,30 +241,13 @@ static void flush_send_queue(struct cardstate *cs)
  * return value:
  *	number of bytes queued, or error code < 0
  */
-static int gigaset_write_cmd(struct cardstate *cs, const unsigned char *buf,
-			     int len, struct tasklet_struct *wake_tasklet)
+static int gigaset_write_cmd(struct cardstate *cs, struct cmdbuf_t *cb)
 {
-	struct cmdbuf_t *cb;
 	unsigned long flags;
 
 	gigaset_dbg_buffer(cs->mstate != MS_LOCKED ?
 				DEBUG_TRANSCMD : DEBUG_LOCKCMD,
-			   "CMD Transmit", len, buf);
-
-	if (len <= 0)
-		return 0;
-
-	cb = kmalloc(sizeof(struct cmdbuf_t) + len, GFP_ATOMIC);
-	if (!cb) {
-		dev_err(cs->dev, "%s: out of memory!\n", __func__);
-		return -ENOMEM;
-	}
-
-	memcpy(cb->buf, buf, len);
-	cb->len = len;
-	cb->offset = 0;
-	cb->next = NULL;
-	cb->wake_tasklet = wake_tasklet;
+			   "CMD Transmit", cb->len, cb->buf);
 
 	spin_lock_irqsave(&cs->cmdlock, flags);
 	cb->prev = cs->lastcmdbuf;
@@ -272,9 +255,9 @@ static int gigaset_write_cmd(struct cardstate *cs, const unsigned char *buf,
 		cs->lastcmdbuf->next = cb;
 	else {
 		cs->cmdbuf = cb;
-		cs->curlen = len;
+		cs->curlen = cb->len;
 	}
-	cs->cmdbytes += len;
+	cs->cmdbytes += cb->len;
 	cs->lastcmdbuf = cb;
 	spin_unlock_irqrestore(&cs->cmdlock, flags);
 
@@ -282,7 +265,7 @@ static int gigaset_write_cmd(struct cardstate *cs, const unsigned char *buf,
 	if (cs->connected)
 		tasklet_schedule(&cs->write_tasklet);
 	spin_unlock_irqrestore(&cs->lock, flags);
-	return len;
+	return cb->len;
 }
 
 /*
diff --git a/drivers/isdn/gigaset/usb-gigaset.c b/drivers/isdn/gigaset/usb-gigaset.c
index f65cf7d..4a66338 100644
--- a/drivers/isdn/gigaset/usb-gigaset.c
+++ b/drivers/isdn/gigaset/usb-gigaset.c
@@ -494,29 +494,13 @@ static int send_cb(struct cardstate *cs, struct cmdbuf_t *cb)
 }
 
 /* Send command to device. */
-static int gigaset_write_cmd(struct cardstate *cs, const unsigned char *buf,
-			     int len, struct tasklet_struct *wake_tasklet)
+static int gigaset_write_cmd(struct cardstate *cs, struct cmdbuf_t *cb)
 {
-	struct cmdbuf_t *cb;
 	unsigned long flags;
 
 	gigaset_dbg_buffer(cs->mstate != MS_LOCKED ?
 			     DEBUG_TRANSCMD : DEBUG_LOCKCMD,
-			   "CMD Transmit", len, buf);
-
-	if (len <= 0)
-		return 0;
-	cb = kmalloc(sizeof(struct cmdbuf_t) + len, GFP_ATOMIC);
-	if (!cb) {
-		dev_err(cs->dev, "%s: out of memory\n", __func__);
-		return -ENOMEM;
-	}
-
-	memcpy(cb->buf, buf, len);
-	cb->len = len;
-	cb->offset = 0;
-	cb->next = NULL;
-	cb->wake_tasklet = wake_tasklet;
+			   "CMD Transmit", cb->len, cb->buf);
 
 	spin_lock_irqsave(&cs->cmdlock, flags);
 	cb->prev = cs->lastcmdbuf;
@@ -524,9 +508,9 @@ static int gigaset_write_cmd(struct cardstate *cs, const unsigned char *buf,
 		cs->lastcmdbuf->next = cb;
 	else {
 		cs->cmdbuf = cb;
-		cs->curlen = len;
+		cs->curlen = cb->len;
 	}
-	cs->cmdbytes += len;
+	cs->cmdbytes += cb->len;
 	cs->lastcmdbuf = cb;
 	spin_unlock_irqrestore(&cs->cmdlock, flags);
 
@@ -534,7 +518,7 @@ static int gigaset_write_cmd(struct cardstate *cs, const unsigned char *buf,
 	if (cs->connected)
 		tasklet_schedule(&cs->write_tasklet);
 	spin_unlock_irqrestore(&cs->lock, flags);
-	return len;
+	return cb->len;
 }
 
 static int gigaset_write_room(struct cardstate *cs)
-- 
1.6.5.3.298.g39add

^ permalink raw reply related

* [PATCH 0/2] ISDN patches for 2.6.36
From: Tilman Schmidt @ 2010-07-06  0:18 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

Karsten, David,

following are two patches to the ISDN subsystem I'd like to see
included in kernel release 2.6.36.

Thanks,
Tilman

Tilman Schmidt (2):
      isdn: CAPI controller interface documentation amendment
      isdn: avoid calling tty_ldisc_flush() in atomic context

 Documentation/isdn/INTERFACE.CAPI |    8 ++++++--
 drivers/isdn/i4l/isdn_tty.c       |    6 ------
 2 files changed, 6 insertions(+), 8 deletions(-)

^ permalink raw reply

* [PATCH 01/11] isdn/gigaset: adjust usb_gigaset tty write buffer limit
From: Tilman Schmidt @ 2010-07-06  0:18 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20100705-patch-gigaset-00.tilman@imap.cc>

The usb_gigaset driver's write buffer limit was different from those
of the others for no good reason. Set it to the same value, derived
from the Siemens documentation.

Impact: cosmetic
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/usb-gigaset.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/isdn/gigaset/usb-gigaset.c b/drivers/isdn/gigaset/usb-gigaset.c
index 76dbb20..f65cf7d 100644
--- a/drivers/isdn/gigaset/usb-gigaset.c
+++ b/drivers/isdn/gigaset/usb-gigaset.c
@@ -39,7 +39,8 @@ MODULE_PARM_DESC(cidmode, "Call-ID mode");
 #define GIGASET_MODULENAME "usb_gigaset"
 #define GIGASET_DEVNAME    "ttyGU"
 
-#define IF_WRITEBUF 2000	/* arbitrary limit */
+/* length limit according to Siemens 3070usb-protokoll.doc ch. 2.1 */
+#define IF_WRITEBUF 264
 
 /* Values for the Gigaset M105 Data */
 #define USB_M105_VENDOR_ID	0x0681
-- 
1.6.5.3.298.g39add

^ permalink raw reply related

* [PATCH 00/11] ISDN patches for 2.6.36
From: Tilman Schmidt @ 2010-07-06  0:18 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

Karsten, David,

following are a series of patches to the Gigaset driver I'd like
to see included in kernel release 2.6.36.

Thanks,
Tilman

Tilman Schmidt (11):
      isdn/gigaset: make tty write buffer limit consistent
      isdn/gigaset: avoid copying AT commands twice
      isdn/gigaset: ignore irrelevant device responses
      isdn/gigaset: drop debug check on isochronous write
      isdn/gigaset: improve CAPI message debugging
      isdn/gigaset: handle Supplementary Service Listen
      isdn/gigaset: remove obsolete compile time options
      isdn/gigaset: reduce syslog spam
      isdn/gigaset: fix leaks in error path
      isdn/gigaset: document dial-out number format
      isdn/gigaset: remove EXPERIMENTAL tag from GIGASET_CAPI option

 Documentation/isdn/README.gigaset  |  117 +++++++++++++++++------------
 drivers/isdn/gigaset/Kconfig       |    4 -
 drivers/isdn/gigaset/bas-gigaset.c |   69 ++---------------
 drivers/isdn/gigaset/capi.c        |   70 ++++++++++++-----
 drivers/isdn/gigaset/common.c      |    2 
 drivers/isdn/gigaset/ev-layer.c    |  148 ++++++++++++-------------------------
 drivers/isdn/gigaset/gigaset.h     |   16 ----
 drivers/isdn/gigaset/i4l.c         |    4 -
 drivers/isdn/gigaset/interface.c   |   37 +++++++--
 drivers/isdn/gigaset/ser-gigaset.c |   27 +-----
 drivers/isdn/gigaset/usb-gigaset.c |   29 +------
 11 files changed, 226 insertions(+), 297 deletions(-)

^ permalink raw reply

* [PATCH 2/2] isdn: avoid calling tty_ldisc_flush() in atomic context
From: Tilman Schmidt @ 2010-07-06  0:18 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel,
	stable
In-Reply-To: <20100705-patch-isdn-00.tilman@imap.cc>

Remove the call to tty_ldisc_flush() from the RESULT_NO_CARRIER
branch of isdn_tty_modem_result(), as already proposed in commit
00409bb045887ec5e7b9e351bc080c38ab6bfd33.
This avoids a "sleeping function called from invalid context" BUG
when the hardware driver calls the statcallb() callback with
command==ISDN_STAT_DHUP in atomic context, which in turn calls
isdn_tty_modem_result(RESULT_NO_CARRIER, ~), and from there,
tty_ldisc_flush() which may sleep.

Impact: bugfix
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/i4l/isdn_tty.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index fc8454d..51dc60d 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -2636,12 +2636,6 @@ isdn_tty_modem_result(int code, modem_info * info)
 		if ((info->flags & ISDN_ASYNC_CLOSING) || (!info->tty)) {
 			return;
 		}
-#ifdef CONFIG_ISDN_AUDIO
-		if ( !info->vonline )
-			tty_ldisc_flush(info->tty);
-#else
-		tty_ldisc_flush(info->tty);
-#endif
 		if ((info->flags & ISDN_ASYNC_CHECK_CD) &&
 		    (!((info->flags & ISDN_ASYNC_CALLOUT_ACTIVE) &&
 		       (info->flags & ISDN_ASYNC_CALLOUT_NOHUP)))) {
-- 
1.6.5.3.298.g39add

^ permalink raw reply related

* [PATCH 1/2] Documentation/isdn: CAPI controller interface amendment
From: Tilman Schmidt @ 2010-07-06  0:18 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20100705-patch-isdn-00.tilman@imap.cc>

Mention that the CAPI controller methods load_firmware() and
reset_ctr() are asynchronous, and should signal completion.

Impact: documentation
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 Documentation/isdn/INTERFACE.CAPI |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/isdn/INTERFACE.CAPI b/Documentation/isdn/INTERFACE.CAPI
index f172091..309eb5e 100644
--- a/Documentation/isdn/INTERFACE.CAPI
+++ b/Documentation/isdn/INTERFACE.CAPI
@@ -113,12 +113,16 @@ char *driver_name
 int (*load_firmware)(struct capi_ctr *ctrlr, capiloaddata *ldata)
 	(optional) pointer to a callback function for sending firmware and
 	configuration data to the device
+	The function may return before the operation has completed.
+	Completion must be signalled by a call to capi_ctr_ready().
 	Return value: 0 on success, error code on error
 	Called in process context.
 
 void (*reset_ctr)(struct capi_ctr *ctrlr)
-	(optional) pointer to a callback function for performing a reset on
-	the device, releasing all registered applications
+	(optional) pointer to a callback function for stopping the device,
+	releasing all registered applications
+	The function may return before the operation has completed.
+	Completion must be signalled by a call to capi_ctr_down().
 	Called in process context.
 
 void (*register_appl)(struct capi_ctr *ctrlr, u16 applid,
-- 
1.6.5.3.298.g39add

^ permalink raw reply related

* [net-next PATCH 1/6] qlge: Restore promiscuous setting after reset.
From: Ron Mercer @ 2010-07-05 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1278368382-23887-1-git-send-email-ron.mercer@qlogic.com>

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h      |    1 +
 drivers/net/qlge/qlge_main.c |    7 ++++++-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index bfb8b32..01b0634 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -2246,6 +2246,7 @@ netdev_tx_t ql_lb_send(struct sk_buff *skb, struct net_device *ndev);
 void ql_check_lb_frame(struct ql_adapter *, struct sk_buff *);
 int ql_own_firmware(struct ql_adapter *qdev);
 int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget);
+void qlge_set_multicast_list(struct net_device *ndev);
 
 #if 1
 #define QL_ALL_DUMP
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index fa4b24c..e99c2c6 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -3919,6 +3919,11 @@ static int ql_adapter_up(struct ql_adapter *qdev)
 	if ((ql_read32(qdev, STS) & qdev->port_init) &&
 			(ql_read32(qdev, STS) & qdev->port_link_up))
 		ql_link_on(qdev);
+	/* Restore rx mode. */
+	clear_bit(QL_ALLMULTI, &qdev->flags);
+	clear_bit(QL_PROMISCUOUS, &qdev->flags);
+	qlge_set_multicast_list(qdev->ndev);
+
 	ql_enable_interrupts(qdev);
 	ql_enable_all_completion_interrupts(qdev);
 	netif_tx_start_all_queues(qdev->ndev);
@@ -4204,7 +4209,7 @@ static struct net_device_stats *qlge_get_stats(struct net_device
 	return &ndev->stats;
 }
 
-static void qlge_set_multicast_list(struct net_device *ndev)
+void qlge_set_multicast_list(struct net_device *ndev)
 {
 	struct ql_adapter *qdev = (struct ql_adapter *)netdev_priv(ndev);
 	struct netdev_hw_addr *ha;
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 2/6] qlge: Don't use firmware when forcing firmware dump.
From: Ron Mercer @ 2010-07-05 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1278368382-23887-1-git-send-email-ron.mercer@qlogic.com>

In some cases the firmware may be dead.  Instead we dump the firmware
parameters and then restart it.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h     |    1 -
 drivers/net/qlge/qlge_dbg.c |    7 +------
 drivers/net/qlge/qlge_mpi.c |   17 -----------------
 3 files changed, 1 insertions(+), 24 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 01b0634..27f83d6 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -2227,7 +2227,6 @@ int ql_dump_risc_ram_area(struct ql_adapter *qdev, void *buf,
 		u32 ram_addr, int word_count);
 int ql_core_dump(struct ql_adapter *qdev,
 		struct ql_mpi_coredump *mpi_coredump);
-int ql_mb_sys_err(struct ql_adapter *qdev);
 int ql_mb_about_fw(struct ql_adapter *qdev);
 int ql_wol(struct ql_adapter *qdev);
 int ql_mb_wol_set_magic(struct ql_adapter *qdev, u32 enable_wol);
diff --git a/drivers/net/qlge/qlge_dbg.c b/drivers/net/qlge/qlge_dbg.c
index 68a1c9b..548e901 100644
--- a/drivers/net/qlge/qlge_dbg.c
+++ b/drivers/net/qlge/qlge_dbg.c
@@ -1237,12 +1237,7 @@ static void ql_get_core_dump(struct ql_adapter *qdev)
 			  "Force Coredump can only be done from interface that is up.\n");
 		return;
 	}
-
-	if (ql_mb_sys_err(qdev)) {
-		netif_err(qdev, ifup, qdev->ndev,
-			  "Fail force coredump with ql_mb_sys_err().\n");
-		return;
-	}
+	ql_queue_fw_error(qdev);
 }
 
 void ql_gen_reg_dump(struct ql_adapter *qdev,
diff --git a/drivers/net/qlge/qlge_mpi.c b/drivers/net/qlge/qlge_mpi.c
index 3c00462..f84e857 100644
--- a/drivers/net/qlge/qlge_mpi.c
+++ b/drivers/net/qlge/qlge_mpi.c
@@ -606,23 +606,6 @@ end:
 	return status;
 }
 
-int ql_mb_sys_err(struct ql_adapter *qdev)
-{
-	struct mbox_params mbc;
-	struct mbox_params *mbcp = &mbc;
-	int status;
-
-	memset(mbcp, 0, sizeof(struct mbox_params));
-
-	mbcp->in_count = 1;
-	mbcp->out_count = 0;
-
-	mbcp->mbox_in[0] = MB_CMD_MAKE_SYS_ERR;
-
-	status = ql_mailbox_command(qdev, mbcp);
-	return status;
-}
-
 /* Get MPI firmware version. This will be used for
  * driver banner and for ethtool info.
  * Returns zero on success.
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 5/6] qlge: Make adapter drop frame errors and pass up csum errors.
From: Ron Mercer @ 2010-07-05 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1278368382-23887-1-git-send-email-ron.mercer@qlogic.com>

Statistics are available and the driver doesn't need the actual frame.
This TCP/UDP and IP headers checksum errors will still be passed to the
driver.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |   30 ++++++++++++++++++++++++++++--
 1 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index a41b6b5..dd9e86c 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -574,6 +574,22 @@ static int ql_set_routing_reg(struct ql_adapter *qdev, u32 index, u32 mask,
 			    (RT_IDX_ALL_ERR_SLOT << RT_IDX_IDX_SHIFT);/* index */
 			break;
 		}
+	case RT_IDX_IP_CSUM_ERR: /* Pass up IP CSUM error frames. */
+		{
+			value = RT_IDX_DST_DFLT_Q | /* dest */
+				RT_IDX_TYPE_NICQ | /* type */
+				(RT_IDX_IP_CSUM_ERR_SLOT <<
+				RT_IDX_IDX_SHIFT); /* index */
+			break;
+		}
+	case RT_IDX_TU_CSUM_ERR: /* Pass up TCP/UDP CSUM error frames. */
+		{
+			value = RT_IDX_DST_DFLT_Q | /* dest */
+				RT_IDX_TYPE_NICQ | /* type */
+				(RT_IDX_TCP_UDP_CSUM_ERR_SLOT <<
+				RT_IDX_IDX_SHIFT); /* index */
+			break;
+		}
 	case RT_IDX_BCAST:	/* Pass up Broadcast frames to default Q. */
 		{
 			value = RT_IDX_DST_DFLT_Q |	/* dest */
@@ -3587,10 +3603,20 @@ static int ql_route_initialize(struct ql_adapter *qdev)
 	if (status)
 		return status;
 
-	status = ql_set_routing_reg(qdev, RT_IDX_ALL_ERR_SLOT, RT_IDX_ERR, 1);
+	status = ql_set_routing_reg(qdev, RT_IDX_IP_CSUM_ERR_SLOT,
+						RT_IDX_IP_CSUM_ERR, 1);
+	if (status) {
+		netif_err(qdev, ifup, qdev->ndev,
+			"Failed to init routing register "
+			"for IP CSUM error packets.\n");
+		goto exit;
+	}
+	status = ql_set_routing_reg(qdev, RT_IDX_TCP_UDP_CSUM_ERR_SLOT,
+						RT_IDX_TU_CSUM_ERR, 1);
 	if (status) {
 		netif_err(qdev, ifup, qdev->ndev,
-			  "Failed to init routing register for error packets.\n");
+			"Failed to init routing register "
+			"for TCP/UDP CSUM error packets.\n");
 		goto exit;
 	}
 	status = ql_set_routing_reg(qdev, RT_IDX_BCAST_SLOT, RT_IDX_BCAST, 1);
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 3/6] qlge: Reduce print level in data path statements.
From: Ron Mercer @ 2010-07-05 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1278368382-23887-1-git-send-email-ron.mercer@qlogic.com>

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index e99c2c6..e39451b 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -1521,7 +1521,7 @@ static void ql_process_mac_rx_page(struct ql_adapter *qdev,
 
 	/* Frame error, so drop the packet. */
 	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) {
-		netif_err(qdev, drv, qdev->ndev,
+		netif_info(qdev, drv, qdev->ndev,
 			  "Receive error, flags2 = 0x%x\n", ib_mac_rsp->flags2);
 		rx_ring->rx_errors++;
 		goto err_out;
@@ -1618,7 +1618,7 @@ static void ql_process_mac_rx_skb(struct ql_adapter *qdev,
 
 	/* Frame error, so drop the packet. */
 	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) {
-		netif_err(qdev, drv, qdev->ndev,
+		netif_info(qdev, drv, qdev->ndev,
 			  "Receive error, flags2 = 0x%x\n", ib_mac_rsp->flags2);
 		dev_kfree_skb_any(skb);
 		rx_ring->rx_errors++;
@@ -1939,7 +1939,7 @@ static void ql_process_mac_split_rx_intr(struct ql_adapter *qdev,
 
 	/* Frame error, so drop the packet. */
 	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) {
-		netif_err(qdev, drv, qdev->ndev,
+		netif_info(qdev, drv, qdev->ndev,
 			  "Receive error, flags2 = 0x%x\n", ib_mac_rsp->flags2);
 		dev_kfree_skb_any(skb);
 		rx_ring->rx_errors++;
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 6/6] qlge: Change version to v1.00.00.25.00.00-01.
From: Ron Mercer @ 2010-07-05 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1278368382-23887-1-git-send-email-ron.mercer@qlogic.com>


Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 27f83d6..06b2188 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -16,7 +16,7 @@
  */
 #define DRV_NAME  	"qlge"
 #define DRV_STRING 	"QLogic 10 Gigabit PCI-E Ethernet Driver "
-#define DRV_VERSION	"v1.00.00.23.00.00-01"
+#define DRV_VERSION	"v1.00.00.25.00.00-01"
 
 #define PFX "qlge: "
 
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 4/6] qlge: Fix possible endian issue for rx UDP csum.
From: Ron Mercer @ 2010-07-05 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1278368382-23887-1-git-send-email-ron.mercer@qlogic.com>

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index e39451b..a41b6b5 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -1677,7 +1677,7 @@ static void ql_process_mac_rx_skb(struct ql_adapter *qdev,
 			/* Unfragmented ipv4 UDP frame. */
 			struct iphdr *iph = (struct iphdr *) skb->data;
 			if (!(iph->frag_off &
-				cpu_to_be16(IP_MF|IP_OFFSET))) {
+				ntohs(IP_MF|IP_OFFSET))) {
 				skb->ip_summed = CHECKSUM_UNNECESSARY;
 				netif_printk(qdev, rx_status, KERN_DEBUG,
 					     qdev->ndev,
@@ -1997,7 +1997,7 @@ static void ql_process_mac_split_rx_intr(struct ql_adapter *qdev,
 		/* Unfragmented ipv4 UDP frame. */
 			struct iphdr *iph = (struct iphdr *) skb->data;
 			if (!(iph->frag_off &
-				cpu_to_be16(IP_MF|IP_OFFSET))) {
+				ntohs(IP_MF|IP_OFFSET))) {
 				skb->ip_summed = CHECKSUM_UNNECESSARY;
 				netif_printk(qdev, rx_status, KERN_DEBUG, qdev->ndev,
 					     "TCP checksum done!\n");
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 0/6] qlge: fixes for qlge.
From: Ron Mercer @ 2010-07-05 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer


1) Restore promiscuous setting after reset.
2) Don't use firmware when forcing firmware dump.
3) Reduce print level in data path statements.
4) Fix possible endian issue for rx UDP csum.
5) Make adapter drop frame errors and pass up csum errors.
6) Change version to v1.00.00.25.00.00-01.


^ permalink raw reply

* Re: bnx2/5709: Strange interrupts spread
From: Rick Jones @ 2010-07-05 22:56 UTC (permalink / raw)
  To: Christophe Ngo Van Duc; +Cc: netdev@vger.kernel.org
In-Reply-To: <AANLkTillLjztYe7mmyO_LoTXavDtTd2nsaAsd22z0dG-@mail.gmail.com>

Christophe Ngo Van Duc wrote:
> Is it a requirement that the interface has an IP address for the TX
> and RX hash to work?

At the risk of typing into Michael's keyboard, chances are, what the NIC does is 
follow Microsoft's specs for RSS, which as far as I can tell, discusses hashing 
either just on the IP addresses (v4 or v6) or the IP addresses and TCP port 
numbers, so unless Broadcom has an enhancement/extension, if the traffic 
arriving is not TCP/IP and not multiple connections, it probably does not get 
spread-out.

rick jones

^ permalink raw reply

* Re: [linux-pm] [PATCH 3/3] pm_qos: get rid of the allocation in pm_qos_add_request()
From: Rafael J. Wysocki @ 2010-07-05 21:07 UTC (permalink / raw)
  To: linux-pm; +Cc: James Bottomley, Takashi Iwai, netdev, markgross
In-Reply-To: <1278338568.2850.1.camel@mulgrave.site>

On Monday, July 05, 2010, James Bottomley wrote:
> On Mon, 2010-07-05 at 08:41 +0200, Takashi Iwai wrote:
> > sorry for the late reply, as I've been on vacation in the last week
> > (and shut off mails intentionally :)
> 
> Envy forbids me from saying that's OK.
> 
> > At Mon, 28 Jun 2010 12:44:48 -0500,
> > James Bottomley wrote:
> > > 
> > > Since every caller has to squirrel away the returned pointer anyway,
> > > they might as well supply the memory area.  This fixes a bug in a few of
> > > the call sites where the returned pointer was dereferenced without
> > > checking it for NULL (which gets returned if the kzalloc failed).
> > > 
> > > I'd like to hear how sound and netdev feels about this: it will add
> > > about two more pointers worth of data to struct netdev and struct
> > > snd_pcm_substream .. but I think it's worth it.  If you're OK, I'll add
> > > your acks and send through the pm tree.
> > > 
> > > This also looks to me like an android independent clean up (even though
> > > it renders the request_add atomically callable).  I also added include
> > > guards to include/linux/pm_qos_params.h
> > 
> > I like the patch very well, too.
> > But, just wondering...
> > 
> > > @@ -262,6 +260,11 @@ void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
> > >  	if (!pm_qos_req) /*guard against callers passing in null */
> > >  		return;
> > >  
> > > +	if (pm_qos_request_active(pm_qos_req)) {
> > > +		WARN(1, KERN_ERR "pm_qos_update_request() called for unknown object\n");
> > > +		return;
> > > +	}
> > > +
> > 
> > Is this correct...?  Shouldn't it be a negative check?
> 
> Yes, it should be a negative check ... I'll update the patch.

I've already fixed it in my tree.

> I guess this still means that no-one has managed to test it on a functional
> system ...

Well, it's been for a while in linux-next ...

Rafael

^ permalink raw reply

* Re: Fwd: Possible bug in net/ipv4/route.c?
From: Eric Dumazet @ 2010-07-05 20:18 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Herbert Xu, yoshfuji, netdev, linux-kernel, Stephen Hemminger
In-Reply-To: <20100705200728.GB11096@khazad-dum.debian.net>

Le lundi 05 juillet 2010 à 17:07 -0300, Henrique de Moraes Holschuh a
écrit :
> On Mon, 05 Jul 2010, Eric Dumazet wrote:
> > Le lundi 05 juillet 2010 à 21:22 +0800, Herbert Xu a écrit :
> > > On Mon, Jul 05, 2010 at 02:59:14PM +0200, Eric Dumazet wrote:
> > > >
> > > > Why do we clear full 48 bytes skb->cb[] in skb_alloc(), if no protocol
> > > > stack should rely it being zero ?
> > > 
> > > Unless a protocol is allocating the skb itself, then the fact
> > > that skb_alloc clears skb->cb is no guarantee that the skb->cb
> > > will be zero.
> > 
> > I see. We could :
> > 
> > Avoid this memset(skb->cb, 0, sizeof(skb->cb)) in fastpath.
> 
> Any chances of skb->cb being leaked to userspace or the network, due to
> driver bugs or other such oddities?
> 

Not "a priori", but a bug is always possible ;)

cb[] is internal use only, should not be sent to network or user land.




^ permalink raw reply

* Re: Fwd: Possible bug in net/ipv4/route.c?
From: Henrique de Moraes Holschuh @ 2010-07-05 20:07 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Herbert Xu, yoshfuji, netdev, linux-kernel, Stephen Hemminger
In-Reply-To: <1278336898.2877.212.camel@edumazet-laptop>

On Mon, 05 Jul 2010, Eric Dumazet wrote:
> Le lundi 05 juillet 2010 à 21:22 +0800, Herbert Xu a écrit :
> > On Mon, Jul 05, 2010 at 02:59:14PM +0200, Eric Dumazet wrote:
> > >
> > > Why do we clear full 48 bytes skb->cb[] in skb_alloc(), if no protocol
> > > stack should rely it being zero ?
> > 
> > Unless a protocol is allocating the skb itself, then the fact
> > that skb_alloc clears skb->cb is no guarantee that the skb->cb
> > will be zero.
> 
> I see. We could :
> 
> Avoid this memset(skb->cb, 0, sizeof(skb->cb)) in fastpath.

Any chances of skb->cb being leaked to userspace or the network, due to
driver bugs or other such oddities?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply

* [PATCH net-next-2.6 V2] net:  fix 64 bit counters on 32 bit arches
From: Eric Dumazet @ 2010-07-05 20:05 UTC (permalink / raw)
  To: Ben Hutchings, David Miller
  Cc: Stephen Hemminger, Arnd Bergmann, netdev, linux-net-drivers
In-Reply-To: <1278354682.2877.639.camel@edumazet-laptop>

Le lundi 05 juillet 2010 à 20:31 +0200, Eric Dumazet a écrit :

> One other way would be to add a rtnl_link_stats64 param to
> ndo_get_stats64() method, and ask drivers to copy their stats in this
> zone, instead of returning &dev->stats64 or something...
> 
> And also change dev_get_stats() with this new parameter.
> 
> Each caller would use a private copy, with no risk of concurrent
> updates.
> 
> 

Here is an updated patch, without added lock but temp storage.

Thanks

[PATCH net-next-2.6 V2] net: fix 64 bit counters on 32 bit arches

There is a small possibility that a reader gets incorrect values on 32
bit arches. SNMP applications could catch incorrect counters when a
32bit high part is changed by another stats consumer/provider.

One way to solve this is to add a rtnl_link_stats64 param to all 
ndo_get_stats64() methods, and also add such a parameter to
dev_get_stats().

Rule is that we are not allowed to use dev->stats64 as a temporary
storage for 64bit stats, but a caller provided area (usually on stack)

Old drivers (only providing get_stats() method) need no changes.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/bonding/bond_main.c   |   64 +++++++++++++---------------
 drivers/net/ixgbe/ixgbe_ethtool.c |    8 ++-
 drivers/net/loopback.c            |    4 -
 drivers/net/macvlan.c             |    6 +-
 drivers/net/sfc/efx.c             |    3 -
 drivers/net/sfc/ethtool.c         |    3 -
 include/linux/netdevice.h         |   12 +++--
 net/8021q/vlan_dev.c              |    6 --
 net/8021q/vlanproc.c              |    3 -
 net/bridge/br_device.c            |    4 -
 net/core/dev.c                    |   25 +++++++---
 net/core/net-sysfs.c              |    4 +
 net/core/rtnetlink.c              |    3 -
 13 files changed, 79 insertions(+), 66 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a95a41b..9bb9bfa 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3804,51 +3804,49 @@ static int bond_close(struct net_device *bond_dev)
 	return 0;
 }
 
-static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev)
+static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev,
+						struct rtnl_link_stats64 *stats)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct rtnl_link_stats64 *stats = &bond_dev->stats64;
-	struct rtnl_link_stats64 local_stats;
+	struct rtnl_link_stats64 temp;
 	struct slave *slave;
 	int i;
 
-	memset(&local_stats, 0, sizeof(local_stats));
+	memset(stats, 0, sizeof(*stats));
 
 	read_lock_bh(&bond->lock);
 
 	bond_for_each_slave(bond, slave, i) {
 		const struct rtnl_link_stats64 *sstats =
-			dev_get_stats(slave->dev);
-
-		local_stats.rx_packets += sstats->rx_packets;
-		local_stats.rx_bytes += sstats->rx_bytes;
-		local_stats.rx_errors += sstats->rx_errors;
-		local_stats.rx_dropped += sstats->rx_dropped;
-
-		local_stats.tx_packets += sstats->tx_packets;
-		local_stats.tx_bytes += sstats->tx_bytes;
-		local_stats.tx_errors += sstats->tx_errors;
-		local_stats.tx_dropped += sstats->tx_dropped;
-
-		local_stats.multicast += sstats->multicast;
-		local_stats.collisions += sstats->collisions;
-
-		local_stats.rx_length_errors += sstats->rx_length_errors;
-		local_stats.rx_over_errors += sstats->rx_over_errors;
-		local_stats.rx_crc_errors += sstats->rx_crc_errors;
-		local_stats.rx_frame_errors += sstats->rx_frame_errors;
-		local_stats.rx_fifo_errors += sstats->rx_fifo_errors;
-		local_stats.rx_missed_errors += sstats->rx_missed_errors;
-
-		local_stats.tx_aborted_errors += sstats->tx_aborted_errors;
-		local_stats.tx_carrier_errors += sstats->tx_carrier_errors;
-		local_stats.tx_fifo_errors += sstats->tx_fifo_errors;
-		local_stats.tx_heartbeat_errors += sstats->tx_heartbeat_errors;
-		local_stats.tx_window_errors += sstats->tx_window_errors;
+			dev_get_stats(slave->dev, &temp);
+
+		stats->rx_packets += sstats->rx_packets;
+		stats->rx_bytes += sstats->rx_bytes;
+		stats->rx_errors += sstats->rx_errors;
+		stats->rx_dropped += sstats->rx_dropped;
+
+		stats->tx_packets += sstats->tx_packets;
+		stats->tx_bytes += sstats->tx_bytes;
+		stats->tx_errors += sstats->tx_errors;
+		stats->tx_dropped += sstats->tx_dropped;
+
+		stats->multicast += sstats->multicast;
+		stats->collisions += sstats->collisions;
+
+		stats->rx_length_errors += sstats->rx_length_errors;
+		stats->rx_over_errors += sstats->rx_over_errors;
+		stats->rx_crc_errors += sstats->rx_crc_errors;
+		stats->rx_frame_errors += sstats->rx_frame_errors;
+		stats->rx_fifo_errors += sstats->rx_fifo_errors;
+		stats->rx_missed_errors += sstats->rx_missed_errors;
+
+		stats->tx_aborted_errors += sstats->tx_aborted_errors;
+		stats->tx_carrier_errors += sstats->tx_carrier_errors;
+		stats->tx_fifo_errors += sstats->tx_fifo_errors;
+		stats->tx_heartbeat_errors += sstats->tx_heartbeat_errors;
+		stats->tx_window_errors += sstats->tx_window_errors;
 	}
 
-	memcpy(stats, &local_stats, sizeof(struct net_device_stats));
-
 	read_unlock_bh(&bond->lock);
 
 	return stats;
diff --git a/drivers/net/ixgbe/ixgbe_ethtool.c b/drivers/net/ixgbe/ixgbe_ethtool.c
index 5275e9c..7e85e61 100644
--- a/drivers/net/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ixgbe/ixgbe_ethtool.c
@@ -55,7 +55,7 @@ struct ixgbe_stats {
 				offsetof(struct ixgbe_adapter, m)
 #define IXGBE_NETDEV_STAT(m)	NETDEV_STATS, \
 				sizeof(((struct net_device *)0)->m), \
-				offsetof(struct net_device, m)
+				offsetof(struct net_device, m) - offsetof(struct net_device, stats)
 
 static struct ixgbe_stats ixgbe_gstrings_stats[] = {
 	{"rx_packets", IXGBE_NETDEV_STAT(stats.rx_packets)},
@@ -998,16 +998,18 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev,
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 	u64 *queue_stat;
 	int stat_count = sizeof(struct ixgbe_queue_stats) / sizeof(u64);
+	struct rtnl_link_stats64 temp;
+	const struct rtnl_link_stats64 *net_stats;
 	int j, k;
 	int i;
 	char *p = NULL;
 
 	ixgbe_update_stats(adapter);
-	dev_get_stats(netdev);
+	net_stats = dev_get_stats(netdev, &temp);
 	for (i = 0; i < IXGBE_GLOBAL_STATS_LEN; i++) {
 		switch (ixgbe_gstrings_stats[i].type) {
 		case NETDEV_STATS:
-			p = (char *) netdev +
+			p = (char *) net_stats +
 					ixgbe_gstrings_stats[i].stat_offset;
 			break;
 		case IXGBE_STATS:
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 4dd0510..9a09967 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -98,10 +98,10 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
 	return NETDEV_TX_OK;
 }
 
-static struct rtnl_link_stats64 *loopback_get_stats64(struct net_device *dev)
+static struct rtnl_link_stats64 *loopback_get_stats64(struct net_device *dev,
+						      struct rtnl_link_stats64 *stats)
 {
 	const struct pcpu_lstats __percpu *pcpu_lstats;
-	struct rtnl_link_stats64 *stats = &dev->stats64;
 	u64 bytes = 0;
 	u64 packets = 0;
 	u64 drops = 0;
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index e6d626e..6112f14 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -431,12 +431,12 @@ static void macvlan_uninit(struct net_device *dev)
 	free_percpu(vlan->rx_stats);
 }
 
-static struct rtnl_link_stats64 *macvlan_dev_get_stats64(struct net_device *dev)
+static struct rtnl_link_stats64 *macvlan_dev_get_stats64(struct net_device *dev,
+							 struct rtnl_link_stats64 *stats)
 {
-	struct rtnl_link_stats64 *stats = &dev->stats64;
 	struct macvlan_dev *vlan = netdev_priv(dev);
 
-	dev_txq_stats_fold(dev, &dev->stats);
+	dev_txq_stats_fold(dev, (struct net_device_stats *)stats);
 
 	if (vlan->rx_stats) {
 		struct macvlan_rx_stats *p, accum = {0};
diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index 35b3f29..ba674c5 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -1533,11 +1533,10 @@ static int efx_net_stop(struct net_device *net_dev)
 }
 
 /* Context: process, dev_base_lock or RTNL held, non-blocking. */
-static struct rtnl_link_stats64 *efx_net_stats(struct net_device *net_dev)
+static struct rtnl_link_stats64 *efx_net_stats(struct net_device *net_dev, struct rtnl_link_stats64 *stats)
 {
 	struct efx_nic *efx = netdev_priv(net_dev);
 	struct efx_mac_stats *mac_stats = &efx->mac_stats;
-	struct rtnl_link_stats64 *stats = &net_dev->stats64;
 
 	spin_lock_bh(&efx->stats_lock);
 	efx->type->update_stats(efx);
diff --git a/drivers/net/sfc/ethtool.c b/drivers/net/sfc/ethtool.c
index 3b8b0a0..fd19d6a 100644
--- a/drivers/net/sfc/ethtool.c
+++ b/drivers/net/sfc/ethtool.c
@@ -469,12 +469,13 @@ static void efx_ethtool_get_stats(struct net_device *net_dev,
 	struct efx_mac_stats *mac_stats = &efx->mac_stats;
 	struct efx_ethtool_stat *stat;
 	struct efx_channel *channel;
+	struct rtnl_link_stats64 temp;
 	int i;
 
 	EFX_BUG_ON_PARANOID(stats->n_stats != EFX_ETHTOOL_NUM_STATS);
 
 	/* Update MAC and NIC statistics */
-	dev_get_stats(net_dev);
+	dev_get_stats(net_dev, &temp);
 
 	/* Fill detailed statistics buffer */
 	for (i = 0; i < EFX_ETHTOOL_NUM_STATS; i++) {
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4d27368..60de653 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -666,7 +666,8 @@ struct netdev_rx_queue {
  *	Callback uses when the transmitter has not made any progress
  *	for dev->watchdog ticks.
  *
- * struct rtnl_link_stats64* (*ndo_get_stats64)(struct net_device *dev);
+ * struct rtnl_link_stats64* (*ndo_get_stats64)(struct net_device *dev
+ *                      struct rtnl_link_stats64 *storage);
  * struct net_device_stats* (*ndo_get_stats)(struct net_device *dev);
  *	Called when a user wants to get the network device usage
  *	statistics. Drivers must do one of the following:
@@ -733,7 +734,8 @@ struct net_device_ops {
 						   struct neigh_parms *);
 	void			(*ndo_tx_timeout) (struct net_device *dev);
 
-	struct rtnl_link_stats64* (*ndo_get_stats64)(struct net_device *dev);
+	struct rtnl_link_stats64* (*ndo_get_stats64)(struct net_device *dev,
+						     struct rtnl_link_stats64 *storage);
 	struct net_device_stats* (*ndo_get_stats)(struct net_device *dev);
 
 	void			(*ndo_vlan_rx_register)(struct net_device *dev,
@@ -2139,8 +2141,10 @@ extern void		netdev_features_change(struct net_device *dev);
 /* Load a device via the kmod */
 extern void		dev_load(struct net *net, const char *name);
 extern void		dev_mcast_init(void);
-extern const struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev);
-extern void		dev_txq_stats_fold(const struct net_device *dev, struct net_device_stats *stats);
+extern const struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
+						     struct rtnl_link_stats64 *storage);
+extern void		dev_txq_stats_fold(const struct net_device *dev,
+					   struct net_device_stats *stats);
 
 extern int		netdev_max_backlog;
 extern int		netdev_tstamp_prequeue;
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index c6456cb..7865a4c 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -803,11 +803,9 @@ static u32 vlan_ethtool_get_flags(struct net_device *dev)
 	return dev_ethtool_get_flags(vlan->real_dev);
 }
 
-static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev)
+static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
 {
-	struct rtnl_link_stats64 *stats = &dev->stats64;
-
-	dev_txq_stats_fold(dev, &dev->stats);
+	dev_txq_stats_fold(dev, (struct net_device_stats *)stats);
 
 	if (vlan_dev_info(dev)->vlan_rx_stats) {
 		struct vlan_rx_stats *p, accum = {0};
diff --git a/net/8021q/vlanproc.c b/net/8021q/vlanproc.c
index df56f5c..80e280f 100644
--- a/net/8021q/vlanproc.c
+++ b/net/8021q/vlanproc.c
@@ -278,6 +278,7 @@ static int vlandev_seq_show(struct seq_file *seq, void *offset)
 {
 	struct net_device *vlandev = (struct net_device *) seq->private;
 	const struct vlan_dev_info *dev_info = vlan_dev_info(vlandev);
+	struct rtnl_link_stats64 temp;
 	const struct rtnl_link_stats64 *stats;
 	static const char fmt[] = "%30s %12lu\n";
 	static const char fmt64[] = "%30s %12llu\n";
@@ -286,7 +287,7 @@ static int vlandev_seq_show(struct seq_file *seq, void *offset)
 	if (!is_vlan_dev(vlandev))
 		return 0;
 
-	stats = dev_get_stats(vlandev);
+	stats = dev_get_stats(vlandev, &temp);
 	seq_printf(seq,
 		   "%s  VID: %d	 REORDER_HDR: %i  dev->priv_flags: %hx\n",
 		   vlandev->name, dev_info->vlan_id,
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index edf639e..075c435 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -98,10 +98,10 @@ static int br_dev_stop(struct net_device *dev)
 	return 0;
 }
 
-static struct rtnl_link_stats64 *br_get_stats64(struct net_device *dev)
+static struct rtnl_link_stats64 *br_get_stats64(struct net_device *dev,
+						struct rtnl_link_stats64 *stats)
 {
 	struct net_bridge *br = netdev_priv(dev);
-	struct rtnl_link_stats64 *stats = &dev->stats64;
 	struct br_cpu_netstats tmp, sum = { 0 };
 	unsigned int cpu;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 93b8929..b6d570a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3703,7 +3703,8 @@ void dev_seq_stop(struct seq_file *seq, void *v)
 
 static void dev_seq_printf_stats(struct seq_file *seq, struct net_device *dev)
 {
-	const struct rtnl_link_stats64 *stats = dev_get_stats(dev);
+	struct rtnl_link_stats64 temp;
+	const struct rtnl_link_stats64 *stats = dev_get_stats(dev, &temp);
 
 	seq_printf(seq, "%6s: %7llu %7llu %4llu %4llu %4llu %5llu %10llu %9llu "
 		   "%8llu %7llu %4llu %4llu %4llu %5llu %7llu %10llu\n",
@@ -5281,23 +5282,29 @@ EXPORT_SYMBOL(dev_txq_stats_fold);
 /**
  *	dev_get_stats	- get network device statistics
  *	@dev: device to get statistics from
+ *	@storage: place to store stats
  *
  *	Get network statistics from device. The device driver may provide
  *	its own method by setting dev->netdev_ops->get_stats64 or
  *	dev->netdev_ops->get_stats; otherwise the internal statistics
  *	structure is used.
  */
-const struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev)
+const struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
+					      struct rtnl_link_stats64 *storage)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
 
-	if (ops->ndo_get_stats64)
-		return ops->ndo_get_stats64(dev);
-	if (ops->ndo_get_stats)
-		return (struct rtnl_link_stats64 *)ops->ndo_get_stats(dev);
-
-	dev_txq_stats_fold(dev, &dev->stats);
-	return &dev->stats64;
+	if (ops->ndo_get_stats64) {
+		memset(storage, 0, sizeof(*storage));
+		return ops->ndo_get_stats64(dev, storage);
+	}
+	if (ops->ndo_get_stats) {
+		memcpy(storage, ops->ndo_get_stats(dev), sizeof(*storage));
+		return storage;
+	}
+	memcpy(storage, &dev->stats, sizeof(*storage));
+	dev_txq_stats_fold(dev, (struct net_device_stats *)storage);
+	return storage;
 }
 EXPORT_SYMBOL(dev_get_stats);
 
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index ea3bb4c..914f42b 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -330,7 +330,9 @@ static ssize_t netstat_show(const struct device *d,
 
 	read_lock(&dev_base_lock);
 	if (dev_isalive(dev)) {
-		const struct rtnl_link_stats64 *stats = dev_get_stats(dev);
+		struct rtnl_link_stats64 temp;
+		const struct rtnl_link_stats64 *stats = dev_get_stats(dev, &temp);
+
 		ret = sprintf(buf, fmt_u64, *(u64 *)(((u8 *) stats) + offset));
 	}
 	read_unlock(&dev_base_lock);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index e645778..5e773ea 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -791,6 +791,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 {
 	struct ifinfomsg *ifm;
 	struct nlmsghdr *nlh;
+	struct rtnl_link_stats64 temp;
 	const struct rtnl_link_stats64 *stats;
 	struct nlattr *attr;
 
@@ -847,7 +848,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	if (attr == NULL)
 		goto nla_put_failure;
 
-	stats = dev_get_stats(dev);
+	stats = dev_get_stats(dev, &temp);
 	copy_rtnl_link_stats(nla_data(attr), stats);
 
 	attr = nla_reserve(skb, IFLA_STATS64,



^ permalink raw reply related

* bridge br_multicast: BUG: unable to handle kernel NULL pointer dereference
From: Frank Arnold @ 2010-07-05 19:05 UTC (permalink / raw)
  To: Stephen Hemminger, YOSHIFUJI Hideaki, Herbert Xu; +Cc: netdev

Hi,

we see a kernel NULL pointer dereference during testing of the KVM tree,
currently based on 2.6.35-rc3. We are using bridge to connect the KVM
guests through the hosts network interface. Here is the trace:

BUG: unable to handle kernel NULL pointer dereference at
0000000000000028                                               
IP: [<ffffffffa0196da0>] __br_ip4_hash+0x0/0x7c [bridge]                                                                
PGD 0                                                                                                                   
Oops: 0000 [#1] SMP                                                                                                     
last sysfs file: /sys/module/lockd/initstate                                                                            
CPU 3                                                                                                                   
Modules linked in: nfsd exportfs nfs lockd nfs_acl auth_rpcgss sunrpc bridge stp ipv6 kvm_amd kvm snd_hda_codec_atihdmi 
snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd pcspkr serio_raw ata_generic r8169 so
undcore i2c_piix4 pata_acpi i2c_core joydev snd_page_alloc mii pata_atiixp shpchp [last unloaded: scsi_wait_scan]       
                                                                                                                        
Pid: 0, comm: swapper Not tainted 2.6.35.20100705_8dea564-1.fc11.osrc.x86_64 #1 GA-MA74GM-S2H/GA-MA74GM-S2H             
RIP: 0010:[<ffffffffa0196da0>]  [<ffffffffa0196da0>] __br_ip4_hash+0x0/0x7c [bridge]                                    
RSP: 0018:ffff880001b838a8  EFLAGS: 00010246                                                                            
RAX: ffff880126028000 RBX: 0000000000000000 RCX: ffff880127b3a828                                                       
RDX: 0000000001b80008 RSI: 0000000064ffffef RDI: 0000000000000000                                                       
RBP: ffff880001b838b0 R08: ffff8800054c3870 R09: 0000000000000000                                                       
R10: 0000000000000000 R11: 0000000000000000 R12: ffff880001b83a00                                                       
R13: ffff880001b83a00 R14: ffff880127b3a800 R15: ffff880125ccc400                                                       
FS:  00007f17d45ea6f0(0000) GS:ffff880001b80000(0000) knlGS:0000000000000000                                            
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b                                                                       
CR2: 0000000000000028 CR3: 00000000016b0000 CR4: 00000000000006e0                                                       
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000                                                       
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400                                                       
Process swapper (pid: 0, threadinfo ffff880127ab4000, task ffff880127ab96b0)                                            
Stack:                                                                                                                  
 ffffffffa0196f48 ffff880001b838d0 ffffffffa01970be ffff880126028640                                                    
<0> ffff880125ccc400 ffff880001b83910 ffffffffa0197511 ffff880001b83900                                                 
<0> ffff880127b3a800 ffff8800054c3868 ffff880126028640 ffff880127b3a800                                                 
Call Trace:                                                                                                             
 <IRQ>                                                                                                                  
 [<ffffffffa0196f48>] ? br_ip_hash+0x1f/0x28 [bridge]                                                                   
 [<ffffffffa01970be>] br_mdb_ip_get+0x12/0x24 [bridge]                                                                  
 [<ffffffffa0197511>] br_multicast_leave_group+0x62/0x160 [bridge]                                                      
 [<ffffffffa0199028>] br_multicast_rcv+0x60e/0xcda [bridge]                                                             
 [<ffffffff81043320>] ? local_bh_enable_ip+0x9/0xb                                                                      
 [<ffffffff81369f85>] ? _raw_spin_unlock_bh+0xf/0x11                                                                    
 [<ffffffff812f9a1a>] ? packet+0x1a/0x24                                                                                
 [<ffffffff812f777b>] ? nf_conntrack_in+0x4ee/0x59f                                                                     
 [<ffffffffa01907d5>] ? fdb_create+0x28/0x73 [bridge]                                                                   
 [<ffffffffa0190945>] ? br_fdb_update+0x125/0x134 [bridge]                                                              
 [<ffffffffa0191e74>] br_handle_frame_finish+0x6d/0x1ba [bridge]                                                        
 [<ffffffffa0191e07>] ? br_handle_frame_finish+0x0/0x1ba [bridge]                                                       
 [<ffffffffa0195c79>] NF_HOOK_THRESH+0x46/0x4d [bridge]                                                                 
 [<ffffffffa0195ed2>] ? nf_bridge_push_encap_header+0x2f/0x3c [bridge]                                                  
 [<ffffffffa0196c65>] br_nf_pre_routing_finish+0x222/0x231 [bridge]                                                     
 [<ffffffff812f4a10>] ? nf_hook_slow+0x65/0xc6                                                                          
 [<ffffffffa0196a43>] ? br_nf_pre_routing_finish+0x0/0x231 [bridge]                                                     
 [<ffffffffa0196a43>] ? br_nf_pre_routing_finish+0x0/0x231 [bridge]                                                     
 [<ffffffffa0195c79>] NF_HOOK_THRESH+0x46/0x4d [bridge]                                                                 
 [<ffffffffa019609a>] ? nf_bridge_alloc+0x1d/0x3a [bridge]                                                              
 [<ffffffffa0196a26>] br_nf_pre_routing+0x550/0x56d [bridge]                                                            
 [<ffffffff812f4968>] nf_iterate+0x41/0x84                                                                              
 [<ffffffffa0191e07>] ? br_handle_frame_finish+0x0/0x1ba [bridge]                                                       
 [<ffffffff812f4a10>] nf_hook_slow+0x65/0xc6                                                                            
 [<ffffffffa0191e07>] ? br_handle_frame_finish+0x0/0x1ba [bridge]                                                       
 [<ffffffffa0191e07>] ? br_handle_frame_finish+0x0/0x1ba [bridge]                                                       
 [<ffffffffa0191df5>] NF_HOOK.clone.0+0x41/0x53 [bridge]                                                                
 [<ffffffffa0192137>] br_handle_frame+0x176/0x18f [bridge]                                                              
 [<ffffffff812d54e5>] __netif_receive_skb+0x2b0/0x3f5                                                                   
 [<ffffffff810592d2>] ? ktime_get_real+0x11/0x3e                                                                        
 [<ffffffff812d612c>] netif_receive_skb+0x52/0x59                                                                       
 [<ffffffff812d0ce6>] ? __netdev_alloc_skb+0x2f/0x4b                                                                    
 [<ffffffffa0054ff1>] rtl8169_rx_interrupt+0x385/0x4d6 [r8169]                                                          
 [<ffffffff81222203>] ? scsi_next_command+0x3e/0x46                                                                     
 [<ffffffff812354b3>] ? __ata_qc_complete+0xdf/0xe7                                                                     
 [<ffffffffa0057614>] rtl8169_poll+0x37/0x1a1 [r8169]                                                                   
 [<ffffffff812d62ed>] net_rx_action+0xab/0x18c                                                                          
 [<ffffffffa00565f4>] ? rtl8169_interrupt+0x2cb/0x36e [r8169]                                                           
 [<ffffffff81043446>] __do_softirq+0x97/0x125                                                                           
 [<ffffffff8101a026>] ? ack_apic_level+0x78/0x1ce                                                                       
 [<ffffffff810038dc>] call_softirq+0x1c/0x28                                                                            
 [<ffffffff81004e61>] do_softirq+0x41/0x7e                                                                              
 [<ffffffff810431ce>] irq_exit+0x36/0x78                                                                                
 [<ffffffff8100459c>] do_IRQ+0xa7/0xbe                                                                                  
 [<ffffffff8136a1d3>] ret_from_intr+0x0/0x11                                                                            
 <EOI>                                                                                                                  
 [<ffffffff8102036c>] ? native_safe_halt+0x6/0x8                                                                        
 [<ffffffff8136d161>] ? atomic_notifier_call_chain+0x13/0x15                                                            
 [<ffffffff81009696>] default_idle+0x27/0x44                                                                            
 [<ffffffff81001d3a>] cpu_idle+0x58/0x93                                                                                
 [<ffffffff81364944>] start_secondary+0x1a4/0x1a8                                                                       
Code: 7e 66 81 fa 81 00 74 0d 31 c0 66 81 fa 88 64 0f 94 c0 c1 e0 03 89 c2 48 29 93 e0 00 00 00 01 43 68 31 c0 5b 41 5c 
c9 c3 90 90 90 <8b> 47 28 89 f1 ba b9 79 37 9e c1 e9 0d 29 f2 55 29 f0 48 89 e5                                         
RIP  [<ffffffffa0196da0>] __br_ip4_hash+0x0/0x7c [bridge]                                                               
 RSP <ffff880001b838a8>                                                                                                 
CR2: 0000000000000028                                                                                                   
---[ end trace c0f05a4e3727475d ]---                                                                                    
Kernel panic - not syncing: Fatal exception in interrupt                                                                


-- 
Frank Arnold 
Systems Design Technician, Software Test
AMD Operating System Research Center
Dresden, Germany
Tel: +49 351 448 356702


Legal Information:
Advanced Micro Devices GmbH
Einsteinring 24
85609 Dornach b. München

Geschäftsführer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis München
Registergericht München, HRB Nr. 43632



^ permalink raw reply

* Re: [PATCH 3/3] pm_qos: get rid of the allocation in pm_qos_add_request()
From: mark gross @ 2010-07-05 19:16 UTC (permalink / raw)
  To: James Bottomley; +Cc: Takashi Iwai, Linux PM, markgross, netdev
In-Reply-To: <1278338568.2850.1.camel@mulgrave.site>

On Mon, Jul 05, 2010 at 09:02:48AM -0500, James Bottomley wrote:
> On Mon, 2010-07-05 at 08:41 +0200, Takashi Iwai wrote:
> > sorry for the late reply, as I've been on vacation in the last week
> > (and shut off mails intentionally :)
> 
> Envy forbids me from saying that's OK.
> 
> > At Mon, 28 Jun 2010 12:44:48 -0500,
> > James Bottomley wrote:
> > > 
> > > Since every caller has to squirrel away the returned pointer anyway,
> > > they might as well supply the memory area.  This fixes a bug in a few of
> > > the call sites where the returned pointer was dereferenced without
> > > checking it for NULL (which gets returned if the kzalloc failed).
> > > 
> > > I'd like to hear how sound and netdev feels about this: it will add
> > > about two more pointers worth of data to struct netdev and struct
> > > snd_pcm_substream .. but I think it's worth it.  If you're OK, I'll add
> > > your acks and send through the pm tree.
> > > 
> > > This also looks to me like an android independent clean up (even though
> > > it renders the request_add atomically callable).  I also added include
> > > guards to include/linux/pm_qos_params.h
> > 
> > I like the patch very well, too.
> > But, just wondering...
> > 
> > > @@ -262,6 +260,11 @@ void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
> > >  	if (!pm_qos_req) /*guard against callers passing in null */
> > >  		return;
> > >  
> > > +	if (pm_qos_request_active(pm_qos_req)) {
> > > +		WARN(1, KERN_ERR "pm_qos_update_request() called for unknown object\n");
> > > +		return;
> > > +	}
> > > +
> > 
> > Is this correct...?  Shouldn't it be a negative check?
> 
> Yes, it should be a negative check ... I'll update the patch.  I guess
> this still means that no-one has managed to test it on a functional
> system ...
> 

well I guess that explains the warning I got on my back port of this
patch.


[   62.944788] ------------[ cut here ]------------                                        
[   62.944788] WARNING: at kernel/pm_qos_params.c:266                                      
pm_qos_update_request+0x21/0x46)                                                           
[   62.944788] pm_qos_update_request() called for unknown object                           
[   62.944788] Modules linked in: mrst_sspi cfspi_slave chnl_chr                           
caif_chr chnl_net caf                                                                      
[   62.944788] Pid: 91, comm: mrst/0 Tainted: G        W  2.6.31.6-mrst
#30                
[   62.944788] Call Trace:                                                                 
[   62.944788]  [<c0145b2e>] ? pm_qos_update_request+0x21/0x46                             
[   62.944788]  [<c012fff3>] warn_slowpath_common+0x60/0x77                                
[   62.944788]  [<c013003e>] warn_slowpath_fmt+0x24/0x27                                   
[   62.944788]  [<c0145b2e>] pm_qos_update_request+0x21/0x46                               
[   62.944788]  [<c03029f2>] int_transfer_complete_work+0x19/0x65                          
[   62.944788]  [<c013f02a>] worker_thread+0x153/0x1df                                     
[   62.944788]  [<c03029d9>] ? int_transfer_complete_work+0x0/0x65                         
[   62.944788]  [<c0141df1>] ? autoremove_wake_function+0x0/0x30                           
[   62.944788]  [<c0141c7c>] kthread+0x64/0x69                                             
[   62.944788]  [<c013eed7>] ? worker_thread+0x0/0x1df                                     
[   62.944788]  [<c0141c18>] ? kthread+0x0/0x69                                            
[   62.944788]  [<c01034df>] kernel_thread_helper+0x7/0x10                                 
[   62.944788] ---[ end trace 1723851b79e06c5d ]---                                        
[   62.944788] ------------[ cut here ]------------                                        
[   62.944788] WARNING: at kernel/pm_qos_params.c:266          

Sorry, I've been swamped by work and personal things the past 2 weeks.

--mgross


^ permalink raw reply

* Re: [PATCH net-next-2.6] tg3: 64bits stats
From: Eric Dumazet @ 2010-07-05 18:59 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Matt Carlson, Michael Chan, netdev, David Miller
In-Reply-To: <1278355263.2087.116.camel@achroite.uk.solarflarecom.com>

Le lundi 05 juillet 2010 à 19:41 +0100, Ben Hutchings a écrit :
> Ugh, I was assuming that callers of dev_get_stats() would hold
> dev_base_lock.  However only netstat_show() seems to do so currently.

Right, but a read_lock() would not be enough, even if all users would
use it.



^ permalink raw reply

* Re: [PATCH net-next-2.6] tg3: 64bits stats
From: Ben Hutchings @ 2010-07-05 18:41 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Matt Carlson, Michael Chan, netdev, David Miller
In-Reply-To: <1278353230.2877.601.camel@edumazet-laptop>

On Mon, 2010-07-05 at 20:07 +0200, Eric Dumazet wrote:
> Le lundi 05 juillet 2010 à 18:31 +0100, Ben Hutchings a écrit :
> > On Mon, 2010-07-05 at 18:03 +0200, Eric Dumazet wrote:
> > > Le lundi 05 juillet 2010 à 11:14 +0200, Eric Dumazet a écrit :
> > > > After commit be1f3c2c027c (net: Enable 64-bit net device statistics on
> > > > 32-bit architectures), we can now provide 64bit stats, even on 32bit
> > > > arches.
> > > > 
> > > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > > > ---
> > > 
> > > Please disregard this patch.
> > > 
> > > There is small possibility a reader might read a 64bit value while
> > > another writer makes a change to it, changing high 32bit value.
> > > 
> > > A change in core network would be needed to make this 100% safe,
> > > possibly using a seqlock to protect dev->stats64
> > 
> > I really didn't want to add this overhead and complication to readers
> > when only some drivers need it.
> > 
> 
> Overhead would be minimal, only in get_stats() method and only for
> drivers that want to provide 64 bit stats...
> 
> Clearly, rx/tx path must not have any overhead.
> 
> Right now, even RTNL 64bit stats are not safe.

Ugh, I was assuming that callers of dev_get_stats() would hold
dev_base_lock.  However only netstat_show() seems to do so currently.

Ben.

> Should we revert prior patches or try to make progress ?
> 
> 
> 
-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* RE: Possible bug in net/ipv4/route.c?
From: Sol Kavy @ 2010-07-05 16:37 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Greg Ren, Guojun Jin, Murat Sezgin, Sener Ilgen, David S. Miller,
	Stephen Hemminger, netdev
In-Reply-To: <20100705120413.GA6219@gondor.apana.org.au>

So is there some place else that should have the clear instead of ipv4_link_failure()?

Sol

-----Original Message-----
From: Herbert Xu [mailto:herbert@gondor.apana.org.au] 
Sent: Monday, July 05, 2010 5:04 AM
To: Sol Kavy
Cc: linux-kernel@vger.kernel.org; Greg Ren; Guojun Jin; Murat Sezgin; Sener Ilgen; David S. Miller; Stephen Hemminger; netdev@vger.kernel.org
Subject: Re: Possible bug in net/ipv4/route.c?

Sol Kavy <skavy@ubicom.com> wrote:
> Found Linux: 2.6.28
> Arch: Ubicom32 <not yet pushed>
> Project: uCLinux based Router
> Test: Bit torrent Stress Test
> 
> Note: The top of Linus git net/ipv4/route.c appears to have the same issue.
> 
> The following is a patch for clearing out IP options area in an input skb during link failure processing.  Without this patch, the icmp_send() can result in a call to ip_options_echo() where the common buffer area of the skb is incorrectly interpreted.  Depending on the previous use of the skb->cb[], the interpreted option length values can cause stack corruption by copying more than 40 bytes to the output options.
> 
> In our case, a driver is using the skb->cb[] area to hold driver specific data.   The driver is not zeroing out the area after use.  I can see three basic solutions:
> 
> 1) Drivers are not allowed to use the skb->cb[] area at all.  Ubicom should modify the driver to use a different approach.
> 
> 2) The layer using skb->cb[] should clear this area after use and before handing the skb to another layer.  Ubicom should modify the driver to clear the skb->cb[] area before sending it up the line.
> 
> 3) Any layer that "uses" the skb->cb[] area must clear the area before use.  In which case, the proposed patch would fix the problem for the ipv4_link_failure().  I believe that this is the correct fix because I see ip_rcv() clears the skb->cb[] before using it.
> 
> Can someone confirm that this is the appropriate fix?  If this is documented somewhere, please direct me to the documentation.

Thanks for the report!
 
> Patch:
> 
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c index 
> 125ee64..d13805f 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1606,6 +1606,14 @@ static void ipv4_link_failure(struct sk_buff 
> *skb) {
>         struct rtable *rt;
> 
> +       /*
> +         * Since link failure can be called with skbs from many 
> +layers (see arp)
> +         * the cb area of the skb must be cleared before use. Because 
> +the cb area
> +         * can be formatted according to the caller layer's cb area 
> +format and it may cause
> +         * corruptions when it is handled in a different network layer.
> +         */
> +       memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
>         icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_UNREACH, 0);
>         rt = skb->rtable;
> 
> The packet is enqueud by:
> do_IRQ()->do_softirq()->__do_softirq()->net_rx_action()->ubi32_eth_napi_poll()->ubi32_eth_receive()->__vlan_hwaccel_rx()->netif_receive_skb()->br_handle_frame()->nf_hook_slow()->br_nf_pre_routing_finish()->br_nfr_pre_routing_finish_bridge()->neight_resolve_output()->__neigh_event_send().
> 
> The packet is then dequeued by: 
> do_IRQ() -> irq_exit() -> do_softirq() -> run_timer_softirq() -> neigh_timer_handler() -> arp_error_report() -> ipv4_link_failure() -> icmp_send() -> ip_options_echo().
> 
> Because the Ubicom Ethernet driver overwrites the common buffer area, 
> the enqueued packet contains garbage when casted as an IP options data structure.  This results in ip_options_echo() miss reading the option length information and overwriting memory.  By clearing the skb->cb[] before processing the icmp_send() against the packet, we ensure that ip_options_echo() does not corrupt memory.

Generally this area should be cleared on entry to each stack intending on using it.  So in this case, I'd point the finger of blame at the bridge stack for letting this packet into the IP stack through the back entrance without taking the proper precautions.

Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH net-next-2.6] tg3: 64bits stats
From: Eric Dumazet @ 2010-07-05 18:35 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Matt Carlson, Michael Chan, netdev, David Miller
In-Reply-To: <1278354656.2087.108.camel@achroite.uk.solarflarecom.com>

Le lundi 05 juillet 2010 à 19:30 +0100, Ben Hutchings a écrit :

> I think you should use a similar approach here as you did in the
> loopback driver, i.e. update private variables in the RX and TX path and
> then copy/aggregate them in the implementation ndo_get_stats64 (only
> without the need for percpu stats).
> 
> If you want to include a seqlock in the driver stats interface, you can
> do that but it's not going to be pretty and we're still going to need
> additional seqlocks for per-queue (or percpy) stats in some drivers.

Yes, I provided one patch but am working on a different one, requiring a
new rtnl_link_stats64 param to ndo_get_stats64() methods and
dev_get_stats() as well.

dev->stats64 should not be overwritten without some synchronization, so
just disallow it for the moment...



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox