Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 0/8] SECURITY ISSUE with connector
From: Greg KH @ 2009-10-02 13:58 UTC (permalink / raw)
  To: Philipp Reisner
  Cc: linux-kernel, netdev, Andrew Morton, David S. Miller, dm-devel,
	Evgeniy Polyakov, linux-fbdev-devel
In-Reply-To: <1254487211-11810-1-git-send-email-philipp.reisner@linbit.com>

On Fri, Oct 02, 2009 at 02:40:03PM +0200, Philipp Reisner wrote:
> Affected: All code that uses connector, in kernel and out of mainline
> 
> The connector, as it is today, does not allow the in kernel receiving
> parts to do any checks on privileges of a message's sender.

So, assume I know nothing about the connector architecture, what does
this mean in a security context?

> I know, there are not many out there that like connector, but as
> long as it is in the kernel, we have to fix the security issues it has!

And what specifically are the security issues?

> Please either drop connector, or someone who feels a bit responsible
> and has our beloved dictator's blessing, PLEASE PLEASE PLEASE take 
> this into your tree, and send the pull request to Linus.
> 
> Patches 1 to 4 are already Acked-by Evgeny, the connector's maintainer.
> Patches 5 to 7 are the obvious fixes to the connector user's code.

Obvious in what way?

thanks,

greg k-h

^ permalink raw reply

* Re: 2.6.32-rc1-git2: Reported regressions from 2.6.31
From: Stefan Richter @ 2009-10-02 13:00 UTC (permalink / raw)
  To: Jaswinder Singh Rajput
  Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Adrian Bunk,
	Andrew Morton, Linus Torvalds, Natalie Protasevich,
	Kernel Testers List, Network Development, Linux ACPI,
	Linux PM List, Linux SCSI List, Linux Wireless List, DRI
In-Reply-To: <1254469139.3531.19.camel-6Ww87KsxWewAvxtiuMwx3w@public.gmane.org>

Jaswinder Singh Rajput wrote:
> If you add one more entry say "Suspected commit :" then it will be great
> and will solve regressions much faster.

Will?  Might.

> You can request submitter to
> submit 'suspected commit' by git bisect and also specify git bisect
> links like : (for more information about git bisect check
> http://kerneltrap.org/node/11753)

I disagree.  A reporter should only be asked to bisect (using git or
other tools) /if/ a developer determined that bisection may speed up the
debugging process or is the only remaining option to make progress with
a bug.

It would be wrong to steal a reporter's valuable time by asking for
bisection before anybody familiar with the matter even had a first look
at the report.

Remember:
  - Not all bugs can be economically narrowed down by bisection.
  - Bisection requires skills, rigor, and time.
  - Alas there are considerable sections in our kernel history which
    are not bisectable.
-- 
Stefan Richter
-=====-==--= =-=- ---=-
http://arcgraph.de/sr/

^ permalink raw reply

* [PATCH 8/8] uvesafb/connector: Disallow unpliviged users to send netlink packets
From: Philipp Reisner @ 2009-10-02 12:40 UTC (permalink / raw)
  To: linux-kernel, netdev, Andrew Morton, David S. Miller, Greg KH
  Cc: dm-devel, Evgeniy Polyakov, linux-fbdev-devel, Philipp Reisner
In-Reply-To: <1254487211-11810-8-git-send-email-philipp.reisner@linbit.com>

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
---
 drivers/video/uvesafb.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index aa7cd95..e35232a 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -72,6 +72,9 @@ static void uvesafb_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *ns
 	struct uvesafb_task *utask;
 	struct uvesafb_ktask *task;
 
+	if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN))
+		return;
+
 	if (msg->seq >= UVESAFB_TASKS_MAX)
 		return;
 
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 7/8] pohmelfs/connector: Disallow unpliviged users to configure pohmelfs
From: Philipp Reisner @ 2009-10-02 12:40 UTC (permalink / raw)
  To: linux-kernel, netdev, Andrew Morton, David S. Miller, Greg KH
  Cc: dm-devel, Evgeniy Polyakov, linux-fbdev-devel, Philipp Reisner
In-Reply-To: <1254487211-11810-7-git-send-email-philipp.reisner@linbit.com>

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
---
 drivers/staging/pohmelfs/config.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/pohmelfs/config.c b/drivers/staging/pohmelfs/config.c
index c9162b3..5d04bf5 100644
--- a/drivers/staging/pohmelfs/config.c
+++ b/drivers/staging/pohmelfs/config.c
@@ -531,6 +531,9 @@ static void pohmelfs_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *n
 {
 	int err;
 
+	if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN))
+		return;
+
 	switch (msg->flags) {
 		case POHMELFS_FLAGS_ADD:
 		case POHMELFS_FLAGS_DEL:
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 6/8] dst/connector: Disallow unpliviged users to configure dst
From: Philipp Reisner @ 2009-10-02 12:40 UTC (permalink / raw)
  To: linux-kernel, netdev, Andrew Morton, David S. Miller, Greg KH
  Cc: dm-devel, Evgeniy Polyakov, linux-fbdev-devel, Philipp Reisner
In-Reply-To: <1254487211-11810-6-git-send-email-philipp.reisner@linbit.com>

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
---
 drivers/staging/dst/dcore.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/dst/dcore.c b/drivers/staging/dst/dcore.c
index 3943c91..ee16010 100644
--- a/drivers/staging/dst/dcore.c
+++ b/drivers/staging/dst/dcore.c
@@ -855,6 +855,11 @@ static void cn_dst_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
 	struct dst_node *n = NULL, *tmp;
 	unsigned int hash;
 
+	if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN)) {
+		err = -EPERM;
+		goto out;
+	}
+
 	if (msg->len < sizeof(struct dst_ctl)) {
 		err = -EBADMSG;
 		goto out;
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 5/8] dm/connector: Only process connector packages from privileged processes
From: Philipp Reisner @ 2009-10-02 12:40 UTC (permalink / raw)
  To: linux-kernel, netdev, Andrew Morton, David S. Miller, Greg KH
  Cc: dm-devel, Evgeniy Polyakov, linux-fbdev-devel, Philipp Reisner
In-Reply-To: <1254487211-11810-5-git-send-email-philipp.reisner@linbit.com>

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
---
 drivers/md/dm-log-userspace-transfer.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/md/dm-log-userspace-transfer.c b/drivers/md/dm-log-userspace-transfer.c
index 1327e1a..54abf9e 100644
--- a/drivers/md/dm-log-userspace-transfer.c
+++ b/drivers/md/dm-log-userspace-transfer.c
@@ -133,6 +133,9 @@ static void cn_ulog_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
 {
 	struct dm_ulog_request *tfr = (struct dm_ulog_request *)(msg + 1);
 
+	if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN))
+		return;
+
 	spin_lock(&receiving_list_lock);
 	if (msg->len == 0)
 		fill_pkg(msg, NULL);
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 3/8] connector/dm: Fixed a compilation warning
From: Philipp Reisner @ 2009-10-02 12:40 UTC (permalink / raw)
  To: linux-kernel, netdev, Andrew Morton, David S. Miller, Greg KH
  Cc: dm-devel, Evgeniy Polyakov, linux-fbdev-devel, Philipp Reisner
In-Reply-To: <1254487211-11810-3-git-send-email-philipp.reisner@linbit.com>

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Acked-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
---
 drivers/md/dm-log-userspace-transfer.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-log-userspace-transfer.c b/drivers/md/dm-log-userspace-transfer.c
index 556131f..1327e1a 100644
--- a/drivers/md/dm-log-userspace-transfer.c
+++ b/drivers/md/dm-log-userspace-transfer.c
@@ -129,9 +129,8 @@ static int fill_pkg(struct cn_msg *msg, struct dm_ulog_request *tfr)
  * This is the connector callback that delivers data
  * that was sent from userspace.
  */
-static void cn_ulog_callback(void *data, struct netlink_skb_parms *nsp)
+static void cn_ulog_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
 {
-	struct cn_msg *msg = (struct cn_msg *)data;
 	struct dm_ulog_request *tfr = (struct dm_ulog_request *)(msg + 1);
 
 	spin_lock(&receiving_list_lock);
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 0/8] SECURITY ISSUE with connector
From: Philipp Reisner @ 2009-10-02 12:40 UTC (permalink / raw)
  To: linux-kernel, netdev, Andrew Morton, David S. Miller, Greg KH
  Cc: dm-devel, Evgeniy Polyakov, linux-fbdev-devel, Philipp Reisner

Affected: All code that uses connector, in kernel and out of mainline

The connector, as it is today, does not allow the in kernel receiving
parts to do any checks on privileges of a message's sender.

I know, there are not many out there that like connector, but as
long as it is in the kernel, we have to fix the security issues it has!

Please either drop connector, or someone who feels a bit responsible
and has our beloved dictator's blessing, PLEASE PLEASE PLEASE take 
this into your tree, and send the pull request to Linus.

Patches 1 to 4 are already Acked-by Evgeny, the connector's maintainer.
Patches 5 to 7 are the obvious fixes to the connector user's code.

