Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v4 1/1] rps: core implementation
From: Tom Herbert @ 2009-11-20 23:53 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, Linux Netdev List
In-Reply-To: <20091120154046.67252d23@nehalam>

On Fri, Nov 20, 2009 at 3:40 PM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Fri, 20 Nov 2009 15:28:58 -0800
> Tom Herbert <therbert@google.com> wrote:
>
>> +static char *get_token(const char **cp, size_t *len)
>> +{
>> +     const char *bp = *cp;
>> +     char *start;
>> +
>> +     while (isspace(*bp))
>> +             bp++;
>> +
>> +     start = (char *)bp;
>> +     while (!isspace(*bp) && *bp != '\0')
>> +             bp++;
>> +
>> +     if (start != bp)
>> +             *len = bp - start;
>> +     else
>> +             start = NULL;
>> +
>> +     *cp = bp;
>> +     return start;
>> +}
>
>
> Sysfs is intentionally one value per file. If you need multiple
> values, then it is the wrong interface.
>

What would be a better interface?

> --
>

^ permalink raw reply

* Re: [PATCH v4 1/1] rps: core implementation
From: David Miller @ 2009-11-20 23:56 UTC (permalink / raw)
  To: shemminger; +Cc: therbert, netdev
In-Reply-To: <20091120154046.67252d23@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 20 Nov 2009 15:40:46 -0800

> Sysfs is intentionally one value per file. If you need multiple
> values, then it is the wrong interface.

This is probably right.


^ permalink raw reply

* Re: [PATCH v4 1/1] rps: core implementation
From: Tom Herbert @ 2009-11-21  0:04 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, Linux Netdev List
In-Reply-To: <20091120154209.01edcbb2@nehalam>

On Fri, Nov 20, 2009 at 3:42 PM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Fri, 20 Nov 2009 15:28:58 -0800
> Tom Herbert <therbert@google.com> wrote:
>
>> @@ -861,6 +884,9 @@ struct net_device {
>>
>>       struct netdev_queue     rx_queue;
>>
>> +     struct dev_rps_maps     *dev_rps_maps;  /* Per-NAPI maps for
>> +                                                receive packet steeing */
>> +
>
> How does this work for devices with:
>   * multiqueue - one device has multiple NAPI instances

Each NAPI instance has its own map (dev_rps_maps hold the array of these maps)

>   * mulitport  - one NAPI shared by multiple devices
>
I have not tested that, so I'm not sure.  But, I believe since the per
NAPI maps are kept in the netdevice (not in napi structure) this would
mean that each of those devices has its own per NAPI rps map(s).

Tom

^ permalink raw reply

* Re: [PATCH v4 1/1] rps: core implementation
From: David Miller @ 2009-11-21  0:05 UTC (permalink / raw)
  To: therbert; +Cc: netdev
In-Reply-To: <65634d660911201550v34bc463ao17dcd7b16f62264f@mail.gmail.com>

From: Tom Herbert <therbert@google.com>
Date: Fri, 20 Nov 2009 15:50:03 -0800

> We only set the bit in remote_softirq_cpus in here.  The actual IPIs
> are sent at the end net_rx_action.  I'm not exactly sure what you're
> thinking on this...

You're scheduling a softirq so you can schedule remote softirqs.

Just do that "for each rps cpu" loop at the end of NAPI ->poll()
processing.  In fact that seems to be what you're doing :-)

I guess my confusion is from the:

				__raise_softirq_irqoff(NET_RX_SOFTIRQ);

you are doing as you set the cpus in rps_remote_softirq_cpus.

Why do you need to schedule the local RX softirq, when we know we're
in a NAPI poll loop and thus that we're in a softirq, and thus that we
will fire off the IPIs at the end of net_rx_action()?

That's what you're doing, the softirq raising just seems superfluous.

^ permalink raw reply

* Re: [PATCH v4 1/1] rps: core implementation
From: Tom Herbert @ 2009-11-21  0:12 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20091120.160507.110482370.davem@davemloft.net>

> I guess my confusion is from the:
>
>                                __raise_softirq_irqoff(NET_RX_SOFTIRQ);
>
> you are doing as you set the cpus in rps_remote_softirq_cpus.
>
> Why do you need to schedule the local RX softirq, when we know we're
> in a NAPI poll loop and thus that we're in a softirq, and thus that we
> will fire off the IPIs at the end of net_rx_action()?
>
> That's what you're doing, the softirq raising just seems superfluous.
>

Ah, right. If RPS can't be used non-NAPI case, this line is now
superfluous.  It can be removed.

Thanks

^ permalink raw reply

* [RFC PATCH] net: persistent device name bitmaps for faster name allocation
From: Octavian Purdila @ 2009-11-21  0:37 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger

This patch solves the current scalability issues of dev_name_alloc() by
making the device name bitmap persistent. It is based on an idea
suggested by Stephen Hemminger.

$ time insmod /lib/modules/dummy.ko numdummies=8000

Without the patch           With the patch

real    0m 43.43s           real    0m 0.52s
user    0m 0.00s            user    0m 0.00s
sys     0m 43.43s           sys     0m 0.52s

For each format string (e.g. eth%d) we maintain a device name
bitmap. The bitmap starts by allocating one page and can grow up to
MAX_DEV_NAME_ORDER.

There is also a limit to the number of bitmaps (MAX_DEV_NAME_BITMAPS).
If this limit is reached while trying to allocate a new bitmap we free
the last used one.

The device name bitmaps are stored in a rbtree on a per namespace
basis.

Open issue: Stephen suggested to register "a VM notifier hook" that
could be used to dump the trees in case of memory pressure.

However, I was only able to find the OOM notifier, and I don't know if
that would be as useful as a real memory pressure event. Any hints?

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 include/net/net_namespace.h |    1 +
 net/core/dev.c              |  293 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 269 insertions(+), 25 deletions(-)

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 0addd45..2da2a09 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -56,6 +56,7 @@ struct net {
 	struct list_head 	dev_base_head;
 	struct hlist_head 	*dev_name_head;
 	struct hlist_head	*dev_index_head;
+	struct rb_root		dev_name_bitmaps;
 
 	/* core fib_rules */
 	struct list_head	rules_ops;
diff --git a/net/core/dev.c b/net/core/dev.c
index 9977288..9e8f909 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -205,6 +205,202 @@ static inline struct hlist_head *dev_index_hash(struct net *net, int ifindex)
 	return &net->dev_index_head[ifindex & (NETDEV_HASHENTRIES - 1)];
 }
 
+/* For fast name allocation we maintain a bitmap of used devices, one for each
+ * format string (e.g. eth%d) */
+struct dev_name_bitmap {
+	char fmt[IFNAMSIZ];
+	unsigned long *data;
+	int size;
+	struct rb_node rb_node;
+	struct list_head list;
+	struct net *net;
+};
+
+#define MAX_DEV_NAME_BITMAPS 10
+#define MAX_DEV_NAME_ORDER 4
+
+static int dev_name_bitmap_count;
+/* List of all device name bitmaps, sorted by how often they are used. Last
+ * bitmap used is last. */
+static LIST_HEAD(dev_name_bitmap_list);
+
+static void kill_dev_name_bitmap(int no)
+{
+	struct dev_name_bitmap *b, *aux;
+	int i = 0;
+
+	list_for_each_entry_safe(b, aux, &dev_name_bitmap_list, list) {
+		if (i++ >= no)
+			return;
+		list_del(&b->list);
+		rb_erase(&b->rb_node, &b->net->dev_name_bitmaps);
+		free_pages((unsigned long)b->data, get_order(b->size/8));
+		dev_name_bitmap_count--;
+		kfree(b);
+	}
+}
+
+/**
+ *	get_dev_name_fmt - returns the format string from a device name
+ *	@name: the device name (e.g. eth0)
+ *	@fmt: the format string which coresponds to the device name (e.g. eth%d)
+ *
+ *	If the name contains multiple digit runs then the format string will be
+ *	be based on the last one. For example, the format string for eth0a12
+ *	will be eth0a%d.
+ */
+static int get_dev_name_fmt(const char *name, char *fmt)
+{
+	int i = 0, last_run_end = -1, last_run_start = -1;
+	char digit[IFNAMSIZ] = { 0, }, c;
+
+	while ((c = name[i++])) {
+		if (c >= '0' && c <= '9')
+			digit[i] = 1;
+	}
+
+	/* find the last digit */
+	for (i = IFNAMSIZ-1; i >= 0; i--)
+		if (digit[i]) {
+			last_run_end = i;
+			break;
+		}
+
+	/* make sure we have space for %d\0 */
+	if (last_run_end < 0 || last_run_start >= IFNAMSIZ - 3)
+		return -EINVAL;
+
+	/* find the first digit of the last digit run */
+	for (i = last_run_end; i >= 0; i--)
+		if (!digit[i]) {
+			last_run_start = i;
+			break;
+		}
+
+	if (last_run_start < 0)
+		return -EINVAL;
+
+	memcpy(fmt, name, last_run_start);
+	memcpy(&fmt[last_run_start], "%d", sizeof("%d"));
+
+	return 0;
+}
+
+static struct dev_name_bitmap *get_name_bitmap(struct net *net, const char *fmt)
+{
+	struct rb_node *i = net->dev_name_bitmaps.rb_node;
+
+	while (i) {
+		struct dev_name_bitmap *b;
+		int cmp;
+
+		b = rb_entry(i, struct dev_name_bitmap, rb_node);
+		cmp = strcmp(fmt, b->fmt);
+
+		if (cmp < 0)
+			i = i->rb_left;
+		else if (cmp > 0)
+			i = i->rb_right;
+		else
+			return b;
+	}
+
+	return NULL;
+}
+
+static int grow_name_bitmap(struct dev_name_bitmap *b, int size)
+{
+	unsigned long *data;
+	int order = get_order(size/8 + size%8);
+
+	if (order > MAX_DEV_NAME_ORDER)
+		return -EDQUOT;
+
+	size = (1<<order)*PAGE_SIZE*8;
+	if (size < b->size)
+		return 0;
+
+	data = (unsigned long *)__get_free_pages(GFP_ATOMIC, order);
+	if (!data)
+		return -ENOMEM;
+
+	memcpy(data, b->data, b->size/8);
+	memset((char *)data + b->size/8, 0, size/8 - b->size/8);
+
+	if (b->size)
+		free_pages((unsigned long)b->data, get_order(b->size/8));
+	b->size = size;
+	b->data = data;
+
+	return 0;
+}
+
+static void fill_name_bitmap(struct dev_name_bitmap *b, int order)
+{
+	struct net_device *d;
+	char buf[IFNAMSIZ];
+	int i;
+
+	for_each_netdev(b->net, d) {
+		if (!sscanf(d->name, b->fmt, &i))
+			continue;
+
+		/*  avoid cases where sscanf is not exact inverse of printf */
+		snprintf(buf, IFNAMSIZ, b->fmt, i);
+		if (!strncmp(buf, d->name, IFNAMSIZ))
+			continue;
+
+		if (i < 0)
+			continue;
+
+		if (i >= b->size && grow_name_bitmap(b, i))
+			continue;
+
+		set_bit(i, b->data);
+	}
+}
+
+static void update_name_bitmap(struct net *net, const char *name, bool set)
+{
+	struct dev_name_bitmap *b = NULL;
+	char fmt[IFNAMSIZ], tmp[IFNAMSIZ];
+	int i;
+
+	if (get_dev_name_fmt(name, fmt))
+		return;
+
+	b = get_name_bitmap(net, fmt);
+	if (!b || sscanf(name, fmt, &i) != 1 ||
+	    snprintf(tmp, IFNAMSIZ, fmt, i) == 0 ||
+	    strcmp(tmp, name) != 0)
+		return;
+
+	if (i > b->size && grow_name_bitmap(b, i))
+		return;
+
+	if (set)
+		set_bit(i, b->data);
+	else
+		clear_bit(i, b->data);
+
+	list_del(&b->list);
+	list_add_tail(&b->list, &dev_name_bitmap_list);
+}
+
+static inline void list_dev_name(struct net_device *dev)
+{
+	struct net *net = dev_net(dev);
+
+	update_name_bitmap(net, dev->name, 1);
+	hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
+}
+
+static inline void unlist_dev_name(struct net_device *dev, const char *name)
+{
+	hlist_del_rcu(&dev->name_hlist);
+	update_name_bitmap(dev_net(dev), name, 0);
+}
+
 /* Device list insertion */
 static int list_netdevice(struct net_device *dev)
 {
@@ -214,7 +410,7 @@ static int list_netdevice(struct net_device *dev)
 
 	write_lock_bh(&dev_base_lock);
 	list_add_tail_rcu(&dev->dev_list, &net->dev_base_head);
-	hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
+	list_dev_name(dev);
 	hlist_add_head_rcu(&dev->index_hlist,
 			   dev_index_hash(net, dev->ifindex));
 	write_unlock_bh(&dev_base_lock);
@@ -231,7 +427,7 @@ static void unlist_netdevice(struct net_device *dev)
 	/* Unlink dev from the device chain */
 	write_lock_bh(&dev_base_lock);
 	list_del_rcu(&dev->dev_list);
-	hlist_del_rcu(&dev->name_hlist);
+	unlist_dev_name(dev, dev->name);
 	hlist_del_rcu(&dev->index_hlist);
 	write_unlock_bh(&dev_base_lock);
 }
@@ -839,6 +1035,55 @@ int dev_valid_name(const char *name)
 }
 EXPORT_SYMBOL(dev_valid_name);
 
