Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] ipv6: Enable RFS sk_rxhash tracking for ipv6 sockets
From: Eric Dumazet @ 2011-04-06 18:23 UTC (permalink / raw)
  To: David Miller
  Cc: nhorman, netdev, kuznet, pekkas, jmorris, yoshfuji, kaber,
	therbert
In-Reply-To: <20110406.111712.226782643.davem@davemloft.net>

Le mercredi 06 avril 2011 à 11:17 -0700, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 06 Apr 2011 20:07:33 +0200
> 
> > This is why we used on ipv4 :
> > 
> > if (inet_sk(sk)->inet_daddr)
> > 	sock_rps_save_rxhash(sk, skb->rxhash);
> > 
> > 
> > Only arm RFS on UDP if socket is bound to a given remote peer.
> 
> Agreed, Neil please make this change to your ipv6 code.


BTW, do you guys know if NFS is using RFS right now (if TCP transport is
used) ?




^ permalink raw reply

* Re: [PATCH] ipv6: Enable RFS sk_rxhash tracking for ipv6 sockets
From: David Miller @ 2011-04-06 18:17 UTC (permalink / raw)
  To: eric.dumazet
  Cc: nhorman, netdev, kuznet, pekkas, jmorris, yoshfuji, kaber,
	therbert
In-Reply-To: <1302113253.3209.126.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 06 Apr 2011 20:07:33 +0200

> This is why we used on ipv4 :
> 
> if (inet_sk(sk)->inet_daddr)
> 	sock_rps_save_rxhash(sk, skb->rxhash);
> 
> 
> Only arm RFS on UDP if socket is bound to a given remote peer.

Agreed, Neil please make this change to your ipv6 code.

Thanks!

^ permalink raw reply

* Re: [Bug 32772] New: PROBLEM: kernel BUG at net/ipv4/inetpeer.c:386
From: David Miller @ 2011-04-06 18:16 UTC (permalink / raw)
  To: dimetrios; +Cc: eric.dumazet, shemminger, netdev
In-Reply-To: <BANLkTinhnPR6fHRQD-vvbxX1n+Zxr=HTZw@mail.gmail.com>

From: Dmitry Novikov <dimetrios@gmail.com>
Date: Wed, 6 Apr 2011 21:15:17 +0300

> 2011/4/6 Eric Dumazet <eric.dumazet@gmail.com>
>>
>> Le mercredi 06 avril 2011 à 18:05 +0200, Eric Dumazet a écrit :
>>
>> >
>> > This reminds me a possible memory corruption (from another layer)
>> >
>>
>> I found the reference of a past bug report, where slub_nomerge was used
>> too.
>>
>>  http://www.spinics.net/lists/netdev/msg154206.html
>>
>>
>>
> I will schedule reboot in 6 hours so that slub_nomerge parmeter been applied

Thanks for helping us track this down.

^ permalink raw reply

* Re: Fw: [Bug 32772] New: PROBLEM: kernel BUG at net/ipv4/inetpeer.c:386
From: Dmitry Novikov @ 2011-04-06 18:15 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Stephen Hemminger, netdev, David Miller
In-Reply-To: <1302107123.3209.113.camel@edumazet-laptop>

2011/4/6 Eric Dumazet <eric.dumazet@gmail.com>
>
> Le mercredi 06 avril 2011 à 18:05 +0200, Eric Dumazet a écrit :
>
> >
> > This reminds me a possible memory corruption (from another layer)
> >
>
> I found the reference of a past bug report, where slub_nomerge was used
> too.
>
>  http://www.spinics.net/lists/netdev/msg154206.html
>
>
>
I will schedule reboot in 6 hours so that slub_nomerge parmeter been applied

^ permalink raw reply

* Re: [PATCH] ipv6: Enable RFS sk_rxhash tracking for ipv6 sockets
From: Eric Dumazet @ 2011-04-06 18:07 UTC (permalink / raw)
  To: Neil Horman
  Cc: netdev, David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Tom Herbert
In-Reply-To: <1302112465-6842-1-git-send-email-nhorman@tuxdriver.com>

Le mercredi 06 avril 2011 à 13:54 -0400, Neil Horman a écrit :

> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index d7037c0..aa3e327 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -505,6 +505,8 @@ int udpv6_queue_rcv_skb(struct sock * sk, struct sk_buff *skb)
>  	int rc;
>  	int is_udplite = IS_UDPLITE(sk);
>  
> +	sock_rps_save_rxhash(sk, skb->rxhash);
> +
>  	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
>  		goto drop;
>  

Problem is every packet received might have a different rxhash if
multiple senders are active. 

This can hurt performance (cache misses on RFS table) on a DNS server
workload for example.

This is why we used on ipv4 :

if (inet_sk(sk)->inet_daddr)
	sock_rps_save_rxhash(sk, skb->rxhash);


Only arm RFS on UDP if socket is bound to a given remote peer.

BTW you forgot Tom Herbert. (I added him in CC)




^ permalink raw reply

* Re: [PATCH 07/19] timberdale: mfd_cell is now implicitly available to drivers
From: Ben Hutchings @ 2011-04-06 18:07 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: Greg KH, Grant Likely, Andres Salomon,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mark Brown,
	khali-PUYAD+kWke1g9hUCZPvPmw, ben-linux-elnMNo+KYs3YtjvyW6yDsg,
	Peter Korsgaard, Mauro Carvalho Chehab, David Brownell,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Mocean Laboratories
In-Reply-To: <20110406175113.GC2757@sortiz-mobl>

On Wed, 2011-04-06 at 19:51 +0200, Samuel Ortiz wrote:
> Hi Ben,
> 
> On Wed, Apr 06, 2011 at 06:16:49PM +0100, Ben Hutchings wrote:
> > > So, adding an MFD cell pointer to the device structure allows us to cleanly
> > > pass both pieces of information, while keeping all the MFD sub drivers
> > > independant from the MFD core if they want/can.
> > 
> > Why isn't an MFD the parent of its component devices?
> It actually is. How would that help here ?

I was thinking you could encode the component address in the
platform_device name (just as the bus address is the name of a normal
bus device).  That plus the parent device pointer would be sufficient
information to look up the mfd_cell.

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

* [PATCH] ipv6: Enable RFS sk_rxhash tracking for ipv6 sockets
From: Neil Horman @ 2011-04-06 17:54 UTC (permalink / raw)
  To: netdev
  Cc: Neil Horman, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy

properly record sk_rxhash in ipv6 sockets

Noticed while working on another project that flows to sockets which I had open
on a test systems weren't getting steered properly when I had RFS enabled.
Looking more closely I found that:

1) The affected sockets were all ipv6
2) They weren't getting steered because sk->sk_rxhash was never set from the
incomming skbs on that socket.

This was occuring because there are several points in the IPv4 tcp and udp code
which save the rxhash value when a new connection is established.  Those calls
to sock_rps_save_rxhash were never added to the corresponding ipv6 code paths.
This patch adds those calls.  Tested by myself to properly enable RFS
functionalty on ipv6.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
CC: "Pekka Savola (ipv6)" <pekkas@netcore.fi>
CC: James Morris <jmorris@namei.org>
CC: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
CC: Patrick McHardy <kaber@trash.net>
---
 net/ipv6/tcp_ipv6.c |    4 +++-
 net/ipv6/udp.c      |    2 ++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 2b0c186..97917bb 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1621,6 +1621,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 		opt_skb = skb_clone(skb, GFP_ATOMIC);
 
 	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
+		sock_rps_save_rxhash(sk, skb->rxhash);
 		if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len))
 			goto reset;
 		if (opt_skb)
@@ -1648,7 +1649,8 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 				__kfree_skb(opt_skb);
 			return 0;
 		}
-	}
+	} else
+		sock_rps_save_rxhash(sk, skb->rxhash);
 
 	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb), skb->len))
 		goto reset;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index d7037c0..aa3e327 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -505,6 +505,8 @@ int udpv6_queue_rcv_skb(struct sock * sk, struct sk_buff *skb)
 	int rc;
 	int is_udplite = IS_UDPLITE(sk);
 
+	sock_rps_save_rxhash(sk, skb->rxhash);
+
 	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
 		goto drop;
 
-- 
1.7.4


^ permalink raw reply related

* Re: [PATCH 07/19] timberdale: mfd_cell is now implicitly available to drivers
From: Greg KH @ 2011-04-06 17:56 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: Grant Likely, Andres Salomon, linux-kernel, Mark Brown, khali,
	ben-linux, Peter Korsgaard, Mauro Carvalho Chehab, David Brownell,
	linux-i2c, linux-media, netdev, spi-devel-general,
	Mocean Laboratories
In-Reply-To: <20110406170537.GB2757@sortiz-mobl>

On Wed, Apr 06, 2011 at 07:05:38PM +0200, Samuel Ortiz wrote:
> Hi Greg,
> 
> On Wed, Apr 06, 2011 at 08:58:05AM -0700, Greg KH wrote:
> > On Wed, Apr 06, 2011 at 05:23:23PM +0200, Samuel Ortiz wrote:
> > > --- a/include/linux/device.h
> > > +++ b/include/linux/device.h
> > > @@ -33,6 +33,7 @@ struct class;
> > >  struct subsys_private;
> > >  struct bus_type;
> > >  struct device_node;
> > > +struct mfd_cell;
> > >  
> > >  struct bus_attribute {
> > >  	struct attribute	attr;
> > > @@ -444,6 +445,8 @@ struct device {
> > >  	struct device_node	*of_node; /* associated device tree node */
> > >  	const struct of_device_id *of_match; /* matching of_device_id from driver */
> > >  
> > > +	struct mfd_cell	*mfd_cell; /* MFD cell pointer */
> > > +
> > 
> > What is a "MFD cell pointer" and why is it needed in struct device?
> An MFD cell is an MFD instantiated device.
> MFD (Multi Function Device) drivers instantiate platform devices. Those
> devices drivers sometimes need a platform data pointer, sometimes an MFD
> specific pointer, and sometimes both. Also, some of those drivers have been
> implemented as MFD sub drivers, while others know nothing about MFD and just
> expect a plain platform_data pointer.

That sounds like a bug in those drivers, why not fix them to properly
pass in the correct pointer?

> We've been faced with the problem of being able to pass both MFD related data
> and a platform_data pointer to some of those drivers. Squeezing the MFD bits
> in the sub driver platform_data pointer doesn't work for drivers that know
> nothing about MFDs. It also adds an additional dependency on the MFD API to
> all MFD sub drivers. That prevents any of those drivers to eventually be used
> as plain platform device drivers.

Then they shouldn't be "plain" platform drivers, that should only be
reserved for drivers that are the "lowest" type.  Just make them MFD
devices and go from there.

> So, adding an MFD cell pointer to the device structure allows us to cleanly
> pass both pieces of information, while keeping all the MFD sub drivers
> independant from the MFD core if they want/can.

They shouldn't be "independant", make them "dependant" and go from
there.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 07/19] timberdale: mfd_cell is now implicitly available to drivers
From: Samuel Ortiz @ 2011-04-06 17:51 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Greg KH, Grant Likely, Andres Salomon,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mark Brown,
	khali-PUYAD+kWke1g9hUCZPvPmw, ben-linux-elnMNo+KYs3YtjvyW6yDsg,
	Peter Korsgaard, Mauro Carvalho Chehab, David Brownell,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Mocean Laboratories
In-Reply-To: <1302110209.2840.20.camel@bwh-desktop>

Hi Ben,

On Wed, Apr 06, 2011 at 06:16:49PM +0100, Ben Hutchings wrote:
> > So, adding an MFD cell pointer to the device structure allows us to cleanly
> > pass both pieces of information, while keeping all the MFD sub drivers
> > independant from the MFD core if they want/can.
> 
> Why isn't an MFD the parent of its component devices?
It actually is. How would that help here ?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply

* Re: [PATCH 01/20] net-core: extending (hw_/wanted_/vlan_)features fields to a bitmap.
From: Mahesh Bandewar @ 2011-04-06 17:34 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: David Miller, netdev
In-Reply-To: <BANLkTik3Uz_zu5qev1bc=mop4fUTYGZaDQ@mail.gmail.com>

On Wed, Apr 6, 2011 at 3:29 AM, Michał Mirosław <mirqus@gmail.com> wrote:
> 2011/4/6 Mahesh Bandewar <maheshb@google.com>:
>> Converting current use of (hw_/wanted_/vlan_)features to
>> legacy_(hw_/wanted_/vlan_)features to differntiate from the proposed usage.
>>
>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>> ---
>>  include/linux/netdevice.h |  110 +++++++++++++++++++++++++++++++-------------
>>  net/core/dev.c            |   51 +++++++++++----------
>>  net/core/ethtool.c        |   97 ++++++++++++++++++++-------------------
>>  net/core/net-sysfs.c      |    4 +-
>>  net/core/sock.c           |    2 +-
>>  5 files changed, 155 insertions(+), 109 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 09d2624..637bf2a 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -980,6 +980,42 @@ struct net_device_ops {
>>                                                    u32 features);
>>  };
>>
>> +enum netdev_features {
>> +       NETIF_F_SG_BIT,                 /* Scatter/gather IO. */
>> +       NETIF_F_IP_CSUM_BIT,            /* Can checksum TCP/UDP over IPv4. */
>> +       NETIF_F_NO_CSUM_BIT,            /* Does not require checksum. F.e. loopack. */
>> +       NETIF_F_HW_CSUM_BIT,            /* Can checksum all the packets. */
>> +       NETIF_F_IPV6_CSUM_BIT,          /* Can checksum TCP/UDP over IPV6 */
>> +       NETIF_F_HIGHDMA_BIT,            /* Can DMA to high memory. */
>> +       NETIF_F_FRAGLIST_BIT,           /* Scatter/gather IO. */
>> +       NETIF_F_HW_VLAN_TX_BIT,         /* Transmit VLAN hw acceleration */
>> +       NETIF_F_HW_VLAN_RX_BIT,         /* Receive VLAN hw acceleration */
>> +       NETIF_F_HW_VLAN_FILTER_BIT,     /* Receive filtering on VLAN */
>> +       NETIF_F_VLAN_CHALLENGED_BIT,    /* Device cannot handle VLAN packets */
>> +       NETIF_F_GSO_BIT,                /* Enable software GSO. */
>> +       NETIF_F_LLTX_BIT,               /* LockLess TX - deprecated. Please */
>> +                                       /* do not use LLTX in new drivers */
>> +       NETIF_F_NETNS_LOCAL_BIT,        /* Does not change network namespaces */
>> +       NETIF_F_GRO_BIT,                /* Generic receive offload */
>> +       NETIF_F_LRO_BIT,                /* large receive offload */
>> +       /* the GSO_MASK reserves bits 16 through 23 */
>> +       RESERVED1_BIT,
>> +       RESERVED2_BIT,
>> +       RESERVED3_BIT,
>> +       RESERVED4_BIT,
>> +       RESERVED5_BIT,
>> +       RESERVED6_BIT,
>> +       RESERVED7_BIT,
>> +       RESERVED8_BIT,
>> +       NETIF_F_FCOE_CRC_BIT,           /* FCoE CRC32 */
>> +       NETIF_F_SCTP_CSUM_BIT,          /* SCTP checksum offload */
>> +       NETIF_F_FCOE_MTU_BIT,           /* Supports max FCoE MTU, 2158 bytes*/
>> +       NETIF_F_NTUPLE_BIT,             /* N-tuple filters supported */
>> +       NETIF_F_RXHASH_BIT,             /* Receive hashing offload */
>> +       NETIF_F_RXCSUM_BIT,             /* Receive checksumming offload */
>> +       NETIF_F_NOCACHE_COPY_BIT,       /* Use no-cache copyfromuser */
>> +};
>> +
>
> This should be a separate cleanup patch. And after that, for the
> conversion you would add as a last entry:
> NETIF_F_NUM_BITS and use it later (see below).
>
I like the idea of NETIF_F_NUM_BITS and I'll change the #defines
accordingly. Couple of bitmaps are defined in this patch and NUM_BITS
will be required to define those bitmaps. This is reason why I have
defined above enum now rather than waiting for the cleanup phase. Also
it should not change radically in that time span.

>>  /*
>>  *     The DEVICE structure.
>>  *     Actually, this whole structure is a big mistake.  It mixes I/O
>> @@ -1029,44 +1065,51 @@ struct net_device {
>>        struct list_head        napi_list;
>>        struct list_head        unreg_list;
>>
>> +#define DEV_FEATURE_WORDS      2
>> +#define        DEV_FEATURE_BITS        (DEV_FEATURE_WORDS*sizeof(long)*BITS_PER_BYTE)
>> +#define LEGACY_FEATURE_WORD    0
>> +
>
> #define DEV_FEATURE_WORDS BITS_TO_LONGS(NETIF_F_NUM_BITS)
> #define DEV_FEATURE_BITS (DEV_FEATURE_WORDS*BITS_PER_LONG)
>
Yes, this is good!

> Though using bitmaps will make a mess for 32 versus 64 bit archs. It
> would be better to stick to u32 as the base type instead of long.
>
Once it's a bitmap; type shouldn't matter and each arch and it's
specific macros/inlines should handle them properly, no?
(I changed the base type since DECLARE_BITMAP() declares 'unsigned
long' and we can make use of readily avaialble set/test/clear_bit
macros)

> [...]
>> @@ -2376,13 +2419,13 @@ static inline void netif_tx_unlock_bh(struct net_device *dev)
>>  }
>>
>>  #define HARD_TX_LOCK(dev, txq, cpu) {                  \
>> -       if ((dev->features & NETIF_F_LLTX) == 0) {      \
>> +       if ((dev->legacy_features & NETIF_F_LLTX) == 0) {       \
> [...]
>
> For those type of conversion there is really no point in using the
> macro. Changing it to
> dev->features[0] instead of dev->legacy_features needs the same effort
> but avoids the
> cleanup later. Flags in other feature words could have names line
> NETIF_F2_xxx so that
> it would be clear in which word they belong.
>
I know! But changing all at once in zillion places is hard. This will
let us make changes progressively. I'm expecting the following
progress path -

(1) (Current patch)
+ #define legacy_feactures          features
+ #define legacy_vlan_features    vlan_features

Slowly make all the changes (relatively easy but a lengthy process!).
So the places where vlan_features, features fields are used will
compile and at the same time updates, where legacy_vlan_features,
legacy_features fields are used, will compile too. Once all is changes
are in place -

(2) Relatively small change
-  unsigned long       features;
+ DECLARE_BITMAP(feature, DEV_FEATURE_BITS);
-  unsigned long        vlan_features;
+ DECLARE_BITMAP(vlan_feature, DEV_FEATURE_BITS);
-  #define   legacy_features          features
+ #define   legacy_features          feature[0]
- #define    legacy_vlan_features  vlan_features
+ #define   legacy_vlan_features  vlan_feature[0]

At this moment old fields are gone and are replaced by the bitmaps and
the legacy usage is indicated by the use "legacy_*" fields. This
should eventually be changed to use the (set/test/clear)_bit()
macros/inlines. So the above place should look like -

#define HARD_TX_LOCK(dev, txq, cpu) {                  \
-       if ((dev->legacy_features & NETIF_F_LLTX) == 0) {       \
+      if (!test_bit(dev->feature, NETIF_F_LLTX_BIT) {      \


Thanks,
--mahesh..

^ permalink raw reply

* Re: [PATCH v2 net-next 3/9] tg3: Reintroduce 5717_PLUS
From: Matt Carlson @ 2011-04-06 17:33 UTC (permalink / raw)
  To: Joe Perches; +Cc: Matthew Carlson, davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <1302067853.1683.29.camel@Joe-Laptop>

On Tue, Apr 05, 2011 at 10:30:53PM -0700, Joe Perches wrote:
> On Tue, 2011-04-05 at 17:22 -0700, Matt Carlson wrote:
> > This patch reintroduces the TG3_FLG3_5717_PLUS to identify 5717 and
> > later devices.
> []
> > @@ -13427,8 +13422,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
> []
> > +	if (tp->tg3_flags3 & TG3_FLG3_5717_PLUS)
> >  		tp->tg3_flags3 |= TG3_FLG3_LRG_PROD_RING_CAP;
> 
> This seems redundant.  Maybe consolidate to just
> TG3_FLG3_5717_PLUS and remove LRG_PROD_RING_CAP?
> 
> I don't know if these attributes really are linked.
> 
> Another option may be to use DECLARE_BITMAP
> and set_bit/test_bit so there's no real need
> to use FLAG/FLG2/FLG3, etc.

A while back, I submitted a patch that extended the rx ring sizes for
those devices that were capable.  Dave Miller commented that he didn't
think the additional buffering was benefitial and in fact had a
downside.  (Larger ring sizes will result in more inefficient cache
usage.)

To fix the problem, the driver will need to be able to easily identify
which devices support the larger ring sizes.  This patch represents a
step in that direction.  Follow-on patches will make more use of the
flag, which should justify its existence.

For the record though, I did consider using the TG3_FLG3_5717_PLUS flag.
I'm uncomfortable with using it here because this is more of a server
class device feature.  Should another mobile or desktop device be
created in the future, I'd have to devise this type of flag to identify
what feature I'm really talking about.  I thought it prudent to just
take care of that now.

(Not that this is a compelling argument, but it also is a handy way to
enable and disable the feature while debugging.)


^ permalink raw reply

* Re: [PATCH 07/19] timberdale: mfd_cell is now implicitly available to drivers
From: Ben Hutchings @ 2011-04-06 17:16 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: Greg KH, Grant Likely, Andres Salomon, linux-kernel, Mark Brown,
	khali, ben-linux, Peter Korsgaard, Mauro Carvalho Chehab,
	David Brownell, linux-i2c, linux-media, netdev, spi-devel-general,
	Mocean Laboratories
In-Reply-To: <20110406170537.GB2757@sortiz-mobl>

On Wed, 2011-04-06 at 19:05 +0200, Samuel Ortiz wrote:
> Hi Greg,
> 
> On Wed, Apr 06, 2011 at 08:58:05AM -0700, Greg KH wrote:
> > On Wed, Apr 06, 2011 at 05:23:23PM +0200, Samuel Ortiz wrote:
> > > --- a/include/linux/device.h
> > > +++ b/include/linux/device.h
> > > @@ -33,6 +33,7 @@ struct class;
> > >  struct subsys_private;
> > >  struct bus_type;
> > >  struct device_node;
> > > +struct mfd_cell;
> > >  
> > >  struct bus_attribute {
> > >  	struct attribute	attr;
> > > @@ -444,6 +445,8 @@ struct device {
> > >  	struct device_node	*of_node; /* associated device tree node */
> > >  	const struct of_device_id *of_match; /* matching of_device_id from driver */
> > >  
> > > +	struct mfd_cell	*mfd_cell; /* MFD cell pointer */
> > > +
> > 
> > What is a "MFD cell pointer" and why is it needed in struct device?
> An MFD cell is an MFD instantiated device.
> MFD (Multi Function Device) drivers instantiate platform devices. Those
> devices drivers sometimes need a platform data pointer, sometimes an MFD
> specific pointer, and sometimes both. Also, some of those drivers have been
> implemented as MFD sub drivers, while others know nothing about MFD and just
> expect a plain platform_data pointer.
> 
> We've been faced with the problem of being able to pass both MFD related data
> and a platform_data pointer to some of those drivers. Squeezing the MFD bits
> in the sub driver platform_data pointer doesn't work for drivers that know
> nothing about MFDs. It also adds an additional dependency on the MFD API to
> all MFD sub drivers. That prevents any of those drivers to eventually be used
> as plain platform device drivers.
> So, adding an MFD cell pointer to the device structure allows us to cleanly
> pass both pieces of information, while keeping all the MFD sub drivers
> independant from the MFD core if they want/can.