For convenience these patches are also available as git tree:
git://git.drbd.org/linux-2.6-drbd.git connector-fix

-Phil

Philipp Reisner (8):
  connector: Keep the skb in cn_callback_data
  connector: Provide the sender's credentials to the callback
  connector/dm: Fixed a compilation warning
  connector: Removed the destruct_data callback since it is always kfree_skb()
  dm/connector: Only process connector packages from privileged processes
  dst/connector: Disallow unpliviged users to configure dst
  pohmelfs/connector: Disallow unpliviged users to configure pohmelfs
  uvesafb/connector: Disallow unpliviged users to send netlink packets

 Documentation/connector/cn_test.c      |    2 +-
 Documentation/connector/connector.txt  |    8 ++++----
 drivers/connector/cn_queue.c           |   12 +++++++-----
 drivers/connector/connector.c          |   22 ++++++++--------------
 drivers/md/dm-log-userspace-transfer.c |    6 ++++--
 drivers/staging/dst/dcore.c            |    7 ++++++-
 drivers/staging/pohmelfs/config.c      |    5 ++++-
 drivers/video/uvesafb.c                |    5 ++++-
 drivers/w1/w1_netlink.c                |    2 +-
 include/linux/connector.h              |   11 ++++-------
 10 files changed, 43 insertions(+), 37 deletions(-)


^ permalink raw reply

* [PATCH 4/8] connector: Removed the destruct_data callback since it is always kfree_skb()
From: Philipp Reisner @ 2009-10-02 12:40 UTC (permalink / raw)
  To: linux-kernel, netdev, Andrew Morton, David S. Miller, Greg KH
  Cc: dm-devel, Evgeniy Polyakov, linux-fbdev-devel, Philipp Reisner
In-Reply-To: <1254487211-11810-4-git-send-email-philipp.reisner@linbit.com>

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Acked-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
---
 drivers/connector/cn_queue.c  |    4 ++--
 drivers/connector/connector.c |   11 +++--------
 include/linux/connector.h     |    3 ---
 3 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
index 163c3e3..210338e 100644
--- a/drivers/connector/cn_queue.c
+++ b/drivers/connector/cn_queue.c
@@ -83,8 +83,8 @@ void cn_queue_wrapper(struct work_struct *work)
 
 	d->callback(msg, nsp);
 
-	d->destruct_data(d->ddata);
-	d->ddata = NULL;
+	kfree_skb(d->skb);
+	d->skb = NULL;
 
 	kfree(d->free);
 }
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index e59f0ab..f060246 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -129,7 +129,7 @@ EXPORT_SYMBOL_GPL(cn_netlink_send);
 /*
  * Callback helper - queues work and setup destructor for given data.
  */