+static struct dev_name_bitmap *new_name_bitmap(struct net *net, const char *fmt)
+{
+	struct rb_node **p = &net->dev_name_bitmaps.rb_node, *parent = NULL;
+	struct dev_name_bitmap *b;
+
+	if (dev_name_bitmap_count == MAX_DEV_NAME_BITMAPS - 1)
+		kill_dev_name_bitmap(1);
+
+	b = kmalloc(sizeof(*b), GFP_ATOMIC);
+	if (!b)
+		return NULL;
+
+	strcpy(b->fmt, fmt);
+	b->size = 0;
+	b->net = net;
+
+	if (grow_name_bitmap(b, 8*PAGE_SIZE)) {
+		kfree(b);
+		return NULL;
+	}
+
+	fill_name_bitmap(b, 0);
+
+	list_add_tail(&b->list, &dev_name_bitmap_list);
+
+	while (*p) {
+		struct dev_name_bitmap *cmp_b;
+		int cmp;
+
+		parent = *p;
+		cmp_b = rb_entry(parent, struct dev_name_bitmap, rb_node);
+		cmp = strcmp(b->fmt, cmp_b->fmt);
+
+		if (cmp < 0)
+			p = &(*p)->rb_left;
+		else if (cmp > 0)
+			p = &(*p)->rb_right;
+		else
+			BUG();
+	}
+	rb_link_node(&b->rb_node, parent, p);
+	rb_insert_color(&b->rb_node, &net->dev_name_bitmaps);
+
+	dev_name_bitmap_count++;
+
+	return b;
+}
+
+
 /**
  *	__dev_alloc_name - allocate a name for a device
  *	@net: network namespace to allocate the device name in
@@ -856,14 +1101,13 @@ EXPORT_SYMBOL(dev_valid_name);
 
 static int __dev_alloc_name(struct net *net, const char *name, char *buf)
 {
-	int i = 0;
+	int i = 0, retry;
 	const char *p;
-	const int max_netdevices = 8*PAGE_SIZE;
-	unsigned long *inuse;
-	struct net_device *d;
 
 	p = strnchr(name, IFNAMSIZ-1, '%');
 	if (p) {
+		struct dev_name_bitmap *b;
+
 		/*
 		 * Verify the string as this thing may have come from
 		 * the user.  There must be either one "%d" and no other "%"
@@ -872,25 +1116,23 @@ static int __dev_alloc_name(struct net *net, const char *name, char *buf)
 		if (p[1] != 'd' || strchr(p + 2, '%'))
 			return -EINVAL;
 
-		/* Use one page as a bit array of possible slots */
-		inuse = (unsigned long *) get_zeroed_page(GFP_ATOMIC);
-		if (!inuse)
-			return -ENOMEM;
-
-		for_each_netdev(net, d) {
-			if (!sscanf(d->name, name, &i))
-				continue;
-			if (i < 0 || i >= max_netdevices)
-				continue;
-
-			/*  avoid cases where sscanf is not exact inverse of printf */
-			snprintf(buf, IFNAMSIZ, name, i);
-			if (!strncmp(buf, d->name, IFNAMSIZ))
-				set_bit(i, inuse);
+		b = get_name_bitmap(net, name);
+		if (!b) {
+			b = new_name_bitmap(net, name);
+			if (!b)
+				return -ENOMEM;
 		}
 
-		i = find_first_zero_bit(inuse, max_netdevices);
-		free_page((unsigned long) inuse);
+		do {
+			retry = 0;
+			i = find_first_zero_bit(b->data, b->size);
+			if (i == b->size) {
+				int err = grow_name_bitmap(b, i + 1);
+				if (err)
+					return err;
+				retry = 1;
+			}
+		} while (retry);
 	}
 
 	if (buf != name)
