* RE: [PATCH] net: add new QCA alx ethernet driver
From: Ren, Cloud @ 2012-08-17 1:49 UTC (permalink / raw)
To: Joe Perches
Cc: davem@davemloft.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, qca-linux-team, nic-devel,
Huang, Xiong, Rodriguez, Luis
In-Reply-To: <1345122983.26882.9.camel@joe2Laptop>
From: Joe Perches [mailto:joe@perches.com]
Sent: Thursday, August 16, 2012 9:16 PM
>Hi Cloud. Please convert this to
>
> netdev_printk(level, hw->adpt->netdev, "%pV", &vaf);
>
>I will submit a patch soon to make the __netdev_printk variant static.
Ok, thanks.
^ permalink raw reply
* Re: suspicious RCU usage in xfrm_net_init()
From: Fan Du @ 2012-08-17 2:03 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: David Miller, Fengguang Wu, Priyanka Jain, netdev, LKML
In-Reply-To: <CAP=VYLqHW+NOS-gPgUkUpZh-Vim2jkmMT7NK+7uRbg730d=xHw@mail.gmail.com>
On 2012年08月17日 09:34, Paul Gortmaker wrote:
> On Thu, Aug 16, 2012 at 9:07 PM, Fan Du<fan.du@windriver.com> wrote:
>>
>>
>> On 2012年08月16日 23:19, Fengguang Wu wrote:
>>>
>>> Hi Fan,
>>>
>>> On Thu, Aug 16, 2012 at 05:36:35PM +0800, Fan Du wrote:
>>>>
>>>>
>>>> Hi, Fengguang
>>>>
>>>> Could you please try the below patch, see if spewing still there?
>>>> thanks
>>>
>>>
>>> Yes, it worked, thank you very much!
>>>
>>
>> Hi, Dave
>>
>> Could you please pick up this patch?
>
> Please do not make extra work for maintainers by sending attachments,
> or requests for status/merge etc. Your 1st patch had to be manually
> set to an RFC, and now you add another patch less than 24h later.
>
> Please see:
>
> http://patchwork.ozlabs.org/patch/177934/
> http://patchwork.ozlabs.org/patch/178132/
>
> Also, a patch should describe the problem it solves (i.e. the symptom
> the end user sees), and how the problem originated, and why the fix
> in the patch is the _right_ fix. The worst description a commit log
> can have is one that just describes the C change in words, since
> most people can read C on their own.
>
> Here you add "_bh" in the code and then repeat exactly that in
> the commit log. Your commit log does not tell me when it broke,
> or why it broke, or who had their use case broken. Can you see
> why this is not acceptable?
>
> Please take the time to look at the traffic in netdev, and read
> the feedback given by maintainers on other patches, so that the
> common errors are understood by you, and not repeated. It
> will be time well spent!
>
Rick Jones has already well informed me the etiquettes off the list,
which I just broken.
Anyway, thanks for your time writing those suggestions.
> Thanks,
> Paul.
> ---
>
>> thanks
>>
>>
>>
>>
>>> btw, your email client wraps long lines..
>>>
>> Oh, I will definitely fix this.
>> thanks feng guang for the testing :)
>>
>>
>>
>>> Thanks,
>>> Fengguang
>>>
>>>> From a3f86ecc3ee16ff81d49416bbf791780422988b3 Mon Sep 17 00:00:00 2001
>>>> From: Fan Du<fan.du@windriver.com>
>>>> Date: Thu, 16 Aug 2012 17:31:25 +0800
>>>> Subject: [PATCH] Use rcu_dereference_bh to deference pointer
>>>> protected by rcu_read_lock_bh
>>>>
>>>> Signed-off-by: Fan Du<fan.du@windriver.com>
>>>> ---
>>>> net/xfrm/xfrm_policy.c | 2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
>>>> index 5ad4d2c..75a9d6a 100644
>>>> --- a/net/xfrm/xfrm_policy.c
>>>> +++ b/net/xfrm/xfrm_policy.c
>>>> @@ -2501,7 +2501,7 @@ static void __net_init
>>>> xfrm_dst_ops_init(struct net *net)
>>>> struct xfrm_policy_afinfo *afinfo;
>>>>
>>>> rcu_read_lock_bh();
>>>> - afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
>>>> + afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET]);
>>>> if (afinfo)
>>>> net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
>>>> #if IS_ENABLED(CONFIG_IPV6)
>>>> --
>>>> 1.7.1
>>>>
>>>>
>>>>
>>>>
>>>> On 2012年08月16日 15:37, Fengguang Wu wrote:
>>>>>
>>>>> Hi Priyanka,
>>>>>
>>>>> The below warning shows up, probably related to this commit:
>>>>>
>>>>> 418a99ac6ad487dc9c42e6b0e85f941af56330f2 Replace rwlock on
>>>>> xfrm_policy_afinfo with rcu
>>>>>
>>>>> [ 0.921216]
>>>>> [ 0.921645] ===============================
>>>>> [ 0.922766] [ INFO: suspicious RCU usage. ]
>>>>> [ 0.923887] 3.5.0-01540-g1669891 #64 Not tainted
>>>>> [ 0.925123] -------------------------------
>>>>> [ 0.932860] /c/kernel-tests/src/tip/net/xfrm/xfrm_policy.c:2504
>>>>> suspicious rcu_dereference_check() usage!
>>>>> [ 0.935361]
>>>>> [ 0.935361] other info that might help us debug this:
>>>>> [ 0.935361]
>>>>> [ 0.937472]
>>>>> [ 0.937472] rcu_scheduler_active = 1, debug_locks = 0
>>>>> [ 0.939182] 2 locks held by swapper/1:
>>>>> [ 0.940171] #0: (net_mutex){+.+.+.}, at: [<ffffffff814e1ad0>]
>>>>> register_pernet_subsys+0x21/0x57
>>>>> [ 0.942705] #1: (rcu_read_lock_bh){......}, at:
>>>>> [<ffffffff822c7329>] xfrm_net_init+0x1e4/0x437
>>>>> [ 0.951507]
>>>>> [ 0.951507] stack backtrace:
>>>>> [ 0.952660] Pid: 1, comm: swapper Not tainted 3.5.0-01540-g1669891
>>>>> #64
>>>>> [ 0.954364] Call Trace:
>>>>> [ 0.955074] [<ffffffff8108b375>] lockdep_rcu_suspicious+0x174/0x187
>>>>> [ 0.956736] [<ffffffff822c7453>] xfrm_net_init+0x30e/0x437
>>>>> [ 0.958205] [<ffffffff822c7329>] ? xfrm_net_init+0x1e4/0x437
>>>>> [ 0.959712] [<ffffffff814e134a>] ops_init+0x1bb/0x1ff
>>>>> [ 0.961067] [<ffffffff810861f9>] ? trace_hardirqs_on+0x1b/0x24
>>>>> [ 0.962644] [<ffffffff814e17cd>]
>>>>> register_pernet_operations.isra.5+0x9d/0xfe
>>>>> [ 0.971376] [<ffffffff814e1adf>] register_pernet_subsys+0x30/0x57
>>>>> [ 0.972992] [<ffffffff822c7130>] xfrm_init+0x17/0x2c
>>>>> [ 0.974316] [<ffffffff822c2f8c>] ip_rt_init+0x82/0xe7
>>>>> [ 0.975668] [<ffffffff822c31dc>] ip_init+0x10/0x25
>>>>> [ 0.976952] [<ffffffff822c3f77>] inet_init+0x235/0x360
>>>>> [ 0.978352] [<ffffffff822c3d42>] ? devinet_init+0xf2/0xf2
>>>>> [ 0.979808] [<ffffffff82283252>] do_one_initcall+0xb4/0x203
>>>>> [ 0.981313] [<ffffffff8228354a>] kernel_init+0x1a9/0x29a
>>>>> [ 0.982732] [<ffffffff822826d9>] ? loglevel+0x46/0x46
>>>>> [ 0.990889] [<ffffffff816d3d84>] kernel_thread_helper+0x4/0x10
>>>>> [ 0.992472] [<ffffffff816d262c>] ? retint_restore_args+0x13/0x13
>>>>> [ 0.994076] [<ffffffff822833a1>] ? do_one_initcall+0x203/0x203
>>>>> [ 0.995636] [<ffffffff816d3d80>] ? gs_change+0x13/0x13
>>>>> [ 0.997197] TCP established hash table entries: 8192 (order: 5,
>>>>> 131072 bytes)
>>>>> [ 1.000074] TCP bind hash table entries: 8192 (order: 7, 655360
>>>>> bytes)
>>>>>
>>>>> Thanks,
>>>>> Fengguang
>>>>
>>>>
>>>> --
>>>>
>>>> Love each day!
>>>> --fan
>>>
>>>
>>
>> --
>>
>> Love each day!
>> --fan
>
--
Love each day!
--fan
^ permalink raw reply
* Re: suspicious RCU usage in xfrm_net_init()
From: David Miller @ 2012-08-17 2:39 UTC (permalink / raw)
To: fan.du; +Cc: fengguang.wu, Priyanka.Jain, netdev, linux-kernel
In-Reply-To: <502D9938.2010908@windriver.com>
From: Fan Du <fan.du@windriver.com>
Date: Fri, 17 Aug 2012 09:07:04 +0800
> Could you please pick up this patch?
Done, but could you please put proper prefixes in your commit
message subject lines? I had to prepend "xfrm: " this time.
^ permalink raw reply
* Re: suspicious RCU usage in xfrm_net_init()
From: David Miller @ 2012-08-17 2:41 UTC (permalink / raw)
To: paul.gortmaker; +Cc: fan.du, fengguang.wu, Priyanka.Jain, netdev, linux-kernel
In-Reply-To: <CAP=VYLqHW+NOS-gPgUkUpZh-Vim2jkmMT7NK+7uRbg730d=xHw@mail.gmail.com>
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Thu, 16 Aug 2012 21:34:25 -0400
> Also, a patch should describe the problem it solves (i.e. the symptom
> the end user sees), and how the problem originated, and why the fix
> in the patch is the _right_ fix. The worst description a commit log
> can have is one that just describes the C change in words, since
> most people can read C on their own.
I've frankly given up on Fan Du submitting sophisticated patches
that are easy to review and are properly documented.
Just getting simple things like a WORKING EMAIL ADDRESS was beyond a
struggle.
So when I get a patch that applies, from a properly working email
address, it's an accomplishment.
^ permalink raw reply
* [PATCH 0/3] raid, kmemleak, netfilter: replace list_for_each_continue_rcu with new interface
From: Michael Wang @ 2012-08-17 4:33 UTC (permalink / raw)
To: LKML, linux-raid, linux-mm, netdev@vger.kernel.org, netfilter,
coreteam, netfilter-devel
Cc: neilb, catalin.marinas, David Miller, kaber, pablo,
paulmck@linux.vnet.ibm.com
In-Reply-To: <502CB91E.4050304@linux.vnet.ibm.com>
From: Michael Wang <wangyun@linux.vnet.ibm.com>
This patch set will replace the list_for_each_continue_rcu with the new
interface list_for_each_entry_continue_rcu, so we could remove the old
one later.
Changed:
raid: in "next_active_rdev"
kmemleak: in "kmemleak_seq_next"
netfilter: in "nf_iterate"
Tested:
raid:
mdadm command with an internal bitmap.
kmemleak:
enable kmemleak and check the info it captured.
netfilter:
add rule to iptables and check result by ping.
nfqnl_test which is a test utility of libnetfilter_queue.
All testing are using printk to make sure the code we want test
was invoked.
Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
---
drivers/md/bitmap.c | 9 +++------
mm/kmemleak.c | 6 ++----
net/netfilter/core.c | 11 +++++++----
3 files changed, 12 insertions(+), 14 deletions(-)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH 3/3] netfilter: replace list_for_each_continue_rcu with new interface
From: Michael Wang @ 2012-08-17 4:33 UTC (permalink / raw)
To: LKML, netdev@vger.kernel.org, netfilter, coreteam,
netfilter-devel
Cc: David Miller, kaber, pablo, paulmck@linux.vnet.ibm.com
In-Reply-To: <502CB939.3050008@linux.vnet.ibm.com>
From: Michael Wang <wangyun@linux.vnet.ibm.com>
This patch replaces list_for_each_continue_rcu() with
list_for_each_entry_continue_rcu() to allow removing
list_for_each_continue_rcu().
Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
---
net/netfilter/core.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index e19f365..50225bd 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -131,14 +131,14 @@ unsigned int nf_iterate(struct list_head *head,
int hook_thresh)
{
unsigned int verdict;
+ struct nf_hook_ops *elem = list_entry_rcu(*i,
+ struct nf_hook_ops, list);
/*
* The caller must not block between calls to this
* function because of risk of continuing from deleted element.
*/
- list_for_each_continue_rcu(*i, head) {
- struct nf_hook_ops *elem = (struct nf_hook_ops *)*i;
-
+ list_for_each_entry_continue_rcu(elem, head, list) {
if (hook_thresh > elem->priority)
continue;
@@ -155,11 +155,14 @@ repeat:
continue;
}
#endif
- if (verdict != NF_REPEAT)
+ if (verdict != NF_REPEAT) {
+ *i = &elem->list;
return verdict;
+ }
goto repeat;
}
}
+ *i = &elem->list;
return NF_ACCEPT;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH] xfrm:Use rcu_dereference_bh to deference pointer protected by rcu_read_lock_bh
From: Fan Du @ 2012-08-17 6:19 UTC (permalink / raw)
To: davem; +Cc: fengguang.wu, netdev
418a99ac6ad487dc9c42e6b0e85f941af56330f2 "Replace rwlock on xfrm_policy_afinfo with rcu"
triggers below warnings, which is caused by abusing rcu_dereference_bh with rcu_read_lock.
RCU rules must be honored:
- rcu_dereference_bh paired with rcu_read_lock_bh/rcu_read_unlock_bh
- rcu_dereference paired with rcu_read_lock/rcu_read_unlock
[ 0.921216]
[ 0.921645] ===============================
[ 0.922766] [ INFO: suspicious RCU usage. ]
[ 0.923887] 3.5.0-01540-g1669891 #64 Not tainted
[ 0.925123] -------------------------------
[ 0.932860] /c/kernel-tests/src/tip/net/xfrm/xfrm_policy.c:2504 suspicious rcu_dereference_check() usage!
[ 0.935361]
[ 0.935361] other info that might help us debug this:
[ 0.935361]
[ 0.937472]
[ 0.937472] rcu_scheduler_active = 1, debug_locks = 0
[ 0.939182] 2 locks held by swapper/1:
[ 0.940171] #0: (net_mutex){+.+.+.}, at: [<ffffffff814e1ad0>] register_pernet_subsys+0x21/0x57
[ 0.942705] #1: (rcu_read_lock_bh){......}, at: [<ffffffff822c7329>] xfrm_net_init+0x1e4/0x437
[ 0.951507]
[ 0.951507] stack backtrace:
[ 0.952660] Pid: 1, comm: swapper Not tainted 3.5.0-01540-g1669891 #64
[ 0.954364] Call Trace:
[ 0.955074] [<ffffffff8108b375>] lockdep_rcu_suspicious+0x174/0x187
[ 0.956736] [<ffffffff822c7453>] xfrm_net_init+0x30e/0x437
[ 0.958205] [<ffffffff822c7329>] ? xfrm_net_init+0x1e4/0x437
[ 0.959712] [<ffffffff814e134a>] ops_init+0x1bb/0x1ff
[ 0.961067] [<ffffffff810861f9>] ? trace_hardirqs_on+0x1b/0x24
[ 0.962644] [<ffffffff814e17cd>] register_pernet_operations.isra.5+0x9d/0xfe
[ 0.971376] [<ffffffff814e1adf>] register_pernet_subsys+0x30/0x57
[ 0.972992] [<ffffffff822c7130>] xfrm_init+0x17/0x2c
[ 0.974316] [<ffffffff822c2f8c>] ip_rt_init+0x82/0xe7
[ 0.975668] [<ffffffff822c31dc>] ip_init+0x10/0x25
[ 0.976952] [<ffffffff822c3f77>] inet_init+0x235/0x360
[ 0.978352] [<ffffffff822c3d42>] ? devinet_init+0xf2/0xf2
[ 0.979808] [<ffffffff82283252>] do_one_initcall+0xb4/0x203
[ 0.981313] [<ffffffff8228354a>] kernel_init+0x1a9/0x29a
[ 0.982732] [<ffffffff822826d9>] ? loglevel+0x46/0x46
[ 0.990889] [<ffffffff816d3d84>] kernel_thread_helper+0x4/0x10
[ 0.992472] [<ffffffff816d262c>] ? retint_restore_args+0x13/0x13
[ 0.994076] [<ffffffff822833a1>] ? do_one_initcall+0x203/0x203
[ 0.995636] [<ffffffff816d3d80>] ? gs_change+0x13/0x13
[ 0.997197] TCP established hash table entries: 8192 (order: 5, 131072 bytes)
[ 1.000074] TCP bind hash table entries: 8192 (order: 7, 655360 bytes)
Reported-by: Wu Fengguang <fengguang.wu@intel.com>
Tested-by: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: Fan Du <fan.du@windriver.com>
---
net/xfrm/xfrm_policy.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 5ad4d2c..6405764 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2501,11 +2501,11 @@ static void __net_init xfrm_dst_ops_init(struct net *net)
struct xfrm_policy_afinfo *afinfo;
rcu_read_lock_bh();
- afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
+ afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET]);
if (afinfo)
net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
#if IS_ENABLED(CONFIG_IPV6)
- afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET6]);
+ afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET6]);
if (afinfo)
net->xfrm.xfrm6_dst_ops = *afinfo->dst_ops;
#endif
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] xfrm:Use rcu_dereference_bh to deference pointer protected by rcu_read_lock_bh
From: David Miller @ 2012-08-17 6:24 UTC (permalink / raw)
To: fan.du; +Cc: fengguang.wu, netdev
In-Reply-To: <1345184349-5849-1-git-send-email-fan.du@windriver.com>
I already applied your patch, as I told you here:
http://marc.info/?l=linux-netdev&m=134517122805719&w=2
This means you are submitting a patch which doesn't not even apply
to the net-next tree.
Instead of continuing to dig yourself deeper and deeper, take a
break, take a deep breath, and work slowly and carefully.
^ permalink raw reply
* Re: [PATCH] xfrm:Use rcu_dereference_bh to deference pointer protected by rcu_read_lock_bh
From: Fan Du @ 2012-08-17 6:36 UTC (permalink / raw)
To: David Miller; +Cc: fengguang.wu, netdev
In-Reply-To: <20120816.232414.545877018277576779.davem@davemloft.net>
On 2012年08月17日 14:24, David Miller wrote:
>
> I already applied your patch, as I told you here:
>
> http://marc.info/?l=linux-netdev&m=134517122805719&w=2
>
> This means you are submitting a patch which doesn't not even apply
> to the net-next tree.
>
> Instead of continuing to dig yourself deeper and deeper, take a
> break, take a deep breath, and work slowly and carefully.
>
OK, thanks for your kind guidance :)
--
Love each day!
--fan
^ permalink raw reply
* [PATCH 1/2] ipv6: do not hold route table lock when send ndisc probe
From: Cong Wang @ 2012-08-17 7:11 UTC (permalink / raw)
To: netdev
Cc: Cong Wang, Banerjee, Debabrata, David S. Miller,
Hideaki YOSHIFUJI, Patrick McHardy
In rt6_probe(), we call ndisc_send_ns() with root->rwlock,
but this is not necessary, so we can drop it before calling
ndisc_send_ns().
This could probably fix the deadlock reported by Debabrata:
https://lkml.org/lkml/2012/8/16/432
Reported-by: "Banerjee, Debabrata" <dbanerje@akamai.com>
Cc: "Banerjee, Debabrata" <dbanerje@akamai.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/ipv6/route.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 0ddf2d1..7a36df2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -460,13 +460,18 @@ static void rt6_probe(struct rt6_info *rt)
time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
struct in6_addr mcaddr;
struct in6_addr *target;
+ struct net_device *dev = rt->dst.dev;
+ struct fib6_table *table = rt->rt6i_table;
neigh->updated = jiffies;
read_unlock_bh(&neigh->lock);
+ read_unlock_bh(&table->tb6_lock);
target = (struct in6_addr *)&neigh->primary_key;
addrconf_addr_solict_mult(target, &mcaddr);
- ndisc_send_ns(rt->dst.dev, NULL, target, &mcaddr, NULL);
+ ndisc_send_ns(dev, NULL, target, &mcaddr, NULL);
+
+ read_lock_bh(&table->tb6_lock);
} else {
read_unlock_bh(&neigh->lock);
}
--
1.7.7.6
^ permalink raw reply related
* [PATCH 2/2] ipv6: remove some useless RCU read lock
From: Cong Wang @ 2012-08-17 7:11 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David S. Miller
In-Reply-To: <1345187499-16929-1-git-send-email-amwang@redhat.com>
After this commit:
commit 97cac0821af4474ec4ba3a9e7a36b98ed9b6db88
Author: David S. Miller <davem@davemloft.net>
Date: Mon Jul 2 22:43:47 2012 -0700
ipv6: Store route neighbour in rt6_info struct.
we no longer use RCU to protect route neighbour.
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/ipv6/ip6_output.c | 13 ++-----------
net/ipv6/route.c | 15 ++-------------
2 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 5b2d63e..9f67746 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -123,16 +123,11 @@ static int ip6_finish_output2(struct sk_buff *skb)
skb->len);
}
- rcu_read_lock();
rt = (struct rt6_info *) dst;
neigh = rt->n;
- if (neigh) {
- int res = dst_neigh_output(dst, neigh, skb);
+ if (neigh)
+ return dst_neigh_output(dst, neigh, skb);
- rcu_read_unlock();
- return res;
- }
- rcu_read_unlock();
IP6_INC_STATS_BH(dev_net(dst->dev),
ip6_dst_idev(dst), IPSTATS_MIB_OUTNOROUTES);
kfree_skb(skb);
@@ -980,7 +975,6 @@ static int ip6_dst_lookup_tail(struct sock *sk,
* dst entry and replace it instead with the
* dst entry of the nexthop router
*/
- rcu_read_lock();
rt = (struct rt6_info *) *dst;
n = rt->n;
if (n && !(n->nud_state & NUD_VALID)) {
@@ -988,7 +982,6 @@ static int ip6_dst_lookup_tail(struct sock *sk,
struct flowi6 fl_gw6;
int redirect;
- rcu_read_unlock();
ifp = ipv6_get_ifaddr(net, &fl6->saddr,
(*dst)->dev, 1);
@@ -1008,8 +1001,6 @@ static int ip6_dst_lookup_tail(struct sock *sk,
if ((err = (*dst)->error))
goto out_err_release;
}
- } else {
- rcu_read_unlock();
}
#endif
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 7a36df2..0aeeb98 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -451,10 +451,9 @@ static void rt6_probe(struct rt6_info *rt)
* Router Reachability Probe MUST be rate-limited
* to no more than one per minute.
*/
- rcu_read_lock();
neigh = rt ? rt->n : NULL;
if (!neigh || (neigh->nud_state & NUD_VALID))
- goto out;
+ return;
read_lock_bh(&neigh->lock);
if (!(neigh->nud_state & NUD_VALID) &&
time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
@@ -475,8 +474,6 @@ static void rt6_probe(struct rt6_info *rt)
} else {
read_unlock_bh(&neigh->lock);
}
-out:
- rcu_read_unlock();
}
#else
static inline void rt6_probe(struct rt6_info *rt)
@@ -503,7 +500,6 @@ static inline int rt6_check_neigh(struct rt6_info *rt)
struct neighbour *neigh;
int m;
- rcu_read_lock();
neigh = rt->n;
if (rt->rt6i_flags & RTF_NONEXTHOP ||
!(rt->rt6i_flags & RTF_GATEWAY))
@@ -521,7 +517,6 @@ static inline int rt6_check_neigh(struct rt6_info *rt)
read_unlock_bh(&neigh->lock);
} else
m = 0;
- rcu_read_unlock();
return m;
}
@@ -2470,15 +2465,11 @@ static int rt6_fill_node(struct net *net,
if (rtnetlink_put_metrics(skb, dst_metrics_ptr(&rt->dst)) < 0)
goto nla_put_failure;
- rcu_read_lock();
n = rt->n;
if (n) {
- if (nla_put(skb, RTA_GATEWAY, 16, &n->primary_key) < 0) {
- rcu_read_unlock();
+ if (nla_put(skb, RTA_GATEWAY, 16, &n->primary_key) < 0)
goto nla_put_failure;
- }
}
- rcu_read_unlock();
if (rt->dst.dev &&
nla_put_u32(skb, RTA_OIF, rt->dst.dev->ifindex))
@@ -2680,14 +2671,12 @@ static int rt6_info_route(struct rt6_info *rt, void *p_arg)
#else
seq_puts(m, "00000000000000000000000000000000 00 ");
#endif
- rcu_read_lock();
n = rt->n;
if (n) {
seq_printf(m, "%pi6", n->primary_key);
} else {
seq_puts(m, "00000000000000000000000000000000");
}
- rcu_read_unlock();
seq_printf(m, " %08x %08x %08x %08x %8s\n",
rt->rt6i_metric, atomic_read(&rt->dst.__refcnt),
rt->dst.__use, rt->rt6i_flags,
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH v0 5/5] cgroup: Assign subsystem IDs during compile time
From: Li Zefan @ 2012-08-17 7:25 UTC (permalink / raw)
To: Tejun Heo
Cc: Daniel Wagner, netdev-u79uwXL29TY76Z2rM5mHXA,
cgroups-u79uwXL29TY76Z2rM5mHXA, Daniel Wagner, David S. Miller,
Andrew Morton, Eric Dumazet, Gao feng, Glauber Costa,
Jamal Hadi Salim, John Fastabend, Kamezawa Hiroyuki, Neil Horman
In-Reply-To: <20120816232010.GJ24861-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On 2012/8/17 7:20, Tejun Heo wrote:
> On Thu, Aug 16, 2012 at 04:12:16PM +0200, Daniel Wagner wrote:
>> From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
>>
>> We are able to safe some space when we assign the subsystem
>> IDs at compile time. Instead of allocating per cgroup
>> cgroup->subsys[CGROUP_SUBSYS_COUNT] where CGROUP_SUBSYS_COUNT is
>> always 64, we allocate 12 + 1 at max (at this point there are 12
>> subsystem).
>
> So, IIUC, this is effectively removing the capability to implement
> modularized controller which isn't known at kernel compile time. Am I
> right?
>
I think so.
> I don't think that's a bad idea but if we're doing that, can't we make
> things even simpler? Do we need to distinguish in-kernel and module
> at all?
>
> Li, what do you think about this?
>
I'm definitely all for simplicity, but I'm not sure if we can do better in
simplifying the code for modularized cgroup subsystem. (I guess you didn't
mean to remove this feature?)
^ permalink raw reply
* Re: [PATCH v0 3/5] cgroup: Protect access to task_cls_classid() when built as module
From: Li Zefan @ 2012-08-17 7:35 UTC (permalink / raw)
To: Daniel Wagner
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
Daniel Wagner, David S. Miller, Gao feng, Jamal Hadi Salim,
John Fastabend, Neil Horman, Tejun Heo
In-Reply-To: <1345126336-20755-4-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
> +#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
> +extern struct static_key cgroup_cls_enabled;
> +#define clscg_enabled static_key_false(&cgroup_cls_enabled)
> +#endif
> +
If it's built-in, clscg_enabled is always true (after we call cgroup_subsys_init at
boot), so we don't need jump label at all.
> extern void sock_update_classid(struct sock *sk);
>
> #if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
> @@ -52,7 +58,7 @@ static inline u32 task_cls_classid(struct task_struct *p)
> int id;
> u32 classid = 0;
>
> - if (in_interrupt())
> + if (!clscg_enabled || in_interrupt())
> return 0;
>
> rcu_read_lock();
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 8f67ced..8d3a400 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -327,6 +327,11 @@ int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> EXPORT_SYMBOL(__sk_backlog_rcv);
>
> #if defined(CONFIG_CGROUPS)
> +#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
> +struct static_key cgroup_cls_enabled = STATIC_KEY_INIT_FALSE;
> +EXPORT_SYMBOL_GPL(cgroup_cls_enabled);
> +#endif
> +
> #if !defined(CONFIG_NET_CLS_CGROUP)
> int net_cls_subsys_id = -1;
> EXPORT_SYMBOL_GPL(net_cls_subsys_id);
> diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
> index 7743ea8..f40086b 100644
> --- a/net/sched/cls_cgroup.c
> +++ b/net/sched/cls_cgroup.c
> @@ -44,12 +44,17 @@ static struct cgroup_subsys_state *cgrp_create(struct cgroup *cgrp)
>
> if (cgrp->parent)
> cs->classid = cgrp_cls_state(cgrp->parent)->classid;
> + else if (!clscg_enabled)
> + static_key_slow_inc(&cgroup_cls_enabled);
It's not necessary to check if !clsg_enabled.
>
> return &cs->css;
> }
>
> static void cgrp_destroy(struct cgroup *cgrp)
> {
> + if (!cgrp->parent && clscg_enabled)
> + static_key_slow_dec(&cgroup_cls_enabled);
> +
ditto.
> kfree(cgrp_cls_state(cgrp));
> }
>
>
^ permalink raw reply
* [PATCH 0/3] fix error return code
From: Julia Lawall @ 2012-08-17 7:46 UTC (permalink / raw)
To: netdev; +Cc: kernel-janitors, linux-kernel
These patches fix cases where the return code appears to be unintentially
nonnegative.
The complete semantic match that finds the problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
identifier ret,l;
expression e1,e2,e3;
statement S;
@@
if (ret < 0)
{ ... return ret; }
... when != ret = e1
when forall
(
goto l;
|
return ...;
|
if (<+... ret = e3 ...+>) S
|
*if(...)
{
... when != ret = e2
* return ret;
}
)
// </smpl>
^ permalink raw reply
* [PATCH 1/3] drivers/net/wimax/i2400m/fw.c: fix error return code
From: Julia Lawall @ 2012-08-17 7:46 UTC (permalink / raw)
To: Inaky Perez-Gonzalez
Cc: netdev, kernel-janitors, linux-kernel, wimax, linux-wimax
In-Reply-To: <1345189618-13758-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
identifier ret;
expression e1,e2;
@@
if (ret < 0)
{ ... return ret; }
... when != ret = e1
when forall
*if(...)
{
... when != ret = e2
* return ret;
}
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/net/wimax/i2400m/fw.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
index 283237f..def12b3 100644
--- a/drivers/net/wimax/i2400m/fw.c
+++ b/drivers/net/wimax/i2400m/fw.c
@@ -326,8 +326,10 @@ int i2400m_barker_db_init(const char *_options)
unsigned barker;
options_orig = kstrdup(_options, GFP_KERNEL);
- if (options_orig == NULL)
+ if (options_orig == NULL) {
+ result = -ENOMEM;
goto error_parse;
+ }
options = options_orig;
while ((token = strsep(&options, ",")) != NULL) {
^ permalink raw reply related
* [PATCH 2/3] drivers/net/wan/dscc4.c: fix error return code
From: Julia Lawall @ 2012-08-17 7:46 UTC (permalink / raw)
To: Francois Romieu; +Cc: kernel-janitors, netdev, linux-kernel
In-Reply-To: <1345189618-13758-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Move up the initialization of rc so that failure of pci_alloc_consistent
returns -ENOMEM as well.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
identifier ret;
expression e1,e2;
@@
if (ret < 0)
{ ... return ret; }
... when != ret = e1
when forall
*if(...)
{
... when != ret = e2
* return ret;
}
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/net/wan/dscc4.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
index 9eb6479..ef36caf 100644
--- a/drivers/net/wan/dscc4.c
+++ b/drivers/net/wan/dscc4.c
@@ -774,14 +774,15 @@ static int __devinit dscc4_init_one(struct pci_dev *pdev,
}
/* Global interrupt queue */
writel((u32)(((IRQ_RING_SIZE >> 5) - 1) << 20), ioaddr + IQLENR1);
+
+ rc = -ENOMEM;
+
priv->iqcfg = (__le32 *) pci_alloc_consistent(pdev,
IRQ_RING_SIZE*sizeof(__le32), &priv->iqcfg_dma);
if (!priv->iqcfg)
goto err_free_irq_5;
writel(priv->iqcfg_dma, ioaddr + IQCFG);
- rc = -ENOMEM;
-
/*
* SCC 0-3 private rx/tx irq structures
* IQRX/TXi needs to be set soon. Learned it the hard way...
^ permalink raw reply related
* [PATCH 3/3] drivers/net/irda: fix error return code
From: Julia Lawall @ 2012-08-17 7:46 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: kernel-janitors, netdev, linux-kernel
In-Reply-To: <1345189618-13758-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
identifier ret;
expression e1,e2;
@@
if (ret < 0)
{ ... return ret; }
... when != ret = e1
when forall
*if(...)
{
... when != ret = e2
* return ret;
}
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/net/irda/ks959-sir.c | 1 +
drivers/net/irda/ksdazzle-sir.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/net/irda/ks959-sir.c b/drivers/net/irda/ks959-sir.c
index 824e2a9..5f3aeac 100644
--- a/drivers/net/irda/ks959-sir.c
+++ b/drivers/net/irda/ks959-sir.c
@@ -542,6 +542,7 @@ static int ks959_net_open(struct net_device *netdev)
sprintf(hwname, "usb#%d", kingsun->usbdev->devnum);
kingsun->irlap = irlap_open(netdev, &kingsun->qos, hwname);
if (!kingsun->irlap) {
+ err = -ENOMEM;
dev_err(&kingsun->usbdev->dev, "irlap_open failed\n");
goto free_mem;
}
diff --git a/drivers/net/irda/ksdazzle-sir.c b/drivers/net/irda/ksdazzle-sir.c
index 5a278ab..2d4b6a1 100644
--- a/drivers/net/irda/ksdazzle-sir.c
+++ b/drivers/net/irda/ksdazzle-sir.c
@@ -436,6 +436,7 @@ static int ksdazzle_net_open(struct net_device *netdev)
sprintf(hwname, "usb#%d", kingsun->usbdev->devnum);
kingsun->irlap = irlap_open(netdev, &kingsun->qos, hwname);
if (!kingsun->irlap) {
+ err = -ENOMEM;
dev_err(&kingsun->usbdev->dev, "irlap_open failed\n");
goto free_mem;
}
^ permalink raw reply related
* Re: [PATCH v0 3/5] cgroup: Protect access to task_cls_classid() when built as module
From: Daniel Wagner @ 2012-08-17 7:55 UTC (permalink / raw)
To: Li Zefan
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
Daniel Wagner, David S. Miller, Gao feng, Jamal Hadi Salim,
John Fastabend, Neil Horman, Tejun Heo
In-Reply-To: <502DF43F.9010209-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On 17.08.2012 09:35, Li Zefan wrote:
>> +#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
>> +extern struct static_key cgroup_cls_enabled;
>> +#define clscg_enabled static_key_false(&cgroup_cls_enabled)
>> +#endif
>> +
>
> If it's built-in, clscg_enabled is always true (after we call cgroup_subsys_init at
> boot), so we don't need jump label at all.
Okay. I'll update the patches so that only the module built contain the
jump labels.
daniel
^ permalink raw reply
* Re: [PATCH 0/5] Call netif_carrier_off() after register_netdev()
From: Ilya Shchepetkov @ 2012-08-17 7:55 UTC (permalink / raw)
To: bjorn
Cc: Ilya Shchepetkov, David S. Miller, Ben Hutchings, netdev,
linux-kernel, ldv-project
In-Reply-To: <87boicz8l7.fsf@nemi.mork.no>
>> Ben Hutchings <bhutchings@solarflare.com> writes:
>>> But if you do it beforehand then it doesn't have the intended effect.
>>> (Supposed to be fixed by 22604c866889c4b2e12b73cbf1683bda1b72a313, which
>>> had to be reverted: c276e098d3ee33059b4a1c747354226cec58487c.)
>>>
>>> So you have to do it after, but without dropping the RTNL lock in
>>> between.
>> So you may want to add something like
>>
>> int register_netdev_carrier_off(struct net_device *dev)
>> {
>> int err;
>>
>> rtnl_lock();
>> err = register_netdevice(dev);
>> if (!err)
>> set_bit(__LINK_STATE_NOCARRIER, &dev->state)
>> rtnl_unlock();
>> return err;
>> }
>>
>>
>> for these drivers?
t looks like this variant is equivalent to the existing code:
netif_carrier_off(dev);
err = register_netdev(dev);
if (err)
goto out;
According to explanation in commit 22604c866889c4b2e12b73cbf1683bda1b72a313,
in this case "this causes these drivers to incorrectly report their
link status as IF_OPER_UNKNOWN which can falsely set the IFF_RUNNING
flag when the interface is first brought up".
As far as I understand, to fix the issue it is required to call
netif_carrier_off() itself:
int register_netdev_carrier_off(struct net_device *dev)
{
int err;
rtnl_lock();
err = register_netdevice(dev);
if (!err)
netif_carrier_off(dev);
rtnl_unlock();
return err;
}
What do you think?
^ permalink raw reply
* Re: [PATCH] net: Fix incorrect comment in netif_tx_stop_queue()
From: Ilya Shchepetkov @ 2012-08-17 8:01 UTC (permalink / raw)
To: David S. Miller; +Cc: Ilya Shchepetkov, netdev, linux-kernel, ldv-project
In-Reply-To: <20120814.144623.1855051650444819552.davem@davemloft.net>
> > netif_stop_queue() can be called before register_netdev() because now
> > TX queues are allocated inside alloc_netdev_mqs().
> >
> > (since ed9af2e839c06c18f721da2c768fbb444c4a10e5 commit)
> >
> > Found by Linux Driver Verification project (linuxtesting.org).
> >
> > Signed-off-by: Ilya Shchepetkov <shchepetkov@ispras.ru>
>
> You're not changing a "comment", you're changing a kernel log message.
>
> Have you actually triggered this condition?
>
> I doubt it, and I'm not applying this patch.
My test driver calls netif_carrier_off before register_netdev and works
well,
without "netif_stop_queue() cannot be called before register_netdev()"
message.
Also, there are drivers that call netif_tx_stop_queue directly
and possible error message (with a reference to netif_stop_queue) will
be wrong.
Look at these commits:
e6484930d7c73d324bccda7d43d131088da697b9
18543a643fae694982c7d89c22436885f3506497
ed9af2e839c06c18f721da2c768fbb444c4a10e5
After the third commit, the second one became unnecessary.
Thus, I suggest either to remove the misleading message, or to replace
it with this:
pr_info("netif_tx_stop_queue(): dev_queue is null()\n");
^ permalink raw reply
* [RFC Patch net-next] ipv6: unify conntrack reassembly expire code with standard one
From: Cong Wang @ 2012-08-17 8:02 UTC (permalink / raw)
To: netdev
Cc: Herbert Xu, David S. Miller, Hideaki YOSHIFUJI, Patrick McHardy,
Shan Wei, Pablo Neira Ayuso, netfilter-devel, Cong Wang
Two years ago, Shan Wei tried to fix this:
http://patchwork.ozlabs.org/patch/43905/
The problem is that RFC2460 requires an ICMP Time
Exceeded -- Fragment Reassembly Time Exceeded message should be
sent to the source of that fragment, if the defragmentation
times out.
"
If insufficient fragments are received to complete reassembly of a
packet within 60 seconds of the reception of the first-arriving
fragment of that packet, reassembly of that packet must be
abandoned and all the fragments that have been received for that
packet must be discarded. If the first fragment (i.e., the one
with a Fragment Offset of zero) has been received, an ICMP Time
Exceeded -- Fragment Reassembly Time Exceeded message should be
sent to the source of that fragment.
"
As Herbert suggested, we could actually use the standard IPv6
reassembly code which follows RFC2460.
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Shan Wei <shanwei@cn.fujitsu.com>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org
Signed-off-by: Cong Wang <amwang@redhat.com>
---
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 6d01fb0..958221b 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -408,6 +408,24 @@ struct ip6_create_arg {
void ip6_frag_init(struct inet_frag_queue *q, void *a);
bool ip6_frag_match(struct inet_frag_queue *q, void *a);
+/*
+ * Equivalent of ipv4 struct ip
+ */
+struct frag_queue {
+ struct inet_frag_queue q;
+
+ __be32 id; /* fragment id */
+ u32 user;
+ struct in6_addr saddr;
+ struct in6_addr daddr;
+
+ int iif;
+ unsigned int csum;
+ __u16 nhoffset;
+};
+
+void ip6_expire_frag_queue(struct frag_queue *fq, struct inet_frags *frags);
+
static inline bool ipv6_addr_any(const struct in6_addr *a)
{
#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index c9c78c2..7c42cf1 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -57,19 +57,6 @@ struct nf_ct_frag6_skb_cb
#define NFCT_FRAG6_CB(skb) ((struct nf_ct_frag6_skb_cb*)((skb)->cb))
-struct nf_ct_frag6_queue
-{
- struct inet_frag_queue q;
-
- __be32 id; /* fragment id */
- u32 user;
- struct in6_addr saddr;
- struct in6_addr daddr;
-
- unsigned int csum;
- __u16 nhoffset;
-};
-
static struct inet_frags nf_frags;
static struct netns_frags nf_init_frags;
@@ -104,9 +91,9 @@ static struct ctl_table_header *nf_ct_frag6_sysctl_header;
static unsigned int nf_hashfn(struct inet_frag_queue *q)
{
- const struct nf_ct_frag6_queue *nq;
+ const struct frag_queue *nq;
- nq = container_of(q, struct nf_ct_frag6_queue, q);
+ nq = container_of(q, struct frag_queue, q);
return inet6_hash_frag(nq->id, &nq->saddr, &nq->daddr, nf_frags.rnd);
}
@@ -116,21 +103,6 @@ static void nf_skb_free(struct sk_buff *skb)
kfree_skb(NFCT_FRAG6_CB(skb)->orig);
}
-/* Destruction primitives. */
-
-static __inline__ void fq_put(struct nf_ct_frag6_queue *fq)
-{
- inet_frag_put(&fq->q, &nf_frags);
-}
-
-/* Kill fq entry. It is not destroyed immediately,
- * because caller (and someone more) holds reference count.
- */
-static __inline__ void fq_kill(struct nf_ct_frag6_queue *fq)
-{
- inet_frag_kill(&fq->q, &nf_frags);
-}
-
static void nf_ct_frag6_evictor(void)
{
local_bh_disable();
@@ -140,26 +112,16 @@ static void nf_ct_frag6_evictor(void)
static void nf_ct_frag6_expire(unsigned long data)
{
- struct nf_ct_frag6_queue *fq;
+ struct frag_queue *fq;
fq = container_of((struct inet_frag_queue *)data,
- struct nf_ct_frag6_queue, q);
-
- spin_lock(&fq->q.lock);
-
- if (fq->q.last_in & INET_FRAG_COMPLETE)
- goto out;
-
- fq_kill(fq);
-
-out:
- spin_unlock(&fq->q.lock);
- fq_put(fq);
+ struct frag_queue, q);
+ ip6_expire_frag_queue(fq, &nf_frags);
}
/* Creation primitives. */
-static __inline__ struct nf_ct_frag6_queue *
+static __inline__ struct frag_queue*
fq_find(__be32 id, u32 user, struct in6_addr *src, struct in6_addr *dst)
{
struct inet_frag_queue *q;
@@ -179,14 +141,14 @@ fq_find(__be32 id, u32 user, struct in6_addr *src, struct in6_addr *dst)
if (q == NULL)
goto oom;
- return container_of(q, struct nf_ct_frag6_queue, q);
+ return container_of(q, struct frag_queue, q);
oom:
return NULL;
}
-static int nf_ct_frag6_queue(struct nf_ct_frag6_queue *fq, struct sk_buff *skb,
+static int nf_ct_frag6_queue(struct frag_queue*fq, struct sk_buff *skb,
const struct frag_hdr *fhdr, int nhoff)
{
struct sk_buff *prev, *next;
@@ -322,7 +284,7 @@ found:
return 0;
discard_fq:
- fq_kill(fq);
+ inet_frag_kill(&fq->q, &nf_frags);
err:
return -1;
}
@@ -337,12 +299,12 @@ err:
* the last and the first frames arrived and all the bits are here.
*/
static struct sk_buff *
-nf_ct_frag6_reasm(struct nf_ct_frag6_queue *fq, struct net_device *dev)
+nf_ct_frag6_reasm(struct frag_queue *fq, struct net_device *dev)
{
struct sk_buff *fp, *op, *head = fq->q.fragments;
int payload_len;
- fq_kill(fq);
+ inet_frag_kill(&fq->q, &nf_frags);
WARN_ON(head == NULL);
WARN_ON(NFCT_FRAG6_CB(head)->offset != 0);
@@ -521,7 +483,7 @@ struct sk_buff *nf_ct_frag6_gather(struct sk_buff *skb, u32 user)
struct sk_buff *clone;
struct net_device *dev = skb->dev;
struct frag_hdr *fhdr;
- struct nf_ct_frag6_queue *fq;
+ struct frag_queue *fq;
struct ipv6hdr *hdr;
int fhoff, nhoff;
u8 prevhdr;
@@ -567,7 +529,7 @@ struct sk_buff *nf_ct_frag6_gather(struct sk_buff *skb, u32 user)
if (nf_ct_frag6_queue(fq, clone, fhdr, nhoff) < 0) {
spin_unlock_bh(&fq->q.lock);
pr_debug("Can't insert skb to queue\n");
- fq_put(fq);
+ inet_frag_put(&fq->q, &nf_frags);
goto ret_orig;
}
@@ -579,7 +541,7 @@ struct sk_buff *nf_ct_frag6_gather(struct sk_buff *skb, u32 user)
}
spin_unlock_bh(&fq->q.lock);
- fq_put(fq);
+ inet_frag_put(&fq->q, &nf_frags);
return ret_skb;
ret_orig:
@@ -614,7 +576,7 @@ int nf_ct_frag6_init(void)
nf_frags.constructor = ip6_frag_init;
nf_frags.destructor = NULL;
nf_frags.skb_free = nf_skb_free;
- nf_frags.qsize = sizeof(struct nf_ct_frag6_queue);
+ nf_frags.qsize = sizeof(struct frag_queue);
nf_frags.match = ip6_frag_match;
nf_frags.frag_expire = nf_ct_frag6_expire;
nf_frags.secret_interval = 10 * 60 * HZ;
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 4ff9af6..abb0d88 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -65,24 +65,6 @@ struct ip6frag_skb_cb
#define FRAG6_CB(skb) ((struct ip6frag_skb_cb*)((skb)->cb))
-/*
- * Equivalent of ipv4 struct ipq
- */
-
-struct frag_queue
-{
- struct inet_frag_queue q;
-
- __be32 id; /* fragment id */
- u32 user;
- struct in6_addr saddr;
- struct in6_addr daddr;
-
- int iif;
- unsigned int csum;
- __u16 nhoffset;
-};
-
static struct inet_frags ip6_frags;
int ip6_frag_nqueues(struct net *net)
@@ -159,21 +141,6 @@ void ip6_frag_init(struct inet_frag_queue *q, void *a)
}
EXPORT_SYMBOL(ip6_frag_init);
-/* Destruction primitives. */
-
-static __inline__ void fq_put(struct frag_queue *fq)
-{
- inet_frag_put(&fq->q, &ip6_frags);
-}
-
-/* Kill fq entry. It is not destroyed immediately,
- * because caller (and someone more) holds reference count.
- */
-static __inline__ void fq_kill(struct frag_queue *fq)
-{
- inet_frag_kill(&fq->q, &ip6_frags);
-}
-
static void ip6_evictor(struct net *net, struct inet6_dev *idev)
{
int evicted;
@@ -183,20 +150,17 @@ static void ip6_evictor(struct net *net, struct inet6_dev *idev)
IP6_ADD_STATS_BH(net, idev, IPSTATS_MIB_REASMFAILS, evicted);
}
-static void ip6_frag_expire(unsigned long data)
+void ip6_expire_frag_queue(struct frag_queue *fq, struct inet_frags *frags)
{
- struct frag_queue *fq;
struct net_device *dev = NULL;
struct net *net;
- fq = container_of((struct inet_frag_queue *)data, struct frag_queue, q);
-
spin_lock(&fq->q.lock);
if (fq->q.last_in & INET_FRAG_COMPLETE)
goto out;
- fq_kill(fq);
+ inet_frag_kill(&fq->q, frags);
net = container_of(fq->q.net, struct net, ipv6.frags);
rcu_read_lock();
@@ -222,7 +186,18 @@ out_rcu_unlock:
rcu_read_unlock();
out:
spin_unlock(&fq->q.lock);
- fq_put(fq);
+ inet_frag_put(&fq->q, frags);
+
+}
+EXPORT_SYMBOL(ip6_expire_frag_queue);
+
+static void ip6_frag_expire(unsigned long data)
+{
+ struct frag_queue *fq;
+
+ fq = container_of((struct inet_frag_queue *)data, struct frag_queue, q);
+
+ ip6_expire_frag_queue(fq, &ip6_frags);
}
static __inline__ struct frag_queue *
@@ -391,7 +366,7 @@ found:
return -1;
discard_fq:
- fq_kill(fq);
+ inet_frag_kill(&fq->q, &ip6_frags);
err:
IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
IPSTATS_MIB_REASMFAILS);
@@ -417,7 +392,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
unsigned int nhoff;
int sum_truesize;
- fq_kill(fq);
+ inet_frag_kill(&fq->q, &ip6_frags);
/* Make the one we just received the head. */
if (prev) {
@@ -586,7 +561,7 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
ret = ip6_frag_queue(fq, skb, fhdr, IP6CB(skb)->nhoff);
spin_unlock(&fq->q.lock);
- fq_put(fq);
+ inet_frag_put(&fq->q, &ip6_frags);
return ret;
}
^ permalink raw reply related
* Re: [RFC PATCH v1.0] af_packet: don't emit packet on orig fanout group
From: Ying Xue @ 2012-08-17 8:03 UTC (permalink / raw)
To: David Miller; +Cc: eric, netdev
In-Reply-To: <20120816.145200.694410419765575459.davem@davemloft.net>
David Miller wrote:
> From: Eric Leblond <eric@regit.org>
> Date: Thu, 16 Aug 2012 17:44:50 +0200
>
>
>> If a packet is emitted on one socket in one group of fanout sockets,
>> it is transmitted again. It is thus read again on one of the sockets
>> of the fanout group. This result in a loop for software which
>> generate packets when receiving one.
>> This retransmission is not the intended behavior: a fanout group
>> must behave like a single socket. The packet should not be
>> transmitted on a socket if it originates from a socket belonging
>> to the same fanout group.
>>
>> This patch fixes the issue by changing the transmission check to
>> take fanout group info account.
>>
>
> This looks mostly fine, thanks for fixing this.
>
> I wonder if it wouldn't be better to simply have a callback? That
> would eliminate all of the ifdefs:
>
> if (ptype->id_match) {
> if (ptype->id_match(ptype, skb->sk))
> return true;
> } else if (ptype->af_packet_priv == skb->sk)
> return true;
>
> It's a shame that we have a user of af_packet_priv outside of
> AF_PACKET, in TIPC. If we could get rid of that we could simplify
> things even futher.
>
Hi David, next week I will take over the job to get rid the usage of
af_packet_priv from TIPC.
Regards,
Ying
> --
> 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
* [PATCH v1.1] af_packet: don't emit packet on orig fanout group
From: Eric Leblond @ 2012-08-17 8:02 UTC (permalink / raw)
To: davem; +Cc: netdev, Eric Leblond
In-Reply-To: <20120816.145253.939105041613744695.davem@davemloft.net>
If a packet is emitted on one socket in one group of fanout sockets,
it is transmitted again. It is thus read again on one of the sockets
of the fanout group. This result in a loop for software which
generate packets when receiving one.
This retransmission is not the intended behavior: a fanout group
must behave like a single socket. The packet should not be
transmitted on a socket if it originates from a socket belonging
to the same fanout group.
This patch fixes the issue by changing the transmission check to
take fanout group info account.
Reported-by: Aleksandr Kotov <a1k@mail.ru>
Signed-off-by: Eric Leblond <eric@regit.org>
---
include/linux/netdevice.h | 2 ++
net/core/dev.c | 16 ++++++++++++++--
net/packet/af_packet.c | 9 +++++++++
3 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1d6ab69..a6060b2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1521,6 +1521,8 @@ struct packet_type {
struct sk_buff **(*gro_receive)(struct sk_buff **head,
struct sk_buff *skb);
int (*gro_complete)(struct sk_buff *skb);
+ bool (*id_match)(struct packet_type *ptype,
+ struct sock *sk);
void *af_packet_priv;
struct list_head list;
};
diff --git a/net/core/dev.c b/net/core/dev.c
index ce1bccb..05eafb2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1651,6 +1651,19 @@ static inline int deliver_skb(struct sk_buff *skb,
return pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
}
+static inline bool skb_loop_sk(struct packet_type *ptype, struct sk_buff *skb)
+{
+ if (ptype->af_packet_priv == NULL)
+ return false;
+
+ if (ptype->id_match)
+ return ptype->id_match(ptype, skb->sk);
+ else if ((struct sock *)ptype->af_packet_priv == skb->sk)
+ return true;
+
+ return false;
+}
+
/*
* Support routine. Sends outgoing frames to any network
* taps currently in use.
@@ -1668,8 +1681,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
* they originated from - MvS (miquels@drinkel.ow.org)
*/
if ((ptype->dev == dev || !ptype->dev) &&
- (ptype->af_packet_priv == NULL ||
- (struct sock *)ptype->af_packet_priv != skb->sk)) {
+ (!skb_loop_sk(ptype, skb))) {
if (pt_prev) {
deliver_skb(skb2, pt_prev, skb->dev);
pt_prev = ptype;
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 8a1605a..7a34ace 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1178,6 +1178,14 @@ static void __fanout_unlink(struct sock *sk, struct packet_sock *po)
spin_unlock(&f->lock);
}
+bool match_fanout_group(struct packet_type *ptype, struct sock * sk)
+{
+ if (ptype->af_packet_priv == (void*)((struct packet_sock *)sk)->fanout)
+ return true;
+
+ return false;
+}
+
static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
{
struct packet_sock *po = pkt_sk(sk);
@@ -1230,6 +1238,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
match->prot_hook.dev = po->prot_hook.dev;
match->prot_hook.func = packet_rcv_fanout;
match->prot_hook.af_packet_priv = match;
+ match->prot_hook.id_match = match_fanout_group;
dev_add_pack(&match->prot_hook);
list_add(&match->list, &fanout_list);
}
--
1.7.10.4
^ permalink raw reply related
* Re: smsc75xx maintainer?
From: Steve Glendinning @ 2012-08-17 8:05 UTC (permalink / raw)
To: Grant Grundler; +Cc: netdev
In-Reply-To: <CANEJEGuh++1-7HajcK65a=NPyKY_5JWkjGQFOKddv4JrVn0gFA@mail.gmail.com>
Hi Grant,
On 15 August 2012 22:48, Grant Grundler <grundler@google.com> wrote:
>
> scripts/get_maintainer.pl can't identify an owner for
> drivers/net/usb/smsc75xx.c (USB GigE dongle).
Thanks for spotting this, I've submitted a patch to add the missing
entry to MAINTAINERS
> Anyone (at SMSC?) interested in fixing suspend/resume for smsc75xx driver?
>
> I don't have time to chase this but it would be nice if this part
> worked. Only suspend/resume appears broken.
>
> Some details in http://crosbug.com/31871.
I'll look into it, thanks.
--
Steve Glendinning
^ permalink raw reply
* Re: [PATCH 05/19] netfilter: nf_conntrack_ipv6: improve fragmentation handling
From: Jesper Dangaard Brouer @ 2012-08-17 8:06 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <1344542943-11588-6-git-send-email-kaber@trash.net>
On Thu, 2012-08-09 at 22:08 +0200, kaber@trash.net wrote:
> From: Patrick McHardy <kaber@trash.net>
>
> The IPv6 conntrack fragmentation currently has a couple of shortcomings.
> Fragmentes are collected in PREROUTING/OUTPUT, are defragmented, the
> defragmented packet is then passed to conntrack, the resulting conntrack
> information is attached to each original fragment and the fragments then
> continue their way through the stack.
>
> Helper invocation occurs in the POSTROUTING hook, at which point only
> the original fragments are available. The result of this is that
> fragmented packets are never passed to helpers.
>
> This patch improves the situation in the following way:
>
> - If a reassembled packet belongs to a connection that has a helper
> assigned, the reassembled packet is passed through the stack instead
> of the original fragments.
I'm working on IPv6 fragment handling for IPVS, and are taking advantage
of the "replay" by nf_ct_frag6_output() at hook prio -399
(NF_IP6_PRI_CONNTRACK_DEFRAG + 1).
By making a hook at NF_INET_PRE_ROUTING at prio -99 (NF_IP6_PRI_NAT_DST
+ 1).
I can see that the code path can be changed (with this patch), if a
helper is assigned. Then the "replay" starts at prio -199
(NF_IP6_PRI_CONNTRACK + 1), I guess I'm safe as I run at -99.
I have tested that your patchset works, with my ipvs patches, but would
like the trigger the changed code path, to make sure.
Could you provide an iptables command/rule, that trigger this code path?
[cut]
> @@ -199,9 +200,13 @@ static unsigned int ipv6_confirm(unsigned int hooknum,
> static unsigned int __ipv6_conntrack_in(struct net *net,
> unsigned int hooknum,
> struct sk_buff *skb,
> + const struct net_device *in,
> + const struct net_device *out,
> int (*okfn)(struct sk_buff *))
> {
> struct sk_buff *reasm = skb->nfct_reasm;
> + struct nf_conn *ct;
> + enum ip_conntrack_info ctinfo;
>
> /* This packet is fragmented and has reassembled packet. */
> if (reasm) {
> @@ -213,6 +218,20 @@ static unsigned int __ipv6_conntrack_in(struct net *net,
> if (ret != NF_ACCEPT)
> return ret;
> }
> +
> + /* Conntrack helpers need the entire reassembled packet in the
> + * POST_ROUTING hook.
> + */
> + ct = nf_ct_get(reasm, &ctinfo);
> + if (ct != NULL && test_bit(IPS_HELPER_BIT, &ct->status)) {
> + nf_conntrack_get_reasm(skb);
> + NF_HOOK_THRESH(NFPROTO_IPV6, hooknum, reasm,
> + (struct net_device *)in,
> + (struct net_device *)out,
> + okfn, NF_IP6_PRI_CONNTRACK + 1);
Hook prio change to NF_IP6_PRI_CONNTRACK + 1
> + return NF_DROP_ERR(-ECANCELED);
> + }
[cut]
> @@ -592,6 +599,7 @@ void nf_ct_frag6_output(unsigned int hooknum, struct sk_buff *skb,
> int (*okfn)(struct sk_buff *))
> {
> struct sk_buff *s, *s2;
> + unsigned int ret = 0;
>
> for (s = NFCT_FRAG6_CB(skb)->orig; s;) {
> nf_conntrack_put_reasm(s->nfct_reasm);
> @@ -601,8 +609,13 @@ void nf_ct_frag6_output(unsigned int hooknum, struct sk_buff *skb,
> s2 = s->next;
> s->next = NULL;
>
> - NF_HOOK_THRESH(NFPROTO_IPV6, hooknum, s, in, out, okfn,
> - NF_IP6_PRI_CONNTRACK_DEFRAG + 1);
> + if (ret != -ECANCELED)
> + ret = NF_HOOK_THRESH(NFPROTO_IPV6, hooknum, s,
> + in, out, okfn,
> + NF_IP6_PRI_CONNTRACK_DEFRAG + 1);
Old hook prio
> + else
> + kfree_skb(s);
> +
> s = s2;
> }
> nf_conntrack_put_reasm(skb);
^ 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