-static int cn_call_callback(struct sk_buff *skb, void (*destruct_data)(void *), void *data)
+static int cn_call_callback(struct sk_buff *skb)
 {
 	struct cn_callback_entry *__cbq, *__new_cbq;
 	struct cn_dev *dev = &cdev;
@@ -140,12 +140,9 @@ static int cn_call_callback(struct sk_buff *skb, void (*destruct_data)(void *),
 	list_for_each_entry(__cbq, &dev->cbdev->queue_list, callback_entry) {
 		if (cn_cb_equal(&__cbq->id.id, &msg->id)) {
 			if (likely(!work_pending(&__cbq->work) &&
-					__cbq->data.ddata == NULL)) {
+					__cbq->data.skb == NULL)) {
 				__cbq->data.skb = skb;
 
-				__cbq->data.ddata = data;
-				__cbq->data.destruct_data = destruct_data;
-
 				if (queue_cn_work(__cbq, &__cbq->work))
 					err = 0;
 				else
@@ -159,8 +156,6 @@ static int cn_call_callback(struct sk_buff *skb, void (*destruct_data)(void *),
 					d = &__new_cbq->data;
 					d->skb = skb;
 					d->callback = __cbq->data.callback;
-					d->ddata = data;
-					d->destruct_data = destruct_data;
 					d->free = __new_cbq;
 
 					__new_cbq->pdev = __cbq->pdev;
@@ -208,7 +203,7 @@ static void cn_rx_skb(struct sk_buff *__skb)
 			return;
 		}
 
-		err = cn_call_callback(skb, (void (*)(void *))kfree_skb, skb);
+		err = cn_call_callback(skb);
 		if (err < 0)
 			kfree_skb(skb);
 	}
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 545728e..3a14615 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -132,9 +132,6 @@ struct cn_callback_id {
 };
 
 struct cn_callback_data {
-	void (*destruct_data) (void *);
-	void *ddata;
-
 	struct sk_buff *skb;
 	void (*callback) (struct cn_msg *, struct netlink_skb_parms *);
 
-- 
1.6.0.4

^ permalink raw reply related

* [PATCH 2/8] connector: Provide the sender's credentials to the callback
From: Philipp Reisner @ 2009-10-02 12:40 UTC (permalink / raw)
  To: linux-kernel, netdev, Andrew Morton, David S. Miller, Greg KH
  Cc: dm-devel, Evgeniy Polyakov, linux-fbdev-devel, Philipp Reisner
In-Reply-To: <1254487211-11810-2-git-send-email-philipp.reisner@linbit.com>

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Acked-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
---
 Documentation/connector/cn_test.c      |    2 +-
 Documentation/connector/connector.txt  |    8 ++++----
 drivers/connector/cn_queue.c           |    7 ++++---
 drivers/connector/connector.c          |    4 ++--
 drivers/md/dm-log-userspace-transfer.c |    2 +-
 drivers/staging/dst/dcore.c            |    2 +-
 drivers/staging/pohmelfs/config.c      |    2 +-
 drivers/video/uvesafb.c                |    2 +-
 drivers/w1/w1_netlink.c                |    2 +-
 include/linux/connector.h              |    6 +++---
 10 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/Documentation/connector/cn_test.c b/Documentation/connector/cn_test.c
index 1711adc..b07add3 100644
--- a/Documentation/connector/cn_test.c
+++ b/Documentation/connector/cn_test.c
@@ -34,7 +34,7 @@ static char cn_test_name[] = "cn_test";
 static struct sock *nls;
 static struct timer_list cn_test_timer;
 
-static void cn_test_callback(struct cn_msg *msg)
+static void cn_test_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
 {
 	pr_info("%s: %lu: idx=%x, val=%x, seq=%u, ack=%u, len=%d: %s.\n",
 	        __func__, jiffies, msg->id.idx, msg->id.val,
diff --git a/Documentation/connector/connector.txt b/Documentation/connector/connector.txt
index 81e6bf6..78c9466 100644
--- a/Documentation/connector/connector.txt
+++ b/Documentation/connector/connector.txt
@@ -23,7 +23,7 @@ handling, etc...  The Connector driver allows any kernelspace agents to use
 netlink based networking for inter-process communication in a significantly
 easier way:
 
-int cn_add_callback(struct cb_id *id, char *name, void (*callback) (void *));
+int cn_add_callback(struct cb_id *id, char *name, void (*callback) (struct cn_msg *, struct netlink_skb_parms *));
 void cn_netlink_send(struct cn_msg *msg, u32 __group, int gfp_mask);
 
 struct cb_id
@@ -53,15 +53,15 @@ struct cn_msg
 Connector interfaces.
 /*****************************************/
 
-int cn_add_callback(struct cb_id *id, char *name, void (*callback) (void *));
+int cn_add_callback(struct cb_id *id, char *name, void (*callback) (struct cn_msg *, struct netlink_skb_parms *));
 
  Registers new callback with connector core.
 
  struct cb_id *id		- unique connector's user identifier.
 				  It must be registered in connector.h for legal in-kernel users.
  char *name			- connector's callback symbolic name.
- void (*callback) (void *)	- connector's callback.
-				  Argument must be dereferenced to struct cn_msg *.
+ void (*callback) (struct cn..)	- connector's callback.
+				  cn_msg and the sender's credentials
 
 
 void cn_del_callback(struct cb_id *id);
diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
index b4cfac9..163c3e3 100644
--- a/drivers/connector/cn_queue.c
+++ b/drivers/connector/cn_queue.c
@@ -79,8 +79,9 @@ void cn_queue_wrapper(struct work_struct *work)
 		container_of(work, struct cn_callback_entry, work);
 	struct cn_callback_data *d = &cbq->data;
 	struct cn_msg *msg = NLMSG_DATA(nlmsg_hdr(d->skb));
+	struct netlink_skb_parms *nsp = &NETLINK_CB(d->skb);
 
-	d->callback(msg);
+	d->callback(msg, nsp);
 
 	d->destruct_data(d->ddata);
 	d->ddata = NULL;
@@ -90,7 +91,7 @@ void cn_queue_wrapper(struct work_struct *work)
 
 static struct cn_callback_entry *
 cn_queue_alloc_callback_entry(char *name, struct cb_id *id,
-			      void (*callback)(struct cn_msg *))
+			      void (*callback)(struct cn_msg *, struct netlink_skb_parms *))
 {
 	struct cn_callback_entry *cbq;
 
@@ -124,7 +125,7 @@ int cn_cb_equal(struct cb_id *i1, struct cb_id *i2)
 }
 
 int cn_queue_add_callback(struct cn_queue_dev *dev, char *name, struct cb_id *id,
-			  void (*callback)(struct cn_msg *))
+			  void (*callback)(struct cn_msg *, struct netlink_skb_parms *))
 {
 	struct cn_callback_entry *cbq, *__cbq;
 	int found = 0;
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index fc9887f..e59f0ab 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -269,7 +269,7 @@ static void cn_notify(struct cb_id *id, u32 notify_event)
  * May sleep.
  */
 int cn_add_callback(struct cb_id *id, char *name,
-		    void (*callback)(struct cn_msg *))
+		    void (*callback)(struct cn_msg *, struct netlink_skb_parms *))
 {
 	int err;
 	struct cn_dev *dev = &cdev;
@@ -351,7 +351,7 @@ static int cn_ctl_msg_equals(struct cn_ctl_msg *m1, struct cn_ctl_msg *m2)
  *
  * Used for notification of a request's processing.
  */
-static void cn_callback(struct cn_msg *msg)
+static void cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
 {
 	struct cn_ctl_msg *ctl;
 	struct cn_ctl_entry *ent;
diff --git a/drivers/md/dm-log-userspace-transfer.c b/drivers/md/dm-log-userspace-transfer.c
index ba0edad..556131f 100644
--- a/drivers/md/dm-log-userspace-transfer.c
+++ b/drivers/md/dm-log-userspace-transfer.c
@@ -129,7 +129,7 @@ static int fill_pkg(struct cn_msg *msg, struct dm_ulog_request *tfr)
  * This is the connector callback that delivers data
  * that was sent from userspace.
  */
-static void cn_ulog_callback(void *data)
+static void cn_ulog_callback(void *data, struct netlink_skb_parms *nsp)
 {
 	struct cn_msg *msg = (struct cn_msg *)data;
 	struct dm_ulog_request *tfr = (struct dm_ulog_request *)(msg + 1);
diff --git a/drivers/staging/dst/dcore.c b/drivers/staging/dst/dcore.c
index ac85773..3943c91 100644
--- a/drivers/staging/dst/dcore.c
+++ b/drivers/staging/dst/dcore.c
@@ -847,7 +847,7 @@ static dst_command_func dst_commands[] = {
 /*
  * Configuration parser.
  */
-static void cn_dst_callback(struct cn_msg *msg)
+static void cn_dst_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
 {
 	struct dst_ctl *ctl;
 	int err;
diff --git a/drivers/staging/pohmelfs/config.c b/drivers/staging/pohmelfs/config.c
index 90f962e..c9162b3 100644
--- a/drivers/staging/pohmelfs/config.c
+++ b/drivers/staging/pohmelfs/config.c
@@ -527,7 +527,7 @@ out_unlock:
 	return err;
 }
 
-static void pohmelfs_cn_callback(struct cn_msg *msg)
+static void pohmelfs_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
 {
 	int err;
 
diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index e98baf6..aa7cd95 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -67,7 +67,7 @@ static DEFINE_MUTEX(uvfb_lock);
  * find the kernel part of the task struct, copy the registers and
  * the buffer contents and then complete the task.
  */
-static void uvesafb_cn_callback(struct cn_msg *msg)
+static void uvesafb_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
 {
 	struct uvesafb_task *utask;
 	struct uvesafb_ktask *task;
diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index 52ccb3d..45c126f 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -306,7 +306,7 @@ static int w1_netlink_send_error(struct cn_msg *rcmsg, struct w1_netlink_msg *rm
 	return error;
 }
 
-static void w1_cn_callback(struct cn_msg *msg)
+static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
 {
 	struct w1_netlink_msg *m = (struct w1_netlink_msg *)(msg + 1);
 	struct w1_netlink_cmd *cmd;
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 05a7a14..545728e 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -136,7 +136,7 @@ struct cn_callback_data {
 	void *ddata;
 
 	struct sk_buff *skb;
-	void (*callback) (struct cn_msg *);
+	void (*callback) (struct cn_msg *, struct netlink_skb_parms *);
 
 	void *free;
 };
@@ -167,11 +167,11 @@ struct cn_dev {
 	struct cn_queue_dev *cbdev;
 };
 
-int cn_add_callback(struct cb_id *, char *, void (*callback) (struct cn_msg *));
+int cn_add_callback(struct cb_id *, char *, void (*callback) (struct cn_msg *, struct netlink_skb_parms *));
 void cn_del_callback(struct cb_id *);
 int cn_netlink_send(struct cn_msg *, u32, gfp_t);
 
-int cn_queue_add_callback(struct cn_queue_dev *dev, char *name, struct cb_id *id, void (*callback)(struct cn_msg *));
+int cn_queue_add_callback(struct cn_queue_dev *dev, char *name, struct cb_id *id, void (*callback)(struct cn_msg *, struct netlink_skb_parms *));
 void cn_queue_del_callback(struct cn_queue_dev *dev, struct cb_id *id);
 
 int queue_cn_work(struct cn_callback_entry *cbq, struct work_struct *work);
-- 
1.6.0.4

^ permalink raw reply related

* [PATCH 1/8] connector: Keep the skb in cn_callback_data
From: Philipp Reisner @ 2009-10-02 12:40 UTC (permalink / raw)
  To: linux-kernel, netdev, Andrew Morton, David S. Miller, Greg KH
  Cc: dm-devel, Evgeniy Polyakov, linux-fbdev-devel, Philipp Reisner
In-Reply-To: <1254487211-11810-1-git-send-email-philipp.reisner@linbit.com>

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Acked-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
---
 drivers/connector/cn_queue.c  |    3 ++-
 drivers/connector/connector.c |   11 +++++------
 include/linux/connector.h     |    4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
index 4a1dfe1..b4cfac9 100644
--- a/drivers/connector/cn_queue.c
+++ b/drivers/connector/cn_queue.c
@@ -78,8 +78,9 @@ void cn_queue_wrapper(struct work_struct *work)
 	struct cn_callback_entry *cbq =
 		container_of(work, struct cn_callback_entry, work);
 	struct cn_callback_data *d = &cbq->data;
+	struct cn_msg *msg = NLMSG_DATA(nlmsg_hdr(d->skb));
 
-	d->callback(d->callback_priv);
+	d->callback(msg);
 
 	d->destruct_data(d->ddata);
 	d->ddata = NULL;
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 74f52af..fc9887f 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -129,10 +129,11 @@ EXPORT_SYMBOL_GPL(cn_netlink_send);
 /*
  * Callback helper - queues work and setup destructor for given data.
  */
-static int cn_call_callback(struct cn_msg *msg, void (*destruct_data)(void *), void *data)
+static int cn_call_callback(struct sk_buff *skb, void (*destruct_data)(void *), void *data)
 {
 	struct cn_callback_entry *__cbq, *__new_cbq;
 	struct cn_dev *dev = &cdev;
+	struct cn_msg *msg = NLMSG_DATA(nlmsg_hdr(skb));
 	int err = -ENODEV;
 
 	spin_lock_bh(&dev->cbdev->queue_lock);
@@ -140,7 +141,7 @@ static int cn_call_callback(struct cn_msg *msg, void (*destruct_data)(void *), v
 		if (cn_cb_equal(&__cbq->id.id, &msg->id)) {
 			if (likely(!work_pending(&__cbq->work) &&
 					__cbq->data.ddata == NULL)) {
-				__cbq->data.callback_priv = msg;
+				__cbq->data.skb = skb;
 
 				__cbq->data.ddata = data;
 				__cbq->data.destruct_data = destruct_data;
@@ -156,7 +157,7 @@ static int cn_call_callback(struct cn_msg *msg, void (*destruct_data)(void *), v
 				__new_cbq = kzalloc(sizeof(struct cn_callback_entry), GFP_ATOMIC);
 				if (__new_cbq) {
 					d = &__new_cbq->data;
-					d->callback_priv = msg;
+					d->skb = skb;
 					d->callback = __cbq->data.callback;
 					d->ddata = data;
 					d->destruct_data = destruct_data;
@@ -191,7 +192,6 @@ static int cn_call_callback(struct cn_msg *msg, void (*destruct_data)(void *), v
  */
 static void cn_rx_skb(struct sk_buff *__skb)
 {
-	struct cn_msg *msg;
 	struct nlmsghdr *nlh;
 	int err;
 	struct sk_buff *skb;
@@ -208,8 +208,7 @@ static void cn_rx_skb(struct sk_buff *__skb)
 			return;
 		}
 
-		msg = NLMSG_DATA(nlh);
-		err = cn_call_callback(msg, (void (*)(void *))kfree_skb, skb);
+		err = cn_call_callback(skb, (void (*)(void *))kfree_skb, skb);
 		if (err < 0)
 			kfree_skb(skb);
 	}
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 47ebf41..05a7a14 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -134,8 +134,8 @@ struct cn_callback_id {
 struct cn_callback_data {
 	void (*destruct_data) (void *);
 	void *ddata;
-	
-	void *callback_priv;
+
+	struct sk_buff *skb;
 	void (*callback) (struct cn_msg *);
 
 	void *free;
-- 
1.6.0.4

^ permalink raw reply related

* Re: [RFC take2] pkt_sched: gen_estimator: Dont report fake rate estimators
From: Eric Dumazet @ 2009-10-02 12:39 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: David Miller, kaber, netdev
In-Reply-To: <20091002112514.GA14100@ff.dom.local>

Jarek Poplawski a écrit :

> So you prefer the additional parameter version, but since these
> _active tests are not needed e.g. for HTB classes, which got it
> active by default, so maybe bstats == NULL would let skip such a test?
> 
> ...
>> --- a/include/net/gen_stats.h
>> +++ b/include/net/gen_stats.h
>> @@ -30,6 +30,7 @@ extern int gnet_stats_start_copy_compat(struct sk_buff *skb, int type,
>>  extern int gnet_stats_copy_basic(struct gnet_dump *d,
>>  				 struct gnet_stats_basic_packed *b);
>>  extern int gnet_stats_copy_rate_est(struct gnet_dump *d,
>> +				    const struct gnet_stats_basic_packed *bstats,
> 
> It seems these *b/*bstats defs could look more consistent. Otherwise
> it looks OK to me.

Agreed, here is the updated version, I added your Signoff if you dont mind :)

[RFC] pkt_sched: gen_estimator: Dont report fake rate estimators

We currently send TCA_STATS_RATE_EST elements to netlink users, even if no estimator
is running.

# tc -s -d qdisc
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 112833764978 bytes 1495081739 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0

User has no way to tell if the "rate 0bit 0pps" is a real estimation, or a fake
one (because no estimator is active)

After this patch, tc command output is :
$ tc -s -d qdisc
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 561075 bytes 1196 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0

We add a parameter to gnet_stats_copy_rate_est() function so that
it can use gen_estimator_active(bstats, r), as suggested by Jarek.

This parameter can be NULL if check is not necessary, (htb for
example has a mandatory rate estimator)


Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
---
 include/net/gen_stats.h |    1 +
 net/core/gen_stats.c    |    7 ++++++-
 net/sched/act_api.c     |    2 +-
 net/sched/sch_api.c     |    2 +-
 net/sched/sch_cbq.c     |    2 +-
 net/sched/sch_drr.c     |    2 +-
 net/sched/sch_hfsc.c    |    2 +-
 net/sched/sch_htb.c     |    2 +-
 8 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/net/gen_stats.h b/include/net/gen_stats.h
index c148855..eb87a14 100644
--- a/include/net/gen_stats.h
+++ b/include/net/gen_stats.h
@@ -30,6 +30,7 @@ extern int gnet_stats_start_copy_compat(struct sk_buff *skb, int type,
 extern int gnet_stats_copy_basic(struct gnet_dump *d,
 				 struct gnet_stats_basic_packed *b);
 extern int gnet_stats_copy_rate_est(struct gnet_dump *d,
+				    const struct gnet_stats_basic_packed *b,
 				    struct gnet_stats_rate_est *r);
 extern int gnet_stats_copy_queue(struct gnet_dump *d,
 				 struct gnet_stats_queue *q);
diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
index 8569310..054a49c 100644
--- a/net/core/gen_stats.c
+++ b/net/core/gen_stats.c
@@ -136,8 +136,13 @@ gnet_stats_copy_basic(struct gnet_dump *d, struct gnet_stats_basic_packed *b)
  * if the room in the socket buffer was not sufficient.
  */
 int
-gnet_stats_copy_rate_est(struct gnet_dump *d, struct gnet_stats_rate_est *r)
+gnet_stats_copy_rate_est(struct gnet_dump *d,
+			 const struct gnet_stats_basic_packed *b,
+			 struct gnet_stats_rate_est *r)
 {
+	if (b && !gen_estimator_active(b, r))
+		return 0;
+
 	if (d->compat_tc_stats) {
 		d->tc_stats.bps = r->bps;
 		d->tc_stats.pps = r->pps;
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 2dfb3e7..2b0d5ee 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -618,7 +618,7 @@ int tcf_action_copy_stats(struct sk_buff *skb, struct tc_action *a,
 			goto errout;
 
 	if (gnet_stats_copy_basic(&d, &h->tcf_bstats) < 0 ||
-	    gnet_stats_copy_rate_est(&d, &h->tcf_rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(&d, &h->tcf_bstats, &h->tcf_rate_est) < 0 ||
 	    gnet_stats_copy_queue(&d, &h->tcf_qstats) < 0)
 		goto errout;
 
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 903e418..1acfd29 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1179,7 +1179,7 @@ static int tc_fill_qdisc(struct sk_buff *skb, struct Qdisc *q, u32 clid,
 		goto nla_put_failure;
 
 	if (gnet_stats_copy_basic(&d, &q->bstats) < 0 ||
-	    gnet_stats_copy_rate_est(&d, &q->rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(&d, &q->bstats, &q->rate_est) < 0 ||
 	    gnet_stats_copy_queue(&d, &q->qstats) < 0)
 		goto nla_put_failure;
 
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 5b132c4..3846d65 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1609,7 +1609,7 @@ cbq_dump_class_stats(struct Qdisc *sch, unsigned long arg,
 		cl->xstats.undertime = cl->undertime - q->now;
 
 	if (gnet_stats_copy_basic(d, &cl->bstats) < 0 ||
-	    gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(d, &cl->bstats, &cl->rate_est) < 0 ||
 	    gnet_stats_copy_queue(d, &cl->qstats) < 0)
 		return -1;
 
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index 5a888af..a65604f 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -280,7 +280,7 @@ static int drr_dump_class_stats(struct Qdisc *sch, unsigned long arg,
 	}
 
 	if (gnet_stats_copy_basic(d, &cl->bstats) < 0 ||
-	    gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(d, &cl->bstats, &cl->rate_est) < 0 ||
 	    gnet_stats_copy_queue(d, &cl->qdisc->qstats) < 0)
 		return -1;
 
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 2c5c76b..b38b39c 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1375,7 +1375,7 @@ hfsc_dump_class_stats(struct Qdisc *sch, unsigned long arg,
 	xstats.rtwork  = cl->cl_cumul;
 
 	if (gnet_stats_copy_basic(d, &cl->bstats) < 0 ||
-	    gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(d, &cl->bstats, &cl->rate_est) < 0 ||
 	    gnet_stats_copy_queue(d, &cl->qstats) < 0)
 		return -1;
 
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 85acab9..2e38d1a 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1105,7 +1105,7 @@ htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d)
 	cl->xstats.ctokens = cl->ctokens;
 
 	if (gnet_stats_copy_basic(d, &cl->bstats) < 0 ||
-	    gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(d, NULL, &cl->rate_est) < 0 ||
 	    gnet_stats_copy_queue(d, &cl->qstats) < 0)
 		return -1;
 

^ permalink raw reply related

* Re: Network hangs with 2.6.30.5
From: Eric Dumazet @ 2009-10-02 12:38 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: David Miller, jarkao2, holger.hoffstaette, Netdev,
	Evgeniy Polyakov
In-Reply-To: <alpine.DEB.2.00.0910021520280.13543@wel-95.cs.helsinki.fi>

Ilpo Järvinen a écrit :
> On Fri, 2 Oct 2009, Ilpo Järvinen wrote:
> 
>> On Thu, 1 Oct 2009, David Miller wrote:
>>
>>> From: Jarek Poplawski <jarkao2@gmail.com>
>>> Date: Mon, 7 Sep 2009 07:21:43 +0000
>>>
>>>> While Eric is analyzing your data, I guess you could try reverting
>>>> some stuff around this tcp_tw_recycle, and my tcp ignorance would
>>>> point these commits for the beginning:
>>>>
>>>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.30.y.git;a=commitdiff;h=fc1ad92dfc4e363a055053746552cdb445ba5c57
>>>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.30.y.git;a=commitdiff;h=c887e6d2d9aee56ee7c9f2af4cec3a5efdcc4c72
>>> Ilpo's cleanup (the second commit listed) looks most likely to
>>> be a possibility.
>>>
>>> But I surely cannot find any bugs in it, even after studying it
>>> a few times.
>>>
>>> Ilpo could you audit it one more time for us just in case?
>> Argh, not that one ...the jungle of negations. But I'll try to go it 
>> through once more but I tell you I did go through those negations multiple 
>> times already before submitting it :-).
>>
>>> I also looked through all the TCP commits in 2.6.29 to 2.6.30
>>> and I could not find anything else that might cause stalls with
>>> time-wait recycled connections.
>> What about the more than 64k connections change a9d8f9110d7e953c2f2 (or 
>> its fixes), it might be another possibility? ...It certainly does 
>> something related to reuse and happens to be in the correct time frame... 
>> (I've added Evgeniy).

I scratched my head to reproduce the conditions of hang but failed.

I am pretty sure both commits are OK (yours and mine), maybe a brute force
git bisection is needed.


^ permalink raw reply

* Re: Network hangs with 2.6.30.5
From: Ilpo Järvinen @ 2009-10-02 12:29 UTC (permalink / raw)
  To: David Miller
  Cc: jarkao2, holger.hoffstaette, Netdev, eric.dumazet,
	Evgeniy Polyakov
In-Reply-To: <alpine.DEB.2.00.0910021104130.13543@wel-95.cs.helsinki.fi>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5364 bytes --]

On Fri, 2 Oct 2009, Ilpo Järvinen wrote:

> On Thu, 1 Oct 2009, David Miller wrote:
> 
> > From: Jarek Poplawski <jarkao2@gmail.com>
> > Date: Mon, 7 Sep 2009 07:21:43 +0000
> > 
> > > While Eric is analyzing your data, I guess you could try reverting
> > > some stuff around this tcp_tw_recycle, and my tcp ignorance would
> > > point these commits for the beginning:
> > > 
> > > http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.30.y.git;a=commitdiff;h=fc1ad92dfc4e363a055053746552cdb445ba5c57
> > > http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.30.y.git;a=commitdiff;h=c887e6d2d9aee56ee7c9f2af4cec3a5efdcc4c72
> > 
> > Ilpo's cleanup (the second commit listed) looks most likely to
> > be a possibility.
> > 
> > But I surely cannot find any bugs in it, even after studying it
> > a few times.
> > 
> > Ilpo could you audit it one more time for us just in case?
> 
> Argh, not that one ...the jungle of negations. But I'll try to go it 
> through once more but I tell you I did go through those negations multiple 
> times already before submitting it :-).
> 
> > I also looked through all the TCP commits in 2.6.29 to 2.6.30
> > and I could not find anything else that might cause stalls with
> > time-wait recycled connections.
> 
> What about the more than 64k connections change a9d8f9110d7e953c2f2 (or 
> its fixes), it might be another possibility? ...It certainly does 
> something related to reuse and happens to be in the correct time frame... 
> (I've added Evgeniy).

Here's my full analysis:

> c887e6d2d9aee56ee7c9f2af4cec3a5efdcc4c72
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index d74ac30..255ca35 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -997,11 +997,21 @@ static inline int tcp_fin_time(const struct sock *sk)
>  	return fin_timeout;
>  }
>  
> -static inline int tcp_paws_check(const struct tcp_options_received *rx_opt, int rst)
> +static inline int tcp_paws_check(const struct tcp_options_received *rx_opt,
> +				 int paws_win)
>  {
> -	if ((s32)(rx_opt->rcv_tsval - rx_opt->ts_recent) >= 0)
> -		return 0;
> -	if (get_seconds() >= rx_opt->ts_recent_stamp + TCP_PAWS_24DAYS)
> +	if ((s32)(rx_opt->ts_recent - rx_opt->rcv_tsval) <= paws_win)
> +		return 1;
> +	if (unlikely(get_seconds() >= rx_opt->ts_recent_stamp + TCP_PAWS_24DAYS))
> +		return 1;
> +
> +	return 0;
> +}
> +
> +static inline int tcp_paws_reject(const struct tcp_options_received *rx_opt,
> +				  int rst)
> +{
> +	if (tcp_paws_check(rx_opt, 0))
>  		return 0;

First condition is * -1 to switch subtraction terms around (and reverses
inequality). The other condition is very much the same. In addition, it 
has an extra negation round but still OK.

>  
>  	/* RST segments are not recommended to carry timestamp,
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index f527a16..b7d02c5 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3883,8 +3883,7 @@ static inline void tcp_replace_ts_recent(struct tcp_sock *tp, u32 seq)
>  		 * Not only, also it occurs for expired timestamps.
>  		 */
>  
> -		if ((s32)(tp->rx_opt.rcv_tsval - tp->rx_opt.ts_recent) >= 0 ||

* -1 here too.

> -		   get_seconds() >= tp->rx_opt.ts_recent_stamp + TCP_PAWS_24DAYS)

The very same condition.

> +		if (tcp_paws_check(&tp->rx_opt, 0))
>  			tcp_store_ts_recent(tp);
>  	}
>  }
> @@ -3936,9 +3935,9 @@ static inline int tcp_paws_discard(const struct sock *sk,
>  				   const struct sk_buff *skb)
>  {
>  	const struct tcp_sock *tp = tcp_sk(sk);
> -	return ((s32)(tp->rx_opt.ts_recent - tp->rx_opt.rcv_tsval) > TCP_PAWS_WINDOW &&
> -		get_seconds() < tp->rx_opt.ts_recent_stamp + TCP_PAWS_24DAYS &&
> -		!tcp_disordered_ack(sk, skb));
> +
> +	return !tcp_paws_check(&tp->rx_opt, TCP_PAWS_WINDOW) &&

DeMorgan: 

   (a > b) &&  (c < d) 
          <==>
!(!(a > b) || !(c < d))
          <==>
!((a <= b) || (c >= d))

> +	       !tcp_disordered_ack(sk, skb);
>  }
>  
>  /* Check segment sequence number for validity.
> @@ -5513,7 +5512,7 @@ discard:
>  
>  	/* PAWS check. */
>  	if (tp->rx_opt.ts_recent_stamp && tp->rx_opt.saw_tstamp &&
> -	    tcp_paws_check(&tp->rx_opt, 0))
> +	    tcp_paws_reject(&tp->rx_opt, 0))

A plain rename, the rest likewise.

>  		goto discard_and_undo;
>  
>  	if (th->syn) {
> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index 4b0df3e..43bbba7 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -107,7 +107,7 @@ tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb,
>  		if (tmp_opt.saw_tstamp) {
>  			tmp_opt.ts_recent	= tcptw->tw_ts_recent;
>  			tmp_opt.ts_recent_stamp	= tcptw->tw_ts_recent_stamp;
> -			paws_reject = tcp_paws_check(&tmp_opt, th->rst);
> +			paws_reject = tcp_paws_reject(&tmp_opt, th->rst);
>  		}
>  	}
>  
> @@ -511,7 +511,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
>  			 * from another data.
>  			 */
>  			tmp_opt.ts_recent_stamp = get_seconds() - ((TCP_TIMEOUT_INIT/HZ)<<req->retrans);
> -			paws_reject = tcp_paws_check(&tmp_opt, th->rst);
> +			paws_reject = tcp_paws_reject(&tmp_opt, th->rst);
>  		}
>  	}
>  

...Which concludes the patch innocent. ...I certainly won't regret this 
cleanup after having to figure that mess out once again - that is to say,
hopefully for the last time :-). ...Sadly the problem remains.

-- 
 i.

^ permalink raw reply

* Re: [PATCH 1/4] qeth: Convert ethtool get_stats_count() ops to get_sset_count()
From: Frank Blaschka @ 2009-10-02 12:13 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: David Miller, Ursula Braun, Frank Blaschka, linux-s390, netdev
In-Reply-To: <1254432272.2735.20.camel@achroite>

works fine, thanks a lot here is my ACK

Ben Hutchings schrieb:
> This string query operation was supposed to be replaced by the
> generic get_sset_count() starting in 2007.  Convert qeth's
> implementation.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> This is not even compile-tested because I don't have an s390 compiler.
> But it's simple enough that I think I got it right...
> 
> Ben.
> 
>  drivers/s390/net/qeth_core.h      |    2 +-
>  drivers/s390/net/qeth_core_main.c |   11 ++++++++---
>  drivers/s390/net/qeth_l2_main.c   |    4 ++--
>  drivers/s390/net/qeth_l3_main.c   |    2 +-
>  4 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
> index 31a2b4e..e8f72d7 100644
> --- a/drivers/s390/net/qeth_core.h
> +++ b/drivers/s390/net/qeth_core.h
> @@ -849,7 +849,7 @@ int qeth_do_send_packet_fast(struct qeth_card *, struct qeth_qdio_out_q *,
>  			struct sk_buff *, struct qeth_hdr *, int, int, int);
>  int qeth_do_send_packet(struct qeth_card *, struct qeth_qdio_out_q *,
>  		    struct sk_buff *, struct qeth_hdr *, int);
> -int qeth_core_get_stats_count(struct net_device *);
> +int qeth_core_get_sset_count(struct net_device *, int);
>  void qeth_core_get_ethtool_stats(struct net_device *,
>  				struct ethtool_stats *, u64 *);
>  void qeth_core_get_strings(struct net_device *, u32, u8 *);
> diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
> index c4a42d9..edee4dc 100644
> --- a/drivers/s390/net/qeth_core_main.c
> +++ b/drivers/s390/net/qeth_core_main.c
> @@ -4305,11 +4305,16 @@ static struct {
>  	{"tx csum"},
>  };
> 
> -int qeth_core_get_stats_count(struct net_device *dev)
> +int qeth_core_get_sset_count(struct net_device *dev, int stringset)
>  {
> -	return (sizeof(qeth_ethtool_stats_keys) / ETH_GSTRING_LEN);
> +	switch (stringset) {
> +	case ETH_SS_STATS:
> +		return (sizeof(qeth_ethtool_stats_keys) / ETH_GSTRING_LEN);
> +	default:
> +		return -EINVAL;
> +	}
>  }
> -EXPORT_SYMBOL_GPL(qeth_core_get_stats_count);
> +EXPORT_SYMBOL_GPL(qeth_core_get_sset_count);
> 
>  void qeth_core_get_ethtool_stats(struct net_device *dev,
>  		struct ethtool_stats *stats, u64 *data)
> diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
> index f4f3ca1..b61d5c7 100644
> --- a/drivers/s390/net/qeth_l2_main.c
> +++ b/drivers/s390/net/qeth_l2_main.c
> @@ -866,7 +866,7 @@ static const struct ethtool_ops qeth_l2_ethtool_ops = {
>  	.get_link = ethtool_op_get_link,
>  	.get_strings = qeth_core_get_strings,
>  	.get_ethtool_stats = qeth_core_get_ethtool_stats,
> -	.get_stats_count = qeth_core_get_stats_count,
> +	.get_sset_count = qeth_core_get_sset_count,
>  	.get_drvinfo = qeth_core_get_drvinfo,
>  	.get_settings = qeth_core_ethtool_get_settings,
>  };
> @@ -874,7 +874,7 @@ static const struct ethtool_ops qeth_l2_ethtool_ops = {
>  static const struct ethtool_ops qeth_l2_osn_ops = {
>  	.get_strings = qeth_core_get_strings,
>  	.get_ethtool_stats = qeth_core_get_ethtool_stats,
> -	.get_stats_count = qeth_core_get_stats_count,
> +	.get_sset_count = qeth_core_get_sset_count,
>  	.get_drvinfo = qeth_core_get_drvinfo,
>  };
> 
> diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
> index 073b6d3..4ca28c1 100644
> --- a/drivers/s390/net/qeth_l3_main.c
> +++ b/drivers/s390/net/qeth_l3_main.c
> @@ -2957,7 +2957,7 @@ static const struct ethtool_ops qeth_l3_ethtool_ops = {
>  	.set_tso     = qeth_l3_ethtool_set_tso,
>  	.get_strings = qeth_core_get_strings,
>  	.get_ethtool_stats = qeth_core_get_ethtool_stats,
> -	.get_stats_count = qeth_core_get_stats_count,
> +	.get_sset_count = qeth_core_get_sset_count,
>  	.get_drvinfo = qeth_core_get_drvinfo,
>  	.get_settings = qeth_core_ethtool_get_settings,
>  };
> 



^ permalink raw reply

* Re: Messages are printed on screen
From: Markus Feldmann @ 2009-10-02 12:01 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1254480996.23350.73.camel@localhost>

Ben Hutchings schrieb:
> On Fri, 2009-10-02 at 11:52 +0200, Markus Feldmann wrote:
>
>>
>> As you see some of my IRQ-Lines are multiply in use, so my Server is 
>> working hard at his limit.
> 
> IRQ sharing is normal on PCs without MSI support, but to see where
> that's happening you need to look at /proc/interrupts and not the BIOS
> setup program or wherever you got the above information from.
Ok i did <cat /proc/interrupts> and got:
            CPU0
   0:     259603    XT-PIC-XT        timer
   1:       1421    XT-PIC-XT        i8042
   2:          0    XT-PIC-XT        cascade
   4:     200000    XT-PIC-XT        ohci_hcd:usb3, pppp0
   5:          0    XT-PIC-XT        ehci_hcd:usb1, lan0
   7:       6959    XT-PIC-XT        lan1
   8:          2    XT-PIC-XT        rtc0
   9:          0    XT-PIC-XT        acpi
  11:      37697    XT-PIC-XT        ide2, ide3, ohci_hcd:usb2, lan2
  14:          0    XT-PIC-XT        ide0
NMI:          0   Non-maskable interrupts
TRM:          0   Thermal event interrupts
MCE:          0   Machine check exceptions
MCP:         13   Machine check polls
ERR:          2

How can i assigned IRQs during Boot?

How can i watch which IRQ Line has most traffic or problems ?

>> The result is sometimes freezing of my 
>> Server, especially if there is much processing on these devices. I 
>> remember that with Kernel 2.6.18 my system didn't does freezing.
> 
> This is simply a bug, not a result of IRQ sharing or 'working hard'.
But something had initiated this freezing. Although i do not know the 
Bug, i should be able to avoide this Problem by do some prevention ?!

> 
>> How can i disable the output of messages (about dropped packets from my 
>> firewall) to my terminal ?
> 
> Edit the value of kernel.printk in /etc/sysctl.conf.
Ok i did add:
kernel.printk= 4 4 1 7
to </etc/sysctl.conf>
> 
>> How can i stabilize my IRQ-System with the kernel 2.6.31.1 ?
> 
> I would expect the standard kernel version for 'lenny' or the 2.6.30
> kernel from 'sid' to be more stable.
Ok i will try the kernel from Debian Sid. :-)
> 
>> What debug features should i disable ?
> 
> No idea, you didn't even specify what you enabled...
I will add some enabled features next week.

regards Markus


^ permalink raw reply

* Re: [RFC take2] pkt_sched: gen_estimator: Dont report fake rate estimators
From: Jarek Poplawski @ 2009-10-02 11:25 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, kaber, netdev
In-Reply-To: <4AC5D78D.3030400@gmail.com>

On Fri, Oct 02, 2009 at 12:35:57PM +0200, Eric Dumazet wrote:
> Here is second attempt to make this change, thanks Jarek !
> 
> This is indeed less intrusive !
> 
> [RFC] pkt_sched: gen_estimator: Dont report fake rate estimators
> 
> We currently send TCA_STATS_RATE_EST elements to netlink users, even if no estimator
> is running.
> 
> # tc -s -d qdisc
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 112833764978 bytes 1495081739 pkt (dropped 0, overlimits 0 requeues 0)
>  rate 0bit 0pps backlog 0b 0p requeues 0
> 
> User has no way to tell if the "rate 0bit 0pps" is a real estimation, or a fake
> one (because no estimator is active)
> 
> After this patch, tc command output is :
> $ tc -s -d qdisc
> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 561075 bytes 1196 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
> 
> We add a parameter to gnet_stats_copy_rate_est() function so that
> it can use gen_estimator_active(bstats, r), as suggested by Jarek.

So you prefer the additional parameter version, but since these
_active tests are not needed e.g. for HTB classes, which got it
active by default, so maybe bstats == NULL would let skip such a test?

...
> --- a/include/net/gen_stats.h
> +++ b/include/net/gen_stats.h
> @@ -30,6 +30,7 @@ extern int gnet_stats_start_copy_compat(struct sk_buff *skb, int type,
>  extern int gnet_stats_copy_basic(struct gnet_dump *d,
>  				 struct gnet_stats_basic_packed *b);
>  extern int gnet_stats_copy_rate_est(struct gnet_dump *d,
> +				    const struct gnet_stats_basic_packed *bstats,

It seems these *b/*bstats defs could look more consistent. Otherwise
it looks OK to me.

Thanks,
Jarek P.

^ permalink raw reply

* Re: [BUG net-2.6] bluetooth/rfcomm : sleeping function called from invalid context at mm/slub.c:1719
From: Dave Young @ 2009-10-02 11:01 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: Marcel Holtmann, Linux Netdev List, linux-bluetooth
In-Reply-To: <4AC59D8A.6000102@hartkopp.net>

On Fri, Oct 2, 2009 at 2:28 PM, Oliver Hartkopp <oliver@hartkopp.net> wrote:
> Hello Marcel,
>
> with current net-2.6 tree ...
>
> While starting my PPP Bluetooth dialup networking, i got this:

Hi, oliver

please try following patch:
http://patchwork.kernel.org/patch/51326/

>
> [  722.461549] PPP generic driver version 2.4.2
> [  722.477519] BUG: sleeping function called from invalid context at
> mm/slub.c:1719
> [  722.477530] in_atomic(): 1, irqs_disabled(): 0, pid: 4677, name: pppd
> [  722.477537] 3 locks held by pppd/4677:
> [  722.477542]  #0:  (rfcomm_mutex){+.+.+.}, at: [<fa5df2a1>]
> rfcomm_dlc_open+0x28/0x2d6 [rfcomm]
> [  722.477568]  #1:  (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.+.}, at:
> [<fa5414f8>] l2cap_sock_connect+0x62/0x2c6 [l2cap]
> [  722.477589]  #2:  (&hdev->lock){+...+.}, at: [<fa5415b4>]
> l2cap_sock_connect+0x11e/0x2c6 [l2cap]
> [  722.477613] Pid: 4677, comm: pppd Not tainted 2.6.31-08939-gdb8abec-dirty #21
> [  722.477619] Call Trace:
> [  722.477633]  [<c1042a2b>] ? __debug_show_held_locks+0x1e/0x20
> [  722.477644]  [<c10212a1>] __might_sleep+0xc9/0xce
> [  722.477655]  [<c1078b62>] __kmalloc+0x6d/0xfb
> [  722.477666]  [<c119e739>] ? kzalloc+0xb/0xd
> [  722.477674]  [<c119e739>] kzalloc+0xb/0xd
> [  722.477683]  [<c119ef1a>] device_private_init+0x15/0x3d
> [  722.477693]  [<c11a0e1b>] dev_set_drvdata+0x18/0x26
> [  722.477718]  [<f8b7ca1b>] hci_conn_init_sysfs+0x3d/0xc7 [bluetooth]
> [  722.477737]  [<f8b791b3>] hci_conn_add+0x1c0/0x1d5 [bluetooth]
> [  722.477756]  [<f8b79360>] hci_connect+0x71/0x17d [bluetooth]
> [  722.477769]  [<fa54162c>] l2cap_sock_connect+0x196/0x2c6 [l2cap]
> [  722.477782]  [<c1246e3d>] kernel_connect+0xd/0x12
> [  722.477795]  [<fa5df3c3>] rfcomm_dlc_open+0x14a/0x2d6 [rfcomm]
> [  722.477810]  [<fa5e10fa>] ? rfcomm_tty_open+0x73/0x227 [rfcomm]
> [  722.477825]  [<fa5e1130>] rfcomm_tty_open+0xa9/0x227 [rfcomm]
> [  722.477836]  [<c1022e3f>] ? default_wake_function+0x0/0xd
> [  722.477847]  [<c1180c79>] tty_open+0x29e/0x399
> [  722.477858]  [<c107e9bd>] chrdev_open+0x13f/0x156
> [  722.477868]  [<c107b0d3>] __dentry_open+0x11b/0x20f
> [  722.477878]  [<c107b261>] nameidata_to_filp+0x2c/0x43
> [  722.477888]  [<c107e87e>] ? chrdev_open+0x0/0x156
> [  722.477898]  [<c1084e9e>] do_filp_open+0x3c6/0x70a
> [  722.477910]  [<c108d3e4>] ? alloc_fd+0xc8/0xd2
> [  722.477920]  [<c108d3e4>] ? alloc_fd+0xc8/0xd2
> [  722.477930]  [<c107aebc>] do_sys_open+0x4a/0xe7
> [  722.477940]  [<c1002acc>] ? restore_all_notrace+0x0/0x18
> [  722.477950]  [<c107af9b>] sys_open+0x1e/0x26
> [  722.477959]  [<c1002a18>] sysenter_do_call+0x12/0x36
> [  729.658613] PPP BSD Compression module registered
> [  729.684789] PPP Deflate Compression module registered
>
> Any idea?
>
> Regards,
> Oliver
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Regards
dave

^ permalink raw reply

* Re: [RFC][PATCH] ethtool: Add reset operation
From: Ben Hutchings @ 2009-10-02 11:00 UTC (permalink / raw)
  To: Ajit Khaparde; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <20091002104010.GA19862@serverengines.com>

On Fri, 2009-10-02 at 16:10 +0530, Ajit Khaparde wrote:
[...]
> Can you tell the intention behind this copy_to_user?
> Do you envision drivers sending back some data to the userland - may be
> sometime in future?

This allows userland to see which components were actually reset.

Ben.

-- 
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: Messages are printed on screen
From: Markus Feldmann @ 2009-10-02 10:56 UTC (permalink / raw)
  To: netdev
In-Reply-To: <ha4igd$ghh$1@ger.gmane.org>

Markus Feldmann schrieb:
> ....
> my <rsyslog> to save this only to </var/log>. Here is my 
> </etc/rsyslog.conf>
http://pastebin.com/m4400fb9e



^ permalink raw reply

* Re: Messages are printed on screen
From: Ben Hutchings @ 2009-10-02 10:56 UTC (permalink / raw)
  To: Markus Feldmann; +Cc: netdev
In-Reply-To: <ha4igd$ghh$1@ger.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2380 bytes --]

On Fri, 2009-10-02 at 11:52 +0200, Markus Feldmann wrote:
> Hi All,
> 
> i am setting up my Server, with Linux Debian lenny. Therefore i am using 
>   Kernel 2.6.31.1.

The current kernel version for 'lenny' is 2.6.26 (with stable updates
and other fixes).

2.6.31 is known to have a large number of regressions outstanding, which
is why it is not in Debian yet.

> I configured this Kernel with <make defconfig> and 
> <make menuconfig>. My motherboard has 5 PCI slots and 1 AGP slot. All 
> slots are in use. All devices are mapped to the following IRQ-Line:
> 
> Mass Storage Device (PCI Slot1)	IRQ-Line 11
> Ethernet (PCI Slot 2) 		IRQ-Line 4
> Ethernet (PCI Slot 3) 		IRQ-Line 5
> Ethernet (PCI Slot 4) 		IRQ-Line 7
> Ethernet (PCI Slot 5) 		IRQ-Line 11
> Onboard USB-Controller		IRQ-Line 5
> Onboard USB-Controller		IRQ-Line 4
> Onboard USB-Controller		IRQ-Line 11
> Onboard IDE			IRQ-Line 14
> AGP VGA				IRQ-Line 11
> 
> As you see some of my IRQ-Lines are multiply in use, so my Server is 
> working hard at his limit.

IRQ sharing is normal on PCs without MSI support, but to see where
that's happening you need to look at /proc/interrupts and not the BIOS
setup program or wherever you got the above information from.

This does not result in 'working hard at his limit'.

> The result is sometimes freezing of my 
> Server, especially if there is much processing on these devices. I 
> remember that with Kernel 2.6.18 my system didn't does freezing.

This is simply a bug, not a result of IRQ sharing or 'working hard'.

> So i am trying to reduce the amount of this processing. I still get 
> messages about dropped network packets on my Terminal, although i set up 
> my <rsyslog> to save this only to </var/log>. Here is my 
> </etc/rsyslog.conf>

You forgot to paste it.

> How can i disable the output of messages (about dropped packets from my 
> firewall) to my terminal ?

Edit the value of kernel.printk in /etc/sysctl.conf.

> How can i stabilize my IRQ-System with the kernel 2.6.31.1 ?

I would expect the standard kernel version for 'lenny' or the 2.6.30
kernel from 'sid' to be more stable.

> What debug features should i disable ?

No idea, you didn't even specify what you enabled...

Ben.

-- 
Ben Hutchings
Who are all these weirdos? - David Bowie, about L-Space IRC channel #afp

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: [RFC][PATCH] ethtool: Add reset operation
From: Ajit Khaparde @ 2009-10-02 10:40 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <1254426195.2735.16.camel@achroite>

On 01/10/09 20:43 +0100, Ben Hutchings wrote:
> After updating firmware stored in flash, users may wish to reset the
> relevant hardware and start the new firmware immediately.  This should
> not be completely automatic as it may be disruptive.
> 
> A selective reset may also be useful for debugging or diagnostics.
> 
> This adds a separate reset operation which takes flags indicating the
> components to be reset.  Drivers are allowed to reset only a subset of
> those requested, and must report the actual subset.  This allows the
> use of generic component masks and some future expansion.
> ---
Looks good. But one question.

> +static int ethtool_reset(struct net_device *dev, char __user *useraddr)
> +{
> +	struct ethtool_value reset;
> +	int ret;
> +
> +	if (!dev->ethtool_ops->reset)
> +		return -EOPNOTSUPP;
> +
> +	if (copy_from_user(&reset, useraddr, sizeof(reset)))
> +		return -EFAULT;
> +
> +	ret = dev->ethtool_ops->reset(dev, &reset.data);
> +	if (ret)
> +		return ret;
> +
> +	if (copy_to_user(useraddr, &reset, sizeof(reset)))
> +		return -EFAULT;
Can you tell the intention behind this copy_to_user?
Do you envision drivers sending back some data to the userland - may be
sometime in future?

Thanks
-Ajit

^ permalink raw reply

* [RFC take2] pkt_sched: gen_estimator: Dont report fake rate estimators
From: Eric Dumazet @ 2009-10-02 10:35 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: David Miller, kaber, netdev
In-Reply-To: <20091002070819.GA9694@ff.dom.local>

Here is second attempt to make this change, thanks Jarek !

This is indeed less intrusive !

[RFC] pkt_sched: gen_estimator: Dont report fake rate estimators

We currently send TCA_STATS_RATE_EST elements to netlink users, even if no estimator
is running.

# tc -s -d qdisc
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 112833764978 bytes 1495081739 pkt (dropped 0, overlimits 0 requeues 0)
 rate 0bit 0pps backlog 0b 0p requeues 0

User has no way to tell if the "rate 0bit 0pps" is a real estimation, or a fake
one (because no estimator is active)

After this patch, tc command output is :
$ tc -s -d qdisc
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 561075 bytes 1196 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0

We add a parameter to gnet_stats_copy_rate_est() function so that
it can use gen_estimator_active(bstats, r), as suggested by Jarek.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/gen_stats.h |    1 +
 net/core/gen_stats.c    |    7 ++++++-
 net/sched/act_api.c     |    2 +-
 net/sched/sch_api.c     |    2 +-
 net/sched/sch_cbq.c     |    2 +-
 net/sched/sch_drr.c     |    2 +-
 net/sched/sch_hfsc.c    |    2 +-
 net/sched/sch_htb.c     |    2 +-
 8 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/net/gen_stats.h b/include/net/gen_stats.h
index c148855..a0800e6 100644
--- a/include/net/gen_stats.h
+++ b/include/net/gen_stats.h
@@ -30,6 +30,7 @@ extern int gnet_stats_start_copy_compat(struct sk_buff *skb, int type,
 extern int gnet_stats_copy_basic(struct gnet_dump *d,
 				 struct gnet_stats_basic_packed *b);
 extern int gnet_stats_copy_rate_est(struct gnet_dump *d,
+				    const struct gnet_stats_basic_packed *bstats,
 				    struct gnet_stats_rate_est *r);
 extern int gnet_stats_copy_queue(struct gnet_dump *d,
 				 struct gnet_stats_queue *q);
diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
index 8569310..6f9513e 100644
--- a/net/core/gen_stats.c
+++ b/net/core/gen_stats.c
@@ -136,8 +136,13 @@ gnet_stats_copy_basic(struct gnet_dump *d, struct gnet_stats_basic_packed *b)
  * if the room in the socket buffer was not sufficient.
  */
 int
-gnet_stats_copy_rate_est(struct gnet_dump *d, struct gnet_stats_rate_est *r)
+gnet_stats_copy_rate_est(struct gnet_dump *d,
+			 const struct gnet_stats_basic_packed *bstats,
+			 struct gnet_stats_rate_est *r)
 {
+	if (!gen_estimator_active(bstats, r))
+		return 0;
+
 	if (d->compat_tc_stats) {
 		d->tc_stats.bps = r->bps;
 		d->tc_stats.pps = r->pps;
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 2dfb3e7..2b0d5ee 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -618,7 +618,7 @@ int tcf_action_copy_stats(struct sk_buff *skb, struct tc_action *a,
 			goto errout;
 
 	if (gnet_stats_copy_basic(&d, &h->tcf_bstats) < 0 ||
-	    gnet_stats_copy_rate_est(&d, &h->tcf_rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(&d, &h->tcf_bstats, &h->tcf_rate_est) < 0 ||
 	    gnet_stats_copy_queue(&d, &h->tcf_qstats) < 0)
 		goto errout;
 
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 903e418..1acfd29 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1179,7 +1179,7 @@ static int tc_fill_qdisc(struct sk_buff *skb, struct Qdisc *q, u32 clid,
 		goto nla_put_failure;
 
 	if (gnet_stats_copy_basic(&d, &q->bstats) < 0 ||
-	    gnet_stats_copy_rate_est(&d, &q->rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(&d, &q->bstats, &q->rate_est) < 0 ||
 	    gnet_stats_copy_queue(&d, &q->qstats) < 0)
 		goto nla_put_failure;
 
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 5b132c4..3846d65 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1609,7 +1609,7 @@ cbq_dump_class_stats(struct Qdisc *sch, unsigned long arg,
 		cl->xstats.undertime = cl->undertime - q->now;
 
 	if (gnet_stats_copy_basic(d, &cl->bstats) < 0 ||
-	    gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(d, &cl->bstats, &cl->rate_est) < 0 ||
 	    gnet_stats_copy_queue(d, &cl->qstats) < 0)
 		return -1;
 
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index 5a888af..a65604f 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -280,7 +280,7 @@ static int drr_dump_class_stats(struct Qdisc *sch, unsigned long arg,
 	}
 
 	if (gnet_stats_copy_basic(d, &cl->bstats) < 0 ||
-	    gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(d, &cl->bstats, &cl->rate_est) < 0 ||
 	    gnet_stats_copy_queue(d, &cl->qdisc->qstats) < 0)
 		return -1;
 
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 2c5c76b..b38b39c 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1375,7 +1375,7 @@ hfsc_dump_class_stats(struct Qdisc *sch, unsigned long arg,
 	xstats.rtwork  = cl->cl_cumul;
 
 	if (gnet_stats_copy_basic(d, &cl->bstats) < 0 ||
-	    gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(d, &cl->bstats, &cl->rate_est) < 0 ||
 	    gnet_stats_copy_queue(d, &cl->qstats) < 0)
 		return -1;
 
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 85acab9..8352fa3 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1105,7 +1105,7 @@ htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d)
 	cl->xstats.ctokens = cl->ctokens;
 
 	if (gnet_stats_copy_basic(d, &cl->bstats) < 0 ||
-	    gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||
+	    gnet_stats_copy_rate_est(d, &cl->bstats, &cl->rate_est) < 0 ||
 	    gnet_stats_copy_queue(d, &cl->qstats) < 0)
 		return -1;
 

^ permalink raw reply related

* Re: [PATCH 30/31] Fix use of uninitialized variable in cache_grow()
From: David Rientjes @ 2009-10-02 10:05 UTC (permalink / raw)
  To: Neil Brown
  Cc: Suresh Jayaraman, Linus Torvalds, Andrew Morton, linux-kernel,
	linux-mm, netdev, Miklos Szeredi, Wouter Verhelst, Peter Zijlstra,
	trond.myklebust
In-Reply-To: <19141.34685.863491.329836@notabene.brown>

On Fri, 2 Oct 2009, Neil Brown wrote:

> > > Index: mmotm/mm/slab.c
> > > ===================================================================
> > > --- mmotm.orig/mm/slab.c
> > > +++ mmotm/mm/slab.c
> > > @@ -2760,7 +2760,7 @@ static int cache_grow(struct kmem_cache
> > >  	size_t offset;
> > >  	gfp_t local_flags;
> > >  	struct kmem_list3 *l3;
> > > -	int reserve;
> > > +	int reserve = -1;
> > >  
> > >  	/*
> > >  	 * Be lazy and only check for valid flags here,  keeping it out of the
> > > @@ -2816,7 +2816,8 @@ static int cache_grow(struct kmem_cache
> > >  	if (local_flags & __GFP_WAIT)
> > >  		local_irq_disable();
> > >  	check_irq_off();
> > > -	slab_set_reserve(cachep, reserve);
> > > +	if (reserve != -1)
> > > +		slab_set_reserve(cachep, reserve);
> > >  	spin_lock(&l3->list_lock);
> > >  
> > >  	/* Make slab active. */
> > 
> > Given the patch description, shouldn't this be a test for objp != NULL 
> > instead, then?
> 
> In between those to patch hunks, cache_grow contains the code:
> 	if (!objp)
> 		objp = kmem_getpages(cachep, local_flags, nodeid, &reserve);
> 	if (!objp)
> 		goto failed;
> 
> We can no longer test if objp was NULL on entry to the function.
> We could take a copy of objp on entry to the function, and test it
> here.  But initialising 'reserve' to an invalid value is easier.
> 

Seems like you could do all this in kmem_getpages(), then, by calling 
slab_set_reserve(cachep, page->reserve) before returning the new page?

 [ I'd also drop the branch in slab_set_reserve(), it's faster to just 
   assign it unconditionally. ]

^ permalink raw reply

* Messages are printed on screen
From: Markus Feldmann @ 2009-10-02  9:52 UTC (permalink / raw)
  To: netdev

Hi All,

i am setting up my Server, with Linux Debian lenny. Therefore i am using 
  Kernel 2.6.31.1. I configured this Kernel with <make defconfig> and 
<make menuconfig>. My motherboard has 5 PCI slots and 1 AGP slot. All 
slots are in use. All devices are mapped to the following IRQ-Line:

Mass Storage Device (PCI Slot1)	IRQ-Line 11
Ethernet (PCI Slot 2) 		IRQ-Line 4
Ethernet (PCI Slot 3) 		IRQ-Line 5
Ethernet (PCI Slot 4) 		IRQ-Line 7
Ethernet (PCI Slot 5) 		IRQ-Line 11
Onboard USB-Controller		IRQ-Line 5
Onboard USB-Controller		IRQ-Line 4
Onboard USB-Controller		IRQ-Line 11
Onboard IDE			IRQ-Line 14
AGP VGA				IRQ-Line 11

As you see some of my IRQ-Lines are multiply in use, so my Server is 
working hard at his limit. The result is sometimes freezing of my 
Server, especially if there is much processing on these devices. I 
remember that with Kernel 2.6.18 my system didn't does freezing.

So i am trying to reduce the amount of this processing. I still get 
messages about dropped network packets on my Terminal, although i set up 
my <rsyslog> to save this only to </var/log>. Here is my 
</etc/rsyslog.conf>

How can i disable the output of messages (about dropped packets from my 
firewall) to my terminal ?

How can i stabilize my IRQ-System with the kernel 2.6.31.1 ?

What debug features should i disable ?

regards Markus


^ 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