* Re: [PATCH v2 2/2] ipv6 addrconf: don't cleanup route prefix for IFA_F_NOPREFIXROUTE
From: Hannes Frederic Sowa @ 2014-01-07 23:09 UTC (permalink / raw)
To: Thomas Haller; +Cc: Jiri Pirko, netdev, stephen, dcbw
In-Reply-To: <1389135262.2248.42.camel@weing>
On Tue, Jan 07, 2014 at 11:54:22PM +0100, Thomas Haller wrote:
> Hi,
>
>
> I think, the modify case is not that hairy and the patch does IMO the
> sensible thing:
>
> case 1) "change NOPREFIXROUTE -> !NOPREFIXROUTE":
> update or add prefix route (as before);;
> case 2) "change !NOPREFIXROUTE -> !NOPREFIXROUTE":
> update or add prefix route (as before);;
> case 3) "change NOPREFIXROUTE -> NOPREFIXROUTE":
> ;;
> case 4) "change !NOPREFIXROUTE -> NOPREFIXROUTE":
> cleanup prefix route;;
>
> where "cleanup" means the same as done in ipv6_del_addr(), as determined
> by check_cleanup_prefix_routes().
>
>
> Allowing modify with case 2) and 3) is important. But for case 4) (and
> possibly 1)), we could also fail with error. I tend to the scheme above
> though because it makes it easier for userspace and is likely what it
> wants.
>
>
>
> The problem of deleting a route created by somebody else is already
> present without this patch in ipv6_del_addr. This is indeed a bit shaky,
> but I guess it's good enough in practice. Do I understand correctly,
> that you think about to use the information from ifp->rt to ensure, that
> what we really cleanup the correct route? If that's what you intend, can
> you elaborate a bit on how to do that?
The ifp->rt thing, I thought of, does not work. It only holds the RTF_LOCAL
route (over loopback) which has nothing to do with the prefix route. We don't
have a link from ifp to the prefix route.
Currently I am fine with the semantics you described above but won't
have time to review them today. I'll do that tomorrow.
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH net-next] net-gre-gro: Add GRE support to the GRO stack
From: David Miller @ 2014-01-07 23:14 UTC (permalink / raw)
To: hkchu; +Cc: edumazet, herbert, ogerlitz, netdev
In-Reply-To: <CAPshTCj+jXjvKCRdBPZYdxPQzTXqKUdF7x2DJE-fBjECVZfH6A@mail.gmail.com>
From: Jerry Chu <hkchu@google.com>
Date: Tue, 7 Jan 2014 14:55:34 -0800
> I hope I've understood this correctly but with Eric's recent patch
> "gre_offload: statically build GRE offloading support", gre_offload.o
> is always present in the kernel regardless of the GRE configuration
> option. And gre_offload.o is all that's needed for GRE-GRO to happen.
I forgot that he changed the registry to happen directly via
module_init(), you're right. Ignore me :-)
^ permalink raw reply
* Re: [PATCH net-next v5] IPv6: add the option to use anycast addresses as source addresses in echo reply
From: Hannes Frederic Sowa @ 2014-01-07 23:17 UTC (permalink / raw)
To: Bill Fink, Francois-Xavier Le Bail, netdev, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki Yoshifuji,
Patrick McHardy
In-Reply-To: <20140107230134.GK30393@order.stressinduktion.org>
On Wed, Jan 08, 2014 at 12:01:34AM +0100, Hannes Frederic Sowa wrote:
> On Tue, Jan 07, 2014 at 05:55:07PM -0500, Bill Fink wrote:
> > On Tue, 7 Jan 2014, Francois-Xavier Le Bail wrote:
> >
> > > This change allows to follow a recommandation of RFC4942.
> > >
> > > - Add "anycast_src_echo_reply" sysctl to control the use of anycast addresses
> > > as source addresses for ICMPv6 echo reply. This sysctl is false by default
> > > to preserve existing behavior.
> > > - Add inline check ipv6_anycast_destination().
> > > - Use them in icmpv6_echo_reply().
> > >
> > > Reference:
> > > RFC4942 - IPv6 Transition/Coexistence Security Considerations
> > > (http://tools.ietf.org/html/rfc4942#section-2.1.6)
> > >
> > > 2.1.6. Anycast Traffic Identification and Security
> > >
> > > [...]
> > > To avoid exposing knowledge about the internal structure of the
> > > network, it is recommended that anycast servers now take advantage of
> > > the ability to return responses with the anycast address as the
> > > source address if possible.
> > >
> > > Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
> > > ---
> > > v4: update Subject and Documentation, this work also with anycast addresses
> > > created via API, not just with Subnet-Router anycast addresses.
> > >
> > > v5: alternative way, replace ipv6_chk_acast_addr() test by
> > > ipv6_anycast_destination() test.
> >
> > Why is ICMPV6 Echo Reply special? Can't the internal structure
> > of the network be divined from other ICMPv6 responses such as
> > Destination Unreachable, Time Exceeded (Hop Limit), and Parameter
> > Problem.
>
> No, here destination must match the source address so that the other side
> handles accordingly when the icmp error is pushed up to the socket layer.
I forgot to mention, rest of kernel followes the old RFC advise to never
use anycast addresses as source addresses.
I guess this will be weakend with upcoming patches so in future this
depends on socket settings and other tweaks. If that's the case default will
be to also respond with anycast source if original destination was anycast.
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH net-next v5] IPv6: add the option to use anycast addresses as source addresses in echo reply
From: David Miller @ 2014-01-07 23:33 UTC (permalink / raw)
To: billfink; +Cc: fx.lebail, netdev, hannes, kuznet, jmorris, yoshfuji, kaber
In-Reply-To: <20140107175507.fb543c70.billfink@mindspring.com>
From: Bill Fink <billfink@mindspring.com>
Date: Tue, 7 Jan 2014 17:55:07 -0500
> Why is ICMPV6 Echo Reply special? Can't the internal structure
> of the network be divined from other ICMPv6 responses such as
> Destination Unreachable, Time Exceeded (Hop Limit), and Parameter
> Problem.
Right, this RFC is talking about source address selection in general,
so this anycast handling should be in a generic place too.
^ permalink raw reply
* Re: [PATCH net-next] net: xfrm: xfrm_policy: fix inline not at beginning of declaration
From: David Miller @ 2014-01-07 23:34 UTC (permalink / raw)
To: dborkman; +Cc: netdev
In-Reply-To: <1389133227-11086-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Tue, 7 Jan 2014 23:20:27 +0100
> Fix three warnings related to:
>
> net/xfrm/xfrm_policy.c:1644:1: warning: 'inline' is not at beginning of declaration [-Wold-style-declaration]
> net/xfrm/xfrm_policy.c:1656:1: warning: 'inline' is not at beginning of declaration [-Wold-style-declaration]
> net/xfrm/xfrm_policy.c:1668:1: warning: 'inline' is not at beginning of declaration [-Wold-style-declaration]
>
> Just removing the inline keyword is sufficient as the compiler will
> decide on its own about inlining or not.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: skbuff: const-ify casts in skb_queue_* functions
From: David Miller @ 2014-01-07 23:34 UTC (permalink / raw)
To: dborkman; +Cc: netdev
In-Reply-To: <1389133424-11176-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Tue, 7 Jan 2014 23:23:44 +0100
> We should const-ify comparisons on skb_queue_* inline helper
> functions as their parameters are const as well, so lets not
> drop that.
>
> Suggested-by: Brad Spengler <spender@grsecurity.net>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Also applied, thanks Daniel.
^ permalink raw reply
* Re: [PATCH 0/2] netfilter fixes for net
From: David Miller @ 2014-01-07 23:38 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1389132819-9697-1-git-send-email-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 7 Jan 2014 23:13:37 +0100
> The following patchset contains two patches:
>
> * fix the IRC NAT helper which was broken when adding (incomplete) IPv6
> support, from Daniel Borkmann.
>
> * Refine the previous bugtrap that Jesper added to catch problems for the
> usage of the sequence adjustment extension in IPVs in Dec 16th, it may
> spam messages in case of finding a real bug.
>
> I know it's fairly late, so please let me know if you prefer that I pass
> you these via net-next.
These look fine, thanks Pablo.
^ permalink raw reply
* Re: [PATCH net-next v5] IPv6: add the option to use anycast addresses as source addresses in echo reply
From: Hannes Frederic Sowa @ 2014-01-07 23:39 UTC (permalink / raw)
To: David Miller
Cc: billfink, fx.lebail, netdev, kuznet, jmorris, yoshfuji, kaber
In-Reply-To: <20140107.183305.1907441201345858474.davem@davemloft.net>
On Tue, Jan 07, 2014 at 06:33:05PM -0500, David Miller wrote:
> From: Bill Fink <billfink@mindspring.com>
> Date: Tue, 7 Jan 2014 17:55:07 -0500
>
> > Why is ICMPV6 Echo Reply special? Can't the internal structure
> > of the network be divined from other ICMPv6 responses such as
> > Destination Unreachable, Time Exceeded (Hop Limit), and Parameter
> > Problem.
>
> Right, this RFC is talking about source address selection in general,
> so this anycast handling should be in a generic place too.
I think it would be reasonable to change future behavior to always
send back icmp errors with the anycast address (same as original packet
destination). Ping is a bit special as it is used for service discovery.
UDP sockets can use their source address at will. So I still think this change
is reasonable.
^ permalink raw reply
* Re: [PATCH 1/2] src: add support for UID/GID socket info
From: Pablo Neira Ayuso @ 2014-01-07 23:42 UTC (permalink / raw)
To: valentina.giusti; +Cc: netfilter-devel, netdev, fw, daniel.wagner
In-Reply-To: <1389101420-16557-2-git-send-email-valentina.giusti@bmw-carit.de>
On Tue, Jan 07, 2014 at 02:30:19PM +0100, valentina.giusti@bmw-carit.de wrote:
> From: Valentina Giusti <Valentina.Giusti@bmw-carit.de>
>
> With this patch libnetfilter_queue is able to parse UID/GID
> socket information.
applied, thanks.
^ permalink raw reply
* Re: [PATCH 2/2] utils: add test for nfq_get_uid and nfq_get_gid
From: Pablo Neira Ayuso @ 2014-01-07 23:43 UTC (permalink / raw)
To: valentina.giusti; +Cc: netfilter-devel, netdev, fw, daniel.wagner
In-Reply-To: <1389101420-16557-3-git-send-email-valentina.giusti@bmw-carit.de>
Applied, but mangled the error message to avoid confusing users in
case they use an old kernel.
^ permalink raw reply
* Re: Use of 'SIOCDEVPRIVATE' in ethernet drivers.
From: Stephen Hemminger @ 2014-01-07 23:44 UTC (permalink / raw)
To: Giri Reddy; +Cc: David Miller, netdev
In-Reply-To: <124064C45E1FC84193A69A61ED3A1F46E230D6@AVMB1.qlogic.org>
On Tue, 7 Jan 2014 22:22:45 +0000
Giri Reddy <giri.reddy@qlogic.com> wrote:
> On 1/7/14 11:05 AM, David Miller wrote:
> > From: Giri Reddy <giri.reddy@qlogic.com>
> > Date: Tue, 7 Jan 2014 18:21:17 +0000
> >
> >> Are we still allowed to use 'SIOCDEVPRIVATE' in drivers if there the
> >> deprecation efforts are on hold.
> > To say that use of SIOCDEVPRIVATE is discouraged would be an
> > understatement.
> >
> > Please create a generic ethtool based facility that other drivers in
> > similar situations can use as well.
>
> Thanks for the input. I will explore the 'ethtool' option. We will have
> multiple control frames exchanged between user space and kernel for
> every flash update operation, most of this will be proprietary stuff
> that will get exchanged - I will explore how to provide a generic
> interface for that.
>
> Giri
Maybe a debugfs or sysfs filesystem would work better than adding ioctl';s
^ permalink raw reply
* Re: [PATCH net-next 0/4] tipc: link setup and failover improvements
From: David Miller @ 2014-01-07 23:45 UTC (permalink / raw)
To: jon.maloy
Cc: netdev, paul.gortmaker, erik.hugne, ying.xue, maloy,
tipc-discussion
In-Reply-To: <1389132164-32451-1-git-send-email-jon.maloy@ericsson.com>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Tue, 7 Jan 2014 17:02:40 -0500
> This series consists of four unrelated commits with different purposes.
>
> - Commit #1 is purely cosmetic and pedagogic, hopefully making the
> failover/tunneling logics slightly easier to understand.
> - Commit #2 fixes a bug that has always been in the code, but was not
> discovered until very recently.
> - Commit #3 fixes a non-fatal race issue in the neighbour discovery
> code.
> - Commit #4 removes an unnecessary indirection step during link
> startup.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH v3 net-next 1/3] sh_eth: Add support for r7s72100
From: Simon Horman @ 2014-01-08 0:06 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-sh, linux-arm-kernel, magnus.damm, sergei.shtylyov
In-Reply-To: <20140107.140755.1722284527902829385.davem@davemloft.net>
On Tue, Jan 07, 2014 at 02:07:55PM -0500, David Miller wrote:
> From: Simon Horman <horms+renesas@verge.net.au>
> Date: Tue, 7 Jan 2014 11:27:13 +0900
>
> > @@ -318,6 +371,14 @@ static int sh_eth_is_gether(struct sh_eth_private *mdp)
> > return 0;
> > }
> >
> > +static int sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
> > +{
> > + if (mdp->reg_offset == sh_eth_offset_fast_rz)
> > + return 1;
> > + else
> > + return 0;
> > +}
>
> Please make this return a boolean and use true/false.
Thanks. I will also refactor the giga_ether version of this
function accordingly.
>
> > @@ -2061,6 +2155,10 @@ static struct net_device_stats *sh_eth_get_stats(struct net_device *ndev)
> > {
> > struct sh_eth_private *mdp = netdev_priv(ndev);
> >
> > + if (sh_eth_is_rz_fast_ether(mdp)) {
> > + return &ndev->stats;
> > + }
>
> Single statement basic blocks do not need curly braces, therefore
> please remove them here.
Will do.
^ permalink raw reply
* Re: [PATCH v3 3/3] ARM: shmobile: genmai: Enable r7s72100-ether
From: Simon Horman @ 2014-01-08 0:07 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev, linux-sh, linux-arm-kernel, Magnus Damm
In-Reply-To: <52CC7C1B.20807@cogentembedded.com>
On Wed, Jan 08, 2014 at 02:13:47AM +0400, Sergei Shtylyov wrote:
> Hello.
>
> On 07-01-2014 6:27, Simon Horman wrote:
>
> You've removed RFC but still haven't signed off. :-)
>
> >---
> >v2
> >* As suggested by Magnus Damm and Sergei Shtylyov
> > - r7s72100 ethernet is not gigabit so do not refer to it as such
>
> >* As suggested by Sergei Shtylyov
> > - set no_ether_link as there is no LINK signal documented
> > in the manual
>
> I'm OK with the patch, you can add my Acked-by.
Thanks, will do.
^ permalink raw reply
* [PATCH net-next v2 0/9] xen-netback: TX grant mapping with SKBTX_DEV_ZEROCOPY instead of copy
From: Zoltan Kiss @ 2014-01-08 0:10 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
A long known problem of the upstream netback implementation that on the TX
path (from guest to Dom0) it copies the whole packet from guest memory into
Dom0. That simply became a bottleneck with 10Gb NICs, and generally it's a
huge perfomance penalty. The classic kernel version of netback used grant
mapping, and to get notified when the page can be unmapped, it used page
destructors. Unfortunately that destructor is not an upstreamable solution.
Ian Campbell's skb fragment destructor patch series [1] tried to solve this
problem, however it seems to be very invasive on the network stack's code,
and therefore haven't progressed very well.
This patch series use SKBTX_DEV_ZEROCOPY flags to tell the stack it needs to
know when the skb is freed up. That is the way KVM solved the same problem,
and based on my initial tests it can do the same for us. Avoiding the extra
copy boosted up TX throughput from 6.8 Gbps to 7.9 (I used a slower
Interlagos box, both Dom0 and guest on upstream kernel, on the same NUMA node,
running iperf 2.0.5, and the remote end was a bare metal box on the same 10Gb
switch)
Based on my investigations the packet get only copied if it is delivered to
Dom0 stack, which is due to this [2] patch. That's a bit unfortunate, but
luckily it doesn't cause a major regression for this usecase. In the future
we should try to eliminate that copy somehow.
There are a few spinoff tasks which will be addressed in separate patches:
- grant copy the header directly instead of map and memcpy. This should help
us avoiding TLB flushing
- use something else than ballooned pages
- fix grant map to use page->index properly
I will run some more extensive tests, but some basic XenRT tests were already
passed with good results.
I've tried to broke it down to smaller patches, with mixed results, so I
welcome suggestions on that part as well:
1: Introduce TX grant map definitions
2: Change TX path from grant copy to mapping
3: Remove old TX grant copy definitons and fix indentations
4: Change RX path for mapped SKB fragments
5: Add stat counters for zerocopy
6: Handle guests with too many frags
7: Add stat counters for frag_list skbs
8: Timeout packets in RX path
9: Aggregate TX unmap operations
v2: I've fixed some smaller things, see the individual patches. I've added a
few new stat counters, and handling the important use case when an older guest
sends lots of slots. Instead of delayed copy now we timeout packets on the RX
path, based on the assumption that otherwise packets should get stucked
anywhere else. Finally some unmap batching to avoid too much TLB flush
v3: Apart from fixing a few things mentioned in responses the important change
is the use the hypercall directly for grant [un]mapping, therefore we can
avoid m2p override.
[1] http://lwn.net/Articles/491522/
[2] https://lkml.org/lkml/2012/7/20/363
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
^ permalink raw reply
* [PATCH net-next v3 2/9] xen-netback: Change TX path from grant copy to mapping
From: Zoltan Kiss @ 2014-01-08 0:10 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1389139818-24458-1-git-send-email-zoltan.kiss@citrix.com>
This patch changes the grant copy on the TX patch to grant mapping
v2:
- delete branch for handling fragmented packets fit PKT_PROT_LEN sized first
request
- mark the effect of using ballooned pages in a comment
- place setting of skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY right
before netif_receive_skb, and mark the importance of it
- grab dealloc_lock before __napi_complete to avoid contention with the
callback's napi_schedule
- handle fragmented packets where first request < PKT_PROT_LEN
- fix up error path when checksum_setup failed
- check before teardown for pending grants, and start complain if they are
there after 10 second
v3:
- delete a surplus checking from tx_action
- remove stray line
- squash xenvif_idx_unmap changes into the first patch
- init spinlocks
- call map hypercall directly instead of gnttab_map_refs()
- fix unmapping timeout in xenvif_free()
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/interface.c | 57 +++++++-
drivers/net/xen-netback/netback.c | 251 ++++++++++++++---------------------
2 files changed, 153 insertions(+), 155 deletions(-)
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 7170f97..3b2b249 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -122,7 +122,9 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
BUG_ON(skb->dev != dev);
/* Drop the packet if vif is not ready */
- if (vif->task == NULL || !xenvif_schedulable(vif))
+ if (vif->task == NULL ||
+ vif->dealloc_task == NULL ||
+ !xenvif_schedulable(vif))
goto drop;
/* At best we'll need one slot for the header and one for each
@@ -345,8 +347,26 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
vif->pending_prod = MAX_PENDING_REQS;
for (i = 0; i < MAX_PENDING_REQS; i++)
vif->pending_ring[i] = i;
- for (i = 0; i < MAX_PENDING_REQS; i++)
- vif->mmap_pages[i] = NULL;
+ spin_lock_init(&vif->dealloc_lock);
+ spin_lock_init(&vif->response_lock);
+ /* If ballooning is disabled, this will consume real memory, so you
+ * better enable it. The long term solution would be to use just a
+ * bunch of valid page descriptors, without dependency on ballooning
+ */
+ err = alloc_xenballooned_pages(MAX_PENDING_REQS,
+ vif->mmap_pages,
+ false);
+ if (err) {
+ netdev_err(dev, "Could not reserve mmap_pages\n");
+ return NULL;
+ }
+ for (i = 0; i < MAX_PENDING_REQS; i++) {
+ vif->pending_tx_info[i].callback_struct = (struct ubuf_info)
+ { .callback = xenvif_zerocopy_callback,
+ .ctx = NULL,
+ .desc = i };
+ vif->grant_tx_handle[i] = NETBACK_INVALID_HANDLE;
+ }
/*
* Initialise a dummy MAC address. We choose the numerically
@@ -390,6 +410,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
goto err;
init_waitqueue_head(&vif->wq);
+ init_waitqueue_head(&vif->dealloc_wq);
if (tx_evtchn == rx_evtchn) {
/* feature-split-event-channels == 0 */
@@ -431,6 +452,14 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
goto err_rx_unbind;
}
+ vif->dealloc_task = kthread_create(xenvif_dealloc_kthread,
+ (void *)vif, "%s-dealloc", vif->dev->name);
+ if (IS_ERR(vif->dealloc_task)) {
+ pr_warn("Could not allocate kthread for %s\n", vif->dev->name);
+ err = PTR_ERR(vif->dealloc_task);
+ goto err_rx_unbind;
+ }
+
vif->task = task;
rtnl_lock();
@@ -443,6 +472,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
rtnl_unlock();
wake_up_process(vif->task);
+ wake_up_process(vif->dealloc_task);
return 0;
@@ -480,6 +510,11 @@ void xenvif_disconnect(struct xenvif *vif)
vif->task = NULL;
}
+ if (vif->dealloc_task) {
+ kthread_stop(vif->dealloc_task);
+ vif->dealloc_task = NULL;
+ }
+
if (vif->tx_irq) {
if (vif->tx_irq == vif->rx_irq)
unbind_from_irqhandler(vif->tx_irq, vif);
@@ -495,6 +530,22 @@ void xenvif_disconnect(struct xenvif *vif)
void xenvif_free(struct xenvif *vif)
{
+ int i, unmap_timeout = 0;
+
+ for (i = 0; i < MAX_PENDING_REQS; ++i) {
+ if (vif->grant_tx_handle[i] != NETBACK_INVALID_HANDLE) {
+ unmap_timeout++;
+ schedule_timeout(msecs_to_jiffies(1000));
+ if (unmap_timeout > 9 &&
+ net_ratelimit())
+ netdev_err(vif->dev,
+ "Page still granted! Index: %x\n", i);
+ i = -1;
+ }
+ }
+
+ free_xenballooned_pages(MAX_PENDING_REQS, vif->mmap_pages);
+
netif_napi_del(&vif->napi);
unregister_netdev(vif->dev);
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 7c241f9..53d7e78 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -644,9 +644,12 @@ static void xenvif_tx_err(struct xenvif *vif,
struct xen_netif_tx_request *txp, RING_IDX end)
{
RING_IDX cons = vif->tx.req_cons;
+ unsigned long flags;
do {
+ spin_lock_irqsave(&vif->response_lock, flags);
make_tx_response(vif, txp, XEN_NETIF_RSP_ERROR);
+ spin_unlock_irqrestore(&vif->response_lock, flags);
if (cons == end)
break;
txp = RING_GET_REQUEST(&vif->tx, cons++);
@@ -784,10 +787,10 @@ static inline void xenvif_tx_create_gop(struct xenvif *vif, u16 pending_idx,
}
-static struct gnttab_copy *xenvif_get_requests(struct xenvif *vif,
+static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
struct sk_buff *skb,
struct xen_netif_tx_request *txp,
- struct gnttab_copy *gop)
+ struct gnttab_map_grant_ref *gop)
{
struct skb_shared_info *shinfo = skb_shinfo(skb);
skb_frag_t *frags = shinfo->frags;
@@ -808,83 +811,12 @@ static struct gnttab_copy *xenvif_get_requests(struct xenvif *vif,
/* Skip first skb fragment if it is on same page as header fragment. */
start = (frag_get_pending_idx(&shinfo->frags[0]) == pending_idx);
- /* Coalesce tx requests, at this point the packet passed in
- * should be <= 64K. Any packets larger than 64K have been
- * handled in xenvif_count_requests().
- */
- for (shinfo->nr_frags = slot = start; slot < nr_slots;
- shinfo->nr_frags++) {
- struct pending_tx_info *pending_tx_info =
- vif->pending_tx_info;
-
- page = alloc_page(GFP_ATOMIC|__GFP_COLD);
- if (!page)
- goto err;
-
- dst_offset = 0;
- first = NULL;
- while (dst_offset < PAGE_SIZE && slot < nr_slots) {
- gop->flags = GNTCOPY_source_gref;
-
- gop->source.u.ref = txp->gref;
- gop->source.domid = vif->domid;
- gop->source.offset = txp->offset;
-
- gop->dest.domid = DOMID_SELF;
-
- gop->dest.offset = dst_offset;
- gop->dest.u.gmfn = virt_to_mfn(page_address(page));
-
- if (dst_offset + txp->size > PAGE_SIZE) {
- /* This page can only merge a portion
- * of tx request. Do not increment any
- * pointer / counter here. The txp
- * will be dealt with in future
- * rounds, eventually hitting the
- * `else` branch.
- */
- gop->len = PAGE_SIZE - dst_offset;
- txp->offset += gop->len;
- txp->size -= gop->len;
- dst_offset += gop->len; /* quit loop */
- } else {
- /* This tx request can be merged in the page */
- gop->len = txp->size;
- dst_offset += gop->len;
-
+ for (shinfo->nr_frags = start; shinfo->nr_frags < nr_slots;
+ shinfo->nr_frags++, txp++, gop++) {
index = pending_index(vif->pending_cons++);
-
pending_idx = vif->pending_ring[index];
-
- memcpy(&pending_tx_info[pending_idx].req, txp,
- sizeof(*txp));
-
- /* Poison these fields, corresponding
- * fields for head tx req will be set
- * to correct values after the loop.
- */
- vif->mmap_pages[pending_idx] = (void *)(~0UL);
- pending_tx_info[pending_idx].head =
- INVALID_PENDING_RING_IDX;
-
- if (!first) {
- first = &pending_tx_info[pending_idx];
- start_idx = index;
- head_idx = pending_idx;
- }
-
- txp++;
- slot++;
- }
-
- gop++;
- }
-
- first->req.offset = 0;
- first->req.size = dst_offset;
- first->head = start_idx;
- vif->mmap_pages[head_idx] = page;
- frag_set_pending_idx(&frags[shinfo->nr_frags], head_idx);
+ xenvif_tx_create_gop(vif, pending_idx, txp, gop);
+ frag_set_pending_idx(&frags[shinfo->nr_frags], pending_idx);
}
BUG_ON(shinfo->nr_frags > MAX_SKB_FRAGS);
@@ -906,9 +838,9 @@ err:
static int xenvif_tx_check_gop(struct xenvif *vif,
struct sk_buff *skb,
- struct gnttab_copy **gopp)
+ struct gnttab_map_grant_ref **gopp)
{
- struct gnttab_copy *gop = *gopp;
+ struct gnttab_map_grant_ref *gop = *gopp;
u16 pending_idx = *((u16 *)skb->data);
struct skb_shared_info *shinfo = skb_shinfo(skb);
struct pending_tx_info *tx_info;
@@ -920,6 +852,18 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
err = gop->status;
if (unlikely(err))
xenvif_idx_release(vif, pending_idx, XEN_NETIF_RSP_ERROR);
+ else {
+ if (vif->grant_tx_handle[pending_idx] !=
+ NETBACK_INVALID_HANDLE) {
+ netdev_err(vif->dev,
+ "Stale mapped handle! pending_idx %x handle %x\n",
+ pending_idx, vif->grant_tx_handle[pending_idx]);
+ BUG();
+ }
+ set_phys_to_machine(idx_to_pfn(vif, pending_idx),
+ FOREIGN_FRAME(gop->dev_bus_addr >> PAGE_SHIFT));
+ vif->grant_tx_handle[pending_idx] = gop->handle;
+ }
/* Skip first skb fragment if it is on same page as header fragment. */
start = (frag_get_pending_idx(&shinfo->frags[0]) == pending_idx);
@@ -933,18 +877,26 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
head = tx_info->head;
/* Check error status: if okay then remember grant handle. */
- do {
newerr = (++gop)->status;
- if (newerr)
- break;
- peek = vif->pending_ring[pending_index(++head)];
- } while (!pending_tx_is_head(vif, peek));
if (likely(!newerr)) {
+ if (vif->grant_tx_handle[pending_idx] !=
+ NETBACK_INVALID_HANDLE) {
+ netdev_err(vif->dev,
+ "Stale mapped handle! pending_idx %x handle %x\n",
+ pending_idx,
+ vif->grant_tx_handle[pending_idx]);
+ xenvif_fatal_tx_err(vif);
+ }
+ set_phys_to_machine(idx_to_pfn(vif, pending_idx),
+ FOREIGN_FRAME(gop->dev_bus_addr >> PAGE_SHIFT));
+ vif->grant_tx_handle[pending_idx] = gop->handle;
/* Had a previous error? Invalidate this fragment. */
- if (unlikely(err))
+ if (unlikely(err)) {
+ xenvif_idx_unmap(vif, pending_idx);
xenvif_idx_release(vif, pending_idx,
XEN_NETIF_RSP_OKAY);
+ }
continue;
}
@@ -957,9 +909,11 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
/* First error: invalidate header and preceding fragments. */
pending_idx = *((u16 *)skb->data);
+ xenvif_idx_unmap(vif, pending_idx);
xenvif_idx_release(vif, pending_idx, XEN_NETIF_RSP_OKAY);
for (j = start; j < i; j++) {
pending_idx = frag_get_pending_idx(&shinfo->frags[j]);
+ xenvif_idx_unmap(vif, pending_idx);
xenvif_idx_release(vif, pending_idx,
XEN_NETIF_RSP_OKAY);
}
@@ -972,7 +926,8 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
return err;
}
-static void xenvif_fill_frags(struct xenvif *vif, struct sk_buff *skb)
+static void xenvif_fill_frags(struct xenvif *vif, struct sk_buff *skb,
+ u16 prev_pending_idx)
{
struct skb_shared_info *shinfo = skb_shinfo(skb);
int nr_frags = shinfo->nr_frags;
@@ -986,6 +941,17 @@ static void xenvif_fill_frags(struct xenvif *vif, struct sk_buff *skb)
pending_idx = frag_get_pending_idx(frag);
+ /* If this is not the first frag, chain it to the previous*/
+ if (unlikely(prev_pending_idx == INVALID_PENDING_IDX))
+ skb_shinfo(skb)->destructor_arg =
+ &vif->pending_tx_info[pending_idx].callback_struct;
+ else if (likely(pending_idx != prev_pending_idx))
+ vif->pending_tx_info[prev_pending_idx].callback_struct.ctx =
+ &(vif->pending_tx_info[pending_idx].callback_struct);
+
+ vif->pending_tx_info[pending_idx].callback_struct.ctx = NULL;
+ prev_pending_idx = pending_idx;
+
txp = &vif->pending_tx_info[pending_idx].req;
page = virt_to_page(idx_to_kaddr(vif, pending_idx));
__skb_fill_page_desc(skb, i, page, txp->offset, txp->size);
@@ -993,10 +959,15 @@ static void xenvif_fill_frags(struct xenvif *vif, struct sk_buff *skb)
skb->data_len += txp->size;
skb->truesize += txp->size;
- /* Take an extra reference to offset xenvif_idx_release */
+ /* Take an extra reference to offset network stack's put_page */
get_page(vif->mmap_pages[pending_idx]);
- xenvif_idx_release(vif, pending_idx, XEN_NETIF_RSP_OKAY);
}
+ /* FIXME: __skb_fill_page_desc set this to true because page->pfmemalloc
+ * overlaps with "index", and "mapping" is not set. I think mapping
+ * should be set. If delivered to local stack, it would drop this
+ * skb in sk_filter unless the socket has the right to use it.
+ */
+ skb->pfmemalloc = false;
}
static int xenvif_get_extras(struct xenvif *vif,
@@ -1358,7 +1329,7 @@ static bool tx_credit_exceeded(struct xenvif *vif, unsigned size)
static unsigned xenvif_tx_build_gops(struct xenvif *vif, int budget)
{
- struct gnttab_copy *gop = vif->tx_copy_ops, *request_gop;
+ struct gnttab_map_grant_ref *gop = vif->tx_map_ops, *request_gop;
struct sk_buff *skb;
int ret;
@@ -1466,30 +1437,10 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif, int budget)
}
}
- /* XXX could copy straight to head */
- page = xenvif_alloc_page(vif, pending_idx);
- if (!page) {
- kfree_skb(skb);
- xenvif_tx_err(vif, &txreq, idx);
- break;
- }
-
- gop->source.u.ref = txreq.gref;
- gop->source.domid = vif->domid;
- gop->source.offset = txreq.offset;
-
- gop->dest.u.gmfn = virt_to_mfn(page_address(page));
- gop->dest.domid = DOMID_SELF;
- gop->dest.offset = txreq.offset;
-
- gop->len = txreq.size;
- gop->flags = GNTCOPY_source_gref;
+ xenvif_tx_create_gop(vif, pending_idx, &txreq, gop);
gop++;
- memcpy(&vif->pending_tx_info[pending_idx].req,
- &txreq, sizeof(txreq));
- vif->pending_tx_info[pending_idx].head = index;
*((u16 *)skb->data) = pending_idx;
__skb_put(skb, data_len);
@@ -1518,17 +1469,17 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif, int budget)
vif->tx.req_cons = idx;
- if ((gop-vif->tx_copy_ops) >= ARRAY_SIZE(vif->tx_copy_ops))
+ if ((gop-vif->tx_map_ops) >= ARRAY_SIZE(vif->tx_map_ops))
break;
}
- return gop - vif->tx_copy_ops;
+ return gop - vif->tx_map_ops;
}
static int xenvif_tx_submit(struct xenvif *vif)
{
- struct gnttab_copy *gop = vif->tx_copy_ops;
+ struct gnttab_map_grant_ref *gop = vif->tx_map_ops;
struct sk_buff *skb;
int work_done = 0;
@@ -1552,12 +1503,17 @@ static int xenvif_tx_submit(struct xenvif *vif)
memcpy(skb->data,
(void *)(idx_to_kaddr(vif, pending_idx)|txp->offset),
data_len);
+ vif->pending_tx_info[pending_idx].callback_struct.ctx = NULL;
if (data_len < txp->size) {
/* Append the packet payload as a fragment. */
txp->offset += data_len;
txp->size -= data_len;
+ skb_shinfo(skb)->destructor_arg =
+ &vif->pending_tx_info[pending_idx].callback_struct;
} else {
/* Schedule a response immediately. */
+ skb_shinfo(skb)->destructor_arg = NULL;
+ xenvif_idx_unmap(vif, pending_idx);
xenvif_idx_release(vif, pending_idx,
XEN_NETIF_RSP_OKAY);
}
@@ -1567,7 +1523,11 @@ static int xenvif_tx_submit(struct xenvif *vif)
else if (txp->flags & XEN_NETTXF_data_validated)
skb->ip_summed = CHECKSUM_UNNECESSARY;
- xenvif_fill_frags(vif, skb);
+ xenvif_fill_frags(vif,
+ skb,
+ skb_shinfo(skb)->destructor_arg ?
+ pending_idx :
+ INVALID_PENDING_IDX);
if (skb_is_nonlinear(skb) && skb_headlen(skb) < PKT_PROT_LEN) {
int target = min_t(int, skb->len, PKT_PROT_LEN);
@@ -1581,6 +1541,8 @@ static int xenvif_tx_submit(struct xenvif *vif)
if (checksum_setup(vif, skb)) {
netdev_dbg(vif->dev,
"Can't setup checksum in net_tx_action\n");
+ if (skb_shinfo(skb)->destructor_arg)
+ skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
kfree_skb(skb);
continue;
}
@@ -1606,6 +1568,14 @@ static int xenvif_tx_submit(struct xenvif *vif)
work_done++;
+ /* Set this flag right before netif_receive_skb, otherwise
+ * someone might think this packet already left netback, and
+ * do a skb_copy_ubufs while we are still in control of the
+ * skb. E.g. the __pskb_pull_tail earlier can do such thing.
+ */
+ if (skb_shinfo(skb)->destructor_arg)
+ skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
+
netif_receive_skb(skb);
}
@@ -1715,7 +1685,7 @@ static inline void xenvif_tx_dealloc_action(struct xenvif *vif)
int xenvif_tx_action(struct xenvif *vif, int budget)
{
unsigned nr_gops;
- int work_done;
+ int work_done, ret;
if (unlikely(!tx_work_todo(vif)))
return 0;
@@ -1725,7 +1695,10 @@ int xenvif_tx_action(struct xenvif *vif, int budget)
if (nr_gops == 0)
return 0;
- gnttab_batch_copy(vif->tx_copy_ops, nr_gops);
+ ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
+ vif->tx_map_ops,
+ nr_gops);
+ BUG_ON(ret);
work_done = xenvif_tx_submit(vif);
@@ -1736,45 +1709,19 @@ static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
u8 status)
{
struct pending_tx_info *pending_tx_info;
- pending_ring_idx_t head;
+ pending_ring_idx_t index;
u16 peek; /* peek into next tx request */
+ unsigned long flags;
- BUG_ON(vif->mmap_pages[pending_idx] == (void *)(~0UL));
-
- /* Already complete? */
- if (vif->mmap_pages[pending_idx] == NULL)
- return;
-
- pending_tx_info = &vif->pending_tx_info[pending_idx];
-
- head = pending_tx_info->head;
-
- BUG_ON(!pending_tx_is_head(vif, head));
- BUG_ON(vif->pending_ring[pending_index(head)] != pending_idx);
-
- do {
- pending_ring_idx_t index;
- pending_ring_idx_t idx = pending_index(head);
- u16 info_idx = vif->pending_ring[idx];
-
- pending_tx_info = &vif->pending_tx_info[info_idx];
+ pending_tx_info = &vif->pending_tx_info[pending_idx];
+ spin_lock_irqsave(&vif->response_lock, flags);
make_tx_response(vif, &pending_tx_info->req, status);
-
- /* Setting any number other than
- * INVALID_PENDING_RING_IDX indicates this slot is
- * starting a new packet / ending a previous packet.
- */
- pending_tx_info->head = 0;
-
- index = pending_index(vif->pending_prod++);
- vif->pending_ring[index] = vif->pending_ring[info_idx];
-
- peek = vif->pending_ring[pending_index(++head)];
-
- } while (!pending_tx_is_head(vif, peek));
-
- put_page(vif->mmap_pages[pending_idx]);
- vif->mmap_pages[pending_idx] = NULL;
+ index = pending_index(vif->pending_prod);
+ vif->pending_ring[index] = pending_idx;
+ /* TX shouldn't use the index before we give it back here */
+ mb();
+ vif->pending_prod++;
+ spin_unlock_irqrestore(&vif->response_lock, flags);
}
void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
^ permalink raw reply related
* [PATCH net-next v3 3/9] xen-netback: Remove old TX grant copy definitons and fix indentations
From: Zoltan Kiss @ 2014-01-08 0:10 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1389139818-24458-1-git-send-email-zoltan.kiss@citrix.com>
These became obsolate with grant mapping. I've left intentionally the
indentations in this way, to improve readability of previous patches.
v2:
- move the indentation fixup patch here
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/common.h | 37 +------------------
drivers/net/xen-netback/netback.c | 72 ++++++++-----------------------------
2 files changed, 15 insertions(+), 94 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 33cb12c..f286879 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -46,39 +46,9 @@
#include <xen/xenbus.h>
typedef unsigned int pending_ring_idx_t;
-#define INVALID_PENDING_RING_IDX (~0U)
-/* For the head field in pending_tx_info: it is used to indicate
- * whether this tx info is the head of one or more coalesced requests.
- *
- * When head != INVALID_PENDING_RING_IDX, it means the start of a new
- * tx requests queue and the end of previous queue.
- *
- * An example sequence of head fields (I = INVALID_PENDING_RING_IDX):
- *
- * ...|0 I I I|5 I|9 I I I|...
- * -->|<-INUSE----------------
- *
- * After consuming the first slot(s) we have:
- *
- * ...|V V V V|5 I|9 I I I|...
- * -----FREE->|<-INUSE--------
- *
- * where V stands for "valid pending ring index". Any number other
- * than INVALID_PENDING_RING_IDX is OK. These entries are considered
- * free and can contain any number other than
- * INVALID_PENDING_RING_IDX. In practice we use 0.
- *
- * The in use non-INVALID_PENDING_RING_IDX (say 0, 5 and 9 in the
- * above example) number is the index into pending_tx_info and
- * mmap_pages arrays.
- */
struct pending_tx_info {
- struct xen_netif_tx_request req; /* coalesced tx request */
- pending_ring_idx_t head; /* head != INVALID_PENDING_RING_IDX
- * if it is head of one or more tx
- * reqs
- */
+ struct xen_netif_tx_request req; /* tx request */
/* callback data for released SKBs. The callback is always
* xenvif_zerocopy_callback, ctx points to the next fragment, desc
* contains the pending_idx
@@ -128,11 +98,6 @@ struct xenvif {
struct pending_tx_info pending_tx_info[MAX_PENDING_REQS];
grant_handle_t grant_tx_handle[MAX_PENDING_REQS];
- /* Coalescing tx requests before copying makes number of grant
- * copy ops greater or equal to number of slots required. In
- * worst case a tx request consumes 2 gnttab_copy.
- */
- struct gnttab_copy tx_copy_ops[2*MAX_PENDING_REQS];
struct gnttab_map_grant_ref tx_map_ops[MAX_PENDING_REQS];
struct gnttab_unmap_grant_ref tx_unmap_ops[MAX_PENDING_REQS];
/* passed to gnttab_[un]map_refs with pages under (un)mapping */
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 20352be..88a0fad 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -71,16 +71,6 @@ module_param(fatal_skb_slots, uint, 0444);
*/
#define XEN_NETBK_LEGACY_SLOTS_MAX XEN_NETIF_NR_SLOTS_MIN
-/*
- * If head != INVALID_PENDING_RING_IDX, it means this tx request is head of
- * one or more merged tx requests, otherwise it is the continuation of
- * previous tx request.
- */
-static inline int pending_tx_is_head(struct xenvif *vif, RING_IDX idx)
-{
- return vif->pending_tx_info[idx].head != INVALID_PENDING_RING_IDX;
-}
-
static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
u8 status);
@@ -762,19 +752,6 @@ static int xenvif_count_requests(struct xenvif *vif,
return slots;
}
-static struct page *xenvif_alloc_page(struct xenvif *vif,
- u16 pending_idx)
-{
- struct page *page;
-
- page = alloc_page(GFP_ATOMIC|__GFP_COLD);
- if (!page)
- return NULL;
- vif->mmap_pages[pending_idx] = page;
-
- return page;
-}
-
static inline void xenvif_tx_create_gop(struct xenvif *vif, u16 pending_idx,
struct xen_netif_tx_request *txp,
struct gnttab_map_grant_ref *gop)
@@ -797,13 +774,9 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
struct skb_shared_info *shinfo = skb_shinfo(skb);
skb_frag_t *frags = shinfo->frags;
u16 pending_idx = *((u16 *)skb->data);
- u16 head_idx = 0;
- int slot, start;
- struct page *page;
- pending_ring_idx_t index, start_idx = 0;
- uint16_t dst_offset;
+ int start;
+ pending_ring_idx_t index;
unsigned int nr_slots;
- struct pending_tx_info *first = NULL;
/* At this point shinfo->nr_frags is in fact the number of
* slots, which can be as large as XEN_NETBK_LEGACY_SLOTS_MAX.
@@ -815,8 +788,8 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
for (shinfo->nr_frags = start; shinfo->nr_frags < nr_slots;
shinfo->nr_frags++, txp++, gop++) {
- index = pending_index(vif->pending_cons++);
- pending_idx = vif->pending_ring[index];
+ index = pending_index(vif->pending_cons++);
+ pending_idx = vif->pending_ring[index];
xenvif_tx_create_gop(vif, pending_idx, txp, gop);
frag_set_pending_idx(&frags[shinfo->nr_frags], pending_idx);
}
@@ -824,18 +797,6 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
BUG_ON(shinfo->nr_frags > MAX_SKB_FRAGS);
return gop;
-err:
- /* Unwind, freeing all pages and sending error responses. */
- while (shinfo->nr_frags-- > start) {
- xenvif_idx_release(vif,
- frag_get_pending_idx(&frags[shinfo->nr_frags]),
- XEN_NETIF_RSP_ERROR);
- }
- /* The head too, if necessary. */
- if (start)
- xenvif_idx_release(vif, pending_idx, XEN_NETIF_RSP_ERROR);
-
- return NULL;
}
static int xenvif_tx_check_gop(struct xenvif *vif,
@@ -848,7 +809,6 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
struct pending_tx_info *tx_info;
int nr_frags = shinfo->nr_frags;
int i, err, start;
- u16 peek; /* peek into next tx request */
/* Check status of header. */
err = gop->status;
@@ -870,14 +830,12 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
for (i = start; i < nr_frags; i++) {
int j, newerr;
- pending_ring_idx_t head;
pending_idx = frag_get_pending_idx(&shinfo->frags[i]);
tx_info = &vif->pending_tx_info[pending_idx];
- head = tx_info->head;
/* Check error status: if okay then remember grant handle. */
- newerr = (++gop)->status;
+ newerr = (++gop)->status;
if (likely(!newerr)) {
if (vif->grant_tx_handle[pending_idx] !=
@@ -1343,7 +1301,6 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif, int budget)
(skb_queue_len(&vif->tx_queue) < budget)) {
struct xen_netif_tx_request txreq;
struct xen_netif_tx_request txfrags[XEN_NETBK_LEGACY_SLOTS_MAX];
- struct page *page;
struct xen_netif_extra_info extras[XEN_NETIF_EXTRA_TYPE_MAX-1];
u16 pending_idx;
RING_IDX idx;
@@ -1705,18 +1662,17 @@ static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
{
struct pending_tx_info *pending_tx_info;
pending_ring_idx_t index;
- u16 peek; /* peek into next tx request */
unsigned long flags;
- pending_tx_info = &vif->pending_tx_info[pending_idx];
- spin_lock_irqsave(&vif->response_lock, flags);
- make_tx_response(vif, &pending_tx_info->req, status);
- index = pending_index(vif->pending_prod);
- vif->pending_ring[index] = pending_idx;
- /* TX shouldn't use the index before we give it back here */
- mb();
- vif->pending_prod++;
- spin_unlock_irqrestore(&vif->response_lock, flags);
+ pending_tx_info = &vif->pending_tx_info[pending_idx];
+ spin_lock_irqsave(&vif->response_lock, flags);
+ make_tx_response(vif, &pending_tx_info->req, status);
+ index = pending_index(vif->pending_prod);
+ vif->pending_ring[index] = pending_idx;
+ /* TX shouldn't use the index before we give it back here */
+ mb();
+ vif->pending_prod++;
+ spin_unlock_irqrestore(&vif->response_lock, flags);
}
void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
^ permalink raw reply related
* [PATCH net-next v3 4/9] xen-netback: Change RX path for mapped SKB fragments
From: Zoltan Kiss @ 2014-01-08 0:10 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1389139818-24458-1-git-send-email-zoltan.kiss@citrix.com>
RX path need to know if the SKB fragments are stored on pages from another
domain.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/netback.c | 46 +++++++++++++++++++++++++++++++++----
1 file changed, 41 insertions(+), 5 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 10d0cf0..e070475 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -322,7 +322,9 @@ static struct xenvif_rx_meta *get_next_rx_buffer(struct xenvif *vif,
static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
struct netrx_pending_operations *npo,
struct page *page, unsigned long size,
- unsigned long offset, int *head)
+ unsigned long offset, int *head,
+ struct xenvif *foreign_vif,
+ grant_ref_t foreign_gref)
{
struct gnttab_copy *copy_gop;
struct xenvif_rx_meta *meta;
@@ -364,8 +366,15 @@ static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
copy_gop->flags = GNTCOPY_dest_gref;
copy_gop->len = bytes;
- copy_gop->source.domid = DOMID_SELF;
- copy_gop->source.u.gmfn = virt_to_mfn(page_address(page));
+ if (foreign_vif) {
+ copy_gop->source.domid = foreign_vif->domid;
+ copy_gop->source.u.ref = foreign_gref;
+ copy_gop->flags |= GNTCOPY_source_gref;
+ } else {
+ copy_gop->source.domid = DOMID_SELF;
+ copy_gop->source.u.gmfn =
+ virt_to_mfn(page_address(page));
+ }
copy_gop->source.offset = offset;
copy_gop->dest.domid = vif->domid;
@@ -426,6 +435,9 @@ static int xenvif_gop_skb(struct sk_buff *skb,
int old_meta_prod;
int gso_type;
int gso_size;
+ struct ubuf_info *ubuf = skb_shinfo(skb)->destructor_arg;
+ grant_ref_t foreign_grefs[MAX_SKB_FRAGS];
+ struct xenvif *foreign_vif = NULL;
old_meta_prod = npo->meta_prod;
@@ -466,6 +478,26 @@ static int xenvif_gop_skb(struct sk_buff *skb,
npo->copy_off = 0;
npo->copy_gref = req->gref;
+ if ((skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) &&
+ (ubuf->callback == &xenvif_zerocopy_callback)) {
+ u16 pending_idx = ubuf->desc;
+ int i = 0;
+ struct pending_tx_info *temp =
+ container_of(ubuf,
+ struct pending_tx_info,
+ callback_struct);
+ foreign_vif =
+ container_of(temp - pending_idx,
+ struct xenvif,
+ pending_tx_info[0]);
+ do {
+ pending_idx = ubuf->desc;
+ foreign_grefs[i++] =
+ foreign_vif->pending_tx_info[pending_idx].req.gref;
+ ubuf = (struct ubuf_info *) ubuf->ctx;
+ } while (ubuf);
+ }
+
data = skb->data;
while (data < skb_tail_pointer(skb)) {
unsigned int offset = offset_in_page(data);
@@ -475,7 +507,9 @@ static int xenvif_gop_skb(struct sk_buff *skb,
len = skb_tail_pointer(skb) - data;
xenvif_gop_frag_copy(vif, skb, npo,
- virt_to_page(data), len, offset, &head);
+ virt_to_page(data), len, offset, &head,
+ NULL,
+ 0);
data += len;
}
@@ -484,7 +518,9 @@ static int xenvif_gop_skb(struct sk_buff *skb,
skb_frag_page(&skb_shinfo(skb)->frags[i]),
skb_frag_size(&skb_shinfo(skb)->frags[i]),
skb_shinfo(skb)->frags[i].page_offset,
- &head);
+ &head,
+ foreign_vif,
+ foreign_grefs[i]);
}
return npo->meta_prod - old_meta_prod;
^ permalink raw reply related
* [PATCH net-next v3 5/9] xen-netback: Add stat counters for zerocopy
From: Zoltan Kiss @ 2014-01-08 0:10 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1389139818-24458-1-git-send-email-zoltan.kiss@citrix.com>
These counters help determine how often the buffers had to be copied. Also
they help find out if packets are leaked, as if "sent != success + fail",
there are probably packets never freed up properly.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/common.h | 3 +++
drivers/net/xen-netback/interface.c | 15 +++++++++++++++
drivers/net/xen-netback/netback.c | 9 ++++++++-
3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 419e63c..e3c28ff 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -155,6 +155,9 @@ struct xenvif {
/* Statistics */
unsigned long rx_gso_checksum_fixup;
+ unsigned long tx_zerocopy_sent;
+ unsigned long tx_zerocopy_success;
+ unsigned long tx_zerocopy_fail;
/* Miscellaneous private stuff. */
struct net_device *dev;
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index af5216f..75fe683 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -239,6 +239,21 @@ static const struct xenvif_stat {
"rx_gso_checksum_fixup",
offsetof(struct xenvif, rx_gso_checksum_fixup)
},
+ /* If (sent != success + fail), there are probably packets never
+ * freed up properly!
+ */
+ {
+ "tx_zerocopy_sent",
+ offsetof(struct xenvif, tx_zerocopy_sent),
+ },
+ {
+ "tx_zerocopy_success",
+ offsetof(struct xenvif, tx_zerocopy_success),
+ },
+ {
+ "tx_zerocopy_fail",
+ offsetof(struct xenvif, tx_zerocopy_fail)
+ },
};
static int xenvif_get_sset_count(struct net_device *dev, int string_set)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index a1b03e4..e2dd565 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1611,8 +1611,10 @@ static int xenvif_tx_submit(struct xenvif *vif, int budget)
* skb_copy_ubufs while we are still in control of the skb. E.g.
* the __pskb_pull_tail earlier can do such thing.
*/
- if (skb_shinfo(skb)->destructor_arg)
+ if (skb_shinfo(skb)->destructor_arg) {
skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
+ vif->tx_zerocopy_sent++;
+ }
netif_receive_skb(skb);
}
@@ -1645,6 +1647,11 @@ void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success)
napi_schedule(&vif->napi);
} while (ubuf);
spin_unlock_irqrestore(&vif->dealloc_lock, flags);
+
+ if (likely(zerocopy_success))
+ vif->tx_zerocopy_success++;
+ else
+ vif->tx_zerocopy_fail++;
}
static inline void xenvif_tx_action_dealloc(struct xenvif *vif)
^ permalink raw reply related
* [PATCH net-next v3 6/9] xen-netback: Handle guests with too many frags
From: Zoltan Kiss @ 2014-01-08 0:10 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1389139818-24458-1-git-send-email-zoltan.kiss@citrix.com>
Xen network protocol had implicit dependency on MAX_SKB_FRAGS. Netback has to
handle guests sending up to XEN_NETBK_LEGACY_SLOTS_MAX slots. To achieve that:
- create a new skb
- map the leftover slots to its frags (no linear buffer here!)
- chain it to the previous through skb_shinfo(skb)->frag_list
- map them
- copy the whole stuff into a brand new skb and send it to the stack
- unmap the 2 old skb's pages
v3:
- adding extra check for frag number
- consolidate alloc_skb's into xenvif_alloc_skb()
- BUG_ON(frag_overflow > MAX_SKB_FRAGS)
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/netback.c | 115 +++++++++++++++++++++++++++++++++----
1 file changed, 105 insertions(+), 10 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index ea1e27d..3796cb3 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -800,6 +800,19 @@ static inline void xenvif_tx_create_gop(struct xenvif *vif, u16 pending_idx,
}
+static inline struct sk_buff *xenvif_alloc_skb(unsigned int size)
+{
+ struct sk_buff *skb = alloc_skb(size + NET_SKB_PAD + NET_IP_ALIGN,
+ GFP_ATOMIC | __GFP_NOWARN);
+ if (unlikely(skb == NULL))
+ return NULL;
+
+ /* Packets passed to netif_rx() must have some headroom. */
+ skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
+
+ return skb;
+}
+
static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
struct sk_buff *skb,
struct xen_netif_tx_request *txp,
@@ -810,11 +823,16 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
u16 pending_idx = *((u16 *)skb->data);
int start;
pending_ring_idx_t index;
- unsigned int nr_slots;
+ unsigned int nr_slots, frag_overflow = 0;
/* At this point shinfo->nr_frags is in fact the number of
* slots, which can be as large as XEN_NETBK_LEGACY_SLOTS_MAX.
*/
+ if (shinfo->nr_frags > MAX_SKB_FRAGS) {
+ frag_overflow = shinfo->nr_frags - MAX_SKB_FRAGS;
+ BUG_ON(frag_overflow > MAX_SKB_FRAGS);
+ shinfo->nr_frags = MAX_SKB_FRAGS;
+ }
nr_slots = shinfo->nr_frags;
/* Skip first skb fragment if it is on same page as header fragment. */
@@ -830,6 +848,29 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
BUG_ON(shinfo->nr_frags > MAX_SKB_FRAGS);
+ if (frag_overflow) {
+ struct sk_buff *nskb = xenvif_alloc_skb(0);
+ if (unlikely(nskb == NULL)) {
+ netdev_err(vif->dev,
+ "Can't allocate the frag_list skb.\n");
+ return NULL;
+ }
+
+ shinfo = skb_shinfo(nskb);
+ frags = shinfo->frags;
+
+ for (shinfo->nr_frags = 0; shinfo->nr_frags < frag_overflow;
+ shinfo->nr_frags++, txp++, gop++) {
+ index = pending_index(vif->pending_cons++);
+ pending_idx = vif->pending_ring[index];
+ xenvif_tx_create_gop(vif, pending_idx, txp, gop);
+ frag_set_pending_idx(&frags[shinfo->nr_frags],
+ pending_idx);
+ }
+
+ skb_shinfo(skb)->frag_list = nskb;
+ }
+
return gop;
}
@@ -843,6 +884,7 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
struct pending_tx_info *tx_info;
int nr_frags = shinfo->nr_frags;
int i, err, start;
+ struct sk_buff *first_skb = NULL;
/* Check status of header. */
err = gop->status;
@@ -862,6 +904,7 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
/* Skip first skb fragment if it is on same page as header fragment. */
start = (frag_get_pending_idx(&shinfo->frags[0]) == pending_idx);
+check_frags:
for (i = start; i < nr_frags; i++) {
int j, newerr;
@@ -896,11 +939,20 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
/* Not the first error? Preceding frags already invalidated. */
if (err)
continue;
-
/* First error: invalidate header and preceding fragments. */
- pending_idx = *((u16 *)skb->data);
- xenvif_idx_unmap(vif, pending_idx);
- xenvif_idx_release(vif, pending_idx, XEN_NETIF_RSP_OKAY);
+ if (!first_skb) {
+ pending_idx = *((u16 *)skb->data);
+ xenvif_idx_unmap(vif, pending_idx);
+ xenvif_idx_release(vif,
+ pending_idx,
+ XEN_NETIF_RSP_OKAY);
+ } else {
+ pending_idx = *((u16 *)first_skb->data);
+ xenvif_idx_unmap(vif, pending_idx);
+ xenvif_idx_release(vif,
+ pending_idx,
+ XEN_NETIF_RSP_OKAY);
+ }
for (j = start; j < i; j++) {
pending_idx = frag_get_pending_idx(&shinfo->frags[j]);
xenvif_idx_unmap(vif, pending_idx);
@@ -912,6 +964,32 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
err = newerr;
}
+ if (shinfo->frag_list) {
+ first_skb = skb;
+ skb = shinfo->frag_list;
+ shinfo = skb_shinfo(skb);
+ nr_frags = shinfo->nr_frags;
+ start = 0;
+
+ goto check_frags;
+ }
+
+ /* There was a mapping error in the frag_list skb. We have to unmap
+ * the first skb's frags
+ */
+ if (first_skb && err) {
+ int j;
+ shinfo = skb_shinfo(first_skb);
+ pending_idx = *((u16 *)first_skb->data);
+ start = (frag_get_pending_idx(&shinfo->frags[0]) == pending_idx);
+ for (j = start; j < shinfo->nr_frags; j++) {
+ pending_idx = frag_get_pending_idx(&shinfo->frags[j]);
+ xenvif_idx_unmap(vif, pending_idx);
+ xenvif_idx_release(vif, pending_idx,
+ XEN_NETIF_RSP_OKAY);
+ }
+ }
+
*gopp = gop + 1;
return err;
}
@@ -1403,8 +1481,7 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif, int budget)
ret < XEN_NETBK_LEGACY_SLOTS_MAX) ?
PKT_PROT_LEN : txreq.size;
- skb = alloc_skb(data_len + NET_SKB_PAD + NET_IP_ALIGN,
- GFP_ATOMIC | __GFP_NOWARN);
+ skb = xenvif_alloc_skb(data_len);
if (unlikely(skb == NULL)) {
netdev_dbg(vif->dev,
"Can't allocate a skb in start_xmit.\n");
@@ -1412,9 +1489,6 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif, int budget)
break;
}
- /* Packets passed to netif_rx() must have some headroom. */
- skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
-
if (extras[XEN_NETIF_EXTRA_TYPE_GSO - 1].type) {
struct xen_netif_extra_info *gso;
gso = &extras[XEN_NETIF_EXTRA_TYPE_GSO - 1];
@@ -1476,6 +1550,7 @@ static int xenvif_tx_submit(struct xenvif *vif)
struct xen_netif_tx_request *txp;
u16 pending_idx;
unsigned data_len;
+ struct sk_buff *nskb = NULL;
pending_idx = *((u16 *)skb->data);
txp = &vif->pending_tx_info[pending_idx].req;
@@ -1518,6 +1593,23 @@ static int xenvif_tx_submit(struct xenvif *vif)
pending_idx :
INVALID_PENDING_IDX);
+ if (skb_shinfo(skb)->frag_list) {
+ nskb = skb_shinfo(skb)->frag_list;
+ xenvif_fill_frags(vif, nskb, INVALID_PENDING_IDX);
+ skb->len += nskb->len;
+ skb->data_len += nskb->len;
+ skb->truesize += nskb->truesize;
+ skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
+ skb_shinfo(nskb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
+ vif->tx_zerocopy_sent += 2;
+ nskb = skb;
+
+ skb = skb_copy_expand(skb,
+ 0,
+ 0,
+ GFP_ATOMIC | __GFP_NOWARN);
+ skb_shinfo(skb)->destructor_arg = NULL;
+ }
if (skb_is_nonlinear(skb) && skb_headlen(skb) < PKT_PROT_LEN) {
int target = min_t(int, skb->len, PKT_PROT_LEN);
__pskb_pull_tail(skb, target - skb_headlen(skb));
@@ -1568,6 +1660,9 @@ static int xenvif_tx_submit(struct xenvif *vif)
}
netif_receive_skb(skb);
+
+ if (nskb)
+ kfree_skb(nskb);
}
return work_done;
^ permalink raw reply related
* [PATCH net-next v3 9/9] xen-netback: Aggregate TX unmap operations
From: Zoltan Kiss @ 2014-01-08 0:10 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1389139818-24458-1-git-send-email-zoltan.kiss@citrix.com>
Unmapping causes TLB flushing, therefore we should make it in the largest
possible batches. However we shouldn't starve the guest for too long. So if
the guest has space for at least two big packets and we don't have at least a
quarter ring to unmap, delay it for at most 1 milisec.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/common.h | 2 ++
drivers/net/xen-netback/interface.c | 2 ++
drivers/net/xen-netback/netback.c | 31 ++++++++++++++++++++++++++++++-
3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 063fcda..55d1f14 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -115,6 +115,8 @@ struct xenvif {
u16 dealloc_ring[MAX_PENDING_REQS];
struct task_struct *dealloc_task;
wait_queue_head_t dealloc_wq;
+ struct timer_list dealloc_delay;
+ bool dealloc_delay_timed_out;
/* Use kthread for guest RX */
struct task_struct *task;
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index ce032f9..0287d62 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -406,6 +406,7 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
.desc = i };
vif->grant_tx_handle[i] = NETBACK_INVALID_HANDLE;
}
+ init_timer(&vif->dealloc_delay);
/*
* Initialise a dummy MAC address. We choose the numerically
@@ -551,6 +552,7 @@ void xenvif_disconnect(struct xenvif *vif)
}
if (vif->dealloc_task) {
+ del_timer_sync(&vif->dealloc_delay);
kthread_stop(vif->dealloc_task);
vif->dealloc_task = NULL;
}
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 6bc5413..27cc36c 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -134,6 +134,11 @@ static inline pending_ring_idx_t nr_pending_reqs(struct xenvif *vif)
vif->pending_prod + vif->pending_cons;
}
+static inline pending_ring_idx_t nr_free_slots(struct xen_netif_tx_back_ring *ring)
+{
+ return ring->nr_ents - (ring->sring->req_prod - ring->rsp_prod_pvt);
+}
+
bool xenvif_rx_ring_slots_available(struct xenvif *vif, int needed)
{
RING_IDX prod, cons;
@@ -1904,10 +1909,34 @@ static inline int tx_work_todo(struct xenvif *vif)
return 0;
}
+static void xenvif_dealloc_delay(unsigned long data)
+{
+ struct xenvif *vif = (struct xenvif *)data;
+
+ vif->dealloc_delay_timed_out = true;
+ wake_up(&vif->dealloc_wq);
+}
+
static inline int tx_dealloc_work_todo(struct xenvif *vif)
{
- if (vif->dealloc_cons != vif->dealloc_prod)
+ if (vif->dealloc_cons != vif->dealloc_prod) {
+ if ((nr_free_slots(&vif->tx) > 2 * XEN_NETBK_LEGACY_SLOTS_MAX) &&
+ (vif->dealloc_prod - vif->dealloc_cons < MAX_PENDING_REQS / 4) &&
+ !vif->dealloc_delay_timed_out) {
+ if (!timer_pending(&vif->dealloc_delay)) {
+ vif->dealloc_delay.function =
+ xenvif_dealloc_delay;
+ vif->dealloc_delay.data = (unsigned long)vif;
+ mod_timer(&vif->dealloc_delay,
+ jiffies + msecs_to_jiffies(1));
+
+ }
+ return 0;
+ }
+ del_timer_sync(&vif->dealloc_delay);
+ vif->dealloc_delay_timed_out = false;
return 1;
+ }
return 0;
}
^ permalink raw reply related
* Re: [PATCH net-next v3 1/3] ipv4: introduce ip_dst_mtu_secure and protect forwarding path against pmtu spoofing
From: David Miller @ 2014-01-08 0:13 UTC (permalink / raw)
To: hannes; +Cc: netdev, eric.dumazet, johnwheffner, steffen.klassert, fweimer
In-Reply-To: <20140106084827.GA5766@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Mon, 6 Jan 2014 09:48:27 +0100
> +ip_forward_use_pmtu - BOOLEAN
> + By default we don't trust protocol path MTUs while forwarding
> + because they could be easily forged and can lead to unwanted
> + fragmentation by the router.
> + You only need to enable this if you have user-space software
> + which tries to discover path mtus by itself and depends on the
> + kernel honoring this information. This is normally not the
> + case.
> + Default: 0 (disabled)
> + Possible values:
> + 0 - disabled
> + 1 - enabled
You made this default to off, great, but the description text still says
that we don't trust PMTU information by default :-)
> +static inline unsigned int ip_dst_mtu_secure(const struct dst_entry *dst,
Please do me a favor and remove the "_secure" bit from this helper function
name. This function doesn't implicitly do anything "secure", rather it
simply calculates the dst's mtu based upon various conditions.
Thanks.
^ permalink raw reply
* [PATCH net-next v3 1/9] xen-netback: Introduce TX grant map definitions
From: Zoltan Kiss @ 2014-01-08 0:10 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1389139818-24458-1-git-send-email-zoltan.kiss@citrix.com>
This patch contains the new definitions necessary for grant mapping.
v2:
- move unmapping to separate thread. The NAPI instance has to be scheduled
even from thread context, which can cause huge delays
- that causes unfortunately bigger struct xenvif
- store grant handle after checking validity
v3:
- fix comment in xenvif_tx_dealloc_action()
- call unmap hypercall directly instead of gnttab_unmap_refs(), which does
unnecessary m2p_override. Also remove pages_to_[un]map members
- BUG() if grant_tx_handle corrupted
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/common.h | 25 ++++++
drivers/net/xen-netback/interface.c | 1 +
drivers/net/xen-netback/netback.c | 163 +++++++++++++++++++++++++++++++++++
3 files changed, 189 insertions(+)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index d218ccd..f1071e3 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -79,6 +79,11 @@ struct pending_tx_info {
* if it is head of one or more tx
* reqs
*/
+ /* callback data for released SKBs. The callback is always
+ * xenvif_zerocopy_callback, ctx points to the next fragment, desc
+ * contains the pending_idx
+ */
+ struct ubuf_info callback_struct;
};
#define XEN_NETIF_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, PAGE_SIZE)
@@ -108,6 +113,8 @@ struct xenvif_rx_meta {
*/
#define MAX_GRANT_COPY_OPS (MAX_SKB_FRAGS * XEN_NETIF_RX_RING_SIZE)
+#define NETBACK_INVALID_HANDLE -1
+
struct xenvif {
/* Unique identifier for this interface. */
domid_t domid;
@@ -126,13 +133,23 @@ struct xenvif {
pending_ring_idx_t pending_cons;
u16 pending_ring[MAX_PENDING_REQS];
struct pending_tx_info pending_tx_info[MAX_PENDING_REQS];
+ grant_handle_t grant_tx_handle[MAX_PENDING_REQS];
/* Coalescing tx requests before copying makes number of grant
* copy ops greater or equal to number of slots required. In
* worst case a tx request consumes 2 gnttab_copy.
*/
struct gnttab_copy tx_copy_ops[2*MAX_PENDING_REQS];
+ struct gnttab_map_grant_ref tx_map_ops[MAX_PENDING_REQS];
+ struct gnttab_unmap_grant_ref tx_unmap_ops[MAX_PENDING_REQS];
+ spinlock_t dealloc_lock;
+ spinlock_t response_lock;
+ pending_ring_idx_t dealloc_prod;
+ pending_ring_idx_t dealloc_cons;
+ u16 dealloc_ring[MAX_PENDING_REQS];
+ struct task_struct *dealloc_task;
+ wait_queue_head_t dealloc_wq;
/* Use kthread for guest RX */
struct task_struct *task;
@@ -221,6 +238,8 @@ int xenvif_tx_action(struct xenvif *vif, int budget);
int xenvif_kthread(void *data);
void xenvif_kick_thread(struct xenvif *vif);
+int xenvif_dealloc_kthread(void *data);
+
/* Determine whether the needed number of slots (req) are available,
* and set req_event if not.
*/
@@ -228,6 +247,12 @@ bool xenvif_rx_ring_slots_available(struct xenvif *vif, int needed);
void xenvif_stop_queue(struct xenvif *vif);
+/* Callback from stack when TX packet can be released */
+void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success);
+
+/* Unmap a pending page, usually has to be called before xenvif_idx_release */
+void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx);
+
extern bool separate_tx_rx_irq;
#endif /* __XEN_NETBACK__COMMON_H__ */
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 8d6def2..7170f97 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -37,6 +37,7 @@
#include <xen/events.h>
#include <asm/xen/hypercall.h>
+#include <xen/balloon.h>
#define XENVIF_QUEUE_LENGTH 32
#define XENVIF_NAPI_WEIGHT 64
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index addfe1d1..7c241f9 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -771,6 +771,19 @@ static struct page *xenvif_alloc_page(struct xenvif *vif,
return page;
}
+static inline void xenvif_tx_create_gop(struct xenvif *vif, u16 pending_idx,
+ struct xen_netif_tx_request *txp,
+ struct gnttab_map_grant_ref *gop)
+{
+ gnttab_set_map_op(gop, idx_to_kaddr(vif, pending_idx),
+ GNTMAP_host_map | GNTMAP_readonly,
+ txp->gref, vif->domid);
+
+ memcpy(&vif->pending_tx_info[pending_idx].req, txp,
+ sizeof(*txp));
+
+}
+
static struct gnttab_copy *xenvif_get_requests(struct xenvif *vif,
struct sk_buff *skb,
struct xen_netif_tx_request *txp,
@@ -1599,6 +1612,105 @@ static int xenvif_tx_submit(struct xenvif *vif)
return work_done;
}
+void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success)
+{
+ unsigned long flags;
+ pending_ring_idx_t index;
+ u16 pending_idx = ubuf->desc;
+ struct pending_tx_info *temp =
+ container_of(ubuf, struct pending_tx_info, callback_struct);
+ struct xenvif *vif =
+ container_of(temp - pending_idx, struct xenvif,
+ pending_tx_info[0]);
+
+ spin_lock_irqsave(&vif->dealloc_lock, flags);
+ do {
+ pending_idx = ubuf->desc;
+ ubuf = (struct ubuf_info *) ubuf->ctx;
+ index = pending_index(vif->dealloc_prod);
+ vif->dealloc_ring[index] = pending_idx;
+ /* Sync with xenvif_tx_action_dealloc:
+ * insert idx then incr producer.
+ */
+ smp_wmb();
+ vif->dealloc_prod++;
+ } while (ubuf);
+ wake_up(&vif->dealloc_wq);
+ spin_unlock_irqrestore(&vif->dealloc_lock, flags);
+}
+
+static inline void xenvif_tx_dealloc_action(struct xenvif *vif)
+{
+ struct gnttab_unmap_grant_ref *gop;
+ pending_ring_idx_t dc, dp;
+ u16 pending_idx, pending_idx_release[MAX_PENDING_REQS];
+ unsigned int i = 0;
+
+ dc = vif->dealloc_cons;
+ gop = vif->tx_unmap_ops;
+
+ /* Free up any grants we have finished using */
+ do {
+ dp = vif->dealloc_prod;
+
+ /* Ensure we see all indices enqueued by all
+ * xenvif_zerocopy_callback().
+ */
+ smp_rmb();
+
+ while (dc != dp) {
+ pending_idx =
+ vif->dealloc_ring[pending_index(dc++)];
+
+ /* Already unmapped? */
+ if (vif->grant_tx_handle[pending_idx] ==
+ NETBACK_INVALID_HANDLE) {
+ netdev_err(vif->dev,
+ "Trying to unmap invalid handle! "
+ "pending_idx: %x\n", pending_idx);
+ continue;
+ }
+
+ pending_idx_release[gop-vif->tx_unmap_ops] =
+ pending_idx;
+ gnttab_set_unmap_op(gop,
+ idx_to_kaddr(vif, pending_idx),
+ GNTMAP_host_map,
+ vif->grant_tx_handle[pending_idx]);
+ vif->grant_tx_handle[pending_idx] =
+ NETBACK_INVALID_HANDLE;
+ ++gop;
+ }
+
+ } while (dp != vif->dealloc_prod);
+
+ vif->dealloc_cons = dc;
+
+ if (gop - vif->tx_unmap_ops > 0) {
+ int ret;
+ ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
+ vif->tx_unmap_ops,
+ gop - vif->tx_unmap_ops);
+ if (ret) {
+ netdev_err(vif->dev, "Unmap fail: nr_ops %x ret %d\n",
+ gop - vif->tx_unmap_ops, ret);
+ for (i = 0; i < gop - vif->tx_unmap_ops; ++i) {
+ netdev_err(vif->dev,
+ " host_addr: %llx handle: %x status: %d\n",
+ gop[i].host_addr,
+ gop[i].handle,
+ gop[i].status);
+ }
+ BUG();
+ }
+ }
+
+ for (i = 0; i < gop - vif->tx_unmap_ops; ++i)
+ xenvif_idx_release(vif, pending_idx_release[i],
+ XEN_NETIF_RSP_OKAY);
+}
+
+
/* Called after netfront has transmitted */
int xenvif_tx_action(struct xenvif *vif, int budget)
{
@@ -1665,6 +1777,27 @@ static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
vif->mmap_pages[pending_idx] = NULL;
}
+void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
+{
+ int ret;
+ struct gnttab_unmap_grant_ref tx_unmap_op;
+
+ if (vif->grant_tx_handle[pending_idx] == NETBACK_INVALID_HANDLE) {
+ netdev_err(vif->dev,
+ "Trying to unmap invalid handle! pending_idx: %x\n",
+ pending_idx);
+ return;
+ }
+ gnttab_set_unmap_op(&tx_unmap_op,
+ idx_to_kaddr(vif, pending_idx),
+ GNTMAP_host_map,
+ vif->grant_tx_handle[pending_idx]);
+ ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
+ &tx_unmap_op,
+ 1);
+ BUG_ON(ret);
+ vif->grant_tx_handle[pending_idx] = NETBACK_INVALID_HANDLE;
+}
static void make_tx_response(struct xenvif *vif,
struct xen_netif_tx_request *txp,
@@ -1726,6 +1859,14 @@ static inline int tx_work_todo(struct xenvif *vif)
return 0;
}
+static inline int tx_dealloc_work_todo(struct xenvif *vif)
+{
+ if (vif->dealloc_cons != vif->dealloc_prod)
+ return 1;
+
+ return 0;
+}
+
void xenvif_unmap_frontend_rings(struct xenvif *vif)
{
if (vif->tx.sring)
@@ -1814,6 +1955,28 @@ int xenvif_kthread(void *data)
return 0;
}
+int xenvif_dealloc_kthread(void *data)
+{
+ struct xenvif *vif = data;
+
+ while (!kthread_should_stop()) {
+ wait_event_interruptible(vif->dealloc_wq,
+ tx_dealloc_work_todo(vif) ||
+ kthread_should_stop());
+ if (kthread_should_stop())
+ break;
+
+ xenvif_tx_dealloc_action(vif);
+ cond_resched();
+ }
+
+ /* Unmap anything remaining*/
+ if (tx_dealloc_work_todo(vif))
+ xenvif_tx_dealloc_action(vif);
+
+ return 0;
+}
+
static int __init netback_init(void)
{
int rc = 0;
^ permalink raw reply related
* [PATCH net-next v3 7/9] xen-netback: Add stat counters for frag_list skbs
From: Zoltan Kiss @ 2014-01-08 0:10 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1389139818-24458-1-git-send-email-zoltan.kiss@citrix.com>
These counters help determine how often the guest sends a packet with more
than MAX_SKB_FRAGS frags.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/common.h | 1 +
drivers/net/xen-netback/interface.c | 7 +++++++
drivers/net/xen-netback/netback.c | 1 +
3 files changed, 9 insertions(+)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index e3c28ff..c037efb 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -158,6 +158,7 @@ struct xenvif {
unsigned long tx_zerocopy_sent;
unsigned long tx_zerocopy_success;
unsigned long tx_zerocopy_fail;
+ unsigned long tx_frag_overflow;
/* Miscellaneous private stuff. */
struct net_device *dev;
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index ac27af3..b7daf8d 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -254,6 +254,13 @@ static const struct xenvif_stat {
"tx_zerocopy_fail",
offsetof(struct xenvif, tx_zerocopy_fail)
},
+ /* Number of packets exceeding MAX_SKB_FRAG slots. You should use
+ * a guest with the same MAX_SKB_FRAG
+ */
+ {
+ "tx_frag_overflow",
+ offsetof(struct xenvif, tx_frag_overflow)
+ },
};
static int xenvif_get_sset_count(struct net_device *dev, int string_set)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 9841429..4305965 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1656,6 +1656,7 @@ static int xenvif_tx_submit(struct xenvif *vif, int budget)
skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
skb_shinfo(nskb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
vif->tx_zerocopy_sent += 2;
+ vif->tx_frag_overflow++;
nskb = skb;
skb = skb_copy_expand(skb, 0, 0, GFP_ATOMIC | __GFP_NOWARN);
^ permalink raw reply related
* [PATCH net-next v3 8/9] xen-netback: Timeout packets in RX path
From: Zoltan Kiss @ 2014-01-08 0:10 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1389139818-24458-1-git-send-email-zoltan.kiss@citrix.com>
A malicious or buggy guest can leave its queue filled indefinitely, in which
case qdisc start to queue packets for that VIF. If those packets came from an
another guest, it can block its slots and prevent shutdown. To avoid that, we
make sure the queue is drained in every 10 seconds.
v3:
- remove stale debug log
- tie unmap timeout in xenvif_free to this timeout
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/common.h | 5 +++++
drivers/net/xen-netback/interface.c | 22 ++++++++++++++++++++--
drivers/net/xen-netback/netback.c | 9 +++++++++
3 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index dda3fd5..063fcda 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -130,6 +130,8 @@ struct xenvif {
*/
bool rx_event;
+ struct timer_list wake_queue;
+
/* This array is allocated seperately as it is large */
struct gnttab_copy *grant_copy_op;
@@ -224,4 +226,7 @@ void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx);
extern bool separate_tx_rx_irq;
+extern unsigned int rx_drain_timeout_msecs;
+extern unsigned int rx_drain_timeout_jiffies;
+
#endif /* __XEN_NETBACK__COMMON_H__ */
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 95fcd63..ce032f9 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -114,6 +114,16 @@ static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static void xenvif_wake_queue(unsigned long data)
+{
+ struct xenvif *vif = (struct xenvif *)data;
+
+ if (netif_queue_stopped(vif->dev)) {
+ netdev_err(vif->dev, "draining TX queue\n");
+ netif_wake_queue(vif->dev);
+ }
+}
+
static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct xenvif *vif = netdev_priv(dev);
@@ -143,8 +153,13 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
* then turn off the queue to give the ring a chance to
* drain.
*/
- if (!xenvif_rx_ring_slots_available(vif, min_slots_needed))
+ if (!xenvif_rx_ring_slots_available(vif, min_slots_needed)) {
+ vif->wake_queue.function = xenvif_wake_queue;
+ vif->wake_queue.data = (unsigned long)vif;
xenvif_stop_queue(vif);
+ mod_timer(&vif->wake_queue,
+ jiffies + rx_drain_timeout_jiffies);
+ }
skb_queue_tail(&vif->rx_queue, skb);
xenvif_kick_thread(vif);
@@ -353,6 +368,8 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
/* Initialize 'expires' now: it's used to track the credit window. */
vif->credit_timeout.expires = jiffies;
+ init_timer(&vif->wake_queue);
+
dev->netdev_ops = &xenvif_netdev_ops;
dev->hw_features = NETIF_F_SG |
NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
@@ -528,6 +545,7 @@ void xenvif_disconnect(struct xenvif *vif)
xenvif_carrier_off(vif);
if (vif->task) {
+ del_timer_sync(&vif->wake_queue);
kthread_stop(vif->task);
vif->task = NULL;
}
@@ -558,7 +576,7 @@ void xenvif_free(struct xenvif *vif)
if (vif->grant_tx_handle[i] != NETBACK_INVALID_HANDLE) {
unmap_timeout++;
schedule_timeout(msecs_to_jiffies(1000));
- if (unmap_timeout > 9 &&
+ if (unmap_timeout > (rx_drain_timeout_msecs/1000) &&
net_ratelimit())
netdev_err(vif->dev,
"Page still granted! Index: %x\n", i);
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index f815395..6bc5413 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -62,6 +62,13 @@ module_param(separate_tx_rx_irq, bool, 0644);
static unsigned int fatal_skb_slots = FATAL_SKB_SLOTS_DEFAULT;
module_param(fatal_skb_slots, uint, 0444);
+/* When guest ring is filled up, qdisc queues the packets for us, but we have
+ * to timeout them, otherwise other guests' packets can get stucked there
+ */
+unsigned int rx_drain_timeout_msecs = 10000;
+module_param(rx_drain_timeout_msecs, uint, 0444);
+unsigned int rx_drain_timeout_jiffies;
+
/*
* To avoid confusion, we define XEN_NETBK_LEGACY_SLOTS_MAX indicating
* the maximum slots a valid packet can use. Now this value is defined
@@ -2032,6 +2039,8 @@ static int __init netback_init(void)
if (rc)
goto failed_init;
+ rx_drain_timeout_jiffies = msecs_to_jiffies(rx_drain_timeout_msecs);
+
return 0;
failed_init:
^ 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