Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] pktgen node allocation
From: Robert Olsson @ 2010-03-22  6:24 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: robert, David Miller, netdev, olofh
In-Reply-To: <1269006465.3048.39.camel@edumazet-laptop>


Eric Dumazet writes:

 > Well, you said "Tested this with 10 Intel 82599 ports w. TYAN S7025
 > E5520 CPU's. Was able to TX/DMA ~80 Gbit/s to Ethernet wires."
 > 
 > I am interested to know what particular setup you did to maximize
 > throughput then, or are you saing you managed to reduce it ? :)


Some notes from the experiment, It's getting
complex and hairy. Anyway results from the first
tests to give you an idea... My colleague Olof 
might have some comments/details

pktgen sending on 10 * 10g interfaces. 

[From pktgen script]
fn()
{
  i=$1  #ifname
  c=$2  #queue / cpu core
  n=$3  # numa node
  PGDEV=/proc/net/pktgen/kpktgend_$c
  pgset "add_device eth$i@$c  "
  PGDEV=/proc/net/pktgen/eth$i@$c
  pgset "node $n"
  pgset "$COUNT"
  pgset "flag NODE_ALLOC"
  pgset "$CLONE_SKB"
  pgset "$PKT_SIZE"
  pgset "$DELAY"
  pgset "dst 10.0.0.0" 
}      

remove_all
# Setup

# TYAN S7025 with two nodes.
# Each node has own bus with it's own TYLERSBURG bridge
# so eth0-eth3 is closest to node0 which in turn "owns"
# CPU-cores 0-3 in this HW setup. So we setup so 
# pktgen according to this. clone_skb=1000000.
# Used slots are PCIe-x16 except when PCIe-x8 is indicated.

# eth0 queue=0(CPU) node=0
fn 0 0 0
fn 1 1 0
fn 2 2 0
fn 3 3 0
fn 4 4 1
fn 5 5 1
fn 6 6 1
fn 7 7 1
fn 8 12 1
fn 9 13 1

Result "manually" tuned. 

eth0 9617.7 M bit/s      822 k pps 
eth1 9619.1 M bit/s      823 k pps 
eth2 9619.1 M bit/s      823 k pps 
eth3 9619.2 M bit/s      823 k pps 
eth4 5995.2 M bit/s      512 k pps  <-  PCIe-x8
eth5 5995.3 M bit/s      512 k pps  <-  PCIe-x8
eth6 9619.2 M bit/s      823 k pps 
eth7 9619.2 M bit/s      823 k pps 
eth8 9619.1 M bit/s      823 k pps 
eth9 9619.0 M bit/s      823 k pps 

> 90 Gbit/s

Result "manually" mistuned by switching node 0 and 1. 

eth0 9613.6 M bit/s      822 k pps 
eth1 9614.9 M bit/s      822 k pps 
eth2 9615.0 M bit/s      822 k pps 
eth3 9615.1 M bit/s      822 k pps 
eth4 2918.5 M bit/s      249 k pps  <-  PCIe-x8
eth5 2918.4 M bit/s      249 k pps  <-  PCIe-x8
eth6 8597.0 M bit/s      735 k pps 
eth7 8597.0 M bit/s      735 k pps 
eth8 8568.3 M bit/s      733 k pps 
eth9 8568.3 M bit/s      733 k pps 

A lot things is to be investgated...

Cheers
					--ro

^ permalink raw reply

* RCU problems in fib_table_insert
From: Robert Olsson @ 2010-03-22  6:18 UTC (permalink / raw)
  To: Andi Kleen; +Cc: robert.olsson, netdev, paulmck
In-Reply-To: <20100321202525.GA966@basil.fritz.box>



Seems like Paul and Eric fixed this problem... We use fib_trie with 
major infrastructure but always disable preempt. It was unsafe w.
preempt at least before Jareks P. patches about a year ago. I havn't
tested w. preempt after that but maybe someone else have...

Cheers
					--ro

