Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] phy: fix phy_device_free memory leak
From: David Miller @ 2013-02-20  3:48 UTC (permalink / raw)
  To: oss; +Cc: netdev
In-Reply-To: <20130219193646.GA10851@bordel.klfree.net>

From: Petr Malat <oss@malat.biz>
Date: Tue, 19 Feb 2013 20:36:46 +0100

> From: Petr Malat <oss@malat.biz>
> 
> Fix memory leak in phy_device_free() for the case when phy_device*
> returned by phy_device_create() is not registered in the system.
> 
> Bug description:
> phy_device_create() sets name of kobject using dev_set_name(), which 
> allocates memory using kvasprintf(), but this memory isn't freed if 
> the underlying device isn't registered properly, because kobject_cleanup()
> is not called in that case. This can happen (and actually is happening on 
> our machines) if phy_device_register(), called by mdiobus_scan(), fails. 
> 
> Patch description:
> Name is freed by phy_device_free(). In the case a device is released 
> trough kobject_cleanup()->device_release()->phy_device_release(), the name
> is set to NULL and it is not freed by phy_device_free(), because it will 
> be freed later by kobject_cleanup().
> 
> Signed-off-by: Petr Malat <oss@malat.biz>
> ---
> Please put me on CC, I'm not signed into the mailing list.

I think it's cleaner to have phy_device_register() use the kobject
reference count properly.

Have it first go:

	device_initialize(&phydev->dev);

And change the device_register() to device_add().

Finally, on the failure paths, do a put_device().

No funny NULL pointer assignments and state to maintainer, just using
kobject reference counting to handle it all transparently.

^ permalink raw reply

* linux-next: manual merge of the akpm tree with the net-next tree
From: Stephen Rothwell @ 2013-02-20  6:02 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-next, linux-kernel, Sasha Levin, Cong Wang, David Miller,
	netdev