Why isn't an MFD the parent of its component devices?

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: mourning the loss of David Brownell
From: Tony Lindgren @ 2011-04-06 17:11 UTC (permalink / raw)
  To: Jidong Xiao
  Cc: Sarah Sharp, Alan Stern, Greg KH, Oliver Neukum, Bill Gatliff,
	Kernel development list, USB list, linux-omap, spi-devel-general,
	netdev
In-Reply-To: <BANLkTikRJ+fBRJYfH=LEJk6dt58Gdk61Zg@mail.gmail.com>

* Jidong Xiao <jidong.xiao@gmail.com> [110405 20:33]:
> >
> Me too. David and the other developers you mentioned here helped me a
> lot. So sad, he is so young.

Dave certainly contributed a lot and helped many people. 
He also helped a lot with omap development.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH 07/19] timberdale: mfd_cell is now implicitly available to drivers
From: Samuel Ortiz @ 2011-04-06 17:05 UTC (permalink / raw)
  To: Greg KH
  Cc: Grant Likely, Andres Salomon, linux-kernel, Mark Brown, khali,
	ben-linux, Peter Korsgaard, Mauro Carvalho Chehab, David Brownell,
	linux-i2c, linux-media, netdev, spi-devel-general,
	Mocean Laboratories
In-Reply-To: <20110406155805.GA20095@suse.de>

Hi Greg,

On Wed, Apr 06, 2011 at 08:58:05AM -0700, Greg KH wrote:
> On Wed, Apr 06, 2011 at 05:23:23PM +0200, Samuel Ortiz wrote:
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -33,6 +33,7 @@ struct class;
> >  struct subsys_private;
> >  struct bus_type;
> >  struct device_node;
> > +struct mfd_cell;
> >  
> >  struct bus_attribute {
> >  	struct attribute	attr;
> > @@ -444,6 +445,8 @@ struct device {
> >  	struct device_node	*of_node; /* associated device tree node */
> >  	const struct of_device_id *of_match; /* matching of_device_id from driver */
> >  
> > +	struct mfd_cell	*mfd_cell; /* MFD cell pointer */
> > +
> 
> What is a "MFD cell pointer" and why is it needed in struct device?
An MFD cell is an MFD instantiated device.
MFD (Multi Function Device) drivers instantiate platform devices. Those
devices drivers sometimes need a platform data pointer, sometimes an MFD
specific pointer, and sometimes both. Also, some of those drivers have been
implemented as MFD sub drivers, while others know nothing about MFD and just
expect a plain platform_data pointer.

We've been faced with the problem of being able to pass both MFD related data
and a platform_data pointer to some of those drivers. Squeezing the MFD bits
in the sub driver platform_data pointer doesn't work for drivers that know
nothing about MFDs. It also adds an additional dependency on the MFD API to
all MFD sub drivers. That prevents any of those drivers to eventually be used
as plain platform device drivers.
So, adding an MFD cell pointer to the device structure allows us to cleanly
pass both pieces of information, while keeping all the MFD sub drivers
independant from the MFD core if they want/can.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply

* Re: [RFC] ixgbe: is DCA really that good ?
From: Alexander Duyck @ 2011-04-06 17:05 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Brattain, Ross B, Kirsher, Jeffrey T, netdev
In-Reply-To: <1302097444.3209.85.camel@edumazet-laptop>

On 4/6/2011 6:44 AM, Eric Dumazet wrote:
> Hi guys
>
>
> In a forwarding [or RPS/RFS] setup, why should we populate cpu caches
> with full frames content ? We only need first cache line to perform
> routing [or RPS/RFS] decision.
>
> ->  DCA should be a knob (ethtool ?) that an admin can switch off and on,
> port by port, not a CONFIG_IXGBE_DCA thing.
>
> Thanks

The problem is the implementation essentially makes it so that DCA is 
controlled by the availability of the DCA providers.  What would 
probably be the best solution would be for the DCA provider to have a 
means of shutting down specific devices.

The quick and dirty way to disable DCA is to rmmod the ioatdma module 
from the system.  With that removed it will remove the DCA provider and 
essentially turn off DCA.

Thanks,

Alex

^ permalink raw reply

* Re: shutdown oops in xt_compat_calc_jump
From: Eric Dumazet @ 2011-04-06 16:49 UTC (permalink / raw)
  To: dann frazier; +Cc: Patrick McHardy, netdev, netfilter-devel@vger.kernel.org
In-Reply-To: <1302108258.3209.117.camel@edumazet-laptop>

Le mercredi 06 avril 2011 à 18:44 +0200, Eric Dumazet a écrit :
> Le mercredi 06 avril 2011 à 10:25 -0600, dann frazier a écrit :
> 
> > Thanks Eric. Unfortunately that didn't solve the problem I am seeing.
> > I rebaselined (same kernel build as before), and found that I'm able
> > to reproduce this 100% of the time by running only:
> > 
> >   sudo ebtables -t filter --init-table
> > 
> > The backtrace I received was this:
> 
> 
> Oh yeah, I forgot to remove the WARN_ON_ONCE(1) at the end of
> xt_compat_calc_jump()
> 
> I focused on restoring ebtables (for example ebtables -A INPUT ...)
> 
> Just ignore the warning for the time being ;)
> 
> 
> # ebtables -A INPUT -p arp -s 00:01:02:03:04:05 -j ACCEPT
> # ebtables -A INPUT -p arp -s 00:01:02:03:04:06 -j ACCEPT
> # ebtables -L
> Bridge table: filter
> 
> Bridge chain: INPUT, entries: 2, policy: ACCEPT
> -p ARP -s 0:1:2:3:4:5 -j ACCEPT 
> -p ARP -s 0:1:2:3:4:6 -j ACCEPT 
> 
> Bridge chain: FORWARD, entries: 0, policy: ACCEPT
> 
> Bridge chain: OUTPUT, entries: 0, policy: ACCEPT
> 

[PATCH] netfilter: fix ebtables

commit 255d0dc34068a976 (netfilter: x_table: speedup compat operations)
made ebtables not working anymore.

1) xt_compat_calc_jump() is not an exact match lookup
2) compat_table_info() has a typo in xt_compat_init_offsets() call
3) compat_do_replace() misses a xt_compat_init_offsets() call

Reported-by: dann frazier <dannf@dannf.org>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
V2: 	remove a comment from V1 (as Patrick suggested)
	remove the WARN_ON_ONCE() in xt_compat_calc_jump()


 net/bridge/netfilter/ebtables.c |    3 ++-
 net/netfilter/x_tables.c        |    4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 893669c..c66aa80 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1766,7 +1766,7 @@ static int compat_table_info(const struct ebt_table_info *info,
 
 	newinfo->entries_size = size;
 
-	xt_compat_init_offsets(AF_INET, info->nentries);
+	xt_compat_init_offsets(NFPROTO_BRIDGE, info->nentries);
 	return EBT_ENTRY_ITERATE(entries, size, compat_calc_entry, info,
 							entries, newinfo);
 }
