* Re: [PATCHv9 3/3] vhost_net: a kernel-level virtio server
From: Rusty Russell @ 2009-11-12 2:18 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, virtualization, kvm, linux-kernel, mingo, linux-mm, akpm,
hpa, gregory.haskins, s.hetze, Daniel Walker, Eric Dumazet
In-Reply-To: <20091110113637.GB6989@redhat.com>
On Tue, 10 Nov 2009 10:06:37 pm Michael S. Tsirkin wrote:
> If tun is a module, vhost must be a module, too.
> If tun is built-in or disabled, vhost can be built-in.
I really like the brainbending :) Keeps readers on their toes...
Applied,
Rusty.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 1/2] rps: core implementation
From: David Miller @ 2009-11-12 2:32 UTC (permalink / raw)
To: andi; +Cc: eric.dumazet, therbert, netdev
In-Reply-To: <87d43oka86.fsf@basil.nowhere.org>
From: Andi Kleen <andi@firstfloor.org>
Date: Wed, 11 Nov 2009 22:44:25 +0100
> Eric Dumazet <eric.dumazet@gmail.com> writes:
>>
>> Problem of possible cpus is the number can be very large on some arches,
>> but yet few cpus online....
>
> Actually that should be rare. Especially not very large compared to online.
The ratio can be as high as 256/1 on Niagara boxes.
^ permalink raw reply
* Re: [PATCH] [next-next-2.6] net: configurable device name hash
From: David Miller @ 2009-11-12 2:36 UTC (permalink / raw)
To: opurdila; +Cc: shemminger, netdev
In-Reply-To: <200911112347.41425.opurdila@ixiacom.com>
From: Octavian Purdila <opurdila@ixiacom.com>
Date: Wed, 11 Nov 2009 23:47:41 +0200
> We could use a similar function that will work in the per namespace
> initialization context, but this might upset net namespace folks
> since we will get a large hash for every namespace.
Use kzalloc(), that's sufficient for a 64K or so hash table which is
way more than you ever will need.
Use the GFP_* flags that will silently (ie. without a log message)
fail, and divide by two until you successfully allocate the table if
you're worried about memory fragmentation at allocation time.
This is so straightforward, I can't believe we're talking so much
about how to implement this, it's a 15 minute hack :-)
^ permalink raw reply
* Re: [PATCH 2/2] rps: changes to bnx2x to get device hash
From: David Miller @ 2009-11-12 2:42 UTC (permalink / raw)
To: mcarlson; +Cc: therbert, netdev
In-Reply-To: <20091111232103.GA1217@xw6200.broadcom.net>
From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Wed, 11 Nov 2009 15:21:03 -0800
> Is a random hash key really better than arbitrary static values?
> Setting the hash key to random values means, from chip reset to chip
> reset, you could get different performance results. Unless we
> we understood how the key affects performance, I would think that
> reproducable performance numbers would be more desirable, no?
Security.
If someone knows the key and the hash function, they can purposely
pick address and port combinations that always hash to the same queue,
decresing your horsepower to handle traffic by orders of magnitude.
^ permalink raw reply
* Re: [PATCH net-2.6] Revert "net: Support inclusion of <linux/socket.h> before <sys/socket.h>"
From: David Miller @ 2009-11-12 2:51 UTC (permalink / raw)
To: ben; +Cc: netdev, 538372
In-Reply-To: <1257991205.2237.73.camel@localhost>
From: Ben Hutchings <ben@decadent.org.uk>
Date: Thu, 12 Nov 2009 02:00:05 +0000
> This reverts commit 9c501935a3cdcf6b1d35aaee3aa11c7a7051a305. That
> commit caused <linux/rtnetlink.h> to require that <sys/socket.h> is
> included first, breaking autoconf tests for <linux/rtnetlink.h> and
> presumably some real programs too.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
I'm not reverting this Ben. The dependency those programs and
autoconf tests have is _dubious_ at best.
It means they were depending upon the libc5 checks passing, that's
rediculious and not something we should cater to.
If you really want it to work that linux/rtnetlink.h is included first
before sys/socket.h in userland, fix that specifically instead. But
I believe such a dependency is reasonable and the userspace bits need
fixing.
^ permalink raw reply
* Re: [patch] missing dev_put() on error path in vif_add()
From: Wang Chen @ 2009-11-12 2:50 UTC (permalink / raw)
To: Dan Carpenter; +Cc: netdev, davem
In-Reply-To: <alpine.DEB.2.00.0911111339100.9485@bicker>
Dan Carpenter said the following on 2009-11-11 20:03:
> The other error paths in front of this one have a dev_put() but this one
> got missed.
>
> Found by smatch static checker.
>
> regards,
> dan carpenter
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
> --- orig/net/ipv4/ipmr.c 2009-11-11 13:34:20.000000000 +0200
> +++ devel/net/ipv4/ipmr.c 2009-11-11 13:34:59.000000000 +0200
> @@ -483,8 +483,10 @@
> return -EINVAL;
> }
>
> - if ((in_dev = __in_dev_get_rtnl(dev)) == NULL)
> + if ((in_dev = __in_dev_get_rtnl(dev)) == NULL) {
> + dev_put(dev);
> return -EADDRNOTAVAIL;
> + }
> IPV4_DEVCONF(in_dev->cnf, MC_FORWARDING)++;
> ip_rt_multicast_event(in_dev);
>
It's my fault. Introduced in 7dc00c82cbb0119cf4663f65bbaa2cc55f961db2.
Acked-by: Wang Chen <ellre923@gmail.com>
^ permalink raw reply
* Re: [PATCH 0/6] gianfar: Some fixes
From: David Miller @ 2009-11-12 3:04 UTC (permalink / raw)
To: galak; +Cc: jdl, B05799, netdev, linuxppc-dev, afleming, shemminger, buytenh
In-Reply-To: <BEE5C957-99C1-4399-AB5D-BCF5BC05A73C@kernel.crashing.org>
From: Kumar Gala <galak@kernel.crashing.org>
Date: Wed, 11 Nov 2009 09:16:41 -0600
>
> On Nov 10, 2009, at 6:10 PM, Anton Vorontsov wrote:
>
>> Hi all,
>>
>> Here are some fixes for the gianfar driver, patches on the way.
>>
>> Thanks,
>
> Acked-by: Kumar Gala <galak@kernel.crashing.org>
All applied to net-next-2.6, thanks.
^ permalink raw reply
* Re: [PATCH net-2.6] Revert "net: Support inclusion of <linux/socket.h> before <sys/socket.h>"
From: Ben Hutchings @ 2009-11-12 3:05 UTC (permalink / raw)
To: David Miller; +Cc: netdev, 538372
In-Reply-To: <20091111.185124.22668587.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]
On Wed, 2009-11-11 at 18:51 -0800, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Thu, 12 Nov 2009 02:00:05 +0000
>
> > This reverts commit 9c501935a3cdcf6b1d35aaee3aa11c7a7051a305. That
> > commit caused <linux/rtnetlink.h> to require that <sys/socket.h> is
> > included first, breaking autoconf tests for <linux/rtnetlink.h> and
> > presumably some real programs too.
> >
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
>
> I'm not reverting this Ben. The dependency those programs and
> autoconf tests have is _dubious_ at best.
>
> It means they were depending upon the libc5 checks passing, that's
> rediculious and not something we should cater to.
[...]
Sorry, I'm getting my headers mixed up. The problem is with
<linux/netlink.h> not <linux/rtnetlink.h>.
<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>.
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
* Re: [PATCH net-2.6] Revert "net: Support inclusion of <linux/socket.h> before <sys/socket.h>"
From: David Miller @ 2009-11-12 3:09 UTC (permalink / raw)
To: ben; +Cc: netdev, 538372
In-Reply-To: <1257995115.2237.90.camel@localhost>
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
2) Put a !__KERNEL__ sys/socket.h include in there, but that's
asking for trouble.
Ok?
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-12 3:12 UTC (permalink / raw)
To: Patrick McHardy
Cc: David S. Miller, Stephen Hemminger, Eric Dumazet, Tom Herbert,
netdev
In-Reply-To: <4AFADF64.8070601@trash.net>
On Wed, Nov 11, 2009 at 11:59 PM, Patrick McHardy <kaber@trash.net> wrote:
> Changli Gao wrote:
>> diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
>> index 69c2566..ac04e85 100644
>> --- a/drivers/net/ifb.c
>> +++ b/drivers/net/ifb.c
>> ...
>> +/* 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 int numtxqs = 1;
>> +module_param(numtxqs, int, 0444);
>> +MODULE_PARM_DESC(numtxqs, "Number of TX queues per ifb");
>
> unsigned?
Yea, unsigned is better, and I need to check whether its value is
smaller than 1 somewhere. The same will be done with IFLA_NTXQ.
>
>> + while ((skb = skb_dequeue(&pq->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();
>> + skb->dev = dev_get_by_index_rcu(&init_net, skb->iif);
>> + if (!skb->dev) {
>> + rcu_read_unlock();
>> + dev_kfree_skb(skb);
>> + txq->tx_dropped++;
>> + break;
>> + }
>> + rcu_read_unlock();
>> + skb->iif = dev->ifindex;
>
> 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.
>> +
>> +static u16 ifb_select_queue(struct net_device *dev, struct sk_buff *skb)
>> +{
>> + u32 addr1, addr2;
>> + u32 hash, ihl;
>> + union {
>> + u16 in16[2];
>> + u32 in32;
>> + } ports;
>> + u8 ip_proto;
>> +
>> + if ((hash = skb_rx_queue_recorded(skb))) {
>> + while (hash >= dev->real_num_tx_queues)
>> + hash -= dev->real_num_tx_queues;
>> + return hash;
>> + }
>> +
>> + switch (skb->protocol) {
>> + case __constant_htons(ETH_P_IP):
>> + if (!(ip_hdr(skb)->frag_off & htons(IP_MF | IP_OFFSET)))
>> + ip_proto = ip_hdr(skb)->protocol;
>> + else
>> + ip_proto = 0;
>
> So fragments will get reordered?
Yea. It is OK, as the number of fragments isn't large. If we decide to
avoid this, we have to reasm them as netfilter does. and it isn't
efficiency.
>
>> + addr1 = ip_hdr(skb)->saddr;
>> + addr2 = ip_hdr(skb)->daddr;
>> + ihl = ip_hdr(skb)->ihl << 2;
>
> ip_hdrlen()?
Yea, I'll use ip_hdrlen() instead.
>
>> + break;
>> + case __constant_htons(ETH_P_IPV6):
>> + ip_proto = ipv6_hdr(skb)->nexthdr;
>> + addr1 = ipv6_hdr(skb)->saddr.s6_addr32[3];
>> + addr2 = ipv6_hdr(skb)->daddr.s6_addr32[3];
>> + ihl = 10;
>
> Where does 10 come from?
It should be 40, after reviewing IPV6, I found that I need to loop
until finding the right protocol value.
>
>> + break;
>> + default:
>> + return 0;
>
> Perhaps hash on skb->protocol here.
use return skb->protocol % dev->real_num_tx_queues; instead.
>
>> + }
>> + 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:
>> + ports.in32 = *((u32 *) (skb_network_header(skb) + ihl));
>> + if (ports.in16[0] > ports.in16[1])
>> + swap(ports.in16[0], ports.in16[1]);
>> + break;
>> +
>> + default:
>> + ports.in32 = 0;
>> + break;
>> + }
>> +
>> + 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 = kmalloc(sizeof(*pq) * dev->real_num_tx_queues, GFP_KERNEL);
>
> kcalloc()?
OK.
>> + 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);
>> + skb_queue_head_init(&pq[i].tq);
>> + init_waitqueue_head(&pq[i].wq);
>> + pq[i].rx_packets = 0;
>> + pq[i].rx_bytes = 0;
>> + pq[i].rx_dropped = 0;
>> + }
>> +
>> + 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_u16(tb[IFLA_NTXQ]);
>
> We currently use unsigned ints for the queue number, so please
> use an u32 for the attribute as well.
>
>> + *real_num_tx_queues = *num_tx_queues;
>> + } else {
>> + *num_tx_queues = numtxqs;
>> + *real_num_tx_queues = numtxqs;
>> + }
>> +
>> + return 0;
>> +}
>> +
>
u16 (*ndo_select_queue)(struct net_device *dev,
struct sk_buff *skb);
use u16 as the return value so ..., and I think u16 is big enough. If
you insist on this, I'll use u32 instead.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* [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
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