Re: [PATCH net v6 4/4] tg3: Fix tx_pending checks for tg3_tso_bug

From: Prashant
Date: Tue Sep 30 2014 - 23:14:59 EST




On 9/5/2014 9:39 PM, David Miller wrote:
From: David Miller <davem@xxxxxxxxxxxxx>
Date: Fri, 05 Sep 2014 17:13:06 -0700 (PDT)

From: Eric Dumazet <eric.dumazet@xxxxxxxxx>
Date: Fri, 05 Sep 2014 17:03:30 -0700

Instead of this private helper (and pretty limited one btw), we could
add a core function, that would build skbs with order-0 fragments.

Instead of skb_linearize(), I guess many call sites could instead use
this new helper.

Because as you said, skb_linearize() of one 64KB GSO packet can ask
order-5 allocations, and this generally does not work reliably.

xen-netback could make use of this helper too.

I was curious what it might look like so I cobbled the following
completely untested patch together :-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index da1378a..eba0ad6 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -955,6 +955,67 @@ struct sk_buff *skb_copy(const struct sk_buff *skb, gfp_t gfp_mask)
EXPORT_SYMBOL(skb_copy);

/**
+ * skb_copy_pskb - copy sk_buff into a paged skb
+ * @oskb: buffer to copy
+ * @gfp_mask: allocation priority
+ *
+ * Normalize a paged skb into one that maximally uses order
+ * zero pages in it's fragment array. This is used to canonicalize
+ * spaghetti SKBs that use the page array inefficiently (f.e. only
+ * one byte per page frag).
+ */
+
+struct sk_buff *skb_copy_pskb(const struct sk_buff *oskb, gfp_t gfp_mask)
+{
+ unsigned int data_len = oskb->data_len;
+ int offset, npages, i;
+ struct sk_buff *skb;
+
+ npages = (data_len + (PAGE_SIZE - 1)) >> PAGE_SHIFT;
+ if (npages > MAX_SKB_FRAGS)
+ return NULL;
+
+ skb = __alloc_skb(skb_end_offset(oskb), gfp_mask,
+ skb_alloc_rx_flag(oskb), NUMA_NO_NODE);
+ if (!skb)
+ return NULL;
+
+ skb_reserve(skb, skb_headroom(oskb));
+ skb_put(skb, skb_headlen(oskb));
+ skb_copy_from_linear_data(oskb, skb->data, skb->len);
+
+ copy_skb_header(skb, oskb);
+
+ skb->truesize += data_len;
+ offset = skb_headlen(oskb);
+ for (i = 0; i < npages; i++) {
+ struct page *page = alloc_page(gfp_mask);
+ unsigned int chunk;
+ u8 *vaddr;
+
+ if (!page) {
+ kfree(skb);
+ skb = NULL;
+ break;
+ }
+
+ chunk = min_t(unsigned int, data_len, PAGE_SIZE);
+ skb_fill_page_desc(skb, i, page, 0, chunk);
+
+ vaddr = kmap_atomic(page);
+ skb_copy_bits(oskb, offset, vaddr, chunk);
+ kunmap_atomic(vaddr);
+
+ offset += chunk;
+ data_len -= chunk;
+ skb->data_len += chunk;
+ }
+
+ return skb;
+}
+EXPORT_SYMBOL(skb_copy_pskb);
+
+/**
* __pskb_copy_fclone - create copy of an sk_buff with private head.
* @skb: buffer to copy
* @headroom: headroom of new skb

Sorry about the late reply, out of all the HW bug conditions checked in tg3_tx_frag_set() the most frequently hit condition is the short 8 byte dma bug, where the chip cannot handle TX descriptors whose data buffer is 8 bytes or less. Most of the LSO skb's given to the driver has their fragments filled upto PAGE_SIZE (expect the last fragment depending on skb->len). And if such a LSO skb's last fragment meets the 8 bytes HW bug condition the above routine will not help workaround this particular case.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/