Commit 4caeccb4 authored by David S. Miller's avatar David S. Miller

Merge branch 'xen-netback-next'

Zoltan Kiss says:

====================
xen-netback: TX grant mapping with SKBTX_DEV_ZEROCOPY instead of copy

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 AMD
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 IP stack through deliver_skb, which is due to this [2] patch. This affects
DomU->Dom0 IP traffic and when Dom0 does routing/NAT for the guest. 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've tried to broke it down to smaller patches, with mixed results, so I
welcome suggestions on that part as well:
1: Use skb->cb to store pending_idx
2: Some refactoring
3: Change RX path for mapped SKB fragments (moved here to keep bisectability,
review it after #4)
4: Introduce TX grant mapping
5: Remove old TX grant copy definitons and fix indentations
6: Add stat counters for zerocopy
7: Handle guests with too many frags
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.

v4: Now we are using a new grant mapping API to avoid m2p_override. The RX queue
timeout logic changed also.

v5: Only minor fixes based on Wei's comments

v6: Important bugfixes for xenvif_poll exit path and zerocopy callback, see
first 2 patches. Also rework of handling packets with too many slots, and
reorder the series a bit.

v7: Small fixes in comments/log messages/error paths, and merging the frag
overflow stats patch into its parent.

[1] http://lwn.net/Articles/491522/
[2] https://lkml.org/lkml/2012/7/20/363
====================
Signed-off-by: default avatarZoltan Kiss <zoltan.kiss@citrix.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 31c70d59 e9275f5e
......@@ -48,37 +48,19 @@
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, desc contains the pending_idx, which is
* also an index in pending_tx_info array. It is initialized in
* xenvif_alloc and it never changes.
* skb_shinfo(skb)->destructor_arg points to the first mapped slot's
* callback_struct in this array of struct pending_tx_info's, then ctx
* to the next, or NULL if there is no more slot for this skb.
* ubuf_to_vif is a helper which finds the struct xenvif from a pointer
* to this field.
*/
struct ubuf_info callback_struct;
};
#define XEN_NETIF_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, PAGE_SIZE)
......@@ -108,6 +90,15 @@ struct xenvif_rx_meta {
*/
#define MAX_GRANT_COPY_OPS (MAX_SKB_FRAGS * XEN_NETIF_RX_RING_SIZE)
#define NETBACK_INVALID_HANDLE -1
/* To avoid confusion, we define XEN_NETBK_LEGACY_SLOTS_MAX indicating
* the maximum slots a valid packet can use. Now this value is defined
* to be XEN_NETIF_NR_SLOTS_MIN, which is supposed to be supported by
* all backend.
*/
#define XEN_NETBK_LEGACY_SLOTS_MAX XEN_NETIF_NR_SLOTS_MIN
struct xenvif {
/* Unique identifier for this interface. */
domid_t domid;
......@@ -126,13 +117,28 @@ struct xenvif {
pending_ring_idx_t pending_cons;
u16 pending_ring[MAX_PENDING_REQS];
struct pending_tx_info pending_tx_info[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.
grant_handle_t grant_tx_handle[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 */
struct page *pages_to_map[MAX_PENDING_REQS];
struct page *pages_to_unmap[MAX_PENDING_REQS];
/* This prevents zerocopy callbacks to race over dealloc_ring */
spinlock_t callback_lock;
/* This prevents dealloc thread and NAPI instance to race over response
* creation and pending_ring in xenvif_idx_release. In xenvif_tx_err
* it only protect response creation
*/
struct gnttab_copy tx_copy_ops[2*MAX_PENDING_REQS];
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;
struct timer_list dealloc_delay;
bool dealloc_delay_timed_out;
/* Use kthread for guest RX */
struct task_struct *task;
......@@ -144,6 +150,9 @@ struct xenvif {
struct xen_netif_rx_back_ring rx;
struct sk_buff_head rx_queue;
RING_IDX rx_last_skb_slots;
bool rx_queue_purge;
struct timer_list wake_queue;
/* This array is allocated seperately as it is large */
struct gnttab_copy *grant_copy_op;
......@@ -175,6 +184,10 @@ struct xenvif {
/* Statistics */
unsigned long rx_gso_checksum_fixup;
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;
......@@ -216,9 +229,11 @@ void xenvif_carrier_off(struct xenvif *vif);
int xenvif_tx_action(struct xenvif *vif, int budget);
int xenvif_kthread(void *data);
int xenvif_kthread_guest_rx(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.
*/
......@@ -226,6 +241,30 @@ 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 and release it back to the guest */
void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx);
static inline pending_ring_idx_t nr_pending_reqs(struct xenvif *vif)
{
return MAX_PENDING_REQS -
vif->pending_prod + vif->pending_cons;
}
static inline bool xenvif_tx_pending_slots_available(struct xenvif *vif)
{
return nr_pending_reqs(vif) + XEN_NETBK_LEGACY_SLOTS_MAX
< MAX_PENDING_REQS;
}
/* Callback from stack when TX packet can be released */
void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success);
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__ */
......@@ -38,6 +38,7 @@
#include <xen/events.h>
#include <asm/xen/hypercall.h>
#include <xen/balloon.h>
#define XENVIF_QUEUE_LENGTH 32
#define XENVIF_NAPI_WEIGHT 64
......@@ -87,7 +88,8 @@ static int xenvif_poll(struct napi_struct *napi, int budget)
local_irq_save(flags);
RING_FINAL_CHECK_FOR_REQUESTS(&vif->tx, more_to_do);
if (!more_to_do)
if (!(more_to_do &&
xenvif_tx_pending_slots_available(vif)))
__napi_complete(napi);
local_irq_restore(flags);
......@@ -113,6 +115,18 @@ 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");
vif->rx_queue_purge = true;
xenvif_kick_thread(vif);
netif_wake_queue(vif->dev);
}
}
static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct xenvif *vif = netdev_priv(dev);
......@@ -121,7 +135,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
......@@ -140,8 +156,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);
......@@ -234,6 +255,28 @@ 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)
},
/* 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)
......@@ -327,6 +370,8 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
init_timer(&vif->credit_timeout);
vif->credit_window_start = get_jiffies_64();
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 |
......@@ -343,8 +388,27 @@ 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->callback_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 ERR_PTR(-ENOMEM);
}
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;
}
init_timer(&vif->dealloc_delay);
/*
* Initialise a dummy MAC address. We choose the numerically
......@@ -382,12 +446,14 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
BUG_ON(vif->tx_irq);
BUG_ON(vif->task);
BUG_ON(vif->dealloc_task);
err = xenvif_map_frontend_rings(vif, tx_ring_ref, rx_ring_ref);
if (err < 0)
goto err;
init_waitqueue_head(&vif->wq);
init_waitqueue_head(&vif->dealloc_wq);
if (tx_evtchn == rx_evtchn) {
/* feature-split-event-channels == 0 */
......@@ -421,8 +487,8 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
disable_irq(vif->rx_irq);
}
task = kthread_create(xenvif_kthread,
(void *)vif, "%s", vif->dev->name);
task = kthread_create(xenvif_kthread_guest_rx,
(void *)vif, "%s-guest-rx", vif->dev->name);
if (IS_ERR(task)) {
pr_warn("Could not allocate kthread for %s\n", vif->dev->name);
err = PTR_ERR(task);
......@@ -431,6 +497,16 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
vif->task = task;
task = kthread_create(xenvif_dealloc_kthread,
(void *)vif, "%s-dealloc", vif->dev->name);
if (IS_ERR(task)) {
pr_warn("Could not allocate kthread for %s\n", vif->dev->name);
err = PTR_ERR(task);
goto err_rx_unbind;
}
vif->dealloc_task = task;
rtnl_lock();
if (!vif->can_sg && vif->dev->mtu > ETH_DATA_LEN)
dev_set_mtu(vif->dev, ETH_DATA_LEN);
......@@ -441,6 +517,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;
......@@ -474,10 +551,17 @@ 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;
}
if (vif->dealloc_task) {
del_timer_sync(&vif->dealloc_delay);
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);
......@@ -493,6 +577,36 @@ void xenvif_disconnect(struct xenvif *vif)
void xenvif_free(struct xenvif *vif)
{
int i, unmap_timeout = 0;
/* Here we want to avoid timeout messages if an skb can be legitimatly
* stucked somewhere else. Realisticly this could be an another vif's
* internal or QDisc queue. That another vif also has this
* rx_drain_timeout_msecs timeout, but the timer only ditches the
* internal queue. After that, the QDisc queue can put in worst case
* XEN_NETIF_RX_RING_SIZE / MAX_SKB_FRAGS skbs into that another vif's
* internal queue, so we need several rounds of such timeouts until we
* can be sure that no another vif should have skb's from us. We are
* not sending more skb's, so newly stucked packets are not interesting
* for us here.
*/
unsigned int worst_case_skb_lifetime = (rx_drain_timeout_msecs/1000) *
DIV_ROUND_UP(XENVIF_QUEUE_LENGTH, (XEN_NETIF_RX_RING_SIZE / MAX_SKB_FRAGS));
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 > worst_case_skb_lifetime &&
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);
......
This diff is collapsed.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment