* [PATCH 1/3] net: phy: micrel: Staticize ksz8873mll_read_status()
From: Jingoo Han @ 2013-08-06 8:29 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Hector Palacios, David J. Choi, Jingoo Han
ksz8873mll_read_status() is used only in this file.
Fix the following sparse warning:
drivers/net/phy/micrel.c:147:5: warning: symbol 'ksz8873mll_read_status' was not declared. Should it be static?
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/phy/micrel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 2510435..9ca4945 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -144,7 +144,7 @@ static int ks8051_config_init(struct phy_device *phydev)
#define KSZ8873MLL_GLOBAL_CONTROL_4 0x06
#define KSZ8873MLL_GLOBAL_CONTROL_4_DUPLEX (1 << 6)
#define KSZ8873MLL_GLOBAL_CONTROL_4_SPEED (1 << 4)
-int ksz8873mll_read_status(struct phy_device *phydev)
+static int ksz8873mll_read_status(struct phy_device *phydev)
{
int regval;
--
1.7.10.4
^ permalink raw reply related
* Re: Linux IPV6_SUBTREES not functioning
From: Hannes Frederic Sowa @ 2013-08-06 8:30 UTC (permalink / raw)
To: Teco Boot; +Cc: netdev, boutier
In-Reply-To: <8CBF8819-E1D9-445E-9E7C-793422934733@inf-net.nl>
Hi!
On Fri, Aug 02, 2013 at 12:15:03PM +0200, Teco Boot wrote:
> For destination & source address routing, I prefer the single routing table approach with IPV6_SUBTREES over the multiple tables with ip rules approach.
> Can some take a look at my findings? It looks broken. It seems the source address check in route cache is missing.
Have you verified this behaviour with actual network traffic?
Greetings,
Hannes
^ permalink raw reply
* [PATCH 2/3] net: phy: mdio: add missing __iomem annotation
From: Jingoo Han @ 2013-08-06 8:32 UTC (permalink / raw)
To: 'David S. Miller'; +Cc: netdev, 'Timur Tabi', Jingoo Han
Added missing __iomem annotation in order to fix the following
sparse warnings:
drivers/net/phy/mdio-mux-mmioreg.c:51:27: warning: incorrect type in initializer (different address spaces)
drivers/net/phy/mdio-mux-mmioreg.c:51:27: expected void *p
drivers/net/phy/mdio-mux-mmioreg.c:51:27: got void [noderef] <asn:2>*
drivers/net/phy/mdio-mux-mmioreg.c:57:21: warning: incorrect type in argument 1 (different address spaces)
drivers/net/phy/mdio-mux-mmioreg.c:57:21: expected void const volatile [noderef] <asn:2>*addr
drivers/net/phy/mdio-mux-mmioreg.c:57:21: got void *p
drivers/net/phy/mdio-mux-mmioreg.c:60:25: warning: incorrect type in argument 2 (different address spaces)
drivers/net/phy/mdio-mux-mmioreg.c:60:25: expected void volatile [noderef] <asn:2>*addr
drivers/net/phy/mdio-mux-mmioreg.c:60:25: got void *p
drivers/net/phy/mdio-mux-mmioreg.c:64:25: warning: incorrect type in argument 1 (different address spaces)
drivers/net/phy/mdio-mux-mmioreg.c:64:25: expected void volatile [noderef] <asn:2>*addr
drivers/net/phy/mdio-mux-mmioreg.c:64:25: got void *p
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/phy/mdio-mux-mmioreg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/mdio-mux-mmioreg.c b/drivers/net/phy/mdio-mux-mmioreg.c
index 9733bd2..f8e305d 100644
--- a/drivers/net/phy/mdio-mux-mmioreg.c
+++ b/drivers/net/phy/mdio-mux-mmioreg.c
@@ -48,7 +48,7 @@ static int mdio_mux_mmioreg_switch_fn(int current_child, int desired_child,
struct mdio_mux_mmioreg_state *s = data;
if (current_child ^ desired_child) {
- void *p = ioremap(s->phys, 1);
+ void __iomem *p = ioremap(s->phys, 1);
uint8_t x, y;
if (!p)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/3] net: wireless: rt2x00: Staticize rt2x00queue_pause_queue_nocheck()
From: Jingoo Han @ 2013-08-06 8:36 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, linux-wireless, 'Ivo van Doorn',
'Gertjan van Wingerde', 'Helmut Schaa',
Jingoo Han
rt2x00queue_pause_queue_nocheck()is used only in this file.
Fix the following sparse warning:
drivers/net/wireless/rt2x00/rt2x00queue.c:939:6: warning: symbol 'rt2x00queue_pause_queue_nocheck' was not declared. Should it be
static?
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/wireless/rt2x00/rt2x00queue.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index aa95c6c..6c8a33b 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -936,7 +936,7 @@ void rt2x00queue_index_inc(struct queue_entry *entry, enum queue_index index)
spin_unlock_irqrestore(&queue->index_lock, irqflags);
}
-void rt2x00queue_pause_queue_nocheck(struct data_queue *queue)
+static void rt2x00queue_pause_queue_nocheck(struct data_queue *queue)
{
switch (queue->qid) {
case QID_AC_VO:
--
1.7.10.4
^ permalink raw reply related
* Re: [net-next,1/3] bonding: fix vlan 0 addition and removal
From: Nikolay Aleksandrov @ 2013-08-06 8:39 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: netdev, fubar, andy, davem, kaber
In-Reply-To: <20130805215126.GB3859@redhat.com>
> From 1c89abefebe90568ed52d2df59fcfdd650bc4696 Mon Sep 17 00:00:00 2001
> From: Veaceslav Falico <vfalico@redhat.com>
> Date: Mon, 5 Aug 2013 23:29:12 +0200
> Subject: [PATCH] bonding: add vlan_uses_dev_rcu() and make bond_vlan_used()
> use it
>
> Currently, bond_vlan_used() looks for any vlan, including the pseudo-vlan
> id 0, and always returns true if 8021q is loaded. This creates several bad
> situations - some warnings in __bond_release_one() because it thinks that
> we still have vlans while removing, sending LB packets with vlan id 0 and,
> possibly, other caused by vlan id 0.
>
> Fix it by adding a new call, vlan_uses_dev_rcu(), which is the same as
> vlan_uses_dev(), but uses rcu_dereference() instead of rtnl, and thus we
> can use it in bond_vlan_used() wrapped in rcu_read_lock().
>
> Also, use the pure vlan_uses_dev() in __bond_release_one() cause the rtnl
> lock is held there.
>
Just 1 more note, you can't trust nr_vlan_devs under RCU.
^ permalink raw reply
* Re: macvtap MAX_SKB_FRAGS fix...
From: Jason Wang @ 2013-08-06 8:39 UTC (permalink / raw)
To: David Miller; +Cc: netdev, mst
In-Reply-To: <20130805.205104.295403081094571911.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 618 bytes --]
On 08/06/2013 11:51 AM, David Miller wrote:
> Jason, I want to get that into v3.4.x -stable but it needs a backport
> and I'd rather someone like you who knows the code does it rather
> than I.
>
> The issue seems that be that the arguments to uarg->callback() were
> different back then.
>
> Thanks.
Sure.
For 3.4.x we need to pick c70aa540c7a9f67add11ad3161096fb95233aa2e
(vhost: zerocopy: poll vq in zerocopy callback) first to let the
backport work. The reason of different arguments is because 3.4.x does
not track DMA failure. So we can just do uarg->callback(uarg);
Please see the attached patches.
Thanks
[-- Attachment #2: 0001-vhost-zerocopy-poll-vq-in-zerocopy-callback.patch --]
[-- Type: text/x-patch, Size: 1229 bytes --]
>From 4bc54cb526fdc65a23cb8a112c0fa0fb7c9527ac Mon Sep 17 00:00:00 2001
From: Jason Wang <jasowang@redhat.com>
Date: Wed, 2 May 2012 11:42:54 +0800
Subject: [PATCH 1/2] vhost: zerocopy: poll vq in zerocopy callback
commit c70aa540c7a9f67add11ad3161096fb95233aa2e upstream.
We add used and signal guest in worker thread but did not poll the virtqueue
during the zero copy callback. This may lead the missing of adding and
signalling during zerocopy. Solve this by polling the virtqueue and let it
wakeup the worker during callback.
Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 1a9e2a9..a50cb9c 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1603,6 +1603,7 @@ void vhost_zerocopy_callback(struct ubuf_info *ubuf)
struct vhost_ubuf_ref *ubufs = ubuf->ctx;
struct vhost_virtqueue *vq = ubufs->vq;
+ vhost_poll_queue(&vq->poll);
/* set len = 1 to mark this desc buffers done DMA */
vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
--
1.7.1
[-- Attachment #3: 0002-macvtap-do-not-zerocopy-if-iov-needs-more-pages-than.patch --]
[-- Type: text/x-patch, Size: 3916 bytes --]
>From 9839e0c9838ea70ad5ce634e6474a82110a14169 Mon Sep 17 00:00:00 2001
From: Jason Wang <jasowang@redhat.com>
Date: Thu, 18 Jul 2013 10:55:16 +0800
Subject: [PATCH 2/2] macvtap: do not zerocopy if iov needs more pages than MAX_SKB_FRAGS
commit ece793fcfc417b3925844be88a6a6dc82ae8f7c6 upstream.
We try to linearize part of the skb when the number of iov is greater than
MAX_SKB_FRAGS. This is not enough since each single vector may occupy more than
one pages, so zerocopy_sg_fromiovec() may still fail and may break the guest
network.
Solve this problem by calculate the pages needed for iov before trying to do
zerocopy and switch to use copy instead of zerocopy if it needs more than
MAX_SKB_FRAGS.
This is done through introducing a new helper to count the pages for iov, and
call uarg->callback() manually when switching from zerocopy to copy to notify
vhost.
We can do further optimization on top.
This bug were introduced from b92946e2919134ebe2a4083e4302236295ea2a73
(macvtap: zerocopy: validate vectors before building skb).
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/macvtap.c | 62 +++++++++++++++++++++++++++++-------------------
1 files changed, 37 insertions(+), 25 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 5151f06..77ce8b2 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -642,6 +642,28 @@ static int macvtap_skb_to_vnet_hdr(const struct sk_buff *skb,
return 0;
}
+static unsigned long iov_pages(const struct iovec *iv, int offset,
+ unsigned long nr_segs)
+{
+ unsigned long seg, base;
+ int pages = 0, len, size;
+
+ while (nr_segs && (offset >= iv->iov_len)) {
+ offset -= iv->iov_len;
+ ++iv;
+ --nr_segs;
+ }
+
+ for (seg = 0; seg < nr_segs; seg++) {
+ base = (unsigned long)iv[seg].iov_base + offset;
+ len = iv[seg].iov_len - offset;
+ size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
+ pages += size;
+ offset = 0;
+ }
+
+ return pages;
+}
/* Get packet from user space buffer */
static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
@@ -688,31 +710,15 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
if (unlikely(count > UIO_MAXIOV))
goto err;
- if (m && m->msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY))
- zerocopy = true;
-
- if (zerocopy) {
- /* Userspace may produce vectors with count greater than
- * MAX_SKB_FRAGS, so we need to linearize parts of the skb
- * to let the rest of data to be fit in the frags.
- */
- if (count > MAX_SKB_FRAGS) {
- copylen = iov_length(iv, count - MAX_SKB_FRAGS);
- if (copylen < vnet_hdr_len)
- copylen = 0;
- else
- copylen -= vnet_hdr_len;
- }
- /* There are 256 bytes to be copied in skb, so there is enough
- * room for skb expand head in case it is used.
- * The rest buffer is mapped from userspace.
- */
- if (copylen < vnet_hdr.hdr_len)
- copylen = vnet_hdr.hdr_len;
- if (!copylen)
- copylen = GOODCOPY_LEN;
+ if (m && m->msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY)) {
+ copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN;
linear = copylen;
- } else {
+ if (iov_pages(iv, vnet_hdr_len + copylen, count)
+ <= MAX_SKB_FRAGS)
+ zerocopy = true;
+ }
+
+ if (!zerocopy) {
copylen = len;
linear = vnet_hdr.hdr_len;
}
@@ -724,9 +730,15 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
if (zerocopy)
err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
- else
+ else {
err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
len);
+ if (!err && m && m->msg_control) {
+ struct ubuf_info *uarg = m->msg_control;
+ uarg->callback(uarg);
+ }
+ }
+
if (err)
goto err_kfree;
--
1.7.1
^ permalink raw reply related
* Re: Linux IPV6_SUBTREES not functioning
From: Teco Boot @ 2013-08-06 8:46 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev, boutier
In-Reply-To: <20130806083005.GC2941@order.stressinduktion.org>
Hi Hannes,
Yes problems with real traffic, with CORE (open source mobile ad hoc network emulator currently maintained by NRL and Boeing).
Mattieu had same problems and therefore stepped to the ip rule. Most, if not all, take this ip rule hack. But then, for each and every source prefix a complete routing table is needed. This is a bad thing and shall be avoided.
Do you think it should work? Have it working? If so, I want to reproduce.
Thanks, Teco
Op 6 aug. 2013, om 10:30 heeft Hannes Frederic Sowa <hannes@stressinduktion.org> het volgende geschreven:
> Hi!
>
> On Fri, Aug 02, 2013 at 12:15:03PM +0200, Teco Boot wrote:
>> For destination & source address routing, I prefer the single routing table approach with IPV6_SUBTREES over the multiple tables with ip rules approach.
>> Can some take a look at my findings? It looks broken. It seems the source address check in route cache is missing.
>
> Have you verified this behaviour with actual network traffic?
>
> Greetings,
>
> Hannes
>
^ permalink raw reply
* Re: [net-next,1/3] bonding: fix vlan 0 addition and removal
From: Veaceslav Falico @ 2013-08-06 8:51 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, fubar, andy, davem, kaber
In-Reply-To: <5200B0C4.2020101@redhat.com>
On Tue, Aug 06, 2013 at 10:16:04AM +0200, Nikolay Aleksandrov wrote:
>On 08/05/2013 11:51 PM, Veaceslav Falico wrote:
>> On Mon, Aug 05, 2013 at 03:28:22PM +0200, nikolay@redhat.com wrote:
>> ...snip...
>>> This is fixed by forbidding the addition/removal of vlan 0 through the
>>> bond's ndo_vlan_rx_add/kill_vid functions, and adding/removing it only when
>>> vlan 0 is in fact being created (or destroyed) on top of a bond interface
>>> in the bond's netdev handling function.
>>
>> Isn't that a bit too intrusive/hacky? I don't think we should treat vlan id
>> 0 somehow differently in terms of adding/removing, though I might be
>> wrong...
>>
>I didn't want to touch the vlan code, so I solved the problem entirely in
>the bonding, mind you there's still a bug when loading the 8021q module
>we'll bump up every slave's vlan 0 refcnt without adding the vlan, and it
>won't get bumped down.
>In my patch that problem still persists but only when an actual vlan 0 is
>being created.
I'll look into that, though maybe we'll require another patch to fix it.
>
>> Maybe we should just fix the bond_vlan_used() function? Something like
>> this (I've done only basic testing, can do more thorough if needed), though
>> it's also not a really clean fix:
>>
>Yes, that would be optimal and I agree.
>
>> From 1c89abefebe90568ed52d2df59fcfdd650bc4696 Mon Sep 17 00:00:00 2001
>> From: Veaceslav Falico <vfalico@redhat.com>
>> Date: Mon, 5 Aug 2013 23:29:12 +0200
>> Subject: [PATCH] bonding: add vlan_uses_dev_rcu() and make bond_vlan_used()
>> use it
>>
>> Currently, bond_vlan_used() looks for any vlan, including the pseudo-vlan
>> id 0, and always returns true if 8021q is loaded. This creates several bad
>> situations - some warnings in __bond_release_one() because it thinks that
>> we still have vlans while removing, sending LB packets with vlan id 0 and,
>> possibly, other caused by vlan id 0.
>>
>> Fix it by adding a new call, vlan_uses_dev_rcu(), which is the same as
>> vlan_uses_dev(), but uses rcu_dereference() instead of rtnl, and thus we
>> can use it in bond_vlan_used() wrapped in rcu_read_lock().
>>
>> Also, use the pure vlan_uses_dev() in __bond_release_one() cause the rtnl
>> lock is held there.
>>
>> CC: Jay Vosburgh <fubar@us.ibm.com>
>> CC: Andy Gospodarek <andy@greyhouse.net>
>> CC: Patrick McHardy <kaber@trash.net>
>> CC: "David S. Miller" <davem@davemloft.net>
>> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>> ---
>I'm okay with this fix, if Dave is also okay with this version then you can
>submit it as a patch, I'll re-base my third one later and resubmit.
Ok, great, then I'll get some testing with this patch and submit it.
^ permalink raw reply
* Re: Linux IPV6_SUBTREES not functioning
From: Hannes Frederic Sowa @ 2013-08-06 8:53 UTC (permalink / raw)
To: Teco Boot; +Cc: netdev, boutier
In-Reply-To: <013B2D02-AE77-4213-A929-3E35F9C9BF0F@inf-net.nl>
On Tue, Aug 06, 2013 at 10:46:43AM +0200, Teco Boot wrote:
> Hi Hannes,
>
> Yes problems with real traffic, with CORE (open source mobile ad hoc network emulator currently maintained by NRL and Boeing).
> Mattieu had same problems and therefore stepped to the ip rule. Most, if not all, take this ip rule hack. But then, for each and every source prefix a complete routing table is needed. This is a bad thing and shall be avoided.
>
> Do you think it should work? Have it working? If so, I want to reproduce.
Definitely, that should work. I can reproduce the same output with your
script on net-next and am looking how to fix this. This could take a
bit because I have another patch in my pipeline for testing. But maybe
it is just a nit. ;)
Thanks,
Hannes
^ permalink raw reply
* Re: [net-next,1/3] bonding: fix vlan 0 addition and removal
From: Veaceslav Falico @ 2013-08-06 8:59 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, fubar, andy, davem, kaber
In-Reply-To: <5200B63A.5070900@redhat.com>
On Tue, Aug 06, 2013 at 10:39:22AM +0200, Nikolay Aleksandrov wrote:
>> From 1c89abefebe90568ed52d2df59fcfdd650bc4696 Mon Sep 17 00:00:00 2001
>> From: Veaceslav Falico <vfalico@redhat.com>
>> Date: Mon, 5 Aug 2013 23:29:12 +0200
>> Subject: [PATCH] bonding: add vlan_uses_dev_rcu() and make bond_vlan_used()
>> use it
>>
>> Currently, bond_vlan_used() looks for any vlan, including the pseudo-vlan
>> id 0, and always returns true if 8021q is loaded. This creates several bad
>> situations - some warnings in __bond_release_one() because it thinks that
>> we still have vlans while removing, sending LB packets with vlan id 0 and,
>> possibly, other caused by vlan id 0.
>>
>> Fix it by adding a new call, vlan_uses_dev_rcu(), which is the same as
>> vlan_uses_dev(), but uses rcu_dereference() instead of rtnl, and thus we
>> can use it in bond_vlan_used() wrapped in rcu_read_lock().
>>
>> Also, use the pure vlan_uses_dev() in __bond_release_one() cause the rtnl
>> lock is held there.
>>
>Just 1 more note, you can't trust nr_vlan_devs under RCU.
Yes, you're right, however we actually don't care anyway if we race with
(un)register_vlan_dev() - we'll end up either in using the (un)registered
vlan or not, and in both cases it's ok. So I don't see a real problem here,
tbh, though I'll look into this also.
^ permalink raw reply
* Re: [net-next,1/3] bonding: fix vlan 0 addition and removal
From: Nikolay Aleksandrov @ 2013-08-06 9:01 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: netdev, fubar, andy, davem, kaber
In-Reply-To: <20130806085111.GL22756@redhat.com>
On 08/06/2013 10:51 AM, Veaceslav Falico wrote:
> On Tue, Aug 06, 2013 at 10:16:04AM +0200, Nikolay Aleksandrov wrote:
>> On 08/05/2013 11:51 PM, Veaceslav Falico wrote:
>>> On Mon, Aug 05, 2013 at 03:28:22PM +0200, nikolay@redhat.com wrote:
>>> ...snip...
>>>> This is fixed by forbidding the addition/removal of vlan 0 through the
>>>> bond's ndo_vlan_rx_add/kill_vid functions, and adding/removing it only
>>>> when
>>>> vlan 0 is in fact being created (or destroyed) on top of a bond interface
>>>> in the bond's netdev handling function.
>>>
>>> Isn't that a bit too intrusive/hacky? I don't think we should treat vlan id
>>> 0 somehow differently in terms of adding/removing, though I might be
>>> wrong...
>>>
>> I didn't want to touch the vlan code, so I solved the problem entirely in
>> the bonding, mind you there's still a bug when loading the 8021q module
>> we'll bump up every slave's vlan 0 refcnt without adding the vlan, and it
>> won't get bumped down.
>> In my patch that problem still persists but only when an actual vlan 0 is
>> being created.
>
> I'll look into that, though maybe we'll require another patch to fix it.
>
You can take a look at:
http://www.spinics.net/lists/netdev/msg241858.html#.UgC5rxoW2Y4
and
http://www.spinics.net/lists/netdev/msg241999.html#.UgC5_BoW2Y4
I think I've explained the refcnt bug well there.
Thanks,
Nik
^ permalink raw reply
* Re: [net-next,1/3] bonding: fix vlan 0 addition and removal
From: Nikolay Aleksandrov @ 2013-08-06 9:07 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: netdev, fubar, andy, davem, kaber
In-Reply-To: <20130806085941.GM22756@redhat.com>
On 08/06/2013 10:59 AM, Veaceslav Falico wrote:
> On Tue, Aug 06, 2013 at 10:39:22AM +0200, Nikolay Aleksandrov wrote:
>>> From 1c89abefebe90568ed52d2df59fcfdd650bc4696 Mon Sep 17 00:00:00 2001
>>> From: Veaceslav Falico <vfalico@redhat.com>
>>> Date: Mon, 5 Aug 2013 23:29:12 +0200
>>> Subject: [PATCH] bonding: add vlan_uses_dev_rcu() and make bond_vlan_used()
>>> use it
>>>
>>> Currently, bond_vlan_used() looks for any vlan, including the pseudo-vlan
>>> id 0, and always returns true if 8021q is loaded. This creates several bad
>>> situations - some warnings in __bond_release_one() because it thinks that
>>> we still have vlans while removing, sending LB packets with vlan id 0 and,
>>> possibly, other caused by vlan id 0.
>>>
>>> Fix it by adding a new call, vlan_uses_dev_rcu(), which is the same as
>>> vlan_uses_dev(), but uses rcu_dereference() instead of rtnl, and thus we
>>> can use it in bond_vlan_used() wrapped in rcu_read_lock().
>>>
>>> Also, use the pure vlan_uses_dev() in __bond_release_one() cause the rtnl
>>> lock is held there.
>>>
>> Just 1 more note, you can't trust nr_vlan_devs under RCU.
>
> Yes, you're right, however we actually don't care anyway if we race with
> (un)register_vlan_dev() - we'll end up either in using the (un)registered
> vlan or not, and in both cases it's ok. So I don't see a real problem here,
> tbh, though I'll look into this also.
You might have stale value in the cache, the implications don't stop there.
I'd like to avoid inconsistent behaviour if there's a way.
A solution that can be relied on and works always would be much more
preferable.
^ permalink raw reply
* Re: [net-next,1/3] bonding: fix vlan 0 addition and removal
From: Veaceslav Falico @ 2013-08-06 9:10 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, fubar, andy, davem, kaber
In-Reply-To: <5200BCD2.4090105@redhat.com>
On Tue, Aug 06, 2013 at 11:07:30AM +0200, Nikolay Aleksandrov wrote:
>On 08/06/2013 10:59 AM, Veaceslav Falico wrote:
>> On Tue, Aug 06, 2013 at 10:39:22AM +0200, Nikolay Aleksandrov wrote:
...snip...
>>> Just 1 more note, you can't trust nr_vlan_devs under RCU.
>>
>> Yes, you're right, however we actually don't care anyway if we race with
>> (un)register_vlan_dev() - we'll end up either in using the (un)registered
>> vlan or not, and in both cases it's ok. So I don't see a real problem here,
>> tbh, though I'll look into this also.
>You might have stale value in the cache, the implications don't stop there.
>I'd like to avoid inconsistent behaviour if there's a way.
>A solution that can be relied on and works always would be much more
>preferable.
Sure :). I currently don't see it, except of using quite offensive locking
strategies, but I'll try to figure something out.
^ permalink raw reply
* Re: [PATCH net-next] xfrm: Make xfrm_state timer monotonic
From: Steffen Klassert @ 2013-08-06 9:22 UTC (permalink / raw)
To: Fan Du; +Cc: davem, netdev
In-Reply-To: <1375772225-19265-1-git-send-email-fan.du@windriver.com>
On Tue, Aug 06, 2013 at 02:57:05PM +0800, Fan Du wrote:
> xfrm_state timer should be independent of system clock change,
> so switch to monotonic clock base.
>
I think a monotonic timer will reintroduce a bug on suspend/resume
that was fixed by commit 9e0d57fd6
(xfrm: SAD entries do not expire correctly after suspend-resume)
Please make sure that this does not happen.
^ permalink raw reply
* Re: macvtap MAX_SKB_FRAGS fix...
From: Jason Wang @ 2013-08-06 9:34 UTC (permalink / raw)
To: David Miller; +Cc: netdev, mst
In-Reply-To: <5200B626.9030006@redhat.com>
On 08/06/2013 04:39 PM, Jason Wang wrote:
> On 08/06/2013 11:51 AM, David Miller wrote:
>> Jason, I want to get that into v3.4.x -stable but it needs a backport
>> and I'd rather someone like you who knows the code does it rather
>> than I.
>>
>> The issue seems that be that the arguments to uarg->callback() were
>> different back then.
>>
>> Thanks.
> Sure.
>
> For 3.4.x we need to pick c70aa540c7a9f67add11ad3161096fb95233aa2e
> (vhost: zerocopy: poll vq in zerocopy callback) first to let the
> backport work. The reason of different arguments is because 3.4.x does
> not track DMA failure. So we can just do uarg->callback(uarg);
>
> Please see the attached patches.
Looks like a git send-email is better, will post them.
>
> Thanks
>
^ permalink raw reply
* [PATCH for 3.4 1/2] vhost: zerocopy: poll vq in zerocopy callback
From: Jason Wang @ 2013-08-06 9:29 UTC (permalink / raw)
To: mst, netdev, davem; +Cc: Jason Wang
commit c70aa540c7a9f67add11ad3161096fb95233aa2e upstream.
We add used and signal guest in worker thread but did not poll the virtqueue
during the zero copy callback. This may lead the missing of adding and
signalling during zerocopy. Solve this by polling the virtqueue and let it
wakeup the worker during callback.
Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 1a9e2a9..a50cb9c 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1603,6 +1603,7 @@ void vhost_zerocopy_callback(struct ubuf_info *ubuf)
struct vhost_ubuf_ref *ubufs = ubuf->ctx;
struct vhost_virtqueue *vq = ubufs->vq;
+ vhost_poll_queue(&vq->poll);
/* set len = 1 to mark this desc buffers done DMA */
vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
--
1.7.1
^ permalink raw reply related
* [PATCH for 3.4 2/2] macvtap: do not zerocopy if iov needs more pages than MAX_SKB_FRAGS
From: Jason Wang @ 2013-08-06 9:29 UTC (permalink / raw)
To: mst, netdev, davem; +Cc: Jason Wang
In-Reply-To: <1375781359-5764-1-git-send-email-jasowang@redhat.com>
commit ece793fcfc417b3925844be88a6a6dc82ae8f7c6 upstream.
We try to linearize part of the skb when the number of iov is greater than
MAX_SKB_FRAGS. This is not enough since each single vector may occupy more than
one pages, so zerocopy_sg_fromiovec() may still fail and may break the guest
network.
Solve this problem by calculate the pages needed for iov before trying to do
zerocopy and switch to use copy instead of zerocopy if it needs more than
MAX_SKB_FRAGS.
This is done through introducing a new helper to count the pages for iov, and
call uarg->callback() manually when switching from zerocopy to copy to notify
vhost.
We can do further optimization on top.
This bug were introduced from b92946e2919134ebe2a4083e4302236295ea2a73
(macvtap: zerocopy: validate vectors before building skb).
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/macvtap.c | 62 +++++++++++++++++++++++++++++-------------------
1 files changed, 37 insertions(+), 25 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 5151f06..77ce8b2 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -642,6 +642,28 @@ static int macvtap_skb_to_vnet_hdr(const struct sk_buff *skb,
return 0;
}
+static unsigned long iov_pages(const struct iovec *iv, int offset,
+ unsigned long nr_segs)
+{
+ unsigned long seg, base;
+ int pages = 0, len, size;
+
+ while (nr_segs && (offset >= iv->iov_len)) {
+ offset -= iv->iov_len;
+ ++iv;
+ --nr_segs;
+ }
+
+ for (seg = 0; seg < nr_segs; seg++) {
+ base = (unsigned long)iv[seg].iov_base + offset;
+ len = iv[seg].iov_len - offset;
+ size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
+ pages += size;
+ offset = 0;
+ }
+
+ return pages;
+}
/* Get packet from user space buffer */
static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
@@ -688,31 +710,15 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
if (unlikely(count > UIO_MAXIOV))
goto err;
- if (m && m->msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY))
- zerocopy = true;
-
- if (zerocopy) {
- /* Userspace may produce vectors with count greater than
- * MAX_SKB_FRAGS, so we need to linearize parts of the skb
- * to let the rest of data to be fit in the frags.
- */
- if (count > MAX_SKB_FRAGS) {
- copylen = iov_length(iv, count - MAX_SKB_FRAGS);
- if (copylen < vnet_hdr_len)
- copylen = 0;
- else
- copylen -= vnet_hdr_len;
- }
- /* There are 256 bytes to be copied in skb, so there is enough
- * room for skb expand head in case it is used.
- * The rest buffer is mapped from userspace.
- */
- if (copylen < vnet_hdr.hdr_len)
- copylen = vnet_hdr.hdr_len;
- if (!copylen)
- copylen = GOODCOPY_LEN;
+ if (m && m->msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY)) {
+ copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN;
linear = copylen;
- } else {
+ if (iov_pages(iv, vnet_hdr_len + copylen, count)
+ <= MAX_SKB_FRAGS)
+ zerocopy = true;
+ }
+
+ if (!zerocopy) {
copylen = len;
linear = vnet_hdr.hdr_len;
}
@@ -724,9 +730,15 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
if (zerocopy)
err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
- else
+ else {
err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
len);
+ if (!err && m && m->msg_control) {
+ struct ubuf_info *uarg = m->msg_control;
+ uarg->callback(uarg);
+ }
+ }
+
if (err)
goto err_kfree;
--
1.7.1
^ permalink raw reply related
* [net-next 0/6] Reduce code duplication of zerocopy of tun and macvtap
From: Jason Wang @ 2013-08-06 9:45 UTC (permalink / raw)
To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
Hi all:
This series tries to reduce the code duplication of zerocopy of tun and macvtap
by:
- Move common helpers iov_pages() and zerocopy_sg_from_iovec() to net/core
- Use exist helpers instead of open coding the new one
With this seires, we can do further optimizations on top.
Please review.
Jason Wang (6):
net: move iov_pages() to net/core/iovec.c
net: move zerocopy_sg_from_iovec() to net/core/datagram.c
net: use skb_fill_page_desc() in zerocopy_sg_from_iovec()
net: remove the useless comment in zerocopy_sg_from_iovec()
net: use release_pages() in zerocopy_sg_from_iovec()
net: use skb_copy_datagram_from_iovec() in zerocopy_sg_from_iovec()
drivers/net/macvtap.c | 103 ------------------------------------------------
drivers/net/tun.c | 103 ------------------------------------------------
include/linux/skbuff.h | 4 ++
include/linux/socket.h | 2 +
net/core/datagram.c | 72 +++++++++++++++++++++++++++++++++
net/core/iovec.c | 24 +++++++++++
6 files changed, 102 insertions(+), 206 deletions(-)
^ permalink raw reply
* [net-next 1/6] net: move iov_pages() to net/core/iovec.c
From: Jason Wang @ 2013-08-06 9:45 UTC (permalink / raw)
To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1375782308-10565-1-git-send-email-jasowang@redhat.com>
To let it be reused and reduce code duplication.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/macvtap.c | 23 -----------------------
drivers/net/tun.c | 23 -----------------------
include/linux/socket.h | 2 ++
net/core/iovec.c | 24 ++++++++++++++++++++++++
4 files changed, 26 insertions(+), 46 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a98fb0e..dfec20d 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -698,29 +698,6 @@ static int macvtap_skb_to_vnet_hdr(const struct sk_buff *skb,
return 0;
}
-static unsigned long iov_pages(const struct iovec *iv, int offset,
- unsigned long nr_segs)
-{
- unsigned long seg, base;
- int pages = 0, len, size;
-
- while (nr_segs && (offset >= iv->iov_len)) {
- offset -= iv->iov_len;
- ++iv;
- --nr_segs;
- }
-
- for (seg = 0; seg < nr_segs; seg++) {
- base = (unsigned long)iv[seg].iov_base + offset;
- len = iv[seg].iov_len - offset;
- size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
- pages += size;
- offset = 0;
- }
-
- return pages;
-}
-
/* Get packet from user space buffer */
static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
const struct iovec *iv, unsigned long total_len,
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 5dce262..18527ef 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1041,29 +1041,6 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
return 0;
}
-static unsigned long iov_pages(const struct iovec *iv, int offset,
- unsigned long nr_segs)
-{
- unsigned long seg, base;
- int pages = 0, len, size;
-
- while (nr_segs && (offset >= iv->iov_len)) {
- offset -= iv->iov_len;
- ++iv;
- --nr_segs;
- }
-
- for (seg = 0; seg < nr_segs; seg++) {
- base = (unsigned long)iv[seg].iov_base + offset;
- len = iv[seg].iov_len - offset;
- size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
- pages += size;
- offset = 0;
- }
-
- return pages;
-}
-
/* Get packet from user space buffer */
static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
void *msg_control, const struct iovec *iv,
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 230c04b..445ef75 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -313,6 +313,8 @@ extern int csum_partial_copy_fromiovecend(unsigned char *kdata,
struct iovec *iov,
int offset,
unsigned int len, __wsum *csump);
+extern unsigned long iov_pages(const struct iovec *iov, int offset,
+ unsigned long nr_segs);
extern int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *address, int mode);
extern int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
diff --git a/net/core/iovec.c b/net/core/iovec.c
index de178e4..b77eeec 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -212,3 +212,27 @@ out_fault:
goto out;
}
EXPORT_SYMBOL(csum_partial_copy_fromiovecend);
+
+unsigned long iov_pages(const struct iovec *iov, int offset,
+ unsigned long nr_segs)
+{
+ unsigned long seg, base;
+ int pages = 0, len, size;
+
+ while (nr_segs && (offset >= iov->iov_len)) {
+ offset -= iov->iov_len;
+ ++iov;
+ --nr_segs;
+ }
+
+ for (seg = 0; seg < nr_segs; seg++) {
+ base = (unsigned long)iov[seg].iov_base + offset;
+ len = iov[seg].iov_len - offset;
+ size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
+ pages += size;
+ offset = 0;
+ }
+
+ return pages;
+}
+EXPORT_SYMBOL(iov_pages);
--
1.7.1
^ permalink raw reply related
* [net-next 2/6] net: move zerocopy_sg_from_iovec() to net/core/datagram.c
From: Jason Wang @ 2013-08-06 9:45 UTC (permalink / raw)
To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1375782308-10565-1-git-send-email-jasowang@redhat.com>
To let it be reused and reduce code duplication. Also document this function.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/macvtap.c | 80 -----------------------------------------
drivers/net/tun.c | 80 -----------------------------------------
include/linux/skbuff.h | 4 ++
net/core/datagram.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 97 insertions(+), 160 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index dfec20d..182364a 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -536,86 +536,6 @@ static inline struct sk_buff *macvtap_alloc_skb(struct sock *sk, size_t prepad,
return skb;
}
-/* set skb frags from iovec, this can move to core network code for reuse */
-static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
- int offset, size_t count)
-{
- int len = iov_length(from, count) - offset;
- int copy = skb_headlen(skb);
- int size, offset1 = 0;
- int i = 0;
-
- /* Skip over from offset */
- while (count && (offset >= from->iov_len)) {
- offset -= from->iov_len;
- ++from;
- --count;
- }
-
- /* copy up to skb headlen */
- while (count && (copy > 0)) {
- size = min_t(unsigned int, copy, from->iov_len - offset);
- if (copy_from_user(skb->data + offset1, from->iov_base + offset,
- size))
- return -EFAULT;
- if (copy > size) {
- ++from;
- --count;
- offset = 0;
- } else
- offset += size;
- copy -= size;
- offset1 += size;
- }
-
- if (len == offset1)
- return 0;
-
- while (count--) {
- struct page *page[MAX_SKB_FRAGS];
- int num_pages;
- unsigned long base;
- unsigned long truesize;
-
- len = from->iov_len - offset;
- if (!len) {
- offset = 0;
- ++from;
- continue;
- }
- base = (unsigned long)from->iov_base + offset;
- size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
- if (i + size > MAX_SKB_FRAGS)
- return -EMSGSIZE;
- num_pages = get_user_pages_fast(base, size, 0, &page[i]);
- if (num_pages != size) {
- int j;
-
- for (j = 0; j < num_pages; j++)
- put_page(page[i + j]);
- return -EFAULT;
- }
- truesize = size * PAGE_SIZE;
- skb->data_len += len;
- skb->len += len;
- skb->truesize += truesize;
- atomic_add(truesize, &skb->sk->sk_wmem_alloc);
- while (len) {
- int off = base & ~PAGE_MASK;
- int size = min_t(int, len, PAGE_SIZE - off);
- __skb_fill_page_desc(skb, i, page[i], off, size);
- skb_shinfo(skb)->nr_frags++;
- /* increase sk_wmem_alloc */
- base += size;
- len -= size;
- i++;
- }
- offset = 0;
- ++from;
- }
- return 0;
-}
-
/*
* macvtap_skb_from_vnet_hdr and macvtap_skb_to_vnet_hdr should
* be shared with the tun/tap driver.
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 18527ef..b163047 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -961,86 +961,6 @@ static struct sk_buff *tun_alloc_skb(struct tun_file *tfile,
return skb;
}
-/* set skb frags from iovec, this can move to core network code for reuse */
-static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
- int offset, size_t count)
-{
- int len = iov_length(from, count) - offset;
- int copy = skb_headlen(skb);
- int size, offset1 = 0;
- int i = 0;
-
- /* Skip over from offset */
- while (count && (offset >= from->iov_len)) {
- offset -= from->iov_len;
- ++from;
- --count;
- }
-
- /* copy up to skb headlen */
- while (count && (copy > 0)) {
- size = min_t(unsigned int, copy, from->iov_len - offset);
- if (copy_from_user(skb->data + offset1, from->iov_base + offset,
- size))
- return -EFAULT;
- if (copy > size) {
- ++from;
- --count;
- offset = 0;
- } else
- offset += size;
- copy -= size;
- offset1 += size;
- }
-
- if (len == offset1)
- return 0;
-
- while (count--) {
- struct page *page[MAX_SKB_FRAGS];
- int num_pages;
- unsigned long base;
- unsigned long truesize;
-
- len = from->iov_len - offset;
- if (!len) {
- offset = 0;
- ++from;
- continue;
- }
- base = (unsigned long)from->iov_base + offset;
- size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
- if (i + size > MAX_SKB_FRAGS)
- return -EMSGSIZE;
- num_pages = get_user_pages_fast(base, size, 0, &page[i]);
- if (num_pages != size) {
- int j;
-
- for (j = 0; j < num_pages; j++)
- put_page(page[i + j]);
- return -EFAULT;
- }
- truesize = size * PAGE_SIZE;
- skb->data_len += len;
- skb->len += len;
- skb->truesize += truesize;
- atomic_add(truesize, &skb->sk->sk_wmem_alloc);
- while (len) {
- int off = base & ~PAGE_MASK;
- int size = min_t(int, len, PAGE_SIZE - off);
- __skb_fill_page_desc(skb, i, page[i], off, size);
- skb_shinfo(skb)->nr_frags++;
- /* increase sk_wmem_alloc */
- base += size;
- len -= size;
- i++;
- }
- offset = 0;
- ++from;
- }
- return 0;
-}
-
/* Get packet from user space buffer */
static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
void *msg_control, const struct iovec *iv,
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ae46475..5ac96f3 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2359,6 +2359,10 @@ extern int skb_copy_datagram_from_iovec(struct sk_buff *skb,
const struct iovec *from,
int from_offset,
int len);
+extern int zerocopy_sg_from_iovec(struct sk_buff *skb,
+ const struct iovec *frm,
+ int offset,
+ size_t count);
extern int skb_copy_datagram_const_iovec(const struct sk_buff *from,
int offset,
const struct iovec *to,
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 8ab48cd..f987fae 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -573,6 +573,99 @@ fault:
}
EXPORT_SYMBOL(skb_copy_datagram_from_iovec);
+/**
+ * zerocopy_sg_from_iovec - Build a zerocopy datagram from an iovec
+ * @skb: buffer to copy
+ * @from: io vector to copy to
+ * @offset: offset in the io vector to start copying from
+ * @count: amount of vectors to copy to buffer from
+ *
+ * The function will first copy up to headlen, and then pin the userspace
+ * pages and build frags through them.
+ *
+ * Returns 0, -EFAULT or -EMSGSIZE.
+ * Note: the iovec is not modified during the copy
+ */
+int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
+ int offset, size_t count)
+{
+ int len = iov_length(from, count) - offset;
+ int copy = skb_headlen(skb);
+ int size, offset1 = 0;
+ int i = 0;
+
+ /* Skip over from offset */
+ while (count && (offset >= from->iov_len)) {
+ offset -= from->iov_len;
+ ++from;
+ --count;
+ }
+
+ /* copy up to skb headlen */
+ while (count && (copy > 0)) {
+ size = min_t(unsigned int, copy, from->iov_len - offset);
+ if (copy_from_user(skb->data + offset1, from->iov_base + offset,
+ size))
+ return -EFAULT;
+ if (copy > size) {
+ ++from;
+ --count;
+ offset = 0;
+ } else
+ offset += size;
+ copy -= size;
+ offset1 += size;
+ }
+
+ if (len == offset1)
+ return 0;
+
+ while (count--) {
+ struct page *page[MAX_SKB_FRAGS];
+ int num_pages;
+ unsigned long base;
+ unsigned long truesize;
+
+ len = from->iov_len - offset;
+ if (!len) {
+ offset = 0;
+ ++from;
+ continue;
+ }
+ base = (unsigned long)from->iov_base + offset;
+ size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
+ if (i + size > MAX_SKB_FRAGS)
+ return -EMSGSIZE;
+ num_pages = get_user_pages_fast(base, size, 0, &page[i]);
+ if (num_pages != size) {
+ int j;
+
+ for (j = 0; j < num_pages; j++)
+ put_page(page[i + j]);
+ return -EFAULT;
+ }
+ truesize = size * PAGE_SIZE;
+ skb->data_len += len;
+ skb->len += len;
+ skb->truesize += truesize;
+ atomic_add(truesize, &skb->sk->sk_wmem_alloc);
+ while (len) {
+ int off = base & ~PAGE_MASK;
+ int size = min_t(int, len, PAGE_SIZE - off);
+ __skb_fill_page_desc(skb, i, page[i], off, size);
+ skb_shinfo(skb)->nr_frags++;
+ /* increase sk_wmem_alloc */
+ base += size;
+ len -= size;
+ i++;
+ }
+ offset = 0;
+ ++from;
+ }
+ return 0;
+}
+EXPORT_SYMBOL(zerocopy_sg_from_iovec);
+
static int skb_copy_and_csum_datagram(const struct sk_buff *skb, int offset,
u8 __user *to, int len,
__wsum *csump)
--
1.7.1
^ permalink raw reply related
* [net-next 3/6] net: use skb_fill_page_desc() in zerocopy_sg_from_iovec()
From: Jason Wang @ 2013-08-06 9:45 UTC (permalink / raw)
To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1375782308-10565-1-git-send-email-jasowang@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/core/datagram.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/net/core/datagram.c b/net/core/datagram.c
index f987fae..bec0867 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -652,8 +652,7 @@ int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
while (len) {
int off = base & ~PAGE_MASK;
int size = min_t(int, len, PAGE_SIZE - off);
- __skb_fill_page_desc(skb, i, page[i], off, size);
- skb_shinfo(skb)->nr_frags++;
+ skb_fill_page_desc(skb, i, page[i], off, size);
/* increase sk_wmem_alloc */
base += size;
len -= size;
--
1.7.1
^ permalink raw reply related
* [net-next 4/6] net: remove the useless comment in zerocopy_sg_from_iovec()
From: Jason Wang @ 2013-08-06 9:45 UTC (permalink / raw)
To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1375782308-10565-1-git-send-email-jasowang@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/core/datagram.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/net/core/datagram.c b/net/core/datagram.c
index bec0867..a8a795c 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -653,7 +653,6 @@ int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
int off = base & ~PAGE_MASK;
int size = min_t(int, len, PAGE_SIZE - off);
skb_fill_page_desc(skb, i, page[i], off, size);
- /* increase sk_wmem_alloc */
base += size;
len -= size;
i++;
--
1.7.1
^ permalink raw reply related
* [net-next 5/6] net: use release_pages() in zerocopy_sg_from_iovec()
From: Jason Wang @ 2013-08-06 9:45 UTC (permalink / raw)
To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1375782308-10565-1-git-send-email-jasowang@redhat.com>
To reduce the duplicated codes.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/core/datagram.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/core/datagram.c b/net/core/datagram.c
index a8a795c..badcd93 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -48,6 +48,7 @@
#include <linux/highmem.h>
#include <linux/spinlock.h>
#include <linux/slab.h>
+#include <linux/pagemap.h>
#include <net/protocol.h>
#include <linux/skbuff.h>
@@ -638,10 +639,7 @@ int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
return -EMSGSIZE;
num_pages = get_user_pages_fast(base, size, 0, &page[i]);
if (num_pages != size) {
- int j;
-
- for (j = 0; j < num_pages; j++)
- put_page(page[i + j]);
+ release_pages(&page[i], num_pages, 0);
return -EFAULT;
}
truesize = size * PAGE_SIZE;
--
1.7.1
^ permalink raw reply related
* [PATCH net] net: rename and move busy poll mib counter
From: Eliezer Tamir @ 2013-08-06 9:52 UTC (permalink / raw)
To: David Miller
Cc: Shawn Bohrer, linux-kernel, netdev, Eliezer Tamir, Eliezer Tamir
Move the low latency mib counter to the ip section.
Rename it from low latency to busy poll.
Reported-by: Shawn Bohrer <sbohrer@rgmadvisors.com>
Signed-off-by: Eliezer Tamir <eliezer.tamir@linux.intel.com>
---
include/net/busy_poll.h | 4 ++--
include/uapi/linux/snmp.h | 2 +-
net/ipv4/proc.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
index 8e2dfc1..1b48741 100644
--- a/include/net/busy_poll.h
+++ b/include/net/busy_poll.h
@@ -121,8 +121,8 @@ static inline bool sk_busy_loop(struct sock *sk, int nonblock)
if (rc > 0)
/* local bh are disabled so it is ok to use _BH */
- NET_ADD_STATS_BH(sock_net(sk),
- LINUX_MIB_LOWLATENCYRXPACKETS, rc);
+ IP_ADD_STATS_BH(dev_net(napi->dev),
+ IPSTATS_MIB_BUSYPOLLRXPACKETS, rc);
} while (!nonblock && skb_queue_empty(&sk->sk_receive_queue) &&
!need_resched() && !busy_loop_timeout(end_time));
diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index af0a674..5575d9b 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -51,6 +51,7 @@ enum
IPSTATS_MIB_INBCASTOCTETS, /* InBcastOctets */
IPSTATS_MIB_OUTBCASTOCTETS, /* OutBcastOctets */
IPSTATS_MIB_CSUMERRORS, /* InCsumErrors */
+ IPSTATS_MIB_BUSYPOLLRXPACKETS, /* BusyPollRxPackets */
__IPSTATS_MIB_MAX
};
@@ -253,7 +254,6 @@ enum
LINUX_MIB_TCPFASTOPENLISTENOVERFLOW, /* TCPFastOpenListenOverflow */
LINUX_MIB_TCPFASTOPENCOOKIEREQD, /* TCPFastOpenCookieReqd */
LINUX_MIB_TCPSPURIOUS_RTX_HOSTQUEUES, /* TCPSpuriousRtxHostQueues */
- LINUX_MIB_LOWLATENCYRXPACKETS, /* LowLatencyRxPackets */
__LINUX_MIB_MAX
};
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 6577a11..3c54a63 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -108,6 +108,7 @@ static const struct snmp_mib snmp4_ipstats_list[] = {
SNMP_MIB_ITEM("FragOKs", IPSTATS_MIB_FRAGOKS),
SNMP_MIB_ITEM("FragFails", IPSTATS_MIB_FRAGFAILS),
SNMP_MIB_ITEM("FragCreates", IPSTATS_MIB_FRAGCREATES),
+ SNMP_MIB_ITEM("BusyPollRxPackets", IPSTATS_MIB_BUSYPOLLRXPACKETS),
SNMP_MIB_SENTINEL
};
@@ -273,7 +274,6 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPFastOpenListenOverflow", LINUX_MIB_TCPFASTOPENLISTENOVERFLOW),
SNMP_MIB_ITEM("TCPFastOpenCookieReqd", LINUX_MIB_TCPFASTOPENCOOKIEREQD),
SNMP_MIB_ITEM("TCPSpuriousRtxHostQueues", LINUX_MIB_TCPSPURIOUS_RTX_HOSTQUEUES),
- SNMP_MIB_ITEM("LowLatencyRxPackets", LINUX_MIB_LOWLATENCYRXPACKETS),
SNMP_MIB_SENTINEL
};
^ permalink raw reply related
* [net-next 6/6] net: use skb_copy_datagram_from_iovec() in zerocopy_sg_from_iovec()
From: Jason Wang @ 2013-08-06 9:45 UTC (permalink / raw)
To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1375782308-10565-1-git-send-email-jasowang@redhat.com>
Use skb_copy_datagram_from_iovec() to avoid code duplication and make it easy to
be read. Also we can do the skipping inside the zero-copy loop.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/core/datagram.c | 37 ++++++++++---------------------------
1 files changed, 10 insertions(+), 27 deletions(-)
diff --git a/net/core/datagram.c b/net/core/datagram.c
index badcd93..af814e7 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -591,48 +591,31 @@ int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
int offset, size_t count)
{
int len = iov_length(from, count) - offset;
- int copy = skb_headlen(skb);
- int size, offset1 = 0;
+ int copy = min_t(int, skb_headlen(skb), len);
+ int size;
int i = 0;
- /* Skip over from offset */
- while (count && (offset >= from->iov_len)) {
- offset -= from->iov_len;
- ++from;
- --count;
- }
-
/* copy up to skb headlen */
- while (count && (copy > 0)) {
- size = min_t(unsigned int, copy, from->iov_len - offset);
- if (copy_from_user(skb->data + offset1, from->iov_base + offset,
- size))
- return -EFAULT;
- if (copy > size) {
- ++from;
- --count;
- offset = 0;
- } else
- offset += size;
- copy -= size;
- offset1 += size;
- }
+ if (skb_copy_datagram_from_iovec(skb, 0, from, offset, copy))
+ return -EFAULT;
- if (len == offset1)
+ if (len == copy)
return 0;
+ offset += copy;
while (count--) {
struct page *page[MAX_SKB_FRAGS];
int num_pages;
unsigned long base;
unsigned long truesize;
- len = from->iov_len - offset;
- if (!len) {
- offset = 0;
+ /* Skip over from offset and copied */
+ if (offset >= from->iov_len) {
+ offset -= from->iov_len;
++from;
continue;
}
+ len = from->iov_len - offset;
base = (unsigned long)from->iov_base + offset;
size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
if (i + size > MAX_SKB_FRAGS)
--
1.7.1
^ 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