* [PATCH net-next-2.6] ipv6: use RCU to walk list of network devices
From: Eric Dumazet @ 2009-11-12 3:34 UTC (permalink / raw)
To: David Miller; +Cc: Stephen Hemminger, netdev
In-Reply-To: <20091110175647.683253741@vyatta.com>
Stephen Hemminger a écrit :
> No longer need read_lock(&dev_base_lock), use RCU instead.
> This also needs to be optimized for large number of devices.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
I gave a look at your patch Stephen and found we need a new
next_net_device_rcu(struct net_device *dev) as well,
as next_net_device() is not RCU safe.
(followup patch is probably needed to use it in net/ipv4/igmp.c,
after commit 61fbab77a843d2e77232 : IPV4: use rcu to walk list of devices in IGMP)
We also can avoid taking references on inet6_dev structs.
[PATCH net-next-2.6] ipv6: use RCU to walk list of network devices
No longer need read_lock(&dev_base_lock), use RCU instead.
We also can avoid taking references on inet6_dev structs.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/netdevice.h | 10 +++++++
net/ipv6/anycast.c | 29 +++++++++-----------
net/ipv6/mcast.c | 51 ++++++++++++++++--------------------
3 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 083b598..2734a67 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1093,6 +1093,16 @@ static inline struct net_device *next_net_device(struct net_device *dev)
return lh == &net->dev_base_head ? NULL : net_device_entry(lh);
}
+static inline struct net_device *next_net_device_rcu(struct net_device *dev)
+{
+ struct list_head *lh;
+ struct net *net;
+
+ net = dev_net(dev);
+ lh = rcu_dereference(dev->dev_list.next);
+ return lh == &net->dev_base_head ? NULL : net_device_entry(lh);
+}
+
static inline struct net_device *first_net_device(struct net *net)
{
return list_empty(&net->dev_base_head) ? NULL :
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index 2f00ca8..f1c74c8 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -431,9 +431,9 @@ static inline struct ifacaddr6 *ac6_get_first(struct seq_file *seq)
struct net *net = seq_file_net(seq);
state->idev = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct inet6_dev *idev;
- idev = in6_dev_get(state->dev);
+ idev = __in6_dev_get(state->dev);
if (!idev)
continue;
read_lock_bh(&idev->lock);
@@ -443,7 +443,6 @@ static inline struct ifacaddr6 *ac6_get_first(struct seq_file *seq)
break;
}
read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
}
return im;
}
@@ -454,16 +453,15 @@ static struct ifacaddr6 *ac6_get_next(struct seq_file *seq, struct ifacaddr6 *im
im = im->aca_next;
while (!im) {
- if (likely(state->idev != NULL)) {
+ if (likely(state->idev != NULL))
read_unlock_bh(&state->idev->lock);
- in6_dev_put(state->idev);
- }
- state->dev = next_net_device(state->dev);
+
+ state->dev = next_net_device_rcu(state->dev);
if (!state->dev) {
state->idev = NULL;
break;
}
- state->idev = in6_dev_get(state->dev);
+ state->idev = __in6_dev_get(state->dev);
if (!state->idev)
continue;
read_lock_bh(&state->idev->lock);
@@ -482,29 +480,30 @@ static struct ifacaddr6 *ac6_get_idx(struct seq_file *seq, loff_t pos)
}
static void *ac6_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(RCU)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return ac6_get_idx(seq, *pos);
}
static void *ac6_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
- struct ifacaddr6 *im;
- im = ac6_get_next(seq, v);
+ struct ifacaddr6 *im = ac6_get_next(seq, v);
+
++*pos;
return im;
}
static void ac6_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(RCU)
{
struct ac6_iter_state *state = ac6_seq_private(seq);
+
if (likely(state->idev != NULL)) {
read_unlock_bh(&state->idev->lock);
- in6_dev_put(state->idev);
+ state->idev = NULL;
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int ac6_seq_show(struct seq_file *seq, void *v)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index f9fcf69..1f9c444 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -2375,9 +2375,9 @@ static inline struct ifmcaddr6 *igmp6_mc_get_first(struct seq_file *seq)
struct net *net = seq_file_net(seq);
state->idev = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct inet6_dev *idev;
- idev = in6_dev_get(state->dev);
+ idev = __in6_dev_get(state->dev);
if (!idev)
continue;
read_lock_bh(&idev->lock);
@@ -2387,7 +2387,6 @@ static inline struct ifmcaddr6 *igmp6_mc_get_first(struct seq_file *seq)
break;
}
read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
}
return im;
}
@@ -2398,16 +2397,15 @@ static struct ifmcaddr6 *igmp6_mc_get_next(struct seq_file *seq, struct ifmcaddr
im = im->next;
while (!im) {
- if (likely(state->idev != NULL)) {
+ if (likely(state->idev != NULL))
read_unlock_bh(&state->idev->lock);
- in6_dev_put(state->idev);
- }
- state->dev = next_net_device(state->dev);
+
+ state->dev = next_net_device_rcu(state->dev);
if (!state->dev) {
state->idev = NULL;
break;
}
- state->idev = in6_dev_get(state->dev);
+ state->idev = __in6_dev_get(state->dev);
if (!state->idev)
continue;
read_lock_bh(&state->idev->lock);
@@ -2426,31 +2424,31 @@ static struct ifmcaddr6 *igmp6_mc_get_idx(struct seq_file *seq, loff_t pos)
}
static void *igmp6_mc_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(RCU)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return igmp6_mc_get_idx(seq, *pos);
}
static void *igmp6_mc_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
- struct ifmcaddr6 *im;
- im = igmp6_mc_get_next(seq, v);
+ struct ifmcaddr6 *im = igmp6_mc_get_next(seq, v);
+
++*pos;
return im;
}
static void igmp6_mc_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(RCU)
{
struct igmp6_mc_iter_state *state = igmp6_mc_seq_private(seq);
+
if (likely(state->idev != NULL)) {
read_unlock_bh(&state->idev->lock);
- in6_dev_put(state->idev);
state->idev = NULL;
}
state->dev = NULL;
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int igmp6_mc_seq_show(struct seq_file *seq, void *v)
@@ -2507,9 +2505,9 @@ static inline struct ip6_sf_list *igmp6_mcf_get_first(struct seq_file *seq)
state->idev = NULL;
state->im = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct inet6_dev *idev;
- idev = in6_dev_get(state->dev);
+ idev = __in6_dev_get(state->dev);
if (unlikely(idev == NULL))
continue;
read_lock_bh(&idev->lock);
@@ -2525,7 +2523,6 @@ static inline struct ip6_sf_list *igmp6_mcf_get_first(struct seq_file *seq)
spin_unlock_bh(&im->mca_lock);
}
read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
}
return psf;
}
@@ -2539,16 +2536,15 @@ static struct ip6_sf_list *igmp6_mcf_get_next(struct seq_file *seq, struct ip6_s
spin_unlock_bh(&state->im->mca_lock);
state->im = state->im->next;
while (!state->im) {
- if (likely(state->idev != NULL)) {
+ if (likely(state->idev != NULL))
read_unlock_bh(&state->idev->lock);
- in6_dev_put(state->idev);
- }
- state->dev = next_net_device(state->dev);
+
+ state->dev = next_net_device_rcu(state->dev);
if (!state->dev) {
state->idev = NULL;
goto out;
}
- state->idev = in6_dev_get(state->dev);
+ state->idev = __in6_dev_get(state->dev);
if (!state->idev)
continue;
read_lock_bh(&state->idev->lock);
@@ -2573,9 +2569,9 @@ static struct ip6_sf_list *igmp6_mcf_get_idx(struct seq_file *seq, loff_t pos)
}
static void *igmp6_mcf_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(RCU)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return *pos ? igmp6_mcf_get_idx(seq, *pos - 1) : SEQ_START_TOKEN;
}
@@ -2591,7 +2587,7 @@ static void *igmp6_mcf_seq_next(struct seq_file *seq, void *v, loff_t *pos)
}
static void igmp6_mcf_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(RCU)
{
struct igmp6_mcf_iter_state *state = igmp6_mcf_seq_private(seq);
if (likely(state->im != NULL)) {
@@ -2600,11 +2596,10 @@ static void igmp6_mcf_seq_stop(struct seq_file *seq, void *v)
}
if (likely(state->idev != NULL)) {
read_unlock_bh(&state->idev->lock);
- in6_dev_put(state->idev);
state->idev = NULL;
}
state->dev = NULL;
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int igmp6_mcf_seq_show(struct seq_file *seq, void *v)
^ permalink raw reply related
* Re: [PATCH net-2.6] Revert "net: Support inclusion of <linux/socket.h> before <sys/socket.h>"
From: Ben Hutchings @ 2009-11-12 3:38 UTC (permalink / raw)
To: Ulrich Drepper; +Cc: David Miller, netdev, 538372
In-Reply-To: <20091111.190924.27363037.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]
On Wed, 2009-11-11 at 19:09 -0800, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Thu, 12 Nov 2009 03:05:15 +0000
>
> > <linux/netlink.h> will not compile for userland, because
> > <linux/socket.h> is no longer defining sa_family_t. For userland, this
> > should be defined by <sys/socket.h>.
>
> Still, you still essentially have two choices:
>
> 1) Tell userland, sorry you need to include sys/socket.h before
> other "socket stuff" and that means linux/netlink.h in
> particular
Which is just nasty; headers should include everything they need.
> 2) Put a !__KERNEL__ sys/socket.h include in there, but that's
> asking for trouble.
Indeed.
Including <bits/sockaddr.h> would get us exactly what we need, but
that's not meant to be included directly.
Ulrich, can you make any suggestions as to how we can resolve this?
Ben.
--
Ben Hutchings
Lowery's Law:
If it jams, force it. If it breaks, it needed replacing anyway.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* [PATCH net-next-2.6] igmp: Use next_net_device_rcu()
From: Eric Dumazet @ 2009-11-12 3:48 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List, Stephen Hemminger
We need to use next_det_device_rcu() in RCU protected section.
We also can avoid in_dev_get()/in_dev_put() overhead (code size mainly)
in rcu_read_lock() sections.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv4/igmp.c | 27 +++++++++++----------------
1 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index bd24f65..6110c6d 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2313,7 +2313,8 @@ static inline struct ip_mc_list *igmp_mc_get_first(struct seq_file *seq)
state->in_dev = NULL;
for_each_netdev_rcu(net, state->dev) {
struct in_device *in_dev;
- in_dev = in_dev_get(state->dev);
+
+ in_dev = __in_dev_get_rcu(state->dev);
if (!in_dev)
continue;
read_lock(&in_dev->mc_list_lock);
@@ -2323,7 +2324,6 @@ static inline struct ip_mc_list *igmp_mc_get_first(struct seq_file *seq)
break;
}
read_unlock(&in_dev->mc_list_lock);
- in_dev_put(in_dev);
}
return im;
}
@@ -2333,16 +2333,15 @@ static struct ip_mc_list *igmp_mc_get_next(struct seq_file *seq, struct ip_mc_li
struct igmp_mc_iter_state *state = igmp_mc_seq_private(seq);
im = im->next;
while (!im) {
- if (likely(state->in_dev != NULL)) {
+ if (likely(state->in_dev != NULL))
read_unlock(&state->in_dev->mc_list_lock);
- in_dev_put(state->in_dev);
- }
- state->dev = next_net_device(state->dev);
+
+ state->dev = next_net_device_rcu(state->dev);
if (!state->dev) {
state->in_dev = NULL;
break;
}
- state->in_dev = in_dev_get(state->dev);
+ state->in_dev = __in_dev_get_rcu(state->dev);
if (!state->in_dev)
continue;
read_lock(&state->in_dev->mc_list_lock);
@@ -2384,7 +2383,6 @@ static void igmp_mc_seq_stop(struct seq_file *seq, void *v)
struct igmp_mc_iter_state *state = igmp_mc_seq_private(seq);
if (likely(state->in_dev != NULL)) {
read_unlock(&state->in_dev->mc_list_lock);
- in_dev_put(state->in_dev);
state->in_dev = NULL;
}
state->dev = NULL;
@@ -2464,7 +2462,7 @@ static inline struct ip_sf_list *igmp_mcf_get_first(struct seq_file *seq)
state->im = NULL;
for_each_netdev_rcu(net, state->dev) {
struct in_device *idev;
- idev = in_dev_get(state->dev);
+ idev = __in_dev_get_rcu(state->dev);
if (unlikely(idev == NULL))
continue;
read_lock(&idev->mc_list_lock);
@@ -2480,7 +2478,6 @@ static inline struct ip_sf_list *igmp_mcf_get_first(struct seq_file *seq)
spin_unlock_bh(&im->lock);
}
read_unlock(&idev->mc_list_lock);
- in_dev_put(idev);
}
return psf;
}
@@ -2494,16 +2491,15 @@ static struct ip_sf_list *igmp_mcf_get_next(struct seq_file *seq, struct ip_sf_l
spin_unlock_bh(&state->im->lock);
state->im = state->im->next;
while (!state->im) {
- if (likely(state->idev != NULL)) {
+ if (likely(state->idev != NULL))
read_unlock(&state->idev->mc_list_lock);
- in_dev_put(state->idev);
- }
- state->dev = next_net_device(state->dev);
+
+ state->dev = next_net_device_rcu(state->dev);
if (!state->dev) {
state->idev = NULL;
goto out;
}
- state->idev = in_dev_get(state->dev);
+ state->idev = __in_dev_get_rcu(state->dev);
if (!state->idev)
continue;
read_lock(&state->idev->mc_list_lock);
@@ -2555,7 +2551,6 @@ static void igmp_mcf_seq_stop(struct seq_file *seq, void *v)
}
if (likely(state->idev != NULL)) {
read_unlock(&state->idev->mc_list_lock);
- in_dev_put(state->idev);
state->idev = NULL;
}
state->dev = NULL;
^ permalink raw reply related
* Re: [net-next PATCH 0/4] qlge: Fixes and clean up for qlge.
From: David Miller @ 2009-11-12 3:55 UTC (permalink / raw)
To: ron.mercer; +Cc: netdev
In-Reply-To: <1257980046-2420-1-git-send-email-ron.mercer@qlogic.com>
All applied to net-next-2.6, thanks.
^ permalink raw reply
* Re: [PATCH 1/2] decnet: add RTNL lock when reading address list
From: David Miller @ 2009-11-12 3:56 UTC (permalink / raw)
To: shemminger
Cc: steve, eric.dumazet, christine.caulfield, hannes, adobriyan,
swhiteho, netdev, linux-decnet-users
In-Reply-To: <20091111093927.4a08999d@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 11 Nov 2009 09:39:27 -0800
> Add missing locking in the case of auto binding to the
> default device. The address list might change while this code is looking
> at the list.
>
> Compile tested only, I am not a decnet user.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied to net-next-2.6
^ permalink raw reply
* Re: [PATCH 2/2] decnet: convert dndev_lock to spinlock
From: David Miller @ 2009-11-12 3:56 UTC (permalink / raw)
To: shemminger
Cc: steve, eric.dumazet, christine.caulfield, hannes, adobriyan,
swhiteho, netdev, linux-decnet-users
In-Reply-To: <20091111094036.736762da@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 11 Nov 2009 09:40:36 -0800
> There is no reason for this lock to be reader/writer since
> the reader only has lock held for a very brief period.
> The overhead of read_lock is more expensive than spinlock.
>
> Compile tested only, I am not a decnet user.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied to net-next-2.6
^ permalink raw reply
* Re: [PATCH] net/compat: fix dev_ifsioc emulation corner cases
From: David Miller @ 2009-11-12 3:56 UTC (permalink / raw)
To: arnd; +Cc: linux-kernel, patrick.ohly, netdev
In-Reply-To: <200911111439.40753.arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 11 Nov 2009 14:39:40 +0100
> Handling for SIOCSHWTSTAMP is broken on architectures
> with a split user/kernel address space like s390,
> because it passes a real user pointer while using
> set_fs(KERNEL_DS).
> A similar problem might arise the next time somebody
> adds code to dev_ifsioc.
>
> Split up dev_ifsioc into three separate functions for
> SIOCSHWTSTAMP, SIOC*IFMAP and all other numbers so
> we can get rid of set_fs in all potentially affected
> cases.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
> Resending this one as well, rebased to current net-next.
> Please tell me if you want a backport to 2.6.32 or -stable
> for this one.
>
No.
^ permalink raw reply
* Re: [PATCH v3] net/atm: move all compat_ioctl handling to atm/ioctl.c
From: David Miller @ 2009-11-12 3:56 UTC (permalink / raw)
To: arnd; +Cc: dwmw2, linux-kernel, netdev, eric.dumazet
In-Reply-To: <200911111445.22673.arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 11 Nov 2009 14:45:22 +0100
> We have two implementations of the compat_ioctl handling for ATM, the
> one that we have had for ages in fs/compat_ioctl.c and the one added to
> net/atm/ioctl.c by David Woodhouse. Unfortunately, both versions are
> incomplete, and in practice we use a very confusing combination of the
> two.
>
> For ioctl numbers that have the same identifier on 32 and 64 bit systems,
> we go directly through the compat_ioctl socket operation, for those that
> differ, we do a conversion in fs/compat_ioctl.c.
>
> This patch moves both variants into the vcc_compat_ioctl() function,
> while preserving the current behaviour. It also kills off the COMPATIBLE_IOCTL
> definitions that we never use here.
> Doing it this way is clearly not a good solution, but I hope it is a
> step into the right direction, so that someone is able to clean up this
> mess for real.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] netdev: fold name hash properly (v3)
From: David Miller @ 2009-11-12 3:58 UTC (permalink / raw)
To: eric.dumazet; +Cc: shemminger, netdev
In-Reply-To: <4AF9A4C7.4010703@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 10 Nov 2009 18:37:11 +0100
> Stephen Hemminger a écrit :
>> The full_name_hash function does not produce well distributed values in
>> the lower bits, so most code uses hash_32() to fold it. This is really
>> a bug introduced when name hashing was added, back in 2.5 when I added
>> name hashing.
>>
>> hash_32 is all that is needed since full_name_hash returns unsigned int
>> which is only 32 bits on 64 bit platforms.
>>
>> Also, there is no point in using hash_32 on ifindex, because the is naturally
>> sequential and usually well distributed.
>>
>> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>>
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: DM9000: Wake on LAN support
From: David Miller @ 2009-11-12 3:58 UTC (permalink / raw)
To: ben; +Cc: netdev, linux
In-Reply-To: <20091110172224.215538042@fluff.org.uk>
From: Ben Dooks <ben@simtec.co.uk>
Date: Tue, 10 Nov 2009 17:22:24 +0000
> Add support for Wake on LAN (WOL) reception and waking the device up from
> this signal via the ethtool interface. Currently we are only supporting
> the magic-packet variant of wakeup.
>
> WOL is enabled by specifying a second interrupt resource to the driver
> which indicates where the interrupt for the WOL is being signalled. This
> then enables the necessary ethtool calls to leave the device in a state
> to receive WOL frames when going into suspend.
>
> Signed-off-by: Ben Dooks <ben@simtec.co.uk>
> Signed-off-by: Simtec Linux Team <linux@simtec.co.uk>
Applied to net-next-2.6
^ permalink raw reply
* Re: [PATCH net-next-2.6] igmp: Use next_net_device_rcu()
From: Stephen Hemminger @ 2009-11-12 5:14 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4AFB85A4.1050408@gmail.com>
On Thu, 12 Nov 2009 04:48:52 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> We need to use next_det_device_rcu() in RCU protected section.
>
> We also can avoid in_dev_get()/in_dev_put() overhead (code size mainly)
> in rcu_read_lock() sections.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
--
^ permalink raw reply
* Re: [PATCH 4/5] net/x25: push BKL usage into x25_proto
From: andrew hendry @ 2009-11-12 5:18 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel, David Miller, John Kacur, Thomas Gleixner,
Frederic Weisbecker, Henner Eisen, linux-x25, netdev
In-Reply-To: <1257431850-20874-5-git-send-email-arnd@arndb.de>
For anyone who looking at the next step, be aware there is an existing
bug in this area.
With single threaded x.25 processes and light X.25 traffic over time
'dead sockets' hang around. They sometimes have negative values in the
inode field and all the timers are timed out. When they exist rmmod
x25 will crash the system, even though lsmod shows 0 usage if all x.25
programs are shutdown.
# cat /proc/net/x25/socket
dest_addr src_addr dev lci st vs vr va t t2 t21 t22 t23 Snd-Q
Rcv-Q inode
0505xxxxxxxxx 0505xxxxxxxxx x25tap0 000 0 0 0 0 0 3 200 180
180 0 0 6198164303842135440
0505xxxxxxxxx 0505xxxxxxxxx x25tap0 000 0 0 0 0 0 3 200 180
180 0 0 111073138
They can be reproduced faster with threaded client/server test
programs doing connect/disconnect. No time to look further at the
moment, but I can help test.
On Fri, Nov 6, 2009 at 1:37 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> The x25 driver uses lock_kernel() implicitly through
> its proto_ops wrapper. The makes the usage explicit
> in order to get rid of that wrapper and to better document
> the usage of the BKL.
>
> The next step should be to get rid of the usage of the BKL
> in x25 entirely, which requires understanding what data
> structures need serialized accesses.
>
> Cc: Henner Eisen <eis@baty.hanse.de>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: linux-x25@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> net/x25/af_x25.c | 71 +++++++++++++++++++++++++++++++++++++++++++++--------
> 1 files changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> index 7fa9c7a..a7a4bc2 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -415,6 +415,7 @@ static int x25_setsockopt(struct socket *sock, int level, int optname,
> struct sock *sk = sock->sk;
> int rc = -ENOPROTOOPT;
>
> + lock_kernel();
> if (level != SOL_X25 || optname != X25_QBITINCL)
> goto out;
>
> @@ -429,6 +430,7 @@ static int x25_setsockopt(struct socket *sock, int level, int optname,
> x25_sk(sk)->qbitincl = !!opt;
> rc = 0;
> out:
> + unlock_kernel();
> return rc;
> }
>
> @@ -438,6 +440,7 @@ static int x25_getsockopt(struct socket *sock, int level, int optname,
> struct sock *sk = sock->sk;
> int val, len, rc = -ENOPROTOOPT;
>
> + lock_kernel();
> if (level != SOL_X25 || optname != X25_QBITINCL)
> goto out;
>
> @@ -458,6 +461,7 @@ static int x25_getsockopt(struct socket *sock, int level, int optname,
> val = x25_sk(sk)->qbitincl;
> rc = copy_to_user(optval, &val, len) ? -EFAULT : 0;
> out:
> + unlock_kernel();
> return rc;
> }
>
> @@ -466,12 +470,14 @@ static int x25_listen(struct socket *sock, int backlog)
> struct sock *sk = sock->sk;
> int rc = -EOPNOTSUPP;
>
> + lock_kernel();
> if (sk->sk_state != TCP_LISTEN) {
> memset(&x25_sk(sk)->dest_addr, 0, X25_ADDR_LEN);
> sk->sk_max_ack_backlog = backlog;
> sk->sk_state = TCP_LISTEN;
> rc = 0;
> }
> + unlock_kernel();
>
> return rc;
> }
> @@ -597,6 +603,7 @@ static int x25_release(struct socket *sock)
> struct sock *sk = sock->sk;
> struct x25_sock *x25;
>
> + lock_kernel();
> if (!sk)
> goto out;
>
> @@ -627,6 +634,7 @@ static int x25_release(struct socket *sock)
>
> sock_orphan(sk);
> out:
> + unlock_kernel();
> return 0;
> }
>
> @@ -634,18 +642,23 @@ static int x25_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
> {
> struct sock *sk = sock->sk;
> struct sockaddr_x25 *addr = (struct sockaddr_x25 *)uaddr;
> + int rc = 0;
>
> + lock_kernel();
> if (!sock_flag(sk, SOCK_ZAPPED) ||
> addr_len != sizeof(struct sockaddr_x25) ||
> - addr->sx25_family != AF_X25)
> - return -EINVAL;
> + addr->sx25_family != AF_X25) {
> + rc = -EINVAL;
> + goto out;
> + }
>
> x25_sk(sk)->source_addr = addr->sx25_addr;
> x25_insert_socket(sk);
> sock_reset_flag(sk, SOCK_ZAPPED);
> SOCK_DEBUG(sk, "x25_bind: socket is bound\n");
> -
> - return 0;
> +out:
> + unlock_kernel();
> + return rc;
> }
>
> static int x25_wait_for_connection_establishment(struct sock *sk)
> @@ -686,6 +699,7 @@ static int x25_connect(struct socket *sock, struct sockaddr *uaddr,
> struct x25_route *rt;
> int rc = 0;
>
> + lock_kernel();
> lock_sock(sk);
> if (sk->sk_state == TCP_ESTABLISHED && sock->state == SS_CONNECTING) {
> sock->state = SS_CONNECTED;
> @@ -763,6 +777,7 @@ out_put_route:
> x25_route_put(rt);
> out:
> release_sock(sk);
> + unlock_kernel();
> return rc;
> }
>
> @@ -802,6 +817,7 @@ static int x25_accept(struct socket *sock, struct socket *newsock, int flags)
> struct sk_buff *skb;
> int rc = -EINVAL;
>
> + lock_kernel();
> if (!sk || sk->sk_state != TCP_LISTEN)
> goto out;
>
> @@ -829,6 +845,7 @@ static int x25_accept(struct socket *sock, struct socket *newsock, int flags)
> out2:
> release_sock(sk);
> out:
> + unlock_kernel();
> return rc;
> }
>
> @@ -838,10 +855,14 @@ static int x25_getname(struct socket *sock, struct sockaddr *uaddr,
> struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)uaddr;
> struct sock *sk = sock->sk;
> struct x25_sock *x25 = x25_sk(sk);
> + int rc = 0;
>
> + lock_kernel();
> if (peer) {
> - if (sk->sk_state != TCP_ESTABLISHED)
> - return -ENOTCONN;
> + if (sk->sk_state != TCP_ESTABLISHED) {
> + rc = -ENOTCONN;
> + goto out;
> + }
> sx25->sx25_addr = x25->dest_addr;
> } else
> sx25->sx25_addr = x25->source_addr;
> @@ -849,7 +870,21 @@ static int x25_getname(struct socket *sock, struct sockaddr *uaddr,
> sx25->sx25_family = AF_X25;
> *uaddr_len = sizeof(*sx25);
>
> - return 0;
> +out:
> + unlock_kernel();
> + return rc;
> +}
> +
> +static unsigned int x25_datagram_poll(struct file *file, struct socket *sock,
> + poll_table *wait)
> +{
> + int rc;
> +
> + lock_kernel();
> + rc = datagram_poll(file, sock, wait);
> + unlock_kernel();
> +
> + return rc;
> }
>
> int x25_rx_call_request(struct sk_buff *skb, struct x25_neigh *nb,
> @@ -1002,6 +1037,7 @@ static int x25_sendmsg(struct kiocb *iocb, struct socket *sock,
> size_t size;
> int qbit = 0, rc = -EINVAL;
>
> + lock_kernel();
> if (msg->msg_flags & ~(MSG_DONTWAIT|MSG_OOB|MSG_EOR|MSG_CMSG_COMPAT))
> goto out;
>
> @@ -1166,6 +1202,7 @@ static int x25_sendmsg(struct kiocb *iocb, struct socket *sock,
> release_sock(sk);
> rc = len;
> out:
> + unlock_kernel();
> return rc;
> out_kfree_skb:
> kfree_skb(skb);
> @@ -1186,6 +1223,7 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
> unsigned char *asmptr;
> int rc = -ENOTCONN;
>
> + lock_kernel();
> /*
> * This works for seqpacket too. The receiver has ordered the queue for
> * us! We do one quick check first though
> @@ -1259,6 +1297,7 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
> out_free_dgram:
> skb_free_datagram(sk, skb);
> out:
> + unlock_kernel();
> return rc;
> }
>
> @@ -1270,6 +1309,7 @@ static int x25_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> void __user *argp = (void __user *)arg;
> int rc;
>
> + lock_kernel();
> switch (cmd) {
> case TIOCOUTQ: {
> int amount = sk->sk_sndbuf - sk_wmem_alloc_get(sk);
> @@ -1472,6 +1512,7 @@ static int x25_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> rc = -ENOIOCTLCMD;
> break;
> }
> + unlock_kernel();
>
> return rc;
> }
> @@ -1542,15 +1583,19 @@ static int compat_x25_ioctl(struct socket *sock, unsigned int cmd,
> break;
> case SIOCGSTAMP:
> rc = -EINVAL;
> + lock_kernel();
> if (sk)
> rc = compat_sock_get_timestamp(sk,
> (struct timeval __user*)argp);
> + unlock_kernel();
> break;
> case SIOCGSTAMPNS:
> rc = -EINVAL;
> + lock_kernel();
> if (sk)
> rc = compat_sock_get_timestampns(sk,
> (struct timespec __user*)argp);
> + unlock_kernel();
> break;
> case SIOCGIFADDR:
> case SIOCSIFADDR:
> @@ -1569,16 +1614,22 @@ static int compat_x25_ioctl(struct socket *sock, unsigned int cmd,
> rc = -EPERM;
> if (!capable(CAP_NET_ADMIN))
> break;
> + lock_kernel();
> rc = x25_route_ioctl(cmd, argp);
> + unlock_kernel();
> break;
> case SIOCX25GSUBSCRIP:
> + lock_kernel();
> rc = compat_x25_subscr_ioctl(cmd, argp);
> + unlock_kernel();
> break;
> case SIOCX25SSUBSCRIP:
> rc = -EPERM;
> if (!capable(CAP_NET_ADMIN))
> break;
> + lock_kernel();
> rc = compat_x25_subscr_ioctl(cmd, argp);
> + unlock_kernel();
> break;
> case SIOCX25GFACILITIES:
> case SIOCX25SFACILITIES:
> @@ -1600,7 +1651,7 @@ static int compat_x25_ioctl(struct socket *sock, unsigned int cmd,
> }
> #endif
>
> -static const struct proto_ops SOCKOPS_WRAPPED(x25_proto_ops) = {
> +static const struct proto_ops x25_proto_ops = {
> .family = AF_X25,
> .owner = THIS_MODULE,
> .release = x25_release,
> @@ -1609,7 +1660,7 @@ static const struct proto_ops SOCKOPS_WRAPPED(x25_proto_ops) = {
> .socketpair = sock_no_socketpair,
> .accept = x25_accept,
> .getname = x25_getname,
> - .poll = datagram_poll,
> + .poll = x25_datagram_poll,
> .ioctl = x25_ioctl,
> #ifdef CONFIG_COMPAT
> .compat_ioctl = compat_x25_ioctl,
> @@ -1624,8 +1675,6 @@ static const struct proto_ops SOCKOPS_WRAPPED(x25_proto_ops) = {
> .sendpage = sock_no_sendpage,
> };
>
> -SOCKOPS_WRAP(x25_proto, AF_X25);
> -
> static struct packet_type x25_packet_type __read_mostly = {
> .type = cpu_to_be16(ETH_P_X25),
> .func = x25_lapb_receive_frame,
> --
> 1.6.3.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply
* Re: [Bugme-new] [Bug 14546] New: Off-by-two stack buffer overflow in function rpc_uaddr2sockaddr() of net/sunrpc/addr.c
From: Neil Brown @ 2009-11-12 5:56 UTC (permalink / raw)
To: Chuck Lever
Cc: Fabio Olive Leite, argp-YZAGAMbGdGKGw+nKnLezzg,
bugzilla-daemon-590EEB7GvNiWaY/ihj7yzEB+6BGkLq7r,
bugme-daemon-590EEB7GvNiWaY/ihj7yzEB+6BGkLq7r,
Linux Network Developers, J. Bruce Fields, Trond Myklebust,
Andrew Morton, Linux NFS Mailing list
In-Reply-To: <2031D0B0-56C9-406E-B5CF-EE050E2CB437@oracle.com>
On Wednesday November 11, chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org wrote:
> On 2009-11-11 Fabio Olive Leite wrote:
> > On 2009-11-11 Patroklos Argyroudis wrote:
> >> There is no need to increase the size of the buffer since the new
> >> check (if (uaddr_len > sizeof(buf) - 2)) will terminate the function
> >> in case the valid universal address is RPCBIND_MAXUADDRLEN bytes.
> > On a second note, why is '\n' needed there? You should only need
> > '\0', as a '\n'
>
> > at the end is not required by any of the string functions used to
> > convert the
> > address. I believe you could go with buf[RPCBIND_MAXUADDRLEN+1] for
> > the extra NUL only.
>
> AFAICT, strict_strtoul() requires the '\n\0' termination.
if ((*tail == '\0') ||
((len == (size_t)(tail - cp) + 1) && (*tail == '\n'))) {
*res = val;
return 0;
}
allows, not requires. Though admittedly that code isn't as clear as
one might like:
if (tail[0] == 0 || (tail[0] == '\n' && tail[1] == 0) {
.....
}
NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/4] drivers/net/enic: decrement sizeof size in strncmp
From: Julia Lawall @ 2009-11-12 7:48 UTC (permalink / raw)
To: Scott Feldman, Joe Eykholt, netdev, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku.dk>
As observed by Joe Perches, sizeof of a constant string includes the
trailing 0. If what is wanted is to check the initial characters of
another string, this trailing 0 should not be taken into account. If an
exact match is wanted, strcmp should be used instead.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression foo;
constant char *abc;
@@
strncmp(foo, abc,
- sizeof(abc)
+ sizeof(abc)-1
)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/net/enic/vnic_dev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff -u -p a/drivers/net/enic/vnic_dev.c b/drivers/net/enic/vnic_dev.c
--- a/drivers/net/enic/vnic_dev.c
+++ b/drivers/net/enic/vnic_dev.c
@@ -358,9 +358,9 @@ int vnic_dev_hw_version(struct vnic_dev
if (err)
return err;
- if (strncmp(fw_info->hw_version, "A1", sizeof("A1")) == 0)
+ if (strncmp(fw_info->hw_version, "A1", sizeof("A1") - 1) == 0)
*hw_ver = VNIC_DEV_HW_VER_A1;
- else if (strncmp(fw_info->hw_version, "A2", sizeof("A2")) == 0)
+ else if (strncmp(fw_info->hw_version, "A2", sizeof("A2") - 1) == 0)
*hw_ver = VNIC_DEV_HW_VER_A2;
else
*hw_ver = VNIC_DEV_HW_VER_UNKNOWN;
^ permalink raw reply
* [patch] Fix: 'return -ENOMEM' instead of 'return ENOMEM'
From: Ingo Molnar @ 2009-11-12 8:10 UTC (permalink / raw)
To: Andrew Morton
Cc: Randy Dunlap, Stephen M. Cameron, Mike Christie, David Airlie,
James Bottomley, Jens Axboe, Evgeniy Polyakov, iss_storagedev,
Eric Dumazet, Joel Becker, Jeff Liu, Andy Whitcroft, Dave Airlie,
Hannes Eder, dri-devel, Alexey Dobriyan, Mike Miller, Mark Fasheh,
Karsten Keil, rostedt, Karen Xie, James E.J. Bottomley,
Hannes Reinecke, Andreas Eversberg
In-Reply-To: <20091111134730.a0da9e38.akpm@linux-foundation.org>
* Andrew Morton <akpm@linux-foundation.org> wrote:
> > @@ -3730,7 +3730,7 @@ tracing_stats_read(struct file *filp, char __user *ubuf,
> >
> > s = kmalloc(sizeof(*s), GFP_KERNEL);
> > if (!s)
> > - return ENOMEM;
> > + return -ENOMEM;
> >
> > trace_seq_init(s);
> >
>
> lol, there we go again.
>
> Andy, can we have a checkpatch rule please?
Note, that will upset creative uses of error codes i guess, such as
fs/xfs/.
But yeah, +1 from me too.
Ob'post'mortem - looked for similar patterns in the kernel and there's
quite a few bugs there:
include/net/inet_hashtables.h: return ENOMEM; # bug
drivers/scsi/aic7xxx/aic7xxx_osm.c: return ENOMEM; # works but weird
drivers/scsi/cxgb3i/cxgb3i_offload.c: return ENOMEM; # works but weird
fs/ocfs2/dlm/dlmrecovery.c: return EAGAIN; # bug
drivers/block/cciss_scsi.c: return ENXIO; # works but weird
drivers/gpu/drm/radeon/radeon_irq.c: return EINVAL; # bug
drivers/gpu/drm/radeon/radeon_irq.c: return EINVAL; # bug
drivers/isdn/hardware/mISDN/hfcmulti.c: return EINVAL; # bug
5 out of 8 places look buggy - i.e. more than 60% - a checkpatch warning
would avoid real bugs here. (even ignoring the cleanliness effects of
using proper error propagation)
Cc:-ed affected maintainers. The rightmost column are my observations.
Below is the patch fixing these.
Ingo
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
drivers/gpu/drm/radeon/radeon_irq.c | 4 ++--
drivers/isdn/hardware/mISDN/hfcmulti.c | 2 +-
fs/ocfs2/dlm/dlmrecovery.c | 2 +-
include/net/inet_hashtables.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_irq.c b/drivers/gpu/drm/radeon/radeon_irq.c
index b79ecc4..fbbc0a1 100644
--- a/drivers/gpu/drm/radeon/radeon_irq.c
+++ b/drivers/gpu/drm/radeon/radeon_irq.c
@@ -76,7 +76,7 @@ int radeon_enable_vblank(struct drm_device *dev, int crtc)
default:
DRM_ERROR("tried to enable vblank on non-existent crtc %d\n",
crtc);
- return EINVAL;
+ return -EINVAL;
}
} else {
switch (crtc) {
@@ -89,7 +89,7 @@ int radeon_enable_vblank(struct drm_device *dev, int crtc)
default:
DRM_ERROR("tried to enable vblank on non-existent crtc %d\n",
crtc);
- return EINVAL;
+ return -EINVAL;
}
}
diff --git a/drivers/isdn/hardware/mISDN/hfcmulti.c b/drivers/isdn/hardware/mISDN/hfcmulti.c
index faed794..cfb45c9 100644
--- a/drivers/isdn/hardware/mISDN/hfcmulti.c
+++ b/drivers/isdn/hardware/mISDN/hfcmulti.c
@@ -2846,7 +2846,7 @@ mode_hfcmulti(struct hfc_multi *hc, int ch, int protocol, int slot_tx,
int conf;
if (ch < 0 || ch > 31)
- return EINVAL;
+ return -EINVAL;
oslot_tx = hc->chan[ch].slot_tx;
oslot_rx = hc->chan[ch].slot_rx;
conf = hc->chan[ch].conf;
diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
index d9fa3d2..0a8a6a4 100644
--- a/fs/ocfs2/dlm/dlmrecovery.c
+++ b/fs/ocfs2/dlm/dlmrecovery.c
@@ -2639,7 +2639,7 @@ int dlm_begin_reco_handler(struct o2net_msg *msg, u32 len, void *data,
dlm->name, br->node_idx, br->dead_node,
dlm->reco.dead_node, dlm->reco.new_master);
spin_unlock(&dlm->spinlock);
- return EAGAIN;
+ return -EAGAIN;
}
spin_unlock(&dlm->spinlock);
diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index d522dcf..5e31447 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -193,7 +193,7 @@ static inline int inet_ehash_locks_alloc(struct inet_hashinfo *hashinfo)
hashinfo->ehash_locks = kmalloc(size * sizeof(spinlock_t),
GFP_KERNEL);
if (!hashinfo->ehash_locks)
- return ENOMEM;
+ return -ENOMEM;
for (i = 0; i < size; i++)
spin_lock_init(&hashinfo->ehash_locks[i]);
}
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
--
^ permalink raw reply related
* Re: [patch] Fix: 'return -ENOMEM' instead of 'return ENOMEM'
From: Dave Airlie @ 2009-11-12 8:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: Randy Dunlap, Stephen M. Cameron, Mike Christie, David Airlie,
Jens Axboe, Evgeniy Polyakov, Mike Miller, Eric Dumazet,
Joel Becker, Jeff Liu, Andy Whitcroft, Dave Airlie, Hannes Eder,
dri-devel, Alexey Dobriyan, iss_storagedev, Mark Fasheh,
Keith Packard, rostedt, Karen Xie, James Bottomley,
James E.J. Bottomley, Hannes Reinecke, Andreas Eversberg
In-Reply-To: <20091112081043.GA25345@elte.hu>
On Thu, Nov 12, 2009 at 6:10 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Andrew Morton <akpm@linux-foundation.org> wrote:
>
>> > @@ -3730,7 +3730,7 @@ tracing_stats_read(struct file *filp, char __user *ubuf,
>> >
>> > s = kmalloc(sizeof(*s), GFP_KERNEL);
>> > if (!s)
>> > - return ENOMEM;
>> > + return -ENOMEM;
>> >
>> > trace_seq_init(s);
>> >
>>
>> lol, there we go again.
>>
>> Andy, can we have a checkpatch rule please?
>
> Note, that will upset creative uses of error codes i guess, such as
> fs/xfs/.
>
> But yeah, +1 from me too.
>
> Ob'post'mortem - looked for similar patterns in the kernel and there's
> quite a few bugs there:
>
> include/net/inet_hashtables.h: return ENOMEM; # bug
> drivers/scsi/aic7xxx/aic7xxx_osm.c: return ENOMEM; # works but weird
> drivers/scsi/cxgb3i/cxgb3i_offload.c: return ENOMEM; # works but weird
> fs/ocfs2/dlm/dlmrecovery.c: return EAGAIN; # bug
> drivers/block/cciss_scsi.c: return ENXIO; # works but weird
> drivers/gpu/drm/radeon/radeon_irq.c: return EINVAL; # bug
> drivers/gpu/drm/radeon/radeon_irq.c: return EINVAL; # bug
> drivers/isdn/hardware/mISDN/hfcmulti.c: return EINVAL; # bug
>
> 5 out of 8 places look buggy - i.e. more than 60% - a checkpatch warning
> would avoid real bugs here. (even ignoring the cleanliness effects of
> using proper error propagation)
>
> Cc:-ed affected maintainers. The rightmost column are my observations.
> Below is the patch fixing these.
>
> Ingo
>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
Looks good to me for radeon bits.
Acked-by: Dave Airlie <airlied@redhat.com>
Dave.
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
--
^ permalink raw reply
* [RACE] net: in process_backlog
From: Changli Gao @ 2009-11-12 8:50 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Patrick McHardy, netdev
Dear Stephen:
I don't think this change
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=6e583ce5242f32e925dcb198f7123256d0798370
is correct.
local_irq_enable();
break;
}
-
local_irq_enable();
- dev = skb->dev;
-
on MP system, flush_backlog() will be called here, and after that
skb->dev will be invalid, if we access it, sth. unexpected may
happens.
netif_receive_skb(skb);
-
- dev_put(dev);
} while (++work < quota && jiffies == start_time);
return work;
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Jarek Poplawski @ 2009-11-12 8:52 UTC (permalink / raw)
To: Changli Gao
Cc: Patrick McHardy, David S. Miller, Stephen Hemminger, Eric Dumazet,
Tom Herbert, netdev
In-Reply-To: <412e6f7f0911111912q27f2b0aate56c637349292c3f@mail.gmail.com>
On 12-11-2009 04:12, Changli Gao wrote:
> On Wed, Nov 11, 2009 at 11:59 PM, Patrick McHardy <kaber@trash.net> wrote:
...
>> What protects the device from disappearing here and below during
>> dev_queue_xmit() and netif_rx_ni()?
>
> For dev_queue_xmit(), dev is holded by skb->_dst, so there is no
> problem. But for netif_rx_ni(), I don't know how to prevent the device
> disappearing, and it seems that all the NIC drivers have this problem.
> Maybe there was the assumption about the execution context of
> netif_rx() before. Now softirq can't be executed by softirqd, so the
> packet receiving path maybe interleaved. I don't know how to prevent
> it happening.
Btw, maybe I miss something, but ifb with act_mirred is for many users
on the fast path. I'm not sure you proved enough why moving it to the
process context and additional multiqueue functionality (which seems
"pretty cool", but probably mostly for testing purposes) don't harm
its main use?
Regards,
Jarek P.
^ permalink raw reply
* Re: [patch] Fix: 'return -ENOMEM' instead of 'return ENOMEM'
From: roel kluin @ 2009-11-12 9:31 UTC (permalink / raw)
To: Ingo Molnar
Cc: Randy Dunlap, Stephen M. Cameron, Mike Christie, David Airlie,
Jens Axboe, Evgeniy Polyakov, iss_storagedev, Eric Dumazet,
Joel Becker, Jeff Liu, Andy Whitcroft, Dave Airlie, Hannes Eder,
dri-devel, Alexey Dobriyan, Mike Miller, Mark Fasheh,
Karsten Keil, rostedt, Karen Xie, James Bottomley,
James E.J. Bottomley, Hannes Reinecke, Andreas Eversberg
In-Reply-To: <20091112081043.GA25345@elte.hu>
> * Andrew Morton <akpm@linux-foundation.org> wrote:
>> Andy, can we have a checkpatch rule please?
>
> Note, that will upset creative uses of error codes i guess, such as
> fs/xfs/.
>
> But yeah, +1 from me too.
>
> Ob'post'mortem - looked for similar patterns in the kernel and there's
> quite a few bugs there:
>
> include/net/inet_hashtables.h: return ENOMEM; # bug
> drivers/scsi/aic7xxx/aic7xxx_osm.c: return ENOMEM; # works but weird
> drivers/scsi/cxgb3i/cxgb3i_offload.c: return ENOMEM; # works but weird
> fs/ocfs2/dlm/dlmrecovery.c: return EAGAIN; # bug
> drivers/block/cciss_scsi.c: return ENXIO; # works but weird
> drivers/gpu/drm/radeon/radeon_irq.c: return EINVAL; # bug
> drivers/gpu/drm/radeon/radeon_irq.c: return EINVAL; # bug
> drivers/isdn/hardware/mISDN/hfcmulti.c: return EINVAL; # bug
I noticed some of these as well. I found some more and sent
a few patches last night with a regex like:
git grep -n -E "[^=]=[[:space:]]*E[[:upper:]2]+ *;"
I am not 100% sure this doesn't give many falsies, I am not at home
so cannot test right now. Something like this can show how
common/uncommon these are:
git grep -E "[^=]=[[:space:]]*E[[:upper:]2]+ *;" |
sed -r "s/^([^:]*:).*(return|[^=]=)[[:space:]]*(-?E)[[:upper:]2]+
*;.*$/\1 \3/" |
sort | uniq -c
> 5 out of 8 places look buggy - i.e. more than 60% - a checkpatch warning
> would avoid real bugs here. (even ignoring the cleanliness effects of
> using proper error propagation)
> Ingo
Thanks for picking this up,
Roel
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
--
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-12 9:32 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Patrick McHardy, David S. Miller, Stephen Hemminger, Eric Dumazet,
Tom Herbert, netdev
In-Reply-To: <20091112085250.GA7112@ff.dom.local>
On Thu, Nov 12, 2009 at 4:52 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> On 12-11-2009 04:12, Changli Gao wrote:
>> On Wed, Nov 11, 2009 at 11:59 PM, Patrick McHardy <kaber@trash.net> wrote:
> ...
>>> What protects the device from disappearing here and below during
>>> dev_queue_xmit() and netif_rx_ni()?
>>
>> For dev_queue_xmit(), dev is holded by skb->_dst, so there is no
>> problem.
Oh, I'm wrong, and not all the skbs using dev_queue_xmit() has a valid
dst, such as AF_PACKET. As skb->dev doesn't own dev, so I think it is
AF_PACKETs fault. Too bad!
>> But for netif_rx_ni(), I don't know how to prevent the device
>> disappearing, and it seems that all the NIC drivers have this problem.
>> Maybe there was the assumption about the execution context of
>> netif_rx() before. Now softirq can't be executed by softirqd, so the
>> packet receiving path maybe interleaved. I don't know how to prevent
>> it happening.
>
> Btw, maybe I miss something, but ifb with act_mirred is for many users
> on the fast path. I'm not sure you proved enough why moving it to the
> process context and additional multiqueue functionality (which seems
> "pretty cool", but probably mostly for testing purposes) don't harm
> its main use?
Because process is more flexible than other activities, and you can
specify its priority and bind it to some CPU, and so on. If you have a
SMP system, and a NIC which doesn't support MQ. When you use it as a
firewall, you'll find all the work is nearly on a CPU. As throughput
increase, that CPU will be full and no more throughput can gain,
though the other CPUs are still idle. With IFB-MQ, you can distribute
the packets among all the CPUs, and archive good throughput.
I know you worry about the latency. From my experiments, It just OK.
No much explicit latency is added.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-12 9:44 UTC (permalink / raw)
To: David S. Miller, Stephen Hemminger, Patrick McHardy
Cc: xiaosuo, Eric Dumazet, Tom Herbert, netdev
In-Reply-To: <4AFA8911.7050204@gmail.com>
ifb: add multi-queue support
Add multi-queue support, and one kernel thread is created for per queue.
It can used to emulate multi-queue NIC in software, and distribute work
among CPUs.
gentux linux # modprobe ifb numtxqs=2
gentux linux # ifconfig ifb0 up
gentux linux # pgrep ifb0
18508
18509
gentux linux # taskset -p 1 18508
pid 18508's current affinity mask: 3
pid 18508's new affinity mask: 1
gentux linux # taskset -p 2 18509
pid 18509's current affinity mask: 3
pid 18509's new affinity mask: 2
gentux linux # tc qdisc add dev br0 ingress
gentux linux # tc filter add dev br0 parent ffff: protocol ip basic
action mirred egress redirect dev ifb0
This patch also introduces a ip link option "numtxqs" for specifying the
number of the TX queues. so you can add a new ifb with the command:
ip link add numtxqs 4 type ifb
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
drivers/net/ifb.c | 506 +++++++++++++++++++++++++++++++++++-------------
include/linux/if_link.h | 1
net/core/rtnetlink.c | 3
3 files changed, 375 insertions(+), 135 deletions(-)
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 69c2566..7116953 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -33,161 +33,166 @@
#include <linux/etherdevice.h>
#include <linux/init.h>
#include <linux/moduleparam.h>
+#include <linux/wait.h>
+#include <linux/sched.h>
+#include <linux/kthread.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/if_vlan.h>
+#include <linux/if_pppox.h>
+#include <net/ip.h>
+#include <net/ipv6.h>
#include <net/pkt_sched.h>
#include <net/net_namespace.h>
-#define TX_TIMEOUT (2*HZ)
-
#define TX_Q_LIMIT 32
+
+struct ifb_private_q {
+ struct net_device *dev;
+ struct sk_buff_head rq;
+ wait_queue_head_t wq;
+ struct task_struct *task;
+ unsigned long rx_packets;
+ unsigned long rx_bytes;
+ unsigned long rx_dropped;
+} ____cacheline_aligned_in_smp;
+
struct ifb_private {
- struct tasklet_struct ifb_tasklet;
- int tasklet_pending;
- /* mostly debug stats leave in for now */
- unsigned long st_task_enter; /* tasklet entered */
- unsigned long st_txq_refl_try; /* transmit queue refill attempt */
- unsigned long st_rxq_enter; /* receive queue entered */
- unsigned long st_rx2tx_tran; /* receive to trasmit transfers */
- unsigned long st_rxq_notenter; /*receiveQ not entered, resched */
- unsigned long st_rx_frm_egr; /* received from egress path */
- unsigned long st_rx_frm_ing; /* received from ingress path */
- unsigned long st_rxq_check;
- unsigned long st_rxq_rsch;
- struct sk_buff_head rq;
- struct sk_buff_head tq;
+ struct ifb_private_q *pq;
};
+/* Number of ifb devices to be set up by this module. */
static int numifbs = 2;
+module_param(numifbs, int, 0444);
+MODULE_PARM_DESC(numifbs, "Number of ifb devices");
-static void ri_tasklet(unsigned long dev);
-static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev);
-static int ifb_open(struct net_device *dev);
-static int ifb_close(struct net_device *dev);
+/* Number of TX queues per ifb */
+static unsigned int numtxqs = 1;
+module_param(numtxqs, uint, 0444);
+MODULE_PARM_DESC(numtxqs, "Number of TX queues per ifb");
-static void ri_tasklet(unsigned long dev)
+static int ifb_thread(void *priv)
{
-
- struct net_device *_dev = (struct net_device *)dev;
- struct ifb_private *dp = netdev_priv(_dev);
- struct net_device_stats *stats = &_dev->stats;
- struct netdev_queue *txq;
+ struct ifb_private_q *pq = priv;
+ struct net_device *dev = pq->dev, *_dev;
+ int num = pq - ((struct ifb_private *)netdev_priv(dev))->pq;
+ struct netdev_queue *txq = netdev_get_tx_queue(dev, num);
struct sk_buff *skb;
-
- txq = netdev_get_tx_queue(_dev, 0);
- dp->st_task_enter++;
- if ((skb = skb_peek(&dp->tq)) == NULL) {
- dp->st_txq_refl_try++;
- if (__netif_tx_trylock(txq)) {
- dp->st_rxq_enter++;
- while ((skb = skb_dequeue(&dp->rq)) != NULL) {
- skb_queue_tail(&dp->tq, skb);
- dp->st_rx2tx_tran++;
- }
- __netif_tx_unlock(txq);
- } else {
- /* reschedule */
- dp->st_rxq_notenter++;
- goto resched;
+ DEFINE_WAIT(wait);
+ struct sk_buff_head tq;
+
+ __skb_queue_head_init(&tq);
+ while (1) {
+ /* move skb from rq to tq */
+ while (1) {
+ prepare_to_wait(&pq->wq, &wait, TASK_UNINTERRUPTIBLE);
+ __netif_tx_lock_bh(txq);
+ skb_queue_splice_tail_init(&pq->rq, &tq);
+ if (netif_queue_stopped(dev))
+ netif_wake_queue(dev);
+ __netif_tx_unlock_bh(txq);
+ if (kthread_should_stop() || !skb_queue_empty(&tq))
+ break;
+ schedule();
}
- }
-
- while ((skb = skb_dequeue(&dp->tq)) != NULL) {
- u32 from = G_TC_FROM(skb->tc_verd);
-
- skb->tc_verd = 0;
- skb->tc_verd = SET_TC_NCLS(skb->tc_verd);
- stats->tx_packets++;
- stats->tx_bytes +=skb->len;
-
- rcu_read_lock();
- skb->dev = dev_get_by_index_rcu(&init_net, skb->iif);
- if (!skb->dev) {
- rcu_read_unlock();
- dev_kfree_skb(skb);
- stats->tx_dropped++;
+ finish_wait(&pq->wq, &wait);
+ if (kthread_should_stop()) {
+ __skb_queue_purge(&tq);
break;
}
- rcu_read_unlock();
- skb->iif = _dev->ifindex;
-
- if (from & AT_EGRESS) {
- dp->st_rx_frm_egr++;
- dev_queue_xmit(skb);
- } else if (from & AT_INGRESS) {
- dp->st_rx_frm_ing++;
- skb_pull(skb, skb->dev->hard_header_len);
- netif_rx(skb);
- } else
- BUG();
- }
-
- if (__netif_tx_trylock(txq)) {
- dp->st_rxq_check++;
- if ((skb = skb_peek(&dp->rq)) == NULL) {
- dp->tasklet_pending = 0;
- if (netif_queue_stopped(_dev))
- netif_wake_queue(_dev);
- } else {
- dp->st_rxq_rsch++;
- __netif_tx_unlock(txq);
- goto resched;
+ if (need_resched())
+ schedule();
+
+ /* transfer packets */
+ while ((skb = __skb_dequeue(&tq)) != NULL) {
+ u32 from = G_TC_FROM(skb->tc_verd);
+
+ skb->tc_verd = 0;
+ skb->tc_verd = SET_TC_NCLS(skb->tc_verd);
+ txq->tx_packets++;
+ txq->tx_bytes +=skb->len;
+
+ rcu_read_lock();
+ _dev = dev_get_by_index_rcu(&init_net, skb->iif);
+ if (!_dev) {
+ rcu_read_unlock();
+ dev_kfree_skb(skb);
+ txq->tx_dropped++;
+ continue;
+ }
+ if (from & AT_INGRESS)
+ dev_hold(_dev);
+ rcu_read_unlock();
+ skb->dev = _dev;
+ skb->iif = dev->ifindex;
+
+ if (from & AT_EGRESS) {
+ dev_queue_xmit(skb);
+ } else if (from & AT_INGRESS) {
+ skb_pull(skb, skb->dev->hard_header_len);
+ netif_rx_ni(skb);
+ dev_put(_dev);
+ } else
+ BUG();
}
- __netif_tx_unlock(txq);
- } else {
-resched:
- dp->tasklet_pending = 1;
- tasklet_schedule(&dp->ifb_tasklet);
}
+ return 0;
}
-static const struct net_device_ops ifb_netdev_ops = {
- .ndo_open = ifb_open,
- .ndo_stop = ifb_close,
- .ndo_start_xmit = ifb_xmit,
- .ndo_validate_addr = eth_validate_addr,
-};
-
-static void ifb_setup(struct net_device *dev)
+struct net_device_stats* ifb_get_stats(struct net_device *dev)
{
- /* Initialize the device structure. */
- dev->destructor = free_netdev;
- dev->netdev_ops = &ifb_netdev_ops;
+ struct net_device_stats *stats = &dev->stats;
+ struct ifb_private *dp = netdev_priv(dev);
+ struct ifb_private_q *pq = dp->pq;
+ struct netdev_queue *txq;
+ int i;
+ unsigned long rx_packets = 0, rx_bytes = 0, rx_dropped = 0;
+ unsigned long tx_packets = 0, tx_bytes = 0, tx_dropped = 0;
+
+ for (i = 0; i < dev->real_num_tx_queues; i++) {
+ rx_packets += pq[i].rx_packets;
+ rx_bytes += pq[i].rx_bytes;
+ rx_dropped += pq[i].rx_dropped;
+ txq = netdev_get_tx_queue(dev, i);
+ tx_packets += txq->tx_packets;
+ tx_bytes += txq->tx_bytes;
+ tx_dropped += txq->tx_dropped;
+ }
- /* Fill in device structure with ethernet-generic values. */
- ether_setup(dev);
- dev->tx_queue_len = TX_Q_LIMIT;
+ stats->rx_packets = rx_packets;
+ stats->rx_bytes = rx_bytes;
+ stats->rx_dropped = rx_dropped;
+ stats->tx_packets = tx_packets;
+ stats->tx_bytes = tx_bytes;
+ stats->tx_dropped = tx_dropped;
- dev->flags |= IFF_NOARP;
- dev->flags &= ~IFF_MULTICAST;
- dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
- random_ether_addr(dev->dev_addr);
+ return stats;
}
static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
{
- struct ifb_private *dp = netdev_priv(dev);
- struct net_device_stats *stats = &dev->stats;
u32 from = G_TC_FROM(skb->tc_verd);
+ int num = skb_get_queue_mapping(skb);
+ struct ifb_private *dp = netdev_priv(dev);
+ struct ifb_private_q *pq = dp->pq + num;
+ struct netdev_queue *txq = netdev_get_tx_queue(dev, num);
- stats->rx_packets++;
- stats->rx_bytes+=skb->len;
+ pq->rx_packets++;
+ pq->rx_bytes += skb->len;
if (!(from & (AT_INGRESS|AT_EGRESS)) || !skb->iif) {
dev_kfree_skb(skb);
- stats->rx_dropped++;
+ pq->rx_dropped++;
return NETDEV_TX_OK;
}
- if (skb_queue_len(&dp->rq) >= dev->tx_queue_len) {
+ txq->trans_start = jiffies;
+ __skb_queue_tail(&pq->rq, skb);
+ if (skb_queue_len(&pq->rq) >= dev->tx_queue_len)
netif_stop_queue(dev);
- }
-
- dev->trans_start = jiffies;
- skb_queue_tail(&dp->rq, skb);
- if (!dp->tasklet_pending) {
- dp->tasklet_pending = 1;
- tasklet_schedule(&dp->ifb_tasklet);
- }
+ if (skb_queue_len(&pq->rq) == 1)
+ wake_up(&pq->wq);
return NETDEV_TX_OK;
}
@@ -195,26 +200,241 @@ static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
static int ifb_close(struct net_device *dev)
{
struct ifb_private *dp = netdev_priv(dev);
+ struct ifb_private_q *pq = dp->pq;
+ int i;
- tasklet_kill(&dp->ifb_tasklet);
netif_stop_queue(dev);
- skb_queue_purge(&dp->rq);
- skb_queue_purge(&dp->tq);
+ for (i = 0; i < dev->real_num_tx_queues; i++) {
+ kthread_stop(pq[i].task);
+ __skb_queue_purge(&pq[i].rq);
+ }
+
return 0;
}
static int ifb_open(struct net_device *dev)
{
struct ifb_private *dp = netdev_priv(dev);
-
- tasklet_init(&dp->ifb_tasklet, ri_tasklet, (unsigned long)dev);
- skb_queue_head_init(&dp->rq);
- skb_queue_head_init(&dp->tq);
+ struct ifb_private_q *pq = dp->pq;
+ int i;
+
+ for (i = 0; i < dev->real_num_tx_queues; i++) {
+ pq[i].task = kthread_run(ifb_thread, &pq[i], "%s/%d", dev->name,
+ i);
+ if (IS_ERR(pq[i].task)) {
+ int err = PTR_ERR(pq[i].task);
+ while (--i >= 0)
+ kthread_stop(pq[i].task);
+ return err;
+ }
+ }
netif_start_queue(dev);
return 0;
}
+static u32 simple_tx_hashrnd;
+
+static inline __be16 pppoe_proto(const struct sk_buff *skb)
+{
+ return *((__be16 *)(skb_mac_header(skb) + ETH_HLEN +
+ sizeof(struct pppoe_hdr)));
+}
+
+static u8 ifb_ipv6_ihl(struct sk_buff *skb, u32 *pihl)
+{
+ struct ipv6hdr *hdr = ipv6_hdr(skb);
+ u32 ihl = sizeof(*hdr);
+ u8 nexthdr = hdr->nexthdr;
+
+ while (1) {
+ switch (nexthdr) {
+ case NEXTHDR_HOP:
+ case NEXTHDR_ROUTING:
+ case NEXTHDR_DEST:
+ if (!pskb_may_pull(skb, ihl + 8) ||
+ !pskb_may_pull(skb, ihl +
+ ((skb_network_header(skb)[ihl + 1] + 1) << 3)))
+ return 0;
+ nexthdr = skb_network_header(skb)[ihl];
+ ihl += (skb_network_header(skb)[ihl + 1] + 1) << 3;
+ break;
+ case NEXTHDR_AUTH:
+ case NEXTHDR_ESP:
+ case NEXTHDR_NONE:
+ case NEXTHDR_FRAGMENT:
+ return 0;
+ default:
+ *pihl = ihl;
+ return nexthdr;
+ }
+ }
+}
+
+static u16 ifb_select_queue(struct net_device *dev, struct sk_buff *skb)
+{
+ u32 addr1, addr2;
+ u32 hash, ihl = 0;
+ union {
+ u16 in16[2];
+ u32 in32;
+ } ports;
+ u8 ip_proto;
+ unsigned int pull_len;
+
+ if ((hash = skb_rx_queue_recorded(skb))) {
+ while (hash >= dev->real_num_tx_queues)
+ hash -= dev->real_num_tx_queues;
+ return hash;
+ }
+
+ pull_len = 0;
+ switch (skb->protocol) {
+ case __constant_htons(ETH_P_8021Q):
+ if (unlikely(skb_pull(skb, VLAN_HLEN) == NULL))
+ goto process_other;
+ pull_len = VLAN_HLEN;
+ skb->network_header += pull_len;
+ switch (vlan_eth_hdr(skb)->h_vlan_encapsulated_proto) {
+ case __constant_htons(ETH_P_IP):
+ goto process_ip;
+ case __constant_htons(ETH_P_IPV6):
+ goto process_ipv6;
+ default:
+ goto process_other;
+ }
+ break;
+ case __constant_htons(ETH_P_PPP_SES):
+ if (unlikely(skb_pull(skb, PPPOE_SES_HLEN) == NULL))
+ goto process_other;
+ pull_len = PPPOE_SES_HLEN;
+ skb->network_header += pull_len;
+ switch (pppoe_proto(skb)) {
+ case __constant_htons(ETH_P_IP):
+ goto process_ip;
+ case __constant_htons(ETH_P_IPV6):
+ goto process_ipv6;
+ default:
+ goto process_other;
+ }
+ break;
+ case __constant_htons(ETH_P_IP):
+process_ip:
+ if (unlikely(!pskb_may_pull(skb, sizeof(struct iphdr))))
+ goto process_other;
+ if (!(ip_hdr(skb)->frag_off & htons(IP_MF | IP_OFFSET)))
+ ip_proto = ip_hdr(skb)->protocol;
+ else
+ ip_proto = 0;
+ addr1 = ip_hdr(skb)->saddr;
+ addr2 = ip_hdr(skb)->daddr;
+ ihl = ip_hdrlen(skb);
+ break;
+ case __constant_htons(ETH_P_IPV6):
+process_ipv6:
+ if (unlikely(!pskb_may_pull(skb, sizeof(struct ipv6hdr))))
+ goto process_other;
+ addr1 = ipv6_hdr(skb)->saddr.s6_addr32[3];
+ addr2 = ipv6_hdr(skb)->daddr.s6_addr32[3];
+ ip_proto = ifb_ipv6_ihl(skb, &ihl);
+ break;
+ default:
+process_other:
+ if (pull_len != 0) {
+ skb_push(skb, pull_len);
+ skb->network_header -= pull_len;
+ }
+ return skb->protocol % dev->real_num_tx_queues;
+ }
+ if (addr1 > addr2)
+ swap(addr1, addr2);
+
+ switch (ip_proto) {
+ case IPPROTO_TCP:
+ case IPPROTO_UDP:
+ case IPPROTO_DCCP:
+ case IPPROTO_ESP:
+ case IPPROTO_AH:
+ case IPPROTO_SCTP:
+ case IPPROTO_UDPLITE:
+ if (unlikely(!pskb_may_pull(skb, ihl + 4)))
+ goto process_other_trans;
+ ports.in32 = *((u32 *) (skb_network_header(skb) + ihl));
+ if (ports.in16[0] > ports.in16[1])
+ swap(ports.in16[0], ports.in16[1]);
+ break;
+
+ default:
+process_other_trans:
+ ports.in32 = 0;
+ break;
+ }
+
+ if (pull_len != 0) {
+ skb_push(skb, pull_len);
+ skb->network_header -= pull_len;
+ }
+
+ hash = jhash_3words(addr1, addr2, ports.in32,
+ simple_tx_hashrnd ^ ip_proto);
+
+ return (u16) (((u64) hash * dev->real_num_tx_queues) >> 32);
+}
+
+static int ifb_init(struct net_device *dev)
+{
+ struct ifb_private *dp = netdev_priv(dev);
+ struct ifb_private_q *pq = dp->pq;
+ int i;
+
+ pq = kcalloc(dev->real_num_tx_queues, sizeof(*pq), GFP_KERNEL);
+ if (pq == NULL)
+ return -ENOMEM;
+ dp->pq = pq;
+
+ for (i = 0; i < dev->real_num_tx_queues; i++) {
+ pq[i].dev = dev;
+ __skb_queue_head_init(&pq[i].rq);
+ init_waitqueue_head(&pq[i].wq);
+ }
+
+ return 0;
+}
+
+static void ifb_uninit(struct net_device *dev)
+{
+ struct ifb_private *dp = netdev_priv(dev);
+
+ kfree(dp->pq);
+}
+
+static const struct net_device_ops ifb_netdev_ops = {
+ .ndo_init = ifb_init,
+ .ndo_uninit = ifb_uninit,
+ .ndo_open = ifb_open,
+ .ndo_stop = ifb_close,
+ .ndo_start_xmit = ifb_xmit,
+ .ndo_validate_addr = eth_validate_addr,
+ .ndo_select_queue = ifb_select_queue,
+ .ndo_get_stats = ifb_get_stats,
+};
+
+static void ifb_setup(struct net_device *dev)
+{
+ /* Initialize the device structure. */
+ dev->destructor = free_netdev;
+ dev->netdev_ops = &ifb_netdev_ops;
+
+ /* Fill in device structure with ethernet-generic values. */
+ ether_setup(dev);
+ dev->tx_queue_len = TX_Q_LIMIT;
+
+ dev->flags |= IFF_NOARP;
+ dev->flags &= ~IFF_MULTICAST;
+ dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
+ random_ether_addr(dev->dev_addr);
+}
+
static int ifb_validate(struct nlattr *tb[], struct nlattr *data[])
{
if (tb[IFLA_ADDRESS]) {
@@ -226,25 +446,38 @@ static int ifb_validate(struct nlattr *tb[], struct nlattr *data[])
return 0;
}
+static int ifb_get_tx_queues(struct net *net, struct nlattr *tb[],
+ unsigned int *num_tx_queues,
+ unsigned int *real_num_tx_queues)
+{
+ if (tb[IFLA_NTXQ]) {
+ *num_tx_queues = nla_get_u32(tb[IFLA_NTXQ]);
+ if (*num_tx_queues < 1)
+ return -EINVAL;
+ *real_num_tx_queues = *num_tx_queues;
+ } else {
+ *num_tx_queues = numtxqs;
+ *real_num_tx_queues = numtxqs;
+ }
+
+ return 0;
+}
+
static struct rtnl_link_ops ifb_link_ops __read_mostly = {
.kind = "ifb",
- .priv_size = sizeof(struct ifb_private),
.setup = ifb_setup,
.validate = ifb_validate,
+ .get_tx_queues = ifb_get_tx_queues,
+ .priv_size = sizeof(struct ifb_private),
};
-/* Number of ifb devices to be set up by this module. */
-module_param(numifbs, int, 0);
-MODULE_PARM_DESC(numifbs, "Number of ifb devices");
-
static int __init ifb_init_one(int index)
{
struct net_device *dev_ifb;
int err;
- dev_ifb = alloc_netdev(sizeof(struct ifb_private),
- "ifb%d", ifb_setup);
-
+ dev_ifb = alloc_netdev_mq(sizeof(struct ifb_private), "ifb%d",
+ ifb_setup, numtxqs);
if (!dev_ifb)
return -ENOMEM;
@@ -268,9 +501,12 @@ static int __init ifb_init_module(void)
{
int i, err;
+ if (numtxqs < 1)
+ return -EINVAL;
+
+ get_random_bytes(&simple_tx_hashrnd, 4);
rtnl_lock();
err = __rtnl_link_register(&ifb_link_ops);
-
for (i = 0; i < numifbs && !err; i++)
err = ifb_init_one(i);
if (err)
diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 1d3b242..99da411 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -78,6 +78,7 @@ enum {
#define IFLA_LINKINFO IFLA_LINKINFO
IFLA_NET_NS_PID,
IFLA_IFALIAS,
+ IFLA_NTXQ,
__IFLA_MAX
};
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 33148a5..1cbe555 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -597,6 +597,7 @@ static inline size_t if_nlmsg_size(const struct net_device *dev)
+ nla_total_size(4) /* IFLA_MASTER */
+ nla_total_size(1) /* IFLA_OPERSTATE */
+ nla_total_size(1) /* IFLA_LINKMODE */
+ + nla_total_size(4) /* IFLA_NTXQ */
+ rtnl_link_get_size(dev); /* IFLA_LINKINFO */
}
@@ -627,6 +628,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
netif_running(dev) ? dev->operstate : IF_OPER_DOWN);
NLA_PUT_U8(skb, IFLA_LINKMODE, dev->link_mode);
NLA_PUT_U32(skb, IFLA_MTU, dev->mtu);
+ NLA_PUT_U32(skb, IFLA_NTXQ, dev->real_num_tx_queues);
if (dev->ifindex != dev->iflink)
NLA_PUT_U32(skb, IFLA_LINK, dev->iflink);
@@ -725,6 +727,7 @@ const struct nla_policy ifla_policy[IFLA_MAX+1] = {
[IFLA_LINKINFO] = { .type = NLA_NESTED },
[IFLA_NET_NS_PID] = { .type = NLA_U32 },
[IFLA_IFALIAS] = { .type = NLA_STRING, .len = IFALIASZ-1 },
+ [IFLA_NTXQ] = { .type = NLA_U32 },
};
EXPORT_SYMBOL(ifla_policy);
^ permalink raw reply related
* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-12 9:48 UTC (permalink / raw)
To: David S. Miller, Stephen Hemminger, Patrick McHardy
Cc: xiaosuo, Eric Dumazet, Tom Herbert, netdev
In-Reply-To: <4AFBD911.6000900@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 96 bytes --]
The corresponding iprout2 patch is attached.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
[-- Attachment #2: iproute2-support-numtxqs.diff --]
[-- Type: application/octet-stream, Size: 1214 bytes --]
--- ip/iplink.c.orig 2009-11-11 16:56:41.000000000 +0800
+++ ip/iplink.c 2009-11-12 14:57:25.000000000 +0800
@@ -48,6 +48,7 @@
fprintf(stderr, " [ address LLADDR ]\n");
fprintf(stderr, " [ broadcast LLADDR ]\n");
fprintf(stderr, " [ mtu MTU ]\n");
+ fprintf(stderr, " [ numtxqs NUMTXQS ]\n");
fprintf(stderr, " type TYPE [ ARGS ]\n");
fprintf(stderr, " ip link delete DEV type TYPE [ ARGS ]\n");
fprintf(stderr, "\n");
@@ -180,6 +181,7 @@
int qlen = -1;
int mtu = -1;
int netns = -1;
+ int numtxqs = -1;
ret = argc;
@@ -221,6 +223,13 @@
if (get_integer(&mtu, *argv, 0))
invarg("Invalid \"mtu\" value\n", *argv);
addattr_l(&req->n, sizeof(*req), IFLA_MTU, &mtu, 4);
+ } else if (strcmp(*argv, "numtxqs") == 0) {
+ NEXT_ARG();
+ if (numtxqs != -1)
+ duparg("numtxqs", *argv);
+ if (get_integer(&numtxqs, *argv, 0))
+ invarg("Invalid \"numtxqs\" value\n", *argv);
+ addattr_l(&req->n, sizeof(*req), IFLA_NTXQ, &numtxqs, 4);
} else if (strcmp(*argv, "netns") == 0) {
NEXT_ARG();
if (netns != -1)
^ permalink raw reply
* Re: [PATCH 1/4] drivers/net/enic: decrement sizeof size in strncmp
From: Jörg-Volker Peetz @ 2009-11-12 10:05 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.0911120848100.16627@ask.diku.dk>
Julia Lawall wrote:
<snip>
> strncmp(foo, abc,
> - sizeof(abc)
> + sizeof(abc)-1
> )
<snip>
> - if (strncmp(fw_info->hw_version, "A1", sizeof("A1")) == 0)
> + if (strncmp(fw_info->hw_version, "A1", sizeof("A1") - 1) == 0)
> *hw_ver = VNIC_DEV_HW_VER_A1;
> - else if (strncmp(fw_info->hw_version, "A2", sizeof("A2")) == 0)
> + else if (strncmp(fw_info->hw_version, "A2", sizeof("A2") - 1) == 0)
<snip>
Why not use strlen(abc) instead of sizeof(abc) - 1 ?
--
Regards,
Jörg-Volker.
^ permalink raw reply
* [patch 3/5] [PATCH] qeth: Recognize return codes of ccw_device_set_online
From: frank.blaschka @ 2009-11-12 10:11 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Ursula Braun
In-Reply-To: <20091112101140.882313000@de.ibm.com>
[-- Attachment #1: 608-qeth-set_online-rc.diff --]
[-- Type: text/plain, Size: 6092 bytes --]
From: Ursula Braun <ursula.braun@de.ibm.com>
Setting a qeth device online requires to call function
ccw_device_set_online() for read-, write-, and data-subchannel.
Failures should be detected immediately without an attempt to
invoke follow-on activity qeth_qdio_clear_card().,
In addition, ccw_device_set_online calls are consolidated in
qeth_core_main.c only.
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 26 ++++++++++++++++----------
drivers/s390/net/qeth_l2_main.c | 26 ++++++++------------------
drivers/s390/net/qeth_l3_main.c | 25 +++++++------------------
3 files changed, 31 insertions(+), 46 deletions(-)
Index: git_net-next/drivers/s390/net/qeth_core_main.c
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_core_main.c
+++ git_net-next/drivers/s390/net/qeth_core_main.c
@@ -3883,30 +3883,36 @@ static int qeth_core_driver_group(const
int qeth_core_hardsetup_card(struct qeth_card *card)
{
struct qdio_ssqd_desc *ssqd;
- int retries = 3;
+ int retries = 0;
int mpno = 0;
int rc;
QETH_DBF_TEXT(SETUP, 2, "hrdsetup");
atomic_set(&card->force_alloc_skb, 0);
retry:
- if (retries < 3) {
+ if (retries)
QETH_DBF_MESSAGE(2, "%s Retrying to do IDX activates.\n",
dev_name(&card->gdev->dev));
- ccw_device_set_offline(CARD_DDEV(card));
- ccw_device_set_offline(CARD_WDEV(card));
- ccw_device_set_offline(CARD_RDEV(card));
- ccw_device_set_online(CARD_RDEV(card));
- ccw_device_set_online(CARD_WDEV(card));
- ccw_device_set_online(CARD_DDEV(card));
- }
+ ccw_device_set_offline(CARD_DDEV(card));
+ ccw_device_set_offline(CARD_WDEV(card));
+ ccw_device_set_offline(CARD_RDEV(card));
+ rc = ccw_device_set_online(CARD_RDEV(card));
+ if (rc)
+ goto retriable;
+ rc = ccw_device_set_online(CARD_WDEV(card));
+ if (rc)
+ goto retriable;
+ rc = ccw_device_set_online(CARD_DDEV(card));
+ if (rc)
+ goto retriable;
rc = qeth_qdio_clear_card(card, card->info.type != QETH_CARD_TYPE_IQD);
+retriable:
if (rc == -ERESTARTSYS) {
QETH_DBF_TEXT(SETUP, 2, "break1");
return rc;
} else if (rc) {
QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
- if (--retries < 0)
+ if (++retries > 3)
goto out;
else
goto retry;
Index: git_net-next/drivers/s390/net/qeth_l2_main.c
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_l2_main.c
+++ git_net-next/drivers/s390/net/qeth_l2_main.c
@@ -940,30 +940,17 @@ static int __qeth_l2_set_online(struct c
qeth_set_allowed_threads(card, QETH_RECOVER_THREAD, 1);
recover_flag = card->state;
- rc = ccw_device_set_online(CARD_RDEV(card));
- if (rc) {
- QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
- return -EIO;
- }
- rc = ccw_device_set_online(CARD_WDEV(card));
- if (rc) {
- QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
- return -EIO;
- }
- rc = ccw_device_set_online(CARD_DDEV(card));
- if (rc) {
- QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
- return -EIO;
- }
-
rc = qeth_core_hardsetup_card(card);
if (rc) {
QETH_DBF_TEXT_(SETUP, 2, "2err%d", rc);
+ rc = -ENODEV;
goto out_remove;
}
- if (!card->dev && qeth_l2_setup_netdev(card))
+ if (!card->dev && qeth_l2_setup_netdev(card)) {
+ rc = -ENODEV;
goto out_remove;
+ }
if (card->info.type != QETH_CARD_TYPE_OSN)
qeth_l2_send_setmac(card, &card->dev->dev_addr[0]);
@@ -983,6 +970,7 @@ static int __qeth_l2_set_online(struct c
card->lan_online = 0;
return 0;
}
+ rc = -ENODEV;
goto out_remove;
} else
card->lan_online = 1;
@@ -999,6 +987,7 @@ static int __qeth_l2_set_online(struct c
rc = qeth_init_qdio_queues(card);
if (rc) {
QETH_DBF_TEXT_(SETUP, 2, "6err%d", rc);
+ rc = -ENODEV;
goto out_remove;
}
card->state = CARD_STATE_SOFTSETUP;
@@ -1020,6 +1009,7 @@ static int __qeth_l2_set_online(struct c
/* let user_space know that device is online */
kobject_uevent(&gdev->dev.kobj, KOBJ_CHANGE);
return 0;
+
out_remove:
card->use_hard_stop = 1;
qeth_l2_stop_card(card, 0);
@@ -1030,7 +1020,7 @@ out_remove:
card->state = CARD_STATE_RECOVER;
else
card->state = CARD_STATE_DOWN;
- return -ENODEV;
+ return rc;
}
static int qeth_l2_set_online(struct ccwgroup_device *gdev)
Index: git_net-next/drivers/s390/net/qeth_l3_main.c
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_l3_main.c
+++ git_net-next/drivers/s390/net/qeth_l3_main.c
@@ -3156,32 +3156,19 @@ static int __qeth_l3_set_online(struct c
qeth_set_allowed_threads(card, QETH_RECOVER_THREAD, 1);
recover_flag = card->state;
- rc = ccw_device_set_online(CARD_RDEV(card));
- if (rc) {
- QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
- return -EIO;
- }
- rc = ccw_device_set_online(CARD_WDEV(card));
- if (rc) {
- QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
- return -EIO;
- }
- rc = ccw_device_set_online(CARD_DDEV(card));
- if (rc) {
- QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
- return -EIO;
- }
-
rc = qeth_core_hardsetup_card(card);
if (rc) {
QETH_DBF_TEXT_(SETUP, 2, "2err%d", rc);
+ rc = -ENODEV;
goto out_remove;
}
qeth_l3_query_ipassists(card, QETH_PROT_IPV4);
- if (!card->dev && qeth_l3_setup_netdev(card))
+ if (!card->dev && qeth_l3_setup_netdev(card)) {
+ rc = -ENODEV;
goto out_remove;
+ }
card->state = CARD_STATE_HARDSETUP;
qeth_print_status_message(card);
@@ -3198,6 +3185,7 @@ static int __qeth_l3_set_online(struct c
card->lan_online = 0;
return 0;
}
+ rc = -ENODEV;
goto out_remove;
} else
card->lan_online = 1;
@@ -3220,6 +3208,7 @@ static int __qeth_l3_set_online(struct c
rc = qeth_init_qdio_queues(card);
if (rc) {
QETH_DBF_TEXT_(SETUP, 2, "6err%d", rc);
+ rc = -ENODEV;
goto out_remove;
}
card->state = CARD_STATE_SOFTSETUP;
@@ -3250,7 +3239,7 @@ out_remove:
card->state = CARD_STATE_RECOVER;
else
card->state = CARD_STATE_DOWN;
- return -ENODEV;
+ return rc;
}
static int qeth_l3_set_online(struct ccwgroup_device *gdev)
^ permalink raw reply
* [patch 2/5] [PATCH] qeth: remaining EDDP cleanup
From: frank.blaschka @ 2009-11-12 10:11 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Ursula Braun
In-Reply-To: <20091112101140.882313000@de.ibm.com>
[-- Attachment #1: 607-qeth-eddp-cleanup.diff --]
[-- Type: text/plain, Size: 1154 bytes --]
From: Ursula Braun <ursula.braun@de.ibm.com>
EDDP code has been removed from qeth in 2009. This patch removes two
useless remaining EDDP-references.
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
---
drivers/s390/net/qeth_core.h | 2 --
1 file changed, 2 deletions(-)
Index: git_net-next/drivers/s390/net/qeth_core.h
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_core.h
+++ git_net-next/drivers/s390/net/qeth_core.h
@@ -122,7 +122,6 @@ struct qeth_perf_stats {
__u64 outbound_do_qdio_start_time;
unsigned int outbound_do_qdio_cnt;
unsigned int outbound_do_qdio_time;
- /* eddp data */
unsigned int large_send_bytes;
unsigned int large_send_cnt;
unsigned int sg_skbs_sent;
@@ -777,7 +776,6 @@ static inline void qeth_put_buffer_pool_
list_add_tail(&entry->list, &card->qdio.in_buf_pool.entry_list);
}
-struct qeth_eddp_context;
extern struct ccwgroup_driver qeth_l2_ccwgroup_driver;
extern struct ccwgroup_driver qeth_l3_ccwgroup_driver;
const char *qeth_get_cardname_short(struct qeth_card *);
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox