Re: [PATCH net-next] igb: use build_skb()
From: Alexander Duyck <hidden>
Date: 2012-08-06 17:35:34
On 08/02/2012 08:51 AM, Eric Dumazet wrote:
From: Eric Dumazet <edumazet@google.com> By using netdev_alloc_frag() & build_skb() instead of legacy netdev_alloc_skb_ip_align() calls, we reduce number of cache misses in RX path and size of working set. For a given rx workload, number of 'inuse' sk_buff can be reduced to a very minimum, especially when packets are dropped by our stack. (Before this patch, default sk_buff allocation was 2048 sk_buffs in rx ring buffer) They are initialized right before being delivered to stack, so can stay hot in cpu caches. Ethernet header prefetching is more effective (old prefetch of skb->data paid a stall to access skb->data pointer) I have 15% performance increase in a RX stress test, removing SLUB slow path in the profiles. Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Alexander Duyck <redacted> --- drivers/net/ethernet/intel/igb/igb.h | 8 ++ drivers/net/ethernet/intel/igb/igb_ethtool.c | 14 ++-- drivers/net/ethernet/intel/igb/igb_main.c | 56 ++++++++++------- 3 files changed, 49 insertions(+), 29 deletions(-)
[...]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index b7c2d50..8b732c9 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c
[...]
quoted hunk ↗ jump to hunk
@@ -6091,6 +6095,15 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget) next_rxd = IGB_RX_DESC(rx_ring, i); prefetch(next_rxd); + if (!skb) { + skb = build_skb(data, IGB_FRAGSZ); + if (unlikely(!skb)) { + rx_ring->rx_stats.alloc_failed++; + buffer_info->data = data; + goto next_desc; + } + skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); + } /* * This memory barrier is needed to keep us from reading * any other fields out of the rx_desc until we know the
This logic is broken. If an allocation failure occurs it would leave the data in the ring and could possibly give you a corrupted packet. I was planning to move igb over to an ixgbe style receive path at some point anyway. Since it seems like this is now a higher priority I figured I would try to get the patches for it implemented in the next week or so. Would there be any issue with us rejecting this patch and instead switching igb over to the ixgbe style path? Thanks, Alex