Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-13  6:16 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S. Miller, Stephen Hemminger, Patrick McHardy, Tom Herbert,
	netdev
In-Reply-To: <4AFCF50D.6040106@gmail.com>

2009/11/13 Eric Dumazet <eric.dumazet@gmail.com>:
>
> Messy ? Because of few tests added in code, and branches always
> correctly predicted ?
>
> Still some people might rely on tasklet instead of workqueues
> and added scheduler stress and latency penalty. Tasklet are softirq
> and normally are processed a few nanosecs later than RX softirq,
> on the same CPU, while with your workqueue, I guess the scheduler will
> try to not migrate it, so we add a penalty for light to moderate load.
>
> I guess this new ifb mode would be a regression for them ?
>
> If you dont want to maintain a compatibility mode, maybe you
> should introduce a complete new driver, drivers/net/ifbmq.c or ifbwq.c
>
> (multiqueue or workqueue references)
>

It sounds a good idea.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [net-next-2.6 PATCH v6 2/7 RFC] TCPCT part 1b: generate Responder Cookie
From: Eric Dumazet @ 2009-11-13  6:21 UTC (permalink / raw)
  To: William Allen Simpson; +Cc: Linux Kernel Network Developers
In-Reply-To: <4AFCDDC4.1090606@gmail.com>

William Allen Simpson a écrit :
> Define (missing) hash message size for SHA1.
> 
> Define hashing size constants specific to TCP cookies, and add new
> function.
> 
> Maintain global secret values for tcp_cookie_generator().
> 
> This is a significantly revised implementation of earlier (15-year-old)
> Photuris [RFC-2522] code for the KA9Q cooperative multitasking platform.
> 
> Linux RCU technique appears to be well-suited to this application, though
> neither of the circular queue items are freed.
> 
> These functions will also be used in subsequent patches that implement
> additional features.
> 
> Signed-off-by: William.Allen.Simpson@gmail.com
> ---
>  include/linux/cryptohash.h |    1 +
>  include/net/tcp.h          |    8 +++
>  net/ipv4/tcp.c             |  146
> ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 155 insertions(+), 0 deletions(-)
> 

Small point :

+	if (unlikely(time_after_eq(jiffy, tcp_secret_generating->expires))) {
+		spin_lock_bh(&tcp_secret_locker);
+		if (!time_after_eq(jiffy, tcp_secret_generating->expires)) {
+			/* refreshed by another */
+			spin_unlock_bh(&tcp_secret_locker);
+			memcpy(bakery,
+			       &tcp_secret_generating->secrets[0],
+			       sizeof(tcp_secret_generating->secrets));

Technically speaking, you should perform the memcpy() before spin_unlock_bh()

+	if (unlikely(time_after_eq(jiffy, tcp_secret_generating->expires))) {
+		spin_lock_bh(&tcp_secret_locker);
+		if (!time_after_eq(jiffy, tcp_secret_generating->expires)) {
+			/* refreshed by another */
+			memcpy(bakery,
+			       &tcp_secret_generating->secrets[0],
+			       sizeof(tcp_secret_generating->secrets));
+			spin_unlock_bh(&tcp_secret_locker);


^ permalink raw reply

* Re: [net-next-2.6 PATCH] net: fast consecutive name allocation
From: Stephen Hemminger @ 2009-11-13  6:26 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Octavian Purdila, netdev
In-Reply-To: <4AFCF8D3.6090905@gmail.com>

On Fri, 13 Nov 2009 07:12:35 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Octavian Purdila a écrit :
> > On Friday 13 November 2009 07:01:14 you wrote:
> >> This patch speeds up the network device name allocation for the case
> >> where a significant number of devices of the same type are created
> >> consecutively.
> >>
> >> Tests performed on a PPC750 @ 800Mhz machine with per device sysctl
> >> and sysfs entries disabled:
> >>
> >> Without the patch           With the patch
> >>
> >> real    0m 43.43s	    real    0m 0.49s
> >> user    0m 0.00s	    user    0m 0.00s
> >> sys     0m 43.43s	    sys     0m 0.48s
> >>
> > 

No one has give a reasonable use case for this network device name
explosion, what is the benchmark doing this nosense, and how do I
get paid to do it...

But I have to say no for another reason. You cause the kernel to choose
a different name for the case where a device is deleted or renamed.
The old code would find and fill the hole when a new device was added.

Since this is a semantic ABI change, the kind that drives users nuts.

-- 

^ permalink raw reply

* Re: [net-next-2.6 PATCH v6 2/7 RFC] TCPCT part 1b: generate Responder Cookie
From: Joe Perches @ 2009-11-13  6:26 UTC (permalink / raw)
  To: William Allen Simpson; +Cc: Linux Kernel Network Developers
In-Reply-To: <4AFCDDC4.1090606@gmail.com>

On Thu, 2009-11-12 at 23:17 -0500, William Allen Simpson wrote:
> +int tcp_cookie_generator(u32 *bakery)
> +{
> +	unsigned long jiffy = jiffies;
> +
> +	if (unlikely(time_after_eq(jiffy, tcp_secret_generating->expires))) {
> +		spin_lock_bh(&tcp_secret_locker);
> +		if (!time_after_eq(jiffy, tcp_secret_generating->expires)) {
> +			/* refreshed by another */
> +			spin_unlock_bh(&tcp_secret_locker);
> +			memcpy(bakery,
> +			       &tcp_secret_generating->secrets[0],
> +			       sizeof(tcp_secret_generating->secrets));
> +		} else {
> +			u32 secrets[COOKIE_WORKSPACE_WORDS];

Rather than using stack, could you use
u32 *secrets = &tcp_secret_secondary->secrets[0];



^ permalink raw reply

* Re: [net-next-2.6 PATCH v6 4/7 RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's
From: Eric Dumazet @ 2009-11-13  6:32 UTC (permalink / raw)
  To: William Allen Simpson; +Cc: Linux Kernel Network Developers
In-Reply-To: <4AFCE65C.2010104@gmail.com>

William Allen Simpson a écrit :
> Data structures are carefully composed to require minimal additions.
> For example, the struct tcp_options_received cookie_plus variable fits
> between existing 16-bit and 8-bit variables, requiring no additional
> space (taking alignment into consideration).  There are no additions to
> tcp_request_sock, and only 1 pointer in tcp_sock.
> 
> This is a significantly revised implementation of an earlier (year-old)
> patch that no longer applies cleanly, with permission of the original
> author (Adam Langley):
> 
>    http://thread.gmane.org/gmane.linux.network/102586
> 
> The principle difference is using a TCP option to carry the cookie nonce,
> instead of a user configured offset in the data.  This is more flexible and
> less subject to user configuration error.  Such a cookie option has been
> suggested for many years, and is also useful without SYN data, allowing
> several related concepts to use the same extension option.
> 
>    "Re: SYN floods (was: does history repeat itself?)", September 9, 1996.
>    http://www.merit.net/mail.archives/nanog/1996-09/msg00235.html
> 
>    "Re: what a new TCP header might look like", May 12, 1998.
>    ftp://ftp.isi.edu/end2end/end2end-interest-1998.mail
> 
> These functions will also be used in subsequent patches that implement
> additional features.
> 
> Requires:
>   TCPCT part 1a: add request_values parameter for sending SYNACK
>   TCPCT part 1b: generate Responder Cookie
>   TCPCT part 1c: sysctl_tcp_cookie_size, socket option
> TCP_COOKIE_TRANSACTIONS
> 
> Signed-off-by: William.Allen.Simpson@gmail.com
> ---
>  include/linux/tcp.h |   29 ++++++++++++++++----
>  include/net/tcp.h   |   72
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 95 insertions(+), 6 deletions(-)
> 

+/**
+ * A tcp_sock contains a pointer to the current value, and this is cloned to
+ * the tcp_timewait_sock.
+ *
+ * @cookie_pair:	variable data from the option exchange.
+ *
+ * @cookie_desired:	user specified tcpct_cookie_desired.  Zero
+ *			indicates default (sysctl_tcp_cookie_size).
+ *			After cookie sent, remembers size of cookie.
+ *			Range 0, TCP_COOKIE_MIN to TCP_COOKIE_MAX.
+ *
+ * @s_data_desired:	user specified tcpct_s_data_desired.  When the
+ *			constant payload is specified (@s_data_constant),
+ *			holds its length instead.
+ *			Range 0 to TCP_MSS_DESIRED.
+ *
+ * @s_data_payload:	constant data that is to be included in the
+ *			payload of SYN or SYNACK segments when the
+ *			cookie option is present.
+ */

Thanks for this kerneldoc William ;)

But header should be :
/**
 *	struct tcp_cookie_values - Some description...



^ permalink raw reply

* (unknown)
From: Wu Fengguang @ 2009-11-13  6:35 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: LKML, Yin, Kangkai, netdev

netfilter: nf_log: fix sleeping function called from invalid context in seq_show()

[  171.925285] BUG: sleeping function called from invalid context at kernel/mutex.c:280
[  171.925296] in_atomic(): 1, irqs_disabled(): 0, pid: 671, name: grep
[  171.925306] 2 locks held by grep/671:
[  171.925312]  #0:  (&p->lock){+.+.+.}, at: [<c10b8acd>] seq_read+0x25/0x36c
[  171.925340]  #1:  (rcu_read_lock){.+.+..}, at: [<c1391dac>] seq_start+0x0/0x44
[  171.925372] Pid: 671, comm: grep Not tainted 2.6.31.6-4-netbook #3
[  171.925380] Call Trace:
[  171.925398]  [<c105104e>] ? __debug_show_held_locks+0x1e/0x20
[  171.925414]  [<c10264ac>] __might_sleep+0xfb/0x102
[  171.925430]  [<c1461521>] mutex_lock_nested+0x1c/0x2ad
[  171.925444]  [<c1391c9e>] seq_show+0x74/0x127
[  171.925456]  [<c10b8c5c>] seq_read+0x1b4/0x36c
[  171.925469]  [<c10b8aa8>] ? seq_read+0x0/0x36c
[  171.925483]  [<c10d5c8e>] proc_reg_read+0x60/0x74
[  171.925496]  [<c10d5c2e>] ? proc_reg_read+0x0/0x74
[  171.925510]  [<c10a4468>] vfs_read+0x87/0x110
[  171.925523]  [<c10a458a>] sys_read+0x3b/0x60
[  171.925538]  [<c1002a49>] syscall_call+0x7/0xb

Fix it by replacing RCU with nf_log_mutex.

CC: Patrick McHardy <kaber@trash.net>
Reported-by: "Yin, Kangkai" <kangkai.yin@intel.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 net/netfilter/nf_log.c |   18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

--- sound-2.6.orig/net/netfilter/nf_log.c	2009-11-13 13:47:14.000000000 +0800
+++ sound-2.6/net/netfilter/nf_log.c	2009-11-13 13:49:23.000000000 +0800
@@ -128,9 +128,8 @@ EXPORT_SYMBOL(nf_log_packet);
 
 #ifdef CONFIG_PROC_FS
 static void *seq_start(struct seq_file *seq, loff_t *pos)
-	__acquires(RCU)
 {
-	rcu_read_lock();
+	mutex_lock(&nf_log_mutex);
 
 	if (*pos >= ARRAY_SIZE(nf_loggers))
 		return NULL;
@@ -149,9 +148,8 @@ static void *seq_next(struct seq_file *s
 }
 
 static void seq_stop(struct seq_file *s, void *v)
-	__releases(RCU)
 {
-	rcu_read_unlock();
+	mutex_unlock(&nf_log_mutex);
 }
 
 static int seq_show(struct seq_file *s, void *v)
@@ -161,7 +159,7 @@ static int seq_show(struct seq_file *s, 
 	struct nf_logger *t;
 	int ret;
 
-	logger = rcu_dereference(nf_loggers[*pos]);
+	logger = nf_loggers[*pos];
 
 	if (!logger)
 		ret = seq_printf(s, "%2lld NONE (", *pos);
@@ -171,22 +169,16 @@ static int seq_show(struct seq_file *s, 
 	if (ret < 0)
 		return ret;
 
-	mutex_lock(&nf_log_mutex);
 	list_for_each_entry(t, &nf_loggers_l[*pos], list[*pos]) {
 		ret = seq_printf(s, "%s", t->name);
-		if (ret < 0) {
-			mutex_unlock(&nf_log_mutex);
+		if (ret < 0)
 			return ret;
-		}
 		if (&t->list[*pos] != nf_loggers_l[*pos].prev) {
 			ret = seq_printf(s, ",");
-			if (ret < 0) {
-				mutex_unlock(&nf_log_mutex);
+			if (ret < 0)
 				return ret;
-			}
 		}
 	}
-	mutex_unlock(&nf_log_mutex);
 
 	return seq_printf(s, ")\n");
 }

^ permalink raw reply

* [PATCH] netfilter: nf_log: fix sleeping function called from invalid context in seq_show()
From: Wu Fengguang @ 2009-11-13  6:36 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: LKML, Yin, Kangkai, netdev

[  171.925285] BUG: sleeping function called from invalid context at kernel/mutex.c:280
[  171.925296] in_atomic(): 1, irqs_disabled(): 0, pid: 671, name: grep
[  171.925306] 2 locks held by grep/671:
[  171.925312]  #0:  (&p->lock){+.+.+.}, at: [<c10b8acd>] seq_read+0x25/0x36c
[  171.925340]  #1:  (rcu_read_lock){.+.+..}, at: [<c1391dac>] seq_start+0x0/0x44
[  171.925372] Pid: 671, comm: grep Not tainted 2.6.31.6-4-netbook #3
[  171.925380] Call Trace:
[  171.925398]  [<c105104e>] ? __debug_show_held_locks+0x1e/0x20
[  171.925414]  [<c10264ac>] __might_sleep+0xfb/0x102
[  171.925430]  [<c1461521>] mutex_lock_nested+0x1c/0x2ad
[  171.925444]  [<c1391c9e>] seq_show+0x74/0x127
[  171.925456]  [<c10b8c5c>] seq_read+0x1b4/0x36c
[  171.925469]  [<c10b8aa8>] ? seq_read+0x0/0x36c
[  171.925483]  [<c10d5c8e>] proc_reg_read+0x60/0x74
[  171.925496]  [<c10d5c2e>] ? proc_reg_read+0x0/0x74
[  171.925510]  [<c10a4468>] vfs_read+0x87/0x110
[  171.925523]  [<c10a458a>] sys_read+0x3b/0x60
[  171.925538]  [<c1002a49>] syscall_call+0x7/0xb

Fix it by replacing RCU with nf_log_mutex.

CC: Patrick McHardy <kaber@trash.net>
Reported-by: "Yin, Kangkai" <kangkai.yin@intel.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 net/netfilter/nf_log.c |   18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

--- sound-2.6.orig/net/netfilter/nf_log.c	2009-11-13 13:47:14.000000000 +0800
+++ sound-2.6/net/netfilter/nf_log.c	2009-11-13 13:49:23.000000000 +0800
@@ -128,9 +128,8 @@ EXPORT_SYMBOL(nf_log_packet);
 
 #ifdef CONFIG_PROC_FS
 static void *seq_start(struct seq_file *seq, loff_t *pos)
-	__acquires(RCU)
 {
-	rcu_read_lock();
+	mutex_lock(&nf_log_mutex);
 
 	if (*pos >= ARRAY_SIZE(nf_loggers))
 		return NULL;
@@ -149,9 +148,8 @@ static void *seq_next(struct seq_file *s
 }
 
 static void seq_stop(struct seq_file *s, void *v)
-	__releases(RCU)
 {
-	rcu_read_unlock();
+	mutex_unlock(&nf_log_mutex);
 }
 
 static int seq_show(struct seq_file *s, void *v)
@@ -161,7 +159,7 @@ static int seq_show(struct seq_file *s, 
 	struct nf_logger *t;
 	int ret;
 
-	logger = rcu_dereference(nf_loggers[*pos]);
+	logger = nf_loggers[*pos];
 
 	if (!logger)
 		ret = seq_printf(s, "%2lld NONE (", *pos);
@@ -171,22 +169,16 @@ static int seq_show(struct seq_file *s, 
 	if (ret < 0)
 		return ret;
 
-	mutex_lock(&nf_log_mutex);
 	list_for_each_entry(t, &nf_loggers_l[*pos], list[*pos]) {
 		ret = seq_printf(s, "%s", t->name);
-		if (ret < 0) {
-			mutex_unlock(&nf_log_mutex);
+		if (ret < 0)
 			return ret;
-		}
 		if (&t->list[*pos] != nf_loggers_l[*pos].prev) {
 			ret = seq_printf(s, ",");
-			if (ret < 0) {
-				mutex_unlock(&nf_log_mutex);
+			if (ret < 0)
 				return ret;
-			}
 		}
 	}
-	mutex_unlock(&nf_log_mutex);
 
 	return seq_printf(s, ")\n");
 }

^ permalink raw reply

* Re: Problem with VLANs and via-velocity driver
From: Patrick McHardy @ 2009-11-13  6:40 UTC (permalink / raw)
  To: Kevin Shanahan; +Cc: netdev
In-Reply-To: <20091113033217.GQ838@cubit>

Kevin Shanahan wrote:
> Hi,
> 
> I've had some problems with getting a fairly simple (I thought) VLAN
> configuration working with the on board Via NICs on my Via M700
> board. Looks like as soon as a tagged VLAN interface is added, the
> underlying "raw" (untagged) interface stops responding.
> 
> ...
> 
> A bit of searching found a few references to similar problems going
> back a few years (2005, 2007). Sounded like there were some driver
> issues, but it wasn't clear from the messages I found whether they
> were believed to be fixed or not. I tried the same test using a
> differnt NIC with the tg3 driver and there were no problems, so it
> looks to me like it's still a via-velocity issue. Unfortunately I
> don't have room to add NICs to this machine and need to use the on
> board Via hardware.

There's some special-casing for VID 0 in velocity_init_cam_filter().
Does "ip link add link eth0 type vlan id 0" make any difference?
If not, does "ip link set eth0 promisc on"?

^ permalink raw reply

* Re: [net-next-2.6 PATCH] net: fast consecutive name allocation
From: Eric Dumazet @ 2009-11-13  7:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Octavian Purdila, netdev
In-Reply-To: <20091112222608.79d90e9e@nehalam>

Stephen Hemminger a écrit :
> No one has give a reasonable use case for this network device name
> explosion, what is the benchmark doing this nosense, and how do I
> get paid to do it...
> 
> But I have to say no for another reason. You cause the kernel to choose
> a different name for the case where a device is deleted or renamed.
> The old code would find and fill the hole when a new device was added.
> 
> Since this is a semantic ABI change, the kind that drives users nuts.
> 

Yes, I see your point Stephen, but it would be trivial to skip already
used devices as well with litle change.



^ permalink raw reply

* Re: [PATCH] act_mirred: cleanup and optimization
From: Patrick McHardy @ 2009-11-13  7:13 UTC (permalink / raw)
  To: xiaosuo; +Cc: Jamal Hadi Salim, Stephen Hemminger, David S. Miller, netdev
In-Reply-To: <4AFCF06B.1090602@gmail.com>

Changli Gao wrote:
> act_mirred: cleanup and optimization.
> 
> cleanup and optimization.
> 1. don't let go back using goto.
> 2. move checking if eaction is valid in tcf_mirred_init().
> 3. don't call skb_act_clone() until it is necessary.
> 4. one exit of the critical context.
> 5. allow eaction is TCA_INGRESS_MIRROR & TCA_INGRESS_REDIR.
> 

>  	if (parm->ifindex) {
> -		dev = __dev_get_by_index(&init_net, parm->ifindex);
> +		dev = dev_get_by_index(&init_net, parm->ifindex);
>  		if (dev == NULL)
>  			return -ENODEV;

This change is not mentioned in the changelog and introduces a
leak. Please split your patches into reasonable portions doing
one change at a time.

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Patrick McHardy @ 2009-11-13  7:18 UTC (permalink / raw)
  To: Changli Gao
  Cc: David S. Miller, Stephen Hemminger, Eric Dumazet, Tom Herbert,
	netdev
In-Reply-To: <412e6f7f0911121732j106f96a2rb77f68098b7fd47e@mail.gmail.com>

Changli Gao wrote:
> On Thu, Nov 12, 2009 at 11:11 PM, Patrick McHardy <kaber@trash.net> wrote:
>> Changli Gao wrote:
>>> The corresponding iprout2 patch is attached.
>> Printing the value during dumps is missing from this patch.
>>
> 
> I doesn't know the rules about adding more dumps. Can I add it in the
> end of the corresponding line?
> 
> ifb0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN qlen 32 ntxqs 4

Yes, sounds fine.

^ permalink raw reply

* Re: [PATCH] act_mirred: cleanup and optimization
From: Changli Gao @ 2009-11-13  7:22 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Jamal Hadi Salim, Stephen Hemminger, David S. Miller, netdev
In-Reply-To: <4AFD0707.50207@trash.net>

On Fri, Nov 13, 2009 at 3:13 PM, Patrick McHardy <kaber@trash.net> wrote:
> Changli Gao wrote:
>> act_mirred: cleanup and optimization.
>>
>> cleanup and optimization.
>> 1. don't let go back using goto.
>> 2. move checking if eaction is valid in tcf_mirred_init().
>> 3. don't call skb_act_clone() until it is necessary.
>> 4. one exit of the critical context.
>> 5. allow eaction is TCA_INGRESS_MIRROR & TCA_INGRESS_REDIR.
>>
>
>>       if (parm->ifindex) {
>> -             dev = __dev_get_by_index(&init_net, parm->ifindex);
>> +             dev = dev_get_by_index(&init_net, parm->ifindex);
>>               if (dev == NULL)
>>                       return -ENODEV;
>
> This change is not mentioned in the changelog and introduces a
> leak. Please split your patches into reasonable portions doing
> one change at a time.
>

Leak? Do I forget to put it some where? I'll keep it as a whole until
Jamal thinks it is in the right form and direction.


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH] act_mirred: cleanup and optimization
From: Patrick McHardy @ 2009-11-13  7:25 UTC (permalink / raw)
  To: Changli Gao; +Cc: Jamal Hadi Salim, Stephen Hemminger, David S. Miller, netdev
In-Reply-To: <412e6f7f0911122322w754f5686jac6c93d39fccab46@mail.gmail.com>

Changli Gao wrote:
> On Fri, Nov 13, 2009 at 3:13 PM, Patrick McHardy <kaber@trash.net> wrote:
>> Changli Gao wrote:
>>> act_mirred: cleanup and optimization.
>>>
>>> cleanup and optimization.
>>> 1. don't let go back using goto.
>>> 2. move checking if eaction is valid in tcf_mirred_init().
>>> 3. don't call skb_act_clone() until it is necessary.
>>> 4. one exit of the critical context.
>>> 5. allow eaction is TCA_INGRESS_MIRROR & TCA_INGRESS_REDIR.
>>>
>>>       if (parm->ifindex) {
>>> -             dev = __dev_get_by_index(&init_net, parm->ifindex);
>>> +             dev = dev_get_by_index(&init_net, parm->ifindex);
>>>               if (dev == NULL)
>>>                       return -ENODEV;
>> This change is not mentioned in the changelog and introduces a
>> leak. Please split your patches into reasonable portions doing
>> one change at a time.
>>
> 
> Leak? Do I forget to put it some where? I'll keep it as a whole until
> Jamal thinks it is in the right form and direction.

Ah sorry, I misread the patch.

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Jarek Poplawski @ 2009-11-13  7:45 UTC (permalink / raw)
  To: Changli Gao
  Cc: Eric Dumazet, David S. Miller, Stephen Hemminger, Patrick McHardy,
	Tom Herbert, netdev
In-Reply-To: <412e6f7f0911122216u6880e855g6a15dac29ad6a100@mail.gmail.com>

On 13-11-2009 07:16, Changli Gao wrote:
> 2009/11/13 Eric Dumazet <eric.dumazet@gmail.com>:
>> Messy ? Because of few tests added in code, and branches always
>> correctly predicted ?
>>
>> Still some people might rely on tasklet instead of workqueues
>> and added scheduler stress and latency penalty. Tasklet are softirq
>> and normally are processed a few nanosecs later than RX softirq,
>> on the same CPU, while with your workqueue, I guess the scheduler will
>> try to not migrate it, so we add a penalty for light to moderate load.
>>
>> I guess this new ifb mode would be a regression for them ?
>>
>> If you dont want to maintain a compatibility mode, maybe you
>> should introduce a complete new driver, drivers/net/ifbmq.c or ifbwq.c
>>
>> (multiqueue or workqueue references)
>>
> 
> It sounds a good idea.

I don't think so. There would be a lot of code duplication and later
maintenance problems only because of the scheduling method. The main
question is to establish if there is really no performance difference
(which I doubt) - unless Changli can show some tests for various
setups now. On the other hand, if there is a difference, why keep
ineffective solution - similar thing should be possible to do in the
softirq context as well.

So it should not be a big problem to do it a bit messy for some
testing time. Since we can use separate ->ndo_start_xmit() etc. it
shouldn't be too messy, I guess.

Jarek P.

^ permalink raw reply

* [patch 0/4] s390: networking patches for 2.6.33
From: Ursula Braun @ 2009-11-13  7:46 UTC (permalink / raw)
  To: davem, netdev, linux-s390; +Cc: schwidefsky, heiko.carstens

Hi Dave,

here is a set of networking patches for 2.6.33.

shortlog:
Frank  Blaschka (1)
ctcm: suspend has to wait for outstanding I/O

Ursula Braun (3)
iucv: add work_queue cleanup for suspend
s390: remove cu3088 layer for lcs and ctcm
netiucv: displayed TX bytes value much too high

Thanks,
	Ursula

^ permalink raw reply

* [patch 1/4] [PATCH] iucv: add work_queue cleanup for suspend
From: Ursula Braun @ 2009-11-13  7:46 UTC (permalink / raw)
  To: davem, netdev, linux-s390; +Cc: schwidefsky, heiko.carstens, Ursula Braun
In-Reply-To: <20091113074626.078348000@linux.vnet.ibm.com>

[-- Attachment #1: 603-iucv-suspend-cleanup.diff --]
[-- Type: text/plain, Size: 1730 bytes --]

From: Ursula Braun <ursula.braun@de.ibm.com>

If iucv_work_queue is not empty during kernel freeze, a kernel panic
occurs. This suspend-patch adds flushing of the work queue for
pending connection requests and severing of remaining pending
connections.

Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---

 net/iucv/iucv.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Index: linux-next-uschi/net/iucv/iucv.c
===================================================================
--- linux-next-uschi.orig/net/iucv/iucv.c
+++ linux-next-uschi/net/iucv/iucv.c
@@ -1768,7 +1768,6 @@ static void iucv_tasklet_fn(unsigned lon
  */
 static void iucv_work_fn(struct work_struct *work)
 {
-	typedef void iucv_irq_fn(struct iucv_irq_data *);
 	LIST_HEAD(work_queue);
 	struct iucv_irq_list *p, *n;
 
@@ -1878,14 +1877,25 @@ int iucv_path_table_empty(void)
 static int iucv_pm_freeze(struct device *dev)
 {
 	int cpu;
+	struct iucv_irq_list *p, *n;
 	int rc = 0;
 
 #ifdef CONFIG_PM_DEBUG
 	printk(KERN_WARNING "iucv_pm_freeze\n");
 #endif
+	if (iucv_pm_state != IUCV_PM_FREEZING) {
+		for_each_cpu_mask_nr(cpu, iucv_irq_cpumask)
+			smp_call_function_single(cpu, iucv_block_cpu_almost,
+						 NULL, 1);
+		cancel_work_sync(&iucv_work);
+		list_for_each_entry_safe(p, n, &iucv_work_queue, list) {
+			list_del_init(&p->list);
+			iucv_sever_pathid(p->data.ippathid,
+					  iucv_error_no_listener);
+			kfree(p);
+		}
+	}
 	iucv_pm_state = IUCV_PM_FREEZING;
-	for_each_cpu_mask_nr(cpu, iucv_irq_cpumask)
-		smp_call_function_single(cpu, iucv_block_cpu_almost, NULL, 1);
 	if (dev->driver && dev->driver->pm && dev->driver->pm->freeze)
 		rc = dev->driver->pm->freeze(dev);
 	if (iucv_path_table_empty())


^ permalink raw reply

* [patch 4/4] [PATCH] netiucv: displayed TX bytes value much too high
From: Ursula Braun @ 2009-11-13  7:46 UTC (permalink / raw)
  To: davem, netdev, linux-s390; +Cc: schwidefsky, heiko.carstens, Ursula Braun
In-Reply-To: <20091113074626.078348000@linux.vnet.ibm.com>

[-- Attachment #1: 616-netiucv-tx-bytes.diff --]
[-- Type: text/plain, Size: 938 bytes --]

From: Ursula Braun <ursula.braun@de.ibm.com>

tx_bytes value must be updated by skb length before skb is freed.

Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---

 drivers/s390/net/netiucv.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-next-uschi/drivers/s390/net/netiucv.c
===================================================================
--- linux-next-uschi.orig/drivers/s390/net/netiucv.c
+++ linux-next-uschi/drivers/s390/net/netiucv.c
@@ -739,13 +739,13 @@ static void conn_action_txdone(fsm_insta
 	if (single_flag) {
 		if ((skb = skb_dequeue(&conn->commit_queue))) {
 			atomic_dec(&skb->users);
-			dev_kfree_skb_any(skb);
 			if (privptr) {
 				privptr->stats.tx_packets++;
 				privptr->stats.tx_bytes +=
 					(skb->len - NETIUCV_HDRLEN
-					 	  - NETIUCV_HDRLEN);
+						  - NETIUCV_HDRLEN);
 			}
+			dev_kfree_skb_any(skb);
 		}
 	}
 	conn->tx_buff->data = conn->tx_buff->head;


^ permalink raw reply

* [patch 2/4] [PATCH] ctcm: suspend has to wait for outstanding I/O
From: Ursula Braun @ 2009-11-13  7:46 UTC (permalink / raw)
  To: davem, netdev, linux-s390
  Cc: schwidefsky, heiko.carstens, Frank Blaschka, Ursula Braun
In-Reply-To: <20091113074626.078348000@linux.vnet.ibm.com>

[-- Attachment #1: 614-ctcm-suspend-wait.diff --]
[-- Type: text/plain, Size: 2270 bytes --]

From: Frank Blaschka <frank.blaschka@de.ibm.com>

State transition to DEV_STATE_STOPPED indicates all outstanding I/O has
finished. Add wait queue to wait for this state.

Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---

 drivers/s390/net/ctcm_main.c |    5 +++++
 drivers/s390/net/fsm.c       |    1 +
 drivers/s390/net/fsm.h       |    2 ++
 3 files changed, 8 insertions(+)

diff -urpN linux-2.6/drivers/s390/net/ctcm_main.c linux-2.6-patched/drivers/s390/net/ctcm_main.c
--- linux-2.6/drivers/s390/net/ctcm_main.c	2009-11-12 14:59:32.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/ctcm_main.c	2009-11-12 14:59:32.000000000 +0100
@@ -1720,6 +1720,11 @@ static int ctcm_pm_suspend(struct ccwgro
 		return 0;
 	netif_device_detach(priv->channel[READ]->netdev);
 	ctcm_close(priv->channel[READ]->netdev);
+	if (!wait_event_timeout(priv->fsm->wait_q,
+	    fsm_getstate(priv->fsm) == DEV_STATE_STOPPED, CTCM_TIME_5_SEC)) {
+		netif_device_attach(priv->channel[READ]->netdev);
+		return -EBUSY;
+	}
 	ccw_device_set_offline(gdev->cdev[1]);
 	ccw_device_set_offline(gdev->cdev[0]);
 	return 0;
diff -urpN linux-2.6/drivers/s390/net/fsm.c linux-2.6-patched/drivers/s390/net/fsm.c
--- linux-2.6/drivers/s390/net/fsm.c	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/fsm.c	2009-11-12 14:59:32.000000000 +0100
@@ -27,6 +27,7 @@ init_fsm(char *name, const char **state_
 		return NULL;
 	}
 	strlcpy(this->name, name, sizeof(this->name));
+	init_waitqueue_head(&this->wait_q);
 
 	f = kzalloc(sizeof(fsm), order);
 	if (f == NULL) {
diff -urpN linux-2.6/drivers/s390/net/fsm.h linux-2.6-patched/drivers/s390/net/fsm.h
--- linux-2.6/drivers/s390/net/fsm.h	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/fsm.h	2009-11-12 14:59:32.000000000 +0100
@@ -66,6 +66,7 @@ typedef struct fsm_instance_t {
 	char name[16];
 	void *userdata;
 	int userint;
+	wait_queue_head_t wait_q;
 #if FSM_DEBUG_HISTORY
 	int         history_index;
 	int         history_size;
@@ -197,6 +198,7 @@ fsm_newstate(fsm_instance *fi, int newst
 	printk(KERN_DEBUG "fsm(%s): New state %s\n", fi->name,
 		fi->f->state_names[newstate]);
 #endif
+	wake_up(&fi->wait_q);
 }
 
 /**


^ permalink raw reply

* [patch 3/4] [PATCH] s390: remove cu3088 layer for lcs and ctcm
From: Ursula Braun @ 2009-11-13  7:46 UTC (permalink / raw)
  To: davem, netdev, linux-s390
  Cc: schwidefsky, heiko.carstens, Cornelia Huck, Ursula Braun
In-Reply-To: <20091113074626.078348000@linux.vnet.ibm.com>

[-- Attachment #1: 615-remove-cu3088.diff --]
[-- Type: text/plain, Size: 24426 bytes --]

From: Ursula Braun <ursula.braun@de.ibm.com>

The cu3088-driver used as common base for lcs- and ctcm-devices
makes it difficult to assign the appropriate driver to an lcs-device
or a ctcm-device. This patch eliminates the cu3088-driver and thus
the root device "cu3088". Path /sys/devices/cu3088 is replaced with
the pathes /sys/devices/lcs and /sys/devices/ctcm.

Patch is based on a proposal from Cornelia Huck.

Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---

 drivers/s390/net/Makefile     |    6 -
 drivers/s390/net/claw.c       |   82 ++++++++++++++++++++---
 drivers/s390/net/claw.h       |   12 +++
 drivers/s390/net/ctcm_fsms.c  |    1 
 drivers/s390/net/ctcm_fsms.h  |    1 
 drivers/s390/net/ctcm_main.c  |  104 +++++++++++++++++++++++------
 drivers/s390/net/ctcm_main.h  |   20 +++++
 drivers/s390/net/ctcm_mpc.c   |    1 
 drivers/s390/net/ctcm_sysfs.c |   11 ++-
 drivers/s390/net/cu3088.c     |  148 ------------------------------------------
 drivers/s390/net/cu3088.h     |   41 -----------
 drivers/s390/net/lcs.c        |  101 ++++++++++++++++++++++++----
 drivers/s390/net/lcs.h        |   18 +++++
 13 files changed, 304 insertions(+), 242 deletions(-)

diff -urpN linux-2.6/drivers/s390/net/claw.c linux-2.6-patched/drivers/s390/net/claw.c
--- linux-2.6/drivers/s390/net/claw.c	2009-11-12 14:58:54.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/claw.c	2009-11-12 14:59:32.000000000 +0100
@@ -90,7 +90,6 @@
 #include <linux/timer.h>
 #include <linux/types.h>
 
-#include "cu3088.h"
 #include "claw.h"
 
 /*
@@ -258,6 +257,9 @@ static int claw_pm_prepare(struct ccwgro
 	return -EPERM;
 }
 
+/* the root device for claw group devices */
+static struct device *claw_root_dev;
+
 /* ccwgroup table  */
 
 static struct ccwgroup_driver claw_group_driver = {
@@ -272,6 +274,47 @@ static struct ccwgroup_driver claw_group
 	.prepare     = claw_pm_prepare,
 };
 
+static struct ccw_device_id claw_ids[] = {
+	{CCW_DEVICE(0x3088, 0x61), .driver_info = claw_channel_type_claw},
+	{},
+};
+MODULE_DEVICE_TABLE(ccw, claw_ids);
+
+static struct ccw_driver claw_ccw_driver = {
+	.owner	= THIS_MODULE,
+	.name	= "claw",
+	.ids	= claw_ids,
+	.probe	= ccwgroup_probe_ccwdev,
+	.remove	= ccwgroup_remove_ccwdev,
+};
+
+static ssize_t
+claw_driver_group_store(struct device_driver *ddrv, const char *buf,
+			size_t count)
+{
+	int err;
+	err = ccwgroup_create_from_string(claw_root_dev,
+					  claw_group_driver.driver_id,
+					  &claw_ccw_driver, 3, buf);
+	return err ? err : count;
+}
+
+static DRIVER_ATTR(group, 0200, NULL, claw_driver_group_store);
+
+static struct attribute *claw_group_attrs[] = {
+	&driver_attr_group.attr,
+	NULL,
+};
+
+static struct attribute_group claw_group_attr_group = {
+	.attrs = claw_group_attrs,
+};
+
+static struct attribute_group *claw_group_attr_groups[] = {
+	&claw_group_attr_group,
+	NULL,
+};
+
 /*
 *       Key functions
 */
@@ -3326,7 +3369,11 @@ claw_remove_files(struct device *dev)
 static void __exit
 claw_cleanup(void)
 {
-	unregister_cu3088_discipline(&claw_group_driver);
+	driver_remove_file(&claw_group_driver.driver,
+			   &driver_attr_group);
+	ccwgroup_driver_unregister(&claw_group_driver);
+	ccw_driver_unregister(&claw_ccw_driver);
+	root_device_unregister(claw_root_dev);
 	claw_unregister_debug_facility();
 	pr_info("Driver unloaded\n");
 
@@ -3348,16 +3395,31 @@ claw_init(void)
 	if (ret) {
 		pr_err("Registering with the S/390 debug feature"
 			" failed with error code %d\n", ret);
-		return ret;
+		goto out_err;
 	}
 	CLAW_DBF_TEXT(2, setup, "init_mod");
-	ret = register_cu3088_discipline(&claw_group_driver);
-	if (ret) {
-		CLAW_DBF_TEXT(2, setup, "init_bad");
-		claw_unregister_debug_facility();
-		pr_err("Registering with the cu3088 device driver failed "
-			   "with error code %d\n", ret);
-	}
+	claw_root_dev = root_device_register("qeth");
+	ret = IS_ERR(claw_root_dev) ? PTR_ERR(claw_root_dev) : 0;
+	if (ret)
+		goto register_err;
+	ret = ccw_driver_register(&claw_ccw_driver);
+	if (ret)
+		goto ccw_err;
+	claw_group_driver.driver.groups = claw_group_attr_groups;
+	ret = ccwgroup_driver_register(&claw_group_driver);
+	if (ret)
+		goto ccwgroup_err;
+	return 0;
+
+ccwgroup_err:
+	ccw_driver_unregister(&claw_ccw_driver);
+ccw_err:
+	root_device_unregister(claw_root_dev);
+register_err:
+	CLAW_DBF_TEXT(2, setup, "init_bad");
+	claw_unregister_debug_facility();
+out_err:
+	pr_err("Initializing the claw device driver failed\n");
 	return ret;
 }
 
diff -urpN linux-2.6/drivers/s390/net/claw.h linux-2.6-patched/drivers/s390/net/claw.h
--- linux-2.6/drivers/s390/net/claw.h	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/claw.h	2009-11-12 14:59:32.000000000 +0100
@@ -129,6 +129,18 @@ static inline int claw_dbf_passes(debug_
 		} \
 	} while (0)
 
+/**
+ * Enum for classifying detected devices.
+ */
+enum claw_channel_types {
+	/* Device is not a channel  */
+	claw_channel_type_none,
+
+	/* Device is a CLAW channel device */
+	claw_channel_type_claw
+};
+
+
 /*******************************************************
 *  Define Control Blocks                               *
 *                                                      *
diff -urpN linux-2.6/drivers/s390/net/ctcm_fsms.c linux-2.6-patched/drivers/s390/net/ctcm_fsms.c
--- linux-2.6/drivers/s390/net/ctcm_fsms.c	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_fsms.c	2009-11-12 14:59:32.000000000 +0100
@@ -44,7 +44,6 @@
 #include <asm/idals.h>
 
 #include "fsm.h"
-#include "cu3088.h"
 
 #include "ctcm_dbug.h"
 #include "ctcm_main.h"
diff -urpN linux-2.6/drivers/s390/net/ctcm_fsms.h linux-2.6-patched/drivers/s390/net/ctcm_fsms.h
--- linux-2.6/drivers/s390/net/ctcm_fsms.h	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_fsms.h	2009-11-12 14:59:32.000000000 +0100
@@ -39,7 +39,6 @@
 #include <asm/idals.h>
 
 #include "fsm.h"
-#include "cu3088.h"
 #include "ctcm_main.h"
 
 /*
diff -urpN linux-2.6/drivers/s390/net/ctcm_main.c linux-2.6-patched/drivers/s390/net/ctcm_main.c
--- linux-2.6/drivers/s390/net/ctcm_main.c	2009-11-12 14:59:32.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/ctcm_main.c	2009-11-12 14:59:32.000000000 +0100
@@ -51,12 +51,16 @@
 
 #include <asm/idals.h>
 
-#include "cu3088.h"
 #include "ctcm_fsms.h"
 #include "ctcm_main.h"
 
 /* Some common global variables */
 
+/**
+ * The root device for ctcm group devices
+ */
+static struct device *ctcm_root_dev;
+
 /*
  * Linked list of all detected channels.
  */
@@ -246,7 +250,7 @@ static void channel_remove(struct channe
  *
  * returns Pointer to a channel or NULL if no matching channel available.
  */
-static struct channel *channel_get(enum channel_types type,
+static struct channel *channel_get(enum ctcm_channel_types type,
 					char *id, int direction)
 {
 	struct channel *ch = channels;
@@ -1342,7 +1346,7 @@ static int ctcm_probe_device(struct ccwg
  *
  * returns 0 on success, !0 on error.
  */
-static int add_channel(struct ccw_device *cdev, enum channel_types type,
+static int add_channel(struct ccw_device *cdev, enum ctcm_channel_types type,
 				struct ctcm_priv *priv)
 {
 	struct channel **c = &channels;
@@ -1501,13 +1505,13 @@ free_return:	/* note that all channel po
 /*
  * Return type of a detected device.
  */
-static enum channel_types get_channel_type(struct ccw_device_id *id)
+static enum ctcm_channel_types get_channel_type(struct ccw_device_id *id)
 {
-	enum channel_types type;
-	type = (enum channel_types)id->driver_info;
+	enum ctcm_channel_types type;
+	type = (enum ctcm_channel_types)id->driver_info;
 
-	if (type == channel_type_ficon)
-		type = channel_type_escon;
+	if (type == ctcm_channel_type_ficon)
+		type = ctcm_channel_type_escon;
 
 	return type;
 }
@@ -1525,7 +1529,7 @@ static int ctcm_new_device(struct ccwgro
 	char read_id[CTCM_ID_SIZE];
 	char write_id[CTCM_ID_SIZE];
 	int direction;
-	enum channel_types type;
+	enum ctcm_channel_types type;
 	struct ctcm_priv *priv;
 	struct net_device *dev;
 	struct ccw_device *cdev0;
@@ -1749,6 +1753,22 @@ err_out:
 	return rc;
 }
 
+static struct ccw_device_id ctcm_ids[] = {
+	{CCW_DEVICE(0x3088, 0x08), .driver_info = ctcm_channel_type_parallel},
+	{CCW_DEVICE(0x3088, 0x1e), .driver_info = ctcm_channel_type_ficon},
+	{CCW_DEVICE(0x3088, 0x1f), .driver_info = ctcm_channel_type_escon},
+	{},
+};
+MODULE_DEVICE_TABLE(ccw, ctcm_ids);
+
+static struct ccw_driver ctcm_ccw_driver = {
+	.owner	= THIS_MODULE,
+	.name	= "ctcm",
+	.ids	= ctcm_ids,
+	.probe	= ccwgroup_probe_ccwdev,
+	.remove	= ccwgroup_remove_ccwdev,
+};
+
 static struct ccwgroup_driver ctcm_group_driver = {
 	.owner       = THIS_MODULE,
 	.name        = CTC_DRIVER_NAME,
@@ -1763,6 +1783,33 @@ static struct ccwgroup_driver ctcm_group
 	.restore     = ctcm_pm_resume,
 };
 
+static ssize_t
+ctcm_driver_group_store(struct device_driver *ddrv, const char *buf,
+			size_t count)
+{
+	int err;
+
+	err = ccwgroup_create_from_string(ctcm_root_dev,
+					  ctcm_group_driver.driver_id,
+					  &ctcm_ccw_driver, 2, buf);
+	return err ? err : count;
+}
+
+static DRIVER_ATTR(group, 0200, NULL, ctcm_driver_group_store);
+
+static struct attribute *ctcm_group_attrs[] = {
+	&driver_attr_group.attr,
+	NULL,
+};
+
+static struct attribute_group ctcm_group_attr_group = {
+	.attrs = ctcm_group_attrs,
+};
+
+static struct attribute_group *ctcm_group_attr_groups[] = {
+	&ctcm_group_attr_group,
+	NULL,
+};
 
 /*
  * Module related routines
@@ -1776,7 +1823,10 @@ static struct ccwgroup_driver ctcm_group
  */
 static void __exit ctcm_exit(void)
 {
-	unregister_cu3088_discipline(&ctcm_group_driver);
+	driver_remove_file(&ctcm_group_driver.driver, &driver_attr_group);
+	ccwgroup_driver_unregister(&ctcm_group_driver);
+	ccw_driver_unregister(&ctcm_ccw_driver);
+	root_device_unregister(ctcm_root_dev);
 	ctcm_unregister_dbf_views();
 	pr_info("CTCM driver unloaded\n");
 }
@@ -1802,17 +1852,31 @@ static int __init ctcm_init(void)
 	channels = NULL;
 
 	ret = ctcm_register_dbf_views();
-	if (ret) {
-		return ret;
-	}
-	ret = register_cu3088_discipline(&ctcm_group_driver);
-	if (ret) {
-		ctcm_unregister_dbf_views();
-		pr_err("%s / register_cu3088_discipline failed, ret = %d\n",
-			__func__, ret);
-		return ret;
-	}
+	if (ret)
+		goto out_err;
+	ctcm_root_dev = root_device_register("ctcm");
+	ret = IS_ERR(ctcm_root_dev) ? PTR_ERR(ctcm_root_dev) : 0;
+	if (ret)
+		goto register_err;
+	ret = ccw_driver_register(&ctcm_ccw_driver);
+	if (ret)
+		goto ccw_err;
+	ctcm_group_driver.driver.groups = ctcm_group_attr_groups;
+	ret = ccwgroup_driver_register(&ctcm_group_driver);
+	if (ret)
+		goto ccwgroup_err;
 	print_banner();
+	return 0;
+
+ccwgroup_err:
+	ccw_driver_unregister(&ctcm_ccw_driver);
+ccw_err:
+	root_device_unregister(ctcm_root_dev);
+register_err:
+	ctcm_unregister_dbf_views();
+out_err:
+	pr_err("%s / Initializing the ctcm device driver failed, ret = %d\n",
+		__func__, ret);
 	return ret;
 }
 
diff -urpN linux-2.6/drivers/s390/net/ctcm_main.h linux-2.6-patched/drivers/s390/net/ctcm_main.h
--- linux-2.6/drivers/s390/net/ctcm_main.h	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_main.h	2009-11-12 14:59:32.000000000 +0100
@@ -16,7 +16,6 @@
 #include <linux/netdevice.h>
 
 #include "fsm.h"
-#include "cu3088.h"
 #include "ctcm_dbug.h"
 #include "ctcm_mpc.h"
 
@@ -66,6 +65,23 @@
 			ctcmpc_dumpit(buf, len); \
 	} while (0)
 
+/**
+ * Enum for classifying detected devices
+ */
+enum ctcm_channel_types {
+	/* Device is not a channel  */
+	ctcm_channel_type_none,
+
+	/* Device is a CTC/A */
+	ctcm_channel_type_parallel,
+
+	/* Device is a FICON channel */
+	ctcm_channel_type_ficon,
+
+	/* Device is a ESCON channel */
+	ctcm_channel_type_escon
+};
+
 /*
  * CCW commands, used in this driver.
  */
@@ -121,7 +137,7 @@ struct channel {
 	 * Type of this channel.
 	 * CTC/A or Escon for valid channels.
 	 */
-	enum channel_types type;
+	enum ctcm_channel_types type;
 	/*
 	 * Misc. flags. See CHANNEL_FLAGS_... below
 	 */
diff -urpN linux-2.6/drivers/s390/net/ctcm_mpc.c linux-2.6-patched/drivers/s390/net/ctcm_mpc.c
--- linux-2.6/drivers/s390/net/ctcm_mpc.c	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_mpc.c	2009-11-12 14:59:32.000000000 +0100
@@ -53,7 +53,6 @@
 #include <linux/moduleparam.h>
 #include <asm/idals.h>
 
-#include "cu3088.h"
 #include "ctcm_mpc.h"
 #include "ctcm_main.h"
 #include "ctcm_fsms.h"
diff -urpN linux-2.6/drivers/s390/net/ctcm_sysfs.c linux-2.6-patched/drivers/s390/net/ctcm_sysfs.c
--- linux-2.6/drivers/s390/net/ctcm_sysfs.c	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_sysfs.c	2009-11-12 14:59:32.000000000 +0100
@@ -158,6 +158,15 @@ static ssize_t ctcm_proto_store(struct d
 	return count;
 }
 
+const char *ctcm_type[] = {
+	"not a channel",
+	"CTC/A",
+	"FICON channel",
+	"ESCON channel",
+	"unknown channel type",
+	"unsupported channel type",
+};
+
 static ssize_t ctcm_type_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
@@ -168,7 +177,7 @@ static ssize_t ctcm_type_show(struct dev
 		return -ENODEV;
 
 	return sprintf(buf, "%s\n",
-			cu3088_type[cgdev->cdev[0]->id.driver_info]);
+			ctcm_type[cgdev->cdev[0]->id.driver_info]);
 }
 
 static DEVICE_ATTR(buffer, 0644, ctcm_buffer_show, ctcm_buffer_write);
diff -urpN linux-2.6/drivers/s390/net/cu3088.c linux-2.6-patched/drivers/s390/net/cu3088.c
--- linux-2.6/drivers/s390/net/cu3088.c	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/cu3088.c	1970-01-01 01:00:00.000000000 +0100
@@ -1,148 +0,0 @@
-/*
- * CTC / LCS ccw_device driver
- *
- * Copyright (C) 2002 IBM Deutschland Entwicklung GmbH, IBM Corporation
- * Author(s): Arnd Bergmann <arndb@de.ibm.com>
- *            Cornelia Huck <cornelia.huck@de.ibm.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2, or (at your option)
- * any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- *
- */
-
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/err.h>
-
-#include <asm/ccwdev.h>
-#include <asm/ccwgroup.h>
-
-#include "cu3088.h"
-
-const char *cu3088_type[] = {
-	"not a channel",
-	"CTC/A",
-	"ESCON channel",
-	"FICON channel",
-	"OSA LCS card",
-	"CLAW channel device",
-	"unknown channel type",
-	"unsupported channel type",
-};
-
-/* static definitions */
-
-static struct ccw_device_id cu3088_ids[] = {
-	{ CCW_DEVICE(0x3088, 0x08), .driver_info = channel_type_parallel },
-	{ CCW_DEVICE(0x3088, 0x1f), .driver_info = channel_type_escon },
-	{ CCW_DEVICE(0x3088, 0x1e), .driver_info = channel_type_ficon },
-	{ CCW_DEVICE(0x3088, 0x60), .driver_info = channel_type_osa2 },
-	{ CCW_DEVICE(0x3088, 0x61), .driver_info = channel_type_claw },
-	{ /* end of list */ }
-};
-
-static struct ccw_driver cu3088_driver;
-
-static struct device *cu3088_root_dev;
-
-static ssize_t
-group_write(struct device_driver *drv, const char *buf, size_t count)
-{
-	int ret;
-	struct ccwgroup_driver *cdrv;
-
-	cdrv = to_ccwgroupdrv(drv);
-	if (!cdrv)
-		return -EINVAL;
-	ret = ccwgroup_create_from_string(cu3088_root_dev, cdrv->driver_id,
-					  &cu3088_driver, 2, buf);
-
-	return (ret == 0) ? count : ret;
-}
-
-static DRIVER_ATTR(group, 0200, NULL, group_write);
-
-/* Register-unregister for ctc&lcs */
-int
-register_cu3088_discipline(struct ccwgroup_driver *dcp)
-{
-	int rc;
-
-	if (!dcp)
-		return -EINVAL;
-
-	/* Register discipline.*/
-	rc = ccwgroup_driver_register(dcp);
-	if (rc)
-		return rc;
-
-	rc = driver_create_file(&dcp->driver, &driver_attr_group);
-	if (rc)
-		ccwgroup_driver_unregister(dcp);
-
-	return rc;
-
-}
-
-void
-unregister_cu3088_discipline(struct ccwgroup_driver *dcp)
-{
-	if (!dcp)
-		return;
-
-	driver_remove_file(&dcp->driver, &driver_attr_group);
-	ccwgroup_driver_unregister(dcp);
-}
-
-static struct ccw_driver cu3088_driver = {
-	.owner	     = THIS_MODULE,
-	.ids	     = cu3088_ids,
-	.name        = "cu3088",
-	.probe	     = ccwgroup_probe_ccwdev,
-	.remove	     = ccwgroup_remove_ccwdev,
-};
-
-/* module setup */
-static int __init
-cu3088_init (void)
-{
-	int rc;
-
-	cu3088_root_dev = root_device_register("cu3088");
-	if (IS_ERR(cu3088_root_dev))
-		return PTR_ERR(cu3088_root_dev);
-	rc = ccw_driver_register(&cu3088_driver);
-	if (rc)
-		root_device_unregister(cu3088_root_dev);
-
-	return rc;
-}
-
-static void __exit
-cu3088_exit (void)
-{
-	ccw_driver_unregister(&cu3088_driver);
-	root_device_unregister(cu3088_root_dev);
-}
-
-MODULE_DEVICE_TABLE(ccw,cu3088_ids);
-MODULE_AUTHOR("Arnd Bergmann <arndb@de.ibm.com>");
-MODULE_LICENSE("GPL");
-
-module_init(cu3088_init);
-module_exit(cu3088_exit);
-
-EXPORT_SYMBOL_GPL(cu3088_type);
-EXPORT_SYMBOL_GPL(register_cu3088_discipline);
-EXPORT_SYMBOL_GPL(unregister_cu3088_discipline);
diff -urpN linux-2.6/drivers/s390/net/cu3088.h linux-2.6-patched/drivers/s390/net/cu3088.h
--- linux-2.6/drivers/s390/net/cu3088.h	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/cu3088.h	1970-01-01 01:00:00.000000000 +0100
@@ -1,41 +0,0 @@
-#ifndef _CU3088_H
-#define _CU3088_H
-
-/**
- * Enum for classifying detected devices.
- */
-enum channel_types {
-        /* Device is not a channel  */
-	channel_type_none,
-
-        /* Device is a CTC/A */
-	channel_type_parallel,
-
-	/* Device is a ESCON channel */
-	channel_type_escon,
-
-	/* Device is a FICON channel */
-	channel_type_ficon,
-
-	/* Device is a OSA2 card */
-	channel_type_osa2,
-
-	/* Device is a CLAW channel device */
-	channel_type_claw,
-
-	/* Device is a channel, but we don't know
-	 * anything about it */
-	channel_type_unknown,
-
-	/* Device is an unsupported model */
-	channel_type_unsupported,
-
-	/* number of type entries */
-	num_channel_types
-};
-
-extern const char *cu3088_type[num_channel_types];
-extern int register_cu3088_discipline(struct ccwgroup_driver *);
-extern void unregister_cu3088_discipline(struct ccwgroup_driver *);
-
-#endif
diff -urpN linux-2.6/drivers/s390/net/lcs.c linux-2.6-patched/drivers/s390/net/lcs.c
--- linux-2.6/drivers/s390/net/lcs.c	2009-11-12 14:59:31.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/lcs.c	2009-11-12 14:59:32.000000000 +0100
@@ -47,7 +47,6 @@
 #include <asm/ccwgroup.h>
 
 #include "lcs.h"
-#include "cu3088.h"
 
 
 #if !defined(CONFIG_NET_ETHERNET) && \
@@ -60,7 +59,11 @@
  */
 
 static char version[] __initdata = "LCS driver";
-static char debug_buffer[255];
+
+/**
+  * the root device for lcs group devices
+  */
+static struct device *lcs_root_dev;
 
 /**
  * Some prototypes.
@@ -76,6 +79,7 @@ static int lcs_recovery(void *ptr);
 /**
  * Debug Facility Stuff
  */
+static char debug_buffer[255];
 static debug_info_t *lcs_dbf_setup;
 static debug_info_t *lcs_dbf_trace;
 
@@ -1968,6 +1972,15 @@ lcs_portno_store (struct device *dev, st
 
 static DEVICE_ATTR(portno, 0644, lcs_portno_show, lcs_portno_store);
 
+const char *lcs_type[] = {
+	"not a channel",
+	"2216 parallel",
+	"2216 channel",
+	"OSA LCS card",
+	"unknown channel type",
+	"unsupported channel type",
+};
+
 static ssize_t
 lcs_type_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
@@ -1977,7 +1990,7 @@ lcs_type_show(struct device *dev, struct
 	if (!cgdev)
 		return -ENODEV;
 
-	return sprintf(buf, "%s\n", cu3088_type[cgdev->cdev[0]->id.driver_info]);
+	return sprintf(buf, "%s\n", lcs_type[cgdev->cdev[0]->id.driver_info]);
 }
 
 static DEVICE_ATTR(type, 0444, lcs_type_show, NULL);
@@ -2370,6 +2383,22 @@ static int lcs_restore(struct ccwgroup_d
 	return lcs_pm_resume(card);
 }
 
+static struct ccw_device_id lcs_ids[] = {
+	{CCW_DEVICE(0x3088, 0x08), .driver_info = lcs_channel_type_parallel},
+	{CCW_DEVICE(0x3088, 0x1f), .driver_info = lcs_channel_type_2216},
+	{CCW_DEVICE(0x3088, 0x60), .driver_info = lcs_channel_type_osa2},
+	{},
+};
+MODULE_DEVICE_TABLE(ccw, lcs_ids);
+
+static struct ccw_driver lcs_ccw_driver = {
+	.owner	= THIS_MODULE,
+	.name	= "lcs",
+	.ids	= lcs_ids,
+	.probe	= ccwgroup_probe_ccwdev,
+	.remove	= ccwgroup_remove_ccwdev,
+};
+
 /**
  * LCS ccwgroup driver registration
  */
@@ -2389,6 +2418,33 @@ static struct ccwgroup_driver lcs_group_
 	.restore     = lcs_restore,
 };
 
+static ssize_t
+lcs_driver_group_store(struct device_driver *ddrv, const char *buf,
+		       size_t count)
+{
+	int err;
+	err = ccwgroup_create_from_string(lcs_root_dev,
+					  lcs_group_driver.driver_id,
+					  &lcs_ccw_driver, 2, buf);
+	return err ? err : count;
+}
+
+static DRIVER_ATTR(group, 0200, NULL, lcs_driver_group_store);
+
+static struct attribute *lcs_group_attrs[] = {
+	&driver_attr_group.attr,
+	NULL,
+};
+
+static struct attribute_group lcs_group_attr_group = {
+	.attrs = lcs_group_attrs,
+};
+
+static struct attribute_group *lcs_group_attr_groups[] = {
+	&lcs_group_attr_group,
+	NULL,
+};
+
 /**
  *  LCS Module/Kernel initialization function
  */
@@ -2400,17 +2456,30 @@ __init lcs_init_module(void)
 	pr_info("Loading %s\n", version);
 	rc = lcs_register_debug_facility();
 	LCS_DBF_TEXT(0, setup, "lcsinit");
-	if (rc) {
-		pr_err("Initialization failed\n");
-		return rc;
-	}
-
-	rc = register_cu3088_discipline(&lcs_group_driver);
-	if (rc) {
-		pr_err("Initialization failed\n");
-		return rc;
-	}
+	if (rc)
+		goto out_err;
+	lcs_root_dev = root_device_register("lcs");
+	rc = IS_ERR(lcs_root_dev) ? PTR_ERR(lcs_root_dev) : 0;
+	if (rc)
+		goto register_err;
+	rc = ccw_driver_register(&lcs_ccw_driver);
+	if (rc)
+		goto ccw_err;
+	lcs_group_driver.driver.groups = lcs_group_attr_groups;
+	rc = ccwgroup_driver_register(&lcs_group_driver);
+	if (rc)
+		goto ccwgroup_err;
 	return 0;
+
+ccwgroup_err:
+	ccw_driver_unregister(&lcs_ccw_driver);
+ccw_err:
+	root_device_unregister(lcs_root_dev);
+register_err:
+	lcs_unregister_debug_facility();
+out_err:
+	pr_err("Initializing the lcs device driver failed\n");
+	return rc;
 }
 
 
@@ -2422,7 +2491,11 @@ __exit lcs_cleanup_module(void)
 {
 	pr_info("Terminating lcs module.\n");
 	LCS_DBF_TEXT(0, trace, "cleanup");
-	unregister_cu3088_discipline(&lcs_group_driver);
+	driver_remove_file(&lcs_group_driver.driver,
+			   &driver_attr_group);
+	ccwgroup_driver_unregister(&lcs_group_driver);
+	ccw_driver_unregister(&lcs_ccw_driver);
+	root_device_unregister(lcs_root_dev);
 	lcs_unregister_debug_facility();
 }
 
diff -urpN linux-2.6/drivers/s390/net/lcs.h linux-2.6-patched/drivers/s390/net/lcs.h
--- linux-2.6/drivers/s390/net/lcs.h	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/lcs.h	2009-11-12 14:59:32.000000000 +0100
@@ -36,6 +36,24 @@ static inline int lcs_dbf_passes(debug_i
 #define CARD_FROM_DEV(cdev) \
 	(struct lcs_card *) dev_get_drvdata( \
 		&((struct ccwgroup_device *)dev_get_drvdata(&cdev->dev))->dev);
+
+/**
+ * Enum for classifying detected devices.
+ */
+enum lcs_channel_types {
+	/* Device is not a channel  */
+	lcs_channel_type_none,
+
+	/* Device is a 2216 channel */
+	lcs_channel_type_parallel,
+
+	/* Device is a 2216 channel */
+	lcs_channel_type_2216,
+
+	/* Device is a OSA2 card */
+	lcs_channel_type_osa2
+};
+
 /**
  * CCW commands used in this driver
  */
diff -urpN linux-2.6/drivers/s390/net/Makefile linux-2.6-patched/drivers/s390/net/Makefile
--- linux-2.6/drivers/s390/net/Makefile	2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/Makefile	2009-11-12 14:59:32.000000000 +0100
@@ -3,11 +3,11 @@
 #
 
 ctcm-y += ctcm_main.o ctcm_fsms.o ctcm_mpc.o ctcm_sysfs.o ctcm_dbug.o
-obj-$(CONFIG_CTCM) += ctcm.o fsm.o cu3088.o
+obj-$(CONFIG_CTCM) += ctcm.o fsm.o
 obj-$(CONFIG_NETIUCV) += netiucv.o fsm.o
 obj-$(CONFIG_SMSGIUCV) += smsgiucv.o
-obj-$(CONFIG_LCS) += lcs.o cu3088.o
-obj-$(CONFIG_CLAW) += claw.o cu3088.o
+obj-$(CONFIG_LCS) += lcs.o
+obj-$(CONFIG_CLAW) += claw.o
 qeth-y += qeth_core_sys.o qeth_core_main.o qeth_core_mpc.o
 obj-$(CONFIG_QETH) += qeth.o
 qeth_l2-y += qeth_l2_main.o


^ permalink raw reply

* Re: [PATCH] netfilter: Should xt_osf_remove_callback() return negative on errors?
From: Patrick McHardy @ 2009-11-13  8:32 UTC (permalink / raw)
  To: Roel Kluin; +Cc: Andrew Morton, LKML, David S. Miller, netfilter-devel, netdev
In-Reply-To: <4AFB3B9D.1080606@gmail.com>

Roel Kluin wrote:
> Return a negative error value

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] netfilter: nf_log: fix sleeping function called from invalid context in seq_show()
From: Patrick McHardy @ 2009-11-13  8:35 UTC (permalink / raw)
  To: Wu Fengguang; +Cc: LKML, Yin, Kangkai, netdev
In-Reply-To: <20091113063640.GA29012@localhost>

Wu Fengguang wrote:
> [  171.925285] BUG: sleeping function called from invalid context at kernel/mutex.c:280
> [  171.925296] in_atomic(): 1, irqs_disabled(): 0, pid: 671, name: grep
> [  171.925306] 2 locks held by grep/671:
> [  171.925312]  #0:  (&p->lock){+.+.+.}, at: [<c10b8acd>] seq_read+0x25/0x36c
> [  171.925340]  #1:  (rcu_read_lock){.+.+..}, at: [<c1391dac>] seq_start+0x0/0x44
> [  171.925372] Pid: 671, comm: grep Not tainted 2.6.31.6-4-netbook #3
> [  171.925380] Call Trace:
> [  171.925398]  [<c105104e>] ? __debug_show_held_locks+0x1e/0x20
> [  171.925414]  [<c10264ac>] __might_sleep+0xfb/0x102
> [  171.925430]  [<c1461521>] mutex_lock_nested+0x1c/0x2ad
> [  171.925444]  [<c1391c9e>] seq_show+0x74/0x127
> [  171.925456]  [<c10b8c5c>] seq_read+0x1b4/0x36c
> [  171.925469]  [<c10b8aa8>] ? seq_read+0x0/0x36c
> [  171.925483]  [<c10d5c8e>] proc_reg_read+0x60/0x74
> [  171.925496]  [<c10d5c2e>] ? proc_reg_read+0x0/0x74
> [  171.925510]  [<c10a4468>] vfs_read+0x87/0x110
> [  171.925523]  [<c10a458a>] sys_read+0x3b/0x60
> [  171.925538]  [<c1002a49>] syscall_call+0x7/0xb
> 
> Fix it by replacing RCU with nf_log_mutex.

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-13  8:54 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: Eric Dumazet, David S. Miller, Stephen Hemminger, Patrick McHardy,
	Tom Herbert, netdev
In-Reply-To: <20091113074508.GA6605@ff.dom.local>

On Fri, Nov 13, 2009 at 3:45 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> On 13-11-2009 07:16, Changli Gao wrote:
>
> I don't think so. There would be a lot of code duplication and later
> maintenance problems only because of the scheduling method. The main
> question is to establish if there is really no performance difference
> (which I doubt) - unless Changli can show some tests for various
> setups now. On the other hand, if there is a difference, why keep
> ineffective solution - similar thing should be possible to do in the
> softirq context as well.
>
> So it should not be a big problem to do it a bit messy for some
> testing time. Since we can use separate ->ndo_start_xmit() etc. it
> shouldn't be too messy, I guess.
>

I have done a simple test. I run a simple program on computer A, which
sends SYN packets with random source ports to Computer B's 80 port (No
socket listens on that port, so tcp reset packets will be sent) in
90kpps. On computer B, I redirect the traffic to IFB. At the same
time, I ping from B to A to get the RTT between them. I can't see any
difference between the original IFB and my MQ version. They are both:

CPU idle: 50%
Latency: 0.3-0.4ms, burst 2ms.


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Documentation: rw_lock lessons learned
From: Stefan Richter @ 2009-11-13  8:59 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: sclark46, William Allen Simpson, Paul E. McKenney, Linus Torvalds,
	Linux Kernel Developers, Linux Kernel Network Developers,
	Eric Dumazet
In-Reply-To: <20091112150029.1a26c641@nehalam>

Stephen Hemminger wrote:
> On Thu, 12 Nov 2009 14:13:03 -0500
> Stephen Clark <sclark46@earthlink.net> wrote:
>> How up to date is this doc?
>> 
>> http://www.kernel.org/pub/linux/kernel/people/rusty/kernel-locking/index.html
>> 
> 
> Out of date.
> 1. Missing mutex's which have largely replaced semaphores.
> 
> 2. Missing change to lock initialization in later kernels.
> 
> 3. Missing description of lock dependency checker which should be in same guide.

4. The section on atomic reference counting should refer to <linux/kref.h>.
-- 
Stefan Richter
-=====-==--= =-== -==-=
http://arcgraph.de/sr/

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Jarek Poplawski @ 2009-11-13  9:18 UTC (permalink / raw)
  To: Changli Gao
  Cc: Eric Dumazet, David S. Miller, Stephen Hemminger, Patrick McHardy,
	Tom Herbert, netdev
In-Reply-To: <412e6f7f0911130054i7a508a6ah16368f11bdc7353d@mail.gmail.com>

On Fri, Nov 13, 2009 at 04:54:50PM +0800, Changli Gao wrote:
> On Fri, Nov 13, 2009 at 3:45 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> > On 13-11-2009 07:16, Changli Gao wrote:
> >
> > I don't think so. There would be a lot of code duplication and later
> > maintenance problems only because of the scheduling method. The main
> > question is to establish if there is really no performance difference
> > (which I doubt) - unless Changli can show some tests for various
> > setups now. On the other hand, if there is a difference, why keep
> > ineffective solution - similar thing should be possible to do in the
> > softirq context as well.
> >
> > So it should not be a big problem to do it a bit messy for some
> > testing time. Since we can use separate ->ndo_start_xmit() etc. it
> > shouldn't be too messy, I guess.
> >
> 
> I have done a simple test. I run a simple program on computer A, which
> sends SYN packets with random source ports to Computer B's 80 port (No
> socket listens on that port, so tcp reset packets will be sent) in
> 90kpps. On computer B, I redirect the traffic to IFB. At the same
> time, I ping from B to A to get the RTT between them. I can't see any
> difference between the original IFB and my MQ version. They are both:
> 
> CPU idle: 50%
> Latency: 0.3-0.4ms, burst 2ms.
> 

I'm mostly concerned with routers doing forwarding with 1Gb or 10Gb
NICs (including multiqueue). Alas/happily I don't have such a problem,
but can't help you with testing either.

Regards,
Jarek P.

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-13  9:38 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: Eric Dumazet, David S. Miller, Stephen Hemminger, Patrick McHardy,
	Tom Herbert, netdev
In-Reply-To: <20091113091825.GA7449@ff.dom.local>

On Fri, Nov 13, 2009 at 5:18 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> On Fri, Nov 13, 2009 at 04:54:50PM +0800, Changli Gao wrote:
>> On Fri, Nov 13, 2009 at 3:45 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
>>
>> I have done a simple test. I run a simple program on computer A, which
>> sends SYN packets with random source ports to Computer B's 80 port (No
>> socket listens on that port, so tcp reset packets will be sent) in
>> 90kpps. On computer B, I redirect the traffic to IFB. At the same
>> time, I ping from B to A to get the RTT between them. I can't see any
>> difference between the original IFB and my MQ version. They are both:
>>
>> CPU idle: 50%
>> Latency: 0.3-0.4ms, burst 2ms.
>>
>
> I'm mostly concerned with routers doing forwarding with 1Gb or 10Gb
> NICs (including multiqueue). Alas/happily I don't have such a problem,
> but can't help you with testing either.
>

Oh, :) . I know more than one companies use kernel threads to forward
packets, and there isn't explicit extra overhead at all. And as you
know, as throughput increases, NAPI will bind the NIC to a CPU, and
softirqd will be waked up to do the work, which should be done in
SoftIRQ context. At that time, there isn't any difference between my
approach and the current kernel's.


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ 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