@@ -994,13 +1236,13 @@ rollback:
 	}
 
 	write_lock_bh(&dev_base_lock);
-	hlist_del(&dev->name_hlist);
+	unlist_dev_name(dev, oldname);
 	write_unlock_bh(&dev_base_lock);
 
 	synchronize_rcu();
 
 	write_lock_bh(&dev_base_lock);
-	hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
+	list_dev_name(dev);
 	write_unlock_bh(&dev_base_lock);
 
 	ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
@@ -5655,6 +5897,7 @@ static struct hlist_head *netdev_create_hash(void)
 static int __net_init netdev_init(struct net *net)
 {
 	INIT_LIST_HEAD(&net->dev_base_head);
+	net->dev_name_bitmaps = RB_ROOT;
 
 	net->dev_name_head = netdev_create_hash();
 	if (net->dev_name_head == NULL)
-- 
1.5.6.5


^ permalink raw reply related

* Re: [PATCH v4 1/1] rps: core implementation
From: Jarek Poplawski @ 2009-11-21  0:40 UTC (permalink / raw)
  To: Tom Herbert; +Cc: David Miller, netdev
In-Reply-To: <65634d660911201612y12a9b5bnb6685da040ad832d@mail.gmail.com>

Tom Herbert wrote, On 11/21/2009 01:12 AM:

>> I guess my confusion is from the:
>>
>>                                __raise_softirq_irqoff(NET_RX_SOFTIRQ);
>>
>> you are doing as you set the cpus in rps_remote_softirq_cpus.
>>
>> Why do you need to schedule the local RX softirq, when we know we're
>> in a NAPI poll loop and thus that we're in a softirq, and thus that we
>> will fire off the IPIs at the end of net_rx_action()?
>>
>> That's what you're doing, the softirq raising just seems superfluous.
>>
> 
> Ah, right. If RPS can't be used non-NAPI case, this line is now
> superfluous.  It can be removed.

Hmm... mostly right. At least if it's wrt. my previous opinion. I meant
only that non-NAPI case would be handled less optimally without this
dedicated softirq, but __raise_softirq_irqoff(NET_RX_SOFTIRQ) or
napi_schedule(&queue->backlog) could be called from netif_rx().

Jarek P.

^ permalink raw reply

* Re: [RFC PATCH] net: persistent device name bitmaps for faster name allocation
From: Stephen Hemminger @ 2009-11-21  0:57 UTC (permalink / raw)
  To: Octavian Purdila; +Cc: netdev
In-Reply-To: <200911210237.25855.opurdila@ixiacom.com>

On Sat, 21 Nov 2009 02:37:25 +0200
Octavian Purdila <opurdila@ixiacom.com> wrote:

> This patch solves the current scalability issues of dev_name_alloc() by
> making the device name bitmap persistent. It is based on an idea
> suggested by Stephen Hemminger.

Still not sure that this much code is worth it. Even with 10000 ISP PPPoE links,
I can't see that bringing up a new PPP device would be the real bottleneck
except in the case of some benchmark that starts them all at once. Even TPC-C
benchmarks have a "warm up period".

-- 

^ permalink raw reply

* [net-2.6 PATCH 2/3] netxen: fix promisc for NX2031.
From: Amit Kumar Salecha @ 2009-11-21  1:09 UTC (permalink / raw)
  To: davem; +Cc: netdev, dhananjay.phadke, Narender Kumar, Amit Kumar Salecha
In-Reply-To: <1258765774-27068-1-git-send-email-amit@netxen.com>

From: Narender Kumar <narender.kumar@qlogic.com>

Kernel crashes, if promisc mode set without disabling rx queue.
Before changing mode in NX2031 chip, wait till rx queue drains.

Signed-off-by: Narender Kumar <narender.kumar@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/netxen/netxen_nic_hdr.h |    2 +
 drivers/net/netxen/netxen_nic_hw.c  |   49 +++++++++++++++++++++++++++--------
 2 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic_hdr.h b/drivers/net/netxen/netxen_nic_hdr.h
index 1c46da6..17bb381 100644
--- a/drivers/net/netxen/netxen_nic_hdr.h
+++ b/drivers/net/netxen/netxen_nic_hdr.h
@@ -545,6 +545,8 @@ enum {
 #define	NETXEN_NIU_TEST_MUX_CTL		(NETXEN_CRB_NIU + 0x00094)
 #define	NETXEN_NIU_XG_PAUSE_CTL		(NETXEN_CRB_NIU + 0x00098)
 #define	NETXEN_NIU_XG_PAUSE_LEVEL	(NETXEN_CRB_NIU + 0x000dc)
+#define	NETXEN_NIU_FRAME_COUNT_SELECT	(NETXEN_CRB_NIU + 0x000ac)
+#define	NETXEN_NIU_FRAME_COUNT		(NETXEN_CRB_NIU + 0x000b0)
 #define	NETXEN_NIU_XG_SEL		(NETXEN_CRB_NIU + 0x00128)
 #define NETXEN_NIU_GB_PAUSE_CTL		(NETXEN_CRB_NIU + 0x0030c)
 
diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index 3185a98..e2c4a01 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -383,24 +383,51 @@ int netxen_niu_disable_xg_port(struct netxen_adapter *adapter)
 
 int netxen_p2_nic_set_promisc(struct netxen_adapter *adapter, u32 mode)
 {
-	__u32 reg;
+	u32 mac_cfg;
+	u32 cnt = 0;
+	__u32 reg = 0x0200;
 	u32 port = adapter->physical_port;
+	u16 board_type = adapter->ahw.board_type;
 
 	if (port > NETXEN_NIU_MAX_XG_PORTS)
 		return -EINVAL;
 
-	reg = NXRD32(adapter, NETXEN_NIU_XGE_CONFIG_1 + (0x10000 * port));
-	if (mode == NETXEN_NIU_PROMISC_MODE)
-		reg = (reg | 0x2000UL);
-	else
-		reg = (reg & ~0x2000UL);
+	mac_cfg = NXRD32(adapter, NETXEN_NIU_XGE_CONFIG_0 + (0x10000 * port));
+	mac_cfg &= ~0x4;
+	NXWR32(adapter, NETXEN_NIU_XGE_CONFIG_0 + (0x10000 * port), mac_cfg);
 
-	if (mode == NETXEN_NIU_ALLMULTI_MODE)
-		reg = (reg | 0x1000UL);
-	else
-		reg = (reg & ~0x1000UL);
+	if ((board_type == NETXEN_BRDTYPE_P2_SB31_10G_IMEZ) ||
+			(board_type == NETXEN_BRDTYPE_P2_SB31_10G_HMEZ))
+		reg = (0x20 << port);
+
+	NXWR32(adapter, NETXEN_NIU_FRAME_COUNT_SELECT, reg);
+
+	mdelay(10);
+
+	while (NXRD32(adapter, NETXEN_NIU_FRAME_COUNT) && ++cnt < 20)
+		mdelay(10);
+
+	if (cnt < 20) {
+
+		reg = NXRD32(adapter,
+			NETXEN_NIU_XGE_CONFIG_1 + (0x10000 * port));
+
+		if (mode == NETXEN_NIU_PROMISC_MODE)
+			reg = (reg | 0x2000UL);
+		else
+			reg = (reg & ~0x2000UL);
+
+		if (mode == NETXEN_NIU_ALLMULTI_MODE)
+			reg = (reg | 0x1000UL);
+		else
+			reg = (reg & ~0x1000UL);
+
+		NXWR32(adapter,
+			NETXEN_NIU_XGE_CONFIG_1 + (0x10000 * port), reg);
+	}
 
-	NXWR32(adapter, NETXEN_NIU_XGE_CONFIG_1 + (0x10000 * port), reg);
+	mac_cfg |= 0x4;
+	NXWR32(adapter, NETXEN_NIU_XGE_CONFIG_0 + (0x10000 * port), mac_cfg);
 
 	return 0;
 }
-- 
1.6.0.2


^ permalink raw reply related

* [net-2.6 PATCH 3/3] netxen : fix BOND_MODE_TLB/ALB mode.
From: Amit Kumar Salecha @ 2009-11-21  1:09 UTC (permalink / raw)
  To: davem; +Cc: netdev, dhananjay.phadke, Narender Kumar, Amit Kumar Salecha
In-Reply-To: <1258765774-27068-1-git-send-email-amit@netxen.com>

From: Narender Kumar <narender.kumar@qlogic.com>

o In case of bonding mode BOND_MODE_TLB and BOND_MODE_ALB,
   mac addr can swap of new_active && old_active slave.
o In set_mac, store mac addr in device private structure for future use.

Signed-off-by: Narender Kumar <narender.kumar@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/netxen/netxen_nic.h      |    2 ++
 drivers/net/netxen/netxen_nic_hw.c   |    6 +++---
 drivers/net/netxen/netxen_nic_main.c |    4 +++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index 7384f59..e1237b8 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -1163,6 +1163,8 @@ struct netxen_adapter {
 	u32 int_vec_bit;
 	u32 heartbit;
 
+	u8 mac_addr[ETH_ALEN];
+
 	struct netxen_adapter_stats stats;
 
 	struct netxen_recv_context recv_ctx;
diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index e2c4a01..52a3798 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -463,7 +463,7 @@ netxen_nic_enable_mcast_filter(struct netxen_adapter *adapter)
 {
 	u32	val = 0;
 	u16 port = adapter->physical_port;
-	u8 *addr = adapter->netdev->dev_addr;
+	u8 *addr = adapter->mac_addr;
 
 	if (adapter->mc_enabled)
 		return 0;
@@ -492,7 +492,7 @@ netxen_nic_disable_mcast_filter(struct netxen_adapter *adapter)
 {
 	u32	val = 0;
 	u16 port = adapter->physical_port;
-	u8 *addr = adapter->netdev->dev_addr;
+	u8 *addr = adapter->mac_addr;
 
 	if (!adapter->mc_enabled)
 		return 0;
@@ -687,7 +687,7 @@ void netxen_p3_nic_set_multi(struct net_device *netdev)
 
 	list_splice_tail_init(&adapter->mac_list, &del_list);
 
-	nx_p3_nic_add_mac(adapter, netdev->dev_addr, &del_list);
+	nx_p3_nic_add_mac(adapter, adapter->mac_addr, &del_list);
 	nx_p3_nic_add_mac(adapter, bcast_addr, &del_list);
 
 	if (netdev->flags & IFF_PROMISC) {
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 0b4a56a..3bf78db 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -437,6 +437,7 @@ netxen_read_mac_addr(struct netxen_adapter *adapter)
 		netdev->dev_addr[i] = *(p + 5 - i);
 
 	memcpy(netdev->perm_addr, netdev->dev_addr, netdev->addr_len);
+	memcpy(adapter->mac_addr, netdev->dev_addr, netdev->addr_len);
 
 	/* set station address */
 
@@ -459,6 +460,7 @@ int netxen_nic_set_mac(struct net_device *netdev, void *p)
 		netxen_napi_disable(adapter);
 	}
 
+	memcpy(adapter->mac_addr, addr->sa_data, netdev->addr_len);
 	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
 	adapter->macaddr_set(adapter, addr->sa_data);
 
@@ -956,7 +958,7 @@ netxen_nic_up(struct netxen_adapter *adapter, struct net_device *netdev)
 		return err;
 	}
 	if (NX_IS_REVISION_P2(adapter->ahw.revision_id))
-		adapter->macaddr_set(adapter, netdev->dev_addr);
+		adapter->macaddr_set(adapter, adapter->mac_addr);
 
 	adapter->set_multi(netdev);
 	adapter->set_mtu(adapter, netdev->mtu);
-- 
1.6.0.2


^ permalink raw reply related

* [net-2.6 PATCH 1/3] netxen: fix memory initialization
From: Amit Kumar Salecha @ 2009-11-21  1:09 UTC (permalink / raw)
  To: davem; +Cc: netdev, dhananjay.phadke
In-Reply-To: <1258765774-27068-1-git-send-email-amit@netxen.com>

Avoid resetting memory during initialization, skip this memory
block during driver probe.

Signed-off-by: Amit Kumar Salecha <amit@netxen.com>
---
 drivers/net/netxen/netxen_nic_init.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic_init.c b/drivers/net/netxen/netxen_nic_init.c
index e40b914..8a09043 100644
--- a/drivers/net/netxen/netxen_nic_init.c
+++ b/drivers/net/netxen/netxen_nic_init.c
@@ -544,6 +544,8 @@ int netxen_pinit_from_rom(struct netxen_adapter *adapter, int verbose)
 				continue;
 			if (off == (ROMUSB_GLB + 0x1c)) /* MS clock */
 				continue;
+			if ((off & 0x0ff00000) == NETXEN_CRB_DDR_NET)
+				continue;
 			if (off == (NETXEN_CRB_PEG_NET_1 + 0x18))
 				buf[i].data = 0x1020;
 			/* skip the function enable register */
-- 
1.6.0.2


^ permalink raw reply related

* [net-2.6 PATCH 0/3] critical fixes
From: Amit Kumar Salecha @ 2009-11-21  1:09 UTC (permalink / raw)
  To: davem; +Cc: netdev, dhananjay.phadke


Hi
   I am sending three patches, to fix some critical issues.

   1) Skip memory initialization during driver load. Device may not come up, if memory is reset.
   2) Fix promisc mode, kernel crashes with this bug. Drain rx queue before changing mode. 
   3) Fix bonding mode TLB/ALB, correct mac address need to communicate to device.

-Amit Salecha

^ permalink raw reply

* Re: A generic kernel compatibilty code
From: Luis R. Rodriguez @ 2009-11-21  2:12 UTC (permalink / raw)
  To: John W. Linville
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-wireless,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <43e72e890911201338w4c403d51q651afc3638d1739c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Nov 20, 2009 at 1:38 PM, Luis R. Rodriguez <mcgrof-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Nov 20, 2009 at 1:18 PM, John W. Linville
> <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote:
>> On Fri, Nov 20, 2009 at 12:53:51PM -0800, Luis R. Rodriguez wrote:
>>> On Fri, Nov 20, 2009 at 12:51 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>>> > On Fri, Nov 20, 2009 at 12:45 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>>> >> Everyone and their mother reinvents the wheel when it comes to
>>> >> backporting kernel modules. It a painful job and it seems to me an
>>> >> alternative is possible. If we can write generic compatibilty code for
>>> >> a new routine introduced on the next kernel how about just merging it
>>> >> to the kernel under some generic compat module. This would be
>>> >> completey ignored by everyone using the stable kernel but can be
>>> >> copied by anyone doing backport work.
>>> >>
>>> >> So I'm thinking something as simple as a generic compat/comat.ko with
>>> >> compat-2.6.32.[ch] files.
>>> >
>>> > FWIW, I meant a compat-2.6.32.[ch] and compat-2.6.31.[ch] and so on.
>>> > All these would link to the compat.ko
>>>
>>> I supose this could juse be a separate tree with some generic
>>> compat.ko module. That might work better.
>>
>> This is what I would suggest for pursuing this idea.  Perhaps you
>> could split-off from compat-wireless, then make that tree depend on
>> the new tree (compat-core?)...
>
> OK thanks, will try that for the next kernel.

I've packaged a compat.ko here:

git://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/compat.git

This should be up as soon as the kernel.org synchs. The missing piece
would just be to define a common place to put the headers for
distributions who want to use external modules which want to use this
thing.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] Au1x00: fix crash when trying register_netdev()
From: Alexander Beregalov @ 2009-11-21  3:07 UTC (permalink / raw)
  To: davem; +Cc: netdev, alohre, Alexander Beregalov

