* Re: [RFC] net/core: Delay neighbor only if it has been used after confirmed
From: David Miller @ 2009-09-15 10:13 UTC (permalink / raw)
To: jens; +Cc: netdev, yoshfuji
In-Reply-To: <1253009246.8352.10.camel@fnki-nb00130>
From: Jens Rosenboom <jens@mcbone.net>
Date: Tue, 15 Sep 2009 12:07:26 +0200
> On Thu, 2009-09-10 at 18:21 +0200, Jens Rosenboom wrote:
>> On Wed, 2009-09-02 at 21:22 +0900, YOSHIFUJI Hideaki wrote:
>> [...]
>> > And, this "if" for REACHABLE->DELAY may be completely needless.
>> > Timer in REACHABLE is only for state transition for toward REACHABLE
>> > or STALE.
>>
>> I did some testing with the following patch, which works fine for me, so
>> I propose this one now instead of my previous one. I still have no real
>> idea about the non-IPv6 implications of this, though.
>>
>> ---
>>
>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
>> index e587e68..f61926f 100644
>> --- a/net/core/neighbour.c
>> +++ b/net/core/neighbour.c
>> @@ -819,13 +819,6 @@ static void neigh_timer_handler(unsigned long arg)
>> neigh->confirmed + neigh->parms->reachable_time)) {
>> NEIGH_PRINTK2("neigh %p is still alive.\n", neigh);
>> next = neigh->confirmed + neigh->parms->reachable_time;
>> - } else if (time_before_eq(now,
>> - neigh->used + neigh->parms->delay_probe_time)) {
>> - NEIGH_PRINTK2("neigh %p is delayed.\n", neigh);
>> - neigh->nud_state = NUD_DELAY;
>> - neigh->updated = jiffies;
>> - neigh_suspect(neigh);
>> - next = now + neigh->parms->delay_probe_time;
>> } else {
>> NEIGH_PRINTK2("neigh %p is suspected.\n", neigh);
>> neigh->nud_state = NUD_STALE;
>>
>
> Hi David, what are your thoughts on this one? There is IMHO a real bug
> to fix, namely sending tons of repeated neighbor solicitations when
> there is no actual traffic to be sent, so this should qualify to go into
> 2.6.32. Do you want to wait for further comments or should I submit this
> for net-2.6 so it can get some testing?
I'm waiting for Yoshifuji's feedback to your latest patch.
^ permalink raw reply
* Re: [RFC] net/core: Delay neighbor only if it has been used after confirmed
From: Jens Rosenboom @ 2009-09-15 10:07 UTC (permalink / raw)
To: David Miller; +Cc: Linux Network Developers, YOSHIFUJI Hideaki
In-Reply-To: <1252599707.5980.13.camel@fnki-nb00130>
On Thu, 2009-09-10 at 18:21 +0200, Jens Rosenboom wrote:
> On Wed, 2009-09-02 at 21:22 +0900, YOSHIFUJI Hideaki wrote:
> [...]
> > And, this "if" for REACHABLE->DELAY may be completely needless.
> > Timer in REACHABLE is only for state transition for toward REACHABLE
> > or STALE.
>
> I did some testing with the following patch, which works fine for me, so
> I propose this one now instead of my previous one. I still have no real
> idea about the non-IPv6 implications of this, though.
>
> ---
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index e587e68..f61926f 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -819,13 +819,6 @@ static void neigh_timer_handler(unsigned long arg)
> neigh->confirmed + neigh->parms->reachable_time)) {
> NEIGH_PRINTK2("neigh %p is still alive.\n", neigh);
> next = neigh->confirmed + neigh->parms->reachable_time;
> - } else if (time_before_eq(now,
> - neigh->used + neigh->parms->delay_probe_time)) {
> - NEIGH_PRINTK2("neigh %p is delayed.\n", neigh);
> - neigh->nud_state = NUD_DELAY;
> - neigh->updated = jiffies;
> - neigh_suspect(neigh);
> - next = now + neigh->parms->delay_probe_time;
> } else {
> NEIGH_PRINTK2("neigh %p is suspected.\n", neigh);
> neigh->nud_state = NUD_STALE;
>
Hi David, what are your thoughts on this one? There is IMHO a real bug
to fix, namely sending tons of repeated neighbor solicitations when
there is no actual traffic to be sent, so this should qualify to go into
2.6.32. Do you want to wait for further comments or should I submit this
for net-2.6 so it can get some testing?
^ permalink raw reply
* Re: [PATCH v2] pkt_sched: Fix tx queue selection in tc_modify_qdisc
From: David Miller @ 2009-09-15 9:53 UTC (permalink / raw)
To: jarkao2; +Cc: kaber, netdev
In-Reply-To: <20090914225022.GA13363@ami.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Tue, 15 Sep 2009 00:50:22 +0200
> -----------------------> (take 2)
> pkt_sched: Fix tx queue selection in tc_modify_qdisc
>
> After the recent mq change there is the new select_queue qdisc class
> method used in tc_modify_qdisc, but it works OK only for direct child
> qdiscs of mq qdisc. Grandchildren always get the first tx queue, which
> would give wrong qdisc_root etc. results (e.g. for sch_htb as child of
> sch_prio). This patch fixes it by using parent's dev_queue for such
> grandchildren qdiscs. The select_queue method's return type is changed
> BTW.
>
> With feedback from: Patrick McHardy <kaber@trash.net>
>
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
Applied, thanks Jarek.
^ permalink raw reply
* Re: [PATCH alt] sky2: Make sure both ports initialize correctly
From: David Miller @ 2009-09-15 9:50 UTC (permalink / raw)
To: shemminger; +Cc: mikem, netdev
In-Reply-To: <20090914092229.31490f45@nehalam>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Mon, 14 Sep 2009 09:22:29 -0700
> Sorry Mike, I sent you off the wrong way. The following is simpler and the
> second port is diffrent enough in setup (because of NAPI), that the
> following is simpler.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH alt] sky2: transmit ring accounting
From: David Miller @ 2009-09-15 9:50 UTC (permalink / raw)
To: shemminger; +Cc: mikem, netdev
In-Reply-To: <20090914091255.2cda7d24@nehalam>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Mon, 14 Sep 2009 09:12:55 -0700
> Be more accurate about number of transmit list elements required.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/4] RxRPC: Declare the security index constants symbolically
From: David Miller @ 2009-09-15 9:46 UTC (permalink / raw)
To: dhowells; +Cc: netdev
All 4 patches applied, but...
The rxrpc code seems to use a hodge-podge of uint32_t, uint16_t et
al. and the Linux kernel preferred "u8".
Please consolidate it all to use u32, u16, etc.
This looks especially weird since you use the endianness aware
fixed sized Linux types (be32, be16) as well.
^ permalink raw reply
* Re: [PATCH kernel 2.6.31] pcnet_cs: add cis of Linksys multifunction pcmcia card
From: David Miller @ 2009-09-15 9:42 UTC (permalink / raw)
To: ken_kawasaki; +Cc: netdev
In-Reply-To: <20090913172257.6c1976ec.ken_kawasaki@spring.nifty.jp>
From: Ken Kawasaki <ken_kawasaki@spring.nifty.jp>
Date: Sun, 13 Sep 2009 17:22:57 +0900
>
> pcnet_cs,serial_cs:
>
> add cis of Linksys lan&modem mulitifunction pcmcia card
> and some modem card(MT5634ZLX, RS-COM-2P).
>
>
> Signed-off-by: Ken Kawasaki <ken_kawasaki@spring.nifty.jp>
Applied, thank you.
^ permalink raw reply
* Re: WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (bf438284)
From: David Miller @ 2009-09-15 9:39 UTC (permalink / raw)
To: mingo; +Cc: eric.dumazet, penberg, vegard.nossum, linux-kernel, netdev
In-Reply-To: <20090915093440.GA7921@elte.hu>
From: Ingo Molnar <mingo@elte.hu>
Date: Tue, 15 Sep 2009 11:34:40 +0200
>
> * Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> Either we add kmemcheck annotations, or we switch sock->type from short
>> to int to avoid the hole, and possibly to speedup things...
>>
>> [PATCH] net: kmemcheck annotation in struct socket
>>
>> struct socket has a 16 bit hole that triggers kmemcheck warnings.
>>
>> As suggested by Ingo, use kmemcheck annotations
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> ---
>> include/linux/net.h | 5 +++++
>> net/socket.c | 1 +
>> 2 files changed, 6 insertions(+)
>
> Acked-by: Ingo Molnar <mingo@elte.hu>
Applied, thanks everyone.
^ permalink raw reply
* Re: WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (bf438284)
From: Ingo Molnar @ 2009-09-15 9:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: Pekka Enberg, Vegard Nossum, linux-kernel, Linux Netdev List,
David S. Miller
In-Reply-To: <4AAF5757.30500@gmail.com>
* Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Ingo Molnar a ?crit :
> > FYI, we still have this one on latest mainline:
> >
> > [ 2.159614] NET: Registered protocol family 16
> > [ 2.163109] initcall netlink_proto_init+0x0/0x1b0 returned 0 after 5859 usecs
> > [ 2.164008] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (bf438284)
> > [ 2.165006] 0100000002000000000000000000000000000000ad4eaddeffffffffffffffff
> > [ 2.172006] i i i i i i u u i i i i i i i i i i i i i i i i i i i i i i i i
> > [ 2.179005] ^
> > [ 2.180005]
> > [ 2.181008] Pid: 1, comm: swapper Not tainted (2.6.31-tip-02389-gc9f313c-dirty #151)
> > [ 2.182006] EIP: 0060:[<815a8101>] EFLAGS: 00010282 CPU: 0
> > [ 2.183009] EIP is at sock_init_data+0xe1/0x210
> > [ 2.184006] EAX: 0001b000 EBX: bf855938 ECX: 8233b614 EDX: 819ac7bf
> > [ 2.185006] ESI: bf855800 EDI: bf438280 EBP: bf867f10 ESP: 81b3afcc
> > [ 2.186006] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
> > [ 2.187006] CR0: 8005003b CR2: bf83bdf0 CR3: 01b2c000 CR4: 000006d0
> > [ 2.188006] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> > [ 2.189006] DR6: ffff4ff0 DR7: 00000400
> > [ 2.190005] [<815d82b5>] __netlink_create+0x35/0xa0
> > [ 2.192005] [<815dabaa>] netlink_kernel_create+0x5a/0x150
> > [ 2.194004] [<815bc8ee>] rtnetlink_net_init+0x1e/0x40
> > [ 2.196005] [<815af381>] register_pernet_operations+0x11/0x30
> > [ 2.198004] [<815af4be>] register_pernet_subsys+0x1e/0x30
> > [ 2.200004] [<81adb49c>] rtnetlink_init+0x4c/0x100
> > [ 2.202004] [<81adbfe5>] netlink_proto_init+0x105/0x1b0
> > [ 2.204004] [<81001127>] do_one_initcall+0x27/0x190
> > [ 2.206004] [<81a9f567>] do_initcalls+0x27/0x40
> > [ 2.208004] [<81a9f5a6>] do_basic_setup+0x26/0x30
> > [ 2.210004] [<81a9f907>] kernel_init+0x57/0xa0
> > [ 2.212004] [<81004867>] kernel_thread_helper+0x7/0x30
> > [ 2.214004] [<ffffffff>] 0xffffffff
> > [ 2.216021] calling bdi_class_init+0x0/0x30 @ 1
> > [ 2.217015] device class 'bdi': registering
> > [ 2.218702] initcall bdi_class_init+0x0/0x30 returned 0 after 976 usecs
> > [ 2.219041] calling kobject_uevent_init+0x0/0x50 @ 1
> >
> > config attached.
> >
> > Ingo
> >
>
> I thought this was already discussed and fixed somehow ?
yes, it looked familar.
> Either we add kmemcheck annotations, or we switch sock->type from short
> to int to avoid the hole, and possibly to speedup things...
>
> [PATCH] net: kmemcheck annotation in struct socket
>
> struct socket has a 16 bit hole that triggers kmemcheck warnings.
>
> As suggested by Ingo, use kmemcheck annotations
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> include/linux/net.h | 5 +++++
> net/socket.c | 1 +
> 2 files changed, 6 insertions(+)
Acked-by: Ingo Molnar <mingo@elte.hu>
Ingo
^ permalink raw reply
* Re: alloc skb based on a given data buffer
From: Zhu Yi @ 2009-09-15 9:15 UTC (permalink / raw)
To: David Miller
Cc: mel@csn.ul.ie, Chatre, Reinette, elendil@planet.nl,
Larry.Finger@lwfinger.net, linville@tuxdriver.com,
penberg@cs.helsinki.fi, linux-kernel@vger.kernel.org,
linux-wireless@vger.kernel.org,
ipw3945-devel@lists.sourceforge.net, akpm@linux-foundation.org,
cl@linux-foundation.org, Krauss, Assaf, johannes@sipsolutions.net,
Abbas, Mohamed, netdev@vger.kernel.org
In-Reply-To: <20090915.020903.93643290.davem@davemloft.net>
On Tue, 2009-09-15 at 17:09 +0800, David Miller wrote:
> From: Zhu Yi <yi.zhu@intel.com>
> Date: Tue, 15 Sep 2009 16:57:29 +0800
>
> > Thanks. So we can put the 8K buffer into 2 skb_shinfo()->frags[] slots
> > and set nr_frags to 2, right? Is this supported allover the network code
> > already? At a first glance, I didn't find any frags handling in mac80211
> > stack.
>
> You have to pre-pull the link level protocol headers into the
> linear area, but that's it.
>
> Again, see niu.c for details, it does:
>
> static void niu_rx_skb_append(struct sk_buff *skb, struct page *page,
> u32 offset, u32 size)
> {
> int i = skb_shinfo(skb)->nr_frags;
> skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
>
> frag->page = page;
> frag->page_offset = offset;
> frag->size = size;
>
> skb->len += size;
> skb->data_len += size;
> skb->truesize += size;
>
> skb_shinfo(skb)->nr_frags = i + 1;
> }
>
> to add pages to SKBs and then at the end it goes:
>
> skb_reserve(skb, NET_IP_ALIGN);
> __pskb_pull_tail(skb, min(len, NIU_RXPULL_MAX));
>
> Right before giving the SKB to the networking stack. NIU_RXPULL_MAX
> should be a value that will be large enough to cover the largest
> possible link level header.
I see. Thanks for this info. I'll try implementing the same for iwlagn.
Thanks,
-yi
^ permalink raw reply
* Re: alloc skb based on a given data buffer
From: David Miller @ 2009-09-15 9:09 UTC (permalink / raw)
To: yi.zhu
Cc: mel, reinette.chatre, elendil, Larry.Finger, linville, penberg,
linux-kernel, linux-wireless, ipw3945-devel, akpm, cl,
assaf.krauss, johannes, mohamed.abbas, netdev
In-Reply-To: <1253005050.7549.58.camel@debian>
From: Zhu Yi <yi.zhu@intel.com>
Date: Tue, 15 Sep 2009 16:57:29 +0800
> Thanks. So we can put the 8K buffer into 2 skb_shinfo()->frags[] slots
> and set nr_frags to 2, right? Is this supported allover the network code
> already? At a first glance, I didn't find any frags handling in mac80211
> stack.
You have to pre-pull the link level protocol headers into the
linear area, but that's it.
Again, see niu.c for details, it does:
static void niu_rx_skb_append(struct sk_buff *skb, struct page *page,
u32 offset, u32 size)
{
int i = skb_shinfo(skb)->nr_frags;
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
frag->page = page;
frag->page_offset = offset;
frag->size = size;
skb->len += size;
skb->data_len += size;
skb->truesize += size;
skb_shinfo(skb)->nr_frags = i + 1;
}
to add pages to SKBs and then at the end it goes:
skb_reserve(skb, NET_IP_ALIGN);
__pskb_pull_tail(skb, min(len, NIU_RXPULL_MAX));
Right before giving the SKB to the networking stack. NIU_RXPULL_MAX
should be a value that will be large enough to cover the largest
possible link level header.
^ permalink raw reply
* Re: WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (bf438284)
From: Eric Dumazet @ 2009-09-15 8:59 UTC (permalink / raw)
To: Ingo Molnar
Cc: Pekka Enberg, Vegard Nossum, linux-kernel, Linux Netdev List,
David S. Miller
In-Reply-To: <20090915080953.GA24958@elte.hu>
Ingo Molnar a écrit :
> FYI, we still have this one on latest mainline:
>
> [ 2.159614] NET: Registered protocol family 16
> [ 2.163109] initcall netlink_proto_init+0x0/0x1b0 returned 0 after 5859 usecs
> [ 2.164008] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (bf438284)
> [ 2.165006] 0100000002000000000000000000000000000000ad4eaddeffffffffffffffff
> [ 2.172006] i i i i i i u u i i i i i i i i i i i i i i i i i i i i i i i i
> [ 2.179005] ^
> [ 2.180005]
> [ 2.181008] Pid: 1, comm: swapper Not tainted (2.6.31-tip-02389-gc9f313c-dirty #151)
> [ 2.182006] EIP: 0060:[<815a8101>] EFLAGS: 00010282 CPU: 0
> [ 2.183009] EIP is at sock_init_data+0xe1/0x210
> [ 2.184006] EAX: 0001b000 EBX: bf855938 ECX: 8233b614 EDX: 819ac7bf
> [ 2.185006] ESI: bf855800 EDI: bf438280 EBP: bf867f10 ESP: 81b3afcc
> [ 2.186006] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
> [ 2.187006] CR0: 8005003b CR2: bf83bdf0 CR3: 01b2c000 CR4: 000006d0
> [ 2.188006] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> [ 2.189006] DR6: ffff4ff0 DR7: 00000400
> [ 2.190005] [<815d82b5>] __netlink_create+0x35/0xa0
> [ 2.192005] [<815dabaa>] netlink_kernel_create+0x5a/0x150
> [ 2.194004] [<815bc8ee>] rtnetlink_net_init+0x1e/0x40
> [ 2.196005] [<815af381>] register_pernet_operations+0x11/0x30
> [ 2.198004] [<815af4be>] register_pernet_subsys+0x1e/0x30
> [ 2.200004] [<81adb49c>] rtnetlink_init+0x4c/0x100
> [ 2.202004] [<81adbfe5>] netlink_proto_init+0x105/0x1b0
> [ 2.204004] [<81001127>] do_one_initcall+0x27/0x190
> [ 2.206004] [<81a9f567>] do_initcalls+0x27/0x40
> [ 2.208004] [<81a9f5a6>] do_basic_setup+0x26/0x30
> [ 2.210004] [<81a9f907>] kernel_init+0x57/0xa0
> [ 2.212004] [<81004867>] kernel_thread_helper+0x7/0x30
> [ 2.214004] [<ffffffff>] 0xffffffff
> [ 2.216021] calling bdi_class_init+0x0/0x30 @ 1
> [ 2.217015] device class 'bdi': registering
> [ 2.218702] initcall bdi_class_init+0x0/0x30 returned 0 after 976 usecs
> [ 2.219041] calling kobject_uevent_init+0x0/0x50 @ 1
>
> config attached.
>
> Ingo
>
I thought this was already discussed and fixed somehow ?
Either we add kmemcheck annotations, or we switch sock->type from short
to int to avoid the hole, and possibly to speedup things...
[PATCH] net: kmemcheck annotation in struct socket
struct socket has a 16 bit hole that triggers kmemcheck warnings.
As suggested by Ingo, use kmemcheck annotations
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/net.h | 5 +++++
net/socket.c | 1 +
2 files changed, 6 insertions(+)
diff --git a/include/linux/net.h b/include/linux/net.h
index 4fc2ffd..9040a10 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -57,6 +57,7 @@ typedef enum {
#include <linux/random.h>
#include <linux/wait.h>
#include <linux/fcntl.h> /* For O_CLOEXEC and O_NONBLOCK */
+#include <linux/kmemcheck.h>
struct poll_table_struct;
struct pipe_inode_info;
@@ -127,7 +128,11 @@ enum sock_shutdown_cmd {
*/
struct socket {
socket_state state;
+
+ kmemcheck_bitfield_begin(type);
short type;
+ kmemcheck_bitfield_end(type);
+
unsigned long flags;
/*
* Please keep fasync_list & wait fields in the same cache line
diff --git a/net/socket.c b/net/socket.c
index 6d47165..2a022c0 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -489,6 +489,7 @@ static struct socket *sock_alloc(void)
sock = SOCKET_I(inode);
+ kmemcheck_annotate_bitfield(sock, type);
inode->i_mode = S_IFSOCK | S_IRWXUGO;
inode->i_uid = current_fsuid();
inode->i_gid = current_fsgid();
^ permalink raw reply related
* Re: alloc skb based on a given data buffer
From: Zhu Yi @ 2009-09-15 8:57 UTC (permalink / raw)
To: David Miller
Cc: mel@csn.ul.ie, Chatre, Reinette, elendil@planet.nl,
Larry.Finger@lwfinger.net, linville@tuxdriver.com,
penberg@cs.helsinki.fi, linux-kernel@vger.kernel.org,
linux-wireless@vger.kernel.org,
ipw3945-devel@lists.sourceforge.net, akpm@linux-foundation.org,
cl@linux-foundation.org, Krauss, Assaf, johannes@sipsolutions.net,
Abbas, Mohamed, netdev@vger.kernel.org
In-Reply-To: <20090915.013321.07006714.davem@davemloft.net>
On Tue, 2009-09-15 at 16:33 +0800, David Miller wrote:
> From: Zhu Yi <yi.zhu@intel.com>
> Date: Tue, 15 Sep 2009 16:30:20 +0800
>
> > This way, device drivers can allocate the Rx buffers with their own size
> > and alignment requirement. i.e. do an order-1 page allocation directly
> > with free_pages() in the iwlagn driver for a 256 bytes aligned 8K Rx
> > buffer. After DMA is finished, drivers can use the above function to
> > assemble an skb based on the Rx buffer. It should resolve the problem
> > for requiring an order-2 allocation by alloc_skb() in the first place.
>
> You can create paged RX skbs just like drivers such as niu.c
> and others already do, there is no need for special APIs for
> this.
Thanks. So we can put the 8K buffer into 2 skb_shinfo()->frags[] slots
and set nr_frags to 2, right? Is this supported allover the network code
already? At a first glance, I didn't find any frags handling in mac80211
stack.
Thanks,
-yi
^ permalink raw reply
* Re: Fwd: [RFC v3] net: Introduce recvmmsg socket syscall
From: Nir Tzachar @ 2009-09-15 8:37 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Linux Networking Development Mailing List, Ziv Ayalon
In-Reply-To: <20090914230902.GC22743@ghostprotocols.net>
On Tue, Sep 15, 2009 at 2:09 AM, Arnaldo Carvalho de
Melo<acme@ghostprotocols.net> wrote:
> Em Thu, Aug 06, 2009 at 10:15:26AM +0300, Nir Tzachar escreveu:
>> Hello.
>>
>> Is there anything new with this patch? What are the plans for merging
>> it upstream?
>
> I'm doing perf runs using a test app using recvmsg, then with the first
> patch, that introduces recvmmsg, then with the second, that locks the
> series of unlocked_recvmmsg calls just once, will try to get this posted
> here soon.
>
> I'd really appreciate if the people interested in this could try it and
> post numbers too, to get this ball rolling again.
>
> As for getting it upstream, well, posting numbers here would definetely
> help with that :-)
Ok, here are some crude results:
Setup:
linux 2.6.29.2 with the third version of the patch, running on an
Intel Xeon X3220 2.4GHz quad core, with 4Gbyte of ram, running Ubuntu
9.04
Application:
A financial application, subscribing to quotes from a stock exchange.
Typical traffic is small (around 50-100 bytes) multicast packets in
large volumes. The application just receives the quotes and pass them
along.
The test:
Run two version of the application, head to head. one version using
recvmsg and the other recvmmsg. The data is passed to a third
application measuring the latency of the data.
Results:
On general, the recvmmsg beats the pants off the regular recvmsg by a
whole millisecond (which might not sound much, but is _really_ a lot
for us ;). The exact distribution fluctuates between half a milli and
2 millis, but the average is 1 milli.
Conclusions:
We would _really_ like to see this patch go upstream. It gives an
important performance boost in out use cases.
We are willing to perform more accurate tests if needed, and would
appreciate the feedback on how to conduct them.
Cheers,
Nir.
^ permalink raw reply
* Re: alloc skb based on a given data buffer
From: David Miller @ 2009-09-15 8:33 UTC (permalink / raw)
To: yi.zhu-ral2JQCrhuEAvxtiuMwx3w
Cc: mel-wPRd99KPJ+uzQB+pC5nmwQ,
reinette.chatre-ral2JQCrhuEAvxtiuMwx3w,
elendil-EIBgga6/0yRmR6Xm/wNWPw,
Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ,
linville-2XuSBdqkA4R54TAoqtyWWQ, penberg-bbCR+/B0CizivPeTLB3BmA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
ipw3945-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
cl-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
assaf.krauss-ral2JQCrhuEAvxtiuMwx3w,
johannes-cdvu00un1VgdHxzADdlk8Q,
mohamed.abbas-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1253003420.7549.51.camel@debian>
From: Zhu Yi <yi.zhu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date: Tue, 15 Sep 2009 16:30:20 +0800
> This way, device drivers can allocate the Rx buffers with their own size
> and alignment requirement. i.e. do an order-1 page allocation directly
> with free_pages() in the iwlagn driver for a 256 bytes aligned 8K Rx
> buffer. After DMA is finished, drivers can use the above function to
> assemble an skb based on the Rx buffer. It should resolve the problem
> for requiring an order-2 allocation by alloc_skb() in the first place.
You can create paged RX skbs just like drivers such as niu.c
and others already do, there is no need for special APIs for
this.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] can: fix NOHZ local_softirq_pending 08 warning
From: David Miller @ 2009-09-15 8:31 UTC (permalink / raw)
To: oliver; +Cc: stable, netdev, urs, mb
In-Reply-To: <4AAF3AF2.60309@hartkopp.net>
From: Oliver Hartkopp <oliver@hartkopp.net>
Date: Tue, 15 Sep 2009 08:57:54 +0200
> When using nanosleep() in an userspace application we get a ratelimit warning
>
> NOHZ: local_softirq_pending 08
>
> for 10 times.
>
> The echo of CAN frames is done from process context and softirq context only.
> Therefore the usage of netif_rx() was wrong (for years).
>
> This patch replaces netif_rx() with netif_rx_ni() which has to be used from
> process/softirq context. It also adds a missing comment that can_send() must
> no be used from hardirq context.
>
> Signed-off-by: Oliver Hartkopp <oliver@hartkopp.net>
> Signed-off-by: Urs Thuermann <urs@isnogud.escape.de>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 2nd try] tcp: fix ssthresh u16 leftover
From: David Miller @ 2009-09-15 8:30 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev
In-Reply-To: <alpine.DEB.2.00.0909151111250.7424@wel-95.cs.helsinki.fi>
From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Tue, 15 Sep 2009 11:19:11 +0300 (EEST)
> [PATCH 2nd try] tcp: fix ssthresh u16 leftover
>
> It was once upon time so that snd_sthresh was a 16-bit quantity.
> ...That has not been true for long period of time. I run across
> some ancient compares which still seem to trust such legacy.
> Put all that magic into a single place, I hopefully found all
> of them.
>
> Compile tested, though linking of allyesconfig is ridiculous
> nowadays it seems.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
Much better, applied, thanks!
^ permalink raw reply
* alloc skb based on a given data buffer
From: Zhu Yi @ 2009-09-15 8:30 UTC (permalink / raw)
To: Mel Gorman
Cc: Chatre, Reinette, Frans Pop, Larry Finger, John W. Linville,
Pekka Enberg, linux-kernel@vger.kernel.org,
linux-wireless@vger.kernel.org,
ipw3945-devel@lists.sourceforge.net, Andrew Morton,
cl@linux-foundation.org, Krauss, Assaf, Johannes Berg,
Abbas, Mohamed, netdev
In-Reply-To: <20090914130612.GA11778@csn.ul.ie>
[ Cc netdev and change the subject ]
On Mon, 2009-09-14 at 21:06 +0800, Mel Gorman wrote:
> > Essentially, the hardware only requires an order-1 allocation aligned on
> > 256 bytes boundary. But as it is used as an SKB, a trailing struct
> > skb_shared_info is added. This forces us to both increase the order and
> > do alignment ourselves. I believe some improvement could be done here.
> > But it should not be an easy one.
> >
>
> Probably not. I can only assume that moving the location of
> skb_shared_info such that it is sometimes located after the buffer and
> sometimes allocated via a separate kmalloc() would be a significant
> undertaking.
Shall I propose below function as a variant to alloc_skb()?
struct sk_buff *alloc_skb_attach_buff(void *data_buff,
unsigned int real_size,
unsigned int size, gfp_t mask);
If real_size >= size + sizeof(struct skb_shared_info), we can still put
the shinfo at the end of the buffer. Otherwise we can allocate from a
new slab that put sk_buff and shinfo together. Of course, kfree_skbmem()
needs to recognize it.
This way, device drivers can allocate the Rx buffers with their own size
and alignment requirement. i.e. do an order-1 page allocation directly
with free_pages() in the iwlagn driver for a 256 bytes aligned 8K Rx
buffer. After DMA is finished, drivers can use the above function to
assemble an skb based on the Rx buffer. It should resolve the problem
for requiring an order-2 allocation by alloc_skb() in the first place.
Comments are welcome.
Thanks,
-yi
^ permalink raw reply
* Re: ipv4 regression in 2.6.31 ?
From: Jarek Poplawski @ 2009-09-15 8:13 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Stephan von Krawczynski, Eric Dumazet, linux-kernel, davem,
Linux Netdev List
In-Reply-To: <20090914093128.4d709ff6@nehalam>
On 14-09-2009 18:31, Stephen Hemminger wrote:
> On Mon, 14 Sep 2009 17:55:05 +0200
> Stephan von Krawczynski <skraw@ithnet.com> wrote:
>
>> On Mon, 14 Sep 2009 15:57:03 +0200
>> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>>> Stephan von Krawczynski a A~(c)crit :
>>>> Hello all,
...
>>> rp_filter - INTEGER
>>> 0 - No source validation.
>>> 1 - Strict mode as defined in RFC3704 Strict Reverse Path
>>> Each incoming packet is tested against the FIB and if the interface
>>> is not the best reverse path the packet check will fail.
>>> By default failed packets are discarded.
>>> 2 - Loose mode as defined in RFC3704 Loose Reverse Path
>>> Each incoming packet's source address is also tested against the FIB
>>> and if the source address is not reachable via any interface
>>> the packet check will fail.
...
> RP filter did not work correctly in 2.6.30. The code added to to the loose
> mode caused a bug; the rp_filter value was being computed as:
> rp_filter = interface_value & all_value;
> So in order to get reverse path filter both would have to be set.
>
> In 2.6.31 this was change to:
> rp_filter = max(interface_value, all_value);
>
> This was the intended behaviour, if user asks all interfaces to have rp
> filtering turned on, then set /proc/sys/net/ipv4/conf/all/rp_filter = 1
> or to turn on just one interface, set it for just that interface.
Alas this max() formula handles also cases where both values are set
and it doesn't look very natural/"user friendly" to me. Especially
with something like this: all_value = 2; interface_value = 1
Why would anybody care to bother with interface_value in such a case?
"All" suggests "default" in this context, so I'd rather expect
something like:
rp_filter = interface_value ? : all_value;
which gives "the inteded behaviour" too, plus more...
We'd only need to add e.g.:
0 - Default ("all") validation. (No source validation if "all" is 0).
3 - No source validation on this interface.
Jarek P.
^ permalink raw reply
* [PATCH] can: fix NOHZ local_softirq_pending 08 warning
From: Oliver Hartkopp @ 2009-09-15 6:57 UTC (permalink / raw)
To: David Miller, stable; +Cc: Linux Netdev List, Urs Thuermann, Michael Buesch
[-- Attachment #1: Type: text/plain, Size: 561 bytes --]
When using nanosleep() in an userspace application we get a ratelimit warning
NOHZ: local_softirq_pending 08
for 10 times.
The echo of CAN frames is done from process context and softirq context only.
Therefore the usage of netif_rx() was wrong (for years).
This patch replaces netif_rx() with netif_rx_ni() which has to be used from
process/softirq context. It also adds a missing comment that can_send() must
no be used from hardirq context.
Signed-off-by: Oliver Hartkopp <oliver@hartkopp.net>
Signed-off-by: Urs Thuermann <urs@isnogud.escape.de>
---
[-- Attachment #2: can-NOHZ-local_softirq_pending-08.patch --]
[-- Type: text/x-patch, Size: 1126 bytes --]
diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
index 6971f6c..80ac563 100644
--- a/drivers/net/can/vcan.c
+++ b/drivers/net/can/vcan.c
@@ -80,7 +80,7 @@ static void vcan_rx(struct sk_buff *skb, struct net_device *dev)
skb->dev = dev;
skb->ip_summed = CHECKSUM_UNNECESSARY;
- netif_rx(skb);
+ netif_rx_ni(skb);
}
static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
diff --git a/net/can/af_can.c b/net/can/af_can.c
index ef1c43a..6068321 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -199,6 +199,8 @@ static int can_create(struct net *net, struct socket *sock, int protocol)
* @skb: pointer to socket buffer with CAN frame in data section
* @loop: loopback for listeners on local CAN sockets (recommended default!)
*
+ * Due to the loopback this routine must not be called from hardirq context.
+ *
* Return:
* 0 on success
* -ENETDOWN when the selected interface is down
@@ -278,7 +280,7 @@ int can_send(struct sk_buff *skb, int loop)
}
if (newskb)
- netif_rx(newskb);
+ netif_rx_ni(newskb);
/* update statistics */
can_stats.tx_frames++;
^ permalink raw reply related
* Re: UDP regression with packets rates < 10k per sec
From: Eric Dumazet @ 2009-09-15 5:29 UTC (permalink / raw)
To: Christoph Lameter; +Cc: netdev
In-Reply-To: <alpine.DEB.1.10.0909141708150.8051@V090114053VZO-1>
Christoph Lameter a écrit :
> Where are we on this? Definitely a regression?
??
I tried to reproduce your numbers and failed on my machines.
2.6.31 is actually faster than 2.6.22 on the bench you provided.
Must be specific to the hardware I guess ?
As text size presumably is bigger in 2.6.31, fetching code
in cpu caches to handle 10 packets per second is what we call
a cold path anyway.
If you want to make it a fast path, you want to make sure code its
always hot in cpu caches, and find a way to inject packets into
the kernel to make sure cpu keep the path hot.
^ permalink raw reply
* Re: [PATCH 1/5] First Patch on TFRC-SP. Copy base files from TFRC
From: Ian McDonald @ 2009-09-15 5:25 UTC (permalink / raw)
To: Ivo Calado; +Cc: Gerrit Renker, dccp, netdev
In-Reply-To: <cb00fa210909141738l60d02cd5p287d990326ce6c3c@mail.gmail.com>
2009/9/15 Ivo Calado <ivocalado@embedded.ufcg.edu.br>
>
> Hi Gerrit and all.
> Thank you for your fast reply. My comments follow below
>
> >
> > net/dccp/ccids/lib/tfrc_equation_sp.c
> > -------------------------------------
> > This is a prime candidate for removal. After editing out the whitespace
> > differences, I found that it is 100% identical with tfrc_equation.c.
> >
> > The result of this editing has been uploaded to
> > http://eden-feed.erg.abdn.ac.uk/tfrc_sp_receiver_01.patch
> >
> One future patch will need to modify this file, but now it's really an
> exact copy.
>
>
Basically the rule with a patch set is that all the patches should
make sense together.
It may well be that it makes sense to make a copy, but if you want to
do this then present it with the patch that then modifies it.
Ian
^ permalink raw reply
* Re: L2 switching in igb
From: Alexander Duyck @ 2009-09-15 2:52 UTC (permalink / raw)
To: Or Gerlitz
Cc: Alexander Duyck, Kirsher, Jeffrey T, Fischer, Anna,
netdev@vger.kernel.org, David Miller, Stephen Hemminger
In-Reply-To: <4AAE14CC.2000609@voltaire.com>
On Mon, Sep 14, 2009 at 3:02 AM, Or Gerlitz <ogerlitz@voltaire.com> wrote:
> To have VEPA support another bit has to be programmed... its the one that
> doesn't let the PF to forward a packet to a VF whose source mac matches the
> one in the packet (e.g multicast sender).
The bit I was referring to not setting would handle that. By
disabling the DTXSWC local loopback bit the PF will not send anything
to the VFs or visa versa.
> Yes, lets do that. I'd like to suggest that a "VF programmable from user
> space" context will contain a <mac, vlan-id, priority-bits, rate> tuple,
> such that in the absence of vlan tag, the VF driver will "sign" the packet
> (skb) with vlan-id and priority-bits assigned by the admin and the PF NIC
> will mandate that the VF originated traffic will not exceed the rate.
Well whenever I can get to it I will try to add that support. In the
meantime I believe there is a BOF session covering "Virtual Ethernet
switch enhancements and configuration" at the Linux Plumbers Conf that
will cover some of this so hopefully we can come up with a solid plan
on how to address this.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH 5/5] Updating documentation accordingly
From: Ivo Calado @ 2009-09-15 0:41 UTC (permalink / raw)
To: Gerrit Renker, dccp, netdev
In-Reply-To: <20090913185050.GA7475@gerrit.erg.abdn.ac.uk>
On Sun, Sep 13, 2009 at 15:50, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
> | Updating documentation accordingly
>
> +/* As defined at section 8.6.1. of RFC 4342 */
> #define TFRC_LOSS_INTERVALS_OPT_MAX_LENGTH 28
> +/* Specified on section 8.7. of CCID4 draft */
> #define TFRC_DROP_OPT_MAX_LENGTH 84
> Need to update the references to CCID4 draft with RFC 5622. See also 4/5.
>
> It would be more helpful to include the documentation for structs with
> the patches that use these structs.
>
Thanks, I'll fix this.
--
Ivo Augusto Andrade Rocha Calado
MSc. Candidate
Embedded Systems and Pervasive Computing Lab - http://embedded.ufcg.edu.br
Systems and Computing Department - http://www.dsc.ufcg.edu.br
Electrical Engineering and Informatics Center - http://www.ceei.ufcg.edu.br
Federal University of Campina Grande - http://www.ufcg.edu.br
PGP: 0x03422935
Quidquid latine dictum sit, altum viditur.
^ permalink raw reply
* Re: [PATCH 4/5] Adds options DROPPED PACKETS and LOSS INTERVALS to receiver
From: Ivo Calado @ 2009-09-15 0:40 UTC (permalink / raw)
To: Gerrit Renker, dccp, netdev
In-Reply-To: <20090913184128.GA7165@gerrit.erg.abdn.ac.uk>
The comments follow below
On Sun, Sep 13, 2009 at 15:41, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
> | Adds options DROPPED PACKETS and LOSS INTERVALS to receiver. In this patch is added the
> | mechanism of gathering information about loss intervals and storing it, for later
> | construction of these two options.
> This also needs some more work, please see inline.
>
<snip>
>
> + tfrc_sp_update_li_data(ld, h, skb, new_loss, new_event);
> +
> I don't understand why 'tfrc_sp_update_li_data' is called twice, one call seems
> to be redundant. What it seems to be wanting to do is
When we are designing the loss count algorithm it seemed to be
necessary, but now that i need to revise this (patch nº 2), I'll
observe this.
> bool new_loss = false;
>
> //...
<snip>
> at the begin of the function, all subsequent 'dccp_data_packet(skb)' are unnecessary.
> Almost every 'if' statement ends in 'return', this seems ad-hoc and could be reduced
> by adding if-else-if-else-if..., which would probably also reduce the code duplication.
>
I'll try to reduce code duplication, thanks.
>
> @@ -244,8 +463,11 @@
> tfrc_lh_slab = kmem_cache_create("tfrc_sp_li_hist",
> sizeof(struct tfrc_loss_interval), 0,
> SLAB_HWCACHE_ALIGN, NULL);
> + tfrc_ld_slab = kmem_cache_create("tfrc_sp_li_data",
> + sizeof(struct tfrc_loss_data_entry), 0,
> + SLAB_HWCACHE_ALIGN, NULL);
>
> - if ((tfrc_lh_slab != NULL))
> + if ((tfrc_lh_slab != NULL) || (tfrc_ld_slab != NULL))
> return 0;
>
> if (tfrc_lh_slab != NULL) {
> @@ -253,6 +475,11 @@
> tfrc_lh_slab = NULL;
> }
>
> + if (tfrc_ld_slab != NULL) {
> + kmem_cache_destroy(tfrc_ld_slab);
> + tfrc_ld_slab = NULL;
> + }
> +
> return -ENOBUFS;
> }
> The condition above should be '&&', not '||'. Suggested alternative:
>
> + if (tfrc_lh_slab == NULL)
> + goto lh_failed;
> +
> + tfrc_ld_slab = kmem_cache_create("tfrc_sp_li_data",
> + sizeof(struct tfrc_loss_data_entry), 0,
> + SLAB_HWCACHE_ALIGN, NULL);
> + if (tfrc_ld_slab != NULL)
> + return 0;
> +
> + kmem_cache_destroy(tfrc_lh_slab);
> + tfrc_lh_slab = NULL;
> +lh_failed:
> + return -ENOBUFS;
> }
>
Thanks for revising this. Adding one label for each failure case will
not scale well. In another patch it will be needed to create another
structure, and so, requiring another label.
>
>
>
> --- dccp_tree_work5.orig/net/dccp/ccids/lib/loss_interval_sp.h
> +++ dccp_tree_work5/net/dccp/ccids/lib/loss_interval_sp.h
> @@ -70,13 +70,52 @@
> struct tfrc_rx_hist;
> #endif
>
> +struct tfrc_loss_data_entry {
> + struct tfrc_loss_data_entry *next;
> + u32 lossless_length:24;
> + u8 ecn_nonce_sum:1;
> + u32 loss_length:24;
> + u32 data_length:24;
> + u32 drop_count:24;
> +};
> According to RFC 4342, 8.6.1, Loss Length is a 23-bit number, i.e.
> u32 loss_length:23;
>
Thanks!
>
>
>
> +#define TFRC_LOSS_INTERVALS_OPT_MAX_LENGTH 28
> +#define TFRC_DROP_OPT_MAX_LENGTH 84
> +#define TFRC_LI_OPT_SZ \
> + (2 + TFRC_LOSS_INTERVALS_OPT_MAX_LENGTH*9)
> +#define TFRC_DROPPED_OPT_SZ \
> + (1 + TFRC_DROP_OPT_MAX_LENGTH*3)
> It would be good to have a reminder where the numbers come from, i.e.
> * the "28" comes from RFC 4342, 8.6
> * the "84" comes from RFC 5622, 8.7
> * the "9" again is from RFC 4342, 8.6
> * the 1 + TFRC_DROP_OPT_MAX_LENGTH*3 = DCCP_SINGLE_OPT_MAXLEN (linux/dccp.h)
>
Sorry, this documentation was written, but i left it in another future patch.
>
> +struct tfrc_loss_data {
> + struct tfrc_loss_data_entry *head;
> + u16 counter;
> + u8 loss_intervals_opts[TFRC_LI_OPT_SZ];
> + u8 drop_opts[TFRC_DROPPED_OPT_SZ];
> + u8 last_loss_count;
> + u8 sto_ecn;
> + u8 sto_is_data;
> +};
>
>
>
> +static inline void tfrc_ld_init(struct tfrc_loss_data *ld)
> +{
> + memset(ld, 0, sizeof(struct tfrc_loss_data));
> +}
> A tip from CodingStyle - using "sizeof(*ld)" will continue to work if there
> are changes in the interface.
>
Thanks.
>
> --- dccp_tree_work5.orig/net/dccp/dccp.h
> +++ dccp_tree_work5/net/dccp/dccp.h
> @@ -403,6 +403,16 @@
> return (DCCP_SKB_CB(skb)->dccpd_ecn & INET_ECN_MASK) == INET_ECN_CE;
> }
>
> +static inline bool dccp_skb_is_ecn_ect0(const struct sk_buff *skb)
> +{
> + return (DCCP_SKB_CB(skb)->dccpd_ecn & INET_ECN_MASK) == INET_ECN_ECT_0;
> +}
> +
> +static inline bool dccp_skb_is_ecn_ect1(const struct sk_buff *skb)
> +{
> + return (DCCP_SKB_CB(skb)->dccpd_ecn & INET_ECN_MASK) == INET_ECN_ECT_0;
> +}
> The routines are not needed, because the dccpd_ecn field is (deliberately) only
> 2 bits wide. In the second case it should have been INET_ECN_ECT_1.
>
And how would be to determine if one packet's ecn is set to ECT 0 or ECT 1?
--
Ivo Augusto Andrade Rocha Calado
MSc. Candidate
Embedded Systems and Pervasive Computing Lab - http://embedded.ufcg.edu.br
Systems and Computing Department - http://www.dsc.ufcg.edu.br
Electrical Engineering and Informatics Center - http://www.ceei.ufcg.edu.br
Federal University of Campina Grande - http://www.ufcg.edu.br
PGP: 0x03422935
Quidquid latine dictum sit, altum viditur.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox