Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

xen-netback: Fix handling of skbs requiring too many slots

A recent commit (a02eb4 "xen-netback: worse-case estimate in xenvif_rx_action is
underestimating") capped the slot estimation to MAX_SKB_FRAGS, but that triggers
the next BUG_ON a few lines down, as the packet consumes more slots than
estimated.
This patch introduces full_coalesce on the skb callback buffer, which is used in
start_new_rx_buffer() to decide whether netback needs coalescing more
aggresively. By doing that, no packet should need more than
(XEN_NETIF_MAX_TX_SIZE + 1) / PAGE_SIZE data slots (excluding the optional GSO
slot, it doesn't carry data, therefore irrelevant in this case), as the provided
buffers are fully utilized.

Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Cc: Paul Durrant <paul.durrant@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@gmail.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Zoltan Kiss and committed by
David S. Miller
59ae9fc6 c5316920

+25 -11
+25 -11
drivers/net/xen-netback/netback.c
··· 163 163 * adding 'size' bytes to a buffer which currently contains 'offset' 164 164 * bytes. 165 165 */ 166 - static bool start_new_rx_buffer(int offset, unsigned long size, int head) 166 + static bool start_new_rx_buffer(int offset, unsigned long size, int head, 167 + bool full_coalesce) 167 168 { 168 169 /* simple case: we have completely filled the current buffer. */ 169 170 if (offset == MAX_BUFFER_OFFSET) ··· 176 175 * (i) this frag would fit completely in the next buffer 177 176 * and (ii) there is already some data in the current buffer 178 177 * and (iii) this is not the head buffer. 178 + * and (iv) there is no need to fully utilize the buffers 179 179 * 180 180 * Where: 181 181 * - (i) stops us splitting a frag into two copies ··· 187 185 * by (ii) but is explicitly checked because 188 186 * netfront relies on the first buffer being 189 187 * non-empty and can crash otherwise. 188 + * - (iv) is needed for skbs which can use up more than MAX_SKB_FRAGS 189 + * slot 190 190 * 191 191 * This means we will effectively linearise small 192 192 * frags but do not needlessly split large buffers ··· 196 192 * own buffers as before. 197 193 */ 198 194 BUG_ON(size > MAX_BUFFER_OFFSET); 199 - if ((offset + size > MAX_BUFFER_OFFSET) && offset && !head) 195 + if ((offset + size > MAX_BUFFER_OFFSET) && offset && !head && 196 + !full_coalesce) 200 197 return true; 201 198 202 199 return false; ··· 231 226 232 227 return meta; 233 228 } 229 + 230 + struct xenvif_rx_cb { 231 + int meta_slots_used; 232 + bool full_coalesce; 233 + }; 234 + 235 + #define XENVIF_RX_CB(skb) ((struct xenvif_rx_cb *)(skb)->cb) 234 236 235 237 /* 236 238 * Set up the grant operations for this fragment. If it's a flipping ··· 273 261 if (bytes > size) 274 262 bytes = size; 275 263 276 - if (start_new_rx_buffer(npo->copy_off, bytes, *head)) { 264 + if (start_new_rx_buffer(npo->copy_off, 265 + bytes, 266 + *head, 267 + XENVIF_RX_CB(skb)->full_coalesce)) { 277 268 /* 278 269 * Netfront requires there to be some data in the head 279 270 * buffer. ··· 556 541 } 557 542 } 558 543 559 - struct xenvif_rx_cb { 560 - int meta_slots_used; 561 - }; 562 - 563 - #define XENVIF_RX_CB(skb) ((struct xenvif_rx_cb *)(skb)->cb) 564 - 565 544 void xenvif_kick_thread(struct xenvif *vif) 566 545 { 567 546 wake_up(&vif->wq); ··· 611 602 612 603 /* To avoid the estimate becoming too pessimal for some 613 604 * frontends that limit posted rx requests, cap the estimate 614 - * at MAX_SKB_FRAGS. 605 + * at MAX_SKB_FRAGS. In this case netback will fully coalesce 606 + * the skb into the provided slots. 615 607 */ 616 - if (max_slots_needed > MAX_SKB_FRAGS) 608 + if (max_slots_needed > MAX_SKB_FRAGS) { 617 609 max_slots_needed = MAX_SKB_FRAGS; 610 + XENVIF_RX_CB(skb)->full_coalesce = true; 611 + } else { 612 + XENVIF_RX_CB(skb)->full_coalesce = false; 613 + } 618 614 619 615 /* We may need one more slot for GSO metadata */ 620 616 if (skb_is_gso(skb) &&