* Re: [BUG] Bug in netprio_cgroup and netcls_cgroup ?
From: Daniel Wagner @ 2013-01-22 10:09 UTC (permalink / raw)
To: John Fastabend
Cc: Li Zefan, John Fastabend, Neil Horman, Daniel Wagner, LKML,
netdev-u79uwXL29TY76Z2rM5mHXA, Cgroups
In-Reply-To: <50FD786E.4050108-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, Jan 21, 2013 at 09:18:38AM -0800, John Fastabend wrote:
> On 01/21/2013 01:57 AM, Li Zefan wrote:
> > On 2013/1/21 17:27, Daniel Wagner wrote:
> >> On 21.01.2013 10:01, Li Zefan wrote:
> >>> On 2013/1/21 16:50, Daniel Wagner wrote:
> >>>> Hi Li,
> >>>>
> >>>> On 21.01.2013 07:08, Li Zefan wrote:
> >>>>> I'm not a network developer, so correct me if I'm wrong.
> >>>>>
> >>>>> Since commit 7955490f732c2b8
> >>>>> ("net: netprio_cgroup: rework update socket logic"), sock->sk->sk_cgrp_prioidx
> >>>>> is set when the socket is created, and won't be updated unless the task is
> >>>>> moved to another cgroup.
> >>>>>
> >>>>> Now the problem is, a socket can be _shared_ by multiple processes (fork, SCM_RIGHT).
> >>>>> If we place those processes in different cgroups, and each cgroup has
> >>>>> different configs, but all of the processes will send data via this socket
> >>>>> with the same network priority.
> >>>>
> >>>> Wouldn't that be addressed by 48a87cc26c13b68f6cce4e9d769fcb17a6b3e4b8
> >>>>
> >>>> net: netprio: fd passed in SCM_RIGHTS datagram not set correctly
> >>>>
> >>>> A socket fd passed in a SCM_RIGHTS datagram was not getting
> >>>> updated with the new tasks cgrp prioidx. This leaves IO on
> >>>> the socket tagged with the old tasks priority.
> >>>>
> >>>> To fix this add a check in the scm recvmsg path to update the
> >>>> sock cgrp prioidx with the new tasks value.
> >>>>
> >>>> As I read this this should work for net_prio.
> >>>>
> >>>
> >>> But after process A passed the socket fd to B, both A and B can use the
> >>> same socket to send data, right? Then if A and B were placed in different
> >>> cgroups with differnt configs, A's config won't take effect anymore.
> >>>
> >>> Am I missing something?
> >>
> >> I don't know. I guess at one point the socket resources are shared and then
> >> one configuration is taking preference. As you can see I am far away of
> >> being
> >> an expert in this field. Hopefully someone who understands this bits
> >> can chip in.
> >>
> >> BTW, isn't this a similar to what should happen with the block io cgroup?
> >> What is the behavior with a fd writing to a file in the scenario you
> >> describe above?
> >>
> >
> > It forbids task moving in this case:
> >
> > /*
> > * We cannot support shared io contexts, as we have no mean to support
> > * two tasks with the same ioc in two different groups without major rework
> > * of the main cic data structures. For now we allow a task to change
> > * its cgroup only if it's the only owner of its ioc.
> > */
> > static int blkcg_can_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
> >
>
> OK, I guess we should do something similar in the netprio, netcls
> cgroups and
> yes document it as you noted in your last comment.
Here is my attempt to add such a check. I really don't know if this is the
correct way to do so. To test this I have written a test program, which
seems to test the right thing. Please have a look and let me know if
it is correct: http://www.monom.org/misc/scm_rights.c
And here a dirty first version of the patch:
>From 49a78d907eaf31c16673025e7e3b4844e419e416 Mon Sep 17 00:00:00 2001
From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
Date: Tue, 22 Jan 2013 11:08:22 +0100
Subject: [PATCH] net: net_prio: Block attach if a socket is shared
---
net/core/netprio_cgroup.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index 847c02b..de4e6c5 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -274,9 +274,39 @@ static struct cftype ss_files[] = {
{ } /* terminate */
};
+static int check_cnt(const void *v, struct file *file, unsigned n)
+{
+ unsigned *flag = (unsigned *)v;
+ int err;
+
+ struct socket *sock = sock_from_file(file, &err);
+ if (sock && file_count(file) > 1)
+ *flag = 1;
+
+ return 0;
+}
+
+static int cgrp_can_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
+{
+ struct task_struct *p;
+ unsigned flag = 0;
+
+ cgroup_taskset_for_each(p, cgrp, tset) {
+ task_lock(p);
+ iterate_fd(p->files, 0, check_cnt, &flag);
+ task_unlock(p);
+ }
+
+ if (flag != 0)
+ return -EINVAL;
+
+ return 0;
+}
+
struct cgroup_subsys net_prio_subsys = {
.name = "net_prio",
.create = cgrp_create,
+ .can_attach = cgrp_can_attach,
.destroy = cgrp_destroy,
.attach = net_prio_attach,
.subsys_id = net_prio_subsys_id,
--
1.8.0.rc0
^ permalink raw reply related
* BUG in netxen_release_tx_buffers when TSO enabled on kernels >= 3.3 and <= 3.6
From: Christoph Paasch @ 2013-01-22 10:15 UTC (permalink / raw)
To: Ian Campbell, Eric Dumazet, Sony Chacko, Rajesh Borundia,
David Miller, netdev
Hello,
I have a scenario where I can trigger a bug on kernels >= 3.3 and <= 3.6.
Thus, I can produce it with the latest longterm-stable v3.4.26.
The crashdumps/warning can be seen below. Sometimes it is only the warning,
sometimes it also produces the crash. But, it happens each time I try out my
scenario.
How to reproduce the bug (I have HP Proliant DL165 machines with HP NC375T 1Gb
interface):
* Launch an iperf-session ( -t 10 ) to a server over a 1Gbps interface.
* After 5 seconds on the client, remove the IP-address from the interface
with ip addr del dev [itf] [ip]
* Wait 10 more seconds and kill the iperf on the client and the server.
* Then do: ifconfig down [itf]
Now the crash happens.
What I observe in netxen_release_tx_buffers is that upon the 18th iteration (j
== 17), buffrag->length == 0. buffrag->frag_count is 18.
Sometimes (much more rare), buffrag->length rather looks like garbage (e.g., >
2^32)
I bisected this, and it was introduced by commit 9d4dde521577 (net: only use a
single page of slop in MAX_SKB_FRAGS).
It was fixed by Eric in commit 5640f7685831 (net: use a per task frag
allocator) since kernel > 3.6.
As this bug is present in the longterm-stable 3.4, should Eric's patch be
backported?
If not, does somebody (with more knowledge than I have of this part of the
code) can have a look at it, or maybe give me a pointer on how I could solve
this properly?
Reverting commit 9d4dde521577 (net: only use a single page of slop in
MAX_SKB_FRAGS) fixes it for me on 3.4.26.
Thanks,
Christoph
[ 610.315966] ------------[ cut here ]------------
[ 610.371099] WARNING: at /home/cpaasch/builder/net-next/lib/dma-debug.c:865
check_unmap+0x18e/0x61e()
[ 610.480197] Hardware name: ProLiant DL165 G7
[ 610.531168] netxen_nic 0000:05:00.2: DMA-API: device driver tries to free
DMA memory it has not allocated [device address=0x0000000000000012] [size=0
bytes]
[ 610.698391] Modules linked in:
[ 610.734935] Pid: 3728, comm: ip Not tainted 3.4.26-mptcp #30
[ 610.802511] Call Trace:
[ 610.831692] [<ffffffff81025af7>] warn_slowpath_common+0x80/0x98
[ 610.903424] [<ffffffff81025ba3>] warn_slowpath_fmt+0x41/0x43
[ 610.972041] [<ffffffff811c93d1>] check_unmap+0x18e/0x61e
[ 611.036505] [<ffffffff811c99aa>] debug_dma_unmap_page+0x50/0x52
[ 611.108236] [<ffffffff813642f3>] netxen_release_tx_buffers+0x11e/0x175
[ 611.187232] [<ffffffff813621a6>] __netxen_nic_down+0x12c/0x13f
[ 611.257922] [<ffffffff81362290>] netxen_nic_close+0x13/0x17
[ 611.325502] [<ffffffff813fc7dc>] __dev_close_many+0x90/0xbc
[ 611.393079] [<ffffffff813fc839>] __dev_close+0x31/0x42
[ 611.455469] [<ffffffff813fa38d>] __dev_change_flags+0xb9/0x13d
[ 611.526160] [<ffffffff813fd16e>] dev_change_flags+0x1c/0x52
[ 611.593740] [<ffffffff814075ad>] do_setlink+0x2c0/0x7d2
[ 611.657167] [<ffffffff8146fcb5>] ? inet6_fill_ifla6_attrs+0x205/0x219
[ 611.735124] [<ffffffff814085a3>] rtnl_newlink+0x26b/0x4a1
[ 611.800626] [<ffffffff81408400>] ? rtnl_newlink+0xc8/0x4a1
[ 611.867166] [<ffffffff81419ebb>] ? netlink_sendmsg+0x22b/0x2b2
[ 611.937859] [<ffffffff8109fcf3>] ? check_object+0x13b/0x1df
[ 612.005437] [<ffffffff8140831d>] rtnetlink_rcv_msg+0x22c/0x247
[ 612.076128] [<ffffffff814080f1>] ? rtnetlink_rcv+0x28/0x28
[ 612.142668] [<ffffffff81419c3f>] netlink_rcv_skb+0x3e/0x8f
[ 612.209209] [<ffffffff814080ea>] rtnetlink_rcv+0x21/0x28
[ 612.273673] [<ffffffff81419a01>] netlink_unicast+0x134/0x1ab
[ 612.342289] [<ffffffff81419eda>] netlink_sendmsg+0x24a/0x2b2
[ 612.410908] [<ffffffff813ebb1a>] sock_sendmsg+0xb8/0xd1
[ 612.474332] [<ffffffff81073a29>] ? filemap_fault+0x199/0x35d
[ 612.542948] [<ffffffff810732ab>] ? unlock_page+0x2d/0x32
[ 612.607412] [<ffffffff810894c2>] ? __do_fault+0x3ce/0x409
[ 612.672916] [<ffffffff813eb239>] ? move_addr_to_kernel+0x3a/0x51
[ 612.745685] [<ffffffff813f4f76>] ? verify_iovec+0x59/0xaf
[ 612.811187] [<ffffffff813ec675>] __sys_sendmsg+0x1b9/0x23e
[ 612.877725] [<ffffffff8108c2ab>] ? handle_mm_fault+0x1b7/0x1cd
[ 612.948420] [<ffffffff8101f08e>] ? do_page_fault+0x336/0x375
[ 613.017035] [<ffffffff81090681>] ? do_brk+0x2e4/0x346
[ 613.078385] [<ffffffff813ec80d>] sys_sendmsg+0x3d/0x5e
[ 613.140775] [<ffffffff814e62a2>] system_call_fastpath+0x16/0x1b
[ 613.212502] ---[ end trace d4f0eb8a4ca35e8a ]---
[ 719.276359] BUG: unable to handle kernel paging request at ffffc900103cd000
[ 719.359733] IP: [<ffffffff81364295>] netxen_release_tx_buffers+0xc0/0x175
[ 719.440926] PGD 42d851067 PUD 42d852067 PMD 42c11b067 PTE 0
[ 719.507897] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 719.562307] CPU 7
[ 719.584214] Modules linked in:
[ 719.622944]
[ 719.640700] Pid: 4563, comm: ip Tainted: G W 3.4.26-mptcp #30 HP
ProLiant DL165 G7
[ 719.741707] RIP: 0010:[<ffffffff81364295>] [<ffffffff81364295>]
netxen_release_tx_buffers+0xc0/0x175
[ 719.851952] RSP: 0018:ffff880422c79668 EFLAGS: 00010246
[ 719.915380] RAX: ffff88042dbce360 RBX: ffffc900103cced0 RCX:
0000000000000000
[ 720.000603] RDX: 0000000000000011 RSI: 0000000000000282 RDI:
0000000000000000
[ 720.085826] RBP: ffff880422c796b8 R08: 0000000000000000 R09:
ffff88042dbce3e8
[ 720.171049] R10: 0000000000000001 R11: 0000000000000206 R12:
ffffc900103ccff8
[ 720.256273] R13: ffff88042d9aa720 R14: 0000000000000012 R15:
0000000000000000
[ 720.341499] FS: 00007f191d157700(0000) GS:ffff88043fdc0000(0000)
knlGS:0000000000000000
[ 720.438138] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 720.506755] CR2: ffffc900103cd000 CR3: 0000000422cbd000 CR4:
00000000000007e0
[ 720.591979] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 720.677203] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
0000000000000400
[ 720.762427] Process ip (pid: 4563, threadinfo ffff880422c78000, task
ffff88042d9b4b90)
[ 720.856990] Stack:
[ 720.880968] ffff880422c796b8 0000000000000000 ffff88042bc53b28
00000011000003ff
[ 720.969736] ffff880422c796b8 ffff88042d9aa720 ffff88042bc6c920
0000000000000001
[ 721.058506] ffff88042d9aa0a0 0000000000000007 ffff880422c796f8
ffffffff813621a6
[ 721.147273] Call Trace:
[ 721.176450] [<ffffffff813621a6>] __netxen_nic_down+0x12c/0x13f
[ 721.247140] [<ffffffff81362290>] netxen_nic_close+0x13/0x17
[ 721.314721] [<ffffffff813fc7dc>] __dev_close_many+0x90/0xbc
[ 721.382295] [<ffffffff813fc839>] __dev_close+0x31/0x42
[ 721.444687] [<ffffffff813fa38d>] __dev_change_flags+0xb9/0x13d
[ 721.515378] [<ffffffff813fd16e>] dev_change_flags+0x1c/0x52
[ 721.582958] [<ffffffff814075ad>] do_setlink+0x2c0/0x7d2
[ 721.646384] [<ffffffff8146fcb5>] ? inet6_fill_ifla6_attrs+0x205/0x219
[ 721.724342] [<ffffffff814085a3>] rtnl_newlink+0x26b/0x4a1
[ 721.789842] [<ffffffff81408400>] ? rtnl_newlink+0xc8/0x4a1
[ 721.856384] [<ffffffff81419ebb>] ? netlink_sendmsg+0x22b/0x2b2
[ 721.927079] [<ffffffff8109fcf3>] ? check_object+0x13b/0x1df
[ 721.994659] [<ffffffff8140831d>] rtnetlink_rcv_msg+0x22c/0x247
[ 722.065351] [<ffffffff814080f1>] ? rtnetlink_rcv+0x28/0x28
[ 722.131893] [<ffffffff81419c3f>] netlink_rcv_skb+0x3e/0x8f
[ 722.198433] [<ffffffff814080ea>] rtnetlink_rcv+0x21/0x28
[ 722.262897] [<ffffffff81419a01>] netlink_unicast+0x134/0x1ab
[ 722.331515] [<ffffffff81419eda>] netlink_sendmsg+0x24a/0x2b2
[ 722.400131] [<ffffffff813ebb1a>] sock_sendmsg+0xb8/0xd1
[ 722.463559] [<ffffffff81073a29>] ? filemap_fault+0x199/0x35d
[ 722.532171] [<ffffffff810732ab>] ? unlock_page+0x2d/0x32
[ 722.596636] [<ffffffff810894c2>] ? __do_fault+0x3ce/0x409
[ 722.662140] [<ffffffff813eb239>] ? move_addr_to_kernel+0x3a/0x51
[ 722.734909] [<ffffffff813f4f76>] ? verify_iovec+0x59/0xaf
[ 722.800410] [<ffffffff813ec675>] __sys_sendmsg+0x1b9/0x23e
[ 722.866951] [<ffffffff8108c2ab>] ? handle_mm_fault+0x1b7/0x1cd
[ 722.937643] [<ffffffff8101f08e>] ? do_page_fault+0x336/0x375
[ 723.006260] [<ffffffff81090681>] ? do_brk+0x2e4/0x346
[ 723.067610] [<ffffffff813ec80d>] sys_sendmsg+0x3d/0x5e
[ 723.129998] [<ffffffff814e62a2>] system_call_fastpath+0x16/0x1b
[ 723.201724] Code: e6 ff 48 c7 43 08 00 00 00 00 4c 8d 63 08 c7 45 cc 00 00
00 00 eb 7d 49 83 c4 10 4d 8b 34 24 4d 85 f6 74 6d 49 8b 45 58 45 31 ff <4d>
8b 4c 24 08 48 85 c0 74 13 4c 8d b8 88 00 00 00 48 8b 80 b8
[ 723.434030] RIP [<ffffffff81364295>] netxen_release_tx_buffers+0xc0/0x175
[ 723.516249] RSP <ffff880422c79668>
[ 723.557878] CR2: ffffc900103cd000
[ 723.597832] ---[ end trace d4f0eb8a4ca35e8b ]---
--
IP Networking Lab --- http://inl.info.ucl.ac.be
MultiPath TCP in the Linux Kernel --- http://mptcp.info.ucl.ac.be
UCLouvain
--
^ permalink raw reply
* [PATCH net-next] ipmr: fix sparse warning when testing origin or group
From: Nicolas Dichtel @ 2013-01-22 10:18 UTC (permalink / raw)
To: fengguang.wu; +Cc: netdev, davem, Nicolas Dichtel
In-Reply-To: <50fe2be1.tv57lZmJcPMuyVta%fengguang.wu@intel.com>
mfc_mcastgrp and mfc_origin are __be32, thus we need to convert INADDR_ANY.
Because INADDR_ANY is 0, this patch just fix sparse warnings.
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
net/ipv4/ipmr.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 4b5e226..7085b9b 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -832,12 +832,12 @@ static struct mfc_cache *ipmr_cache_find(struct mr_table *mrt,
static struct mfc_cache *ipmr_cache_find_any_parent(struct mr_table *mrt,
int vifi)
{
- int line = MFC_HASH(INADDR_ANY, INADDR_ANY);
+ int line = MFC_HASH(htonl(INADDR_ANY), htonl(INADDR_ANY));
struct mfc_cache *c;
list_for_each_entry_rcu(c, &mrt->mfc_cache_array[line], list)
- if (c->mfc_origin == INADDR_ANY &&
- c->mfc_mcastgrp == INADDR_ANY &&
+ if (c->mfc_origin == htonl(INADDR_ANY) &&
+ c->mfc_mcastgrp == htonl(INADDR_ANY) &&
c->mfc_un.res.ttls[vifi] < 255)
return c;
@@ -848,14 +848,14 @@ static struct mfc_cache *ipmr_cache_find_any_parent(struct mr_table *mrt,
static struct mfc_cache *ipmr_cache_find_any(struct mr_table *mrt,
__be32 mcastgrp, int vifi)
{
- int line = MFC_HASH(mcastgrp, INADDR_ANY);
+ int line = MFC_HASH(mcastgrp, htonl(INADDR_ANY));
struct mfc_cache *c, *proxy;
- if (mcastgrp == INADDR_ANY)
+ if (mcastgrp == htonl(INADDR_ANY))
goto skip;
list_for_each_entry_rcu(c, &mrt->mfc_cache_array[line], list)
- if (c->mfc_origin == INADDR_ANY &&
+ if (c->mfc_origin == htonl(INADDR_ANY) &&
c->mfc_mcastgrp == mcastgrp) {
if (c->mfc_un.res.ttls[vifi] < 255)
return c;
@@ -1148,7 +1148,7 @@ static int ipmr_mfc_add(struct net *net, struct mr_table *mrt,
return 0;
}
- if (mfc->mfcc_mcastgrp.s_addr != INADDR_ANY &&
+ if (mfc->mfcc_mcastgrp.s_addr != htonl(INADDR_ANY) &&
!ipv4_is_multicast(mfc->mfcc_mcastgrp.s_addr))
return -EINVAL;
@@ -1807,7 +1807,7 @@ static int ip_mr_forward(struct net *net, struct mr_table *mrt,
cache->mfc_un.res.pkt++;
cache->mfc_un.res.bytes += skb->len;
- if (cache->mfc_origin == INADDR_ANY && true_vifi >= 0) {
+ if (cache->mfc_origin == htonl(INADDR_ANY) && true_vifi >= 0) {
struct mfc_cache *cache_proxy;
/* For an (*,G) entry, we only check that the incomming
@@ -1863,8 +1863,8 @@ forward:
/*
* Forward the frame
*/
- if (cache->mfc_origin == INADDR_ANY &&
- cache->mfc_mcastgrp == INADDR_ANY) {
+ if (cache->mfc_origin == htonl(INADDR_ANY) &&
+ cache->mfc_mcastgrp == htonl(INADDR_ANY)) {
if (true_vifi >= 0 &&
true_vifi != cache->mfc_parent &&
ip_hdr(skb)->ttl >
@@ -1881,7 +1881,8 @@ forward:
for (ct = cache->mfc_un.res.maxvif - 1;
ct >= cache->mfc_un.res.minvif; ct--) {
/* For (*,G) entry, don't forward to the incoming interface */
- if ((cache->mfc_origin != INADDR_ANY || ct != true_vifi) &&
+ if ((cache->mfc_origin != htonl(INADDR_ANY) ||
+ ct != true_vifi) &&
ip_hdr(skb)->ttl > cache->mfc_un.res.ttls[ct]) {
if (psend != -1) {
struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
--
1.8.0.1
^ permalink raw reply related
* Re: [PATCH net 3/3] net: cdc_ncm: fix error path for single interface probing
From: Bjørn Mork @ 2013-01-22 10:32 UTC (permalink / raw)
To: Oliver Neukum; +Cc: netdev, linux-usb, Greg Suarez, Alexey Orishko
In-Reply-To: <2830395.rGXyQGCbQc@linux-5eaq.site>
Oliver Neukum <oneukum@suse.de> writes:
> On Monday 21 January 2013 16:50:40 Bjørn Mork wrote:
>> commit bbc8d92 (net: cdc_ncm: add Huawei devices) implemented
>> support for devices with a single combined control and data
>> interface. Fix up the error path so that we do not double
>> release such interfaces in case of probing failures.
>
> Hi Bjørn,
>
> this should go also into stable. Could you submit?
Yes, it should. But AFAIK David prefers to do the stable submissions
for net, including drivers/net/usb.
Bjørn
^ permalink raw reply
* Re: [PATCH net] net: cdc_ncm: workaround for missing CDC Union
From: Bjørn Mork @ 2013-01-22 10:35 UTC (permalink / raw)
To: Oliver Neukum
Cc: Alexey Orishko, netdev, linux-usb, Greg Suarez, Alexey Orishko
In-Reply-To: <6132136.Whp52cipJH@linux-5eaq.site>
Oliver Neukum <oneukum@suse.de> writes:
> On Monday 21 January 2013 16:28:48 Bjørn Mork wrote:
>
>> Agreed. But I believe the condition should be
>>
>> if (!(info->flags & FLAG_SEND_ZLP) && !(info->flags & FLAG_MULTI_PACKET)) {
>> ..
>> } else {
>> urb->transfer_flags |= URB_ZERO_PACKET;
>> }
>>
>> to ensure that we send the ZLP in this case.
>
> Why? If a driver wants ZLP, it can set FLAG_SEND_ZLP. Your proposed change
> would take away an option from drivers without any gain.
OK.
Bjørn
^ permalink raw reply
* Re: [PATCH v3] net, wireless: overwrite default_ethtool_ops
From: Stanislaw Gruszka @ 2013-01-22 10:56 UTC (permalink / raw)
To: David Miller
Cc: mcgrof-3uybbJdB1yH774rrrx3eTA, bhutchings-s/n/eUQHGBpZroRs9YW3xA,
netdev-u79uwXL29TY76Z2rM5mHXA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
greearb-my8/4N5VtI7c+919tysfdA, bjorn-yOkvZcmFvRU,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
mirqus-Re5JQEeQqe8AvxtiuMwx3w, johannes-cdvu00un1VgdHxzADdlk8Q
In-Reply-To: <20130121.160404.1007504714461228602.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Mon, Jan 21, 2013 at 04:04:04PM -0500, David Miller wrote:
>
> It's queued up for -stable already as is clearly seen at:
>
> http://patchwork.ozlabs.org/user/bundle/2566/?state=*
Hmm, this link does not work for me (need user/password).
Anyway, I marked cc -stable in v2 patch, but forgot that in v3.
Committed patch does not include the mark, so I'll post patch
to stable just in case.
Thanks
Stanislaw
--
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: [RFC] State resolution packet queue for IPsec
From: Steffen Klassert @ 2013-01-22 11:10 UTC (permalink / raw)
To: David Miller; +Cc: herbert, netdev
In-Reply-To: <20130121.161716.752461350033731834.davem@davemloft.net>
On Mon, Jan 21, 2013 at 04:17:16PM -0500, David Miller wrote:
> From: Steffen Klassert <steffen.klassert@secunet.com>
> Date: Mon, 21 Jan 2013 10:48:48 +0100
>
> > I did a implementation of a state resolution packet queue for IPsec.
> > The original idea is described in git commit 14e50e57a
> > ([XFRM]: Allow packet drops during larval state resolution.)
> >
> > Since we don't have any notifiers for xfrm state resolution, I used
> > a timer that does a relookup in given intervals. This is mainly to speed
> > up the start of tcp connections, so I tried to do the relookup in a higher
> > rate that tcp would resend the syn packet. However, the relookup intervals,
> > the queue size and the timeout value are choosen by gut instincts, so
> > could be suboptimal. If anyone has some insight on good choices of these
> > values, please let me know. All other comment are welcome too, of course.
>
> This is probably the best way to solve this problem without adding
> xfrm_state resolution notifications.
>
> But really why would notifications be so bad even if they would not
> be fine grained?
>
> Any time a state is resolved, any state, you run what you have put
> currently into this new timer function.
>
When we use state resolution notifiers, we have two problems.
First, we would probably not dequeue the packets on the same
cpu we enqueued them, this can introduce packet reorder. The
timer will at least try to run on the local cpu.
Second, as you already mentioned, even with state resolution
notifiers, we don't know if the inserted state is the one we
need and if our state bundle is complete with this state.
We would need to do a lookup whenever a state is inserted,
so we hog the cpu that tries to insert the states we need.
With the timer, we don't have these two problems for the price
that we have take a good choice on the relookup intervals.
Not sure what's better here.
^ permalink raw reply
* Re: 69b08f62 breaks ethernet on atl1c based Eee PC 1011px
From: Michael Büsch @ 2013-01-22 12:31 UTC (permalink / raw)
To: Eric Dumazet
Cc: linux-netdev, Alexander Duyck, Benjamin LaHaise, David S. Miller,
Jay Cliburn, Chris Snook
In-Reply-To: <CANn89iK6kuLeg1TCobdHMKK5dYZuP0mRSb25v9C4pdP2F7MrLA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 316 bytes --]
On Mon, 21 Jan 2013 15:25:02 -0800
Eric Dumazet <edumazet@google.com> wrote:
> I guess you should reproduce the bug using current Linus tree.
The problem seems to be fixed in latest Linus tree.
Any idea which commit fixed it, so we can backport it to 3.7.y?
--
Greetings, Michael.
PGP: 908D8B0E
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Need A Personal Assistant
From: Paul @ 2013-01-22 12:33 UTC (permalink / raw)
To: netdev
We are seeking an upbeat, energetic and self-motivated individual to fill a part-time administrative assistant position.
Successful candidate will be highly organized and possess exceptional people skills, excellent written and verbal skills,be good with numbers and have the ability to multi-task in a fast-paced environment.
Computer proficiency required: MS Office Excel and Word. Must be able to work weekdays Mon-fri. 10:am-3:00pm
Candidates should be able to start ASAP,
Please submit resume with cover letter
if interested for more details
Smith Arrison
^ permalink raw reply
* Re: [PATCH 20/33] net: Convert to devm_ioremap_resource()
From: Arnd Bergmann @ 2013-01-22 13:03 UTC (permalink / raw)
To: Thierry Reding
Cc: David Miller, linux-kernel, gregkh, dmitry.torokhov, w.sang,
netdev
In-Reply-To: <20130122065629.GA14523@avionic-0098.adnet.avionic-design.de>
On Tuesday 22 January 2013, Thierry Reding wrote:
> I planned to do so initially, but that yielded a Cc list of 156 people
> and mailing lists, which I thought wasn't going to go down so well
> either. In general I like Cc'ing everyone concerned on all patches of
> the series, specifically for reasons of context. Some people have been
> annoyed when I did so. Still, for small series where only a few dozen
> people are concerned that seems to me to be the best way. But 156 email
> addresses is a different story.
>
> Either you add to many people or you don't add enough. Where do we draw
> the line?
I've had the same problem a couple of times. The best compromise seems
to be to Cc only the top-level subsystem maintainers and mailing lists
on the first email.
Arnd
^ permalink raw reply
* Re: [PATCH 20/33] net: Convert to devm_ioremap_resource()
From: Thierry Reding @ 2013-01-22 13:09 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David Miller, linux-kernel, gregkh, dmitry.torokhov, w.sang,
netdev
In-Reply-To: <201301221303.06785.arnd@arndb.de>
[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]
On Tue, Jan 22, 2013 at 01:03:06PM +0000, Arnd Bergmann wrote:
> On Tuesday 22 January 2013, Thierry Reding wrote:
> > I planned to do so initially, but that yielded a Cc list of 156 people
> > and mailing lists, which I thought wasn't going to go down so well
> > either. In general I like Cc'ing everyone concerned on all patches of
> > the series, specifically for reasons of context. Some people have been
> > annoyed when I did so. Still, for small series where only a few dozen
> > people are concerned that seems to me to be the best way. But 156 email
> > addresses is a different story.
> >
> > Either you add to many people or you don't add enough. Where do we draw
> > the line?
>
> I've had the same problem a couple of times. The best compromise seems
> to be to Cc only the top-level subsystem maintainers and mailing lists
> on the first email.
Even that would have been about 50 addresses IIRC. But perhaps that's
still the best compromise to avoid any confusion.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 20/33] net: Convert to devm_ioremap_resource()
From: Arnd Bergmann @ 2013-01-22 13:17 UTC (permalink / raw)
To: Thierry Reding
Cc: David Miller, linux-kernel, gregkh, dmitry.torokhov, w.sang,
netdev
In-Reply-To: <20130122130957.GA18165@avionic-0098.adnet.avionic-design.de>
On Tuesday 22 January 2013, Thierry Reding wrote:
> > I've had the same problem a couple of times. The best compromise seems
> > to be to Cc only the top-level subsystem maintainers and mailing lists
> > on the first email.
>
> Even that would have been about 50 addresses IIRC. But perhaps that's
> still the best compromise to avoid any confusion.
Be careful though that the Cc list must not exceed 1024 characters or
it will get rejected by some of the mailing list servers.
Arnd
^ permalink raw reply
* Need A Personal Assistant
From: Paul @ 2013-01-22 13:15 UTC (permalink / raw)
To: netdev
We are seeking an upbeat, energetic and self-motivated individual to fill a part-time administrative assistant position.
Successful candidate will be highly organized and possess exceptional people skills, excellent written and verbal skills,be good with numbers and have the ability to multi-task in a fast-paced environment.
Computer proficiency required: MS Office Excel and Word. Must be able to work weekdays Mon-fri. 10:am-3:00pm
Candidates should be able to start ASAP,
Please submit resume with cover letter
if interested for more details
Smith Arrison
^ permalink raw reply
* Re: BUG in netxen_release_tx_buffers when TSO enabled on kernels >= 3.3 and <= 3.6
From: Eric Dumazet @ 2013-01-22 13:32 UTC (permalink / raw)
To: christoph.paasch
Cc: Ian Campbell, Sony Chacko, Rajesh Borundia, David Miller, netdev
In-Reply-To: <1659861.dHpGAGoXHj@cpaasch-mac>
On Tue, 2013-01-22 at 11:15 +0100, Christoph Paasch wrote:
> Hello,
>
Hi Christoph
> I have a scenario where I can trigger a bug on kernels >= 3.3 and <= 3.6.
> Thus, I can produce it with the latest longterm-stable v3.4.26.
>
> The crashdumps/warning can be seen below. Sometimes it is only the warning,
> sometimes it also produces the crash. But, it happens each time I try out my
> scenario.
>
>
> How to reproduce the bug (I have HP Proliant DL165 machines with HP NC375T 1Gb
> interface):
>
> * Launch an iperf-session ( -t 10 ) to a server over a 1Gbps interface.
>
> * After 5 seconds on the client, remove the IP-address from the interface
> with ip addr del dev [itf] [ip]
>
> * Wait 10 more seconds and kill the iperf on the client and the server.
>
> * Then do: ifconfig down [itf]
>
> Now the crash happens.
>
> What I observe in netxen_release_tx_buffers is that upon the 18th iteration (j
> == 17), buffrag->length == 0. buffrag->frag_count is 18.
> Sometimes (much more rare), buffrag->length rather looks like garbage (e.g., >
> 2^32)
>
>
> I bisected this, and it was introduced by commit 9d4dde521577 (net: only use a
> single page of slop in MAX_SKB_FRAGS).
> It was fixed by Eric in commit 5640f7685831 (net: use a per task frag
> allocator) since kernel > 3.6.
>
Its a side effect of this patch, as it permits to build a full TSO
packet using 2 or 3 frags, instead of 16 to 17 frags.
But you could theoretically still hit the bug if the application uses
several sockets and does short write() on them. Because each short
write() would use a small frag.
> As this bug is present in the longterm-stable 3.4, should Eric's patch be
> backported?
I don't think so. It had some side effects that we are still sorting
out. (see recent splice() fixes for example)
> If not, does somebody (with more knowledge than I have of this part of the
> code) can have a look at it, or maybe give me a pointer on how I could solve
> this properly?
>
> Reverting commit 9d4dde521577 (net: only use a single page of slop in
> MAX_SKB_FRAGS) fixes it for me on 3.4.26.
>
Something doesn't properly test MAX_SKB_FRAGS, we should track it and
fix.
^ permalink raw reply
* Re: BUG in netxen_release_tx_buffers when TSO enabled on kernels >= 3.3 and <= 3.6
From: Eric Dumazet @ 2013-01-22 13:56 UTC (permalink / raw)
To: christoph.paasch
Cc: Ian Campbell, Sony Chacko, Rajesh Borundia, David Miller, netdev
In-Reply-To: <1358861524.3464.3768.camel@edumazet-glaptop>
On Tue, 2013-01-22 at 05:32 -0800, Eric Dumazet wrote:
>
> Something doesn't properly test MAX_SKB_FRAGS, we should track it and
> fix.
I guess netxen driver has a bug.
Please try the following patch :
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index bc165f4..695667d 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -144,7 +144,7 @@ void netxen_release_tx_buffers(struct netxen_adapter *adapter)
buffrag->length, PCI_DMA_TODEVICE);
buffrag->dma = 0ULL;
}
- for (j = 0; j < cmd_buf->frag_count; j++) {
+ for (j = 1; j < cmd_buf->frag_count; j++) {
buffrag++;
if (buffrag->dma) {
pci_unmap_page(adapter->pdev, buffrag->dma,
^ permalink raw reply related
* Re: [PATCH net-next V6 02/14] bridge: Add vlan filtering infrastructure
From: Vlad Yasevich @ 2013-01-22 14:31 UTC (permalink / raw)
To: Shmulik Ladkani
Cc: bridge, Stephen Hemminger, Michał Mirosław, netdev,
davem, shemminger, mst
In-Reply-To: <20130121134534.78032a54.shmulik.ladkani@gmail.com>
On 01/21/2013 06:45 AM, Shmulik Ladkani wrote:
> Hi Vlad,
>
> On Sun, 20 Jan 2013 20:50:59 -0500 Vlad Yasevich <vyasevic@redhat.com> wrote:
>> On 01/20/2013 02:38 PM, Stephen Hemminger wrote:
>>> Let's assume the people that really want this feature are using a lot
>>> of vlan's. i.e n = 1000 or so. A bitmap is O(1). Any hash list would
>>> incur a just a big memory penalty for the list head. In other words
>>> a full bitmap is 4096 bits = 512 bytes. If you use hash list,
>>> then the equivalent memory size would be only 64 list heads, therefore
>>> a bitmap is a better choice than a hlist.
>>>
>>
>> This was the approach taken in the RFC v1 of this series. What I found
>> was that while it worked very well as far as speed goes, it was a bit
>> cumbersome to extend it to support pvids and it would completely fall
>> on its face for egress policy that Shmulik is suggesting. So any kinds
>> of extensions to it were tough to do.
>
> I don't see why this is the case.
>
> How about (sketch only, names questionable...):
>
> struct net_bridge {
> + unsigned long vlan_port_membership_bitmap[VLAN_N_VID][PORT_BITMAP_LEN];
> + unsigned long vlan_port_egress_policy_bitmap[VLAN_N_VID][PORT_BITMAP_LEN];
> }
>
> (can be alloc'ed instead of the arrays being part of the struct)
>
> struct net_bridge_port {
> + u16 pvid;
> };
>
> Allows O(1) to the query "is port P member of vlan V".
> Allows O(1) to the query "should vlan V egress tagged/untagged on port P".
>
> I guess this might simplify the data structures involved, avoiding the
> refcounts, etc...
>
> The penaties are:
> - memory
> - aesthetics (?)
> - inefficient if query is "give me the entire list of VLANs port P is
> member of". But do we have such a query in bridge's code?
Yes. When a mac address is added to a port without an explicit vlan tag
we try to add it for every vlan available on the port.
Also, in the API, the user may request vlans configured on a port.
>
> You say it went cumbersome. Am I missing something?
>
> BTW, altenatively, you may:
>
> struct net_bridge_port {
> + unsigned long vlan_membership_bitmap[BITS_TO_LONGS(VLAN_N_VID)];
> + unsigned long vlan_egress_policy_bitmap[BITS_TO_LONGS(VLAN_N_VID)];
> + u16 pvid;
> };
>
> Which also allows O(1) to "is port 'nbp' member of vlan V".
>
This is what the earlier RFC patches did. You are paying a large memory
penalty and carrying a mostly empty bitmap when only a small number of
vlans is used.
If someone decides that they'd like priority support, you'd need another
array or list to hold priority values.
-vlad
> Difference:
> - For the membership structure:
> former (within net_bridge) uses 4096 * BR_MAX_PORTS bits,
> latter (within net_bridge_port) uses NumOfNBPs * 4096 bits
> - better aesthetics (?)
>
> Regards,
> Shmulik
>
^ permalink raw reply
* Re: [PATCH] net: fec: Add support for multiple phys on mdiobus
From: Florian Fainelli @ 2013-01-22 14:47 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Sascha Hauer, netdev, linux-arm-kernel, shawn.guo, davem
In-Reply-To: <50FE3E37.4010004@grandegger.com>
On 01/22/2013 08:22 AM, Wolfgang Grandegger wrote:
>> Well this could be done when the fixed phy driver could be registered
>> with the devicetree, maybe like this:
>>
>> fixed-phy: mdiophy {
>> compatible = "mdio-fixed-phy";
>> link = "100FD";
>> };
>
> I find that confusing. There is *no* phy but just a fixed link to the
> switch...
>
>> The good thing about this would be that every ethernet driver could just
>> use such a fixed phy, any external mdio phy (like on Marvell Armada) or
>> just a phy connected to the internal mdio interface provided by the ethernet
>> core.
>
> What is wrong with the existing "fixed-link" property of the *ethernet*
> node. The fixed-link handling should/could be done in the phy layer, and
> not in the driver as it currently is implemented. Maybe that's the
> reason why the current code is regarded as hack!
As far as I have used it with the CPMAC driver, the fixed PHY is a
specific PHY device and there is no specific handling to be done by the
Ethernet MAC driver but eventually changing its MII bus id so that it is
named "fixed-0" to allow the fixed PHY driver to bind. Put differently,
using the fixed PHY driver is a kind of "last" resort thing to cope with
situations like:
- switch not providing a consistent PHY-like interface on the MDC/MDIO bus
- switches not connected to the MDC/MDIO bus of the Ethernet MAC they
forward to
What exactly do you mean by "as it currently is implemented"? that you
do not like?
--
Florian
^ permalink raw reply
* Re: [PATCH] net: fec: Add support for multiple phys on mdiobus
From: Wolfgang Grandegger @ 2013-01-22 15:03 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Sascha Hauer, netdev, linux-arm-kernel, shawn.guo, davem
In-Reply-To: <50FEA691.4050904@openwrt.org>
On 01/22/2013 03:47 PM, Florian Fainelli wrote:
> On 01/22/2013 08:22 AM, Wolfgang Grandegger wrote:
>>> Well this could be done when the fixed phy driver could be registered
>>> with the devicetree, maybe like this:
>>>
>>> fixed-phy: mdiophy {
>>> compatible = "mdio-fixed-phy";
>>> link = "100FD";
>>> };
>>
>> I find that confusing. There is *no* phy but just a fixed link to the
>> switch...
>>
>>> The good thing about this would be that every ethernet driver could just
>>> use such a fixed phy, any external mdio phy (like on Marvell Armada) or
>>> just a phy connected to the internal mdio interface provided by the
>>> ethernet
>>> core.
>>
>> What is wrong with the existing "fixed-link" property of the *ethernet*
>> node. The fixed-link handling should/could be done in the phy layer, and
>> not in the driver as it currently is implemented. Maybe that's the
>> reason why the current code is regarded as hack!
>
> As far as I have used it with the CPMAC driver, the fixed PHY is a
> specific PHY device and there is no specific handling to be done by the
> Ethernet MAC driver but eventually changing its MII bus id so that it is
> named "fixed-0" to allow the fixed PHY driver to bind. Put differently,
There is special handling for the fixed link, e.g. here:
http://lxr.linux.no/#linux+v3.7.4/drivers/net/ethernet/freescale/gianfar.c#L1462
This could be hidden in the PHY layer allowing all ethernet drivers
using the "fixed-link" property.
> using the fixed PHY driver is a kind of "last" resort thing to cope with
> situations like:
>
> - switch not providing a consistent PHY-like interface on the MDC/MDIO bus
> - switches not connected to the MDC/MDIO bus of the Ethernet MAC they
> forward to
>
> What exactly do you mean by "as it currently is implemented"? that you
> do not like?
As Sascha pointed out, the implementation is labeled as "hack", which
should be fixed sooner than later. It's just not clear why ;).
Wolfgang.
^ permalink raw reply
* Re: BUG in netxen_release_tx_buffers when TSO enabled on kernels >= 3.3 and <= 3.6
From: Christoph Paasch @ 2013-01-22 15:03 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ian Campbell, Sony Chacko, Rajesh Borundia, David Miller, netdev
In-Reply-To: <1358862966.3464.3797.camel@edumazet-glaptop>
On Tuesday 22 January 2013 05:56:06 Eric Dumazet wrote:
> I guess netxen driver has a bug.
>
> Please try the following patch :
>
> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c index
> bc165f4..695667d 100644
> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
> @@ -144,7 +144,7 @@ void netxen_release_tx_buffers(struct netxen_adapter
> *adapter) buffrag->length, PCI_DMA_TODEVICE); buffrag->dma = 0ULL;
> }
> - for (j = 0; j < cmd_buf->frag_count; j++) {
> + for (j = 1; j < cmd_buf->frag_count; j++) {
> buffrag++;
> if (buffrag->dma) {
> pci_unmap_page(adapter->pdev, buffrag->dma,
Perfect, I tested it, and this fixes the bug.
I should have found it on my own, I have been starring for too long at this
function... :)
Feel free to add
Tested-by: Christoph Paasch <christoph.paasch@uclouvain.be>
Thanks, Eric!
Christoph
--
IP Networking Lab --- http://inl.info.ucl.ac.be
MultiPath TCP in the Linux Kernel --- http://mptcp.info.ucl.ac.be
UCLouvain
--
^ permalink raw reply
* [PATCH net-next] neigh: Keep neighbour cache entries if number of them is small enough.
From: YOSHIFUJI Hideaki @ 2013-01-22 15:09 UTC (permalink / raw)
To: davem, netdev; +Cc: yoshfuji
Since we have removed NCE (Neighbour Cache Entry) reference from
routing entries, the only refcnt holders of an NCE are timer (if run)
and the entry's owner table. As a result, neigh_periodic_timer()
purges NCEs over and over even for gateways.
It does not make sense to purge NCEs even if number of them is
very small.
The number of entries can be specified by gc_thresh1.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
Documentation/networking/ip-sysctl.txt | 5 +++++
net/core/neighbour.c | 4 ++++
2 files changed, 9 insertions(+)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 4976564..19ac180 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -26,6 +26,11 @@ route/max_size - INTEGER
Maximum number of routes allowed in the kernel. Increase
this when using large numbers of interfaces and/or routes.
+neigh/default/gc_thresh1 - INTEGER
+ Minimum number of entries to keep. Garbage collector will not
+ purge entries if there are fewer than this number.
+ Default: 256
+
neigh/default/gc_thresh3 - INTEGER
Maximum number of neighbor entries allowed. Increase this
when using large numbers of interfaces and when communicating
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index c815f28..7bd0eed 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -778,6 +778,9 @@ static void neigh_periodic_work(struct work_struct *work)
nht = rcu_dereference_protected(tbl->nht,
lockdep_is_held(&tbl->lock));
+ if (atomic_read(&tbl->entries) < tbl->gc_thresh1)
+ goto out;
+
/*
* periodically recompute ReachableTime from random function
*/
@@ -832,6 +835,7 @@ next_elt:
nht = rcu_dereference_protected(tbl->nht,
lockdep_is_held(&tbl->lock));
}
+out:
/* Cycle through all hash buckets every base_reachable_time/2 ticks.
* ARP entry timeouts range from 1/2 base_reachable_time to 3/2
* base_reachable_time.
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net-next] neigh: Keep neighbour cache entries if number of them is small enough.
From: YOSHIFUJI Hideaki @ 2013-01-22 15:12 UTC (permalink / raw)
To: davem; +Cc: YOSHIFUJI Hideaki, netdev
In-Reply-To: <50FEAB99.5060408@linux-ipv6.org>
YOSHIFUJI Hideaki wrote:
> Since we have removed NCE (Neighbour Cache Entry) reference from
> routing entries, the only refcnt holders of an NCE are timer (if run)
> and the entry's owner table. As a result, neigh_periodic_timer()
Oops, I should say neigh_periodic_work()...
--yoshfuji
^ permalink raw reply
* [PATCH] l2tp: prevent l2tp_tunnel_delete racing with userspace close
From: Tom Parkin @ 2013-01-22 15:13 UTC (permalink / raw)
To: netdev; +Cc: celston, Tom Parkin, James Chapman
If a tunnel socket is created by userspace, l2tp hooks the socket destructor
in order to clean up resources if userspace closes the socket or crashes. It
also caches a pointer to the struct sock for use in the data path and in the
netlink interface.
While it is safe to use the cached sock pointer in the data path, where the
skb references keep the socket alive, it is not safe to use it elsewhere as
such access introduces a race with userspace closing the socket. In
particular, l2tp_tunnel_delete is prone to oopsing if a multithreaded
userspace application closes a socket at the same time as sending a netlink
delete command for the tunnel.
This patch fixes this oops by forcing l2tp_tunnel_delete to explicitly look up
a tunnel socket held by userspace using sockfd_lookup().
Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
net/l2tp/l2tp_core.c | 76 ++++++++++++++++++++++++++++++++++++++++++--------
net/l2tp/l2tp_core.h | 5 +++-
2 files changed, 69 insertions(+), 12 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 1a9f372..06389d5 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -168,6 +168,51 @@ l2tp_session_id_hash_2(struct l2tp_net *pn, u32 session_id)
}
+/* Lookup the tunnel socket, possibly involving the fs code if the socket is
+ * owned by userspace. A struct sock returned from this function must be
+ * released using l2tp_tunnel_sock_put once you're done with it.
+ */
+struct sock *l2tp_tunnel_sock_lookup(struct l2tp_tunnel *tunnel)
+{
+ int err = 0;
+ struct socket *sock = NULL;
+ struct sock *sk = NULL;
+
+ if (!tunnel)
+ goto out;
+
+ if (tunnel->fd >= 0) {
+ /* Socket is owned by userspace, who might be in the process
+ * of closing it. Look the socket up using the fd to ensure
+ * consistency.
+ */
+ sock = sockfd_lookup(tunnel->fd, &err);
+ if (sock)
+ sk = sock->sk;
+ } else {
+ /* Socket is owned by kernelspace */
+ sk = tunnel->sock;
+ }
+
+out:
+ return sk;
+}
+EXPORT_SYMBOL_GPL(l2tp_tunnel_sock_lookup);
+
+/* Drop a reference to a tunnel socket obtained via. l2tp_tunnel_sock_put */
+void l2tp_tunnel_sock_put(struct sock *sk)
+{
+ struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk);
+ if (tunnel) {
+ if (tunnel->fd >= 0) {
+ /* Socket is owned by userspace */
+ sockfd_put(sk->sk_socket);
+ }
+ sock_put(sk);
+ }
+}
+EXPORT_SYMBOL_GPL(l2tp_tunnel_sock_put);
+
/* Lookup a session by id in the global session list
*/
static struct l2tp_session *l2tp_session_find_2(struct net *net, u32 session_id)
@@ -1607,6 +1652,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
tunnel->old_sk_destruct = sk->sk_destruct;
sk->sk_destruct = &l2tp_tunnel_destruct;
tunnel->sock = sk;
+ tunnel->fd = fd;
lockdep_set_class_and_name(&sk->sk_lock.slock, &l2tp_socket_class, "l2tp_sock");
sk->sk_allocation = GFP_ATOMIC;
@@ -1642,24 +1688,32 @@ EXPORT_SYMBOL_GPL(l2tp_tunnel_create);
*/
int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel)
{
- int err = 0;
- struct socket *sock = tunnel->sock ? tunnel->sock->sk_socket : NULL;
+ int err = -EBADF;
+ struct socket *sock = NULL;
+ struct sock *sk = NULL;
+
+ sk = l2tp_tunnel_sock_lookup(tunnel);
+ if (!sk)
+ goto out;
+
+ sock = sk->sk_socket;
+ BUG_ON(!sock);
/* Force the tunnel socket to close. This will eventually
* cause the tunnel to be deleted via the normal socket close
* mechanisms when userspace closes the tunnel socket.
*/
- if (sock != NULL) {
- err = inet_shutdown(sock, 2);
+ err = inet_shutdown(sock, 2);
- /* If the tunnel's socket was created by the kernel,
- * close the socket here since the socket was not
- * created by userspace.
- */
- if (sock->file == NULL)
- err = inet_release(sock);
- }
+ /* If the tunnel's socket was created by the kernel,
+ * close the socket here since the socket was not
+ * created by userspace.
+ */
+ if (sock->file == NULL)
+ err = inet_release(sock);
+ l2tp_tunnel_sock_put(sk);
+out:
return err;
}
EXPORT_SYMBOL_GPL(l2tp_tunnel_delete);
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 56d583e..e62204c 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -188,7 +188,8 @@ struct l2tp_tunnel {
int (*recv_payload_hook)(struct sk_buff *skb);
void (*old_sk_destruct)(struct sock *);
struct sock *sock; /* Parent socket */
- int fd;
+ int fd; /* Parent fd, if tunnel socket
+ * was created by userspace */
uint8_t priv[0]; /* private data */
};
@@ -228,6 +229,8 @@ out:
return tunnel;
}
+extern struct sock *l2tp_tunnel_sock_lookup(struct l2tp_tunnel *tunnel);
+extern void l2tp_tunnel_sock_put(struct sock *sk);
extern struct l2tp_session *l2tp_session_find(struct net *net, struct l2tp_tunnel *tunnel, u32 session_id);
extern struct l2tp_session *l2tp_session_find_nth(struct l2tp_tunnel *tunnel, int nth);
extern struct l2tp_session *l2tp_session_find_by_ifname(struct net *net, char *ifname);
--
1.7.9.5
^ permalink raw reply related
* [PATCH net-next (v2, reword)] neigh: Keep neighbour cache entries if number of them is small enough.
From: YOSHIFUJI Hideaki @ 2013-01-22 15:20 UTC (permalink / raw)
To: davem, netdev; +Cc: yoshfuji
Since we have removed NCE (Neighbour Cache Entry) reference from
routing entries, the only refcnt holders of an NCE are its timer
(if running) and its owner table, in usual cases. As a result,
neigh_periodic_work() purges NCEs over and over again even for
gateways.
It does not make sense to purge entries, if number of them is
very small, so keep them. The minimum number of entries to keep
is specified by gc_thresh1.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
Documentation/networking/ip-sysctl.txt | 5 +++++
net/core/neighbour.c | 4 ++++
2 files changed, 9 insertions(+)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 4976564..19ac180 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -26,6 +26,11 @@ route/max_size - INTEGER
Maximum number of routes allowed in the kernel. Increase
this when using large numbers of interfaces and/or routes.
+neigh/default/gc_thresh1 - INTEGER
+ Minimum number of entries to keep. Garbage collector will not
+ purge entries if there are fewer than this number.
+ Default: 256
+
neigh/default/gc_thresh3 - INTEGER
Maximum number of neighbor entries allowed. Increase this
when using large numbers of interfaces and when communicating
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index c815f28..7bd0eed 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -778,6 +778,9 @@ static void neigh_periodic_work(struct work_struct *work)
nht = rcu_dereference_protected(tbl->nht,
lockdep_is_held(&tbl->lock));
+ if (atomic_read(&tbl->entries) < tbl->gc_thresh1)
+ goto out;
+
/*
* periodically recompute ReachableTime from random function
*/
@@ -832,6 +835,7 @@ next_elt:
nht = rcu_dereference_protected(tbl->nht,
lockdep_is_held(&tbl->lock));
}
+out:
/* Cycle through all hash buckets every base_reachable_time/2 ticks.
* ARP entry timeouts range from 1/2 base_reachable_time to 3/2
* base_reachable_time.
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] net: fec: Add support for multiple phys on mdiobus
From: Florian Fainelli @ 2013-01-22 15:19 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Sascha Hauer, netdev, linux-arm-kernel, shawn.guo, davem
In-Reply-To: <50FEAA28.1070003@grandegger.com>
On 01/22/2013 04:03 PM, Wolfgang Grandegger wrote:
> On 01/22/2013 03:47 PM, Florian Fainelli wrote:
>> On 01/22/2013 08:22 AM, Wolfgang Grandegger wrote:
>>>> Well this could be done when the fixed phy driver could be registered
>>>> with the devicetree, maybe like this:
>>>>
>>>> fixed-phy: mdiophy {
>>>> compatible = "mdio-fixed-phy";
>>>> link = "100FD";
>>>> };
>>> I find that confusing. There is *no* phy but just a fixed link to the
>>> switch...
>>>
>>>> The good thing about this would be that every ethernet driver could just
>>>> use such a fixed phy, any external mdio phy (like on Marvell Armada) or
>>>> just a phy connected to the internal mdio interface provided by the
>>>> ethernet
>>>> core.
>>> What is wrong with the existing "fixed-link" property of the *ethernet*
>>> node. The fixed-link handling should/could be done in the phy layer, and
>>> not in the driver as it currently is implemented. Maybe that's the
>>> reason why the current code is regarded as hack!
>> As far as I have used it with the CPMAC driver, the fixed PHY is a
>> specific PHY device and there is no specific handling to be done by the
>> Ethernet MAC driver but eventually changing its MII bus id so that it is
>> named "fixed-0" to allow the fixed PHY driver to bind. Put differently,
> There is special handling for the fixed link, e.g. here:
>
> http://lxr.linux.no/#linux+v3.7.4/drivers/net/ethernet/freescale/gianfar.c#L1462
>
> This could be hidden in the PHY layer allowing all ethernet drivers
> using the "fixed-link" property.
Ok, so in the end you could have potentially an ethernet PHY node
containing the following properties:
- fixed-link
- fixed-speed
- fixed-duplex
and treat it as a fixed-phy. If so, this would be pretty handy.
>
>> using the fixed PHY driver is a kind of "last" resort thing to cope with
>> situations like:
>>
>> - switch not providing a consistent PHY-like interface on the MDC/MDIO bus
>> - switches not connected to the MDC/MDIO bus of the Ethernet MAC they
>> forward to
>>
>> What exactly do you mean by "as it currently is implemented"? that you
>> do not like?
> As Sascha pointed out, the implementation is labeled as "hack", which
> should be fixed sooner than later. It's just not clear why ;).
>
> Wolfgang.
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: 69b08f62 breaks ethernet on atl1c based Eee PC 1011px
From: Michael Büsch @ 2013-01-22 15:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: linux-netdev, Alexander Duyck, Benjamin LaHaise, David S. Miller,
Jay Cliburn, Chris Snook
In-Reply-To: <CANn89iK5iEL5HnGxtRR2Gk_EEcV5wUmiAB9XgNwn9c-+c2M5gw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 415 bytes --]
On Tue, 22 Jan 2013 07:16:49 -0800
Eric Dumazet <edumazet@google.com> wrote:
> Its probably a VM issue fixed by Mel Gorman in
> commit 8fb74b9fb2b182d54beee592350d9ea1f325917a
>
> mm: compaction: partially revert capture of suitable high-order page
That was already applied in 3.7.3 as e5e65a5233c3f368c442413cebe7de2a4580b564,
so it doesn't fix the issue.
--
Greetings, Michael.
PGP: 908D8B0E
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ 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