Andi Kleen writes:
 > Hi,
 > 
 > I got the following warning at boot with a 2.6.34-rc2ish git kernel
 > with RCU debugging and preemption enabled.
 > 
 > It seems the problem is that not all callers of fib_find_node
 > call it with rcu_read_lock() to stabilize access to the fib. 
 > 
 > I tried to fix it, but especially for fib_table_insert() that's rather 
 > tricky: it does a lot of memory allocations and also route flushing and 
 > other blocking operations while assuming the original fa is RCU stable.
 > 
 > I first tried to move some allocations to the beginning and keep
 > preemption disabled in the rest, but it's difficult with all of them.
 > No patch because of that.
 > 
 > Does the fa need an additional reference count for this problem?
 > Or perhaps some optimistic locking?
 > 
 > -Andi
 > 
 > 
 > ==================================================
 > [ INFO: suspicious rcu_dereference_check() usage. ]
 > ---------------------------------------------------
 > /home/lsrc/git/linux-2.6/net/ipv4/fib_trie.c:964 invoked rcu_dereference_check() without protection!
 > 
 > other info that might help us debug this:
 > 
 > 
 > rcu_scheduler_active = 1, debug_locks = 0
 > 2 locks held by ip/4521:
 >  #0:  (rtnl_mutex){+.+.+.}, at: [<ffffffff816466af>] rtnetlink_rcv+0x1f/0x40
 >  #1:  ((inetaddr_chain).rwsem){.+.+.+}, at: [<ffffffff8107cde7>] __blocking_notifier_call_chain+0x47/0x90
 > 
 > stack backtrace:
 > Pid: 4521, comm: ip Not tainted 2.6.34-rc2 #5
 > Call Trace:
 >  [<ffffffff8108b7e9>] lockdep_rcu_dereference+0xb9/0xc0
 >  [<ffffffff81696a05>] fib_find_node+0x185/0x1b0
 >  [<ffffffff8101155f>] ? save_stack_trace+0x2f/0x50
 >  [<ffffffff81699b1c>] fib_table_insert+0xdc/0xa90
 >  [<ffffffff8107cde7>] ? __blocking_notifier_call_chain+0x47/0x90
 >  [<ffffffff8108edb5>] ? __lock_acquire+0x1485/0x1d50
 >  [<ffffffff816926b0>] fib_magic+0xc0/0xd0
 >  [<ffffffff81692738>] fib_add_ifaddr+0x78/0x1a0
 >  [<ffffffff81692e60>] fib_inetaddr_event+0x50/0x2a0
 >  [<ffffffff8173152d>] notifier_call_chain+0x6d/0xb0
 >  [<ffffffff8107cdfd>] __blocking_notifier_call_chain+0x5d/0x90
 >  [<ffffffff8107ce46>] blocking_notifier_call_chain+0x16/0x20
 >  [<ffffffff81688c0a>] __inet_insert_ifa+0xea/0x180
 >  [<ffffffff8168971d>] inetdev_event+0x43d/0x490
 >  [<ffffffff8173152d>] notifier_call_chain+0x6d/0xb0
 >  [<ffffffff8107cb06>] raw_notifier_call_chain+0x16/0x20
 >  [<ffffffff81639f00>] __dev_notify_flags+0x40/0xa0
 >  [<ffffffff81639fa5>] dev_change_flags+0x45/0x70
 >  [<ffffffff81645c2c>] do_setlink+0x2fc/0x4a0
 >  [<ffffffff81294176>] ? nla_parse+0x36/0x110
 >  [<ffffffff81646d54>] rtnl_newlink+0x444/0x540
 >  [<ffffffff8108c44d>] ? mark_held_locks+0x6d/0x90
 >  [<ffffffff8172b8c5>] ? mutex_lock_nested+0x335/0x3c0
 >  [<ffffffff8164685e>] rtnetlink_rcv_msg+0x18e/0x240
 >  [<ffffffff816466d0>] ? rtnetlink_rcv_msg+0x0/0x240
 >  [<ffffffff816520b9>] netlink_rcv_skb+0x89/0xb0
 >  [<ffffffff816466be>] rtnetlink_rcv+0x2e/0x40
 >  [<ffffffff81651b6b>] ? netlink_unicast+0x11b/0x2f0
 >  [<ffffffff81651d2c>] netlink_unicast+0x2dc/0x2f0
 >  [<ffffffff81630a3c>] ? memcpy_fromiovec+0x7c/0xa0
 >  [<ffffffff81652643>] netlink_sendmsg+0x1d3/0x2e0
 >  [<ffffffff81624e20>] sock_sendmsg+0xc0/0xf0
 >  [<ffffffff8108f9cd>] ? lock_release_non_nested+0x9d/0x340
 >  [<ffffffff810fa33b>] ? might_fault+0x7b/0xd0
 >  [<ffffffff810fa33b>] ? might_fault+0x7b/0xd0
 >  [<ffffffff810fa386>] ? might_fault+0xc6/0xd0
 >  [<ffffffff810fa33b>] ? might_fault+0x7b/0xd0
 >  [<ffffffff81630bfc>] ? verify_iovec+0x4c/0xe0
 >  [<ffffffff81625c3e>] sys_sendmsg+0x1ae/0x360
 >  [<ffffffff810fadf9>] ? __do_fault+0x3f9/0x550
 >  [<ffffffff810fd143>] ? handle_mm_fault+0x1a3/0x790
 >  [<ffffffff8112cc77>] ? fget_light+0xe7/0x2f0
 >  [<ffffffff8108c735>] ? trace_hardirqs_on_caller+0x135/0x180
 >  [<ffffffff8172ccc2>] ? trace_hardirqs_on_thunk+0x3a/0x3f
 >  [<ffffffff810030db>] system_call_fastpath+0x16/0x1b
 > 
 > 
 > 
 > 
 > 
 > -- 
 > ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply

* [PATCH 2.6.34-rc2] rxrpc: Check allocation failure.
From: Tetsuo Handa @ 2010-03-22  5:44 UTC (permalink / raw)
  To: dhowells; +Cc: netdev

alloc_skb() can return NULL.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 net/rxrpc/ar-accept.c |    5 +++++
 1 file changed, 5 insertions(+)

--- linux-2.6.34-rc2.orig/net/rxrpc/ar-accept.c
+++ linux-2.6.34-rc2/net/rxrpc/ar-accept.c
@@ -88,6 +88,11 @@ static int rxrpc_accept_incoming_call(st
 
 	/* get a notification message to send to the server app */
 	notification = alloc_skb(0, GFP_NOFS);
+	if (!notification) {
+		_debug("no memory");
+		ret = -ENOMEM;
+		goto error;
+	}
 	rxrpc_new_skb(notification);
 	notification->mark = RXRPC_SKB_MARK_NEW_CALL;
 

^ permalink raw reply

* Re: [RFC PATCH 10/19] pcmcia: dev_node removal (drivers with updated printk call)
From: Karsten Keil @ 2010-03-22  5:22 UTC (permalink / raw)
  To: Dominik Brodowski
  Cc: linux-pcmcia, Harald Welte, linux-ide, linux-wireless, netdev,
	linux-usb
In-Reply-To: <1269212857-25364-10-git-send-email-linux@dominikbrodowski.net>

On Montag, 22. März 2010 00:07:28 Dominik Brodowski wrote:
> As a second step, remove any usage of dev_node_t from drivers which
> only wrote to this typedef/struct, except one printk() which can
> easily be replaced by a dev_info()/dev_warn() call.
> 
> CC: Harald Welte <laforge@gnumonks.org>
> CC: linux-ide@vger.kernel.org
> CC: Karsten Keil <isdn@linux-pingi.de>
> CC: linux-wireless@vger.kernel.org
> CC: netdev@vger.kernel.org
> CC: linux-usb@vger.kernel.org
> Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

Acked-by: Karsten Keil  <isdn@linux-pingi.de>

Thanks.

^ permalink raw reply

* Re: Gianfar: RX Recycle skb->len error
From: David Miller @ 2010-03-22  4:46 UTC (permalink / raw)
  To: ben; +Cc: netdev, avorontsov, Sandeep.Kumar
In-Reply-To: <A6A1774AFD79E346AE6D49A33CB294530DC19EB5@EX-BE-017-SFO.shared.themessagecenter.com>

From: "Ben Menchaca (ben@bigfootnetworks.com)" <ben@bigfootnetworks.com>
Date: Sat, 20 Mar 2010 12:54:59 -0700

> We are seeing some random skb data length errors on RX after long-running, full-gigabit traffic.  First, my debugging and solution are based on the following invariant assumption:
> (skb->tail - skb->data) == skb->len
> 
> If this is wrong, please educate.
> 
> After some tracing, here is where the error packets seem to originate:
> 1.  We are cleaning rx, in gfar_clean_rx_ring;
> 2.  A new RX skb is drawn from the rx_recycle queue, and obey the above invariant (so, in gfar_new_skb(), __skb_dequeue returns an skb);
> 3.  At this point skb_reserve is called, which moves data and tail by the same calculated alignamount;
> 4.  So, newskb is not NULL.  However, !(bdp->status & RXBD_LAST) || (bdp->status & RXBD_ERR)) is evaluates to true;
> 5.  Since newskb is not NULL, we arrive at the else if (skb), which is true;
> 6.  skb->data = skb->head + NET_SKB_PAD is applied, and then the skb is requeued for recycling.
> 
> At this point, skb->data != skb->tail, but skb->len == 0.  When this skb is used for the next RX, it is causing issues later when we skb_put trailers, and then trust skb->len.
> 
> I would propose something like:

Thanks for debugging this, some gianfar developers CC:'d.

> @@ -2540,6 +2540,7 @@ 
> 				 * recycle list.
>  				 */
>  				skb->data = skb->head + NET_SKB_PAD;
> +				skb_reset_tail_pointer(skb);
> 				__skb_queue_head(&priv->rx_recycle, skb);
> 			}
> 		} else {

This code is essentially trying to undo skb_reserve()
but as you found it's doing so in a buggy manner.

skb_reserve() adjusts both the 'data' and 'tail' pointers,
but this attempt at a reversal is only modifying 'data'.

Your fix is fine, but really any by-hand modification of
skb->data is a bug, and we should provide an skb_unreserve()
or similar to hide such details away, and use it here.

Anton?

^ permalink raw reply

* Re: [patch] bridge: cleanup: remove unused assignment
From: David Miller @ 2010-03-22  4:30 UTC (permalink / raw)
  To: herbert
  Cc: error27, shemminger, yoshfuji, paulmck, michael-dev, bridge,
	netdev, kernel-janitors
In-Reply-To: <20100320115747.GA1849@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sat, 20 Mar 2010 19:57:47 +0800

> On Sat, Mar 20, 2010 at 02:20:49PM +0300, Dan Carpenter wrote:
>> We never actually use iph again so this assignment can be removed.
>> 
>> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] netlink: use the appropriate namespace pid
From: David Miller @ 2010-03-22  4:30 UTC (permalink / raw)
  To: thomas.goff; +Cc: netdev, adobriyan