[-- Attachment #1: Type: text/plain, Size: 1067 bytes --]

Hi Andrew,

Today's linux-next merge of the akpm tree got a conflict in
net/core/dev.c between commit 900ff8c63214 ("net: move procfs code to
net/core/net-procfs.c") from the net-next tree and commit "hlist: drop
the node parameter from iterators" from the akpm tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --git a/net/core/net-procfs.c b/net/core/net-procfs.c
index 0f6bb6f..3174f19 100644
--- a/net/core/net-procfs.c
+++ b/net/core/net-procfs.c
@@ -16,12 +16,11 @@ static inline struct net_device *dev_from_same_bucket(struct seq_file *seq, loff
 {
 	struct net *net = seq_file_net(seq);
 	struct net_device *dev;
-	struct hlist_node *p;
 	struct hlist_head *h;
 	unsigned int count = 0, offset = get_offset(*pos);
 
 	h = &net->dev_name_head[get_bucket(*pos)];
-	hlist_for_each_entry_rcu(dev, p, h, name_hlist) {
+	hlist_for_each_entry_rcu(dev, h, name_hlist) {
 		if (++count == offset)
 			return dev;
 	}

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related

* Good Day
From: Shri Gandhi @ 2013-02-20  4:17 UTC (permalink / raw)




I need your assistance to transfer an abandoned $22.5 Million Dollars from India to your country. You will get more info when i receive a positive reply from you.

Best Regards,
Mr.Shri Gandhi

^ permalink raw reply

* Re: [PATCH] b43: Increase number of RX DMA slots
From: Gábor Stefanik @ 2013-02-20  6:26 UTC (permalink / raw)
  To: Larry Finger
  Cc: Julian Calaby, David Miller, David.Laight-JxhZ9S5GRejQT0dZR+AlfA,
	linville-2XuSBdqkA4R54TAoqtyWWQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, bittorf-yxHw+vPQXZDZJqsBc5GL+g,
	stable-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <512437FA.2030105-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>

On Wed, Feb 20, 2013 at 3:42 AM, Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org> wrote:
> On 02/19/2013 06:47 PM, Julian Calaby wrote:
>>
>>
>> Is it be possible to increase the number of slots at runtime? Maybe an
>> even better solution would be to keep the existing number of slots,
>> and if they run out, reset and increase incrementally to some sensible
>> maximum value.
>
>
> The number could be increased a bit, but on systems with 32-bit DMA such as
> the BCM4318, the maximum size of the ring buffer is 4KB. Even more
> importantly, each slot is allocated an skb of 2390 bytes. Even at 256 slots,
> the memory allocation is pretty large.
>
> Larry
>

Is this an issue that vendor drivers are also vulnerable to? If it is
a firmware issue, I would certainly think so.
--
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

* linux-next: build failure after merge of the akpm tree
From: Stephen Rothwell @ 2013-02-20  6:28 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-next, linux-kernel, Shahed Shaikh, David Miller, netdev,
	Rajesh Borundia, Jitendra Kalsaria

[-- Attachment #1: Type: text/plain, Size: 4450 bytes --]

Hi Andrew,

After merging the akpm tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c: In function 'qlcnic_prune_lb_filters':
drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c:607:63: error: macro "hlist_for_each_entry_safe" passed 5 arguments, but takes just 4
drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c:607:3: error: 'hlist_for_each_entry_safe' undeclared (first use in this function)
drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c:608:3: error: expected ';' before '{' token

Caused by commit c4b75b428da3 ("hlist: drop the node parameter from
iterators") interacting with commit 53643a75b147 ("qlcnic: fix ping
resumption to a VM after a live migration") from the net-next tree.

I applied this merge fix:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 20 Feb 2013 17:17:54 +1100
Subject: [PATCH] hlist-drop-the-node-parameter-from-iterators-qlnic-fix

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
index a5422cd..f89cc7a 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
@@ -604,7 +604,7 @@ void qlcnic_prune_lb_filters(struct qlcnic_adapter *adapter)
 	for (i = 0; i < adapter->rx_fhash.fbucket_size; i++) {
 		head = &(adapter->rx_fhash.fhead[i]);
 
-		hlist_for_each_entry_safe(tmp_fil, tmp_hnode, n, head, fnode)
+		hlist_for_each_entry_safe(tmp_fil, n, head, fnode)
 		{
 			time = tmp_fil->ftime;
 			if (jiffies > (QLCNIC_FILTER_AGE * HZ + time)) {
-- 
1.8.1

which then lead to these erroes:

drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c: In function 'qlcnic_add_lb_filter':
drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:182:63: error: macro "hlist_for_each_entry_safe" passed 5 arguments, but takes just 4
drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:182:3: error: 'hlist_for_each_entry_safe' undeclared (first use in this function)
drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:182:65: error: expected ';' before '{' token
drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:208:63: error: macro "hlist_for_each_entry_safe" passed 5 arguments, but takes just 4
drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:208:65: error: expected ';' before '{' token

for which I applied this merge fix patch:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 20 Feb 2013 17:25:26 +1100
Subject: [PATCH] hlist-drop-the-node-parameter-from-iterators-qlnic-2-fix

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
index 27196ed..0e63006 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
@@ -162,7 +162,7 @@ void qlcnic_add_lb_filter(struct qlcnic_adapter *adapter, struct sk_buff *skb,
 {
 	struct ethhdr *phdr = (struct ethhdr *)(skb->data);
 	struct qlcnic_filter *fil, *tmp_fil;
-	struct hlist_node *tmp_hnode, *n;
+	struct hlist_node *n;
 	struct hlist_head *head;
 	unsigned long time;
 	u64 src_addr = 0;
@@ -179,7 +179,7 @@ void qlcnic_add_lb_filter(struct qlcnic_adapter *adapter, struct sk_buff *skb,
 			 (adapter->fhash.fbucket_size - 1);
 		head = &(adapter->rx_fhash.fhead[hindex]);
 
-		hlist_for_each_entry_safe(tmp_fil, tmp_hnode, n, head, fnode) {
+		hlist_for_each_entry_safe(tmp_fil, n, head, fnode) {
 			if (!memcmp(tmp_fil->faddr, &src_addr, ETH_ALEN) &&
 			    tmp_fil->vlan_id == vlan_id) {
 				time = tmp_fil->ftime;
@@ -205,7 +205,7 @@ void qlcnic_add_lb_filter(struct qlcnic_adapter *adapter, struct sk_buff *skb,
 			 (adapter->fhash.fbucket_size - 1);
 		head = &(adapter->rx_fhash.fhead[hindex]);
 		spin_lock(&adapter->rx_mac_learn_lock);
-		hlist_for_each_entry_safe(tmp_fil, tmp_hnode, n, head, fnode) {
+		hlist_for_each_entry_safe(tmp_fil, n, head, fnode) {
 			if (!memcmp(tmp_fil->faddr, &src_addr, ETH_ALEN) &&
 			    tmp_fil->vlan_id == vlan_id) {
 				found = 1;
-- 
1.8.1

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related

* Re: [RFC PATCH] ipv6: Split from and expires field in dst_entry out of union [net-next]
From: Gao feng @ 2013-02-20  6:33 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Neil Horman, netdev, David Miller, Jiri Bohac
In-Reply-To: <1361310958.19353.164.camel@edumazet-glaptop>

Hi Eric & Neil,
On 2013/02/20 05:55, Eric Dumazet wrote:
> On Tue, 2013-02-19 at 16:49 -0500, Neil Horman wrote:
>> On Tue, Feb 19, 2013 at 01:17:45PM -0800, Eric Dumazet wrote:
>>> On Tue, 2013-02-19 at 15:28 -0500, Neil Horman wrote:
>>>
>>>>  static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
>>>>  {
>>>>  	if (!(rt->rt6i_flags & RTF_EXPIRES)) {
>>>> -		if (rt->dst.from)
>>>> -			dst_release(rt->dst.from);
>>>> +		dst_release(rt->dst.from);
>>>>  		/* dst_set_expires relies on expires == 0 
>>>>  		 * if it has not been set previously.
>>>>  		 */
>>>>  		rt->dst.expires = 0;
>>>> +		rt6->dst.from = NULL;
>>>>  	}
>>>>  
>>>
>>> Sorry you didnt really address the problem, only reduce the race window.
>>>
>> I kinda had a feeling you would say that, but the only other solution I see here
>> is to either introduce some locking to protect the from pointer, or two revert
>> the patch that introduced the from pointer alltogether, neither of which sounds
>> appealing to me.  I suppose we could use an xchng to atomically update the from
>> pointer, so there was only ever one context that was able to free it in
>> rt6_update_path.  Does that seem reasonable to you?
> 

Thanks for your research on this problem.

> I believe the setting of rt6->dst.from is safe :
> It should be done at :
> - dst creation time, when we are the only user.
> - dst destry time, when we are the only user.
> 
> We only have to do the dst_release() at the right place, when we are the
> last user of the dst.
> 
> So rt6_update_expires() should not mess with rt6->dst.from at all.
> 

How can we?
one usage of rt6_update_expires and rt6_set_expires
is changing rt6->dst.from to rt6->dst.expires, we should release the
already holded reference of rt6->dst.from.

I think maybe we should rework the commit 1716a96101c49186b
(ipv6: fix problem with expired dst cache). just force setting flag
RTF_EXPIRES and expires for the rt6 which copied from the rt6(which
is generated from RA). The only problem of this situation is this
copied rt6's expire time may be imprecise, we may receive a new
RA with a new expire time.

^ permalink raw reply

* Re: [PATCH 01/10] netfilter: make /proc/net/netfilter pernet
From: Gao feng @ 2013-02-20  6:36 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, containers, ebiederm, netdev, lve
In-Reply-To: <20130207183318.GA18359@localhost>

Hi Pablo,
On 2013/02/08 02:33, Pablo Neira Ayuso wrote:
> Hi Gao,
> 
> Thanks again for improving netns support for netfilter. Comments
> below:
> 
> On Thu, Feb 07, 2013 at 03:49:41PM +0800, Gao feng wrote:
>> Now,only init net has directroy /proc/net/netfilter,
>> this patch make this proc dentry pernet.
>>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>> ---
>>  include/net/netns/x_tables.h |  3 +++
>>  net/netfilter/core.c         | 40 ++++++++++++++++++++++++++++++++++------
>>  2 files changed, 37 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/net/netns/x_tables.h b/include/net/netns/x_tables.h
>> index c24060e..aa6a545 100644
>> --- a/include/net/netns/x_tables.h
>> +++ b/include/net/netns/x_tables.h
>> @@ -9,6 +9,9 @@ struct ebt_table;
>>  struct netns_xt {
>>  	struct list_head tables[NFPROTO_NUMPROTO];
>>  	bool notrack_deprecated_warning;
>> +#if defined CONFIG_PROC_FS
>> +	struct proc_dir_entry *proc_netfilter;
>> +#endif
> 
> This doesn't belong here to x_tables, it should be place in
> include/net/net_namespace.h.
> 

Get it,will fix it in next version.
Should I wait for the nf-next being open again?

^ permalink raw reply

* Re: linux-next: manual merge of the akpm tree with the net-next tree
From: Cong Wang @ 2013-02-20  6:43 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Andrew Morton, linux-next, linux-kernel, Sasha Levin,
	David Miller, netdev
In-Reply-To: <20130220170223.806c3c5e9f240d8bacc7c940@canb.auug.org.au>

On 02/20/2013 02:02 PM, Stephen Rothwell wrote:
> Hi Andrew,
>
> Today's linux-next merge of the akpm tree got a conflict in
> net/core/dev.c between commit 900ff8c63214 ("net: move procfs code to
> net/core/net-procfs.c") from the net-next tree and commit "hlist: drop
> the node parameter from iterators" from the akpm tree.
>
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).
>

Looks good.

Thanks, Stephen!

^ permalink raw reply

* Re: [RFC PATCH] ipv6: Split from and expires field in dst_entry out of union [net-next]
From: Eric Dumazet @ 2013-02-20  7:00 UTC (permalink / raw)
  To: Gao feng; +Cc: Neil Horman, netdev, David Miller, Jiri Bohac
In-Reply-To: <51246E43.5010202@cn.fujitsu.com>

On Wed, 2013-02-20 at 14:33 +0800, Gao feng wrote:

> How can we?
> one usage of rt6_update_expires and rt6_set_expires
> is changing rt6->dst.from to rt6->dst.expires, we should release the
> already holded reference of rt6->dst.from.
> 

Just don't union the two fields, as Neil did.

Setting the dst.expires value should not change dst.from at all.

Something like the (untested, because its too late here) patch ?

 include/net/dst.h     |   11 +++++------
 include/net/ip6_fib.h |   21 +++------------------
 net/ipv6/route.c      |    3 +--
 3 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/include/net/dst.h b/include/net/dst.h
index 3da47e0..3f31a48 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -36,13 +36,12 @@ struct dst_entry {
 	struct net_device       *dev;
 	struct  dst_ops	        *ops;
 	unsigned long		_metrics;
-	union {
-		unsigned long           expires;
-		/* point to where the dst_entry copied from */
-		struct dst_entry        *from;
-	};
+	unsigned long           expires;
+
+	/* point to where the dst_entry copied from */
+	struct dst_entry        *from;
+
 	struct dst_entry	*path;
-	void			*__pad0;
 #ifdef CONFIG_XFRM
 	struct xfrm_state	*xfrm;
 #else
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 6919a50..731b35c 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -164,33 +164,19 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
 
 static inline void rt6_clean_expires(struct rt6_info *rt)
 {
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
-		dst_release(rt->dst.from);
-
 	rt->rt6i_flags &= ~RTF_EXPIRES;
-	rt->dst.from = NULL;
+	rt->dst.expires = 0;
 }
 
 static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
 {
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
-		dst_release(rt->dst.from);
-
 	rt->rt6i_flags |= RTF_EXPIRES;
 	rt->dst.expires = expires;
 }
 
 static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
 {
-	if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-		if (rt->dst.from)
-			dst_release(rt->dst.from);
-		/* dst_set_expires relies on expires == 0 
-		 * if it has not been set previously.
-		 */
-		rt->dst.expires = 0;
-	}
-
+	rt->dst.expires = 0;
 	dst_set_expires(&rt->dst, timeout);
 	rt->rt6i_flags |= RTF_EXPIRES;
 }
@@ -199,13 +185,12 @@ static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
 {
 	struct dst_entry *new = (struct dst_entry *) from;
 
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
+	if (rt->dst.from) {
 		if (new == rt->dst.from)
 			return;
 		dst_release(rt->dst.from);
 	}
 
-	rt->rt6i_flags &= ~RTF_EXPIRES;
 	rt->dst.from = new;
 	dst_hold(new);
 }
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 515bb51..6874b6e 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -296,8 +296,7 @@ static void ip6_dst_destroy(struct dst_entry *dst)
 		in6_dev_put(idev);
 	}
 
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && dst->from)
-		dst_release(dst->from);
+	dst_release(dst->from);
 
 	if (rt6_has_peer(rt)) {
 		struct inet_peer *peer = rt6_peer_ptr(rt);

^ permalink raw reply related

* Re: [PATCH] b43: Increase number of RX DMA slots
From: Larry Finger @ 2013-02-20  7:15 UTC (permalink / raw)
  To: Gábor Stefanik
  Cc: Julian Calaby, David Miller, David.Laight, linville,
	linux-wireless, netdev, bittorf, stable
In-Reply-To: <CA+XFjipJHGTvzUx6yy1DTc74kbrPEQxM08zW=N1uf43=Q6pwdg@mail.gmail.com>

On 02/20/2013 12:26 AM, Gábor Stefanik wrote:
>
> Is this an issue that vendor drivers are also vulnerable to? If it is
> a firmware issue, I would certainly think so.

I also think so, at least if they are using the firmware version that Bastian is 
using. His logs don't have that info in them, but I certainly saw the problem on 
my BCM4312 using firmware 508.154 from 2009.

Larry

^ permalink raw reply

* Re: [PATCH] b43: Increase number of RX DMA slots
From: Bastian Bittorf @ 2013-02-20  8:15 UTC (permalink / raw)
  To: Larry Finger
  Cc: Gábor Stefanik, Julian Calaby, David Miller,
	David.Laight-JxhZ9S5GRejQT0dZR+AlfA,
	linville-2XuSBdqkA4R54TAoqtyWWQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, bittorf-yxHw+vPQXZDZJqsBc5GL+g,
	stable-u79uwXL29TY76Z2rM5mHXA,
	openwrt-devel-ZwoEplunGu0+xA9t4nZiwti2O/JbrIOy
In-Reply-To: <5124782D.3050803-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>

* Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org> [20.02.2013 08:32]:
> On 02/20/2013 12:26 AM, Gábor Stefanik wrote:
> >
> >Is this an issue that vendor drivers are also vulnerable to? If it is
> >a firmware issue, I would certainly think so.
> 
> I also think so, at least if they are using the firmware version that
> Bastian is using. His logs don't have that info in them, but I
> certainly saw the problem on my BCM4312 using firmware 508.154 from
> 2009.
 
Another test this morning with heavy downloading (but tcp only)
show slot usage auf max 204/256. we are using firmware

"version 666.2 (2011-02-23 01:15:07)" which is OpenWrt's default
for b43. see here the full logs, including minstrel output and dmesg:

http://intercity-vpn.de/files/openwrt/b43test2.dmesg.txt

if a slot needs ~2500 bytes, so 256 slot are only 640kb which seems
ok to me. ofcourse it raises the memory consumption by 500kb, but now
the router is useful 8-)

thank you! bye, bastian

--
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

* [Patch] net: fix a wrong assignment in skb_split()
From: Cong Wang @ 2013-02-20  8:51 UTC (permalink / raw)
  To: netdev; +Cc: Pravin B Shelar, David S. Miller, Cong Wang

commit c9af6db4c11ccc6c3e7f1 (net: Fix possible wrong checksum generation)
has a suspicous piece:

	-       skb_shinfo(skb1)->gso_type = skb_shinfo(skb)->gso_type;
	-
	+       skb_shinfo(skb)->tx_flags = skb_shinfo(skb1)->tx_flags & SKBTX_SHARED_FRAG;

skb1 is the new skb, therefore should be on the left side of the assignment.
This patch fixes it.

Cc: Pravin B Shelar <pshelar@nicira.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>

---
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 2a3ca33..33245ef 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2326,7 +2326,7 @@ void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len)
 {
 	int pos = skb_headlen(skb);
 
-	skb_shinfo(skb)->tx_flags = skb_shinfo(skb1)->tx_flags & SKBTX_SHARED_FRAG;
+	skb_shinfo(skb1)->tx_flags = skb_shinfo(skb)->tx_flags & SKBTX_SHARED_FRAG;
 	if (len < pos)	/* Split line is inside header. */
 		skb_split_inside_header(skb, skb1, len, pos);
 	else		/* Second chunk has no header, nothing to copy. */

^ permalink raw reply related

* Re: 3.7.8/amd64 full interrupt hangs due to iwlwifi under big nfs copies out
From: Johannes Berg @ 2013-02-20  9:12 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Marc MERLIN, David Miller, Larry.Finger, bhutchings,
	linux-wireless, netdev
In-Reply-To: <1361290909.19353.140.camel@edumazet-glaptop>

On Tue, 2013-02-19 at 08:21 -0800, Eric Dumazet wrote:
> On Tue, 2013-02-19 at 11:03 +0100, Johannes Berg wrote:
> > On Mon, 2013-02-18 at 21:17 -0800, Eric Dumazet wrote:
> > 
> > > > chrome: page allocation failure: order:1, mode:0x4020
> > > > Pid: 8730, comm: chrome Tainted: G           O 3.7.8-amd64-preempt-20121226-fixwd #1
> > > > Call Trace:
> > > >  <IRQ>  [<ffffffff810d5f38>] warn_alloc_failed+0x117/0x12c
> > 
> > > You could try to load iwlwifi with amsdu_size_8K set to 0 (disable)
> > > 
> > > It should hopefully use order-0 pages
> > 
> > It will, do that then, unfortunately it can't switch at runtime because
> > it advertised this support to the access point or clients.
> 
> What are the drawbacks of setting amsdu_size_8K to 0 by default ?

We're discussing this now, the only downside would be that we couldn't
receive 8k A-MSDUs. Thing is, practically nobody uses A-MSDU anyway, and
even when I suspect the difference between 4k and 8k won't be huge.

johannes

^ permalink raw reply

* Re: 3.7.8/amd64 full interrupt hangs due to iwlwifi under big nfs copies out
From: Johannes Berg @ 2013-02-20  9:15 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Marc MERLIN, David Miller, Larry.Finger, bhutchings,
	linux-wireless, netdev
In-Reply-To: <1361351538.8629.3.camel@jlt4.sipsolutions.net>

On Wed, 2013-02-20 at 10:12 +0100, Johannes Berg wrote:
> On Tue, 2013-02-19 at 08:21 -0800, Eric Dumazet wrote:
> > On Tue, 2013-02-19 at 11:03 +0100, Johannes Berg wrote:
> > > On Mon, 2013-02-18 at 21:17 -0800, Eric Dumazet wrote:
> > > 
> > > > > chrome: page allocation failure: order:1, mode:0x4020
> > > > > Pid: 8730, comm: chrome Tainted: G           O 3.7.8-amd64-preempt-20121226-fixwd #1
> > > > > Call Trace:
> > > > >  <IRQ>  [<ffffffff810d5f38>] warn_alloc_failed+0x117/0x12c
> > > 
> > > > You could try to load iwlwifi with amsdu_size_8K set to 0 (disable)
> > > > 
> > > > It should hopefully use order-0 pages
> > > 
> > > It will, do that then, unfortunately it can't switch at runtime because
> > > it advertised this support to the access point or clients.
> > 
> > What are the drawbacks of setting amsdu_size_8K to 0 by default ?
> 
> We're discussing this now, the only downside would be that we couldn't
> receive 8k A-MSDUs. Thing is, practically nobody uses A-MSDU anyway, and
> even when I suspect the difference between 4k and 8k won't be huge.

OTOH, this affects the protocol, and when you really can't allocate any
order-1 pages you pointed out yourself that many other things also won't
work, so I'm not really sure it makes a big difference if we change the
driver?

johannes

^ permalink raw reply

* Re: [Xen-devel] [PATCH] xen: netback: remove redundant xenvif_put
From: Ian Campbell @ 2013-02-20  9:23 UTC (permalink / raw)
  To: David Miller
  Cc: JBeulich@suse.com, drjones@redhat.com,
	xen-devel@lists.xensource.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
In-Reply-To: <20130219.130607.100487415380997622.davem@davemloft.net>

On Tue, 2013-02-19 at 18:06 +0000, David Miller wrote:
> From: Ian Campbell <Ian.Campbell@citrix.com>
> Date: Tue, 19 Feb 2013 08:58:44 +0000
> 
> > On Tue, 2013-02-19 at 08:03 +0000, Jan Beulich wrote:
> >> >>> On 19.02.13 at 06:53, David Miller <davem@davemloft.net> wrote:
> >> > From: Andrew Jones <drjones@redhat.com>
> >> > Date: Mon, 18 Feb 2013 21:29:20 +0100
> >> > 
> >> >> netbk_fatal_tx_err() calls xenvif_carrier_off(), which does
> >> >> a xenvif_put(). As callers of netbk_fatal_tx_err should only
> >> >> have one reference to the vif at this time, then the xenvif_put
> >> >> in netbk_fatal_tx_err is one too many.
> >> >> 
> >> >> Signed-off-by: Andrew Jones <drjones@redhat.com>
> >> > 
> >> > Applied.
> >> 
> >> But this is wrong from all we can tell,
> > 
> > Yes, please can this be reverted.
> 
> Done and I've annotated the revert commit message with as much
> information as possible.

Thanks, 629821d9b looks good to me.

May be worth considering the patch in
<1361281636.1051.100.camel@zakaz.uk.xensource.com> if we get many more
of these queries...

Ian.

^ permalink raw reply

* Re: [RFC PATCH] ipv6: Split from and expires field in dst_entry out of union [net-next]
From: Gao feng @ 2013-02-20  9:31 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Neil Horman, netdev, David Miller, Jiri Bohac
In-Reply-To: <1361343610.19353.177.camel@edumazet-glaptop>

On 2013/02/20 15:00, Eric Dumazet wrote:
> On Wed, 2013-02-20 at 14:33 +0800, Gao feng wrote:
> 
>> How can we?
>> one usage of rt6_update_expires and rt6_set_expires
>> is changing rt6->dst.from to rt6->dst.expires, we should release the
>> already holded reference of rt6->dst.from.
>>
> 
> Just don't union the two fields, as Neil did.
> 
> Setting the dst.expires value should not change dst.from at all.
> 

Get it,it looks good to me.we can make sure dst->from doesn't
being operated at same time.

Thanks!

^ permalink raw reply

* ASSISTANCE NEEDED
From: Amidu Hassan @ 2013-02-20  9:53 UTC (permalink / raw)



Dear Friend,
I know that this mail will come to you as a surprise as we have never met before, but need not to worry as I am contacting you independently of my investigation and no one is informed of this communication. I need your urgent assistance in transferring the sum of $11.3million immediately to your private account.The money has been here in our Bank lying dormant for years now without anybody coming for the claim of it.
I want to release the money to you as the relative to our deceased customer (the account owner) who died a long with her supposed NEXT OF KING since 31 January 2000. The Banking laws here does not allow such money to stay more than 14 years,because the money will be recalled to the Bank treasury account as unclaimed fund.
By indicating your interest I will send you the full details on how the business will be executed.
Please respond urgently and delete if you are not interested
Regards,
Mr Hassan Amidu

^ permalink raw reply

* Re: [RFC PATCH] ipv6: Split from and expires field in dst_entry out of union [net-next]
From: YOSHIFUJI Hideaki @ 2013-02-20 10:02 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Gao feng, Neil Horman, netdev, David Miller, Jiri Bohac
In-Reply-To: <1361343610.19353.177.camel@edumazet-glaptop>

Eric Dumazet wrote:
> On Wed, 2013-02-20 at 14:33 +0800, Gao feng wrote:
> 
>> How can we?
>> one usage of rt6_update_expires and rt6_set_expires
>> is changing rt6->dst.from to rt6->dst.expires, we should release the
>> already holded reference of rt6->dst.from.
>>
> 
> Just don't union the two fields, as Neil did.
> 
> Setting the dst.expires value should not change dst.from at all.
> 
> Something like the (untested, because its too late here) patch ?
> 
>  include/net/dst.h     |   11 +++++------
>  include/net/ip6_fib.h |   21 +++------------------
>  net/ipv6/route.c      |    3 +--
>  3 files changed, 9 insertions(+), 26 deletions(-)
> 
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 3da47e0..3f31a48 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -36,13 +36,12 @@ struct dst_entry {
>  	struct net_device       *dev;
>  	struct  dst_ops	        *ops;
>  	unsigned long		_metrics;
> -	union {
> -		unsigned long           expires;
> -		/* point to where the dst_entry copied from */
> -		struct dst_entry        *from;
> -	};
> +	unsigned long           expires;
> +
> +	/* point to where the dst_entry copied from */
> +	struct dst_entry        *from;
> +
>  	struct dst_entry	*path;
> -	void			*__pad0;
>  #ifdef CONFIG_XFRM
>  	struct xfrm_state	*xfrm;
>  #else

Initialize "from" in dst_alloc().

> @@ -199,13 +185,12 @@ static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
>  {
>  	struct dst_entry *new = (struct dst_entry *) from;
>  
> -	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
> +	if (rt->dst.from) {
>  		if (new == rt->dst.from)
>  			return;
>  		dst_release(rt->dst.from);
>  	}
>  
> -	rt->rt6i_flags &= ~RTF_EXPIRES;
>  	rt->dst.from = new;
>  	dst_hold(new);
>  }

rt6_set_from can work only for fresh entries.

In fact, I have very similar patch.

--yoshfuji

^ permalink raw reply

* [PATCH 1/2][net-next] gianfar: Remove superfluous gfar_new_skb()
From: Claudiu Manoil @ 2013-02-20 10:04 UTC (permalink / raw)
  To: netdev; +Cc: Paul Gortmaker, David S. Miller

gfar_new_skb() does nothing more than to call the static
gfar_alloc_skb() routine.

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/ethernet/freescale/gianfar.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 4b5e8a6..b41739b 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -110,7 +110,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev);
 static void gfar_reset_task(struct work_struct *work);
 static void gfar_timeout(struct net_device *dev);
 static int gfar_close(struct net_device *dev);
-struct sk_buff *gfar_new_skb(struct net_device *dev);
+static struct sk_buff *gfar_alloc_skb(struct net_device *dev);
 static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
 			   struct sk_buff *skb);
 static int gfar_set_mac_address(struct net_device *dev);
@@ -207,7 +207,7 @@ static int gfar_init_bds(struct net_device *ndev)
 				gfar_init_rxbdp(rx_queue, rxbdp,
 						rxbdp->bufPtr);
 			} else {
-				skb = gfar_new_skb(ndev);
+				skb = gfar_alloc_skb(ndev);
 				if (!skb) {
 					netdev_err(ndev, "Can't allocate RX buffers\n");
 					return -ENOMEM;
@@ -2626,11 +2626,6 @@ static struct sk_buff *gfar_alloc_skb(struct net_device *dev)
 	return skb;
 }
 
-struct sk_buff *gfar_new_skb(struct net_device *dev)
-{
-	return gfar_alloc_skb(dev);
-}
-
 static inline void count_errors(unsigned short status, struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
@@ -2766,7 +2761,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 		rmb();
 
 		/* Add another skb for the future */
-		newskb = gfar_new_skb(dev);
+		newskb = gfar_alloc_skb(dev);
 
 		skb = rx_queue->rx_skbuff[rx_queue->skb_currx];
 
-- 
1.6.6

^ permalink raw reply related

* [PATCH 2/2][net-next] gianfar: Bundle rx buffer allocation
From: Claudiu Manoil @ 2013-02-20 10:04 UTC (permalink / raw)
  To: netdev; +Cc: Paul Gortmaker, David S. Miller
In-Reply-To: <1361354690-9862-1-git-send-email-claudiu.manoil@freescale.com>

Use a more common and reasonable pattern for rx buffer processing.
Instead of the current method which allocates one new buffer and
processes (cleans up) the "current" buffer for each processing
iteration, use a more efficient approach that differentiates b/w the two
tasks and bundles the same individual steps together.

So, for each Rx ring we have the next_to_clean index which points to
the first received buffer and gets updated by the buffer cleanup
routine. The next_to_alloc index is updated during buffer allocation.
Once a given number of Rx buffers have been cleaned up/ processed,
rx allocation is being invoked to refill the cleaned-up BDs with new
buffers to accommodate the subsequent Rx packets. This is how the
bundling effect is achieved.

The number of unused BDs to be refilled with new allocated buffers
can be computed based on the next_to_clean and next_to_alloc indexes.
The implementation follows a commonly used pattern in other drivers too.
gfar_alloc_rx_buff() is now the rx buffer allocation routine, the only
method that updates the next_to_alloc index of a given ring. It tries
(on a best effort basis) to refill/ alloc the requested number of
buffers, and updates next_to_alloc to reflect the actual number of allocated
buffers. While it has work to do, the rx cleanup routine requests buffer
allocations for a fixed number of buffers (bundle), and when there's
nothing left to clean it makes a final request to refill the remaining
unused BDs with new buffers.

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/ethernet/freescale/gianfar.c |  126 +++++++++++++++---------------
 drivers/net/ethernet/freescale/gianfar.h |   14 +++-
 2 files changed, 73 insertions(+), 67 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index b41739b..e1ec6a7 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -110,7 +110,8 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev);
 static void gfar_reset_task(struct work_struct *work);
 static void gfar_timeout(struct net_device *dev);
 static int gfar_close(struct net_device *dev);
-static struct sk_buff *gfar_alloc_skb(struct net_device *dev);
+static void gfar_alloc_rx_buff(struct gfar_priv_rx_q *rx_queue,
+			       int alloc_cnt);
 static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
 			   struct sk_buff *skb);
 static int gfar_set_mac_address(struct net_device *dev);
@@ -163,13 +164,12 @@ static void gfar_init_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
 	bdp->lstatus = lstatus;
 }
 
-static int gfar_init_bds(struct net_device *ndev)
+static void gfar_init_bds(struct net_device *ndev)
 {
 	struct gfar_private *priv = netdev_priv(ndev);
 	struct gfar_priv_tx_q *tx_queue = NULL;
 	struct gfar_priv_rx_q *rx_queue = NULL;
 	struct txbd8 *txbdp;
-	struct rxbd8 *rxbdp;
 	int i, j;
 
 	for (i = 0; i < priv->num_tx_queues; i++) {
@@ -196,33 +196,10 @@ static int gfar_init_bds(struct net_device *ndev)
 
 	for (i = 0; i < priv->num_rx_queues; i++) {
 		rx_queue = priv->rx_queue[i];
-		rx_queue->cur_rx = rx_queue->rx_bd_base;
-		rx_queue->skb_currx = 0;
-		rxbdp = rx_queue->rx_bd_base;
-
-		for (j = 0; j < rx_queue->rx_ring_size; j++) {
-			struct sk_buff *skb = rx_queue->rx_skbuff[j];
-
-			if (skb) {
-				gfar_init_rxbdp(rx_queue, rxbdp,
-						rxbdp->bufPtr);
-			} else {
-				skb = gfar_alloc_skb(ndev);
-				if (!skb) {
-					netdev_err(ndev, "Can't allocate RX buffers\n");
-					return -ENOMEM;
-				}
-				rx_queue->rx_skbuff[j] = skb;
-
-				gfar_new_rxbdp(rx_queue, rxbdp, skb);
-			}
-
-			rxbdp++;
-		}
-
+		rx_queue->next_to_clean = rx_queue->next_to_alloc = 0;
+		gfar_alloc_rx_buff(rx_queue, GFAR_RXBD_UNUSED(rx_queue));
 	}
 
-	return 0;
 }
 
 static int gfar_alloc_skb_resources(struct net_device *ndev)
@@ -301,8 +278,7 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
 			rx_queue->rx_skbuff[j] = NULL;
 	}
 
-	if (gfar_init_bds(ndev))
-		goto cleanup;
+	gfar_init_bds(ndev);
 
 	return 0;
 
@@ -1366,10 +1342,7 @@ static int gfar_restore(struct device *dev)
 		return 0;
 	}
 
-	if (gfar_init_bds(ndev)) {
-		free_skb_resources(priv);
-		return -ENOMEM;
-	}
+	gfar_init_bds(ndev);
 
 	init_registers(ndev);
 	gfar_set_mac_address(ndev);
@@ -2612,18 +2585,46 @@ static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
 	gfar_init_rxbdp(rx_queue, bdp, buf);
 }
 
-static struct sk_buff *gfar_alloc_skb(struct net_device *dev)
+/* Best effort Rx buff allocation routine. Updates the next_to_alloc
+ * index of the given ring with the # of completed skb allocations.
+ */
+static void gfar_alloc_rx_buff(struct gfar_priv_rx_q *rx_queue,
+			       int alloc_cnt)
 {
+	struct net_device *dev = rx_queue->dev;
 	struct gfar_private *priv = netdev_priv(dev);
-	struct sk_buff *skb;
+	struct rxbd8 *bdp, *base;
+	unsigned int i;
 
-	skb = netdev_alloc_skb(dev, priv->rx_buffer_size + RXBUF_ALIGNMENT);
-	if (!skb)
-		return NULL;
+	i = rx_queue->next_to_alloc;
+	base = rx_queue->rx_bd_base;
+	bdp = &rx_queue->rx_bd_base[i];
 
-	gfar_align_skb(skb);
+	while (alloc_cnt--) {
+		struct sk_buff *skb;
 
-	return skb;
+		skb = netdev_alloc_skb(dev,
+				       priv->rx_buffer_size + RXBUF_ALIGNMENT);
+		if (unlikely(!skb))
+			break; /* try again with the next alloc request */
+
+		gfar_align_skb(skb);
+
+		rx_queue->rx_skbuff[i] = skb;
+
+		/* Setup the new RxBD */
+		gfar_new_rxbdp(rx_queue, bdp, skb);
+
+		/* Update to the next pointer */
+		bdp = next_bd(bdp, base, rx_queue->rx_ring_size);
+
+		if (unlikely(++i == rx_queue->rx_ring_size))
+			i = 0;
+	}
+
+	rx_queue->next_to_alloc = i;
+
+	return;
 }
 
 static inline void count_errors(unsigned short status, struct net_device *dev)
@@ -2746,24 +2747,26 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 	struct sk_buff *skb;
 	int pkt_len;
 	int amount_pull;
-	int howmany = 0;
+	unsigned int i;
+	int howmany = 0, cleaned_cnt = 0;
 	struct gfar_private *priv = netdev_priv(dev);
 
+	i = rx_queue->next_to_clean;
 	/* Get the first full descriptor */
-	bdp = rx_queue->cur_rx;
 	base = rx_queue->rx_bd_base;
+	bdp = &rx_queue->rx_bd_base[i];
 
 	amount_pull = priv->uses_rxfcb ? GMAC_FCB_LEN : 0;
 
 	while (!((bdp->status & RXBD_EMPTY) || (--rx_work_limit < 0))) {
-		struct sk_buff *newskb;
-
 		rmb();
 
-		/* Add another skb for the future */
-		newskb = gfar_alloc_skb(dev);
+		skb = rx_queue->rx_skbuff[i];
+		rx_queue->rx_skbuff[i] = NULL;
 
-		skb = rx_queue->rx_skbuff[rx_queue->skb_currx];
+		cleaned_cnt++;
+		if (unlikely(++i == rx_queue->rx_ring_size))
+			i = 0;
 
 		dma_unmap_single(priv->dev, bdp->bufPtr,
 				 priv->rx_buffer_size, DMA_FROM_DEVICE);
@@ -2772,15 +2775,13 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 			     bdp->length > priv->rx_buffer_size))
 			bdp->status = RXBD_LARGE;
 
-		/* We drop the frame if we failed to allocate a new buffer */
-		if (unlikely(!newskb || !(bdp->status & RXBD_LAST) ||
+		if (unlikely(!(bdp->status & RXBD_LAST) ||
 			     bdp->status & RXBD_ERR)) {
 			count_errors(bdp->status, dev);
 
-			if (unlikely(!newskb))
-				newskb = skb;
-			else if (skb)
-				dev_kfree_skb(skb);
+			/* discard faulty buffer */
+			dev_kfree_skb(skb);
+
 		} else {
 			/* Increment the number of packets */
 			rx_queue->stats.rx_packets++;
@@ -2803,21 +2804,20 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 
 		}
 
-		rx_queue->rx_skbuff[rx_queue->skb_currx] = newskb;
-
-		/* Setup the new bdp */
-		gfar_new_rxbdp(rx_queue, bdp, newskb);
+		if (unlikely(cleaned_cnt >= GFAR_RX_BUFF_ALLOC)) {
+			gfar_alloc_rx_buff(rx_queue, cleaned_cnt);
+			cleaned_cnt = 0;
+		}
 
 		/* Update to the next pointer */
 		bdp = next_bd(bdp, base, rx_queue->rx_ring_size);
 
-		/* update to point at the next skb */
-		rx_queue->skb_currx = (rx_queue->skb_currx + 1) &
-				      RX_RING_MOD_MASK(rx_queue->rx_ring_size);
 	}
+	rx_queue->next_to_clean = i;
 
-	/* Update the current rxbd pointer to be the next one */
-	rx_queue->cur_rx = bdp;
+	cleaned_cnt = GFAR_RXBD_UNUSED(rx_queue);
+	if (cleaned_cnt)
+		gfar_alloc_rx_buff(rx_queue, cleaned_cnt);
 
 	return howmany;
 }
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 63a28d2..95e5027 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -93,6 +93,9 @@ extern const char gfar_driver_version[];
 #define DEFAULT_TX_RING_SIZE	256
 #define DEFAULT_RX_RING_SIZE	256
 
+/* Rx buffer allocation bundle size */
+#define GFAR_RX_BUFF_ALLOC	16
+
 #define GFAR_RX_MAX_RING_SIZE   256
 #define GFAR_TX_MAX_RING_SIZE   256
 
@@ -964,9 +967,9 @@ struct rx_q_stats {
  *	struct gfar_priv_rx_q - per rx queue structure
  *	@rxlock: per queue rx spin lock
  *	@rx_skbuff: skb pointers
- *	@skb_currx: currently use skb pointer
  *	@rx_bd_base: First rx buffer descriptor
- *	@cur_rx: Next free rx ring entry
+ *	@next_to_alloc: index of the next buff to be alloc'd
+ *	@next_to_clean: index of the next buffer to be cleaned
  *	@qindex: index of this queue
  *	@dev: back pointer to the dev structure
  *	@rx_ring_size: Rx ring size
@@ -979,11 +982,11 @@ struct gfar_priv_rx_q {
 	struct	sk_buff ** rx_skbuff;
 	dma_addr_t rx_bd_dma_base;
 	struct	rxbd8 *rx_bd_base;
-	struct	rxbd8 *cur_rx;
+	u16 next_to_clean;
+	u16 next_to_alloc;
 	struct	net_device *dev;
 	struct gfar_priv_grp *grp;
 	struct rx_q_stats stats;
-	u16	skb_currx;
 	u16	qindex;
 	unsigned int	rx_ring_size;
 	/* RX Coalescing values */
@@ -991,6 +994,9 @@ struct gfar_priv_rx_q {
 	unsigned long rxic;
 };
 
+#define GFAR_RXBD_UNUSED(Q)	((((Q)->next_to_clean > (Q)->next_to_alloc) ? \
+	0 : (Q)->rx_ring_size) + (Q)->next_to_clean - (Q)->next_to_alloc - 1)
+
 enum gfar_irqinfo_id {
 	GFAR_TX = 0,
 	GFAR_RX = 1,
-- 
1.6.6

^ permalink raw reply related

* Re: [PATCH 1/1] net: rfkill: Fix sparse warning in rfkill-regulator.c
From: Johannes Berg @ 2013-02-20 10:08 UTC (permalink / raw)
  To: Sachin Kamat; +Cc: netdev, linux-wireless, davem, linville
In-Reply-To: <1361267868-2316-1-git-send-email-sachin.kamat@linaro.org>

On Tue, 2013-02-19 at 15:27 +0530, Sachin Kamat wrote:
> 'rfkill_regulator_ops' is used only in this file. Hence make it static.
> Silences the following warning:
> net/rfkill/rfkill-regulator.c:54:19: warning:
> symbol 'rfkill_regulator_ops' was not declared. Should it be static?

I'll take this, but it isn't going anywhere until the merge window opens
again.

johannes

^ permalink raw reply

* Re: [PATCH 1/1] net: rfkill: Fix sparse warning in rfkill-regulator.c
From: Johannes Berg @ 2013-02-20 10:09 UTC (permalink / raw)
  To: Sachin Kamat
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, linville-2XuSBdqkA4R54TAoqtyWWQ
In-Reply-To: <1361354928.8629.17.camel-8Nb76shvtaUJvtFkdXX2HixXY32XiHfO@public.gmane.org>

On Wed, 2013-02-20 at 11:08 +0100, Johannes Berg wrote:
> On Tue, 2013-02-19 at 15:27 +0530, Sachin Kamat wrote:
> > 'rfkill_regulator_ops' is used only in this file. Hence make it static.
> > Silences the following warning:
> > net/rfkill/rfkill-regulator.c:54:19: warning:
> > symbol 'rfkill_regulator_ops' was not declared. Should it be static?
> 
> I'll take this, but it isn't going anywhere until the merge window opens
> again.

Err, I mean closes, and we open -next.

johannes

--
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

* [PATCH] ipv6: fix race condition regarding dst->expires and dst->from.
From: YOSHIFUJI Hideaki @ 2013-02-20 10:17 UTC (permalink / raw)
  To: David Miller, Eric Dumazet, Neil Horman, Gao feng, netdev
  Cc: YOSHIFUJI Hideaki
In-Reply-To: <51249F42.3030908@linux-ipv6.org>

Eric Dumazet wrote:
| Some strange crashes happen in rt6_check_expired(), with access
| to random addresses.
|
| At first glance, it looks like the RTF_EXPIRES and
| stuff added in commit 1716a96101c49186b
| (ipv6: fix problem with expired dst cache)
| are racy : same dst could be manipulated at the same time
| on different cpus.
|
| At some point, our stack believes rt->dst.from contains a dst pointer,
| while its really a jiffie value (as rt->dst.expires shares the same area
| of memory)
|
| rt6_update_expires() should be fixed, or am I missing something ?
|
| CC Neil because of https://bugzilla.redhat.com/show_bug.cgi?id=892060

Because we do not have any locks for dst_entry, we cannot change
essential structure in the entry; e.g., we cannot change reference
to other entities.

To fix this issue, split 'from' and 'expires' field in dst_entry
out of union.  Once it is 'from' is assigned in the constructor,
keep the reference until the very last stage of the life time of
the object.

Of course, it is unsafe to change 'from', so make rt6_set_from simple
just for fresh entries.

Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Reported-by: Neil Horman <nhorman@tuxdriver.com>
CC: Gao Feng <gaofeng@cn.fujitsu.com>
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/dst.h     |    8 ++------
 include/net/ip6_fib.h |   38 +++++++++++---------------------------
 net/core/dst.c        |    1 +
 net/ipv6/route.c      |    8 +++-----
 4 files changed, 17 insertions(+), 38 deletions(-)

diff --git a/include/net/dst.h b/include/net/dst.h
index 3da47e0..853cda1 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -36,13 +36,9 @@ struct dst_entry {
 	struct net_device       *dev;
 	struct  dst_ops	        *ops;
 	unsigned long		_metrics;
-	union {
-		unsigned long           expires;
-		/* point to where the dst_entry copied from */
-		struct dst_entry        *from;
-	};
+	unsigned long           expires;
 	struct dst_entry	*path;
-	void			*__pad0;
+	struct dst_entry	*from;
 #ifdef CONFIG_XFRM
 	struct xfrm_state	*xfrm;
 #else
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 6919a50..6a3c8ab 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -164,50 +164,34 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
 
 static inline void rt6_clean_expires(struct rt6_info *rt)
 {
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
-		dst_release(rt->dst.from);
-
 	rt->rt6i_flags &= ~RTF_EXPIRES;
-	rt->dst.from = NULL;
 }
 
 static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
 {
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
-		dst_release(rt->dst.from);
-
-	rt->rt6i_flags |= RTF_EXPIRES;
 	rt->dst.expires = expires;
+	rt->rt6i_flags |= RTF_EXPIRES;
 }
 
-static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
+static inline void rt6_update_expires(struct rt6_info *rt0, int timeout)
 {
-	if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-		if (rt->dst.from)
-			dst_release(rt->dst.from);
-		/* dst_set_expires relies on expires == 0 
-		 * if it has not been set previously.
-		 */
-		rt->dst.expires = 0;
-	}
-
-	dst_set_expires(&rt->dst, timeout);
-	rt->rt6i_flags |= RTF_EXPIRES;
+	struct rt6_info *rt;
+
+	for (rt = rt0; rt && !(rt->rt6i_flags & RTF_EXPIRES); rt = rt->dst.from);
+	if (rt && rt != rt0)
+		rt0->dst.expires = rt->dst.expires;
+
+	dst_set_expires(&rt0->dst, timeout);
+	rt0->rt6i_flags |= RTF_EXPIRES;
 }
 
 static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
 {
 	struct dst_entry *new = (struct dst_entry *) from;
 
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
-		if (new == rt->dst.from)
-			return;
-		dst_release(rt->dst.from);
-	}
-
 	rt->rt6i_flags &= ~RTF_EXPIRES;
-	rt->dst.from = new;
 	dst_hold(new);
+	rt->dst.from = new;
 }
 
 static inline void ip6_rt_put(struct rt6_info *rt)
diff --git a/net/core/dst.c b/net/core/dst.c
index ee6153e..35fd12f 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -179,6 +179,7 @@ void *dst_alloc(struct dst_ops *ops, struct net_device *dev,
 	dst_init_metrics(dst, dst_default_metrics, true);
 	dst->expires = 0UL;
 	dst->path = dst;
+	dst->from = NULL;
 #ifdef CONFIG_XFRM
 	dst->xfrm = NULL;
 #endif
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 515bb51..9db8622 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -287,6 +287,7 @@ static void ip6_dst_destroy(struct dst_entry *dst)
 {
 	struct rt6_info *rt = (struct rt6_info *)dst;
 	struct inet6_dev *idev = rt->rt6i_idev;
+	struct dst_entry *from = dst->from;
 
 	if (!(rt->dst.flags & DST_HOST))
 		dst_destroy_metrics_generic(dst);
@@ -296,8 +297,8 @@ static void ip6_dst_destroy(struct dst_entry *dst)
 		in6_dev_put(idev);
 	}
 
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && dst->from)
-		dst_release(dst->from);
+	dst->from = NULL;
+	dst_release(dst->from);
 
 	if (rt6_has_peer(rt)) {
 		struct inet_peer *peer = rt6_peer_ptr(rt);
@@ -1010,7 +1011,6 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
 		rt->rt6i_flags = ort->rt6i_flags;
-		rt6_clean_expires(rt);
 		rt->rt6i_metric = 0;
 
 		memcpy(&rt->rt6i_dst, &ort->rt6i_dst, sizeof(struct rt6key));
@@ -1784,8 +1784,6 @@ static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 		if ((ort->rt6i_flags & (RTF_DEFAULT | RTF_ADDRCONF)) ==
 		    (RTF_DEFAULT | RTF_ADDRCONF))
 			rt6_set_from(rt, ort);
-		else
-			rt6_clean_expires(rt);
 		rt->rt6i_metric = 0;
 
 #ifdef CONFIG_IPV6_SUBTREES
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH] ipv6: fix race condition regarding dst->expires and dst->from.
From: YOSHIFUJI Hideaki @ 2013-02-20 10:25 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki
  Cc: David Miller, Eric Dumazet, Neil Horman, Gao feng, netdev
In-Reply-To: <5124A2B9.9060806@linux-ipv6.org>

YOSHIFUJI Hideaki wrote:

> -static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
> +static inline void rt6_update_expires(struct rt6_info *rt0, int timeout)
>  {
> -	if (!(rt->rt6i_flags & RTF_EXPIRES)) {
> -		if (rt->dst.from)
> -			dst_release(rt->dst.from);
> -		/* dst_set_expires relies on expires == 0 
> -		 * if it has not been set previously.
> -		 */
> -		rt->dst.expires = 0;
> -	}
> -
> -	dst_set_expires(&rt->dst, timeout);
> -	rt->rt6i_flags |= RTF_EXPIRES;
> +	struct rt6_info *rt;
> +
> +	for (rt = rt0; rt && !(rt->rt6i_flags & RTF_EXPIRES); rt = rt->dst.from);
> +	if (rt && rt != rt0)
> +		rt0->dst.expires = rt->dst.expires;
> +
> +	dst_set_expires(&rt0->dst, timeout);
> +	rt0->rt6i_flags |= RTF_EXPIRES;
>  }
>  

Oops, I sent wrong version, will resend.

--yoshfuji

^ permalink raw reply

* [PATCH V2] ipv6: fix race condition regarding dst->expires and dst->from.
From: YOSHIFUJI Hideaki @ 2013-02-20 10:29 UTC (permalink / raw)
  To: Eric Dumazet, Neil Horman, Gao Feng, netdev, davem; +Cc: yoshfuji
In-Reply-To: <5124A48F.3060907@linux-ipv6.org>

Eric Dumazet wrote:
| Some strange crashes happen in rt6_check_expired(), with access
| to random addresses.
|
| At first glance, it looks like the RTF_EXPIRES and
| stuff added in commit 1716a96101c49186b
| (ipv6: fix problem with expired dst cache)
| are racy : same dst could be manipulated at the same time
| on different cpus.
|
| At some point, our stack believes rt->dst.from contains a dst pointer,
| while its really a jiffie value (as rt->dst.expires shares the same area
| of memory)
|
| rt6_update_expires() should be fixed, or am I missing something ?
|
| CC Neil because of https://bugzilla.redhat.com/show_bug.cgi?id=892060

Because we do not have any locks for dst_entry, we cannot change
essential structure in the entry; e.g., we cannot change reference
to other entity.

To fix this issue, split 'from' and 'expires' field in dst_entry
out of union.  Once it is 'from' is assigned in the constructor,
keep the reference until the very last stage of the life time of
the object.

Of course, it is unsafe to change 'from', so make rt6_set_from simple
just for fresh entries.

Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Reported-by: Neil Horman <nhorman@tuxdriver.com>
CC: Gao Feng <gaofeng@cn.fujitsu.com>
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/dst.h     |    8 ++------
 include/net/ip6_fib.h |   39 ++++++++++++---------------------------
 net/core/dst.c        |    1 +
 net/ipv6/route.c      |    8 +++-----
 4 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/include/net/dst.h b/include/net/dst.h
index 3da47e0..853cda1 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -36,13 +36,9 @@ struct dst_entry {
 	struct net_device       *dev;
 	struct  dst_ops	        *ops;
 	unsigned long		_metrics;
-	union {
-		unsigned long           expires;
-		/* point to where the dst_entry copied from */
-		struct dst_entry        *from;
-	};
+	unsigned long           expires;
 	struct dst_entry	*path;
-	void			*__pad0;
+	struct dst_entry	*from;
 #ifdef CONFIG_XFRM
 	struct xfrm_state	*xfrm;
 #else
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 6919a50..2a601e7 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -164,50 +164,35 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
 
 static inline void rt6_clean_expires(struct rt6_info *rt)
 {
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
-		dst_release(rt->dst.from);
-
 	rt->rt6i_flags &= ~RTF_EXPIRES;
-	rt->dst.from = NULL;
 }
 
 static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
 {
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
-		dst_release(rt->dst.from);
-
-	rt->rt6i_flags |= RTF_EXPIRES;
 	rt->dst.expires = expires;
+	rt->rt6i_flags |= RTF_EXPIRES;
 }
 
-static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
+static inline void rt6_update_expires(struct rt6_info *rt0, int timeout)
 {
-	if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-		if (rt->dst.from)
-			dst_release(rt->dst.from);
-		/* dst_set_expires relies on expires == 0 
-		 * if it has not been set previously.
-		 */
-		rt->dst.expires = 0;
-	}
-
-	dst_set_expires(&rt->dst, timeout);
-	rt->rt6i_flags |= RTF_EXPIRES;
+	struct rt6_info *rt;
+
+	for (rt = rt0; rt && !(rt->rt6i_flags & RTF_EXPIRES);
+	     rt = (struct rt6_info *)rt->dst.from);
+	if (rt && rt != rt0)
+		rt0->dst.expires = rt->dst.expires;
+
+	dst_set_expires(&rt0->dst, timeout);
+	rt0->rt6i_flags |= RTF_EXPIRES;
 }
 
 static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
 {
 	struct dst_entry *new = (struct dst_entry *) from;
 
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
-		if (new == rt->dst.from)
-			return;
-		dst_release(rt->dst.from);
-	}
-
 	rt->rt6i_flags &= ~RTF_EXPIRES;
-	rt->dst.from = new;
 	dst_hold(new);
+	rt->dst.from = new;
 }
 
 static inline void ip6_rt_put(struct rt6_info *rt)
diff --git a/net/core/dst.c b/net/core/dst.c
index ee6153e..35fd12f 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -179,6 +179,7 @@ void *dst_alloc(struct dst_ops *ops, struct net_device *dev,
 	dst_init_metrics(dst, dst_default_metrics, true);
 	dst->expires = 0UL;
 	dst->path = dst;
+	dst->from = NULL;
 #ifdef CONFIG_XFRM
 	dst->xfrm = NULL;
 #endif
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 515bb51..9282665 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -287,6 +287,7 @@ static void ip6_dst_destroy(struct dst_entry *dst)
 {
 	struct rt6_info *rt = (struct rt6_info *)dst;
 	struct inet6_dev *idev = rt->rt6i_idev;
+	struct dst_entry *from = dst->from;
 
 	if (!(rt->dst.flags & DST_HOST))
 		dst_destroy_metrics_generic(dst);
@@ -296,8 +297,8 @@ static void ip6_dst_destroy(struct dst_entry *dst)
 		in6_dev_put(idev);
 	}
 
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && dst->from)
-		dst_release(dst->from);
+	dst->from = NULL;
+	dst_release(from);
 
 	if (rt6_has_peer(rt)) {
 		struct inet_peer *peer = rt6_peer_ptr(rt);
@@ -1010,7 +1011,6 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
 		rt->rt6i_flags = ort->rt6i_flags;
-		rt6_clean_expires(rt);
 		rt->rt6i_metric = 0;
 
 		memcpy(&rt->rt6i_dst, &ort->rt6i_dst, sizeof(struct rt6key));
@@ -1784,8 +1784,6 @@ static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 		if ((ort->rt6i_flags & (RTF_DEFAULT | RTF_ADDRCONF)) ==
 		    (RTF_DEFAULT | RTF_ADDRCONF))
 			rt6_set_from(rt, ort);
-		else
-			rt6_clean_expires(rt);
 		rt->rt6i_metric = 0;
 
 #ifdef CONFIG_IPV6_SUBTREES
-- 
1.7.9.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox