* Re: [PATCH V6 3/3] tuntap: allow polling/writing/reading when detached
From: Jason Wang @ 2013-01-24 10:12 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: davem, netdev, linux-kernel
In-Reply-To: <50F750E4.4040002@redhat.com>
On 01/17/2013 09:16 AM, Jason Wang wrote:
> On 01/17/2013 01:03 AM, Michael S. Tsirkin wrote:
>> On Wed, Jan 16, 2013 at 11:44:38PM +0800, Jason Wang wrote:
>>> We forbid polling, writing and reading when the file were detached, this may
>>> complex the user in several cases:
>>>
>>> - when guest pass some buffers to vhost/qemu and then disable some queues,
>>> host/qemu needs to do its own cleanup on those buffers which is complex
>>> sometimes. We can do this simply by allowing a user can still write to an
>>> disabled queue. Write to an disabled queue will cause the packet pass to the
>>> kernel and read will get nothing.
>>> - align the polling behavior with macvtap which never fails when the queue is
>>> created. This can simplify the polling errors handling of its user (e.g vhost)
>>>
>>> In order to achieve this, tfile->tun were not assign to NULL when detached. And
>>> tfile->tun were converted to be RCU protected in order to let the data path can
>>> check whether the file is deated in a lockless manner. This will be used to
>>> prevent the flow caches from being updated for a detached queue.
>>>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>> drivers/net/tun.c | 45 ++++++++++++++++++++++++++-------------------
>>> 1 files changed, 26 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>>> index c81680d..ec539a9 100644
>>> --- a/drivers/net/tun.c
>>> +++ b/drivers/net/tun.c
>>> @@ -139,7 +139,7 @@ struct tun_file {
>>> unsigned int flags;
>>> u16 queue_index;
>>> struct list_head next;
>>> - struct tun_struct *detached;
>>> + struct tun_struct __rcu *detached;
>>> };
>>>
>>> struct tun_flow_entry {
>>> @@ -295,11 +295,12 @@ static void tun_flow_cleanup(unsigned long data)
>>> }
>>>
>>> static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
>>> - u16 queue_index)
>>> + struct tun_file *tfile)
>>> {
>>> struct hlist_head *head;
>>> struct tun_flow_entry *e;
>>> unsigned long delay = tun->ageing_time;
>>> + u16 queue_index = tfile->queue_index;
>>>
>>> if (!rxhash)
>>> return;
>>> @@ -308,7 +309,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
>>>
>>> rcu_read_lock();
>>>
>>> - if (tun->numqueues == 1)
>>> + if (tun->numqueues == 1 || rcu_dereference(tfile->detached))
>>> goto unlock;
>>>
>>> e = tun_flow_find(head, rxhash);
>> Sorry, still an issue with this one.
> No problem, thanks for the checking.
>> u16 index = tfile->queue_index;
>> BUG_ON(index >= tun->numqueues);
>> dev = tun->dev;
>>
>> rcu_assign_pointer(tun->tfiles[index],
>> tun->tfiles[tun->numqueues - 1]);
>> rcu_assign_pointer(tfile->tun, NULL);
>> ntfile = rtnl_dereference(tun->tfiles[index]);
>> ntfile->queue_index = index;
>>
>> --tun->numqueues;
>> if (clean)
>> sock_put(&tfile->sk);
>> else
>> tun_disable_queue(tun, tfile);
>>
>> You should first disable queue then synchronize network
>> only then play with tfiles array.
>> As it is you might have removed file from array but
>> did not set detached flag yet, so queue_index
>> above is stable.
> I think the code is ok here. With this patch, before synchronize_net(),
> the only thing we do for the tfile that will be detached is to set the
> tfile->detached (tun_disable_queue), and the queue_index is kept
> unchanged. So if the data path don't see the new value of detached, it
> still can treat the tfile is undetached and do the sending and receiving
> as usual. We only do the cleanup after the synchronization which all
> reader are guaranteed to see the new detached value.
>
> For the tfile that will be moved to the new place, some (should be very
> little) OOO will occur which I think is acceptable and can be optimized
> in the future.
Having a thought about this patch, looks like it's suboptimal since:
- If we can make sure no packets were sent to the disabled queue and
stop the vhost thread during switching (then it can flush). There's no
need for this patch.
- Allowing writing/polling to a detached fd is strange and can hide the
bugs of userspace / guest driver.
So looks like we'd better drop this patch?
>> On enable, clear detached last thing.
>>
>>> @@ -384,16 +385,16 @@ static void tun_set_real_num_queues(struct tun_struct *tun)
>>>
>>> static void tun_disable_queue(struct tun_struct *tun, struct tun_file *tfile)
>>> {
>>> - tfile->detached = tun;
>>> + rcu_assign_pointer(tfile->detached, tun);
>>> list_add_tail(&tfile->next, &tun->disabled);
>>> ++tun->numdisabled;
>>> }
>>>
>>> static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
>>> {
>>> - struct tun_struct *tun = tfile->detached;
>>> + struct tun_struct *tun = rtnl_dereference(tfile->detached);
>>>
>>> - tfile->detached = NULL;
>>> + rcu_assign_pointer(tfile->detached, NULL);
>>> list_del_init(&tfile->next);
>>> --tun->numdisabled;
>>> return tun;
>>> @@ -402,26 +403,27 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
>>> static void __tun_detach(struct tun_file *tfile, bool clean)
>>> {
>>> struct tun_file *ntfile;
>>> - struct tun_struct *tun;
>>> + struct tun_struct *tun, *detached;
>>> struct net_device *dev;
>>>
>>> tun = rtnl_dereference(tfile->tun);
>>> + detached = rtnl_dereference(tfile->detached);
>>>
>>> - if (tun) {
>>> + if (tun && !detached) {
>>> u16 index = tfile->queue_index;
>>> BUG_ON(index >= tun->numqueues);
>>> dev = tun->dev;
>>>
>>> rcu_assign_pointer(tun->tfiles[index],
>>> tun->tfiles[tun->numqueues - 1]);
>>> - rcu_assign_pointer(tfile->tun, NULL);
>>> ntfile = rtnl_dereference(tun->tfiles[index]);
>>> ntfile->queue_index = index;
>>>
>>> --tun->numqueues;
>>> - if (clean)
>>> + if (clean) {
>>> + rcu_assign_pointer(tfile->tun, NULL);
>>> sock_put(&tfile->sk);
>>> - else
>>> + } else
>>> tun_disable_queue(tun, tfile);
>>>
>>> synchronize_net();
>>> @@ -429,7 +431,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
>>> /* Drop read queue */
>>> skb_queue_purge(&tfile->sk.sk_receive_queue);
>>> tun_set_real_num_queues(tun);
>>> - } else if (tfile->detached && clean) {
>>> + } else if (detached && clean) {
>>> tun = tun_enable_queue(tfile);
>>> sock_put(&tfile->sk);
>>> }
>>> @@ -466,6 +468,10 @@ static void tun_detach_all(struct net_device *dev)
>>> rcu_assign_pointer(tfile->tun, NULL);
>>> --tun->numqueues;
>>> }
>>> + list_for_each_entry(tfile, &tun->disabled, next) {
>>> + wake_up_all(&tfile->wq.wait);
>>> + rcu_assign_pointer(tfile->tun, NULL);
>>> + }
>>> BUG_ON(tun->numqueues != 0);
>>>
>>> synchronize_net();
>>> @@ -496,7 +502,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>>> goto out;
>>>
>>> err = -EINVAL;
>>> - if (rtnl_dereference(tfile->tun))
>>> + if (rtnl_dereference(tfile->tun) && !rtnl_dereference(tfile->detached))
>>> goto out;
>>>
>>> err = -EBUSY;
>>> @@ -504,7 +510,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>>> goto out;
>>>
>>> err = -E2BIG;
>>> - if (!tfile->detached &&
>>> + if (!rtnl_dereference(tfile->detached) &&
>>> tun->numqueues + tun->numdisabled == MAX_TAP_QUEUES)
>>> goto out;
>>>
>>> @@ -521,7 +527,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>>> rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
>>> tun->numqueues++;
>>>
>>> - if (tfile->detached)
>>> + if (rtnl_dereference(tfile->detached))
>>> tun_enable_queue(tfile);
>>> else
>>> sock_hold(&tfile->sk);
>>> @@ -1195,7 +1201,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>>> tun->dev->stats.rx_packets++;
>>> tun->dev->stats.rx_bytes += len;
>>>
>>> - tun_flow_update(tun, rxhash, tfile->queue_index);
>>> + tun_flow_update(tun, rxhash, tfile);
>>> return total_len;
>>> }
>>>
>>> @@ -1552,7 +1558,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>>> struct net_device *dev;
>>> int err;
>>>
>>> - if (tfile->detached)
>>> + if (rtnl_dereference(tfile->detached))
>>> return -EINVAL;
>>>
>>> dev = __dev_get_by_name(net, ifr->ifr_name);
>>> @@ -1796,7 +1802,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>>> rtnl_lock();
>>>
>>> if (ifr->ifr_flags & IFF_ATTACH_QUEUE) {
>>> - tun = tfile->detached;
>>> + tun = rtnl_dereference(tfile->detached);
>>> if (!tun) {
>>> ret = -EINVAL;
>>> goto unlock;
>>> @@ -1807,7 +1813,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>>> ret = tun_attach(tun, file);
>>> } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
>>> tun = rtnl_dereference(tfile->tun);
>>> - if (!tun || !(tun->flags & TUN_TAP_MQ))
>>> + if (!tun || !(tun->flags & TUN_TAP_MQ) ||
>>> + rtnl_dereference(tfile->detached))
>>> ret = -EINVAL;
>>> else
>>> __tun_detach(tfile, false);
>>> --
>>> 1.7.1
>> --
>> 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/
> --
> 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: [PATCH net-next] ipv6: fix handling of throw routes
From: Axel Neumann @ 2013-01-24 10:11 UTC (permalink / raw)
To: netdev
In-Reply-To: <504D93A4.4090706@6wind.com>
Hi
Nicolas Dichtel <nicolas.dichtel <at> 6wind.com> writes:
>
> Le 07/09/2012 20:18, David Miller a écrit :
> > From: Nicolas Dichtel <nicolas.dichtel <at> 6wind.com>
> > Date: Thu, 6 Sep 2012 11:53:35 -0400
> >
> >> It's the same problem that previous fix about blackhole and prohibit routes.
> >>
> >> When adding a throw route, it was handled like a classic route.
> >> Moreover, it was only possible to add this kind of routes by specifying
> >> an interface.
> >>
> >> Before the patch:
> >> $ ip route add throw 2001::2/128
> >> RTNETLINK answers: No such device
> >> $ ip route add throw 2001::2/128 dev eth0
> >> $ ip -6 route | grep 2001::2
> >> 2001::2 dev eth0 metric 1024
> >>
> >> After:
> >> $ ip route add throw 2001::2/128
> >> $ ip -6 route | grep 2001::2
> >> throw 2001::2 dev lo metric 1024 error -11
> >>
> >> Reported-by: Markus Stenberg <markus.stenberg <at> iki.fi>
> >> Signed-off-by: Nicolas Dichtel <nicolas.dichtel <at> 6wind.com>
> >
> > Applied, thanks.
Although 'ip -6 route show' now reports a "throw" instead of an "unreachable"
route the behavior of a configured IPv6 "throw" route still seems incorrect and
similar to that of an "unreachable" route!
I've tested with kernel 3.7.4 which includes this patch:
http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=commitdiff;h=ef2c7d7b59708d54213c7556a82d14de9a7e4475
An example scenario using a dedicated routing table (IMHO the main use case for
throw routes) is given below...
greetings
/axel
The following scenario shows an example:
computer 1001 and 1002 connected via ethernet eth2
computer 1002 has 1001:2::2/64 on eth2
Now at computer 1001:
root@mlc1001:~# ping6 1001:2::2 -c 1
connect: Network is unreachable
root@mlc1001:~# ip a add 1001:2::1/64 dev eth2
root@mlc1001:~# ip -6 rule add from all lookup 10 pref 1000
root@mlc1001:~# ip -6 rule
0: from all lookup local
1000: from all lookup 10
32766: from all lookup main
root@mlc1001:~# ip -6 route list table 10
root@mlc1001:~# ping6 1001:2::2 -c 1
PING 1001:2::2(1001:2::2) 56 data bytes
64 bytes from 1001:2::2: icmp_seq=1 ttl=64 time=0.263 ms
--- 1001:2::2 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 0.263/0.263/0.263/0.000 ms
root@mlc1001:~# ip -6 route add throw 1001:2::/64 table 10
root@mlc1001:~# ip -6 route list table 10
throw 1001:2::/64 dev lo metric 1024 error -11
root@mlc1001:~# ping6 1001:2::2 -c 1
connect: Resource temporarily unavailable
# Although the destination lookup should only be thrown for table 10
# and continue on the main table where a valid local route exists
# it fails. For remote throw routes the error says something like:
# From 1001:2::2 icmp_seq=1 Destination unreachable: No route
# Removing the throw route again it works again...
root@mlc1001:~# ip -6 r
1001:2::/64 dev eth2 proto kernel metric 256
fe80::/64 dev eth0 proto kernel metric 256
fe80::/64 dev eth1 proto kernel metric 256
fe80::/64 dev eth2 proto kernel metric 256
root@mlc1001:~# ip -6 route del throw 1001:2::/64 table 10
root@mlc1001:~# ping6 1001:2::2 -c 1
PING 1001:2::2(1001:2::2) 56 data bytes
64 bytes from 1001:2::2: icmp_seq=1 ttl=64 time=0.264 ms
--- 1001:2::2 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 0.264/0.264/0.264/0.000 ms
root@mlc1001:~# ip -6 r
1001:2::2 via 1001:2::2 dev eth2 metric 0
cache
1001:2::/64 dev eth2 proto kernel metric 256
fe80::/64 dev eth0 proto kernel metric 256
fe80::/64 dev eth1 proto kernel metric 256
fe80::/64 dev eth2 proto kernel metric 256
root@mlc1001:~#
^ permalink raw reply
* [RFC] net: usbnet: prevent buggy devices from killing us
From: Bjørn Mork @ 2013-01-24 10:25 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-usb, netdev, Bjørn Mork
A device sending 0 length frames as fast as it can has been
observed killing the host system due to the resulting memory
pressure. We handle the done queue as fast as we can, so
if this queue is filling up then that is an indication that we
are under too heavy pressure. Refusing further allocations
until the done queue is handled prevents the buggy device
from taking the system down.
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
Hello Oliver,
The MBIM firmware for the Sierra Wireless MC7710 is a nice source
of "interesting" device issues. One of the uglier ones is that
it under certain conditions will start flooding us with frames
having length 0 as fast as it can. And that is pretty fast...
My older laptop dies immediately under this. It just cannot keep
up with the infinite allocations usbnet will do when the done
queue first starts growing beyond reason.
I really do not have a clue how to handle this problem, but this
patch seems to do the job for me without affecting normal devices.
The queue limit is just a number which Works For Me, leaving the
system running with the buggy device and not kicking in under
normal load.
What do you think? Is there some other way this should be solved?
Bjørn
drivers/net/usb/usbnet.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index f34b2eb..85c7ffd 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -380,6 +380,14 @@ static int rx_submit (struct usbnet *dev, struct urb *urb, gfp_t flags)
unsigned long lockflags;
size_t size = dev->rx_urb_size;
+ /* Do not let a device flood us to death! */
+ if (dev->done.qlen > 1024) {
+ netif_dbg(dev, rx_err, dev->net, "done queue filling up (%u) - throttling\n", dev->done.qlen);
+ usbnet_defer_kevent (dev, EVENT_RX_MEMORY);
+ usb_free_urb (urb);
+ return -ENOMEM;
+ }
+
skb = __netdev_alloc_skb_ip_align(dev->net, size, flags);
if (!skb) {
netif_dbg(dev, rx_err, dev->net, "no rx skb\n");
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH V6 3/3] tuntap: allow polling/writing/reading when detached
From: Michael S. Tsirkin @ 2013-01-24 10:37 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, netdev, linux-kernel
In-Reply-To: <5101091C.90301@redhat.com>
On Thu, Jan 24, 2013 at 06:12:44PM +0800, Jason Wang wrote:
> On 01/17/2013 09:16 AM, Jason Wang wrote:
> > On 01/17/2013 01:03 AM, Michael S. Tsirkin wrote:
> >> On Wed, Jan 16, 2013 at 11:44:38PM +0800, Jason Wang wrote:
> >>> We forbid polling, writing and reading when the file were detached, this may
> >>> complex the user in several cases:
> >>>
> >>> - when guest pass some buffers to vhost/qemu and then disable some queues,
> >>> host/qemu needs to do its own cleanup on those buffers which is complex
> >>> sometimes. We can do this simply by allowing a user can still write to an
> >>> disabled queue. Write to an disabled queue will cause the packet pass to the
> >>> kernel and read will get nothing.
> >>> - align the polling behavior with macvtap which never fails when the queue is
> >>> created. This can simplify the polling errors handling of its user (e.g vhost)
> >>>
> >>> In order to achieve this, tfile->tun were not assign to NULL when detached. And
> >>> tfile->tun were converted to be RCU protected in order to let the data path can
> >>> check whether the file is deated in a lockless manner. This will be used to
> >>> prevent the flow caches from being updated for a detached queue.
> >>>
> >>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >>> ---
> >>> drivers/net/tun.c | 45 ++++++++++++++++++++++++++-------------------
> >>> 1 files changed, 26 insertions(+), 19 deletions(-)
> >>>
> >>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >>> index c81680d..ec539a9 100644
> >>> --- a/drivers/net/tun.c
> >>> +++ b/drivers/net/tun.c
> >>> @@ -139,7 +139,7 @@ struct tun_file {
> >>> unsigned int flags;
> >>> u16 queue_index;
> >>> struct list_head next;
> >>> - struct tun_struct *detached;
> >>> + struct tun_struct __rcu *detached;
> >>> };
> >>>
> >>> struct tun_flow_entry {
> >>> @@ -295,11 +295,12 @@ static void tun_flow_cleanup(unsigned long data)
> >>> }
> >>>
> >>> static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
> >>> - u16 queue_index)
> >>> + struct tun_file *tfile)
> >>> {
> >>> struct hlist_head *head;
> >>> struct tun_flow_entry *e;
> >>> unsigned long delay = tun->ageing_time;
> >>> + u16 queue_index = tfile->queue_index;
> >>>
> >>> if (!rxhash)
> >>> return;
> >>> @@ -308,7 +309,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
> >>>
> >>> rcu_read_lock();
> >>>
> >>> - if (tun->numqueues == 1)
> >>> + if (tun->numqueues == 1 || rcu_dereference(tfile->detached))
> >>> goto unlock;
> >>>
> >>> e = tun_flow_find(head, rxhash);
> >> Sorry, still an issue with this one.
> > No problem, thanks for the checking.
> >> u16 index = tfile->queue_index;
> >> BUG_ON(index >= tun->numqueues);
> >> dev = tun->dev;
> >>
> >> rcu_assign_pointer(tun->tfiles[index],
> >> tun->tfiles[tun->numqueues - 1]);
> >> rcu_assign_pointer(tfile->tun, NULL);
> >> ntfile = rtnl_dereference(tun->tfiles[index]);
> >> ntfile->queue_index = index;
> >>
> >> --tun->numqueues;
> >> if (clean)
> >> sock_put(&tfile->sk);
> >> else
> >> tun_disable_queue(tun, tfile);
> >>
> >> You should first disable queue then synchronize network
> >> only then play with tfiles array.
> >> As it is you might have removed file from array but
> >> did not set detached flag yet, so queue_index
> >> above is stable.
> > I think the code is ok here. With this patch, before synchronize_net(),
> > the only thing we do for the tfile that will be detached is to set the
> > tfile->detached (tun_disable_queue), and the queue_index is kept
> > unchanged. So if the data path don't see the new value of detached, it
> > still can treat the tfile is undetached and do the sending and receiving
> > as usual. We only do the cleanup after the synchronization which all
> > reader are guaranteed to see the new detached value.
> >
> > For the tfile that will be moved to the new place, some (should be very
> > little) OOO will occur which I think is acceptable and can be optimized
> > in the future.
>
> Having a thought about this patch, looks like it's suboptimal since:
>
> - If we can make sure no packets were sent to the disabled queue and
> stop the vhost thread during switching (then it can flush). There's no
> need for this patch.
This assumes synchronization in userspace/vhost, this will make
datapath slower without real need.
> - Allowing writing/polling to a detached fd
It's not a detached fd - it's attached to tun.
We just disabled receiving packets on it.
> is strange and can hide the
> bugs of userspace / guest driver.
That's a good thing, you don't want a fragile interface.
>
> So looks like we'd better drop this patch?
I actually think it's the right approach.
And since you clarified I think the patch is allright.
> >> On enable, clear detached last thing.
> >>
> >>> @@ -384,16 +385,16 @@ static void tun_set_real_num_queues(struct tun_struct *tun)
> >>>
> >>> static void tun_disable_queue(struct tun_struct *tun, struct tun_file *tfile)
> >>> {
> >>> - tfile->detached = tun;
> >>> + rcu_assign_pointer(tfile->detached, tun);
> >>> list_add_tail(&tfile->next, &tun->disabled);
> >>> ++tun->numdisabled;
> >>> }
> >>>
> >>> static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
> >>> {
> >>> - struct tun_struct *tun = tfile->detached;
> >>> + struct tun_struct *tun = rtnl_dereference(tfile->detached);
> >>>
> >>> - tfile->detached = NULL;
> >>> + rcu_assign_pointer(tfile->detached, NULL);
> >>> list_del_init(&tfile->next);
> >>> --tun->numdisabled;
> >>> return tun;
> >>> @@ -402,26 +403,27 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
> >>> static void __tun_detach(struct tun_file *tfile, bool clean)
> >>> {
> >>> struct tun_file *ntfile;
> >>> - struct tun_struct *tun;
> >>> + struct tun_struct *tun, *detached;
> >>> struct net_device *dev;
> >>>
> >>> tun = rtnl_dereference(tfile->tun);
> >>> + detached = rtnl_dereference(tfile->detached);
> >>>
> >>> - if (tun) {
> >>> + if (tun && !detached) {
> >>> u16 index = tfile->queue_index;
> >>> BUG_ON(index >= tun->numqueues);
> >>> dev = tun->dev;
> >>>
> >>> rcu_assign_pointer(tun->tfiles[index],
> >>> tun->tfiles[tun->numqueues - 1]);
> >>> - rcu_assign_pointer(tfile->tun, NULL);
> >>> ntfile = rtnl_dereference(tun->tfiles[index]);
> >>> ntfile->queue_index = index;
> >>>
> >>> --tun->numqueues;
> >>> - if (clean)
> >>> + if (clean) {
> >>> + rcu_assign_pointer(tfile->tun, NULL);
> >>> sock_put(&tfile->sk);
> >>> - else
> >>> + } else
> >>> tun_disable_queue(tun, tfile);
> >>>
> >>> synchronize_net();
> >>> @@ -429,7 +431,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
> >>> /* Drop read queue */
> >>> skb_queue_purge(&tfile->sk.sk_receive_queue);
> >>> tun_set_real_num_queues(tun);
> >>> - } else if (tfile->detached && clean) {
> >>> + } else if (detached && clean) {
> >>> tun = tun_enable_queue(tfile);
> >>> sock_put(&tfile->sk);
> >>> }
> >>> @@ -466,6 +468,10 @@ static void tun_detach_all(struct net_device *dev)
> >>> rcu_assign_pointer(tfile->tun, NULL);
> >>> --tun->numqueues;
> >>> }
> >>> + list_for_each_entry(tfile, &tun->disabled, next) {
> >>> + wake_up_all(&tfile->wq.wait);
> >>> + rcu_assign_pointer(tfile->tun, NULL);
> >>> + }
> >>> BUG_ON(tun->numqueues != 0);
> >>>
> >>> synchronize_net();
> >>> @@ -496,7 +502,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
> >>> goto out;
> >>>
> >>> err = -EINVAL;
> >>> - if (rtnl_dereference(tfile->tun))
> >>> + if (rtnl_dereference(tfile->tun) && !rtnl_dereference(tfile->detached))
> >>> goto out;
> >>>
> >>> err = -EBUSY;
> >>> @@ -504,7 +510,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
> >>> goto out;
> >>>
> >>> err = -E2BIG;
> >>> - if (!tfile->detached &&
> >>> + if (!rtnl_dereference(tfile->detached) &&
> >>> tun->numqueues + tun->numdisabled == MAX_TAP_QUEUES)
> >>> goto out;
> >>>
> >>> @@ -521,7 +527,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
> >>> rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
> >>> tun->numqueues++;
> >>>
> >>> - if (tfile->detached)
> >>> + if (rtnl_dereference(tfile->detached))
> >>> tun_enable_queue(tfile);
> >>> else
> >>> sock_hold(&tfile->sk);
> >>> @@ -1195,7 +1201,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >>> tun->dev->stats.rx_packets++;
> >>> tun->dev->stats.rx_bytes += len;
> >>>
> >>> - tun_flow_update(tun, rxhash, tfile->queue_index);
> >>> + tun_flow_update(tun, rxhash, tfile);
> >>> return total_len;
> >>> }
> >>>
> >>> @@ -1552,7 +1558,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
> >>> struct net_device *dev;
> >>> int err;
> >>>
> >>> - if (tfile->detached)
> >>> + if (rtnl_dereference(tfile->detached))
> >>> return -EINVAL;
> >>>
> >>> dev = __dev_get_by_name(net, ifr->ifr_name);
> >>> @@ -1796,7 +1802,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
> >>> rtnl_lock();
> >>>
> >>> if (ifr->ifr_flags & IFF_ATTACH_QUEUE) {
> >>> - tun = tfile->detached;
> >>> + tun = rtnl_dereference(tfile->detached);
> >>> if (!tun) {
> >>> ret = -EINVAL;
> >>> goto unlock;
> >>> @@ -1807,7 +1813,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
> >>> ret = tun_attach(tun, file);
> >>> } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
> >>> tun = rtnl_dereference(tfile->tun);
> >>> - if (!tun || !(tun->flags & TUN_TAP_MQ))
> >>> + if (!tun || !(tun->flags & TUN_TAP_MQ) ||
> >>> + rtnl_dereference(tfile->detached))
> >>> ret = -EINVAL;
> >>> else
> >>> __tun_detach(tfile, false);
> >>> --
> >>> 1.7.1
> >> --
> >> 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/
> > --
> > 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: [PATCH V6 0/3] handle polling errors in vhost/vhost_net
From: Michael S. Tsirkin @ 2013-01-24 10:38 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1358351078-58915-1-git-send-email-jasowang@redhat.com>
On Wed, Jan 16, 2013 at 11:44:35PM +0800, Jason Wang wrote:
> This is an update version of last version to fix the handling of polling errors
> in vhost/vhost_net.
>
> Currently, vhost and vhost_net ignore polling errors which can lead kernel
> crashing when it tries to remove itself from waitqueue after the polling
> failure. Fix this by:
>
> - examing the POLLERR when setting backend and report erros to userspace
> - let tun always add to waitqueue in .poll() after the queue is created even if
> it was detached.
>
> Changes from V5:
> - use rcu_dereference() instead of the wrong rtnl_dereference() in data path
> - test with CONFIG_PROVE_RCU
>
> Changes from V4:
> - check the detached state by tfile->detached and protect it by RCU
>
> Changes from V3:
> - make a smaller patch that doesn't touch the whole polling state and only check
> the polliner errors in backend setting.
> - add a patch that allows tuntap to do polling/reading/writing when detached
> which could simplify the work of its user.
>
> Changes from v2:
> - check poll->wqh instead of the wrong assumption about POLLERR and waitqueue
> - drop the whole tx polling state check since it was replaced by the wqh
> checking
> - drop the buggy tuntap patch
>
> Changes from v1:
> - restore the state before the ioctl when vhost_init_used() fails
> - log the error when meet polling errors in the data path
> - don't put into waitqueue when tun_chr_poll() return POLLERR
>
> Jason Wang (3):
> vhost_net: correct error handling in vhost_net_set_backend()
> vhost_net: handle polling errors when setting backend
> tuntap: allow polling/writing/reading when detached
>
> drivers/net/tun.c | 45 ++++++++++++++++++++++++++-------------------
> drivers/vhost/net.c | 41 ++++++++++++++++++++++++++++-------------
> drivers/vhost/vhost.c | 18 +++++++++++++++---
> drivers/vhost/vhost.h | 2 +-
> 4 files changed, 70 insertions(+), 36 deletions(-)
Acked-by: Michael S. Tsirkin <mst@redhat.com>
^ permalink raw reply
* Re: [PATCH V6 1/3] vhost_net: correct error handling in vhost_net_set_backend()
From: Michael S. Tsirkin @ 2013-01-24 10:38 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1358351078-58915-2-git-send-email-jasowang@redhat.com>
On Wed, Jan 16, 2013 at 11:44:36PM +0800, Jason Wang wrote:
> Currently, when vhost_init_used() fails the sock refcnt and ubufs were
> leaked. Correct this by calling vhost_init_used() before assign ubufs and
> restore the oldsock when it fails.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/net.c | 16 +++++++++++-----
> 1 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index ebd08b2..d10ad6f 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -827,15 +827,16 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> r = PTR_ERR(ubufs);
> goto err_ubufs;
> }
> - oldubufs = vq->ubufs;
> - vq->ubufs = ubufs;
> +
> vhost_net_disable_vq(n, vq);
> rcu_assign_pointer(vq->private_data, sock);
> - vhost_net_enable_vq(n, vq);
> -
> r = vhost_init_used(vq);
> if (r)
> - goto err_vq;
> + goto err_used;
> + vhost_net_enable_vq(n, vq);
> +
> + oldubufs = vq->ubufs;
> + vq->ubufs = ubufs;
>
> n->tx_packets = 0;
> n->tx_zcopy_err = 0;
> @@ -859,6 +860,11 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> mutex_unlock(&n->dev.mutex);
> return 0;
>
> +err_used:
> + rcu_assign_pointer(vq->private_data, oldsock);
> + vhost_net_enable_vq(n, vq);
> + if (ubufs)
> + vhost_ubuf_put_and_wait(ubufs);
> err_ubufs:
> fput(sock->file);
> err_vq:
> --
> 1.7.1
^ permalink raw reply
* Re: [PATCH V6 2/3] vhost_net: handle polling errors when setting backend
From: Michael S. Tsirkin @ 2013-01-24 10:38 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1358351078-58915-3-git-send-email-jasowang@redhat.com>
On Wed, Jan 16, 2013 at 11:44:37PM +0800, Jason Wang wrote:
> Currently, the polling errors were ignored, which can lead following issues:
>
> - vhost remove itself unconditionally from waitqueue when stopping the poll,
> this may crash the kernel since the previous attempt of starting may fail to
> add itself to the waitqueue
> - userspace may think the backend were successfully set even when the polling
> failed.
>
> Solve this by:
>
> - check poll->wqh before trying to remove from waitqueue
> - report polling errors in vhost_poll_start(), tx_poll_start(), the return value
> will be checked and returned when userspace want to set the backend
>
> After this fix, there still could be a polling failure after backend is set, it
> will addressed by the next patch.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/net.c | 27 ++++++++++++++++++---------
> drivers/vhost/vhost.c | 18 +++++++++++++++---
> drivers/vhost/vhost.h | 2 +-
> 3 files changed, 34 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index d10ad6f..959b1cd 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -165,12 +165,16 @@ static void tx_poll_stop(struct vhost_net *net)
> }
>
> /* Caller must have TX VQ lock */
> -static void tx_poll_start(struct vhost_net *net, struct socket *sock)
> +static int tx_poll_start(struct vhost_net *net, struct socket *sock)
> {
> + int ret;
> +
> if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
> - return;
> - vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
> - net->tx_poll_state = VHOST_NET_POLL_STARTED;
> + return 0;
> + ret = vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
> + if (!ret)
> + net->tx_poll_state = VHOST_NET_POLL_STARTED;
> + return ret;
> }
>
> /* In case of DMA done not in order in lower device driver for some reason.
> @@ -642,20 +646,23 @@ static void vhost_net_disable_vq(struct vhost_net *n,
> vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
> }
>
> -static void vhost_net_enable_vq(struct vhost_net *n,
> +static int vhost_net_enable_vq(struct vhost_net *n,
> struct vhost_virtqueue *vq)
> {
> struct socket *sock;
> + int ret;
>
> sock = rcu_dereference_protected(vq->private_data,
> lockdep_is_held(&vq->mutex));
> if (!sock)
> - return;
> + return 0;
> if (vq == n->vqs + VHOST_NET_VQ_TX) {
> n->tx_poll_state = VHOST_NET_POLL_STOPPED;
> - tx_poll_start(n, sock);
> + ret = tx_poll_start(n, sock);
> } else
> - vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
> + ret = vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
> +
> + return ret;
> }
>
> static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> @@ -833,7 +840,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> r = vhost_init_used(vq);
> if (r)
> goto err_used;
> - vhost_net_enable_vq(n, vq);
> + r = vhost_net_enable_vq(n, vq);
> + if (r)
> + goto err_used;
>
> oldubufs = vq->ubufs;
> vq->ubufs = ubufs;
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 34389f7..9759249 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -77,26 +77,38 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> init_poll_funcptr(&poll->table, vhost_poll_func);
> poll->mask = mask;
> poll->dev = dev;
> + poll->wqh = NULL;
>
> vhost_work_init(&poll->work, fn);
> }
>
> /* Start polling a file. We add ourselves to file's wait queue. The caller must
> * keep a reference to a file until after vhost_poll_stop is called. */
> -void vhost_poll_start(struct vhost_poll *poll, struct file *file)
> +int vhost_poll_start(struct vhost_poll *poll, struct file *file)
> {
> unsigned long mask;
> + int ret = 0;
>
> mask = file->f_op->poll(file, &poll->table);
> if (mask)
> vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
> + if (mask & POLLERR) {
> + if (poll->wqh)
> + remove_wait_queue(poll->wqh, &poll->wait);
> + ret = -EINVAL;
> + }
> +
> + return ret;
> }
>
> /* Stop polling a file. After this function returns, it becomes safe to drop the
> * file reference. You must also flush afterwards. */
> void vhost_poll_stop(struct vhost_poll *poll)
> {
> - remove_wait_queue(poll->wqh, &poll->wait);
> + if (poll->wqh) {
> + remove_wait_queue(poll->wqh, &poll->wait);
> + poll->wqh = NULL;
> + }
> }
>
> static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
> @@ -792,7 +804,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> fput(filep);
>
> if (pollstart && vq->handle_kick)
> - vhost_poll_start(&vq->poll, vq->kick);
> + r = vhost_poll_start(&vq->poll, vq->kick);
>
> mutex_unlock(&vq->mutex);
>
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 2639c58..17261e2 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -42,7 +42,7 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
>
> void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> unsigned long mask, struct vhost_dev *dev);
> -void vhost_poll_start(struct vhost_poll *poll, struct file *file);
> +int vhost_poll_start(struct vhost_poll *poll, struct file *file);
> void vhost_poll_stop(struct vhost_poll *poll);
> void vhost_poll_flush(struct vhost_poll *poll);
> void vhost_poll_queue(struct vhost_poll *poll);
> --
> 1.7.1
^ permalink raw reply
* Re: [PATCH V6 3/3] tuntap: allow polling/writing/reading when detached
From: Michael S. Tsirkin @ 2013-01-24 10:38 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1358351078-58915-4-git-send-email-jasowang@redhat.com>
On Wed, Jan 16, 2013 at 11:44:38PM +0800, Jason Wang wrote:
> We forbid polling, writing and reading when the file were detached, this may
> complex the user in several cases:
>
> - when guest pass some buffers to vhost/qemu and then disable some queues,
> host/qemu needs to do its own cleanup on those buffers which is complex
> sometimes. We can do this simply by allowing a user can still write to an
> disabled queue. Write to an disabled queue will cause the packet pass to the
> kernel and read will get nothing.
> - align the polling behavior with macvtap which never fails when the queue is
> created. This can simplify the polling errors handling of its user (e.g vhost)
>
> In order to achieve this, tfile->tun were not assign to NULL when detached. And
> tfile->tun were converted to be RCU protected in order to let the data path can
> check whether the file is deated in a lockless manner. This will be used to
> prevent the flow caches from being updated for a detached queue.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/tun.c | 45 ++++++++++++++++++++++++++-------------------
> 1 files changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index c81680d..ec539a9 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -139,7 +139,7 @@ struct tun_file {
> unsigned int flags;
> u16 queue_index;
> struct list_head next;
> - struct tun_struct *detached;
> + struct tun_struct __rcu *detached;
> };
>
> struct tun_flow_entry {
> @@ -295,11 +295,12 @@ static void tun_flow_cleanup(unsigned long data)
> }
>
> static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
> - u16 queue_index)
> + struct tun_file *tfile)
> {
> struct hlist_head *head;
> struct tun_flow_entry *e;
> unsigned long delay = tun->ageing_time;
> + u16 queue_index = tfile->queue_index;
>
> if (!rxhash)
> return;
> @@ -308,7 +309,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
>
> rcu_read_lock();
>
> - if (tun->numqueues == 1)
> + if (tun->numqueues == 1 || rcu_dereference(tfile->detached))
> goto unlock;
>
> e = tun_flow_find(head, rxhash);
> @@ -384,16 +385,16 @@ static void tun_set_real_num_queues(struct tun_struct *tun)
>
> static void tun_disable_queue(struct tun_struct *tun, struct tun_file *tfile)
> {
> - tfile->detached = tun;
> + rcu_assign_pointer(tfile->detached, tun);
> list_add_tail(&tfile->next, &tun->disabled);
> ++tun->numdisabled;
> }
>
> static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
> {
> - struct tun_struct *tun = tfile->detached;
> + struct tun_struct *tun = rtnl_dereference(tfile->detached);
>
> - tfile->detached = NULL;
> + rcu_assign_pointer(tfile->detached, NULL);
> list_del_init(&tfile->next);
> --tun->numdisabled;
> return tun;
> @@ -402,26 +403,27 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
> static void __tun_detach(struct tun_file *tfile, bool clean)
> {
> struct tun_file *ntfile;
> - struct tun_struct *tun;
> + struct tun_struct *tun, *detached;
> struct net_device *dev;
>
> tun = rtnl_dereference(tfile->tun);
> + detached = rtnl_dereference(tfile->detached);
>
> - if (tun) {
> + if (tun && !detached) {
> u16 index = tfile->queue_index;
> BUG_ON(index >= tun->numqueues);
> dev = tun->dev;
>
> rcu_assign_pointer(tun->tfiles[index],
> tun->tfiles[tun->numqueues - 1]);
> - rcu_assign_pointer(tfile->tun, NULL);
> ntfile = rtnl_dereference(tun->tfiles[index]);
> ntfile->queue_index = index;
>
> --tun->numqueues;
> - if (clean)
> + if (clean) {
> + rcu_assign_pointer(tfile->tun, NULL);
> sock_put(&tfile->sk);
> - else
> + } else
> tun_disable_queue(tun, tfile);
>
> synchronize_net();
> @@ -429,7 +431,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
> /* Drop read queue */
> skb_queue_purge(&tfile->sk.sk_receive_queue);
> tun_set_real_num_queues(tun);
> - } else if (tfile->detached && clean) {
> + } else if (detached && clean) {
> tun = tun_enable_queue(tfile);
> sock_put(&tfile->sk);
> }
> @@ -466,6 +468,10 @@ static void tun_detach_all(struct net_device *dev)
> rcu_assign_pointer(tfile->tun, NULL);
> --tun->numqueues;
> }
> + list_for_each_entry(tfile, &tun->disabled, next) {
> + wake_up_all(&tfile->wq.wait);
> + rcu_assign_pointer(tfile->tun, NULL);
> + }
> BUG_ON(tun->numqueues != 0);
>
> synchronize_net();
> @@ -496,7 +502,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
> goto out;
>
> err = -EINVAL;
> - if (rtnl_dereference(tfile->tun))
> + if (rtnl_dereference(tfile->tun) && !rtnl_dereference(tfile->detached))
> goto out;
>
> err = -EBUSY;
> @@ -504,7 +510,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
> goto out;
>
> err = -E2BIG;
> - if (!tfile->detached &&
> + if (!rtnl_dereference(tfile->detached) &&
> tun->numqueues + tun->numdisabled == MAX_TAP_QUEUES)
> goto out;
>
> @@ -521,7 +527,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
> rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
> tun->numqueues++;
>
> - if (tfile->detached)
> + if (rtnl_dereference(tfile->detached))
> tun_enable_queue(tfile);
> else
> sock_hold(&tfile->sk);
> @@ -1195,7 +1201,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> tun->dev->stats.rx_packets++;
> tun->dev->stats.rx_bytes += len;
>
> - tun_flow_update(tun, rxhash, tfile->queue_index);
> + tun_flow_update(tun, rxhash, tfile);
> return total_len;
> }
>
> @@ -1552,7 +1558,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
> struct net_device *dev;
> int err;
>
> - if (tfile->detached)
> + if (rtnl_dereference(tfile->detached))
> return -EINVAL;
>
> dev = __dev_get_by_name(net, ifr->ifr_name);
> @@ -1796,7 +1802,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
> rtnl_lock();
>
> if (ifr->ifr_flags & IFF_ATTACH_QUEUE) {
> - tun = tfile->detached;
> + tun = rtnl_dereference(tfile->detached);
> if (!tun) {
> ret = -EINVAL;
> goto unlock;
> @@ -1807,7 +1813,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
> ret = tun_attach(tun, file);
> } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
> tun = rtnl_dereference(tfile->tun);
> - if (!tun || !(tun->flags & TUN_TAP_MQ))
> + if (!tun || !(tun->flags & TUN_TAP_MQ) ||
> + rtnl_dereference(tfile->detached))
> ret = -EINVAL;
> else
> __tun_detach(tfile, false);
> --
> 1.7.1
^ permalink raw reply
* RE: [PATCH] vhost-net: fall back to vmalloc if high-order allocation fails
From: David Laight @ 2013-01-24 10:37 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Romain Francoise, kvm, netdev, linux-kernel
In-Reply-To: <20130124100600.GB8710@redhat.com>
> > I think this means that kmalloc() is likely to be faster
> > (if it doesn't have to sleep), but that vmalloc() is
> > allocating from a much larger resource.
> >
> > This make me that that large allocations that are not
> > temporary should probably be allocated with vmalloc().
>
> vmalloc has some issues for example afaik it's not backed by
> a huge page so I think its use puts more stress on the TLB cache.
Thinks further ...
64bit systems are likely to have enough kernel VA to be able
to map all of physical memory into contiguous VA.
I don't know if Linux does that, but I know code to map it was added
to NetBSD amd64 in order to speed up kernel accesses to 'random'
pages - it might have been partially backed out due to bugs!
If physical memory is mapped like that then kmalloc() requests
can any of physical memory and be unlikely to fail - since user
pages can be moved in order to generate contiguous free blocks.
Doesn't help with 32bit systems - they had to stop mapping all
of physical memory into kernel space a long time ago.
David
^ permalink raw reply
* [PATCH net-next] net neigh: Optimize neighbor entry size calculation.
From: YOSHIFUJI Hideaki @ 2013-01-24 10:44 UTC (permalink / raw)
To: davem; +Cc: yoshfuji, netdev
When allocating memory for neighbour cache entry, if
tbl->entry_size is not set, we always calculate
sizeof(struct neighbour) + tbl->key_len, which is common
in the same table.
With this change, set tbl->entry_size during the table
initialization phase, if it was not set, and use it in
neigh_alloc() and neighbour_priv().
This change also allow us to have both of protocol private
data and device priate data at tha same time.
Note that the only user of prototcol private is DECnet
and the only user of device private is ATM CLIP.
Since those are exclusive, we have not been facing issues
here.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
include/net/neighbour.h | 2 +-
net/core/neighbour.c | 16 +++++++---------
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 0dab173..629ee57 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -184,7 +184,7 @@ struct neigh_table {
static inline void *neighbour_priv(const struct neighbour *n)
{
- return (char *)n + ALIGN(sizeof(*n) + n->tbl->key_len, NEIGH_PRIV_ALIGN);
+ return (char *)n + n->tbl->entry_size;
}
/* flags for neigh_update() */
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 7bd0eed..3863b8f 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -290,15 +290,7 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl, struct net_device
goto out_entries;
}
- if (tbl->entry_size)
- n = kzalloc(tbl->entry_size, GFP_ATOMIC);
- else {
- int sz = sizeof(*n) + tbl->key_len;
-
- sz = ALIGN(sz, NEIGH_PRIV_ALIGN);
- sz += dev->neigh_priv_len;
- n = kzalloc(sz, GFP_ATOMIC);
- }
+ n = kzalloc(tbl->entry_size + dev->neigh_priv_len, GFP_ATOMIC);
if (!n)
goto out_entries;
@@ -1546,6 +1538,12 @@ static void neigh_table_init_no_netlink(struct neigh_table *tbl)
if (!tbl->nht || !tbl->phash_buckets)
panic("cannot allocate neighbour cache hashes");
+ if (!tbl->entry_size)
+ tbl->entry_size = ALIGN(offsetof(struct neighbour, primary_key) +
+ tbl->key_len, NEIGH_PRIV_ALIGN);
+ else
+ WARN_ON(tbl->entry_size % NEIGH_PRIV_ALIGN);
+
rwlock_init(&tbl->lock);
INIT_DEFERRABLE_WORK(&tbl->gc_work, neigh_periodic_work);
schedule_delayed_work(&tbl->gc_work, tbl->parms.reachable_time);
--
1.7.9.5
^ permalink raw reply related
* Re: [RFC] net: usbnet: prevent buggy devices from killing us
From: Oliver Neukum @ 2013-01-24 10:46 UTC (permalink / raw)
To: Bjørn Mork
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1359023152-32576-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
On Thursday 24 January 2013 11:25:52 Bjørn Mork wrote:
> The MBIM firmware for the Sierra Wireless MC7710 is a nice source
> of "interesting" device issues. One of the uglier ones is that
> it under certain conditions will start flooding us with frames
> having length 0 as fast as it can. And that is pretty fast...
If you can tell that those frames are bogus, why not count them
as opposed to generic qlen? Say, if you see 20 among the last 30
are of this type, throttle.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* Re: [RFC] net: usbnet: prevent buggy devices from killing us
From: Bjørn Mork @ 2013-01-24 10:47 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-usb, netdev
In-Reply-To: <1359023152-32576-1-git-send-email-bjorn@mork.no>
Bjørn Mork <bjorn@mork.no> writes:
> A device sending 0 length frames as fast as it can has been
> observed killing the host system due to the resulting memory
> pressure. We handle the done queue as fast as we can, so
> if this queue is filling up then that is an indication that we
> are under too heavy pressure. Refusing further allocations
> until the done queue is handled prevents the buggy device
> from taking the system down.
>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
> Hello Oliver,
>
> The MBIM firmware for the Sierra Wireless MC7710 is a nice source
> of "interesting" device issues. One of the uglier ones is that
> it under certain conditions will start flooding us with frames
> having length 0 as fast as it can. And that is pretty fast...
>
> My older laptop dies immediately under this. It just cannot keep
> up with the infinite allocations usbnet will do when the done
> queue first starts growing beyond reason.
>
> I really do not have a clue how to handle this problem, but this
> patch seems to do the job for me without affecting normal devices.
> The queue limit is just a number which Works For Me, leaving the
> system running with the buggy device and not kicking in under
> normal load.
>
> What do you think? Is there some other way this should be solved?
To illustrate the problem, this the start and stop debug output for such
a buggy device session *with* the RFC patch applied:
Jan 24 11:16:23 nemi kernel: [ 3187.624164] qmi_wwan 8-4:1.8 wwan0: open: enable queueing (rx 60, tx 60) mtu 1500 simple framing
Jan 24 11:16:38 nemi kernel: [ 3202.536921] qmi_wwan 8-4:1.8 wwan0: stop stats: rx/tx 1/11, errs 738980/0
I believe the stats tell the full story...
I do not have any logs without the throttling patch, as that takes down
everything on my laptop including the ahci driver and keyboard. Not
even the magic sysrq is working then.
If anyone is interested in the full debug log (211KB compressed) from
the above session, then I've put it on
http://www.mork.no/~bjorn/usbnet-zero-packet-fix.log.gz
It is mostly full of "rx length 0" lines, but with an occasional
sequence of
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: done queue filling up (1025) - throttling
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: rx length 0
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: done queue filling up (1026) - throttling
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: rx length 0
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: done queue filling up (1027) - throttling
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: rx length 0
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: done queue filling up (1028) - throttling
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: rx length 0
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: done queue filling up (1029) - throttling
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: rx length 0
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: done queue filling up (1030) - throttling
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: rx length 0
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: done queue filling up (1031) - throttling
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: rx length 0
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: done queue filling up (1032) - throttling
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: rx length 0
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: done queue filling up (1033) - throttling
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: rx length 0
Jan 24 11:16:23 nemi kernel: [ 3187.682669] qmi_wwan 8-4:1.8 wwan0: done queue filling up (1034) - throttling
Jan 24 11:16:23 nemi kernel: [ 3187.697826] qmi_wwan 8-4:1.8 wwan0: rxqlen 0 --> 10
showing that the throttling is kicking in and doing its job.
Bjørn
^ permalink raw reply
* Re: [RFC] net: usbnet: prevent buggy devices from killing us
From: Bjørn Mork @ 2013-01-24 10:52 UTC (permalink / raw)
To: Oliver Neukum
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <6505263.QGCG9bfoCC-7ztolUikljGernLeA6q8OA@public.gmane.org>
Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> writes:
> On Thursday 24 January 2013 11:25:52 Bjørn Mork wrote:
>> The MBIM firmware for the Sierra Wireless MC7710 is a nice source
>> of "interesting" device issues. One of the uglier ones is that
>> it under certain conditions will start flooding us with frames
>> having length 0 as fast as it can. And that is pretty fast...
>
> If you can tell that those frames are bogus, why not count them
> as opposed to generic qlen? Say, if you see 20 among the last 30
> are of this type, throttle.
Sounds like a good idea, but where do I add/get that statistics?
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* Re: [RFC] net: usbnet: prevent buggy devices from killing us
From: Oliver Neukum @ 2013-01-24 11:03 UTC (permalink / raw)
To: Bjørn Mork
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87bocehmzd.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
On Thursday 24 January 2013 11:52:22 Bjørn Mork wrote:
> Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> writes:
>
> > On Thursday 24 January 2013 11:25:52 Bjørn Mork wrote:
> >> The MBIM firmware for the Sierra Wireless MC7710 is a nice source
> >> of "interesting" device issues. One of the uglier ones is that
> >> it under certain conditions will start flooding us with frames
> >> having length 0 as fast as it can. And that is pretty fast...
> >
> > If you can tell that those frames are bogus, why not count them
> > as opposed to generic qlen? Say, if you see 20 among the last 30
> > are of this type, throttle.
>
> Sounds like a good idea, but where do I add/get that statistics?
rx_complete
It need not be very accurate.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* Re: BUG: unable to handle kernel paging request at 0000000000609920 in networking code on 3.2.23.
From: Rafal Kupka @ 2013-01-24 11:05 UTC (permalink / raw)
To: netdev
In-Reply-To: <A60E26F6-16DC-4A29-BFDE-1B17DC51F64F@telemetry.com>
Rafal Kupka @ Telemetry <rkupka <at> telemetry.com> writes:
Hello,
> After upgrade to 3.2.23 (debian backports 2.6.32-45 package) from 2.6.32 I
experience server crash.
New round of tests on 3.2.35-2~bpo60+1. Still similar crashes.
> Iptables:
>
> Chain INPUT (policy ACCEPT)
> target prot opt source destination
> dumbtcp tcp -- 0.0.0.0/0 91.217.135.0/24
>
> Chain OUTPUT (policy ACCEPT)
> target prot opt source destination
> dumbtcp tcp -- 91.217.135.0/24 0.0.0.0/0
>
> Chain dumbtcp (2 references)
> target prot opt source destination
> TCPOPTSTRIP tcp -- 0.0.0.0/0 0.0.0.0/0 tcpflags:
0x02/0x02 TCPOPTSTRIP options 3,4,5,8,19
> ECN tcp -- 0.0.0.0/0 0.0.0.0/0 ECN TCP remove
This Netfilter rules are causing it. Either ECN or TCPOPTSTRIP module.
3.2.35 calltrace:
[15368.854247] Call Trace:
[15368.856749] <IRQ>
[15368.858898] [<ffffffff812a02a8>] ? skb_release_data+0x6c/0xe4
[15368.864791] [<ffffffff812a08b0>] ? __kfree_skb+0x11/0x73
[15368.870254] [<ffffffff812e5c5f>] ? tcp_rcv_state_process+0x74/0x8d9
[15368.876632] [<ffffffff812ed0b7>] ? tcp_v4_do_rcv+0x388/0x3eb
[15368.882448] [<ffffffff812ee54e>] ? tcp_v4_rcv+0x447/0x6ed
[15368.888007] [<ffffffff812cb746>] ? nf_hook_slow+0x68/0xfd
[15368.893572] [<ffffffff812d197e>] ? T.1004+0x4f/0x4f
[15368.898614] [<ffffffff812d1abb>] ? ip_local_deliver_finish+0x13d/0x1aa
[15368.905301] [<ffffffff812aab66>] ? __netif_receive_skb+0x47d/0x4b0
[15368.911642] [<ffffffff81013a01>] ? read_tsc+0x5/0x16
[15368.916768] [<ffffffff812aadc7>] ? netif_receive_skb+0x67/0x6d
[15368.922757] [<ffffffff812ab335>] ? napi_gro_receive+0x1f/0x2c
[15368.928661] [<ffffffff812aaea1>] ? napi_skb_finish+0x1c/0x31
[15368.934495] [<ffffffffa0049a61>] ? e1000_clean_rx_irq+0x1ea/0x29a [e1000e]
[15368.941533] [<ffffffffa0049fa2>] ? e1000_clean+0x71/0x229 [e1000e]
[15368.947875] [<ffffffff8103b982>] ? __wake_up+0x35/0x46
[15368.953171] [<ffffffff812ab460>] ? net_rx_action+0xa8/0x207
[15368.958908] [<ffffffff81046351>] ? finish_task_switch+0x50/0xc7
[15368.964995] [<ffffffff8104f2ca>] ? __do_softirq+0xc4/0x1a0
[15368.970636] [<ffffffff81097ec6>] ? handle_irq_event_percpu+0x163/0x181
[15368.977324] [<ffffffff8136f8ac>] ? call_softirq+0x1c/0x30
[15368.982884] [<ffffffff8100fa3f>] ? do_softirq+0x3f/0x79
[15368.988266] [<ffffffff8104f09a>] ? irq_exit+0x44/0xb5
[15368.993473] [<ffffffff8100f38a>] ? do_IRQ+0x94/0xaa
[15368.998489] [<ffffffff8136836e>] ? common_interrupt+0x6e/0x6e
[15369.004397] <EOI>
[15369.006537] [<ffffffff81107e38>] ? fput+0x17a/0x1a2
[15369.011576] [<ffffffff81046351>] ? finish_task_switch+0x50/0xc7
[15369.017653] [<ffffffff81366b46>] ? __schedule+0x57a/0x5cd
[15369.023209] [<ffffffff81368416>] ? retint_careful+0x14/0x32
Regards,
Rafal Kupka
^ permalink raw reply
* Re: [RFC] net: usbnet: prevent buggy devices from killing us
From: Bjørn Mork @ 2013-01-24 11:22 UTC (permalink / raw)
To: Oliver Neukum
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2321910.EQLkSoxl50-7ztolUikljGernLeA6q8OA@public.gmane.org>
Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> writes:
> On Thursday 24 January 2013 11:52:22 Bjørn Mork wrote:
>> Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> writes:
>>
>> > On Thursday 24 January 2013 11:25:52 Bjørn Mork wrote:
>> >> The MBIM firmware for the Sierra Wireless MC7710 is a nice source
>> >> of "interesting" device issues. One of the uglier ones is that
>> >> it under certain conditions will start flooding us with frames
>> >> having length 0 as fast as it can. And that is pretty fast...
>> >
>> > If you can tell that those frames are bogus, why not count them
>> > as opposed to generic qlen? Say, if you see 20 among the last 30
>> > are of this type, throttle.
>>
>> Sounds like a good idea, but where do I add/get that statistics?
>
> rx_complete
>
> It need not be very accurate.
Sorry for being daft, but how do I code the "20 among the last 30" part
there?
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* Re: [PATCH] CMAC support for CryptoAPI, fixed patch issues, indent, and testmgr build issues
From: Jussi Kivilinna @ 2013-01-24 11:25 UTC (permalink / raw)
To: Steffen Klassert
Cc: Herbert Xu, netdev, linux-kernel, linux-crypto, Tom St Denis,
David Miller
In-Reply-To: <20130124094337.GJ9147@secunet.com>
Quoting Steffen Klassert <steffen.klassert@secunet.com>:
> On Wed, Jan 23, 2013 at 05:35:10PM +0200, Jussi Kivilinna wrote:
>>
>> Problem seems to be that PFKEYv2 does not quite work with IKEv2, and
>> XFRM API should be used instead. There is new numbers assigned for
>> IKEv2:
>> https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xml#ikev2-parameters-7
>>
>> For new SADB_X_AALG_*, I'd think you should use value from "Reserved
>> for private use" range. Maybe 250?
>
> This would be an option, but we have just a few slots for private
> algorithms.
>
>>
>> But maybe better solution might be to not make AES-CMAC (or other
>> new algorithms) available throught PFKEY API at all, just XFRM?
>>
>
> It is probably the best to make new algorithms unavailable for pfkey
> as long as they have no official ikev1 iana transform identifier.
>
> But how to do that? Perhaps we can assign SADB_X_AALG_NOPFKEY to
> the private value 255 and return -EINVAL if pfkey tries to register
> such an algorithm. The netlink interface does not use these
> identifiers, everything should work as expected. So it should be
> possible to use these algoritms with iproute2 and the most modern
> ike deamons.
Maybe it would be cleaner to not mess with pfkeyv2.h at all, but instead mark algorithms that do not support pfkey with flag. See patch below.
Then I started looking up if sadb_alg_id is being used somewhere outside pfkey. Seems that its value is just being copied around.. but at "http://lxr.linux.no/linux+v3.7/net/xfrm/xfrm_policy.c#L1991" it's used as bit-index. So do larger values than 31 break some stuff? Can multiple algorithms have same sadb_alg_id value? Also in af_key.c, sadb_alg_id being used as bit-index.
-Jussi
---
ONLY COMPILE TESTED!
---
include/net/xfrm.h | 5 +++--
net/key/af_key.c | 39 +++++++++++++++++++++++++++++++--------
net/xfrm/xfrm_algo.c | 12 ++++++------
3 files changed, 40 insertions(+), 16 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 421f764..5d5eec2 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1320,6 +1320,7 @@ struct xfrm_algo_desc {
char *name;
char *compat;
u8 available:1;
+ u8 sadb_disabled:1;
union {
struct xfrm_algo_aead_info aead;
struct xfrm_algo_auth_info auth;
@@ -1561,8 +1562,8 @@ extern void xfrm_input_init(void);
extern int xfrm_parse_spi(struct sk_buff *skb, u8 nexthdr, __be32 *spi, __be32 *seq);
extern void xfrm_probe_algs(void);
-extern int xfrm_count_auth_supported(void);
-extern int xfrm_count_enc_supported(void);
+extern int xfrm_count_sadb_auth_supported(void);
+extern int xfrm_count_sadb_enc_supported(void);
extern struct xfrm_algo_desc *xfrm_aalg_get_byidx(unsigned int idx);
extern struct xfrm_algo_desc *xfrm_ealg_get_byidx(unsigned int idx);
extern struct xfrm_algo_desc *xfrm_aalg_get_byid(int alg_id);
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 5b426a6..307cf1d 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -816,18 +816,21 @@ static struct sk_buff *__pfkey_xfrm_state2msg(const struct xfrm_state *x,
sa->sadb_sa_auth = 0;
if (x->aalg) {
struct xfrm_algo_desc *a = xfrm_aalg_get_byname(x->aalg->alg_name, 0);
- sa->sadb_sa_auth = a ? a->desc.sadb_alg_id : 0;
+ sa->sadb_sa_auth = (a && !a->sadb_disabled) ?
+ a->desc.sadb_alg_id : 0;
}
sa->sadb_sa_encrypt = 0;
BUG_ON(x->ealg && x->calg);
if (x->ealg) {
struct xfrm_algo_desc *a = xfrm_ealg_get_byname(x->ealg->alg_name, 0);
- sa->sadb_sa_encrypt = a ? a->desc.sadb_alg_id : 0;
+ sa->sadb_sa_encrypt = (a && !a->sadb_disabled) ?
+ a->desc.sadb_alg_id : 0;
}
/* KAME compatible: sadb_sa_encrypt is overloaded with calg id */
if (x->calg) {
struct xfrm_algo_desc *a = xfrm_calg_get_byname(x->calg->alg_name, 0);
- sa->sadb_sa_encrypt = a ? a->desc.sadb_alg_id : 0;
+ sa->sadb_sa_encrypt = (a && !a->sadb_disabled) ?
+ a->desc.sadb_alg_id : 0;
}
sa->sadb_sa_flags = 0;
@@ -1138,7 +1141,7 @@ static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net,
if (sa->sadb_sa_auth) {
int keysize = 0;
struct xfrm_algo_desc *a = xfrm_aalg_get_byid(sa->sadb_sa_auth);
- if (!a) {
+ if (!a || a->sadb_disabled) {
err = -ENOSYS;
goto out;
}
@@ -1160,7 +1163,7 @@ static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net,
if (sa->sadb_sa_encrypt) {
if (hdr->sadb_msg_satype == SADB_X_SATYPE_IPCOMP) {
struct xfrm_algo_desc *a = xfrm_calg_get_byid(sa->sadb_sa_encrypt);
- if (!a) {
+ if (!a || a->sadb_disabled) {
err = -ENOSYS;
goto out;
}
@@ -1172,7 +1175,7 @@ static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net,
} else {
int keysize = 0;
struct xfrm_algo_desc *a = xfrm_ealg_get_byid(sa->sadb_sa_encrypt);
- if (!a) {
+ if (!a || a->sadb_disabled) {
err = -ENOSYS;
goto out;
}
@@ -1578,13 +1581,13 @@ static struct sk_buff *compose_sadb_supported(const struct sadb_msg *orig,
struct sadb_msg *hdr;
int len, auth_len, enc_len, i;
- auth_len = xfrm_count_auth_supported();
+ auth_len = xfrm_count_sadb_auth_supported();
if (auth_len) {
auth_len *= sizeof(struct sadb_alg);
auth_len += sizeof(struct sadb_supported);
}
- enc_len = xfrm_count_enc_supported();
+ enc_len = xfrm_count_sadb_enc_supported();
if (enc_len) {
enc_len *= sizeof(struct sadb_alg);
enc_len += sizeof(struct sadb_supported);
@@ -1615,6 +1618,8 @@ static struct sk_buff *compose_sadb_supported(const struct sadb_msg *orig,
struct xfrm_algo_desc *aalg = xfrm_aalg_get_byidx(i);
if (!aalg)
break;
+ if (aalg->sadb_disabled)
+ continue;
if (aalg->available)
*ap++ = aalg->desc;
}
@@ -1634,6 +1639,8 @@ static struct sk_buff *compose_sadb_supported(const struct sadb_msg *orig,
struct xfrm_algo_desc *ealg = xfrm_ealg_get_byidx(i);
if (!ealg)
break;
+ if (ealg->sadb_disabled)
+ continue;
if (ealg->available)
*ap++ = ealg->desc;
}
@@ -2825,6 +2832,8 @@ static int count_ah_combs(const struct xfrm_tmpl *t)
const struct xfrm_algo_desc *aalg = xfrm_aalg_get_byidx(i);
if (!aalg)
break;
+ if (aalg->sadb_disabled)
+ continue;
if (aalg_tmpl_set(t, aalg) && aalg->available)
sz += sizeof(struct sadb_comb);
}
@@ -2840,6 +2849,9 @@ static int count_esp_combs(const struct xfrm_tmpl *t)
if (!ealg)
break;
+ if (ealg->sadb_disabled)
+ continue;
+
if (!(ealg_tmpl_set(t, ealg) && ealg->available))
continue;
@@ -2848,6 +2860,9 @@ static int count_esp_combs(const struct xfrm_tmpl *t)
if (!aalg)
break;
+ if (aalg->sadb_disabled)
+ continue;
+
if (aalg_tmpl_set(t, aalg) && aalg->available)
sz += sizeof(struct sadb_comb);
}
@@ -2871,6 +2886,9 @@ static void dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
if (!aalg)
break;
+ if (aalg->sadb_disabled)
+ continue;
+
if (aalg_tmpl_set(t, aalg) && aalg->available) {
struct sadb_comb *c;
c = (struct sadb_comb*)skb_put(skb, sizeof(struct sadb_comb));
@@ -2903,6 +2921,9 @@ static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
if (!ealg)
break;
+ if (ealg->sadb_disabled)
+ continue;
+
if (!(ealg_tmpl_set(t, ealg) && ealg->available))
continue;
@@ -2911,6 +2932,8 @@ static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
const struct xfrm_algo_desc *aalg = xfrm_aalg_get_byidx(k);
if (!aalg)
break;
+ if (aalg->sadb_disabled)
+ continue;
if (!(aalg_tmpl_set(t, aalg) && aalg->available))
continue;
c = (struct sadb_comb*)skb_put(skb, sizeof(struct sadb_comb));
diff --git a/net/xfrm/xfrm_algo.c b/net/xfrm/xfrm_algo.c
index 4694cca..dab881c 100644
--- a/net/xfrm/xfrm_algo.c
+++ b/net/xfrm/xfrm_algo.c
@@ -713,27 +713,27 @@ void xfrm_probe_algs(void)
}
EXPORT_SYMBOL_GPL(xfrm_probe_algs);
-int xfrm_count_auth_supported(void)
+int xfrm_count_sadb_auth_supported(void)
{
int i, n;
for (i = 0, n = 0; i < aalg_entries(); i++)
- if (aalg_list[i].available)
+ if (aalg_list[i].available && !aalg_list[i].sadb_disabled)
n++;
return n;
}
-EXPORT_SYMBOL_GPL(xfrm_count_auth_supported);
+EXPORT_SYMBOL_GPL(xfrm_count_sadb_auth_supported);
-int xfrm_count_enc_supported(void)
+int xfrm_count_sadb_enc_supported(void)
{
int i, n;
for (i = 0, n = 0; i < ealg_entries(); i++)
- if (ealg_list[i].available)
+ if (ealg_list[i].available && !ealg_list[i].sadb_disabled)
n++;
return n;
}
-EXPORT_SYMBOL_GPL(xfrm_count_enc_supported);
+EXPORT_SYMBOL_GPL(xfrm_count_sadb_enc_supported);
#if defined(CONFIG_INET_ESP) || defined(CONFIG_INET_ESP_MODULE) || defined(CONFIG_INET6_ESP) || defined(CONFIG_INET6_ESP_MODULE)
^ permalink raw reply related
* Re: [RFC] net: usbnet: prevent buggy devices from killing us
From: Oliver Neukum @ 2013-01-24 11:31 UTC (permalink / raw)
To: Bjørn Mork
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <877gn2hlkh.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
On Thursday 24 January 2013 12:22:54 Bjørn Mork wrote:
> Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> writes:
>
> > On Thursday 24 January 2013 11:52:22 Bjørn Mork wrote:
> >> Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> writes:
> >>
> >> > On Thursday 24 January 2013 11:25:52 Bjørn Mork wrote:
> >> >> The MBIM firmware for the Sierra Wireless MC7710 is a nice source
> >> >> of "interesting" device issues. One of the uglier ones is that
> >> >> it under certain conditions will start flooding us with frames
> >> >> having length 0 as fast as it can. And that is pretty fast...
> >> >
> >> > If you can tell that those frames are bogus, why not count them
> >> > as opposed to generic qlen? Say, if you see 20 among the last 30
> >> > are of this type, throttle.
> >>
> >> Sounds like a good idea, but where do I add/get that statistics?
> >
> > rx_complete
> >
> > It need not be very accurate.
>
> Sorry for being daft, but how do I code the "20 among the last 30" part
> there?
Just by agreeing that you can live with false negatives but not false positives
if (++counter > 30) {
counter = bogus = 0;
} else {
if (is_bogus(packet)
bogus++;
if (bogus > counter/2)
throttle();
}
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* Extending struct sk_buff
From: Alex Lochmann @ 2013-01-24 11:13 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 2178 bytes --]
Hi all,
i'm currently writing a patch to annotate a single sendmsg-call with
meta information - like a maximum amount of time this data could be delayed.
Thus i extended the sendmsg systemcall to handle a recently added
MSG_MSGMETA flag, which tells the kernel to copy the struct msgmeta as
well as the struct msghdr from userspace.
This information needs be propagated to the qdisc, so it can delay a
single sk_buff until the maximum delay is reached. Therefore i modified
the tcp subsystem (tcp_sendmsg) to assign the pointer to a kmalloced
kernelspace to a member of struct sk_buff.
Each time i assign this pointer i increment a referencecounter located
in the struct msgmeta. After doing so i'm able to decrement it on each
call to __kfree_skbuff. If the counter reaches zero, i free the
allocated kernelspace.
Sometimes a skbuff gets freeed during the context of a syscall,
everything goes fine.
If the kernel tries to access the allocated kernelspace, after returning
to userspace, the memory area gets corrupted.....
The kernellog says the following:
[ 189.445630] [msgmeta] Assign msgmeta(0xf5d516d0) to skb(0xf3c4a480)
[ 189.445636] [msgmeta] Assigned msgmeta(0xf5d516d0) to skb(0xf3c4a480)
- delay is: 0x133700, refcnt is: 0x1
[ 189.445646] [msgmeta] Assign msgmeta(0xf5d516d0) to skb(0xf3c4a530)
[ 189.445651] [msgmeta] Assigned msgmeta(0xf5d516d0) to skb(0xf3c4a530)
- delay is: 0x133700, refcnt is: 0x2
[ 189.445666] [msgmeta] Assign msgmeta(0xf5d516d0) to skb(0xf4ac4540)
[ 189.445671] [msgmeta] Assigned msgmeta(0xf5d516d0) to skb(0xf4ac4540)
- delay is: 0x133700, refcnt is: 0x3
[ 189.445680] [msgmeta] Not freeing msgmeta(0xf5d516d0), there are
still references to it (0x2). Cloned? 0
[ 189.445719] [msgmeta] return to userspace
[ 189.445974] [msgmeta] Magic value corrupted! skb = 0xf3c4a480, meta =
0xf5d516d0, magicA = 0xf5d51e00, delay = 0x133700, magicB = 0xff350011,
counter = 0x2, phys = 0x35d516d0, function: (null)
[ 189.445983] [msgmeta] Not freeing msgmeta(0xf5d516d0), there are
still references to it (0x1). Cloned? 0
I don't know what's goging wrong. :-(
Can you please help me?
Thanks in advance!
Greetings
Alex
[-- Attachment #2: msgmeta.patch --]
[-- Type: text/x-patch, Size: 12389 bytes --]
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f13b52b..c8b8c29 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -332,6 +332,7 @@ struct sk_buff {
struct sock *sk;
struct net_device *dev;
+ struct msgmeta *meta_info;
/*
* This is the control buffer. It is free to use for every
* layer. Please put your private variables there. If you
@@ -1404,6 +1405,56 @@ static inline int pskb_network_may_pull(struct sk_buff *skb, unsigned int len)
{
return pskb_may_pull(skb, skb_network_offset(skb) + len);
}
+#if MSGMETADEBUG_LVL > 1
+static inline void skb_assert_meta(struct sk_buff *skb,const char *func)
+{
+ if (skb->meta_info) {
+ if (skb->meta_info->magicA != 0xcaffee || skb->meta_info->magicB != ~0xcaffee) {
+ MSGMETADEBUG(1,"Magic value corrupted! skb = 0x%p, meta = 0x%p, magicA = 0x%x, delay = 0x%x, magicB = 0x%x, counter = 0x%d, phys = 0x%x, function: %s\n", skb, skb->meta_info, skb->meta_info->magicA,skb->meta_info->delay_us,skb->meta_info->magicB,skb->meta_info->refcnt.counter,virt_to_phys(skb->meta_info),func);
+ }
+ }
+}
+#else
+#define skb_assert_meta(x,y)
+#endif
+static inline void skb_set_meta_info(struct sk_buff *skb, struct msgmeta *meta)
+{
+ skb_assert_meta(skb,__func__);
+ if (meta) {
+ if (skb->meta_info) {
+ if (skb->meta_info != meta) {
+ MSGMETADEBUG(1,"msgmeta already set, but it is not the same as the supplied one. (set: 0x%p, desired: 0x%p) Resetting it to NULL\n",skb->meta_info,meta);
+ skb->meta_info = NULL;
+ return;
+ } else {
+ MSGMETADEBUG(1,"msgmeta(0x%p) already set on skb(0x%p)\n",skb->meta_info,skb);
+ return;
+ }
+ }
+ MSGMETADEBUG(1,"Assign msgmeta(0x%p) to skb(0x%p)\n",meta,skb);
+ atomic_inc(&meta->refcnt);
+ skb->meta_info = meta;
+ MSGMETADEBUG(1,"Assigned msgmeta(0x%p) to skb(0x%p) - delay is: 0x%x, refcnt is: 0x%x\n",skb->meta_info,skb,skb->meta_info->delay_us,atomic_read(&skb->meta_info->refcnt));
+ }
+}
+
+static inline void skb_free_meta_info(struct sk_buff *skb)
+{
+ skb_assert_meta(skb,__func__);
+ if (skb->meta_info) {
+ if (atomic_read(&skb->meta_info->refcnt) == 0) {
+ MSGMETADEBUG(1,"msgmeta(0x%p) in skb(0x%p) refcnt is already 0\n",skb->meta_info,skb);
+ } else {
+ if (atomic_sub_return(1,&skb->meta_info->refcnt) == 0) {
+ //kfree(skb->meta_info);
+ MSGMETADEBUG(1,"Freed msgmeta(0x%p)\n",skb->meta_info);
+ skb->meta_info = NULL;
+ } else {
+ MSGMETADEBUG(1,"Not freeing msgmeta(0x%p), there are still references to it (0x%x). Cloned? %d\n",skb->meta_info,atomic_read(&skb->meta_info->refcnt),skb->fclone == SKB_FCLONE_CLONE);
+ }
+ }
+ }
+}
/*
* CPUs often take a performance hit when accessing unaligned memory
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 635c213..195f99a 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -67,6 +67,7 @@ struct msghdr {
void * msg_control; /* Per protocol magic (eg BSD file descriptor passing) */
__kernel_size_t msg_controllen; /* Length of cmsg list */
unsigned msg_flags;
+ struct msgmeta *msg_meta; /* Has to be at the end of this struct */
};
/* For recvmmsg/sendmmsg */
@@ -75,6 +76,32 @@ struct mmsghdr {
unsigned msg_len;
};
+#define MSGMETADEBUG_LVL 5
+#ifdef MSGMETADEBUG_LVL
+#if MSGMETADEBUG_LVL > 0
+#define MSGMETADEBUG_TAG "[msgmeta] "
+#define MSGMETADEBUG(prio,args...) do { \
+ if ((prio) <= MSGMETADEBUG_LVL) { \
+ printk(KERN_INFO MSGMETADEBUG_TAG args ); \
+ } \
+} while(0);
+#else
+#define MSGMETADEBUG(prio,...) do {} while(0);
+#endif
+#else
+#define MSGMETADEBUG(prio,...) do {} while(0);
+#endif
+struct msgmeta {
+#if MSGMETADEBUG_LVL > 1
+ __u32 magicA;
+#endif
+ __u32 delay_us;
+#if MSGMETADEBUG_LVL > 1
+ __u32 magicB;
+#endif
+ atomic_t refcnt;
+};
+
/*
* POSIX 1003.1g - ancillary data object information
* Ancillary data consits of a sequence of pairs of
@@ -263,6 +290,7 @@ struct ucred {
#define MSG_WAITFORONE 0x10000 /* recvmmsg(): block until 1+ packets avail */
#define MSG_SENDPAGE_NOTLAST 0x20000 /* sendpage() internal : not the last page */
#define MSG_EOF MSG_FIN
+#define MSG_METAINFO 0x40000
#define MSG_CMSG_CLOEXEC 0x40000000 /* Set close_on_exit for file
descriptor received through
@@ -273,7 +301,6 @@ struct ucred {
#define MSG_CMSG_COMPAT 0 /* We never have 32 bit fixups */
#endif
-
/* Setsockoptions(2) level. Thanks to BSD these must match IPPROTO_xxx */
#define SOL_IP 0
/* #define SOL_ICMP 1 No-no-no! Due to Linux :-) we cannot use SOL_ICMP=1 */
diff --git a/net/compat.c b/net/compat.c
index c578d93..35db4bd 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -73,6 +73,7 @@ int get_compat_msghdr(struct msghdr *kmsg, struct compat_msghdr __user *umsg)
kmsg->msg_name = compat_ptr(tmp1);
kmsg->msg_iov = compat_ptr(tmp2);
kmsg->msg_control = compat_ptr(tmp3);
+ kmsg->msg_meta = NULL;
return 0;
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 4821df8..f58718f 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -203,6 +203,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
skb->data = data;
skb_reset_tail_pointer(skb);
skb->end = skb->tail + size;
+ skb->meta_info = NULL;
#ifdef NET_SKBUFF_DATA_USES_OFFSET
skb->mac_header = ~0U;
#endif
@@ -396,6 +397,7 @@ static void skb_release_head_state(struct sk_buff *skb)
skb->tc_verd = 0;
#endif
#endif
+ skb_free_meta_info(skb);
}
/* Free everything but the sk_buff shell. */
@@ -500,6 +502,7 @@ bool skb_recycle_check(struct sk_buff *skb, int skb_size)
memset(skb, 0, offsetof(struct sk_buff, tail));
skb->data = skb->head + NET_SKB_PAD;
skb_reset_tail_pointer(skb);
+ skb->meta_info = NULL;
return true;
}
@@ -542,6 +545,12 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
#endif
#endif
new->vlan_tci = old->vlan_tci;
+
+ if (old->meta_info) {
+ skb_set_meta_info(new,old->meta_info);
+ } else {
+ new->meta_info = NULL;
+ }
skb_copy_secmark(new, old);
}
@@ -556,6 +565,7 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb)
n->next = n->prev = NULL;
n->sk = NULL;
+ n->meta_info = NULL;
__copy_skb_header(n, skb);
C(len);
@@ -614,6 +624,7 @@ struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
{
struct sk_buff *n;
+ skb_assert_meta(skb,__func__);
n = skb + 1;
if (skb->fclone == SKB_FCLONE_ORIG &&
n->fclone == SKB_FCLONE_UNAVAILABLE) {
@@ -850,6 +861,7 @@ adjust_others:
skb->cloned = 0;
skb->hdr_len = 0;
skb->nohdr = 0;
+ skb->meta_info = NULL;
atomic_set(&skb_shinfo(skb)->dataref, 1);
return 0;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b0e5330..d579175 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1004,8 +1004,10 @@ new_segment:
if (copy > skb_tailroom(skb))
copy = skb_tailroom(skb);
err = skb_add_data_nocache(sk, skb, from, copy);
- if (err)
+ if (err) {
goto do_fault;
+ }
+ skb_set_meta_info(skb,msg->msg_meta);
} else {
int merge = 0;
int i = skb_shinfo(skb)->nr_frags;
@@ -1059,6 +1061,7 @@ new_segment:
}
goto do_error;
}
+ skb_set_meta_info(skb,msg->msg_meta);
/* Update the skb. */
if (merge) {
diff --git a/net/socket.c b/net/socket.c
index cf41afc..a0dd6ac 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -844,6 +844,7 @@ static ssize_t do_sock_read(struct msghdr *msg, struct kiocb *iocb,
msg->msg_controllen = 0;
msg->msg_iov = (struct iovec *)iov;
msg->msg_iovlen = nr_segs;
+ msg->msg_meta = NULL;
msg->msg_flags = (file->f_flags & O_NONBLOCK) ? MSG_DONTWAIT : 0;
return __sock_recvmsg(iocb, sock, msg, size, msg->msg_flags);
@@ -884,6 +885,7 @@ static ssize_t do_sock_write(struct msghdr *msg, struct kiocb *iocb,
msg->msg_controllen = 0;
msg->msg_iov = (struct iovec *)iov;
msg->msg_iovlen = nr_segs;
+ msg->msg_meta = NULL;
msg->msg_flags = (file->f_flags & O_NONBLOCK) ? MSG_DONTWAIT : 0;
if (sock->type == SOCK_SEQPACKET)
msg->msg_flags |= MSG_EOR;
@@ -1695,6 +1697,8 @@ SYSCALL_DEFINE6(sendto, int, fd, void __user *, buff, size_t, len,
msg.msg_control = NULL;
msg.msg_controllen = 0;
msg.msg_namelen = 0;
+ msg.msg_meta = NULL;
+
if (addr) {
err = move_addr_to_kernel(addr, addr_len, (struct sockaddr *)&address);
if (err < 0)
@@ -1713,6 +1717,7 @@ out:
return err;
}
+
/*
* Send a datagram down a socket.
*/
@@ -1723,6 +1728,7 @@ SYSCALL_DEFINE4(send, int, fd, void __user *, buff, size_t, len,
return sys_sendto(fd, buff, len, flags, NULL, 0);
}
+
/*
* Receive a frame from the socket and optionally record the address of the
* sender. We verify the buffers are writable and if needed move the
@@ -1754,6 +1760,7 @@ SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, ubuf, size_t, size,
iov.iov_base = ubuf;
msg.msg_name = (struct sockaddr *)&address;
msg.msg_namelen = sizeof(address);
+ msg.msg_meta = NULL;
if (sock->file->f_flags & O_NONBLOCK)
flags |= MSG_DONTWAIT;
err = sock_recvmsg(sock, &msg, size, flags);
@@ -1888,14 +1895,55 @@ static int __sys_sendmsg(struct socket *sock, struct msghdr __user *msg,
__attribute__ ((aligned(sizeof(__kernel_size_t))));
/* 20 is size of ipv6_pktinfo */
unsigned char *ctl_buf = ctl;
+#if MSGMETADEBUG_LVL < 2
+ struct msgmeta __user *u_msgmeta = NULL;
+#endif
int err, ctl_len, iov_size, total_len;
err = -EFAULT;
+ msg_sys->msg_meta = NULL;
if (MSG_CMSG_COMPAT & flags) {
if (get_compat_msghdr(msg_sys, msg_compat))
return -EFAULT;
- } else if (copy_from_user(msg_sys, msg, sizeof(struct msghdr)))
- return -EFAULT;
+ } else if (MSG_METAINFO & flags) {
+ /* the user supplied meta information for this message -- copy the pointer to the struct aswell */
+ if (copy_from_user(msg_sys, msg, sizeof(struct msghdr)))
+ goto out;
+#if MSGMETADEBUG_LVL < 2
+ u_msgmeta = msg_sys->msg_meta;
+ /* First verify the memory area pointed to by u_msgmeta is readable */
+ if (unlikely(!access_ok(VERIFY_READ, u_msgmeta, sizeof(struct msgmeta) - sizeof(atomic_t)))) {
+ goto out;
+ }
+#endif
+ /* Allocate the kernel space for the struct msgmeta */
+ msg_sys->msg_meta = kmalloc(sizeof(struct msgmeta),GFP_KERNEL|GFP_ATOMIC);
+ if (!msg_sys->msg_meta) {
+ err = -ENOMEM;
+ goto out;
+ }
+#if MSGMETADEBUG_LVL > 1
+ msg_sys->msg_meta->magicA = 0xcaffee;
+ msg_sys->msg_meta->magicB = ~0xcaffee;
+ msg_sys->msg_meta->delay_us = 0x133700;
+#else
+ /* Now copy the struct msgmeta. omit sizeof(atomic_t) bytes. The user does *not* supply it.*/
+ if (copy_from_user(msg_sys->msg_meta,u_msgmeta,sizeof(struct msgmeta) - sizeof(atomic_t))) {
+ goto out_freemeta;
+ }
+#endif
+ /* No references to it by any instance of sk_buff */
+ atomic_set(&msg_sys->msg_meta->refcnt,0);
+ MSGMETADEBUG(1,"Copied msgmeta(0x%p) from user - delay is: 0x%x\n",msg_sys->msg_meta,msg_sys->msg_meta->delay_us);
+
+ //kfree(msg_sys->msg_meta);
+ //msg_sys->msg_meta = NULL;
+ } else {
+ /* Normally the user uses the old struct msghdr which does not contain a pointer to a struct msgmeta */
+ if (copy_from_user(msg_sys, msg, sizeof(struct msghdr) - sizeof(struct msgmeta*)))
+ return -EFAULT;
+ msg_sys->msg_meta = NULL;
+ }
/* do not move before msg_sys is valid */
err = -EMSGSIZE;
@@ -1983,7 +2031,12 @@ static int __sys_sendmsg(struct socket *sock, struct msghdr __user *msg,
memcpy(&used_address->name, msg_sys->msg_name,
used_address->name_len);
}
-
+#if MSGMETADEBUG_LVL > 1
+ if (msg_sys->msg_meta) {
+ /* Just for debug purpose print a message right before returning to userspace */
+ MSGMETADEBUG(1,"return to userspace\n");
+ }
+#endif
out_freectl:
if (ctl_buf != ctl)
sock_kfree_s(sock->sk, ctl_buf, ctl_len);
@@ -1991,6 +2044,10 @@ out_freeiov:
if (iov != iovstack)
sock_kfree_s(sock->sk, iov, iov_size);
out:
+ if (err != 0 && msg_sys->msg_meta != NULL) {
+ kfree(msg_sys->msg_meta);
+ msg_sys->msg_meta = NULL;
+ }
return err;
}
@@ -2065,7 +2122,6 @@ int __sys_sendmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
}
fput_light(sock->file, fput_needed);
-
/* We only return an error if no datagrams were able to be sent */
if (datagrams != 0)
return datagrams;
^ permalink raw reply related
* [PATCH net-next V2 1/6] net/mlx4_en: Issue the dump eth statistics command under lock
From: Amir Vadai @ 2013-01-24 11:54 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Or Gerlitz, Amir Vadai, Eugenia Emantayev
In-Reply-To: <1359028459-30961-1-git-send-email-amirv@mellanox.com>
From: Eugenia Emantayev <eugenia@mellanox.com>
Performing the DUMP_ETH_STATS firmware command outside the lock leads to kernel
panic when data structures such as RX/TX rings are freed in parallel, e.g when
one changes the mtu or ring sizes.
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index b467513..b6c645f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -977,12 +977,12 @@ static void mlx4_en_do_get_stats(struct work_struct *work)
struct mlx4_en_dev *mdev = priv->mdev;
int err;
- err = mlx4_en_DUMP_ETH_STATS(mdev, priv->port, 0);
- if (err)
- en_dbg(HW, priv, "Could not update stats\n");
-
mutex_lock(&mdev->state_lock);
if (mdev->device_up) {
+ err = mlx4_en_DUMP_ETH_STATS(mdev, priv->port, 0);
+ if (err)
+ en_dbg(HW, priv, "Could not update stats\n");
+
if (priv->port_up)
mlx4_en_auto_moderation(priv);
--
1.7.8.2
^ permalink raw reply related
* [PATCH net-next V2 0/6] Mellanox Ethernet driver updates 2013-01-16
From: Amir Vadai @ 2013-01-24 11:54 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Or Gerlitz, Amir Vadai
Hi Dave,
These are all small bug fixes to the Mellanox mlx4 driver, patches done against
net-next commit d59577b "sk-filter: Add ability to lock a socket filter program"
Changes from V1:
- added memory barrier to "Fix a race when closing TX queue"
- Fixed indentation
Changes from V0:
- Removed patch "Set carrier to off when a port is stopped"
Need some time to make sure using netif_device_detach instead of
netif_carrier_off, as suggested by Ben, won't break anything.
Thanks,
Amir
Amir Vadai (2):
net/mlx4_en: Fix a race when closing TX queue
net/mlx4_en: Initialize RFS filters lock and list in init_netdev
Aviad Yehezkel (1):
net/mlx4_en: Fix traffic loss under promiscuous mode
Eugenia Emantayev (2):
net/mlx4_en: Issue the dump eth statistics command under lock
net/mlx4_en: Use the correct netif lock on ndo_set_rx_mode
Jack Morgenstein (1):
net/mlx4_core: Return proper error code when __mlx4_add_one fails
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 55 ++++++++++++++++--------
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 16 ++++++-
drivers/net/ethernet/mellanox/mlx4/main.c | 7 ++-
3 files changed, 57 insertions(+), 21 deletions(-)
--
1.7.8.2
^ permalink raw reply
* [PATCH net-next V2 2/6] net/mlx4_en: Fix traffic loss under promiscuous mode
From: Amir Vadai @ 2013-01-24 11:54 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Or Gerlitz, Amir Vadai, Aviad Yehezkel, Eugenia Emantayev,
Hadar Hen Zion
In-Reply-To: <1359028459-30961-1-git-send-email-amirv@mellanox.com>
From: Aviad Yehezkel <aviadye@mellanox.com>
When port is stopped and flow steering mode is not device managed: promisc QP
rule wasn't removed from MCG table.
Added code to remove it in all flow steering modes.
In addition, promsic rule removal should be in stop port and not in start
port - moved it accordingly.
Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 35 +++++++++++++++++------
1 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index b6c645f..bab8cec 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1167,15 +1167,6 @@ int mlx4_en_start_port(struct net_device *dev)
/* Must redo promiscuous mode setup. */
priv->flags &= ~(MLX4_EN_FLAG_PROMISC | MLX4_EN_FLAG_MC_PROMISC);
- if (mdev->dev->caps.steering_mode ==
- MLX4_STEERING_MODE_DEVICE_MANAGED) {
- mlx4_flow_steer_promisc_remove(mdev->dev,
- priv->port,
- MLX4_FS_PROMISC_UPLINK);
- mlx4_flow_steer_promisc_remove(mdev->dev,
- priv->port,
- MLX4_FS_PROMISC_ALL_MULTI);
- }
/* Schedule multicast task to populate multicast list */
queue_work(mdev->workqueue, &priv->mcast_task);
@@ -1227,6 +1218,32 @@ void mlx4_en_stop_port(struct net_device *dev)
/* Set port as not active */
priv->port_up = false;
+ /* Promsicuous mode */
+ if (mdev->dev->caps.steering_mode ==
+ MLX4_STEERING_MODE_DEVICE_MANAGED) {
+ priv->flags &= ~(MLX4_EN_FLAG_PROMISC |
+ MLX4_EN_FLAG_MC_PROMISC);
+ mlx4_flow_steer_promisc_remove(mdev->dev,
+ priv->port,
+ MLX4_FS_PROMISC_UPLINK);
+ mlx4_flow_steer_promisc_remove(mdev->dev,
+ priv->port,
+ MLX4_FS_PROMISC_ALL_MULTI);
+ } else if (priv->flags & MLX4_EN_FLAG_PROMISC) {
+ priv->flags &= ~MLX4_EN_FLAG_PROMISC;
+
+ /* Disable promiscouos mode */
+ mlx4_unicast_promisc_remove(mdev->dev, priv->base_qpn,
+ priv->port);
+
+ /* Disable Multicast promisc */
+ if (priv->flags & MLX4_EN_FLAG_MC_PROMISC) {
+ mlx4_multicast_promisc_remove(mdev->dev, priv->base_qpn,
+ priv->port);
+ priv->flags &= ~MLX4_EN_FLAG_MC_PROMISC;
+ }
+ }
+
/* Detach All multicasts */
memset(&mc_list[10], 0xff, ETH_ALEN);
mc_list[5] = priv->port; /* needed for B0 steering support */
--
1.7.8.2
^ permalink raw reply related
* [PATCH net-next V2 3/6] net/mlx4_en: Use the correct netif lock on ndo_set_rx_mode
From: Amir Vadai @ 2013-01-24 11:54 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Or Gerlitz, Amir Vadai, Eugenia Emantayev
In-Reply-To: <1359028459-30961-1-git-send-email-amirv@mellanox.com>
From: Eugenia Emantayev <eugenia@mellanox.com>
The device multicast list is protected by netif_addr_lock_bh in the networking core, we should
use this locking practice in mlx4_en too.
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
Reviewed-by: Yevgeny Petrilin <yevgenyp@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index bab8cec..805e242 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -767,9 +767,9 @@ static void mlx4_en_do_set_multicast(struct work_struct *work)
/* Update multicast list - we cache all addresses so they won't
* change while HW is updated holding the command semaphor */
- netif_tx_lock_bh(dev);
+ netif_addr_lock_bh(dev);
mlx4_en_cache_mclist(dev);
- netif_tx_unlock_bh(dev);
+ netif_addr_unlock_bh(dev);
list_for_each_entry(mclist, &priv->mc_list, list) {
mcast_addr = mlx4_en_mac_to_u64(mclist->addr);
mlx4_SET_MCAST_FLTR(mdev->dev, priv->port,
--
1.7.8.2
^ permalink raw reply related
* [PATCH net-next V2 5/6] net/mlx4_en: Fix a race when closing TX queue
From: Amir Vadai @ 2013-01-24 11:54 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Or Gerlitz, Amir Vadai, Eric Dumazet, Yevgeny Petrilin,
Eugenia Emantayev
In-Reply-To: <1359028459-30961-1-git-send-email-amirv@mellanox.com>
There is a possible race where the TX completion handler can clean the
entire TX queue between the decision that the queue is full and actually
closing it. To avoid this situation, check again if the queue is really
full, if not, reopen the transmit and continue with sending the packet.
CC: Eric Dumazet <edumazet@google.com>
Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.com>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 16 +++++++++++++++-
1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 16af338..1e793e3 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -588,7 +588,21 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
netif_tx_stop_queue(ring->tx_queue);
priv->port_stats.queue_stopped++;
- return NETDEV_TX_BUSY;
+ /* If queue was emptied after the if, and before the
+ * stop_queue - need to wake the queue, or else it will remain
+ * stopped forever.
+ * Need a memory barrier to make sure ring->cons was not
+ * updated before queue was stopped.
+ */
+ wmb();
+
+ if (unlikely(((int)(ring->prod - ring->cons)) <=
+ ring->size - HEADROOM - MAX_DESC_TXBBS)) {
+ netif_tx_wake_queue(ring->tx_queue);
+ priv->port_stats.wake_queue++;
+ } else {
+ return NETDEV_TX_BUSY;
+ }
}
/* Track current inflight packets for performance analysis */
--
1.7.8.2
^ permalink raw reply related
* [PATCH net-next V2 4/6] net/mlx4_core: Return proper error code when __mlx4_add_one fails
From: Amir Vadai @ 2013-01-24 11:54 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Or Gerlitz, Amir Vadai, Jack Morgenstein,
Eugenia Emantayev
In-Reply-To: <1359028459-30961-1-git-send-email-amirv@mellanox.com>
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
Returning 0 (success) when in fact we are aborting the load, leads to kernel
panic when unloading the module. Fix that by returning the actual error code.
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/main.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index e1bafff..983fd3d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2169,7 +2169,8 @@ slave_start:
dev->num_slaves = MLX4_MAX_NUM_SLAVES;
else {
dev->num_slaves = 0;
- if (mlx4_multi_func_init(dev)) {
+ err = mlx4_multi_func_init(dev);
+ if (err) {
mlx4_err(dev, "Failed to init slave mfunc"
" interface, aborting.\n");
goto err_cmd;
@@ -2193,7 +2194,8 @@ slave_start:
/* In master functions, the communication channel must be initialized
* after obtaining its address from fw */
if (mlx4_is_master(dev)) {
- if (mlx4_multi_func_init(dev)) {
+ err = mlx4_multi_func_init(dev);
+ if (err) {
mlx4_err(dev, "Failed to init master mfunc"
"interface, aborting.\n");
goto err_close;
@@ -2210,6 +2212,7 @@ slave_start:
mlx4_enable_msi_x(dev);
if ((mlx4_is_mfunc(dev)) &&
!(dev->flags & MLX4_FLAG_MSI_X)) {
+ err = -ENOSYS;
mlx4_err(dev, "INTx is not supported in multi-function mode."
" aborting.\n");
goto err_free_eq;
--
1.7.8.2
^ 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