Netdev List
 help / color / mirror / Atom feed
* [PATCH][RESEND] myri10ge: improve port type reporting in ethtool
From: Brice Goglin @ 2009-10-20  6:21 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Network Development list, Ben Hutchings

Improve the reporting of myri10ge port type in ethtool,
and update for new boards.

Signed-off-by: Brice Goglin <brice@myri.com>
Signed-off-by: Andrew Gallatin <gallatin@myri.com>

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 29c9fe2..5319db9 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -75,7 +75,7 @@
 #include "myri10ge_mcp.h"
 #include "myri10ge_mcp_gen_header.h"
 
-#define MYRI10GE_VERSION_STR "1.5.0-1.432"
+#define MYRI10GE_VERSION_STR "1.5.1-1.451"
 
 MODULE_DESCRIPTION("Myricom 10G driver (10GbE)");
 MODULE_AUTHOR("Maintainer: help@myri.com");
@@ -1623,10 +1623,21 @@ myri10ge_get_settings(struct net_device *netdev, struct ethtool_cmd *cmd)
 			return 0;
 		}
 	}
-	if (*ptr == 'R' || *ptr == 'Q') {
-		/* We've found either an XFP or quad ribbon fiber */
+	if (*ptr == '2')
+		ptr++;
+	if (*ptr == 'R' || *ptr == 'Q' || *ptr == 'S') {
+		/* We've found either an XFP, quad ribbon fiber, or SFP+ */
 		cmd->port = PORT_FIBRE;
+		cmd->supported |= SUPPORTED_FIBRE;
+		cmd->advertising |= ADVERTISED_FIBRE;
+	} else {
+		cmd->port = PORT_OTHER;
 	}
+	if (*ptr == 'R' || *ptr == 'S')
+		cmd->transceiver = XCVR_EXTERNAL;
+	else
+		cmd->transceiver = XCVR_INTERNAL;
+
 	return 0;
 }
 



^ permalink raw reply related

* build test your changes!
From: David Miller @ 2009-10-20  6:22 UTC (permalink / raw)
  To: krkumar2; +Cc: netdev


Krishna, you add:

+static inline void sk_tx_queue_set(struct sock *sk, int tx_queue)
...
+static inline void sk_tx_queue_clear(struct sock *sk)
...
+static inline int sk_tx_queue_get(const struct sock *sk)
...
+static inline bool sk_tx_queue_recorded(const struct sock *sk)

But then in the net/core/dev.c patch you go:

+			if (sk && sk->sk_dst_cache)
+				sk_record_tx_queue(sk, queue_index);

I cannot see how you did a build test of this, it's absolutely impossible.

I'm reverting.

^ permalink raw reply

* [PATCH RFC] net: add alloc_skb_mustcopy() to make clone copy
From: Jeremy Fitzhardinge @ 2009-10-20  6:19 UTC (permalink / raw)
  To: Herbert Xu, David Miller; +Cc: NetDev

[ RFC only ]

When doing network IO on behalf of guests, a Xen dom0 kernel gets granted
pages from guests.  These pages cannot be released normally, so we must
handle releasing them specially, which can do with a skb_destructor.

This is complicated by the fact that the lifetime of an skb's
frags can be extended via cloning.  To address this, this patch adds
alloc_skb_mustcopy() which allocates an skb with fclone==SKB_MUST_COPY.
If skb_clone() sees an skb with this set, it simply calls skb_copy
instead so that there are no aliases to the granted pages.

/* Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com> */

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f2c69a2..830203f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -228,6 +228,7 @@ enum {
 	SKB_FCLONE_UNAVAILABLE,
 	SKB_FCLONE_ORIG,
 	SKB_FCLONE_CLONE,
+	SKB_MUST_COPY,
 };
 
 enum {
@@ -449,13 +450,19 @@ extern struct sk_buff *__alloc_skb(unsigned int size,
 static inline struct sk_buff *alloc_skb(unsigned int size,
 					gfp_t priority)
 {
-	return __alloc_skb(size, priority, 0, -1);
+	return __alloc_skb(size, priority, SKB_FCLONE_UNAVAILABLE, -1);
 }
 
 static inline struct sk_buff *alloc_skb_fclone(unsigned int size,
 					       gfp_t priority)
 {
-	return __alloc_skb(size, priority, 1, -1);
+	return __alloc_skb(size, priority, SKB_FCLONE_ORIG, -1);
+}
+
+static inline struct sk_buff *alloc_skb_mustcopy(unsigned int size,
+					       gfp_t priority)
+{
+	return __alloc_skb(size, priority, SKB_MUST_COPY, -1);
 }
 
 extern int skb_recycle_check(struct sk_buff *skb, int skb_size);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 9e0597d..b130fab 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -177,7 +177,9 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
 	struct sk_buff *skb;
 	u8 *data;
 
-	cache = fclone ? skbuff_fclone_cache : skbuff_head_cache;
+	cache = skbuff_head_cache;
+	if (fclone == SKB_FCLONE_ORIG)
+		cache = skbuff_fclone_cache;
 
 	/* Get the HEAD */
 	skb = kmem_cache_alloc_node(cache, gfp_mask & ~__GFP_DMA, node);
@@ -220,13 +222,13 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
 	skb_frag_list_init(skb);
 	memset(&shinfo->hwtstamps, 0, sizeof(shinfo->hwtstamps));
 
-	if (fclone) {
+	skb->fclone = fclone;
+	if (fclone == SKB_FCLONE_ORIG) {
 		struct sk_buff *child = skb + 1;
 		atomic_t *fclone_ref = (atomic_t *) (child + 1);
 
 		kmemcheck_annotate_bitfield(child, flags1);
 		kmemcheck_annotate_bitfield(child, flags2);
-		skb->fclone = SKB_FCLONE_ORIG;
 		atomic_set(fclone_ref, 1);
 
 		child->fclone = SKB_FCLONE_UNAVAILABLE;
@@ -259,7 +261,8 @@ struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
 	int node = dev->dev.parent ? dev_to_node(dev->dev.parent) : -1;
 	struct sk_buff *skb;
 
-	skb = __alloc_skb(length + NET_SKB_PAD, gfp_mask, 0, node);
+	skb = __alloc_skb(length + NET_SKB_PAD, gfp_mask,
+			  SKB_FCLONE_UNAVAILABLE, node);
 	if (likely(skb)) {
 		skb_reserve(skb, NET_SKB_PAD);
 		skb->dev = dev;
@@ -364,6 +367,7 @@ static void kfree_skbmem(struct sk_buff *skb)
 
 	switch (skb->fclone) {
 	case SKB_FCLONE_UNAVAILABLE:
+	case SKB_MUST_COPY:
 		kmem_cache_free(skbuff_head_cache, skb);
 		break;
 
@@ -493,7 +497,9 @@ int skb_recycle_check(struct sk_buff *skb, int skb_size)
 {
 	struct skb_shared_info *shinfo;
 
-	if (skb_is_nonlinear(skb) || skb->fclone != SKB_FCLONE_UNAVAILABLE)
+	if (skb_is_nonlinear(skb) ||
+	    skb->fclone == SKB_FCLONE_ORIG ||
+	    skb->fclone == SKB_FCLONE_CLONE)
 		return 0;
 
 	skb_size = SKB_DATA_ALIGN(skb_size + NET_SKB_PAD);
@@ -640,6 +646,8 @@ struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
 		atomic_t *fclone_ref = (atomic_t *) (n + 1);
 		n->fclone = SKB_FCLONE_CLONE;
 		atomic_inc(fclone_ref);
+	} else if (skb->fclone == SKB_MUST_COPY) {
+		return skb_copy(skb, gfp_mask);
 	} else {
 		n = kmem_cache_alloc(skbuff_head_cache, gfp_mask);
 		if (!n)



^ permalink raw reply related

* Re: [PATCH v0 1/7] gianfar: Add per queue structure support
From: David Miller @ 2009-10-20  6:16 UTC (permalink / raw)
  To: sandeep.kumar; +Cc: netdev
In-Reply-To: <12556242491498-git-send-email-sandeep.kumar@freescale.com>


These changes look pretty good and I've let them sit on the list
for several days so I'm applying them to net-next-2.6, thanks!

^ permalink raw reply

* Re: [PATCH v2] can: provide library functions for skb allocation
From: David Miller @ 2009-10-20  6:05 UTC (permalink / raw)
  To: wg; +Cc: netdev, socketcan-core, haas, anantgole, mkl
In-Reply-To: <20091019.215440.256002623.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Mon, 19 Oct 2009 21:54:40 -0700 (PDT)

> Applied, thanks.

Actually I have to back this out, you have to add an extern
declaration for these public functions to a header the CAN
drivers will see, or else there'll be build warnings and
errors.

I wonder if you actually did this, but forgot that part of
the change or lost it somehow while respinning this.

Please fix this up and resubmit, thanks.

^ permalink raw reply

* Re: kernel panic in latest vanilla stable, while using nameif with "alive" pppoe interfaces
From: Cyrill Gorcunov @ 2009-10-20  6:04 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Michal Ostrowski, Denys Fedoryschenko, netdev, linux-ppp, paulus,
	mostrows
In-Reply-To: <aa79d98a0910192217x2a9bb142r114ae2fa633e3bd2@mail.gmail.com>

On 10/20/09, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> On 10/20/09, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>> This ultimately calls pppoe_flush_dev() and this function
>> takes care of taking appropriate sock_locks() on each sockets ?
>>
> This hold and lock socks but set pppoe_dev to null as well. I'll back
> later. And i need to reread the code.
>
from a second glance we have a race with pppoe_connect (err_put label)
and pppoe_flush_dev. Connect lock socket but while flushing we may
dev_put with null device. So as Eric pointed early the error path is
racy and should be protected with flush_lock.  Note that while
flushing pppoe we may do dev_put without socket locked. This should be
the corner case. Michal could you add this lock in pppoe_connect (i
cant do this at moment).

^ permalink raw reply

* tcp: Try to catch MSG_PEEK bug
From: Herbert Xu @ 2009-10-20  5:41 UTC (permalink / raw)
  To: David S. Miller, netdev

Hi:

tcp: Try to catch MSG_PEEK bug

This patch tries to print out more information when we hit the
MSG_PEEK bug in tcp_recvmsg.  It's been around since at least
2005 and it's about time that we finally fix it.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheer,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 64d0af6..fb881d5 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1405,7 +1405,9 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 				goto found_ok_skb;
 			if (tcp_hdr(skb)->fin)
 				goto found_fin_ok;
-			WARN_ON(!(flags & MSG_PEEK));
+			if (WARN_ON(!(flags & MSG_PEEK)))
+				printk(KERN_INFO "recvmsg bug 2: copied %X "
+				       "seq %X\n", *seq, TCP_SKB_CB(skb)->seq);
 		}
 
 		/* Well, if we have backlog, try to process it now yet. */

^ permalink raw reply related

* Re: [PATCH net-next-2.6] net: Introduce dev_get_by_index_rcu()
From: Eric Dumazet @ 2009-10-20  5:18 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20091020140632.79efb738@s6510>

Stephen Hemminger a écrit :
> On Tue, 20 Oct 2009 07:03:44 +0200
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
>> David Miller a écrit :
>>> From: Eric Dumazet <eric.dumazet@gmail.com>
>>> Date: Tue, 20 Oct 2009 06:23:54 +0200
>>>
>>>> I wonder if the whole thing could use RCU somehow, since some
>>>> workloads hit this dev_base_lock rwlock pretty hard...
>>> True, but for now we'll put your fix in :-)
>> [PATCH net-next-2.6] net: Introduce dev_get_by_index_rcu()
>>
>> Some workloads hit dev_base_lock rwlock pretty hard.
>> We can use RCU lookups to avoid touching this rwlock.
>>
>> netdevices are already freed after a RCU grace period, so this patch
>> adds no penalty at device dismantle time.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> All usage dev_base_lock should be replaceable by using combination of rtnl_mutex
> and RCU?

Yes probably, but I believe we should make step-by-step patches ?

1) __dev_get_by_index() is faster than dev_get_by_index_rcu()

2) I am not sure holding RTNL means we also have rcu_lock() implied.

However dev_ifname() could use rcu_lock() in the same patch, 
here is an updated version.

[PATCH net-next-2.6] net: Introduce dev_get_by_index_rcu()

Some workloads hit dev_base_lock rwlock pretty hard.
We can use RCU lookups to avoid touching this rwlock.

netdevices are already freed after a RCU grace period, so this patch
adds no penalty at device dismantle time.

dev_ifname() converted to dev_get_by_index_rcu()

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/netdevice.h |    1
 net/core/dev.c            |   48 ++++++++++++++++++++++++++++--------
 2 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8380009..4eda680 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1127,6 +1127,7 @@ extern void		netdev_resync_ops(struct net_device *dev);
 extern int call_netdevice_notifiers(unsigned long val, struct net_device *dev);
 extern struct net_device	*dev_get_by_index(struct net *net, int ifindex);
 extern struct net_device	*__dev_get_by_index(struct net *net, int ifindex);
+extern struct net_device	*dev_get_by_index_rcu(struct net *net, int ifindex);
 extern int		dev_restart(struct net_device *dev);
 #ifdef CONFIG_NETPOLL_TRAP
 extern int		netpoll_trap(void);
diff --git a/net/core/dev.c b/net/core/dev.c
index 28b0b9e..4564596 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -217,12 +217,15 @@ static int list_netdevice(struct net_device *dev)
 	write_lock_bh(&dev_base_lock);
 	list_add_tail(&dev->dev_list, &net->dev_base_head);
 	hlist_add_head(&dev->name_hlist, dev_name_hash(net, dev->name));
-	hlist_add_head(&dev->index_hlist, dev_index_hash(net, dev->ifindex));
+	hlist_add_head_rcu(&dev->index_hlist,
+			   dev_index_hash(net, dev->ifindex));
 	write_unlock_bh(&dev_base_lock);
 	return 0;
 }
 
-/* Device list removal */
+/* Device list removal
+ * caller must respect a RCU grace period before freeing/reusing dev
+ */
 static void unlist_netdevice(struct net_device *dev)
 {
 	ASSERT_RTNL();
@@ -231,7 +234,7 @@ static void unlist_netdevice(struct net_device *dev)
 	write_lock_bh(&dev_base_lock);
 	list_del(&dev->dev_list);
 	hlist_del(&dev->name_hlist);
-	hlist_del(&dev->index_hlist);
+	hlist_del_rcu(&dev->index_hlist);
 	write_unlock_bh(&dev_base_lock);
 }
 
@@ -649,6 +652,31 @@ struct net_device *__dev_get_by_index(struct net *net, int ifindex)
 }
 EXPORT_SYMBOL(__dev_get_by_index);
 
+/**
+ *	dev_get_by_index_rcu - find a device by its ifindex
+ *	@net: the applicable net namespace
+ *	@ifindex: index of device
+ *
+ *	Search for an interface by index. Returns %NULL if the device
+ *	is not found or a pointer to the device. The device has not
+ *	had its reference counter increased so the caller must be careful
+ *	about locking. The caller must hold RCU lock.
+ */
+
+struct net_device *dev_get_by_index_rcu(struct net *net, int ifindex)
+{
+	struct hlist_node *p;
+	struct net_device *dev;
+	struct hlist_head *head = dev_index_hash(net, ifindex);
+
+	hlist_for_each_entry_rcu(dev, p, head, index_hlist)
+		if (dev->ifindex == ifindex)
+			return dev;
+
+	return NULL;
+}
+EXPORT_SYMBOL(dev_get_by_index_rcu);
+
 
 /**
  *	dev_get_by_index - find a device by its ifindex
@@ -665,11 +693,11 @@ struct net_device *dev_get_by_index(struct net *net, int ifindex)
 {
 	struct net_device *dev;
 
-	read_lock(&dev_base_lock);
-	dev = __dev_get_by_index(net, ifindex);
+	rcu_read_lock();
+	dev = dev_get_by_index_rcu(net, ifindex);
 	if (dev)
 		dev_hold(dev);
-	read_unlock(&dev_base_lock);
+	rcu_read_unlock();
 	return dev;
 }
 EXPORT_SYMBOL(dev_get_by_index);
@@ -2930,15 +2958,15 @@ static int dev_ifname(struct net *net, struct ifreq __user *arg)
 	if (copy_from_user(&ifr, arg, sizeof(struct ifreq)))
 		return -EFAULT;
 
-	read_lock(&dev_base_lock);
-	dev = __dev_get_by_index(net, ifr.ifr_ifindex);
+	rcu_read_lock();
+	dev = dev_get_by_index_rcu(net, ifr.ifr_ifindex);
 	if (!dev) {
-		read_unlock(&dev_base_lock);
+		rcu_read_unlock();
 		return -ENODEV;
 	}
 
 	strcpy(ifr.ifr_name, dev->name);
-	read_unlock(&dev_base_lock);
+	rcu_read_unlock();
 
 	if (copy_to_user(arg, &ifr, sizeof(struct ifreq)))
 		return -EFAULT;


^ permalink raw reply related

* Re: kernel panic in latest vanilla stable, while using nameif with "alive" pppoe interfaces
From: Cyrill Gorcunov @ 2009-10-20  5:17 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Michal Ostrowski, Denys Fedoryschenko, netdev, linux-ppp, paulus,
	mostrows
In-Reply-To: <4ADD4518.8020909@gmail.com>

On 10/20/09, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Cyrill Gorcunov a écrit :
>> On 10/20/09, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> Michal Ostrowski a écrit :
>>>> Access of po->pppoe_dev is guarded by sk->sk_state & PPPOX_CONNECTED,
>>>> and all use cases now rely on the socket lock.  Because of this, the
>>>> ref-count on the namespace held by the socket object suffices to hold
>>>> the namespace in existence and so we don't need to ref-count the
>>>> namespace in PPPoE. The flush_lock is gone.
>>>>
>>> Seems good !
>>>
>>> But can we use lock_sock() in __pppoe_xmit() context ?
>>>
>>
>> Eric, most probably i miss something, but how lock sock protect us
>> from mtu changed via sysfs. This action calls change mtu notifier
>> which doesn't care about sockets at all...
>
> This ultimately calls pppoe_flush_dev() and this function
> takes care of taking appropriate sock_locks() on each sockets ?
>
This hold and lock socks but set pppoe_dev to null as well. I'll back
later. And i need to reread the code.

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: Introduce dev_get_by_index_rcu()
From: Stephen Hemminger @ 2009-10-20  5:06 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <4ADD44B0.8030204@gmail.com>

On Tue, 20 Oct 2009 07:03:44 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> David Miller a écrit :
> > From: Eric Dumazet <eric.dumazet@gmail.com>
> > Date: Tue, 20 Oct 2009 06:23:54 +0200
> > 
> >> I wonder if the whole thing could use RCU somehow, since some
> >> workloads hit this dev_base_lock rwlock pretty hard...
> > 
> > True, but for now we'll put your fix in :-)
> 
> [PATCH net-next-2.6] net: Introduce dev_get_by_index_rcu()
> 
> Some workloads hit dev_base_lock rwlock pretty hard.
> We can use RCU lookups to avoid touching this rwlock.
> 
> netdevices are already freed after a RCU grace period, so this patch
> adds no penalty at device dismantle time.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

All usage dev_base_lock should be replaceable by using combination of rtnl_mutex
and RCU?

^ permalink raw reply

* Re: kernel panic in latest vanilla stable, while using nameif with "alive" pppoe interfaces
From: Eric Dumazet @ 2009-10-20  5:05 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: Michal Ostrowski, Denys Fedoryschenko, netdev, linux-ppp, paulus,
	mostrows
In-Reply-To: <aa79d98a0910192202j4ea9f189g2ff719d57aa5a5eb@mail.gmail.com>

Cyrill Gorcunov a écrit :
> On 10/20/09, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> Michal Ostrowski a écrit :
>>> Access of po->pppoe_dev is guarded by sk->sk_state & PPPOX_CONNECTED,
>>> and all use cases now rely on the socket lock.  Because of this, the
>>> ref-count on the namespace held by the socket object suffices to hold
>>> the namespace in existence and so we don't need to ref-count the
>>> namespace in PPPoE. The flush_lock is gone.
>>>
>> Seems good !
>>
>> But can we use lock_sock() in __pppoe_xmit() context ?
>>
> 
> Eric, most probably i miss something, but how lock sock protect us
> from mtu changed via sysfs. This action calls change mtu notifier
> which doesn't care about sockets at all...

This ultimately calls pppoe_flush_dev() and this function
takes care of taking appropriate sock_locks() on each sockets ?

^ permalink raw reply

* [PATCH net-next-2.6] net: Introduce dev_get_by_index_rcu()
From: Eric Dumazet @ 2009-10-20  5:03 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20091019.212855.179405364.davem@davemloft.net>

David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 20 Oct 2009 06:23:54 +0200
> 
>> I wonder if the whole thing could use RCU somehow, since some
>> workloads hit this dev_base_lock rwlock pretty hard...
> 
> True, but for now we'll put your fix in :-)

[PATCH net-next-2.6] net: Introduce dev_get_by_index_rcu()

Some workloads hit dev_base_lock rwlock pretty hard.
We can use RCU lookups to avoid touching this rwlock.

netdevices are already freed after a RCU grace period, so this patch
adds no penalty at device dismantle time.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/netdevice.h |    1
 net/core/dev.c            |   40 ++++++++++++++++++++++++++++++------
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8380009..4eda680 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1127,6 +1127,7 @@ extern void		netdev_resync_ops(struct net_device *dev);
 extern int call_netdevice_notifiers(unsigned long val, struct net_device *dev);
 extern struct net_device	*dev_get_by_index(struct net *net, int ifindex);
 extern struct net_device	*__dev_get_by_index(struct net *net, int ifindex);
+extern struct net_device	*dev_get_by_index_rcu(struct net *net, int ifindex);
 extern int		dev_restart(struct net_device *dev);
 #ifdef CONFIG_NETPOLL_TRAP
 extern int		netpoll_trap(void);
diff --git a/net/core/dev.c b/net/core/dev.c
index 28b0b9e..cb011b7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -217,12 +217,15 @@ static int list_netdevice(struct net_device *dev)
 	write_lock_bh(&dev_base_lock);
 	list_add_tail(&dev->dev_list, &net->dev_base_head);
 	hlist_add_head(&dev->name_hlist, dev_name_hash(net, dev->name));
-	hlist_add_head(&dev->index_hlist, dev_index_hash(net, dev->ifindex));
+	hlist_add_head_rcu(&dev->index_hlist,
+			   dev_index_hash(net, dev->ifindex));
 	write_unlock_bh(&dev_base_lock);
 	return 0;
 }
 
-/* Device list removal */
+/* Device list removal
+ * caller must respect a RCU grace period before freeing/reusing dev
+ */
 static void unlist_netdevice(struct net_device *dev)
 {
 	ASSERT_RTNL();
@@ -231,7 +234,7 @@ static void unlist_netdevice(struct net_device *dev)
 	write_lock_bh(&dev_base_lock);
 	list_del(&dev->dev_list);
 	hlist_del(&dev->name_hlist);
-	hlist_del(&dev->index_hlist);
+	hlist_del_rcu(&dev->index_hlist);
 	write_unlock_bh(&dev_base_lock);
 }
 
@@ -649,6 +652,31 @@ struct net_device *__dev_get_by_index(struct net *net, int ifindex)
 }
 EXPORT_SYMBOL(__dev_get_by_index);
 
+/**
+ *	dev_get_by_index_rcu - find a device by its ifindex
+ *	@net: the applicable net namespace
+ *	@ifindex: index of device
+ *
+ *	Search for an interface by index. Returns %NULL if the device
+ *	is not found or a pointer to the device. The device has not
+ *	had its reference counter increased so the caller must be careful
+ *	about locking. The caller must hold RCU lock.
+ */
+
+struct net_device *dev_get_by_index_rcu(struct net *net, int ifindex)
+{
+	struct hlist_node *p;
+	struct net_device *dev;
+	struct hlist_head *head = dev_index_hash(net, ifindex);
+
+	hlist_for_each_entry_rcu(dev, p, head, index_hlist)
+		if (dev->ifindex == ifindex)
+			return dev;
+
+	return NULL;
+}
+EXPORT_SYMBOL(dev_get_by_index_rcu);
+
 
 /**
  *	dev_get_by_index - find a device by its ifindex
@@ -665,11 +693,11 @@ struct net_device *dev_get_by_index(struct net *net, int ifindex)
 {
 	struct net_device *dev;
 
-	read_lock(&dev_base_lock);
-	dev = __dev_get_by_index(net, ifindex);
+	rcu_read_lock();
+	dev = dev_get_by_index_rcu(net, ifindex);
 	if (dev)
 		dev_hold(dev);
-	read_unlock(&dev_base_lock);
+	rcu_read_unlock();
 	return dev;
 }
 EXPORT_SYMBOL(dev_get_by_index);

^ permalink raw reply related

* Re: kernel panic in latest vanilla stable, while using nameif with "alive" pppoe interfaces
From: Cyrill Gorcunov @ 2009-10-20  5:02 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Michal Ostrowski, Denys Fedoryschenko, netdev, linux-ppp, paulus,
	mostrows
In-Reply-To: <4ADD31A2.4030702@gmail.com>

On 10/20/09, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Michal Ostrowski a écrit :
>> Access of po->pppoe_dev is guarded by sk->sk_state & PPPOX_CONNECTED,
>> and all use cases now rely on the socket lock.  Because of this, the
>> ref-count on the namespace held by the socket object suffices to hold
>> the namespace in existence and so we don't need to ref-count the
>> namespace in PPPoE. The flush_lock is gone.
>>
>
> Seems good !
>
> But can we use lock_sock() in __pppoe_xmit() context ?
>

Eric, most probably i miss something, but how lock sock protect us
from mtu changed via sysfs. This action calls change mtu notifier
which doesn't care about sockets at all...

^ permalink raw reply

* Re: [PATCH net-next V2 3/3] i2400m-sdio: select IWMC3200TOP in Kconfig
From: David Miller @ 2009-10-20  4:55 UTC (permalink / raw)
  To: tomas.winkler
  Cc: linville, netdev, linux-wireless, linux-mmc, yi.zhu,
	inaky.perez-gonzalez, cindy.h.kao, guy.cohen, ron.rindjunsky
In-Reply-To: <1255806576-26869-3-git-send-email-tomas.winkler@intel.com>

From: Tomas Winkler <tomas.winkler@intel.com>
Date: Sat, 17 Oct 2009 21:09:36 +0200

> i2400m-sdio requires iwmc3200top for its operation
> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Acked-by: Inaky Perez-Gonzalez <inaky@linux.intel.com>

Applied to net-next-2.6, thanks!

^ permalink raw reply

* Re: PATCH [net-next-2.6] IP: Cleanups
From: David Miller @ 2009-10-20  4:55 UTC (permalink / raw)
  To: john.dykstra1; +Cc: netdev
In-Reply-To: <1255980690.20673.5.camel@Maple>

From: John Dykstra <john.dykstra1@gmail.com>
Date: Mon, 19 Oct 2009 14:31:30 -0500

> Use symbols instead of magic constants while checking PMTU discovery
> setsockopt.
> 
> Remove redundant test in ip_rt_frag_needed() (done by caller).
> 
> Signed-off-by: John Dykstra <john.dykstra1@gmail.com>

Applied to net-next-2.6, thanks.

^ permalink raw reply

* Re: [PATCH net-next V2 2/3] iwmc3200wifi: select IWMC3200TOP in Kconfig
From: David Miller @ 2009-10-20  4:55 UTC (permalink / raw)
  To: tomas.winkler
  Cc: linville, netdev, linux-wireless, linux-mmc, yi.zhu,
	inaky.perez-gonzalez, cindy.h.kao, guy.cohen, ron.rindjunsky
In-Reply-To: <1255806576-26869-2-git-send-email-tomas.winkler@intel.com>

From: Tomas Winkler <tomas.winkler@intel.com>
Date: Sat, 17 Oct 2009 21:09:35 +0200

> iwmc3200wifi requires iwmc3200top  for its operation
> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Acked-by: Zhu Yi <yi.zhu@intel.com>

Applied to net-next-2.6

^ permalink raw reply

* Re: [PATCH net-next V2 1/3] iwmc3200top: Add Intel Wireless MultiCom 3200 top driver.
From: David Miller @ 2009-10-20  4:54 UTC (permalink / raw)
  To: tomas.winkler
  Cc: linville, netdev, linux-wireless, linux-mmc, yi.zhu,
	inaky.perez-gonzalez, cindy.h.kao, guy.cohen, ron.rindjunsky
In-Reply-To: <1255806576-26869-1-git-send-email-tomas.winkler@intel.com>

From: Tomas Winkler <tomas.winkler@intel.com>
Date: Sat, 17 Oct 2009 21:09:34 +0200

> This patch adds Intel Wireless MultiCom 3200 top driver.
> IWMC3200 is 4Wireless Com CHIP (GPS/BT/WiFi/WiMAX).
> Top driver is responsible for device initialization and firmware download.
> Firmware handled by top is responsible for top itself and
> as well as bluetooth and GPS coms. (Wifi and WiMax provide their own firmware)
> In addition top driver is used to retrieve firmware logs
> and supports other debugging features
> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

Applied to net-next-2.6

^ permalink raw reply

* Re: [PATCH v2] can: provide library functions for skb allocation
From: David Miller @ 2009-10-20  4:54 UTC (permalink / raw)
  To: wg; +Cc: netdev, socketcan-core, haas, anantgole, mkl
In-Reply-To: <4ADB6243.2040109@grandegger.com>

From: Wolfgang Grandegger <wg@grandegger.com>
Date: Sun, 18 Oct 2009 20:45:23 +0200

> This patch makes the private functions alloc_can_skb() and
> alloc_can_err_skb() of the at91_can driver public and adapts all
> drivers to use these. While making the patch I realized, that
> the skb's are *not* setup consistently. It's now done as shown
> below:
> 
>   skb->protocol = htons(ETH_P_CAN);
>   skb->pkt_type = PACKET_BROADCAST;
>   skb->ip_summed = CHECKSUM_UNNECESSARY;
>   *cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
>   memset(*cf, 0, sizeof(struct can_frame));
> 
> The frame is zeroed out to avoid uninitialized data to be passed to
> user space. Some drivers or library code did not set "pkt_type" or 
> "ip_summed". Also,  "__constant_htons()" should not be used for
> runtime invocations, as pointed out by David Miller.
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>

Applied, thanks.

^ permalink raw reply

* Re: pktgen and spin_lock_bh in xmit path
From: Ben Greear @ 2009-10-20  4:52 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: NetDev
In-Reply-To: <4ADD32FA.6030409@gmail.com>

Eric Dumazet wrote:
> Ben Greear a écrit :
>   
>> I'm having strange issues when running pktgen on 10G interfaces while
>> also running
>> pktgen on mac-vlans on that interface, when the mac-vlan pktgen threads
>> are on a different
>> CPU.
>>
>> First, lockdep gives up and says that things are not properly
>> annotated.  I believe this is because
>> the macvlan tx path will lock it's txq and will also lock the
>> lower-dev's txq.  To fix this, perhaps
>> we need some new lockdep aware primitives for netdev txq locking?
>>
>> Second, is using _bh() locking really sufficient if we have pktgen
>> writing to a physical device
>> and also have other pktgen threads writing to that same device though
>> mac-vlans?   I'm seeing
>> deadlocks spinning on the _bh() lock in pktgen as well as strange
>> corruptions, so I think there
>> must be *some* problem somewhere, I just don't know quite what it is yet.
>>
>>     
>
> Could you please give us a copy if your pktgen scripts ?
>   
I'm driving it with another program, and my pktgen is a bit hacked, but 
the basic idea is:

1 pktgen connection on cpu 0 running as fast as it can (trying for 
10Gbps, but getting maybe 3-4),
  running between two 10G ports (intel 82599).
  Multi-pkt is set to 10,000 on each side.
3 pairs of mac-vlans on each of the two physical 10G ports.
 3 pktgen 'connections' between these..each are running at about 1Gbps.
 These 3 pktgen connections are on CPU 4.
 Multi-pkt is set to 1 since multi-pkt is a very bad idea on virtual 
devices.

1514 byte pkts.  No IPs on the interfaces, using ToS in pktgen, but 
nothing else is configured to
care.

The two physical ports are cabled together directly with a fibre cable.

All pktgen connections are full duplex (both sides transmitting to each 
other..and I have
rx logic to gather stats on received pkts as well).  With no kernel 
debugging, this can run right at 10Gbps bi-directional,
with lockdep it gets around 5-6Gbps in each direction.

The lockup often occurs near starting/stopping pktgen, but also happens 
while just normally
running under load, usually within 10 minutes.

I tried and failed to reproduce this on a 1G network, but maybe I'm just 
not getting (un)lucky,
didn't try for too long.

Among other things, it appears as if the mac-vlan interfaces sometimes 
become locked to transmit
by pktgen, but a raw socket in user-space can send on them fine.  I'm 
going to add some debugging
for this particular issue tomorrow to try to figure out why that happens.

Please note I have the rest of my network patches applied (but not using 
any proprietary modules),
so it could easily be something I've caused.  I think fixing lockdep to 
work with the txq _bh locks
would be a good first step to fixing this..

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com



^ permalink raw reply

* Re: [PATCH] net: Fix IP_MULTICAST_IF
From: David Miller @ 2009-10-20  4:28 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <4ADD3B5A.1080905@gmail.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 20 Oct 2009 06:23:54 +0200

> I wonder if the whole thing could use RCU somehow, since some
> workloads hit this dev_base_lock rwlock pretty hard...

True, but for now we'll put your fix in :-)

^ permalink raw reply

* Re: [PATCH] net: Fix IP_MULTICAST_IF
From: Eric Dumazet @ 2009-10-20  4:23 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20091019.212018.79580287.davem@davemloft.net>

David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 20 Oct 2009 06:07:48 +0200
> 
>> Final target might be fine, but an element in the chain, before target
>> could be deleted while reader scans hash chain.
>  ...
>> So it should visit a nice memory location ?
> 
> It should hit a NULL eventually and deterministically even if an
> unlink happens at the same time..... unless the object gets free'd
> meanwhile, hmmm...
> 
> This code is definitely intentional, I remember when I added it to
> the tree, Alexey wrote it :-)

I wonder if the whole thing could use RCU somehow, since some workloads hit
this dev_base_lock rwlock pretty hard...


^ permalink raw reply

* Re: [PATCH] net: Fix IP_MULTICAST_IF
From: David Miller @ 2009-10-20  4:21 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <4ADD3982.2040100@gmail.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 20 Oct 2009 06:16:02 +0200

> BTW, even an insertion can crash a lockless reader, since reader
>  could see a corrupt n->next (hlist_add_head() has no barrier
>  between n->next = first and h->first = n;)

Ok, now that convinces it for me, I'll apply your patch, thanks!

^ permalink raw reply

* Re: [PATCH] net: Fix IP_MULTICAST_IF
From: David Miller @ 2009-10-20  4:20 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <4ADD3794.8030906@gmail.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 20 Oct 2009 06:07:48 +0200

> Final target might be fine, but an element in the chain, before target
> could be deleted while reader scans hash chain.
 ...
> So it should visit a nice memory location ?

It should hit a NULL eventually and deterministically even if an
unlink happens at the same time..... unless the object gets free'd
meanwhile, hmmm...

This code is definitely intentional, I remember when I added it to
the tree, Alexey wrote it :-)

^ permalink raw reply

* Re: [PATCH 4/4 v3] net: Fix for dst_negative_advice
From: David Miller @ 2009-10-20  4:17 UTC (permalink / raw)
  To: krkumar2; +Cc: shemminger, dada1, herbert, netdev
In-Reply-To: <OF47BDCCCB.33D1775A-ON65257654.00178901-65257654.00192AA2@in.ibm.com>

From: Krishna Kumar2 <krkumar2@in.ibm.com>
Date: Mon, 19 Oct 2009 10:04:53 +0530

> Should I resubmit with the changed order?

I took care of this.

I put the patch that actually makes dev.c use sk_tx_queue_mapping
last in the set.

All applied, thanks everyone.

^ permalink raw reply

* Re: [PATCH] net: Fix IP_MULTICAST_IF
From: Eric Dumazet @ 2009-10-20  4:16 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <4ADD3794.8030906@gmail.com>

Eric Dumazet a écrit :
> David Miller a écrit :
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Mon, 19 Oct 2009 18:41:58 +0200
>>
>>> ipv4/ipv6 setsockopt(IP_MULTICAST_IF) have dubious __dev_get_by_index() calls.
>> Dubious, how so?
>>
>> Yes, I know RTNL/dev_base_lock, but it's not using what it gets
>> back at all.
>>
>> It's testing existence, a boolean, it doesn't dereference the
>> 'dev' it gets back at all.
>>
>> This code is intentional and perfectly fine.
> 
> If this was intentional, something changed and made the prereq false.
> 
> Final target might be fine, but an element in the chain, before target
> could be deleted while reader scans hash chain.
> 

BTW, even an insertion can crash a lockless reader, since reader could see a corrupt
 n->next (hlist_add_head() has no barrier between n->next = first and h->first = n;)

static inline void hlist_add_head(struct hlist_node *n, struct hlist_head *h)
{
        struct hlist_node *first = h->first;
        n->next = first;
        if (first)
                first->pprev = &n->next;
        h->first = n;
        n->pprev = &h->first;
}


^ 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