@@ -2240,6 +2240,7 @@ static int compat_do_replace(struct net *net, void __user *user,
 
 	xt_compat_lock(NFPROTO_BRIDGE);
 
+	xt_compat_init_offsets(NFPROTO_BRIDGE, tmp.nentries);
 	ret = compat_copy_entries(entries_tmp, tmp.entries_size, &state);
 	if (ret < 0)
 		goto out_unlock;
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index a9adf4c..8a025a5 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -455,6 +455,7 @@ void xt_compat_flush_offsets(u_int8_t af)
 		vfree(xt[af].compat_tab);
 		xt[af].compat_tab = NULL;
 		xt[af].number = 0;
+		xt[af].cur = 0;
 	}
 }
 EXPORT_SYMBOL_GPL(xt_compat_flush_offsets);
@@ -473,8 +474,7 @@ int xt_compat_calc_jump(u_int8_t af, unsigned int offset)
 		else
 			return mid ? tmp[mid - 1].delta : 0;
 	}
-	WARN_ON_ONCE(1);
-	return 0;
+	return left ? tmp[left - 1].delta : 0;
 }
 EXPORT_SYMBOL_GPL(xt_compat_calc_jump);
 


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: shutdown oops in xt_compat_calc_jump
From: Eric Dumazet @ 2011-04-06 16:44 UTC (permalink / raw)
  To: dann frazier; +Cc: Patrick McHardy, netdev, netfilter-devel@vger.kernel.org
In-Reply-To: <20110406162547.GA3064@dannf.org>

Le mercredi 06 avril 2011 à 10:25 -0600, dann frazier a écrit :

> Thanks Eric. Unfortunately that didn't solve the problem I am seeing.
> I rebaselined (same kernel build as before), and found that I'm able
> to reproduce this 100% of the time by running only:
> 
>   sudo ebtables -t filter --init-table
> 
> The backtrace I received was this:


Oh yeah, I forgot to remove the WARN_ON_ONCE(1) at the end of
xt_compat_calc_jump()

I focused on restoring ebtables (for example ebtables -A INPUT ...)

Just ignore the warning for the time being ;)


# ebtables -A INPUT -p arp -s 00:01:02:03:04:05 -j ACCEPT
# ebtables -A INPUT -p arp -s 00:01:02:03:04:06 -j ACCEPT
# ebtables -L
Bridge table: filter

Bridge chain: INPUT, entries: 2, policy: ACCEPT
-p ARP -s 0:1:2:3:4:5 -j ACCEPT 
-p ARP -s 0:1:2:3:4:6 -j ACCEPT 

Bridge chain: FORWARD, entries: 0, policy: ACCEPT

Bridge chain: OUTPUT, entries: 0, policy: ACCEPT


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: shutdown oops in xt_compat_calc_jump
From: dann frazier @ 2011-04-06 16:25 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Patrick McHardy, netdev, netfilter-devel@vger.kernel.org
In-Reply-To: <1301987879.3021.714.camel@edumazet-laptop>

On Tue, Apr 05, 2011 at 09:17:59AM +0200, Eric Dumazet wrote:
> Le mardi 05 avril 2011 à 08:24 +0200, Eric Dumazet a écrit :
> > Le mardi 05 avril 2011 à 00:48 +0200, Eric Dumazet a écrit :
> > > Le lundi 04 avril 2011 à 22:37 +0200, Eric Dumazet a écrit :
> > > > Le lundi 04 avril 2011 à 22:02 +0200, Patrick McHardy a écrit :
> > > > > CCed netfilter-devel.
> > > > > 
> > > > > Am 04.04.2011 21:48, schrieb dann frazier:
> > > > > > fyi, noticed this oops when shutting down a system running top of git
> > > > > > (@ 78fca1be)
> > > > > > 
> > > > > > [ 1169.794644] cfg80211: Calling CRDA to update world regulatory domain
> > > > > > [ 1170.490646] bluetoothd[2029]: segfault at f8ad9944 ip 00000000f77045e0 sp 00000000ffcb14e0 error 4 in bluetoothd[f76bf000+8b000]
> > > > > > [ 1170.543817] BUG: unable to handle kernel paging request at 00000001dc1be9f8
> > > > > > [ 1170.543875] IP: [<ffffffffa051e7b0>] xt_compat_calc_jump+0x25/0x6f [x_tables]
> > > > > > [ 1170.543927] PGD 1215b3067 PUD 0 
> > > > > > [ 1170.543955] Oops: 0000 [#1] SMP 
> > > > > > [ 1170.543982] last sysfs file: /sys/module/bridge/initstate
> > > > > > [ 1170.544017] CPU 3 
> > > > > > [ 1170.544031] Modules linked in: ebtable_broute ebtable_filter vfat msdos fat ext3 jbd ip6table_filter ip6_tables ebtable_nat ebtables ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc acpi_cpufreq mperf cpufreq_powersave cpufreq_userspace cpufreq_conservative cpufreq_stats binfmt_misc kvm(-) fuse ext2 loop snd_hda_codec_hdmi snd_hda_codec_conexant arc4 ecb snd_usb_audio snd_usbmidi_lib snd_seq_midi snd_seq_midi_event snd_hda_intel snd_hda_codec snd_hwdep snd_pcm snd_rawmidi i915 drm_kms_helper thinkpad_acpi snd_seq iwlagn snd_timer snd_seq_device drm snd mac80211 psmouse btusb serio_raw bluetooth evdev tpm_tis snd_page_alloc tpm i2c_i801 i2c_algo_bit cfg80211 battery soundcore nvram tpm_bios i2c_core rfkill wmi ac power_supply video button processor ext4 mbcache jbd2 crc16 sha256_generic aesni_intel cryptd aes_x86_64 aes_generic cbc dm_crypt dm_mod sd_mod crc_t10di
> > > > > f 
> > > > > >  usbhid
> > > > > > hid usb_storage ahci libahci libata ehci_hcd scsi_mod usbcore e1000e thermal thermal_sys [last unloaded: kvm_intel]
> > > > > > [ 1170.544836] 
> > > > > > [ 1170.544849] Pid: 4901, comm: ebtables Not tainted 2.6.39-rc1+ #9 LENOVO 2516CTO/2516CTO
> > > > > > [ 1170.544902] RIP: 0010:[<ffffffffa051e7b0>]  [<ffffffffa051e7b0>] xt_compat_calc_jump+0x25/0x6f [x_tables]
> > > > > > [ 1170.544958] RSP: 0018:ffff880121473cf8  EFLAGS: 00010217
> > > > > > [ 1170.544989] RAX: 000000003b837d3f RBX: 0000000000000090 RCX: 000000007706fa7f
> > > > > > [ 1170.545029] RDX: 0000000000000000 RSI: 0000000000000090 RDI: 000000003b837d3f
> > > > > > [ 1170.545067] RBP: ffffc900111a3000 R08: 0000000000000000 R09: dead000000200200
> > > > > > [ 1170.545104] R10: dead000000100100 R11: 0000000000001311 R12: ffff880121473d88
> > > > > > [ 1170.545147] R13: ffffc900111a6000 R14: ffffffff817de300 R15: 0000000000000000
> > > > > > [ 1170.545185] FS:  0000000000000000(0000) GS:ffff880137d80000(0063) knlGS:00000000f761b6c0
> > > > > > [ 1170.545227] CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
> > > > > > [ 1170.545258] CR2: 00000001dc1be9f8 CR3: 0000000125868000 CR4: 00000000000006e0
> > > > > > [ 1170.545297] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > > [ 1170.545334] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> > > > > > [ 1170.545375] Process ebtables (pid: 4901, threadinfo ffff880121472000, task ffff8801322d1ac0)
> > > > > > [ 1170.545418] Stack:
> > > > > > [ 1170.545433]  0000000000000090 ffffffffa0576d46 f7007265746c6966 0000000000000054
> > > > > > [ 1170.545479]  0000000000000000 0000000000000000 000000000000000e 0000000000000090
> > > > > > [ 1170.545529]  0000000000000000 0000000008af2180 0000000008af21b0 0000000008af21e0
> > > > > > [ 1170.545579] Call Trace:
> > > > > > [ 1170.545600]  [<ffffffffa0576d46>] ? compat_do_replace+0x117/0x221 [ebtables]
> > > > > > [ 1170.545639]  [<ffffffffa0577392>] ? compat_do_ebt_set_ctl+0x55/0xbb [ebtables]
> > > > > > [ 1170.545688]  [<ffffffff810337e3>] ? need_resched+0x1a/0x23
> > > > > > [ 1170.545723]  [<ffffffff810337f1>] ? should_resched+0x5/0x24
> > > > > > [ 1170.545730]  [<ffffffff81314cc5>] ? _cond_resched+0x9/0x20
> > > > > > [ 1170.545733]  [<ffffffff813152fe>] ? mutex_lock_interruptible+0x18/0x32
> > > > > > [ 1170.545738]  [<ffffffff8128490b>] ? nf_sockopt_find.clone.1+0xda/0xec
> > > > > > [ 1170.545742]  [<ffffffff81284996>] ? compat_nf_sockopt+0x79/0xa5
> > > > > > [ 1170.545744]  [<ffffffff810337f1>] ? should_resched+0x5/0x24
> > > > > > [ 1170.545747]  [<ffffffff812849f3>] ? compat_nf_setsockopt+0x1a/0x1f
> > > > > > [ 1170.545751]  [<ffffffff8128fb35>] ? compat_ip_setsockopt+0x80/0xa0
> > > > > > [ 1170.545756]  [<ffffffff812784a2>] ? compat_sys_setsockopt+0x1d5/0x204
> > > > > > [ 1170.545759]  [<ffffffff810337f1>] ? should_resched+0x5/0x24
> > > > > > [ 1170.545761]  [<ffffffff81314cc5>] ? _cond_resched+0x9/0x20
> > > > > > [ 1170.545764]  [<ffffffff812788a5>] ? compat_sys_socketcall+0x148/0x1a7
> > > > > > [ 1170.545768]  [<ffffffff8131d2c0>] ? sysenter_dispatch+0x7/0x2e
> > > > > > [ 1170.545769] Code: 5d 41 5e 41 5f c3 40 0f b6 ff 53 31 d2 48 6b ff 70 48 03 3d 03 1b 00 00 8b 4f 6c 4c 8b 47 60 ff c9 eb 27 8d 04 11 d1 f8 48 63 f8 
> > > > > > [ 1170.545787] RIP  [<ffffffffa051e7b0>] xt_compat_calc_jump+0x25/0x6f [x_tables]
> > > > > > [ 1170.545792]  RSP <ffff880121473cf8>
> > > > > > [ 1170.545794] CR2: 00000001dc1be9f8
> > > > > > [ 1170.654269] ---[ end trace d44667d90dcbd115 ]---
> > > > > > [ 1170.662411] fuse exit
> > > > > > Kernel logging (proc) stopped.
> > > > > > --
> > > > 
> > > > 
> > > > Hmm, commit 255d0dc34068a976550ce555e must have a problem for ebtables ?
> > > > 
> > > > Dann, could you give us what you do with ebtables ?
> > > > 
> > > > Thanks
> > > > 
> > > 
> > > For sure, there was a typo in above commit, but this is not enough to
> > > make ebtables work in COMPAT mode.
> > > 
> > > Hmm...
> > > 
> > 
> > Update : xt_compat_calc_jump() misses this bit, and I still have to find
> > the ebtables problem.
> > 
> > I'll provide a cumulative patch once done
> > 
> 
> Here is the cumulative patch

Thanks Eric. Unfortunately that didn't solve the problem I am seeing.
I rebaselined (same kernel build as before), and found that I'm able
to reproduce this 100% of the time by running only:

  sudo ebtables -t filter --init-table

The backtrace I received was this:
[   73.393223] ------------[ cut here ]------------
[   73.394944] WARNING: at net/netfilter/x_tables.c:476 xt_compat_calc_jump+0x64/0x6f [x_tables]()
[   73.396427] Hardware name: 2516CTO
[   73.398079] Modules linked in: ebtable_broute ebtable_filter ip6table_filter ip6_tables ebtable_nat ebtables ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc acpi_cpufreq mperf cpufreq_powersave cpufreq_userspace cpufreq_conservative cpufreq_stats kvm_intel kvm binfmt_misc fuse ext2 loop snd_hda_codec_hdmi snd_hda_codec_conexant arc4 ecb snd_usb_audio snd_usbmidi_lib snd_seq_midi snd_seq_midi_event snd_hda_intel snd_hda_codec iwlagn snd_hwdep snd_pcm snd_seq i915 snd_rawmidi thinkpad_acpi mac80211 snd_timer snd_seq_device btusb bluetooth psmouse battery tpm_tis cfg80211 drm_kms_helper drm serio_raw nvram evdev ac tpm tpm_bios i2c_algo_bit i2c_i801 snd power_supply soundcore rfkill wmi snd_page_alloc button i2c_core video processor ext4 mbcache jbd2 crc16 sha256_generic aesni_intel cryptd aes_x86_64 aes_generic cbc dm_crypt dm_mod sd_mod crc_t10dif usbhid hid usb_storage 
ahci libahci libata ehci_hcd scsi_mod usbcore e1000e thermal thermal_sys [last unloaded: scsi_wait_scan]
[   73.412341] Pid: 2891, comm: ebtables.orig Not tainted 2.6.39-rc1+ #9
[   73.414396] Call Trace:
[   73.416525]  [<ffffffff81041b99>] ? warn_slowpath_common+0x78/0x8c
[   73.418631]  [<ffffffffa05227ef>] ? xt_compat_calc_jump+0x64/0x6f [x_tables]
[   73.420758]  [<ffffffffa0571d46>] ? compat_do_replace+0x117/0x221 [ebtables]
[   73.422859]  [<ffffffffa0572392>] ? compat_do_ebt_set_ctl+0x55/0xbb [ebtables]
[   73.425030]  [<ffffffff810337e3>] ? need_resched+0x1a/0x23
[   73.427110]  [<ffffffff810337f1>] ? should_resched+0x5/0x24
[   73.429183]  [<ffffffff81314cc5>] ? _cond_resched+0x9/0x20
[   73.431290]  [<ffffffff813152fe>] ? mutex_lock_interruptible+0x18/0x32
[   73.433418]  [<ffffffff8128490b>] ? nf_sockopt_find.clone.1+0xda/0xec
[   73.435520]  [<ffffffff81284996>] ? compat_nf_sockopt+0x79/0xa5
[   73.437565]  [<ffffffff810337f1>] ? should_resched+0x5/0x24
[   73.439612]  [<ffffffff812849f3>] ? compat_nf_setsockopt+0x1a/0x1f
[   73.441666]  [<ffffffff8128fb35>] ? compat_ip_setsockopt+0x80/0xa0
[   73.443697]  [<ffffffff812784a2>] ? compat_sys_setsockopt+0x1d5/0x204
[   73.445705]  [<ffffffff810337f1>] ? should_resched+0x5/0x24
[   73.447739]  [<ffffffff81314cc5>] ? _cond_resched+0x9/0x20
[   73.449813]  [<ffffffff812788a5>] ? compat_sys_socketcall+0x148/0x1a7
[   73.451873]  [<ffffffff8131d2c0>] ? sysenter_dispatch+0x7/0x2e
[   73.453894] ---[ end trace 2285ecdee0e743d3 ]---
[   73.745725] Ebtables v2.0 unregistered

I reliably get the same backtrace, which is slightly different than
the one I originally submitted. I've only seen that original backtrace
once.

I then applied your patch, but I'm still seeing a similar backtrace:
[   33.143939] ------------[ cut here ]------------
[   33.146063] WARNING: at net/netfilter/x_tables.c:479 xt_compat_calc_jump+0x6f/0x7a [x_tables]()
[   33.148360] Hardware name: 2516CTO
[   33.150654] Modules linked in: ebtable_filter ip6table_filter ip6_tables ebtable_nat ebtables ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc acpi_cpufreq mperf cpufreq_powersave cpufreq_userspace cpufreq_conservative cpufreq_stats kvm_intel kvm binfmt_misc fuse ext2 loop snd_hda_codec_hdmi snd_hda_codec_conexant arc4 ecb thinkpad_acpi i915 snd_hda_intel iwlagn snd_hda_codec snd_hwdep snd_pcm mac80211 drm_kms_helper drm snd_seq snd_timer psmouse i2c_i801 btusb snd_seq_device bluetooth ac cfg80211 evdev tpm_tis snd serio_raw rfkill i2c_algo_bit tpm battery power_supply nvram wmi i2c_core tpm_bios soundcore snd_page_alloc button processor video ext4 mbcache jbd2 crc16 sha256_generic aesni_intel cryptd aes_x86_64 aes_generic cbc dm_crypt dm_mod sd_mod crc_t10dif ahci libahci ehci_hcd libata usbcore scsi_mod e1000e thermal thermal_sys [last unloaded: scsi_wait_scan]
[   33.167207] Pid: 2279, comm: ebtables Not tainted 2.6.39-rc1+ #11
[   33.169998] Call Trace:
[   33.172814]  [<ffffffff81041be9>] ? warn_slowpath_common+0x78/0x8c
[   33.175723]  [<ffffffffa04d7801>] ? xt_compat_calc_jump+0x6f/0x7a [x_tables]
[   33.178549]  [<ffffffffa0526d54>] ? compat_do_replace+0x125/0x22f [ebtables]
[   33.181370]  [<ffffffffa05273a0>] ? compat_do_ebt_set_ctl+0x55/0xb9 [ebtables]
[   33.184240]  [<ffffffff810337e3>] ? need_resched+0x1a/0x23
[   33.187055]  [<ffffffff810337f1>] ? should_resched+0x5/0x24
[   33.189805]  [<ffffffff81314d25>] ? _cond_resched+0x9/0x20
[   33.192578]  [<ffffffff8131535e>] ? mutex_lock_interruptible+0x18/0x32
[   33.195385]  [<ffffffff8128496b>] ? nf_sockopt_find.clone.1+0xda/0xec
[   33.198093]  [<ffffffff812849f6>] ? compat_nf_sockopt+0x79/0xa5
[   33.200852]  [<ffffffff810337f1>] ? should_resched+0x5/0x24
[   33.203618]  [<ffffffff81284a53>] ? compat_nf_setsockopt+0x1a/0x1f
[   33.206291]  [<ffffffff8128fb95>] ? compat_ip_setsockopt+0x80/0xa0
[   33.209001]  [<ffffffff81278502>] ? compat_sys_setsockopt+0x1d5/0x204
[   33.211726]  [<ffffffff810337f1>] ? should_resched+0x5/0x24
[   33.214374]  [<ffffffff81314d25>] ? _cond_resched+0x9/0x20
[   33.217083]  [<ffffffff81278905>] ? compat_sys_socketcall+0x148/0x1a7
[   33.219811]  [<ffffffff8131d340>] ? sysenter_dispatch+0x7/0x2e
[   33.222433] ---[ end trace 96f8ae34f1f5ad81 ]---

    -dann

> Thanks
> 
> [PATCH] netfilter: fix ebtables
> 
> commit 255d0dc34068a976 (netfilter: x_table: speedup compat operations)
> made ebtables not working anymore.
> 
> 1) xt_compat_calc_jump() is not an exact match lookup, and 
> 2) compat_table_info() has a typo in xt_compat_init_offsets() call
> 3) compat_do_replace() misses a xt_compat_init_offsets() call
> 
> Reported-by: dann frazier <dannf@dannf.org>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  net/bridge/netfilter/ebtables.c |    3 ++-
>  net/netfilter/x_tables.c        |    3 +++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
> index 893669c..c66aa80 100644
> --- a/net/bridge/netfilter/ebtables.c
> +++ b/net/bridge/netfilter/ebtables.c
> @@ -1766,7 +1766,7 @@ static int compat_table_info(const struct ebt_table_info *info,
>  
>  	newinfo->entries_size = size;
>  
> -	xt_compat_init_offsets(AF_INET, info->nentries);
> +	xt_compat_init_offsets(NFPROTO_BRIDGE, info->nentries /* + 4*/);
>  	return EBT_ENTRY_ITERATE(entries, size, compat_calc_entry, info,
>  							entries, newinfo);
>  }
> @@ -2240,6 +2240,7 @@ static int compat_do_replace(struct net *net, void __user *user,
>  
>  	xt_compat_lock(NFPROTO_BRIDGE);
>  
> +	xt_compat_init_offsets(NFPROTO_BRIDGE, tmp.nentries);
>  	ret = compat_copy_entries(entries_tmp, tmp.entries_size, &state);
>  	if (ret < 0)
>  		goto out_unlock;
> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index a9adf4c..e6dbec5 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -455,6 +455,7 @@ void xt_compat_flush_offsets(u_int8_t af)
>  		vfree(xt[af].compat_tab);
>  		xt[af].compat_tab = NULL;
>  		xt[af].number = 0;
> +		xt[af].cur = 0;
>  	}
>  }
>  EXPORT_SYMBOL_GPL(xt_compat_flush_offsets);
> @@ -473,6 +474,8 @@ int xt_compat_calc_jump(u_int8_t af, unsigned int offset)
>  		else
>  			return mid ? tmp[mid - 1].delta : 0;
>  	}
> +	if (left)
> +		return tmp[left - 1].delta;
>  	WARN_ON_ONCE(1);
>  	return 0;
>  }
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Fw: [Bug 32772] New: PROBLEM: kernel BUG at net/ipv4/inetpeer.c:386
From: Eric Dumazet @ 2011-04-06 16:25 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, dimetrios, David Miller
In-Reply-To: <1302105923.3209.103.camel@edumazet-laptop>

