Netdev List
 help / color / mirror / Atom feed
* Re: Add NAPI support to ll_temac driver
From: Michal Simek @ 2011-04-20 11:06 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Eric Dumazet, netdev
In-Reply-To: <1303218898.3464.59.camel@localhost>

Hi,

Ben Hutchings wrote:
> On Tue, 2011-04-19 at 14:48 +0200, Michal Simek wrote:
>> Ben Hutchings wrote:
>>> On Tue, 2011-04-19 at 12:43 +0200, Eric Dumazet wrote:
>>> [...]
>>>> One possible way to get better performance is to change driver to
>>>> allocate skbs only right before calling netif_rx(), so that you dont
>>>> have to access cold sk_buff data twice (once when allocating skb and put
>>>> it in ring buffer, a second time when receiving frame)
>>>>
>>>> drivers/net/niu.c is a good example for this (NAPI + netdev_alloc_skb()
>>>> just in time + pull in skbhead only first cache line of packet)
>>> [...]
>>>
>>> If the hardware can do RX checksumming (it's not clear) then the driver
>>> should pass the paged buffers into GRO and that will take care of skb
>>> allocation as necessary.
>> Hardware supports RX and TX partial checksumming. I can enable it. The driver 
>> has also this option and from my tests there is of course some performance 
>> improvemetn.
>>
>> Just for sure - here are links on documentation.
>> http://www.xilinx.com/support/documentation/ip_documentation/xps_ll_temac.pdf
>> or
>> http://www.xilinx.com/support/documentation/ip_documentation/axi_ethernet/v2_01_a/ds759_axi_ethernet.pdf
> 
> I'm not going to read those.  Just providing brief advice.
> 
>> About SKB allocation. I fixed our non mainline driver to allocate skb based on 
>> current mtu size. Mainline driver allocate max mtu (9k). This has also impact on 
>> performance because Microblaze works with smaller SKBs.
>>
>> Can you please be more specific about passing the paged buffers into GRO?
>> Or point me to any documentation or code which can help me to understand what 
>> that means.
> 
> You would use napi_get_frags() to get a new or recycled skb, fill in
> skb->frags, then call napi_gro_frags() to pass it into GRO.  The benet,
> cxgb3 and sfc drivers do this.

I have measured TX path and I have found that driver design is not so good.
It is always create one BD for one SKB and it starts DMA to copy packet to 
controller and send it. On 66MHz cpu it takes approximately 800 cpu cycles (not 
800 instructions) for sending (1.5k packet).
Current driver also enable irq for TX and when the packet is send interrupt is 
generated and skb is freed.
I see that it takes more time to handle the IRQ than busy waiting when DMA is 
done. I looked at sfc driver and there is any TX queue and any notifier. Hos 
does it work? Is it required to have any hw support?

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

^ permalink raw reply

* Re: [patch net-next-2.6] bonding: move processing of recv handlers into handle_frame()
From: Jiri Pirko @ 2011-04-20 11:30 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev, davem, shemminger, kaber, fubar, nicolas.2p.debian, andy
In-Reply-To: <1303221752.3480.217.camel@edumazet-laptop>

Tue, Apr 19, 2011 at 04:02:32PM CEST, eric.dumazet@gmail.com wrote:
>Le mardi 19 avril 2011 à 15:48 +0200, Jiri Pirko a écrit :
>> Since now when bonding uses rx_handler, all traffic going into bond
>> device goes thru bond_handle_frame. So there's no need to go back into
>> bonding code later via ptype handlers. This patch converts
>> original ptype handlers into "bonding receive probes". These functions
>> are called from bond_handle_frame and they are registered per-mode.
>> 
>> Note that vlan packets are also handled because they are always untagged
>> thanks to vlan_untag()
>> 
>> Note that this also allows arpmon for eth-bond-bridge-vlan topology.
>> 
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>
>
>
>>  			}
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index 6126c6a..85fb822 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -226,6 +226,8 @@ struct bonding {
>>  	struct   slave *primary_slave;
>>  	bool     force_primary;
>>  	s32      slave_cnt; /* never change this value outside the attach/detach wrappers */
>> +	void     (*recv_probe)(struct sk_buff *, struct bonding *,
>> +			       struct slave *);
>>  	rwlock_t lock;
>>  	rwlock_t curr_slave_lock;
>>  	s8       kill_timers;
>
>
>Any performance numbers ?

Getting these. (Hitting nasty panic on another machine running pktgen @
vlan dev).

>
>I am asking because recv_probe sits in a often dirtied cache line...
>
>It would be nice to separate rx & tx path needs
>
>
>

^ permalink raw reply

* can: add missing socket check in can/raw release.
From: Oliver Hartkopp @ 2011-04-20 11:46 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Dave Jones

We can get here with a NULL socket argument passed from userspace,
so we need to handle it accordingly.

Thanks to Dave Jones pointing at this issue in net/can/bcm.c

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

---

diff --git a/net/can/raw.c b/net/can/raw.c
index 649acfa..8f215e6 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -305,7 +305,12 @@ static int raw_init(struct sock *sk)
 static int raw_release(struct socket *sock)
 {
 	struct sock *sk = sock->sk;
-	struct raw_sock *ro = raw_sk(sk);
+	struct raw_sock *ro;
+
+	if(!sk)
+		return 0;
+
+	ro = raw_sk(sk);

 	unregister_netdevice_notifier(&ro->notifier);


^ permalink raw reply related

* [PATCH v2] can: add missing socket check in can/raw release
From: Oliver Hartkopp @ 2011-04-20 11:57 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Dave Jones

v2: added space after 'if' according code style.

We can get here with a NULL socket argument passed from userspace,
so we need to handle it accordingly.

Thanks to Dave Jones pointing at this issue in net/can/bcm.c

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

---

diff --git a/net/can/raw.c b/net/can/raw.c
index 649acfa..8f215e6 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -305,7 +305,12 @@ static int raw_init(struct sock *sk)
 static int raw_release(struct socket *sock)
 {
 	struct sock *sk = sock->sk;
-	struct raw_sock *ro = raw_sk(sk);
+	struct raw_sock *ro;
+
+	if (!sk)
+		return 0;
+
+	ro = raw_sk(sk);

 	unregister_netdevice_notifier(&ro->notifier);


^ permalink raw reply related

* Re: [PATCH 3/3] IPVS: init and cleanup restructuring.
From: Hans Schillstrom @ 2011-04-20 12:00 UTC (permalink / raw)
  To: Simon Horman
  Cc: ja, ebiederm, lvs-devel, netdev, netfilter-devel,
	hans.schillstrom
In-Reply-To: <20110419231234.GE6418@verge.net.au>

On Wednesday, April 20, 2011 01:12:34 Simon Horman wrote:
> On Tue, Apr 19, 2011 at 05:25:05PM +0200, Hans Schillstrom wrote:
> > This patch tries to restore the initial init and cleanup
> > sequences that was before name space patch.
[snip]
> perhaps enable or active would be names that fits better with the
> schemantics used.  Using a bool might also make things more obvious.

I'll use enable
> 
[snip]
> 
> Can we just remove ip_vs_app_init() and ip_vs_app_cleanup() as
> they no longer do anything? Likewise with other init and cleanup
> functions below.

I will add a "final" patch that removes empty functions,
(They are nice to have during the review, to keep track of the order in different contexts)

> 
> > diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> > index 36cd5ea..f8d6702 100644
> > --- a/net/netfilter/ipvs/ip_vs_conn.c
> > +++ b/net/netfilter/ipvs/ip_vs_conn.c
> > @@ -1251,30 +1251,30 @@ int __net_init __ip_vs_conn_init(struct net *net)
> >  {
> >  	struct netns_ipvs *ipvs = net_ipvs(net);
> > 
> > +	EnterFunction(2);
> >  	atomic_set(&ipvs->conn_count, 0);
> > 
> >  	proc_net_fops_create(net, "ip_vs_conn", 0, &ip_vs_conn_fops);
> >  	proc_net_fops_create(net, "ip_vs_conn_sync", 0, &ip_vs_conn_sync_fops);
> > +	LeaveFunction(2);
> >  	return 0;
> >  }
> 
> Does adding these EnterFunction() and LeaveFunction() calls
> restore some previous behaviour? If not, I think they should at the very
> least be in a separate patch. Likewise for similar changes below.
> 

I can remove them if you want, (but they are nice for debugging)

[snip]
> 
> While I do prefer labels to be in column 0, putting those changes
> here is rather a lot of noise. Could you put them in a separate patch?

OK it will be patch no 1 later on

Regards
Hans

^ permalink raw reply

* Re: Add NAPI support to ll_temac driver
From: Ben Hutchings @ 2011-04-20 12:47 UTC (permalink / raw)
  To: monstr; +Cc: Eric Dumazet, netdev
In-Reply-To: <4DAEBE44.4060801@monstr.eu>

On Wed, 2011-04-20 at 13:06 +0200, Michal Simek wrote:
[...]
> I have measured TX path and I have found that driver design is not so good.
> It is always create one BD for one SKB and it starts DMA to copy packet to 
> controller and send it.

You will always get a single packet at a time to push to the hardware.
The only improvement you can make on this is to implement segmentation
offload, but if the hardware doesn't do this then... well, it's not
easy.

> On 66MHz cpu it takes approximately 800 cpu cycles (not 
> 800 instructions) for sending (1.5k packet).
> Current driver also enable irq for TX and when the packet is send interrupt is 
> generated and skb is freed.
> I see that it takes more time to handle the IRQ than busy waiting when DMA is 
> done. I looked at sfc driver and there is any TX queue and any notifier. Hos 
> does it work? Is it required to have any hw support?

The principle of NAPI is that once you receive an IRQ you mask it and
poll until there are no more completions to handle.  So it greatly
reduces this IRQ overhead.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Christoph Lameter @ 2011-04-20 14:04 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Andrew Morton, netdev, bugzilla-daemon, bugme-daemon,
	casteyde.christian, Vegard Nossum, Pekka Enberg
In-Reply-To: <1303275858.2756.8.camel@edumazet-laptop>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1001 bytes --]

On Wed, 20 Apr 2011, Eric Dumazet wrote:

> Le mardi 19 avril 2011 à 16:18 -0500, Christoph Lameter a écrit :
> > On Tue, 19 Apr 2011, Eric Dumazet wrote:
> >
> > > > Ugg.. Isnt there some way to indicate to kmemcheck that a speculative
> > > > access is occurring?
> > >
> > > Yes, here is a totally untested patch (only compiled here), to keep
> > > kmemcheck & cmpxchg_double together.
> >
> > Ok looks somewhat saner. Why does DEBUG_PAGEALLOC need this? Pages are
> > never freed as long as a page is frozen and a page needs to be frozen to
> > be allocated from. I dont see how we could reference a free page.
>
> We run lockless and IRQ enabled, are you sure we cannot at this point :
>
> CPU0 - Receive an IRQ
> CPU0 - Allocate this same object consume the page(s) containing this object
> CPU0 - Pass this object/memory to another cpu1
> CPU1 - Free memory and free the page(s)
> CPU0 - Return from IRQ
> CPU0 - Access now unreachable memory ?

True this can occur.

^ permalink raw reply

* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Christoph Lameter @ 2011-04-20 14:05 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Pekka Enberg, casteyde.christian, Andrew Morton, netdev,
	bugzilla-daemon, bugme-daemon, Vegard Nossum
In-Reply-To: <1303293765.3186.74.camel@edumazet-laptop>

On Wed, 20 Apr 2011, Eric Dumazet wrote:

> > Then, just disable SLUB_CMPXCHG_DOUBLE if KMEMCHECK is defined, as I did in my first patch.

Ok your first patch seems to be the sanest approach.

>  {
> @@ -1889,16 +1895,18 @@ static __always_inline void *slab_alloc(struct kmem_cache *s,
>  	struct kmem_cache_cpu *c;
>  #ifdef CONFIG_CMPXCHG_LOCAL
>  	unsigned long tid;
> -#else
> +#endif
> +#ifdef MASK_IRQ_IN_SLAB_ALLOC
>  	unsigned long flags;
>  #endif
>

Yea well that does not bring us much.

^ permalink raw reply

* [PATCH] staging: octeon-ethernet: remove .get_sg, etc. ethtool_ops
From: Michał Mirosław @ 2011-04-20 14:25 UTC (permalink / raw)
  To: netdev; +Cc: Greg Kroah-Hartman, devel

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/staging/octeon/ethernet-mdio.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-mdio.c b/drivers/staging/octeon/ethernet-mdio.c
index 10a82ef..8a11ffc 100644
--- a/drivers/staging/octeon/ethernet-mdio.c
+++ b/drivers/staging/octeon/ethernet-mdio.c
@@ -91,8 +91,6 @@ const struct ethtool_ops cvm_oct_ethtool_ops = {
 	.set_settings = cvm_oct_set_settings,
 	.nway_reset = cvm_oct_nway_reset,
 	.get_link = ethtool_op_get_link,
-	.get_sg = ethtool_op_get_sg,
-	.get_tx_csum = ethtool_op_get_tx_csum,
 };
 
 /**
-- 
1.7.2.5


^ permalink raw reply related

* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Eric Dumazet @ 2011-04-20 14:26 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, casteyde.christian, Andrew Morton, netdev,
	bugzilla-daemon, bugme-daemon, Vegard Nossum
In-Reply-To: <alpine.DEB.2.00.1104200904440.8634@router.home>

Le mercredi 20 avril 2011 à 09:05 -0500, Christoph Lameter a écrit :
> On Wed, 20 Apr 2011, Eric Dumazet wrote:
> 
> > > Then, just disable SLUB_CMPXCHG_DOUBLE if KMEMCHECK is defined, as I did in my first patch.
> 
> Ok your first patch seems to be the sanest approach.
> 
> >  {
> > @@ -1889,16 +1895,18 @@ static __always_inline void *slab_alloc(struct kmem_cache *s,
> >  	struct kmem_cache_cpu *c;
> >  #ifdef CONFIG_CMPXCHG_LOCAL
> >  	unsigned long tid;
> > -#else
> > +#endif
> > +#ifdef MASK_IRQ_IN_SLAB_ALLOC
> >  	unsigned long flags;
> >  #endif
> >
> 
> Yea well that does not bring us much.

Well, we keep the fast free path ?

only slab_alloc() would have to disable irqs for ~20 instructions.




^ permalink raw reply

* Re: [PATCH] staging: octeon-ethernet: remove .get_sg, etc. ethtool_ops
From: Greg KH @ 2011-04-20 14:36 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev, devel
In-Reply-To: <20110420142553.D497813909@rere.qmqm.pl>

On Wed, Apr 20, 2011 at 04:25:53PM +0200, Michał Mirosław wrote:
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>  drivers/staging/octeon/ethernet-mdio.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/octeon/ethernet-mdio.c b/drivers/staging/octeon/ethernet-mdio.c
> index 10a82ef..8a11ffc 100644
> --- a/drivers/staging/octeon/ethernet-mdio.c
> +++ b/drivers/staging/octeon/ethernet-mdio.c
> @@ -91,8 +91,6 @@ const struct ethtool_ops cvm_oct_ethtool_ops = {
>  	.set_settings = cvm_oct_set_settings,
>  	.nway_reset = cvm_oct_nway_reset,
>  	.get_link = ethtool_op_get_link,
> -	.get_sg = ethtool_op_get_sg,
> -	.get_tx_csum = ethtool_op_get_tx_csum,
>  };

Why are you doing this?  Please explain it better in the changelog and
resend this.

thanks,

greg k-h

^ permalink raw reply

* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Christoph Lameter @ 2011-04-20 14:42 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Pekka Enberg, casteyde.christian, Andrew Morton, netdev,
	bugzilla-daemon, bugme-daemon, Vegard Nossum
In-Reply-To: <1303309591.3186.84.camel@edumazet-laptop>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 996 bytes --]

On Wed, 20 Apr 2011, Eric Dumazet wrote:

> Le mercredi 20 avril 2011 à 09:05 -0500, Christoph Lameter a écrit :
> > On Wed, 20 Apr 2011, Eric Dumazet wrote:
> >
> > > > Then, just disable SLUB_CMPXCHG_DOUBLE if KMEMCHECK is defined, as I did in my first patch.
> >
> > Ok your first patch seems to be the sanest approach.
> >
> > >  {
> > > @@ -1889,16 +1895,18 @@ static __always_inline void *slab_alloc(struct kmem_cache *s,
> > >  	struct kmem_cache_cpu *c;
> > >  #ifdef CONFIG_CMPXCHG_LOCAL
> > >  	unsigned long tid;
> > > -#else
> > > +#endif
> > > +#ifdef MASK_IRQ_IN_SLAB_ALLOC
> > >  	unsigned long flags;
> > >  #endif
> > >
> >
> > Yea well that does not bring us much.
>
> Well, we keep the fast free path ?
>
> only slab_alloc() would have to disable irqs for ~20 instructions.

Avoiding the irq handling yields the savings that improve the fastpath. if
you do both then there is only a regression left. So lets go with
disabling the CMPXCHG_LOCAL.

^ permalink raw reply

* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Eric Dumazet @ 2011-04-20 15:01 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, casteyde.christian, Andrew Morton, netdev,
	bugzilla-daemon, bugme-daemon, Vegard Nossum
In-Reply-To: <alpine.DEB.2.00.1104200942020.9266@router.home>

Le mercredi 20 avril 2011 à 09:42 -0500, Christoph Lameter a écrit :

> Avoiding the irq handling yields the savings that improve the fastpath. if
> you do both then there is only a regression left. So lets go with
> disabling the CMPXCHG_LOCAL.

OK, let's do that then.

Thanks

[PATCH v4] slub: dont use cmpxchg_double if KMEMCHECK or DEBUG_PAGEALLOC

Christian Casteyde reported a KMEMCHECK splat in slub code.

Problem is now we are lockless and allow IRQ in slab_alloc(), the object
we manipulate from freelist can be allocated and freed right before we
try to read object->next.

Same problem can happen with DEBUG_PAGEALLOC

Just dont use cmpxchg_double() if either CONFIG_KMEMCHECK or
CONFIG_DEBUG_PAGEALLOC is defined.

Reported-by: Christian Casteyde <casteyde.christian@free.fr>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Vegard Nossum <vegardno@ifi.uio.no>
Cc: Christoph Lameter <cl@linux.com>
---
 mm/slub.c |   45 +++++++++++++++++++++++++--------------------
 1 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 94d2a33..f31ab2c 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1540,7 +1540,12 @@ static void unfreeze_slab(struct kmem_cache *s, struct page *page, int tail)
 	}
 }
 
-#ifdef CONFIG_CMPXCHG_LOCAL
+#if defined(CONFIG_CMPXCHG_LOCAL) && !defined(CONFIG_KMEMCHECK) && \
+				     !defined(CONFIG_DEBUG_PAGEALLOC)
+#define SLUB_USE_CMPXCHG_DOUBLE
+#endif
+
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 #ifdef CONFIG_PREEMPT
 /*
  * Calculate the next globally unique transaction for disambiguiation
@@ -1604,7 +1609,7 @@ static inline void note_cmpxchg_failure(const char *n,
 
 void init_kmem_cache_cpus(struct kmem_cache *s)
 {
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	int cpu;
 
 	for_each_possible_cpu(cpu)
@@ -1643,7 +1648,7 @@ static void deactivate_slab(struct kmem_cache *s, struct kmem_cache_cpu *c)
 		page->inuse--;
 	}
 	c->page = NULL;
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	c->tid = next_tid(c->tid);
 #endif
 	unfreeze_slab(s, page, tail);
@@ -1780,7 +1785,7 @@ static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
 {
 	void **object;
 	struct page *new;
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	unsigned long flags;
 
 	local_irq_save(flags);
@@ -1819,7 +1824,7 @@ load_freelist:
 	c->node = page_to_nid(c->page);
 unlock_out:
 	slab_unlock(c->page);
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	c->tid = next_tid(c->tid);
 	local_irq_restore(flags);
 #endif
@@ -1858,7 +1863,7 @@ new_slab:
 	}
 	if (!(gfpflags & __GFP_NOWARN) && printk_ratelimit())
 		slab_out_of_memory(s, gfpflags, node);
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	local_irq_restore(flags);
 #endif
 	return NULL;
@@ -1887,7 +1892,7 @@ static __always_inline void *slab_alloc(struct kmem_cache *s,
 {
 	void **object;
 	struct kmem_cache_cpu *c;
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	unsigned long tid;
 #else
 	unsigned long flags;
@@ -1896,7 +1901,7 @@ static __always_inline void *slab_alloc(struct kmem_cache *s,
 	if (slab_pre_alloc_hook(s, gfpflags))
 		return NULL;
 
-#ifndef CONFIG_CMPXCHG_LOCAL
+#ifndef SLUB_USE_CMPXCHG_DOUBLE
 	local_irq_save(flags);
 #else
 redo:
@@ -1910,7 +1915,7 @@ redo:
 	 */
 	c = __this_cpu_ptr(s->cpu_slab);
 
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	/*
 	 * The transaction ids are globally unique per cpu and per operation on
 	 * a per cpu queue. Thus they can be guarantee that the cmpxchg_double
@@ -1927,7 +1932,7 @@ redo:
 		object = __slab_alloc(s, gfpflags, node, addr, c);
 
 	else {
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 		/*
 		 * The cmpxchg will only match if there was no additional
 		 * operation and if we are on the right processor.
@@ -1954,7 +1959,7 @@ redo:
 		stat(s, ALLOC_FASTPATH);
 	}
 
-#ifndef CONFIG_CMPXCHG_LOCAL
+#ifndef SLUB_USE_CMPXCHG_DOUBLE
 	local_irq_restore(flags);
 #endif
 
@@ -2034,7 +2039,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
 {
 	void *prior;
 	void **object = (void *)x;
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	unsigned long flags;
 
 	local_irq_save(flags);
@@ -2070,7 +2075,7 @@ checks_ok:
 
 out_unlock:
 	slab_unlock(page);
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	local_irq_restore(flags);
 #endif
 	return;
@@ -2084,7 +2089,7 @@ slab_empty:
 		stat(s, FREE_REMOVE_PARTIAL);
 	}
 	slab_unlock(page);
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	local_irq_restore(flags);
 #endif
 	stat(s, FREE_SLAB);
@@ -2113,7 +2118,7 @@ static __always_inline void slab_free(struct kmem_cache *s,
 {
 	void **object = (void *)x;
 	struct kmem_cache_cpu *c;
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	unsigned long tid;
 #else
 	unsigned long flags;
@@ -2121,7 +2126,7 @@ static __always_inline void slab_free(struct kmem_cache *s,
 
 	slab_free_hook(s, x);
 
-#ifndef CONFIG_CMPXCHG_LOCAL
+#ifndef SLUB_USE_CMPXCHG_DOUBLE
 	local_irq_save(flags);
 
 #else
@@ -2136,7 +2141,7 @@ redo:
 	 */
 	c = __this_cpu_ptr(s->cpu_slab);
 
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	tid = c->tid;
 	barrier();
 #endif
@@ -2144,7 +2149,7 @@ redo:
 	if (likely(page == c->page && c->node != NUMA_NO_NODE)) {
 		set_freepointer(s, object, c->freelist);
 
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 		if (unlikely(!this_cpu_cmpxchg_double(
 				s->cpu_slab->freelist, s->cpu_slab->tid,
 				c->freelist, tid,
@@ -2160,7 +2165,7 @@ redo:
 	} else
 		__slab_free(s, page, x, addr);
 
-#ifndef CONFIG_CMPXCHG_LOCAL
+#ifndef SLUB_USE_CMPXCHG_DOUBLE
 	local_irq_restore(flags);
 #endif
 }
@@ -2354,7 +2359,7 @@ static inline int alloc_kmem_cache_cpus(struct kmem_cache *s)
 	BUILD_BUG_ON(PERCPU_DYNAMIC_EARLY_SIZE <
 			SLUB_PAGE_SHIFT * sizeof(struct kmem_cache_cpu));
 
-#ifdef CONFIG_CMPXCHG_LOCAL
+#ifdef SLUB_USE_CMPXCHG_DOUBLE
 	/*
 	 * Must align to double word boundary for the double cmpxchg instructions
 	 * to work.



^ permalink raw reply related

* Re: [PATCH] bnx2x: dont dereference tcp header on non tcp frames
From: Dmitry Kravkov @ 2011-04-20 15:08 UTC (permalink / raw)
  To: David Miller
  Cc: eric.dumazet@gmail.com, Eilon Greenstein, netdev@vger.kernel.org
In-Reply-To: <1303292410.1813.23.camel@lb-tlvb-dmitry>

On Wed, 2011-04-20 at 02:40 -0700, Dmitry Kravkov wrote:

> Following patch fixes udp checksum offload flow and also addresses
> issues raised by Eric. We are testing it now.

It passed local regression, i was also unable to reproduce kmemcheck
warning.

I hit this one (on net-2.6 from today) not sure if it's related:


INFO: rcu_sched_state detected stall on CPU 0 (t=60000 jiffies)
sending NMI to all CPUs:
NMI backtrace for cpu 0
CPU 0 
Modules linked in: bnx2x nfs fscache nfsd nfs_acl auth_rpcgss autofs4
bluetooth rfkill lockd sunrpc ipv6 loop dm_mirror dm_multipath scsi_dh
video sbs sbshc power_meter battery acpi_memhotplug ac parport_pc lp
parport ide_cd_mod cdrom serio_raw option usb_wwan usbserial button b
nx2 tpm_tis tpm tpm_bios ipmi_si ipmi_msghandler hpilo mdio rtc_cmos
pcspkr rtc_core i5k_amb hwmon i5000_edac edac_core rtc_lib
dm_region_hash dm_log dm_mod ata_piix libata shpchp cciss sd_mod
scsi_mod ext3 jbd uhci_hcd ohci_hcd ehci_hcd [last unloaded: bnx2x]

Pid: 3441, comm: kworker/0:3 Not tainted 2.6.39-rc2test+ #2 HP ProLiant
DL380 G5
RIP: 0010:[<ffffffff811cbf65>]  [<ffffffff811cbf65>] delay_tsc+0x15/0x60
RSP: 0018:ffff88023fc03e78  EFLAGS: 00000803
RAX: 00000000ddf79987 RBX: 0000000000000001 RCX: ffffffff817894e0
RDX: 00000000000001aa RSI: 00000000000000ff RDI: 00000000002dc788
RBP: ffff88023fc03e78 R08: 0000000000000003 R09: 0000000000000000
R10: 0000000000000000 R11: 000000000000000a R12: ffff88023fc0e5e0
R13: ffff88023497cf60 R14: ffffffff81729900 R15: ffffffff81729a00
FS:  0000000000000000(0000) GS:ffff88023fc00000(0000)
knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 00007f24456760a0 CR3: 0000000001713000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff4ff0 DR7: 0000000000000400
Process kworker/0:3 (pid: 3441, threadinfo ffff88021fe74000, task
ffff88023497cf60)
Stack:
 ffff88023fc03e88 ffffffff811cbf1a ffff88023fc03e98 ffffffff811cbf47
 ffff88023fc03eb8 ffffffff8101d155 0000000000000000 ffffffff81729900
 ffff88023fc03ef8 ffffffff81092b9c 000000004daf0bb9 0000000000000000
Call Trace:
 <IRQ> 
 [<ffffffff811cbf1a>] __delay+0xa/0x10
 [<ffffffff811cbf47>] __const_udelay+0x27/0x30
 [<ffffffff8101d155>] arch_trigger_all_cpu_backtrace+0x75/0xb0
 [<ffffffff81092b9c>] __rcu_pending+0x9c/0x370
 [<ffffffff81092efd>] rcu_check_callbacks+0x8d/0xd0
 [<ffffffff810530c1>] update_process_times+0x41/0x80
 [<ffffffff81070c07>] tick_periodic+0x27/0x70
 [<ffffffff81070c71>] tick_handle_periodic+0x21/0x80
 [<ffffffff8101cbd6>] smp_apic_timer_interrupt+0x66/0xa0
 [<ffffffff813c1993>] apic_timer_interrupt+0x13/0x20
 <EOI> 
 [<ffffffff811b1964>] ? blk_delay_work+0x34/0x40
 [<ffffffff8105d449>] process_one_work+0xf9/0x390
 [<ffffffff811b1930>] ? submit_bio+0xd0/0xd0
 [<ffffffff8105fa45>] worker_thread+0xe5/0x270
 [<ffffffff8105f960>] ? gcwq_mayday_timeout+0x80/0x80
 [<ffffffff810638e6>] kthread+0x96/0xa0
 [<ffffffff813c20d4>] kernel_thread_helper+0x4/0x10
 [<ffffffff81063850>] ? kthread_stop+0xd0/0xd0
 [<ffffffff813c20d0>] ? gs_change+0xb/0xb
Code: 89 e5 f7 e2 48 8d 7a 01 e8 c9 ff ff ff c9 c3 0f 1f 80 00 00 00 00
55 65 44 8b 0c 25 68 c5 00 00 48 89 e5 66 66 90 0f ae e8 0f 31 
 89 c0 44 89 ce eb 11 0f 1f 00 f3 90 65 8b 0c 25 68 c5 00 00 
Call Trace:
 <IRQ>  [<ffffffff811cbf1a>] __delay+0xa/0x10
 [<ffffffff811cbf47>] __const_udelay+0x27/0x30
 [<ffffffff8101d155>] arch_trigger_all_cpu_backtrace+0x75/0xb0
 [<ffffffff81092b9c>] __rcu_pending+0x9c/0x370
 [<ffffffff81092efd>] rcu_check_callbacks+0x8d/0xd0
 [<ffffffff810530c1>] update_process_times+0x41/0x80
 [<ffffffff81070c07>] tick_periodic+0x27/0x70
 [<ffffffff81070c71>] tick_handle_periodic+0x21/0x80
 [<ffffffff8101cbd6>] smp_apic_timer_interrupt+0x66/0xa0
 [<ffffffff813c1993>] apic_timer_interrupt+0x13/0x20
 <EOI>  [<ffffffff811b1964>] ? blk_delay_work+0x34/0x40
 [<ffffffff8105d449>] process_one_work+0xf9/0x390
 [<ffffffff811b1930>] ? submit_bio+0xd0/0xd0
 [<ffffffff8105fa45>] worker_thread+0xe5/0x270
 [<ffffffff8105f960>] ? gcwq_mayday_timeout+0x80/0x80
 [<ffffffff810638e6>] kthread+0x96/0xa0
 [<ffffffff813c20d4>] kernel_thread_helper+0x4/0x10
 [<ffffffff81063850>] ? kthread_stop+0xd0/0xd0
 [<ffffffff813c20d0>] ? gs_change+0xb/0xb




^ permalink raw reply

* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Christoph Lameter @ 2011-04-20 15:17 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Pekka Enberg, casteyde.christian, Andrew Morton, netdev,
	bugzilla-daemon, bugme-daemon, Vegard Nossum
In-Reply-To: <1303311687.3186.100.camel@edumazet-laptop>

On Wed, 20 Apr 2011, Eric Dumazet wrote:

> [PATCH v4] slub: dont use cmpxchg_double if KMEMCHECK or DEBUG_PAGEALLOC

Acked-by: Christoph Lameter <cl@linux.com>

>
> -#ifdef CONFIG_CMPXCHG_LOCAL
> +#if defined(CONFIG_CMPXCHG_LOCAL) && !defined(CONFIG_KMEMCHECK) && \
> +				     !defined(CONFIG_DEBUG_PAGEALLOC)
> +#define SLUB_USE_CMPXCHG_DOUBLE

#undef CONFIG_CMPXCHG_LOCAL instead? Would reduce patch size but its kind
of hacky.


^ permalink raw reply

* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Eric Dumazet @ 2011-04-20 15:30 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, casteyde.christian, Andrew Morton, netdev,
	bugzilla-daemon, bugme-daemon, Vegard Nossum
In-Reply-To: <alpine.DEB.2.00.1104201015520.9266@router.home>

Le mercredi 20 avril 2011 à 10:17 -0500, Christoph Lameter a écrit :
> On Wed, 20 Apr 2011, Eric Dumazet wrote:
> 
> > [PATCH v4] slub: dont use cmpxchg_double if KMEMCHECK or DEBUG_PAGEALLOC
> 
> Acked-by: Christoph Lameter <cl@linux.com>
> 
> >
> > -#ifdef CONFIG_CMPXCHG_LOCAL
> > +#if defined(CONFIG_CMPXCHG_LOCAL) && !defined(CONFIG_KMEMCHECK) && \
> > +				     !defined(CONFIG_DEBUG_PAGEALLOC)
> > +#define SLUB_USE_CMPXCHG_DOUBLE
> 
> #undef CONFIG_CMPXCHG_LOCAL instead? Would reduce patch size but its kind
> of hacky.
> 

Well, its definitely hacky :)

I chose a local definition (instead of Kconfig game), referring to
cmpxchg_double()




^ permalink raw reply

* [RFC net-next-2.6] can: replace spinlocks with mutexes
From: Oliver Hartkopp @ 2011-04-20 15:31 UTC (permalink / raw)
  To: David Miller, Eric Dumazet
  Cc: Linux Netdev List, Kurt Van Dijck, Urs Thuermann

This patch removes spinlocks for the CAN netdevice specific receive lists.
The RCU-based receive lists can be modified from process context or from the
netdevice notifier call. As both might sleep we can safely replace the
spinlocks with mutexes.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

---

diff --git a/net/can/af_can.c b/net/can/af_can.c
index a8dcaa4..e52ed358 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -47,7 +47,7 @@
 #include <linux/kmod.h>
 #include <linux/slab.h>
 #include <linux/list.h>
-#include <linux/spinlock.h>
+#include <linux/mutex.h>
 #include <linux/rcupdate.h>
 #include <linux/uaccess.h>
 #include <linux/net.h>
@@ -79,7 +79,7 @@ MODULE_PARM_DESC(stats_timer, "enable timer for statistics (default:on)");
 
 /* receive filters subscribed for 'all' CAN devices */
 struct dev_rcv_lists can_rx_alldev_list;
-static DEFINE_SPINLOCK(can_rcvlists_lock);
+static DEFINE_MUTEX(can_rcvlists_lock);
 
 static struct kmem_cache *rcv_cache __read_mostly;
 
@@ -435,7 +435,7 @@ int can_rx_register(struct net_device *dev, canid_t can_id, canid_t mask,
 	if (!r)
 		return -ENOMEM;
 
-	spin_lock(&can_rcvlists_lock);
+	mutex_lock(&can_rcvlists_lock);
 
 	d = find_dev_rcv_lists(dev);
 	if (d) {
@@ -459,7 +459,7 @@ int can_rx_register(struct net_device *dev, canid_t can_id, canid_t mask,
 		err = -ENODEV;
 	}
 
-	spin_unlock(&can_rcvlists_lock);
+	mutex_unlock(&can_rcvlists_lock);
 
 	return err;
 }
@@ -497,7 +497,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
 	if (dev && dev->type != ARPHRD_CAN)
 		return;
 
-	spin_lock(&can_rcvlists_lock);
+	mutex_lock(&can_rcvlists_lock);
 
 	d = find_dev_rcv_lists(dev);
 	if (!d) {
@@ -548,7 +548,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
 	}
 
  out:
-	spin_unlock(&can_rcvlists_lock);
+	mutex_unlock(&can_rcvlists_lock);
 
 	/* schedule the receiver item for deletion */
 	if (r)
@@ -775,7 +775,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
 		break;
 
 	case NETDEV_UNREGISTER:
-		spin_lock(&can_rcvlists_lock);
+		mutex_lock(&can_rcvlists_lock);
 
 		d = dev->ml_priv;
 		if (d) {
@@ -789,7 +789,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
 			printk(KERN_ERR "can: notifier: receive list not "
 			       "found for dev %s\n", dev->name);
 
-		spin_unlock(&can_rcvlists_lock);
+		mutex_unlock(&can_rcvlists_lock);
 
 		break;
 	}


^ permalink raw reply related

* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Eric Dumazet @ 2011-04-20 15:34 UTC (permalink / raw)
  To: vegardno
  Cc: Christoph Lameter, Pekka Enberg, casteyde.christian,
	Andrew Morton, netdev, bugzilla-daemon, bugme-daemon
In-Reply-To: <66e0b8f575075d009bdb3633837951a3@ulrik.uio.no>

Le mercredi 20 avril 2011 à 17:15 +0200, Vegard Nossum a écrit :

>  Thanks, guys. I wonder: Is it possible to make a reproducible test-case 
>  (e.g. loadable module) for this?
> 

Not easy to code a specific test-case, but just use regular workload,
(and network trafic to get interrupts). Use CONFIG_PREEMPT to trigger
preemptions.

>  Also, pardon my ignorance, but can you explain why this is a bug with 
>  kmemcheck/page-alloc debug and not without them?

Without them, we can read object->next without trigerring a fault.
We can read garbage data (if object is in use, it was overwritten with
user data), but this doesnt matter because cmpxchg_double() wont perform
its work (since tid will have change)




^ permalink raw reply

* Re: [RFC net-next-2.6] can: replace spinlocks with mutexes
From: Eric Dumazet @ 2011-04-20 15:39 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: David Miller, Linux Netdev List, Kurt Van Dijck, Urs Thuermann
In-Reply-To: <4DAEFC55.2010500@hartkopp.net>

Le mercredi 20 avril 2011 à 17:31 +0200, Oliver Hartkopp a écrit :
> This patch removes spinlocks for the CAN netdevice specific receive lists.
> The RCU-based receive lists can be modified from process context or from the
> netdevice notifier call. As both might sleep we can safely replace the
> spinlocks with mutexes.
> 
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> ---

But... why ?

A spinlock is faster/smaller than a mutex.

Maybe you wanted to _remove_ spinlock, since/if writer hold RTNL and
doesnt need to exclude another writer(s) ?

Note : I did not check the RTNL assertion, you might add appropriate
ASSERT_RTNL() calls just to be 100% safe.




^ permalink raw reply

* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Vegard Nossum @ 2011-04-20 15:15 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Christoph Lameter, Pekka Enberg, casteyde.christian,
	Andrew Morton, netdev, bugzilla-daemon, bugme-daemon
In-Reply-To: <1303311687.3186.100.camel@edumazet-laptop>

 On Wed, 20 Apr 2011 17:01:27 +0200, Eric Dumazet 
 <eric.dumazet@gmail.com> wrote:
> Le mercredi 20 avril 2011 à 09:42 -0500, Christoph Lameter a écrit :
>
>> Avoiding the irq handling yields the savings that improve the 
>> fastpath. if
>> you do both then there is only a regression left. So lets go with
>> disabling the CMPXCHG_LOCAL.
>
> OK, let's do that then.
>
> Thanks
>
> [PATCH v4] slub: dont use cmpxchg_double if KMEMCHECK or 
> DEBUG_PAGEALLOC
>
> Christian Casteyde reported a KMEMCHECK splat in slub code.
>
> Problem is now we are lockless and allow IRQ in slab_alloc(), the 
> object
> we manipulate from freelist can be allocated and freed right before 
> we
> try to read object->next.
>
> Same problem can happen with DEBUG_PAGEALLOC
>
> Just dont use cmpxchg_double() if either CONFIG_KMEMCHECK or
> CONFIG_DEBUG_PAGEALLOC is defined.
>
> Reported-by: Christian Casteyde <casteyde.christian@free.fr>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Pekka Enberg <penberg@cs.helsinki.fi>
> Cc: Vegard Nossum <vegardno@ifi.uio.no>
> Cc: Christoph Lameter <cl@linux.com>

 Thanks, guys. I wonder: Is it possible to make a reproducible test-case 
 (e.g. loadable module) for this?

 Also, pardon my ignorance, but can you explain why this is a bug with 
 kmemcheck/page-alloc debug and not without them?

 Thanks,


 Vegard

^ permalink raw reply

* Re: Add missing socket check in can/bcm release.
From: Dave Jones @ 2011-04-20 16:03 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20110419.203720.02289813.davem@davemloft.net>

On Tue, Apr 19, 2011 at 08:37:20PM -0700, David Miller wrote:
 > From: Dave Jones <davej@redhat.com>
 > Date: Tue, 19 Apr 2011 23:30:01 -0400
 > 
 > > We can get here with a NULL socket argument passed from userspace,
 > > so we need to handle it accordingly.
 > > 
 > > Signed-off-by: Dave Jones <davej@redhat.com>
 > 
 > Applied and queued up for -stable, thanks Dave.

Out of curiousity, while I was asleep it occured to me.. is it ever valid
for a ->release to get passed a NULL socket->sk ?  I'm wondering if we
can't do this check a layer up in sock_release, in case future protocols
reintroduce the same bug.

>From a quick look, almost every protocol has this check in its ->release.
Though it seems some do something different instead of using socket->sk,
so it would be a pointless check for some of the lesser used ones.

thoughts?

	Dave


^ permalink raw reply

* Re: [RFC net-next-2.6] can: replace spinlocks with mutexes
From: Oliver Hartkopp @ 2011-04-20 16:18 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Linux Netdev List, Kurt Van Dijck, Urs Thuermann
In-Reply-To: <1303313954.3186.117.camel@edumazet-laptop>

On 20.04.2011 17:39, Eric Dumazet wrote:
> Le mercredi 20 avril 2011 à 17:31 +0200, Oliver Hartkopp a écrit :
>> This patch removes spinlocks for the CAN netdevice specific receive lists.
>> The RCU-based receive lists can be modified from process context or from the
>> netdevice notifier call. As both might sleep we can safely replace the
>> spinlocks with mutexes.
>>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>
>> ---
> 
> But... why ?
> 
> A spinlock is faster/smaller than a mutex.

Hm, i expected the mutex to have some advantages especially in multicore
systems ...

But if it doesn't has any vital advantage, we can leave it as-is.

> Maybe you wanted to _remove_ spinlock, since/if writer hold RTNL and
> doesnt need to exclude another writer(s) ?

That's an interesting idea. The filters are modified at socket
creation/removal time and can also be modified in between using sockopts by
_ordinary_ users. Could that be a problem?

> Note : I did not check the RTNL assertion, you might add appropriate
> ASSERT_RTNL() calls just to be 100% safe.

I'll investigate some similar places in the networking code and then replace
the spinlocks with rtnl_locks for some testing.

Thanks for the feedback,
Oliver


^ permalink raw reply

* Re: [PATCH 1/1] powerpc: Fix multicast problem in fs_enet driver
From: Scott Wood @ 2011-04-20 16:23 UTC (permalink / raw)
  To: Andrea Galbusera
  Cc: Pantelis Antoniou, Vitaly Bordug, netdev, linuxppc-dev,
	linux-kernel
In-Reply-To: <1303289719-16913-1-git-send-email-gizero@gmail.com>

On Wed, 20 Apr 2011 10:55:19 +0200
Andrea Galbusera <gizero@gmail.com> wrote:

> mac-fec.c was setting individual UDP address registers instead of multicast
> group address registers when joining a multicast group.
> This prevented from correctly receiving UDP multicast packets.
> According to datasheet, replaced hash_table_high and hash_table_low
> with grp_hash_table_high and grp_hash_table_low respectively.
> 
> Tested on a MPC5121 based board.
> 
> Signed-off-by: Andrea Galbusera <gizero@gmail.com>
> ---
>  drivers/net/fs_enet/mac-fec.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/fs_enet/mac-fec.c b/drivers/net/fs_enet/mac-fec.c
> index 61035fc..b9fbc83 100644
> --- a/drivers/net/fs_enet/mac-fec.c
> +++ b/drivers/net/fs_enet/mac-fec.c
> @@ -226,8 +226,8 @@ static void set_multicast_finish(struct net_device *dev)
>  	}
>  
>  	FC(fecp, r_cntrl, FEC_RCNTRL_PROM);
> -	FW(fecp, hash_table_high, fep->fec.hthi);
> -	FW(fecp, hash_table_low, fep->fec.htlo);
> +	FW(fecp, grp_hash_table_high, fep->fec.hthi);
> +	FW(fecp, grp_hash_table_low, fep->fec.htlo);
>  }
>  
>  static void set_multicast_list(struct net_device *dev)
> @@ -273,8 +273,8 @@ static void restart(struct net_device *dev)
>  	/*
>  	 * Reset all multicast.
>  	 */
> -	FW(fecp, hash_table_high, fep->fec.hthi);
> -	FW(fecp, hash_table_low, fep->fec.htlo);
> +	FW(fecp, grp_hash_table_high, fep->fec.hthi);
> +	FW(fecp, grp_hash_table_low, fep->fec.htlo);
>  
>  	/*
>  	 * Set maximum receive buffer size.

This will break on 8xx which does not have grp_hash_table_*.  It has only
one set of hash table registers which are used only for multicast -- so
8xx should have fec_hash_table_* renamed to fec_grp_hash_table_* in struct
fec.

-Scott

^ permalink raw reply

* [PATCH] staging: octeon-ethernet: remove .get_sg, etc. ethtool_ops
From: Michał Mirosław @ 2011-04-20 16:27 UTC (permalink / raw)
  To: netdev; +Cc: Greg Kroah-Hartman, devel
In-Reply-To: <20110420143612.GA22661@suse.de>

Driver sets .get_sg and .get_tx_csum ethtool_ops to their default
values anyway. Those fields are deprecated, starting in 2.6.39.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/staging/octeon/ethernet-mdio.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-mdio.c b/drivers/staging/octeon/ethernet-mdio.c
index 10a82ef..8a11ffc 100644
--- a/drivers/staging/octeon/ethernet-mdio.c
+++ b/drivers/staging/octeon/ethernet-mdio.c
@@ -91,8 +91,6 @@ const struct ethtool_ops cvm_oct_ethtool_ops = {
 	.set_settings = cvm_oct_set_settings,
 	.nway_reset = cvm_oct_nway_reset,
 	.get_link = ethtool_op_get_link,
-	.get_sg = ethtool_op_get_sg,
-	.get_tx_csum = ethtool_op_get_tx_csum,
 };
 
 /**
-- 
1.7.2.5


^ permalink raw reply related

* Re: [PATCH v2] can: add missing socket check in can/raw release
From: Dave Jones @ 2011-04-20 17:03 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: David Miller, netdev
In-Reply-To: <4DAECA1B.5050300@hartkopp.net>

On Wed, Apr 20, 2011 at 01:57:15PM +0200, Oliver Hartkopp wrote:
 > v2: added space after 'if' according code style.
 > 
 > We can get here with a NULL socket argument passed from userspace,
 > so we need to handle it accordingly.
 > 
 > Thanks to Dave Jones pointing at this issue in net/can/bcm.c
 > 
 > Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

ACK. That was the first thing I hit this morning ;-)

	Dave


^ permalink raw reply


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