In-Reply-To: <20100320013850.GA13239@boeing.com>

From: Tom Goff <thomas.goff@boeing.com>
Date: Fri, 19 Mar 2010 18:38:50 -0700

> This was included in OpenVZ kernels but wasn't integrated upstream.
>>From git://git.openvz.org/pub/linux-2.6.24-openvz:
> 
> 	commit 5c69402f18adf7276352e051ece2cf31feefab02
> 	Author: Alexey Dobriyan <adobriyan@openvz.org>
> 	Date:   Mon Dec 24 14:37:45 2007 +0300
> 
> 	    netlink: fixup ->tgid to work in multiple PID namespaces
> 
> Signed-off-by: Tom Goff <thomas.goff@boeing.com>

Well, at least CC: the person whose patches your pushing
around, maybe they had a reason to not include it?  The
openvz guys have been extremely good about submitting
their stuff so they deserve the benefit of the doubt.

Alexey, can you review Tom's namespace patches in this
set?

Thanks!

^ permalink raw reply

* Re: [PATCH] ip_gre: include route header_len in max_headroom calculation
From: David Miller @ 2010-03-22  4:26 UTC (permalink / raw)
  To: herbert; +Cc: timo.teras, netdev
In-Reply-To: <20100320150049.GA2950@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sat, 20 Mar 2010 23:00:49 +0800

> On Sat, Mar 20, 2010 at 04:47:28PM +0200, Timo Teräs wrote:
>>
>> Actually, isn't the above right?
>>
>> max_headroom is calculated with LL_RESERVED_SPACE of the tdev, which
>> is the interface to which the gre packet is being sent to, not the
>> gre interface. Thus, max_headroom won't include gre devices
>> previous needed_headroom.
> 
> Indeed you're right.  Sorry for the confusion.
> 
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied, thanks everyone.

^ permalink raw reply

* Re: [PATCH] Improved network performance by balancing Rx against other work
From: David Miller @ 2010-03-22  4:24 UTC (permalink / raw)
  To: peter.chubb; +Cc: netdev
In-Reply-To: <8739ztrnrp.wl%peterc@chubb.wattle.id.au>

From: Peter Chubb <peter.chubb@nicta.com.au>
Date: Mon, 22 Mar 2010 15:19:54 +1100

> David> It's one of the best drivers in the locking area.
> 
> It looked good from that aspect, but used too many features of NAPI
> for me to modify quickly, and still have a simple and
> easy-to-understand patch.

What's so complicated about it?  All of the NAPI logic is
locks into a wrapper function that encapsulates all of
the top-level budget and interrupt masking logic.

> Anyway, I'm intending to try to reproduce the results with a different
> driver, as I said.

Well, you do so at your own peril.  tg3 is the best, whereas
r8169 is really awful and people report NAPI wedges with it
all the time.

^ permalink raw reply

* Re: [PATCH] Improved network performance by balancing Rx against other work
From: Peter Chubb @ 2010-03-22  4:19 UTC (permalink / raw)
  To: David Miller; +Cc: peter.chubb, netdev
In-Reply-To: <20100321.211124.216749316.davem@davemloft.net>

>>>>> "David" == David Miller <davem@davemloft.net> writes:

David> From: Peter Chubb <peter.chubb@nicta.com.au> Date: Mon, 22 Mar
David> 2010 15:04:24 +1100

>>>>>>> "David" == David Miller <davem@davemloft.net> writes:
>>
David> From: Peter Chubb <peter.chubb@nicta.com.au> Date: Wed, 17 Mar

>> However, softIRQ processing is still not being preempted by a
>> real-time process that wakes up.

David> I thought softirqs ran as threads in the -rt kernel, why
David> doesn't preemption work properly for those threads?

I'm using the standard kernel here -- softIRQs run as threads
scheduled as SCHED_NORMAL, but because they're in the kernel, they're
not preemptible without an explicit preemption point, as far as I can
tell.  So hardware interrupts will run, and the budget/work done
mechanism will ensure that other softIRQs and in-kernel real-time
threads run, but userspace doesn't seem to get a look in.

>> I had a look at the Broadcom tg3, but it looks too hard to modify;

David> It's one of the best drivers in the locking area.

It looked good from that aspect, but used too many features of NAPI
for me to modify quickly, and still have a simple and
easy-to-understand patch.

Anyway, I'm intending to try to reproduce the results with a different
driver, as I said.

Peter C

--
Dr Peter Chubb  http://www.gelato.unsw.edu.au  peterc AT gelato.unsw.edu.au
http://www.ertos.nicta.com.au           ERTOS within National ICT Australia
--
Dr Peter Chubb  http://www.gelato.unsw.edu.au  peterc AT gelato.unsw.edu.au
http://www.ertos.nicta.com.au           ERTOS within National ICT Australia

^ permalink raw reply

* Re: [PATCH v2] if_tunnel.h: add missing ams/byteorder.h include
From: David Miller @ 2010-03-22  4:19 UTC (permalink / raw)
  To: paulius.zaleckas
  Cc: contact, eric.dumazet, Fred.L.Templin, netdev, linux-kernel
In-Reply-To: <20100319150926.11037.39136.stgit@pauliusz>

From: Paulius Zaleckas <paulius.zaleckas@gmail.com>
Date: Fri, 19 Mar 2010 17:09:26 +0200

> When compiling userspace application which includes
> if_tunnel.h and uses GRE_* defines you will get undefined
> reference to __cpu_to_be16.
> 
> Fix this by adding missing #include <asm/byteorder.h>
> 
> Cc: stable@kernel.org
> Signed-off-by: Paulius Zaleckas <paulius.zaleckas@gmail.com>

This slipped through the cracks, someone else submitted something
similar back in August 2009 it seems.

Thanks for sending this, applied.

^ permalink raw reply

* Re: [PATCH] Improved network performance by balancing Rx against other work
From: David Miller @ 2010-03-22  4:11 UTC (permalink / raw)
  To: peter.chubb; +Cc: netdev
In-Reply-To: <877hp5rohj.wl%peterc@chubb.wattle.id.au>

From: Peter Chubb <peter.chubb@nicta.com.au>
Date: Mon, 22 Mar 2010 15:04:24 +1100

>>>>>> "David" == David Miller <davem@davemloft.net> writes:
> 
> David> From: Peter Chubb <peter.chubb@nicta.com.au> Date: Wed, 17 Mar
> David> 2010 13:55:58 +1100
> 
>>> The general approach is to restrict the work done in the Rx-side
>>> processing to just 32 or so packets at a time then call
>>> sys_sched_yield() to allow other system processing to get a look
>>> in.  Currently, NAPI processing happens in soft IRQ context, and
>>> much of it with interrupts off.
> 
> David> This is a deficiency in the locking done by such drivers.  Many
> David> drivers lock properly and do not disable hardware interrupts
> David> during NAPI processing.  Not only is this more efficient, it
> David> also makes the driver more profilable.  For example, on cpus
> David> with only timer based profiling everything done in NAPI context
> David> can be seen.
> 
> Actually, the e1000 does not appear to disable interrupts during NAPI
> processing.

So you don't know the fundamental aspects of what you were actually
analyzing? :-)

> However, softIRQ processing is still not being preempted by a
> real-time process that wakes up.

I thought softirqs ran as threads in the -rt kernel, why doesn't
preemption work properly for those threads?

> I had a look at the Broadcom tg3, but it looks too
> hard to modify;

It's one of the best drivers in the locking area.

It doesn't take any locks at all in it's hardware interrupt handler.

It doesn't take any locks at all for RX packet processing.

And it only takes a lock for TX processing very briefly in one
specific case, when we need to wake up the TX queue because it became
full and was stopped and we need to wake it up in tg3_tx()

^ permalink raw reply

* Re: [PATCH] Improved network performance by balancing Rx against other work
From: Peter Chubb @ 2010-03-22  4:04 UTC (permalink / raw)
  To: David Miller; +Cc: peter.chubb, netdev
In-Reply-To: <20100321.202159.235697842.davem@davemloft.net>

>>>>> "David" == David Miller <davem@davemloft.net> writes:

David> From: Peter Chubb <peter.chubb@nicta.com.au> Date: Wed, 17 Mar
David> 2010 13:55:58 +1100

>> The general approach is to restrict the work done in the Rx-side
>> processing to just 32 or so packets at a time then call
>> sys_sched_yield() to allow other system processing to get a look
>> in.  Currently, NAPI processing happens in soft IRQ context, and
>> much of it with interrupts off.

David> This is a deficiency in the locking done by such drivers.  Many
David> drivers lock properly and do not disable hardware interrupts
David> during NAPI processing.  Not only is this more efficient, it
David> also makes the driver more profilable.  For example, on cpus
David> with only timer based profiling everything done in NAPI context
David> can be seen.

Actually, the e1000 does not appear to disable interrupts during NAPI
processing.  However, softIRQ processing is still not being preempted
by a real-time process that wakes up.