Le mercredi 06 avril 2011 à 18:05 +0200, Eric Dumazet a écrit :

> 
> This reminds me a possible memory corruption (from another layer)
> 

I found the reference of a past bug report, where slub_nomerge was used
too.

 http://www.spinics.net/lists/netdev/msg154206.html




^ permalink raw reply

* Re: [RFC] ixgbe: is DCA really that good ?
From: Eric Dumazet @ 2011-04-06 16:14 UTC (permalink / raw)
  To: Tom Herbert; +Cc: Alexander Duyck, Brattain, Jeff Kirsher, netdev
In-Reply-To: <BANLkTimSRhuVTcOX-EHx74_y3pnDdCRi8A@mail.gmail.com>

Le mercredi 06 avril 2011 à 08:24 -0700, Tom Herbert a écrit :
> On Wed, Apr 6, 2011 at 6:44 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Hi guys
> >
> >
> > In a forwarding [or RPS/RFS] setup, why should we populate cpu caches
> > with full frames content ? We only need first cache line to perform
> > routing [or RPS/RFS] decision.
> >
> DCA + accelerated RFS might make good.  But I agree that it should be
> configurable.

An IRQ affinity mismatch is fatal with dca-core current code, because

dca3_get_tag() ->
	dca_common_get_tag() ->
		spin_lock_irqsave(&dca_lock, flags);

Wei Gu hit this on a 64 cpus machine (but only 8 cpus servicing ixgbe
interrupts), and finding the problem took lot of time.




^ permalink raw reply

* Re: Fw: [Bug 32772] New: PROBLEM: kernel BUG at net/ipv4/inetpeer.c:386
From: Eric Dumazet @ 2011-04-06 16:05 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, dimetrios, David Miller
In-Reply-To: <20110406081818.4f0489b5@nehalam>

Le mercredi 06 avril 2011 à 08:18 -0700, Stephen Hemminger a écrit :
> 
> Begin forwarded message:
> 
> Date: Wed, 6 Apr 2011 07:39:54 GMT
> From: bugzilla-daemon@bugzilla.kernel.org
> To: shemminger@linux-foundation.org
> Subject: [Bug 32772] New: PROBLEM: kernel BUG at net/ipv4/inetpeer.c:386
> 
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=32772
> 
>            Summary: PROBLEM: kernel BUG at net/ipv4/inetpeer.c:386
>            Product: Networking
>            Version: 2.5
>     Kernel Version: 2.6.38
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: IPV4
>         AssignedTo: shemminger@linux-foundation.org
>         ReportedBy: dimetrios@gmail.com
>         Regression: No
> 
> 
> Kernel oopses periodically with 'kernel BUG at net/ipv4/inetpeer.c:386'
> message. Machine is used as BGP router and runs Quagga. Nonordinary kernel
> config option set: CONFIG_IP_FIB_TRIE=y.
> Two traces:
> --------------------trace begin--------------
> [625279.329241] kernel BUG at net/ipv4/inetpeer.c:386!

Hmm...

        if (atomic_cmpxchg(&p->refcnt, 1, -1) == 1) {
                struct inet_peer __rcu **stack[PEER_MAXDEPTH];
                struct inet_peer __rcu ***stackptr, ***delp;
                if (lookup(&p->daddr, stack, base) != p)
                        BUG();

So we cant find a peer in AVL tree, while we really should at this stage.


This reminds me a possible memory corruption (from another layer)

Could Dmitry try to boot with boot parameter "slub_nomerge" , to make sure
inetpeer layer doesnt share its kmem_cache with a corrupter ?




^ permalink raw reply

* Re: [PATCH 07/19] timberdale: mfd_cell is now implicitly available to drivers
From: Greg KH @ 2011-04-06 15:58 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: Grant Likely, Andres Salomon, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Mark Brown, khali-PUYAD+kWke1g9hUCZPvPmw,
	ben-linux-elnMNo+KYs3YtjvyW6yDsg, Peter Korsgaard,
	Mauro Carvalho Chehab, David Brownell,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Mocean Laboratories
In-Reply-To: <20110406152322.GA2757@sortiz-mobl>

On Wed, Apr 06, 2011 at 05:23:23PM +0200, Samuel Ortiz wrote:
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -33,6 +33,7 @@ struct class;
>  struct subsys_private;
>  struct bus_type;
>  struct device_node;
> +struct mfd_cell;
>  
>  struct bus_attribute {
>  	struct attribute	attr;
> @@ -444,6 +445,8 @@ struct device {
>  	struct device_node	*of_node; /* associated device tree node */
>  	const struct of_device_id *of_match; /* matching of_device_id from driver */
>  
> +	struct mfd_cell	*mfd_cell; /* MFD cell pointer */
> +

What is a "MFD cell pointer" and why is it needed in struct device?

thanks,

greg k-h

^ permalink raw reply

* [PATCH] be2net: Fix a potential crash during shutdown.
From: Ajit Khaparde @ 2011-04-06 15:53 UTC (permalink / raw)
  To: netdev, davem

adapter could remain uninitialized if probe fails for some reason.
A null pointer access could cause a crash if be_shutdown
is called after that.

Signed-off-by: Ajit Khaparde <ajit.khaparde@emulex.com>
---
 drivers/net/benet/be_main.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index a71163f..6e8e211 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -3141,12 +3141,14 @@ static int be_resume(struct pci_dev *pdev)
 static void be_shutdown(struct pci_dev *pdev)
 {
 	struct be_adapter *adapter = pci_get_drvdata(pdev);
-	struct net_device *netdev =  adapter->netdev;
 
-	if (netif_running(netdev))
+	if (!adapter)
+		return;
+
+	if (netif_running(adapter->netdev))
 		cancel_delayed_work_sync(&adapter->work);
 
-	netif_device_detach(netdev);
+	netif_device_detach(adapter->netdev);
 
 	be_cmd_reset_function(adapter);
 
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH net-next-2.6 v2] can: convert protocol handling to RCU
From: Kurt Van Dijck @ 2011-04-06 15:34 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: David Miller, Linux Netdev List, Eric Dumazet, Urs Thuermann
In-Reply-To: <4D9C7B07.20903@hartkopp.net>

On Wed, Apr 06, 2011 at 04:39:03PM +0200, Oliver Hartkopp wrote:
> On 06.04.2011 11:27, Kurt Van Dijck wrote:
> > On Tue, Apr 05, 2011 at 08:01:16PM +0200, Oliver Hartkopp wrote:
> 
> >>  
> >> +static struct can_proto *can_try_module_get(int protocol)
> >> +{
> >> +	struct can_proto *cp;
> >> +
> >> +	rcu_read_lock();
> >> +	cp = rcu_dereference(proto_tab[protocol]);
> >> +	if (cp && !try_module_get(cp->prot->owner))
> > After the xxx_get, is the 'cp' pointer persistent?
> 
> try_module_get() increases the usage counter of the module - therefore it is.
> It is protected until module_put(cp->prot->owner) at the end of can_create() .
Ok, I understand correctly now.
> 
> >>  	/* check for available protocol and correct usage */
> >>  
> >>  	if (!cp)
> >>  		return -EPROTONOSUPPORT;
> >>  
> >>  	if (cp->type != sock->type) {
> > I don't see how this will evaluate to true?
> > can_proto_register takes care of it.
> 
> This check compares the type of the socket that is to be created with the type
> that's defined for this protocol.
> 
> E.g. if you would give
> 
> s = socket(PF_CAN, SOCK_STREAM, CAN_RAW);
> 
> instead of the correct
> 
> s = socket(PF_CAN, SOCK_RAW, CAN_RAW);
> 
> you will get this error.

Right, I see my mistake now.
> 
> >> -		err = -EPROTONOSUPPORT;
> >> +		err = -EPROTOTYPE;
> >>  		goto errout;
> >>  	}
> >>  
> 
> Regards,
> Oliver

Regards,
Kurt

Acked-by: Kurt Van Dijck <kurt.van.dijck@eia.be>

^ permalink raw reply

* Re: [Patch] iwlwifi: remove obsoleted module alias and parameters
From: John W. Linville @ 2011-04-06 15:19 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Cong Wang, Johannes Berg, linux-wireless, netdev,
	Intel Linux Wireless, Wey-Yi Guy, Meenakshi Venkataraman,
	Larry Finger
In-Reply-To: <20110406151429.GD11941@tuxdriver.com>

On Wed, Apr 06, 2011 at 11:14:29AM -0400, John W. Linville wrote:
> On Wed, Apr 06, 2011 at 02:57:29PM +0200, Stanislaw Gruszka wrote:
> > On Wed, Apr 06, 2011 at 06:42:48PM +0800, Cong Wang wrote:
> > > 于 2011年04月06日 18:09, Johannes Berg 写道:
> > > >On Wed, 2011-04-06 at 17:49 +0800, Amerigo Wang wrote:
> > > >>As scheduled in Documentation/feature-removal-schedule.txt,
> > > >>remove "*50", "disable_hw_scan" module parameters and MODULE_ALIAS("iwl4965").
> > > >
> > > >Mostly fine, but for iwlegacy Stanislaw we want to keep hw scan (and it
> > > >was actually made default now)
> > 
> > Indeed, disable_hw_scan should be removed in iwlwifi but leaved in iwlegacy.
> > 
> > > Ok, I will wait for Stanislaw's response and then send an updated patch.
> > 
> > Have it now :-)
> 
> Maybe the MODULE_ALIAS("iwl4965") should go to iwlegacy too?

Nevermind, forgot the "Internal alias support has been present in
module-init-tools" part...

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ 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