[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [RFC PATCHv1 net-next] xen-netback: always fully coalesce guest Rx packets



Always fully coalesce guest Rx packets into the minimum number of ring
slots.  Reducing the number of slots per packet has significant
performance benefits (e.g., 7.2 Gbit/s to 11 Gbit/s in an off-host
receive test).

However, this does increase the number of grant ops per packet which
decreases performance with some workloads (intrahost VM to VM)
/unless/ grant copy has been optimized for adjacent ops with the same
source or destination (see "grant-table: defer releasing pages
acquired in a grant copy"[1]).

Do we need to retain the existing path and make the always coalesce
path conditional on a suitable version of Xen?

[1] http://lists.xen.org/archives/html/xen-devel/2015-01/msg01118.html

Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
---
 drivers/net/xen-netback/common.h  |    1 -
 drivers/net/xen-netback/netback.c |  106 ++-----------------------------------
 2 files changed, 3 insertions(+), 104 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 5f1fda4..589fa25 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -251,7 +251,6 @@ struct xenvif {
 struct xenvif_rx_cb {
        unsigned long expires;
        int meta_slots_used;
-       bool full_coalesce;
 };
 
 #define XENVIF_RX_CB(skb) ((struct xenvif_rx_cb *)(skb)->cb)
diff --git a/drivers/net/xen-netback/netback.c 
b/drivers/net/xen-netback/netback.c
index 908e65e..568238d 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -233,51 +233,6 @@ static void xenvif_rx_queue_drop_expired(struct 
xenvif_queue *queue)
        }
 }
 
-/*
- * Returns true if we should start a new receive buffer instead of
- * adding 'size' bytes to a buffer which currently contains 'offset'
- * bytes.
- */
-static bool start_new_rx_buffer(int offset, unsigned long size, int head,
-                               bool full_coalesce)
-{
-       /* simple case: we have completely filled the current buffer. */
-       if (offset == MAX_BUFFER_OFFSET)
-               return true;
-
-       /*
-        * complex case: start a fresh buffer if the current frag
-        * would overflow the current buffer but only if:
-        *     (i)   this frag would fit completely in the next buffer
-        * and (ii)  there is already some data in the current buffer
-        * and (iii) this is not the head buffer.
-        * and (iv)  there is no need to fully utilize the buffers
-        *
-        * Where:
-        * - (i) stops us splitting a frag into two copies
-        *   unless the frag is too large for a single buffer.
-        * - (ii) stops us from leaving a buffer pointlessly empty.
-        * - (iii) stops us leaving the first buffer
-        *   empty. Strictly speaking this is already covered
-        *   by (ii) but is explicitly checked because
-        *   netfront relies on the first buffer being
-        *   non-empty and can crash otherwise.
-        * - (iv) is needed for skbs which can use up more than MAX_SKB_FRAGS
-        *   slot
-        *
-        * This means we will effectively linearise small
-        * frags but do not needlessly split large buffers
-        * into multiple copies tend to give large frags their
-        * own buffers as before.
-        */
-       BUG_ON(size > MAX_BUFFER_OFFSET);
-       if ((offset + size > MAX_BUFFER_OFFSET) && offset && !head &&
-           !full_coalesce)
-               return true;
-
-       return false;
-}
-
 struct netrx_pending_operations {
        unsigned copy_prod, copy_cons;
        unsigned meta_prod, meta_cons;
@@ -336,24 +291,13 @@ static void xenvif_gop_frag_copy(struct xenvif_queue 
*queue, struct sk_buff *skb
                BUG_ON(offset >= PAGE_SIZE);
                BUG_ON(npo->copy_off > MAX_BUFFER_OFFSET);
 
-               bytes = PAGE_SIZE - offset;
+               if (npo->copy_off == MAX_BUFFER_OFFSET)
+                       meta = get_next_rx_buffer(queue, npo);
 
+               bytes = PAGE_SIZE - offset;
                if (bytes > size)
                        bytes = size;
 
-               if (start_new_rx_buffer(npo->copy_off,
-                                       bytes,
-                                       *head,
-                                       XENVIF_RX_CB(skb)->full_coalesce)) {
-                       /*
-                        * Netfront requires there to be some data in the head
-                        * buffer.
-                        */
-                       BUG_ON(*head);
-
-                       meta = get_next_rx_buffer(queue, npo);
-               }
-
                if (npo->copy_off + bytes > MAX_BUFFER_OFFSET)
                        bytes = MAX_BUFFER_OFFSET - npo->copy_off;
 
@@ -652,60 +596,16 @@ static void xenvif_rx_action(struct xenvif_queue *queue)
 
        while (xenvif_rx_ring_slots_available(queue, XEN_NETBK_RX_SLOTS_MAX)
               && (skb = xenvif_rx_dequeue(queue)) != NULL) {
-               RING_IDX max_slots_needed;
                RING_IDX old_req_cons;
                RING_IDX ring_slots_used;
                int i;
 
                queue->last_rx_time = jiffies;
 
-               /* We need a cheap worse case estimate for the number of
-                * slots we'll use.
-                */
-
-               max_slots_needed = DIV_ROUND_UP(offset_in_page(skb->data) +
-                                               skb_headlen(skb),
-                                               PAGE_SIZE);
-               for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
-                       unsigned int size;
-                       unsigned int offset;
-
-                       size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
-                       offset = skb_shinfo(skb)->frags[i].page_offset;
-
-                       /* For a worse-case estimate we need to factor in
-                        * the fragment page offset as this will affect the
-                        * number of times xenvif_gop_frag_copy() will
-                        * call start_new_rx_buffer().
-                        */
-                       max_slots_needed += DIV_ROUND_UP(offset + size,
-                                                        PAGE_SIZE);
-               }
-
-               /* To avoid the estimate becoming too pessimal for some
-                * frontends that limit posted rx requests, cap the estimate
-                * at MAX_SKB_FRAGS. In this case netback will fully coalesce
-                * the skb into the provided slots.
-                */
-               if (max_slots_needed > MAX_SKB_FRAGS) {
-                       max_slots_needed = MAX_SKB_FRAGS;
-                       XENVIF_RX_CB(skb)->full_coalesce = true;
-               } else {
-                       XENVIF_RX_CB(skb)->full_coalesce = false;
-               }
-
-               /* We may need one more slot for GSO metadata */
-               if (skb_is_gso(skb) &&
-                  (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4 ||
-                   skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6))
-                       max_slots_needed++;
-
                old_req_cons = queue->rx.req_cons;
                XENVIF_RX_CB(skb)->meta_slots_used = xenvif_gop_skb(skb, &npo, 
queue);
                ring_slots_used = queue->rx.req_cons - old_req_cons;
 
-               BUG_ON(ring_slots_used > max_slots_needed);
-
                __skb_queue_tail(&rxq, skb);
        }
 
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.