From 121fa4b77775549c3c5eb41eb335d7dcbb801f90 Mon Sep 17 00:00:00 2001 From: Zoltan Kiss Date: Thu, 6 Mar 2014 21:48:24 +0000 Subject: xen-netback: Minor refactoring of netback code This patch contains a few bits of refactoring before introducing the grant mapping changes: - introducing xenvif_tx_pending_slots_available(), as this is used several times, and will be used more often - rename the thread to vifX.Y-guest-rx, to signify it does RX work from the guest point of view Signed-off-by: Zoltan Kiss Signed-off-by: David S. Miller --- drivers/net/xen-netback/interface.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/net/xen-netback/interface.c') diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c index 7669d49a67e2..bc32627a22cb 100644 --- a/drivers/net/xen-netback/interface.c +++ b/drivers/net/xen-netback/interface.c @@ -421,8 +421,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); -- cgit From f53c3fe8dad725b014e9c7682720d8e3e2a8a5b3 Mon Sep 17 00:00:00 2001 From: Zoltan Kiss Date: Thu, 6 Mar 2014 21:48:26 +0000 Subject: xen-netback: Introduce TX grant mapping This patch introduces grant mapping on netback TX path. It replaces grant copy operations, ditching grant copy coalescing along the way. Another solution for copy coalescing is introduced in "xen-netback: Handle guests with too many frags", older guests and Windows can broke before that patch applies. There is a callback (xenvif_zerocopy_callback) from core stack to release the slots back to the guests when kfree_skb or skb_orphan_frags called. It feeds a separate dealloc thread, as scheduling NAPI instance from there is inefficient, therefore we can't do dealloc from the instance. Signed-off-by: Zoltan Kiss Signed-off-by: David S. Miller --- drivers/net/xen-netback/common.h | 39 +++- drivers/net/xen-netback/interface.c | 65 +++++- drivers/net/xen-netback/netback.c | 432 +++++++++++++++++++++++------------- 3 files changed, 371 insertions(+), 165 deletions(-) (limited to 'drivers/net/xen-netback/interface.c') diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h index 8f264df8818a..5a991266a394 100644 --- a/drivers/net/xen-netback/common.h +++ b/drivers/net/xen-netback/common.h @@ -79,6 +79,17 @@ 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, 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) @@ -135,13 +146,31 @@ 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]; + /* 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 + */ + 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; @@ -228,6 +257,8 @@ int xenvif_tx_action(struct xenvif *vif, int budget); 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. */ @@ -235,6 +266,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 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 - diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c index bc32627a22cb..1fe9fe523cc8 100644 --- a/drivers/net/xen-netback/interface.c +++ b/drivers/net/xen-netback/interface.c @@ -38,6 +38,7 @@ #include #include +#include #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); @@ -121,7 +123,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 @@ -343,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->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; + } /* * Initialise a dummy MAC address. We choose the numerically @@ -382,12 +404,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 */ @@ -431,6 +455,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 +475,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; @@ -478,6 +513,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); @@ -493,6 +533,23 @@ 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 e9391badfa4a..cb29134147d1 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -101,10 +101,18 @@ static inline unsigned long idx_to_kaddr(struct xenvif *vif, return (unsigned long)pfn_to_kaddr(idx_to_pfn(vif, idx)); } +/* Find the containing VIF's structure from a pointer in pending_tx_info array + */ static inline struct xenvif* ubuf_to_vif(struct ubuf_info *ubuf) { - return NULL; + u16 pending_idx = ubuf->desc; + struct pending_tx_info *temp = + container_of(ubuf, struct pending_tx_info, callback_struct); + return container_of(temp - pending_idx, + struct xenvif, + pending_tx_info[0]); } + /* This is a miniumum size for the linear area to avoid lots of * calls to __pskb_pull_tail() as we set up checksum offsets. The * value 128 was chosen as it covers all IPv4 and most likely @@ -665,9 +673,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++); @@ -799,10 +810,24 @@ struct xenvif_tx_cb { #define XENVIF_TX_CB(skb) ((struct xenvif_tx_cb *)(skb)->cb) -static struct gnttab_copy *xenvif_get_requests(struct xenvif *vif, - struct sk_buff *skb, - struct xen_netif_tx_request *txp, - struct gnttab_copy *gop) +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) +{ + vif->pages_to_map[gop-vif->tx_map_ops] = vif->mmap_pages[pending_idx]; + 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_map_grant_ref *xenvif_get_requests(struct xenvif *vif, + struct sk_buff *skb, + struct xen_netif_tx_request *txp, + struct gnttab_map_grant_ref *gop) { struct skb_shared_info *shinfo = skb_shinfo(skb); skb_frag_t *frags = shinfo->frags; @@ -823,83 +848,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); @@ -919,11 +873,38 @@ err: return NULL; } +static inline void xenvif_grant_handle_set(struct xenvif *vif, + u16 pending_idx, + grant_handle_t handle) +{ + if (unlikely(vif->grant_tx_handle[pending_idx] != + NETBACK_INVALID_HANDLE)) { + netdev_err(vif->dev, + "Trying to overwrite active handle! pending_idx: %x\n", + pending_idx); + BUG(); + } + vif->grant_tx_handle[pending_idx] = handle; +} + +static inline void xenvif_grant_handle_reset(struct xenvif *vif, + u16 pending_idx) +{ + if (unlikely(vif->grant_tx_handle[pending_idx] == + NETBACK_INVALID_HANDLE)) { + netdev_err(vif->dev, + "Trying to unmap invalid handle! pending_idx: %x\n", + pending_idx); + BUG(); + } + vif->grant_tx_handle[pending_idx] = NETBACK_INVALID_HANDLE; +} + 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 = XENVIF_TX_CB(skb)->pending_idx; struct skb_shared_info *shinfo = skb_shinfo(skb); struct pending_tx_info *tx_info; @@ -935,6 +916,8 @@ 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 + xenvif_grant_handle_set(vif, 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); @@ -948,18 +931,13 @@ 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)) { + xenvif_grant_handle_set(vif, pending_idx , gop->handle); /* Had a previous error? Invalidate this fragment. */ if (unlikely(err)) - xenvif_idx_release(vif, pending_idx, - XEN_NETIF_RSP_OKAY); + xenvif_idx_unmap(vif, pending_idx); continue; } @@ -972,11 +950,10 @@ static int xenvif_tx_check_gop(struct xenvif *vif, /* First error: invalidate header and preceding fragments. */ pending_idx = XENVIF_TX_CB(skb)->pending_idx; - xenvif_idx_release(vif, pending_idx, XEN_NETIF_RSP_OKAY); + xenvif_idx_unmap(vif, pending_idx); for (j = start; j < i; j++) { pending_idx = frag_get_pending_idx(&shinfo->frags[j]); - xenvif_idx_release(vif, pending_idx, - XEN_NETIF_RSP_OKAY); + xenvif_idx_unmap(vif, pending_idx); } /* Remember the error: invalidate all subsequent fragments. */ @@ -992,6 +969,10 @@ static void xenvif_fill_frags(struct xenvif *vif, struct sk_buff *skb) struct skb_shared_info *shinfo = skb_shinfo(skb); int nr_frags = shinfo->nr_frags; int i; + u16 prev_pending_idx = INVALID_PENDING_IDX; + + if (skb_shinfo(skb)->destructor_arg) + prev_pending_idx = XENVIF_TX_CB(skb)->pending_idx; for (i = 0; i < nr_frags; i++) { skb_frag_t *frag = shinfo->frags + i; @@ -1001,6 +982,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); @@ -1008,10 +1000,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, @@ -1131,7 +1128,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; @@ -1238,30 +1235,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; XENVIF_TX_CB(skb)->pending_idx = pending_idx; __skb_put(skb, data_len); @@ -1290,17 +1267,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; @@ -1324,14 +1301,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. */ - xenvif_idx_release(vif, pending_idx, - XEN_NETIF_RSP_OKAY); + skb_shinfo(skb)->destructor_arg = NULL; + xenvif_idx_unmap(vif, pending_idx); } if (txp->flags & XEN_NETTXF_csum_blank) @@ -1353,6 +1333,9 @@ 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"); + /* We have to set this flag to trigger the callback */ + if (skb_shinfo(skb)->destructor_arg) + skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY; kfree_skb(skb); continue; } @@ -1378,6 +1361,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); } @@ -1386,14 +1377,111 @@ static int xenvif_tx_submit(struct xenvif *vif) void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success) { - return; + unsigned long flags; + pending_ring_idx_t index; + struct xenvif *vif = ubuf_to_vif(ubuf); + + /* This is the only place where we grab this lock, to protect callbacks + * from each other. + */ + spin_lock_irqsave(&vif->callback_lock, flags); + do { + u16 pending_idx = ubuf->desc; + ubuf = (struct ubuf_info *) ubuf->ctx; + BUG_ON(vif->dealloc_prod - vif->dealloc_cons >= + MAX_PENDING_REQS); + index = pending_index(vif->dealloc_prod); + vif->dealloc_ring[index] = pending_idx; + /* Sync with xenvif_tx_dealloc_action: + * insert idx then incr producer. + */ + smp_wmb(); + vif->dealloc_prod++; + } while (ubuf); + wake_up(&vif->dealloc_wq); + spin_unlock_irqrestore(&vif->callback_lock, flags); + + if (RING_HAS_UNCONSUMED_REQUESTS(&vif->tx) && + xenvif_tx_pending_slots_available(vif)) { + local_bh_disable(); + napi_schedule(&vif->napi); + local_bh_enable(); + } +} + +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) { + BUG_ON(gop - vif->tx_unmap_ops > MAX_PENDING_REQS); + pending_idx = + vif->dealloc_ring[pending_index(dc++)]; + + pending_idx_release[gop-vif->tx_unmap_ops] = + pending_idx; + vif->pages_to_unmap[gop-vif->tx_unmap_ops] = + vif->mmap_pages[pending_idx]; + gnttab_set_unmap_op(gop, + idx_to_kaddr(vif, pending_idx), + GNTMAP_host_map, + vif->grant_tx_handle[pending_idx]); + /* Btw. already unmapped? */ + xenvif_grant_handle_reset(vif, pending_idx); + ++gop; + } + + } while (dp != vif->dealloc_prod); + + vif->dealloc_cons = dc; + + if (gop - vif->tx_unmap_ops > 0) { + int ret; + ret = gnttab_unmap_refs(vif->tx_unmap_ops, + NULL, + vif->pages_to_unmap, + 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) { + if (gop[i].status != GNTST_okay) + 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) { unsigned nr_gops; - int work_done; + int work_done, ret; if (unlikely(!tx_work_todo(vif))) return 0; @@ -1403,7 +1491,11 @@ 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 = gnttab_map_refs(vif->tx_map_ops, + NULL, + vif->pages_to_map, + nr_gops); + BUG_ON(ret); work_done = xenvif_tx_submit(vif); @@ -1414,45 +1506,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); } @@ -1500,6 +1566,25 @@ static struct xen_netif_rx_response *make_rx_response(struct xenvif *vif, return resp; } +void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx) +{ + int ret; + struct gnttab_unmap_grant_ref tx_unmap_op; + + gnttab_set_unmap_op(&tx_unmap_op, + idx_to_kaddr(vif, pending_idx), + GNTMAP_host_map, + vif->grant_tx_handle[pending_idx]); + /* Btw. already unmapped? */ + xenvif_grant_handle_reset(vif, pending_idx); + + ret = gnttab_unmap_refs(&tx_unmap_op, NULL, + &vif->mmap_pages[pending_idx], 1); + BUG_ON(ret); + + xenvif_idx_release(vif, pending_idx, XEN_NETIF_RSP_OKAY); +} + static inline int rx_work_todo(struct xenvif *vif) { return !skb_queue_empty(&vif->rx_queue) && @@ -1516,6 +1601,11 @@ static inline int tx_work_todo(struct xenvif *vif) return 0; } +static inline bool tx_dealloc_work_todo(struct xenvif *vif) +{ + return vif->dealloc_cons != vif->dealloc_prod; +} + void xenvif_unmap_frontend_rings(struct xenvif *vif) { if (vif->tx.sring) @@ -1602,6 +1692,28 @@ int xenvif_kthread_guest_rx(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; -- cgit From 1bb332af4cd889e4b64dacbf4a793ceb3a70445d Mon Sep 17 00:00:00 2001 From: Zoltan Kiss Date: Thu, 6 Mar 2014 21:48:28 +0000 Subject: xen-netback: Add stat counters for zerocopy 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. NOTE: if bisect brought you here, you should apply the series up until "xen-netback: Timeout packets in RX path", otherwise Windows guests can't work properly and malicious guests can block other guests by not releasing their sent packets. Signed-off-by: Zoltan Kiss Signed-off-by: David S. Miller --- 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(-) (limited to 'drivers/net/xen-netback/interface.c') diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h index 49109afa2253..683d30160a7c 100644 --- a/drivers/net/xen-netback/common.h +++ b/drivers/net/xen-netback/common.h @@ -179,6 +179,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 1fe9fe523cc8..44df8581b4d7 100644 --- a/drivers/net/xen-netback/interface.c +++ b/drivers/net/xen-netback/interface.c @@ -238,6 +238,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 46a75706cb78..3cb586357df7 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -1323,8 +1323,10 @@ static int xenvif_tx_submit(struct xenvif *vif) * 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) + if (skb_shinfo(skb)->destructor_arg) { skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY; + vif->tx_zerocopy_sent++; + } netif_receive_skb(skb); } @@ -1364,6 +1366,11 @@ void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success) napi_schedule(&vif->napi); local_bh_enable(); } + + if (likely(zerocopy_success)) + vif->tx_zerocopy_success++; + else + vif->tx_zerocopy_fail++; } static inline void xenvif_tx_dealloc_action(struct xenvif *vif) -- cgit From e3377f36ca20a034dce56335dc9b89f41094d845 Mon Sep 17 00:00:00 2001 From: Zoltan Kiss Date: Thu, 6 Mar 2014 21:48:29 +0000 Subject: xen-netback: Handle guests with too many frags 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 and coalesce the frags into a brand new one and send it to the stack - unmap the 2 old skb's pages It's also introduces new stat counters, which help determine how often the guest sends a packet with more than MAX_SKB_FRAGS frags. NOTE: if bisect brought you here, you should apply the series up until "xen-netback: Timeout packets in RX path", otherwise malicious guests can block other guests by not releasing their sent packets. Signed-off-by: Zoltan Kiss Signed-off-by: David S. Miller --- drivers/net/xen-netback/common.h | 1 + drivers/net/xen-netback/interface.c | 7 ++ drivers/net/xen-netback/netback.c | 164 +++++++++++++++++++++++++++++++++--- 3 files changed, 162 insertions(+), 10 deletions(-) (limited to 'drivers/net/xen-netback/interface.c') diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h index 683d30160a7c..f2f8a02afc36 100644 --- a/drivers/net/xen-netback/common.h +++ b/drivers/net/xen-netback/common.h @@ -182,6 +182,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 44df8581b4d7..b646039e539b 100644 --- a/drivers/net/xen-netback/interface.c +++ b/drivers/net/xen-netback/interface.c @@ -253,6 +253,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 3cb586357df7..58effc49f526 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -37,6 +37,7 @@ #include #include #include +#include #include @@ -801,6 +802,23 @@ static inline void xenvif_tx_create_gop(struct xenvif *vif, sizeof(*txp)); } +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); + + /* Initialize it here to avoid later surprises */ + skb_shinfo(skb)->destructor_arg = NULL; + + return skb; +} + static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif, struct sk_buff *skb, struct xen_netif_tx_request *txp, @@ -811,11 +829,16 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif, u16 pending_idx = XENVIF_TX_CB(skb)->pending_idx; 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. */ @@ -829,7 +852,29 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif, frag_set_pending_idx(&frags[shinfo->nr_frags], pending_idx); } - BUG_ON(shinfo->nr_frags > MAX_SKB_FRAGS); + if (frag_overflow) { + struct sk_buff *nskb = xenvif_alloc_skb(0); + if (unlikely(nskb == NULL)) { + if (net_ratelimit()) + 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; } @@ -871,6 +916,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; @@ -882,6 +928,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; @@ -905,9 +952,11 @@ 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 = XENVIF_TX_CB(skb)->pending_idx; + if (!first_skb) + pending_idx = XENVIF_TX_CB(skb)->pending_idx; + else + pending_idx = XENVIF_TX_CB(skb)->pending_idx; xenvif_idx_unmap(vif, pending_idx); for (j = start; j < i; j++) { pending_idx = frag_get_pending_idx(&shinfo->frags[j]); @@ -918,6 +967,30 @@ static int xenvif_tx_check_gop(struct xenvif *vif, err = newerr; } + if (skb_has_frag_list(skb)) { + 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 = XENVIF_TX_CB(skb)->pending_idx; + 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); + } + } + *gopp = gop + 1; return err; } @@ -1169,8 +1242,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"); @@ -1178,9 +1250,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]; @@ -1231,6 +1300,71 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif, int budget) return gop - vif->tx_map_ops; } +/* Consolidate skb with a frag_list into a brand new one with local pages on + * frags. Returns 0 or -ENOMEM if can't allocate new pages. + */ +static int xenvif_handle_frag_list(struct xenvif *vif, struct sk_buff *skb) +{ + unsigned int offset = skb_headlen(skb); + skb_frag_t frags[MAX_SKB_FRAGS]; + int i; + struct ubuf_info *uarg; + struct sk_buff *nskb = skb_shinfo(skb)->frag_list; + + vif->tx_zerocopy_sent += 2; + vif->tx_frag_overflow++; + + xenvif_fill_frags(vif, nskb); + /* Subtract frags size, we will correct it later */ + skb->truesize -= skb->data_len; + skb->len += nskb->len; + skb->data_len += nskb->len; + + /* create a brand new frags array and coalesce there */ + for (i = 0; offset < skb->len; i++) { + struct page *page; + unsigned int len; + + BUG_ON(i >= MAX_SKB_FRAGS); + page = alloc_page(GFP_ATOMIC|__GFP_COLD); + if (!page) { + int j; + skb->truesize += skb->data_len; + for (j = 0; j < i; j++) + put_page(frags[j].page.p); + return -ENOMEM; + } + + if (offset + PAGE_SIZE < skb->len) + len = PAGE_SIZE; + else + len = skb->len - offset; + if (skb_copy_bits(skb, offset, page_address(page), len)) + BUG(); + + offset += len; + frags[i].page.p = page; + frags[i].page_offset = 0; + skb_frag_size_set(&frags[i], len); + } + /* swap out with old one */ + memcpy(skb_shinfo(skb)->frags, + frags, + i * sizeof(skb_frag_t)); + skb_shinfo(skb)->nr_frags = i; + skb->truesize += i * PAGE_SIZE; + + /* remove traces of mapped pages and frag_list */ + skb_frag_list_init(skb); + uarg = skb_shinfo(skb)->destructor_arg; + uarg->callback(uarg, true); + skb_shinfo(skb)->destructor_arg = NULL; + + skb_shinfo(nskb)->tx_flags |= SKBTX_DEV_ZEROCOPY; + kfree_skb(nskb); + + return 0; +} static int xenvif_tx_submit(struct xenvif *vif) { @@ -1267,7 +1401,6 @@ static int xenvif_tx_submit(struct xenvif *vif) &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); } @@ -1278,6 +1411,17 @@ static int xenvif_tx_submit(struct xenvif *vif) xenvif_fill_frags(vif, skb); + if (unlikely(skb_has_frag_list(skb))) { + if (xenvif_handle_frag_list(vif, skb)) { + if (net_ratelimit()) + netdev_err(vif->dev, + "Not enough memory to consolidate frag_list!\n"); + skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY; + kfree_skb(skb); + continue; + } + } + 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)); -- cgit From 093507885ae5dc0288af07fbb922d2f85b3a88a6 Mon Sep 17 00:00:00 2001 From: Zoltan Kiss Date: Thu, 6 Mar 2014 21:48:30 +0000 Subject: xen-netback: Timeout packets in RX path 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. The QDisc queue in worst case takes 3 round to flush usually. Signed-off-by: Zoltan Kiss Signed-off-by: David S. Miller --- drivers/net/xen-netback/common.h | 6 ++++++ drivers/net/xen-netback/interface.c | 37 +++++++++++++++++++++++++++++++++++-- drivers/net/xen-netback/netback.c | 23 ++++++++++++++++++++--- 3 files changed, 61 insertions(+), 5 deletions(-) (limited to 'drivers/net/xen-netback/interface.c') diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h index f2f8a02afc36..0355f8767e3b 100644 --- a/drivers/net/xen-netback/common.h +++ b/drivers/net/xen-netback/common.h @@ -148,6 +148,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; @@ -259,4 +262,7 @@ 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__ */ diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c index b646039e539b..9cc9f638f442 100644 --- a/drivers/net/xen-netback/interface.c +++ b/drivers/net/xen-netback/interface.c @@ -115,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); @@ -144,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); @@ -353,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 | @@ -531,6 +550,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; } @@ -556,12 +576,25 @@ 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 > 9 && + if (unmap_timeout > worst_case_skb_lifetime && net_ratelimit()) netdev_err(vif->dev, "Page still granted! Index: %x\n", diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c index 58effc49f526..8518a0d1f6f9 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -55,6 +55,13 @@ bool separate_tx_rx_irq = 1; module_param(separate_tx_rx_irq, bool, 0644); +/* 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; + /* * This is the maximum slots a skb can have. If a guest sends a skb * which exceeds this limit it is considered malicious. @@ -1694,8 +1701,9 @@ void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx) static inline int rx_work_todo(struct xenvif *vif) { - return !skb_queue_empty(&vif->rx_queue) && - xenvif_rx_ring_slots_available(vif, vif->rx_last_skb_slots); + return (!skb_queue_empty(&vif->rx_queue) && + xenvif_rx_ring_slots_available(vif, vif->rx_last_skb_slots)) || + vif->rx_queue_purge; } static inline int tx_work_todo(struct xenvif *vif) @@ -1782,12 +1790,19 @@ int xenvif_kthread_guest_rx(void *data) if (kthread_should_stop()) break; + if (vif->rx_queue_purge) { + skb_queue_purge(&vif->rx_queue); + vif->rx_queue_purge = false; + } + if (!skb_queue_empty(&vif->rx_queue)) xenvif_rx_action(vif); if (skb_queue_empty(&vif->rx_queue) && - netif_queue_stopped(vif->dev)) + netif_queue_stopped(vif->dev)) { + del_timer_sync(&vif->wake_queue); xenvif_start_queue(vif); + } cond_resched(); } @@ -1838,6 +1853,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: -- cgit From e9275f5e2df1b2098a8cc405d87b88b9affd73e6 Mon Sep 17 00:00:00 2001 From: Zoltan Kiss Date: Thu, 6 Mar 2014 21:48:31 +0000 Subject: xen-netback: Aggregate TX unmap operations 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 Signed-off-by: David S. Miller --- drivers/net/xen-netback/common.h | 2 ++ drivers/net/xen-netback/interface.c | 2 ++ drivers/net/xen-netback/netback.c | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 1 deletion(-) (limited to 'drivers/net/xen-netback/interface.c') diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h index 0355f8767e3b..bef37be402b8 100644 --- a/drivers/net/xen-netback/common.h +++ b/drivers/net/xen-netback/common.h @@ -137,6 +137,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 9cc9f638f442..83a71ac5b93a 100644 --- a/drivers/net/xen-netback/interface.c +++ b/drivers/net/xen-netback/interface.c @@ -408,6 +408,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 @@ -556,6 +557,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 8518a0d1f6f9..bc943205a691 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -133,6 +133,11 @@ static inline pending_ring_idx_t pending_index(unsigned i) return i & (MAX_PENDING_REQS-1); } +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; @@ -1716,9 +1721,36 @@ 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 bool tx_dealloc_work_todo(struct xenvif *vif) { - return 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 false; + } + del_timer_sync(&vif->dealloc_delay); + vif->dealloc_delay_timed_out = false; + return true; + } + + return false; } void xenvif_unmap_frontend_rings(struct xenvif *vif) -- cgit