The performance issue is not this, however, but the receive livelock problem
caused by too many packets being queued to higher layers --- more than
can be handled before the next interrupt causes more packets to be
batched up.  When the NAPI budget is consumed, but there are more packets
to handle, the NAPI layer currently reraises the softIRQ --- which
fires before anything else gets a go on the processor.


Anyway, if you think the e1000 driver is broken, I'll try with a
different one.  I had a look at the Broadcom tg3, but it looks too
hard to modify; so I'm now looking at the Realtek r8169 driver.  It
might take a few days.

PeterC
--
Dr Peter Chubb  http://www.gelato.unsw.edu.au  peterc AT gelato.unsw.edu.au
http://www.ertos.nicta.com.au           ERTOS within National ICT Australia
--
Dr Peter Chubb  http://www.gelato.unsw.edu.au  peterc AT gelato.unsw.edu.au
http://www.ertos.nicta.com.au           ERTOS within National ICT Australia

^ permalink raw reply

* Re: [Uclinux-dist-devel] [PATCH] can: bfin_can: switch to common Blackfin can header
From: David Miller @ 2010-03-22  3:58 UTC (permalink / raw)
  To: vapier.adi
  Cc: socketcan-core, netdev, uclinux-dist-devel, oliver.hartkopp,
	urs.thuermann
In-Reply-To: <8bd0f97a1003212054h6c25f99dvb35869f786807c88@mail.gmail.com>

From: Mike Frysinger <vapier.adi@gmail.com>
Date: Sun, 21 Mar 2010 23:54:41 -0400

> either you've deleted the thread or your mail client sucks ?  the
> patch in question started this thread you keep replying to ...

You're an idiot and you don't care how much work you are
making for me, so you are now set to ignore.

When I say "resubmit" I've deleted your patch from my inbox
and marked it "changed requested" or similar in patchwork
so it doesn't show up in the todo list any more.

Thanks.

^ permalink raw reply

* Re: [Uclinux-dist-devel] [PATCH] can: bfin_can: switch to common Blackfin can header
From: Mike Frysinger @ 2010-03-22  3:54 UTC (permalink / raw)
  To: David Miller
  Cc: socketcan-core, netdev, uclinux-dist-devel, oliver.hartkopp,
	urs.thuermann
In-Reply-To: <20100321.203927.90806980.davem@davemloft.net>

On Sun, Mar 21, 2010 at 23:39, David Miller wrote:
> From: Mike Frysinger <vapier.adi@gmail.com>
>> On Wed, Mar 17, 2010 at 01:51, David Miller wrote:
>>> From: Mike Frysinger <vapier.adi@gmail.com>
>>>> if the next tree is merged into linux-next, then the header is already there
>>>
>>> Just get your stuff merged properly to Linus then resubmit
>>> your patch.
>>
>> Linus has pulled my tree, so there shouldnt be any reason to not merge
>> this patch now
>
> If you can't understand what the heck the word "RESUMBIT" means,
> then I'm not even going to pay attention to you anymore.
>
> I keep telling you to resubmit the thing when it'll work correctly
> because it makes things 1,000 times easier for me then crawling
> through the mailing list and patchwork history trying to fish out
> your patch and figure out if it's the right one or not.

either you've deleted the thread or your mail client sucks ?  the
patch in question started this thread you keep replying to ...
-mike

^ permalink raw reply

* Re: [PATCH] net: Don't drop route cache entry in ipv4_negative_advice unless PTMU expired
From: David Miller @ 2010-03-22  3:53 UTC (permalink / raw)
  To: guenter.roeck; +Cc: netdev, viro, den
In-Reply-To: <1269009673-25497-1-git-send-email-guenter.roeck@ericsson.com>

From: Guenter Roeck <guenter.roeck@ericsson.com>
Date: Fri, 19 Mar 2010 07:41:13 -0700

> TCP sessions over IPv4 can get stuck if routers between endpoints
> do not fragment packets but implement PMTU instead.

This bug actually only applies to a much more specific case.

It only occurs when the router we end up using is the result of
receiving a redirect to it.

If we use the configured route, and do not get redirected, the problem
never happens.

In any case, your patch is correct, and I'll add some clarification to
the commit message when I check this in.

Thanks a lot!

^ permalink raw reply

* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Herbert Xu @ 2010-03-22  3:52 UTC (permalink / raw)
  To: David Miller; +Cc: timo.teras, netdev
In-Reply-To: <20100321.201258.184825751.davem@davemloft.net>

On Sun, Mar 21, 2010 at 08:12:58PM -0700, David Miller wrote:
> 
> Good point, I was misunderstanding how things work now and how
> that would change with your proposal.
> 
> Having multiple xfrm_dsts exist for an IPSEC route seems fine
> to me.

Thanks for the confirmation.

Timo, let's roll along with the per-cpu xfrm_dst approach.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Herbert Xu @ 2010-03-22  3:52 UTC (permalink / raw)
  To: Timo Teräs; +Cc: netdev, David S. Miller
In-Reply-To: <4BA5D95B.4020004@iki.fi>

On Sun, Mar 21, 2010 at 10:31:23AM +0200, Timo Teräs wrote:
>
>> Ok, we can do that to skip 2. But I think 1 would be still useful.
>> It'd probably be good to actually have flow_cache_ops pointer in
>> each entry instead of the atomic_t pointer.
>>
>> The reasoning:
>> - we can then have type-based checks that the reference count
>>  is valid (e.g. policy's refcount must not go to zero, it's bug,
>>  and we can call dst_release which warns if refcount goes to
>>  negative); imho it's hack to call atomic_dec instead of the
>>  real type's xxx_put
>> - the flow cache needs to somehow know if the entry is stale so
>>  it'll try to refresh it atomically; e.g. if there's no
>>  check for 'stale', the lookup returns stale xfrm_dst. we'd
>>  then need new api to update the stale entry, or flush it out
>>  and repeat the lookup. the virtual get could check for it being
>>  stale (if so release the entry) and then return null for the
>>  generic code to call the resolver atomically
>> - for paranoia we can actually check the type of the object in
>>  cache via the ops (if needed)

The reason I'd prefer to keep the current scheme is to avoid
an additional indirect function call on each packet.

The way it would work is (we need flow_cache_lookup to return
fle instead of the object):

	fle = flow_cache_lookup
	xdst = fle->object
	if (xdst is stale) {
		flow_cache_mark_obsolete(fle)
		fle = flow_cache_lookup
		xdst = fle->object
		if (xdst is stale)
			return error
	}

Where flow_cache_mark_obsolete would set a flag in the fle that's
checked by flow_cache_lookup.  To prevent the very rare case
where we mark an entry obsolete incorrectly, the resolver function
should double-check that the existing entry is indeed obsolete
before making a new one.

This way we give the overhead over to the slow path where the
bundle is stale.

You were saying that our bundles are going stale very frequently,
that would sound like a bug that we should look into.  The whole
caching scheme is pointless if the bundle is going stale every
other packet.

> - could cache bundle OR policy for outgoing stuff. it's useful
>  to cache the policy in case we need to sleep, or if it's a
>  policy forbidding traffic. in those cases there's no bundle
>  to cache at all. alternatively we can make dummy bundles that
>  are marked invalid and are just used to keep a reference to
>  the policy.

My instinct is to go with dummy bundles.  That way given the
direction we know exactly what object type it is.  Having mixed
object types is just too much of a pain.

> Oh, this also implies that the resolver function should be
> changed to get the old stale object so it can re-use it to
> get the policy object instead of searching it all over again.

That should be easy to implement.  Just prefill the obj argument
to the resolver with either NULL or the stale object.

For the bundle resolver, it should also remove the stale bundle
from the policy bundle list and drop its reference.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] net: dev_getfirstbyhwtype() optimization
From: David Miller @ 2010-03-22  3:39 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1268947645.2894.166.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 18 Mar 2010 22:27:25 +0100

> Use RCU to avoid RTNL use in dev_getfirstbyhwtype()
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks Eric.

^ permalink raw reply

* Re: [Uclinux-dist-devel] [PATCH] can: bfin_can: switch to common Blackfin can header
From: David Miller @ 2010-03-22  3:39 UTC (permalink / raw)
  To: vapier.adi
  Cc: socketcan-core, netdev, uclinux-dist-devel, oliver.hartkopp,
	urs.thuermann
In-Reply-To: <8bd0f97a1003182335l3c121d87v9369145852c9545d@mail.gmail.com>

From: Mike Frysinger <vapier.adi@gmail.com>
Date: Fri, 19 Mar 2010 02:35:17 -0400

> On Wed, Mar 17, 2010 at 01:51, David Miller wrote:
>> From: Mike Frysinger <vapier.adi@gmail.com>
>>> if the next tree is merged into linux-next, then the header is already there
>>
>> Just get your stuff merged properly to Linus then resubmit
>> your patch.
> 
> Linus has pulled my tree, so there shouldnt be any reason to not merge
> this patch now

If you can't understand what the heck the word "RESUMBIT" means,
then I'm not even going to pay attention to you anymore.

I keep telling you to resubmit the thing when it'll work correctly
because it makes things 1,000 times easier for me then crawling
through the mailing list and patchwork history trying to fish out
your patch and figure out if it's the right one or not.

^ permalink raw reply

* Re: [PATCH] pktgen node allocation
From: David Miller @ 2010-03-22  3:37 UTC (permalink / raw)
  To: robert; +Cc: eric.dumazet, netdev
In-Reply-To: <19363.32154.39665.185451@gargle.gargle.HOWL>

From: robert@herjulf.net
Date: Fri, 19 Mar 2010 14:35:22 +0100

> 
> Eric Dumazet writes:
>  > Le vendredi 19 mars 2010 à 09:44 +0100, Robert Olsson a écrit :
>  > 
>  > I cannot understand how this can help.
>  > 
>  > __netdev_alloc_skb() is supposed to already take into account NUMA
>  > properties :
>  > 
>  > int node = dev->dev.parent ? dev_to_node(dev->dev.parent) : -1;
>  > 
>  > If this doesnt work, we should correct core stack, not only pktgen :)
>  > 
>  > Are you allocating memory in the node where pktgen CPU is running or the
>  > node close to the NIC ?
> 
>  I didn't say it should help the idea was to give some hooks to 
>  experiment and see effects with different node memory allocations.
>  There are many degrees of freedom wrt buses(device)/CPU/menory.

I think it's a useful feature and by default the netdev alloc
is still used, so... applied to net-next-2.6

^ permalink raw reply

* Re: [PATCH net-next-2.6 2/2] can: sja1000: add read/write routines for 8, 16 and 32-bit register access
From: David Miller @ 2010-03-22  3:37 UTC (permalink / raw)
  To: w.sang; +Cc: yegor_sub1, netdev, socketcan-core
In-Reply-To: <20100320035639.GB26934@pengutronix.de>

From: Wolfram Sang <w.sang@pengutronix.de>
Date: Sat, 20 Mar 2010 04:56:39 +0100

> On Fri, Mar 19, 2010 at 11:50:44AM +0100, Yegor Yefremov wrote:
>> SJA1000: add read/write routines for 8, 16 and 32-bit register access
>> 
>> add routines for 8, 16 and 32-bit access like in 
>> drivers/i2c/busses/i2c-pca-platform.c
>> 
>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> 
> Acked-by: Wolfram Sang <w.sang@pengutronix.de>

Applied.

^ permalink raw reply

* Re: [PATCH net-next-2.6 1/2] can: sja1000: allow shared interrupt definition
From: David Miller @ 2010-03-22  3:36 UTC (permalink / raw)
  To: wg; +Cc: yegor_sub1, netdev, socketcan-core
In-Reply-To: <4BA37868.5040303@grandegger.com>

From: Wolfgang Grandegger <wg@grandegger.com>
Date: Fri, 19 Mar 2010 14:13:12 +0100

> Yegor Yefremov wrote:
>> SJA1000: allow shared interrupt definition
>> 
>> extend the AND mask, so that IRQF_SHARED flag remains
>> 
>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> 
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>

Applied.

^ permalink raw reply

* Re: [PATCH] Improved network performance by balancing Rx against other work
From: David Miller @ 2010-03-22  3:21 UTC (permalink / raw)
  To: peter.chubb; +Cc: netdev
In-Reply-To: <87tysfu05d.wl%peterc@chubb.wattle.id.au>

From: Peter Chubb <peter.chubb@nicta.com.au>
Date: Wed, 17 Mar 2010 13:55:58 +1100

> The general approach is to restrict the work done in the Rx-side
> processing to just 32 or so packets at a time then call
> sys_sched_yield() to allow other system processing to get a look in.
> Currently, NAPI processing happens in soft IRQ context, and much of it
> with interrupts off.

This is a deficiency in the locking done by such drivers.  Many
drivers lock properly and do not disable hardware interrupts during
NAPI processing.  Not only is this more efficient, it also makes
the driver more profilable.  For example, on cpus with only timer
based profiling everything done in NAPI context can be seen.

> In addition, because so much runs with interrupts disabled,
> real-time performance sucks.

Please respin your analysis to be done with a sane driver, one that
does not disable hardware interrupts.  Doing so is a driver, rather
than a fundamental, issue of NAPI.

Avoiding hardware interrupt disabling also has another huge benefit,
it means that the transmit path of the driver need not disable
hardware interrupts either since that already runs in software
interrupt disabled context.

Your analysis was essentially done with broken driver(s), so I think
I can stop reading here.

^ permalink raw reply

* Re: [PATCH RFC] tun: add ioctl to modify vnet header size
From: David Miller @ 2010-03-22  3:17 UTC (permalink / raw)
  To: mst
  Cc: dlstevens, David.Woodhouse, herbert, linux-kernel, netdev,
	netdev-owner, paul.moore, sri
In-Reply-To: <20100317221117.GA7541@redhat.com>

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 18 Mar 2010 00:11:17 +0200

> All this does is set how much of the buffer to skip, this option does
> not allocate any memory.  So if you set it to a value > length that you
> passed in, you get -EINVAL. Anything else should work.  Negative values
> are checked for and return -EINVAL when you try to set it.  At least,
> all that's by design - pls take a look at the code and if you see any
> issues, speak up please.
> 
> I agree we don't really need to support very large values here,
> it just seemed less work.

This all looks fine to me, submit a final version to me via
whatever means you feel is appropriate.

^ 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