Andreas Lohre reported that the driver crashes when trying
to register_netdev(), he sugessted to move dev->netdev_ops initialization
before calling register_netdev(), it worked for him.

Reported-by: Andreas Lohre <alohre@gmail.com>
Signed-off-by: Alexander Beregalov <a.beregalov@gmail.com>
---

David, please send it to stable@ for 2.6.31


 drivers/net/au1000_eth.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
index ce6f1ac..3f4b430 100644
--- a/drivers/net/au1000_eth.c
+++ b/drivers/net/au1000_eth.c
@@ -1088,7 +1088,14 @@ static struct net_device * au1000_probe(int port_num)
 		return NULL;
 	}
 
-	if ((err = register_netdev(dev)) != 0) {
+	dev->base_addr = base;
+	dev->irq = irq;
+	dev->netdev_ops = &au1000_netdev_ops;
+	SET_ETHTOOL_OPS(dev, &au1000_ethtool_ops);
+	dev->watchdog_timeo = ETH_TX_TIMEOUT;
+
+	err = register_netdev(dev);
+	if (err != 0) {
 		printk(KERN_ERR "%s: Cannot register net device, error %d\n",
 				DRV_NAME, err);
 		free_netdev(dev);
@@ -1209,12 +1216,6 @@ static struct net_device * au1000_probe(int port_num)
 		aup->tx_db_inuse[i] = pDB;
 	}
 
-	dev->base_addr = base;
-	dev->irq = irq;
-	dev->netdev_ops = &au1000_netdev_ops;
-	SET_ETHTOOL_OPS(dev, &au1000_ethtool_ops);
-	dev->watchdog_timeo = ETH_TX_TIMEOUT;
-
 	/*
 	 * The boot code uses the ethernet controller, so reset it to start
 	 * fresh.  au1000_init() expects that the device is in reset state.
-- 
1.6.5.3


^ permalink raw reply related

* Re: [net-2.6 PATCH 3/3] netxen : fix BOND_MODE_TLB/ALB mode.
From: David Miller @ 2009-11-21  5:48 UTC (permalink / raw)
  To: amit; +Cc: netdev, dhananjay.phadke, narender.kumar, amit.salecha
In-Reply-To: <1258765774-27068-4-git-send-email-amit@netxen.com>

From: Amit Kumar Salecha <amit@netxen.com>
Date: Fri, 20 Nov 2009 17:09:34 -0800

> @@ -437,6 +437,7 @@ netxen_read_mac_addr(struct netxen_adapter *adapter)
>  		netdev->dev_addr[i] = *(p + 5 - i);
>  
>  	memcpy(netdev->perm_addr, netdev->dev_addr, netdev->addr_len);
> +	memcpy(adapter->mac_addr, netdev->dev_addr, netdev->addr_len);
>  
>  	/* set station address */
>  

I don't think this is correct.

You have to maintain a valid netdev->perm_addr even if you don't use
that value later on internally.  This is how the permanent, probed,
MAC address of the card is made available to the user.

Second of all, it is not clear at all why you need to maintain this
value internally.  Why does it not work to simply use
netdev->dev_addr?

Whatever the reason, you need to explain the details of this in your
commit message.

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: Xmit Packet Steering (XPS)
From: Eric Dumazet @ 2009-11-21  6:58 UTC (permalink / raw)
  To: Tom Herbert; +Cc: Andi Kleen, David S. Miller, Linux Netdev List
In-Reply-To: <65634d660911201642k3930dc78vd576e0e89dc0c794@mail.gmail.com>

Tom Herbert a écrit :
> 
> It probably is a special case, but addresses a real problem.  Like rps,
> this addresses the case where a single queue NIC is the bottleneck on a
> system.  Anything that takes work off the interrupting core and spreads
> it to other cores may alleviate the bottleneck. 
> 


Yes, and this brings back an idea I mentioned earlier in April, for a multicast
problem we had with AthenaCR.

XPS could be 'started' only if caller runs from ksoftirqd, eg we are in a stress
situation, and additional cost of XPS is nothing compared to huge gains we have
to spread part of work to more CPUS.

(Costs of XPS includes : ~500 bytes of ICACHE, and IPI)



^ permalink raw reply

* Re: [PATCH v4 1/1] rps: core implementation
From: Eric Dumazet @ 2009-11-21  8:07 UTC (permalink / raw)
  To: Tom Herbert; +Cc: David Miller, Linux Netdev List, Andi Kleen
In-Reply-To: <65634d660911201528k5a07135el471b65fff9dd7c9d@mail.gmail.com>

Tom Herbert a écrit :
> Version 4 of RPS patch.  I think this addresses most of the comments
> on the previous versions.  Thanks for all the insightful comments and
> patience!
> 
> Changes from previous version:
> 
> - Added rxhash to kernel-doc for struct sk_buff
> - Removed /** on comments not for kernel-doc
> - Change get_token declaration to kernel style
> - Added struct dev_rps_maps.  Each netdevice now has a pointer to this
> structure which contains the array of per NAPI rps maps, the number of
> this maps.  rcu is used to protect pointer
> - In store_rps_cpus a new map set is allocated each call.
> - Removed dev_isalive check and other locks since rps struct in
> netdevice are protected by rcu
> - Removed NET_RPS_SOFTIRQ and call net_rps_action from net_rx_action instead
> - Queue to remote backlog queues only in NAPI case.  This means
> rps_remote_softirq_cpus does not need to be accessed with interrupts
> disabled and __smp_call_function_single will not be called with
> interrupts disabled
> - Limit the number of entries in an rps map to min(256, num_possible_cpus())
> - Removed unnecessary irq_local_disable
> - Renamed skb_tx_hashrnd to just hashrnd and use that for the rps hash as well
> - Make index u16 in index=skb_get_rx_queue() and don't check index<0 now
> - Replace spin_lock_irqsave with simpler spin_lock_irq in process_backlog

Excellent !

>   *	The DEVICE structure.
>   *	Actually, this whole structure is a big mistake.  It mixes I/O
>   *	data with strictly "high-level" data, and it has to know about
> @@ -861,6 +884,9 @@ struct net_device {
> 
>  	struct netdev_queue	rx_queue;
> 
> +	struct dev_rps_maps	*dev_rps_maps;	/* Per-NAPI maps for
> +						   receive packet steeing */
> +
>  	struct netdev_queue	*_tx ____cacheline_aligned_in_smp;
> 
>  	/* Number of TX queues allocated at alloc_netdev_mq() time  */
> @@ -1276,6 +1302,9 @@ struct softnet_data {
>  	struct Qdisc		*output_queue;
>  	struct sk_buff_head	input_pkt_queue;
>  	struct list_head	poll_list;
> +
> +	struct call_single_data	csd;

This is problematic. softnet_data used to be only used by local cpu.
With RPS, other cpus are going to access csd, input_pkt_queue, backlog
and dirty cache lines.

Maybe we should split sofnet_data in two cache lines, one private,
one chared, and 
DEFINE_PER_CPU(struct softnet_data, softnet_data);
-> 
DEFINE_PER_CPU_ALIGNED(struct softnet_data, softnet_data);

Also you need to change comment at start of struct softnet_data,
since it is wrong after RPS.

/*
 * Incoming packets are placed on per-cpu queues so that
 * no locking is needed.
 */

> +
>  	struct sk_buff		*completion_queue;
> 
>  	struct napi_struct	backlog;
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 63f4742..f188301 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -267,6 +267,7 @@ typedef unsigned char *sk_buff_data_t;
>   *	@mac_header: Link layer header
>   *	@_skb_dst: destination entry
>   *	@sp: the security path, used for xfrm
> + *	@rxhash: the packet hash computed on receive
>   *	@cb: Control buffer. Free for use by every layer. Put private vars here
>   *	@len: Length of actual data
>   *	@data_len: Data length
> @@ -323,6 +324,8 @@ struct sk_buff {
>  #ifdef CONFIG_XFRM
>  	struct	sec_path	*sp;
>  #endif
> +	__u32			rxhash;
> +
>  	/*
>  	 * This is the control buffer. It is free to use for every
>  	 * layer. Please put your private variables there. If you
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 9977288..f2c199c 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1834,7 +1834,7 @@ out_kfree_skb:
>  	return rc;
>  }
> 
> -static u32 skb_tx_hashrnd;
> +static u32 hashrnd;
adding a __read_mostly here could help :)

> 
>  u16 skb_tx_hash(const struct net_device *dev, const struct sk_buff *skb)
>  {
> @@ -1852,7 +1852,7 @@ u16 skb_tx_hash(const struct net_device *dev,
> const struct sk_buff *skb)
>  	else
>  		hash = skb->protocol;
> 
> -	hash = jhash_1word(hash, skb_tx_hashrnd);
> +	hash = jhash_1word(hash, hashrnd);
> 
>  	return (u16) (((u64) hash * dev->real_num_tx_queues) >> 32);
>  }
> @@ -2073,6 +2073,146 @@ int weight_p __read_mostly = 64;            /*
> old backlog weight */
> 
>  DEFINE_PER_CPU(struct netif_rx_stats, netdev_rx_stat) = { 0, };
> 
> +/*
> + * get_rps_cpu is called from netif_receive_skb and returns the target
> + * CPU from the RPS map of the receiving NAPI instance for a given skb.
> + */
> +static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb)
> +{
> +	u32 addr1, addr2, ports;
> +	struct ipv6hdr *ip6;
> +	struct iphdr *ip;
> +	u32 hash, ihl;
> +	u8 ip_proto;
> +	int cpu = -1;
> +	struct dev_rps_maps *drmap;
> +	struct rps_map *map = NULL;
> +	u16 index;
> +

> +	rcu_read_lock();
If called from netif_receive_skb(), we already are in a rcu protected section,
but this could be a cleanup patch, because many other parts in stack could
be changed as well.


> +/*
> + * enqueue_to_backlog is called to queue an skb to a per CPU backlog
> + * queue (may be a remote CPU queue).
> + */
> +static int enqueue_to_backlog(struct sk_buff *skb, int cpu)
> +{
> +	struct softnet_data *queue;
> +	unsigned long flags;
> +
> +	queue = &per_cpu(softnet_data, cpu);
> +


> +	local_irq_save(flags);
> +	__get_cpu_var(netdev_rx_stat).total++;
> +
> +	spin_lock(&queue->input_pkt_queue.lock);

could reduce to :
	percpu_add(netdev_rx_stat.total, 1);
	spin_lock_irqsave(&queue->input_pkt_queue.lock, flags);

> +	if (queue->input_pkt_queue.qlen <= netdev_max_backlog) {
> +		if (queue->input_pkt_queue.qlen) {
> +enqueue:
> +			__skb_queue_tail(&queue->input_pkt_queue, skb);
> +			spin_unlock_irqrestore(&queue->input_pkt_queue.lock,
> +			    flags);
> +			return NET_RX_SUCCESS;
> +		}
> +
> +		/* Schedule NAPI for backlog device */

> +		if (napi_schedule_prep(&queue->backlog)) {

  Is it legal (or wanter at all) to call napi_schedule_prep() on remote cpu backlog ?

> +			if (cpu != smp_processor_id()) {
> +				cpu_set(cpu,
> +				    get_cpu_var(rps_remote_softirq_cpus));
> +				__raise_softirq_irqoff(NET_RX_SOFTIRQ);
> +			} else
> +				__napi_schedule(&queue->backlog);

Why not the more easy :
	if (cpu != smp_processor_id())
		cpu_set(cpu, get_cpu_var(rps_remote_softirq_cpus));
	else
		napi_schedule(&queue->backlog);

This way we dont touch to remote backlog (and this backlog could stay in the private
cache line of remote cpu)

> +		}
> +		goto enqueue;
> +	}
> +

> +	spin_unlock(&queue->input_pkt_queue.lock);
> +
> +	__get_cpu_var(netdev_rx_stat).dropped++;
> +	local_irq_restore(flags);

-> 
	spin_unlock_irqrestore(&queue->input_pkt_queue.lock, flags);
	percpu_add(netdev_rx_stat.dropped, 1);


+/*
+ * net_rps_action sends any pending IPI's for rps.  This is only called from
+ * softirq and interrupts must be enabled.
+ */
+static void net_rps_action(void)
+{
+       int cpu;
+
+       /* Send pending IPI's to kick RPS processing on remote cpus. */
+       for_each_cpu_mask_nr(cpu, __get_cpu_var(rps_remote_softirq_cpus)) {
+               struct softnet_data *queue = &per_cpu(softnet_data, cpu);
+               __smp_call_function_single(cpu, &queue->csd, 0);
+       }
+       cpus_clear(__get_cpu_var(rps_remote_softirq_cpus));
+}


net_rps_action() might be not very descriptive name and bit expensive...

Did you tried smp_call_function_many() ?
(I suspect you did, but found it was not that optimized ?)

CC Andi to get feedback from him :)

static void net_rps_remcpus_fire(void)
{
	smp_call_function_many(__get_cpu_var(rps_remote_softirq_cpus),
			       trigger_softirq, NULL, 0);
}

Of course you would have to use following code as well :
(eg ignore void *data argument)

static void trigger_softirq(void *data)
{
/* kind of :
 * __napi_schedule(__get_cpu_var(softnet_data).backlog);
 * without local_irq_save(flags);/local_irq_restore(flags);
 */
	list_add_tail(&n->poll_list, &__get_cpu_var(softnet_data).poll_list);
	__raise_softirq_irqoff(NET_RX_SOFTIRQ);
}

Thanks Herbert


^ permalink raw reply

* [net-2.6 PATCH 3/3] netxen : fix BOND_MODE_TLB/ALB mode.
From: Amit Kumar Salecha @ 2009-11-21  8:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, dhananjay.phadke, Narender Kumar, Amit Kumar Salecha

From: Narender Kumar <narender.kumar@qlogic.com>

o Along with netdev->perm_addr, mac address will be
  maintained in device private structure.

o Device limitation: We need to set mac address when ever
  interface comes up.

In ALB/TAL mode, bonding driver calls set_mac for all slave with bond mac address.
But bonding driver set netdev->dev_addr field to its original value,
after enslaving interfaces.

When ever active slave changes, it swap dev_addr of inactive slave with active.
Yet it doesn't notify driver about change in netdev->dev_addr.

As netxen driver need to set mac addr when ever interface comes up,
it can't rely on netdev->dev_addr field. Specially in case of bonding mode ALB/TLB.

Signed-off-by: Narender Kumar <narender.kumar@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/netxen/netxen_nic.h      |    2 ++
 drivers/net/netxen/netxen_nic_hw.c   |    6 +++---
 drivers/net/netxen/netxen_nic_main.c |    4 +++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index 7384f59..e1237b8 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -1163,6 +1163,8 @@ struct netxen_adapter {
 	u32 int_vec_bit;
 	u32 heartbit;
 
+	u8 mac_addr[ETH_ALEN];
+
 	struct netxen_adapter_stats stats;
 
 	struct netxen_recv_context recv_ctx;
diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index e2c4a01..52a3798 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -463,7 +463,7 @@ netxen_nic_enable_mcast_filter(struct netxen_adapter *adapter)
 {
 	u32	val = 0;
 	u16 port = adapter->physical_port;
-	u8 *addr = adapter->netdev->dev_addr;
+	u8 *addr = adapter->mac_addr;
 
 	if (adapter->mc_enabled)
 		return 0;
@@ -492,7 +492,7 @@ netxen_nic_disable_mcast_filter(struct netxen_adapter *adapter)
 {
 	u32	val = 0;
 	u16 port = adapter->physical_port;
-	u8 *addr = adapter->netdev->dev_addr;
+	u8 *addr = adapter->mac_addr;
 
 	if (!adapter->mc_enabled)
 		return 0;
@@ -687,7 +687,7 @@ void netxen_p3_nic_set_multi(struct net_device *netdev)
 
 	list_splice_tail_init(&adapter->mac_list, &del_list);
 
-	nx_p3_nic_add_mac(adapter, netdev->dev_addr, &del_list);
+	nx_p3_nic_add_mac(adapter, adapter->mac_addr, &del_list);
 	nx_p3_nic_add_mac(adapter, bcast_addr, &del_list);
 
 	if (netdev->flags & IFF_PROMISC) {
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 0b4a56a..3bf78db 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -437,6 +437,7 @@ netxen_read_mac_addr(struct netxen_adapter *adapter)
 		netdev->dev_addr[i] = *(p + 5 - i);
 
 	memcpy(netdev->perm_addr, netdev->dev_addr, netdev->addr_len);
+	memcpy(adapter->mac_addr, netdev->dev_addr, netdev->addr_len);
 
 	/* set station address */
 
@@ -459,6 +460,7 @@ int netxen_nic_set_mac(struct net_device *netdev, void *p)
 		netxen_napi_disable(adapter);
 	}
 
+	memcpy(adapter->mac_addr, addr->sa_data, netdev->addr_len);
 	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
 	adapter->macaddr_set(adapter, addr->sa_data);
 
@@ -956,7 +958,7 @@ netxen_nic_up(struct netxen_adapter *adapter, struct net_device *netdev)
 		return err;
 	}
 	if (NX_IS_REVISION_P2(adapter->ahw.revision_id))
-		adapter->macaddr_set(adapter, netdev->dev_addr);
+		adapter->macaddr_set(adapter, adapter->mac_addr);
 
 	adapter->set_multi(netdev);
 	adapter->set_mtu(adapter, netdev->mtu);
-- 
1.5.6.1


^ permalink raw reply related

* Re: [RFC-PATCH] libiscsi dhcp handler
From: Dan Carpenter @ 2009-11-21  8:13 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Rakesh Ranjan, David Miller, michaelc, open-iscsi, netdev,
	linux-kernel, James.Bottomley, kxie
In-Reply-To: <b263e5900911200529n1c002097q48a625d1bcda4db6@mail.gmail.com>

On Fri, Nov 20, 2009 at 03:29:24PM +0200, Dan Carpenter wrote:
> On 11/16/09, Rakesh Ranjan <rakesh@chelsio.com> wrote:
> > David Miller wrote:
> >> From: Rakesh Ranjan <rakesh@chelsio.com>
> >> Date: Mon, 16 Nov 2009 18:41:49 +0530
> >>
> >>> Herein attached patches to support dhcp based provisioning for iSCSI
> >>> offload capable cards. I have made dhcp code as generic as possible,
> >>> please go through the code. Based on the feedback I will submit final
> >>> version of these patches.
> >>
> >> You can't really add objects to the build before the patch that
> >> adds the source for that object.
> >>
> >
> > Hi david,
> >
> > Fixed patch attached.

+       spin_unlock(&rcv_lock);                                                                                              
+                                                                                                                            
+drop:                                                                                                                       
+       kfree(pskb);                                                                                                         

This should be kfree_skb(pskb)

+out:                                                                                                                        
+       return rc;                                                                                                           
+}                                                                                                                           
+EXPORT_SYMBOL(libiscsi_ipconfig_recv);                                                                                      

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH v4 1/1] rps: core implementation
From: Tom Herbert @ 2009-11-21  9:03 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, Linux Netdev List, Andi Kleen
In-Reply-To: <4B079FDF.9040809@gmail.com>

>>   *   The DEVICE structure.
>>   *   Actually, this whole structure is a big mistake.  It mixes I/O
>>   *   data with strictly "high-level" data, and it has to know about
>> @@ -861,6 +884,9 @@ struct net_device {
>>
>>       struct netdev_queue     rx_queue;
>>
>> +     struct dev_rps_maps     *dev_rps_maps;  /* Per-NAPI maps for
>> +                                                receive packet steeing */
>> +
>>       struct netdev_queue     *_tx ____cacheline_aligned_in_smp;
>>
>>       /* Number of TX queues allocated at alloc_netdev_mq() time  */
>> @@ -1276,6 +1302,9 @@ struct softnet_data {
>>       struct Qdisc            *output_queue;
>>       struct sk_buff_head     input_pkt_queue;
>>       struct list_head        poll_list;
>> +
>> +     struct call_single_data csd;
>
> This is problematic. softnet_data used to be only used by local cpu.
> With RPS, other cpus are going to access csd, input_pkt_queue, backlog
> and dirty cache lines.
>
> Maybe we should split sofnet_data in two cache lines, one private,
> one chared, and
> DEFINE_PER_CPU(struct softnet_data, softnet_data);
> ->
> DEFINE_PER_CPU_ALIGNED(struct softnet_data, softnet_data);
>

That would make sense.

> Also you need to change comment at start of struct softnet_data,
> since it is wrong after RPS.
>
> /*
>  * Incoming packets are placed on per-cpu queues so that
>  * no locking is needed.
>  */
>
Okay.


>> -static u32 skb_tx_hashrnd;
>> +static u32 hashrnd;
> adding a __read_mostly here could help :)
>

Yep.

>>
>>  u16 skb_tx_hash(const struct net_device *dev, const struct sk_buff *skb)
>>  {
>> @@ -1852,7 +1852,7 @@ u16 skb_tx_hash(const struct net_device *dev,
>> const struct sk_buff *skb)
>>       else
>>               hash = skb->protocol;
>>
>> -     hash = jhash_1word(hash, skb_tx_hashrnd);
>> +     hash = jhash_1word(hash, hashrnd);
>>
>>       return (u16) (((u64) hash * dev->real_num_tx_queues) >> 32);
>>  }
>> @@ -2073,6 +2073,146 @@ int weight_p __read_mostly = 64;            /*
>> old backlog weight */
>>
>>  DEFINE_PER_CPU(struct netif_rx_stats, netdev_rx_stat) = { 0, };
>>
>> +/*
>> + * get_rps_cpu is called from netif_receive_skb and returns the target
>> + * CPU from the RPS map of the receiving NAPI instance for a given skb.
>> + */
>> +static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb)
>> +{
>> +     u32 addr1, addr2, ports;
>> +     struct ipv6hdr *ip6;
>> +     struct iphdr *ip;
>> +     u32 hash, ihl;
>> +     u8 ip_proto;
>> +     int cpu = -1;
>> +     struct dev_rps_maps *drmap;
>> +     struct rps_map *map = NULL;
>> +     u16 index;
>> +
>
>> +     rcu_read_lock();
> If called from netif_receive_skb(), we already are in a rcu protected section,
> but this could be a cleanup patch, because many other parts in stack could
> be changed as well.
>
So convention would be to not call these then?

>
>> +/*
>> + * enqueue_to_backlog is called to queue an skb to a per CPU backlog
>> + * queue (may be a remote CPU queue).
>> + */
>> +static int enqueue_to_backlog(struct sk_buff *skb, int cpu)
>> +{
>> +     struct softnet_data *queue;
>> +     unsigned long flags;
>> +
>> +     queue = &per_cpu(softnet_data, cpu);
>> +
>
>
>> +     local_irq_save(flags);
>> +     __get_cpu_var(netdev_rx_stat).total++;
>> +
>> +     spin_lock(&queue->input_pkt_queue.lock);
>
> could reduce to :
>        percpu_add(netdev_rx_stat.total, 1);
>        spin_lock_irqsave(&queue->input_pkt_queue.lock, flags);
>
Would it make sense to percpu_add into dev.c just for this when other
parts in dev.c would still use __get_cpu_var(stat)++?  Also, I think
this results in more instructions...

>> +     if (queue->input_pkt_queue.qlen <= netdev_max_backlog) {
>> +             if (queue->input_pkt_queue.qlen) {
>> +enqueue:
>> +                     __skb_queue_tail(&queue->input_pkt_queue, skb);
>> +                     spin_unlock_irqrestore(&queue->input_pkt_queue.lock,
>> +                         flags);
>> +                     return NET_RX_SUCCESS;
>> +             }
>> +
>> +             /* Schedule NAPI for backlog device */
>
>> +             if (napi_schedule_prep(&queue->backlog)) {
>
>  Is it legal (or wanter at all) to call napi_schedule_prep() on remote cpu backlog ?
>

I think it is useful.  If the remote backlog is scheduled then there
is no need to do an IPI, so this means there would only ever be on IPI
pending for a backlog queue and hence we can get away with only having
one call_function_data per backlog.  Without the napi schedule, we
would probably need a call structure for each backlog per CPU-- and
also we'd still need some shared bits between source and target CPUs
anyway for when the IPI is completed (want to avoid blocking on
call_function_data).

>> +                     if (cpu != smp_processor_id()) {
>> +                             cpu_set(cpu,
>> +                                 get_cpu_var(rps_remote_softirq_cpus));
>> +                             __raise_softirq_irqoff(NET_RX_SOFTIRQ);
>> +                     } else
>> +                             __napi_schedule(&queue->backlog);
>
> Why not the more easy :
>        if (cpu != smp_processor_id())
>                cpu_set(cpu, get_cpu_var(rps_remote_softirq_cpus));
>        else
>                napi_schedule(&queue->backlog);
>
> This way we dont touch to remote backlog (and this backlog could stay in the private
> cache line of remote cpu)
>
>> +             }
>> +             goto enqueue;
>> +     }
>> +
>
>> +     spin_unlock(&queue->input_pkt_queue.lock);
>> +
>> +     __get_cpu_var(netdev_rx_stat).dropped++;
>> +     local_irq_restore(flags);
>
> ->
>        spin_unlock_irqrestore(&queue->input_pkt_queue.lock, flags);
>        percpu_add(netdev_rx_stat.dropped, 1);
>
>
> +/*
> + * net_rps_action sends any pending IPI's for rps.  This is only called from
> + * softirq and interrupts must be enabled.
> + */
> +static void net_rps_action(void)
> +{
> +       int cpu;
> +
> +       /* Send pending IPI's to kick RPS processing on remote cpus. */
> +       for_each_cpu_mask_nr(cpu, __get_cpu_var(rps_remote_softirq_cpus)) {
> +               struct softnet_data *queue = &per_cpu(softnet_data, cpu);
> +               __smp_call_function_single(cpu, &queue->csd, 0);
> +       }
> +       cpus_clear(__get_cpu_var(rps_remote_softirq_cpus));
> +}
>
>
> net_rps_action() might be not very descriptive name and bit expensive...
>
> Did you tried smp_call_function_many() ?
> (I suspect you did, but found it was not that optimized ?)
>
> CC Andi to get feedback from him :)
>
> static void net_rps_remcpus_fire(void)
> {
>        smp_call_function_many(__get_cpu_var(rps_remote_softirq_cpus),
>                               trigger_softirq, NULL, 0);
> }
>

I'm thinking it is better to avoid possibly blocking on locked cfd_data.

Tom

^ permalink raw reply

* [net-next-2.6 PATCH 01/23] e1000e: check WoL mode is among set of supported modes
From: Jeff Kirsher @ 2009-11-21  9:22 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, Bruce Allan, Jeff Kirsher

From: Bruce Allan <bruce.w.allan@intel.com>

When setting WoL feature, check the supplied modes are all supported rather
than checking for no support.  This way, if any new modes are added the
driver does not default to not complaining about it if we don't really
support it.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/e1000e/ethtool.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index c430dc8..3af5ee4 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -1777,12 +1777,11 @@ static int e1000_set_wol(struct net_device *netdev,
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 
-	if (wol->wolopts & WAKE_MAGICSECURE)
-		return -EOPNOTSUPP;
-
 	if (!(adapter->flags & FLAG_HAS_WOL) ||
-	    !device_can_wakeup(&adapter->pdev->dev))
-		return wol->wolopts ? -EOPNOTSUPP : 0;
+	    !device_can_wakeup(&adapter->pdev->dev) ||
+	    (wol->wolopts & ~(WAKE_UCAST | WAKE_MCAST | WAKE_BCAST |
+	                      WAKE_MAGIC | WAKE_PHY | WAKE_ARP)))
+		return -EOPNOTSUPP;
 
 	/* these settings will always override what we currently have */
 	adapter->wol = 0;


^ permalink raw reply related

* [net-next-2.6 PATCH 02/23] e1000e: add missing tests for 82583 in ethtool functions
From: Jeff Kirsher @ 2009-11-21  9:22 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, Bruce Allan, Jeff Kirsher
In-Reply-To: <20091121092126.5715.41618.stgit@localhost.localdomain>

From: Bruce Allan <bruce.w.allan@intel.com>

Add tests for 82583 in a couple ethtool functions that were missed from the
initial hardware enablement submission.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/e1000e/ethtool.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index 3af5ee4..1ff43b4 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -604,7 +604,9 @@ static int e1000_set_eeprom(struct net_device *netdev,
 	 * and flush shadow RAM for applicable controllers
 	 */
 	if ((first_word <= NVM_CHECKSUM_REG) ||
-	    (hw->mac.type == e1000_82574) || (hw->mac.type == e1000_82573))
+	    (hw->mac.type == e1000_82583) ||
+	    (hw->mac.type == e1000_82574) ||
+	    (hw->mac.type == e1000_82573))
 		ret_val = e1000e_update_nvm_checksum(hw);
 
 out:
@@ -1839,6 +1841,7 @@ static int e1000_phys_id(struct net_device *netdev, u32 data)
 
 	if ((hw->phy.type == e1000_phy_ife) ||
 	    (hw->mac.type == e1000_pchlan) ||
+	    (hw->mac.type == e1000_82583) ||
 	    (hw->mac.type == e1000_82574)) {
 		INIT_WORK(&adapter->led_blink_task, e1000e_led_blink_task);
 		if (!adapter->blink_timer.function) {


^ permalink raw reply related

* [net-next-2.6 PATCH 03/23] e1000e: clearing interrupt timers causes descriptors to get flushed
From: Jeff Kirsher @ 2009-11-21  9:22 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, Bruce Allan, Jeff Kirsher
In-Reply-To: <20091121092126.5715.41618.stgit@localhost.localdomain>

From: Bruce Allan <bruce.w.allan@intel.com>

Clearing the interrupt timers following an IMS clear has the unwanted
side-effect of flushing all descriptors immediately following a partial
write when interrupts are disabled.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/e1000e/netdev.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 3caa1d5..3845fb6 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -2441,8 +2441,6 @@ static void e1000_configure_rx(struct e1000_adapter *adapter)
 		ew32(ITR, 1000000000 / (adapter->itr * 256));
 
 	ctrl_ext = er32(CTRL_EXT);
-	/* Reset delay timers after every interrupt */
-	ctrl_ext |= E1000_CTRL_EXT_INT_TIMER_CLR;
 	/* Auto-Mask interrupts upon ICR access */
 	ctrl_ext |= E1000_CTRL_EXT_IAME;
 	ew32(IAM, 0xffffffff);


^ permalink raw reply related

* [net-next-2.6 PATCH 04/23] e1000e: function pointers for ethtool set/get offloads
From: Jeff Kirsher @ 2009-11-21  9:22 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, Bruce Allan, Jeff Kirsher
In-Reply-To: <20091121092126.5715.41618.stgit@localhost.localdomain>

From: Bruce Allan <bruce.w.allan@intel.com>

Provide missing function pointers for ethtool set/get offloads.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/e1000e/ethtool.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index 1ff43b4..3d73f20 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -1996,6 +1996,8 @@ static const struct ethtool_ops e1000_ethtool_ops = {
 	.get_sset_count		= e1000e_get_sset_count,
 	.get_coalesce		= e1000_get_coalesce,
 	.set_coalesce		= e1000_set_coalesce,
+	.get_flags		= ethtool_op_get_flags,
+	.set_flags		= ethtool_op_set_flags,
 };
 
 void e1000e_set_ethtool_ops(struct net_device *netdev)


^ permalink raw reply related

* [net-next-2.6 PATCH 05/23] e1000e: don't clean Rx ring while resetting
From: Jeff Kirsher @ 2009-11-21  9:23 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, Bruce Allan, Jeff Kirsher
In-Reply-To: <20091121092126.5715.41618.stgit@localhost.localdomain>

From: Bruce Allan <bruce.w.allan@intel.com>

When using legacy interrupts, do not clean the Rx ring while resetting
otherwise traffic will not pass.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/e1000e/netdev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 3845fb6..e819f19 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -1176,7 +1176,7 @@ static irqreturn_t e1000_intr(int irq, void *data)
 	struct e1000_hw *hw = &adapter->hw;
 	u32 rctl, icr = er32(ICR);
 
-	if (!icr)
+	if (!icr || test_bit(__E1000_DOWN, &adapter->state))
 		return IRQ_NONE;  /* Not our interrupt */
 
 	/*


^ permalink raw reply related


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